fix(explore): order by on eds attributes

gateway-service-factory/build.gradle.kts

@@ -3,7 +3,7 @@ plugins {
 }
 
 dependencies {
-  api("org.hypertrace.core.serviceframework:platform-grpc-service-framework:0.1.58")
+  api("org.hypertrace.core.serviceframework:platform-grpc-service-framework:0.1.62")
 
   implementation(project(":gateway-service-impl"))
 }

gateway-service-impl/build.gradle.kts

@@ -15,14 +15,14 @@ dependencies {
   annotationProcessor("org.projectlombok:lombok:1.18.22")
   compileOnly("org.projectlombok:lombok:1.18.18")
 
-  implementation("org.hypertrace.core.query.service:query-service-client:0.8.0")
-  implementation("org.hypertrace.core.attribute.service:attribute-service-client:0.14.25")
-
-  implementation("org.hypertrace.entity.service:entity-service-client:0.8.56")
-  implementation("org.hypertrace.entity.service:entity-service-api:0.8.56")
-  implementation("org.hypertrace.core.grpcutils:grpc-context-utils:0.12.5")
-  implementation("org.hypertrace.core.grpcutils:grpc-client-utils:0.12.5")
-  implementation("org.hypertrace.core.serviceframework:platform-metrics:0.1.58")
+  implementation("org.hypertrace.core.query.service:query-service-client:0.8.20")
+  implementation("org.hypertrace.core.attribute.service:attribute-service-client:0.14.35")
+
+  implementation("org.hypertrace.entity.service:entity-service-client:0.8.86")
+  implementation("org.hypertrace.entity.service:entity-service-api:0.8.86")
+  implementation("org.hypertrace.core.grpcutils:grpc-context-utils:0.12.6")
+  implementation("org.hypertrace.core.grpcutils:grpc-client-utils:0.12.6")
+  implementation("org.hypertrace.core.serviceframework:platform-metrics:0.1.62")
 
   // Config
   implementation("com.typesafe:config:1.4.1")

gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/ExploreService.java

@@ -122,9 +122,7 @@ private IRequestHandler getRequestHandler(
       ExploreRequest request,
       Map<String, AttributeMetadata> attributeMetadataMap,
       RequestContext requestContext) {
-    if (isContextAnEntityType(request, requestContext)
-        && !hasTimeAggregations(request)
-        && !request.getGroupByList().isEmpty()) {
+    if (isContextAnEntityType(request, requestContext) && !hasTimeAggregations(request)) {
       ExpressionContext expressionContext =
           new ExpressionContext(
               attributeMetadataMap,

gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/entity/EntityRequestHandler.java

@@ -41,19 +41,27 @@
 import org.hypertrace.gateway.service.v1.explore.ExploreResponse;
 
 /**
- * {@link EntityRequestHandler} is currently used only when the selections, group bys and filters
- * are on EDS. Can be extended later to support multiple sources. Only needed, when there is a group
- * by on the request, else can directly use {@link
- * org.hypertrace.gateway.service.v1.entity.EntitiesRequest}
+ * {@link EntityRequestHandler} is currently used either
+ *
+ * <ul>
+ *   <li>when the selections, group bys and order bys are on EDS. A group by would need an attribute
+ *       selection, aggregation on the same attribute, and order by on any attribute
+ *   <li>when aggregated selection is on EDS. No group by would mean a single aggregated selection
+ *       on attribute
+ * </ul>
+ *
+ * Can be extended later to support multiple sources. Filters can be present both on QS and EDS.
+ * Only needed, when there is a group by on the request, or an aggregated selection on an attribute,
+ * else can directly use {@link org.hypertrace.gateway.service.v1.entity.EntitiesRequest}
  *
  * <p>Currently,
  *
  * <ul>
  *   <li>Query to {@link
  *       org.hypertrace.gateway.service.common.datafetcher.QueryServiceEntityFetcher} with the time
- *       filter to get set of entity ids. Can be extended to support QS filters
- *   <li>Query to {@link EntityServiceEntityFetcher} with selections, group bys, and filters with an
- *       IN clause on entity ids
+ *       filter to get set of entity ids after applying QS filters
+ *   <li>Query to {@link EntityServiceEntityFetcher} with selections(attribute + aggregated), group
+ *       bys(if present), and filters with an IN clause on entity ids
  * </ul>
  */
 public class EntityRequestHandler extends RequestHandler {
@@ -94,7 +102,7 @@ public EntityRequestHandler(
   @Override
   public ExploreResponse.Builder handleRequest(
       ExploreRequestContext requestContext, ExploreRequest exploreRequest) {
-    // Track if we have Group By so we can determine if we need to do Order By, Limit and Offset
+    // Track if we have Group By, so we can determine if we need to do Order By, Limit and Offset
     // ourselves.
     if (!exploreRequest.getGroupByList().isEmpty()) {
       requestContext.setHasGroupBy(true);

gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/entity/EntityServiceEntityFetcher.java

@@ -18,12 +18,18 @@
 import org.hypertrace.gateway.service.common.converters.EntityServiceAndGatewayServiceConverter;
 import org.hypertrace.gateway.service.common.util.AttributeMetadataUtil;
 import org.hypertrace.gateway.service.common.util.ExpressionReader;
+import org.hypertrace.gateway.service.common.util.OrderByUtil;
 import org.hypertrace.gateway.service.entity.config.EntityIdColumnsConfigs;
 import org.hypertrace.gateway.service.explore.ExploreRequestContext;
+import org.hypertrace.gateway.service.v1.common.OrderByExpression;
 import org.hypertrace.gateway.service.v1.explore.ExploreRequest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class EntityServiceEntityFetcher {
+  private static final Logger LOG = LoggerFactory.getLogger(EntityServiceEntityFetcher.class);
   private static final int DEFAULT_ENTITY_REQUEST_LIMIT = 10_000;
+
   private final AttributeMetadataProvider attributeMetadataProvider;
   private final EntityIdColumnsConfigs entityIdColumnsConfigs;
   private final EntityQueryServiceClient entityQueryServiceClient;
@@ -57,7 +63,23 @@ private EntityQueryRequest buildRequest(
 
     addGroupBys(exploreRequest, builder);
     addSelections(requestContext, exploreRequest, builder);
-    builder.setLimit(DEFAULT_ENTITY_REQUEST_LIMIT);
+    addLimitAndOffset(requestContext, exploreRequest, builder);
+
+    // TODO: Push order by down to EQS
+    // EQS (and document-store) currently doesn't support order by on functional expressions
+    // If there are order by expressions, specify a large limit and track actual limit, offset and
+    // order by
+    // expression list, so we can compute these once we get the results.
+    if (!requestContext.getOrderByExpressions().isEmpty()) {
+      // Ideally, needs the limit and offset for group by, since the fetcher is only triggered when
+      // there is a group by, or a single aggregation selection. A single aggregated selection would
+      // always return a single result (i.e. limit 1)
+      builder.setOffset(0);
+      builder.setLimit(DEFAULT_ENTITY_REQUEST_LIMIT);
+      // Will need to do the ordering, limit and offset ourselves after we get the group by results
+      requestContext.setOrderByExpressions(getRequestOrderByExpressions(exploreRequest));
+    }
+
     return builder.build();
   }
 
@@ -91,6 +113,33 @@ private void addSelections(
         });
   }
 
+  private void addLimitAndOffset(
+      ExploreRequestContext requestContext,
+      ExploreRequest exploreRequest,
+      EntityQueryRequest.Builder builder) {
+    // handle group by scenario with group limit set
+    if (requestContext.hasGroupBy()) {
+      int limit = exploreRequest.getLimit();
+      if (exploreRequest.getGroupLimit() > 0) {
+        // in group by scenario, set limit to minimum of limit or group-limit
+        limit = Math.min(exploreRequest.getLimit(), exploreRequest.getGroupLimit());
+      }
+      // don't exceed default group by limit
+      if (limit > DEFAULT_ENTITY_REQUEST_LIMIT) {
+        LOG.error(
+            "Trying to query for rows more than the default limit {} : {}",
+            DEFAULT_ENTITY_REQUEST_LIMIT,
+            exploreRequest);
+        throw new UnsupportedOperationException(
+            "Trying to query for rows more than the default limit " + exploreRequest);
+      }
+      builder.setLimit(limit);
+    } else {
+      builder.setLimit(exploreRequest.getLimit());
+      builder.setOffset(exploreRequest.getOffset());
+    }
+  }
+
   private Filter.Builder buildFilter(
       ExploreRequest exploreRequest, List<String> entityIdAttributeIds, Set<String> entityIds) {
     Builder filterBuilder =
@@ -126,4 +175,9 @@ private Filter.Builder buildFilter(
 
     return filterBuilder.addAllChildFilter(entityIdsInFilter);
   }
+
+  private List<OrderByExpression> getRequestOrderByExpressions(ExploreRequest request) {
+    return OrderByUtil.matchOrderByExpressionsAliasToSelectionAlias(
+        request.getOrderByList(), request.getSelectionList(), request.getTimeAggregationList());
+  }
 }

gateway-service/build.gradle.kts

@@ -8,8 +8,8 @@ plugins {
 dependencies {
   implementation(project(":gateway-service-factory"))
 
-  implementation("org.hypertrace.core.grpcutils:grpc-server-utils:0.12.5")
-  implementation("org.hypertrace.core.serviceframework:platform-grpc-service-framework:0.1.58")
+  implementation("org.hypertrace.core.grpcutils:grpc-server-utils:0.12.6")
+  implementation("org.hypertrace.core.serviceframework:platform-grpc-service-framework:0.1.62")
   implementation("org.slf4j:slf4j-api:1.7.30")
   implementation("com.typesafe:config:1.4.1")
 

owasp-suppressions.xml

@@ -17,4 +17,16 @@
     <packageUrl regex="true">^pkg:maven/com\.fasterxml\.jackson\.core/jackson\-databind@.*$</packageUrl>
     <vulnerabilityName>CVE-2023-35116</vulnerabilityName>
   </suppress>
+  <suppress until="2023-11-30Z">
+    <notes><![CDATA[
+  This vulnerability is disputed, with the argument that SSL configuration is the responsibility of the client rather
+  than the transport. The change in default is under consideration for the next major Netty release, revisit then.
+  Regardless, our client (which is what brings in this dependency) enables the concerned feature, hostname verification
+  Ref:
+  https://github.com/grpc/grpc-java/issues/10033
+  https://github.com/netty/netty/issues/8537#issuecomment-1527896917
+   ]]></notes>
+    <packageUrl regex="true">^pkg:maven/io\.netty/netty.*@.*$</packageUrl>
+    <vulnerabilityName>CVE-2023-4586</vulnerabilityName>
+  </suppress>
 </suppressions>