Revert "Delete doesn't need reloading 🤦 removing it"

This reverts commit 9c8e0b6.

Author: carlosdelest

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

@@ -33,6 +33,12 @@
           }
         }
       ]
+    },
+    "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,6 +44,27 @@ 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,6 +366,25 @@ 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<>() {
@@ -399,5 +418,21 @@ 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,6 +9,7 @@
 
 package org.elasticsearch.action.synonyms;
 
+import org.elasticsearch.TransportVersions;
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.ActionType;
@@ -31,18 +32,24 @@ 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) {
+        public Request(String synonymsSetId, String synonymRuleId, boolean refresh) {
             this.synonymsSetId = synonymsSetId;
             this.synonymRuleId = synonymRuleId;
+            this.refresh = refresh;
         }
 
         @Override
@@ -63,6 +70,9 @@ 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() {
@@ -73,6 +83,10 @@ 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,6 +41,7 @@ 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,7 +40,8 @@ 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.param("synonymRuleId"),
+            restRequest.paramAsBoolean("refresh", true)
         );
         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;
 
-                    checkIndexSearchableAndReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener);
+                    maybeReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener);
                 })
             );
         }));
@@ -424,7 +424,7 @@ private void indexSynonymRule(
                 ? UpdateSynonymsResultStatus.CREATED
                 : UpdateSynonymsResultStatus.UPDATED;
 
-            checkIndexSearchableAndReloadAnalyzers(synonymsSetId, refresh, false, updateStatus, l2);
+            maybeReloadAnalyzers(synonymsSetId, refresh, false, updateStatus, l2);
         }));
     }
 
@@ -444,7 +444,12 @@ public void getSynonymRule(String synonymSetId, String synonymRuleId, ActionList
         );
     }
 
-    public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, ActionListener<SynonymsReloadResult> listener) {
+    public void deleteSynonymRule(
+        String synonymsSetId,
+        String synonymRuleId,
+        boolean refresh,
+        ActionListener<SynonymsReloadResult> listener
+    ) {
         client.prepareDelete(SYNONYMS_ALIAS_NAME, internalSynonymRuleId(synonymsSetId, synonymRuleId))
             .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
             .execute(new DelegatingIndexNotFoundActionListener<>(synonymsSetId, listener, (l, deleteResponse) -> {
@@ -463,7 +468,7 @@ public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, Action
                     return;
                 }
 
-                reloadAnalyzers(synonymsSetId, false, UpdateSynonymsResultStatus.DELETED, listener);
+                maybeReloadAnalyzers(synonymsSetId, refresh, false, UpdateSynonymsResultStatus.DELETED, listener);
             }));
     }
 
@@ -520,8 +525,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. It's OK to reload as we're doing preview mode
-        reloadAnalyzers(synonymSetId, true, null, listener.delegateFailure((reloadListener, reloadResult) -> {
+        // Previews reloading the resource to understand its usage on indices
+        maybeReloadAnalyzers(synonymSetId, true, true, null, listener.delegateFailure((reloadListener, reloadResult) -> {
             Map<String, ReloadAnalyzersResponse.ReloadDetails> reloadDetails = reloadResult.reloadAnalyzersResponse.getReloadDetails();
             if (reloadDetails.isEmpty() == false) {
                 Set<String> indices = reloadDetails.entrySet()
@@ -561,7 +566,7 @@ public void deleteSynonymsSet(String synonymSetId, ActionListener<AcknowledgedRe
         }));
     }
 
-    private <T> void checkIndexSearchableAndReloadAnalyzers(
+    private <T> void maybeReloadAnalyzers(
         String synonymSetId,
         boolean refresh,
         boolean preview,
@@ -591,20 +596,16 @@ private <T> void checkIndexSearchableAndReloadAnalyzers(
                 return;
             }
 
-            reloadAnalyzers(synonymSetId, preview, synonymsOperationResult, 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))
+            );
         }));
     }
 
-    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());
+        return new DeleteSynonymRuleAction.Request(randomIdentifier(), randomIdentifier(), randomBoolean());
     }
 
     @Override