Skip to content

Commit

Permalink
fix issue with getReplacement when version is empty string
Browse files Browse the repository at this point in the history
It is possible that JakartaPackageAlignmentPlugin will call
getReplacement with an empty string for the version. Previously, this
code would erroneously return a replacement version because the
comparator for ComparableVersion returns -1 when an empty string is
compared with the maximum Jakarta version.

To address that, only look for a replacement when the version is
non-null and non-empty.

Fixes #25
  • Loading branch information
Brian Laub committed Feb 8, 2023
1 parent 271351e commit 8f9b9ef
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ public final class VersionMappings {
private VersionMappings() {}

public static Optional<MavenCoordinate> getReplacement(String group, String name, String version) {
// https://github.com/palantir/jakarta-package-alignment/issues/25
// we must have non-null, non-empty version, otherwise the version comparison below
// can return the incorrect result
// this can happen if getReplacement is called from a ModuleComponentSelector that only has the group/name set
if (version == null || version.isEmpty()) {
return Optional.empty();
}

String key = group + ":" + name;
VersionMapping mapping = mappings.get(key);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import groovy.transform.CompileStatic
import nebula.test.IntegrationSpec
import nebula.test.dependencies.DependencyGraph
import nebula.test.dependencies.GradleDependencyGenerator
import nebula.test.functional.ExecutionResult

/**
* Integration tests for {@link JakartaPackageAlignmentPlugin}.
Expand Down Expand Up @@ -110,4 +111,41 @@ class JakartaPackageAlignmentPluginIntegrationSpec extends IntegrationSpec {
versionsLockFile.text.contains("jakarta.servlet:jakarta.servlet-api:5.0.0")
!versionsLockFile.text.contains("javax.servlet:javax.servlet-api")
}

def "leaves usage of acceptable versions untouched, but still replaces bad versions"() {
setup:
mavenRepo = generateMavenRepo(
"org:direct-dep:1.0.0 -> org:transitive-dep:1.2.3",
"org:transitive-dep:1.2.3 -> jakarta.ws.rs:jakarta.ws.rs-api:2.1.6",
"org:direct-dep:2.0.0 -> org:transitive-dep:2.0.0",
"org:transitive-dep:2.0.0 -> jakarta.servlet:jakarta.servlet-api:5.0.0",
"jakarta.ws.rs:jakarta.ws.rs-api:3.1.0",
"jakarta.ws.rs:jakarta.ws.rs-api:2.1.6",
"javax.ws.rs:javax.ws.rs-api:2.1.1"
)

buildFile << """
dependencies {
implementation "org:direct-dep"
implementation "jakarta.ws.rs:jakarta.ws.rs-api"
}
""".stripIndent()

// no idea why stripIndent isnt working here, maybe the encoding on this file is weird?
versionsProps << """org:direct-dep = 1.0.0\njakarta.ws.rs:jakarta.ws.rs-api = 3.1.0""".stripIndent()

when:
runTasksSuccessfully("--write-locks")
ExecutionResult result = runTasksSuccessfully("dependencies", "--configuration", "runtimeClasspath")

then:
def versionsLockFile = new File(projectDir, "versions.lock")
fileExists("versions.lock")
versionsLockFile.text.contains("jakarta.ws.rs:jakarta.ws.rs-api:3.1.0")
versionsLockFile.text.contains("javax.ws.rs:javax.ws.rs-api:2.1.1")
!versionsLockFile.text.contains("jakarta.ws.rs:jakarta.ws.rs-api:2.1.6")

result.standardOutput.contains("jakarta.ws.rs:jakarta.ws.rs-api -> 3.1.0")
result.standardOutput.contains("jakarta.ws.rs:jakarta.ws.rs-api:2.1.6 -> javax.ws.rs:javax.ws.rs-api:2.1.1")
}
}

0 comments on commit 8f9b9ef

Please sign in to comment.