Skip to content

Commit ca7bd66

Browse files
jknackjknack
committed
openapi: bug fixing and enhancements
- openapi: unused path parameter produces an error fix #3483 - openapi: kotlin: support kotlin anonymous object as router fix #3484 - openapi: kotlin: support extension method on kooby subclass fix #3485
1 parent 0ae2010 commit ca7bd66

File tree

7 files changed

+240
-73
lines changed

7 files changed

+240
-73
lines changed

‎modules/jooby-openapi/src/main/java/io/jooby/internal/openapi/OperationExt.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,10 @@ public Parameter getParameter(int i) {
135135
}
136136

137137
public Optional<Parameter> getParameter(String name) {
138-
return getParameters().stream().filter(p -> p.getName().equals(name)).findFirst();
138+
if (getParameters() != null) {
139+
return getParameters().stream().filter(p -> p.getName().equals(name)).findFirst();
140+
}
141+
return Optional.empty();
139142
}
140143

141144
public ResponseExt addResponse(String code) {

‎modules/jooby-openapi/src/main/java/io/jooby/internal/openapi/ParserContext.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,13 @@ public ClassNode classNode(Type type) {
352352
return nodes.computeIfAbsent(type, this::newClassNode);
353353
}
354354

355+
public MethodNode findMethodNode(Type type, String name) {
356+
return nodes.computeIfAbsent(type, this::newClassNode).methods.stream()
357+
.filter(it -> it.name.equals(name))
358+
.findFirst()
359+
.orElseThrow(() -> new IllegalArgumentException("Method not found: " + type + "." + name));
360+
}
361+
355362
public ClassNode classNodeOrNull(Type type) {
356363
try {
357364
return nodes.computeIfAbsent(type, this::newClassNode);
@@ -413,7 +420,8 @@ public <T extends ClassVisitor> T createClassVisitor(Function<Integer, T> factor
413420
}
414421

415422
public boolean isRouter(Type type) {
416-
return asList(router, JOOBY, KOOBY, ROUTER, COROUTINE_ROUTER).contains(type);
423+
return asList(router, JOOBY, KOOBY, ROUTER, COROUTINE_ROUTER).contains(type)
424+
|| (router.getClassName() + "Kt").equals(type.getClassName());
417425
}
418426

419427
public boolean process(AbstractInsnNode instruction) {

‎modules/jooby-openapi/src/main/java/io/jooby/internal/openapi/RequestParser.java‎

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,7 @@
55
*/
66
package io.jooby.internal.openapi;
77

8-
import java.util.ArrayList;
9-
import java.util.Arrays;
10-
import java.util.Collections;
11-
import java.util.LinkedHashMap;
12-
import java.util.List;
13-
import java.util.Map;
14-
import java.util.Objects;
15-
import java.util.Optional;
16-
import java.util.Set;
17-
import java.util.Spliterator;
18-
import java.util.Spliterators;
8+
import java.util.*;
199
import java.util.function.BiConsumer;
2010
import java.util.function.Predicate;
2111
import java.util.stream.Collectors;
@@ -34,6 +24,7 @@
3424

3525
import io.jooby.FileUpload;
3626
import io.jooby.MediaType;
27+
import io.jooby.Router;
3728
import io.swagger.v3.oas.models.media.Content;
3829
import io.swagger.v3.oas.models.media.ObjectSchema;
3930
import io.swagger.v3.oas.models.media.Schema;
@@ -191,7 +182,7 @@ private static void formField(
191182
.ifPresent(schema -> consumer.accept(name, argument.set(schema)));
192183
}
193184

194-
public static List<ParameterExt> parameters(MethodNode node) {
185+
public static List<ParameterExt> parameters(MethodNode node, String path) {
195186
List<MethodInsnNode> nodes =
196187
StreamSupport.stream(
197188
Spliterators.spliteratorUnknownSize(
@@ -200,9 +191,10 @@ public static List<ParameterExt> parameters(MethodNode node) {
200191
.filter(MethodInsnNode.class::isInstance)
201192
.map(MethodInsnNode.class::cast)
202193
.filter(i -> i.owner.equals("io/jooby/Context"))
203-
.collect(Collectors.toList());
194+
.toList();
195+
var pathParams = new LinkedHashSet<>(Router.pathKeys(path));
204196
List<ParameterExt> args = new ArrayList<>();
205-
for (MethodInsnNode methodInsnNode : nodes) {
197+
for (var methodInsnNode : nodes) {
206198
Signature signature = Signature.create(methodInsnNode);
207199
ParameterExt argument = new ParameterExt();
208200
String scope = signature.getMethod();
@@ -216,13 +208,13 @@ public static List<ParameterExt> parameters(MethodNode node) {
216208
{
217209
argument.setIn(scope);
218210
if (signature.matches(String.class)) {
219-
argument.setName(argumentName(methodInsnNode));
211+
var name = argumentName(methodInsnNode);
212+
pathParams.remove(name);
213+
argument.setName(name);
220214
argumentValue(argument.getName(), methodInsnNode).set(argument);
221215
} else if (signature.matches(Class.class)) {
222216
argument.setName(signature.getMethod());
223217
contextObjectToType(argument, methodInsnNode);
224-
} else {
225-
// Unsupported path usage
226218
}
227219
}
228220
}
@@ -231,10 +223,21 @@ public static List<ParameterExt> parameters(MethodNode node) {
231223
}
232224
if (argument.getJavaType() != null) {
233225
args.add(argument);
234-
} else {
235-
// Unsupported parameter usage
236226
}
237227
}
228+
if (!pathParams.isEmpty()) {
229+
pathParams.stream()
230+
.map(
231+
name -> {
232+
var it = new ParameterExt();
233+
it.setName(name);
234+
it.setRequired(true);
235+
it.setIn("path");
236+
it.setJavaType("java.lang.String");
237+
return it;
238+
})
239+
.forEach(args::add);
240+
}
238241
return args;
239242
}
240243

‎modules/jooby-openapi/src/main/java/io/jooby/internal/openapi/RouteParser.java‎

Lines changed: 88 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -333,18 +333,21 @@ private List<OperationExt> routeHandler(ParserContext ctx, String prefix, Method
333333
if (signature.matches("mvc")) {
334334
handlerList.addAll(AnnotationParser.parse(ctx, prefix, signature, (MethodInsnNode) it));
335335
} else if (signature.matches("<init>", KT_FUN_1)) {
336-
handlerList.addAll(kotlinHandler(ctx, null, prefix, node));
336+
handlerList.addAll(kotlinHandler(ctx, null, prefix, node, false));
337+
} else if (signature.matches(KOOBY)
338+
&& signature.getDescriptor() != null
339+
&& Type.getReturnType(signature.getDescriptor()) == Type.VOID_TYPE) {
340+
handlerList.addAll(kotlinHandler(ctx, null, prefix, node, true));
337341
} else if (signature.matches("mount", Router.class)) {
338-
handlerList.addAll(mountRouter(ctx, prefix, node, findRouterInstruction(node)));
342+
handlerList.addAll(mountHandler(ctx, prefix, null, node));
339343
} else if (signature.matches("install", String.class, SneakyThrows.Supplier.class)) {
340344
String pattern = routePattern(node, node);
341345
handlerList.addAll(installApp(ctx, path(prefix, pattern), node, node));
342346
} else if (signature.matches("install", SneakyThrows.Supplier.class)) {
343347
handlerList.addAll(installApp(ctx, prefix, node, node));
344348
} else if (signature.matches("mount", String.class, Router.class)) {
345-
AbstractInsnNode routerInstruction = findRouterInstruction(node);
346-
String pattern = routePattern(node, node);
347-
handlerList.addAll(mountRouter(ctx, path(prefix, pattern), node, routerInstruction));
349+
handlerList.addAll(
350+
mountHandler(ctx, prefix, routePatternNode(node, node).cst.toString(), node));
348351
} else if (signature.matches("path", String.class, Runnable.class)
349352
|| signature.matches("routes", Runnable.class)) {
350353
boolean routes = signature.matches("routes", Runnable.class);
@@ -360,7 +363,7 @@ private List<OperationExt> routeHandler(ParserContext ctx, String prefix, Method
360363
.orElseThrow(
361364
() -> new IllegalStateException("Subroute definition not found"));
362365
String path = routes ? "/" : routePattern(node, subrouteInsn);
363-
handlerList.addAll(kotlinHandler(ctx, null, path(prefix, path), subrouteInsn));
366+
handlerList.addAll(kotlinHandler(ctx, null, path(prefix, path), subrouteInsn, false));
364367
} else {
365368
InvokeDynamicInsnNode subrouteInsn =
366369
InsnSupport.prev(node)
@@ -382,7 +385,7 @@ private List<OperationExt> routeHandler(ParserContext ctx, String prefix, Method
382385
String path = routePattern(node, previous);
383386
String httpMethod = signature.getMethod().toUpperCase();
384387
if (node.owner.equals(TypeFactory.KOOBY.getInternalName())) {
385-
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node));
388+
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node, false));
386389
} else {
387390
if (previous instanceof InvokeDynamicInsnNode) {
388391
MethodNode handler = findLambda(ctx, (InvokeDynamicInsnNode) previous);
@@ -432,15 +435,15 @@ private List<OperationExt> routeHandler(ParserContext ctx, String prefix, Method
432435
instructionTo = node;
433436
String path = routePattern(node, node.getPrevious());
434437
String httpMethod = signature.getMethod().toUpperCase();
435-
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node));
438+
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node, false));
436439
} else if (Router.METHODS.contains(signature.getMethod().toUpperCase())
437440
&& signature.matches(STRING, TypeFactory.KT_FUN_2)) {
438441
instructionTo = node;
439442
String path = routePattern(node, node.getPrevious());
440443
String httpMethod = signature.getMethod().toUpperCase();
441-
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node));
444+
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node, false));
442445
} else if (signature.getMethod().startsWith("coroutine")) {
443-
handlerList.addAll(kotlinHandler(ctx, null, prefix, node));
446+
handlerList.addAll(kotlinHandler(ctx, null, prefix, node, false));
444447
}
445448
} else if (signature.matches(KOOBYKT, "runApp")) {
446449
handlerList.addAll(
@@ -489,6 +492,32 @@ private List<OperationExt> routeHandler(ParserContext ctx, String prefix, Method
489492
return handlerList;
490493
}
491494

495+
private List<OperationExt> mountHandler(
496+
ParserContext ctx, String prefix, String pattern, MethodInsnNode node) {
497+
var ktAnonymousRouter = kotlinAnonymousRouter(node);
498+
var path = pattern == null ? prefix : path(prefix, pattern);
499+
if (ktAnonymousRouter != null) {
500+
return routeHandler(
501+
ctx,
502+
path,
503+
ctx.findMethodNode(
504+
Type.getObjectType(ktAnonymousRouter.getOwner()), ktAnonymousRouter.getName()));
505+
} else {
506+
var routerInstruction = findRouterInstruction(node);
507+
return mountRouter(ctx, path, node, routerInstruction);
508+
}
509+
}
510+
511+
private Handle kotlinAnonymousRouter(AbstractInsnNode node) {
512+
return InsnSupport.prev(node)
513+
.filter(InvokeDynamicInsnNode.class::isInstance)
514+
.map(InvokeDynamicInsnNode.class::cast)
515+
.filter(it -> it.name.equals("invoke") && Type.getReturnType(it.desc).equals(KT_FUN_1))
516+
.map(it -> (Handle) it.bsmArgs[1])
517+
.findFirst()
518+
.orElse(null);
519+
}
520+
492521
private AbstractInsnNode parseText(
493522
AbstractInsnNode start, AbstractInsnNode end, Consumer<String> consumer) {
494523
if (end != null) {
@@ -560,15 +589,14 @@ private static Function<AbstractInsnNode, Stream<String>> mediaType() {
560589
.filter(LdcInsnNode.class::isInstance)
561590
.findFirst()
562591
.map(i -> ((LdcInsnNode) i).cst.toString())
563-
.map(Stream::of)
564-
.orElse(Stream.of());
592+
.stream();
565593
}
566594
}
567595
return Stream.of();
568596
};
569597
}
570598

571-
private AbstractInsnNode findRouterInstruction(MethodInsnNode node) {
599+
private AbstractInsnNode findRouterInstruction(AbstractInsnNode node) {
572600
return InsnSupport.prev(node)
573601
.filter(
574602
e -> {
@@ -580,10 +608,7 @@ private AbstractInsnNode findRouterInstruction(MethodInsnNode node) {
580608
return false;
581609
})
582610
.findFirst()
583-
.orElseThrow(
584-
() ->
585-
new IllegalStateException(
586-
"Unsupported router type: " + InsnSupport.toString(node)));
611+
.orElseThrow(() -> new IllegalStateException("Unsupported router type: " + node));
587612
}
588613

589614
private List<OperationExt> mountRouter(
@@ -704,40 +729,48 @@ private List<MethodNode> findMethods(
704729
}
705730

706731
private List<OperationExt> kotlinHandler(
707-
ParserContext ctx, String httpMethod, String prefix, MethodInsnNode node) {
732+
ParserContext ctx,
733+
String httpMethod,
734+
String prefix,
735+
MethodInsnNode node,
736+
boolean extensionMethod) {
708737
List<OperationExt> handlerList = new ArrayList<>();
709-
// [0] - Owner
710-
// [1] - Method name. Optional
711-
// [2] - Method descriptor. Optional
712-
List<String> lookup =
713-
InsnSupport.prev(node.getPrevious())
714-
.map(
715-
it -> {
716-
if (it instanceof InvokeDynamicInsnNode) {
717-
InvokeDynamicInsnNode invokeDynamic = (InvokeDynamicInsnNode) it;
718-
Object[] args = invokeDynamic.bsmArgs;
719-
if (args.length > 1 && args[1] instanceof Handle) {
720-
Handle handle = (Handle) args[1];
721-
return Arrays.asList(handle.getOwner(), handle.getName(), handle.getDesc());
738+
List<String> lookup;
739+
// Extension method must be defined in router but not in Kooby
740+
if (extensionMethod) {
741+
lookup = List.of(node.owner, node.name, node.desc);
742+
} else {
743+
// [0] - Owner
744+
// [1] - Method name. Optional
745+
// [2] - Method descriptor. Optional
746+
lookup =
747+
InsnSupport.prev(node.getPrevious())
748+
.map(
749+
it -> {
750+
if (it instanceof InvokeDynamicInsnNode invokeDynamic) {
751+
Object[] args = invokeDynamic.bsmArgs;
752+
if (args.length > 1 && args[1] instanceof Handle handle) {
753+
return Arrays.asList(handle.getOwner(), handle.getName(), handle.getDesc());
754+
}
722755
}
723-
}
724-
if (it instanceof FieldInsnNode) {
725-
return Collections.singletonList(((FieldInsnNode) it).owner);
726-
}
727-
if (it instanceof MethodInsnNode) {
728-
Signature signature = Signature.create((MethodInsnNode) it);
729-
if (!signature.matches("<init>", KT_FUN_1)) {
730-
return Collections.singletonList(((MethodInsnNode) it).owner);
756+
if (it instanceof FieldInsnNode fieldInsnNode) {
757+
return Collections.singletonList(fieldInsnNode.owner);
731758
}
732-
}
733-
return null;
734-
})
735-
.filter(Objects::nonNull)
736-
.findFirst()
737-
.orElseThrow(
738-
() ->
739-
new IllegalStateException(
740-
"Kotlin lambda not found: " + InsnSupport.toString(node)));
759+
if (it instanceof MethodInsnNode methodInsnNode) {
760+
Signature signature = Signature.create(methodInsnNode);
761+
if (!signature.matches("<init>", KT_FUN_1)) {
762+
return Collections.singletonList(((MethodInsnNode) it).owner);
763+
}
764+
}
765+
return null;
766+
})
767+
.filter(Objects::nonNull)
768+
.findFirst()
769+
.orElseThrow(
770+
() ->
771+
new IllegalStateException(
772+
"Kotlin lambda not found: " + InsnSupport.toString(node)));
773+
}
741774

742775
ClassNode classNode = ctx.classNode(Type.getObjectType(lookup.get(0)));
743776
MethodNode apply = null;
@@ -867,7 +900,7 @@ private MethodNode kotlinFunctionReference(
867900
private OperationExt newRouteDescriptor(
868901
ParserContext ctx, MethodNode node, String httpMethod, String prefix) {
869902
Optional<RequestBodyExt> requestBody = RequestParser.requestBody(ctx, node);
870-
List<ParameterExt> arguments = RequestParser.parameters(node);
903+
List<ParameterExt> arguments = RequestParser.parameters(node, prefix);
871904
ResponseExt response = new ResponseExt();
872905
List<String> returnTypes = ReturnTypeParser.parse(ctx, node);
873906
response.setJavaTypes(returnTypes);
@@ -898,17 +931,21 @@ private int returnTypePrecedence(MethodNode method) {
898931
* @param node
899932
* @return
900933
*/
901-
private String routePattern(MethodInsnNode methodInsnNode, AbstractInsnNode node) {
934+
private LdcInsnNode routePatternNode(MethodInsnNode methodInsnNode, AbstractInsnNode node) {
902935
return InsnSupport.prev(node)
903936
.filter(it -> it instanceof LdcInsnNode && (((LdcInsnNode) it).cst instanceof String))
904937
.findFirst()
905-
.map(it -> ((LdcInsnNode) it).cst.toString())
938+
.map(LdcInsnNode.class::cast)
906939
.orElseThrow(
907940
() ->
908941
new IllegalStateException(
909942
"Route pattern not found: " + InsnSupport.toString(methodInsnNode)));
910943
}
911944

945+
private String routePattern(MethodInsnNode methodInsnNode, AbstractInsnNode node) {
946+
return routePatternNode(methodInsnNode, node).cst.toString();
947+
}
948+
912949
private MethodNode findRouteHandler(ParserContext ctx, MethodInsnNode node) {
913950
Type owner = TypeFactory.fromInternalName(node.owner);
914951
return ctx.classNode(owner).methods.stream()

‎modules/jooby-openapi/src/test/java/io/jooby/openapi/OpenAPIGeneratorTest.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,14 @@ public void routePatternIdioms(RouteIterator iterator) {
6161
assertEquals("GET /variable", route.toString());
6262
})
6363
.next(
64-
route -> {
64+
(route, params) -> {
6565
assertEquals("DELETE /variable/{id}", route.toString());
66+
params.next(
67+
param -> {
68+
assertEquals("path", param.getIn());
69+
assertEquals("id", param.getName());
70+
assertEquals(String.class.getName(), param.getJavaType());
71+
});
6672
})
6773
.next(
6874
route -> {

0 commit comments

Comments
 (0)