Handle not null Kotlin properties and Java Optional / nullable Kotlin parameters (1.0.x) (#231)

* Handle not null Kotlin properties and Java Optional / nullable Kotlin parameters (#230)

* Detect required Java fields and not-null and thus required Kotlin properties

* Detect Optional and Kotlin nullables in parameters

* Fix comments

* Remove tests that depend on Java 8

Author: fbenz

spring-auto-restdocs-core/src/main/java/capital/scalable/restdocs/jackson/FieldDocumentationObjectVisitor.java

@@ -51,13 +51,25 @@ public FieldDocumentationObjectVisitor(SerializerProvider provider,
         this.typeFactory = typeFactory;
     }
 
+    /**
+     * Called for required bean properties like fields/properties annotated with @JsonProperty(required = true)
+     * or non-null Kotlin properties.
+     */
     @Override
     public void property(BeanProperty prop) throws JsonMappingException {
-        optionalProperty(prop);
+        property(prop, true);
     }
 
+    /**
+     * Called for all non-required bean properties like all not annotated Java fields
+     * or not annotated nullable Kotlin properties.
+     */
     @Override
     public void optionalProperty(BeanProperty prop) throws JsonMappingException {
+        property(prop, false);
+    }
+
+    public void property(BeanProperty prop, boolean required) throws JsonMappingException {
         String jsonName = prop.getName();
         String fieldName = prop.getMember().getName();
 
@@ -71,19 +83,19 @@ public void optionalProperty(BeanProperty prop) throws JsonMappingException {
                 return;
             }
 
-            visitType(prop, jsonName, fieldName, javaType, ser);
+            visitType(prop, jsonName, fieldName, javaType, ser, required);
         }
     }
 
     private void visitType(BeanProperty prop, String jsonName, String fieldName, JavaType fieldType,
-            JsonSerializer<?> ser) throws JsonMappingException {
+            JsonSerializer<?> ser, boolean required) throws JsonMappingException {
         String fieldPath = path + (path.isEmpty() ? "" : ".") + jsonName;
         log.debug("({}) {}", fieldPath, fieldType.getRawClass().getSimpleName());
         Class<?> javaBaseClass = prop.getMember().getDeclaringClass();
         boolean shouldExpand = shouldExpand(prop);
 
         InternalFieldInfo fieldInfo = new InternalFieldInfo(javaBaseClass, fieldName, fieldType,
-                fieldPath, shouldExpand);
+                fieldPath, shouldExpand, required);
 
         JsonFormatVisitorWrapper visitor = new FieldDocumentationVisitorWrapper(getProvider(),
                 context, fieldPath, fieldInfo, typeRegistry, typeFactory);

spring-auto-restdocs-core/src/main/java/capital/scalable/restdocs/jackson/FieldDocumentationVisitorContext.java

@@ -84,7 +84,7 @@ public void addField(InternalFieldInfo info, String jsonType) {
                 .description(comment);
 
         Attribute constraints = constraintAttribute(javaBaseClass, javaFieldName);
-        Attribute optionals = optionalAttribute(javaBaseClass, javaFieldName);
+        Attribute optionals = optionalAttribute(javaBaseClass, javaFieldName, info.isRequired());
         Attribute deprecated = deprecatedAttribute(javaBaseClass, javaFieldName);
         fieldDescriptor.attributes(constraints, optionals, deprecated);
 
@@ -110,19 +110,24 @@ private Attribute constraintAttribute(Class<?> javaBaseClass, String javaFieldNa
                 resolveConstraintDescriptions(javaBaseClass, javaFieldName));
     }
 
-    private Attribute optionalAttribute(Class<?> javaBaseClass, String javaFieldName) {
+    private Attribute optionalAttribute(Class<?> javaBaseClass, String javaFieldName, boolean requiredField) {
         return new Attribute(OPTIONAL_ATTRIBUTE,
-                resolveOptionalMessages(javaBaseClass, javaFieldName));
+                resolveOptionalMessages(javaBaseClass, javaFieldName, requiredField));
     }
 
     private Attribute deprecatedAttribute(Class<?> javaBaseClass, String javaFieldName) {
         return new Attribute(DEPRECATED_ATTRIBUTE,
                 resolveDeprecatedMessage(javaBaseClass, javaFieldName));
     }
 
-    private List<String> resolveOptionalMessages(Class<?> javaBaseClass, String javaFieldName) {
+    private List<String> resolveOptionalMessages(Class<?> javaBaseClass, String javaFieldName, boolean requiredField) {
         List<String> optionalMessages = new ArrayList<>();
 
+        if (requiredField) {
+            optionalMessages.add("false");
+            return optionalMessages;
+        }
+
         if (isPrimitive(javaBaseClass, javaFieldName)) {
             boolean failOnNullForPrimitives = deserializationConfig.hasDeserializationFeatures(
                     DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES.getMask());

spring-auto-restdocs-core/src/main/java/capital/scalable/restdocs/jackson/InternalFieldInfo.java

@@ -27,14 +27,20 @@ class InternalFieldInfo {
     private final JavaType javaFieldType;
     private final String jsonFieldPath;
     private final boolean shouldExpand;
+    /**
+     * True for required bean properties like fields annotated with @JsonProperty(required = true)
+     * or non-null properties in Kotlin.
+     */
+    private final boolean required;
 
     public InternalFieldInfo(Class<?> javaBaseClass, String javaFieldName, JavaType javaFieldType,
-            String jsonFieldPath, boolean shouldExpand) {
+            String jsonFieldPath, boolean shouldExpand, boolean required) {
         this.javaBaseClass = javaBaseClass;
         this.javaFieldName = javaFieldName;
         this.javaFieldType = javaFieldType;
         this.jsonFieldPath = jsonFieldPath;
         this.shouldExpand = shouldExpand;
+        this.required = required;
     }
 
     public Class<?> getJavaBaseClass() {
@@ -56,4 +62,8 @@ public String getJsonFieldPath() {
     public boolean shouldExpand() {
         return shouldExpand;
     }
+
+    public boolean isRequired() {
+        return required;
+    }
 }

spring-auto-restdocs-core/src/main/java/capital/scalable/restdocs/request/AbstractParameterSnippet.java

@@ -84,7 +84,7 @@ private void addFieldDescriptor(HandlerMethod handlerMethod,
         String pathName = getPath(annot);
 
         String parameterName = hasLength(pathName) ? pathName : javaParameterName;
-        String parameterTypeName = determineTypeName(param.getParameterType());
+        String parameterTypeName = determineTypeName(param.nestedIfOptional().getNestedParameterType());
         String description = javadocReader.resolveMethodParameterComment(
                 handlerMethod.getBeanType(), handlerMethod.getMethod().getName(),
                 javaParameterName);

spring-auto-restdocs-core/src/main/java/capital/scalable/restdocs/request/PathParametersSnippet.java

@@ -42,8 +42,10 @@ public PathParametersSnippet failOnUndocumentedParams(boolean failOnUndocumented
 
     @Override
     protected boolean isRequired(MethodParameter param, PathVariable annot) {
-        // Spring disallows null for primitive types
-        return param.getParameterType().isPrimitive() || annot.required();
+        // Spring disallows null for primitive types.
+        // For types wrapped in Optional or nullable Kotlin types, the required flag in
+        // the annotation is ignored by Spring.
+        return param.getParameterType().isPrimitive() || (!param.isOptional() && annot.required());
     }
 
     @Override

spring-auto-restdocs-core/src/main/java/capital/scalable/restdocs/request/RequestHeaderSnippet.java

@@ -43,7 +43,9 @@ public RequestHeaderSnippet failOnUndocumentedParams(boolean failOnUndocumentedP
     @Override
     protected boolean isRequired(MethodParameter param, RequestHeader annot) {
         // Spring disallows null for primitive types
-        return param.getParameterType().isPrimitive() || annot.required();
+        // For types wrapped in Optional or nullable Kotlin types, the required flag in
+        // the annotation is ignored by Spring.
+        return param.getParameterType().isPrimitive() || (!param.isOptional() && annot.required());
     }
 
     @Override

spring-auto-restdocs-core/src/main/java/capital/scalable/restdocs/request/RequestParametersSnippet.java

@@ -57,7 +57,9 @@ protected boolean isRequired(MethodParameter param, RequestParam annot) {
             // the required flag
             return true;
         } else {
-            return annot.required();
+            // For Types wrapped in Optional or nullable Kotlin types, the required flag in
+            // the annotation is ignored by Spring.
+            return !param.isOptional() && annot.required();
         }
     }
 

spring-auto-restdocs-core/src/test/java/capital/scalable/restdocs/jackson/FieldDocumentationGeneratorTest.java

@@ -458,6 +458,27 @@ public void testGenerateDocumentationWithTags() throws Exception {
                         "true")));
     }
 
+    @Test
+    public void testGenerateDocumentationForRequiredProperties() throws Exception {
+        // given
+        ObjectMapper mapper = createMapper();
+        mockFieldComment(RequiredProperties.class, "string", "A string");
+        mockFieldComment(RequiredProperties.class, "number", "An integer");
+
+        FieldDocumentationGenerator generator =
+                new FieldDocumentationGenerator(mapper.writer(), mapper.getDeserializationConfig(),
+                        javadocReader, constraintReader);
+        Type type = RequiredProperties.class;
+
+        // when
+        List<ExtendedFieldDescriptor> result = cast(generator
+                .generateDocumentation(type, mapper.getTypeFactory()));
+        // then
+        assertThat(result.size(), is(2));
+        assertThat(result.get(0), is(descriptor("string", "String", "A string", "false")));
+        assertThat(result.get(1), is(descriptor("number", "Integer", "An integer", "false")));
+    }
+
     private OngoingStubbing<List<String>> mockOptional(Class<?> javaBaseClass, String fieldName,
             String value) {
         return when(constraintReader.getOptionalMessages(javaBaseClass, fieldName))
@@ -695,4 +716,11 @@ private static class JsonType2SubType1 extends JsonType2 {
     private static class JsonType2SubType2 extends JsonType2 {
         private String base2Sub2;
     }
+
+    private static class RequiredProperties {
+        @JsonProperty(required = true)
+        private String string;
+        @JsonProperty(required = true)
+        private Integer number;
+    }
 }

spring-auto-restdocs-core/src/test/java/capital/scalable/restdocs/request/PathParametersSnippetTest.java

@@ -185,7 +185,7 @@ enum Shape {
             SPHERIC, SQUARE
         }
 
-        @PostMapping("/items/{id}/subitem/{subid}/{partId}/{yetAnotherId}")
+        @PostMapping("/items/{id}/subitem/{subid}/{partId}/{yetAnotherId}/{optionalId}")
         public void addItem(@PathVariable Integer id,
                 @PathVariable("subid") String otherId,
                 // partId is required anyway, because it's a primitive type

spring-auto-restdocs-core/src/test/java/capital/scalable/restdocs/request/RequestParametersSnippetTest.java

@@ -303,19 +303,22 @@ public enum Size {
             SMALL, LARGE
         }
 
-        public void searchItem(@RequestParam Integer type,
+        public void searchItem(
+                @RequestParam Integer type,
                 @RequestParam(value = "text", required = false) String description) {
             // NOOP
         }
 
-        public void searchItem2(@RequestParam double param1,    // required
+        public void searchItem2(
+                @RequestParam double param1,                    // required
                 @RequestParam(required = false) boolean param2, // required anyway
                 @RequestParam(defaultValue = "1") int param3) { // not required
             // NOOP
         }
 
-        public void searchItem2String(@RequestParam double param1,    // required
-                @RequestParam(required = false) boolean param2, // required anyway
+        public void searchItem2String(
+                @RequestParam double param1,                        // required
+                @RequestParam(required = false) boolean param2,     // required anyway
                 @RequestParam(defaultValue = "de") String param3) { // not required
             // NOOP
         }