Add option to permit validation when schema and data are empty #417

chibicco · 2024-03-15T12:31:35Z

Background

In our project, there are scenarios where the API returns an empty response body with status code 200.
However, the current OpenAPI 2 schema validation does not allow an empty schema for the response body, causing validation errors in these cases.

Specifically, in cases such as the schema and controller example below, I expect the Committee::Middleware::ResponseValidation check to pass successfully.

"/pets/cat": {
  "get": {
    "description": "Returns pets which are cats",
    "operationId": "find pets which are cats",
    "responses": {
      "200": {
        "description": "empty schema"
      }
    }
  }
}

class CatController
    # pets/cat
    get "/" do
        status 200
        present nil
    end
end

Changes

To address this issue, I have added an option to allow an empty schema for the response body in OpenAPI 2.
This change enables the validation to pass when both the schema and response body are nil, accommodating the specific use case in our project.

If allow_blank_structures is set to true, the validation will pass if the schema is empty and the data is nil.
I have also modified the error message for the case where data is present and the schema does not exist.

chibicco · 2024-03-15T12:54:24Z

lib/committee/schema_validator/hyper_schema/response_validator.rb

+              raise InvalidResponse, "Invalid response.\n\n#{errors}"
+            end
+          rescue => e
+            raise InvalidResponse, "Invalid response.\n\nschema is undefined" if /undefined method .all_of. for nil/ =~ e.message


ruby 3.2.2

NoMethodError: undefined method `all_of' for nil:NilClass

ruby 3.3.0

NoMethodError: undefined method `all_of' for nil

ruby head

NoMethodError: undefined method 'all_of' for nil

chibicco · 2024-03-18T05:28:24Z

@brandur @ota42y

If you have a moment, I would appreciate it if you could review it.
I apologize for my poor English skills, so please bear with me.

brandur · 2024-05-04T17:56:57Z

@chibicco Sorry for the delay. Do you still need this?

…nAPI2::Link

chibicco · 2024-05-09T00:49:47Z

@brandur
Thank you for response :)

Yes, I need this pull requests.
I have rebased on the latest master, If possible, please review.

chibicco · 2024-05-10T09:25:13Z

Greetings.

I have just noticed the following correction.
#420

If my PR is not appropriate, I would appreciate it if you could close it.

brandur · 2024-05-10T09:30:51Z

@chibicco Let's hold on that a bit longer. We have someone offering to help with maintenance so we may yet be able to rescue the project.

ydah

@chibicco I left a few comments. Could you check?

ydah · 2024-05-10T10:04:05Z

lib/committee/schema_validator/option.rb

@@ -46,6 +47,7 @@ def initialize(options, schema, schema_type)
        @optimistic_json                  = options.fetch(:optimistic_json, false)
        @parse_response_by_content_type   = options.fetch(:parse_response_by_content_type, true)
        @parameter_overwite_by_rails_rule = options.fetch(:parameter_overwite_by_rails_rule, true)
+        @permit_blank_structures          = options.fetch(:permit_blank_structures, false)


nits) As for the name of the parameter, how about allow_blank_structures to match the name of the other parameter if you want to allow it?

committee/lib/committee/schema_validator/option.rb

Lines 42 to 43 in d1488a8

@allow_form_params = options.fetch(:allow_form_params, true)

@allow_query_params = options.fetch(:allow_query_params, true)

Thank you.
I have renamed permit_blank_structures to allow_blank_structures.

ref: 6de7583

ydah · 2024-05-10T10:05:54Z

lib/committee/schema_validator/option.rb

@@ -46,6 +47,7 @@ def initialize(options, schema, schema_type)
        @optimistic_json                  = options.fetch(:optimistic_json, false)
        @parse_response_by_content_type   = options.fetch(:parse_response_by_content_type, true)
        @parameter_overwite_by_rails_rule = options.fetch(:parameter_overwite_by_rails_rule, true)
+        @permit_blank_structures          = options.fetch(:permit_blank_structures, false)


Could you add a description of this parameter to the README?

https://github.com/interagent/committee?tab=readme-ov-file#configuration-options

I have added a note to the README : )

[memo]

In OpenAPI 3, the following becomes nil, so the validation of empty response succeeds.
Therefore, this option is expected to be valid only for Hyper-schema and OpenAPI 2.

ydah · 2024-05-10T10:15:07Z

test/data/openapi2/petstore-expanded.json

