Merge pull request #190 from jongyeol/feature/fix-doc-service-struct-…

…param

Fix #186 DocService Struct field docstrings are not reliably included

src/main/java/com/linecorp/armeria/server/docs/FieldInfo.java

@@ -40,7 +40,7 @@ static FieldInfo of(FieldMetaData fieldMetaData, @Nullable String namespace,
         final String docStringKey = ThriftDocString.key(namespace, fieldMetaData.fieldName);
         return new FieldInfo(fieldMetaData.fieldName,
                              RequirementType.of(fieldMetaData.requirementType),
-                             TypeInfo.of(fieldMetaData.valueMetaData, namespace, docStrings),
+                             TypeInfo.of(fieldMetaData.valueMetaData, docStrings),
                              docStrings.get(docStringKey));
     }
 

src/main/java/com/linecorp/armeria/server/docs/ListInfo.java

@@ -30,16 +30,16 @@
 class ListInfo extends TypeInfo implements CollectionInfo {
 
     static ListInfo of(ListMetaData listMetaData) {
-        return of(listMetaData, null, Collections.emptyMap());
+        return of(listMetaData, Collections.emptyMap());
     }
 
-    static ListInfo of(ListMetaData listMetaData, @Nullable String namespace, Map<String, String> docStrings) {
+    static ListInfo of(ListMetaData listMetaData, Map<String, String> docStrings) {
         requireNonNull(listMetaData, "listMetaData");
 
         assert listMetaData.type == TType.LIST;
         assert !listMetaData.isBinary();
 
-        return new ListInfo(of(listMetaData.elemMetaData, namespace, docStrings));
+        return new ListInfo(of(listMetaData.elemMetaData, docStrings));
     }
 
     static ListInfo of(TypeInfo elementType) {

src/main/java/com/linecorp/armeria/server/docs/MapInfo.java

@@ -32,17 +32,17 @@
 class MapInfo extends TypeInfo {
 
     static MapInfo of(MapMetaData mapMetaData) {
-        return of(mapMetaData, null, Collections.emptyMap());
+        return of(mapMetaData, Collections.emptyMap());
     }
 
-    static MapInfo of(MapMetaData mapMetaData, @Nullable String namespace, Map<String, String> docStrings) {
+    static MapInfo of(MapMetaData mapMetaData, Map<String, String> docStrings) {
         requireNonNull(mapMetaData, "mapMetaData");
 
         assert mapMetaData.type == TType.MAP;
         assert !mapMetaData.isBinary();
 
-        return new MapInfo(TypeInfo.of(mapMetaData.keyMetaData, namespace, docStrings),
-                           TypeInfo.of(mapMetaData.valueMetaData, namespace, docStrings));
+        return new MapInfo(TypeInfo.of(mapMetaData.keyMetaData, docStrings),
+                           TypeInfo.of(mapMetaData.valueMetaData, docStrings));
     }
 
     static MapInfo of(TypeInfo keyType, TypeInfo valueType) {

src/main/java/com/linecorp/armeria/server/docs/SetInfo.java

@@ -30,16 +30,16 @@
 class SetInfo extends TypeInfo implements CollectionInfo {
 
     static SetInfo of(SetMetaData setMetaData) {
-        return of(setMetaData, null, Collections.emptyMap());
+        return of(setMetaData, Collections.emptyMap());
     }
 
-    static SetInfo of(SetMetaData setMetaData, @Nullable String namespace, Map<String, String> docStrings) {
+    static SetInfo of(SetMetaData setMetaData, Map<String, String> docStrings) {
         requireNonNull(setMetaData, "setMetaData");
 
         assert setMetaData.type == TType.SET;
         assert !setMetaData.isBinary();
 
-        return new SetInfo(of(setMetaData.elemMetaData, namespace, docStrings));
+        return new SetInfo(of(setMetaData.elemMetaData, docStrings));
     }
 
     static SetInfo of(TypeInfo elementType) {

src/main/java/com/linecorp/armeria/server/docs/StructInfo.java

@@ -33,22 +33,22 @@
 class StructInfo extends TypeInfo implements ClassInfo {
 
     static StructInfo of(StructMetaData structMetaData) {
-        return of(structMetaData, null, Collections.emptyMap());
+        return of(structMetaData, Collections.emptyMap());
     }
 
-    static StructInfo of(StructMetaData structMetaData, @Nullable String namespace, Map<String, String> docStrings) {
+    static StructInfo of(StructMetaData structMetaData, Map<String, String> docStrings) {
         final Class<?> structClass = structMetaData.structClass;
+        final String name = structClass.getName();
 
         assert structMetaData.type == TType.STRUCT;
         assert !structMetaData.isBinary();
 
         final Map<?, FieldMetaData> metaDataMap =
                 FieldMetaData.getStructMetaDataMap(structMetaData.structClass);
         final List<FieldInfo> fields = metaDataMap.values().stream()
-                .map(fieldMetaData -> FieldInfo.of(fieldMetaData, namespace, docStrings))
+                .map(fieldMetaData -> FieldInfo.of(fieldMetaData, name, docStrings))
                 .collect(Collectors.toList());
 
-        final String name = structClass.getName();
         return new StructInfo(name, fields, docStrings.get(name));
     }
 

src/main/java/com/linecorp/armeria/server/docs/ThriftDocString.java

@@ -158,7 +158,7 @@ private static void traverseChildren(ImmutableMap.Builder<String, String> docStr
             if (name != null) {
                 childPrefix = (prefix != null ? prefix : "") + delimiter + name;
                 if (doc != null) {
-                    docStrings.put(childPrefix, doc);
+                    docStrings.put(childPrefix, doc.trim());
                 }
             } else {
                 childPrefix = prefix;

src/main/java/com/linecorp/armeria/server/docs/TypeInfo.java

@@ -21,8 +21,6 @@
 import java.util.Map;
 import java.util.Objects;
 
-import javax.annotation.Nullable;
-
 import org.apache.thrift.meta_data.EnumMetaData;
 import org.apache.thrift.meta_data.FieldValueMetaData;
 import org.apache.thrift.meta_data.ListMetaData;
@@ -35,26 +33,25 @@
 class TypeInfo {
     static final TypeInfo VOID = new TypeInfo(ValueType.VOID, false);
 
-    static TypeInfo of(FieldValueMetaData fieldValueMetaData, @Nullable String namespace,
-                       Map<String, String> docStrings) {
+    static TypeInfo of(FieldValueMetaData fieldValueMetaData, Map<String, String> docStrings) {
         if (fieldValueMetaData instanceof StructMetaData) {
-            return StructInfo.of((StructMetaData) fieldValueMetaData, namespace, docStrings);
+            return StructInfo.of((StructMetaData) fieldValueMetaData, docStrings);
         }
 
         if (fieldValueMetaData instanceof EnumMetaData) {
             return EnumInfo.of((EnumMetaData) fieldValueMetaData, docStrings);
         }
 
         if (fieldValueMetaData instanceof ListMetaData) {
-            return ListInfo.of((ListMetaData) fieldValueMetaData, namespace, docStrings);
+            return ListInfo.of((ListMetaData) fieldValueMetaData, docStrings);
         }
 
         if (fieldValueMetaData instanceof SetMetaData) {
-            return SetInfo.of((SetMetaData) fieldValueMetaData, namespace, docStrings);
+            return SetInfo.of((SetMetaData) fieldValueMetaData, docStrings);
         }
 
         if (fieldValueMetaData instanceof MapMetaData) {
-            return MapInfo.of((MapMetaData) fieldValueMetaData, namespace, docStrings);
+            return MapInfo.of((MapMetaData) fieldValueMetaData, docStrings);
         }
 
         return new TypeInfo(ValueType.of(fieldValueMetaData.type), fieldValueMetaData.isBinary());

src/test/java/com/linecorp/armeria/server/docs/ThriftDocStringTest.java

@@ -34,9 +34,9 @@ public void testThriftTestJson() {
         Map<String, String> docStrings = ThriftDocString.getDocStringsFromJsonResource(
                 getClass().getClassLoader(),
                 "META-INF/armeria/thrift/ThriftTest.json");
-        assertThat(docStrings.get("thrift.test.Numberz"), is("Docstring!\n"));
+        assertThat(docStrings.get("thrift.test.Numberz"), is("Docstring!"));
         assertThat(docStrings.get("thrift.test.ThriftTest#testVoid"),
-                   is("Prints \"testVoid()\" and returns nothing.\n"));
+                   is("Prints \"testVoid()\" and returns nothing."));
     }
 
     @Test
@@ -45,7 +45,7 @@ public void testCassandraJson() {
                 getClass().getClassLoader(),
                 "META-INF/armeria/thrift/cassandra.json");
         assertThat(docStrings.get("com.linecorp.armeria.service.test.thrift.cassandra.Compression"),
-                   is("CQL query compression\n"));
+                   is("CQL query compression"));
         assertThat(docStrings.get("com.linecorp.armeria.service.test.thrift.cassandra.CqlResultType"),
                    is(nullValue()));
     }

src/test/resources/META-INF/armeria/thrift/cassandra.json

@@ -119,46 +119,51 @@
   "structs": [
     {
       "name": "Column",
-      "doc": "Basic unit of data within a ColumnFamily.\n@param name, the name by which this column is set and retrieved.  Maximum 64KB long.\n@param value. The data associated with the name.  Maximum 2GB long, but in practice you should limit it to small numbers of MB (since Thrift must read the full value into memory to operate on it).\n@param timestamp. The timestamp is used for conflict detection\/resolution when two columns with same name need to be compared.\n@param ttl. An optional, positive delay (in seconds) after which the column will be automatically deleted.\n",
+      "doc": "Basic unit of data within a ColumnFamily.\n",
       "isException": false,
       "isUnion": false,
       "fields": [
         {
           "key": 1,
           "name": "name",
           "typeId": "binary",
+          "doc": "the name by which this column is set and retrieved.  Maximum 64KB long.\n\n",
           "required": "required"
         },
         {
           "key": 2,
           "name": "value",
           "typeId": "binary",
+          "doc": "The data associated with the name.  Maximum 2GB long, but in practice you should limit it to small numbers of MB (since Thrift must read the full value into memory to operate on it).\n\n",
           "required": "optional"
         },
         {
           "key": 3,
           "name": "timestamp",
           "typeId": "i64",
+          "doc": "The timestamp is used for conflict detection\/resolution when two columns with same name need to be compared.\n\n",
           "required": "optional"
         },
         {
           "key": 4,
           "name": "ttl",
           "typeId": "i32",
+          "doc": "An optional, positive delay (in seconds) after which the column will be automatically deleted.\n\n",
           "required": "optional"
         }
       ]
     },
     {
       "name": "SuperColumn",
-      "doc": "A named list of columns.\n@param name. see Column.name.\n@param columns. A collection of standard Columns.  The columns within a super column are defined in an adhoc manner.\n                Columns within a super column do not have to have matching structures (similarly named child columns).\n",
+      "doc": "A named list of columns.\n",
       "isException": false,
       "isUnion": false,
       "fields": [
         {
           "key": 1,
           "name": "name",
           "typeId": "binary",
+          "doc": "see Column.name.\n",
           "required": "required"
         },
         {
@@ -173,6 +178,7 @@
               "class": "Column"
             }
           },
+          "doc": "A collection of standard Columns.  The columns within a super column are defined in an adhoc manner.\nColumns within a super column do not have to have matching structures (similarly named child columns).\n",
           "required": "required"
         }
       ]
@@ -225,7 +231,7 @@
     },
     {
       "name": "ColumnOrSuperColumn",
-      "doc": "Methods for fetching rows\/records from Cassandra will return either a single instance of ColumnOrSuperColumn or a list\nof ColumnOrSuperColumns (get_slice()). If you're looking up a SuperColumn (or list of SuperColumns) then the resulting\ninstances of ColumnOrSuperColumn will have the requested SuperColumn in the attribute super_column. For queries resulting\nin Columns, those values will be in the attribute column. This change was made between 0.3 and 0.4 to standardize on\nsingle query methods that may return either a SuperColumn or Column.\n\nIf the query was on a counter column family, you will either get a counter_column (instead of a column) or a\ncounter_super_column (instead of a super_column)\n\n@param column. The Column returned by get() or get_slice().\n@param super_column. The SuperColumn returned by get() or get_slice().\n@param counter_column. The Counterolumn returned by get() or get_slice().\n@param counter_super_column. The CounterSuperColumn returned by get() or get_slice().\n",
+      "doc": "Methods for fetching rows\/records from Cassandra will return either a single instance of ColumnOrSuperColumn or a list\nof ColumnOrSuperColumns (get_slice()). If you're looking up a SuperColumn (or list of SuperColumns) then the resulting\ninstances of ColumnOrSuperColumn will have the requested SuperColumn in the attribute super_column. For queries resulting\nin Columns, those values will be in the attribute column. This change was made between 0.3 and 0.4 to standardize on\nsingle query methods that may return either a SuperColumn or Column.\n\nIf the query was on a counter column family, you will either get a counter_column (instead of a column) or a\ncounter_super_column (instead of a super_column)\n",
       "isException": false,
       "isUnion": false,
       "fields": [
@@ -237,6 +243,7 @@
             "typeId": "struct",
             "class": "Column"
           },
+          "doc": "The Column returned by get() or get_slice().\n\n",
           "required": "optional"
         },
         {
@@ -247,6 +254,7 @@
             "typeId": "struct",
             "class": "SuperColumn"
           },
+          "doc": "The SuperColumn returned by get() or get_slice().\n\n",
           "required": "optional"
         },
         {
@@ -257,6 +265,7 @@
             "typeId": "struct",
             "class": "CounterColumn"
           },
+          "doc": "The Counterolumn returned by get() or get_slice().\n\n",
           "required": "optional"
         },
         {
@@ -267,6 +276,7 @@
             "typeId": "struct",
             "class": "CounterSuperColumn"
           },
+          "doc": "The CounterSuperColumn returned by get() or get_slice().\n\n",
           "required": "optional"
         }
       ]
@@ -1144,7 +1154,7 @@
     {
       "name": "VERSION",
       "typeId": "string",
-      "value": "19.24.0"
+      "value": "19.24.0-ARMERIA"
     }
   ],
   "services": [
@@ -1223,7 +1233,7 @@
             "class": "ColumnOrSuperColumn"
           },
           "oneway": false,
-          "doc": "Get the Column or SuperColumn at the given column_path. If no value is present, NotFoundException is thrown. (This is\nthe only method that can throw an exception under non-failure conditions.)\n",
+          "doc": "Get the Column or SuperColumn at the given column_path. If no value is present, NotFoundException is thrown. (This is\nthe only method that can throw an exception under non-failure conditions.)\n\n@param key. key of column.\n",
           "arguments": [
             {
               "key": 1,
@@ -1545,7 +1555,7 @@
             "valueTypeId": "i32"
           },
           "oneway": false,
-          "doc": "Perform a get_count in parallel on the given list<binary> keys. The return value maps keys to the count found.\n@param keys. this is test doc string.",
+          "doc": "Perform a get_count in parallel on the given list<binary> keys. The return value maps keys to the count found.\n",
           "arguments": [
             {
               "key": 1,

src/test/thrift/cassandra.thrift

@@ -36,33 +36,45 @@ namespace java com.linecorp.armeria.service.test.thrift.cassandra
 #           for every edit that doesn't result in a change to major/minor.
 #
 # See the Semantic Versioning Specification (SemVer) http://semver.org.
-const string VERSION = "19.24.0"
+const string VERSION = "19.24.0-ARMERIA"
 
 
 #
 # data structures
 #
 
 /** Basic unit of data within a ColumnFamily.
- * @param name, the name by which this column is set and retrieved.  Maximum 64KB long.
- * @param value. The data associated with the name.  Maximum 2GB long, but in practice you should limit it to small numbers of MB (since Thrift must read the full value into memory to operate on it).
- * @param timestamp. The timestamp is used for conflict detection/resolution when two columns with same name need to be compared.
- * @param ttl. An optional, positive delay (in seconds) after which the column will be automatically deleted.
  */
 struct Column {
+   /**
+    * the name by which this column is set and retrieved.  Maximum 64KB long.
+    **/
    1: required binary name,
+   /**
+    * The data associated with the name.  Maximum 2GB long, but in practice you should limit it to small numbers of MB (since Thrift must read the full value into memory to operate on it).
+    **/
    2: optional binary value,
+   /**
+    * The timestamp is used for conflict detection/resolution when two columns with same name need to be compared.
+    **/
    3: optional i64 timestamp,
+   /**
+    * An optional, positive delay (in seconds) after which the column will be automatically deleted.
+    **/
    4: optional i32 ttl,
 }
 
 /** A named list of columns.
- * @param name. see Column.name.
- * @param columns. A collection of standard Columns.  The columns within a super column are defined in an adhoc manner.
- *                 Columns within a super column do not have to have matching structures (similarly named child columns).
  */
 struct SuperColumn {
+   /**
+    * see Column.name.
+    */
    1: required binary name,
+   /**
+    * A collection of standard Columns.  The columns within a super column are defined in an adhoc manner.
+    * Columns within a super column do not have to have matching structures (similarly named child columns).
+    */
    2: required list<Column> columns,
 }
 
@@ -85,16 +97,23 @@ struct CounterSuperColumn {
 
     If the query was on a counter column family, you will either get a counter_column (instead of a column) or a
     counter_super_column (instead of a super_column)
-
-    @param column. The Column returned by get() or get_slice().
-    @param super_column. The SuperColumn returned by get() or get_slice().
-    @param counter_column. The Counterolumn returned by get() or get_slice().
-    @param counter_super_column. The CounterSuperColumn returned by get() or get_slice().
  */
 struct ColumnOrSuperColumn {
+    /**
+     * The Column returned by get() or get_slice().
+     **/
     1: optional Column column,
+    /**
+     * The SuperColumn returned by get() or get_slice().
+     **/
     2: optional SuperColumn super_column,
+    /**
+     * The Counterolumn returned by get() or get_slice().
+     **/
     3: optional CounterColumn counter_column,
+    /**
+     * The CounterSuperColumn returned by get() or get_slice().
+     **/
     4: optional CounterSuperColumn counter_super_column
 }
 
@@ -464,6 +483,8 @@ service Cassandra {
   /**
     Get the Column or SuperColumn at the given column_path. If no value is present, NotFoundException is thrown. (This is
     the only method that can throw an exception under non-failure conditions.)
+
+    @param key. key of column.
    */
   ColumnOrSuperColumn get(1:required binary key,
                           2:required ColumnPath column_path,
@@ -697,4 +718,4 @@ service Cassandra {
             4:SchemaDisagreementException sde)
 
 
-}
+}