Skip to content

responseFields beneathPath does not work with arrays #473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bsb319 opened this issue Jan 12, 2018 · 22 comments
Closed

responseFields beneathPath does not work with arrays #473

bsb319 opened this issue Jan 12, 2018 · 22 comments
Labels
Milestone

Comments

@bsb319
Copy link

bsb319 commented Jan 12, 2018

Using:

org.springframework.restdocs:spring-restdocs-core:2.0.0.RELEASE
org.springframework.restdocs:spring-restdocs-mockmvc:2.0.0.RELEASE

Error:

org.springframework.restdocs.snippet.SnippetException: The following parts of the payload were not documented:
[ {
  "details" : "test details",
  "name" : "testName"
} ]
Fields with the following paths were not found in the payload: [details, name]

Test Code that causes error:

List<FieldDescriptor> responseFieldDescriptors = new ArrayList<>();
responseFieldDescriptors.add(fieldWithPath("details").type(JsonFieldType.STRING).description("Details to display to the customer."));
responseFieldDescriptors.add(fieldWithPath("name").type(JsonFieldType.STRING).description("Customer's name."));


resultActions.andDo(
        MockMvcRestDocumentation.document("content-example",
                preprocessRequest(prettyPrint()),
                preprocessResponse(prettyPrint()),
                responseFields(beneathPath("information.customers[]").withSubsectionId("customer"), responseFieldDescriptors)
        )
);

Test Code that runs but displays unwanted "[]." in generated table

List<FieldDescriptor> responseFieldDescriptors = new ArrayList<>();
responseFieldDescriptors.add(fieldWithPath("[].details").type(JsonFieldType.STRING).description("Details to display to the customer."));
responseFieldDescriptors.add(fieldWithPath("[].name").type(JsonFieldType.STRING).description("Customer's name."));


resultActions.andDo(
        MockMvcRestDocumentation.document("content-example",
                preprocessRequest(prettyPrint()),
                preprocessResponse(prettyPrint()),
                responseFields(beneathPath("information.customers").withSubsectionId("customer"), responseFieldDescriptors)
        )
);

Sample Response:

{
    "information": {
        "customers": [
            "details": "test details",
            "name": "testName"
        ]
    }
}
@wilkinsona
Copy link
Member

wilkinsona commented Jan 13, 2018

Thanks for the detailed reported.

The sample response that you have shown contains two fields:

  • information.customers.[].details
  • information.customers.[].name

Combining the beneath path and the field path, you are using the following to document the fields:

  • information.customers.details
  • information.customers.name

Neither field exists so REST Docs is correctly failing as two fields that exist have not been documented and an attempt has been made to document two fields that do not exist.

I guess what you’d like to be able to do is to use information.customers.[] as the beneath path? It may not work right now as I think it may be interpreted as identifying a non-unique part of the payload. Have you tried it?

@bsb319
Copy link
Author

bsb319 commented Jan 15, 2018

Thanks for the response. I had not tried information.customers.[]. However, I have now and get the same error as beneathPath("information.customers[]"). If it combines beneathPath with fieldWithPath I would think the outcome would be a path of information.customers[].details and information.customers[].name.

@wilkinsona
Copy link
Member

wilkinsona commented Jan 15, 2018

@bsb319 I see you've edited your original description. When you did so, did you change information.customers to information.customers[]? My response above was written based on the beneath path being information.customers which is what I thought you had originally written.

information.customers[] and information.customers.[] are equivalent so I am not surprised that using the latter did not make any difference if you were, in fact, originally using the former.

@bsb319
Copy link
Author

bsb319 commented Jan 16, 2018

@wilkinsona Aw, yes sorry I did edit the initial post to add more details. I also have a typo in the sample response above as "customers" is invalid JSON.

Thank you for your help, you have lead me down the following path (detailed below) where I have uncovered what I feel is my issue.
I'm going to keep digging, but it seems I need to get my extracted field to be set with a PathType.MULTI. If it is, then the tests pass.
If you have any additional suggestions I greatly welcome them, but regardless thanks for your assistance so far.

spring-restdocs-core-2.0.0.RELEASE

org.springframework.restdocs.payload.FieldPathPayloadSubsectionExtractor.java

@Override
public byte[] extractSubsection(byte[] payload, MediaType contentType) {
    try {
        ExtractedField extractedField = new JsonFieldProcessor().extract(
                this.fieldPath, objectMapper.readValue(payload, Object.class));
        Object value = extractedField.getValue();
        if (value instanceof List && extractedField.getType() == PathType.MULTI) {
            List<?> extractedList = (List<?>) value;
            if (extractedList.size() == 1) {
                value = extractedList.get(0);
            }
            else {
                throw new PayloadHandlingException(this.fieldPath
                        + " does not uniquely identify a subsection of the payload");
            }
        }
        return objectMapper.writeValueAsBytes(value);
    }
    catch (IOException ex) {
        throw new PayloadHandlingException(ex);
    }
}

