Rename partition spec fields

core/src/main/java/org/apache/druid/indexer/partitions/Checks.java

@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexer.partitions;
+
+/**
+ * Various helper methods useful for checking the validity of arguments to spec constructors.
+ */
+class Checks
+{
+  /**
+   * @return Non-null value, or first one if both are null
+   */
+  @SuppressWarnings("VariableNotUsedInsideIf")  // false positive: checked for 'null' not used inside 'if
+  static Property<Integer> checkAtMostOneNotNull(String name1, Integer value1, String name2, Integer value2)
+  {
+    final Property<Integer> property;
+
+    if (value1 == null && value2 == null) {
+      property = new Property<>(name1, value1);
+    } else if (value1 == null) {
+      property = new Property<>(name2, value2);
+    } else if (value2 == null) {
+      property = new Property<>(name1, value1);
+    } else {
+      throw new IllegalArgumentException("At most one of " + name1 + " or " + name2 + " must be present");
+    }
+
+    return property;
+  }
+}

core/src/main/java/org/apache/druid/indexer/partitions/DimensionBasedPartitionsSpec.java

@@ -26,5 +26,11 @@
  */
 public interface DimensionBasedPartitionsSpec extends PartitionsSpec
 {
+  String TARGET_ROWS_PER_SEGMENT = "targetRowsPerSegment";
+
+  // Deprecated properties preserved for backward compatibility:
+  @Deprecated
+  String TARGET_PARTITION_SIZE = "targetPartitionSize";
+
   List<String> getPartitionDimensions();
 }

core/src/main/java/org/apache/druid/indexer/partitions/DynamicPartitionsSpec.java

