This is an automated email from the ASF dual-hosted git repository.
FrankChen021 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 65bda617cad refactor: disambiguate method names and overloads (#19825)
65bda617cad is described below
commit 65bda617cad978d3394044c8e08d6878c318c0ef
Author: Frank Chen <[email protected]>
AuthorDate: Tue Aug 4 16:29:05 2026 +0800
refactor: disambiguate method names and overloads (#19825)
* Disambiguate method names and overloads
* Test blacklisted task slot count
* Use generic ExprEval in unwrap helper
* Preserve deprecated blacklisted worker API
---
codestyle/pmd-ruleset.xml | 25 +++++++++++
.../kafka/supervisor/KafkaSupervisorTest.java | 51 +++++++++++-----------
.../overlord/hrtr/HttpRemoteTaskRunner.java | 12 ++++-
.../overlord/hrtr/HttpRemoteTaskRunnerTest.java | 1 +
.../query/expression/NestedDataExpressions.java | 24 +++++-----
.../groupby/epinephelinae/GroupByQueryEngine.java | 8 ++--
.../org/apache/druid/math/expr/ParserTest.java | 14 +++---
.../query/operator/NaiveSortOperatorTest.java | 10 ++---
.../apache/druid/client/cache/CacheConfigTest.java | 4 +-
9 files changed, 93 insertions(+), 56 deletions(-)
diff --git a/codestyle/pmd-ruleset.xml b/codestyle/pmd-ruleset.xml
index ef1858bf2d8..69187d496e1 100644
--- a/codestyle/pmd-ruleset.xml
+++ b/codestyle/pmd-ruleset.xml
@@ -31,4 +31,29 @@ This ruleset defines the PMD rules for the Apache Druid
project.
<rule ref="category/java/codestyle.xml/TooManyStaticImports" />
<rule ref="category/java/bestpractices.xml/UnusedFormalParameter" />
<rule ref="category/java/bestpractices.xml/UnusedLocalVariable" />
+ <rule ref="category/java/bestpractices.xml/MissingOverride" />
+ <rule name="ConfusingMethodName"
+ language="java"
+ message="Methods in the same class must not differ only by
capitalization"
+ class="net.sourceforge.pmd.lang.rule.xpath.XPathRule">
+ <description>
+ Prevent case-only method-name differences, which are easy to confuse at
call sites.
+ Methods that override inherited APIs are excluded because their names
cannot be changed.
+ </description>
+ <priority>2</priority>
+ <properties>
+ <property name="xpath">
+ <value>
+<![CDATA[
+for $method in //MethodDeclaration[@Overridden = false()]
+return $method[
+ some $other in $method/preceding-sibling::MethodDeclaration[@Overridden =
false()]
+ satisfies lower-case($method/@Name) = lower-case($other/@Name)
+ and $method/@Name != $other/@Name
+]
+]]>
+ </value>
+ </property>
+ </properties>
+ </rule>
</ruleset>
diff --git
a/extensions-core/kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorTest.java
b/extensions-core/kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorTest.java
index a954b644e42..87aad166b40 100644
---
a/extensions-core/kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorTest.java
+++
b/extensions-core/kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorTest.java
@@ -4837,7 +4837,7 @@ public class KafkaSupervisorTest extends EasyMockSupport
.withMaxRowsInMemory(42)
.build();
- KafkaIndexTask completedTaskFromStorage = createKafkaIndexTask(
+ KafkaIndexTask completedTaskFromStorage =
createKafkaIndexTaskFromSupervisorTuningConfig(
"id0",
0,
new SeekableStreamStartSequenceNumbers<>(
@@ -4858,7 +4858,7 @@ public class KafkaSupervisorTest extends EasyMockSupport
// Expect metadata call only for tasks that are not active
EasyMock.expect(taskStorage.getTask("id0")).andReturn(Optional.of(completedTaskFromStorage));
- KafkaIndexTask taskFromStorage = createKafkaIndexTask(
+ KafkaIndexTask taskFromStorage =
createKafkaIndexTaskFromSupervisorTuningConfig(
"id1",
0,
new SeekableStreamStartSequenceNumbers<>(
@@ -4876,7 +4876,7 @@ public class KafkaSupervisorTest extends EasyMockSupport
supervisor.getTuningConfig()
);
- KafkaIndexTask taskFromStorageMismatchedDataSchema = createKafkaIndexTask(
+ KafkaIndexTask taskFromStorageMismatchedDataSchema =
createKafkaIndexTaskFromSupervisorTuningConfig(
"id2",
0,
new SeekableStreamStartSequenceNumbers<>(
@@ -4894,7 +4894,7 @@ public class KafkaSupervisorTest extends EasyMockSupport
supervisor.getTuningConfig()
);
- KafkaIndexTask taskFromStorageMismatchedTuningConfig =
createKafkaIndexTask(
+ KafkaIndexTask taskFromStorageMismatchedTuningConfig =
createKafkaIndexTaskFromSupervisorTuningConfig(
"id3",
0,
new SeekableStreamStartSequenceNumbers<>(
@@ -4912,23 +4912,24 @@ public class KafkaSupervisorTest extends EasyMockSupport
modifiedTuningConfig
);
- KafkaIndexTask taskFromStorageMismatchedPartitionsWithTaskGroup =
createKafkaIndexTask(
- "id4",
- 0,
- new SeekableStreamStartSequenceNumbers<>(
- "topic",
- singlePartitionMap(topic, 0, 0L, 2, 6L),
- ImmutableSet.of()
- ),
- new SeekableStreamEndSequenceNumbers<>(
- "topic",
- singlePartitionMap(topic, 0, Long.MAX_VALUE, 2, Long.MAX_VALUE)
- ),
- minMessageTime,
- maxMessageTime,
- dataSchema,
- supervisor.getTuningConfig()
- );
+ KafkaIndexTask taskFromStorageMismatchedPartitionsWithTaskGroup =
+ createKafkaIndexTaskFromSupervisorTuningConfig(
+ "id4",
+ 0,
+ new SeekableStreamStartSequenceNumbers<>(
+ "topic",
+ singlePartitionMap(topic, 0, 0L, 2, 6L),
+ ImmutableSet.of()
+ ),
+ new SeekableStreamEndSequenceNumbers<>(
+ "topic",
+ singlePartitionMap(topic, 0, Long.MAX_VALUE, 2, Long.MAX_VALUE)
+ ),
+ minMessageTime,
+ maxMessageTime,
+ dataSchema,
+ supervisor.getTuningConfig()
+ );
Map<String, Task> taskMap = ImmutableMap.of(
taskFromStorage.getId(), taskFromStorage,
@@ -4967,7 +4968,7 @@ public class KafkaSupervisorTest extends EasyMockSupport
);
// Create task1 with some start and end offsets
- final KafkaIndexTask task1 = createKafkaIndexTask(
+ final KafkaIndexTask task1 =
createKafkaIndexTaskFromSupervisorTuningConfig(
"id0",
0,
new SeekableStreamStartSequenceNumbers<>(
@@ -4986,7 +4987,7 @@ public class KafkaSupervisorTest extends EasyMockSupport
);
// Create task2 with same offsets
- final KafkaIndexTask task2 = createKafkaIndexTask(
+ final KafkaIndexTask task2 =
createKafkaIndexTaskFromSupervisorTuningConfig(
"id1",
0,
task1.getIOConfig().getStartSequenceNumbers(),
@@ -6082,7 +6083,7 @@ public class KafkaSupervisorTest extends EasyMockSupport
KafkaSupervisorTuningConfig tuningConfig
)
{
- return createKafkaIndexTask(
+ return createKafkaIndexTaskFromSupervisorTuningConfig(
id,
taskGroupId,
startPartitions,
@@ -6094,7 +6095,7 @@ public class KafkaSupervisorTest extends EasyMockSupport
);
}
- private KafkaIndexTask createKafkaIndexTask(
+ private KafkaIndexTask createKafkaIndexTaskFromSupervisorTuningConfig(
String id,
int taskGroupId,
SeekableStreamStartSequenceNumbers<KafkaTopicPartition, Long>
startPartitions,
diff --git
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunner.java
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunner.java
index dcf4bdf266e..08b635346cd 100644
---
a/indexing-service/src/main/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunner.java
+++
b/indexing-service/src/main/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunner.java
@@ -1385,7 +1385,17 @@ public class HttpRemoteTaskRunner implements
WorkerTaskRunner, TaskLogStreamer,
).collect(Collectors.toList());
}
+ /**
+ * @deprecated Use {@link #getBlacklistedWorkerInfos()} instead.
+ */
+ @Deprecated
+ @SuppressWarnings("PMD.ConfusingMethodName")
public Collection<ImmutableWorkerInfo> getBlackListedWorkers()
+ {
+ return getBlacklistedWorkerInfos();
+ }
+
+ public Collection<ImmutableWorkerInfo> getBlacklistedWorkerInfos()
{
return
ImmutableList.copyOf(Collections2.transform(blackListedWorkers.values(),
WorkerHolder::toImmutable));
}
@@ -1683,7 +1693,7 @@ public class HttpRemoteTaskRunner implements
WorkerTaskRunner, TaskLogStreamer,
public Map<String, Long> getBlacklistedTaskSlotCount()
{
Map<String, Long> totalBlacklistedPeons = new HashMap<>();
- for (ImmutableWorkerInfo worker : getBlackListedWorkers()) {
+ for (ImmutableWorkerInfo worker : getBlacklistedWorkerInfos()) {
String workerCategory = worker.getWorker().getCategory();
int workerBlacklistedPeons = worker.getWorker().getCapacity();
totalBlacklistedPeons.compute(
diff --git
a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java
b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java
index 578bb7d9491..931a192c753 100644
---
a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java
+++
b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java
@@ -156,6 +156,7 @@ public class HttpRemoteTaskRunnerTest
Assert.assertEquals(numTasks, taskRunner.getKnownTasks().size());
Assert.assertEquals(numTasks, taskRunner.getCompletedTasks().size());
Assert.assertEquals(4, taskRunner.getTotalCapacity());
+ Assert.assertTrue(taskRunner.getBlacklistedTaskSlotCount().isEmpty());
Assert.assertEquals(-1, taskRunner.getMaximumCapacityWithAutoscale());
Assert.assertEquals(0, taskRunner.getUsedCapacity());
}
diff --git
a/processing/src/main/java/org/apache/druid/query/expression/NestedDataExpressions.java
b/processing/src/main/java/org/apache/druid/query/expression/NestedDataExpressions.java
index a3a59d36adf..45906919be1 100644
---
a/processing/src/main/java/org/apache/druid/query/expression/NestedDataExpressions.java
+++
b/processing/src/main/java/org/apache/druid/query/expression/NestedDataExpressions.java
@@ -85,7 +85,7 @@ public class NestedDataExpressions
if (!field.type().is(ExprType.STRING)) {
throw JsonObjectExprMacro.this.validationFailed("field name must
be a STRING");
}
- theMap.put(field.asString(), unwrap(value));
+ theMap.put(field.asString(), unwrapEval(value));
}
return ExprEval.ofComplex(ExpressionType.NESTED_DATA, theMap);
@@ -275,7 +275,7 @@ public class NestedDataExpressions
{
ExprEval input = args.get(0).eval(bindings);
try {
- final Object unwrapped = unwrap(input);
+ final Object unwrapped = unwrapEval(input);
final String stringify = unwrapped == null ? null :
jsonMapper.writeValueAsString(unwrapped);
return ExprEval.ofType(
ExpressionType.STRING,
@@ -472,7 +472,7 @@ public class NestedDataExpressions
{
final ExprEval input = args.get(0).eval(bindings);
final ExprEval valAtPath = ExprEval.bestEffortOf(
- NestedPathFinder.find(unwrap(input), parts)
+ NestedPathFinder.find(unwrapEval(input), parts)
);
if (valAtPath.type().isPrimitive() ||
valAtPath.type().isPrimitiveArray()) {
return valAtPath;
@@ -512,7 +512,7 @@ public class NestedDataExpressions
{
final ExprEval input = args.get(0).eval(bindings);
final ExprEval valAtPath = ExprEval.bestEffortOf(
- NestedPathFinder.find(unwrap(input), parts)
+ NestedPathFinder.find(unwrapEval(input), parts)
);
if (valAtPath.type().isPrimitive() ||
valAtPath.type().isPrimitiveArray()) {
return valAtPath.castTo(castTo);
@@ -553,7 +553,7 @@ public class NestedDataExpressions
castTo = null;
}
final List<NestedPathPart> parts =
NestedPathFinder.parseJsonPath(path.asString());
- final ExprEval<?> valAtPath =
ExprEval.bestEffortOf(NestedPathFinder.find(unwrap(input), parts));
+ final ExprEval<?> valAtPath =
ExprEval.bestEffortOf(NestedPathFinder.find(unwrapEval(input), parts));
if (valAtPath.type().isPrimitive() ||
valAtPath.type().isPrimitiveArray()) {
return castTo == null ? valAtPath : valAtPath.castTo(castTo);
}
@@ -606,7 +606,7 @@ public class NestedDataExpressions
ExprEval input = args.get(0).eval(bindings);
return ExprEval.ofComplex(
ExpressionType.NESTED_DATA,
- NestedPathFinder.find(unwrap(input), parts)
+ NestedPathFinder.find(unwrapEval(input), parts)
);
}
@@ -634,7 +634,7 @@ public class NestedDataExpressions
final List<NestedPathPart> parts =
NestedPathFinder.parseJsonPath(path.asString());
return ExprEval.ofComplex(
ExpressionType.NESTED_DATA,
- NestedPathFinder.find(unwrap(input), parts)
+ NestedPathFinder.find(unwrapEval(input), parts)
);
}
@@ -682,7 +682,7 @@ public class NestedDataExpressions
public ExprEval eval(ObjectBinding bindings)
{
ExprEval input = args.get(0).eval(bindings);
- final Object value = NestedPathFinder.find(unwrap(input), parts);
+ final Object value = NestedPathFinder.find(unwrapEval(input), parts);
if (value instanceof List) {
return ExprEval.ofArray(
JSON_ARRAY,
@@ -717,7 +717,7 @@ public class NestedDataExpressions
ExprEval input = args.get(0).eval(bindings);
ExprEval path = args.get(1).eval(bindings);
final List<NestedPathPart> parts =
NestedPathFinder.parseJsonPath(path.asString());
- final Object value = NestedPathFinder.find(unwrap(input), parts);
+ final Object value = NestedPathFinder.find(unwrapEval(input), parts);
if (value instanceof List) {
return ExprEval.ofArray(
JSON_ARRAY,
@@ -790,7 +790,7 @@ public class NestedDataExpressions
{
ExprEval input = args.get(0).eval(bindings);
// maybe in the future ProcessResults should deal in
PathFinder.PathPart instead of strings for fields
- StructuredDataProcessor.ProcessResults info =
processor.processFields(unwrap(input));
+ StructuredDataProcessor.ProcessResults info =
processor.processFields(unwrapEval(input));
List<String> transformed = info.getLiteralFields()
.stream()
.map(NestedPathFinder::toNormalizedJsonPath)
@@ -839,7 +839,7 @@ public class NestedDataExpressions
ExprEval input = args.get(0).eval(bindings);
return ExprEval.ofType(
ExpressionType.STRING_ARRAY,
- NestedPathFinder.findKeys(unwrap(input), parts)
+ NestedPathFinder.findKeys(unwrapEval(input), parts)
);
}
@@ -854,7 +854,7 @@ public class NestedDataExpressions
}
@Nullable
- static Object unwrap(ExprEval input)
+ static Object unwrapEval(ExprEval<?> input)
{
return unwrap(input.value());
}
diff --git
a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/GroupByQueryEngine.java
b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/GroupByQueryEngine.java
index 0e17cc4572d..c24a322f9ad 100644
---
a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/GroupByQueryEngine.java
+++
b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/GroupByQueryEngine.java
@@ -751,16 +751,16 @@ public class GroupByQueryEngine
@Override
protected void aggregateSingleValueDims(Grouper<IntKey> grouper)
{
- aggregateSingleValueDims((IntGrouper) grouper);
+ aggregateSingleValueDimsWithIntGrouper((IntGrouper) grouper);
}
@Override
protected void aggregateMultiValueDims(Grouper<IntKey> grouper)
{
- aggregateMultiValueDims((IntGrouper) grouper);
+ aggregateMultiValueDimsWithIntGrouper((IntGrouper) grouper);
}
- private void aggregateSingleValueDims(IntGrouper grouper)
+ private void aggregateSingleValueDimsWithIntGrouper(IntGrouper grouper)
{
// No need to track strategy internal state footprint, because
array-based grouping does not use strategies.
// It accesses dimension selectors directly and only works on truly
dictionary-coded columns.
@@ -783,7 +783,7 @@ public class GroupByQueryEngine
}
}
- private void aggregateMultiValueDims(IntGrouper grouper)
+ private void aggregateMultiValueDimsWithIntGrouper(IntGrouper grouper)
{
// No need to track strategy internal state footprint, because
array-based grouping does not use strategies.
// It accesses dimension selectors directly and only works on truly
dictionary-coded columns.
diff --git
a/processing/src/test/java/org/apache/druid/math/expr/ParserTest.java
b/processing/src/test/java/org/apache/druid/math/expr/ParserTest.java
index 54c81095bf0..5ef7c8647ae 100644
--- a/processing/src/test/java/org/apache/druid/math/expr/ParserTest.java
+++ b/processing/src/test/java/org/apache/druid/math/expr/ParserTest.java
@@ -273,11 +273,11 @@ public class ParserTest extends
InitializedNullHandlingTest
@Test
public void testLiterals()
{
- validateConstantExpression("\'foo\'", "foo");
- validateConstantExpression("\'foo bar\'", "foo bar");
- validateConstantExpression("\'föo bar\'", "föo bar");
- validateConstantExpression("\'f\\u0040o bar\'", "f@o bar");
- validateConstantExpression("\'f\\u000Ao \\'b\\\\\\\"ar\'", "f\no
'b\\\"ar");
+ validateConstantScalarExpression("\'foo\'", "foo");
+ validateConstantScalarExpression("\'foo bar\'", "foo bar");
+ validateConstantScalarExpression("\'föo bar\'", "föo bar");
+ validateConstantScalarExpression("\'f\\u0040o bar\'", "f@o bar");
+ validateConstantScalarExpression("\'f\\u000Ao \\'b\\\\\\\"ar\'", "f\no
'b\\\"ar");
}
@Test
@@ -409,7 +409,7 @@ public class ParserTest extends InitializedNullHandlingTest
TypeStrategiesTest.NULLABLE_TEST_PAIR_TYPE.getComplexTypeName(),
StringUtils.encodeBase64String(b2)
);
- validateConstantExpression(
+ validateConstantScalarExpression(
l1String,
l1
);
@@ -887,7 +887,7 @@ public class ParserTest extends InitializedNullHandlingTest
Assert.assertEquals(transformed.stringify(),
transformedRoundTrip.stringify());
}
- private void validateConstantExpression(String expression, Object expected)
+ private void validateConstantScalarExpression(String expression, Object
expected)
{
Expr parsed = Parser.parse(expression, ExprMacroTable.nil());
Assert.assertEquals(
diff --git
a/processing/src/test/java/org/apache/druid/query/operator/NaiveSortOperatorTest.java
b/processing/src/test/java/org/apache/druid/query/operator/NaiveSortOperatorTest.java
index 05ade15642a..fa8374eaecf 100644
---
a/processing/src/test/java/org/apache/druid/query/operator/NaiveSortOperatorTest.java
+++
b/processing/src/test/java/org/apache/druid/query/operator/NaiveSortOperatorTest.java
@@ -47,8 +47,8 @@ public class NaiveSortOperatorTest
@Test
public void testSortAscending()
{
- RowsAndColumns rac1 = racForColumn("c", new int[] {5, 3, 1});
- RowsAndColumns rac2 = racForColumn("c", new int[] {2, 6, 4});
+ RowsAndColumns rac1 = racForArrayColumn("c", new int[] {5, 3, 1});
+ RowsAndColumns rac2 = racForArrayColumn("c", new int[] {2, 6, 4});
NaiveSortOperator op = new NaiveSortOperator(
InlineScanOperator.make(rac1, rac2),
@@ -66,8 +66,8 @@ public class NaiveSortOperatorTest
@Test
public void testSortDescending()
{
- RowsAndColumns rac1 = racForColumn("c", new int[] {5, 3, 1});
- RowsAndColumns rac2 = racForColumn("c", new int[] {2, 6, 4});
+ RowsAndColumns rac1 = racForArrayColumn("c", new int[] {5, 3, 1});
+ RowsAndColumns rac2 = racForArrayColumn("c", new int[] {2, 6, 4});
NaiveSortOperator op = new NaiveSortOperator(
InlineScanOperator.make(rac1, rac2),
@@ -82,7 +82,7 @@ public class NaiveSortOperatorTest
.runToCompletion(op);
}
- private MapOfColumnsRowsAndColumns racForColumn(String k1, Object arr)
+ private MapOfColumnsRowsAndColumns racForArrayColumn(String k1, Object arr)
{
if (int.class.equals(arr.getClass().getComponentType())) {
return racForColumn(k1, new IntArrayColumn((int[]) arr));
diff --git
a/server/src/test/java/org/apache/druid/client/cache/CacheConfigTest.java
b/server/src/test/java/org/apache/druid/client/cache/CacheConfigTest.java
index aa728b5aaf4..6ece9dfdca6 100644
--- a/server/src/test/java/org/apache/druid/client/cache/CacheConfigTest.java
+++ b/server/src/test/java/org/apache/druid/client/cache/CacheConfigTest.java
@@ -142,7 +142,7 @@ public class CacheConfigTest
}
@Test
- public void testFALSE()
+ public void testUppercaseFalse()
{
properties.put(PROPERTY_PREFIX + ".populateCache", "FALSE");
configProvider.inject(properties, configurator);
@@ -152,7 +152,7 @@ public class CacheConfigTest
@Test(expected = ProvisionException.class)
- public void testFaLse()
+ public void testMixedCaseFalseIsRejected()
{
properties.put(PROPERTY_PREFIX + ".populateCache", "FaLse");
configProvider.inject(properties, configurator);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
