leventov commented on a change in pull request #8157: Enum of ResponseContext keys URL: https://github.com/apache/incubator-druid/pull/8157#discussion_r309835592
########## File path: processing/src/main/java/org/apache/druid/query/context/ResponseContext.java ########## @@ -236,95 +253,124 @@ public static ResponseContext createEmpty() return DefaultResponseContext.createEmpty(); } + /** + * Deserializes a string into {@link ResponseContext} using given {@link ObjectMapper}. + * @throws IllegalStateException if one of the deserialized map keys has not been registered. + */ public static ResponseContext deserialize(String responseContext, ObjectMapper objectMapper) throws IOException { - final Map<String, Object> delegate = objectMapper.readValue( + final Map<String, Object> keyNameToObjects = objectMapper.readValue( responseContext, JacksonUtils.TYPE_REFERENCE_MAP_STRING_OBJECT ); - return new ResponseContext() - { - @Override - protected Map<String, Object> getDelegate() - { - return delegate; - } - }; + final ResponseContext context = ResponseContext.createEmpty(); + keyNameToObjects.forEach((keyName, value) -> { + final BaseKey key = Key.keyOf(keyName); + context.add(key, value); + }); + return context; } - protected abstract Map<String, Object> getDelegate(); + protected abstract Map<BaseKey, Object> getDelegate(); + /** + * Associates the specified object with the specified key. + * @throws IllegalStateException if the key has not been registered. + */ public Object put(BaseKey key, Object value) { - return getDelegate().put(key.getName(), value); + final BaseKey registeredKey = Key.keyOf(key.getName()); + return getDelegate().put(registeredKey, value); } public Object get(BaseKey key) { - return getDelegate().get(key.getName()); + return getDelegate().get(key); } public Object remove(BaseKey key) { - return getDelegate().remove(key.getName()); + return getDelegate().remove(key); } /** * Adds (merges) a new value associated with a key to an old value. * See merge function of a context key for a specific implementation. + * @throws IllegalStateException if the key has not been registered. */ public Object add(BaseKey key, Object value) { - return getDelegate().merge(key.getName(), value, key.getMergeFunction()); + final BaseKey registeredKey = Key.keyOf(key.getName()); + return getDelegate().merge(registeredKey, value, key.getMergeFunction()); } /** - * Merges a response context into current. - * This method merges only keys from the enum {@link Key}. + * Merges a response context into the current. + * @throws IllegalStateException If a key of the {@code responseContext} has not been registered. */ public void merge(ResponseContext responseContext) { - for (BaseKey key : Key.getKeys()) { - final Object newValue = responseContext.get(key); + responseContext.getDelegate().forEach((key, newValue) -> { if (newValue != null) { add(key, newValue); } - } + }); } /** * Serializes the context given that the resulting string length is less than the provided limit. - * The method removes max-length fields one by one if the resulting string length is greater than the limit. - * The resulting string might be correctly deserialized as a {@link ResponseContext}. + * This method tries to remove some elements from context collections if it's needed to satisfy the limit. + * The resulting string might be correctly deserialized to {@link ResponseContext}. Review comment: Please comment on why explicit priorities of keys are not implemented. ---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org