From ec1e66a8cc764cfd9f84b67ee75c6f06bda0e36e Mon Sep 17 00:00:00 2001 From: Ashley Scopes <73482956+ascopes@users.noreply.github.com> Date: Sat, 6 Apr 2024 13:37:30 +0100 Subject: [PATCH] Change PlatformArtifactFactory to only emit classifier strings --- .../dependency/BinaryPluginResolver.java | 17 +- ...ry.java => PlatformClassifierFactory.java} | 33 +--- .../dependency/ProtocResolver.java | 20 +-- .../PlatformArtifactFactoryTest.java | 164 ------------------ .../PlatformClassifierFactoryTest.java | 119 +++++++++++++ 5 files changed, 141 insertions(+), 212 deletions(-) rename src/main/java/io/github/ascopes/protobufmavenplugin/dependency/{PlatformArtifactFactory.java => PlatformClassifierFactory.java} (68%) delete mode 100644 src/test/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformArtifactFactoryTest.java create mode 100644 src/test/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformClassifierFactoryTest.java diff --git a/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/BinaryPluginResolver.java b/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/BinaryPluginResolver.java index 12347e78..831c300d 100644 --- a/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/BinaryPluginResolver.java +++ b/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/BinaryPluginResolver.java @@ -38,19 +38,19 @@ public final class BinaryPluginResolver { private final MavenDependencyPathResolver dependencyResolver; - private final PlatformArtifactFactory platformDependencyFactory; + private final PlatformClassifierFactory platformClassifierFactory; private final SystemPathBinaryResolver systemPathResolver; private final UrlResourceFetcher urlResourceFetcher; @Inject public BinaryPluginResolver( MavenDependencyPathResolver dependencyResolver, - PlatformArtifactFactory platformDependencyFactory, + PlatformClassifierFactory platformClassifierFactory, SystemPathBinaryResolver systemPathResolver, UrlResourceFetcher urlResourceFetcher ) { this.dependencyResolver = dependencyResolver; - this.platformDependencyFactory = platformDependencyFactory; + this.platformClassifierFactory = platformClassifierFactory; this.systemPathResolver = systemPathResolver; this.urlResourceFetcher = urlResourceFetcher; } @@ -78,13 +78,10 @@ private ResolvedPlugin resolveMavenPlugin( MavenSession session, MavenArtifact plugin ) throws ResolutionException { - plugin = platformDependencyFactory.createArtifact( - plugin.getGroupId().orElse(null), - plugin.getArtifactId().orElse(null), - plugin.getVersion().orElse(null), - plugin.getType().orElse("exe"), - plugin.getClassifier().orElse(null) - ); + var artifactId = plugin.getArtifactId().orElse(null); + var classifier = plugin.getClassifier() + .orElseGet(() -> platformClassifierFactory.getClassifier(artifactId)); + plugin.setClassifier(classifier); // Only one dependency should ever be returned here. var path = dependencyResolver.resolveOne(session, plugin, DependencyResolutionDepth.DIRECT) diff --git a/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformArtifactFactory.java b/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformClassifierFactory.java similarity index 68% rename from src/main/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformArtifactFactory.java rename to src/main/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformClassifierFactory.java index d01b1380..1509ccb3 100644 --- a/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformArtifactFactory.java +++ b/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformClassifierFactory.java @@ -16,49 +16,26 @@ package io.github.ascopes.protobufmavenplugin.dependency; -import static java.util.Objects.requireNonNullElse; -import static java.util.Objects.requireNonNullElseGet; - -import io.github.ascopes.protobufmavenplugin.MavenArtifact; import io.github.ascopes.protobufmavenplugin.platform.HostSystem; import javax.inject.Inject; import javax.inject.Named; -import org.jspecify.annotations.Nullable; /** - * Factory that can produce dependencies marked with platform-specific classifiers. + * Factory that can produce classifiers for dependencies based on the current platform. * * @author Ashley Scopes */ @Named -public final class PlatformArtifactFactory { +public final class PlatformClassifierFactory { + private final HostSystem hostSystem; @Inject - public PlatformArtifactFactory(HostSystem hostSystem) { + public PlatformClassifierFactory(HostSystem hostSystem) { this.hostSystem = hostSystem; } - public MavenArtifact createArtifact( - String groupId, - String artifactId, - String version, - @Nullable String extension, - @Nullable String classifier - ) { - var artifact = new MavenArtifact(); - artifact.setGroupId(groupId); - artifact.setArtifactId(artifactId); - artifact.setVersion(version); - artifact.setType(requireNonNullElse(extension, "exe")); - artifact.setClassifier(requireNonNullElseGet( - classifier, - () -> getPlatformExecutableClassifier(artifactId) - )); - return artifact; - } - - private String getPlatformExecutableClassifier(String artifactId) { + public String getClassifier(String artifactId) { var rawOs = hostSystem.getOperatingSystem(); var rawArch = hostSystem.getCpuArchitecture(); diff --git a/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/ProtocResolver.java b/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/ProtocResolver.java index e98a58f4..c392b4f7 100644 --- a/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/ProtocResolver.java +++ b/src/main/java/io/github/ascopes/protobufmavenplugin/dependency/ProtocResolver.java @@ -17,6 +17,7 @@ package io.github.ascopes.protobufmavenplugin.dependency; import io.github.ascopes.protobufmavenplugin.DependencyResolutionDepth; +import io.github.ascopes.protobufmavenplugin.MavenArtifact; import io.github.ascopes.protobufmavenplugin.platform.FileUtils; import io.github.ascopes.protobufmavenplugin.platform.HostSystem; import java.io.IOException; @@ -44,7 +45,7 @@ public final class ProtocResolver { private final HostSystem hostSystem; private final MavenDependencyPathResolver dependencyResolver; - private final PlatformArtifactFactory platformArtifactFactory; + private final PlatformClassifierFactory platformClassifierFactory; private final SystemPathBinaryResolver systemPathResolver; private final UrlResourceFetcher urlResourceFetcher; @@ -52,13 +53,13 @@ public final class ProtocResolver { public ProtocResolver( HostSystem hostSystem, MavenDependencyPathResolver dependencyResolver, - PlatformArtifactFactory platformArtifactFactory, + PlatformClassifierFactory platformClassifierFactory, SystemPathBinaryResolver systemPathResolver, UrlResourceFetcher urlResourceFetcher ) { this.hostSystem = hostSystem; this.dependencyResolver = dependencyResolver; - this.platformArtifactFactory = platformArtifactFactory; + this.platformClassifierFactory = platformClassifierFactory; this.systemPathResolver = systemPathResolver; this.urlResourceFetcher = urlResourceFetcher; } @@ -107,13 +108,12 @@ private Path resolveFromMavenRepositories( ); } - var artifact = platformArtifactFactory.createArtifact( - GROUP_ID, - ARTIFACT_ID, - version, - null, - null - ); + var artifact = new MavenArtifact(); + artifact.setGroupId(GROUP_ID); + artifact.setArtifactId(ARTIFACT_ID); + artifact.setVersion(version); + artifact.setType("exe"); + artifact.setClassifier(platformClassifierFactory.getClassifier(ARTIFACT_ID)); return dependencyResolver.resolveOne(session, artifact, DependencyResolutionDepth.DIRECT) .iterator() diff --git a/src/test/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformArtifactFactoryTest.java b/src/test/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformArtifactFactoryTest.java deleted file mode 100644 index 43e301a3..00000000 --- a/src/test/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformArtifactFactoryTest.java +++ /dev/null @@ -1,164 +0,0 @@ -/* - * Copyright (C) 2023 - 2024, Ashley Scopes. - * - * 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 - * - * http://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 io.github.ascopes.protobufmavenplugin.dependency; - -import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.arch; -import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.hostSystem; -import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.linux; -import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.macOs; -import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.otherOs; -import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.windows; -import static io.github.ascopes.protobufmavenplugin.fixtures.RandomFixtures.someText; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.assertj.core.api.SoftAssertions.assertSoftly; -import static org.junit.jupiter.params.provider.Arguments.arguments; - -import io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.HostSystemMockConfigurer; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.stream.Stream; -import org.jspecify.annotations.Nullable; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; - - -/** - * @author Ashley Scopes - */ -@DisplayName("PlatformArtifactFactory tests") -class PlatformArtifactFactoryTest { - - @DisplayName(".createArtifact(...) returns the expected results on valid systems") - @MethodSource("validClassifierCases") - @ParameterizedTest(name = "for system {0}, extension {1}, classifier {3}, expect " - + "extension {2} and classifier {4}") - void createArtifactReturnsExpectedResultsOnValidSystems( - HostSystemMockConfigurer configurer, - @Nullable String givenExtension, - String expectedExtension, - @Nullable String givenClassifier, - String expectedClassifier - ) { - // Given - var hostSystem = hostSystem(); - configurer.configure(hostSystem); - var groupId = someText(); - var artifactId = someText(); - var version = someText(); - var factory = new PlatformArtifactFactory(hostSystem); - - // When - var artifact = factory.createArtifact( - groupId, - artifactId, - version, - givenExtension, - givenClassifier - ); - - // Then - assertSoftly(softly -> { - softly.assertThat(artifact.getGroupId().orElse(null)) - .as("groupId") - .isEqualTo(groupId); - softly.assertThat(artifact.getArtifactId().orElse(null)) - .as("artifactId") - .isEqualTo(artifactId); - softly.assertThat(artifact.getVersion().orElse(null)) - .as("version") - .isEqualTo(version); - softly.assertThat(artifact.getType().orElse(null)) - .as("type") - .isEqualTo(expectedExtension); - softly.assertThat(artifact.getClassifier().orElse(null)) - .as("classifier") - .isEqualTo(expectedClassifier); - }); - } - - @DisplayName(".createArtifact(...) raises an exception for unknown systems") - @MethodSource("invalidClassifierCases") - @ParameterizedTest(name = "when {0}, then expect an exception") - void createArtifactRaisesAnExceptionForUnknownSystems( - HostSystemMockConfigurer configurer - ) { - // Given - var hostSystem = hostSystem(); - configurer.configure(hostSystem); - var groupId = someText(); - var artifactId = someText(); - var version = someText(); - var factory = new PlatformArtifactFactory(hostSystem); - - // Then - assertThatThrownBy(() -> factory.createArtifact(groupId, artifactId, version, null, null)) - .isInstanceOf(UnsupportedOperationException.class) - .hasMessageMatching( - "No '[^']+' binary is available for reported OS '[^']+' and CPU architecture '[^']+'" - ); - } - - static Stream validClassifierCases() { - var systemToClassifiers = new HashMap(); - systemToClassifiers.put(linux().and(arch("amd64")), "linux-x86_64"); - systemToClassifiers.put(linux().and(arch("aarch64")), "linux-aarch_64"); - systemToClassifiers.put(linux().and(arch("s390x")), "linux-s390_64"); - systemToClassifiers.put(linux().and(arch("zarch_64")), "linux-s390_64"); - systemToClassifiers.put(linux().and(arch("ppc64le")), "linux-ppcle_64"); - systemToClassifiers.put(linux().and(arch("ppc64")), "linux-ppcle_64"); - systemToClassifiers.put(macOs().and(arch("amd64")), "osx-x86_64"); - systemToClassifiers.put(macOs().and(arch("x86_64")), "osx-x86_64"); - systemToClassifiers.put(macOs().and(arch("aarch64")), "osx-aarch_64"); - systemToClassifiers.put(windows().and(arch("amd64")), "windows-x86_64"); - systemToClassifiers.put(windows().and(arch("x86_64")), "windows-x86_64"); - systemToClassifiers.put(windows().and(arch("x86")), "windows-x86_32"); - systemToClassifiers.put(windows().and(arch("x86_32")), "windows-x86_32"); - - var cases = new ArrayList(); - systemToClassifiers.forEach((system, expectedClassifier) -> { - cases.add(arguments(system, null, "exe", null, expectedClassifier)); - cases.add(arguments(system, "foobar", "foobar", null, expectedClassifier)); - }); - - systemToClassifiers.keySet().forEach(system -> { - cases.add(arguments(system, null, "exe", "bazbork", "bazbork")); - cases.add(arguments(system, "foobar", "foobar", "bazbork", "bazbork")); - }); - - return cases.stream(); - } - - static Stream invalidClassifierCases() { - return Stream.of( - linux().and(arch("some unknown CPU arch")), - macOs().and(arch("some unknown CPU arch")), - windows().and(arch("some unknown CPU arch")), - otherOs().and(arch("x86_32")), - otherOs().and(arch("x86_64")), - otherOs().and(arch("amd64")), - otherOs().and(arch("aarch64")), - otherOs().and(arch("ppc64")), - otherOs().and(arch("ppc64le")), - otherOs().and(arch("s390")), - otherOs().and(arch("zarch_64")), - otherOs().and(arch("some unknown CPU arch")) - ); - } - -} diff --git a/src/test/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformClassifierFactoryTest.java b/src/test/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformClassifierFactoryTest.java new file mode 100644 index 00000000..276a3f2a --- /dev/null +++ b/src/test/java/io/github/ascopes/protobufmavenplugin/dependency/PlatformClassifierFactoryTest.java @@ -0,0 +1,119 @@ +/* + * Copyright (C) 2023 - 2024, Ashley Scopes. + * + * 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 + * + * http://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 io.github.ascopes.protobufmavenplugin.dependency; + +import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.arch; +import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.hostSystem; +import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.linux; +import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.macOs; +import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.otherOs; +import static io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.windows; +import static io.github.ascopes.protobufmavenplugin.fixtures.RandomFixtures.someText; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.junit.jupiter.params.provider.Arguments.arguments; + +import io.github.ascopes.protobufmavenplugin.fixtures.HostSystemFixtures.HostSystemMockConfigurer; +import java.util.stream.Stream; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + + +/** + * @author Ashley Scopes + */ +@DisplayName("PlatformClassifierFactory tests") +class PlatformClassifierFactoryTest { + + @DisplayName(".getClassifier(...) returns the expected results on valid systems") + @MethodSource("validClassifierCases") + @ParameterizedTest(name = "for system {0}, expect classifier {1}") + void getClassifierReturnsExpectedResultsOnValidSystems( + HostSystemMockConfigurer configurer, + String expectedClassifier + ) { + // Given + var hostSystem = hostSystem(); + configurer.configure(hostSystem); + var artifactId = someText(); + var factory = new PlatformClassifierFactory(hostSystem); + + // When + var actualClassifier = factory.getClassifier(artifactId); + + // Then + assertThat(actualClassifier).isEqualTo(expectedClassifier); + } + + @DisplayName(".getClassifier(...) raises an exception for unknown systems") + @MethodSource("invalidClassifierCases") + @ParameterizedTest(name = "when {0}, then expect an exception") + void getClassifierRaisesAnExceptionForUnknownSystems( + HostSystemMockConfigurer configurer + ) { + // Given + var hostSystem = hostSystem(); + configurer.configure(hostSystem); + var artifactId = someText(); + var factory = new PlatformClassifierFactory(hostSystem); + + // Then + assertThatThrownBy(() -> factory.getClassifier(artifactId)) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessageMatching( + "No '[^']+' binary is available for reported OS '[^']+' and CPU architecture '[^']+'" + ); + } + + static Stream validClassifierCases() { + return Stream.of( + arguments(linux().and(arch("amd64")), "linux-x86_64"), + arguments(linux().and(arch("aarch64")), "linux-aarch_64"), + arguments(linux().and(arch("s390x")), "linux-s390_64"), + arguments(linux().and(arch("zarch_64")), "linux-s390_64"), + arguments(linux().and(arch("ppc64le")), "linux-ppcle_64"), + arguments(linux().and(arch("ppc64")), "linux-ppcle_64"), + arguments(macOs().and(arch("amd64")), "osx-x86_64"), + arguments(macOs().and(arch("x86_64")), "osx-x86_64"), + arguments(macOs().and(arch("aarch64")), "osx-aarch_64"), + arguments(windows().and(arch("amd64")), "windows-x86_64"), + arguments(windows().and(arch("x86_64")), "windows-x86_64"), + arguments(windows().and(arch("x86")), "windows-x86_32"), + arguments(windows().and(arch("x86_32")), "windows-x86_32") + ); + } + + static Stream invalidClassifierCases() { + return Stream.of( + linux().and(arch("some unknown CPU arch")), + macOs().and(arch("some unknown CPU arch")), + windows().and(arch("some unknown CPU arch")), + otherOs().and(arch("x86_32")), + otherOs().and(arch("x86_64")), + otherOs().and(arch("amd64")), + otherOs().and(arch("aarch64")), + otherOs().and(arch("ppc64")), + otherOs().and(arch("ppc64le")), + otherOs().and(arch("s390")), + otherOs().and(arch("zarch_64")), + otherOs().and(arch("some unknown CPU arch")) + ); + } + +}