Rework breaking changes for new structure (#80907)

The structure of the breaking changes / migration guide was updated in #79162 to change the categories and split the breaking changes into different files. This PR amends the changelog generator code in line with this rework.
elastic · Nov 23, 2021 · 9326c7f · 9326c7f
1 parent 7fd074d
commit 9326c7f
Show file tree

Hide file tree

Showing 11 changed files with 293 additions and 121 deletions.
diff --git a/...nal/src/main/java/org/elasticsearch/gradle/internal/release/BreakingChangesGenerator.java b/...nal/src/main/java/org/elasticsearch/gradle/internal/release/BreakingChangesGenerator.java
@@ -11,56 +11,118 @@
 import com.google.common.annotations.VisibleForTesting;
 
 import org.elasticsearch.gradle.VersionProperties;
+import org.gradle.api.GradleException;
 
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import java.util.TreeMap;
+import java.util.TreeSet;
 import java.util.stream.Collectors;
 
 import static java.util.Comparator.comparing;
 import static java.util.stream.Collectors.groupingBy;
+import static java.util.stream.Collectors.toCollection;
 
 /**
- * Generates the page that lists the breaking changes and deprecations for a minor version release.
+ * Generates the page that contains an index into the breaking changes and lists deprecations for a minor version release,
+ * and the individual pages for each breaking area.
  */
 public class BreakingChangesGenerator {
 
-    static void update(File templateFile, File outputFile, List<ChangelogEntry> entries) throws IOException {
-        try (FileWriter output = new FileWriter(outputFile)) {
+    // Needs to match `changelog-schema.json`
+    private static final List<String> BREAKING_AREAS = List.of(
+        "Cluster and node setting",
+        "Command line tool",
+        "Index setting",
+        "JVM option",
+        "Java API",
+        "Logging",
+        "Mapping",
+        "Packaging",
+        "Painless",
+        "REST API",
+        "System requirement",
+        "Transform"
+    );
+
+    static void update(
+        File indexTemplateFile,
+        File indexOutputFile,
+        File outputDirectory,
+        File areaTemplateFile,
+        List<ChangelogEntry> entries
+    ) throws IOException {
+        if (outputDirectory.exists()) {
+            if (outputDirectory.isDirectory() == false) {
+                throw new GradleException("Path [" + outputDirectory + "] exists but isn't a directory!");
+            }
+        } else {
+            Files.createDirectory(outputDirectory.toPath());
+        }
+
+        try (FileWriter output = new FileWriter(indexOutputFile)) {
             output.write(
-                generateFile(QualifiedVersion.of(VersionProperties.getElasticsearch()), Files.readString(templateFile.toPath()), entries)
+                generateIndexFile(
+                    QualifiedVersion.of(VersionProperties.getElasticsearch()),
+                    Files.readString(indexTemplateFile.toPath()),
+                    entries
+                )
             );
         }
-    }
 
-    @VisibleForTesting
-    static String generateFile(QualifiedVersion version, String template, List<ChangelogEntry> entries) throws IOException {
+        String areaTemplate = Files.readString(areaTemplateFile.toPath());
 
-        final Map<Boolean, Map<String, List<ChangelogEntry.Breaking>>> breakingChangesByNotabilityByArea = entries.stream()
-            .map(ChangelogEntry::getBreaking)
-            .filter(Objects::nonNull)
-            .sorted(comparing(ChangelogEntry.Breaking::getTitle))
-            .collect(
-                groupingBy(
-                    ChangelogEntry.Breaking::isNotable,
-                    groupingBy(ChangelogEntry.Breaking::getArea, TreeMap::new, Collectors.toList())
-                )
-            );
+        for (String breakingArea : BREAKING_AREAS) {
+            final List<ChangelogEntry.Breaking> entriesForArea = entries.stream()
+                .map(ChangelogEntry::getBreaking)
+                .filter(entry -> entry != null && breakingArea.equals(entry.getArea()))
+                .collect(Collectors.toList());
+
+            if (entriesForArea.isEmpty()) {
+                continue;
+            }
+
+            final String outputFilename = breakingArea.toLowerCase(Locale.ROOT).replaceFirst(" and", "").replaceAll(" ", "-")
+                + "-changes.asciidoc";
+
+            try (FileWriter output = new FileWriter(outputDirectory.toPath().resolve(outputFilename).toFile())) {
+                output.write(
+                    generateBreakingAreaFile(
+                        QualifiedVersion.of(VersionProperties.getElasticsearch()),
+                        areaTemplate,
+                        breakingArea,
+                        entriesForArea
+                    )
+                );
+            }
+        }
+    }
 
+    @VisibleForTesting
+    static String generateIndexFile(QualifiedVersion version, String template, List<ChangelogEntry> entries) throws IOException {
         final Map<String, List<ChangelogEntry.Deprecation>> deprecationsByArea = entries.stream()
             .map(ChangelogEntry::getDeprecation)
             .filter(Objects::nonNull)
             .sorted(comparing(ChangelogEntry.Deprecation::getTitle))
             .collect(groupingBy(ChangelogEntry.Deprecation::getArea, TreeMap::new, Collectors.toList()));
 
+        final List<String> breakingIncludeList = entries.stream()
+            .filter(each -> each.getBreaking() != null)
+            .map(each -> each.getBreaking().getArea().toLowerCase(Locale.ROOT).replaceFirst(" and", "").replaceAll(" ", "-"))
+            .distinct()
+            .sorted()
+            .toList();
+
         final Map<String, Object> bindings = new HashMap<>();
-        bindings.put("breakingChangesByNotabilityByArea", breakingChangesByNotabilityByArea);
+        bindings.put("breakingIncludeList", breakingIncludeList);
         bindings.put("deprecationsByArea", deprecationsByArea);
         bindings.put("isElasticsearchSnapshot", version.isSnapshot());
         bindings.put("majorDotMinor", version.getMajor() + "." + version.getMinor());
@@ -70,4 +132,28 @@ static String generateFile(QualifiedVersion version, String template, List<Chang
 
         return TemplateUtils.render(template, bindings);
     }
+
+    @VisibleForTesting
+    static String generateBreakingAreaFile(
+        QualifiedVersion version,
+        String template,
+        String breakingArea,
+        List<ChangelogEntry.Breaking> entriesForArea
+    ) throws IOException {
+        final Map<Boolean, Set<ChangelogEntry.Breaking>> breakingEntriesByNotability = entriesForArea.stream()
+            .collect(
+                groupingBy(
+                    ChangelogEntry.Breaking::isNotable,
+                    toCollection(() -> new TreeSet<>(comparing(ChangelogEntry.Breaking::getTitle)))
+                )
+            );
+
+        final Map<String, Object> bindings = new HashMap<>();
+        bindings.put("breakingArea", breakingArea);
+        bindings.put("breakingEntriesByNotability", breakingEntriesByNotability);
+        bindings.put("breakingAreaAnchor", breakingArea.toLowerCase(Locale.ROOT).replaceFirst(" and", "").replaceAll(" ", "_"));
+        bindings.put("majorMinor", String.valueOf(version.getMajor()) + version.getMinor());
+
+        return TemplateUtils.render(template, bindings);
+    }
 }
diff --git a/...ools-internal/src/main/java/org/elasticsearch/gradle/internal/release/ChangelogEntry.java b/...ools-internal/src/main/java/org/elasticsearch/gradle/internal/release/ChangelogEntry.java
@@ -215,6 +215,7 @@ public static class Breaking {
         private String details;
         private String impact;
         private boolean notable;
+        private boolean essSettingChange;
 
         public String getArea() {
             return area;
@@ -260,6 +261,14 @@ public String getAnchor() {
             return generatedAnchor(this.title);
         }
 
+        public boolean isEssSettingChange() {
+            return essSettingChange;
+        }
+
+        public void setEssSettingChange(boolean essSettingChange) {
+            this.essSettingChange = essSettingChange;
+        }
+
         @Override
         public boolean equals(Object o) {
             if (this == o) {
@@ -273,23 +282,25 @@ public boolean equals(Object o) {
                 && Objects.equals(area, breaking.area)
                 && Objects.equals(title, breaking.title)
                 && Objects.equals(details, breaking.details)
-                && Objects.equals(impact, breaking.impact);
+                && Objects.equals(impact, breaking.impact)
+                && Objects.equals(essSettingChange, breaking.essSettingChange);
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(area, title, details, impact, notable);
+            return Objects.hash(area, title, details, impact, notable, essSettingChange);
         }
 
         @Override
         public String toString() {
             return String.format(
-                "Breaking{area='%s', title='%s', details='%s', impact='%s', isNotable=%s}",
+                "Breaking{area='%s', title='%s', details='%s', impact='%s', notable=%s, essSettingChange=%s}",
                 area,
                 title,
                 details,
                 impact,
-                notable
+                notable,
+                essSettingChange
             );
         }
     }
@@ -351,7 +362,7 @@ public String toString() {
     }
 
     private static String generatedAnchor(String input) {
-        final List<String> excludes = List.of("the", "is", "a");
+        final List<String> excludes = List.of("the", "is", "a", "and");
 
         final String[] words = input.toLowerCase(Locale.ROOT)
             .replaceAll("[^\\w]+", "_")

diff --git a/...nal/src/main/java/org/elasticsearch/gradle/internal/release/GenerateReleaseNotesTask.java b/...nal/src/main/java/org/elasticsearch/gradle/internal/release/GenerateReleaseNotesTask.java
@@ -14,6 +14,8 @@
 import org.gradle.api.DefaultTask;
 import org.gradle.api.GradleException;
 import org.gradle.api.file.ConfigurableFileCollection;
+import org.gradle.api.file.Directory;
+import org.gradle.api.file.DirectoryProperty;
 import org.gradle.api.file.FileCollection;
 import org.gradle.api.file.RegularFile;
 import org.gradle.api.file.RegularFileProperty;
@@ -22,6 +24,7 @@
 import org.gradle.api.model.ObjectFactory;
 import org.gradle.api.tasks.InputFile;
 import org.gradle.api.tasks.InputFiles;
+import org.gradle.api.tasks.OutputDirectory;
 import org.gradle.api.tasks.OutputFile;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.process.ExecOperations;
@@ -55,11 +58,13 @@ public class GenerateReleaseNotesTask extends DefaultTask {
     private final RegularFileProperty releaseNotesTemplate;
     private final RegularFileProperty releaseHighlightsTemplate;
     private final RegularFileProperty breakingChangesTemplate;
+    private final RegularFileProperty breakingChangesAreaTemplate;
 
     private final RegularFileProperty releaseNotesIndexFile;
     private final RegularFileProperty releaseNotesFile;
     private final RegularFileProperty releaseHighlightsFile;
-    private final RegularFileProperty breakingChangesFile;
+    private final RegularFileProperty breakingChangesIndexFile;
+    private final DirectoryProperty breakingChangesDirectory;
 
     private final GitWrapper gitWrapper;
 
@@ -71,11 +76,13 @@ public GenerateReleaseNotesTask(ObjectFactory objectFactory, ExecOperations exec
         releaseNotesTemplate = objectFactory.fileProperty();
         releaseHighlightsTemplate = objectFactory.fileProperty();
         breakingChangesTemplate = objectFactory.fileProperty();
+        breakingChangesAreaTemplate = objectFactory.fileProperty();
 
         releaseNotesIndexFile = objectFactory.fileProperty();
         releaseNotesFile = objectFactory.fileProperty();
         releaseHighlightsFile = objectFactory.fileProperty();
-        breakingChangesFile = objectFactory.fileProperty();
+        breakingChangesIndexFile = objectFactory.fileProperty();
+        breakingChangesDirectory = objectFactory.directoryProperty();
 
         gitWrapper = new GitWrapper(execOperations);
     }
@@ -129,7 +136,9 @@ public void executeTask() throws IOException {
         LOGGER.info("Generating breaking changes / deprecations notes...");
         BreakingChangesGenerator.update(
             this.breakingChangesTemplate.get().getAsFile(),
-            this.breakingChangesFile.get().getAsFile(),
+            this.breakingChangesIndexFile.get().getAsFile(),
+            this.breakingChangesDirectory.get().getAsFile(),
+            this.breakingChangesAreaTemplate.get().getAsFile(),
             entries
         );
     }
@@ -339,11 +348,29 @@ public void setReleaseHighlightsFile(RegularFile file) {
     }
 
     @OutputFile
-    public RegularFileProperty getBreakingChangesFile() {
-        return breakingChangesFile;
+    public RegularFileProperty getBreakingChangesIndexFile() {
+        return breakingChangesIndexFile;
     }
 
-    public void setBreakingChangesFile(RegularFile file) {
-        this.breakingChangesFile.set(file);
+    public void setBreakingChangesIndexFile(RegularFile file) {
+        this.breakingChangesIndexFile.set(file);
+    }
+
+    public void setBreakingChangesDirectory(Directory breakingChangesDirectory) {
+        this.breakingChangesDirectory.set(breakingChangesDirectory);
+    }
+
+    @OutputDirectory
+    public DirectoryProperty getBreakingChangesDirectory() {
+        return breakingChangesDirectory;
+    }
+
+    @InputFile
+    public RegularFileProperty getBreakingChangesAreaTemplate() {
+        return breakingChangesAreaTemplate;
+    }
+
+    public void setBreakingChangesAreaTemplate(RegularFile file) {
+        this.breakingChangesAreaTemplate.set(file);
     }
 }
diff --git a/...-internal/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseToolsPlugin.java b/...-internal/src/main/java/org/elasticsearch/gradle/internal/release/ReleaseToolsPlugin.java
@@ -84,11 +84,15 @@ public void apply(Project project) {
             task.setReleaseHighlightsFile(projectDirectory.file("docs/reference/release-notes/highlights.asciidoc"));
 
             task.setBreakingChangesTemplate(projectDirectory.file(RESOURCES + "templates/breaking-changes.asciidoc"));
-            task.setBreakingChangesFile(
+            task.setBreakingChangesIndexFile(
                 projectDirectory.file(
                     String.format("docs/reference/migration/migrate_%d_%d.asciidoc", version.getMajor(), version.getMinor())
                 )
             );
+            task.setBreakingChangesAreaTemplate(projectDirectory.file(RESOURCES + "templates/breaking-changes-area.asciidoc"));
+            task.setBreakingChangesDirectory(
+                projectDirectory.dir(String.format("docs/reference/migration/migrate_%d_%d", version.getMajor(), version.getMinor()))
+            );
 
             task.dependsOn(validateChangelogsTask);
         });

diff --git a/build-tools-internal/src/main/resources/changelog-schema.json b/build-tools-internal/src/main/resources/changelog-schema.json
@@ -157,6 +157,9 @@
         },
         "notable": {
           "type": "boolean"
+        },
+        "ess_setting_change": {
+          "type": "boolean"
         }
       },
       "required": [
@@ -179,6 +182,9 @@
         "body": {
           "type": "string",
           "minLength": 1
+        },
+        "ess_setting_change": {
+          "type": "boolean"
         }
       },
       "required": [

diff --git a/build-tools-internal/src/main/resources/templates/breaking-changes-area.asciidoc b/build-tools-internal/src/main/resources/templates/breaking-changes-area.asciidoc
@@ -0,0 +1,39 @@
+[discrete]
+[[breaking_${majorMinor}_${breakingAreaAnchor}]]
+==== ${breakingArea}
+
+//NOTE: The notable-breaking-changes tagged regions are re-used in the
+//Installation and Upgrade Guide
+
+TIP: {ess-setting-change}
+
+<%
+[true, false].each { isNotable ->
+    def breakingChanges = breakingEntriesByNotability.getOrDefault(isNotable, [])
+
+    if (breakingChanges.isEmpty() == false) {
+        if (isNotable) {
+            /* No newline here, one will be added below */
+            print "// tag::notable-breaking-changes[]"
+        }
+
+        for (breaking in breakingChanges) { %>
+[[${ breaking.anchor }]]
+. ${breaking.title}${ breaking.essSettingChange ? ' {ess-icon}' : '' }
+[%collapsible]
+====
+*Details* +
+${breaking.details.trim()}
+
+*Impact* +
+${breaking.impact.trim()}
+====
+<%
+        }
+
+        if (isNotable) {
+            print "// end::notable-breaking-changes[]\n"
+        }
+    }
+}
+%>