From dd16cf66e68b42f9b2846ec7aa54c8dd2d1a9f7c Mon Sep 17 00:00:00 2001 From: Ashley Scopes <73482956+ascopes@users.noreply.github.com> Date: Sun, 6 Jul 2025 13:46:25 +0100 Subject: [PATCH 1/2] Use immutables:datatype to remove the need for modifiable parameter type duplicates Implement initial POC for hooking Plexus into immutable builder classes Use fix from https://github.com/paul-hammant/qdox/pull/271 temporarily to work around https://github.com/apache/maven-plugin-tools/issues/944 Fix test compilation failure Fix ImmutablesDataPlexusConverter to handle stdlib/primitive types correctly Start writing unit tests for plexus converter for Immutables types Update logic in plexus converters Use Datatypes rather than Data for immutables Ran into https://github.com/immutables/immutables/issues/1608 when initialising the data type descriptors. Add missing test case for https://github.com/immutables/immutables/issues/1608 Update Plexus converter to use weak references to class types Move test data to subpackage Fix issues with nested type handling, add new test cases Suppress testdata in Checkstyle --- .mvn/checkstyle/suppressions.xml | 3 +- protobuf-maven-plugin/pom.xml | 6 + .../dependencies/MavenDependency.java | 6 +- .../dependencies/MavenExclusion.java | 6 +- .../mojo/AbstractGenerateMojo.java | 12 +- .../protobufmavenplugin/package-info.java | 2 - .../plexus/ImmutablesDataPlexusConverter.java | 178 ++++++++++++++++++ .../protobufmavenplugin/plexus/KindHint.java | 20 +- .../plexus/SealedTypePlexusConverter.java | 11 +- .../BinaryMavenProtocPlugin.java | 5 +- .../distributions/JvmMavenProtocPlugin.java | 8 +- .../distributions/PathProtocPlugin.java | 8 +- .../distributions/UriProtocPlugin.java | 15 +- .../BinaryMavenProtocDistribution.java | 4 +- .../distributions/PathProtocDistribution.java | 4 +- .../distributions/UriProtocDistribution.java | 5 +- .../fixtures/DependencyFixtures.java | 8 +- .../AbstractGenerateMojoTestTemplate.java | 28 +-- .../ImmutablesDataPlexusConverterTest.java | 159 ++++++++++++++++ .../plexus/SealedTypePlexusConverterTest.java | 27 ++- .../testdata/Datatypes_SomeBrokenModel.java | 21 +++ .../plexus/testdata/SomeBrokenModel.java | 19 ++ .../plexus/testdata/ValidInnerModel.java | 28 +++ .../plexus/testdata/ValidOuterModel.java | 31 +++ 24 files changed, 525 insertions(+), 89 deletions(-) create mode 100644 protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverter.java create mode 100644 protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverterTest.java create mode 100644 protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/Datatypes_SomeBrokenModel.java create mode 100644 protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/SomeBrokenModel.java create mode 100644 protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/ValidInnerModel.java create mode 100644 protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/ValidOuterModel.java diff --git a/.mvn/checkstyle/suppressions.xml b/.mvn/checkstyle/suppressions.xml index 0157283f..f0038fb2 100644 --- a/.mvn/checkstyle/suppressions.xml +++ b/.mvn/checkstyle/suppressions.xml @@ -20,4 +20,5 @@ "-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN" "https://checkstyle.org/dtds/suppressions_1_2.dtd"> - \ No newline at end of file + + diff --git a/protobuf-maven-plugin/pom.xml b/protobuf-maven-plugin/pom.xml index b957c0ad..e27d90cd 100644 --- a/protobuf-maven-plugin/pom.xml +++ b/protobuf-maven-plugin/pom.xml @@ -89,6 +89,12 @@ provided + + org.immutables + datatype + compile + + org.immutables value diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/dependencies/MavenDependency.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/dependencies/MavenDependency.java index 7dac2aad..1d02f4e7 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/dependencies/MavenDependency.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/dependencies/MavenDependency.java @@ -17,8 +17,8 @@ import io.github.ascopes.protobufmavenplugin.utils.DeadCodeGenerated; import java.util.Set; +import org.immutables.datatype.Data; import org.immutables.value.Value.Immutable; -import org.immutables.value.Value.Modifiable; import org.jspecify.annotations.Nullable; @@ -28,13 +28,13 @@ * @author Ashley Scopes * @since 3.3.1 */ +@Data @Immutable -@Modifiable public abstract class MavenDependency extends MavenArtifact { public abstract @Nullable DependencyResolutionDepth getDependencyResolutionDepth(); - public abstract Set getExclusions(); + public abstract Set getExclusions(); @DeadCodeGenerated(reason = "Must be overridden to keep immutables happy.") @Override diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/dependencies/MavenExclusion.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/dependencies/MavenExclusion.java index 890806d0..3d56b65a 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/dependencies/MavenExclusion.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/dependencies/MavenExclusion.java @@ -15,7 +15,8 @@ */ package io.github.ascopes.protobufmavenplugin.dependencies; -import org.immutables.value.Value.Modifiable; +import org.immutables.datatype.Data; +import org.immutables.value.Value.Immutable; /** * Marker to exclude a specific transitive dependency. @@ -26,7 +27,8 @@ * @author Ashley Scopes * @since 2.12.0 */ -@Modifiable +@Data +@Immutable public interface MavenExclusion { /** diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/mojo/AbstractGenerateMojo.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/mojo/AbstractGenerateMojo.java index 19e788d6..53dae85a 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/mojo/AbstractGenerateMojo.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/mojo/AbstractGenerateMojo.java @@ -20,7 +20,7 @@ import static java.util.function.Predicate.not; import io.github.ascopes.protobufmavenplugin.dependencies.DependencyResolutionDepth; -import io.github.ascopes.protobufmavenplugin.dependencies.MavenDependencyBean; +import io.github.ascopes.protobufmavenplugin.dependencies.MavenDependency; import io.github.ascopes.protobufmavenplugin.digests.Digest; import io.github.ascopes.protobufmavenplugin.generation.GenerationResult; import io.github.ascopes.protobufmavenplugin.generation.ImmutableGenerationRequest; @@ -301,7 +301,7 @@ public AbstractGenerateMojo() { * @since 1.2.0 */ @Parameter - @Nullable List importDependencies; + @Nullable List importDependencies; /** * Specify additional paths to import protobuf sources from on the local file system. @@ -743,7 +743,7 @@ public AbstractGenerateMojo() { * @since 1.2.0 */ @Parameter - @Nullable List sourceDependencies; + @Nullable List sourceDependencies; /** * Protobuf Descriptor files to compile. @@ -777,7 +777,7 @@ public AbstractGenerateMojo() { * @since 3.1.0 */ @Parameter - @Nullable List sourceDescriptorDependencies; + @Nullable List sourceDescriptorDependencies; /** * The source directories to compile protobuf sources from. @@ -813,6 +813,10 @@ public AbstractGenerateMojo() { @Parameter @Nullable List sourceDescriptorPaths; + /* + * Implementation-specific details. + */ + /** * Provides the default source directory to read protobuf sources from. * diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/package-info.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/package-info.java index 2f44a8dd..92baf24a 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/package-info.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/package-info.java @@ -19,8 +19,6 @@ */ @NullMarked @Style( - beanFriendlyModifiables = true, - create = "new", defaults = @Immutable(copy = false), defaultAsDefault = true, get = {"get*", "is*"}, diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverter.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverter.java new file mode 100644 index 00000000..fa9dcb71 --- /dev/null +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverter.java @@ -0,0 +1,178 @@ +/* + * Copyright (C) 2023 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.plexus; + +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.Collections; +import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Optional; +import java.util.WeakHashMap; +import javax.inject.Named; +import javax.inject.Singleton; +import org.codehaus.plexus.component.configurator.ComponentConfigurationException; +import org.codehaus.plexus.component.configurator.ConfigurationListener; +import org.codehaus.plexus.component.configurator.converters.ParameterizedConfigurationConverter; +import org.codehaus.plexus.component.configurator.converters.basic.AbstractBasicConverter; +import org.codehaus.plexus.component.configurator.converters.lookup.ConverterLookup; +import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator; +import org.codehaus.plexus.configuration.PlexusConfiguration; +import org.immutables.datatype.Datatype; +import org.jspecify.annotations.Nullable; + +/** + * Converter for Plexus components that can build a generated "immutables" value type, based + * on the interface it was derived from. + * + *

