Skip to content

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

Open
aleung wants to merge 1 commit into
Redocly:mainfrom
aleung:feat-only-explicit-example
Open

Feature: Don't auto generate sample for non-required object properties#123
aleung wants to merge 1 commit into
Redocly:mainfrom
aleung:feat-only-explicit-example

Conversation

@aleung

@aleung aleung commented May 25, 2021

Copy link
Copy Markdown

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

aleung commented Dec 1, 2021

Copy link
Copy Markdown
Author

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

@RomanHotsiy

Copy link
Copy Markdown
Member

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

@aleung

aleung commented Dec 2, 2021

Copy link
Copy Markdown
Author

@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
Copy Markdown
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