Skip to content

Commit 6a3b0f2

Browse files
committed
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 paul-hammant/qdox#271 temporarily to work around apache/maven-plugin-tools#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 immutables/immutables#1608 when initialising the data type descriptors. Add missing test case for immutables/immutables#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
1 parent 055ef5b commit 6a3b0f2

21 files changed

Lines changed: 524 additions & 111 deletions

.mvn/checkstyle/suppressions.xml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@
2020
"-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN"
2121
"https://checkstyle.org/dtds/suppressions_1_2.dtd">
2222
<suppressions>
23-
</suppressions>
23+
<suppress files="Datatypes_SomeBrokenModel.java" checks=".+"/>
24+
</suppressions>

protobuf-maven-plugin/pom.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,12 @@
8282
<scope>provided</scope>
8383
</dependency>
8484

85+
<dependency>
86+
<groupId>org.immutables</groupId>
87+
<artifactId>datatype</artifactId>
88+
<scope>compile</scope>
89+
</dependency>
90+
8591
<dependency>
8692
<groupId>org.immutables</groupId>
8793
<artifactId>value</artifactId>

protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/dependencies/MavenDependency.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
package io.github.ascopes.protobufmavenplugin.dependencies;
1717

1818
import java.util.Set;
19+
import org.immutables.datatype.Data;
1920
import org.immutables.value.Value.Immutable;
20-
import org.immutables.value.Value.Modifiable;
2121
import org.jspecify.annotations.Nullable;
2222

2323

