address review comments

hypertrace · Dec 7, 2023 · 76e4bb4 · 76e4bb4
1 parent cb44fd5
commit 76e4bb4
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 19 deletions.
diff --git a/...impl/src/main/java/org/hypertrace/gateway/service/common/converters/QueryRequestUtil.java b/...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<String> 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<String> 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/...impl/src/main/java/org/hypertrace/gateway/service/explore/TheRestGroupRequestHandler.java b/...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<String> LIST_WITH_NULL_STRING = List.of("null");
   private final RequestHandlerWithSorting requestHandler;
 
   TheRestGroupRequestHandler(RequestHandlerWithSorting requestHandler) {
@@ -235,14 +236,7 @@ private Stream<String> 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/...ava/org/hypertrace/gateway/service/explore/TimeAggregationsWithGroupByRequestHandler.java b/...ava/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<String> 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());