Properly configure settings update consumer

Signed-off-by: Siddhant Deshmukh <deshsid@amazon.com>
opensearch-project · Aug 2, 2024 · b39ca16 · b39ca16
1 parent d985a3e
commit b39ca16
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 10 deletions.
diff --git a/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java b/src/main/java/org/opensearch/plugin/insights/core/listener/QueryInsightsListener.java
@@ -86,8 +86,13 @@ public QueryInsightsListener(final ClusterService clusterService, final QueryIns
             this.queryInsightsService.setWindowSize(type, clusterService.getClusterSettings().get(getTopNWindowSizeSetting(type)));
         }
         clusterService.getClusterSettings()
-            .addSettingsUpdateConsumer(TOP_N_QUERIES_GROUP_BY, v -> this.queryInsightsService.validateAndSetGrouping(v));
-        this.queryInsightsService.validateAndSetGrouping(clusterService.getClusterSettings().get(TOP_N_QUERIES_GROUP_BY));
+            .addSettingsUpdateConsumer(
+                TOP_N_QUERIES_GROUP_BY,
+                v -> this.queryInsightsService.setGrouping(v),
+                v -> this.queryInsightsService.validateGrouping(v)
+            );
+        this.queryInsightsService.validateGrouping(clusterService.getClusterSettings().get(TOP_N_QUERIES_GROUP_BY));
+        this.queryInsightsService.setGrouping(clusterService.getClusterSettings().get(TOP_N_QUERIES_GROUP_BY));
     }
 
     /**

diff --git a/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java b/src/main/java/org/opensearch/plugin/insights/core/service/QueryInsightsService.java
@@ -196,11 +196,18 @@ public void enableCollection(final MetricType metricType, final boolean enable)
     }
 
     /**
-     * Set grouping based on the given GroupingType setting for the given metric type
-     *
-     * @param groupingTypeSetting grouping type setting
+     * Validate grouping
+     * @param groupingTypeSetting grouping
+     */
+    public void validateGrouping(final String groupingTypeSetting) {
+        GroupingType.getGroupingTypeFromSettingAndValidate(groupingTypeSetting);
+    }
+
+    /**
+     * Set grouping
+     * @param groupingTypeSetting grouping
      */
-    public void validateAndSetGrouping(final String groupingTypeSetting) {
+    public void setGrouping(final String groupingTypeSetting) {
         GroupingType newGroupingType = GroupingType.getGroupingTypeFromSettingAndValidate(groupingTypeSetting);
         GroupingType oldGroupingType = groupingType;
 

diff --git a/src/main/java/org/opensearch/plugin/insights/rules/model/GroupingType.java b/src/main/java/org/opensearch/plugin/insights/rules/model/GroupingType.java
@@ -50,7 +50,7 @@ public static GroupingType getGroupingTypeFromSettingAndValidate(String settingV
             return GroupingType.valueOf(settingValue.toUpperCase(Locale.ROOT));
         } catch (IllegalArgumentException e) {
             throw new IllegalArgumentException(
-                String.format(Locale.ROOT, "Invalid exporter type [%s], type should be one of %s", settingValue, allGroupingTypes())
+                String.format(Locale.ROOT, "Invalid grouping type [%s], type should be one of %s", settingValue, allGroupingTypes())
             );
         }
     }

diff --git a/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java b/src/test/java/org/opensearch/plugin/insights/core/service/QueryInsightsServiceTests.java
@@ -120,7 +120,7 @@ public void testAddRecordGroupBySimilarityWithDifferentGroups() {
             5
         );
 
-        queryInsightsService.validateAndSetGrouping(GroupingType.SIMILARITY.getValue());
+        queryInsightsService.setGrouping(GroupingType.SIMILARITY.getValue());
         assertEquals(queryInsightsService.getGrouping(), GroupingType.SIMILARITY);
 
         for (int i = 0; i < numberOfRecordsRequired; i++) {
@@ -146,7 +146,7 @@ public void testAddRecordGroupBySimilarityWithOneGroup() {
         );
         QueryInsightsTestUtils.populateSameQueryHashcodes(records);
 
-        queryInsightsService.validateAndSetGrouping(GroupingType.SIMILARITY.getValue());
+        queryInsightsService.setGrouping(GroupingType.SIMILARITY.getValue());
         assertEquals(queryInsightsService.getGrouping(), GroupingType.SIMILARITY);
 
         for (int i = 0; i < numberOfRecordsRequired; i++) {
@@ -166,7 +166,7 @@ public void testAddRecordGroupBySimilarityWithTwoGroups() {
         List<SearchQueryRecord> records2 = QueryInsightsTestUtils.generateQueryInsightRecords(2, 2, System.currentTimeMillis(), 0);
         QueryInsightsTestUtils.populateHashcode(records2, 2);
 
-        queryInsightsService.validateAndSetGrouping(GroupingType.SIMILARITY.getValue());
+        queryInsightsService.setGrouping(GroupingType.SIMILARITY.getValue());
         assertEquals(queryInsightsService.getGrouping(), GroupingType.SIMILARITY);
 
         for (int i = 0; i < 2; i++) {