Skip to content

Commit 002eb06

Browse files
committed
Route inheritance from base controller no longer works when multiple subclasses extend it (Jooby 4.x vs 1.x) for 3.x
1 parent e7cf67d commit 002eb06

File tree

3 files changed

+111
-80
lines changed

3 files changed

+111
-80
lines changed

modules/jooby-apt/src/main/java/io/jooby/apt/JoobyProcessor.java

Lines changed: 37 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
113113
context.add(router);
114114
var sourceCode = router.toSourceCode(null);
115115
var sourceLocation = router.getGeneratedFilename();
116-
onGeneratedSource(toJavaFileObject(sourceLocation, sourceCode));
116+
onGeneratedSource(
117+
router.getGeneratedType(), toJavaFileObject(sourceLocation, sourceCode));
117118
context.debug("router %s: %s", router.getTargetType(), router.getGeneratedType());
118119
router.getRoutes().forEach(it -> context.debug(" %s", it));
119120
writeSource(router, sourceLocation, sourceCode);
@@ -182,7 +183,7 @@ public String toString() {
182183
};
183184
}
184185

185-
protected void onGeneratedSource(JavaFileObject source) {}
186+
protected void onGeneratedSource(String className, JavaFileObject source) {}
186187

187188
private void doServices(Filer filer, List<MvcRouter> routers) {
188189
try {
@@ -261,44 +262,40 @@ private Map<TypeElement, MvcRouter> buildRouteRegistry(
261262
*/
262263
private void buildRouteRegistry(Map<TypeElement, MvcRouter> registry, TypeElement currentType) {
263264
for (TypeElement superType : context.superTypes(currentType)) {
264-
if (processed.add(superType)) {
265-
// collect all declared methods
266-
superType.getEnclosedElements().stream()
267-
.filter(ExecutableElement.class::isInstance)
268-
.map(ExecutableElement.class::cast)
269-
.forEach(
270-
method -> {
271-
if (method.getModifiers().contains(Modifier.ABSTRACT)) {
272-
context.debug("ignoring abstract method: %s %s", superType, method);
273-
} else {
274-
method.getAnnotationMirrors().stream()
275-
.map(AnnotationMirror::getAnnotationType)
276-
.map(DeclaredType::asElement)
277-
.filter(TypeElement.class::isInstance)
278-
.map(TypeElement.class::cast)
279-
.filter(HttpMethod::hasAnnotation)
280-
.forEach(
281-
annotation -> {
282-
Stream.of(currentType, superType)
283-
.distinct()
284-
.forEach(
285-
routerClass ->
286-
registry
287-
.computeIfAbsent(
288-
routerClass, type -> new MvcRouter(context, type))
289-
.put(annotation, method));
290-
});
291-
}
292-
});
293-
} else {
294-
if (!currentType.equals(superType)) {
295-
// edge-case #1: when controller has no method and extends another class which has.
296-
// edge-case #2: some odd usage a controller could be empty.
297-
// See https://github.com/jooby-project/jooby/issues/3656
298-
if (registry.containsKey(superType)) {
299-
registry.computeIfAbsent(
300-
currentType, key -> new MvcRouter(key, registry.get(superType)));
301-
}
265+
// collect all declared methods
266+
superType.getEnclosedElements().stream()
267+
.filter(ExecutableElement.class::isInstance)
268+
.map(ExecutableElement.class::cast)
269+
.forEach(
270+
method -> {
271+
if (method.getModifiers().contains(Modifier.ABSTRACT)) {
272+
context.debug("ignoring abstract method: %s %s", superType, method);
273+
} else {
274+
method.getAnnotationMirrors().stream()
275+
.map(AnnotationMirror::getAnnotationType)
276+
.map(DeclaredType::asElement)
277+
.filter(TypeElement.class::isInstance)
278+
.map(TypeElement.class::cast)
279+
.filter(HttpMethod::hasAnnotation)
280+
.forEach(
281+
annotation -> {
282+
Stream.of(currentType, superType)
283+
.distinct()
284+
.forEach(
285+
routerClass ->
286+
registry
287+
.computeIfAbsent(
288+
routerClass, type -> new MvcRouter(context, type))
289+
.put(annotation, method));
290+
});
291+
}
292+
});
293+
if (!currentType.equals(superType)) {
294+
// edge-case #1: when controller has no method and extends another class which has.
295+
// edge-case #2: some odd usage a controller could be empty.
296+
// See https://github.com/jooby-project/jooby/issues/3656
297+
if (registry.containsKey(superType)) {
298+
registry.computeIfAbsent(currentType, key -> new MvcRouter(key, registry.get(superType)));
302299
}
303300
}
304301
}

modules/jooby-apt/src/test/java/io/jooby/apt/ProcessorRunner.java

Lines changed: 66 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,18 @@
2727
public class ProcessorRunner {
2828

2929
private static class GeneratedSourceClassLoader extends ClassLoader {
30-
private final JavaFileObject classFile;
31-
private final String className;
30+
private final Map<String, JavaFileObject> classes = new LinkedHashMap<>();
3231

33-
public GeneratedSourceClassLoader(ClassLoader parent, JavaFileObject source) {
32+
public GeneratedSourceClassLoader(ClassLoader parent, Map<String, JavaFileObject> sources) {
3433
super(parent);
35-
this.classFile = javac().compile(List.of(source)).generatedFiles().get(0);
36-
this.className = source.getName().replace('/', '.').replace(".java", "");
37-
}
38-
39-
public String getClassName() {
40-
return className;
34+
for (var e : sources.entrySet()) {
35+
classes.put(e.getKey(), javac().compile(List.of(e.getValue())).generatedFiles().get(0));
36+
}
4137
}
4238

4339
protected Class<?> findClass(String name) throws ClassNotFoundException {
44-
if (name.equals(className)) {
40+
if (classes.containsKey(name)) {
41+
var classFile = classes.get(name);
4542
try (var in = classFile.openInputStream()) {
4643
var bytes = in.readAllBytes();
4744
return defineClass(name, bytes, 0, bytes.length);
@@ -54,48 +51,52 @@ protected Class<?> findClass(String name) throws ClassNotFoundException {
5451
}
5552

5653
private static class HookJoobyProcessor extends JoobyProcessor {
57-
private JavaFileObject source;
58-
private String kotlinSource;
54+
private Map<String, JavaFileObject> javaFiles = new LinkedHashMap<>();
55+
private Map<String, String> kotlinFiles = new LinkedHashMap<>();
5956

6057
public HookJoobyProcessor(Consumer<String> console) {
6158
super((kind, message) -> console.accept(message));
6259
}
6360

6461
public GeneratedSourceClassLoader createClassLoader() {
65-
Objects.requireNonNull(source);
66-
return new GeneratedSourceClassLoader(getClass().getClassLoader(), source);
62+
return new GeneratedSourceClassLoader(getClass().getClassLoader(), javaFiles);
6763
}
6864

6965
public JavaFileObject getSource() {
70-
return source;
66+
return javaFiles.isEmpty() ? null : javaFiles.entrySet().iterator().next().getValue();
7167
}
7268

7369
public String getKotlinSource() {
74-
return kotlinSource;
70+
return kotlinFiles.entrySet().iterator().next().getValue();
7571
}
7672

7773
public MvcContext getContext() {
7874
return context;
7975
}
8076

8177
@Override
82-
protected void onGeneratedSource(JavaFileObject source) {
83-
this.source = source;
78+
protected void onGeneratedSource(String classname, JavaFileObject source) {
79+
javaFiles.put(classname, source);
8480
try {
8581
// Generate kotlin source code inside the compiler scope... avoid false positive errors
86-
this.kotlinSource = context.getRouters().get(0).toSourceCode(true);
82+
kotlinFiles.put(classname, context.getRouters().get(0).toSourceCode(true));
8783
} catch (IOException e) {
8884
SneakyThrows.propagate(e);
8985
}
9086
}
9187
}
9288

89+
private final List<Object> instances;
9390
private final HookJoobyProcessor processor;
9491

9592
public ProcessorRunner(Object instance) throws IOException {
9693
this(instance, Map.of());
9794
}
9895

96+
public ProcessorRunner(List<Object> instances) throws IOException {
97+
this(instances, System.out::println, Map.of());
98+
}
99+
99100
public ProcessorRunner(Object instance, Consumer<String> stdout) throws IOException {
100101
this(instance, stdout, Map.of());
101102
}
@@ -106,12 +107,19 @@ public ProcessorRunner(Object instance, Map<String, Object> options) throws IOEx
106107

107108
public ProcessorRunner(Object instance, Consumer<String> stdout, Map<String, Object> options)
108109
throws IOException {
109-
this.processor = new HookJoobyProcessor(stdout::accept);
110+
this(List.of(instance), stdout, options);
111+
}
112+
113+
public ProcessorRunner(
114+
List<Object> instances, Consumer<String> stdout, Map<String, Object> options)
115+
throws IOException {
116+
this.instances = instances;
117+
this.processor = new HookJoobyProcessor(stdout);
110118
var optionsArray =
111119
options.entrySet().stream().map(e -> "-A" + e.getKey() + "=" + e.getValue()).toList();
112120
Truth.assert_()
113121
.about(JavaSourcesSubjectFactory.javaSources())
114-
.that(sources(sourceNames(instance.getClass())))
122+
.that(sources(sourceNames(instances.stream().map(Object::getClass).toList())))
115123
.withCompilerOptions(optionsArray.toArray(new String[0]))
116124
.processedWith(processor)
117125
.compilesWithoutError();
@@ -123,11 +131,33 @@ public ProcessorRunner withRouter(SneakyThrows.Consumer<Jooby> consumer) throws
123131

124132
public ProcessorRunner withRouter(SneakyThrows.Consumer2<Jooby, JavaFileObject> consumer)
125133
throws Exception {
134+
return withRouter(instances.get(0).getClass(), consumer);
135+
}
136+
137+
public ProcessorRunner withRouter(Class<?> routerType, SneakyThrows.Consumer<Jooby> consumer)
138+
throws Exception {
139+
return withRouter(routerType, (app, source) -> consumer.accept(app));
140+
}
141+
142+
public ProcessorRunner withRouter(
143+
Class<?> routerType, SneakyThrows.Consumer2<Jooby, JavaFileObject> consumer)
144+
throws Exception {
126145
var classLoader = processor.createClassLoader();
127-
var factoryName = classLoader.getClassName();
128-
var factoryClass = (Class<? extends MvcExtension>) classLoader.loadClass(factoryName);
129-
var constructor = factoryClass.getDeclaredConstructor();
130-
var extension = constructor.newInstance();
146+
var factoryName = routerType.getName() + "_";
147+
var factoryClass = (Class<? extends Extension>) classLoader.loadClass(factoryName);
148+
Extension extension;
149+
try {
150+
var constructor = factoryClass.getDeclaredConstructor();
151+
extension = constructor.newInstance();
152+
} catch (NoSuchMethodException x) {
153+
var instance =
154+
instances.stream()
155+
.filter(it -> it.getClass().equals(routerType))
156+
.findFirst()
157+
.orElseThrow(() -> new IllegalArgumentException("Not found: " + routerType));
158+
extension = factoryClass.getDeclaredConstructor(routerType).newInstance(instance);
159+
}
160+
131161
var application = new Jooby();
132162
application.install(extension);
133163
consumer.accept(application, processor.getSource());
@@ -146,17 +176,22 @@ public ProcessorRunner withSourceCode(SneakyThrows.Consumer<String> consumer) {
146176
public ProcessorRunner withSourceCode(boolean kt, SneakyThrows.Consumer<String> consumer) {
147177
consumer.accept(
148178
kt
149-
? processor.kotlinSource
179+
? processor.kotlinFiles.values().iterator().next()
150180
: Optional.ofNullable(processor.getSource()).map(Objects::toString).orElse(null));
151181
return this;
152182
}
153183

154-
private String[] sourceNames(Class input) {
184+
private String[] sourceNames(List<Class<? extends Object>> inputs) {
155185
List<String> result = new ArrayList<>();
156-
while (input != Object.class) {
157-
result.add(input.getName());
158-
input = input.getSuperclass();
159-
}
186+
Set<Class> visited = new HashSet<>();
187+
inputs.stream()
188+
.forEach(
189+
input -> {
190+
while (input != Object.class && visited.add(input)) {
191+
result.add(input.getName());
192+
input = input.getSuperclass();
193+
}
194+
});
160195
return result.toArray(new String[0]);
161196
}
162197

modules/jooby-apt/src/test/java/tests/i3786/Issue3786.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
import static org.junit.jupiter.api.Assertions.*;
99

10+
import java.util.List;
11+
1012
import org.junit.jupiter.api.Test;
1113

1214
import io.jooby.apt.ProcessorRunner;
@@ -16,9 +18,10 @@
1618
public class Issue3786 {
1719
@Test
1820
public void shouldCheckBase() throws Exception {
19-
new ProcessorRunner(new C3786())
21+
new ProcessorRunner(List.of(new C3786(), new D3786()))
2022
.withRouter(
21-
app -> {
23+
C3786.class,
24+
(app, source) -> {
2225
var router = new MockRouter(app);
2326
assertEquals("base", router.get("/inherited", new MockContext()).value());
2427
assertEquals(
@@ -30,14 +33,10 @@ public void shouldCheckBase() throws Exception {
3033
assertEquals(
3134
"POST/inherited/childOnly",
3235
router.post("/inherited/childOnly", new MockContext()).value());
33-
});
34-
}
35-
36-
@Test
37-
public void shouldCheckOverride() throws Exception {
38-
new ProcessorRunner(new D3786())
36+
})
3937
.withRouter(
40-
app -> {
38+
D3786.class,
39+
(app, source) -> {
4140
var router = new MockRouter(app);
4241
assertEquals("base", router.get("/overrideMethod", new MockContext()).value());
4342
assertEquals(

0 commit comments

Comments
 (0)