API tool needs to ensure operationIds are unique across all routes fix …

…#1075

- redo default tag generator

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

@@ -209,6 +209,7 @@
 import org.jooby.Route;
 
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -454,7 +455,27 @@ public Map<String, Object> attributes() {
    * @return This method.
    */
   public RouteMethod attributes(Map<String, Object> attributes) {
-    this.attributes = attributes;
+    if (attributes != null) {
+      if (this.attributes == null) {
+        this.attributes = new LinkedHashMap<>();
+      }
+      this.attributes.putAll(attributes);
+    }
+    return this;
+  }
+
+  /**
+   * Set route attribute.
+   *
+   * @param name Attribute name.
+   * @param value Attribute value.
+   * @return This method.
+   */
+  public RouteMethod attribute(String name, Object value) {
+    if (this.attributes == null) {
+      this.attributes = new LinkedHashMap<>();
+    }
+    this.attributes.put(name, value);
     return this;
   }
 

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

@@ -211,7 +211,6 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.stream.Stream;
 
 /**
  * Represent a route response type.
@@ -270,9 +269,7 @@ public Optional<String> description() {
       } else {
         typename = type.getTypeName();
       }
-      return Stream.of(typename.split("\\."))
-          .filter(name -> name.length() > 0)
-          .reduce((it, next) -> next);
+      return Optional.of(typename);
     }
     return Optional.of(description);
   }

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

@@ -428,6 +428,9 @@ public List<RouteMethod> parse(String classname) throws Exception {
           }
           RouteMethod route = new RouteMethod(lambda.name, lambda.pattern,
               routeResponse).parameters(parameters);
+          if (lambda.tag != null) {
+            route.attribute("route.tag", lambda.tag);
+          }
           javadoc(route, javadoc.pop(lambda.declaringClass, lambda.name, lambda.pattern));
           methods.add(route);
         } else {
@@ -565,7 +568,7 @@ private List<Lambda> pathOperator(ClassNode owner, List<MethodNode> methods,
                                 .filter(Lambda.class::isInstance)
                                 .map(Lambda.class::cast)
                                 .forEach(lambda -> {
-                                  result.add(lambda.prefix(path));
+                                  result.add(lambda.prefix(path).tag(path));
                                 });
                           });
 
@@ -805,7 +808,7 @@ private List<Lambda> kotlinPathOperator(ClassNode owner, List<MethodNode> method
                                 .stream()
                                 .filter(Lambda.class::isInstance)
                                 .map(Lambda.class::cast)
-                                .map(lambda -> lambda.prefix(path))
+                                .map(lambda -> lambda.prefix(path).tag(path))
                                 .forEach(result::add);
                           });
                     });

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

@@ -212,8 +212,6 @@
 import org.objectweb.asm.tree.InvokeDynamicInsnNode;
 import org.objectweb.asm.tree.MethodInsnNode;
 import org.objectweb.asm.tree.MethodNode;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.util.Arrays;
 import java.util.List;
@@ -232,20 +230,27 @@ class Lambda {
   public final String owner;
   public final String declaringClass;
   public final Optional<MethodNode> method;
+  public final String tag;
 
   private Lambda(String declaringClass, MethodInsnNode method, Handle h) {
     this(declaringClass, h.getOwner(), h.getDesc(), h.getName(), method.name, null, null);
   }
 
   private Lambda(String declaringClass, String owner, String desc, String implementationName,
       String name, String pattern, MethodNode method) {
+    this(declaringClass, owner, desc, implementationName, name, pattern, method, null);
+  }
+
+  private Lambda(String declaringClass, String owner, String desc, String implementationName,
+      String name, String pattern, MethodNode method, String tag) {
     this.declaringClass = declaringClass;
     this.owner = owner;
     this.desc = desc;
     this.implementationName = implementationName;
     this.name = name.equals("use") || name.equals("all") ? "*" : name;
     this.pattern = pattern;
     this.method = Optional.ofNullable(method);
+    this.tag = tag;
   }
 
   @Override public String toString() {
@@ -260,7 +265,16 @@ private Lambda(String declaringClass, String owner, String desc, String implemen
    */
   public Lambda prefix(String prefix) {
     return new Lambda(declaringClass, owner, desc, implementationName, name,
-        Route.normalize(prefix + "/" + pattern), method.orElse(null));
+        Route.normalize(prefix + "/" + pattern), method.orElse(null), tag);
+  }
+
+  public Lambda tag(String tag) {
+    if (this.tag != null || tag.startsWith("/:") || tag.startsWith("/{")) {
+      // NOOP, keep the most specific
+      return this;
+    }
+    return new Lambda(declaringClass, owner, desc, implementationName, name, pattern,
+        method.orElse(null), tag);
   }
 
   /**
@@ -270,7 +284,7 @@ public Lambda prefix(String prefix) {
    * @return A new lambda.
    */
   public Lambda method(MethodNode method) {
-    return new Lambda(declaringClass, owner, desc, implementationName, name, pattern, method);
+    return new Lambda(declaringClass, owner, desc, implementationName, name, pattern, method, tag);
   }
 
   public static Stream<Lambda> create(String owner, Optional<String> prefix, MethodInsnNode method,
@@ -316,7 +330,7 @@ public static Stream<Lambda> create(ClassLoader loader,
                     .orElse(0));
                 int from = 0;
                 // use(verb, pattern)
-                if ("use" .equals(method.name) && count.get() == 2) {
+                if ("use".equals(method.name) && count.get() == 2) {
                   from += 1;
                 }
 

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

@@ -207,9 +207,6 @@
 import com.fasterxml.jackson.databind.Module;
 import com.fasterxml.jackson.databind.introspect.BeanPropertyDefinition;
 import com.fasterxml.jackson.datatype.jdk8.Jdk8Module;
-import static com.google.common.base.CaseFormat.LOWER_CAMEL;
-import static com.google.common.base.CaseFormat.UPPER_CAMEL;
-import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.inject.TypeLiteral;
@@ -247,24 +244,29 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Type;
 import java.util.EnumMap;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Function;
-import java.util.regex.Pattern;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 public class SwaggerBuilder {
-  private static final Pattern TAG = Pattern.compile("(api)|/");
-
   private static final Function<RouteMethod, String> TAG_PROVIDER = r -> {
-    Iterator<String> segments = Splitter.on(TAG)
-        .trimResults()
-        .omitEmptyStrings()
-        .split(r.pattern())
-        .iterator();
-    return segments.hasNext() ? segments.next() : r.pattern();
+    Map<String, Object> attributes = r.attributes();
+    if (attributes == null) {
+      return r.pattern();
+    }
+    return Stream
+        .of(attributes.get("tag"), attributes.get("swagger.tag"), attributes.get("route.tag"))
+        .filter(Objects::nonNull)
+        .findFirst()
+        .map(Objects::toString)
+        .map(path -> Stream.of(path.split("/")).reduce((head, tail) -> tail).orElse(r.pattern()))
+        .orElse(r.pattern());
   };
   private Function<RouteMethod, String> tagger = TAG_PROVIDER;
 
@@ -320,13 +322,7 @@ public Swagger build(Swagger base, final List<RouteMethod> routes) throws Except
     Function<String, Tag> tagFactory = value ->
         Optional.ofNullable(swagger.getTag(value))
             .orElseGet(() -> {
-              Tag tag = new Tag();
-              if (value.length() > 0) {
-                tag.name(Character.toUpperCase(value.charAt(0)) + value.substring(1));
-                swagger.addTag(tag);
-              } else {
-                tag.name(value);
-              }
+              Tag tag = new Tag().name(value);
               swagger.addTag(tag);
               return tag;
             });
@@ -352,6 +348,8 @@ public Swagger build(Swagger base, final List<RouteMethod> routes) throws Except
       return PropertyBuilder.toModel(PropertyBuilder.merge(property, args));
     };
 
+    Map<String, Integer> opIds = new HashMap<>();
+
     for (RouteMethod route : routes) {
       /** Find or create tag: */
       Tag tag = tagFactory.apply(this.tagger.apply(route));
@@ -364,13 +362,15 @@ public Swagger build(Swagger base, final List<RouteMethod> routes) throws Except
       /** Operation: */
       Operation op = new Operation();
       op.addTag(tag.getName());
-      op.operationId(route.name().orElseGet(
-          () -> route.method().toLowerCase() + tag.getName() + route.parameters().stream()
-              .filter(it -> route.method().equalsIgnoreCase("get")
-                  && it.kind() == RouteParameter.Kind.PATH)
-              .findFirst()
-              .map(it -> "By" + LOWER_CAMEL.to(UPPER_CAMEL, it.name()))
-              .orElse("")));
+      op.operationId(route.name().orElseGet(() -> {
+        String opId = route.method().toLowerCase() + ucase(tag.getName());
+        int c = opIds.getOrDefault(opId, 0);
+        opIds.put(opId, c + 1);
+        if (c == 0) {
+          return opId;
+        }
+        return opId + c;
+      }));
 
       /** Doc and summary: */
       route.description().ifPresent(description -> {
@@ -467,6 +467,10 @@ public Swagger build(Swagger base, final List<RouteMethod> routes) throws Except
     return swagger;
   }
 
+  private String ucase(String name) {
+    return Character.toUpperCase(name.charAt(0)) + name.substring(1);
+  }
+
   /**
    * Mostly for kotlin null safe operator and immutable properties.
    *

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

@@ -229,10 +229,6 @@ class TypeJsonDeserializer extends JsonDeserializer<Type> {
     return null;
   }
 
-  static List<Type> parse(final ClassLoader loader, final String type) {
-    return parse(loader, type, 0);
-  }
-
   private static List<Type> parse(final ClassLoader loader, final String type, final int start) {
     List<Type> types = new ArrayList<>();
     StringBuilder singleType = new StringBuilder();

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

@@ -31,7 +31,7 @@ public void mediaTypeOnControllers() throws IOException {
         + "  get:\n"
         + "    responses:\n"
         + "      200:\n"
-        + "        description: String\n"
+        + "        description: java.lang.String\n"
         + "        body:\n"
         + "          application/json:\n"
         + "            type: string\n"
@@ -42,7 +42,7 @@ public void mediaTypeOnControllers() throws IOException {
         + "    get:\n"
         + "      responses:\n"
         + "        200:\n"
-        + "          description: String\n"
+        + "          description: java.lang.String\n"
         + "          body:\n"
         + "            application/json:\n"
         + "              type: string\n"

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

@@ -48,18 +48,21 @@ public void shouldContainsSwaggerResponseDescription() throws Exception {
     assertEquals("{\n"
             + "  \"swagger\" : \"2.0\",\n"
             + "  \"tags\" : [ {\n"
-            + "    \"name\" : \"V2\",\n"
+            + "    \"name\" : \"rates\",\n"
             + "    \"description\" : \"Top\\nSubTop1\"\n"
+            + "  }, {\n"
+            + "    \"name\" : \"currencies\",\n"
+            + "    \"description\" : \"Top\\nSubTop2\"\n"
             + "  } ],\n"
             + "  \"consumes\" : [ \"application/json\" ],\n"
             + "  \"produces\" : [ \"application/json\" ],\n"
             + "  \"paths\" : {\n"
             + "    \"/v2/currencies/rates\" : {\n"
             + "      \"get\" : {\n"
-            + "        \"tags\" : [ \"V2\" ],\n"
+            + "        \"tags\" : [ \"rates\" ],\n"
             + "        \"summary\" : \"yadayada\",\n"
             + "        \"description\" : \"\",\n"
-            + "        \"operationId\" : \"getV2\",\n"
+            + "        \"operationId\" : \"getRates\",\n"
             + "        \"parameters\" : [ ],\n"
             + "        \"responses\" : {\n"
             + "          \"200\" : {\n"
@@ -71,10 +74,10 @@ public void shouldContainsSwaggerResponseDescription() throws Exception {
             + "        }\n"
             + "      },\n"
             + "      \"delete\" : {\n"
-            + "        \"tags\" : [ \"V2\" ],\n"
+            + "        \"tags\" : [ \"rates\" ],\n"
             + "        \"summary\" : \"yadayada2\",\n"
             + "        \"description\" : \"\",\n"
-            + "        \"operationId\" : \"deleteV2\",\n"
+            + "        \"operationId\" : \"deleteRates\",\n"
             + "        \"parameters\" : [ ],\n"
             + "        \"responses\" : {\n"
             + "          \"200\" : {\n"
@@ -88,10 +91,10 @@ public void shouldContainsSwaggerResponseDescription() throws Exception {
             + "    },\n"
             + "    \"/v2/currencies/{isoCode}\" : {\n"
             + "      \"get\" : {\n"
-            + "        \"tags\" : [ \"V2\" ],\n"
+            + "        \"tags\" : [ \"currencies\" ],\n"
             + "        \"summary\" : \"Gets the currency for a given ISO code\",\n"
             + "        \"description\" : \"\",\n"
-            + "        \"operationId\" : \"getV2\",\n"
+            + "        \"operationId\" : \"getCurrencies\",\n"
             + "        \"parameters\" : [ ],\n"
             + "        \"responses\" : {\n"
             + "          \"200\" : {\n"
@@ -113,33 +116,36 @@ public void shouldContainsSwaggerResponseDescription() throws Exception {
     assertEquals("{\n"
             + "  \"swagger\" : \"2.0\",\n"
             + "  \"tags\" : [ {\n"
-            + "    \"name\" : \"V2\",\n"
+            + "    \"name\" : \"rates\",\n"
             + "    \"description\" : \"Top\\nSubTop1\"\n"
+            + "  }, {\n"
+            + "    \"name\" : \"currencies\",\n"
+            + "    \"description\" : \"Top\\nSubTop2\"\n"
             + "  } ],\n"
             + "  \"consumes\" : [ \"application/json\" ],\n"
             + "  \"produces\" : [ \"application/json\" ],\n"
             + "  \"paths\" : {\n"
             + "    \"/v2/currencies/rates\" : {\n"
             + "      \"get\" : {\n"
-            + "        \"tags\" : [ \"V2\" ],\n"
+            + "        \"tags\" : [ \"rates\" ],\n"
             + "        \"summary\" : \"yadayada\",\n"
             + "        \"description\" : \"\",\n"
-            + "        \"operationId\" : \"getV2\",\n"
+            + "        \"operationId\" : \"getRates\",\n"
             + "        \"parameters\" : [ ],\n"
             + "        \"responses\" : {\n"
             + "          \"200\" : {\n"
-            + "            \"description\" : \"String\",\n"
+            + "            \"description\" : \"java.lang.String\",\n"
             + "            \"schema\" : {\n"
             + "              \"type\" : \"string\"\n"
             + "            }\n"
             + "          }\n"
             + "        }\n"
             + "      },\n"
             + "      \"delete\" : {\n"
-            + "        \"tags\" : [ \"V2\" ],\n"
+            + "        \"tags\" : [ \"rates\" ],\n"
             + "        \"summary\" : \"yadayada2\",\n"
             + "        \"description\" : \"\",\n"
-            + "        \"operationId\" : \"deleteV2\",\n"
+            + "        \"operationId\" : \"deleteRates\",\n"
             + "        \"parameters\" : [ ],\n"
             + "        \"responses\" : {\n"
             + "          \"200\" : {\n"
@@ -153,10 +159,10 @@ public void shouldContainsSwaggerResponseDescription() throws Exception {
             + "    },\n"
             + "    \"/v2/currencies/{isoCode}\" : {\n"
             + "      \"get\" : {\n"
-            + "        \"tags\" : [ \"V2\" ],\n"
+            + "        \"tags\" : [ \"currencies\" ],\n"
             + "        \"summary\" : \"Gets the currency for a given ISO code\",\n"
             + "        \"description\" : \"\",\n"
-            + "        \"operationId\" : \"getV2\",\n"
+            + "        \"operationId\" : \"getCurrencies\",\n"
             + "        \"parameters\" : [ ],\n"
             + "        \"responses\" : {\n"
             + "          \"200\" : {\n"