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

Feature: Don't auto generate sample for non-required object properties #123

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aleung
Copy link

@aleung aleung commented May 25, 2021

Add an option to disable sample auto generation for non-required object properties when the schema hasn't explicit example nor default value.

This MR is for Redocly/redoc#1490 (comment)

@aleung aleung force-pushed the feat-only-explicit-example branch from ce6e6ec to ccab373 Compare December 1, 2021 05:53
@aleung
Copy link
Author

aleung commented Dec 1, 2021

@RomanHotsiy I have rebased it on latest commit on upstream.

@RomanHotsiy
Copy link
Member

@aleung how this option is different from skipNonRequired we already have?

@aleung
Copy link
Author

aleung commented Dec 2, 2021

@RomanHotsiy

When skipNonRequired is enabled, only the required properties are left. The new option will keep optional properties which have explicit example or default/const value. It just try to remove the auto-generated values which might be invalid in business. Below test cases show the difference:

    it('should skip non-required properties without explicit example value', () => {
      res = sampleObject({
        required: ['e'],
        properties: {
          a: { type: 'string', enum: ['foo', 'bar'] },
          b: { type: 'integer', default: 100 },
          c: { type: 'string' },
          d: { type: 'string', example: 'Example' },
          e: { type: 'string', enum: ['foo', 'bar'] },
          f: { type: 'string', const: 'const' },
        },
      }, { disableNonRequiredAutoGen: true });
      expect(res).to.deep.equal({
        b: 100,
        d: 'Example',
        e: 'foo',
        f: 'const',
      });
    });

    it('should skip non-required properties', () => {
      res = sampleObject({
        required: ['e'],
        properties: {
          a: { type: 'string', enum: ['foo', 'bar'] },
          b: { type: 'integer', default: 100 },
          c: { type: 'string' },
          d: { type: 'string', example: 'Example' },
          e: { type: 'string', enum: ['foo', 'bar'] },
          f: { type: 'string', const: 'const' },
        },
      }, { skipNonRequired: true });
      expect(res).to.deep.equal({
        e: 'foo',
      });
    });

The rationale of adding this option is:

  1. The example must conform to the schema. Object is invalid if it misses any required properties.
  2. On top of the premise that the first rule is met, avoid to auto-generate value for a property, because the value and/or their combination in an object might be invalid to the API business logic.

An API request/response body has a lot of optional properties. Sometimes it's too heavy to give example to every properties; but we want to ensure reader understand the usage of some important properties so we give example with "real" value for those properties only to make the whole request/response example reflect to a business use case. It isn't support in current software, so I propose this disableNonRequiredAutoGen option (the naming may not be good enough).

@RomanHotsiy
Copy link
Member

I see. I would spend a little more time to think about the name of this option to make it more clear.

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.

2 participants