Where this.fieldPath is "information.customers[]" (also tested with various paths, see below)
Object value is the LinkedHashMap (which is expected)
However, extractedField.getType() equals PathType.SINGLE, which causes value instanceof List && extractedField.getType() == PathType.MULTI to evaluate to false. If this evaluates to true then the tests pass.

The only place that I am finding the PathType get set is by:

org.springframework.restdocs.payload.JsonFieldPath.java

static boolean matchesSingleValue(List<String> segments) {
        Iterator<String> iterator = segments.iterator();
        while (iterator.hasNext()) {
            String segment = iterator.next();
            if ((isArraySegment(segment) && iterator.hasNext())
                    || isWildcardSegment(segment)) {
                return false;
            }
        }
        return true;
    }

I am not able to get the segment to fulfill the isArraySegment or the isWildcardSegment and still be a valid path.

Test Code

List<FieldDescriptor> responseFieldDescriptors = new ArrayList<>();
responseFieldDescriptors.add(fieldWithPath("details").type(JsonFieldType.STRING).description("Details to display to the customer."));
responseFieldDescriptors.add(fieldWithPath("name").type(JsonFieldType.STRING).description("Customer's name."));


resultActions.andDo(
        MockMvcRestDocumentation.document("content-example",
                preprocessRequest(prettyPrint()),
                preprocessResponse(prettyPrint()),
                responseFields(beneathPath("information.customers[]").withSubsectionId("customer"), responseFieldDescriptors)
        )
);

Test also passes with the following if I manually modify extractedField's type while debugging to PathType.MULTI:
beneathPath("information.customers")
beneathPath("information['customers']")
beneathPath("information.customers[]")
beneathPath("information.customers.[]")

Sample Response:

{
    "information": {
        "customers": [{
            "details": "test details",
            "name": "testName"
        }]
    }
}

@wilkinsona wilkinsona added the type: bug A bug label Mar 28, 2018
@ingogriebsch
Copy link

We are currently facing the same problem as @bsb319 but we are using Spring Restdocs 1.2.4. Therefore it would be nice if @wilkinsona could give us a hint if this bug will also be solved in this version.

@wilkinsona
Copy link
Member

@gbtec-ingogriebsch This issue's still open. It's yet to be solved in any version.

@ingogriebsch
Copy link

@wilkinsona This was clear to me as I wrote the comment. To say it the other way: It would be nice if you would also fix this bug in the 1.2.x branch. :)

@wilkinsona
Copy link
Member

Ah, I understand now. Yes, in all likelihood the fix will be made in both 1.2.x and 2.0.x although I've yet to figure out what the fix should be…

@Jeff-Walker
Copy link

Jeff-Walker commented Sep 20, 2018

I'm assuming the open issue means that it still isn't fixed. I'm using 2.0.1.RELEASE.

Is there a workaround so I can make this documentation happen? I'm willing to extend the involved classes and make this work, but at this point, I'd like to avoid spending the time to figure out how to do that.

Edit: Tried it with 2.0.2.RELEASE with no change.

@wilkinsona
Copy link
Member

I'm assuming the open issue means that it still isn't fixed

Correct.

Is there a workaround so I can make this documentation happen?

I'm not aware of one.

@Jeff-Walker
Copy link

I agree with @bsb319 that the problem is in JsonFieldPath.matchesSingleValue(List<String> segments). As it loops over segments, it sees that [] is an array segment, but it requres there to be a second segment after that.

I'm not sure I understand why the iterator.hasNext() is even there. I'm sure there's some use-case that I can't think of, but if we removed that, this case would work as desired. The JsonFieldPath class is used for all paths, so I'm not sure if it works for everything.

I also experimented with adding a class like FieldPathPayloadSubsectionExtractor but explicitly for array subsections. It basically drops the check for PathType.MULTI. It gets the first element of the List that comes back from extract() which then will return the object that AbstractFieldsSnippet wants.

I'm willing to submit a pull request. I think the JsonFieldPath change is cleaner and makes sense to me, but the new subsection extractor affects fewer scenarios.

@Jeff-Walker
Copy link

I've been looking at the unit tests for FieldPathPayloadSubsectionExtractor and comparing those to the JsonPath rules. You could say that it's working as designed, there's a test case for this, but it expects to get the whole list back, I find this surprising. This makes the path a.[] return the same as a, which may be what's expected.

