Skip to content

Commit bcd5d9c

Browse files
gnodetgnodet
authored
DI improvements (#1717)
* Code cleanup * Provide build path with causes when an exception occur * Fix toString to display meaningful info * Javadoc * Add @nonnull and @OVERRIDES annotations * Support for @nullable on fields and parameters
1 parent 36de1c6 commit bcd5d9c

File tree

12 files changed

+183
-84
lines changed

12 files changed

+183
-84
lines changed

‎api/maven-api-di/src/main/java/org/apache/maven/api/di/MojoExecutionScoped.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@
2929
/**
3030
* Indicates that the annotated bean has a lifespan limited to a given mojo execution,
3131
* which means each mojo execution will result in a different instance being injected.
32+
* <p>
33+
* The following objects will be bound to the mojo execution scope:
34+
* <ul>
35+
* <li>{@code org.apache.maven.api.MojoExecution}</li>
36+
* <li>{@code org.apache.maven.api.Project}</li>
37+
* <li>{@code org.apache.maven.api.plugin.Log}</li>
38+
* </ul>
3239
*
3340
* @since 4.0.0
3441
*/

‎api/maven-api-di/src/main/java/org/apache/maven/api/di/SessionScoped.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
/**
3030
* Indicates that annotated component should be instantiated before session execution starts
3131
* and discarded after session execution completes.
32+
* <p>
33+
* A {@code org.apache.maven.api.Session} object is available in the scope of this annotation.
3234
*
3335
* @since 4.0.0
3436
*/

‎api/maven-api-meta/src/main/java/org/apache/maven/api/annotations/Nullable.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,5 @@
3030
*/
3131
@Experimental
3232
@Documented
33-
@Retention(RetentionPolicy.CLASS)
33+
@Retention(RetentionPolicy.RUNTIME)
3434
public @interface Nullable {}

‎maven-core/src/main/java/org/apache/maven/internal/impl/SisuDiBridgeModule.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import org.apache.maven.di.Key;
4949
import org.apache.maven.di.impl.Binding;
5050
import org.apache.maven.di.impl.DIException;
51+
import org.apache.maven.di.impl.Dependency;
5152
import org.apache.maven.di.impl.InjectorImpl;
5253
import org.apache.maven.execution.scope.internal.MojoExecutionScope;
5354
import org.apache.maven.session.scope.internal.SessionScope;
@@ -66,7 +67,8 @@ protected void configure() {
6667

6768
injector = new InjectorImpl() {
6869
@Override
69-
public <Q> Supplier<Q> getCompiledBinding(Key<Q> key) {
70+
public <Q> Supplier<Q> getCompiledBinding(Dependency<Q> dep) {
71+
Key<Q> key = dep.key();
7072
Set<Binding<Q>> res = getBindings(key);
7173
if (res != null && !res.isEmpty()) {
7274
List<Binding<Q>> bindingList = new ArrayList<>(res);
@@ -119,6 +121,9 @@ public <Q> Supplier<Q> getCompiledBinding(Key<Q> key) {
119121
// ignore
120122
e.printStackTrace();
121123
}
124+
if (dep.optional()) {
125+
return () -> null;
126+
}
122127
throw new DIException("No binding to construct an instance for key "
123128
+ key.getDisplayString() + ". Existing bindings:\n"
124129
+ getBoundKeys().stream()

‎maven-di/src/main/java/org/apache/maven/di/Injector.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.lang.annotation.Annotation;
2222
import java.util.function.Supplier;
2323

24+
import org.apache.maven.api.annotations.Nonnull;
2425
import org.apache.maven.di.impl.InjectorImpl;
2526

2627
public interface Injector {
@@ -29,18 +30,24 @@ public interface Injector {
2930
// Builder API
3031
//
3132

33+
@Nonnull
3234
static Injector create() {
3335
return new InjectorImpl();
3436
}
3537

38+
@Nonnull
3639
Injector discover(ClassLoader classLoader);
3740

41+
@Nonnull
3842
Injector bindScope(Class<? extends Annotation> scopeAnnotation, Scope scope);
3943

44+
@Nonnull
4045
Injector bindScope(Class<? extends Annotation> scopeAnnotation, Supplier<Scope> scope);
4146

47+
@Nonnull
4248
Injector bindImplicit(Class<?> cls);
4349

50+
@Nonnull
4451
<T> Injector bindInstance(Class<T> cls, T instance);
4552

4653
//

‎maven-di/src/main/java/org/apache/maven/di/impl/Binding.java‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,16 @@
3434
import static java.util.stream.Collectors.joining;
3535

3636
public abstract class Binding<T> {
37-
private final Set<Key<?>> dependencies;
37+
private final Set<Dependency<?>> dependencies;
3838
private Annotation scope;
3939
private int priority;
4040
private Key<?> originalKey;
4141

42-
protected Binding(Key<? extends T> originalKey, Set<Key<?>> dependencies) {
42+
protected Binding(Key<? extends T> originalKey, Set<Dependency<?>> dependencies) {
4343
this(originalKey, dependencies, null, 0);
4444
}
4545

46-
protected Binding(Key<?> originalKey, Set<Key<?>> dependencies, Annotation scope, int priority) {
46+
protected Binding(Key<?> originalKey, Set<Dependency<?>> dependencies, Annotation scope, int priority) {
4747
this.originalKey = originalKey;
4848
this.dependencies = dependencies;
4949
this.scope = scope;
@@ -56,15 +56,18 @@ public static <T> Binding<T> toInstance(T instance) {
5656

5757
public static <R> Binding<R> to(Key<R> originalKey, TupleConstructorN<R> constructor, Class<?>[] types) {
5858
return Binding.to(
59-
originalKey, constructor, Stream.of(types).map(Key::of).toArray(Key<?>[]::new));
59+
originalKey,
60+
constructor,
61+
Stream.of(types).map(c -> new Dependency<>(Key.of(c), false)).toArray(Dependency<?>[]::new));
6062
}
6163

62-
public static <R> Binding<R> to(Key<R> originalKey, TupleConstructorN<R> constructor, Key<?>[] dependencies) {
64+
public static <R> Binding<R> to(
65+
Key<R> originalKey, TupleConstructorN<R> constructor, Dependency<?>[] dependencies) {
6366
return to(originalKey, constructor, dependencies, 0);
6467
}
6568

6669
public static <R> Binding<R> to(
67-
Key<R> originalKey, TupleConstructorN<R> constructor, Key<?>[] dependencies, int priority) {
70+
Key<R> originalKey, TupleConstructorN<R> constructor, Dependency<?>[] dependencies, int priority) {
6871
return new BindingToConstructor<>(originalKey, constructor, dependencies, priority);
6972
}
7073

@@ -94,13 +97,17 @@ public Binding<T> initializeWith(BindingInitializer<T> bindingInitializer) {
9497
this.scope,
9598
this.priority) {
9699
@Override
97-
public Supplier<T> compile(Function<Key<?>, Supplier<?>> compiler) {
100+
public Supplier<T> compile(Function<Dependency<?>, Supplier<?>> compiler) {
98101
final Supplier<T> compiledBinding = Binding.this.compile(compiler);
99102
final Consumer<T> consumer = bindingInitializer.compile(compiler);
100103
return () -> {
101-
T instance = compiledBinding.get();
102-
consumer.accept(instance);
103-
return instance;
104+
try {
105+
T instance = compiledBinding.get();
106+
consumer.accept(instance);
107+
return instance;
108+
} catch (DIException e) {
109+
throw new DIException("Error while initializing binding " + Binding.this, e);
110+
}
104111
};
105112
}
106113

@@ -111,9 +118,9 @@ public String toString() {
111118
};
112119
}
113120

114-
public abstract Supplier<T> compile(Function<Key<?>, Supplier<?>> compiler);
121+
public abstract Supplier<T> compile(Function<Dependency<?>, Supplier<?>> compiler);
115122

116-
public Set<Key<?>> getDependencies() {
123+
public Set<Dependency<?>> getDependencies() {
117124
return dependencies;
118125
}
119126

@@ -122,7 +129,7 @@ public Annotation getScope() {
122129
}
123130

124131
public String getDisplayString() {
125-
return dependencies.stream().map(Key::getDisplayString).collect(joining(", ", "[", "]"));
132+
return dependencies.stream().map(Dependency::getDisplayString).collect(joining(", ", "[", "]"));
126133
}
127134

128135
public Key<?> getOriginalKey() {
@@ -152,7 +159,7 @@ public BindingToInstance(T instance) {
152159
}
153160

154161
@Override
155-
public Supplier<T> compile(Function<Key<?>, Supplier<?>> compiler) {
162+
public Supplier<T> compile(Function<Dependency<?>, Supplier<?>> compiler) {
156163
return () -> instance;
157164
}
158165

@@ -164,17 +171,17 @@ public String toString() {
164171

165172
public static class BindingToConstructor<T> extends Binding<T> {
166173
final TupleConstructorN<T> constructor;
167-
final Key<?>[] args;
174+
final Dependency<?>[] args;
168175

169176
BindingToConstructor(
170-
Key<? extends T> key, TupleConstructorN<T> constructor, Key<?>[] dependencies, int priority) {
177+
Key<? extends T> key, TupleConstructorN<T> constructor, Dependency<?>[] dependencies, int priority) {
171178
super(key, new HashSet<>(Arrays.asList(dependencies)), null, priority);
172179
this.constructor = constructor;
173180
this.args = dependencies;
174181
}
175182

176183
@Override
177-
public Supplier<T> compile(Function<Key<?>, Supplier<?>> compiler) {
184+
public Supplier<T> compile(Function<Dependency<?>, Supplier<?>> compiler) {
178185
return () -> {
179186
Object[] args =
180187
Stream.of(this.args).map(compiler).map(Supplier::get).toArray();
@@ -184,7 +191,7 @@ public Supplier<T> compile(Function<Key<?>, Supplier<?>> compiler) {
184191

185192
@Override
186193
public String toString() {
187-
return "BindingToConstructor[" + constructor + "]" + getDependencies();
194+
return "BindingToConstructor[" + getOriginalKey() + "]" + getDependencies();
188195
}
189196
}
190197
}

‎maven-di/src/main/java/org/apache/maven/di/impl/BindingInitializer.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,32 +25,30 @@
2525
import java.util.function.Function;
2626
import java.util.function.Supplier;
2727

28-
import org.apache.maven.di.Key;
29-
3028
import static java.util.stream.Collectors.toSet;
3129

3230
public abstract class BindingInitializer<T> {
3331

34-
private final Set<Key<?>> dependencies;
32+
private final Set<Dependency<?>> dependencies;
3533

36-
protected BindingInitializer(Set<Key<?>> dependencies) {
34+
protected BindingInitializer(Set<Dependency<?>> dependencies) {
3735
this.dependencies = dependencies;
3836
}
3937

40-
public Set<Key<?>> getDependencies() {
38+
public Set<Dependency<?>> getDependencies() {
4139
return dependencies;
4240
}
4341

44-
public abstract Consumer<T> compile(Function<Key<?>, Supplier<?>> compiler);
42+
public abstract Consumer<T> compile(Function<Dependency<?>, Supplier<?>> compiler);
4543

4644
public static <T> BindingInitializer<T> combine(List<BindingInitializer<T>> bindingInitializers) {
47-
Set<Key<?>> deps = bindingInitializers.stream()
45+
Set<Dependency<?>> deps = bindingInitializers.stream()
4846
.map(BindingInitializer::getDependencies)
4947
.flatMap(Collection::stream)
5048
.collect(toSet());
51-
return new BindingInitializer<T>(deps) {
49+
return new BindingInitializer<>(deps) {
5250
@Override
53-
public Consumer<T> compile(Function<Key<?>, Supplier<?>> compiler) {
51+
public Consumer<T> compile(Function<Dependency<?>, Supplier<?>> compiler) {
5452
return instance -> bindingInitializers.stream()
5553
.map(bindingInitializer -> bindingInitializer.compile(compiler))
5654
.forEach(i -> i.accept(instance));

‎maven-di/src/main/java/org/apache/maven/di/impl/Dependency.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.maven.di.impl;
20+
21+
import org.apache.maven.di.Key;
22+
23+
public record Dependency<T>(Key<T> key, boolean optional) {
24+
public String getDisplayString() {
25+
String s = key.getDisplayString();
26+
if (optional) {
27+
s = "?" + s;
28+
}
29+
return s;
30+
}
31+
}

‎maven-di/src/main/java/org/apache/maven/di/impl/InjectorImpl.java‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,14 @@ public InjectorImpl() {
6464
bindScope(Singleton.class, new SingletonScope());
6565
}
6666

67+
@Override
6768
public <T> T getInstance(Class<T> key) {
6869
return getInstance(Key.of(key));
6970
}
7071

72+
@Override
7173
public <T> T getInstance(Key<T> key) {
72-
return getCompiledBinding(key).get();
74+
return getCompiledBinding(new Dependency<>(key, false)).get();
7375
}
7476

7577
@SuppressWarnings("unchecked")
@@ -88,7 +90,7 @@ public Injector discover(ClassLoader classLoader) {
8890
try (InputStream is = enumeration.nextElement().openStream();
8991
BufferedReader reader = new BufferedReader(new InputStreamReader(Objects.requireNonNull(is)))) {
9092
for (String line :
91-
reader.lines().filter(l -> !l.startsWith("#")).collect(Collectors.toList())) {
93+
reader.lines().filter(l -> !l.startsWith("#")).toList()) {
9294
Class<?> clazz = classLoader.loadClass(line);
9395
bindImplicit(clazz);
9496
}
@@ -100,10 +102,12 @@ public Injector discover(ClassLoader classLoader) {
100102
return this;
101103
}
102104

105+
@Override
103106
public Injector bindScope(Class<? extends Annotation> scopeAnnotation, Scope scope) {
104107
return bindScope(scopeAnnotation, () -> scope);
105108
}
106109

110+
@Override
107111
public Injector bindScope(Class<? extends Annotation> scopeAnnotation, Supplier<Scope> scope) {
108112
if (scopes.put(scopeAnnotation, scope) != null) {
109113
throw new DIException(
@@ -112,6 +116,7 @@ public Injector bindScope(Class<? extends Annotation> scopeAnnotation, Supplier<
112116
return this;
113117
}
114118

119+
@Override
115120
public <U> Injector bindInstance(Class<U> clazz, U instance) {
116121
Key<?> key = Key.of(clazz, ReflectionUtils.qualifierOf(clazz));
117122
Binding<U> binding = Binding.toInstance(instance);
@@ -133,7 +138,7 @@ public Injector bindImplicit(Class<?> clazz) {
133138
return this;
134139
}
135140

136-
private LinkedHashSet<Key<?>> current = new LinkedHashSet<>();
141+
private final LinkedHashSet<Key<?>> current = new LinkedHashSet<>();
137142

138143
private Injector doBind(Key<?> key, Binding<?> binding) {
139144
if (!current.add(key)) {
@@ -175,7 +180,8 @@ public Map<Key<?>, Set<Binding<?>>> getBindings() {
175180
return bindings;
176181
}
177182

178-
public <Q> Supplier<Q> getCompiledBinding(Key<Q> key) {
183+
public <Q> Supplier<Q> getCompiledBinding(Dependency<Q> dep) {
184+
Key<Q> key = dep.key();
179185
Set<Binding<Q>> res = getBindings(key);
180186
if (res != null && !res.isEmpty()) {
181187
List<Binding<Q>> bindingList = new ArrayList<>(res);
@@ -211,6 +217,9 @@ public <Q> Supplier<Q> getCompiledBinding(Key<Q> key) {
211217
return () -> (Q) map(map);
212218
}
213219
}
220+
if (dep.optional()) {
221+
return () -> null;
222+
}
214223
throw new DIException("No binding to construct an instance for key "
215224
+ key.getDisplayString() + ". Existing bindings:\n"
216225
+ getBoundKeys().stream()
@@ -312,14 +321,13 @@ private static class WrappingMap<K, V, T> extends AbstractMap<K, V> {
312321
this.mapper = mapper;
313322
}
314323

315-
@SuppressWarnings("NullableProblems")
316324
@Override
317325
public Set<Entry<K, V>> entrySet() {
318-
return new AbstractSet<Entry<K, V>>() {
326+
return new AbstractSet<>() {
319327
@Override
320328
public Iterator<Entry<K, V>> iterator() {
321329
Iterator<Entry<K, T>> it = delegate.entrySet().iterator();
322-
return new Iterator<Entry<K, V>>() {
330+
return new Iterator<>() {
323331
@Override
324332
public boolean hasNext() {
325333
return it.hasNext();

0 commit comments

Comments
 (0)