diff --git a/codestyle/pmd-ruleset.xml b/codestyle/pmd-ruleset.xml index ef1858bf2d8b..69187d496e19 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. + + + + 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. + + 2 + + + + + + + + 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 a954b644e427..87aad166b40c 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 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 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 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 dcf4bdf266ec..08b635346cd8 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 List getBlacklistedWorkers() ).collect(Collectors.toList()); } + /** + * @deprecated Use {@link #getBlacklistedWorkerInfos()} instead. + */ + @Deprecated + @SuppressWarnings("PMD.ConfusingMethodName") public Collection getBlackListedWorkers() + { + return getBlacklistedWorkerInfos(); + } + + public Collection getBlacklistedWorkerInfos() { return ImmutableList.copyOf(Collections2.transform(blackListedWorkers.values(), WorkerHolder::toImmutable)); } @@ -1683,7 +1693,7 @@ public Map getLazyTaskSlotCount() public Map getBlacklistedTaskSlotCount() { Map 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 578bb7d9491d..931a192c7534 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 @@ 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()); } 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 a3a59d36adf8..45906919be15 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 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 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 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 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 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 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()); } 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 0e17cc4572db..c24a322f9ad9 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 @@ protected IntGrouper newGrouper() @Override protected void aggregateSingleValueDims(Grouper grouper) { - aggregateSingleValueDims((IntGrouper) grouper); + aggregateSingleValueDimsWithIntGrouper((IntGrouper) grouper); } @Override protected void aggregateMultiValueDims(Grouper 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. 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 54c81095bf0a..5ef7c8647ae2 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 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( 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 05ade15642a0..fa8374eaecf9 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 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)); 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 aa728b5aaf42..6ece9dfdca6b 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 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);