Codearte / accurest

Accurest - Consumer Driven Contracts verifier for Java. Moved to:
https://github.com/spring-cloud/spring-cloud-contract
Apache License 2.0
99 stars 23 forks source link

Incorrect array size check generated #293

Closed hator closed 8 years ago

hator commented 8 years ago

For input:

response {
    status 200
    body( content: [
        value(
            id: 'article_accurest::1',
            tags: ['tag1', 'tag2', 'tag3']
        ), value (
            id: 'article_accurest::2',
            tags: ['tag1', 'tag2', 'tag3']
        )
])}

Generated:

DocumentContext parsedJson = JsonPath.parse(response.body.asString())
assertThatJson(parsedJson).array("content").array("tags").hasSize(3)

The problem is that using Json path <$.content[*].tags[*]> all elements of all tags arrays are counted giving total of 6 instead of 3. This of course causes tests to fail.

marcingrzejszczak commented 8 years ago

hmm I think that your contract is a little bit wrong. We weren't expecting to insert maps between value(...). Try putting a concrete value there

hator commented 8 years ago

How would I model maps, then?

For clarity JSON that I would like to model and groovy contract: https://gist.github.com/hator/60d3104d12c23a5ffa5eea0c9e4b469d

marcingrzejszczak commented 8 years ago

value() is a helper method for Verifier to provide client side stubbing. It's a synonim of the metdho $ that you used.

In Groovy to use a map you use the [:] notation. I've rearanged your contract - now it should work better.

org.springframework.cloud.contract.spec.Contract.make {
    request {
        method 'GET'
        url '/api/articles?title=test'
        headers {
            header 'Content-Type': 'application/json;charset=UTF-8'
        }
    }
    response {
        status 200
        body( content: [
                [
                    id: 'article_accurest::1',
                    title: 'test title',
                    lead: 'test lead',
                    leadPhoto: [
                        url: "http://photo.png",
                        alt: "photo",
                        title: "photo"
                    ],
                    content: 'test content',
                    tags: ['tag1', 'tag2', 'tag3'],
                    bioId: 'test-bio-id',
                    publishDate: $(test(execute('isZonedDateTime($it)')), stub('2018-05-17T12:32:28Z')),
                    modifiedDate: $(test(execute('isZonedDateTime($it)')), stub('2018-05-18T14:32:20Z')),
                    modifiedBy: $(test(regex(".+")), stub('author1'))
                ],
               [
                    id: 'article_accurest::3',
                    title: 'test game',
                    lead: 'test lead',
                    leadPhoto: [
                        url: "http://photo.png",
                        alt: "photo",
                        title: "photo"
                    ],
                    content: 'test content',
                    tags: ['tag1', 'tag2', 'tag3'],
                    bioId: 'test-bio-id',
                    publishDate: $(test(execute('isZonedDateTime($it)')), stub('2018-05-17T12:32:28Z')),
                    modifiedDate: $(test(execute('isZonedDateTime($it)')), stub('2018-05-18T14:32:20Z')),
                    modifiedBy: $(test(regex(".+")), stub('author2'))
                )
              ]
            ]
        )
    }
}
jakubnabrdalik commented 8 years ago

It doesn't seem to be a problem with value() but rather with how size is verified by com.toomuchcoding.jsonassert:

<$.content[*].tags[*]>

The outcomes from a test fixed by your suggestions look like this:

java.lang.IllegalStateException: Parsed JSON <{"content":[{"id":"article_accurest::1","title":"test title","lead":"test lead","leadPhoto":{"url":"http://grey.png","alt":"Sasha photo","title":"Sasha Grey"},"content":"test content","tags":["tag1","tag2","tag3"],"createdDate":null,"createdBy":null,"modifiedDate":"2016-07-06T08:10:22.433Z","modifiedBy":"accurest_editor","publishDate":"2018-05-17T12:32:28Z","status":"DRAFT","bioId":"test-bio-id"},{"id":"article_accurest::3","title":"test game","lead":"test lead","leadPhoto":{"url":"http://grey.png","alt":"Sasha photo","title":"Sasha Grey"},"content":"test content","tags":["tag1","tag2","tag3"],"createdDate":null,"createdBy":null,"modifiedDate":"2016-07-06T08:10:22.451Z","modifiedBy":"accurest_editor","publishDate":"2018-05-17T12:32:28Z","status":"DRAFT","bioId":"test-bio-id"}],"totalPages":1,"totalElements":2,"last":true,"size":20,"number":0,"sort":null,"numberOfElements":2,"first":true}> doesn't have the size <3> for JSON path <$.content[*].tags[*]>. The size is <6>
    at com.toomuchcoding.jsonassert.JsonAsserter.hasSize(JsonAsserter.java:244)
    at 

In other words, the count counts all tags in the json, not tags for a given element. Does it even have to check the size?

marcingrzejszczak commented 8 years ago

Ah true - I get it.

It's close to impossible to properly check the size sometimes (I'd have to check every single element in the list). So I'd suggest to pass the system prop spring.cloud.contract.verifier.assert.size=false and disable the size check.

I'll try to fix it one day but any help would be more than welcome.

jakubnabrdalik commented 8 years ago

Any documentation on spring.cloud.contract.verifier.assert.size? Is it boolean? Should we just set it to false? Couldn't find it in official docs.

marcingrzejszczak commented 8 years ago

Yeah it's a boolean. Set it to false. I'll add it to the docs.

jakubnabrdalik commented 8 years ago

Thanks.

jakubnabrdalik commented 8 years ago

Setting

spring.cloud.contract.verifier.assert.size: false

in both application.yml and/or bootstrap.yml doesn't seem to have any effect.

Which makes sense, considering this is a gradle task, that has little to do with the rest of the spring app.

Should it be part of contractVerifier properties in gradle? Like:

contractVerifier {
   spring.cloud.contract.verifier.assert.size = false 
   ...
}
marcingrzejszczak commented 8 years ago

It's a system prop - you'd have to pass it as -D.

marcingrzejszczak commented 8 years ago

Let's move here - https://gitter.im/Codearte/accurest

jakubnabrdalik commented 8 years ago

Ah, ok.

marcingrzejszczak commented 8 years ago

Ok workaround is here. Check the latest snapshots

marcingrzejszczak commented 8 years ago

The docs got updated - http://codearte.github.io/accurest/#limitations

marcingrzejszczak commented 8 years ago

This issue was moved to spring-cloud/spring-cloud-contract#10