refactor: disambiguate method names and overloads

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>

extensions-core/kafka-indexing-service/src/test/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisorTest.java

@@ -4837,7 +4837,7 @@ public void testIsTaskCurrent()
         .withMaxRowsInMemory(42)
         .build();
 
-    KafkaIndexTask completedTaskFromStorage = createKafkaIndexTask(
+    KafkaIndexTask completedTaskFromStorage = createKafkaIndexTaskFromSupervisorTuningConfig(
         "id0",
         0,
         new SeekableStreamStartSequenceNumbers<>(
@@ -4858,7 +4858,7 @@ public void testIsTaskCurrent()
     // 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 void testIsTaskCurrent()
         supervisor.getTuningConfig()
     );
 
-    KafkaIndexTask taskFromStorageMismatchedDataSchema = createKafkaIndexTask(
+    KafkaIndexTask taskFromStorageMismatchedDataSchema = createKafkaIndexTaskFromSupervisorTuningConfig(
         "id2",
         0,
         new SeekableStreamStartSequenceNumbers<>(
@@ -4894,7 +4894,7 @@ public void testIsTaskCurrent()
         supervisor.getTuningConfig()
     );
 
-    KafkaIndexTask taskFromStorageMismatchedTuningConfig = createKafkaIndexTask(
+    KafkaIndexTask taskFromStorageMismatchedTuningConfig = createKafkaIndexTaskFromSupervisorTuningConfig(
         "id3",
         0,
         new SeekableStreamStartSequenceNumbers<>(
@@ -4912,23 +4912,24 @@ public void testIsTaskCurrent()
         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 void testSequenceNameDoesNotChangeWithTaskId()
     );
 
     // 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 void testSequenceNameDoesNotChangeWithTaskId()
     );
 
     // Create task2 with same offsets
-    final KafkaIndexTask task2 = createKafkaIndexTask(
+    final KafkaIndexTask task2 = createKafkaIndexTaskFromSupervisorTuningConfig(
         "id1",
         0,
         task1.getIOConfig().getStartSequenceNumbers(),
@@ -6082,7 +6083,7 @@ private KafkaIndexTask createKafkaIndexTask(
       KafkaSupervisorTuningConfig tuningConfig
   )
   {
-    return createKafkaIndexTask(
+    return createKafkaIndexTaskFromSupervisorTuningConfig(
         id,
         taskGroupId,
         startPartitions,
@@ -6094,7 +6095,7 @@ private KafkaIndexTask createKafkaIndexTask(
     );
   }
 
-  private KafkaIndexTask createKafkaIndexTask(
+  private KafkaIndexTask createKafkaIndexTaskFromSupervisorTuningConfig(
       String id,
       int taskGroupId,
       SeekableStreamStartSequenceNumbers<KafkaTopicPartition, Long> startPartitions,

indexing-service/src/main/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunner.java

@@ -1385,7 +1385,17 @@ public List<String> getBlacklistedWorkers()
     ).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 Map<String, Long> getLazyTaskSlotCount()
   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(

indexing-service/src/test/java/org/apache/druid/indexing/overlord/hrtr/HttpRemoteTaskRunnerTest.java

@@ -156,6 +156,7 @@ WorkerNodeService.DISCOVERY_SERVICE_KEY, new WorkerNodeService("ip2", 2, "0", Wo
     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());
   }

processing/src/main/java/org/apache/druid/query/expression/NestedDataExpressions.java

@@ -85,7 +85,7 @@ public ExprEval eval(ObjectBinding bindings)
             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 ExprEval eval(ObjectBinding bindings)
         {
           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 ExprEval eval(ObjectBinding bindings)
       {
         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 ExprEval eval(ObjectBinding bindings)
       {
         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 ExprEval eval(ObjectBinding bindings)
           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 ExprEval eval(ObjectBinding bindings)
         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 ExprEval eval(ObjectBinding bindings)
         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 JsonQueryArrayExpr(List<Expr> args)
       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 ExprEval eval(ObjectBinding bindings)
         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 ExprEval eval(ObjectBinding bindings)
         {
           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 ExprEval eval(ObjectBinding bindings)
           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 ExpressionType getOutputType(InputBindingInspector inspector)
   }
 
   @Nullable
-  static Object unwrap(ExprEval input)
+  static Object unwrapEval(ExprEval<?> input)
   {
     return unwrap(input.value());
   }

processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/GroupByQueryEngine.java

@@ -751,16 +751,16 @@ protected IntGrouper newGrouper()
     @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 @@ private void aggregateSingleValueDims(IntGrouper grouper)
       }
     }
 
-    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.

processing/src/test/java/org/apache/druid/math/expr/ParserTest.java

@@ -273,11 +273,11 @@ public void testIdentifiers()
   @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 void testConstantComplexAndNestedArrays()
         TypeStrategiesTest.NULLABLE_TEST_PAIR_TYPE.getComplexTypeName(),
         StringUtils.encodeBase64String(b2)
     );
-    validateConstantExpression(
+    validateConstantScalarExpression(
         l1String,
         l1
     );
@@ -887,7 +887,7 @@ private void validateFoldUnapplied(
     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(

processing/src/test/java/org/apache/druid/query/operator/NaiveSortOperatorTest.java

@@ -47,8 +47,8 @@ public void testNoInputisHandledCorrectly()
   @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 void testSortAscending()
   @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 void testSortDescending()
         .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));

server/src/test/java/org/apache/druid/client/cache/CacheConfigTest.java

@@ -142,7 +142,7 @@ public void testTRUE()
   }
 
   @Test
-  public void testFALSE()
+  public void testUppercaseFalse()
   {
     properties.put(PROPERTY_PREFIX + ".populateCache", "FALSE");
     configProvider.inject(properties, configurator);
@@ -152,7 +152,7 @@ public void testFALSE()
 
 
   @Test(expected = ProvisionException.class)
-  public void testFaLse()
+  public void testMixedCaseFalseIsRejected()
   {
     properties.put(PROPERTY_PREFIX + ".populateCache", "FaLse");
     configProvider.inject(properties, configurator);