Delete doesn't need reloading 🤦 removing it

Author: carlosdelest

rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json

@@ -33,12 +33,6 @@
           }
         }
       ]
-    },
-    "params": {
-      "refresh": {
-        "type": "boolean",
-        "description": "Refresh search analyzers to update synonyms"
-      }
     }
   }
 }

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml

@@ -44,27 +44,6 @@ setup:
         - synonyms: "test => check"
           id: "test-id-3"
 
----
-"Refresh can be specified":
-
-  - requires:
-      test_runner_features: [ capabilities ]
-      capabilities:
-        - method: PUT
-          path: /_synonyms/{rule_id}
-          capabilities: [ synonyms_refresh_param ]
-      reason: "synonyms refresh param capability needed"
-
-  - do:
-      synonyms.delete_synonym_rule:
-        set_id: test-synonyms
-        rule_id: test-id-2
-        refresh: false
-
-  - match: { result: "deleted" }
-  # Reload analyzers info is not included
-  - not_exists: reload_analyzers_details
-
 ---
 "Delete synonym rule - missing synonym set":
   - do:

server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java

@@ -366,25 +366,6 @@ public void onFailure(Exception e) {
 
         updateLatch.await(5, TimeUnit.SECONDS);
 
-        // Delete a rule fails
-        CountDownLatch deleteLatch = new CountDownLatch(1);
-        synonymsManagementAPIService.deleteSynonymRule(synonymSetId, synonymRuleId, true, new ActionListener<>() {
-            @Override
-            public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) {
-                fail("Shouldn't have been able to delete a synonym rule with refresh in synonyms index health");
-            }
-
-            @Override
-            public void onFailure(Exception e) {
-                // Expected
-                assertTrue(e instanceof IndexCreationException);
-                assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable"));
-                updateLatch.countDown();
-            }
-        });
-
-        deleteLatch.await(5, TimeUnit.SECONDS);
-
         // But, we can still create a synonyms set without refresh
         CountDownLatch putNoRefreshLatch = new CountDownLatch(1);
         synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(1, 1), false, new ActionListener<>() {
@@ -418,21 +399,5 @@ public void onFailure(Exception e) {
         });
 
         putRuleNoRefreshLatch.await(5, TimeUnit.SECONDS);
-
-        // Same for delete
-        CountDownLatch deleteNoRefreshLatch = new CountDownLatch(1);
-        synonymsManagementAPIService.deleteSynonymRule(synonymSetId, synonymRuleId, false, new ActionListener<>() {
-            @Override
-            public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) {
-                deleteNoRefreshLatch.countDown();
-            }
-
-            @Override
-            public void onFailure(Exception e) {
-                fail(e);
-            }
-        });
-
-        deleteNoRefreshLatch.await(5, TimeUnit.SECONDS);
     }
 }

server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java

@@ -9,7 +9,6 @@
 
 package org.elasticsearch.action.synonyms;
 
-import org.elasticsearch.TransportVersions;
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.ActionType;
@@ -32,24 +31,18 @@ public DeleteSynonymRuleAction() {
 
     public static class Request extends ActionRequest {
         private final String synonymsSetId;
+
         private final String synonymRuleId;
-        private final boolean refresh;
 
         public Request(StreamInput in) throws IOException {
             super(in);
             this.synonymsSetId = in.readString();
             this.synonymRuleId = in.readString();
-            if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) {
-                this.refresh = in.readBoolean();
-            } else {
-                this.refresh = true;
-            }
         }
 
-        public Request(String synonymsSetId, String synonymRuleId, boolean refresh) {
+        public Request(String synonymsSetId, String synonymRuleId) {
             this.synonymsSetId = synonymsSetId;
             this.synonymRuleId = synonymRuleId;
-            this.refresh = refresh;
         }
 
         @Override
@@ -70,9 +63,6 @@ public void writeTo(StreamOutput out) throws IOException {
             super.writeTo(out);
             out.writeString(synonymsSetId);
             out.writeString(synonymRuleId);
-            if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) {
-                out.writeBoolean(refresh);
-            }
         }
 
         public String synonymsSetId() {
@@ -83,10 +73,6 @@ public String synonymRuleId() {
             return synonymRuleId;
         }
 
-        public boolean refresh() {
-            return refresh;
-        }
-
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;

server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java

@@ -41,7 +41,6 @@ protected void doExecute(Task task, DeleteSynonymRuleAction.Request request, Act
         synonymsManagementAPIService.deleteSynonymRule(
             request.synonymsSetId(),
             request.synonymRuleId(),
-            request.refresh(),
             listener.map(SynonymUpdateResponse::new)
         );
     }

server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java

@@ -40,8 +40,7 @@ public List<Route> routes() {
     protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
         DeleteSynonymRuleAction.Request request = new DeleteSynonymRuleAction.Request(
             restRequest.param("synonymsSet"),
-            restRequest.param("synonymRuleId"),
-            restRequest.paramAsBoolean("refresh", true)
+            restRequest.param("synonymRuleId")
         );
         return channel -> client.execute(DeleteSynonymRuleAction.INSTANCE, request, new RestToXContentListener<>(channel));
     }

server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java

@@ -354,7 +354,7 @@ public void putSynonymsSet(
                         ? UpdateSynonymsResultStatus.CREATED
                         : UpdateSynonymsResultStatus.UPDATED;
 
-                    maybeReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener);
+                    checkIndexSearchableAndReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener);
                 })
             );
         }));
