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

Unknown validator exception for read_only when using requires/optional with Entity #2345

Open
michebble opened this issue Aug 15, 2023 · 2 comments
Labels

Comments

@michebble
Copy link

Hello Grape team,

I am using grape-entity objects for defining my endpoint's parameters, and I ran across this issue.

My entity exposes ID as a read only value, as our back end controls this value

 expose :id, documentation: {type: String, read_only: true}
 # .. other attributes

When defining a PUT request that uses the entity documentation for defining the params it generates an error

Failure/Error: requires :all, using: Entity.documentation.except(:created_at, :updated_at)

Grape::Exceptions::UnknownValidator:
  unknown validator: read_only

I found #2338 which fixed a similar issue for other documentation keys, i.e. is_array, which have been using. I noticed in the PR @mscrivo mentions there maybe other keywords that belong in RESERVED_DOCUMENTATION_KEYWORDS and I wonder if this is one of them?

At first I thought it wasn't, but as a PUT request requires an ID to determine which record to update and our back end controls ID creation, then ID should be marked as read_only to inform users of the API that they cannot change or supply their own value.

If this thinking is correct, then I would be happy to create a PR to include read_only in RESERVED_DOCUMENTATION_KEYWORDS. If not then I will re-evaluate my approach.

@mscrivo
Copy link
Contributor

mscrivo commented Aug 15, 2023

seems reasonable to me to add to the RESERVED_DOCUMENTATION_KEYWORDS if there is no validation for it. Though I think the intent of read only fields is not for indicating that an id on a PUT cannot be changed (I think that's always implied? required prop is more apt for ids), but more for using the same entity for GETs/POSTs/PUTs and those marked as read only would only come back in responses and not used in requests. The docs say this:

You can use the readOnly and writeOnly keywords to mark specific properties as read-only or write-only. This is useful, for example, when GET returns more properties than used in POST – you can use the same schema in both GET and POST and mark the extra properties as readOnly. readOnly properties are included in responses but not in requests, and writeOnly properties may be sent in requests but not in responses.

@dblock dblock added the bug? label Aug 15, 2023
@michebble
Copy link
Author

Thank you for the direction to the relevant part of the docs.
For now I will not mark the ID as read only, as it seems I am using it incorrectly. However I would agree that adding these keywords to the RESERVED_DOCUMENTATION_KEYWORDS is the right thing to do.

I appreciate the work you are doing with Grape.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants