address comments

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

server/src/main/java/org/opensearch/cluster/metadata/QueryGroup.java

@@ -11,7 +11,7 @@
 import org.opensearch.cluster.AbstractDiffable;
 import org.opensearch.cluster.Diff;
 import org.opensearch.common.UUIDs;
-import org.opensearch.common.annotation.PublicApi;
+import org.opensearch.common.annotation.ExperimentalApi;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
 import org.opensearch.core.xcontent.ToXContentObject;
@@ -37,10 +37,10 @@
  *              "updatedAt": 4513232415
  * }
  */
-@PublicApi(since = "2.15")
+@ExperimentalApi
 public class QueryGroup extends AbstractDiffable<QueryGroup> implements ToXContentObject {
 
-    public static final int MAX_CHARS_ALLOWED_IN_NAME = 50;
+    private static final int MAX_CHARS_ALLOWED_IN_NAME = 50;
     private final String name;
     private final String _id;
     private final ResiliencyMode resiliencyMode;
@@ -49,8 +49,8 @@
     private final Map<ResourceType, Object> resourceLimits;
 
     public QueryGroup(String name, ResiliencyMode resiliencyMode, Map<ResourceType, Object> resourceLimits) {
         this(name, UUIDs.randomBase64UUID(), resiliencyMode, resourceLimits, Instant.now().getMillis());
     }
 
     public QueryGroup(String name, String _id, ResiliencyMode resiliencyMode, Map<ResourceType, Object> resourceLimits, long updatedAt) {
         Objects.requireNonNull(name, "QueryGroup.name can't be null");
@@ -59,7 +59,7 @@
         Objects.requireNonNull(_id, "QueryGroup._id can't be null");
 
         if (name.length() > MAX_CHARS_ALLOWED_IN_NAME) {
             throw new IllegalArgumentException("QueryGroup.name shouldn't be more than 50 chars long");
         }
 
         if (resourceLimits.isEmpty()) {
@@ -67,7 +67,7 @@
         }
         validateResourceLimits(resourceLimits);
         if (!isValid(updatedAt)) {
             throw new IllegalArgumentException("QueryGroup.updatedAtInMillis is not a valid epoch");
         }
 
         this.name = name;
@@ -148,13 +148,13 @@
         XContentParser.Token token = parser.currentToken();
 
         if (token != XContentParser.Token.START_OBJECT) {
             throw new IllegalArgumentException("Expected START_OBJECT token but found [" + parser.currentName() + "]");
         }
 
         String fieldName = "";
         // Map to hold resources
         final Map<ResourceType, Object> resourceLimits = new HashMap<>();
-        while ((token = parser.nextToken()) != null) {
+        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
             if (token == XContentParser.Token.FIELD_NAME) {
                 fieldName = parser.currentName();
             } else if (token.isValue()) {
@@ -167,12 +167,12 @@
                 } else if (fieldName.equals("updatedAt")) {
                     builder.updatedAt(parser.longValue());
                 } else {
                     throw new IllegalArgumentException(fieldName + " is not a valid field in QueryGroup");
                 }
             } else if (token == XContentParser.Token.START_OBJECT) {
 
                 if (!fieldName.equals("resourceLimits")) {
                     throw new IllegalArgumentException(
                         "QueryGroup.resourceLimits is an object and expected token was { " + " but found " + token
                     );
                 }
@@ -245,7 +245,7 @@
      * ENFORCED - means that it will never breach the assigned limits and will cancel as soon as the limits are breached
      * MONITOR - it will not cause any cancellation but just log the eligible task cancellations
      */
-    @PublicApi(since = "2.15")
+    @ExperimentalApi
     public enum ResiliencyMode {
         SOFT("soft"),
         ENFORCED("enforced"),
@@ -266,7 +266,7 @@
                 if (mode.getName().equalsIgnoreCase(s)) return mode;
 
             }
             throw new IllegalArgumentException("Invalid value for QueryGroupMode: " + s);
         }
 
     }
@@ -274,7 +274,7 @@
     /**
      * Builder class for {@link QueryGroup}
      */
-    @PublicApi(since = "2.15")
+    @ExperimentalApi
     public static class Builder {
         private String name;
         private String _id;

server/src/main/java/org/opensearch/cluster/metadata/QueryGroupMetadata.java

@@ -16,7 +16,6 @@
 import org.opensearch.core.common.Strings;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.common.io.stream.StreamOutput;
-import org.opensearch.core.xcontent.ConstructingObjectParser;
 import org.opensearch.core.xcontent.MediaTypeRegistry;
 import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.core.xcontent.XContentParser;
@@ -47,22 +46,6 @@
 
     private final Map<String, QueryGroup> queryGroups;
 
-    @SuppressWarnings("unchecked")
-    static final ConstructingObjectParser<QueryGroupMetadata, Void> PARSER = new ConstructingObjectParser<>(
-        "queryGroupsParser",
-        args -> new QueryGroupMetadata((Map<String, QueryGroup>) args[0])
-    );
-
-    static {
-        PARSER.declareObjectArray(ConstructingObjectParser.constructorArg(), (p, c) -> {
-            Map<String, QueryGroup> queryGroupMap = new HashMap<>();
-            while (p.nextToken() != XContentParser.Token.END_OBJECT) {
-                queryGroupMap.put(p.currentName(), QueryGroup.fromXContent(p));
-            }
-            return queryGroupMap;
-        }, QUERY_GROUP_FIELD);
-    }
-
     public QueryGroupMetadata(Map<String, QueryGroup> queryGroups) {
         this.queryGroups = queryGroups;
     }
@@ -72,7 +55,7 @@
     }
 
     public Map<String, QueryGroup> queryGroups() {
         return this.queryGroups;
     }
 
     /**
@@ -80,7 +63,7 @@
      */
     @Override
     public String getWriteableName() {
         return TYPE;
     }
 
     /**
@@ -88,7 +71,7 @@
      */
     @Override
     public Version getMinimalSupportedVersion() {
         return Version.V_3_0_0;
     }
 
     @Override
@@ -105,7 +88,29 @@
     }
 
     public static QueryGroupMetadata fromXContent(XContentParser parser) throws IOException {
-        return PARSER.parse(parser, null);
+        Map<String, QueryGroup> queryGroupMap = new HashMap<>();
+
+        if (parser.currentToken() == null) {
+            parser.nextToken();
+        }
+
+        if (parser.currentToken() != XContentParser.Token.START_OBJECT) {
+            throw new IllegalArgumentException(
+                "QueryGroupMetadata.fromXContent was expecting a { token but found : " + parser.currentToken()
+            );
+        }
+        XContentParser.Token token = parser.currentToken();
+        String fieldName = parser.currentName();
+        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
+            if (token == XContentParser.Token.FIELD_NAME) {
+                fieldName = parser.currentName();
+            } else {
+                QueryGroup queryGroup = QueryGroup.fromXContent(parser);
+                queryGroupMap.put(fieldName, queryGroup);
+            }
+        }
+
+        return new QueryGroupMetadata(queryGroupMap);
     }
 
     @Override
