Strict schema validation #604

IlkhamGaysin · 2023-03-05T21:11:00Z

Problem

I see some problems in ruby projects - it's data bloating. As projects grow they get complicated so it's hard to keep responses clean. It happed to projects I am working on and others(see the issue). So I decided to bring the changes back from initial issue.

Solution

I want to add a new configuration option on top of the main called additionalProperties from json-schema to allow users to validate their schemas strictly.

This concerns this parts of the OpenAPI Specification:

The changes I made are compatible with:

OAS2
OAS3
OAS3.1

Related Issues

Closes #163

Checklist

Added tests
Changelog updated
Added documentation to README.md

Steps to Test or Reproduce

Outline the steps to test or reproduce the PR here.

IlkhamGaysin · 2023-03-06T09:07:52Z

@jtannas
Could you please take a look, thank you so much! 🍻

IlkhamGaysin · 2023-03-27T09:56:20Z

@romanblanco I've fixed conflicts and updated the branch

rusllonrails · 2023-03-27T13:21:59Z

Looks good 👍 Plus 1 for merging 🥇

romanblanco · 2023-03-27T19:39:18Z

GitHub had issues earlier today. Re-opening the PR to try and run CI tests.

romanblanco

@IlkhamGaysin, thank you for the PR!

I like the suggestion, but I've tried to run the tests locally and they're failing as well as in the CI.

Please, take a look at https://github.com/rswag/rswag/blob/master/CONTRIBUTING.md#test and update the PR.

romanblanco · 2023-03-27T19:57:36Z

I'm also thinking that it would be really nice if you could update the test-app to provide an example of use of the strict option.

IlkhamGaysin · 2023-03-28T09:14:32Z

@romanblanco I will fix the specs and add a test case, thank you. I had a problem with docker locally so I relied on CI...

IlkhamGaysin · 2023-03-29T17:28:03Z

@romanblanco I've fixed the specs and added the test case. The GitHub Actions has issues but locally the test suites aregreen

a-lavis · 2023-03-30T03:09:14Z

rswag-specs/lib/rswag/specs/example_group_helpers.rb

@@ -132,7 +138,7 @@ def run_test!(&block)
          end

          it "returns a #{metadata[:response][:code]} response", rswag: true do |example|
-            assert_response_matches_metadata(example.metadata, &block)
+            assert_response_matches_metadata(example.metadata.merge(options), &block)


Instead of merging **options with metadata, could we pass **options to it?

def run_test!(**options, &block) options[:rswag] = true unless options.key?(:rswag) if RSPEC_VERSION < 3 ActiveSupport::Deprecation.warn('Rswag::Specs: WARNING: Support for RSpec 2.X will be dropped in v3.0') before do submit_request(example.metadata) end it "returns a #{metadata[:response][:code]} response", **options do assert_response_matches_metadata(metadata) block.call(response) if block_given? end else before do |example| submit_request(example.metadata) end it "returns a #{metadata[:response][:code]} response", **options do |example| assert_response_matches_metadata(example.metadata, &block) example.instance_exec(response, &block) if block_given? end end end

This should work, according to the README:

rswag/README.md

Lines 276 to 278 in 4043166

it 'returns a valid 201 response', swagger_strict_schema_validation: true do |example|

assert_response_matches_metadata(example.metadata)

end

This would allow us to pass in other options to it, which is a feature I'd like to have. For example, this would allow us to do the following:

run_test! focus: true

I could put this up in a separate PR after this one is merged if folks would prefer that, but I thought I'd mention it here since we're already adding the **options parameter.

I really like the idea 👍

Thank you! I put up a sub-PR here if that is helpful:
IlkhamGaysin#1

@romanblanco I'm gonna merge changes of @a-lavis to my PR to push it all together
are you ok with that?

@IlkhamGaysin, sounds good!

@IlkhamGaysin, I'll need you to rebase your branch once more because of the recent merge.

@romanblanco done!

* pass `options` to `it` * update README * Update README.md with one more example Co-authored-by: Ilkham Gaysin <ilgamgaysin@gmail.com> --------- Co-authored-by: Ilkham Gaysin <ilgamgaysin@gmail.com>

rswag-specs/lib/rswag/specs/example_group_helpers.rb

romanblanco

LGTM 👍

