Sayi / swagger-diff

:ski: Compare two swagger API specifications(1.x or v2.0)
http://deepoove.com/swagger-diff/
Apache License 2.0
276 stars 85 forks source link

Array of Object Response Changes Not Detected #21

Open stt19 opened 5 years ago

stt19 commented 5 years ago

For example, if in petstore_v2_1.json and petstore_v2_2.json you change lines ~723 and ~722 respectively from "schema": { "$ref": "#/definitions/User" } to "schema": { "type" : "array", "items" : { "$ref" : "#/definitions/User" } } it will no longer detect that there was a change, even though it previously was caught (GET /user/{username}) when it was only a single object being returned. This is pretty important if you only return a certain object type from an endpoint as an array.

stt19 commented 5 years ago

I think this is the fix for the array-to-array comparison. It works for me as far as I can tell. In SpecificationDiff.java:

Original:

private static Property getResponseProperty(Operation operation) {
    Map<String, Response> responses = operation.getResponses();
    // temporary workaround for missing response messages
    if (responses == null) return null;
    Response response = responses.get("200");
    return null == response ? null : response.getSchema();
}

Refactor to handle array:

private static Property getResponseProperty(Operation operation) {
    Map<String, Response> responses = operation.getResponses();
    // temporary workaround for missing response messages
    if (responses == null) return null;
    Response response = responses.get("200");
    if(response != null && response.getSchema() instanceof ArrayProperty) {
        ArrayProperty prop = (ArrayProperty)response.getSchema();
        return prop.getItems();
    }
    return null == response ? null : response.getSchema();
}
herve-brun commented 4 years ago

Hi ! This issue is a blocker for us ...

@Sayi Do you plan to merge #30 anytime soon ?

nmische commented 3 years ago

@Sayi - ~Any plans to merge and release an update with this fix?~ I see this was merged in v1.2.2, however I do not see this difference in the HTML report. I will take another look at this and file a separate issue around HTML reporting if needed.