Skip to content

Commit

Permalink
Support wildcard based matching for togglestatus endpoint (#203)
Browse files Browse the repository at this point in the history
This PR contains logically two components: 
- The changes in
`iceberg/openhouse/internalcatalog/src/main/java/com/linkedin/openhouse/internal/catalog/OpenHouseInternalCatalog.java`
is due to the discovery in the testing phase that the an inheritance
doesn't apply for annotation and AOP's proxy model requires the
annotation to be applied on the object that got directly invoked. Thus
the annotation in this `createTable` is not necessary. Since adding
annotation was the only reason why rewriting this method, removing this
whole method doesn't change any behavior.
- The main part of this PR is to add wildcard matching for
`hts/togglestatus` endpoint, so that for features like temporarily
forbidding table-creation, we can simply insert one rule that contains
wildcard for both tableId and databaseId.
  • Loading branch information
autumnust committed Sep 19, 2024
1 parent 43275a8 commit d9f99ce
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.linkedin.openhouse.internal.catalog;

import static com.linkedin.openhouse.internal.catalog.CatalogConstants.FEATURE_TOGGLE_STOP_CREATE;
import static com.linkedin.openhouse.internal.catalog.InternalCatalogMetricsConstant.METRICS_PREFIX;

import com.linkedin.openhouse.cluster.metrics.micrometer.MetricsReporter;
Expand All @@ -14,18 +13,13 @@
import com.linkedin.openhouse.internal.catalog.repository.HouseTableRepository;
import com.linkedin.openhouse.internal.catalog.repository.exception.HouseTableNotFoundException;
import com.linkedin.openhouse.internal.catalog.repository.exception.HouseTableRepositoryException;
import com.linkedin.openhouse.internal.catalog.toggle.IcebergFeatureGate;
import io.micrometer.core.instrument.MeterRegistry;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
import lombok.extern.slf4j.Slf4j;
import org.apache.iceberg.BaseMetastoreCatalog;
import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Schema;
import org.apache.iceberg.Table;
import org.apache.iceberg.TableOperations;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
Expand Down Expand Up @@ -72,18 +66,6 @@ protected TableOperations newTableOps(TableIdentifier tableIdentifier) {
new MetricsReporter(this.meterRegistry, METRICS_PREFIX, Lists.newArrayList()));
}

/** Overwritten for annotation purpose. */
@Override
@IcebergFeatureGate(value = FEATURE_TOGGLE_STOP_CREATE)
public Table createTable(
TableIdentifier identifier,
Schema schema,
PartitionSpec spec,
String location,
Map<String, String> properties) {
return super.createTable(identifier, schema, spec, location, properties);
}

