From 7b20ecd71ea0441ab3b200490b80b0dcaa637be1 Mon Sep 17 00:00:00 2001 From: Lei Sun Date: Mon, 15 Apr 2024 17:34:42 -0700 Subject: [PATCH 1/4] Plumbing HTS for feature toggle --- .../cluster/configs/ClusterProperties.java | 2 +- .../common/api/spec/ToggleStatusEnum.java | 7 ++ .../OpenHouseToggleStatusesApiHandler.java | 48 +++++++++++++ .../api/handler/ToggleStatusesApiHandler.java | 7 ++ .../api/spec/model/TableToggleStatusKey.java | 40 +++++++++++ .../api/spec/model/ToggleStatus.java | 20 ++++++ .../api/spec/model/UserTableKey.java | 2 +- .../db/jdbc/JdbcProviderConfiguration.java | 2 +- .../controller/ToggleStatusesController.java | 52 ++++++++++++++ .../housetables/model/TableToggleRule.java | 32 +++++++++ .../jdbc/ToggleStatusHtsJdbcRepository.java | 8 +++ .../services/ToggleStatusesService.java | 16 +++++ .../services/ToggleStatusesServiceImpl.java | 17 +++++ .../src/main/resources/application.properties | 1 + .../housetables/src/main/resources/data.sql | 4 ++ .../housetables/src/main/resources/schema.sql | 11 +++ .../ToggleStatusControllerTest.java | 72 +++++++++++++++++++ .../ToggleStatusRepositoryTest.java | 33 +++++++++ .../togglerule/ToggleStatusesServiceTest.java | 37 ++++++++++ .../ToggleStatusesTestConstants.java | 26 +++++++ .../src/test/resources/application.properties | 3 + .../housetables/src/test/resources/data.sql | 4 ++ .../tables/TablesSpringApplication.java | 1 + .../impl/OpenHouseInternalRepositoryImpl.java | 2 +- .../OpenHouseInternalRepositoryImpl.java.rej | 10 +++ .../tables/toggle/BaseTableFeatureToggle.java | 8 ++- .../toggle/model/TableToggleStatus.java | 1 - .../BaseToggleStatusesRepository.java | 2 + .../tablestest/SpringH2TestApplication.java | 2 + 29 files changed, 463 insertions(+), 7 deletions(-) create mode 100644 services/common/src/main/java/com/linkedin/openhouse/common/api/spec/ToggleStatusEnum.java create mode 100644 services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/OpenHouseToggleStatusesApiHandler.java create mode 100644 services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/ToggleStatusesApiHandler.java create mode 100644 services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/TableToggleStatusKey.java create mode 100644 services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatus.java create mode 100644 services/housetables/src/main/java/com/linkedin/openhouse/housetables/controller/ToggleStatusesController.java create mode 100644 services/housetables/src/main/java/com/linkedin/openhouse/housetables/model/TableToggleRule.java create mode 100644 services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java create mode 100644 services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesService.java create mode 100644 services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java create mode 100644 services/housetables/src/main/resources/data.sql create mode 100644 services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusControllerTest.java create mode 100644 services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java create mode 100644 services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesServiceTest.java create mode 100644 services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesTestConstants.java create mode 100644 services/housetables/src/test/resources/application.properties create mode 100644 services/housetables/src/test/resources/data.sql create mode 100644 services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java.rej diff --git a/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java b/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java index e722382f..5678dbf2 100644 --- a/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java +++ b/cluster/configs/src/main/java/com/linkedin/openhouse/cluster/configs/ClusterProperties.java @@ -54,7 +54,7 @@ public class ClusterProperties { @Value("${cluster.housetables.database.type:IN_MEMORY}") private String clusterHouseTablesDatabaseType; - @Value("${cluster.housetables.database.url:jdbc:h2:mem:htsdb;DB_CLOSE_DELAY=-1}") + @Value("${cluster.housetables.database.url:jdbc:h2:mem:htsdb;MODE=MYSQL;DB_CLOSE_DELAY=-1}") private String clusterHouseTablesDatabaseUrl; @Value("${HTS_DB_USER:}") diff --git a/services/common/src/main/java/com/linkedin/openhouse/common/api/spec/ToggleStatusEnum.java b/services/common/src/main/java/com/linkedin/openhouse/common/api/spec/ToggleStatusEnum.java new file mode 100644 index 00000000..cd9fc40d --- /dev/null +++ b/services/common/src/main/java/com/linkedin/openhouse/common/api/spec/ToggleStatusEnum.java @@ -0,0 +1,7 @@ +package com.linkedin.openhouse.common.api.spec; + +/** Indicate if a feature is active or inactive on an entity (e.g. table) */ +public enum ToggleStatusEnum { + ACTIVE, + INACTIVE +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/OpenHouseToggleStatusesApiHandler.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/OpenHouseToggleStatusesApiHandler.java new file mode 100644 index 00000000..b47af11c --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/OpenHouseToggleStatusesApiHandler.java @@ -0,0 +1,48 @@ +package com.linkedin.openhouse.housetables.api.handler; + +import com.linkedin.openhouse.common.api.spec.ApiResponse; +import com.linkedin.openhouse.housetables.api.spec.model.TableToggleStatusKey; +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatus; +import com.linkedin.openhouse.housetables.api.spec.response.EntityResponseBody; +import com.linkedin.openhouse.housetables.api.spec.response.GetAllEntityResponseBody; +import com.linkedin.openhouse.housetables.services.ToggleStatusesService; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; +import org.springframework.stereotype.Component; + +/** + * {@link ToggleStatusesApiHandler} is essentially read only. Thus, any write API are not + * implemented here. + */ +@Component +public class OpenHouseToggleStatusesApiHandler implements ToggleStatusesApiHandler { + @Autowired private ToggleStatusesService toggleStatusesService; + + @Override + public ApiResponse> getEntity(TableToggleStatusKey key) { + return ApiResponse.>builder() + .httpStatus(HttpStatus.OK) + .responseBody( + EntityResponseBody.builder() + .entity( + toggleStatusesService.getTableToggleStatus( + key.getFeatureId(), key.getDatabaseId(), key.getTableId())) + .build()) + .build(); + } + + @Override + public ApiResponse> getEntities(ToggleStatus entity) { + throw new UnsupportedOperationException("Get all toggle status is unsupported"); + } + + @Override + public ApiResponse deleteEntity(TableToggleStatusKey key) { + throw new UnsupportedOperationException("Delete toggle status is unsupported"); + } + + @Override + public ApiResponse> putEntity(ToggleStatus entity) { + throw new UnsupportedOperationException("Update toggle status is unsupported"); + } +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/ToggleStatusesApiHandler.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/ToggleStatusesApiHandler.java new file mode 100644 index 00000000..1ceb8409 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/handler/ToggleStatusesApiHandler.java @@ -0,0 +1,7 @@ +package com.linkedin.openhouse.housetables.api.handler; + +import com.linkedin.openhouse.housetables.api.spec.model.TableToggleStatusKey; +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatus; + +public interface ToggleStatusesApiHandler + extends HouseTablesApiHandler {} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/TableToggleStatusKey.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/TableToggleStatusKey.java new file mode 100644 index 00000000..c1a567e0 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/TableToggleStatusKey.java @@ -0,0 +1,40 @@ +package com.linkedin.openhouse.housetables.api.spec.model; + +import static com.linkedin.openhouse.common.api.validator.ValidatorConstants.*; + +import com.fasterxml.jackson.annotation.JsonProperty; +import io.swagger.v3.oas.annotations.media.Schema; +import javax.validation.constraints.NotEmpty; +import javax.validation.constraints.Pattern; +import lombok.Builder; +import lombok.Value; + +/** Key to query feature-toggle status of a table. */ +@Builder +@Value +public class TableToggleStatusKey { + @Schema( + description = + "Unique Resource identifier for the Database containing the Table. Together with tableID" + + " they form a composite primary key for a user table.", + example = "my_database") + @JsonProperty(value = "databaseId") + @NotEmpty(message = "databaseId cannot be empty") + @Pattern(regexp = ALPHA_NUM_UNDERSCORE_REGEX, message = ALPHA_NUM_UNDERSCORE_ERROR_MSG) + String databaseId; + + @Schema( + description = "Unique Resource identifier for a table within a Database.", + example = "my_table") + @JsonProperty(value = "tableId") + @NotEmpty(message = "tableId cannot be empty") + @Pattern(regexp = ALPHA_NUM_UNDERSCORE_REGEX, message = ALPHA_NUM_UNDERSCORE_ERROR_MSG) + String tableId; + + @Schema( + description = "Unique Resource identifier for a feature within OpenHouse Service", + example = "dummy") + @JsonProperty(value = "featureId") + @NotEmpty(message = "featureId cannot be empty") + String featureId; +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatus.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatus.java new file mode 100644 index 00000000..e507c541 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatus.java @@ -0,0 +1,20 @@ +package com.linkedin.openhouse.housetables.api.spec.model; + +import com.fasterxml.jackson.annotation.JsonProperty; +import com.linkedin.openhouse.common.api.spec.ToggleStatusEnum; +import io.swagger.v3.oas.annotations.media.Schema; +import javax.validation.constraints.NotEmpty; +import lombok.Builder; +import lombok.Value; + +/** This layer on top of {@link ToggleStatusEnum} is ensuring API extensibility. */ +@Builder(toBuilder = true) +@Value +public class ToggleStatus { + @Schema( + description = "Status of an entity with respect to whether a feature has been toggled on", + example = "Active") + @JsonProperty(value = "status") + @NotEmpty(message = "Toggle status cannot be empty") + ToggleStatusEnum status; +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTableKey.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTableKey.java index ab8685ca..e2c933bf 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTableKey.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/UserTableKey.java @@ -29,7 +29,7 @@ public class UserTableKey { + " they form a composite primary key for a user table.", example = "my_database") @JsonProperty(value = "databaseId") - @NotEmpty(message = "tableId cannot be empty") + @NotEmpty(message = "databaseId cannot be empty") @Pattern(regexp = ALPHA_NUM_UNDERSCORE_REGEX, message = ALPHA_NUM_UNDERSCORE_ERROR_MSG) private String databaseId; diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/jdbc/JdbcProviderConfiguration.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/jdbc/JdbcProviderConfiguration.java index fb2598b2..225dd5d7 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/jdbc/JdbcProviderConfiguration.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/config/db/jdbc/JdbcProviderConfiguration.java @@ -30,7 +30,7 @@ public class JdbcProviderConfiguration { * "htsdb" database. With DB_CLOSE_DELAY=-1, the database is kept alive as long as the JVM lives, * otherwise it shuts down when the database-creating-thread dies. */ - private static final String H2_DEFAULT_URL = "jdbc:h2:mem:htsdb;DB_CLOSE_DELAY=-1"; + private static final String H2_DEFAULT_URL = "jdbc:h2:mem:htsdb;MODE=MySQL;DB_CLOSE_DELAY=-1"; @Bean public DataSource provideJdbcDataSource() { diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/controller/ToggleStatusesController.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/controller/ToggleStatusesController.java new file mode 100644 index 00000000..6b6f38ca --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/controller/ToggleStatusesController.java @@ -0,0 +1,52 @@ +package com.linkedin.openhouse.housetables.controller; + +import com.linkedin.openhouse.housetables.api.handler.ToggleStatusesApiHandler; +import com.linkedin.openhouse.housetables.api.spec.model.TableToggleStatusKey; +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatus; +import com.linkedin.openhouse.housetables.api.spec.response.EntityResponseBody; +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.responses.ApiResponse; +import io.swagger.v3.oas.annotations.responses.ApiResponses; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; + +/** + * Toggle Statuses are read-only for HTS, thus create/update paths are intentionally not + * implemented. + */ +@RestController +public class ToggleStatusesController { + + private static final String TOGGLE_ENDPOINT = "/hts/togglestatuses"; + + @Autowired private ToggleStatusesApiHandler toggleStatuesApiHandler; + + @Operation( + summary = "Get a toggle status applied to a table.", + description = "Returns a toggle status of databaseID and tableId on a featureId", + tags = {"ToggleStatus"}) + @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "Toggle status GET: OK")}) + @GetMapping( + value = TOGGLE_ENDPOINT, + produces = {"application/json"}) + public ResponseEntity> getTableToggleStatus( + @RequestParam(value = "databaseId") String databaseId, + @RequestParam(value = "tableId") String tableId, + @RequestParam(value = "featureId") String featureId) { + + com.linkedin.openhouse.common.api.spec.ApiResponse> + apiResponse = + toggleStatuesApiHandler.getEntity( + TableToggleStatusKey.builder() + .databaseId(databaseId) + .tableId(tableId) + .featureId(featureId) + .build()); + + return new ResponseEntity<>( + apiResponse.getResponseBody(), apiResponse.getHttpHeaders(), apiResponse.getHttpStatus()); + } +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/model/TableToggleRule.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/model/TableToggleRule.java new file mode 100644 index 00000000..cf1cedfc --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/model/TableToggleRule.java @@ -0,0 +1,32 @@ +package com.linkedin.openhouse.housetables.model; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import lombok.AccessLevel; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; + +/** Data Model for persisting a Toggle Rule Object in the HouseTable. */ +@Entity +@Builder(toBuilder = true) +@Getter +@EqualsAndHashCode +@NoArgsConstructor(access = AccessLevel.PROTECTED) +@AllArgsConstructor(access = AccessLevel.PROTECTED) +public class TableToggleRule { + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id", updatable = false, nullable = false) + private Long id; + + private String feature; + private String databasePattern; + private String tablePattern; + private Long creationTimeMs; +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java new file mode 100644 index 00000000..944a7917 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java @@ -0,0 +1,8 @@ +package com.linkedin.openhouse.housetables.repository.impl.jdbc; + +import com.linkedin.openhouse.housetables.model.TableToggleRule; +import com.linkedin.openhouse.housetables.repository.HtsRepository; + +public interface ToggleStatusHtsJdbcRepository extends HtsRepository { + Iterable findAllByFeature(String feature); +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesService.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesService.java new file mode 100644 index 00000000..e5b498c2 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesService.java @@ -0,0 +1,16 @@ +package com.linkedin.openhouse.housetables.services; + +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatus; + +public interface ToggleStatusesService { + /** + * Obtain the status of a {@link com.linkedin.openhouse.housetables.api.spec.model.UserTable}'s + * feature. + * + * @param featureId identifier of the feature + * @param databaseId identifier of the database + * @param tableId identifier of the table + * @return {@link ToggleStatus} of the requested entity. + */ + ToggleStatus getTableToggleStatus(String featureId, String databaseId, String tableId); +} diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java new file mode 100644 index 00000000..5ebb24d8 --- /dev/null +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java @@ -0,0 +1,17 @@ +package com.linkedin.openhouse.housetables.services; + +import com.linkedin.openhouse.common.api.spec.ToggleStatusEnum; +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatus; +import com.linkedin.openhouse.housetables.repository.impl.jdbc.ToggleStatusHtsJdbcRepository; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Component; + +@Component +public class ToggleStatusesServiceImpl implements ToggleStatusesService { + @Autowired ToggleStatusHtsJdbcRepository htsRepository; + + @Override + public ToggleStatus getTableToggleStatus(String featureId, String databaseId, String tableId) { + return ToggleStatus.builder().status(ToggleStatusEnum.ACTIVE).build(); + } +} diff --git a/services/housetables/src/main/resources/application.properties b/services/housetables/src/main/resources/application.properties index b1e514bd..564dae89 100644 --- a/services/housetables/src/main/resources/application.properties +++ b/services/housetables/src/main/resources/application.properties @@ -6,6 +6,7 @@ springdoc.swagger-ui.path=/hts/api-docs springdoc.swagger-ui.operationsSorter=method spring.jpa.hibernate.ddl-auto=none spring.sql.init.mode=always +spring.jpa.defer-datasource-initialization=true management.endpoints.web.exposure.include=health, shutdown, prometheus, beans management.endpoint.health.enabled=true management.endpoint.shutdown.enabled=true diff --git a/services/housetables/src/main/resources/data.sql b/services/housetables/src/main/resources/data.sql new file mode 100644 index 00000000..ed77188b --- /dev/null +++ b/services/housetables/src/main/resources/data.sql @@ -0,0 +1,4 @@ +-- Initial value for feature toggle tables +-- When enabling/disabling some feature, please ensure they are checked-in and reviewed through this file + +INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern, id, creation_time_ms) VALUES ('demo', 'demodb', 'demotable', DEFAULT, DEFAULT); \ No newline at end of file diff --git a/services/housetables/src/main/resources/schema.sql b/services/housetables/src/main/resources/schema.sql index 7b7d1d45..37d559cd 100644 --- a/services/housetables/src/main/resources/schema.sql +++ b/services/housetables/src/main/resources/schema.sql @@ -28,3 +28,14 @@ CREATE TABLE IF NOT EXISTS job_row ( ETL_TS datetime(6) DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), PRIMARY KEY (job_id) ); + +CREATE TABLE IF NOT EXISTS table_toggle_rule ( + feature VARCHAR (128) NOT NULL, + database_pattern VARCHAR (128) NOT NULL, + table_pattern VARCHAR (512) NOT NULL, + id BIGINT AUTO_INCREMENT, + creation_time_ms BIGINT , + ETL_TS DATETIME(6) DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), + PRIMARY KEY (id), + UNIQUE (feature, database_pattern, table_pattern) + ); \ No newline at end of file diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusControllerTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusControllerTest.java new file mode 100644 index 00000000..b0b161fe --- /dev/null +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusControllerTest.java @@ -0,0 +1,72 @@ +package com.linkedin.openhouse.housetables.e2e.togglerule; + +import static com.linkedin.openhouse.housetables.e2e.togglerule.ToggleStatusesTestConstants.*; +import static org.hamcrest.Matchers.*; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; + +import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; +import com.linkedin.openhouse.housetables.repository.impl.jdbc.ToggleStatusHtsJdbcRepository; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.MediaType; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; + +@SpringBootTest +@ContextConfiguration(initializers = PropertyOverrideContextInitializer.class) +@AutoConfigureMockMvc +public class ToggleStatusControllerTest { + @Autowired MockMvc mvc; + + @Autowired ToggleStatusHtsJdbcRepository htsRepository; + + @Test + public void testGetTableToggleStatus() throws Exception { + mvc.perform( + MockMvcRequestBuilders.get("/hts/togglestatuses") + .param("databaseId", TEST_DB_NAME) + .param("tableId", TEST_TABLE_NAME) + .param("featureId", TEST_FEATURE_NAME) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(jsonPath("$.entity.status", is(equalTo("ACTIVE")))); + + mvc.perform( + MockMvcRequestBuilders.get("/hts/togglestatuses") + /* Knowing these are the exact Id, instead of patterns with wildcard */ + .param("databaseId", TEST_RULE_1.getDatabasePattern()) + .param("tableId", TEST_RULE_1.getTablePattern()) + .param("featureId", TEST_RULE_1.getFeature()) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(jsonPath("$.entity.status", is(equalTo("ACTIVE")))); + + mvc.perform( + MockMvcRequestBuilders.get("/hts/togglestatuses") + /* Knowing these are the exact Id, instead of patterns with wildcard */ + .param("databaseId", TEST_RULE_2.getDatabasePattern()) + .param("tableId", TEST_RULE_2.getTablePattern()) + .param("featureId", TEST_RULE_2.getFeature()) + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(jsonPath("$.entity.status", is(equalTo("ACTIVE")))); + + mvc.perform( + MockMvcRequestBuilders.get("/hts/togglestatuses") + .param("databaseId", TEST_DB_NAME) + .param("tableId", TEST_TABLE_NAME) + .param( + "featureId", + TEST_FEATURE_NAME + "postfix") /* something that are not activated*/ + .accept(MediaType.APPLICATION_JSON)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON)) + .andExpect(jsonPath("$.entity.status", is(equalTo("INACTIVE")))); + } +} diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java new file mode 100644 index 00000000..060c79ad --- /dev/null +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java @@ -0,0 +1,33 @@ +package com.linkedin.openhouse.housetables.e2e.togglerule; + +import static com.linkedin.openhouse.housetables.e2e.togglerule.ToggleStatusesTestConstants.*; + +import com.google.common.collect.ImmutableList; +import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; +import com.linkedin.openhouse.housetables.model.TableToggleRule; +import com.linkedin.openhouse.housetables.repository.impl.jdbc.ToggleStatusHtsJdbcRepository; +import java.util.List; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ContextConfiguration; + +@SpringBootTest +@ContextConfiguration(initializers = PropertyOverrideContextInitializer.class) +public class ToggleStatusRepositoryTest { + @Autowired ToggleStatusHtsJdbcRepository htsRepository; + + @Test + public void testFindAllByFeature() { + List toggleRuleList = + ImmutableList.copyOf(htsRepository.findAllByFeature(TEST_RULE_0.getFeature())); + Assertions.assertEquals(2, toggleRuleList.size()); + + // Now there should be 2 rules under dummy2 feature + htsRepository.save( + TEST_RULE_0.toBuilder().feature(TEST_RULE_2.getFeature()).databasePattern("dbnew").build()); + toggleRuleList = ImmutableList.copyOf(htsRepository.findAllByFeature(TEST_RULE_2.getFeature())); + Assertions.assertEquals(2, toggleRuleList.size()); + } +} diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesServiceTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesServiceTest.java new file mode 100644 index 00000000..e8bf5fba --- /dev/null +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesServiceTest.java @@ -0,0 +1,37 @@ +package com.linkedin.openhouse.housetables.e2e.togglerule; + +import com.linkedin.openhouse.common.api.spec.ToggleStatusEnum; +import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; +import com.linkedin.openhouse.housetables.services.ToggleStatusesService; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.jdbc.Sql; + +@SpringBootTest +@ContextConfiguration(initializers = PropertyOverrideContextInitializer.class) +@Sql({"/schema.sql", "/data.sql"}) +public class ToggleStatusesServiceTest { + @Autowired ToggleStatusesService toggleStatusesService; + + @Test + public void testActivatedTableForDummy() { + Assertions.assertEquals( + toggleStatusesService.getTableToggleStatus("demo", "demodb", "demotable").getStatus(), + ToggleStatusEnum.ACTIVE); + Assertions.assertEquals( + toggleStatusesService.getTableToggleStatus("dummy1", "db", "tbl").getStatus(), + ToggleStatusEnum.ACTIVE); + Assertions.assertEquals( + toggleStatusesService.getTableToggleStatus("dummy1", "db", "testtbl1").getStatus(), + ToggleStatusEnum.ACTIVE); + Assertions.assertEquals( + toggleStatusesService.getTableToggleStatus("dummy2", "db", "tbl").getStatus(), + ToggleStatusEnum.ACTIVE); + Assertions.assertEquals( + toggleStatusesService.getTableToggleStatus("dummy2", "db", "testtbl1").getStatus(), + ToggleStatusEnum.INACTIVE); + } +} diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesTestConstants.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesTestConstants.java new file mode 100644 index 00000000..766b8ba1 --- /dev/null +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesTestConstants.java @@ -0,0 +1,26 @@ +package com.linkedin.openhouse.housetables.e2e.togglerule; + +import com.linkedin.openhouse.housetables.model.TableToggleRule; + +public class ToggleStatusesTestConstants { + private ToggleStatusesTestConstants() { + // Util class ctor noop + } + + // Following are single parameters that aligns with rule_0 + static final String TEST_DB_NAME = "db"; + static final String TEST_TABLE_NAME = "tbl"; + static final String TEST_FEATURE_NAME = "dummy1"; + + static final TableToggleRule TEST_RULE_0 = + TableToggleRule.builder() + .feature(TEST_FEATURE_NAME) + .creationTimeMs(System.currentTimeMillis()) + .tablePattern(TEST_TABLE_NAME) + .databasePattern(TEST_DB_NAME) + .build(); + + static final TableToggleRule TEST_RULE_1 = + TEST_RULE_0.toBuilder().tablePattern("testtbl1").build(); + static final TableToggleRule TEST_RULE_2 = TEST_RULE_0.toBuilder().feature("dummy2").build(); +} diff --git a/services/housetables/src/test/resources/application.properties b/services/housetables/src/test/resources/application.properties new file mode 100644 index 00000000..6a14fb46 --- /dev/null +++ b/services/housetables/src/test/resources/application.properties @@ -0,0 +1,3 @@ +spring.jpa.hibernate.ddl-auto=none +spring.sql.init.mode=always +spring.jpa.defer-datasource-initialization=true \ No newline at end of file diff --git a/services/housetables/src/test/resources/data.sql b/services/housetables/src/test/resources/data.sql new file mode 100644 index 00000000..a8a319aa --- /dev/null +++ b/services/housetables/src/test/resources/data.sql @@ -0,0 +1,4 @@ +INSERT IGNORE INTO table_toggle_rule (feature, database_pattern, table_pattern, id, creation_time_ms) VALUES ('demo', 'demodb', 'demotable', DEFAULT, 987L); +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); diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/TablesSpringApplication.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/TablesSpringApplication.java index d2464518..be7557aa 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/TablesSpringApplication.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/TablesSpringApplication.java @@ -13,6 +13,7 @@ scanBasePackages = { "com.linkedin.openhouse.tables", "com.linkedin.openhouse.tables.utils", + "com.linkedin.openhouse.tables.toggle", "com.linkedin.openhouse.cluster.configs", "com.linkedin.openhouse.cluster.storage", "com.linkedin.openhouse.common.audit", diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java index f5e16ce3..f04d086e 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java @@ -181,7 +181,7 @@ protected void updateEligibilityCheck(Table existingTable, TableDto tableDto) { * Ensure existing table's tableLocation (path to metadata.json) matches user provided baseVersion * (path to metadata.json of the table where the updates are based upon) */ - private void versionCheck(Table existingTable, TableDto mergedTableDto) { + void versionCheck(Table existingTable, TableDto mergedTableDto) { String baseTableVersion = mergedTableDto.getTableVersion(); if (existingTable != null) { diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java.rej b/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java.rej new file mode 100644 index 00000000..65cd28cf --- /dev/null +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java.rej @@ -0,0 +1,10 @@ +diff a/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java (rejected hunks) +@@ -85,6 +86,8 @@ public class OpenHouseInternalRepositoryImpl implements OpenHouseInternalReposit + + @Autowired PreservedKeyChecker preservedKeyChecker; + ++ @Autowired TableFeatureToggle baseTableFeatureToggle; ++ + @Override + public TableDto save(TableDto tableDto) { + TableIdentifier tableIdentifier = diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java index 1b960ec5..e201f1c1 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.tables.toggle; +import com.linkedin.openhouse.common.api.spec.ToggleStatusEnum; import com.linkedin.openhouse.tables.toggle.model.TableToggleStatus; import com.linkedin.openhouse.tables.toggle.model.ToggleStatusKey; import com.linkedin.openhouse.tables.toggle.repository.ToggleStatusesRepository; @@ -28,8 +29,11 @@ public boolean isFeatureActivated(String databaseId, String tableId, String feat .featureId(featureId) .build(); Optional toggleStatus = toggleStatusesRepository.findById(toggleStatusKey); - // TODO: Change this once HTS PR is in + return toggleStatus.isPresent() - && toggleStatus.get().getToggleStatusEnum().equalsIgnoreCase("active"); + && toggleStatus + .get() + .getToggleStatusEnum() + .equalsIgnoreCase(ToggleStatusEnum.ACTIVE.toString()); } } diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java index a80d5cb5..35f319f3 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java @@ -26,6 +26,5 @@ public class TableToggleStatus { @Id private String databaseId; - // TODO: Need to change it to the shared Enum class once HTS PR is in private String toggleStatusEnum; } diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/BaseToggleStatusesRepository.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/BaseToggleStatusesRepository.java index 99550aca..c3a68445 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/BaseToggleStatusesRepository.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/repository/BaseToggleStatusesRepository.java @@ -3,11 +3,13 @@ import com.linkedin.openhouse.tables.toggle.model.TableToggleStatus; import com.linkedin.openhouse.tables.toggle.model.ToggleStatusKey; import java.util.Optional; +import org.springframework.stereotype.Component; /** * THIS IS A TEMPORARY PLACEHOLDER, without this the whole springboot application is failed to start * given missing injection entity for {@link ToggleStatusesRepository}. */ +@Component public class BaseToggleStatusesRepository implements ToggleStatusesRepository { @Override public S save(S entity) { diff --git a/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/SpringH2TestApplication.java b/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/SpringH2TestApplication.java index 394d8f7e..615bc1b6 100644 --- a/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/SpringH2TestApplication.java +++ b/tables-test-fixtures/src/main/java/com/linkedin/openhouse/tablestest/SpringH2TestApplication.java @@ -24,12 +24,14 @@ basePackages = { "com.linkedin.openhouse.tables.api", "com.linkedin.openhouse.tables.audit", + "com.linkedin.openhouse.tables.toggle", "com.linkedin.openhouse.tables.authorization", "com.linkedin.openhouse.tables.dto.mapper", "com.linkedin.openhouse.tables.utils", "com.linkedin.openhouse.tables.controller", "com.linkedin.openhouse.tables.services", "com.linkedin.openhouse.tables.config", + "com.linkedin.openhouse.tables.toggle.repository", "com.linkedin.openhouse.internal.catalog", "com.linkedin.openhouse.cluster.configs", "com.linkedin.openhouse.cluster.storage", From dddf1da2f695b3679b9dbb1fa6ea69374ac9edd7 Mon Sep 17 00:00:00 2001 From: Lei Sun Date: Wed, 17 Apr 2024 15:58:29 -0700 Subject: [PATCH 2/4] Fix unit tests by reintroducing exact matching with HTS --- .../housetables/api/spec/model/ToggleStatus.java | 1 - .../api/spec/model}/ToggleStatusEnum.java | 2 +- .../services/ToggleStatusesServiceImpl.java | 14 ++++++++++++-- .../e2e/togglerule/ToggleStatusesServiceTest.java | 2 +- .../tables/toggle/BaseTableFeatureToggle.java | 4 ++-- 5 files changed, 16 insertions(+), 7 deletions(-) rename services/{common/src/main/java/com/linkedin/openhouse/common/api/spec => housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model}/ToggleStatusEnum.java (69%) diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatus.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatus.java index e507c541..2f1ba5c0 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatus.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatus.java @@ -1,7 +1,6 @@ package com.linkedin.openhouse.housetables.api.spec.model; import com.fasterxml.jackson.annotation.JsonProperty; -import com.linkedin.openhouse.common.api.spec.ToggleStatusEnum; import io.swagger.v3.oas.annotations.media.Schema; import javax.validation.constraints.NotEmpty; import lombok.Builder; diff --git a/services/common/src/main/java/com/linkedin/openhouse/common/api/spec/ToggleStatusEnum.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatusEnum.java similarity index 69% rename from services/common/src/main/java/com/linkedin/openhouse/common/api/spec/ToggleStatusEnum.java rename to services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatusEnum.java index cd9fc40d..a1ecdeab 100644 --- a/services/common/src/main/java/com/linkedin/openhouse/common/api/spec/ToggleStatusEnum.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/ToggleStatusEnum.java @@ -1,4 +1,4 @@ -package com.linkedin.openhouse.common.api.spec; +package com.linkedin.openhouse.housetables.api.spec.model; /** Indicate if a feature is active or inactive on an entity (e.g. table) */ public enum ToggleStatusEnum { diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java index 5ebb24d8..0c0d1b3b 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java @@ -1,7 +1,8 @@ package com.linkedin.openhouse.housetables.services; -import com.linkedin.openhouse.common.api.spec.ToggleStatusEnum; import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatus; +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatusEnum; +import com.linkedin.openhouse.housetables.model.TableToggleRule; import com.linkedin.openhouse.housetables.repository.impl.jdbc.ToggleStatusHtsJdbcRepository; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; @@ -12,6 +13,15 @@ public class ToggleStatusesServiceImpl implements ToggleStatusesService { @Override public ToggleStatus getTableToggleStatus(String featureId, String databaseId, String tableId) { - return ToggleStatus.builder().status(ToggleStatusEnum.ACTIVE).build(); + for (TableToggleRule tableToggleRule : htsRepository.findAllByFeature(featureId)) { + + // TODO: Evolve this rule engine to support wildcards + if (tableToggleRule.getTablePattern().equals(tableId) + && tableToggleRule.getDatabasePattern().equals(databaseId)) { + return ToggleStatus.builder().status(ToggleStatusEnum.ACTIVE).build(); + } + } + + return ToggleStatus.builder().status(ToggleStatusEnum.INACTIVE).build(); } } diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesServiceTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesServiceTest.java index e8bf5fba..597262c2 100644 --- a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesServiceTest.java +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusesServiceTest.java @@ -1,7 +1,7 @@ package com.linkedin.openhouse.housetables.e2e.togglerule; -import com.linkedin.openhouse.common.api.spec.ToggleStatusEnum; import com.linkedin.openhouse.common.test.cluster.PropertyOverrideContextInitializer; +import com.linkedin.openhouse.housetables.api.spec.model.ToggleStatusEnum; import com.linkedin.openhouse.housetables.services.ToggleStatusesService; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java index e201f1c1..9f80412a 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java @@ -1,6 +1,6 @@ package com.linkedin.openhouse.tables.toggle; -import com.linkedin.openhouse.common.api.spec.ToggleStatusEnum; +import com.linkedin.openhouse.housetables.client.model.ToggleStatus; import com.linkedin.openhouse.tables.toggle.model.TableToggleStatus; import com.linkedin.openhouse.tables.toggle.model.ToggleStatusKey; import com.linkedin.openhouse.tables.toggle.repository.ToggleStatusesRepository; @@ -34,6 +34,6 @@ public boolean isFeatureActivated(String databaseId, String tableId, String feat && toggleStatus .get() .getToggleStatusEnum() - .equalsIgnoreCase(ToggleStatusEnum.ACTIVE.toString()); + .equalsIgnoreCase(ToggleStatus.StatusEnum.ACTIVE.toString()); } } From 39b898e147a3d80bf1a3e1dce547728857e704aa Mon Sep 17 00:00:00 2001 From: Lei Sun Date: Wed, 17 Apr 2024 16:07:22 -0700 Subject: [PATCH 3/4] Addressing cosmetics comments --- .../api/spec/model/TableToggleStatusKey.java | 2 +- .../controller/ToggleStatusesController.java | 6 +++++- .../impl/jdbc/ToggleStatusHtsJdbcRepository.java | 2 +- .../services/ToggleStatusesServiceImpl.java | 2 +- .../e2e/togglerule/ToggleStatusRepositoryTest.java | 5 +++-- .../impl/OpenHouseInternalRepositoryImpl.java.rej | 10 ---------- 6 files changed, 11 insertions(+), 16 deletions(-) delete mode 100644 services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java.rej diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/TableToggleStatusKey.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/TableToggleStatusKey.java index c1a567e0..fa0d0197 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/TableToggleStatusKey.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/api/spec/model/TableToggleStatusKey.java @@ -33,7 +33,7 @@ public class TableToggleStatusKey { @Schema( description = "Unique Resource identifier for a feature within OpenHouse Service", - example = "dummy") + example = "wap-branch") @JsonProperty(value = "featureId") @NotEmpty(message = "featureId cannot be empty") String featureId; diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/controller/ToggleStatusesController.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/controller/ToggleStatusesController.java index 6b6f38ca..2dc72075 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/controller/ToggleStatusesController.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/controller/ToggleStatusesController.java @@ -28,7 +28,11 @@ public class ToggleStatusesController { summary = "Get a toggle status applied to a table.", description = "Returns a toggle status of databaseID and tableId on a featureId", tags = {"ToggleStatus"}) - @ApiResponses(value = {@ApiResponse(responseCode = "200", description = "Toggle status GET: OK")}) + @ApiResponses( + value = { + @ApiResponse(responseCode = "200", description = "Toggle status GET: OK"), + @ApiResponse(responseCode = "404", description = "Toggle status GET: NOT FOUND") + }) @GetMapping( value = TOGGLE_ENDPOINT, produces = {"application/json"}) diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java index 944a7917..72fc9818 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java @@ -4,5 +4,5 @@ import com.linkedin.openhouse.housetables.repository.HtsRepository; public interface ToggleStatusHtsJdbcRepository extends HtsRepository { - Iterable findAllByFeature(String feature); + Iterable findAllByFeatureId(String featureId); } diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java index 0c0d1b3b..6d5d4acc 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java @@ -13,7 +13,7 @@ public class ToggleStatusesServiceImpl implements ToggleStatusesService { @Override public ToggleStatus getTableToggleStatus(String featureId, String databaseId, String tableId) { - for (TableToggleRule tableToggleRule : htsRepository.findAllByFeature(featureId)) { + for (TableToggleRule tableToggleRule : htsRepository.findAllByFeatureId(featureId)) { // TODO: Evolve this rule engine to support wildcards if (tableToggleRule.getTablePattern().equals(tableId) diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java index 060c79ad..0684800a 100644 --- a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java @@ -21,13 +21,14 @@ public class ToggleStatusRepositoryTest { @Test public void testFindAllByFeature() { List toggleRuleList = - ImmutableList.copyOf(htsRepository.findAllByFeature(TEST_RULE_0.getFeature())); + ImmutableList.copyOf(htsRepository.findAllByFeatureId(TEST_RULE_0.getFeature())); Assertions.assertEquals(2, toggleRuleList.size()); // Now there should be 2 rules under dummy2 feature htsRepository.save( TEST_RULE_0.toBuilder().feature(TEST_RULE_2.getFeature()).databasePattern("dbnew").build()); - toggleRuleList = ImmutableList.copyOf(htsRepository.findAllByFeature(TEST_RULE_2.getFeature())); + toggleRuleList = + ImmutableList.copyOf(htsRepository.findAllByFeatureId(TEST_RULE_2.getFeature())); Assertions.assertEquals(2, toggleRuleList.size()); } } diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java.rej b/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java.rej deleted file mode 100644 index 65cd28cf..00000000 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java.rej +++ /dev/null @@ -1,10 +0,0 @@ -diff a/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/repository/impl/OpenHouseInternalRepositoryImpl.java (rejected hunks) -@@ -85,6 +86,8 @@ public class OpenHouseInternalRepositoryImpl implements OpenHouseInternalReposit - - @Autowired PreservedKeyChecker preservedKeyChecker; - -+ @Autowired TableFeatureToggle baseTableFeatureToggle; -+ - @Override - public TableDto save(TableDto tableDto) { - TableIdentifier tableIdentifier = From aacff7922d20e129f5b8665ab4fb394a0e2c4590 Mon Sep 17 00:00:00 2001 From: Lei Sun Date: Wed, 17 Apr 2024 16:27:39 -0700 Subject: [PATCH 4/4] Revoke of changing feature to featureId to preserve the possibility of fuzzy search --- .../impl/jdbc/ToggleStatusHtsJdbcRepository.java | 2 +- .../housetables/services/ToggleStatusesServiceImpl.java | 2 +- .../e2e/togglerule/ToggleStatusRepositoryTest.java | 5 ++--- .../openhouse/tables/toggle/BaseTableFeatureToggle.java | 1 + .../openhouse/tables/toggle/model/TableToggleStatus.java | 3 ++- .../tables/e2e/h2/BaseTableFeatureToggleTest.java | 7 ++++--- 6 files changed, 11 insertions(+), 9 deletions(-) diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java index 72fc9818..944a7917 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/repository/impl/jdbc/ToggleStatusHtsJdbcRepository.java @@ -4,5 +4,5 @@ import com.linkedin.openhouse.housetables.repository.HtsRepository; public interface ToggleStatusHtsJdbcRepository extends HtsRepository { - Iterable findAllByFeatureId(String featureId); + Iterable findAllByFeature(String feature); } diff --git a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java index 6d5d4acc..0c0d1b3b 100644 --- a/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java +++ b/services/housetables/src/main/java/com/linkedin/openhouse/housetables/services/ToggleStatusesServiceImpl.java @@ -13,7 +13,7 @@ public class ToggleStatusesServiceImpl implements ToggleStatusesService { @Override public ToggleStatus getTableToggleStatus(String featureId, String databaseId, String tableId) { - for (TableToggleRule tableToggleRule : htsRepository.findAllByFeatureId(featureId)) { + for (TableToggleRule tableToggleRule : htsRepository.findAllByFeature(featureId)) { // TODO: Evolve this rule engine to support wildcards if (tableToggleRule.getTablePattern().equals(tableId) diff --git a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java index 0684800a..060c79ad 100644 --- a/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java +++ b/services/housetables/src/test/java/com/linkedin/openhouse/housetables/e2e/togglerule/ToggleStatusRepositoryTest.java @@ -21,14 +21,13 @@ public class ToggleStatusRepositoryTest { @Test public void testFindAllByFeature() { List toggleRuleList = - ImmutableList.copyOf(htsRepository.findAllByFeatureId(TEST_RULE_0.getFeature())); + ImmutableList.copyOf(htsRepository.findAllByFeature(TEST_RULE_0.getFeature())); Assertions.assertEquals(2, toggleRuleList.size()); // Now there should be 2 rules under dummy2 feature htsRepository.save( TEST_RULE_0.toBuilder().feature(TEST_RULE_2.getFeature()).databasePattern("dbnew").build()); - toggleRuleList = - ImmutableList.copyOf(htsRepository.findAllByFeatureId(TEST_RULE_2.getFeature())); + toggleRuleList = ImmutableList.copyOf(htsRepository.findAllByFeature(TEST_RULE_2.getFeature())); Assertions.assertEquals(2, toggleRuleList.size()); } } diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java index 9f80412a..9ad3a7cb 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/BaseTableFeatureToggle.java @@ -34,6 +34,7 @@ public boolean isFeatureActivated(String databaseId, String tableId, String feat && toggleStatus .get() .getToggleStatusEnum() + .toString() .equalsIgnoreCase(ToggleStatus.StatusEnum.ACTIVE.toString()); } } diff --git a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java index 35f319f3..500511b0 100644 --- a/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java +++ b/services/tables/src/main/java/com/linkedin/openhouse/tables/toggle/model/TableToggleStatus.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.tables.toggle.model; +import com.linkedin.openhouse.housetables.client.model.ToggleStatus; import javax.persistence.Entity; import javax.persistence.Id; import javax.persistence.IdClass; @@ -26,5 +27,5 @@ public class TableToggleStatus { @Id private String databaseId; - private String toggleStatusEnum; + private ToggleStatus.StatusEnum toggleStatusEnum; } diff --git a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/BaseTableFeatureToggleTest.java b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/BaseTableFeatureToggleTest.java index 17fa59de..f17e20a2 100644 --- a/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/BaseTableFeatureToggleTest.java +++ b/services/tables/src/test/java/com/linkedin/openhouse/tables/e2e/h2/BaseTableFeatureToggleTest.java @@ -1,5 +1,6 @@ package com.linkedin.openhouse.tables.e2e.h2; +import com.linkedin.openhouse.housetables.client.model.ToggleStatus; import com.linkedin.openhouse.tables.toggle.BaseTableFeatureToggle; import com.linkedin.openhouse.tables.toggle.model.TableToggleStatus; import com.linkedin.openhouse.tables.toggle.repository.ToggleStatusesRepository; @@ -21,21 +22,21 @@ public void testDummyToggle() { .featureId("dummy") .databaseId("db1") .tableId("tbl1") - .toggleStatusEnum("ACTIVE") + .toggleStatusEnum(ToggleStatus.StatusEnum.ACTIVE) .build(); TableToggleStatus rule2 = TableToggleStatus.builder() .featureId("dummy") .databaseId("db2") .tableId("tbl2") - .toggleStatusEnum("ACTIVE") + .toggleStatusEnum(ToggleStatus.StatusEnum.ACTIVE) .build(); TableToggleStatus rule3 = TableToggleStatus.builder() .featureId("random") .databaseId("db1") .tableId("tbl3") - .toggleStatusEnum("ACTIVE") + .toggleStatusEnum(ToggleStatus.StatusEnum.ACTIVE) .build(); toggleStatusesRepository.save(rule1); toggleStatusesRepository.save(rule2);