From aa374bf406b792c7662237736830bbff4e2ca8ed Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 17 Dec 2024 12:13:56 -0800 Subject: [PATCH] Correct links used in upgrade issues Update BOMR so that the `Upgrade` type now contains a copy of the library we're upgrading to complete with an updated version number. This allows the correct links to be generated. Closes gh-43545 --- .../boot/build/bom/Library.java | 11 ++- .../bom/bomr/InteractiveUpgradeResolver.java | 29 +++++--- .../boot/build/bom/bomr/MoveToSnapshots.java | 34 +-------- .../boot/build/bom/bomr/Upgrade.java | 29 ++++---- .../build/bom/bomr/UpgradeApplicator.java | 20 +++-- .../boot/build/bom/bomr/UpgradeBom.java | 28 ------- .../build/bom/bomr/UpgradeDependencies.java | 21 ++++-- .../bomr/InteractiveUpgradeResolverTests.java | 73 +++++++++++++++++++ .../boot/build/bom/bomr/UpgradeTests.java | 54 ++++++++++++++ 9 files changed, 198 insertions(+), 101 deletions(-) create mode 100644 buildSrc/src/test/java/org/springframework/boot/build/bom/bomr/InteractiveUpgradeResolverTests.java create mode 100644 buildSrc/src/test/java/org/springframework/boot/build/bom/bomr/UpgradeTests.java diff --git a/buildSrc/src/main/java/org/springframework/boot/build/bom/Library.java b/buildSrc/src/main/java/org/springframework/boot/build/bom/Library.java index 382c058630a..5ab16ecdf82 100644 --- a/buildSrc/src/main/java/org/springframework/boot/build/bom/Library.java +++ b/buildSrc/src/main/java/org/springframework/boot/build/bom/Library.java @@ -101,7 +101,7 @@ public class Library { this.versionAlignment = versionAlignment; this.alignsWithBom = alignsWithBom; this.linkRootName = (linkRootName != null) ? linkRootName : generateLinkRootName(name); - this.links = Collections.unmodifiableMap(new TreeMap<>(links)); + this.links = (links != null) ? Collections.unmodifiableMap(new TreeMap<>(links)) : Collections.emptyMap(); } private static String generateLinkRootName(String name) { @@ -167,6 +167,15 @@ public class Library { return this.links.get(name); } + public String getNameAndVersion() { + return getName() + " " + getVersion(); + } + + public Library withVersion(LibraryVersion version) { + return new Library(this.name, this.calendarName, version, this.groups, this.prohibitedVersions, + this.considerSnapshots, this.versionAlignment, this.alignsWithBom, this.linkRootName, this.links); + } + /** * A version or range of versions that are prohibited from being used in a bom. */ diff --git a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/InteractiveUpgradeResolver.java b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/InteractiveUpgradeResolver.java index 532dfc972c2..ac6515749f5 100644 --- a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/InteractiveUpgradeResolver.java +++ b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/InteractiveUpgradeResolver.java @@ -51,10 +51,12 @@ public final class InteractiveUpgradeResolver implements UpgradeResolver { for (Library library : libraries) { librariesByName.put(library.getName(), library); } - List libraryUpdates = this.libraryUpdateResolver - .findLibraryUpdates(librariesToUpgrade, librariesByName); try { - return libraryUpdates.stream().map(this::resolveUpgrade).filter(Objects::nonNull).toList(); + return this.libraryUpdateResolver.findLibraryUpdates(librariesToUpgrade, librariesByName) + .stream() + .map(this::resolveUpgrade) + .filter(Objects::nonNull) + .toList(); } catch (UpgradesInterruptedException ex) { return Collections.emptyList(); @@ -62,24 +64,29 @@ public final class InteractiveUpgradeResolver implements UpgradeResolver { } private Upgrade resolveUpgrade(LibraryWithVersionOptions libraryWithVersionOptions) { - if (libraryWithVersionOptions.getVersionOptions().isEmpty()) { + Library library = libraryWithVersionOptions.getLibrary(); + List versionOptions = libraryWithVersionOptions.getVersionOptions(); + if (versionOptions.isEmpty()) { return null; } - VersionOption defaultOption = new VersionOption( - libraryWithVersionOptions.getLibrary().getVersion().getVersion()); + VersionOption defaultOption = new VersionOption(library.getVersion().getVersion()); + VersionOption selected = selectOption(defaultOption, library, versionOptions); + return (selected.equals(defaultOption)) ? null : new Upgrade(library, selected.getVersion()); + } + + private VersionOption selectOption(VersionOption defaultOption, Library library, + List versionOptions) { VersionOption selected = this.userInputHandler.askUser((questions) -> { - String question = libraryWithVersionOptions.getLibrary().getName() + " " - + libraryWithVersionOptions.getLibrary().getVersion().getVersion(); + String question = library.getNameAndVersion(); List options = new ArrayList<>(); options.add(defaultOption); - options.addAll(libraryWithVersionOptions.getVersionOptions()); + options.addAll(versionOptions); return questions.selectOption(question, options, defaultOption); }).get(); if (this.userInputHandler.interrupted()) { throw new UpgradesInterruptedException(); } - return (selected.equals(defaultOption)) ? null - : new Upgrade(libraryWithVersionOptions.getLibrary(), selected.getVersion()); + return selected; } static class UpgradesInterruptedException extends RuntimeException { diff --git a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/MoveToSnapshots.java b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/MoveToSnapshots.java index f0475987311..a4147a74ee5 100644 --- a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/MoveToSnapshots.java +++ b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/MoveToSnapshots.java @@ -17,7 +17,6 @@ package org.springframework.boot.build.bom.bomr; import java.time.OffsetDateTime; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.function.BiPredicate; @@ -33,7 +32,6 @@ import org.slf4j.LoggerFactory; import org.springframework.boot.build.bom.BomExtension; import org.springframework.boot.build.bom.Library; import org.springframework.boot.build.bom.bomr.ReleaseSchedule.Release; -import org.springframework.boot.build.bom.bomr.github.Issue; import org.springframework.boot.build.bom.bomr.github.Milestone; import org.springframework.boot.build.bom.bomr.version.DependencyVersion; import org.springframework.boot.build.properties.BuildProperties; @@ -67,38 +65,10 @@ public abstract class MoveToSnapshots extends UpgradeDependencies { super.upgradeDependencies(); } - @Override - protected String issueTitle(Upgrade upgrade) { - return "Upgrade to " + description(upgrade); - } - - private String description(Upgrade upgrade) { - String snapshotVersion = upgrade.getVersion().toString(); - String releaseVersion = snapshotVersion.substring(0, snapshotVersion.length() - "-SNAPSHOT".length()); - return upgrade.getLibrary().getName() + " " + releaseVersion; - } - - @Override - protected String issueBody(Upgrade upgrade, Issue existingUpgrade) { - Library library = upgrade.getLibrary(); - String releaseNotesLink = library.getLinkUrl("releaseNotes"); - List lines = new ArrayList<>(); - String description = description(upgrade); - if (releaseNotesLink != null) { - lines.add("Upgrade to [%s](%s).".formatted(description, releaseNotesLink)); - } - else { - lines.add("Upgrade to %s.".formatted(description)); - } - if (existingUpgrade != null) { - lines.add("Supersedes #" + existingUpgrade.getNumber()); - } - return String.join("\n\n", lines); - } - @Override protected String commitMessage(Upgrade upgrade, int issueNumber) { - return "Start building against " + description(upgrade) + " snapshots" + "\n\nSee gh-" + issueNumber; + return "Start building against " + upgrade.toRelease().getNameAndVersion() + " snapshots" + "\n\nSee gh-" + + issueNumber; } @Override diff --git a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/Upgrade.java b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/Upgrade.java index a7778d55264..4ce6a8cfbef 100644 --- a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/Upgrade.java +++ b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/Upgrade.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,30 +17,33 @@ package org.springframework.boot.build.bom.bomr; import org.springframework.boot.build.bom.Library; +import org.springframework.boot.build.bom.Library.LibraryVersion; import org.springframework.boot.build.bom.bomr.version.DependencyVersion; /** * An upgrade to change a {@link Library} to use a new version. * * @author Andy Wilkinson + * @author Phillip Webb + * @param from the library we're upgrading from + * @param to the library we're upgrading to (may be a SNAPSHOT) + * @param toRelease the release version of the library we're ultimately upgrading to */ -final class Upgrade { +record Upgrade(Library from, Library to, Library toRelease) { - private final Library library; - - private final DependencyVersion version; - - Upgrade(Library library, DependencyVersion version) { - this.library = library; - this.version = version; + Upgrade(Library from, DependencyVersion to) { + this(from, from.withVersion(new LibraryVersion(to))); } - Library getLibrary() { - return this.library; + Upgrade(Library from, Library to) { + this(from, to, withReleaseVersion(to)); } - DependencyVersion getVersion() { - return this.version; + private static Library withReleaseVersion(Library to) { + String version = to.getVersion().toString(); + version = version.replace(".BUILD-SNAPSHOT", ""); + version = version.replace("-SNAPSHOT", ""); + return to.withVersion(new LibraryVersion(DependencyVersion.parse(version))); } } diff --git a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeApplicator.java b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeApplicator.java index 71d8872e4b9..7f818839157 100644 --- a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeApplicator.java +++ b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeApplicator.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2023 the original author or authors. + * Copyright 2012-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -42,16 +42,14 @@ class UpgradeApplicator { Path apply(Upgrade upgrade) throws IOException { String buildFileContents = Files.readString(this.buildFile); - Matcher matcher = Pattern.compile("library\\(\"" + upgrade.getLibrary().getName() + "\", \"(.+)\"\\)") - .matcher(buildFileContents); + String toName = upgrade.to().getName(); + Matcher matcher = Pattern.compile("library\\(\"" + toName + "\", \"(.+)\"\\)").matcher(buildFileContents); if (!matcher.find()) { - matcher = Pattern - .compile("library\\(\"" + upgrade.getLibrary().getName() + "\"\\) \\{\\s+version\\(\"(.+)\"\\)", - Pattern.MULTILINE) + matcher = Pattern.compile("library\\(\"" + toName + "\"\\) \\{\\s+version\\(\"(.+)\"\\)", Pattern.MULTILINE) .matcher(buildFileContents); if (!matcher.find()) { - throw new IllegalStateException("Failed to find definition for library '" - + upgrade.getLibrary().getName() + "' in bom '" + this.buildFile + "'"); + throw new IllegalStateException("Failed to find definition for library '" + upgrade.to().getName() + + "' in bom '" + this.buildFile + "'"); } } String version = matcher.group(1); @@ -68,14 +66,14 @@ class UpgradeApplicator { private void updateGradleProperties(Upgrade upgrade, String version) throws IOException { String property = version.substring(2, version.length() - 1); String gradlePropertiesContents = Files.readString(this.gradleProperties); - String modified = gradlePropertiesContents.replace( - property + "=" + upgrade.getLibrary().getVersion().getVersion(), property + "=" + upgrade.getVersion()); + String modified = gradlePropertiesContents.replace(property + "=" + upgrade.from().getVersion(), + property + "=" + upgrade.to().getVersion()); overwrite(this.gradleProperties, modified); } private void updateBuildFile(Upgrade upgrade, String buildFileContents, int versionStart, int versionEnd) throws IOException { - String modified = buildFileContents.substring(0, versionStart) + upgrade.getVersion() + String modified = buildFileContents.substring(0, versionStart) + upgrade.to().getVersion() + buildFileContents.substring(versionEnd); overwrite(this.buildFile, modified); } diff --git a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeBom.java b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeBom.java index b78628629d6..c6d486e728d 100644 --- a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeBom.java +++ b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeBom.java @@ -16,9 +16,6 @@ package org.springframework.boot.build.bom.bomr; -import java.util.ArrayList; -import java.util.List; - import javax.inject.Inject; import org.gradle.api.Task; @@ -27,8 +24,6 @@ import org.gradle.api.artifacts.dsl.RepositoryHandler; import org.gradle.api.artifacts.repositories.MavenArtifactRepository; import org.springframework.boot.build.bom.BomExtension; -import org.springframework.boot.build.bom.Library.LibraryVersion; -import org.springframework.boot.build.bom.bomr.github.Issue; import org.springframework.boot.build.properties.BuildProperties; /** @@ -63,32 +58,9 @@ public abstract class UpgradeBom extends UpgradeDependencies { "spring-commercial-release"); } - @Override - protected String issueTitle(Upgrade upgrade) { - return "Upgrade to " + upgrade.getLibrary().getName() + " " + upgrade.getVersion(); - } - @Override protected String commitMessage(Upgrade upgrade, int issueNumber) { return issueTitle(upgrade) + "\n\nCloses gh-" + issueNumber; } - @Override - protected String issueBody(Upgrade upgrade, Issue existingUpgrade) { - LibraryVersion upgradeVersion = new LibraryVersion(upgrade.getVersion()); - String releaseNotesLink = upgrade.getLibrary().getLinkUrl("releaseNotes"); - List lines = new ArrayList<>(); - String description = upgrade.getLibrary().getName() + " " + upgradeVersion; - if (releaseNotesLink != null) { - lines.add("Upgrade to [%s](%s).".formatted(description, releaseNotesLink)); - } - else { - lines.add("Upgrade to %s.".formatted(description)); - } - if (existingUpgrade != null) { - lines.add("Supersedes #" + existingUpgrade.getNumber()); - } - return String.join("\n\n", lines); - } - } diff --git a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeDependencies.java b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeDependencies.java index 5d236b2e1fb..713a6b51e0d 100644 --- a/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeDependencies.java +++ b/buildSrc/src/main/java/org/springframework/boot/build/bom/bomr/UpgradeDependencies.java @@ -118,7 +118,7 @@ public abstract class UpgradeDependencies extends DefaultTask { System.out.println("Applying upgrades..."); System.out.println(""); for (Upgrade upgrade : upgrades) { - System.out.println(upgrade.getLibrary().getName() + " " + upgrade.getVersion()); + System.out.println(upgrade.to().getNameAndVersion()); Issue existingUpgradeIssue = findExistingUpgradeIssue(existingUpgradeIssues, upgrade); try { Path modified = this.upgradeApplicator.apply(upgrade); @@ -207,7 +207,7 @@ public abstract class UpgradeDependencies extends DefaultTask { } private Issue findExistingUpgradeIssue(List existingUpgradeIssues, Upgrade upgrade) { - String toMatch = "Upgrade to " + upgrade.getLibrary().getName(); + String toMatch = "Upgrade to " + upgrade.toRelease().getName(); for (Issue existingUpgradeIssue : existingUpgradeIssues) { String title = existingUpgradeIssue.getTitle(); int lastSpaceIndex = title.lastIndexOf(' '); @@ -285,10 +285,21 @@ public abstract class UpgradeDependencies extends DefaultTask { return libraryPredicate.test(library.getName()); } - protected abstract String issueTitle(Upgrade upgrade); - protected abstract String commitMessage(Upgrade upgrade, int issueNumber); - protected abstract String issueBody(Upgrade upgrade, Issue existingUpgrade); + protected String issueTitle(Upgrade upgrade) { + return "Upgrade to " + upgrade.toRelease().getNameAndVersion(); + } + + protected String issueBody(Upgrade upgrade, Issue existingUpgrade) { + String description = upgrade.toRelease().getNameAndVersion(); + String releaseNotesLink = upgrade.toRelease().getLinkUrl("releaseNotes"); + String body = (releaseNotesLink != null) ? "Upgrade to [%s](%s).".formatted(description, releaseNotesLink) + : "Upgrade to %s.".formatted(description); + if (existingUpgrade != null) { + body += "\n\nSupersedes #" + existingUpgrade.getNumber(); + } + return body; + } } diff --git a/buildSrc/src/test/java/org/springframework/boot/build/bom/bomr/InteractiveUpgradeResolverTests.java b/buildSrc/src/test/java/org/springframework/boot/build/bom/bomr/InteractiveUpgradeResolverTests.java new file mode 100644 index 00000000000..25ba712301a --- /dev/null +++ b/buildSrc/src/test/java/org/springframework/boot/build/bom/bomr/InteractiveUpgradeResolverTests.java @@ -0,0 +1,73 @@ +/* + * Copyright 2024-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.build.bom.bomr; + +import java.util.ArrayList; +import java.util.List; + +import org.gradle.api.internal.tasks.userinput.UserInputHandler; +import org.gradle.api.provider.Provider; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.build.bom.Library; +import org.springframework.boot.build.bom.Library.LibraryVersion; +import org.springframework.boot.build.bom.bomr.version.DependencyVersion; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Tests for {@link InteractiveUpgradeResolver}. + * + * @author Phillip Webb + */ +class InteractiveUpgradeResolverTests { + + @Test + void resolveUpgradeUpdateVersionNumberInLibrary() { + UserInputHandler userInputHandler = mock(UserInputHandler.class); + LibraryUpdateResolver libaryUpdateResolver = mock(LibraryUpdateResolver.class); + InteractiveUpgradeResolver upgradeResolver = new InteractiveUpgradeResolver(userInputHandler, + libaryUpdateResolver); + List libraries = new ArrayList<>(); + DependencyVersion version = DependencyVersion.parse("1.0.0"); + LibraryVersion libraryVersion = new LibraryVersion(version); + Library library = new Library("test", null, libraryVersion, null, null, false, null, null, null, null); + libraries.add(library); + List librariesToUpgrade = new ArrayList<>(); + librariesToUpgrade.add(library); + List updates = new ArrayList<>(); + DependencyVersion updateVersion = DependencyVersion.parse("1.0.1"); + VersionOption versionOption = new VersionOption(updateVersion); + updates.add(new LibraryWithVersionOptions(library, List.of(versionOption))); + given(libaryUpdateResolver.findLibraryUpdates(any(), any())).willReturn(updates); + Provider providerOfVersionOption = providerOf(versionOption); + given(userInputHandler.askUser(any())).willReturn(providerOfVersionOption); + List upgrades = upgradeResolver.resolveUpgrades(librariesToUpgrade, libraries); + assertThat(upgrades.get(0).to().getVersion().getVersion()).isEqualTo(updateVersion); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private Provider providerOf(VersionOption versionOption) { + Provider provider = mock(Provider.class); + given(provider.get()).willReturn(versionOption); + return provider; + } + +} diff --git a/buildSrc/src/test/java/org/springframework/boot/build/bom/bomr/UpgradeTests.java b/buildSrc/src/test/java/org/springframework/boot/build/bom/bomr/UpgradeTests.java new file mode 100644 index 00000000000..ee7a0c3e9b4 --- /dev/null +++ b/buildSrc/src/test/java/org/springframework/boot/build/bom/bomr/UpgradeTests.java @@ -0,0 +1,54 @@ +/* + * Copyright 2024-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.build.bom.bomr; + +import org.junit.jupiter.api.Test; + +import org.springframework.boot.build.bom.Library; +import org.springframework.boot.build.bom.Library.LibraryVersion; +import org.springframework.boot.build.bom.bomr.version.DependencyVersion; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link Upgrade}. + * + * @author Phillip Webb + */ +class UpgradeTests { + + @Test + void createToRelease() { + Library from = new Library("Test", null, new LibraryVersion(DependencyVersion.parse("1.0.0")), null, null, + false, null, null, null, null); + Upgrade upgrade = new Upgrade(from, DependencyVersion.parse("1.0.1")); + assertThat(upgrade.from().getNameAndVersion()).isEqualTo("Test 1.0.0"); + assertThat(upgrade.to().getNameAndVersion()).isEqualTo("Test 1.0.1"); + assertThat(upgrade.toRelease().getNameAndVersion()).isEqualTo("Test 1.0.1"); + } + + @Test + void createToSnapshot() { + Library from = new Library("Test", null, new LibraryVersion(DependencyVersion.parse("1.0.0")), null, null, + false, null, null, null, null); + Upgrade upgrade = new Upgrade(from, DependencyVersion.parse("1.0.1-SNAPSHOT")); + assertThat(upgrade.from().getNameAndVersion()).isEqualTo("Test 1.0.0"); + assertThat(upgrade.to().getNameAndVersion()).isEqualTo("Test 1.0.1-SNAPSHOT"); + assertThat(upgrade.toRelease().getNameAndVersion()).isEqualTo("Test 1.0.1"); + } + +}