@Override
public String name() {
return getClass().getSimpleName();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.linkedin.openhouse.housetables.services;

import com.linkedin.openhouse.housetables.model.TableToggleRule;

public interface TableToggleRuleMatcher {
boolean matches(TableToggleRule rule, String tableId, String databaseId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@
@Component
public class ToggleStatusesServiceImpl implements ToggleStatusesService {
@Autowired ToggleStatusHtsJdbcRepository htsRepository;
@Autowired TableToggleRuleMatcher ruleMatcher;

@Override
public ToggleStatus getTableToggleStatus(String featureId, String databaseId, String tableId) {
for (TableToggleRule tableToggleRule : htsRepository.findAllByFeature(featureId)) {

// TODO: Evolve this rule engine to support wildcards
if (tableToggleRule.getTablePattern().equals(tableId)
&& tableToggleRule.getDatabasePattern().equals(databaseId)) {
if (ruleMatcher.matches(tableToggleRule, tableId, databaseId)) {
return ToggleStatus.builder().status(ToggleStatusEnum.ACTIVE).build();
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.linkedin.openhouse.housetables.services;

import com.linkedin.openhouse.housetables.model.TableToggleRule;
import org.springframework.stereotype.Component;

/** An implementation of {@link TableToggleRuleMatcher} that supports '*' to match any entities */
@Component
public class WildcardTableToggleRuleMatcher implements TableToggleRuleMatcher {
@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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class ToggleStatusControllerTest {

@Autowired ToggleStatusHtsJdbcRepository htsRepository;

/** See data.sql for insertion of data being validated for the testing spring application. */
@Test
public void testGetTableToggleStatus() throws Exception {
mvc.perform(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,16 @@ public void testActivatedTableForDummy() {
toggleStatusesService.getTableToggleStatus("dummy2", "db", "testtbl1").getStatus(),
ToggleStatusEnum.INACTIVE);
}

@Test
public void testDBNoCreateWildcardMatch() {
Assertions.assertEquals(
toggleStatusesService
.getTableToggleStatus("stop_create", "db_no_create", "random")
.getStatus(),
ToggleStatusEnum.ACTIVE);
Assertions.assertEquals(
toggleStatusesService.getTableToggleStatus("stop_create", "db", "random").getStatus(),
ToggleStatusEnum.INACTIVE);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package com.linkedin.openhouse.housetables.mock;

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.when;

import com.linkedin.openhouse.housetables.model.TableToggleRule;
import com.linkedin.openhouse.housetables.services.WildcardTableToggleRuleMatcher;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

class WildcardTableToggleRuleMatcherTest {

@Mock private TableToggleRule mockRule;

private WildcardTableToggleRuleMatcher matcher;

@BeforeEach
void setUp() {
MockitoAnnotations.openMocks(this);
matcher = new WildcardTableToggleRuleMatcher();
}

@Test
void testExactMatch() {
when(mockRule.getTablePattern()).thenReturn("table1");
when(mockRule.getDatabasePattern()).thenReturn("db1");

assertTrue(matcher.matches(mockRule, "table1", "db1"));
assertFalse(matcher.matches(mockRule, "table2", "db1"));
assertFalse(matcher.matches(mockRule, "table1", "db2"));
}

@Test
void testWildcardTableMatch() {
when(mockRule.getTablePattern()).thenReturn("*");
when(mockRule.getDatabasePattern()).thenReturn("db1");

assertTrue(matcher.matches(mockRule, "table1", "db1"));
assertTrue(matcher.matches(mockRule, "table2", "db1"));
assertFalse(matcher.matches(mockRule, "table1", "db2"));
}

@Test
void testWildcardDatabaseMatch() {
when(mockRule.getTablePattern()).thenReturn("table1");
when(mockRule.getDatabasePattern()).thenReturn("*");

assertTrue(matcher.matches(mockRule, "table1", "db1"));
assertTrue(matcher.matches(mockRule, "table1", "db2"));
assertFalse(matcher.matches(mockRule, "table2", "db1"));
}

@Test
void testBothWildcardMatch() {
when(mockRule.getTablePattern()).thenReturn("*");
when(mockRule.getDatabasePattern()).thenReturn("*");

assertTrue(matcher.matches(mockRule, "table1", "db1"));
assertTrue(matcher.matches(mockRule, "table2", "db2"));
assertTrue(matcher.matches(mockRule, "anyTable", "anyDb"));
}

@Test
void testNoMatch() {
when(mockRule.getTablePattern()).thenReturn("table1");
when(mockRule.getDatabasePattern()).thenReturn("db1");

assertFalse(matcher.matches(mockRule, "table2", "db2"));
}
}
2 changes: 2 additions & 0 deletions services/housetables/src/test/resources/data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern,
INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern, id, creation_time_ms) VALUES ('dummy1', 'db', 'tbl', DEFAULT, 987L);
INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern, id, creation_time_ms) VALUES ('dummy1', 'db', 'testtbl1', DEFAULT, 987L);
INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern, id, creation_time_ms) VALUES ('dummy2', 'db', 'tbl', DEFAULT, 987L);
INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern, id, creation_time_ms) VALUES ('stop_create', 'db_no_create', '*', DEFAULT, 987L);

0 comments on commit d9f99ce

Please sign in to comment.