Updates to changelog processing after docs redesign (#89463)

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/release/ValidateChangelogEntryTask.java

@@ -8,6 +8,8 @@
 
 package org.elasticsearch.gradle.internal.release;
 
+import com.google.common.annotations.VisibleForTesting;
+
 import org.gradle.api.DefaultTask;
 import org.gradle.api.GradleException;
 import org.gradle.api.file.ConfigurableFileCollection;
@@ -30,6 +32,21 @@ public class ValidateChangelogEntryTask extends DefaultTask {
     private final ConfigurableFileCollection changelogs;
     private final ProjectLayout projectLayout;
 
+    public static final String TRIPLE_BACKTICK = "```";
+    private static final String CODE_BLOCK_ERROR = """
+        [%s] uses a triple-backtick in the [%s] section, but it must be
+        formatted as a Asciidoc code block. For example:
+
+            [source,yaml]
+            ----
+            {
+              "metrics.time" : 10,
+              "metrics.time.min" : 1,
+              "metrics.time.max" : 500
+            }
+            ----
+        """;
+
     @Inject
     public ValidateChangelogEntryTask(ObjectFactory objectFactory, ProjectLayout projectLayout) {
         this.changelogs = objectFactory.fileCollection();
@@ -43,37 +60,60 @@ public void executeTask() {
             .stream()
             .collect(Collectors.toMap(file -> rootDir.relativize(file.toURI()).toString(), ChangelogEntry::parse));
 
+        changelogs.forEach(ValidateChangelogEntryTask::validate);
+    }
+
+    @VisibleForTesting
+    static void validate(String path, ChangelogEntry entry) {
         // We don't try to find all such errors, because we expect them to be rare e.g. only
         // when a new file is added.
-        changelogs.forEach((path, entry) -> {
-            final String type = entry.getType();
-
-            if (type.equals("known-issue") == false && type.equals("security") == false) {
-                if (entry.getPr() == null) {
-                    throw new GradleException(
-                        "[" + path + "] must provide a [pr] number (only 'known-issue' and " + "'security' entries can omit this"
-                    );
-                }
-
-                if (entry.getArea() == null) {
-                    throw new GradleException(
-                        "[" + path + "] must provide an [area] (only 'known-issue' and " + "'security' entries can omit this"
-                    );
-                }
+        final String type = entry.getType();
+
+        if (type.equals("known-issue") == false && type.equals("security") == false) {
+            if (entry.getPr() == null) {
+                throw new GradleException(
+                    "[" + path + "] must provide a [pr] number (only 'known-issue' and 'security' entries can omit this"
+                );
             }
 
-            if ((type.equals("breaking") || type.equals("breaking-java")) && entry.getBreaking() == null) {
+            if (entry.getArea() == null) {
+                throw new GradleException("[" + path + "] must provide an [area] (only 'known-issue' and 'security' entries can omit this");
+            }
+        }
+
+        if (type.equals("breaking") || type.equals("breaking-java")) {
+            if (entry.getBreaking() == null) {
                 throw new GradleException(
                     "[" + path + "] has type [" + type + "] and must supply a [breaking] section with further information"
                 );
             }
 
-            if (type.equals("deprecation") && entry.getDeprecation() == null) {
+            if (entry.getBreaking().getDetails().contains(TRIPLE_BACKTICK)) {
+                throw new GradleException(CODE_BLOCK_ERROR.formatted(path, "breaking.details"));
+            }
+            if (entry.getBreaking().getImpact().contains(TRIPLE_BACKTICK)) {
+                throw new GradleException(CODE_BLOCK_ERROR.formatted(path, "breaking.impact"));
+            }
+        }
+
+        if (type.equals("deprecation")) {
+            if (entry.getDeprecation() == null) {
                 throw new GradleException(
                     "[" + path + "] has type [deprecation] and must supply a [deprecation] section with further information"
                 );
             }
-        });
+
+            if (entry.getDeprecation().getDetails().contains(TRIPLE_BACKTICK)) {
+                throw new GradleException(CODE_BLOCK_ERROR.formatted(path, "deprecation.details"));
+            }
+            if (entry.getDeprecation().getImpact().contains(TRIPLE_BACKTICK)) {
+                throw new GradleException(CODE_BLOCK_ERROR.formatted(path, "deprecation.impact"));
+            }
+        }
+
+        if (entry.getHighlight() != null && entry.getHighlight().getBody().contains(TRIPLE_BACKTICK)) {
+            throw new GradleException(CODE_BLOCK_ERROR.formatted(path, "highlight.body"));
+        }
     }
 
     @InputFiles

build-tools-internal/src/main/resources/templates/release-highlights.asciidoc

@@ -32,14 +32,18 @@ if (notableHighlights.isEmpty()) { %>
 <% for (highlight in notableHighlights) { %>
 [discrete]
 [[${ highlight.anchor }]]
-=== {es-pull}${highlight.pr}[${highlight.title}]
+=== ${highlight.title}
 ${highlight.body.trim()}
+
+{es-pull}${highlight.pr}[#${highlight.pr}]
 <% } %>
 // end::notable-highlights[]
 <% } %>
 <% for (highlight in nonNotableHighlights) { %>
 [discrete]
 [[${ highlight.anchor }]]
-=== {es-pull}${highlight.pr}[${highlight.title}]
+=== ${highlight.title}
 ${highlight.body.trim()}
+
+{es-pull}${highlight.pr}[#${highlight.pr}]
 <% } %>

build-tools-internal/src/test/java/org/elasticsearch/gradle/internal/release/ReleaseHighlightsGeneratorTest.java

@@ -60,11 +60,11 @@ public void generateFile_rendersCorrectMarkup() throws Exception {
     }
 
     private List<ChangelogEntry> getEntries() {
-        ChangelogEntry entry1 = makeChangelogEntry(1, true);
-        ChangelogEntry entry2 = makeChangelogEntry(2, true);
-        ChangelogEntry entry3 = makeChangelogEntry(3, false);
+        ChangelogEntry entry123 = makeChangelogEntry(123, true);
+        ChangelogEntry entry456 = makeChangelogEntry(456, true);
+        ChangelogEntry entry789 = makeChangelogEntry(789, false);
         // Return unordered list, to test correct re-ordering
-        return List.of(entry2, entry1, entry3);
+        return List.of(entry456, entry123, entry789);
     }
 
     private ChangelogEntry makeChangelogEntry(int pr, boolean notable) {

build-tools-internal/src/test/java/org/elasticsearch/gradle/internal/release/ValidateChangelogEntryTaskTest.java

@@ -0,0 +1,179 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.gradle.internal.release;
+
+import org.gradle.api.GradleException;
+import org.hamcrest.Matchers;
+import org.junit.jupiter.api.Test;
+
+import java.util.stream.Stream;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.endsWith;
+
+class ValidateChangelogEntryTaskTest {
+
+    @Test
+    void test_prNumber_isRequired() {
+        ChangelogEntry changelog = new ChangelogEntry();
+        changelog.setType("enhancement");
+
+        final String message = doValidate(changelog);
+
+        assertThat(message, endsWith("must provide a [pr] number (only 'known-issue' and 'security' entries can omit this"));
+    }
+
+    @Test
+    void test_prNumber_notRequired() {
+        Stream.of("known-issue", "security").forEach(type -> {
+            ChangelogEntry changelog = new ChangelogEntry();
+            changelog.setType(type);
+
+            // Should not throw an exception!
+            ValidateChangelogEntryTask.validate("", changelog);
+        });
+    }
+
+    @Test
+    void test_area_isRequired() {
+        final ChangelogEntry changelog = new ChangelogEntry();
+        changelog.setType("enhancement");
+        changelog.setPr(123);
+
+        final String message = doValidate(changelog);
+
+        assertThat(message, endsWith("must provide an [area] (only 'known-issue' and 'security' entries can omit this"));
+    }
+
+    @Test
+    void test_breaking_requiresBreakingSection() {
+        Stream.of("breaking", "breaking-java").forEach(type -> {
+            final ChangelogEntry changelog = buildChangelog(type);
+
+            final String message = doValidate(changelog);
+
+            assertThat(message, endsWith("has type [" + type + "] and must supply a [breaking] section with further information"));
+        });
+    }
+
+    @Test
+    void test_breaking_rejectsTripleBackticksInDetails() {
+        Stream.of("breaking", "breaking-java").forEach(type -> {
+            final ChangelogEntry.Breaking breaking = new ChangelogEntry.Breaking();
+            breaking.setDetails("""
+                Some waffle.
+                ```
+                I AM CODE!
+                ```
+                """);
+
+            final ChangelogEntry changelog = buildChangelog(type);
+            changelog.setBreaking(breaking);
+
+            final String message = doValidate(changelog);
+
+            assertThat(message, containsString("uses a triple-backtick in the [breaking.details] section"));
+        });
+    }
+
+    @Test
+    void test_breaking_rejectsTripleBackticksInImpact() {
+        Stream.of("breaking", "breaking-java").forEach(type -> {
+            final ChangelogEntry.Breaking breaking = new ChangelogEntry.Breaking();
+            breaking.setDetails("Waffle waffle");
+            breaking.setImpact("""
+                More waffle.
+                ```
+                THERE ARE WEASEL RAKING THROUGH MY GARBAGE!
+                ```
+                """);
+
+            final ChangelogEntry changelog = buildChangelog(type);
+            changelog.setBreaking(breaking);
+
+            final String message = doValidate(changelog);
+
+            assertThat(message, containsString("uses a triple-backtick in the [breaking.impact] section"));
+        });
+    }
+
+    @Test
+    void test_deprecation_rejectsTripleBackticksInImpact() {
+        final ChangelogEntry.Deprecation deprecation = new ChangelogEntry.Deprecation();
+        deprecation.setDetails("Waffle waffle");
+        deprecation.setImpact("""
+            More waffle.
+            ```
+            THERE ARE WEASEL RAKING THROUGH MY GARBAGE!
+            ```
+            """);
+
+        final ChangelogEntry changelog = buildChangelog("deprecation");
+        changelog.setDeprecation(deprecation);
+
+        final String message = doValidate(changelog);
+
+        assertThat(message, containsString("uses a triple-backtick in the [deprecation.impact] section"));
+    }
+
+    @Test
+    void test_deprecation_rejectsTripleBackticksInDetails() {
+        final ChangelogEntry.Deprecation deprecation = new ChangelogEntry.Deprecation();
+        deprecation.setDetails("""
+            Some waffle.
+            ```
+            I AM CODE!
+            ```
+            """);
+
+        final ChangelogEntry changelog = buildChangelog("deprecation");
+        changelog.setDeprecation(deprecation);
+
+        final String message = doValidate(changelog);
+
+        assertThat(message, containsString("uses a triple-backtick in the [deprecation.details] section"));
+    }
+
+    @Test
+    void test_highlight_rejectsTripleBackticksInBody() {
+        final ChangelogEntry.Highlight highlight = new ChangelogEntry.Highlight();
+        highlight.setBody("""
+            Some waffle.
+            ```
+            I AM CODE!
+            ```
+            """);
+
+        final ChangelogEntry changelog = buildChangelog("enhancement");
+        changelog.setHighlight(highlight);
+
+        final String message = doValidate(changelog);
+
+        assertThat(message, containsString("uses a triple-backtick in the [highlight.body] section"));
+    }
+
+    private static ChangelogEntry buildChangelog(String type) {
+        final ChangelogEntry changelog = new ChangelogEntry();
+        changelog.setType(type);
+        changelog.setPr(123);
+        changelog.setArea("Infra/Core");
+        return changelog;
+    }
+
+    private String doValidate(ChangelogEntry entry) {
+        try {
+            ValidateChangelogEntryTask.validate("docs/123.yaml", entry);
+            throw new AssertionError("No exception thrown!");
+        } catch (Exception e) {
+            assertThat(e, Matchers.instanceOf(GradleException.class));
+            return e.getMessage();
+        }
+    }
+}

build-tools-internal/src/test/resources/org/elasticsearch/gradle/internal/release/ReleaseHighlightsGeneratorTest.generateFile.asciidoc

@@ -20,20 +20,26 @@ Other versions:
 // tag::notable-highlights[]
 
 [discrete]
-[[notable_release_highlight_number_1]]
-=== {es-pull}1[Notable release highlight number 1]
-Notable release body number 1
+[[notable_release_highlight_number_123]]
+=== Notable release highlight number 123
+Notable release body number 123
+
+{es-pull}123[#123]
 
 [discrete]
-[[notable_release_highlight_number_2]]
-=== {es-pull}2[Notable release highlight number 2]
-Notable release body number 2
+[[notable_release_highlight_number_456]]
+=== Notable release highlight number 456
+Notable release body number 456
+
+{es-pull}456[#456]
 
 // end::notable-highlights[]
 
 
 [discrete]
-[[notable_release_highlight_number_3]]
-=== {es-pull}3[Notable release highlight number 3]
-Notable release body number 3
+[[notable_release_highlight_number_789]]
+=== Notable release highlight number 789
+Notable release body number 789
+
+{es-pull}789[#789]
 

docs/changelog/83345.yaml

@@ -14,7 +14,8 @@ highlight:
      As an example, the following ILM policy would roll an index over if it is at least 7 days old or
      at least 100 gigabytes, but only as long as the index is not empty.
 
-     ```
+     [source,console]
+     ----
      PUT _ilm/policy/my_policy
      {
        "policy": {
@@ -31,5 +32,5 @@ highlight:
          }
        }
      }
-     ```
+     ----
    notable: true