@IlkhamGaysin, @a-lavis, thank you guys!

romanblanco · 2023-07-19T11:50:31Z

Hey @IlkhamGaysin,

I'm trying to use this feature in a new API spec I'm working on, and I'm running into a problem with fields that are not required.

In README.md we have an example of how to mark required fields:

rswag/README.md

Line 117 in 0edbe25

required: [ 'title', 'content' ]

Using the same approach to describe pagination schema:

LINKS = {
  type: 'object',
  properties: {
    first: {
      type: 'string'
    },
    last: {
      type: 'string'
    },
    previous: {
      type: 'string'
    },
    next: {
      type: 'string'
    },
  },
  required: ['first', 'last']
}.freeze

If I have an example request that only has one page, I'm getting back error message:

...
       The property '#/links' did not contain a required property of 'previous' in schema 3efaf5d2-6a88-5939-8604-d152fa0e4574#
       The property '#/links' did not contain a required property of 'next' in schema 3efaf5d2-6a88-5939-8604-d152fa0e4574#
...

I suspect, that parametrizing JSON::Validator.fully_validate with { strict: true } does not take the required field into consideration.

I'm wondering if I'm using it wrong or if it is a bug. Any ideas?

a-lavis · 2023-07-19T12:08:40Z

@romanblanco

The strict option for the json-schema gem does two things:

All Properties are Required
Additional Properties are Not Permitted

Because of number 1, it will ignore what you set in the required field. All fields are required regardless.

In the future, we should take advantage of this new feature in json-schema: voxpupuli/json-schema#494

That will allow us to either make All Properties Required or make Additional Properties Not Permitted.

mattpolito · 2023-07-19T13:55:32Z

We ran into that as well when we manually made json-schema strict. The findings were the same as @a-lavis. Strict schema validation makes all declared keys required regardless of what is defined as 'required' in rswag.

IlkhamGaysin · 2023-09-14T12:16:57Z

@mattpolito @romanblanco Hi guys, sorry for the delay, I've been on vacation. The initial idea of the strict option is to strictly ask for the presence of all options listed. I can apply the new flexible changes from json-schema if you would like to have them.
To do that I need:

update the dependency in the rswag
add a new option and replace the existing one to fit the json-schema options

Are you ok with me doing that?

IlkhamGaysin · 2023-09-14T15:46:46Z

Here is the first PR guys
#675
@romanblanco @mattpolito

IlkhamGaysin · 2023-12-30T23:25:19Z

@mattpolito @romanblanco hey guys, here is the fix that will make the restrictions more flexible.
#721

IlkhamGaysin force-pushed the fix-164-issue branch from ff3ea5b to f2aee85 Compare March 5, 2023 21:13

frostmark approved these changes Mar 17, 2023

View reviewed changes

romanblanco added the enhancement label Mar 19, 2023

romanblanco linked an issue Mar 20, 2023 that may be closed by this pull request

Strict schema validation #163

Closed

romanblanco changed the title ~~Strict schema validation, closes #163~~ Strict schema validation Mar 20, 2023

romanblanco added this to the Gem 2.X.0 milestone Mar 20, 2023

IlkhamGaysin force-pushed the fix-164-issue branch from f2aee85 to 2a1ae07 Compare March 27, 2023 09:55

romanblanco closed this Mar 27, 2023

romanblanco reopened this Mar 27, 2023

romanblanco requested changes Mar 27, 2023

View reviewed changes

romanblanco self-assigned this Mar 27, 2023

IlkhamGaysin requested a review from romanblanco March 29, 2023 20:54

a-lavis reviewed Mar 30, 2023

View reviewed changes

IlkhamGaysin added 3 commits March 31, 2023 12:47

Strict schema validation, closes rswag#163

d548cb1

Fix specs, add an intergational test case for test-app

c97cacf

Update README

64ef801

IlkhamGaysin force-pushed the fix-164-issue branch from 307d138 to c645fb6 Compare March 31, 2023 09:48

