From 74b8163b19b065794180ffa8776c34bfecb91d45 Mon Sep 17 00:00:00 2001 From: Tyler Van Gorder Date: Mon, 12 Sep 2022 13:52:12 -0700 Subject: [PATCH 1/3] Configuration for how failures in UpgradeDependencyVersion should be handled --- .../org/openrewrite/RecipeRunException.java | 6 +- .../maven/UpgradeDependencyVersion.java | 193 +++++++++++------- 2 files changed, 121 insertions(+), 78 deletions(-) diff --git a/rewrite-core/src/main/java/org/openrewrite/RecipeRunException.java b/rewrite-core/src/main/java/org/openrewrite/RecipeRunException.java index 02891749275..3b34f4810c7 100644 --- a/rewrite-core/src/main/java/org/openrewrite/RecipeRunException.java +++ b/rewrite-core/src/main/java/org/openrewrite/RecipeRunException.java @@ -41,12 +41,12 @@ public RecipeRunException(Throwable cause) { this(cause, null); } - public String getSanitizedStackTrace() { + public static String getSanitizedStackTrace(Throwable cause) { StringJoiner sanitized = new StringJoiner("\n"); - sanitized.add(getCause().getClass().getName() + ": " + getCause().getLocalizedMessage()); + sanitized.add(cause.getClass().getName() + ": " + cause.getLocalizedMessage()); int i = 0; - for (StackTraceElement stackTraceElement : getCause().getStackTrace()) { + for (StackTraceElement stackTraceElement : cause.getStackTrace()) { if (stackTraceElement.getClassName().equals(RecipeScheduler.class.getName())) { break; } diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 6314d2a7b1f..f193a483aba 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -20,6 +20,7 @@ import org.openrewrite.*; import org.openrewrite.internal.ListUtils; import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.maven.internal.MavenDownloadingException; import org.openrewrite.maven.tree.*; import org.openrewrite.semver.Semver; import org.openrewrite.semver.VersionComparator; @@ -75,6 +76,14 @@ public class UpgradeDependencyVersion extends Recipe { @Nullable Boolean overrideManagedVersion; + @Option(displayName = "Metadata failure mode", + description = "In the event that metadata for a matching dependency cannot be determined, this option determines how to handle the failure", + example = "fail", + valid = {"ignore", "warn", "fail"}, + required = false) + @Nullable + String metadataFailureMode; + @SuppressWarnings("ConstantConditions") @Override public Validated validate() { @@ -140,105 +149,139 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) { @Override public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { Xml.Tag t = super.visitTag(tag, ctx); - if (isDependencyTag(groupId, artifactId)) { - - ResolvedDependency d = findDependency(tag); - if (d != null && d.getRepository() != null) { - //If the resolved dependency exists AND it does not represent an artifact that was parsed - //as a source file, attempt to find a new version. - String newerVersion = findNewerVersion(d.getGroupId(), d.getArtifactId(), d.getVersion(), ctx); - if (newerVersion != null) { - ResolvedManagedDependency dm = findManagedDependency(t); - if (dm != null) { - String requestedVersion = dm.getRequested().getVersion(); - if (requestedVersion.startsWith("${")) { - doAfterVisit(new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false)); - return t; + try { + if (isDependencyTag(groupId, artifactId)) { + + ResolvedDependency d = findDependency(tag); + if (d != null && d.getRepository() != null) { + //If the resolved dependency exists AND it does not represent an artifact that was parsed + //as a source file, attempt to find a new version. + String newerVersion = findNewerVersion(d.getGroupId(), d.getArtifactId(), d.getVersion(), ctx); + if (newerVersion != null) { + ResolvedManagedDependency dm = findManagedDependency(t); + if (dm != null) { + String requestedVersion = dm.getRequested().getVersion(); + if (requestedVersion.startsWith("${")) { + doAfterVisit(new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false)); + return t; + } } - } - Optional version = t.getChild("version"); - if (version.isPresent()) { - String requestedVersion = d.getRequested().getVersion(); - if (requestedVersion != null && requestedVersion.startsWith("${")) { - doAfterVisit(new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false)); - return t; + Optional version = t.getChild("version"); + if (version.isPresent()) { + String requestedVersion = d.getRequested().getVersion(); + if (requestedVersion != null && requestedVersion.startsWith("${")) { + doAfterVisit(new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false)); + return t; + } + t = (Xml.Tag) new ChangeTagValueVisitor<>(version.get(), newerVersion).visitNonNull(t, 0, getCursor()); + } else if (Boolean.TRUE.equals(overrideManagedVersion)) { + //If the version is not present and the override managed version is set, add a new, explicit version tag. + Xml.Tag versionTag = Xml.Tag.build("" + newerVersion + ""); + //noinspection ConstantConditions + t = (Xml.Tag) new AddToTagVisitor(t, versionTag, new MavenTagInsertionComparator(t.getChildren())).visitNonNull(t, ctx, getCursor().getParent()); } - t = (Xml.Tag) new ChangeTagValueVisitor(version.get(), newerVersion).visitNonNull(t, 0, getCursor()); - } else if (Boolean.TRUE.equals(overrideManagedVersion)) { - //If the version is not present and the override managed version is set, add a new, explicit version tag. - Xml.Tag versionTag = Xml.Tag.build("" + newerVersion + ""); - //noinspection ConstantConditions - t = (Xml.Tag) new AddToTagVisitor(t, versionTag, new MavenTagInsertionComparator(t.getChildren())).visitNonNull(t, ctx, getCursor().getParent()); } } - } - } else if (isManagedDependencyTag(groupId, artifactId)) { + } else if (isManagedDependencyTag(groupId, artifactId)) { - ResolvedManagedDependency matchedManagedDependency = findManagedDependency(t); - if (matchedManagedDependency != null) { - if (!projectArtifacts.contains(new GroupArtifact(matchedManagedDependency.getGroupId(), matchedManagedDependency.getArtifactId())) && - matchesGlob(matchedManagedDependency.getGroupId(), groupId) && matchesGlob(matchedManagedDependency.getArtifactId(), artifactId)) { + ResolvedManagedDependency matchedManagedDependency = findManagedDependency(t); + if (matchedManagedDependency != null) { + if (!projectArtifacts.contains(new GroupArtifact(matchedManagedDependency.getGroupId(), matchedManagedDependency.getArtifactId())) && + matchesGlob(matchedManagedDependency.getGroupId(), groupId) && matchesGlob(matchedManagedDependency.getArtifactId(), artifactId)) { - String requestedVersion = matchedManagedDependency.getRequested().getVersion(); - assert (matchedManagedDependency.getVersion() != null); - String newerVersion = findNewerVersion(matchedManagedDependency.getGroupId(), matchedManagedDependency.getArtifactId(), matchedManagedDependency.getVersion(), ctx); - if (newerVersion != null) { - if (requestedVersion.startsWith("${")) { - doAfterVisit(new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false)); - return t; - } - Xml.Tag childVersionTag = t.getChild("version").orElse(null); - if (childVersionTag != null) { - t = (Xml.Tag) new ChangeTagValueVisitor(childVersionTag, newerVersion).visitNonNull(t, 0, getCursor()); + String requestedVersion = matchedManagedDependency.getRequested().getVersion(); + assert (matchedManagedDependency.getVersion() != null); + String newerVersion = findNewerVersion(matchedManagedDependency.getGroupId(), matchedManagedDependency.getArtifactId(), matchedManagedDependency.getVersion(), ctx); + if (newerVersion != null) { + if (requestedVersion.startsWith("${")) { + doAfterVisit(new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false)); + return t; + } + Xml.Tag childVersionTag = t.getChild("version").orElse(null); + if (childVersionTag != null) { + t = (Xml.Tag) new ChangeTagValueVisitor(childVersionTag, newerVersion).visitNonNull(t, 0, getCursor()); + } } } - } - } else { - for (ResolvedManagedDependency dm : getResolutionResult().getPom().getDependencyManagement()) { - if (dm.getBomGav() != null) { - String tagGroup = getResolutionResult().getPom().getValue(tag.getChildValue("groupId").orElse(getResolutionResult().getPom().getGroupId())); - String tagArtifactId = getResolutionResult().getPom().getValue(tag.getChildValue("artifactId").orElse("")); - - if (!projectArtifacts.contains(new GroupArtifact(tagGroup, tagArtifactId))) { - ResolvedGroupArtifactVersion bom = dm.getBomGav(); - - if (tagGroup != null && tagArtifactId != null && tagGroup.equals(bom.getGroupId()) && tagArtifactId.equals(bom.getArtifactId())) { - - //noinspection ConstantConditions - String requestedVersion = dm.getRequestedBom().getVersion(); - String newerVersion = findNewerVersion(bom.getGroupId(), bom.getArtifactId(), bom.getVersion(), ctx); - if (newerVersion != null) { - if (requestedVersion.startsWith("${")) { - doAfterVisit(new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false)); - return t; - } - Xml.Tag childVersionTag = t.getChild("version").orElse(null); - if (childVersionTag != null) { - t = (Xml.Tag) new ChangeTagValueVisitor(childVersionTag, newerVersion).visitNonNull(t, 0, getCursor()); + } else { + for (ResolvedManagedDependency dm : getResolutionResult().getPom().getDependencyManagement()) { + if (dm.getBomGav() != null) { + String tagGroup = getResolutionResult().getPom().getValue(tag.getChildValue("groupId").orElse(getResolutionResult().getPom().getGroupId())); + String tagArtifactId = getResolutionResult().getPom().getValue(tag.getChildValue("artifactId").orElse("")); + + if (!projectArtifacts.contains(new GroupArtifact(tagGroup, tagArtifactId))) { + ResolvedGroupArtifactVersion bom = dm.getBomGav(); + + if (tagGroup != null && tagArtifactId != null && tagGroup.equals(bom.getGroupId()) && tagArtifactId.equals(bom.getArtifactId())) { + + //noinspection ConstantConditions + String requestedVersion = dm.getRequestedBom().getVersion(); + String newerVersion = findNewerVersion(bom.getGroupId(), bom.getArtifactId(), bom.getVersion(), ctx); + if (newerVersion != null) { + if (requestedVersion.startsWith("${")) { + doAfterVisit(new ChangePropertyValue(requestedVersion.substring(2, requestedVersion.length() - 1), newerVersion, overrideManagedVersion, false)); + return t; + } + Xml.Tag childVersionTag = t.getChild("version").orElse(null); + if (childVersionTag != null) { + t = (Xml.Tag) new ChangeTagValueVisitor(childVersionTag, newerVersion).visitNonNull(t, 0, getCursor()); + } } + break; } - break; } } } } } + } catch (MavenDownloadingException exception) { + FailureMode failureMode = FailureMode.getFailureMode(metadataFailureMode); + if (failureMode == FailureMode.FAIL) { + throw exception; + } else if (failureMode == FailureMode.IGNORE) { + ctx.getOnError().accept(exception); + } else if (failureMode == FailureMode.WARN) { + return t.withMarkers(t.getMarkers().searchResult(UncaughtVisitorException.getSanitizedStackTrace(exception))); + } } return t; } @Nullable private String findNewerVersion(String groupId, String artifactId, String version, ExecutionContext ctx) { - - MavenMetadata mavenMetadata = downloadMetadata(groupId, artifactId, ctx); - List versions = new ArrayList<>(); - for (String v : mavenMetadata.getVersioning().getVersions()) { - if (versionComparator.isValid(version, v)) { - versions.add(v); + try { + MavenMetadata mavenMetadata = downloadMetadata(groupId, artifactId, ctx); + List versions = new ArrayList<>(); + for (String v : mavenMetadata.getVersioning().getVersions()) { + if (versionComparator.isValid(version, v)) { + versions.add(v); + } } + return versionComparator.upgrade(version, versions).orElse(null); + } catch (IllegalStateException e) { + //This can happen when we encounter exotic versions. Pass the error to the error handler and + //in the spirit of "do no harm", return null. + ctx.getOnError().accept(e); + return null; } - return versionComparator.upgrade(version, versions).orElse(null); } } + + private enum FailureMode { + IGNORE, + WARN, + FAIL; + + private static FailureMode getFailureMode(@Nullable String configuration) { + if ("ignore".equals(configuration)) { + return FailureMode.IGNORE; + } else if ("warn".equals(configuration)) { + return FailureMode.WARN; + } else { + return FailureMode.FAIL; + } + } + } + } From 78e9eee1dc5959a2fe694db78f492a2515eebf87 Mon Sep 17 00:00:00 2001 From: Tyler Van Gorder Date: Mon, 12 Sep 2022 14:15:00 -0700 Subject: [PATCH 2/3] Add warnings when metadata cannot be downloaded --- .../maven/UpgradeDependencyVersion.java | 35 ++----------------- 1 file changed, 2 insertions(+), 33 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index f193a483aba..62e7da51661 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -76,14 +76,6 @@ public class UpgradeDependencyVersion extends Recipe { @Nullable Boolean overrideManagedVersion; - @Option(displayName = "Metadata failure mode", - description = "In the event that metadata for a matching dependency cannot be determined, this option determines how to handle the failure", - example = "fail", - valid = {"ignore", "warn", "fail"}, - required = false) - @Nullable - String metadataFailureMode; - @SuppressWarnings("ConstantConditions") @Override public Validated validate() { @@ -236,14 +228,8 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { } } } catch (MavenDownloadingException exception) { - FailureMode failureMode = FailureMode.getFailureMode(metadataFailureMode); - if (failureMode == FailureMode.FAIL) { - throw exception; - } else if (failureMode == FailureMode.IGNORE) { - ctx.getOnError().accept(exception); - } else if (failureMode == FailureMode.WARN) { - return t.withMarkers(t.getMarkers().searchResult(UncaughtVisitorException.getSanitizedStackTrace(exception))); - } + ctx.getOnError().accept(exception); + return t.withMarkers(t.getMarkers().searchResult(UncaughtVisitorException.getSanitizedStackTrace(exception))); } return t; } @@ -267,21 +253,4 @@ private String findNewerVersion(String groupId, String artifactId, String versio } } } - - private enum FailureMode { - IGNORE, - WARN, - FAIL; - - private static FailureMode getFailureMode(@Nullable String configuration) { - if ("ignore".equals(configuration)) { - return FailureMode.IGNORE; - } else if ("warn".equals(configuration)) { - return FailureMode.WARN; - } else { - return FailureMode.FAIL; - } - } - } - } From 7ed9a55c84137c2861abd516bac25eacbe45f112 Mon Sep 17 00:00:00 2001 From: Tyler Van Gorder Date: Tue, 13 Sep 2022 11:50:10 -0700 Subject: [PATCH 3/3] Leverage wither to add downloading exceptions. --- .../java/org/openrewrite/RecipeRunException.java | 6 +++--- .../src/main/java/org/openrewrite/Tree.java | 1 + .../openrewrite/maven/UpgradeDependencyVersion.java | 13 +++++++++++-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/rewrite-core/src/main/java/org/openrewrite/RecipeRunException.java b/rewrite-core/src/main/java/org/openrewrite/RecipeRunException.java index 3b34f4810c7..02891749275 100644 --- a/rewrite-core/src/main/java/org/openrewrite/RecipeRunException.java +++ b/rewrite-core/src/main/java/org/openrewrite/RecipeRunException.java @@ -41,12 +41,12 @@ public RecipeRunException(Throwable cause) { this(cause, null); } - public static String getSanitizedStackTrace(Throwable cause) { + public String getSanitizedStackTrace() { StringJoiner sanitized = new StringJoiner("\n"); - sanitized.add(cause.getClass().getName() + ": " + cause.getLocalizedMessage()); + sanitized.add(getCause().getClass().getName() + ": " + getCause().getLocalizedMessage()); int i = 0; - for (StackTraceElement stackTraceElement : cause.getStackTrace()) { + for (StackTraceElement stackTraceElement : getCause().getStackTrace()) { if (stackTraceElement.getClassName().equals(RecipeScheduler.class.getName())) { break; } diff --git a/rewrite-core/src/main/java/org/openrewrite/Tree.java b/rewrite-core/src/main/java/org/openrewrite/Tree.java index e7797b0ab7d..939acfc17e3 100644 --- a/rewrite-core/src/main/java/org/openrewrite/Tree.java +++ b/rewrite-core/src/main/java/org/openrewrite/Tree.java @@ -134,6 +134,7 @@ default T2 safeCast() { @SuppressWarnings("unchecked") default T withException(Throwable throwable, @Nullable ExecutionContext ctx) { if (ctx != null) { + ctx.getOnError().accept(throwable); ctx.putMessage(Recipe.PANIC, "true"); } RecipeRunException rre; diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java index 62e7da51661..875714f1cb0 100755 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/UpgradeDependencyVersion.java @@ -22,6 +22,7 @@ import org.openrewrite.internal.lang.Nullable; import org.openrewrite.maven.internal.MavenDownloadingException; import org.openrewrite.maven.tree.*; +import org.openrewrite.semver.LatestPatch; import org.openrewrite.semver.Semver; import org.openrewrite.semver.VersionComparator; import org.openrewrite.xml.AddToTagVisitor; @@ -228,14 +229,22 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) { } } } catch (MavenDownloadingException exception) { - ctx.getOnError().accept(exception); - return t.withMarkers(t.getMarkers().searchResult(UncaughtVisitorException.getSanitizedStackTrace(exception))); + return t.withException(exception, ctx); } return t; } @Nullable private String findNewerVersion(String groupId, String artifactId, String version, ExecutionContext ctx) { + + if (versionComparator instanceof LatestPatch) { + //In the case of latest patch, a new version can only be derived if the current version is a semantic + //version. Check if the current version is valid candidate before attempting to download metadata. + if (!versionComparator.isValid(version, version)) { + return null; + } + } + try { MavenMetadata mavenMetadata = downloadMetadata(groupId, artifactId, ctx); List versions = new ArrayList<>();