Add aggregation name validation matching REST API behavior

Apply the same aggregation name validation as REST API using
AggregatorFactories.VALID_AGG_NAME pattern in the gRPC registry.
This ensures aggregation names cannot contain '[', ']', or '>'
characters, matching REST validation behavior exactly.

Changes:
- Add name validation in AggregationBuilderProtoConverterSpiRegistry.fromProto()
- Remove outdated javadoc stating "Name validation is handled by the registry"
  from MinAggregationProtoUtils and MaxAggregationProtoUtils
- Add comprehensive tests for invalid aggregation name validation

Mirrors REST-side validation from AggregatorFactories#parseAggregators.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: yiyuabc

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/aggregation/AggregationBuilderProtoConverterRegistryImpl.java

@@ -20,7 +20,6 @@
 
 /**
  * Registry for AggregationBuilderProtoConverter implementations.
- * Wraps the SPI registry and registers built-in converters (Min, Max).
  */
 @Singleton
 public class AggregationBuilderProtoConverterRegistryImpl implements AggregationBuilderProtoConverterRegistry {

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/aggregation/AggregationBuilderProtoConverterSpiRegistry.java

@@ -20,6 +20,7 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.regex.Matcher;
 
 /**
  * SPI registry for AggregationBuilderProtoConverter implementations.
@@ -51,6 +52,13 @@ public AggregationBuilder fromProto(String name, AggregationContainer container)
             throw new IllegalArgumentException("Aggregation name cannot be null or empty");
         }
 
+        Matcher validAggMatcher = AggregatorFactories.VALID_AGG_NAME.matcher(name);
+        if (!validAggMatcher.matches()) {
+            throw new IllegalArgumentException(
+                "Invalid aggregation name [" + name + "]. Aggregation names can contain any character except '[', ']', and '>'"
+            );
+        }
+
         AggregationContainer.AggregationContainerCase aggregationCase = container.getAggregationContainerCase();
         AggregationBuilderProtoConverter converter = converterMap.get(aggregationCase);
 

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/aggregation/metrics/MaxAggregationBuilderProtoConverter.java

@@ -12,8 +12,7 @@
 import org.opensearch.transport.grpc.spi.AggregationBuilderProtoConverter;
 
 /**
- * Converter for Max aggregation. Delegates to {@link MaxAggregationProtoUtils}.
- * Mirrors REST parsing via {@link org.opensearch.search.aggregations.metrics.MaxAggregationBuilder#PARSER}.
+ * Converter for Max aggregation.
  */
 public class MaxAggregationBuilderProtoConverter implements AggregationBuilderProtoConverter {
 

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/aggregation/metrics/MaxAggregationProtoUtils.java

@@ -16,8 +16,6 @@
 
 /**
  * Utility class for converting MaxAggregation Protocol Buffers to MaxAggregationBuilder.
- *
- * @see MaxAggregationBuilder#PARSER
  */
 public class MaxAggregationProtoUtils {
 
@@ -31,9 +29,7 @@ private MaxAggregationProtoUtils() {
      * <p>This method parallels the REST parsing logic in {@link MaxAggregationBuilder#PARSER},
      * processing fields in the exact same sequence to ensure consistent validation and behavior.
      *
-     * <p>Name validation is handled by the registry before this method is called.
-     *
-     * @param name The name of the aggregation (validated by registry)
+     * @param name The name of the aggregation
      * @param maxAggProto The Protocol Buffer MaxAggregation to convert
      * @return A configured MaxAggregationBuilder
      * @throws IllegalArgumentException if required fields are missing or validation fails

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/aggregation/metrics/MinAggregationBuilderProtoConverter.java

@@ -12,8 +12,7 @@
 import org.opensearch.transport.grpc.spi.AggregationBuilderProtoConverter;
 
 /**
- * Converter for Min aggregation. Delegates to {@link MinAggregationProtoUtils}.
- * Mirrors REST parsing via {@link org.opensearch.search.aggregations.metrics.MinAggregationBuilder#PARSER}.
+ * Converter for Min aggregation.
  */
 public class MinAggregationBuilderProtoConverter implements AggregationBuilderProtoConverter {
 

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/aggregation/metrics/MinAggregationProtoUtils.java

@@ -16,8 +16,6 @@
 
 /**
  * Utility class for converting MinAggregation Protocol Buffers to MinAggregationBuilder.
- *
- * @see MinAggregationBuilder#PARSER
  */
 public class MinAggregationProtoUtils {
 
@@ -31,9 +29,7 @@ private MinAggregationProtoUtils() {
      * <p>This method parallels the REST parsing logic in {@link MinAggregationBuilder#PARSER},
      * processing fields in the exact same sequence to ensure consistent validation and behavior.
      *
-     * <p>Name validation is handled by the registry before this method is called.
-     *
-     * @param name The name of the aggregation (validated by registry)
+     * @param name The name of the aggregation
      * @param minAggProto The Protocol Buffer MinAggregation to convert
      * @return A configured MinAggregationBuilder
      * @throws IllegalArgumentException if required fields are missing or validation fails

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/aggregation/support/ValuesSourceAggregationProtoUtils.java

@@ -18,7 +18,6 @@
 
 /**
  * Utility class for parsing common fields from ValuesSource-based aggregation Protocol Buffer messages.
- * Mirrors {@link ValuesSourceAggregationBuilder#declareFields} using {@link ObjectParserProtoUtils}.
  */
 public class ValuesSourceAggregationProtoUtils {
 

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/request/search/aggregation/support/ValuesSourceProtoFields.java

@@ -12,9 +12,6 @@
 /**
  * Wrapper for common ValuesSource aggregation fields extracted from proto messages.
  * Reduces parameter count in {@link ValuesSourceAggregationProtoUtils#declareFields}.
- *
- * <p>All fields use null to indicate "not set". The proto3 limitation (strings default to "",
- * enums default to first value) is handled at the proto boundary when constructing this wrapper.
  */
 public class ValuesSourceProtoFields {
     private final String field;

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/aggregation/metrics/package-info.java

@@ -5,8 +5,6 @@
 /**
  * Protocol Buffer utilities for metrics aggregation responses.
  * Contains converters from OpenSearch internal metrics aggregation results to Protocol Buffer messages.
- * <p>
- * Metrics aggregations compute metrics (like min, max, avg, sum, cardinality) over a set of documents.
  *
  * @see org.opensearch.search.aggregations.metrics
  */

modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/request/search/aggregation/AggregationBuilderProtoConverterRegistryTests.java

@@ -288,4 +288,55 @@ public void testNestedAggregationsInfrastructure() {
         //     exception.getMessage().contains("cannot accept sub-aggregations")
         // );
     }
+
+    /**
+     * Test that invalid aggregation names are rejected.
+     * Verifies that aggregation name validation matches REST API behavior.
+     * See {@link org.opensearch.search.aggregations.AggregatorFactories#parseAggregators}.
+     */
+    public void testInvalidAggregationNames() {
+        AggregationBuilderProtoConverterRegistryImpl registry = new AggregationBuilderProtoConverterRegistryImpl();
+        AggregationContainer container = AggregationContainer.newBuilder()
+            .setMin(MinAggregation.newBuilder().setField("price").build())
+            .build();
+
+        // Test name with '[' character
+        IllegalArgumentException ex1 = expectThrows(
+            IllegalArgumentException.class,
+            () -> registry.fromProto("invalid[name", container)
+        );
+        assertTrue(
+            "Exception should mention invalid aggregation name",
+            ex1.getMessage().contains("Invalid aggregation name")
+        );
+        assertTrue(
+            "Exception should mention forbidden characters",
+            ex1.getMessage().contains("'[', ']', and '>'")
+        );
+
+        // Test name with ']' character
+        IllegalArgumentException ex2 = expectThrows(
+            IllegalArgumentException.class,
+            () -> registry.fromProto("invalid]name", container)
+        );
+        assertTrue(
+            "Exception should mention invalid aggregation name",
+            ex2.getMessage().contains("Invalid aggregation name")
+        );
+
+        // Test name with '>' character
+        IllegalArgumentException ex3 = expectThrows(
+            IllegalArgumentException.class,
+            () -> registry.fromProto("invalid>name", container)
+        );
+        assertTrue(
+            "Exception should mention invalid aggregation name",
+            ex3.getMessage().contains("Invalid aggregation name")
+        );
+
+        // Test that valid names work fine
+        AggregationBuilder validBuilder = registry.fromProto("valid_name-with.dots:and_more", container);
+        assertNotNull("Valid aggregation name should be accepted", validBuilder);
+        assertEquals("valid_name-with.dots:and_more", validBuilder.getName());
+    }
 }