Preserve grok pattern ordering and add sort option (#61671)

libs/grok/src/main/java/org/elasticsearch/grok/Grok.java

@@ -37,6 +37,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -288,7 +289,7 @@ public final class Grok {
             "java", "junos", "linux-syslog", "maven", "mcollective-patterns", "mongodb", "nagios",
             "postgresql", "rails", "redis", "ruby", "squid"
         };
-        Map<String, String> builtinPatterns = new HashMap<>();
+        Map<String, String> builtinPatterns = new LinkedHashMap<>();
         for (String pattern : PATTERN_NAMES) {
             try(InputStream is = Grok.class.getResourceAsStream("/patterns/" + pattern)) {
                 loadPatterns(builtinPatterns, is);

modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/GrokProcessorGetAction.java

@@ -18,6 +18,7 @@
  */
 package org.elasticsearch.ingest.common;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionRequestValidationException;
@@ -40,6 +41,7 @@ import org.elasticsearch.transport.TransportService;
 import java.io.IOException;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
 
 import static org.elasticsearch.ingest.common.IngestCommonPlugin.GROK_PATTERNS;
 import static org.elasticsearch.rest.RestRequest.Method.GET;
@@ -55,16 +57,33 @@ public class GrokProcessorGetAction extends ActionType<GrokProcessorGetAction.Re
 
     public static class Request extends ActionRequest {
 
-        public Request() {}
+        private final boolean sorted;
+
+        public Request(boolean sorted) {
+            this.sorted = sorted;
+        }
 
         Request(StreamInput in) throws IOException {
             super(in);
+            this.sorted = in.getVersion().onOrAfter(Version.V_8_0_0) ? in.readBoolean() : false;
         }
 
         @Override
         public ActionRequestValidationException validate() {
             return null;
         }
+
+        @Override
+        public void writeTo(StreamOutput out) throws IOException {
+            super.writeTo(out);
+            if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
+                out.writeBoolean(sorted);
+            }
+        }
+
+        public boolean sorted() {
+            return sorted;
+        }
     }
 
     public static class Response extends ActionResponse implements ToXContentObject {
@@ -100,15 +119,25 @@ public class GrokProcessorGetAction extends ActionType<GrokProcessorGetAction.Re
 
     public static class TransportAction extends HandledTransportAction<Request, Response> {
 
+        private final Map<String, String> grokPatterns;
+        private final Map<String, String> sortedGrokPatterns;
+
         @Inject
         public TransportAction(TransportService transportService, ActionFilters actionFilters) {
+            this(transportService, actionFilters, GROK_PATTERNS);
+        }
+
+        // visible for testing
+        TransportAction(TransportService transportService, ActionFilters actionFilters, Map<String, String> grokPatterns) {
             super(NAME, transportService, actionFilters, Request::new);
+            this.grokPatterns = grokPatterns;
+            this.sortedGrokPatterns = new TreeMap<>(this.grokPatterns);
         }
 
         @Override
         protected void doExecute(Task task, Request request, ActionListener<Response> listener) {
             try {
-                listener.onResponse(new Response(GROK_PATTERNS));
+                listener.onResponse(new Response(request.sorted() ? sortedGrokPatterns : grokPatterns));
             } catch (Exception e) {
                 listener.onFailure(e);
             }
@@ -129,7 +158,9 @@ public class GrokProcessorGetAction extends ActionType<GrokProcessorGetAction.Re
 
         @Override
         protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) {
-            return channel -> client.executeLocally(INSTANCE, new Request(), new RestToXContentListener<>(channel));
+            boolean sorted = request.paramAsBoolean("s", false);
+            Request grokPatternsRequest = new Request(sorted);
+            return channel -> client.executeLocally(INSTANCE, grokPatternsRequest, new RestToXContentListener<>(channel));
         }
     }
 }

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GrokProcessorGetActionTests.java

@@ -19,6 +19,8 @@
 
 package org.elasticsearch.ingest.common;
 
+import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
 import org.elasticsearch.common.io.stream.StreamInput;
@@ -27,18 +29,25 @@ import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.transport.TransportService;
 
+import java.util.ArrayList;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 
+import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.sameInstance;
+import static org.hamcrest.core.IsNull.notNullValue;
 import static org.hamcrest.core.IsNull.nullValue;
+import static org.mockito.Mockito.mock;
 
 public class GrokProcessorGetActionTests extends ESTestCase {
-    private static final Map<String, String> TEST_PATTERNS = Collections.singletonMap("PATTERN", "foo");
+    private static final Map<String, String> TEST_PATTERNS = Map.of("PATTERN2", "foo2", "PATTERN1", "foo1");
 
     public void testRequest() throws Exception {
-        GrokProcessorGetAction.Request request = new GrokProcessorGetAction.Request();
+        GrokProcessorGetAction.Request request = new GrokProcessorGetAction.Request(false);
         BytesStreamOutput out = new BytesStreamOutput();
         request.writeTo(out);
         StreamInput streamInput = out.bytes().streamInput();
@@ -56,6 +65,43 @@ public class GrokProcessorGetActionTests extends ESTestCase {
         assertThat(response.getGrokPatterns(), equalTo(otherResponse.getGrokPatterns()));
     }
 
+    public void testResponseSorting() {
+        List<String> sortedKeys = new ArrayList<>(TEST_PATTERNS.keySet());
+        Collections.sort(sortedKeys);
+        GrokProcessorGetAction.TransportAction transportAction =
+            new GrokProcessorGetAction.TransportAction(mock(TransportService.class), mock(ActionFilters.class), TEST_PATTERNS);
+        GrokProcessorGetAction.Response[] receivedResponse = new GrokProcessorGetAction.Response[1];
+        transportAction.doExecute(null, new GrokProcessorGetAction.Request(true), new ActionListener<>() {
+            @Override
+            public void onResponse(GrokProcessorGetAction.Response response) {
+                receivedResponse[0] = response;
+            }
+
+            @Override
+            public void onFailure(Exception e) {
+                fail();
+            }
+        });
+        assertThat(receivedResponse[0], notNullValue());
+        assertThat(receivedResponse[0].getGrokPatterns().keySet().toArray(), equalTo(sortedKeys.toArray()));
+
+        GrokProcessorGetAction.Response firstResponse = receivedResponse[0];
+        transportAction.doExecute(null, new GrokProcessorGetAction.Request(true), new ActionListener<>() {
+            @Override
+            public void onResponse(GrokProcessorGetAction.Response response) {
+                receivedResponse[0] = response;
+            }
+
+            @Override
+            public void onFailure(Exception e) {
+                fail();
+            }
+        });
+        assertThat(receivedResponse[0], notNullValue());
+        assertThat(receivedResponse[0], not(sameInstance(firstResponse)));
+        assertThat(receivedResponse[0].getGrokPatterns(), sameInstance(firstResponse.getGrokPatterns()));
+    }
+
     @SuppressWarnings("unchecked")
     public void testResponseToXContent() throws Exception {
         GrokProcessorGetAction.Response response = new GrokProcessorGetAction.Response(TEST_PATTERNS);
@@ -63,8 +109,9 @@ public class GrokProcessorGetActionTests extends ESTestCase {
             response.toXContent(builder, ToXContent.EMPTY_PARAMS);
             Map<String, Object> converted = XContentHelper.convertToMap(BytesReference.bytes(builder), false, builder.contentType()).v2();
             Map<String, String> patterns = (Map<String, String>) converted.get("patterns");
-            assertThat(patterns.size(), equalTo(1));
-            assertThat(patterns.get("PATTERN"), equalTo("foo"));
+            assertThat(patterns.size(), equalTo(2));
+            assertThat(patterns.get("PATTERN1"), equalTo("foo1"));
+            assertThat(patterns.get("PATTERN2"), equalTo("foo2"));
         }
     }
 }