Skip to content

Commit f8e72f5

Browse files
pavan-traceablepavan-traceable
authored
Bugfix user filters (#289)
* Add generic api's for filtering, pagination in getAllConfigs call
1 parent 1ca592c commit f8e72f5

6 files changed

Lines changed: 185 additions & 68 deletions

File tree

‎config-service-impl/build.gradle.kts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ dependencies {
2424
compileOnly(commonLibs.lombok)
2525

2626
testImplementation(commonLibs.junit.jupiter)
27+
testImplementation(commonLibs.mockito.junit)
2728
testImplementation(commonLibs.mockito.core)
2829
testImplementation(commonLibs.hypertrace.grpcutils.client)
2930
}

‎config-service-impl/gradle.lockfile‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,16 +82,22 @@ org.hypertrace.core.kafkastreams.framework:kafka-bom:0.5.4=compileClasspath,runt
8282
org.hypertrace.core.serviceframework:docstore-metrics:0.1.86=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
8383
org.hypertrace.core.serviceframework:platform-metrics:0.1.86=runtimeClasspath,testRuntimeClasspath
8484
org.hypertrace.core.serviceframework:service-framework-spi:0.1.86=compileClasspath,runtimeClasspath,testCompileClasspath,testRuntimeClasspath
85-
org.junit.jupiter:junit-jupiter-api:5.10.0=testCompileClasspath,testRuntimeClasspath
86-
org.junit.jupiter:junit-jupiter-engine:5.10.0=testRuntimeClasspath
87-
org.junit.jupiter:junit-jupiter-params:5.10.0=testCompileClasspath,testRuntimeClasspath
88-
org.junit.jupiter:junit-jupiter:5.10.0=testCompileClasspath,testRuntimeClasspath
89-
org.junit.platform:junit-platform-commons:1.10.0=testCompileClasspath,testRuntimeClasspath
90-
org.junit.platform:junit-platform-engine:1.10.0=testRuntimeClasspath
91-
org.junit:junit-bom:5.10.0=testCompileClasspath,testRuntimeClasspath
85+
org.junit.jupiter:junit-jupiter-api:5.10.0=testCompileClasspath
86+
org.junit.jupiter:junit-jupiter-api:5.10.1=testRuntimeClasspath
87+
org.junit.jupiter:junit-jupiter-engine:5.10.1=testRuntimeClasspath
88+
org.junit.jupiter:junit-jupiter-params:5.10.0=testCompileClasspath
89+
org.junit.jupiter:junit-jupiter-params:5.10.1=testRuntimeClasspath
90+
org.junit.jupiter:junit-jupiter:5.10.0=testCompileClasspath
91+
org.junit.jupiter:junit-jupiter:5.10.1=testRuntimeClasspath
92+
org.junit.platform:junit-platform-commons:1.10.0=testCompileClasspath
93+
org.junit.platform:junit-platform-commons:1.10.1=testRuntimeClasspath
94+
org.junit.platform:junit-platform-engine:1.10.1=testRuntimeClasspath
95+
org.junit:junit-bom:5.10.0=testCompileClasspath
96+
org.junit:junit-bom:5.10.1=testRuntimeClasspath
9297
org.latencyutils:LatencyUtils:2.0.3=runtimeClasspath,testRuntimeClasspath
9398
org.lz4:lz4-java:1.8.0=runtimeClasspath,testRuntimeClasspath
9499
org.mockito:mockito-core:5.8.0=testCompileClasspath,testRuntimeClasspath
100+
org.mockito:mockito-junit-jupiter:5.8.0=testCompileClasspath,testRuntimeClasspath
95101
org.mongodb:bson-record-codec:5.2.0=runtimeClasspath,testRuntimeClasspath
96102
org.mongodb:bson:5.2.0=runtimeClasspath,testRuntimeClasspath
97103
org.mongodb:mongodb-driver-core:5.2.0=runtimeClasspath,testRuntimeClasspath

‎config-service-impl/src/main/java/org/hypertrace/config/service/ConfigServiceUtils.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package org.hypertrace.config.service;
22

3+
import static org.hypertrace.config.service.store.ConfigDocument.CONFIG_FIELD_NAME;
4+
35
import com.google.protobuf.NullValue;
46
import com.google.protobuf.Struct;
57
import com.google.protobuf.Value;
@@ -108,6 +110,10 @@ public static ContextSpecificConfig emptyConfig(String context) {
108110
.build();
109111
}
110112

113+
public static String buildConfigFieldPath(String configJsonPath) {
114+
return String.format("%s.%s", CONFIG_FIELD_NAME, configJsonPath);
115+
}
116+
111117
private static ContextSpecificConfig buildContextSpecificConfig(
112118
Value config, long creationTimestamp, long updateTimestamp) {
113119
return ContextSpecificConfig.newBuilder()

‎config-service-impl/src/main/java/org/hypertrace/config/service/store/DocumentConfigStore.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,9 @@ private Query buildQuery(
262262
sortByList.forEach(
263263
sortBy ->
264264
queryBuilder.addSort(
265-
IdentifierExpression.of(sortBy.getSelection().getConfigJsonPath()),
265+
IdentifierExpression.of(
266+
ConfigServiceUtils.buildConfigFieldPath(
267+
sortBy.getSelection().getConfigJsonPath())),
266268
convertSortOrder(sortBy)));
267269
} else {
268270
queryBuilder.addSort(IdentifierExpression.of(CREATION_TIMESTAMP_FIELD_NAME), SortOrder.DESC);

‎config-service-impl/src/main/java/org/hypertrace/config/service/store/FilterExpressionBuilder.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
package org.hypertrace.config.service.store;
22

3-
import static org.hypertrace.config.service.store.ConfigDocument.CONFIG_FIELD_NAME;
4-
53
import io.grpc.Status;
64
import java.util.List;
75
import java.util.stream.Collectors;
6+
import org.hypertrace.config.service.ConfigServiceUtils;
87
import org.hypertrace.config.service.v1.Filter;
98
import org.hypertrace.config.service.v1.LogicalFilter;
109
import org.hypertrace.config.service.v1.RelationalFilter;
@@ -87,12 +86,9 @@ private FilterTypeExpression buildRelationalExpression(RelationalFilter relation
8786
}
8887

8988
return RelationalExpression.of(
90-
IdentifierExpression.of(buildConfigFieldPath(relationalFilter.getConfigJsonPath())),
89+
IdentifierExpression.of(
90+
ConfigServiceUtils.buildConfigFieldPath(relationalFilter.getConfigJsonPath())),
9191
operator,
9292
ConstantExpressionConverter.fromProtoValue(relationalFilter.getValue()));
9393
}
94-
95-
private String buildConfigFieldPath(String configJsonPath) {
96-
return String.format("%s.%s", CONFIG_FIELD_NAME, configJsonPath);
97-
}
9894
}

‎config-service-impl/src/test/java/org/hypertrace/config/service/store/DocumentConfigStoreTest.java‎

Lines changed: 159 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.hypertrace.config.service.store;
22

3+
import static java.util.Collections.emptyList;
34
import static org.hypertrace.config.service.TestUtils.CONTEXT1;
45
import static org.hypertrace.config.service.TestUtils.RESOURCE_NAME;
56
import static org.hypertrace.config.service.TestUtils.RESOURCE_NAMESPACE;
@@ -9,8 +10,10 @@
910
import static org.hypertrace.config.service.TestUtils.getConfigResourceContext;
1011
import static org.hypertrace.config.service.store.DocumentConfigStore.CONFIGURATIONS_COLLECTION;
1112
import static org.junit.jupiter.api.Assertions.assertEquals;
13+
import static org.junit.jupiter.api.Assertions.assertNotNull;
1214
import static org.junit.jupiter.api.Assertions.assertThrows;
1315
import static org.mockito.ArgumentMatchers.any;
16+
import static org.mockito.ArgumentMatchers.argThat;
1417
import static org.mockito.Mockito.mock;
1518
import static org.mockito.Mockito.times;
1619
import static org.mockito.Mockito.verify;
@@ -22,6 +25,7 @@
2225
import io.grpc.StatusRuntimeException;
2326
import java.io.IOException;
2427
import java.time.Clock;
28+
import java.util.Arrays;
2529
import java.util.Collections;
2630
import java.util.Iterator;
2731
import java.util.List;
@@ -37,6 +41,7 @@
3741
import org.hypertrace.config.service.v1.Pagination;
3842
import org.hypertrace.config.service.v1.RelationalFilter;
3943
import org.hypertrace.config.service.v1.RelationalOperator;
44+
import org.hypertrace.config.service.v1.SortOrder;
4045
import org.hypertrace.config.service.v1.UpsertAllConfigsResponse.UpsertedConfig;
4146
import org.hypertrace.config.service.v1.UpsertConfigRequest;
4247
import org.hypertrace.core.documentstore.CloseableIterator;
@@ -45,12 +50,24 @@
4550
import org.hypertrace.core.documentstore.Document;
4651
import org.hypertrace.core.documentstore.Key;
4752
import org.hypertrace.core.documentstore.UpdateResult;
48-
import org.hypertrace.core.documentstore.metric.DocStoreMetricProvider;
53+
import org.hypertrace.core.documentstore.expression.impl.ConstantExpression;
54+
import org.hypertrace.core.documentstore.expression.impl.IdentifierExpression;
55+
import org.hypertrace.core.documentstore.expression.impl.LogicalExpression;
56+
import org.hypertrace.core.documentstore.expression.impl.RelationalExpression;
57+
import org.hypertrace.core.documentstore.expression.type.FilterTypeExpression;
58+
import org.hypertrace.core.documentstore.model.options.QueryOptions;
4959
import org.hypertrace.core.documentstore.query.Query;
60+
import org.hypertrace.core.documentstore.query.SortingSpec;
5061
import org.junit.jupiter.api.BeforeEach;
5162
import org.junit.jupiter.api.Test;
63+
import org.junit.jupiter.api.extension.ExtendWith;
64+
import org.mockito.Answers;
5265
import org.mockito.ArgumentCaptor;
66+
import org.mockito.InjectMocks;
67+
import org.mockito.Mock;
68+
import org.mockito.junit.jupiter.MockitoExtension;
5369

70+
@ExtendWith(MockitoExtension.class)
5471
class DocumentConfigStoreTest {
5572

5673
private static final long CONFIG_VERSION = 5;
@@ -62,17 +79,19 @@ class DocumentConfigStoreTest {
6279
private static Value config1 = getConfig1();
6380
private static Value config2 = getConfig2();
6481
private static ConfigResourceContext configResourceContext = getConfigResourceContext();
65-
private static Collection collection;
66-
private DocumentConfigStore configStore;
67-
private Clock mockClock;
68-
private FilterBuilder filterBuilder;
6982

70-
@BeforeEach()
83+
private Collection collection;
84+
@Mock private Clock mockClock;
85+
@Mock private FilterBuilder filterBuilder;
86+
87+
@Mock(answer = Answers.RETURNS_DEEP_STUBS)
88+
private Datastore mockDatastore;
89+
90+
@InjectMocks private DocumentConfigStore configStore;
91+
92+
@BeforeEach
7193
void beforeEach() {
72-
collection = mock(Collection.class);
73-
this.mockClock = mock(Clock.class);
74-
this.filterBuilder = mock(FilterBuilder.class);
75-
this.configStore = new DocumentConfigStore(mockClock, new MockDatastore());
94+
this.collection = mockDatastore.getCollection(CONFIGURATIONS_COLLECTION);
7695
}
7796

7897
@Test
@@ -182,10 +201,7 @@ void WriteConfigForUpdateWithUpsertCondition() throws IOException {
182201
.setValue(Values.of("v2")))))
183202
.build();
184203

185-
org.hypertrace.core.documentstore.Filter docFilter =
186-
new org.hypertrace.core.documentstore.Filter();
187204
when(request.getUpsertCondition()).thenReturn(upsertCondition);
188-
when(filterBuilder.buildDocStoreFilter(upsertCondition)).thenReturn(docFilter);
189205
UpsertedConfig upsertedConfig =
190206
configStore.writeConfig(configResourceContext, USER_ID, request, USER_EMAIL);
191207
assertEquals(config2, upsertedConfig.getConfig());
@@ -212,7 +228,6 @@ void WriteConfigForUpdateWithUpsertCondition() throws IOException {
212228
document);
213229

214230
// failed upsert condition
215-
when(updateResult.getUpdatedCount()).thenReturn(0L);
216231
assertThrows(
217232
StatusRuntimeException.class,
218233
() -> configStore.writeConfig(configResourceContext, USER_ID, request, USER_EMAIL));
@@ -273,7 +288,7 @@ void getAllConfigs() throws IOException {
273288
new ConfigResource(RESOURCE_NAME, RESOURCE_NAMESPACE, TENANT_ID),
274289
Filter.getDefaultInstance(), // for empty filter
275290
Pagination.getDefaultInstance(),
276-
Collections.emptyList());
291+
emptyList());
277292

