From 76e4bb420f73b3bbfc7460adbb668e9ff2b6efe4 Mon Sep 17 00:00:00 2001 From: saxenakshitiz Date: Thu, 7 Dec 2023 22:02:39 +0530 Subject: [PATCH] address review comments --- .../common/converters/QueryRequestUtil.java | 14 ++++++++++++++ .../explore/TheRestGroupRequestHandler.java | 12 +++--------- .../TimeAggregationsWithGroupByRequestHandler.java | 13 +++---------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/QueryRequestUtil.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/QueryRequestUtil.java index 42541fa4..46b1df4c 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/QueryRequestUtil.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/common/converters/QueryRequestUtil.java @@ -20,6 +20,7 @@ public class QueryRequestUtil { private static final String COUNT_FUNCTION_NAME = "COUNT"; private static final String DISTINCTCOUNT_FUNCTION_NAME = "DISTINCTCOUNT"; + private static final List LIST_WITH_NULL_STRING = List.of("null"); public static Filter createBetweenTimesFilter(String columnName, long lower, long higher) { return Filter.newBuilder() @@ -127,4 +128,17 @@ public static Expression createTimeColumnGroupByExpression( .addArguments(createStringLiteralExpression(periodSecs + ":SECONDS"))) .build(); } + + public static List convertStringArrayValue( + org.hypertrace.gateway.service.v1.common.Value value) { + // The downstream QS service returns the default value "null" for a string array when it is + // empty. GW returns an empty list as output. This transformation occurs in the value converter. + // To handle this scenario when querying downstream with an empty list, set its default value to + // "null." + if (value.getStringArrayList().isEmpty()) { + return LIST_WITH_NULL_STRING; + } else { + return value.getStringArrayList(); + } + } } diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/TheRestGroupRequestHandler.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/TheRestGroupRequestHandler.java index 278a8917..52acc6a7 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/TheRestGroupRequestHandler.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/TheRestGroupRequestHandler.java @@ -1,5 +1,7 @@ package org.hypertrace.gateway.service.explore; +import static org.hypertrace.gateway.service.common.converters.QueryRequestUtil.convertStringArrayValue; + import com.google.common.collect.ImmutableSet; import java.util.List; import java.util.Map; @@ -30,7 +32,6 @@ */ public class TheRestGroupRequestHandler { private static final String OTHER_COLUMN_VALUE = "__Other"; - private static final List LIST_WITH_NULL_STRING = List.of("null"); private final RequestHandlerWithSorting requestHandler; TheRestGroupRequestHandler(RequestHandlerWithSorting requestHandler) { @@ -235,14 +236,7 @@ private Stream getStringValues(Value value) { case STRING: return Stream.of(value.getString()); case STRING_ARRAY: - // Handle special case when "null" string is returned as string array value(default value - // scenario). This will be converted to empty list in value converter. In such a case use - // list with "null" value - if (value.getStringArrayList().isEmpty()) { - return LIST_WITH_NULL_STRING.stream(); - } else { - return value.getStringArrayList().stream(); - } + return convertStringArrayValue(value).stream(); default: return Stream.of(); } diff --git a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/TimeAggregationsWithGroupByRequestHandler.java b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/TimeAggregationsWithGroupByRequestHandler.java index 57d36f18..2c988008 100644 --- a/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/TimeAggregationsWithGroupByRequestHandler.java +++ b/gateway-service-impl/src/main/java/org/hypertrace/gateway/service/explore/TimeAggregationsWithGroupByRequestHandler.java @@ -1,5 +1,7 @@ package org.hypertrace.gateway.service.explore; +import static org.hypertrace.gateway.service.common.converters.QueryRequestUtil.convertStringArrayValue; + import com.google.common.collect.Streams; import java.util.List; import java.util.Map; @@ -30,8 +32,6 @@ public class TimeAggregationsWithGroupByRequestHandler implements IRequestHandle private static final Logger LOG = LoggerFactory.getLogger(TimeAggregationsWithGroupByRequestHandler.class); - private static final List LIST_WITH_NULL_STRING = List.of("null"); - private final AttributeMetadataProvider attributeMetadataProvider; private final RequestHandler normalRequestHandler; private final TimeAggregationsRequestHandler timeAggregationsRequestHandler; @@ -230,14 +230,7 @@ private void buildInClauseFilterValue(Value value, Value.Builder valueBuilder) { valueBuilder.addStringArray(value.getString()); break; case STRING_ARRAY: - // Handle special case when "null" string is returned as string array value(default value - // scenario). This will be converted to empty list in value converter. In such a case use - // list with "null" value - if (value.getStringArrayList().isEmpty()) { - valueBuilder.addAllStringArray(LIST_WITH_NULL_STRING); - } else { - valueBuilder.addAllStringArray(value.getStringArrayList()); - } + valueBuilder.addAllStringArray(convertStringArrayValue(value)); break; case LONG: valueBuilder.addLongArray(value.getLong());