Skip to content

Commit fae3291

Browse files
jknackjknack
committed
API tool needs to ensure operationIds are unique across all routes fix jooby-project#1075
- redo default tag generator
1 parent 8b78874 commit fae3291

File tree

14 files changed

+194
-67
lines changed

14 files changed

+194
-67
lines changed

‎modules/jooby-apitool/src/main/java/org/jooby/apitool/RouteMethod.java‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@
209209
import org.jooby.Route;
210210

211211
import java.util.Collections;
212+
import java.util.LinkedHashMap;
212213
import java.util.List;
213214
import java.util.Map;
214215
import java.util.Objects;
@@ -454,7 +455,27 @@ public Map<String, Object> attributes() {
454455
* @return This method.
455456
*/
456457
public RouteMethod attributes(Map<String, Object> attributes) {
457-
this.attributes = attributes;
458+
if (attributes != null) {
459+
if (this.attributes == null) {
460+
this.attributes = new LinkedHashMap<>();
461+
}
462+
this.attributes.putAll(attributes);
463+
}
464+
return this;
465+
}
466+
467+
/**
468+
* Set route attribute.
469+
*
470+
* @param name Attribute name.
471+
* @param value Attribute value.
472+
* @return This method.
473+
*/
474+
public RouteMethod attribute(String name, Object value) {
475+
if (this.attributes == null) {
476+
this.attributes = new LinkedHashMap<>();
477+
}
478+
this.attributes.put(name, value);
458479
return this;
459480
}
460481

‎modules/jooby-apitool/src/main/java/org/jooby/apitool/RouteResponse.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,6 @@
211211
import java.util.Map;
212212
import java.util.Objects;
213213
import java.util.Optional;
214-
import java.util.stream.Stream;
215214

216215
/**
217216
* Represent a route response type.
@@ -270,9 +269,7 @@ public Optional<String> description() {
270269
} else {
271270
typename = type.getTypeName();
272271
}
273-
return Stream.of(typename.split("\\."))
274-
.filter(name -> name.length() > 0)
275-
.reduce((it, next) -> next);
272+
return Optional.of(typename);
276273
}
277274
return Optional.of(description);
278275
}

‎modules/jooby-apitool/src/main/java/org/jooby/internal/apitool/BytecodeRouteParser.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,9 @@ public List<RouteMethod> parse(String classname) throws Exception {
428428
}
429429
RouteMethod route = new RouteMethod(lambda.name, lambda.pattern,
430430
routeResponse).parameters(parameters);
431+
if (lambda.tag != null) {
432+
route.attribute("route.tag", lambda.tag);
433+
}
431434
javadoc(route, javadoc.pop(lambda.declaringClass, lambda.name, lambda.pattern));
432435
methods.add(route);
433436
} else {
@@ -565,7 +568,7 @@ private List<Lambda> pathOperator(ClassNode owner, List<MethodNode> methods,
565568
.filter(Lambda.class::isInstance)
566569
.map(Lambda.class::cast)
567570
.forEach(lambda -> {
568-
result.add(lambda.prefix(path));
571+
result.add(lambda.prefix(path).tag(path));
569572
});
570573
});
571574

@@ -805,7 +808,7 @@ private List<Lambda> kotlinPathOperator(ClassNode owner, List<MethodNode> method
805808
.stream()
806809
.filter(Lambda.class::isInstance)
807810
.map(Lambda.class::cast)
808-
.map(lambda -> lambda.prefix(path))
811+
.map(lambda -> lambda.prefix(path).tag(path))
809812
.forEach(result::add);
810813
});
811814
});

‎modules/jooby-apitool/src/main/java/org/jooby/internal/apitool/Lambda.java‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,6 @@
212212
import org.objectweb.asm.tree.InvokeDynamicInsnNode;
213213
import org.objectweb.asm.tree.MethodInsnNode;
214214
import org.objectweb.asm.tree.MethodNode;
215-
import org.slf4j.Logger;
216-
import org.slf4j.LoggerFactory;
217215

218216
import java.util.Arrays;
219217
import java.util.List;
@@ -232,20 +230,27 @@ class Lambda {
232230
public final String owner;
233231
public final String declaringClass;
234232
public final Optional<MethodNode> method;
233+
public final String tag;
235234

236235
private Lambda(String declaringClass, MethodInsnNode method, Handle h) {
237236
this(declaringClass, h.getOwner(), h.getDesc(), h.getName(), method.name, null, null);
238237
}
239238

240239
private Lambda(String declaringClass, String owner, String desc, String implementationName,
241240
String name, String pattern, MethodNode method) {
241+
this(declaringClass, owner, desc, implementationName, name, pattern, method, null);
242+
}
243+
244+
private Lambda(String declaringClass, String owner, String desc, String implementationName,
245+
String name, String pattern, MethodNode method, String tag) {
242246
this.declaringClass = declaringClass;
243247
this.owner = owner;
244248
this.desc = desc;
245249
this.implementationName = implementationName;
246250
this.name = name.equals("use") || name.equals("all") ? "*" : name;
247251
this.pattern = pattern;
248252
this.method = Optional.ofNullable(method);
253+
this.tag = tag;
249254
}
250255

251256
@Override public String toString() {
@@ -260,7 +265,16 @@ private Lambda(String declaringClass, String owner, String desc, String implemen
260265
*/
261266
public Lambda prefix(String prefix) {
262267
return new Lambda(declaringClass, owner, desc, implementationName, name,
263-
Route.normalize(prefix + "/" + pattern), method.orElse(null));
268+
Route.normalize(prefix + "/" + pattern), method.orElse(null), tag);
269+
}
270+
271+
public Lambda tag(String tag) {
272+
if (this.tag != null || tag.startsWith("/:") || tag.startsWith("/{")) {
273+
// NOOP, keep the most specific
274+
return this;
275+
}
276+
return new Lambda(declaringClass, owner, desc, implementationName, name, pattern,
277+
method.orElse(null), tag);
264278
}
265279