278293
assertEquals(2, contextSpecificConfigList.size());
279294
assertEquals(
@@ -358,7 +373,135 @@ void writeAllConfigs() throws IOException {
358373
updateTime)));
359374
}
360375

361-
private static Document getConfigDocument(
376+
@Test
377+
void buildQuery_withDefaultPagination() throws Exception {
378+
List<SortingSpec> expectedSorts =
379+
Arrays.asList(
380+
SortingSpec.of(
381+
IdentifierExpression.of("configVersion"),
382+
org.hypertrace.core.documentstore.expression.operators.SortOrder.DESC),
383+
SortingSpec.of(
384+
IdentifierExpression.of("creationTimestamp"),
385+
org.hypertrace.core.documentstore.expression.operators.SortOrder.DESC));
386+
387+
configStore.getAllConfigs(
388+
new ConfigResource(RESOURCE_NAMESPACE, RESOURCE_NAME, "tenant1"),
389+
Filter.getDefaultInstance(),
390+
Pagination.getDefaultInstance(),
391+
emptyList());
392+
393+
verify(collection)
394+
.query(argThat(query -> query.getSorts().equals(expectedSorts)), any(QueryOptions.class));
395+
}
396+
397+
@Test
398+
void buildQuery_withCustomPagination() throws IOException {
399+
Pagination pagination = Pagination.newBuilder().setOffset(10).setLimit(20).build();
400+
org.hypertrace.core.documentstore.query.Pagination expectedPagination =
401+
org.hypertrace.core.documentstore.query.Pagination.builder().offset(10).limit(20).build();
402+
403+
configStore.getAllConfigs(
404+
new ConfigResource(RESOURCE_NAMESPACE, RESOURCE_NAME, "tenant1"),
405+
Filter.getDefaultInstance(),
406+
pagination,
407+
emptyList());
408+
409+
verify(collection)
410+
.query(
411+
argThat(query -> query.getPagination().orElseThrow().equals(expectedPagination)),
412+
any(QueryOptions.class));
413+
}
414+
415+
@Test
416+
void buildQuery_withCustomSorting() throws IOException {
417+
org.hypertrace.config.service.v1.Selection selection =
418+
org.hypertrace.config.service.v1.Selection.newBuilder()
419+
.setConfigJsonPath("test.path")
420+
.build();
421+
org.hypertrace.config.service.v1.SortBy sortBy =
422+
org.hypertrace.config.service.v1.SortBy.newBuilder()
423+
.setSelection(selection)
424+
.setSortOrder(SortOrder.SORT_ORDER_DESC)
425+
.build();
426+
427+
List<SortingSpec> expectedSorts =
428+
Arrays.asList(
429+
SortingSpec.of(
430+
IdentifierExpression.of("configVersion"),
431+
org.hypertrace.core.documentstore.expression.operators.SortOrder.DESC),
432+
SortingSpec.of(
433+
IdentifierExpression.of("config.test.path"),
434+
org.hypertrace.core.documentstore.expression.operators.SortOrder.DESC));
435+
436+
configStore.getAllConfigs(
437+
new ConfigResource(RESOURCE_NAMESPACE, RESOURCE_NAME, "tenant1"),
438+
Filter.getDefaultInstance(),
439+
Pagination.getDefaultInstance(),
440+
Collections.singletonList(sortBy));
441+
442+
verify(collection)
443+
.query(argThat(query -> query.getSorts().equals(expectedSorts)), any(QueryOptions.class));
444+
}
445+
446+
@Test
447+
void buildQuery_withFilter() throws IOException {
448+
ConfigResource configResource =
449+
new ConfigResource(RESOURCE_NAMESPACE, RESOURCE_NAME, "tenant1");
450+
451+
RelationalFilter relationalFilter =
452+
RelationalFilter.newBuilder()
453+
.setConfigJsonPath("test.path")
454+
.setOperator(RelationalOperator.RELATIONAL_OPERATOR_EQ)
455+
.setValue(Values.of("test-value"))
456+
.build();
457+
458+
Filter filter = Filter.newBuilder().setRelationalFilter(relationalFilter).build();
459+
460+
Pagination pagination = Pagination.getDefaultInstance();
461+
List<org.hypertrace.config.service.v1.SortBy> sortByList = emptyList();
462+
463+
// Create a mock for CloseableIterator<Document>
464+
CloseableIterator<Document> mockIterator = mock(CloseableIterator.class);
465+
when(mockIterator.hasNext()).thenReturn(false); // No documents to process
466+
467+
// Mock the collection.query to return the mock iterator
468+
when(collection.query(any(Query.class), any(QueryOptions.class))).thenReturn(mockIterator);
469+
470+
// Act
471+
configStore.getAllConfigs(configResource, filter, pagination, emptyList());
472+
473+
// Capture the Query object passed to collection.query
474+
ArgumentCaptor<Query> queryCaptor = ArgumentCaptor.forClass(Query.class);
475+
verify(collection).query(queryCaptor.capture(), any(QueryOptions.class));
476+
477+
// Assert the properties of the captured Query
478+
Query capturedQuery = queryCaptor.getValue();
479+
assertNotNull(capturedQuery);
480+
481+
// Verify filter is present in the query
482+
assertNotNull(capturedQuery.getFilter());
483+
484+
// Compare the actual filter with the expected filter
485+
FilterTypeExpression actualFilter =
486+
((LogicalExpression) capturedQuery.getFilter().get()).getOperands().get(1);
487+
FilterTypeExpression expectedFilter = createExpectedFilter();
488+
489+
assertEquals(expectedFilter, actualFilter); // Compare the entire filter expressions
490+
}
491+
492+
private FilterTypeExpression createExpectedFilter() {
493+
// Create the expected left-hand side and right-hand side expressions
494+
IdentifierExpression lhs = IdentifierExpression.of("config.test.path");
495+
ConstantExpression rhs = ConstantExpression.of("test-value");
496+
497+
// Build the relational expression
498+
499+
// Assuming you have a logical expression that contains this relational expression
500+
return RelationalExpression.of(
501+
lhs, org.hypertrace.core.documentstore.expression.operators.RelationalOperator.EQ, rhs);
502+
}
503+
504+
private Document getConfigDocument(
362505
String context, long version, Value config, long creationTimestamp, long updateTimestamp) {
363506
return new ConfigDocument(
364507
RESOURCE_NAME,
@@ -395,41 +538,4 @@ public Document next() {
395538
return documentIterator.next();
396539
}
397540
}
398-
399-
public static class MockDatastore implements Datastore {
400-
401-
@Override
402-
public Set<String> listCollections() {
403-
return Collections.singleton(CONFIGURATIONS_COLLECTION);
404-
}
405-
406-
@Override
407-
public Collection getCollection(String s) {
408-
return collection;
409-
}
410-
411-
// default implementation for other methods as they are unused
412-
@Override
413-
public boolean createCollection(String s, Map<String, String> map) {
414-
return false;
415-
}
416-
417-
@Override
418-
public boolean deleteCollection(String s) {
419-
return false;
420-
}
421-
422-
@Override
423-
public boolean healthCheck() {
424-
return false;
425-
}
426-
427-
@Override
428-
public DocStoreMetricProvider getDocStoreMetricProvider() {
429-
return null;
430-
}
431-
432-
@Override
433-
public void close() {}
434-
}
435541
}

0 commit comments

Comments
 (0)