Add generic table feature activation gate

Author: cbb330

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.
+ *
+ * <p>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);
   }
 }

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");

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.
+ *
+ * <p>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.
+   * <p>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.
+   *
+   * <p>An explicit {@code <featureId>.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<String, String> 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;
+  }
 }

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<String, String> 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;
+    }
+  }
+}