Skip to content

Commit 1bcb1d9

Browse files
committed
Route inheritance from base controller no longer works when multiple subclasses extend it (Jooby 4.x vs 1.x)
- fix #3786
1 parent f9c552a commit 1bcb1d9

File tree

3 files changed

+103
-102
lines changed

3 files changed

+103
-102
lines changed

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

Lines changed: 37 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import javax.tools.Diagnostic;
2727
import javax.tools.JavaFileObject;
2828
import javax.tools.SimpleJavaFileObject;
29-
import javax.tools.StandardLocation;
3029

3130
import io.jooby.internal.apt.*;
3231

@@ -68,7 +67,6 @@ static String string(ProcessingEnvironment environment, String option, String de
6867

6968
protected MvcContext context;
7069
private BiConsumer<Diagnostic.Kind, String> output;
71-
private final Set<Object> processed = new HashSet<>();
7270

7371
public JoobyProcessor(BiConsumer<Diagnostic.Kind, String> output) {
7472
this.output = output;
@@ -104,7 +102,8 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
104102
context.add(router);
105103
var sourceCode = router.toSourceCode(null);
106104
var sourceLocation = router.getGeneratedFilename();
107-
onGeneratedSource(toJavaFileObject(sourceLocation, sourceCode));
105+
onGeneratedSource(
106+
router.getGeneratedType(), toJavaFileObject(sourceLocation, sourceCode));
108107
context.debug("router %s: %s", router.getTargetType(), router.getGeneratedType());
109108
router.getRoutes().forEach(it -> context.debug(" %s", it));
110109
writeSource(router, sourceLocation, sourceCode);
@@ -173,27 +172,7 @@ public String toString() {
173172
};
174173
}
175174

176-
protected void onGeneratedSource(JavaFileObject source) {}
177-
178-
private void doServices(Filer filer, List<MvcRouter> routers) {
179-
try {
180-
var location = "META-INF/services/io.jooby.MvcFactory";
181-
context.debug(location);
182-
183-
var resource = filer.createResource(StandardLocation.CLASS_OUTPUT, "", location);
184-
var content = new StringBuilder();
185-
for (var router : routers) {
186-
var classname = router.getGeneratedType();
187-
context.debug(" %s", classname);
188-
content.append(classname).append(System.lineSeparator());
189-
}
190-
try (var writer = new PrintWriter(resource.openOutputStream())) {
191-
writer.println(content);
192-
}
193-
} catch (IOException cause) {
194-
throw propagate(cause);
195-
}
196-
}
175+
protected void onGeneratedSource(String className, JavaFileObject source) {}
197176

198177
private Map<TypeElement, MvcRouter> buildRouteRegistry(
199178
Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
@@ -252,44 +231,40 @@ private Map<TypeElement, MvcRouter> buildRouteRegistry(
252231
*/
253232
private void buildRouteRegistry(Map<TypeElement, MvcRouter> registry, TypeElement currentType) {
254233
for (TypeElement superType : context.superTypes(currentType)) {
255-
if (processed.add(superType)) {
256-
// collect all declared methods
257-
superType.getEnclosedElements().stream()
258-
.filter(ExecutableElement.class::isInstance)
259-
.map(ExecutableElement.class::cast)
260-
.forEach(
261-
method -> {
262-
if (method.getModifiers().contains(Modifier.ABSTRACT)) {
263-
context.debug("ignoring abstract method: %s %s", superType, method);
264-
} else {
265-
method.getAnnotationMirrors().stream()
266-
.map(AnnotationMirror::getAnnotationType)
267-
.map(DeclaredType::asElement)
268-
.filter(TypeElement.class::isInstance)
269-
.map(TypeElement.class::cast)
270-
.filter(HttpMethod::hasAnnotation)
271-
.forEach(
272-
annotation -> {
273-
Stream.of(currentType, superType)
274-
.distinct()
275-
.forEach(
276-
routerClass ->
277-
registry
278-
.computeIfAbsent(
279-
routerClass, type -> new MvcRouter(context, type))
280-
.put(annotation, method));
281-
});
282-
}
283-
});
284-
} else {
285-
if (!currentType.equals(superType)) {
286-
// edge-case #1: when controller has no method and extends another class which has.
287-
// edge-case #2: some odd usage a controller could be empty.
288-
// See https://github.com/jooby-project/jooby/issues/3656
289-
if (registry.containsKey(superType)) {
290-
registry.computeIfAbsent(
291-
currentType, key -> new MvcRouter(key, registry.get(superType)));
292-
}
234+
// collect all declared methods
235+
superType.getEnclosedElements().stream()
236+
.filter(ExecutableElement.class::isInstance)
237+
.map(ExecutableElement.class::cast)
238+
.forEach(
239+
method -> {
240+
if (method.getModifiers().contains(Modifier.ABSTRACT)) {
241+
context.debug("ignoring abstract method: %s %s", superType, method);
242+
} else {
243+
method.getAnnotationMirrors().stream()
244+
.map(AnnotationMirror::getAnnotationType)
245+
.map(DeclaredType::asElement)
246+
.filter(TypeElement.class::isInstance)
247+
.map(TypeElement.class::cast)
248+
.filter(HttpMethod::hasAnnotation)
249+
.forEach(
250+
annotation -> {
251+
Stream.of(currentType, superType)
252+
.distinct()
253+
.forEach(
254+
routerClass ->
255+
registry
256+
.computeIfAbsent(
257+
routerClass, type -> new MvcRouter(context, type))
258+
.put(annotation, method));
259+
});
260+
}
261+
});
262+
if (!currentType.equals(superType)) {
263+
// edge-case #1: when controller has no method and extends another class which has.
264+
// edge-case #2: some odd usage a controller could be empty.
265+
// See https://github.com/jooby-project/jooby/issues/3656
266+
if (registry.containsKey(superType)) {
267+
registry.computeIfAbsent(currentType, key -> new MvcRouter(key, registry.get(superType)));
293268
}
294269
}
295270
}

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

Lines changed: 58 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,49 +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

93-
private final Object instance;
89+
private final List<Object> instances;
9490
private final HookJoobyProcessor processor;
9591

9692
public ProcessorRunner(Object instance) throws IOException {
9793
this(instance, Map.of());
9894
}
9995

96+
public ProcessorRunner(List<Object> instances) throws IOException {
97+
this(instances, System.out::println, Map.of());
98+
}
99+
100100
public ProcessorRunner(Object instance, Consumer<String> stdout) throws IOException {
101101
this(instance, stdout, Map.of());
102102
}
@@ -107,13 +107,19 @@ public ProcessorRunner(Object instance, Map<String, Object> options) throws IOEx
107107

108108
public ProcessorRunner(Object instance, Consumer<String> stdout, Map<String, Object> options)
109109
throws IOException {
110-
this.instance = instance;
111-
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);
112118
var optionsArray =
113119
options.entrySet().stream().map(e -> "-A" + e.getKey() + "=" + e.getValue()).toList();
114120
Truth.assert_()
115121
.about(JavaSourcesSubjectFactory.javaSources())
116-
.that(sources(sourceNames(instance.getClass())))
122+
.that(sources(sourceNames(instances.stream().map(Object::getClass).toList())))
117123
.withCompilerOptions(optionsArray.toArray(new String[0]))
118124
.processedWith(processor)
119125
.compilesWithoutError();
@@ -125,15 +131,31 @@ public ProcessorRunner withRouter(SneakyThrows.Consumer<Jooby> consumer) throws
125131

126132
public ProcessorRunner withRouter(SneakyThrows.Consumer2<Jooby, JavaFileObject> consumer)
127133
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 {
128145
var classLoader = processor.createClassLoader();
129-
var factoryName = classLoader.getClassName();
146+
var factoryName = routerType.getName() + "_";
130147
var factoryClass = (Class<? extends Extension>) classLoader.loadClass(factoryName);
131148
Extension extension;
132149
try {
133150
var constructor = factoryClass.getDeclaredConstructor();
134151
extension = constructor.newInstance();
135152
} catch (NoSuchMethodException x) {
136-
extension = factoryClass.getDeclaredConstructor(instance.getClass()).newInstance(instance);
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);
137159
}
138160

139161
var application = new Jooby();
@@ -154,17 +176,22 @@ public ProcessorRunner withSourceCode(SneakyThrows.Consumer<String> consumer) {
154176
public ProcessorRunner withSourceCode(boolean kt, SneakyThrows.Consumer<String> consumer) {
155177
consumer.accept(
156178
kt
157-
? processor.kotlinSource
179+
? processor.kotlinFiles.values().iterator().next()
158180
: Optional.ofNullable(processor.getSource()).map(Objects::toString).orElse(null));
159181
return this;
160182
}
161183

162-
private String[] sourceNames(Class input) {
184+
private String[] sourceNames(List<Class<? extends Object>> inputs) {
163185
List<String> result = new ArrayList<>();
164-
while (input != Object.class) {
165-
result.add(input.getName());
166-
input = input.getSuperclass();
167-
}
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+
});
168195
return result.toArray(new String[0]);
169196
}
170197

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)