Skip to content

Commit

Permalink
openapi: bug fixing and enhancements
Browse files Browse the repository at this point in the history 
- 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
  • Loading branch information
@jknack
jknack committed Jul 26, 2024
1 parent 0ae2010 commit ca7bd66
Show file tree
Hide file tree
Showing 7 changed files with 240 additions and 73 deletions.
5 changes: 4 additions & 1 deletion modules/jooby-openapi/src/main/java/io/jooby/internal/openapi/OperationExt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,10 @@ public Parameter getParameter(int i) {
}

public Optional<Parameter> getParameter(String name) {
return getParameters().stream().filter(p -> p.getName().equals(name)).findFirst();
if (getParameters() != null) {
return getParameters().stream().filter(p -> p.getName().equals(name)).findFirst();
}
return Optional.empty();
}

public ResponseExt addResponse(String code) {
Expand Down
10 changes: 9 additions & 1 deletion modules/jooby-openapi/src/main/java/io/jooby/internal/openapi/ParserContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,13 @@ public ClassNode classNode(Type type) {
return nodes.computeIfAbsent(type, this::newClassNode);
}

public MethodNode findMethodNode(Type type, String name) {
return nodes.computeIfAbsent(type, this::newClassNode).methods.stream()
.filter(it -> it.name.equals(name))
.findFirst()
.orElseThrow(() -> new IllegalArgumentException("Method not found: " + type + "." + name));
}

public ClassNode classNodeOrNull(Type type) {
try {
return nodes.computeIfAbsent(type, this::newClassNode);
Expand Down Expand Up @@ -413,7 +420,8 @@ public <T extends ClassVisitor> T createClassVisitor(Function<Integer, T> factor
}

public boolean isRouter(Type type) {
return asList(router, JOOBY, KOOBY, ROUTER, COROUTINE_ROUTER).contains(type);
return asList(router, JOOBY, KOOBY, ROUTER, COROUTINE_ROUTER).contains(type)
|| (router.getClassName() + "Kt").equals(type.getClassName());
}

public boolean process(AbstractInsnNode instruction) {
Expand Down
41 changes: 22 additions & 19 deletions modules/jooby-openapi/src/main/java/io/jooby/internal/openapi/RequestParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,7 @@
*/
package io.jooby.internal.openapi;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.Spliterator;
import java.util.Spliterators;
import java.util.*;
import java.util.function.BiConsumer;
import java.util.function.Predicate;
import java.util.stream.Collectors;
Expand All @@ -34,6 +24,7 @@

import io.jooby.FileUpload;
import io.jooby.MediaType;
import io.jooby.Router;
import io.swagger.v3.oas.models.media.Content;
import io.swagger.v3.oas.models.media.ObjectSchema;
import io.swagger.v3.oas.models.media.Schema;
Expand Down Expand Up @@ -191,7 +182,7 @@ private static void formField(
.ifPresent(schema -> consumer.accept(name, argument.set(schema)));
}

public static List<ParameterExt> parameters(MethodNode node) {
public static List<ParameterExt> parameters(MethodNode node, String path) {
List<MethodInsnNode> nodes =
StreamSupport.stream(
Spliterators.spliteratorUnknownSize(
Expand All @@ -200,9 +191,10 @@ public static List<ParameterExt> parameters(MethodNode node) {
.filter(MethodInsnNode.class::isInstance)
.map(MethodInsnNode.class::cast)
.filter(i -> i.owner.equals("io/jooby/Context"))
.collect(Collectors.toList());
.toList();
var pathParams = new LinkedHashSet<>(Router.pathKeys(path));
List<ParameterExt> args = new ArrayList<>();
for (MethodInsnNode methodInsnNode : nodes) {
for (var methodInsnNode : nodes) {
Signature signature = Signature.create(methodInsnNode);
ParameterExt argument = new ParameterExt();
String scope = signature.getMethod();
Expand All @@ -216,13 +208,13 @@ public static List<ParameterExt> parameters(MethodNode node) {
{
argument.setIn(scope);
if (signature.matches(String.class)) {
argument.setName(argumentName(methodInsnNode));
var name = argumentName(methodInsnNode);
pathParams.remove(name);
argument.setName(name);
argumentValue(argument.getName(), methodInsnNode).set(argument);
} else if (signature.matches(Class.class)) {
argument.setName(signature.getMethod());
contextObjectToType(argument, methodInsnNode);
} else {
// Unsupported path usage
}
}
}
Expand All @@ -231,10 +223,21 @@ public static List<ParameterExt> parameters(MethodNode node) {
}
if (argument.getJavaType() != null) {
args.add(argument);
} else {
// Unsupported parameter usage
}
}
if (!pathParams.isEmpty()) {
pathParams.stream()
.map(
name -> {
var it = new ParameterExt();
it.setName(name);
it.setRequired(true);
it.setIn("path");
it.setJavaType("java.lang.String");
return it;
})
.forEach(args::add);
}
return args;
}

Expand Down
139 changes: 88 additions & 51 deletions modules/jooby-openapi/src/main/java/io/jooby/internal/openapi/RouteParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,18 +333,21 @@ private List<OperationExt> routeHandler(ParserContext ctx, String prefix, Method
if (signature.matches("mvc")) {
handlerList.addAll(AnnotationParser.parse(ctx, prefix, signature, (MethodInsnNode) it));
} else if (signature.matches("<init>", KT_FUN_1)) {
handlerList.addAll(kotlinHandler(ctx, null, prefix, node));
handlerList.addAll(kotlinHandler(ctx, null, prefix, node, false));
} else if (signature.matches(KOOBY)
&& signature.getDescriptor() != null
&& Type.getReturnType(signature.getDescriptor()) == Type.VOID_TYPE) {
handlerList.addAll(kotlinHandler(ctx, null, prefix, node, true));
} else if (signature.matches("mount", Router.class)) {
handlerList.addAll(mountRouter(ctx, prefix, node, findRouterInstruction(node)));
handlerList.addAll(mountHandler(ctx, prefix, null, node));
} else if (signature.matches("install", String.class, SneakyThrows.Supplier.class)) {
String pattern = routePattern(node, node);
handlerList.addAll(installApp(ctx, path(prefix, pattern), node, node));
} else if (signature.matches("install", SneakyThrows.Supplier.class)) {
handlerList.addAll(installApp(ctx, prefix, node, node));
} else if (signature.matches("mount", String.class, Router.class)) {
AbstractInsnNode routerInstruction = findRouterInstruction(node);
String pattern = routePattern(node, node);
handlerList.addAll(mountRouter(ctx, path(prefix, pattern), node, routerInstruction));
handlerList.addAll(
mountHandler(ctx, prefix, routePatternNode(node, node).cst.toString(), node));
} else if (signature.matches("path", String.class, Runnable.class)
|| signature.matches("routes", Runnable.class)) {
boolean routes = signature.matches("routes", Runnable.class);
Expand All @@ -360,7 +363,7 @@ private List<OperationExt> routeHandler(ParserContext ctx, String prefix, Method
.orElseThrow(
() -> new IllegalStateException("Subroute definition not found"));
String path = routes ? "/" : routePattern(node, subrouteInsn);
handlerList.addAll(kotlinHandler(ctx, null, path(prefix, path), subrouteInsn));
handlerList.addAll(kotlinHandler(ctx, null, path(prefix, path), subrouteInsn, false));
} else {
InvokeDynamicInsnNode subrouteInsn =
InsnSupport.prev(node)
Expand All @@ -382,7 +385,7 @@ private List<OperationExt> routeHandler(ParserContext ctx, String prefix, Method
String path = routePattern(node, previous);
String httpMethod = signature.getMethod().toUpperCase();
if (node.owner.equals(TypeFactory.KOOBY.getInternalName())) {
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node));
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node, false));
} else {
if (previous instanceof InvokeDynamicInsnNode) {
MethodNode handler = findLambda(ctx, (InvokeDynamicInsnNode) previous);
Expand Down Expand Up @@ -432,15 +435,15 @@ private List<OperationExt> routeHandler(ParserContext ctx, String prefix, Method
instructionTo = node;
String path = routePattern(node, node.getPrevious());
String httpMethod = signature.getMethod().toUpperCase();
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node));
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node, false));
} else if (Router.METHODS.contains(signature.getMethod().toUpperCase())
&& signature.matches(STRING, TypeFactory.KT_FUN_2)) {
instructionTo = node;
String path = routePattern(node, node.getPrevious());
String httpMethod = signature.getMethod().toUpperCase();
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node));
handlerList.addAll(kotlinHandler(ctx, httpMethod, path(prefix, path), node, false));
} else if (signature.getMethod().startsWith("coroutine")) {
handlerList.addAll(kotlinHandler(ctx, null, prefix, node));
handlerList.addAll(kotlinHandler(ctx, null, prefix, node, false));
}
} else if (signature.matches(KOOBYKT, "runApp")) {
handlerList.addAll(
Expand Down Expand Up @@ -489,6 +492,32 @@ private List<OperationExt> routeHandler(ParserContext ctx, String prefix, Method
return handlerList;
}

private List<OperationExt> mountHandler(
ParserContext ctx, String prefix, String pattern, MethodInsnNode node) {
var ktAnonymousRouter = kotlinAnonymousRouter(node);
var path = pattern == null ? prefix : path(prefix, pattern);
if (ktAnonymousRouter != null) {
return routeHandler(
ctx,
path,
ctx.findMethodNode(
Type.getObjectType(ktAnonymousRouter.getOwner()), ktAnonymousRouter.getName()));
} else {
var routerInstruction = findRouterInstruction(node);
return mountRouter(ctx, path, node, routerInstruction);
}
}

private Handle kotlinAnonymousRouter(AbstractInsnNode node) {
return InsnSupport.prev(node)
.filter(InvokeDynamicInsnNode.class::isInstance)
.map(InvokeDynamicInsnNode.class::cast)
.filter(it -> it.name.equals("invoke") && Type.getReturnType(it.desc).equals(KT_FUN_1))
.map(it -> (Handle) it.bsmArgs[1])
.findFirst()
.orElse(null);
}

private AbstractInsnNode parseText(
AbstractInsnNode start, AbstractInsnNode end, Consumer<String> consumer) {
if (end != null) {
Expand Down Expand Up @@ -560,15 +589,14 @@ private static Function<AbstractInsnNode, Stream<String>> mediaType() {
.filter(LdcInsnNode.class::isInstance)
.findFirst()
.map(i -> ((LdcInsnNode) i).cst.toString())
.map(Stream::of)
.orElse(Stream.of());
.stream();
}
}
return Stream.of();
};
}

private AbstractInsnNode findRouterInstruction(MethodInsnNode node) {
private AbstractInsnNode findRouterInstruction(AbstractInsnNode node) {
return InsnSupport.prev(node)
.filter(
e -> {
Expand All @@ -580,10 +608,7 @@ private AbstractInsnNode findRouterInstruction(MethodInsnNode node) {
return false;
})
.findFirst()
.orElseThrow(
() ->
new IllegalStateException(
"Unsupported router type: " + InsnSupport.toString(node)));
.orElseThrow(() -> new IllegalStateException("Unsupported router type: " + node));
}

private List<OperationExt> mountRouter(
Expand Down Expand Up @@ -704,40 +729,48 @@ private List<MethodNode> findMethods(
}

private List<OperationExt> kotlinHandler(
ParserContext ctx, String httpMethod, String prefix, MethodInsnNode node) {
ParserContext ctx,
String httpMethod,
String prefix,
MethodInsnNode node,
boolean extensionMethod) {
List<OperationExt> handlerList = new ArrayList<>();
// [0] - Owner
// [1] - Method name. Optional
// [2] - Method descriptor. Optional
List<String> lookup =
InsnSupport.prev(node.getPrevious())
.map(
it -> {
if (it instanceof InvokeDynamicInsnNode) {
InvokeDynamicInsnNode invokeDynamic = (InvokeDynamicInsnNode) it;
Object[] args = invokeDynamic.bsmArgs;
if (args.length > 1 && args[1] instanceof Handle) {
Handle handle = (Handle) args[1];
return Arrays.asList(handle.getOwner(), handle.getName(), handle.getDesc());
List<String> lookup;
// Extension method must be defined in router but not in Kooby
if (extensionMethod) {
lookup = List.of(node.owner, node.name, node.desc);
} else {
// [0] - Owner
// [1] - Method name. Optional
// [2] - Method descriptor. Optional
lookup =
InsnSupport.prev(node.getPrevious())
.map(
it -> {
if (it instanceof InvokeDynamicInsnNode invokeDynamic) {
Object[] args = invokeDynamic.bsmArgs;
if (args.length > 1 && args[1] instanceof Handle handle) {
return Arrays.asList(handle.getOwner(), handle.getName(), handle.getDesc());
}
}
}
if (it instanceof FieldInsnNode) {
return Collections.singletonList(((FieldInsnNode) it).owner);
}
if (it instanceof MethodInsnNode) {
Signature signature = Signature.create((MethodInsnNode) it);
if (!signature.matches("<init>", KT_FUN_1)) {
return Collections.singletonList(((MethodInsnNode) it).owner);
if (it instanceof FieldInsnNode fieldInsnNode) {
return Collections.singletonList(fieldInsnNode.owner);
}
}
return null;
})
.filter(Objects::nonNull)
.findFirst()
.orElseThrow(
() ->
new IllegalStateException(
"Kotlin lambda not found: " + InsnSupport.toString(node)));
if (it instanceof MethodInsnNode methodInsnNode) {
Signature signature = Signature.create(methodInsnNode);
if (!signature.matches("<init>", KT_FUN_1)) {
return Collections.singletonList(((MethodInsnNode) it).owner);
}
}
return null;
})
.filter(Objects::nonNull)
.findFirst()
.orElseThrow(
() ->
new IllegalStateException(
"Kotlin lambda not found: " + InsnSupport.toString(node)));
}

ClassNode classNode = ctx.classNode(Type.getObjectType(lookup.get(0)));
MethodNode apply = null;
Expand Down Expand Up @@ -867,7 +900,7 @@ private MethodNode kotlinFunctionReference(
private OperationExt newRouteDescriptor(
ParserContext ctx, MethodNode node, String httpMethod, String prefix) {
Optional<RequestBodyExt> requestBody = RequestParser.requestBody(ctx, node);
List<ParameterExt> arguments = RequestParser.parameters(node);
List<ParameterExt> arguments = RequestParser.parameters(node, prefix);
ResponseExt response = new ResponseExt();
List<String> returnTypes = ReturnTypeParser.parse(ctx, node);
response.setJavaTypes(returnTypes);
Expand Down Expand Up @@ -898,17 +931,21 @@ private int returnTypePrecedence(MethodNode method) {
* @param node
* @return
*/
private String routePattern(MethodInsnNode methodInsnNode, AbstractInsnNode node) {
private LdcInsnNode routePatternNode(MethodInsnNode methodInsnNode, AbstractInsnNode node) {
return InsnSupport.prev(node)
.filter(it -> it instanceof LdcInsnNode && (((LdcInsnNode) it).cst instanceof String))
.findFirst()
.map(it -> ((LdcInsnNode) it).cst.toString())
.map(LdcInsnNode.class::cast)
.orElseThrow(
() ->
new IllegalStateException(
"Route pattern not found: " + InsnSupport.toString(methodInsnNode)));
}

private String routePattern(MethodInsnNode methodInsnNode, AbstractInsnNode node) {
return routePatternNode(methodInsnNode, node).cst.toString();
}

private MethodNode findRouteHandler(ParserContext ctx, MethodInsnNode node) {
Type owner = TypeFactory.fromInternalName(node.owner);
return ctx.classNode(owner).methods.stream()
Expand Down
8 changes: 7 additions & 1 deletion modules/jooby-openapi/src/test/java/io/jooby/openapi/OpenAPIGeneratorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,14 @@ public void routePatternIdioms(RouteIterator iterator) {
assertEquals("GET /variable", route.toString());
})
.next(
route -> {
(route, params) -> {
assertEquals("DELETE /variable/{id}", route.toString());
params.next(
param -> {
assertEquals("path", param.getIn());
assertEquals("id", param.getName());
assertEquals(String.class.getName(), param.getJavaType());
});
})
.next(
route -> {
Expand Down
Loading

0 comments on commit ca7bd66

Please sign in to comment.