266280
/**
@@ -270,7 +284,7 @@ public Lambda prefix(String prefix) {
270284
* @return A new lambda.
271285
*/
272286
public Lambda method(MethodNode method) {
273-
return new Lambda(declaringClass, owner, desc, implementationName, name, pattern, method);
287+
return new Lambda(declaringClass, owner, desc, implementationName, name, pattern, method, tag);
274288
}
275289

276290
public static Stream<Lambda> create(String owner, Optional<String> prefix, MethodInsnNode method,
@@ -316,7 +330,7 @@ public static Stream<Lambda> create(ClassLoader loader,
316330
.orElse(0));
317331
int from = 0;
318332
// use(verb, pattern)
319-
if ("use" .equals(method.name) && count.get() == 2) {
333+
if ("use".equals(method.name) && count.get() == 2) {
320334
from += 1;
321335
}
322336

‎modules/jooby-apitool/src/main/java/org/jooby/internal/apitool/SwaggerBuilder.java‎

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,6 @@
207207
import com.fasterxml.jackson.databind.Module;
208208
import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
209209
import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
210-
import static com.google.common.base.CaseFormat.LOWER_CAMEL;
211-
import static com.google.common.base.CaseFormat.UPPER_CAMEL;
212-
import com.google.common.base.Splitter;
213210
import com.google.common.base.Strings;
214211
import com.google.common.collect.ImmutableList;
215212
import com.google.inject.TypeLiteral;
@@ -247,24 +244,29 @@
247244
import java.lang.annotation.Annotation;
248245
import java.lang.reflect.Type;
249246
import java.util.EnumMap;
247+
import java.util.HashMap;
250248
import java.util.Iterator;
251249
import java.util.List;
252250
import java.util.Map;
251+
import java.util.Objects;
253252
import java.util.Optional;
254253
import java.util.function.Function;
255-
import java.util.regex.Pattern;
256254
import java.util.stream.Collectors;
255+
import java.util.stream.Stream;
257256

258257
public class SwaggerBuilder {
259-
private static final Pattern TAG = Pattern.compile("(api)|/");
260-
261258
private static final Function<RouteMethod, String> TAG_PROVIDER = r -> {
262-
Iterator<String> segments = Splitter.on(TAG)
263-
.trimResults()
264-
.omitEmptyStrings()
265-
.split(r.pattern())
266-
.iterator();
267-
return segments.hasNext() ? segments.next() : r.pattern();
259+
Map<String, Object> attributes = r.attributes();
260+
if (attributes == null) {
261+
return r.pattern();
262+
}
263+
return Stream
264+
.of(attributes.get("tag"), attributes.get("swagger.tag"), attributes.get("route.tag"))
265+
.filter(Objects::nonNull)
266+
.findFirst()
267+
.map(Objects::toString)
268+
.map(path -> Stream.of(path.split("/")).reduce((head, tail) -> tail).orElse(r.pattern()))
269+
.orElse(r.pattern());
268270
};
269271
private Function<RouteMethod, String> tagger = TAG_PROVIDER;
270272

@@ -320,13 +322,7 @@ public Swagger build(Swagger base, final List<RouteMethod> routes) throws Except
320322
Function<String, Tag> tagFactory = value ->
321323
Optional.ofNullable(swagger.getTag(value))
322324
.orElseGet(() -> {
323-
Tag tag = new Tag();
324-
if (value.length() > 0) {
325-
tag.name(Character.toUpperCase(value.charAt(0)) + value.substring(1));
326-
swagger.addTag(tag);
327-
} else {
328-
tag.name(value);
329-
}
325+
Tag tag = new Tag().name(value);
330326
swagger.addTag(tag);
331327
return tag;
332328
});
@@ -352,6 +348,8 @@ public Swagger build(Swagger base, final List<RouteMethod> routes) throws Except
352348
return PropertyBuilder.toModel(PropertyBuilder.merge(property, args));
353349
};
354350

351+
Map<String, Integer> opIds = new HashMap<>();
352+
355353
for (RouteMethod route : routes) {
356354
/** Find or create tag: */
357355
Tag tag = tagFactory.apply(this.tagger.apply(route));
@@ -364,13 +362,15 @@ public Swagger build(Swagger base, final List<RouteMethod> routes) throws Except
364362
/** Operation: */
365363
Operation op = new Operation();
366364
op.addTag(tag.getName());
367-
op.operationId(route.name().orElseGet(
368-
() -> route.method().toLowerCase() + tag.getName() + route.parameters().stream()
369-
.filter(it -> route.method().equalsIgnoreCase("get")
370-
&& it.kind() == RouteParameter.Kind.PATH)
371-
.findFirst()
372-
.map(it -> "By" + LOWER_CAMEL.to(UPPER_CAMEL, it.name()))
373-
.orElse("")));
365+
op.operationId(route.name().orElseGet(() -> {
366+
String opId = route.method().toLowerCase() + ucase(tag.getName());
367+
int c = opIds.getOrDefault(opId, 0);
368+
opIds.put(opId, c + 1);
369+
if (c == 0) {
370+
return opId;
371+
}
372+
return opId + c;
373+
}));
374374

375375
/** Doc and summary: */
376376
route.description().ifPresent(description -> {
@@ -467,6 +467,10 @@ public Swagger build(Swagger base, final List<RouteMethod> routes) throws Except
467467
return swagger;
468468
}
469469

470+
private String ucase(String name) {
471+
return Character.toUpperCase(name.charAt(0)) + name.substring(1);
472+
}
473+
470474
/**
471475
* Mostly for kotlin null safe operator and immutable properties.
472476
*

‎modules/jooby-apitool/src/main/java/org/jooby/internal/apitool/TypeJsonDeserializer.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,6 @@ class TypeJsonDeserializer extends JsonDeserializer<Type> {
229229
return null;
230230
}
231231

232-
static List<Type> parse(final ClassLoader loader, final String type) {
233-
return parse(loader, type, 0);
234-
}
235-
236232
private static List<Type> parse(final ClassLoader loader, final String type, final int start) {
237233
List<Type> types = new ArrayList<>();
238234
StringBuilder singleType = new StringBuilder();

‎modules/jooby-apitool/src/test/java/issues/Issue1050.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public void mediaTypeOnControllers() throws IOException {
3131
+ " get:\n"
3232
+ " responses:\n"
3333
+ " 200:\n"
34-
+ " description: String\n"
34+
+ " description: java.lang.String\n"
3535
+ " body:\n"
3636
+ " application/json:\n"
3737
+ " type: string\n"
@@ -42,7 +42,7 @@ public void mediaTypeOnControllers() throws IOException {
4242
+ " get:\n"
4343
+ " responses:\n"
4444
+ " 200:\n"
45-
+ " description: String\n"
45+
+ " description: java.lang.String\n"
4646
+ " body:\n"
4747
+ " application/json:\n"
4848
+ " type: string\n"

‎modules/jooby-apitool/src/test/java/issues/Issue1070.java‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,21 @@ public void shouldContainsSwaggerResponseDescription() throws Exception {
4848
assertEquals("{\n"
4949
+ " \"swagger\" : \"2.0\",\n"
5050
+ " \"tags\" : [ {\n"
51-
+ " \"name\" : \"V2\",\n"
51+
+ " \"name\" : \"rates\",\n"
5252
+ " \"description\" : \"Top\\nSubTop1\"\n"
53+
+ " }, {\n"
54+
+ " \"name\" : \"currencies\",\n"
55+
+ " \"description\" : \"Top\\nSubTop2\"\n"
5356
+ " } ],\n"
5457
+ " \"consumes\" : [ \"application/json\" ],\n"
5558
+ " \"produces\" : [ \"application/json\" ],\n"
5659
+ " \"paths\" : {\n"
5760
+ " \"/v2/currencies/rates\" : {\n"
5861
+ " \"get\" : {\n"
59-
+ " \"tags\" : [ \"V2\" ],\n"
62+
+ " \"tags\" : [ \"rates\" ],\n"
6063
+ " \"summary\" : \"yadayada\",\n"
6164
+ " \"description\" : \"\",\n"
62-
+ " \"operationId\" : \"getV2\",\n"
65+
+ " \"operationId\" : \"getRates\",\n"
6366
+ " \"parameters\" : [ ],\n"
6467
+ " \"responses\" : {\n"
6568
+ " \"200\" : {\n"
@@ -71,10 +74,10 @@ public void shouldContainsSwaggerResponseDescription() throws Exception {
7174
+ " }\n"
7275
+ " },\n"
7376
+ " \"delete\" : {\n"
74-
+ " \"tags\" : [ \"V2\" ],\n"
77+
+ " \"tags\" : [ \"rates\" ],\n"
7578
+ " \"summary\" : \"yadayada2\",\n"
7679
+ " \"description\" : \"\",\n"
77-
+ " \"operationId\" : \"deleteV2\",\n"
80+
+ " \"operationId\" : \"deleteRates\",\n"
7881
+ " \"parameters\" : [ ],\n"
7982
+ " \"responses\" : {\n"
8083
+ " \"200\" : {\n"
@@ -88,10 +91,10 @@ public void shouldContainsSwaggerResponseDescription() throws Exception {
8891
+ " },\n"
8992
+ " \"/v2/currencies/{isoCode}\" : {\n"
9093
+ " \"get\" : {\n"
91-
+ " \"tags\" : [ \"V2\" ],\n"
94+
+ " \"tags\" : [ \"currencies\" ],\n"
9295
+ " \"summary\" : \"Gets the currency for a given ISO code\",\n"
9396
+ " \"description\" : \"\",\n"
94-
+ " \"operationId\" : \"getV2\",\n"
97+
+ " \"operationId\" : \"getCurrencies\",\n"
9598
+ " \"parameters\" : [ ],\n"
9699
+ " \"responses\" : {\n"
97100
+ " \"200\" : {\n"
@@ -113,33 +116,36 @@ public void shouldContainsSwaggerResponseDescription() throws Exception {
113116
assertEquals("{\n"
114117
+ " \"swagger\" : \"2.0\",\n"
115118
+ " \"tags\" : [ {\n"
116-
+ " \"name\" : \"V2\",\n"
119+
+ " \"name\" : \"rates\",\n"
117120
+ " \"description\" : \"Top\\nSubTop1\"\n"
121+
+ " }, {\n"
122+
+ " \"name\" : \"currencies\",\n"
123+
+ " \"description\" : \"Top\\nSubTop2\"\n"
118124
+ " } ],\n"
119125
+ " \"consumes\" : [ \"application/json\" ],\n"
120126
+ " \"produces\" : [ \"application/json\" ],\n"
121127
+ " \"paths\" : {\n"
122128
+ " \"/v2/currencies/rates\" : {\n"
123129
+ " \"get\" : {\n"
124-
+ " \"tags\" : [ \"V2\" ],\n"
130+
+ " \"tags\" : [ \"rates\" ],\n"
125131
+ " \"summary\" : \"yadayada\",\n"
126132
+ " \"description\" : \"\",\n"
127-
+ " \"operationId\" : \"getV2\",\n"
133+
+ " \"operationId\" : \"getRates\",\n"
128134
+ " \"parameters\" : [ ],\n"
129135
+ " \"responses\" : {\n"
130136
+ " \"200\" : {\n"
131-
+ " \"description\" : \"String\",\n"
137+
+ " \"description\" : \"java.lang.String\",\n"
132138
+ " \"schema\" : {\n"
133139
+ " \"type\" : \"string\"\n"
134140
+ " }\n"
135141
+ " }\n"
136142
+ " }\n"
137143
+ " },\n"
138144
+ " \"delete\" : {\n"
139-
+ " \"tags\" : [ \"V2\" ],\n"
145+
+ " \"tags\" : [ \"rates\" ],\n"
140146
+ " \"summary\" : \"yadayada2\",\n"
141147
+ " \"description\" : \"\",\n"
142-
+ " \"operationId\" : \"deleteV2\",\n"
148+
+ " \"operationId\" : \"deleteRates\",\n"
143149
+ " \"parameters\" : [ ],\n"
144150
+ " \"responses\" : {\n"
145151
+ " \"200\" : {\n"
@@ -153,10 +159,10 @@ public void shouldContainsSwaggerResponseDescription() throws Exception {
153159
+ " },\n"
154160
+ " \"/v2/currencies/{isoCode}\" : {\n"
155161
+ " \"get\" : {\n"
156-
+ " \"tags\" : [ \"V2\" ],\n"
162+
+ " \"tags\" : [ \"currencies\" ],\n"
157163
+ " \"summary\" : \"Gets the currency for a given ISO code\",\n"
158164
+ " \"description\" : \"\",\n"
159-
+ " \"operationId\" : \"getV2\",\n"
165+
+ " \"operationId\" : \"getCurrencies\",\n"
160166
+ " \"parameters\" : [ ],\n"
161167
+ " \"responses\" : {\n"
162168
+ " \"200\" : {\n"

0 commit comments

Comments
 (0)