diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/WildcardTableToggleRuleMatcher.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/WildcardTableToggleRuleMatcher.java index a1ba81a73..f27011ddf 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/WildcardTableToggleRuleMatcher.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/WildcardTableToggleRuleMatcher.java @@ -2,17 +2,24 @@ import com.linkedin.openhouse.housetables.model.TableToggleRule; import org.springframework.stereotype.Component; +import org.springframework.util.AntPathMatcher; +import org.springframework.util.PathMatcher; -/** An implementation of {@link TableToggleRuleMatcher} that supports '*' to match any entities */ +/** + * A {@link TableToggleRuleMatcher} matching database and table names as case-sensitive globs, so + * {@code *} matches any run of characters wherever it appears and {@code ?} matches one. + * + *
TODO: patterns are unvalidated because rules are inserted straight into MySQL, so a malformed + * pattern is only discovered by matching nothing. Validation belongs with a rule-write path, which + * does not exist yet. + */ @Component public class WildcardTableToggleRuleMatcher implements TableToggleRuleMatcher { + private static final PathMatcher MATCHER = new AntPathMatcher(); + @Override public boolean matches(TableToggleRule rule, String tableId, String databaseId) { - boolean tableMatches = - rule.getTablePattern().equals("*") || rule.getTablePattern().equals(tableId); - boolean databaseMatches = - rule.getDatabasePattern().equals("*") || rule.getDatabasePattern().equals(databaseId); - - return tableMatches && databaseMatches; + return MATCHER.match(rule.getTablePattern(), tableId) + && MATCHER.match(rule.getDatabasePattern(), databaseId); } } diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/WildcardTableToggleRuleMatcherTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/WildcardTableToggleRuleMatcherTest.java index 8c92e6335..a565b096f 100644 --- a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/WildcardTableToggleRuleMatcherTest.java +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/mock/WildcardTableToggleRuleMatcherTest.java @@ -62,6 +62,36 @@ void testBothWildcardMatch() { assertTrue(matcher.matches(mockRule, "anyTable", "anyDb")); } + @Test + void testDatabaseAndTablePrefixMatch() { + when(mockRule.getTablePattern()).thenReturn("events_*"); + when(mockRule.getDatabasePattern()).thenReturn("tracking_*"); + + assertTrue(matcher.matches(mockRule, "events_daily", "tracking_prod")); + assertFalse(matcher.matches(mockRule, "metrics_daily", "tracking_prod")); + assertFalse(matcher.matches(mockRule, "events_daily", "analytics_prod")); + } + + @Test + void testWildcardMatchesInsidePattern() { + when(mockRule.getTablePattern()).thenReturn("events_*_daily"); + when(mockRule.getDatabasePattern()).thenReturn("*_prod"); + + assertTrue(matcher.matches(mockRule, "events_click_daily", "tracking_prod")); + assertFalse(matcher.matches(mockRule, "events_click_hourly", "tracking_prod")); + assertFalse(matcher.matches(mockRule, "events_click_daily", "tracking_test")); + } + + @Test + void testMatchIsCaseSensitive() { + when(mockRule.getTablePattern()).thenReturn("events_*"); + when(mockRule.getDatabasePattern()).thenReturn("tracking"); + + assertTrue(matcher.matches(mockRule, "events_daily", "tracking")); + assertFalse(matcher.matches(mockRule, "Events_daily", "tracking")); + assertFalse(matcher.matches(mockRule, "events_daily", "Tracking")); + } + @Test void testNoMatch() { when(mockRule.getTablePattern()).thenReturn("table1"); diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/TableFeatureToggle.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/TableFeatureToggle.java index e7250ad2b..2b3e373f7 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/TableFeatureToggle.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/TableFeatureToggle.java @@ -1,14 +1,65 @@ package com.linkedin.openhouse.tables.toggle; -/** Interface to check if a feature is toggled-on for a table */ +import com.linkedin.openhouse.tables.model.TableDto; +import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Interface to check if a feature is toggled-on for a table. + * + *
TODO: the two forms below differ in who may influence the decision, which today is conveyed by + * their names and javadoc rather than enforced. The intended model declares that per feature + * instead of per call site — {@code TableFeature.capability(id)} vs {@code + * TableFeature.rollout(id)} — so a permission-bearing feature cannot acquire self-service opt-in by + * calling the wrong method. The same change would carry a decision's cause for metrics, give rules + * an explicit effect, priority and expiry rather than presence-implies-active, and evaluate rules + * from a locally replicated snapshot so a table read no longer blocks on HouseTables. + */ public interface TableFeatureToggle { + Logger LOG = LoggerFactory.getLogger(TableFeatureToggle.class); + + /** Suffix appended to a feature id to form its self-service table property. */ + String ENABLED_PROPERTY_SUFFIX = ".enabled"; + /** - * Determine if given feature is activated for the table. + * Determines the server-side activation decision for a table. * - * @param databaseId databaseId - * @param tableId tableId - * @param featureId featureId - * @return True if the feature is activated for the table. + *
Authorization gates — features deciding whether a user may write an otherwise preserved + * property, like {@code enable_mor} — must use this form, since the table property honored by + * {@link #isFeatureActivatedWithOverride(TableDto, String)} is writable by the user being gated. */ boolean isFeatureActivated(String databaseId, String tableId, String featureId); + + /** + * Determines activation, letting the table override the server-side decision. + * + *
An explicit {@code