@@ -31,13 +31,14 @@
 public class DynamicPartitionsSpec implements PartitionsSpec
 {
   public static final long DEFAULT_MAX_TOTAL_ROWS = 20_000_000;
+  static final String NAME = "dynamic";
 
   private final int maxRowsPerSegment;
   private final long maxTotalRows;
 
   @JsonCreator
   public DynamicPartitionsSpec(
-      @JsonProperty("maxRowsPerSegment") @Nullable Integer maxRowsPerSegment,
+      @JsonProperty(PartitionsSpec.MAX_ROWS_PER_SEGMENT) @Nullable Integer maxRowsPerSegment,
       @JsonProperty("maxTotalRows") @Nullable Long maxTotalRows
   )
   {

core/src/main/java/org/apache/druid/indexer/partitions/HashedPartitionsSpec.java

@@ -31,6 +31,7 @@
 
 public class HashedPartitionsSpec implements DimensionBasedPartitionsSpec
 {
+  static final String NAME = "hashed";
   private static final Logger LOG = new Logger(HashedPartitionsSpec.class);
 
   @Nullable
@@ -41,34 +42,37 @@ public class HashedPartitionsSpec implements DimensionBasedPartitionsSpec
 
   public static HashedPartitionsSpec defaultSpec()
   {
-    return new HashedPartitionsSpec(null, null, null, null);
-  }
-
-  public HashedPartitionsSpec(
-      @Nullable Integer maxRowsPerSegment,
-      @Nullable Integer numShards,
-      @Nullable List<String> partitionDimensions
-  )
-  {
-    this(null, maxRowsPerSegment, numShards, partitionDimensions);
+    return new HashedPartitionsSpec(null, null, null, null, null);
   }
 
   @JsonCreator
   public HashedPartitionsSpec(
-      @JsonProperty("targetPartitionSize") @Deprecated @Nullable Integer targetPartitionSize,
-      @JsonProperty("maxRowsPerSegment") @Nullable Integer maxRowsPerSegment,
+      @JsonProperty(DimensionBasedPartitionsSpec.TARGET_ROWS_PER_SEGMENT) @Nullable Integer targetRowsPerSegment,
       @JsonProperty("numShards") @Nullable Integer numShards,
-      @JsonProperty("partitionDimensions") @Nullable List<String> partitionDimensions
+      @JsonProperty("partitionDimensions") @Nullable List<String> partitionDimensions,
+
+      // Deprecated properties preserved for backward compatibility:
+      @Deprecated @JsonProperty(DimensionBasedPartitionsSpec.TARGET_PARTITION_SIZE) @Nullable
+          Integer targetPartitionSize,
+      @Deprecated @JsonProperty(PartitionsSpec.MAX_ROWS_PER_SEGMENT) @Nullable
+          Integer maxRowsPerSegment
   )
   {
+    Property<Integer> target = Checks.checkAtMostOneNotNull(
+        DimensionBasedPartitionsSpec.TARGET_ROWS_PER_SEGMENT,
+        targetRowsPerSegment,
+        DimensionBasedPartitionsSpec.TARGET_PARTITION_SIZE,
+        targetPartitionSize
+    );
+
     Preconditions.checkArgument(
-        PartitionsSpec.isEffectivelyNull(targetPartitionSize) || PartitionsSpec.isEffectivelyNull(maxRowsPerSegment),
-        "Can't set both targetPartitionSize and maxRowsPerSegment"
+        PartitionsSpec.isEffectivelyNull(target.getValue()) || PartitionsSpec.isEffectivelyNull(maxRowsPerSegment),
+        "Can't set both " + target.getName() + " and maxRowsPerSegment"
     );
-    final Integer realMaxRowsPerSegment = targetPartitionSize == null ? maxRowsPerSegment : targetPartitionSize;
+    final Integer realMaxRowsPerSegment = target.getValue() == null ? maxRowsPerSegment : target.getValue();
     Preconditions.checkArgument(
         PartitionsSpec.isEffectivelyNull(realMaxRowsPerSegment) || PartitionsSpec.isEffectivelyNull(numShards),
-        "Can't use maxRowsPerSegment or targetPartitionSize and numShards together"
+        "Can't use maxRowsPerSegment or " + target.getName() + " and numShards together"
     );
     // Needs to determine partitions if the _given_ numShards is null
     this.maxRowsPerSegment = getValidMaxRowsPerSegment(realMaxRowsPerSegment, numShards);
@@ -100,6 +104,16 @@ public HashedPartitionsSpec(
     }
   }
 
+  public HashedPartitionsSpec(
+      @Nullable Integer maxRowsPerSegment,
+      @Nullable Integer numShards,
+      @Nullable List<String> partitionDimensions
+  )
+  {
+    this(null, numShards, partitionDimensions, null, maxRowsPerSegment);
+  }
+
+
   private static boolean needsDeterminePartitions(@Nullable Integer numShards)
   {
     return PartitionsSpec.isEffectivelyNull(numShards);

core/src/main/java/org/apache/druid/indexer/partitions/PartitionsSpec.java

@@ -29,14 +29,15 @@
  */
 @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type", defaultImpl = HashedPartitionsSpec.class)
 @JsonSubTypes(value = {
-    @JsonSubTypes.Type(name = "single_dim", value = SingleDimensionPartitionsSpec.class),
-    @JsonSubTypes.Type(name = "dimension", value = SingleDimensionPartitionsSpec.class), // for backward compatibility
-    @JsonSubTypes.Type(name = "hashed", value = HashedPartitionsSpec.class),
-    @JsonSubTypes.Type(name = "dynamic", value = DynamicPartitionsSpec.class)
+    @JsonSubTypes.Type(name = SingleDimensionPartitionsSpec.NAME, value = SingleDimensionPartitionsSpec.class),
+    @JsonSubTypes.Type(name = SingleDimensionPartitionsSpec.OLD_NAME, value = SingleDimensionPartitionsSpec.class),  // for backward compatibility
+    @JsonSubTypes.Type(name = HashedPartitionsSpec.NAME, value = HashedPartitionsSpec.class),
+    @JsonSubTypes.Type(name = DynamicPartitionsSpec.NAME, value = DynamicPartitionsSpec.class)
 })
 public interface PartitionsSpec
 {
   int DEFAULT_MAX_ROWS_PER_SEGMENT = 5_000_000;
+  String MAX_ROWS_PER_SEGMENT = "maxRowsPerSegment";
 
   /**
    * Returns the max number of rows per segment.

core/src/main/java/org/apache/druid/indexer/partitions/Property.java

@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexer.partitions;
+
+import java.util.Objects;
+
+/**
+ * Convenience class for holding a pair of string key and templated value.
+ */
+class Property<T>
+{
+  private final String name;
+  private final T value;
+
+  Property(String name, T value)
+  {
+    this.name = name;
+    this.value = value;
+  }
+
+  public String getName()
+  {
+    return name;
+  }
+
+  public T getValue()
+  {
+    return value;
+  }
+
+  @Override
+  public boolean equals(Object o)
+  {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
+    Property<?> property = (Property<?>) o;
+    return Objects.equals(name, property.name) &&
+           Objects.equals(value, property.value);
+  }
+
+  @Override
+  public int hashCode()
+  {
+    return Objects.hash(name, value);
+  }
+}