Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Comment thread
cbb330 marked this conversation as resolved.
*/
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;
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
Loading