FrankChen021 commented on code in PR #19457:
URL: https://github.com/apache/druid/pull/19457#discussion_r3234371357
##########
server/src/main/java/org/apache/druid/server/QueryBlocklistRule.java:
##########
@@ -19,158 +19,32 @@
package org.apache.druid.server;
-import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonProperty;
-import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
-import com.google.common.collect.Sets;
+import com.fasterxml.jackson.annotation.JsonSubTypes;
+import com.fasterxml.jackson.annotation.JsonTypeInfo;
import org.apache.druid.query.Query;
-import javax.annotation.Nullable;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Set;
-
/**
- * A rule for matching queries against blocklist criteria. A query matches
this rule if ALL
- * specified criteria match (AND logic). Null or empty criteria match
everything.
+ * A rule that determines whether a query should be blocked. Implementations
define their own
+ * matching logic and JSON fields. Use {@link DefaultQueryBlocklistRule} for
the standard criteria
+ * (datasources, query types, context).
+ *
+ * <p>Rules with no {@code "type"} field in JSON deserialize as {@link
DefaultQueryBlocklistRule}
+ * for backwards compatibility. Extensions can register additional
implementations as Jackson
+ * subtypes via {@code SimpleModule.registerSubtypes(...)}.
+ *
+ * <p>Implementations must define {@code equals} and {@code hashCode} so that
+ * {@link org.apache.druid.server.broker.BrokerDynamicConfig} can detect
changes correctly.
*/
-public class QueryBlocklistRule
+@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type", defaultImpl =
DefaultQueryBlocklistRule.class)
Review Comment:
[P2] Fail unknown blocklist rule types instead of defaulting
Using `defaultImpl = DefaultQueryBlocklistRule.class` makes Jackson use the
default rule not only when `type` is absent, but also when a non-empty type id
cannot be resolved. For the new extension point, a broker or coordinator that
lacks a registered custom rule subtype can silently deserialize that rule as
`DefaultQueryBlocklistRule`; with Druid's unknown-property-tolerant mapper,
extension-specific fields are ignored and any standard fields such as
`dataSources` can turn into a broader block than intended. Please distinguish
the legacy missing-type case from an unrecognized explicit `type`, so unknown
extension rules fail fast rather than being interpreted as default rules.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
