From 31dddca80ac94d5ff16b8bb93cb4eb3c6428a3f2 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Thu, 16 Sep 2021 17:11:25 +0530 Subject: [PATCH 1/8] ENG-11241: Changes to fetch labels as part of entity joiner 1. Updated entity joiner request builder to fetch labels 2. No change needed in NeighborEntitiesRequestBuilder as it is called from DefaultEntityRequestBuilder which handles labels fetching part 3. Minor refactoring --- .../entity/dao/GatewayServiceEntityDao.java | 23 +++--- .../joiner/DefaultEntityJoinerBuilder.java | 73 +++++++++++++++---- .../DefaultEntityJoinerBuilderTest.java | 13 ++++ .../joiner/DefaultLabelJoinerBuilder.java | 17 +++-- 4 files changed, 93 insertions(+), 33 deletions(-) diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/dao/GatewayServiceEntityDao.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/dao/GatewayServiceEntityDao.java index 2cca0dd1..dba07312 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/dao/GatewayServiceEntityDao.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/dao/GatewayServiceEntityDao.java @@ -6,6 +6,7 @@ import io.reactivex.rxjava3.core.Scheduler; import io.reactivex.rxjava3.core.Single; import java.util.Collections; +import java.util.List; import java.util.Map; import javax.inject.Inject; import javax.inject.Singleton; @@ -120,15 +121,17 @@ private Single> buildLabelResultSetMap( } private LabelJoiner.LabelIdGetter getEntityLabelsGetter(EntityRequest request) { - return entity -> { - Value labelAttributeValue = - entity.getAttributeOrDefault( - request.labelRequest().get().labelIdArrayAttributeRequest().attribute().id(), null); - if (labelAttributeValue == null) { - log.warn("Unable to fetch labels attribute for entity with id {}", entity.getId()); - return Single.just(Collections.emptyList()); - } - return Single.just(labelAttributeValue.getStringArrayList()); - }; + return entity -> Single.just(getLabelAttributeValue(request, entity)); + } + + private List getLabelAttributeValue(EntityRequest request, Entity entity) { + Value labelAttributeValue = + entity.getAttributeOrDefault( + request.labelRequest().get().labelIdArrayAttributeRequest().attribute().id(), null); + if (labelAttributeValue == null) { + log.warn("Unable to fetch labels attribute for entity with id {}", entity.getId()); + return Collections.emptyList(); + } + return labelAttributeValue.getStringArrayList(); } } diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java index 42021fed..7561f840 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java @@ -3,6 +3,7 @@ import static com.google.common.collect.ImmutableList.copyOf; import static com.google.common.collect.Iterables.concat; import static com.google.common.collect.Tables.immutableCell; +import static io.reactivex.rxjava3.core.Single.zip; import static java.util.Objects.requireNonNull; import com.google.common.collect.ImmutableSetMultimap; @@ -35,6 +36,7 @@ import lombok.extern.slf4j.Slf4j; import org.hypertrace.core.graphql.common.request.AttributeAssociation; import org.hypertrace.core.graphql.common.request.AttributeRequest; +import org.hypertrace.core.graphql.common.request.AttributeRequestBuilder; import org.hypertrace.core.graphql.common.request.FilterRequestBuilder; import org.hypertrace.core.graphql.common.request.ResultSetRequest; import org.hypertrace.core.graphql.common.request.ResultSetRequestBuilder; @@ -62,7 +64,7 @@ @Slf4j class DefaultEntityJoinerBuilder implements EntityJoinerBuilder { - + private static final String LABELS_KEY_NAME = "labels"; private static final int ZERO_OFFSET = 0; private final EntityDao entityDao; @@ -70,6 +72,7 @@ class DefaultEntityJoinerBuilder implements EntityJoinerBuilder { private final ArgumentDeserializer argumentDeserializer; private final ResultSetRequestBuilder resultSetRequestBuilder; private final FilterRequestBuilder filterRequestBuilder; + private final AttributeRequestBuilder attributeRequestBuilder; private final Scheduler boundedIoScheduler; @Inject @@ -79,6 +82,7 @@ class DefaultEntityJoinerBuilder implements EntityJoinerBuilder { ArgumentDeserializer argumentDeserializer, ResultSetRequestBuilder resultSetRequestBuilder, FilterRequestBuilder filterRequestBuilder, + AttributeRequestBuilder attributeRequestBuilder, @BoundedIoScheduler Scheduler boundedIoScheduler) { this.entityDao = entityDao; @@ -86,6 +90,7 @@ class DefaultEntityJoinerBuilder implements EntityJoinerBuilder { this.argumentDeserializer = argumentDeserializer; this.resultSetRequestBuilder = resultSetRequestBuilder; this.filterRequestBuilder = filterRequestBuilder; + this.attributeRequestBuilder = attributeRequestBuilder; this.boundedIoScheduler = boundedIoScheduler; } @@ -241,19 +246,34 @@ Single buildEntityRequestForType( return buildIdFilter(context, entityType, entityIdsToFilter) .flatMap( filterArguments -> - resultSetRequestBuilder.build( - context, - entityType, - entityIdsToFilter.size(), - ZERO_OFFSET, - new InstantTimeRange(), - List - .> - of(), // Order does not matter - filterArguments, - this.entityFieldsByType.get(entityType).stream(), - Optional.empty())) - .map(resultSetRequest -> new DefaultEntityRequest(entityType, resultSetRequest)); + buildEntityRequest( + context, entityType, entityIdsToFilter.size(), filterArguments)); + } + + private Single buildEntityRequest( + GraphQlRequestContext context, + String entityType, + int entityIdsToFilterSize, + List> filterArguments) { + return zip( + resultSetRequestBuilder.build( + context, + entityType, + entityIdsToFilterSize, + ZERO_OFFSET, + new InstantTimeRange(), + List.>of(), // Order does not matter + filterArguments, + this.entityFieldsByType.get(entityType).stream(), + Optional.empty()), + attributeRequestBuilder.buildForKey(context, entityType, LABELS_KEY_NAME), + (resultSetRequest, labelsAttributeRequest) -> + new DefaultEntityRequest( + entityType, + resultSetRequest, + canFetchLabels(entityType) + ? Optional.of(new DefaultLabelRequest(labelsAttributeRequest)) + : Optional.empty())); } private Single>> buildIdFilter( @@ -261,6 +281,23 @@ private Single>> buildIdFilter( return filterRequestBuilder.build( context, entityScope, Set.of(new EntityIdFilter(entityIds, entityScope))); } + + private boolean canFetchLabels(String entityType) { + return this.entityFieldsByType.get(entityType).stream() + .map(SelectedField::getSelectionSet) + .filter( + selectionSet -> + selectionFinder + .findSelections( + selectionSet, + SelectionQuery.builder() + .selectionPath(List.of(Entity.LABELS_KEY)) + .build()) + .count() + > 0) + .findAny() + .isPresent(); + } } @Value @@ -273,7 +310,7 @@ private static class DefaultEntityRequest implements EntityRequest { EdgeSetGroupRequest incomingEdgeRequests = new EmptyEdgeSetGroupRequest(); EdgeSetGroupRequest outgoingEdgeRequests = new EmptyEdgeSetGroupRequest(); boolean includeInactive = true; // When joining we want the entity regardless of time range - Optional labelRequest = Optional.empty(); + Optional labelRequest; } @Value @@ -312,4 +349,10 @@ private static class EntityIdFilter implements FilterArgument { AttributeScope idType = null; String idScope; } + + @Value + @Accessors(fluent = true) + private static class DefaultLabelRequest implements LabelRequest { + AttributeRequest labelIdArrayAttributeRequest; + } } diff --git a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java index 4991ff71..c06830d6 100644 --- a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java +++ b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -25,6 +26,8 @@ import lombok.Value; import lombok.experimental.Accessors; import org.hypertrace.core.graphql.common.request.AttributeAssociation; +import org.hypertrace.core.graphql.common.request.AttributeRequest; +import org.hypertrace.core.graphql.common.request.AttributeRequestBuilder; import org.hypertrace.core.graphql.common.request.FilterRequestBuilder; import org.hypertrace.core.graphql.common.request.ResultSetRequest; import org.hypertrace.core.graphql.common.request.ResultSetRequestBuilder; @@ -60,10 +63,12 @@ class DefaultEntityJoinerBuilderTest { @Mock ArgumentDeserializer mockDeserializer; @Mock ResultSetRequestBuilder mockResultSetRequestBuilder; @Mock FilterRequestBuilder mockFilterRequestBuilder; + @Mock AttributeRequestBuilder attributeRequestBuilder; @Mock GraphQlRequestContext mockRequestContext; @Mock DataFetchingFieldSelectionSet mockSelectionSet; @Mock AttributeAssociation mockFilter; @Mock ResultSetRequest mockResultSetRequest; + @Mock AttributeRequest mockAttributeRequest; Scheduler testScheduler = Schedulers.single(); @@ -78,6 +83,7 @@ void beforeEach() { mockDeserializer, mockResultSetRequestBuilder, mockFilterRequestBuilder, + attributeRequestBuilder, testScheduler); } @@ -118,6 +124,7 @@ void fetchesEntitiesOfMultipleTypesInBatches() { List.of(firstTypeFirstIdEntity, firstTypeSecondIdEntity), SECOND_ENTITY_TYPE, List.of(secondTypeFirstIdEntity, secondTypeSecondIdEntity))); + mockLabelAttributeRequest(); EntityJoiner joiner = this.entityJoinerBuilder @@ -216,6 +223,12 @@ private void mockResult(Map> resultsByEntityType) { }); } + private void mockLabelAttributeRequest() { + doReturn(Single.just(mockAttributeRequest)) + .when(attributeRequestBuilder) + .buildForKey(any(), any(), any()); + } + @Value private static class TestJoinSource { String firstEntityId; diff --git a/hypertrace-graphql-labels-schema/src/main/java/org/hypertrace/graphql/label/joiner/DefaultLabelJoinerBuilder.java b/hypertrace-graphql-labels-schema/src/main/java/org/hypertrace/graphql/label/joiner/DefaultLabelJoinerBuilder.java index c3f7049d..9c8ba845 100644 --- a/hypertrace-graphql-labels-schema/src/main/java/org/hypertrace/graphql/label/joiner/DefaultLabelJoinerBuilder.java +++ b/hypertrace-graphql-labels-schema/src/main/java/org/hypertrace/graphql/label/joiner/DefaultLabelJoinerBuilder.java @@ -79,16 +79,17 @@ private Single> buildLabelResultSetMapEntry( private Single> filterLabels(List labelIds, Map labelMap) { return Single.just( labelIds.stream() - .map( - labelId -> { - Label label = labelMap.get(labelId); - if (label == null) { - log.warn("Label config doesn't exist for label id {}", labelId); - } - return label; - }) + .map(labelId -> getLabelFromMap(labelId, labelMap)) .filter(Objects::nonNull) .collect(Collectors.toUnmodifiableList())); } + + private Label getLabelFromMap(String labelId, Map labelMap) { + Label label = labelMap.get(labelId); + if (label == null) { + log.warn("Label config doesn't exist for label id {}", labelId); + } + return label; + } } } From 6367bf23c8004a663b82a76844f5c3fb32587099 Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Thu, 16 Sep 2021 10:46:53 -0400 Subject: [PATCH 2/8] refactor: separate label request builder --- .../joiner/DefaultEntityJoinerBuilder.java | 45 +++-------- .../DefaultEntityLabelRequestBuilder.java | 79 +++++++++++++++++++ .../request/DefaultEntityRequestBuilder.java | 39 ++------- ...elRequest.java => EntityLabelRequest.java} | 2 +- .../request/EntityLabelRequestBuilder.java | 17 ++++ .../graphql/entity/request/EntityRequest.java | 2 +- .../entity/request/EntityRequestModule.java | 1 + .../NeighborEntitiesRequestBuilder.java | 2 +- .../DefaultEntityJoinerBuilderTest.java | 18 ++--- 9 files changed, 126 insertions(+), 79 deletions(-) create mode 100644 hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java rename hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/{LabelRequest.java => EntityLabelRequest.java} (81%) create mode 100644 hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityLabelRequestBuilder.java diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java index 7561f840..f3a37294 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java @@ -52,8 +52,9 @@ import org.hypertrace.core.graphql.utils.schema.SelectionQuery; import org.hypertrace.graphql.entity.dao.EntityDao; import org.hypertrace.graphql.entity.request.EdgeSetGroupRequest; +import org.hypertrace.graphql.entity.request.EntityLabelRequest; +import org.hypertrace.graphql.entity.request.EntityLabelRequestBuilder; import org.hypertrace.graphql.entity.request.EntityRequest; -import org.hypertrace.graphql.entity.request.LabelRequest; import org.hypertrace.graphql.entity.schema.Entity; import org.hypertrace.graphql.entity.schema.EntityJoinable; import org.hypertrace.graphql.entity.schema.EntityResultSet; @@ -74,6 +75,7 @@ class DefaultEntityJoinerBuilder implements EntityJoinerBuilder { private final FilterRequestBuilder filterRequestBuilder; private final AttributeRequestBuilder attributeRequestBuilder; private final Scheduler boundedIoScheduler; + private final EntityLabelRequestBuilder entityLabelRequestBuilder; @Inject DefaultEntityJoinerBuilder( @@ -83,7 +85,8 @@ class DefaultEntityJoinerBuilder implements EntityJoinerBuilder { ResultSetRequestBuilder resultSetRequestBuilder, FilterRequestBuilder filterRequestBuilder, AttributeRequestBuilder attributeRequestBuilder, - @BoundedIoScheduler Scheduler boundedIoScheduler) { + @BoundedIoScheduler Scheduler boundedIoScheduler, + EntityLabelRequestBuilder entityLabelRequestBuilder) { this.entityDao = entityDao; this.selectionFinder = selectionFinder; @@ -92,6 +95,7 @@ class DefaultEntityJoinerBuilder implements EntityJoinerBuilder { this.filterRequestBuilder = filterRequestBuilder; this.attributeRequestBuilder = attributeRequestBuilder; this.boundedIoScheduler = boundedIoScheduler; + this.entityLabelRequestBuilder = entityLabelRequestBuilder; } @Override @@ -266,14 +270,10 @@ private Single buildEntityRequest( filterArguments, this.entityFieldsByType.get(entityType).stream(), Optional.empty()), - attributeRequestBuilder.buildForKey(context, entityType, LABELS_KEY_NAME), - (resultSetRequest, labelsAttributeRequest) -> - new DefaultEntityRequest( - entityType, - resultSetRequest, - canFetchLabels(entityType) - ? Optional.of(new DefaultLabelRequest(labelsAttributeRequest)) - : Optional.empty())); + entityLabelRequestBuilder.buildLabelRequestIfPresentInAnyEntity( + context, entityType, this.entityFieldsByType.get(entityType)), + (resultSetRequest, optionalLabelRequest) -> + new DefaultEntityRequest(entityType, resultSetRequest, optionalLabelRequest)); } private Single>> buildIdFilter( @@ -281,23 +281,6 @@ private Single>> buildIdFilter( return filterRequestBuilder.build( context, entityScope, Set.of(new EntityIdFilter(entityIds, entityScope))); } - - private boolean canFetchLabels(String entityType) { - return this.entityFieldsByType.get(entityType).stream() - .map(SelectedField::getSelectionSet) - .filter( - selectionSet -> - selectionFinder - .findSelections( - selectionSet, - SelectionQuery.builder() - .selectionPath(List.of(Entity.LABELS_KEY)) - .build()) - .count() - > 0) - .findAny() - .isPresent(); - } } @Value @@ -310,7 +293,7 @@ private static class DefaultEntityRequest implements EntityRequest { EdgeSetGroupRequest incomingEdgeRequests = new EmptyEdgeSetGroupRequest(); EdgeSetGroupRequest outgoingEdgeRequests = new EmptyEdgeSetGroupRequest(); boolean includeInactive = true; // When joining we want the entity regardless of time range - Optional labelRequest; + Optional labelRequest; } @Value @@ -349,10 +332,4 @@ private static class EntityIdFilter implements FilterArgument { AttributeScope idType = null; String idScope; } - - @Value - @Accessors(fluent = true) - private static class DefaultLabelRequest implements LabelRequest { - AttributeRequest labelIdArrayAttributeRequest; - } } diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java new file mode 100644 index 00000000..a8bd21fb --- /dev/null +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java @@ -0,0 +1,79 @@ +package org.hypertrace.graphql.entity.request; + +import graphql.schema.DataFetchingFieldSelectionSet; +import graphql.schema.SelectedField; +import io.reactivex.rxjava3.core.Single; +import java.util.Collection; +import java.util.List; +import java.util.Optional; +import javax.inject.Inject; +import lombok.Value; +import lombok.experimental.Accessors; +import org.hypertrace.core.graphql.common.request.AttributeRequest; +import org.hypertrace.core.graphql.common.request.AttributeRequestBuilder; +import org.hypertrace.core.graphql.common.schema.results.ResultSet; +import org.hypertrace.core.graphql.context.GraphQlRequestContext; +import org.hypertrace.core.graphql.utils.schema.GraphQlSelectionFinder; +import org.hypertrace.core.graphql.utils.schema.SelectionQuery; +import org.hypertrace.graphql.entity.schema.Entity; + +public class DefaultEntityLabelRequestBuilder implements EntityLabelRequestBuilder { + + private static final String LABELS_ATTRIBUTE_KEY = "labels"; + + private static final SelectionQuery RESULT_SET_LABEL_FIELD_QUERY = + SelectionQuery.builder() + .selectionPath(List.of(ResultSet.RESULT_SET_RESULTS_NAME, Entity.LABELS_KEY)) + .build(); + + private static final SelectionQuery LABEL_FIELD_QUERY = + SelectionQuery.namedChild(Entity.LABELS_KEY); + + private final AttributeRequestBuilder attributeRequestBuilder; + private final GraphQlSelectionFinder selectionFinder; + + @Inject + DefaultEntityLabelRequestBuilder( + AttributeRequestBuilder attributeRequestBuilder, GraphQlSelectionFinder selectionFinder) { + this.attributeRequestBuilder = attributeRequestBuilder; + this.selectionFinder = selectionFinder; + } + + @Override + public Single> buildLabelRequestIfPresentInResultSet( + GraphQlRequestContext context, String scope, DataFetchingFieldSelectionSet selectionSet) { + if (isLabelFieldRequested(selectionSet, RESULT_SET_LABEL_FIELD_QUERY)) { + return buildRequest(context, scope); + } + return Single.just(Optional.empty()); + } + + @Override + public Single> buildLabelRequestIfPresentInAnyEntity( + GraphQlRequestContext context, String scope, Collection entityFields) { + if (entityFields.stream() + .anyMatch(field -> isLabelFieldRequested(field.getSelectionSet(), LABEL_FIELD_QUERY))) { + return Single.just(Optional.empty()); + } + return buildRequest(context, scope); + } + + private boolean isLabelFieldRequested( + DataFetchingFieldSelectionSet selectionSet, SelectionQuery labelFieldQuery) { + return this.selectionFinder.findSelections(selectionSet, labelFieldQuery).findAny().isPresent(); + } + + private Single> buildRequest( + GraphQlRequestContext context, String scope) { + return this.attributeRequestBuilder + .buildForKey(context, scope, LABELS_ATTRIBUTE_KEY) + .map(DefaultLabelRequest::new) + .map(Optional::of); + } + + @Value + @Accessors(fluent = true) + private static class DefaultLabelRequest implements EntityLabelRequest { + AttributeRequest labelIdArrayAttributeRequest; + } +} diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityRequestBuilder.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityRequestBuilder.java index d9c74376..39a9e154 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityRequestBuilder.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityRequestBuilder.java @@ -14,8 +14,6 @@ import javax.inject.Inject; import lombok.Value; import lombok.experimental.Accessors; -import org.hypertrace.core.graphql.common.request.AttributeRequest; -import org.hypertrace.core.graphql.common.request.AttributeRequestBuilder; import org.hypertrace.core.graphql.common.request.ResultSetRequest; import org.hypertrace.core.graphql.common.request.ResultSetRequestBuilder; import org.hypertrace.core.graphql.common.schema.arguments.TimeRangeArgument; @@ -25,7 +23,6 @@ import org.hypertrace.core.graphql.deserialization.ArgumentDeserializer; import org.hypertrace.core.graphql.utils.schema.GraphQlSelectionFinder; import org.hypertrace.core.graphql.utils.schema.SelectionQuery; -import org.hypertrace.graphql.entity.schema.Entity; import org.hypertrace.graphql.entity.schema.EntityType; import org.hypertrace.graphql.entity.schema.argument.EntityScopeArgument; import org.hypertrace.graphql.entity.schema.argument.EntityTypeArgument; @@ -36,14 +33,12 @@ class DefaultEntityRequestBuilder implements EntityRequestBuilder { - private static final String LABELS_KEY_NAME = "labels"; - private final ResultSetRequestBuilder resultSetRequestBuilder; private final MetricRequestBuilder metricRequestBuilder; private final ArgumentDeserializer argumentDeserializer; private final GraphQlSelectionFinder selectionFinder; private final EdgeRequestBuilder edgeRequestBuilder; - private final AttributeRequestBuilder attributeRequestBuilder; + private final EntityLabelRequestBuilder entityLabelRequestBuilder; @Inject DefaultEntityRequestBuilder( @@ -52,13 +47,13 @@ class DefaultEntityRequestBuilder implements EntityRequestBuilder { ArgumentDeserializer argumentDeserializer, GraphQlSelectionFinder selectionFinder, EdgeRequestBuilder edgeRequestBuilder, - AttributeRequestBuilder attributeRequestBuilder) { + EntityLabelRequestBuilder entityLabelRequestBuilder) { this.resultSetRequestBuilder = resultSetRequestBuilder; this.metricRequestBuilder = metricRequestBuilder; this.argumentDeserializer = argumentDeserializer; this.selectionFinder = selectionFinder; this.edgeRequestBuilder = edgeRequestBuilder; - this.attributeRequestBuilder = attributeRequestBuilder; + this.entityLabelRequestBuilder = entityLabelRequestBuilder; } @Override @@ -96,17 +91,6 @@ private Single build( .count() > 0; - boolean canFetchLabels = - this.selectionFinder - .findSelections( - selectionSet, - SelectionQuery.builder() - .selectionPath( - List.of(ResultSet.RESULT_SET_RESULTS_NAME, Entity.LABELS_KEY)) - .build()) - .count() - > 0; - return zip( this.resultSetRequestBuilder.build( context, scope, arguments, selectionSet, AggregatableOrderArgument.class), @@ -121,12 +105,13 @@ private Single build( this.timeRange(arguments), this.space(arguments), this.getOutgoingEdges(selectionSet)), - attributeRequestBuilder.buildForKey(context, scope, LABELS_KEY_NAME), + this.entityLabelRequestBuilder.buildLabelRequestIfPresentInResultSet( + context, scope, selectionSet), (resultSetRequest, metricRequestList, incomingEdges, outgoingEdges, - labelsAttributeRequest) -> + optionalLabelsAttributeRequest) -> new DefaultEntityRequest( scope, resultSetRequest, @@ -135,9 +120,7 @@ private Single build( outgoingEdges, includeInactive, fetchTotal, - canFetchLabels - ? Optional.of(new DefaultLabelRequest(labelsAttributeRequest)) - : Optional.empty())); + optionalLabelsAttributeRequest)); } private Stream getResultSets(DataFetchingFieldSelectionSet selectionSet) { @@ -181,12 +164,6 @@ private static class DefaultEntityRequest implements EntityRequest { EdgeSetGroupRequest outgoingEdgeRequests; boolean includeInactive; boolean fetchTotal; - Optional labelRequest; - } - - @Value - @Accessors(fluent = true) - private static class DefaultLabelRequest implements LabelRequest { - AttributeRequest labelIdArrayAttributeRequest; + Optional labelRequest; } } diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/LabelRequest.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityLabelRequest.java similarity index 81% rename from hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/LabelRequest.java rename to hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityLabelRequest.java index 641e1bf1..9da61a18 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/LabelRequest.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityLabelRequest.java @@ -2,7 +2,7 @@ import org.hypertrace.core.graphql.common.request.AttributeRequest; -public interface LabelRequest { +public interface EntityLabelRequest { AttributeRequest labelIdArrayAttributeRequest(); } diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityLabelRequestBuilder.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityLabelRequestBuilder.java new file mode 100644 index 00000000..dcd28785 --- /dev/null +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityLabelRequestBuilder.java @@ -0,0 +1,17 @@ +package org.hypertrace.graphql.entity.request; + +import graphql.schema.DataFetchingFieldSelectionSet; +import graphql.schema.SelectedField; +import io.reactivex.rxjava3.core.Single; +import java.util.Collection; +import java.util.Optional; +import org.hypertrace.core.graphql.context.GraphQlRequestContext; + +public interface EntityLabelRequestBuilder { + + Single> buildLabelRequestIfPresentInResultSet( + GraphQlRequestContext context, String scope, DataFetchingFieldSelectionSet selectionSet); + + Single> buildLabelRequestIfPresentInAnyEntity( + GraphQlRequestContext context, String scope, Collection entityField); +} diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityRequest.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityRequest.java index b3a2048d..7f25cf51 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityRequest.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityRequest.java @@ -21,5 +21,5 @@ public interface EntityRequest { boolean fetchTotal(); - Optional labelRequest(); + Optional labelRequest(); } diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityRequestModule.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityRequestModule.java index 85cce0d0..481678f9 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityRequestModule.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/EntityRequestModule.java @@ -14,6 +14,7 @@ public class EntityRequestModule extends AbstractModule { @Override protected void configure() { bind(EntityRequestBuilder.class).to(DefaultEntityRequestBuilder.class); + bind(EntityLabelRequestBuilder.class).to(DefaultEntityLabelRequestBuilder.class); requireBinding(ResultSetRequestBuilder.class); requireBinding(ArgumentDeserializer.class); requireBinding(MetricRequestBuilder.class); diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/NeighborEntitiesRequestBuilder.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/NeighborEntitiesRequestBuilder.java index 71e48f4a..be59a4b5 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/NeighborEntitiesRequestBuilder.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/NeighborEntitiesRequestBuilder.java @@ -179,6 +179,6 @@ private static class NeighborEntityRequest implements EntityRequest { EdgeSetGroupRequest outgoingEdgeRequests; boolean includeInactive; boolean fetchTotal; - Optional labelRequest; + Optional labelRequest; } } diff --git a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java index c06830d6..75e52439 100644 --- a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java +++ b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java @@ -4,7 +4,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -26,7 +25,6 @@ import lombok.Value; import lombok.experimental.Accessors; import org.hypertrace.core.graphql.common.request.AttributeAssociation; -import org.hypertrace.core.graphql.common.request.AttributeRequest; import org.hypertrace.core.graphql.common.request.AttributeRequestBuilder; import org.hypertrace.core.graphql.common.request.FilterRequestBuilder; import org.hypertrace.core.graphql.common.request.ResultSetRequest; @@ -39,6 +37,7 @@ import org.hypertrace.core.graphql.utils.schema.SelectionQuery; import org.hypertrace.graphql.entity.dao.EntityDao; import org.hypertrace.graphql.entity.joiner.EntityJoiner.EntityIdGetter; +import org.hypertrace.graphql.entity.request.EntityLabelRequestBuilder; import org.hypertrace.graphql.entity.request.EntityRequest; import org.hypertrace.graphql.entity.schema.EdgeResultSet; import org.hypertrace.graphql.entity.schema.Entity; @@ -68,7 +67,7 @@ class DefaultEntityJoinerBuilderTest { @Mock DataFetchingFieldSelectionSet mockSelectionSet; @Mock AttributeAssociation mockFilter; @Mock ResultSetRequest mockResultSetRequest; - @Mock AttributeRequest mockAttributeRequest; + @Mock EntityLabelRequestBuilder mockEntityLabelRequestBuilder; Scheduler testScheduler = Schedulers.single(); @@ -84,7 +83,8 @@ void beforeEach() { mockResultSetRequestBuilder, mockFilterRequestBuilder, attributeRequestBuilder, - testScheduler); + testScheduler, + mockEntityLabelRequestBuilder); } @Test @@ -117,6 +117,9 @@ void fetchesEntitiesOfMultipleTypesInBatches() { mock(SelectedField.class)), "pathToEntity"); + when(this.mockEntityLabelRequestBuilder.buildLabelRequestIfPresentInAnyEntity( + eq(mockRequestContext), any(), any())) + .thenReturn(Single.just(Optional.empty())); mockRequestBuilding(); mockResult( Map.of( @@ -124,7 +127,6 @@ void fetchesEntitiesOfMultipleTypesInBatches() { List.of(firstTypeFirstIdEntity, firstTypeSecondIdEntity), SECOND_ENTITY_TYPE, List.of(secondTypeFirstIdEntity, secondTypeSecondIdEntity))); - mockLabelAttributeRequest(); EntityJoiner joiner = this.entityJoinerBuilder @@ -223,12 +225,6 @@ private void mockResult(Map> resultsByEntityType) { }); } - private void mockLabelAttributeRequest() { - doReturn(Single.just(mockAttributeRequest)) - .when(attributeRequestBuilder) - .buildForKey(any(), any(), any()); - } - @Value private static class TestJoinSource { String firstEntityId; From be8e6856222e05c6e7603b08fcfd89bb4fd3d9ab Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld <45047841+aaron-steinfeld@users.noreply.github.com> Date: Thu, 16 Sep 2021 07:51:34 -0700 Subject: [PATCH 3/8] Update hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java --- .../entity/request/DefaultEntityLabelRequestBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java index a8bd21fb..ff1f2d98 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java @@ -17,7 +17,7 @@ import org.hypertrace.core.graphql.utils.schema.SelectionQuery; import org.hypertrace.graphql.entity.schema.Entity; -public class DefaultEntityLabelRequestBuilder implements EntityLabelRequestBuilder { +class DefaultEntityLabelRequestBuilder implements EntityLabelRequestBuilder { private static final String LABELS_ATTRIBUTE_KEY = "labels"; From 70bbe12d91fa723f514305bacc529b252a2299d6 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Thu, 16 Sep 2021 21:20:30 +0530 Subject: [PATCH 4/8] Minor correction --- .../entity/request/DefaultEntityLabelRequestBuilder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java index ff1f2d98..bf8aa986 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java @@ -53,9 +53,9 @@ public Single> buildLabelRequestIfPresentInAnyEntit GraphQlRequestContext context, String scope, Collection entityFields) { if (entityFields.stream() .anyMatch(field -> isLabelFieldRequested(field.getSelectionSet(), LABEL_FIELD_QUERY))) { - return Single.just(Optional.empty()); + return buildRequest(context, scope); } - return buildRequest(context, scope); + return Single.just(Optional.empty()); } private boolean isLabelFieldRequested( From fa7bc3b526887d9be8fd50aa1dbfccd3a84c7dc1 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Mon, 18 Oct 2021 17:35:06 +0530 Subject: [PATCH 5/8] Added test for label request builder --- .../joiner/DefaultEntityJoinerBuilder.java | 1 - .../DefaultEntityLabelRequestBuilderTest.java | 91 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) create mode 100644 hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilderTest.java diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java index f3a37294..ed563004 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilder.java @@ -65,7 +65,6 @@ @Slf4j class DefaultEntityJoinerBuilder implements EntityJoinerBuilder { - private static final String LABELS_KEY_NAME = "labels"; private static final int ZERO_OFFSET = 0; private final EntityDao entityDao; diff --git a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilderTest.java b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilderTest.java new file mode 100644 index 00000000..e54dfd11 --- /dev/null +++ b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilderTest.java @@ -0,0 +1,91 @@ +package org.hypertrace.graphql.entity.request; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +import graphql.schema.DataFetchingFieldSelectionSet; +import graphql.schema.SelectedField; +import io.reactivex.rxjava3.core.Single; +import java.util.List; +import java.util.Optional; +import java.util.stream.Stream; +import org.hypertrace.core.graphql.common.request.AttributeRequest; +import org.hypertrace.core.graphql.common.request.AttributeRequestBuilder; +import org.hypertrace.core.graphql.context.GraphQlRequestContext; +import org.hypertrace.core.graphql.utils.schema.GraphQlSelectionFinder; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class DefaultEntityLabelRequestBuilderTest { + + private static final String SCOPE = "scope"; + + @Mock GraphQlSelectionFinder mockSelectionFinder; + @Mock AttributeRequestBuilder attributeRequestBuilder; + @Mock GraphQlRequestContext mockRequestContext; + + EntityLabelRequestBuilder entityLabelRequestBuilder; + + @BeforeEach + void beforeEach() { + this.entityLabelRequestBuilder = + spy(new DefaultEntityLabelRequestBuilder(attributeRequestBuilder, mockSelectionFinder)); + } + + @Test + void test_buildLabelRequestIfPresentInResultSet_returnsEmpty() { + DataFetchingFieldSelectionSet mockSelectionSet = mock(DataFetchingFieldSelectionSet.class); + when(mockSelectionFinder.findSelections(eq(mockSelectionSet), any())) + .thenReturn(Stream.empty()); + Optional entityLabelRequestOptional = + this.entityLabelRequestBuilder + .buildLabelRequestIfPresentInResultSet(mockRequestContext, SCOPE, mockSelectionSet) + .blockingGet(); + assertTrue(entityLabelRequestOptional.isEmpty()); + } + + @Test + void test_buildLabelRequestIfPresentInResultSet_returnsLabelRequest() { + DataFetchingFieldSelectionSet mockSelectionSet = mock(DataFetchingFieldSelectionSet.class); + when(mockSelectionFinder.findSelections(eq(mockSelectionSet), any())) + .thenReturn(Stream.of(mock(SelectedField.class))); + AttributeRequest mockAttributeRequest = mock(AttributeRequest.class); + when(attributeRequestBuilder.buildForKey(eq(mockRequestContext), any(), eq("labels"))) + .thenReturn(Single.just(mockAttributeRequest)); + Optional entityLabelRequestOptional = + this.entityLabelRequestBuilder + .buildLabelRequestIfPresentInResultSet(mockRequestContext, SCOPE, mockSelectionSet) + .blockingGet(); + assertFalse(entityLabelRequestOptional.isEmpty()); + assertEquals( + mockAttributeRequest, entityLabelRequestOptional.get().labelIdArrayAttributeRequest()); + } + + @Test + void test_buildLabelRequestIfPresentInAnyEntity_returnsLabelRequest() { + SelectedField mockSelectionField = mock(SelectedField.class); + DataFetchingFieldSelectionSet mockSelectionSet = mock(DataFetchingFieldSelectionSet.class); + when(mockSelectionField.getSelectionSet()).thenReturn(mockSelectionSet); + when(mockSelectionFinder.findSelections(eq(mockSelectionSet), any())) + .thenReturn(Stream.of(mock(SelectedField.class))); + AttributeRequest mockAttributeRequest = mock(AttributeRequest.class); + when(attributeRequestBuilder.buildForKey(eq(mockRequestContext), any(), eq("labels"))) + .thenReturn(Single.just(mockAttributeRequest)); + Optional entityLabelRequestOptional = + this.entityLabelRequestBuilder + .buildLabelRequestIfPresentInAnyEntity( + mockRequestContext, SCOPE, List.of(mockSelectionField)) + .blockingGet(); + assertFalse(entityLabelRequestOptional.isEmpty()); + assertEquals( + mockAttributeRequest, entityLabelRequestOptional.get().labelIdArrayAttributeRequest()); + } +} From e5fa0bf61973aa013f1a009717d7247431d0bad7 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Mon, 18 Oct 2021 21:02:47 +0530 Subject: [PATCH 6/8] Added test for labels in EntityJointerBuilder --- .../DefaultEntityLabelRequestBuilder.java | 4 +- .../DefaultEntityJoinerBuilderTest.java | 91 ++++++++++++++++++- 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java index bf8aa986..aeddbf39 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java @@ -17,7 +17,7 @@ import org.hypertrace.core.graphql.utils.schema.SelectionQuery; import org.hypertrace.graphql.entity.schema.Entity; -class DefaultEntityLabelRequestBuilder implements EntityLabelRequestBuilder { +public class DefaultEntityLabelRequestBuilder implements EntityLabelRequestBuilder { private static final String LABELS_ATTRIBUTE_KEY = "labels"; @@ -33,7 +33,7 @@ class DefaultEntityLabelRequestBuilder implements EntityLabelRequestBuilder { private final GraphQlSelectionFinder selectionFinder; @Inject - DefaultEntityLabelRequestBuilder( + public DefaultEntityLabelRequestBuilder( AttributeRequestBuilder attributeRequestBuilder, GraphQlSelectionFinder selectionFinder) { this.attributeRequestBuilder = attributeRequestBuilder; this.selectionFinder = selectionFinder; diff --git a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java index 75e52439..24bf20b8 100644 --- a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java +++ b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java @@ -3,9 +3,12 @@ import static java.util.Collections.emptyList; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -25,6 +28,7 @@ import lombok.Value; import lombok.experimental.Accessors; import org.hypertrace.core.graphql.common.request.AttributeAssociation; +import org.hypertrace.core.graphql.common.request.AttributeRequest; import org.hypertrace.core.graphql.common.request.AttributeRequestBuilder; import org.hypertrace.core.graphql.common.request.FilterRequestBuilder; import org.hypertrace.core.graphql.common.request.ResultSetRequest; @@ -37,6 +41,7 @@ import org.hypertrace.core.graphql.utils.schema.SelectionQuery; import org.hypertrace.graphql.entity.dao.EntityDao; import org.hypertrace.graphql.entity.joiner.EntityJoiner.EntityIdGetter; +import org.hypertrace.graphql.entity.request.DefaultEntityLabelRequestBuilder; import org.hypertrace.graphql.entity.request.EntityLabelRequestBuilder; import org.hypertrace.graphql.entity.request.EntityRequest; import org.hypertrace.graphql.entity.schema.EdgeResultSet; @@ -67,7 +72,7 @@ class DefaultEntityJoinerBuilderTest { @Mock DataFetchingFieldSelectionSet mockSelectionSet; @Mock AttributeAssociation mockFilter; @Mock ResultSetRequest mockResultSetRequest; - @Mock EntityLabelRequestBuilder mockEntityLabelRequestBuilder; + EntityLabelRequestBuilder entityLabelRequestBuilder; Scheduler testScheduler = Schedulers.single(); @@ -75,6 +80,9 @@ class DefaultEntityJoinerBuilderTest { @BeforeEach void beforeEach() { + this.entityLabelRequestBuilder = + spy(new DefaultEntityLabelRequestBuilder(attributeRequestBuilder, mockSelectionFinder)); + this.entityJoinerBuilder = new DefaultEntityJoinerBuilder( mockEntityDao, @@ -84,7 +92,7 @@ void beforeEach() { mockFilterRequestBuilder, attributeRequestBuilder, testScheduler, - mockEntityLabelRequestBuilder); + entityLabelRequestBuilder); } @Test @@ -117,9 +125,6 @@ void fetchesEntitiesOfMultipleTypesInBatches() { mock(SelectedField.class)), "pathToEntity"); - when(this.mockEntityLabelRequestBuilder.buildLabelRequestIfPresentInAnyEntity( - eq(mockRequestContext), any(), any())) - .thenReturn(Single.just(Optional.empty())); mockRequestBuilding(); mockResult( Map.of( @@ -176,6 +181,71 @@ void doesNotFetchIfNoEntitiesRequested() { verify(this.mockEntityDao, never()).getEntities(any()); } + @Test + void fetchesEntitiesWithLabels() { + Entity firstTypeFirstIdEntity = new TestEntity(FIRST_ENTITY_TYPE, "first-id-1"); + Entity firstTypeSecondIdEntity = new TestEntity(FIRST_ENTITY_TYPE, "first-id-2"); + Entity secondTypeFirstIdEntity = new TestEntity(SECOND_ENTITY_TYPE, "second-id-1"); + Entity secondTypeSecondIdEntity = new TestEntity(SECOND_ENTITY_TYPE, "second-id-2"); + TestJoinSource firstJoinSource = new TestJoinSource("first-id-1", null); + TestJoinSource secondJoinSource = new TestJoinSource("first-id-2", null); + TestJoinSource thirdJoinSource = new TestJoinSource(null, "second-id-1"); + TestJoinSource fourthJoinSource = new TestJoinSource("first-id-1", "second-id-2"); + Table expected = + ImmutableTable.builder() + .put(firstJoinSource, FIRST_ENTITY_TYPE, firstTypeFirstIdEntity) + .put(secondJoinSource, FIRST_ENTITY_TYPE, firstTypeSecondIdEntity) + .put(thirdJoinSource, SECOND_ENTITY_TYPE, secondTypeFirstIdEntity) + .put(fourthJoinSource, FIRST_ENTITY_TYPE, firstTypeFirstIdEntity) + .put(fourthJoinSource, SECOND_ENTITY_TYPE, secondTypeSecondIdEntity) + .build(); + + List joinSources = + List.of(firstJoinSource, secondJoinSource, thirdJoinSource, fourthJoinSource); + + AttributeRequest mockLabelAttributeRequest = mock(AttributeRequest.class); + mockRequestedEntityFields( + Map.of( + FIRST_ENTITY_TYPE, + mockSelectionField(mockLabelAttributeRequest), + SECOND_ENTITY_TYPE, + mock(SelectedField.class)), + "pathToEntity"); + + mockRequestBuilding(); + mockResult( + Map.of( + FIRST_ENTITY_TYPE, + List.of(firstTypeFirstIdEntity, firstTypeSecondIdEntity), + SECOND_ENTITY_TYPE, + List.of(secondTypeFirstIdEntity, secondTypeSecondIdEntity))); + + EntityJoiner joiner = + this.entityJoinerBuilder + .build(this.mockRequestContext, this.mockSelectionSet, List.of("pathToEntity")) + .blockingGet(); + + assertEquals( + expected, joiner.joinEntities(joinSources, new TestJoinSourceIdGetter()).blockingGet()); + + verify(mockEntityDao, times(1)) + .getEntities( + argThat( + request -> + request.entityType().equals(FIRST_ENTITY_TYPE) + && request + .labelRequest() + .get() + .labelIdArrayAttributeRequest() + .equals(mockLabelAttributeRequest))); + verify(mockEntityDao, times(1)) + .getEntities( + argThat( + request -> + request.entityType().equals(SECOND_ENTITY_TYPE) + && request.labelRequest().isEmpty())); + } + private void mockRequestedEntityFields( Map selectedFieldsByEntityType, String location) { @@ -225,6 +295,17 @@ private void mockResult(Map> resultsByEntityType) { }); } + private SelectedField mockSelectionField(AttributeRequest mockLabelRequest) { + SelectedField mockSelectionField = mock(SelectedField.class); + DataFetchingFieldSelectionSet mockSelectionSet = mock(DataFetchingFieldSelectionSet.class); + when(mockSelectionField.getSelectionSet()).thenReturn(mockSelectionSet); + when(mockSelectionFinder.findSelections(eq(mockSelectionSet), any())) + .thenReturn(Stream.of(mock(SelectedField.class))); + when(attributeRequestBuilder.buildForKey(eq(mockRequestContext), any(), eq("labels"))) + .thenReturn(Single.just(mockLabelRequest)); + return mockSelectionField; + } + @Value private static class TestJoinSource { String firstEntityId; From c985e39f13a7f3347753ff6fb38e0d2a8646f00b Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Mon, 18 Oct 2021 21:18:08 +0530 Subject: [PATCH 7/8] Address review comments --- .../graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java | 3 +-- .../entity/request/DefaultEntityLabelRequestBuilderTest.java | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java index 24bf20b8..d6bd375d 100644 --- a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java +++ b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java @@ -7,7 +7,6 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -81,7 +80,7 @@ class DefaultEntityJoinerBuilderTest { @BeforeEach void beforeEach() { this.entityLabelRequestBuilder = - spy(new DefaultEntityLabelRequestBuilder(attributeRequestBuilder, mockSelectionFinder)); + new DefaultEntityLabelRequestBuilder(attributeRequestBuilder, mockSelectionFinder); this.entityJoinerBuilder = new DefaultEntityJoinerBuilder( diff --git a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilderTest.java b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilderTest.java index e54dfd11..da20392b 100644 --- a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilderTest.java +++ b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilderTest.java @@ -64,7 +64,6 @@ void test_buildLabelRequestIfPresentInResultSet_returnsLabelRequest() { this.entityLabelRequestBuilder .buildLabelRequestIfPresentInResultSet(mockRequestContext, SCOPE, mockSelectionSet) .blockingGet(); - assertFalse(entityLabelRequestOptional.isEmpty()); assertEquals( mockAttributeRequest, entityLabelRequestOptional.get().labelIdArrayAttributeRequest()); } @@ -84,7 +83,6 @@ void test_buildLabelRequestIfPresentInAnyEntity_returnsLabelRequest() { .buildLabelRequestIfPresentInAnyEntity( mockRequestContext, SCOPE, List.of(mockSelectionField)) .blockingGet(); - assertFalse(entityLabelRequestOptional.isEmpty()); assertEquals( mockAttributeRequest, entityLabelRequestOptional.get().labelIdArrayAttributeRequest()); } From 12ba6fe71d4b166cb3625bb82c7f51865870e408 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Mon, 18 Oct 2021 22:56:24 +0530 Subject: [PATCH 8/8] Address review comments --- .../DefaultEntityLabelRequestBuilder.java | 4 +- .../DefaultEntityJoinerBuilderTest.java | 47 ++++++++----------- 2 files changed, 21 insertions(+), 30 deletions(-) diff --git a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java index aeddbf39..bf8aa986 100644 --- a/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java +++ b/hypertrace-graphql-entity-schema/src/main/java/org/hypertrace/graphql/entity/request/DefaultEntityLabelRequestBuilder.java @@ -17,7 +17,7 @@ import org.hypertrace.core.graphql.utils.schema.SelectionQuery; import org.hypertrace.graphql.entity.schema.Entity; -public class DefaultEntityLabelRequestBuilder implements EntityLabelRequestBuilder { +class DefaultEntityLabelRequestBuilder implements EntityLabelRequestBuilder { private static final String LABELS_ATTRIBUTE_KEY = "labels"; @@ -33,7 +33,7 @@ public class DefaultEntityLabelRequestBuilder implements EntityLabelRequestBuild private final GraphQlSelectionFinder selectionFinder; @Inject - public DefaultEntityLabelRequestBuilder( + DefaultEntityLabelRequestBuilder( AttributeRequestBuilder attributeRequestBuilder, GraphQlSelectionFinder selectionFinder) { this.attributeRequestBuilder = attributeRequestBuilder; this.selectionFinder = selectionFinder; diff --git a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java index d6bd375d..cc92526e 100644 --- a/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java +++ b/hypertrace-graphql-entity-schema/src/test/java/org/hypertrace/graphql/entity/joiner/DefaultEntityJoinerBuilderTest.java @@ -23,11 +23,11 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.stream.Stream; import lombok.Value; import lombok.experimental.Accessors; import org.hypertrace.core.graphql.common.request.AttributeAssociation; -import org.hypertrace.core.graphql.common.request.AttributeRequest; import org.hypertrace.core.graphql.common.request.AttributeRequestBuilder; import org.hypertrace.core.graphql.common.request.FilterRequestBuilder; import org.hypertrace.core.graphql.common.request.ResultSetRequest; @@ -40,7 +40,7 @@ import org.hypertrace.core.graphql.utils.schema.SelectionQuery; import org.hypertrace.graphql.entity.dao.EntityDao; import org.hypertrace.graphql.entity.joiner.EntityJoiner.EntityIdGetter; -import org.hypertrace.graphql.entity.request.DefaultEntityLabelRequestBuilder; +import org.hypertrace.graphql.entity.request.EntityLabelRequest; import org.hypertrace.graphql.entity.request.EntityLabelRequestBuilder; import org.hypertrace.graphql.entity.request.EntityRequest; import org.hypertrace.graphql.entity.schema.EdgeResultSet; @@ -71,7 +71,7 @@ class DefaultEntityJoinerBuilderTest { @Mock DataFetchingFieldSelectionSet mockSelectionSet; @Mock AttributeAssociation mockFilter; @Mock ResultSetRequest mockResultSetRequest; - EntityLabelRequestBuilder entityLabelRequestBuilder; + @Mock EntityLabelRequestBuilder mockEntityLabelRequestBuilder; Scheduler testScheduler = Schedulers.single(); @@ -79,9 +79,6 @@ class DefaultEntityJoinerBuilderTest { @BeforeEach void beforeEach() { - this.entityLabelRequestBuilder = - new DefaultEntityLabelRequestBuilder(attributeRequestBuilder, mockSelectionFinder); - this.entityJoinerBuilder = new DefaultEntityJoinerBuilder( mockEntityDao, @@ -91,7 +88,7 @@ void beforeEach() { mockFilterRequestBuilder, attributeRequestBuilder, testScheduler, - entityLabelRequestBuilder); + mockEntityLabelRequestBuilder); } @Test @@ -124,6 +121,9 @@ void fetchesEntitiesOfMultipleTypesInBatches() { mock(SelectedField.class)), "pathToEntity"); + when(this.mockEntityLabelRequestBuilder.buildLabelRequestIfPresentInAnyEntity( + eq(mockRequestContext), any(), any())) + .thenReturn(Single.just(Optional.empty())); mockRequestBuilding(); mockResult( Map.of( @@ -202,13 +202,19 @@ void fetchesEntitiesWithLabels() { List joinSources = List.of(firstJoinSource, secondJoinSource, thirdJoinSource, fourthJoinSource); - AttributeRequest mockLabelAttributeRequest = mock(AttributeRequest.class); + SelectedField mockSelectionField1 = mock(SelectedField.class); + SelectedField mockSelectionField2 = mock(SelectedField.class); + EntityLabelRequest mockEntityLabelRequest = mock(EntityLabelRequest.class); + when(mockEntityLabelRequestBuilder.buildLabelRequestIfPresentInAnyEntity( + eq(mockRequestContext), eq(FIRST_ENTITY_TYPE), eq(Set.of(mockSelectionField1)))) + .thenReturn(Single.just(Optional.of(mockEntityLabelRequest))); + when(mockEntityLabelRequestBuilder.buildLabelRequestIfPresentInAnyEntity( + eq(mockRequestContext), eq(SECOND_ENTITY_TYPE), eq(Set.of(mockSelectionField2)))) + .thenReturn(Single.just(Optional.empty())); mockRequestedEntityFields( Map.of( - FIRST_ENTITY_TYPE, - mockSelectionField(mockLabelAttributeRequest), - SECOND_ENTITY_TYPE, - mock(SelectedField.class)), + FIRST_ENTITY_TYPE, mockSelectionField1, + SECOND_ENTITY_TYPE, mockSelectionField2), "pathToEntity"); mockRequestBuilding(); @@ -232,11 +238,7 @@ void fetchesEntitiesWithLabels() { argThat( request -> request.entityType().equals(FIRST_ENTITY_TYPE) - && request - .labelRequest() - .get() - .labelIdArrayAttributeRequest() - .equals(mockLabelAttributeRequest))); + && request.labelRequest().get().equals(mockEntityLabelRequest))); verify(mockEntityDao, times(1)) .getEntities( argThat( @@ -294,17 +296,6 @@ private void mockResult(Map> resultsByEntityType) { }); } - private SelectedField mockSelectionField(AttributeRequest mockLabelRequest) { - SelectedField mockSelectionField = mock(SelectedField.class); - DataFetchingFieldSelectionSet mockSelectionSet = mock(DataFetchingFieldSelectionSet.class); - when(mockSelectionField.getSelectionSet()).thenReturn(mockSelectionSet); - when(mockSelectionFinder.findSelections(eq(mockSelectionSet), any())) - .thenReturn(Stream.of(mock(SelectedField.class))); - when(attributeRequestBuilder.buildForKey(eq(mockRequestContext), any(), eq("labels"))) - .thenReturn(Single.just(mockLabelRequest)); - return mockSelectionField; - } - @Value private static class TestJoinSource { String firstEntityId;