spring-projects / spring-restdocs

Test-driven documentation for RESTful services
https://spring.io/projects/spring-restdocs
Apache License 2.0
1.16k stars 736 forks source link

Field beneath an array that is sometimes null cannot have a type other than varies #398

Closed ghost closed 7 years ago

ghost commented 7 years ago

In a response containing an array, certain fields could be sometimes null and sometimes not. Currently JsonFieldTypeResolver marks them as Varies. This forces me to document the fields as Varies. Which IMO doesnt add any value to the docs. This leads to following error.

org.springframework.restdocs.payload.FieldTypesDoNotMatchException: The documented type of the field '[].obj' is Array but the actual type is Varies

Would it wise to make an exception for null?

here is an example patch

diff --git a/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldTypeResolver.java b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldTypeResolver.java
index 25c373c..e618a4b 100644
--- a/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldTypeResolver.java
+++ b/spring-restdocs-core/src/main/java/org/springframework/restdocs/payload/JsonFieldTypeResolver.java
@@ -35,10 +35,10 @@ class JsonFieldTypeResolver {
                        JsonFieldType commonType = null;
                        for (Object item : (Collection<?>) field) {
                                JsonFieldType fieldType = determineFieldType(item);
-                               if (commonType == null) {
+                               if (commonType == null && fieldType !== JsonFieldType.NULL) {
                                        commonType = fieldType;
                                }
-                               else if (fieldType != commonType) {
+                               else if (fieldType != commonType && fieldType !== JsonFieldType.NULL) {
                                        return JsonFieldType.VARIES;
                                }
                        }

I could use @JsonInclude(JsonInclude.Include.NON_NULL) to prevent null fields in REST response but I'd like to avoid that and use null and have consistent REST response with all fields.

wilkinsona commented 7 years ago

365 appears to be related. Two questions:

ghost commented 7 years ago

1.

        <dependency>
            <groupId>org.springframework.restdocs</groupId>
            <artifactId>spring-restdocs-restassured</artifactId>
            <version>1.1.3.RELEASE</version>
            <scope>test</scope>
        </dependency>
  1. Yes

Example response below which causes problem

[
    {
        "obj": ["asdq2e31"],
    },
    {
        "obj": ["asd"],
    },
    {
        "obj": null
    }
]
wilkinsona commented 7 years ago

Thanks. Just to be sure that we're looking at exactly the same thing, could you please put together a test that reproduces the behaviour you've described? One of the existing tests in ResponseFieldsSnippetTests would probably provide a good base to work from.

wilkinsona commented 7 years ago

Thanks for reporting this, @nakulcg. The problem should now be fixed in the latest 1.2.x and 2.0 snapshots. By default, the type will still be resolved as JsonFieldType.VARIES, but you can now use FieldDescriptor.type(JsonFieldType) to override it as long as all of the fields are either null or are of the specified type.

wilkinsona commented 7 years ago

Please note that the problem with the field not having to be marked as optional still exists. That will be tackled in #402 and I hope to include that fix in 1.2.2 as well.

ghost commented 7 years ago

@wilkinsona thanks, I usually look up here https://mvnrepository.com/artifact/org.springframework.restdocs/spring-restdocs-core, is the latest build update there? Because the last update looks like may 12th

wilkinsona commented 7 years ago

1.2.2 hasn't be released yet. You can using 1.2.2.BUILD-SNAPSHOT from https://repo.spring.io/snapshot in the meantime.

wilkinsona commented 7 years ago

This doesn't work as intended when the array contains null and then non-null values.