Note that all objects must be annotated with both {@link org.immutables.value.Value} + * and {@link org.immutables.datatype.Data}, otherwise deserialization will fail at + * runtime. + * + * @author Ashley Scopes + * @since TBC + */ +@Named +@Singleton +final class ImmutablesDataPlexusConverter extends AbstractBasicConverter { + private final Map, Datatype> knownDatatypes; + + ImmutablesDataPlexusConverter() { + // Weak hashmap keys will be deregistered upon classloader destruction safely. + knownDatatypes = Collections.synchronizedMap(new WeakHashMap<>()); + } + + @Override + public boolean canConvert(Class cls) { + return datatypeFor(cls).isPresent(); + } + + @Override + public Object fromConfiguration( + ConverterLookup lookup, + PlexusConfiguration configuration, + Class type, + @Nullable Class enclosingType, + @Nullable ClassLoader loader, + ExpressionEvaluator evaluator, + @Nullable ConfigurationListener listener + ) throws ComponentConfigurationException { + var datatype = datatypeFor(type) + .orElseThrow(() -> new NoSuchElementException("No datatype converter " + type.getName())); + + var builder = datatype.builder(); + for (var child : configuration.getChildren()) { + consumeChild(builder, child, lookup, datatype, loader, evaluator, listener); + } + + return builder.build(); + } + + private void consumeChild( + Datatype.Builder builder, + PlexusConfiguration child, + ConverterLookup lookup, + Datatype datatype, + @Nullable ClassLoader loader, + ExpressionEvaluator evaluator, + @Nullable ConfigurationListener listener + ) throws ComponentConfigurationException { + try { + @SuppressWarnings("unchecked") + var feature = (Datatype.Feature) datatype.feature(child.getName()); + var valueType = feature.type(); + var rawType = rawTypeOf(valueType); + + var converter = lookup.lookupConverterForType(rawType); + + Object value; + + if (converter instanceof ParameterizedConfigurationConverter parameterizedConverter) { + var parameterizedType = (ParameterizedType) valueType; + value = parameterizedConverter.fromConfiguration( + lookup, + child, + rawType, + parameterizedType.getActualTypeArguments(), + rawType.getEnclosingClass(), + loader, + evaluator, + listener + ); + } else { + value = converter.fromConfiguration( + lookup, + child, + rawType, + rawType.getEnclosingClass(), + loader, + evaluator, + listener + ); + } + + builder.set(feature, value); + + } catch (NoSuchElementException ex) { + throw new ComponentConfigurationException( + "No attribute " + child.getName() + " exists for " + datatype.name(), + ex + ); + } + } + + private Optional> datatypeFor(Class cls) { + if (cls.isPrimitive() || cls.getClassLoader() == null) { + return Optional.empty(); + } + + // Horrible generic voodoo that probably is not safe, but the APIs have conflicting types + // and the compiler is not smart enough to help us. + @SuppressWarnings("unchecked") + var castCls = (Class) cls; + + var datatype = knownDatatypes.computeIfAbsent(castCls, ignored -> { + var loader = cls.getClassLoader(); + var outerClsName = cls.getPackageName() + ".Datatypes_" + cls.getSimpleName(); + + try { + var outerCls = loader.loadClass(outerClsName); + var method = outerCls.getMethod("_" + cls.getSimpleName()); + + @SuppressWarnings("unchecked") + var result = (Datatype) method.invoke(null); + + return result; + } catch (ClassNotFoundException ex) { + return null; + } catch (ReflectiveOperationException ex) { + throw new IllegalStateException( + "Failed to find datatype for " + cls.getName() + ": " + ex, + ex + ); + } + }); + + return Optional.ofNullable(datatype); + } + + private static Class rawTypeOf(Type type) { + return type instanceof ParameterizedType parameterizedType + ? rawTypeOf(parameterizedType.getRawType()) + // Assumption: this cannot ever be a wildcard or union type. + : (Class) type; + } +} diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/KindHint.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/KindHint.java index a9520630..272b8fa6 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/KindHint.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/KindHint.java @@ -25,6 +25,9 @@ /** * Marker to advise the "kind" of the implementation when used with a sealed-type hierarchy. * + *

When this marker is spotted, the {@code ConverterLookup} will be consulted to produce + * an instance of that type. + * * @author Ashley Scopes * @since 4.1.0 */ @@ -38,20 +41,5 @@ * * @return the kind. */ - String kind(); - - /** - * The implementation that the kind should point to. - * - *

This is needed until - * GH-880 - * can be merged, as we cannot easily infer the immutable implementation class from the base - * when working with the Immutables library. GH-880 will enable better integration with that - * library based on compile-time metadata that avoids this issue. We may just need to include - * this annotation in {@link Style#passAnnotations()} to achieve this correctly closer to the - * time. - * - * @return the implementation type. - */ - Class implementation(); + String value(); } diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/SealedTypePlexusConverter.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/SealedTypePlexusConverter.java index 292f01cb..0676fa87 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/SealedTypePlexusConverter.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/SealedTypePlexusConverter.java @@ -183,21 +183,22 @@ private Object parseFromObject( private Optional> findRequestedImplementation(Class type, String kind) { return listKindedImplementations(type) .filter(kindPair -> kindPair.kind().equals(kind)) - .map(KindHint::implementation) + .map(KindPair::implementation) .findFirst(); } private String nameValidKinds(Class type) { return listKindedImplementations(type) - .map(KindHint::kind) + .map(KindPair::kind) .sorted() .map(s -> "\"" + s + "\"") .collect(Collectors.joining(", ")); } - private Stream listKindedImplementations(Class type) { + private Stream listKindedImplementations(Class type) { var thisKindHint = AnnotationProxy.findAnnotation(KindHint.class, type) - .stream(); + .stream() + .map(annotation -> new KindPair(annotation.value(), type)); var kindedSubtypes = Optional.ofNullable(type.getPermittedSubclasses()) .stream() @@ -217,4 +218,6 @@ private ComponentConfigurationException missingKindAttribute( + nameValidKinds(type) + "." ); } + + private record KindPair(String kind, Class implementation) {} } diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/BinaryMavenProtocPlugin.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/BinaryMavenProtocPlugin.java index 50dccd20..5221b243 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/BinaryMavenProtocPlugin.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/BinaryMavenProtocPlugin.java @@ -19,8 +19,6 @@ import io.github.ascopes.protobufmavenplugin.plexus.KindHint; import io.github.ascopes.protobufmavenplugin.utils.DeadCodeGenerated; import org.immutables.value.Value.Immutable; -import org.immutables.value.Value.Modifiable; - /** * Implementation independent descriptor for a protoc plugin that can be resolved from a Maven @@ -30,8 +28,7 @@ * @since 4.1.0 */ @Immutable -@Modifiable -@KindHint(kind = "binary-maven", implementation = BinaryMavenProtocPluginBean.class) +@KindHint("binary-maven") public abstract non-sealed class BinaryMavenProtocPlugin extends MavenArtifact implements ProtocPlugin { diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/JvmMavenProtocPlugin.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/JvmMavenProtocPlugin.java index 47257284..d63097b5 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/JvmMavenProtocPlugin.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/JvmMavenProtocPlugin.java @@ -19,7 +19,8 @@ import io.github.ascopes.protobufmavenplugin.plexus.KindHint; import io.github.ascopes.protobufmavenplugin.utils.DeadCodeGenerated; import java.util.List; -import org.immutables.value.Value.Modifiable; +import org.immutables.datatype.Data; +import org.immutables.value.Value.Immutable; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; @@ -31,8 +32,9 @@ * @author Ashley Scopes * @since 2.0.0 */ -@Modifiable -@KindHint(kind = "jvm-maven", implementation = JvmMavenProtocPluginBean.class) +@Data +@Immutable +@KindHint("jvm-maven") public abstract non-sealed class JvmMavenProtocPlugin extends MavenArtifact implements ProtocPlugin { diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/PathProtocPlugin.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/PathProtocPlugin.java index c54c7123..30fe8b07 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/PathProtocPlugin.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/PathProtocPlugin.java @@ -18,8 +18,9 @@ import io.github.ascopes.protobufmavenplugin.digests.Digest; import io.github.ascopes.protobufmavenplugin.plexus.KindHint; import io.github.ascopes.protobufmavenplugin.utils.DeadCodeGenerated; +import org.immutables.datatype.Data; import org.immutables.value.Value.Default; -import org.immutables.value.Value.Modifiable; +import org.immutables.value.Value.Immutable; import org.jspecify.annotations.Nullable; @@ -33,8 +34,9 @@ * @author Ashley Scopes * @since 2.0.0 */ -@Modifiable -@KindHint(kind = "path", implementation = PathProtocPluginBean.class) +@Data +@Immutable +@KindHint("path") public abstract non-sealed class PathProtocPlugin implements ProtocPlugin { public abstract String getName(); diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/UriProtocPlugin.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/UriProtocPlugin.java index 37e04ec5..f514a724 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/UriProtocPlugin.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plugins/distributions/UriProtocPlugin.java @@ -20,8 +20,9 @@ import io.github.ascopes.protobufmavenplugin.utils.DeadCodeGenerated; import java.net.URI; import java.util.Optional; +import org.immutables.datatype.Data; import org.immutables.value.Value.Default; -import org.immutables.value.Value.Modifiable; +import org.immutables.value.Value.Immutable; import org.jspecify.annotations.Nullable; @@ -34,14 +35,18 @@ * @author Ashley Scopes * @since 2.0.0 */ -@Modifiable -@KindHint(kind = "url", implementation = UriProtocPluginBean.class) +@Data +@Immutable +@KindHint("url") public abstract non-sealed class UriProtocPlugin implements ProtocPlugin { - public abstract @Nullable Digest getDigest(); - public abstract URI getUrl(); + @Default + public @Nullable Digest getDigest() { + return null; + } + @Default.Boolean(false) public abstract boolean isOptional(); diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/BinaryMavenProtocDistribution.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/BinaryMavenProtocDistribution.java index b8836370..2a4a3c67 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/BinaryMavenProtocDistribution.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/BinaryMavenProtocDistribution.java @@ -20,7 +20,6 @@ import io.github.ascopes.protobufmavenplugin.utils.DeadCodeGenerated; import org.immutables.value.Value.Default; import org.immutables.value.Value.Immutable; -import org.immutables.value.Value.Modifiable; import org.jspecify.annotations.NonNull; /** @@ -34,8 +33,7 @@ * @since 5.1.0 */ @Immutable -@KindHint(kind = "binary-maven", implementation = BinaryMavenProtocDistributionBean.class) -@Modifiable +@KindHint("binary-maven") public abstract non-sealed class BinaryMavenProtocDistribution extends MavenArtifact implements ProtocDistribution { diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/PathProtocDistribution.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/PathProtocDistribution.java index 76767d0e..186c17cf 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/PathProtocDistribution.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/PathProtocDistribution.java @@ -19,7 +19,6 @@ import io.github.ascopes.protobufmavenplugin.plexus.KindHint; import org.immutables.value.Value.Default; import org.immutables.value.Value.Immutable; -import org.immutables.value.Value.Modifiable; import org.jspecify.annotations.Nullable; /** @@ -29,8 +28,7 @@ * @since 5.1.0 */ @Immutable -@KindHint(kind = "path", implementation = PathProtocDistributionBean.class) -@Modifiable +@KindHint("path") public abstract non-sealed class PathProtocDistribution implements ProtocDistribution { /** diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/UriProtocDistribution.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/UriProtocDistribution.java index 32eccacf..bf2cdc73 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/UriProtocDistribution.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/protoc/distributions/UriProtocDistribution.java @@ -19,10 +19,8 @@ import io.github.ascopes.protobufmavenplugin.plexus.KindHint; import java.net.URI; import org.immutables.value.Value.Immutable; -import org.immutables.value.Value.Modifiable; import org.jspecify.annotations.Nullable; - /** * Model base for a {@code protoc} distribution that is located at a URI. * @@ -30,8 +28,7 @@ * @since 5.1.0 */ @Immutable -@KindHint(kind = "url", implementation = UriProtocDistributionBean.class) -@Modifiable +@KindHint("url") public abstract non-sealed class UriProtocDistribution implements ProtocDistribution { /** diff --git a/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/fixtures/DependencyFixtures.java b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/fixtures/DependencyFixtures.java index a9278b83..be8918f4 100644 --- a/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/fixtures/DependencyFixtures.java +++ b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/fixtures/DependencyFixtures.java @@ -21,7 +21,7 @@ import static org.mockito.Mockito.withSettings; import io.github.ascopes.protobufmavenplugin.dependencies.DependencyResolutionDepth; -import io.github.ascopes.protobufmavenplugin.dependencies.MavenExclusionBean; +import io.github.ascopes.protobufmavenplugin.dependencies.MavenExclusion; import java.util.Set; import org.jspecify.annotations.Nullable; import org.mockito.Answers; @@ -64,7 +64,7 @@ public static io.github.ascopes.protobufmavenplugin.dependencies.MavenDependency @Nullable String classifier, @Nullable String type, @Nullable DependencyResolutionDepth dependencyResolutionDepth, - MavenExclusionBean... exclusions + MavenExclusion... exclusions ) { var artifact = mock( io.github.ascopes.protobufmavenplugin.dependencies.MavenDependency.class, @@ -80,7 +80,7 @@ public static io.github.ascopes.protobufmavenplugin.dependencies.MavenDependency return artifact; } - public static io.github.ascopes.protobufmavenplugin.dependencies.MavenExclusionBean pmpExclusion( + public static io.github.ascopes.protobufmavenplugin.dependencies.MavenExclusion pmpExclusion( String groupId, String artifactId, @Nullable String classifier, @@ -89,7 +89,7 @@ public static io.github.ascopes.protobufmavenplugin.dependencies.MavenExclusionB // Kind of gross that we have to return MavenExclusionBean here but Maven forces our hand // due to not being able to cope with interface types. var exclusion = mock( - io.github.ascopes.protobufmavenplugin.dependencies.MavenExclusionBean.class, + io.github.ascopes.protobufmavenplugin.dependencies.MavenExclusion.class, defaultSettings("some protobuf maven plugin exclusion") ); when(exclusion.getGroupId()).thenReturn(groupId); diff --git a/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/mojo/AbstractGenerateMojoTestTemplate.java b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/mojo/AbstractGenerateMojoTestTemplate.java index 1cad2e5e..dac5e1c0 100644 --- a/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/mojo/AbstractGenerateMojoTestTemplate.java +++ b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/mojo/AbstractGenerateMojoTestTemplate.java @@ -29,7 +29,7 @@ import static org.mockito.Mockito.withSettings; import io.github.ascopes.protobufmavenplugin.dependencies.DependencyResolutionDepth; -import io.github.ascopes.protobufmavenplugin.dependencies.MavenDependencyBean; +import io.github.ascopes.protobufmavenplugin.dependencies.MavenDependency; import io.github.ascopes.protobufmavenplugin.digests.Digest; import io.github.ascopes.protobufmavenplugin.fixtures.UsesSystemProperties; import io.github.ascopes.protobufmavenplugin.generation.GenerationRequest; @@ -38,7 +38,11 @@ import io.github.ascopes.protobufmavenplugin.generation.ProtobufBuildOrchestrator; import io.github.ascopes.protobufmavenplugin.generation.SourceRootRegistrar; import io.github.ascopes.protobufmavenplugin.plugins.distributions.BinaryMavenProtocPlugin; -import io.github.ascopes.protobufmavenplugin.plugins.distributions.JvmMavenProtocPluginBean; +import io.github.ascopes.protobufmavenplugin.plugins.distributions.ImmutableBinaryMavenProtocPlugin; +import io.github.ascopes.protobufmavenplugin.plugins.distributions.ImmutableJvmMavenProtocPlugin; +import io.github.ascopes.protobufmavenplugin.plugins.distributions.ImmutablePathProtocPlugin; +import io.github.ascopes.protobufmavenplugin.plugins.distributions.ImmutableUriProtocPlugin; +import io.github.ascopes.protobufmavenplugin.plugins.distributions.JvmMavenProtocPlugin; import io.github.ascopes.protobufmavenplugin.plugins.distributions.PathProtocPlugin; import io.github.ascopes.protobufmavenplugin.plugins.distributions.ProtocPlugin; import io.github.ascopes.protobufmavenplugin.plugins.distributions.UriProtocPlugin; @@ -395,7 +399,7 @@ void ignoreProjectDependenciesIsSetToSpecifiedValue(boolean value) throws Throwa @NullAndEmptySource @ParameterizedTest(name = "when {0}") void whenImportDependenciesNullExpectEmptyListInRequest( - @Nullable List dependencies + @Nullable List dependencies ) throws Throwable { // Given mojo.importDependencies = dependencies; @@ -414,7 +418,7 @@ void whenImportDependenciesNullExpectEmptyListInRequest( @Test void whenImportDependenciesProvidedExpectPluginsInRequest() throws Throwable { // Given - List plugins = mock(); + List plugins = mock(); mojo.importDependencies = plugins; // When @@ -715,10 +719,10 @@ void whenPluginsNullExpectEmptyListInRequest( void whenPluginsProvidedExpectPluginsInRequest() throws Throwable { // Given List plugins = List.of( - mock(JvmMavenProtocPluginBean.class), - mock(BinaryMavenProtocPlugin.class), - mock(UriProtocPlugin.class), - mock(PathProtocPlugin.class) + mock(ImmutableJvmMavenProtocPlugin.class), + mock(ImmutableBinaryMavenProtocPlugin.class), + mock(ImmutableUriProtocPlugin.class), + mock(ImmutablePathProtocPlugin.class) ); mojo.plugins = plugins; @@ -841,7 +845,7 @@ void whenSanctionedExecutablePathNotProvidedExpectNoFileToBeUsed() throws Throwa @NullAndEmptySource @ParameterizedTest(name = "when {0}") void whenSourceDependenciesNullExpectEmptyListInRequest( - @Nullable List dependencies + @Nullable List dependencies ) throws Throwable { // Given mojo.sourceDependencies = dependencies; @@ -860,7 +864,7 @@ void whenSourceDependenciesNullExpectEmptyListInRequest( @Test void whenSourceDependenciesProvidedExpectDependenciesInRequest() throws Throwable { // Given - List plugins = mock(); + List plugins = mock(); mojo.sourceDependencies = plugins; // When @@ -877,7 +881,7 @@ void whenSourceDependenciesProvidedExpectDependenciesInRequest() throws Throwabl @NullAndEmptySource @ParameterizedTest(name = "when {0}") void whenSourceDescriptorDependenciesNullExpectEmptyListInRequest( - @Nullable List dependencies + @Nullable List dependencies ) throws Throwable { // Given mojo.sourceDescriptorDependencies = dependencies; @@ -898,7 +902,7 @@ void whenSourceDescriptorDependenciesNullExpectEmptyListInRequest( @Test void whenSourceDescriptorDependenciesProvidedExpectDependenciesInRequest() throws Throwable { // Given - List plugins = mock(); + List plugins = mock(); mojo.sourceDescriptorDependencies = plugins; // When diff --git a/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverterTest.java b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverterTest.java new file mode 100644 index 00000000..852039de --- /dev/null +++ b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverterTest.java @@ -0,0 +1,159 @@ +/* + * Copyright (C) 2023 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.plexus; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.junit.jupiter.params.provider.Arguments.arguments; + +import io.github.ascopes.protobufmavenplugin.plexus.testdata.ImmutableValidInnerModel; +import io.github.ascopes.protobufmavenplugin.plexus.testdata.ImmutableValidOuterModel; +import io.github.ascopes.protobufmavenplugin.plexus.testdata.SomeBrokenModel; +import io.github.ascopes.protobufmavenplugin.plexus.testdata.ValidInnerModel; +import io.github.ascopes.protobufmavenplugin.plexus.testdata.ValidOuterModel; +import java.io.StringReader; +import java.util.List; +import java.util.Set; +import java.util.stream.Stream; +import org.codehaus.plexus.component.configurator.converters.lookup.ConverterLookup; +import org.codehaus.plexus.component.configurator.converters.lookup.DefaultConverterLookup; +import org.codehaus.plexus.component.configurator.expression.DefaultExpressionEvaluator; +import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator; +import org.codehaus.plexus.configuration.PlexusConfiguration; +import org.codehaus.plexus.configuration.xml.XmlPlexusConfiguration; +import org.codehaus.plexus.util.xml.Xpp3DomBuilder; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +@DisplayName("ImmutablesDataPlexusConverter tests") +class ImmutablesDataPlexusConverterTest { + + ImmutablesDataPlexusConverter converter; + ConverterLookup converterLookup; + ExpressionEvaluator expressionEvaluator; + + @BeforeEach + void setUp() { + converter = new ImmutablesDataPlexusConverter(); + converterLookup = new DefaultConverterLookup(); + expressionEvaluator = new DefaultExpressionEvaluator(); + + converterLookup.registerConverter(converter); + + } + + @DisplayName(".canConvert(Class) returns the expected values") + @MethodSource("canConvertTestCases") + @ParameterizedTest(name = "expect {1} when calling with {0}") + void canConvertReturnsTheExpectedValue(Class cls, boolean expectedResult) { + // When + var actualResult = converter.canConvert(cls); + + // Then + assertThat(actualResult).isEqualTo(expectedResult); + } + + @DisplayName(".fromConfiguration(...) returns the expected results for nested models") + @Test + void fromConfigurationReturnsTheExpectedResults() throws Exception { + // Given + var configuration = xml2PlexusConfiguration(""" + + this-is-foo + 123456 + + + this-is-also-foo + 98765 + + + + """.stripIndent()); + + // When + var result = converterLookup.lookupConverterForType(ValidOuterModel.class) + .fromConfiguration( + converterLookup, + configuration, + ValidOuterModel.class, + null, + getClass().getClassLoader(), + expressionEvaluator + ); + + // Then + assertThat(result) + .isNotNull() + .isEqualTo(ImmutableValidOuterModel.builder() + .foo("this-is-foo") + .bar(123456) + .validInnerModels(Set.of( + ImmutableValidInnerModel.builder() + .foo("this-is-also-foo") + .bar(98765) + .build())) + .build()); + } + + @DisplayName( + "datatype lookup raises an IllegalStateException if unexpected reflective errors occur" + ) + @Test + void datatypeLookupRaisesIllegalStateExceptionIfUnexpectedReflectiveErrorsOccur() { + // Then + assertThatExceptionOfType(IllegalStateException.class) + .isThrownBy(() -> converter.canConvert(SomeBrokenModel.class)) + .withCauseExactlyInstanceOf(NoSuchMethodException.class); + } + + static PlexusConfiguration xml2PlexusConfiguration(String lines) { + try { + var dom = Xpp3DomBuilder.build(new StringReader(lines)); + return new XmlPlexusConfiguration(dom); + } catch (Exception ex) { + throw new RuntimeException("Failed to parse XML... welp", ex); + } + } + + static Stream canConvertTestCases() { + return Stream.of( + arguments(void.class, false), + arguments(boolean.class, false), + arguments(byte.class, false), + arguments(short.class, false), + arguments(int.class, false), + arguments(long.class, false), + arguments(float.class, false), + arguments(double.class, false), + arguments(Void.class, false), + arguments(Object.class, false), + arguments(String.class, false), + arguments(List.class, false), + arguments(Set.class, false), + arguments(Class.class, false), + arguments(SomeJunkType.class, false), + arguments(ValidInnerModel.class, true), + arguments(ValidOuterModel.class, true) + ); + } + + interface SomeJunkType { + } +} diff --git a/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/SealedTypePlexusConverterTest.java b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/SealedTypePlexusConverterTest.java index 98565429..b3cfd31e 100644 --- a/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/SealedTypePlexusConverterTest.java +++ b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/SealedTypePlexusConverterTest.java @@ -247,14 +247,14 @@ void fromConfigurationReturnsTheExpectedValueGivenKind() throws Exception { assertThat(result.users.get(2)) .as("result.users.2 %s", result.users.get(2)) .isNotNull() - .isExactlyInstanceOf(Owner.class) - .extracting(Owner.class::cast) + .isExactlyInstanceOf(Admin.class) + .extracting(Admin.class::cast) .satisfies( owner -> assertThat(owner.name) - .as("owner.name") + .as("admin.name") .isEqualTo("Ashley"), owner -> assertThat(owner.employeeId) - .as("owner.employeeId") + .as("admin.employeeId") .isEqualTo("54321")); } @@ -416,7 +416,7 @@ public sealed interface User permits Person, Bot, Admin, InvalidLeafType { String getName(); } - @KindHint(kind = "person", implementation = Person.class) + @KindHint("person") public static final class Person implements User { String name = ""; @@ -431,7 +431,7 @@ public void setName(String name) { } } - @KindHint(kind = "bot", implementation = Bot.class) + @KindHint("bot") public static final class Bot implements User { String id = ""; @@ -446,9 +446,8 @@ public void setId(String id) { } } - @KindHint(kind = "admin", implementation = Owner.class) - public abstract static non-sealed class Admin implements User { - + @KindHint("admin") + public static final class Admin implements User { String name = ""; String employeeId = ""; @@ -470,10 +469,6 @@ public void setEmployeeId(String employeeId) { } } - public static final class Owner extends Admin { - - } - // Missing annotation public static final class InvalidLeafType implements User { @@ -491,12 +486,12 @@ static Animal fromString(String value) { } } - @KindHint(kind = "cat", implementation = Cat.class) + @KindHint("cat") public static final class Cat implements Animal { } - @KindHint(kind = "dog", implementation = Dog.class) + @KindHint("dog") public static final class Dog implements Animal { } @@ -519,7 +514,7 @@ static BrokenFromString foo(String arg) { } } - @KindHint(kind = "broken", implementation = BrokenFromStringImpl.class) + @KindHint("broken") public static final class BrokenFromStringImpl implements BrokenFromString { } diff --git a/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/Datatypes_SomeBrokenModel.java b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/Datatypes_SomeBrokenModel.java new file mode 100644 index 00000000..a943470d --- /dev/null +++ b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/Datatypes_SomeBrokenModel.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2023 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.plexus.testdata; + +public class Datatypes_SomeBrokenModel { + // Purposely empty to trigger IllegalStateException when datatype lookup is performed + // within ImmutablesDataPlexusConverterTest. +} diff --git a/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/SomeBrokenModel.java b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/SomeBrokenModel.java new file mode 100644 index 00000000..e4eceec4 --- /dev/null +++ b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/SomeBrokenModel.java @@ -0,0 +1,19 @@ +/* + * Copyright (C) 2023 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.plexus.testdata; + +public final class SomeBrokenModel { +} diff --git a/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/ValidInnerModel.java b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/ValidInnerModel.java new file mode 100644 index 00000000..ad8f34eb --- /dev/null +++ b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/ValidInnerModel.java @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2023 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.plexus.testdata; + +import org.immutables.datatype.Data; +import org.immutables.value.Value; + +@Data +@Value.Immutable +public interface ValidInnerModel { + + String getFoo(); + + int getBar(); +} diff --git a/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/ValidOuterModel.java b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/ValidOuterModel.java new file mode 100644 index 00000000..822004c4 --- /dev/null +++ b/protobuf-maven-plugin/src/test/java/io/github/ascopes/protobufmavenplugin/plexus/testdata/ValidOuterModel.java @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2023 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.plexus.testdata; + +import java.util.Set; +import org.immutables.datatype.Data; +import org.immutables.value.Value; + +@Data +@Value.Immutable +public interface ValidOuterModel { + + String getFoo(); + + int getBar(); + + Set getValidInnerModels(); +} From 2dcb8fe923c82515eb4adb3858039da3e91cef90 Mon Sep 17 00:00:00 2001 From: Ashley Scopes <73482956+ascopes@users.noreply.github.com> Date: Thu, 2 Apr 2026 08:52:19 +0100 Subject: [PATCH 2/2] GH-974, GH-976: remove class references --- .../plexus/ImmutablesDataPlexusConverter.java | 73 ++++++++----------- 1 file changed, 29 insertions(+), 44 deletions(-) diff --git a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverter.java b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverter.java index fa9dcb71..c277128f 100644 --- a/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverter.java +++ b/protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/ImmutablesDataPlexusConverter.java @@ -17,11 +17,9 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; -import java.util.Collections; import java.util.Map; import java.util.NoSuchElementException; import java.util.Optional; -import java.util.WeakHashMap; import javax.inject.Named; import javax.inject.Singleton; import org.codehaus.plexus.component.configurator.ComponentConfigurationException; @@ -42,19 +40,15 @@ * and {@link org.immutables.datatype.Data}, otherwise deserialization will fail at * runtime. * + *

This will not store any references to the datatypes, to allow for sharing across classloaders + * safely (see GH-974 and GH-976). + * * @author Ashley Scopes * @since TBC */ @Named @Singleton final class ImmutablesDataPlexusConverter extends AbstractBasicConverter { - private final Map, Datatype> knownDatatypes; - - ImmutablesDataPlexusConverter() { - // Weak hashmap keys will be deregistered upon classloader destruction safely. - knownDatatypes = Collections.synchronizedMap(new WeakHashMap<>()); - } - @Override public boolean canConvert(Class cls) { return datatypeFor(cls).isPresent(); @@ -81,6 +75,32 @@ public Object fromConfiguration( return builder.build(); } + private Optional> datatypeFor(Class cls) { + if (cls.isPrimitive() || cls.getClassLoader() == null) { + return Optional.empty(); + } + + var loader = cls.getClassLoader(); + var outerClsName = cls.getPackageName() + ".Datatypes_" + cls.getSimpleName(); + + try { + var outerCls = loader.loadClass(outerClsName); + var method = outerCls.getMethod("_" + cls.getSimpleName()); + + @SuppressWarnings("unchecked") + var result = (Datatype) method.invoke(null); + + return Optional.of(result); + } catch (ClassNotFoundException ex) { + return Optional.empty(); + } catch (ReflectiveOperationException ex) { + throw new IllegalStateException( + "Failed to find datatype for " + cls.getName() + ": " + ex, + ex + ); + } + } + private void consumeChild( Datatype.Builder builder, PlexusConfiguration child, @@ -134,41 +154,6 @@ private void consumeChild( } } - private Optional> datatypeFor(Class cls) { - if (cls.isPrimitive() || cls.getClassLoader() == null) { - return Optional.empty(); - } - - // Horrible generic voodoo that probably is not safe, but the APIs have conflicting types - // and the compiler is not smart enough to help us. - @SuppressWarnings("unchecked") - var castCls = (Class) cls; - - var datatype = knownDatatypes.computeIfAbsent(castCls, ignored -> { - var loader = cls.getClassLoader(); - var outerClsName = cls.getPackageName() + ".Datatypes_" + cls.getSimpleName(); - - try { - var outerCls = loader.loadClass(outerClsName); - var method = outerCls.getMethod("_" + cls.getSimpleName()); - - @SuppressWarnings("unchecked") - var result = (Datatype) method.invoke(null); - - return result; - } catch (ClassNotFoundException ex) { - return null; - } catch (ReflectiveOperationException ex) { - throw new IllegalStateException( - "Failed to find datatype for " + cls.getName() + ": " + ex, - ex - ); - } - }); - - return Optional.ofNullable(datatype); - } - private static Class rawTypeOf(Type type) { return type instanceof ParameterizedType parameterizedType ? rawTypeOf(parameterizedType.getRawType())