Skip to content
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

Add support for Atlassian swagger-request-validator library #118

Merged

Conversation

smals-mavh
Copy link
Collaborator

@smals-mavh smals-mavh commented Oct 14, 2024

Currently only implemented for Spring Boot 3.

atlassian lib for SB3 is different from the SB2 one (swagger-request-validator-springmvc for sb2, swagger-request-validator-spring-webmvc for sb3).

Now the belgif-rest-problem-openapi-validation-spring library also comes with the belgif-rest-problem-spring-boot-3 lib which might be confusing.

Maybe we should rename belgif-rest-problem-openapi-validation-spring to belgif-rest-problem-openapi-validation-spring-boot-3 to be more clear on the fact that it (now) only supports SB3, and it already includes the other errormappers.

Or if someone has a better / more efficient idea, please let me know.

@jpraet ready for review.

@smals-mavh smals-mavh linked an issue Oct 14, 2024 that may be closed by this pull request
@smals-mavh smals-mavh requested a review from jpraet October 14, 2024 13:06
@pvdbosch pvdbosch changed the title Implemented support for exceptions from atlassian-swagger-request-validator lib Support for atlassian-swagger-request-validator lib Oct 14, 2024
@pvdbosch
Copy link
Contributor

@smals-mavh , would be best to update doc for this new feature, but might have to be coordinated with other doc changes in https://github.com/belgif/rest-problem-java/tree/feature/%23117-improve-documentation

@smals-mavh
Copy link
Collaborator Author

@pvdbosch I think a note somewhere in the docs that it supports mapping the InvalidRequestException from the atlassian lib to belgif-problem should be enough, the rest seems to work automagically with this conditionalOnClass.
I think the mvn-build that runs for every branch automatically pushes the docs, so I haven't changed this yet.

@jpraet ready for further review.

@smals-mavh
Copy link
Collaborator Author

Implemented feedback

@jpraet jpraet requested a review from clone1612 October 17, 2024 15:36
@jpraet jpraet mentioned this pull request Oct 21, 2024
@smals-mavh
Copy link
Collaborator Author

smals-mavh commented Oct 21, 2024

@clone1612 @jpraet @pvdbosch ,

Tried to implement all the feedback.

The atlassian library gives one violation (message in validation report) for each object that is missing a required property. When one object is missing multiple properties, it provides all missing properties in the detail message, and the 'name' is the parent object. I think this is clear enough, and it might not be worth our time to split this out even further, because it would involve regex on the message (which might change / be translated).
Example:

{
    "type": "urn:problem-type:belgif:badRequest",
    "href": "https://www.belgif.be/specification/rest/api-guide/problems/badRequest.html",
    "title": "Bad Request",
    "status": 400,
    "detail": "The input message is incorrect",
    "issues": [
        {
            "type": "urn:problem-type:belgif:input-validation:schemaViolation",
            "title": "Input value is invalid with respect to the schema",
            "detail": "[Path '/myInnerObject'] Object has missing required properties ([\"myParam\"])",
            "in": "body",
            "name": "/myInnerObject"
        },
        {
            "type": "urn:problem-type:belgif:input-validation:schemaViolation",
            "title": "Input value is invalid with respect to the schema",
            "detail": "Object has missing required properties ([\"myFirstProperty\"])",
            "in": "body",
            "name": "/"
        }
    ]
}

or

{
    "type": "urn:problem-type:belgif:badRequest",
    "href": "https://www.belgif.be/specification/rest/api-guide/problems/badRequest.html",
    "title": "Bad Request",
    "status": 400,
    "detail": "The input message is incorrect",
    "issues": [
        {
            "type": "urn:problem-type:belgif:input-validation:schemaViolation",
            "title": "Input value is invalid with respect to the schema",
            "detail": "Object has missing required properties ([\"myFirstProperty\",\"myInnerObject\"])",
            "in": "body",
            "name": "/"
        }
}

The / in name is because it's in the root schema of the request body.

For the allOf you had a good catch @clone1612 !
Modified it to 'explode' the messages in case of allOf:

{
    "type": "urn:problem-type:belgif:badRequest",
    "href": "https://www.belgif.be/specification/rest/api-guide/problems/badRequest.html",
    "title": "Bad Request",
    "status": 400,
    "detail": "The input message is incorrect",
    "issues": [
        {
            "type": "urn:problem-type:belgif:input-validation:schemaViolation",
            "title": "Input value is invalid with respect to the schema",
            "detail": "[Path '/myInnerObject/myParam'] String \"abc\" is too short (length: 3, required minimum: 10)",
            "in": "body",
            "name": "/myInnerObject/myParam",
            "value": "abc"
        },
        {
            "type": "urn:problem-type:belgif:input-validation:schemaViolation",
            "title": "Input value is invalid with respect to the schema",
            "detail": "[Path '/myInnerObject/myOtherParam'] String \"abc\" is too short (length: 3, required minimum: 10)",
            "in": "body",
            "name": "/myInnerObject/myOtherParam",
            "value": "abc"
        }
    ]
}

But just getting all the nested messages wouldn't work for oneOf / anyOf schemas, so in those cases, the detail message will be different, but it will remain just one issue:

{
    "type": "urn:problem-type:belgif:badRequest",
    "href": "https://www.belgif.be/specification/rest/api-guide/problems/badRequest.html",
    "title": "Bad Request",
    "status": 400,
    "detail": "The input message is incorrect",
    "issues": [
        {
            "type": "urn:problem-type:belgif:input-validation:schemaViolation",
            "title": "Input value is invalid with respect to the schema",
            "detail": "[Path '/myInnerObject'] Instance failed to match exactly one schema (matched 0 out of 2) :  -- [Path '/myInnerObject'] Object has missing required properties ([\"myParam\"]) -- [Path '/myInnerObject'] Object has missing required properties ([\"myOtherParam\"])",
            "in": "body",
            "name": "/myInnerObject"
        }
    ]
}

Copy link
Collaborator

@clone1612 clone1612 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smals-mavh Changes look good, thanks! Don't forget to add a release note in src/main/asciidoc/release-notes.adoc when you're ready to merge.

@smals-mavh
Copy link
Collaborator Author

Added documentation and releasenote (still for v0.9, since I didn't see that on maven central yet)

@smals-mavh smals-mavh requested a review from pvdbosch October 24, 2024 13:58
Copy link

@jpraet jpraet changed the title Support for atlassian-swagger-request-validator lib Add support for Atlassian swagger-request-validator library Oct 30, 2024
@jpraet jpraet merged commit 5fde430 into main Oct 30, 2024
2 checks passed
@jpraet jpraet deleted the 97-support-exceptions-from-atlassian-swagger-request-validator branch December 5, 2024 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support exceptions from atlassian-swagger-request-validator
4 participants