@Test
@SuppressWarnings("unchecked")
public void extractSingleElementArraySubsectionOfJsonMap()
throws JsonParseException, JsonMappingException, IOException {
byte[] extractedPayload = new FieldPathPayloadSubsectionExtractor("a.[]")
.extractSubsection("{\"a\":[{\"b\":5}]}".getBytes(),
MediaType.APPLICATION_JSON);
List<Map<String, Object>> extracted = new ObjectMapper()
.readValue(extractedPayload, List.class);
assertThat(extracted.size(), is(equalTo(1)));
assertThat(extracted.get(0).get("b"), is(equalTo((Object) 5)));
}

The a.[] construct isn't valid JsonPath according to Jayway, but it looks like the intent is for it to be equivalent to a.[*], which is the same as a, so that matches. But, in JsonPath, you can use a[0] to get the first object/value in the array. So, using the example in the test of {"a": [{"b":5}]} it would give me {"b":5} which would solve this problem for us.

Looking at JsonFieldProcessor, when doing traverse(), it just returns the collection when it hits []. It should instead do more work to see what's between the brackets, whether or not we're trying to get the array, as in a.[] or some element of the array, as in a.[0].

With this implemented, a new test for this would be:

  @Test
  @SuppressWarnings("unchecked")
  public void __extractSingleElementArraySubsectionOfJsonMap()
      throws JsonParseException, JsonMappingException, IOException {
    byte[] extractedPayload = new FieldPathPayloadSubsectionExtractor("a.[0]")
        .extractSubsection("{\"a\":[{\"b\":5}]}".getBytes(),
            MediaType.APPLICATION_JSON);
    Map<String, Object> extracted = new ObjectMapper()
        .readValue(extractedPayload, Map.class);
    assertThat(extracted.get("b"), is(equalTo((Object) 5)));
  }

I haven't looked deep enough to know what the effort of implementing this is, but what do you think, @wilkinsona ?

@wilkinsona
Copy link
Member

Thanks for taking a look, @Jeff-Walker. a.[] is indeed intended to be equivalent to a.[*] in JSON path. I don't think that introducing support for a.[n] will address the original problem that @bsb319 has reported. Looking at their original example, I believe they'd like every item in the array to have the same structure. Using an index to identify a specific item in the array would not give them what they want.

My hope is that this problem can be fixed with a change to JsonFieldPath that doesn't require the introduction of any new features. Once this week's SpringOne Platform is over, I hope to be able to get a solid block of time to make some progress on figuring out what that fix should be.

@Jeff-Walker
Copy link

Understood. That's why wrote it up before making a pull request.

For reference, here's a link to my work-around.
https://gist.github.com/Jeff-Walker/5602b416ea84b21c64bc0bbca16910ab

It's a copy of FieldPathPayloadSubsectionExtractor that grabs the first element of a list. It's ad-hoc and not a full solution, but it get's my docs to look like I want them without doing a custom snippet template.

Since [].a and [].b work, maybe it's all a matter of presentation? Again, that was more or less my problem.

Anyway, even if what I've said doesn't help with the real issue, if there's anything I can do to help, let me know.

Also, thanks for this great product.

@Jeff-Walker
Copy link

Jeff-Walker commented Oct 2, 2018

It's me again. I ran into a problem with my little work around when I had more than one item in the array. I got does not uniquely identify a subsection of the payload, because there were two this time. To fix this I added an optional index to get. I'll just paste it down here since my gist formatting didn't work out super well.

package org.springframework.restdocs.payload;

import java.io.IOException;
import java.util.List;

import org.springframework.http.MediaType;
import org.springframework.restdocs.payload.JsonFieldProcessor.ExtractedField;

import com.fasterxml.jackson.databind.ObjectMapper;

/**
 * Copy of {@link FieldPathPayloadSubsectionExtractor} but gets the first element
 * of a list, no matter the value of {@link ExtractedField#getType()}.
 *
 * The original version would only get the first element of the list if the type is {@code PathType.MULTI}
 *
 * @author Jeff Walker
 * @see FieldPathPayloadSubsectionExtractor
 * @see FieldPathPayloadSubsectionExtractor#extractSubsection(byte[], MediaType)
 */
