Skip to content
This repository has been archived by the owner on Jun 26, 2024. It is now read-only.

fix(explore): return early, if no entities #196

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(explore): return early, if no entities #196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e53f38a
fix(explore): return early, if no entities
skjindal93 Mar 7, 2024
31f2447
fixed tests
skjindal93 Mar 8, 2024
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
17 changes: 14 additions & 3 deletions ...way-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,21 @@
@Override
public ExploreResponse.Builder handleRequest(
ExploreRequestContext requestContext, ExploreRequest request) {
QueryRequest queryRequest =
Optional<QueryRequest> maybeQueryRequest =
buildQueryRequest(requestContext, request, attributeMetadataProvider);

if (maybeQueryRequest.isEmpty()) {
return ExploreResponse.newBuilder();

Check warning on line 95 in gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java

View check run for this annotation

Codecov / codecov/patch

gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java#L95 

Added line #L95 was not covered by tests
}

QueryRequest queryRequest = maybeQueryRequest.get();
Copy link
Contributor

Choose a reason for hiding this comment

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

nit - prefer .orElseThrow over .get (or move code around to avoid the direct access)

Iterator<ResultSetChunk> resultSetChunkIterator = executeQuery(requestContext, queryRequest);

return handleQueryServiceResponse(
request, requestContext, resultSetChunkIterator, requestContext, attributeMetadataProvider);
}

QueryRequest buildQueryRequest(
Optional<QueryRequest> buildQueryRequest(
ExploreRequestContext requestContext,
ExploreRequest request,
AttributeMetadataProvider attributeMetadataProvider) {
Expand All @@ -113,12 +118,18 @@
attributeMetadataProvider.getAttributesMetadata(requestContext, request.getContext());
if (hasOnlyAttributeSource(request.getFilter(), AttributeSource.EDS, attributeMetadataMap)) {
entityIds = getEntityIdsToFilterFromSourceEDS(requestContext, request, attributeMetadataMap);

if (entityIds.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit - worth a comment explaining why we shouldn't build a query request in this case

return Optional.empty();

Check warning on line 123 in gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java

View check run for this annotation

Codecov / codecov/patch

gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/RequestHandler.java#L123 

Added line #L123 was not covered by tests
}

qsSourceFilter =
buildFilter(request.getFilter(), AttributeSource.QS, attributeMetadataMap)
.orElse(request.getFilter());
}

QueryRequest.Builder builder = QueryRequest.newBuilder();

// 1. Add selections. All selections should either be only column or only function, never both.
// The validator should catch this.
List<Expression> aggregatedSelections =
Expand Down Expand Up @@ -160,7 +171,7 @@
// 4. Add order by along with setting limit, offset
addSortLimitAndOffset(request, requestContext, builder);

return builder.build();
return Optional.of(builder.build());
}

// This is to get all the entity Ids for the EDS source filter.
Expand Down
4 changes: 2 additions & 2 deletions .../src/main/java/org/hypertrace/gateway/service/explore/TimeAggregationsRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public class TimeAggregationsRequestHandler extends RequestHandler {
}

@Override
QueryRequest buildQueryRequest(
Optional<QueryRequest> buildQueryRequest(
ExploreRequestContext requestContext,
ExploreRequest request,
AttributeMetadataProvider attributeMetadataProvider) {
Expand Down Expand Up @@ -79,7 +79,7 @@ QueryRequest buildQueryRequest(
// 6. Set Limit.
builder.setLimit(request.getLimit());

return builder.build();
return Optional.of(builder.build());
}

private void addTimeAggregationsAsSelectionsToRequest(
Expand Down
Loading