diff --git a/config-service-impl/build.gradle.kts b/config-service-impl/build.gradle.kts index 4a84e25a..501cadc9 100644 --- a/config-service-impl/build.gradle.kts +++ b/config-service-impl/build.gradle.kts @@ -24,6 +24,7 @@ dependencies { compileOnly(commonLibs.lombok) testImplementation(commonLibs.junit.jupiter) + testImplementation(commonLibs.mockito.junit) testImplementation(commonLibs.mockito.core) testImplementation(commonLibs.hypertrace.grpcutils.client) } diff --git a/config-service-impl/gradle.lockfile b/config-service-impl/gradle.lockfile index 5ff44502..065e711b 100644 --- a/config-service-impl/gradle.lockfile +++ b/config-service-impl/gradle.lockfile @@ -82,16 +82,22 @@ org.hypertrace.core.kafkastreams.framework:kafka-bom:0.5.4=compileClasspath,runt org.hypertrace.core.serviceframework:docstore-metrics:0.1.86=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath org.hypertrace.core.serviceframework:platform-metrics:0.1.86=runtimeClasspath,testRuntimeClasspath org.hypertrace.core.serviceframework:service-framework-spi:0.1.86=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath -org.junit.jupiter:junit-jupiter-api:5.10.0=testCompileClasspath,testRuntimeClasspath -org.junit.jupiter:junit-jupiter-engine:5.10.0=testRuntimeClasspath -org.junit.jupiter:junit-jupiter-params:5.10.0=testCompileClasspath,testRuntimeClasspath -org.junit.jupiter:junit-jupiter:5.10.0=testCompileClasspath,testRuntimeClasspath -org.junit.platform:junit-platform-commons:1.10.0=testCompileClasspath,testRuntimeClasspath -org.junit.platform:junit-platform-engine:1.10.0=testRuntimeClasspath -org.junit:junit-bom:5.10.0=testCompileClasspath,testRuntimeClasspath +org.junit.jupiter:junit-jupiter-api:5.10.0=testCompileClasspath +org.junit.jupiter:junit-jupiter-api:5.10.1=testRuntimeClasspath +org.junit.jupiter:junit-jupiter-engine:5.10.1=testRuntimeClasspath +org.junit.jupiter:junit-jupiter-params:5.10.0=testCompileClasspath +org.junit.jupiter:junit-jupiter-params:5.10.1=testRuntimeClasspath +org.junit.jupiter:junit-jupiter:5.10.0=testCompileClasspath +org.junit.jupiter:junit-jupiter:5.10.1=testRuntimeClasspath +org.junit.platform:junit-platform-commons:1.10.0=testCompileClasspath +org.junit.platform:junit-platform-commons:1.10.1=testRuntimeClasspath +org.junit.platform:junit-platform-engine:1.10.1=testRuntimeClasspath +org.junit:junit-bom:5.10.0=testCompileClasspath +org.junit:junit-bom:5.10.1=testRuntimeClasspath org.latencyutils:LatencyUtils:2.0.3=runtimeClasspath,testRuntimeClasspath org.lz4:lz4-java:1.8.0=runtimeClasspath,testRuntimeClasspath org.mockito:mockito-core:5.8.0=testCompileClasspath,testRuntimeClasspath +org.mockito:mockito-junit-jupiter:5.8.0=testCompileClasspath,testRuntimeClasspath org.mongodb:bson-record-codec:5.2.0=runtimeClasspath,testRuntimeClasspath org.mongodb:bson:5.2.0=runtimeClasspath,testRuntimeClasspath org.mongodb:mongodb-driver-core:5.2.0=runtimeClasspath,testRuntimeClasspath diff --git a/config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceUtils.java b/config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceUtils.java index 624c126b..1c803f93 100644 --- a/config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceUtils.java +++ b/config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceUtils.java @@ -1,5 +1,7 @@ package org.hypertrace.config.service; +import static org.hypertrace.config.service.store.ConfigDocument.CONFIG_FIELD_NAME; + import com.google.protobuf.NullValue; import com.google.protobuf.Struct; import com.google.protobuf.Value; @@ -108,6 +110,10 @@ public static ContextSpecificConfig emptyConfig(String context) { .build(); } + public static String buildConfigFieldPath(String configJsonPath) { + return String.format("%s.%s", CONFIG_FIELD_NAME, configJsonPath); + } + private static ContextSpecificConfig buildContextSpecificConfig( Value config, long creationTimestamp, long updateTimestamp) { return ContextSpecificConfig.newBuilder() diff --git a/config-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java b/config-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java index 69292852..7693a053 100644 --- a/config-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java +++ b/config-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java @@ -262,7 +262,9 @@ private Query buildQuery( sortByList.forEach( sortBy -> queryBuilder.addSort( - IdentifierExpression.of(sortBy.getSelection().getConfigJsonPath()), + IdentifierExpression.of( + ConfigServiceUtils.buildConfigFieldPath( + sortBy.getSelection().getConfigJsonPath())), convertSortOrder(sortBy))); } else { queryBuilder.addSort(IdentifierExpression.of(CREATION_TIMESTAMP_FIELD_NAME), SortOrder.DESC); diff --git a/config-service-impl/src/main/java/org/hypertrace/config/service/store/FilterExpressionBuilder.java b/config-service-impl/src/main/java/org/hypertrace/config/service/store/FilterExpressionBuilder.java index 7ad61dde..eb5c400b 100644 --- a/config-service-impl/src/main/java/org/hypertrace/config/service/store/FilterExpressionBuilder.java +++ b/config-service-impl/src/main/java/org/hypertrace/config/service/store/FilterExpressionBuilder.java @@ -1,10 +1,9 @@ package org.hypertrace.config.service.store; -import static org.hypertrace.config.service.store.ConfigDocument.CONFIG_FIELD_NAME; - import io.grpc.Status; import java.util.List; import java.util.stream.Collectors; +import org.hypertrace.config.service.ConfigServiceUtils; import org.hypertrace.config.service.v1.Filter; import org.hypertrace.config.service.v1.LogicalFilter; import org.hypertrace.config.service.v1.RelationalFilter; @@ -87,12 +86,9 @@ private FilterTypeExpression buildRelationalExpression(RelationalFilter relation } return RelationalExpression.of( - IdentifierExpression.of(buildConfigFieldPath(relationalFilter.getConfigJsonPath())), + IdentifierExpression.of( + ConfigServiceUtils.buildConfigFieldPath(relationalFilter.getConfigJsonPath())), operator, ConstantExpressionConverter.fromProtoValue(relationalFilter.getValue())); } - - private String buildConfigFieldPath(String configJsonPath) { - return String.format("%s.%s", CONFIG_FIELD_NAME, configJsonPath); - } } diff --git a/config-service-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java b/config-service-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java index f82c6f8c..e062b766 100644 --- a/config-service-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java +++ b/config-service-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java @@ -1,5 +1,6 @@ package org.hypertrace.config.service.store; +import static java.util.Collections.emptyList; import static org.hypertrace.config.service.TestUtils.CONTEXT1; import static org.hypertrace.config.service.TestUtils.RESOURCE_NAME; import static org.hypertrace.config.service.TestUtils.RESOURCE_NAMESPACE; @@ -9,8 +10,10 @@ import static org.hypertrace.config.service.TestUtils.getConfigResourceContext; import static org.hypertrace.config.service.store.DocumentConfigStore.CONFIGURATIONS_COLLECTION; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -22,6 +25,7 @@ import io.grpc.StatusRuntimeException; import java.io.IOException; import java.time.Clock; +import java.util.Arrays; import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -37,6 +41,7 @@ import org.hypertrace.config.service.v1.Pagination; import org.hypertrace.config.service.v1.RelationalFilter; import org.hypertrace.config.service.v1.RelationalOperator; +import org.hypertrace.config.service.v1.SortOrder; import org.hypertrace.config.service.v1.UpsertAllConfigsResponse.UpsertedConfig; import org.hypertrace.config.service.v1.UpsertConfigRequest; import org.hypertrace.core.documentstore.CloseableIterator; @@ -45,12 +50,24 @@ import org.hypertrace.core.documentstore.Document; import org.hypertrace.core.documentstore.Key; import org.hypertrace.core.documentstore.UpdateResult; -import org.hypertrace.core.documentstore.metric.DocStoreMetricProvider; +import org.hypertrace.core.documentstore.expression.impl.ConstantExpression; +import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression; +import org.hypertrace.core.documentstore.expression.impl.LogicalExpression; +import org.hypertrace.core.documentstore.expression.impl.RelationalExpression; +import org.hypertrace.core.documentstore.expression.type.FilterTypeExpression; +import org.hypertrace.core.documentstore.model.options.QueryOptions; import org.hypertrace.core.documentstore.query.Query; +import org.hypertrace.core.documentstore.query.SortingSpec; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Answers; import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +@ExtendWith(MockitoExtension.class) class DocumentConfigStoreTest { private static final long CONFIG_VERSION = 5; @@ -62,17 +79,19 @@ class DocumentConfigStoreTest { private static Value config1 = getConfig1(); private static Value config2 = getConfig2(); private static ConfigResourceContext configResourceContext = getConfigResourceContext(); - private static Collection collection; - private DocumentConfigStore configStore; - private Clock mockClock; - private FilterBuilder filterBuilder; - @BeforeEach() + private Collection collection; + @Mock private Clock mockClock; + @Mock private FilterBuilder filterBuilder; + + @Mock(answer = Answers.RETURNS_DEEP_STUBS) + private Datastore mockDatastore; + + @InjectMocks private DocumentConfigStore configStore; + + @BeforeEach void beforeEach() { - collection = mock(Collection.class); - this.mockClock = mock(Clock.class); - this.filterBuilder = mock(FilterBuilder.class); - this.configStore = new DocumentConfigStore(mockClock, new MockDatastore()); + this.collection = mockDatastore.getCollection(CONFIGURATIONS_COLLECTION); } @Test @@ -182,10 +201,7 @@ void WriteConfigForUpdateWithUpsertCondition() throws IOException { .setValue(Values.of("v2"))))) .build(); - org.hypertrace.core.documentstore.Filter docFilter = - new org.hypertrace.core.documentstore.Filter(); when(request.getUpsertCondition()).thenReturn(upsertCondition); - when(filterBuilder.buildDocStoreFilter(upsertCondition)).thenReturn(docFilter); UpsertedConfig upsertedConfig = configStore.writeConfig(configResourceContext, USER_ID, request, USER_EMAIL); assertEquals(config2, upsertedConfig.getConfig()); @@ -212,7 +228,6 @@ void WriteConfigForUpdateWithUpsertCondition() throws IOException { document); // failed upsert condition - when(updateResult.getUpdatedCount()).thenReturn(0L); assertThrows( StatusRuntimeException.class, () -> configStore.writeConfig(configResourceContext, USER_ID, request, USER_EMAIL)); @@ -273,7 +288,7 @@ void getAllConfigs() throws IOException { new ConfigResource(RESOURCE_NAME, RESOURCE_NAMESPACE, TENANT_ID), Filter.getDefaultInstance(), // for empty filter Pagination.getDefaultInstance(), - Collections.emptyList()); + emptyList()); assertEquals(2, contextSpecificConfigList.size()); assertEquals( @@ -358,7 +373,135 @@ void writeAllConfigs() throws IOException { updateTime))); } - private static Document getConfigDocument( + @Test + void buildQuery_withDefaultPagination() throws Exception { + List expectedSorts = + Arrays.asList( + SortingSpec.of( + IdentifierExpression.of("configVersion"), + org.hypertrace.core.documentstore.expression.operators.SortOrder.DESC), + SortingSpec.of( + IdentifierExpression.of("creationTimestamp"), + org.hypertrace.core.documentstore.expression.operators.SortOrder.DESC)); + + configStore.getAllConfigs( + new ConfigResource(RESOURCE_NAMESPACE, RESOURCE_NAME, "tenant1"), + Filter.getDefaultInstance(), + Pagination.getDefaultInstance(), + emptyList()); + + verify(collection) + .query(argThat(query -> query.getSorts().equals(expectedSorts)), any(QueryOptions.class)); + } + + @Test + void buildQuery_withCustomPagination() throws IOException { + Pagination pagination = Pagination.newBuilder().setOffset(10).setLimit(20).build(); + org.hypertrace.core.documentstore.query.Pagination expectedPagination = + org.hypertrace.core.documentstore.query.Pagination.builder().offset(10).limit(20).build(); + + configStore.getAllConfigs( + new ConfigResource(RESOURCE_NAMESPACE, RESOURCE_NAME, "tenant1"), + Filter.getDefaultInstance(), + pagination, + emptyList()); + + verify(collection) + .query( + argThat(query -> query.getPagination().orElseThrow().equals(expectedPagination)), + any(QueryOptions.class)); + } + + @Test + void buildQuery_withCustomSorting() throws IOException { + org.hypertrace.config.service.v1.Selection selection = + org.hypertrace.config.service.v1.Selection.newBuilder() + .setConfigJsonPath("test.path") + .build(); + org.hypertrace.config.service.v1.SortBy sortBy = + org.hypertrace.config.service.v1.SortBy.newBuilder() + .setSelection(selection) + .setSortOrder(SortOrder.SORT_ORDER_DESC) + .build(); + + List expectedSorts = + Arrays.asList( + SortingSpec.of( + IdentifierExpression.of("configVersion"), + org.hypertrace.core.documentstore.expression.operators.SortOrder.DESC), + SortingSpec.of( + IdentifierExpression.of("config.test.path"), + org.hypertrace.core.documentstore.expression.operators.SortOrder.DESC)); + + configStore.getAllConfigs( + new ConfigResource(RESOURCE_NAMESPACE, RESOURCE_NAME, "tenant1"), + Filter.getDefaultInstance(), + Pagination.getDefaultInstance(), + Collections.singletonList(sortBy)); + + verify(collection) + .query(argThat(query -> query.getSorts().equals(expectedSorts)), any(QueryOptions.class)); + } + + @Test + void buildQuery_withFilter() throws IOException { + ConfigResource configResource = + new ConfigResource(RESOURCE_NAMESPACE, RESOURCE_NAME, "tenant1"); + + RelationalFilter relationalFilter = + RelationalFilter.newBuilder() + .setConfigJsonPath("test.path") + .setOperator(RelationalOperator.RELATIONAL_OPERATOR_EQ) + .setValue(Values.of("test-value")) + .build(); + + Filter filter = Filter.newBuilder().setRelationalFilter(relationalFilter).build(); + + Pagination pagination = Pagination.getDefaultInstance(); + List sortByList = emptyList(); + + // Create a mock for CloseableIterator + CloseableIterator mockIterator = mock(CloseableIterator.class); + when(mockIterator.hasNext()).thenReturn(false); // No documents to process + + // Mock the collection.query to return the mock iterator + when(collection.query(any(Query.class), any(QueryOptions.class))).thenReturn(mockIterator); + + // Act + configStore.getAllConfigs(configResource, filter, pagination, emptyList()); + + // Capture the Query object passed to collection.query + ArgumentCaptor queryCaptor = ArgumentCaptor.forClass(Query.class); + verify(collection).query(queryCaptor.capture(), any(QueryOptions.class)); + + // Assert the properties of the captured Query + Query capturedQuery = queryCaptor.getValue(); + assertNotNull(capturedQuery); + + // Verify filter is present in the query + assertNotNull(capturedQuery.getFilter()); + + // Compare the actual filter with the expected filter + FilterTypeExpression actualFilter = + ((LogicalExpression) capturedQuery.getFilter().get()).getOperands().get(1); + FilterTypeExpression expectedFilter = createExpectedFilter(); + + assertEquals(expectedFilter, actualFilter); // Compare the entire filter expressions + } + + private FilterTypeExpression createExpectedFilter() { + // Create the expected left-hand side and right-hand side expressions + IdentifierExpression lhs = IdentifierExpression.of("config.test.path"); + ConstantExpression rhs = ConstantExpression.of("test-value"); + + // Build the relational expression + + // Assuming you have a logical expression that contains this relational expression + return RelationalExpression.of( + lhs, org.hypertrace.core.documentstore.expression.operators.RelationalOperator.EQ, rhs); + } + + private Document getConfigDocument( String context, long version, Value config, long creationTimestamp, long updateTimestamp) { return new ConfigDocument( RESOURCE_NAME, @@ -395,41 +538,4 @@ public Document next() { return documentIterator.next(); } } - - public static class MockDatastore implements Datastore { - - @Override - public Set listCollections() { - return Collections.singleton(CONFIGURATIONS_COLLECTION); - } - - @Override - public Collection getCollection(String s) { - return collection; - } - - // default implementation for other methods as they are unused - @Override - public boolean createCollection(String s, Map map) { - return false; - } - - @Override - public boolean deleteCollection(String s) { - return false; - } - - @Override - public boolean healthCheck() { - return false; - } - - @Override - public DocStoreMetricProvider getDocStoreMetricProvider() { - return null; - } - - @Override - public void close() {} - } }