@@ -424,7 +424,7 @@ private void indexSynonymRule(
                 ? UpdateSynonymsResultStatus.CREATED
                 : UpdateSynonymsResultStatus.UPDATED;
 
-            maybeReloadAnalyzers(synonymsSetId, refresh, false, updateStatus, l2);
+            checkIndexSearchableAndReloadAnalyzers(synonymsSetId, refresh, false, updateStatus, l2);
         }));
     }
 
@@ -444,12 +444,7 @@ public void getSynonymRule(String synonymSetId, String synonymRuleId, ActionList
         );
     }
 
-    public void deleteSynonymRule(
-        String synonymsSetId,
-        String synonymRuleId,
-        boolean refresh,
-        ActionListener<SynonymsReloadResult> listener
-    ) {
+    public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, ActionListener<SynonymsReloadResult> listener) {
         client.prepareDelete(SYNONYMS_ALIAS_NAME, internalSynonymRuleId(synonymsSetId, synonymRuleId))
             .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
             .execute(new DelegatingIndexNotFoundActionListener<>(synonymsSetId, listener, (l, deleteResponse) -> {
@@ -468,7 +463,7 @@ public void deleteSynonymRule(
                     return;
                 }
 
-                maybeReloadAnalyzers(synonymsSetId, refresh, false, UpdateSynonymsResultStatus.DELETED, listener);
+                reloadAnalyzers(synonymsSetId, false, UpdateSynonymsResultStatus.DELETED, listener);
             }));
     }
 
@@ -525,8 +520,8 @@ private void deleteSynonymsSetObjects(String synonymSetId, ActionListener<BulkBy
 
     public void deleteSynonymsSet(String synonymSetId, ActionListener<AcknowledgedResponse> listener) {
 
-        // Previews reloading the resource to understand its usage on indices
-        maybeReloadAnalyzers(synonymSetId, true, true, null, listener.delegateFailure((reloadListener, reloadResult) -> {
+        // Previews reloading the resource to understand its usage on indices. It's OK to reload as we're doing preview mode
+        reloadAnalyzers(synonymSetId, true, null, listener.delegateFailure((reloadListener, reloadResult) -> {
             Map<String, ReloadAnalyzersResponse.ReloadDetails> reloadDetails = reloadResult.reloadAnalyzersResponse.getReloadDetails();
             if (reloadDetails.isEmpty() == false) {
                 Set<String> indices = reloadDetails.entrySet()
@@ -566,7 +561,7 @@ public void deleteSynonymsSet(String synonymSetId, ActionListener<AcknowledgedRe
         }));
     }
 
-    private <T> void maybeReloadAnalyzers(
+    private <T> void checkIndexSearchableAndReloadAnalyzers(
         String synonymSetId,
         boolean refresh,
         boolean preview,
@@ -596,16 +591,20 @@ private <T> void maybeReloadAnalyzers(
                 return;
             }
 
-            // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters)
-            ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*");
-            client.execute(
-                TransportReloadAnalyzersAction.TYPE,
-                reloadAnalyzersRequest,
-                listener.safeMap(reloadResponse -> new SynonymsReloadResult(synonymsOperationResult, reloadResponse))
-            );
+            reloadAnalyzers(synonymSetId, preview, synonymsOperationResult, listener);
         }));
     }
 
+    private void reloadAnalyzers(String synonymSetId, boolean preview, UpdateSynonymsResultStatus synonymsOperationResult, ActionListener<SynonymsReloadResult> listener) {
+        // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters)
+        ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*");
+        client.execute(
+            TransportReloadAnalyzersAction.TYPE,
+            reloadAnalyzersRequest,
+            listener.safeMap(reloadResponse -> new SynonymsReloadResult(synonymsOperationResult, reloadResponse))
+        );
+    }
+
     // Allows checking failures in tests
     void checkSynonymsIndexHealth(ActionListener<ClusterHealthResponse> listener) {
         ClusterHealthRequest healthRequest = new ClusterHealthRequest(

server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java

@@ -23,7 +23,7 @@ protected Writeable.Reader<DeleteSynonymRuleAction.Request> instanceReader() {
 
     @Override
     protected DeleteSynonymRuleAction.Request createTestInstance() {
-        return new DeleteSynonymRuleAction.Request(randomIdentifier(), randomIdentifier(), randomBoolean());
+        return new DeleteSynonymRuleAction.Request(randomIdentifier(), randomIdentifier());
     }
 
     @Override