public class FieldPathPayloadArraySubsectionExtrator
implements PayloadSubsectionExtractor<FieldPathPayloadArraySubsectionExtrator>
{

  private static final ObjectMapper objectMapper = new ObjectMapper();

  private final String fieldPath;

  private final String subsectionId;

  private final Integer index;

  public FieldPathPayloadArraySubsectionExtrator(String fieldPath, String subsectionId) {
    this(fieldPath, subsectionId, null);
  }
  public FieldPathPayloadArraySubsectionExtrator(String fieldPath) {
    this(fieldPath, "beneath-array-" + fieldPath, null);
  }
  public FieldPathPayloadArraySubsectionExtrator(String fieldPath, Integer index) {
    this(fieldPath, "beneath-array-" + fieldPath, index);
  }
  public FieldPathPayloadArraySubsectionExtrator(String fieldPath, String subsectionId, Integer index) {
    this.fieldPath = fieldPath;
    this.subsectionId = subsectionId;
    this.index = index;
  }

  @Override
  public byte[] extractSubsection(byte[] payload, MediaType contentType) {
    try {
      ExtractedField extractedField = new JsonFieldProcessor().extract(
          this.getFieldPath(), objectMapper.readValue(payload, Object.class));
      Object value = extractedField.getValue();
      if (value instanceof List) {
        List<?> extractedList = (List<?>) value;
        if (extractedList.size() == 1 || index != null) {
          value = extractedList.get(index == null ? 0 : index);
        }
        else {
          throw new PayloadHandlingException(this.getFieldPath()
              + " does not uniquely identify a subsection of the payload");
        }
      }
      return objectMapper.writeValueAsBytes(value);
    }
    catch (IOException ex) {
      throw new PayloadHandlingException(ex);
    }
  }

  @Override
  public String getSubsectionId() {
    return this.subsectionId;
  }

  /**
   * Returns the path of the field that will be extracted.
   *
   * @return the path of the field
   */
  protected String getFieldPath() {
    return this.fieldPath;
  }



  @Override
  public FieldPathPayloadArraySubsectionExtrator withSubsectionId(String subsectionId) {
    return new FieldPathPayloadArraySubsectionExtrator(this.fieldPath, subsectionId, index);
  }
  public FieldPathPayloadArraySubsectionExtrator atArrayIndex(int index) {
    return new FieldPathPayloadArraySubsectionExtrator(this.fieldPath, subsectionId, index);
  }
}

@wilkinsona wilkinsona added this to the 1.2.6.RELEASE milestone Nov 22, 2018
@wilkinsona
Copy link
Member

Thank you for your patience @bsb319, @ingogriebsch, and @Jeff-Walker. I've made some changes in both 1.2.x and master that I believe should fix the problem described here. I would be most grateful if you could try the latest 1.2.6 or 2.0.3 snapshots and let me know if the changes address each of your individual problems. Snapshots are available from https://repo.spring.io/libs-snapshot.

@Jeff-Walker
Copy link

Ok, I tried it and I think it's working for me, but let me make sure I haven't forgotten what's going on.

I have something like:

{
"roles": [{"key":"1","name":"one"},{"key":"1","name":"one"}]
}

And:

document(snippetName,
        responseFields(
            beneathPath("roles").withSubsectionId("roles"),
            fieldWithPath("key").description("key"),
            fieldWithPath("name").description("name"),
           ...

And I get a snippet that looks like:

|===
|Path|Type|Description|Optional

|`+key+`
|`+String+`
|key
|`+false+`

|`+name+`
|`+String+`
|name
|`+false+`

Which looks ok to me. It figures out that it's an array and drops the [].

@wilkinsona
Copy link
Member

Excellent. Thank you.

@Jeff-Walker
Copy link

Thanks again for taking care of this.

What is your current timeline for when this build makes it into milestone/rc/ga?

@wilkinsona
Copy link
Member

I'd like to wait a while longer to give @bsb319 and @ingogriebsch a reasonable chance to try a snapshot. A pencil something in for a week today.

@Jeff-Walker
Copy link

Jeff-Walker commented Dec 12, 2018

I have another question, but let me know if this needs to be sent to a new issue.
I'm now writing restdocs for another project, this time the payload has two nested arrays and I'm still having the issue. That is, unless I'm missing something. To extend my example above:

{
  "roles": [
    {
      "key": "1",
      "name": "one",
      "more": [
        {
          "id": 1
        }
      ]
    },
    {
      "key": "1",
      "name": "one",
      "more": []
    }
  ]
}

I now have nested arrays. What I do with this:

document(snippetName,
        responseFields(
            beneathPath("roles").withSubsectionId("roles"),
            fieldWithPath("key").description("key"),
            fieldWithPath("name").description("name"),
            subsectionWithPath("more").description("array")));

And, to get another table:

document(snippetName,
        responseFields(
            beneathPath("roles[].more").withSubsectionId("more"),
            fieldWithPath("id").description("id")));

This doesn't work like the one above does, I'm forced to put [].id to get it to work, and the only way for beneathPath to find the payload is roles[].more, nothing else works. And again, this puts the square brackets in the adoc file.

@wilkinsona
Copy link
Member

Interesting. Please open a new issue and I’ll take a look.

cocota93 added a commit to cocota93/advance-multi-module-lab that referenced this issue Jun 23, 2021
버전 낮을경우 리스트처리 제대로 안되는 이슈 있었음 : spring-projects/spring-restdocs#473
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants