fix: send alias on order by expression (#225)

hypertrace · May 18, 2024 · d0135a5 · d0135a5
1 parent cd911fb
commit d0135a5
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 31 deletions.
diff --git a/.../hypertrace/graphql/utils/metrics/gateway/AggregatableOrderByExpressionListConverter.java b/.../hypertrace/graphql/utils/metrics/gateway/AggregatableOrderByExpressionListConverter.java
@@ -5,8 +5,8 @@
 import io.reactivex.rxjava3.core.Maybe;
 import io.reactivex.rxjava3.core.Observable;
 import io.reactivex.rxjava3.core.Single;
+import java.util.Collections;
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
 import javax.inject.Inject;
@@ -19,6 +19,7 @@
 import org.hypertrace.gateway.service.v1.common.Expression;
 import org.hypertrace.gateway.service.v1.common.OrderByExpression;
 import org.hypertrace.gateway.service.v1.common.SortOrder;
+import org.hypertrace.graphql.metric.request.MetricAggregationRequestBuilder;
 import org.hypertrace.graphql.metric.schema.argument.AggregatableOrderArgument;
 
 class AggregatableOrderByExpressionListConverter
@@ -31,18 +32,21 @@ class AggregatableOrderByExpressionListConverter
   private final Converter<OrderDirection, SortOrder> sortOrderConverter;
   private final Converter<MetricAggregationType, AttributeModelMetricAggregationType>
       aggregationTypeConverter;
+  private final MetricAggregationRequestBuilder metricAggregationRequestBuilder;
 
   @Inject
   AggregatableOrderByExpressionListConverter(
       Converter<AttributeAssociation<AttributeExpression>, Expression> columnExpressionConverter,
       MetricAggregationExpressionConverter metricAggregationExpressionConverter,
       Converter<OrderDirection, SortOrder> sortOrderConverter,
       Converter<MetricAggregationType, AttributeModelMetricAggregationType>
-          aggregationTypeConverter) {
+          aggregationTypeConverter,
+      MetricAggregationRequestBuilder metricAggregationRequestBuilder) {
     this.columnExpressionConverter = columnExpressionConverter;
     this.metricAggregationExpressionConverter = metricAggregationExpressionConverter;
     this.sortOrderConverter = sortOrderConverter;
     this.aggregationTypeConverter = aggregationTypeConverter;
+    this.metricAggregationRequestBuilder = metricAggregationRequestBuilder;
   }
 
   @Override
@@ -66,17 +70,20 @@ private Single<Expression> buildSelectionExpression(
       AttributeAssociation<AggregatableOrderArgument> orderArgument) {
     AttributeAssociation<AttributeExpression> attributeExpressionAssociation =
         this.buildAttributeExpressionAssociation(orderArgument);
+    List<Object> aggregationArguments =
+        Optional.<Object>ofNullable(orderArgument.value().size())
+            .map(List::of)
+            .orElseGet(Collections::emptyList);
     return Maybe.fromOptional(Optional.ofNullable(orderArgument.value().aggregation()))
         .flatMapSingle(this.aggregationTypeConverter::convert)
-        .flatMapSingle(
+        .map(
             aggregationType ->
-                orderArgument.value().size() == null
-                    ? this.metricAggregationExpressionConverter.convertForNoArgsOrAlias(
-                        attributeExpressionAssociation, aggregationType)
-                    : this.metricAggregationExpressionConverter.convertForArgsButNoAlias(
-                        attributeExpressionAssociation,
-                        aggregationType,
-                        List.of(Objects.requireNonNull(orderArgument.value().size()))))
+                this.metricAggregationRequestBuilder.build(
+                    attributeExpressionAssociation, aggregationType, aggregationArguments))
+        .flatMapSingle(
+            aggregationRequest ->
+                this.metricAggregationExpressionConverter.convert(
+                    aggregationRequest, aggregationRequest.alias()))
         .switchIfEmpty(this.columnExpressionConverter.convert(attributeExpressionAssociation));
   }
 

diff --git a/.../src/main/java/org/hypertrace/graphql/utils/metrics/gateway/GatewayMetricUtilsModule.java b/.../src/main/java/org/hypertrace/graphql/utils/metrics/gateway/GatewayMetricUtilsModule.java
@@ -24,6 +24,7 @@
 import org.hypertrace.gateway.service.v1.common.Value;
 import org.hypertrace.gateway.service.v1.entity.Entity;
 import org.hypertrace.graphql.metric.request.MetricAggregationRequest;
+import org.hypertrace.graphql.metric.request.MetricAggregationRequestBuilder;
 import org.hypertrace.graphql.metric.request.MetricRequest;
 import org.hypertrace.graphql.metric.request.MetricSeriesRequest;
 import org.hypertrace.graphql.metric.schema.BaselinedMetricAggregationContainer;
@@ -76,5 +77,6 @@ protected void configure() {
         Key.get(
             new TypeLiteral<
                 Converter<MetricAggregationType, AttributeModelMetricAggregationType>>() {}));
+    requireBinding(MetricAggregationRequestBuilder.class);
   }
 }
diff --git a/...va/org/hypertrace/graphql/utils/metrics/gateway/MetricAggregationExpressionConverter.java b/...va/org/hypertrace/graphql/utils/metrics/gateway/MetricAggregationExpressionConverter.java
@@ -2,11 +2,9 @@
 
 import static io.reactivex.rxjava3.core.Single.zip;
 
-import com.google.protobuf.StringValue;
 import io.reactivex.rxjava3.core.Observable;
 import io.reactivex.rxjava3.core.Single;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -52,25 +50,6 @@ Single<Expression> convert(MetricAggregationRequest metricAggregation, String al
         .map(functionExpression -> Expression.newBuilder().setFunction(functionExpression).build());
   }
 
-  Single<Expression> convertForNoArgsOrAlias(
-      AttributeAssociation<AttributeExpression> attributeExpressionAssociation,
-      AttributeModelMetricAggregationType aggregationType) {
-    return convertForArgsButNoAlias(
-        attributeExpressionAssociation, aggregationType, Collections.emptyList());
-  }
-
-  Single<Expression> convertForArgsButNoAlias(
-      AttributeAssociation<AttributeExpression> attributeExpressionAssociation,
-      AttributeModelMetricAggregationType aggregationType,
-      List<Object> arguments) {
-    return this.buildAggregationFunctionExpression(
-            attributeExpressionAssociation,
-            aggregationType,
-            arguments,
-            StringValue.getDefaultInstance().getValue())
-        .map(functionExpression -> Expression.newBuilder().setFunction(functionExpression).build());
-  }
-
   private Single<FunctionExpression> buildAggregationFunctionExpression(
       AttributeAssociation<AttributeExpression> attributeExpressionAssociation,
       AttributeModelMetricAggregationType aggregationType,