leventov commented on a change in pull request #8157: Enum of ResponseContext 
keys
URL: https://github.com/apache/incubator-druid/pull/8157#discussion_r310215079
 
 

 ##########
 File path: 
processing/src/main/java/org/apache/druid/query/context/ResponseContext.java
 ##########
 @@ -19,53 +19,236 @@
 
 package org.apache.druid.query.context;
 
+import com.fasterxml.jackson.annotation.JsonValue;
 import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
 import org.apache.druid.guice.annotations.PublicApi;
 import org.apache.druid.java.util.common.jackson.JacksonUtils;
+import org.apache.druid.query.SegmentDescriptor;
+import org.joda.time.Interval;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
+import java.util.function.BiFunction;
 
 /**
  * The context for storing and passing data between chains of {@link 
org.apache.druid.query.QueryRunner}s.
  * The context is also transferred between Druid nodes with all the data it 
contains.
- * All the keys associated with data inside the context should be stored here.
- * CTX_* keys might be aggregated into an enum. Consider refactoring that.
  */
 @PublicApi
 public abstract class ResponseContext
 {
   /**
-   * Lists intervals for which NO segment is present.
+   * The base interface of a response context key.
+   * Should be implemented by every context key.
    */
-  public static final String CTX_UNCOVERED_INTERVALS = "uncoveredIntervals";
-  /**
-   * Indicates if the number of uncovered intervals exceeded the limit 
(true/false).
-   */
-  public static final String CTX_UNCOVERED_INTERVALS_OVERFLOWED = 
"uncoveredIntervalsOverflowed";
-  /**
-   * Lists missing segments.
-   */
-  public static final String CTX_MISSING_SEGMENTS = "missingSegments";
-  /**
-   * Entity tag. A part of HTTP cache validation mechanism.
-   * Is being removed from the context before sending and used as a separate 
HTTP header.
-   */
-  public static final String CTX_ETAG = "ETag";
-  /**
-   * Query total bytes gathered.
-   */
-  public static final String CTX_QUERY_TOTAL_BYTES_GATHERED = 
"queryTotalBytesGathered";
-  /**
-   * This variable indicates when a running query should be expired,
-   * and is effective only when 'timeout' of queryContext has a positive value.
-   */
-  public static final String CTX_TIMEOUT_AT = "timeoutAt";
+  public interface BaseKey
+  {
+    @JsonValue
+    String getName();
+    /**
+     * Merge function associated with a key: Object (Object oldValue, Object 
newValue)
+     */
+    BiFunction<Object, Object, Object> getMergeFunction();
+  }
+
   /**
-   * The number of scanned rows.
+   * Keys associated with objects in the context.
+   * <p>
+   * If it's necessary to have some new keys in the context then they might be 
listed in a separate enum:
+   * <pre>{@code
+   * public enum ExtensionResponseContextKey implements BaseKey
+   * {
+   *   EXTENSION_KEY_1("extension_key_1"), EXTENSION_KEY_2("extension_key_2");
+   *
+   *   static {
+   *     for (BaseKey key : values()) ResponseContext.Key.registerKey(key);
+   *   }
+   *
+   *   private final String name;
+   *   private final BiFunction<Object, Object, Object> mergeFunction;
+   *
+   *   ExtensionResponseContextKey(String name)
+   *   {
+   *     this.name = name;
+   *     this.mergeFunction = (oldValue, newValue) -> newValue;
+   *   }
+   *
+   *   @Override public String getName() { return name; }
+   *
+   *   @Override public BiFunction<Object, Object, Object> getMergeFunction() 
{ return mergeFunction; }
+   * }
+   * }</pre>
+   * Make sure all extension enum values added with {@link Key#registerKey} 
method.
    */
-  public static final String CTX_COUNT = "count";
+  public enum Key implements BaseKey
+  {
+    /**
+     * Lists intervals for which NO segment is present.
+     */
+    UNCOVERED_INTERVALS(
+        "uncoveredIntervals",
+            (oldValue, newValue) -> {
+              final ArrayList<Interval> result = new 
ArrayList<Interval>((List) oldValue);
+              result.addAll((List) newValue);
+              return result;
+            }
+    ),
+    /**
+     * Indicates if the number of uncovered intervals exceeded the limit 
(true/false).
+     */
+    UNCOVERED_INTERVALS_OVERFLOWED(
+        "uncoveredIntervalsOverflowed",
+            (oldValue, newValue) -> (boolean) oldValue || (boolean) newValue
+    ),
+    /**
+     * Lists missing segments.
+     */
+    MISSING_SEGMENTS(
+        "missingSegments",
+            (oldValue, newValue) -> {
+              final ArrayList<SegmentDescriptor> result = new 
ArrayList<SegmentDescriptor>((List) oldValue);
+              result.addAll((List) newValue);
+              return result;
+            }
+    ),
+    /**
+     * Entity tag. A part of HTTP cache validation mechanism.
+     * Is being removed from the context before sending and used as a separate 
HTTP header.
+     */
+    ETAG("ETag"),
+    /**
+     * Query fail time (current time + timeout).
+     * It is not updated continuously as TIMEOUT_AT.
 
 Review comment:
   Please wrap `TIMEOUT_AT` with `{@link }`.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to