Skip to content

Commit

Permalink
ENG-48723: Filter builder fix and object store changes (#230)
Browse files Browse the repository at this point in the history 
* filter builder fix and store changes

* nit

* revert-change

* refactor object building

* review comment
  • Loading branch information
@harshit-kumar-v2
harshit-kumar-v2 authored Aug 24, 2024
1 parent 20f405e commit e9bad2a
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 43 deletions.
1 change: 1 addition & 0 deletions config-service-impl/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ dependencies {
api(projects.configServiceApi)

implementation(projects.configServiceChangeEventGenerator)
implementation(projects.configProtoConverter)

implementation(commonLibs.jackson.databind)
implementation(commonLibs.guava)
Expand Down
45 changes: 24 additions & 21 deletions config-service-impl/src/main/java/org/hypertrace/config/service/store/FilterBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,22 @@

import static org.hypertrace.config.service.store.ConfigDocument.CONFIG_FIELD_NAME;

import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.protobuf.Value;
import io.grpc.Status;
import lombok.SneakyThrows;
import org.hypertrace.config.proto.converter.ConfigProtoConverter;
import org.hypertrace.config.service.v1.LogicalFilter;
import org.hypertrace.config.service.v1.RelationalFilter;
import org.hypertrace.core.documentstore.Filter;

class FilterBuilder {
private final ObjectMapper objectMapper;

FilterBuilder() {
this.objectMapper = new ObjectMapper();
}

public Filter buildDocStoreFilter(org.hypertrace.config.service.v1.Filter filter) {
switch (filter.getTypeCase()) {
Expand Down Expand Up @@ -54,47 +64,34 @@ private Filter evaluateCompositeExpression(LogicalFilter logicalFilter) {
}

private Filter evaluateLeafExpression(RelationalFilter relationalFilter) {
Object value = this.convertValueToObject(relationalFilter.getValue());
switch (relationalFilter.getOperator()) {
case RELATIONAL_OPERATOR_EQ:
return new Filter(
Filter.Op.EQ,
buildConfigFieldPath(relationalFilter.getConfigJsonPath()),
relationalFilter.getValue());
Filter.Op.EQ, buildConfigFieldPath(relationalFilter.getConfigJsonPath()), value);
case RELATIONAL_OPERATOR_NEQ:
return new Filter(
Filter.Op.NEQ,
buildConfigFieldPath(relationalFilter.getConfigJsonPath()),
relationalFilter.getValue());
case RELATIONAL_OPERATOR_IN:
return new Filter(
Filter.Op.IN,
buildConfigFieldPath(relationalFilter.getConfigJsonPath()),
relationalFilter.getValue());
Filter.Op.IN, buildConfigFieldPath(relationalFilter.getConfigJsonPath()), value);
case RELATIONAL_OPERATOR_NOT_IN:
return new Filter(
Filter.Op.NOT_IN,
buildConfigFieldPath(relationalFilter.getConfigJsonPath()),
relationalFilter.getValue());
Filter.Op.NOT_IN, buildConfigFieldPath(relationalFilter.getConfigJsonPath()), value);
case RELATIONAL_OPERATOR_LT:
return new Filter(
Filter.Op.LT,
buildConfigFieldPath(relationalFilter.getConfigJsonPath()),
relationalFilter.getValue());
Filter.Op.LT, buildConfigFieldPath(relationalFilter.getConfigJsonPath()), value);
case RELATIONAL_OPERATOR_GT:
return new Filter(
Filter.Op.GT,
buildConfigFieldPath(relationalFilter.getConfigJsonPath()),
relationalFilter.getValue());
Filter.Op.GT, buildConfigFieldPath(relationalFilter.getConfigJsonPath()), value);
case RELATIONAL_OPERATOR_LTE:
return new Filter(
Filter.Op.LTE,
buildConfigFieldPath(relationalFilter.getConfigJsonPath()),
relationalFilter.getValue());
Filter.Op.LTE, buildConfigFieldPath(relationalFilter.getConfigJsonPath()), value);
case RELATIONAL_OPERATOR_GTE:
return new Filter(
Filter.Op.GTE,
buildConfigFieldPath(relationalFilter.getConfigJsonPath()),
relationalFilter.getValue());
Filter.Op.GTE, buildConfigFieldPath(relationalFilter.getConfigJsonPath()), value);
case UNRECOGNIZED:
default:
throw Status.INVALID_ARGUMENT
Expand All @@ -106,4 +103,10 @@ private Filter evaluateLeafExpression(RelationalFilter relationalFilter) {
private String buildConfigFieldPath(String configJsonPath) {
return String.format("%s.%s", CONFIG_FIELD_NAME, configJsonPath);
}

@SneakyThrows
private Object convertValueToObject(Value value) {
return objectMapper.readValue(
ConfigProtoConverter.convertToJsonString(value), new TypeReference<>() {});
}
}
20 changes: 9 additions & 11 deletions config-service-impl/src/test/java/org/hypertrace/config/service/store/FilterBuilderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ void buildDocStoreFilter1() {
RelationalFilter.newBuilder()
.setConfigJsonPath("key1")
.setOperator(RelationalOperator.RELATIONAL_OPERATOR_EQ)
.setValue(Value.newBuilder().setStringValue("value1")))
.setValue(Values.of("value1")))
.build();

