From fa28104fda384ef4fb68e02afda8a585a459b73d Mon Sep 17 00:00:00 2001 From: Christian Bush Date: Thu, 30 Jul 2026 21:27:29 -0700 Subject: [PATCH] Add generic table feature activation gate --- .../WildcardTableToggleRuleMatcher.java | 21 ++-- .../WildcardTableToggleRuleMatcherTest.java | 30 ++++++ .../tables/toggle/TableFeatureToggle.java | 63 ++++++++++-- .../tables/toggle/TableFeatureToggleTest.java | 98 +++++++++++++++++++ 4 files changed, 199 insertions(+), 13 deletions(-) create mode 100644 services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/TableFeatureToggleTest.java 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 .enabled} property opts the table in or out; when absent, the + * server-side toggle decides. An unparseable value fails closed, so a typo cannot make the table + * unusable. + */ + default boolean isFeatureActivatedWithOverride(TableDto tableDto, String featureId) { + Map properties = tableDto.getTableProperties(); + String tableProperty = featureId + ENABLED_PROPERTY_SUFFIX; + String override = properties == null ? null : properties.get(tableProperty); + if (override == null) { + return isFeatureActivated(tableDto.getDatabaseId(), tableDto.getTableId(), featureId); + } + + String normalized = override.trim(); + if ("true".equalsIgnoreCase(normalized)) { + return true; + } + if ("false".equalsIgnoreCase(normalized)) { + return false; + } + LOG.warn( + "Ignoring unparseable table property {}={} for {}.{}; treating feature {} as inactive", + tableProperty, + override, + tableDto.getDatabaseId(), + tableDto.getTableId(), + featureId); + return false; + } } diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/TableFeatureToggleTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/TableFeatureToggleTest.java new file mode 100644 index 000000000..72254d673 --- /dev/null +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/toggle/TableFeatureToggleTest.java @@ -0,0 +1,98 @@ +package com.linkedin.openhouse.tables.toggle; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.linkedin.openhouse.tables.model.TableDto; +import java.util.Collections; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class TableFeatureToggleTest { + private static final String DATABASE = "database"; + private static final String TABLE = "table"; + private static final String FEATURE = "feature"; + private static final String PROPERTY = "feature.enabled"; + + private TestTableFeatureToggle featureToggle; + + @BeforeEach + void setUp() { + featureToggle = new TestTableFeatureToggle(); + } + + @Test + void testUsesServerToggleWhenPropertyIsAbsent() { + TableDto tableDto = tableWithProperties(Collections.emptyMap()); + featureToggle.serverDecision = true; + + assertTrue(featureToggle.isFeatureActivatedWithOverride(tableDto, FEATURE)); + assertEquals(1, featureToggle.serverInvocationCount); + } + + @Test + void testUsesServerToggleWhenPropertiesAreNull() { + TableDto tableDto = tableWithProperties(null); + featureToggle.serverDecision = false; + + assertFalse(featureToggle.isFeatureActivatedWithOverride(tableDto, FEATURE)); + assertEquals(1, featureToggle.serverInvocationCount); + } + + @Test + void testTruePropertyOptsInWithoutServerToggle() { + TableDto tableDto = tableWithProperties(Collections.singletonMap(PROPERTY, "true")); + + assertTrue(featureToggle.isFeatureActivatedWithOverride(tableDto, FEATURE)); + assertEquals(0, featureToggle.serverInvocationCount); + } + + @Test + void testFalsePropertyOptsOutWithoutServerToggle() { + TableDto tableDto = tableWithProperties(Collections.singletonMap(PROPERTY, "false")); + + assertFalse(featureToggle.isFeatureActivatedWithOverride(tableDto, FEATURE)); + assertEquals(0, featureToggle.serverInvocationCount); + } + + @Test + void testPropertyParsingIgnoresCaseAndWhitespace() { + TableDto tableDto = tableWithProperties(Collections.singletonMap(PROPERTY, " TRUE ")); + + assertTrue(featureToggle.isFeatureActivatedWithOverride(tableDto, FEATURE)); + assertEquals(0, featureToggle.serverInvocationCount); + } + + @Test + void testUnparseablePropertyFailsClosed() { + TableDto tableDto = tableWithProperties(Collections.singletonMap(PROPERTY, "sometimes")); + featureToggle.serverDecision = true; + + assertFalse(featureToggle.isFeatureActivatedWithOverride(tableDto, FEATURE)); + assertEquals(0, featureToggle.serverInvocationCount); + } + + private static TableDto tableWithProperties(Map properties) { + return TableDto.builder() + .databaseId(DATABASE) + .tableId(TABLE) + .tableProperties(properties) + .build(); + } + + private static class TestTableFeatureToggle implements TableFeatureToggle { + private boolean serverDecision; + private int serverInvocationCount; + + @Override + public boolean isFeatureActivated(String databaseId, String tableId, String featureId) { + assertEquals(DATABASE, databaseId); + assertEquals(TABLE, tableId); + assertEquals(FEATURE, featureId); + serverInvocationCount++; + return serverDecision; + } + } +}