+        "responses": {
+          "200": {
+            "description": "empty schema"


imo) 204 No Content I think the response body is empty as a specification. So why not make the test data follow the common use case?

Suggested change

"responses": {

"200": {

"description": "empty schema"

"responses": {

"204": {

"description": "The resource was deleted successfully."

Refs: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/204#compatibility_notes

In my recent experiences, I've noticed the following scenarios:

status_code=200 && empty response
=> This PR addresses this case. While it's technically correct to use status_code=204, there are instances where APIs are operated with 200, and ideally, I'd like to accommodate this case as well.

status_code=204 && empty response
=> While it's being skipped here for 204, it seemed like a better approach to modify it to skip within Committee::SchemaValidator::HyperSchema::ResponseValidator. If it's not an inconvenience, I'm considering addressing this in a separate PR. Could you please share your thoughts on this?

Thank you for your detailed explanation!
I understood the intent and thought it make sense. So I agree with your opinion.

ydah · 2024-05-10T11:33:09Z

memo) I noticed in writing the following comment that 204 responses and so on expect to describe a response without a body. In other words, I think the next major update will change this parameters default value.

#417 (comment)

chibicco · 2024-05-11T10:08:46Z

@ydah

Thank you for your review!
I have added the necessary commits for the corrections and replied to the comments.
I kindly ask for your confirmation.

ydah

@chibicco Thank you for your response! I think it looks good 😀
Finally, could you add a description of this change in CHANGELOG.md?

chibicco · 2024-05-11T17:44:48Z

@ydah I have updated the CHANGELOG.md :)
We appreciate your kind support.

ydah

Looks good! Thank you for your work.

cc/ @brandur

chibicco · 2024-05-15T11:42:32Z

@brandur
Thank you for merging the PR.
Could you kindly release the latest master at a time that is convenient for you?

brandur · 2024-05-15T11:53:31Z

@chibicco Yes! I believe @ydah is waiting for one more change on review feedback over on #396, after which he'll cut a release and get this shipped.

chibicco · 2024-05-15T23:59:19Z

I see, I understand !

ydah · 2024-05-17T00:11:01Z

@chibicco Thank you for waiting. Since v5.3.0 has been released, so your patch has been released! Thank you for your great work!
https://rubygems.org/gems/committee/versions/5.3.0

chibicco · 2024-05-21T12:06:46Z

@ydah

I have confirmed the v5.3.0 release : )
Thank you!

chibicco force-pushed the open_api_2_link_targaet_schema branch 2 times, most recently from 3d68f0d to 90fb86f Compare March 15, 2024 12:51

chibicco commented Mar 15, 2024

View reviewed changes

Add option to permit validation when schema and data are empty in Ope…

d1488a8

…nAPI2::Link

chibicco force-pushed the open_api_2_link_targaet_schema branch from 90fb86f to d1488a8 Compare May 9, 2024 00:47

brandur mentioned this pull request May 10, 2024

Add maintenance warning to README #420

Merged

ydah reviewed May 10, 2024

View reviewed changes

chibicco added 2 commits May 11, 2024 16:18

Rename options from permit_blank_structures to allow_blank_structures

6de7583

Add explanation of allow_blank_structures to README

0d63bfd

ydah reviewed May 11, 2024

View reviewed changes

Update CHANGELOG.md

97ece37

chibicco requested a review from ydah May 11, 2024 17:44

ydah approved these changes May 11, 2024

View reviewed changes

brandur approved these changes May 13, 2024

View reviewed changes

ydah merged commit 79bad85 into interagent:master May 13, 2024
6 checks passed

chibicco deleted the open_api_2_link_targaet_schema branch June 6, 2024 19:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to permit validation when schema and data are empty #417

Add option to permit validation when schema and data are empty #417

chibicco commented Mar 15, 2024 •

edited

Loading

chibicco Mar 15, 2024

chibicco commented Mar 18, 2024

brandur commented May 4, 2024

chibicco commented May 9, 2024

chibicco commented May 10, 2024

brandur commented May 10, 2024

ydah left a comment •

edited

Loading

ydah May 10, 2024

chibicco May 11, 2024

ydah May 10, 2024

chibicco May 11, 2024

ydah May 10, 2024

chibicco May 11, 2024

ydah May 11, 2024

ydah commented May 10, 2024

chibicco commented May 11, 2024

ydah left a comment

chibicco commented May 11, 2024

ydah left a comment

chibicco commented May 15, 2024

brandur commented May 15, 2024

chibicco commented May 15, 2024

ydah commented May 17, 2024

chibicco commented May 21, 2024

	@allow_form_params = options.fetch(:allow_form_params, true)
	@allow_query_params = options.fetch(:allow_query_params, true)

Add option to permit validation when schema and data are empty #417

Add option to permit validation when schema and data are empty #417

Conversation

chibicco commented Mar 15, 2024 • edited Loading

Background

Changes

Choose a reason for hiding this comment

chibicco commented Mar 18, 2024

brandur commented May 4, 2024

chibicco commented May 9, 2024

chibicco commented May 10, 2024

brandur commented May 10, 2024

ydah left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ydah commented May 10, 2024

chibicco commented May 11, 2024

ydah left a comment

Choose a reason for hiding this comment

chibicco commented May 11, 2024

ydah left a comment

Choose a reason for hiding this comment

chibicco commented May 15, 2024

brandur commented May 15, 2024

chibicco commented May 15, 2024

ydah commented May 17, 2024

chibicco commented May 21, 2024

chibicco commented Mar 15, 2024 •

edited

Loading

ydah left a comment •

edited

Loading