Skip to content

fix:addresses not_in filter query for array type field - attributes.lables #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kotharironak merged 7 commits into from
Feb 12, 2021
Merged

fix:addresses not_in filter query for array type field - attributes.lables #83

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aec7797
fix:addresses not_in filter query for array type field - attributes.l…
kotharironak Feb 11, 2021
d209ee0
adds integration tests for newly added checks
kotharironak Feb 11, 2021
ed15403
updated doc-store version to get neq filter
kotharironak Feb 11, 2021
ea6bdb7
Adds tests for EQ, IN and NOT_IN filter for attributes.labels
kotharironak Feb 12, 2021
583fb9e
added supports for NEQ for attributes.labels
kotharironak Feb 12, 2021
4a0d161
Removes exists filter as entites w/o having attributes.labels expecte…
kotharironak Feb 12, 2021
3fc02c2
Addressed review comments
kotharironak Feb 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion entity-service-impl/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ plugins {
dependencies {
api(project(":entity-service-api"))
api("org.hypertrace.core.serviceframework:service-framework-spi:0.1.19")
implementation("org.hypertrace.core.documentstore:document-store:0.5.1")
implementation("org.hypertrace.core.documentstore:document-store:0.5.2")
implementation("org.hypertrace.core.grpcutils:grpc-context-utils:0.3.1")
implementation(project(":entity-type-service-rx-client"))

Expand Down
46 changes: 41 additions & 5 deletions entity-service-impl/src/main/java/org/hypertrace/entity/service/util/DocStoreConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ private static Filter transform(AttributeFilter filter) {
return transformToEqFilterWithValueListRhs(filter);
} else if (ATTRIBUTES_LABELS_FIELD_NAME.equals(filter.getName()) && filter.getOperator() == Operator.IN) {
return transformToOrFilterChainForStrArray(filter);
} else if (ATTRIBUTES_LABELS_FIELD_NAME.equals(filter.getName()) && filter.getOperator() == Operator.NOT_IN) {
return transformToAndFilterChainForStrArray(filter);
} else {
return transformNonListRhsFilterTypes(filter);
}
Expand Down Expand Up @@ -183,6 +185,28 @@ private static Filter transformToOrFilterChainForStrArray(AttributeFilter attrib
return f;
}

private static Filter transformToAndFilterChainForStrArray(AttributeFilter attributeFilter) {
String fieldName = attributeFilter.getName() + VALUE_LIST_VALUES_CONST;

Filter f = new Filter();
f.setFieldName("");
f.setOp(Op.AND);

Filter exists = new Filter();
exists.setFieldName(fieldName);
exists.setOp(Op.EXISTS);
Copy link
Contributor

@skjindal93 skjindal93 Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not exactly sure whether we need Exists or Not Exists here. Won't we want to return all the entities which do not have this field?

Here we are doing labels NOT IN [A, B]

  • Do we only want to return entities which has labels but does not have A or B
  • Or, do we also want to return entities where labels does not exist as well?

Copy link

@anandtiwary anandtiwary Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would think #2.

Copy link
Contributor

@skjindal93 skjindal93 Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kotharironak In that case, we need to do "NOT_EXISTS OR the other filter"

Copy link
Contributor Author

@kotharironak kotharironak Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skjindal93 Can you check this integration test - testNotInFilterForMatchingLabelsQuery? It has 5 documents. One doesn't have attributes.labels fields. Is it expected in the result?

Copy link
Contributor

@skjindal93 skjindal93 Feb 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check this integration test - testNotInFilterForMatchingLabelsQuery? It has 5 documents. One doesn't have attributes.labels fields. Is it expected in the result?

Yeah, createdEntity5 is expected in the result

Copy link
Contributor Author

@kotharironak kotharironak Feb 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skjindal93 I have added some more integration tests for NOT_IN, IN, and EQ - testDifferentFilterForMatchingLabelsQuery

Copy link
Contributor

@avinashkolluru avinashkolluru Feb 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes @kotharironak That entity is expected in the result

Copy link
Contributor Author

@kotharironak kotharironak Feb 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes. @avinashkolluru @skjindal93, now it's good to go. Have a look.

exists.setValue(true);

List<Filter> filters = attributeFilter.getAttributeValue().getValueList().getValuesList().stream()
.map(rhsAttributeValue -> createNeqFilterForAttributeValue(fieldName, rhsAttributeValue))
.collect(Collectors.toList());

filters.add(exists);
f.setChildFilters(filters.toArray(new Filter[]{}));

return f;
}

private static Filter transformToEqFilterWithValueListRhs(AttributeFilter attributeFilter) {
String fieldName = attributeFilter.getName() + VALUE_LIST_VALUES_CONST;
return createEqFilterForAttributeValue(fieldName, attributeFilter.getAttributeValue());
Expand All @@ -192,24 +216,36 @@ private static Filter createEqFilterForAttributeValue(String fieldName, Attribut
Filter f = new Filter();
f.setFieldName(fieldName);
f.setOp(Op.EQ);
f.setValue(prepareRhsValueForSpecialValueListCase(attributeValue));
// Set child filters to empty array
f.setChildFilters(new Filter[]{});
return f;
}

private static Filter createNeqFilterForAttributeValue(String fieldName, AttributeValue attributeValue) {
Filter f = new Filter();
f.setFieldName(fieldName);
f.setOp(Op.NEQ);
f.setValue(prepareRhsValueForSpecialValueListCase(attributeValue));
// Set child filters to empty array
f.setChildFilters(new Filter[]{});
return f;
}

private static Object prepareRhsValueForSpecialValueListCase(AttributeValue attributeValue) {
org.hypertrace.entity.data.service.v1.AttributeValue.TypeCase typeCase = attributeValue.getTypeCase();
if (typeCase == TypeCase.VALUE) {
try {
JsonNode mapNode = OBJECT_MAPPER.readTree(JSONFORMAT_PRINTER.print(attributeValue));
Map map = OBJECT_MAPPER.convertValue(mapNode, Map.class);
f.setValue(map);
return map;
} catch (JsonProcessingException | InvalidProtocolBufferException e) {
throw new RuntimeException(e);
}
} else { // For now, just expecting VALUE type on the RHS
throw new UnsupportedOperationException(
String.format("The RHS of filter for string array types can only be VALUE: %s", attributeValue));
}

// Set child filters to empty array
f.setChildFilters(new Filter[]{});
return f;
}

private static void transform(AttributeValue attributeValue, Filter filter,
Expand Down
81 changes: 81 additions & 0 deletions ...-service-impl/src/test/java/org/hypertrace/entity/service/util/DocStoreConverterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,87 @@ public void testStringArrayValueTypeColumnOrChain() throws JsonProcessingExcepti
transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[2].getValue());
}

@Test
public void testStringArrayValueTypeColumnAndChain() throws JsonProcessingException {
Query query = Query.newBuilder()
.addEntityId("some id")
.setFilter(
AttributeFilter.newBuilder().setOperator(Operator.AND)
.addChildFilter(
AttributeFilter.newBuilder()
.setName("attributes.some_col")
.setOperator(Operator.EQ)
.setAttributeValue(AttributeValue.newBuilder()
.setValue(Value.newBuilder().setString("some_val"))
)
)
.addChildFilter(
AttributeFilter.newBuilder()
.setName(ATTRIBUTES_LABELS_FIELD_NAME)
.setOperator(Operator.NOT_IN)
.setAttributeValue(AttributeValue.newBuilder()
.setValueList(
AttributeValueList.newBuilder()
.addValues(
AttributeValue.newBuilder().setValue(Value.newBuilder().setString("l1"))
)
.addValues(
AttributeValue.newBuilder().setValue(Value.newBuilder().setString("l2"))
)
.addValues(
AttributeValue.newBuilder().setValue(Value.newBuilder().setString("l3"))
)
)
)
)
)
.build();
org.hypertrace.core.documentstore.Query transformedQuery =
DocStoreConverter.transform(TENANT_ID, query, Collections.emptyList());

Filter transformedFilter = transformedQuery.getFilter();
Assertions.assertEquals(Filter.Op.AND, transformedFilter.getOp());

Assertions.assertEquals(3, transformedFilter.getChildFilters().length);
Assertions.assertEquals(EntityServiceConstants.ENTITY_ID,
transformedFilter.getChildFilters()[1].getFieldName());
Assertions.assertEquals(Collections.singletonList("some id"),
transformedFilter.getChildFilters()[1].getValue());

Assertions.assertEquals(Op.AND, transformedFilter.getChildFilters()[2].getOp());

Assertions.assertEquals("attributes.some_col.value.string",
transformedFilter.getChildFilters()[2].getChildFilters()[0].getFieldName());
Assertions.assertEquals(Op.EQ, transformedFilter.getChildFilters()[2].getChildFilters()[0].getOp());
Assertions.assertEquals("some_val", transformedFilter.getChildFilters()[2].getChildFilters()[0].getValue());

Assertions.assertEquals(Op.AND, transformedFilter.getChildFilters()[2].getChildFilters()[1].getOp());

Assertions.assertEquals(ATTRIBUTES_LABELS_FIELD_NAME + ".valueList.values",
transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[0].getFieldName());
Assertions.assertEquals(Op.NEQ, transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[0].getOp());
Assertions.assertEquals(OBJECT_MAPPER.convertValue(OBJECT_MAPPER.readTree("{\"value\": {\"string\":\"l1\"}}"), Map.class),
transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[0].getValue());

Assertions.assertEquals(ATTRIBUTES_LABELS_FIELD_NAME + ".valueList.values",
transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[1].getFieldName());
Assertions.assertEquals(Op.NEQ, transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[1].getOp());
Assertions.assertEquals(OBJECT_MAPPER.convertValue(OBJECT_MAPPER.readTree("{\"value\": {\"string\":\"l2\"}}"), Map.class),
transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[1].getValue());

Assertions.assertEquals(ATTRIBUTES_LABELS_FIELD_NAME + ".valueList.values",
transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[2].getFieldName());
Assertions.assertEquals(Op.NEQ, transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[2].getOp());
Assertions.assertEquals(OBJECT_MAPPER.convertValue(OBJECT_MAPPER.readTree("{\"value\": {\"string\":\"l3\"}}"), Map.class),
transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[2].getValue());

Assertions.assertEquals(ATTRIBUTES_LABELS_FIELD_NAME + ".valueList.values",
transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[3].getFieldName());
Assertions.assertEquals(Op.EXISTS, transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[3].getOp());
Assertions.assertEquals(true,
transformedFilter.getChildFilters()[2].getChildFilters()[1].getChildFilters()[3].getValue());
}

@Test
public void testNeqFilterConversionForValueListThrowsException() {
Assertions.assertThrows(UnsupportedOperationException.class, () -> {
Expand Down
2 changes: 1 addition & 1 deletion entity-service/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ dependencies {
implementation("org.hypertrace.core.grpcutils:grpc-server-utils:0.3.1")
implementation("org.hypertrace.core.grpcutils:grpc-client-utils:0.3.1")
implementation("org.hypertrace.core.serviceframework:platform-service-framework:0.1.19")
implementation("org.hypertrace.core.documentstore:document-store:0.5.1")
implementation("org.hypertrace.core.documentstore:document-store:0.5.2")

runtimeOnly("io.grpc:grpc-netty:1.33.1")
runtimeOnly("io.netty:netty-codec-http2:4.1.59.Final")
Expand Down
93 changes: 93 additions & 0 deletions ...src/integrationTest/java/org/hypertrace/entity/service/service/EntityDataServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,99 @@ public void testEntityQueryOrderBy() {

}

@Test
public void testNotInFilterForMatchingLabelsQuery() {
Entity entity = createEntityWithAttributeLabels("Some Service 1",
List.of("v1", "v2", "v3"));
Entity createdEntity1 = entityDataServiceClient.upsert(entity);
assertNotNull(createdEntity1);
assertNotNull(createdEntity1.getEntityId().trim());

entity = createEntityWithAttributeLabels("Some Service 2",
List.of("v01", "v02", "v03"));
Entity createdEntity2 = entityDataServiceClient.upsert(entity);
assertNotNull(createdEntity2);
assertNotNull(createdEntity2.getEntityId().trim());

entity = createEntityWithAttributeLabels("Some Service 3",
List.of("v01", "v2", "v4"));
Entity createdEntity3 = entityDataServiceClient.upsert(entity);
assertNotNull(createdEntity3);
assertNotNull(createdEntity3.getEntityId().trim());

entity = createEntityWithAttributeLabels("Some Service 4",
List.of("b1", "b2", "b3"));
Entity createdEntity4 = entityDataServiceClient.upsert(entity);
assertNotNull(createdEntity4);
assertNotNull(createdEntity4.getEntityId().trim());

entity = createEntityWithOutAttributeLabels("Some Service 5",
"foo", "bar");
Entity createdEntity5 = entityDataServiceClient.upsert(entity);
assertNotNull(createdEntity5);
assertNotNull(createdEntity5.getEntityId().trim());

AttributeFilter attributeFilter = AttributeFilter.newBuilder()
.setName("attributes.labels")
.setOperator(Operator.NOT_IN)
.setAttributeValue(AttributeValue.newBuilder()
.setValueList(
AttributeValueList.newBuilder()
.addValues(
AttributeValue.newBuilder().setValue(Value.newBuilder().setString("b1"))
)
.addValues(
AttributeValue.newBuilder().setValue(Value.newBuilder().setString("v4"))
)
)
).build();

Query query = Query.newBuilder()
.setEntityType(EntityType.K8S_POD.name())
.setFilter(attributeFilter).build();
List<Entity> entitiesList = entityDataServiceClient.query(TENANT_ID, query);
assertTrue(entitiesList.size() == 2);
assertTrue(entitiesList.contains(createdEntity1) && entitiesList.contains(createdEntity2));
}

private Entity createEntityWithAttributeLabels(String entityName, List<String> labels) {
AttributeValueList.Builder listBuilder = AttributeValueList.newBuilder();
labels.forEach(label -> listBuilder.addValues(
AttributeValue.newBuilder().setValue(Value.newBuilder().setString(label)))
);
AttributeValue value = AttributeValue.newBuilder().setValueList(listBuilder.build()).build();

Entity entity = Entity.newBuilder()
.setTenantId(TENANT_ID)
.setEntityType(EntityType.K8S_POD.name())
.setEntityName(entityName)
.putIdentifyingAttributes(
EntityConstants.getValue(CommonAttribute.COMMON_ATTRIBUTE_EXTERNAL_ID),
generateRandomUUIDAttrValue())
.putAttributes(
"labels", value)
.build();

return entity;
}

private Entity createEntityWithOutAttributeLabels(String entityName, String attrName, String attrValue) {
Entity entity = Entity.newBuilder()
.setTenantId(TENANT_ID)
.setEntityType(EntityType.K8S_POD.name())
.setEntityName(entityName)
.putIdentifyingAttributes(
EntityConstants.getValue(CommonAttribute.COMMON_ATTRIBUTE_EXTERNAL_ID),
generateRandomUUIDAttrValue())
.putAttributes(
attrName,
AttributeValue.newBuilder()
.setValue(Value.newBuilder().setString(attrValue).build())
.build())
.build();
return entity;
}

private AttributeValue generateRandomUUIDAttrValue() {
return AttributeValue.newBuilder()
.setValue(Value.newBuilder()
Expand Down