@@ -27,13 +27,13 @@
2727
* @author Ashley Scopes
2828
* @since 3.3.1
2929
*/
30+
@Data
3031
@Immutable
31-
@Modifiable
3232
public abstract class MavenDependency extends MavenArtifact {
3333

3434
public abstract @Nullable DependencyResolutionDepth getDependencyResolutionDepth();
3535

36-
public abstract Set<MavenExclusionBean> getExclusions();
36+
public abstract Set<MavenExclusion> getExclusions();
3737

3838
// Must be overridden to keep immutables happy.
3939
@Override

protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/dependencies/MavenExclusion.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@
1515
*/
1616
package io.github.ascopes.protobufmavenplugin.dependencies;
1717

18-
import org.immutables.value.Value.Modifiable;
18+
import org.immutables.datatype.Data;
19+
import org.immutables.value.Value.Immutable;
1920

2021
/**
2122
* Marker to exclude a specific transitive dependency.
@@ -26,7 +27,8 @@
2627
* @author Ashley Scopes
2728
* @since 2.12.0
2829
*/
29-
@Modifiable
30+
@Data
31+
@Immutable
3032
public interface MavenExclusion {
3133

3234
/**

protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/mojo/AbstractGenerateMojo.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,19 @@
2020
import static java.util.function.Predicate.not;
2121

2222
import io.github.ascopes.protobufmavenplugin.dependencies.DependencyResolutionDepth;
23-
import io.github.ascopes.protobufmavenplugin.dependencies.MavenDependencyBean;
23+
import io.github.ascopes.protobufmavenplugin.dependencies.MavenDependency;
2424
import io.github.ascopes.protobufmavenplugin.digests.Digest;
2525
import io.github.ascopes.protobufmavenplugin.generation.GenerationResult;
2626
import io.github.ascopes.protobufmavenplugin.generation.ImmutableGenerationRequest;
2727
import io.github.ascopes.protobufmavenplugin.generation.Language;
2828
import io.github.ascopes.protobufmavenplugin.generation.OutputDescriptorAttachmentRegistrar;
2929
import io.github.ascopes.protobufmavenplugin.generation.ProtobufBuildOrchestrator;
3030
import io.github.ascopes.protobufmavenplugin.generation.SourceRootRegistrar;
31-
import io.github.ascopes.protobufmavenplugin.plugins.BinaryMavenProtocPluginBean;
32-
import io.github.ascopes.protobufmavenplugin.plugins.JvmMavenProtocPluginBean;
33-
import io.github.ascopes.protobufmavenplugin.plugins.PathProtocPluginBean;
31+
import io.github.ascopes.protobufmavenplugin.plugins.BinaryMavenProtocPlugin;
32+
import io.github.ascopes.protobufmavenplugin.plugins.JvmMavenProtocPlugin;
33+
import io.github.ascopes.protobufmavenplugin.plugins.PathProtocPlugin;
3434
import io.github.ascopes.protobufmavenplugin.plugins.ProtocPlugin;
35-
import io.github.ascopes.protobufmavenplugin.plugins.UriProtocPluginBean;
35+
import io.github.ascopes.protobufmavenplugin.plugins.UriProtocPlugin;
3636
import java.nio.file.Files;
3737
import java.nio.file.Path;
3838
import java.util.ArrayList;
@@ -312,7 +312,7 @@ public AbstractGenerateMojo() {
312312
* @since 1.2.0
313313
*/
314314
@Parameter
315-
@Nullable List<MavenDependencyBean> importDependencies;
315+
@Nullable List<MavenDependency> importDependencies;
316316

317317
/**
318318
* Specify additional paths to import protobuf sources from on the local file system.
@@ -701,7 +701,7 @@ public AbstractGenerateMojo() {
701701
* @since 1.2.0
702702
*/
703703
@Parameter
704-
@Nullable List<MavenDependencyBean> sourceDependencies;
704+
@Nullable List<MavenDependency> sourceDependencies;
705705

706706
/**
707707
* Protobuf Descriptor files to compile.
@@ -735,7 +735,7 @@ public AbstractGenerateMojo() {
735735
* @since 3.1.0
736736
*/
737737
@Parameter
738-
@Nullable List<MavenDependencyBean> sourceDescriptorDependencies;
738+
@Nullable List<MavenDependency> sourceDescriptorDependencies;
739739

740740
/**
741741
* The source directories to compile protobuf sources from.
@@ -787,7 +787,7 @@ public AbstractGenerateMojo() {
787787
*/
788788
@Deprecated(forRemoval = true)
789789
@Parameter
790-
@Nullable List<BinaryMavenProtocPluginBean> binaryMavenPlugins;
790+
@Nullable List<BinaryMavenProtocPlugin> binaryMavenPlugins;
791791

792792
/**
793793
* Binary plugins to use with the protobuf compiler, sourced from the system {@code PATH}.
@@ -804,7 +804,7 @@ public AbstractGenerateMojo() {
804804
*/
805805
@Deprecated(forRemoval = true)
806806
@Parameter
807-
@Nullable List<PathProtocPluginBean> binaryPathPlugins;
807+
@Nullable List<PathProtocPlugin> binaryPathPlugins;
808808

809809
/**
810810
* Binary plugins to use with the protobuf compiler, specified as a valid URL.
@@ -821,7 +821,7 @@ public AbstractGenerateMojo() {
821821
*/
822822
@Deprecated(forRemoval = true)
823823
@Parameter
824-
@Nullable List<UriProtocPluginBean> binaryUrlPlugins;
824+
@Nullable List<UriProtocPlugin> binaryUrlPlugins;
825825

826826
/**
827827
* Additional <strong>pure-Java</strong> plugins to use with the protobuf compiler.
@@ -838,7 +838,7 @@ public AbstractGenerateMojo() {
838838
*/
839839
@Deprecated(forRemoval = true)
840840
@Parameter
841-
@Nullable List<JvmMavenProtocPluginBean> jvmMavenPlugins;
841+
@Nullable List<JvmMavenProtocPlugin> jvmMavenPlugins;
842842

843843
/*
844844
* Implementation-specific details.

protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/package-info.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,10 @@
2020
*/
2121
@NullMarked
2222
@Style(
23-
beanFriendlyModifiables = true,
24-
create = "new",
2523
defaults = @Immutable(copy = false),
2624
defaultAsDefault = true,
27-
deferCollectionAllocation = true,
2825
get = {"get*", "is*"},
29-
headerComments = true,
3026
jacksonIntegration = false,
31-
// Eventually may be true when Maven upgrades.
32-
jakarta = false,
3327
jdkOnly = true,
3428
jdk9Collections = true,
3529
nullableAnnotation = "org.jspecify.annotations.Nullable",
Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
/*
2+
* Copyright (C) 2023 - 2025, Ashley Scopes.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.github.ascopes.protobufmavenplugin.plexus;
17+
18+
import java.lang.reflect.ParameterizedType;
19+
import java.lang.reflect.Type;
20+
import java.util.Collections;
21+
import java.util.Map;
22+
import java.util.NoSuchElementException;
23+
import java.util.Optional;
24+
import java.util.WeakHashMap;
25+
import javax.inject.Named;
26+
import javax.inject.Singleton;
27+
import org.codehaus.plexus.component.configurator.ComponentConfigurationException;
28+
import org.codehaus.plexus.component.configurator.ConfigurationListener;
29+
import org.codehaus.plexus.component.configurator.converters.ParameterizedConfigurationConverter;
30+
import org.codehaus.plexus.component.configurator.converters.basic.AbstractBasicConverter;
31+
import org.codehaus.plexus.component.configurator.converters.lookup.ConverterLookup;
32+
import org.codehaus.plexus.component.configurator.expression.ExpressionEvaluator;
33+
import org.codehaus.plexus.configuration.PlexusConfiguration;
34+
import org.immutables.datatype.Datatype;
35+
import org.jspecify.annotations.Nullable;
36+
37+
/**
38+
* Converter for Plexus components that can build a generated "immutables" value type, based
39+
* on the interface it was derived from.
40+
*
41+
* <p>Note that all objects must be annotated with both {@link org.immutables.value.Value}
42+
* <strong>and</strong> {@link org.immutables.datatype.Data}, otherwise deserialization will fail at
43+
* runtime.
44+
*
45+
* @author Ashley Scopes
46+
* @since TBC
47+
*/
48+
@Named
49+
@Singleton
50+
final class ImmutablesDataPlexusConverter extends AbstractBasicConverter {
51+
private final Map<Class<Object>, Datatype<Object>> knownDatatypes;
52+
53+
ImmutablesDataPlexusConverter() {
54+
// Weak hashmap keys will be deregistered upon classloader destruction safely.
55+
knownDatatypes = Collections.synchronizedMap(new WeakHashMap<>());
56+
}
57+
58+
@Override
59+
public boolean canConvert(Class<?> cls) {
60+
return datatypeFor(cls).isPresent();
61+
}
62+
63+
@Override
64+
public Object fromConfiguration(
65+
ConverterLookup lookup,
66+
PlexusConfiguration configuration,
67+
Class<?> type,
68+
@Nullable Class<?> enclosingType,
69+
@Nullable ClassLoader loader,
70+
ExpressionEvaluator evaluator,
71+
@Nullable ConfigurationListener listener
72+
) throws ComponentConfigurationException {
73+
var datatype = datatypeFor(type)
74+
.orElseThrow(() -> new NoSuchElementException("No datatype converter " + type.getName()));
75+
76+
var builder = datatype.builder();
77+
for (var child : configuration.getChildren()) {
78+
consumeChild(builder, child, lookup, datatype, loader, evaluator, listener);
79+
}
80+
81+
return builder.build();
82+
}
83+
84+
private void consumeChild(
85+
Datatype.Builder<Object> builder,
86+
PlexusConfiguration child,
87+
ConverterLookup lookup,
88+
Datatype<?> datatype,
89+
@Nullable ClassLoader loader,
90+
ExpressionEvaluator evaluator,
91+
@Nullable ConfigurationListener listener
92+
) throws ComponentConfigurationException {
93+
try {
94+
@SuppressWarnings("unchecked")
95+
var feature = (Datatype.Feature<Object, Object>) datatype.feature(child.getName());
96+
var valueType = feature.type();
97+
var rawType = rawTypeOf(valueType);
98+
99+
var converter = lookup.lookupConverterForType(rawType);
100+
101+
Object value;
102+
103+
if (converter instanceof ParameterizedConfigurationConverter parameterizedConverter) {
104+
var parameterizedType = (ParameterizedType) valueType;
105+
value = parameterizedConverter.fromConfiguration(
106+
lookup,
107+
child,
108+
rawType,
109+
parameterizedType.getActualTypeArguments(),
110+
rawType.getEnclosingClass(),
111+
loader,
112+
evaluator,
113+
listener
114+
);
115+
} else {
116+
value = converter.fromConfiguration(
117+
lookup,
118+
child,
119+
rawType,
120+
rawType.getEnclosingClass(),
121+
loader,
122+
evaluator,
123+
listener
124+
);
125+
}
126+
127+
builder.set(feature, value);
128+
129+
} catch (NoSuchElementException ex) {
130+
throw new ComponentConfigurationException(
131+
"No attribute " + child.getName() + " exists for " + datatype.name(),
132+
ex
133+
);
134+
}
135+
}
136+
137+
private Optional<Datatype<Object>> datatypeFor(Class<?> cls) {
138+
if (cls.isPrimitive() || cls.getClassLoader() == null) {
139+
return Optional.empty();
140+
}
141+
142+
// Horrible generic voodoo that probably is not safe, but the APIs have conflicting types
143+
// and the compiler is not smart enough to help us.
144+
@SuppressWarnings("unchecked")
145+
var castCls = (Class<Object>) cls;
146+
147+
var datatype = knownDatatypes.computeIfAbsent(castCls, ignored -> {
148+
var loader = cls.getClassLoader();
149+
var outerClsName = cls.getPackageName() + ".Datatypes_" + cls.getSimpleName();
150+
151+
try {
152+
var outerCls = loader.loadClass(outerClsName);
153+
var method = outerCls.getMethod("_" + cls.getSimpleName());
154+
155+
@SuppressWarnings("unchecked")
156+
var result = (Datatype<Object>) method.invoke(null);
157+
158+
return result;
159+
} catch (ClassNotFoundException ex) {
160+
return null;
161+
} catch (ReflectiveOperationException ex) {
162+
throw new IllegalStateException(
163+
"Failed to find datatype for " + cls.getName() + ": " + ex,
164+
ex
165+
);
166+
}
167+
});
168+
169+
return Optional.ofNullable(datatype);
170+
}
171+
172+
private static Class<?> rawTypeOf(Type type) {
173+
return type instanceof ParameterizedType parameterizedType
174+
? rawTypeOf(parameterizedType.getRawType())
175+
// Assumption: this cannot ever be a wildcard or union type.
176+
: (Class<?>) type;
177+
}
178+
}

protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/KindHint.java

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
/**
2626
* Marker to advise the "kind" of the implementation when used with a sealed-type hierarchy.
2727
*
28+
* <p>When this marker is spotted, the {@code ConverterLookup} will be consulted to produce
29+
* an instance of that type.
30+
*
2831
* @author Ashley Scopes
2932
* @since 4.1.0
3033
*/
@@ -38,20 +41,5 @@
3841
*
3942
* @return the kind.
4043
*/
41-
String kind();
42-
43-
/**
44-
* The implementation that the kind should point to.
45-
*
46-
* <p>This is needed until
47-
* <a href="https://github.com/ascopes/protobuf-maven-plugin/pull/880">GH-880</a>
48-
* can be merged, as we cannot easily infer the immutable implementation class from the base
49-
* when working with the Immutables library. GH-880 will enable better integration with that
50-
* library based on compile-time metadata that avoids this issue. We may just need to include
51-
* this annotation in {@link Style#passAnnotations()} to achieve this correctly closer to the
52-
* time.
53-
*
54-
* @return the implementation type.
55-
*/
56-
Class<?> implementation();
44+
String value();
5745
}

protobuf-maven-plugin/src/main/java/io/github/ascopes/protobufmavenplugin/plexus/SealedTypePlexusConverter.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,11 @@ private static Map<String, Class<?>> computeKindMappingFor(Class<?> base) {
162162
log.trace(
163163
"Found concrete kind for base \"{}\": \"{}\" will map to \"{}\"",
164164
base.getName(),
165-
kind.kind(),
166-
kind.implementation().getName()
165+
kind.value(),
166+
next.getName()
167167
);
168168

169-
mapping.put(kind.kind(), kind.implementation());
169+
mapping.put(kind.value(), next);
170170
}
171171
}
172172
}

0 commit comments

Comments
 (0)