@@ -119,7 +124,7 @@
 
     @Override
     public EnumSet<Metadata.XContentContext> context() {
         return ALL_CONTEXTS;
     }
 
     @Override
@@ -137,7 +142,7 @@
 
     @Override
     public String toString() {
         return Strings.toString(MediaTypeRegistry.JSON, this);
     }
 
     /**
@@ -164,7 +169,7 @@
          */
         @Override
         public String getWriteableName() {
             return TYPE;
         }
 
         @Override

server/src/main/java/org/opensearch/search/ResourceType.java

@@ -16,7 +16,7 @@
 /**
  * Enum to hold the resource type
  */
-@PublicApi(since = "1.3")
+@PublicApi(since = "2.x")
 public enum ResourceType {
     CPU("cpu"),
     MEMORY("memory");

server/src/test/java/org/opensearch/cluster/metadata/QueryGroupMetadataTests.java

@@ -8,15 +8,46 @@
 
 package org.opensearch.cluster.metadata;
 
+import org.opensearch.cluster.Diff;
+import org.opensearch.common.xcontent.json.JsonXContent;
 import org.opensearch.core.common.io.stream.NamedWriteableRegistry;
-import org.opensearch.test.AbstractNamedWriteableTestCase;
+import org.opensearch.core.common.io.stream.Writeable;
+import org.opensearch.core.xcontent.XContentBuilder;
+import org.opensearch.core.xcontent.XContentParser;
+import org.opensearch.search.ResourceType;
+import org.opensearch.test.AbstractDiffableSerializationTestCase;
 
+import java.io.IOException;
 import java.util.Collections;
 import java.util.Map;
 
 import static org.opensearch.cluster.metadata.QueryGroupTests.createRandomQueryGroup;
 
-public class QueryGroupMetadataTests extends AbstractNamedWriteableTestCase<QueryGroupMetadata> {
+public class QueryGroupMetadataTests extends AbstractDiffableSerializationTestCase<Metadata.Custom> {
+
+    public void testToXContent() throws IOException {
+        long updatedAt = 1720047207;
+        QueryGroupMetadata queryGroupMetadata = new QueryGroupMetadata(
+            Map.of(
+                "ajakgakg983r92_4242",
+                new QueryGroup(
+                    "test",
+                    "ajakgakg983r92_4242",
+                    QueryGroup.ResiliencyMode.ENFORCED,
+                    Map.of(ResourceType.MEMORY, 0.5),
+                    updatedAt
+                )
+            )
+        );
+        XContentBuilder builder = JsonXContent.contentBuilder();
+        builder.startObject();
+        queryGroupMetadata.toXContent(builder, null);
+        builder.endObject();
+        assertEquals(
+            "{\"ajakgakg983r92_4242\":{\"_id\":\"ajakgakg983r92_4242\",\"name\":\"test\",\"resiliency_mode\":\"enforced\",\"updatedAt\":1720047207,\"resourceLimits\":{\"memory\":0.5}}}",
+            builder.toString()
+        );
+    }
 
     @Override
     protected NamedWriteableRegistry getNamedWriteableRegistry() {
@@ -28,8 +59,25 @@ protected NamedWriteableRegistry getNamedWriteableRegistry() {
     }
 
     @Override
-    protected Class<QueryGroupMetadata> categoryClass() {
-        return QueryGroupMetadata.class;
+    protected Metadata.Custom makeTestChanges(Metadata.Custom testInstance) {
+        final QueryGroup queryGroup = createRandomQueryGroup("asdfakgjwrir23r25");
+        final QueryGroupMetadata queryGroupMetadata = new QueryGroupMetadata(Map.of(queryGroup.get_id(), queryGroup));
+        return queryGroupMetadata;
+    }
+
+    @Override
+    protected Writeable.Reader<Diff<Metadata.Custom>> diffReader() {
+        return QueryGroupMetadata::readDiffFrom;
+    }
+
+    @Override
+    protected Metadata.Custom doParseInstance(XContentParser parser) throws IOException {
+        return QueryGroupMetadata.fromXContent(parser);
+    }
+
+    @Override
+    protected Writeable.Reader<Metadata.Custom> instanceReader() {
+        return QueryGroupMetadata::new;
     }
 
     @Override