Skip to content

Conversation

@yelizhenden-mdb
Copy link
Collaborator

@yelizhenden-mdb yelizhenden-mdb commented Mar 4, 2025

Proposed changes

Jira ticket: CLOUDP-304055

The Create method request body should be a Request suffixed object

Assumptions:

  • Create method is defined by POST method for REST APIs. This rule validates against the POST method requests
  • Multiple media types (versions) for requestBody content can exist. API producers can define exceptions for each of them, adding JavaExtensionProperty at @Content level
  • The schema of content should refer to a predefined Request-suffixed schema. If the schema is defined inline, it is against the guideline

Checklist

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works

Changes to Spectral

  • I have read the README file for Spectral Updates

Further comments

@yelizhenden-mdb yelizhenden-mdb marked this pull request as ready for review March 4, 2025 17:31
@yelizhenden-mdb yelizhenden-mdb requested a review from a team as a code owner March 4, 2025 17:31
Comment on lines +43 to +44
| xgen-IPA-106-create-method-request-body-is-request-suffixed-object | The Create method request should be a Request suffixed object. http://go/ipa/106 | warn |

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you point me to where this is expected in the IPA 106? I disagree with enforcing this particular rule because it creates unnecessary redundancy in our codebase since developers are forced to duplicate views with only minor name changes

Copy link
Contributor

Choose a reason for hiding this comment

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

API producers should implement as a Request suffixed object
A Request object must include only input fields

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah I missed it. Thanks

I’m curious about the reasons behind prefixing view objects with “Request”. This naming convention concerns me because it might force us to create duplicate view definitions with different names, when we could potentially use Schema.AccessMode to maintain a single view that adapts based on context. Am I missing something? Maybe I misunderstood the proposed change 🤔

Copy link
Collaborator Author

@yelizhenden-mdb yelizhenden-mdb Mar 5, 2025

Choose a reason for hiding this comment

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

You can think of CreateRequest suffixed schemas as an example for this guideline. View suffixed schemas are being used for responses as far as I know

Copy link
Contributor

Choose a reason for hiding this comment

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

I’m curious about the reasons behind prefixing view objects wit

It's a somewhat recommend practice that helps isolate changes between payloads, some feel strongly it helps to make clear what's write only vs read only while like you some have concerns on duplication

Copy link
Collaborator

@andreaangiolillo andreaangiolillo left a comment

Choose a reason for hiding this comment

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

LGTM

@yelizhenden-mdb yelizhenden-mdb merged commit a260a2a into main Mar 5, 2025
13 checks passed
@yelizhenden-mdb yelizhenden-mdb deleted the CLOUDP-304055 branch March 5, 2025 15:32
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.

3 participants