pass options to it within run_spec (#1)

76711b1

* pass `options` to `it` * update README * Update README.md with one more example Co-authored-by: Ilkham Gaysin <ilgamgaysin@gmail.com> --------- Co-authored-by: Ilkham Gaysin <ilgamgaysin@gmail.com>

IlkhamGaysin force-pushed the fix-164-issue branch from c645fb6 to 76711b1 Compare March 31, 2023 09:50

IlkhamGaysin commented Mar 31, 2023

View reviewed changes

rswag-specs/lib/rswag/specs/example_group_helpers.rb Outdated Show resolved Hide resolved

IlkhamGaysin added 2 commits March 31, 2023 12:53

Update run_test! method docs

972b19e

Update CHANGELOG.md

ae8ba23

romanblanco approved these changes Mar 31, 2023

View reviewed changes

romanblanco merged commit 68aaeb6 into rswag:master Mar 31, 2023
10 checks passed

romanblanco removed this from the Gem 2.10.0 milestone May 8, 2023

romanblanco mentioned this pull request Jul 20, 2023

[REQUEST] Improve strict schema validation #666

Open

IlkhamGaysin mentioned this pull request Sep 14, 2023

Upgrade json-schema #675

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Strict schema validation #604

Strict schema validation #604

IlkhamGaysin commented Mar 5, 2023 •

edited by romanblanco

IlkhamGaysin commented Mar 6, 2023

IlkhamGaysin commented Mar 27, 2023 •

edited

rusllonrails commented Mar 27, 2023

romanblanco commented Mar 27, 2023

romanblanco left a comment

romanblanco commented Mar 27, 2023

IlkhamGaysin commented Mar 28, 2023 •

edited

IlkhamGaysin commented Mar 29, 2023

a-lavis Mar 30, 2023 •

edited

IlkhamGaysin Mar 30, 2023 •

edited

a-lavis Mar 30, 2023

IlkhamGaysin Mar 31, 2023

romanblanco Mar 31, 2023

romanblanco Mar 31, 2023

IlkhamGaysin Mar 31, 2023

romanblanco left a comment

romanblanco commented Jul 19, 2023

a-lavis commented Jul 19, 2023

mattpolito commented Jul 19, 2023

IlkhamGaysin commented Sep 14, 2023 •

edited

IlkhamGaysin commented Sep 14, 2023

IlkhamGaysin commented Dec 30, 2023

	it 'returns a valid 201 response', swagger_strict_schema_validation: true do \|example\|
	assert_response_matches_metadata(example.metadata)
	end

Strict schema validation #604

Strict schema validation #604

Conversation

IlkhamGaysin commented Mar 5, 2023 • edited by romanblanco

Problem

Solution

This concerns this parts of the OpenAPI Specification:

The changes I made are compatible with:

Related Issues

Checklist

Steps to Test or Reproduce

IlkhamGaysin commented Mar 6, 2023

IlkhamGaysin commented Mar 27, 2023 • edited

rusllonrails commented Mar 27, 2023

romanblanco commented Mar 27, 2023

romanblanco left a comment

Choose a reason for hiding this comment

romanblanco commented Mar 27, 2023

IlkhamGaysin commented Mar 28, 2023 • edited

IlkhamGaysin commented Mar 29, 2023

a-lavis Mar 30, 2023 • edited

Choose a reason for hiding this comment

IlkhamGaysin Mar 30, 2023 • edited

Choose a reason for hiding this comment

a-lavis Mar 30, 2023

Choose a reason for hiding this comment

IlkhamGaysin Mar 31, 2023

Choose a reason for hiding this comment

romanblanco Mar 31, 2023

Choose a reason for hiding this comment

romanblanco Mar 31, 2023

Choose a reason for hiding this comment

IlkhamGaysin Mar 31, 2023

Choose a reason for hiding this comment

romanblanco left a comment

Choose a reason for hiding this comment

romanblanco commented Jul 19, 2023

a-lavis commented Jul 19, 2023

mattpolito commented Jul 19, 2023

IlkhamGaysin commented Sep 14, 2023 • edited

IlkhamGaysin commented Sep 14, 2023

IlkhamGaysin commented Dec 30, 2023

IlkhamGaysin commented Mar 5, 2023 •

edited by romanblanco

IlkhamGaysin commented Mar 27, 2023 •

edited

IlkhamGaysin commented Mar 28, 2023 •

edited

a-lavis Mar 30, 2023 •

edited

IlkhamGaysin Mar 30, 2023 •

edited

IlkhamGaysin commented Sep 14, 2023 •

edited