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

Behaviour of __removeRequired when using selection #24

Open
maritter opened this issue May 18, 2020 · 4 comments
Open

Behaviour of __removeRequired when using selection #24

maritter opened this issue May 18, 2020 · 4 comments

Comments

@maritter
Copy link

When using the jsonSchema function on the mongoose model together with a selection, the required property is completely removed. Wouldn't it be better to check, which properties are going to be included and only delete those from the required field, that are excluded?

Example:

const BookSchema = new Schema({
 title: { type: String, required: true },
 year: { type: Number, required: true }
});
const Book = mongoose.model('Book', BookSchema);
Book.jsonSchema('title');

Response would look like:

{
  title: 'Book',
  type: 'object',
  properties: {
    title: { type: 'string'  },
  },
 required: [ 'title' ]
}

instead of

{
  title: 'Book',
  type: 'object',
  properties: {
    title: { type: 'string'  },
  }
}
@DScheglov
Copy link
Owner

@maritter , hi,
Thanks for the issue.
I considered such behavior on early stages and decided to remove "required" from the model, when using "selection".

The motivation is:

  • when we specify fields to be selected we want to build schema of object that we return to api consumers. So, we should not check if it contain all required field. That's why "required" is absent on the selection.

So, if you need the "required", could you describe your use case. It can help to create more suitable solution (actually to implement proposed behavior could be such solution).

@maritter
Copy link
Author

Hi,
thx for your fast reply.

I'll try to explain our use case:
We want to use your library to generate our api documentation using swagger/openapi. We have some required properties on the model that are generated on the server, such as 'createdBy'. Those should not be present in the schema for the POST/PATCH requests.

While writing this down another point came to my mind:
In addition to above a property that is required in the POST request might not be needed in the PATCH request

Example:
model schema

const BookSchema = new Schema({
 title: { type: String, required: true },
 year: { type: Number },
 createdBy: { type: ObjectId, required: true }
});

const Book = mongoose.model('Book', BookSchema);
Book.jsonSchema(['title', 'year']);

schema for POST request

{
  title: 'Book',
  type: 'object',
  properties: {
    title: { type: 'string' },
    year: { type: 'number' }
  },
  required: [ 'title' ]
}

schema for PATCH request

{
  title: 'Book',
  type: 'object',
  properties: {
    title: { type: 'string' },
    year: { type: 'number' } 
  }
}

any ideas on this? Maybe we have to alter the json schema after the creation.

@DScheglov
Copy link
Owner

DScheglov commented May 22, 2020

@maritter Hi, yes, I have an idea. But you don't like it ...

You have different (at least) three different messages in you API:

  • CreateBookInput
  • Book
  • PatchBookInput

So, the correct solution is to define three schemas for these messages and then get their json schemas.

You can use some mongoose practices to re-use schemas to avoid code repeating.

However, ) probable I will consider something like: inputJsonSchema that will work with required fields in more strict ways

@maritter
Copy link
Author

@DScheglov
Hi, thanks for your feedback.
Guess you are right, maybe we have to give our implementation a second thought.

Anyhow, I think a different implementation for the required handling would be good, maybe as suggested by you within a new method.

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

No branches or pull requests

2 participants