org.hypertrace.core.documentstore.Filter docFilter =
org.hypertrace.core.documentstore.Filter.eq("config.key1", Values.of("value1"));
org.hypertrace.core.documentstore.Filter.eq("config.key1", "value1");
assertEquals(docFilter.toString(), filterBuilder.buildDocStoreFilter(filter).toString());

Filter filter2 =
Expand All @@ -44,11 +44,11 @@ void buildDocStoreFilter1() {
RelationalFilter.newBuilder()
.setConfigJsonPath("key2")
.setOperator(RelationalOperator.RELATIONAL_OPERATOR_EQ)
.setValue(Value.newBuilder().setNumberValue(300)))
.setValue(Values.of(300)))
.build();

org.hypertrace.core.documentstore.Filter docFilter2 =
org.hypertrace.core.documentstore.Filter.eq("config.key2", Values.of(300));
org.hypertrace.core.documentstore.Filter.eq("config.key2", 300.0);
assertEquals(docFilter2.toString(), filterBuilder.buildDocStoreFilter(filter2).toString());
}

Expand Down Expand Up @@ -76,12 +76,10 @@ void buildDocStoreFilter2() {
.build();

org.hypertrace.core.documentstore.Filter docFilter =
org.hypertrace.core.documentstore.Filter.eq("config.key3", Values.of(true))
org.hypertrace.core.documentstore.Filter.eq("config.key3", true)
.and(
new org.hypertrace.core.documentstore.Filter(
org.hypertrace.core.documentstore.Filter.Op.LTE,
"config.key4",
Values.of(100)));
org.hypertrace.core.documentstore.Filter.Op.LTE, "config.key4", 100.0));
assertEquals(docFilter.toString(), filterBuilder.buildDocStoreFilter(filter).toString());
}

Expand Down Expand Up @@ -133,11 +131,11 @@ void buildDocStoreFilter3() {
new org.hypertrace.core.documentstore.Filter(
org.hypertrace.core.documentstore.Filter.Op.IN,
"config.key5",
Values.of(List.of(Values.of("listValue1"), Values.of("listValue2"))))
List.of("listValue1", "listValue2"))
.and(
new org.hypertrace.core.documentstore.Filter(
org.hypertrace.core.documentstore.Filter.Op.GTE, "config.key6", Values.of(100)))
.or(org.hypertrace.core.documentstore.Filter.eq("config.key7", Values.of("value7")));
org.hypertrace.core.documentstore.Filter.Op.GTE, "config.key6", 100.0))
.or(org.hypertrace.core.documentstore.Filter.eq("config.key7", "value7"));

assertEquals(docFilter.toString(), filterBuilder.buildDocStoreFilter(filter).toString());
}
Expand Down
38 changes: 27 additions & 11 deletions object-store/src/main/java/org/hypertrace/config/objectstore/IdentifiedObjectStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.hypertrace.config.service.v1.DeleteConfigRequest;
import org.hypertrace.config.service.v1.DeleteConfigsRequest;
import org.hypertrace.config.service.v1.DeleteConfigsRequest.ConfigToDelete;
import org.hypertrace.config.service.v1.Filter;
import org.hypertrace.config.service.v1.GetAllConfigsRequest;
import org.hypertrace.config.service.v1.GetConfigRequest;
import org.hypertrace.config.service.v1.GetConfigResponse;
Expand Down Expand Up @@ -153,24 +154,39 @@ public Optional<T> getData(RequestContext context, String id) {
return getObject(context, id).map(ConfigObject::getData);
}

public ContextualConfigObject<T> upsertObject(RequestContext context, T data) {
private ContextualConfigObject<T> upsertObject(
RequestContext context, UpsertConfigRequest request) {
UpsertConfigResponse response =
context.call(
() ->
this.configServiceBlockingStub
.withDeadline(getDeadline())
.upsertConfig(
UpsertConfigRequest.newBuilder()
.setResourceName(this.resourceName)
.setResourceNamespace(this.resourceNamespace)
.setContext(this.getContextFromData(data))
.setConfig(this.buildValueFromData(data))
.build()));
() -> this.configServiceBlockingStub.withDeadline(getDeadline()).upsertConfig(request));

return this.processUpsertResult(context, response)
.orElseThrow(Status.INTERNAL::asRuntimeException);
}

public ContextualConfigObject<T> upsertObject(RequestContext context, T data) {
return this.upsertObject(
context,
UpsertConfigRequest.newBuilder()
.setResourceName(this.resourceName)
.setResourceNamespace(this.resourceNamespace)
.setContext(this.getContextFromData(data))
.setConfig(this.buildValueFromData(data))
.build());
}

protected ContextualConfigObject<T> upsertObject(RequestContext context, T data, Filter filter) {
return this.upsertObject(
context,
UpsertConfigRequest.newBuilder()
.setResourceName(this.resourceName)
.setResourceNamespace(this.resourceNamespace)
.setContext(this.getContextFromData(data))
.setConfig(this.buildValueFromData(data))
.setUpsertCondition(filter)
.build());
}

public Optional<DeletedContextualConfigObject<T>> deleteObject(
RequestContext requestContext, String context) {
try {
Expand Down

0 comments on commit e9bad2a

Please sign in to comment.