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

Support for non-updatable fields [DATAREST-530] #905

Open
spring-projects-issues opened this issue May 5, 2015 · 8 comments
Open

Support for non-updatable fields [DATAREST-530] #905

spring-projects-issues opened this issue May 5, 2015 · 8 comments
Assignees
Labels
status: feedback-provided type: enhancement

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented May 5, 2015

Dave LeBlanc opened DATAREST-530 and commented

Currently with spring-data-jpa and SDR, when we submit an update to an entity that has a non-updatable property (@Column(updatable=false)) we don't get an error message when we try to edit that property - the update won't happen, but we don't get any error notification.

If there's an reasonable way to implement this, my preference would be for an update attempt to that field to throw an error


Issue Links:

  • DATAREST-531 JSON Schema should expose read-only fields based on PersistentProperty.isWritable()

  • DATAJPA-716 JpaPersistentProperty should consider updatable flag of mapping annotations

Referenced from: commits 70a2488, f05a6ba

0 votes, 5 watchers

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 5, 2015

Oliver Drotbohm commented

Actually, that's the way @Column is specified - the value not being contained in the SQL update statement. It's not supposed to throw an error in the first place. Following Postel's law that's actually the right way to implement it, at least by default.

What we could do though is inspecting certain annotations and consider the fact that they basically are read-only properties in the JSON Schema that we expose. Spring Data Commons already has an @ReadOnlyProperty that could be generally used to mark such properties. I am not really sure actively rejecting changed values is a good idea here.

My hesitance aside, what kind of error (HTTP status code) were you imagining, actually?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 8, 2015

Dave LeBlanc commented

Well, speaking for myself I'm ok with it simply not persisting changes to the entity. The group I'm working with was expecting an error message to be thrown when trying to update a read-only field -- probably with a 403 Forbidden status code.

Alternatively, if there was a reasonable way to hook in to SDR to provide such a mechanism ourselves (so it's not the default, but still possible) this might be interesting

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 15, 2015

Ben Madore commented

I, for one, would love a mechanism to expose somethign as readOnly in the json schema. - seems like to ensure it's a match with serialization, instead of using hte JPA annotation, you could use the upcoming jackson ```
@JsonIgnore(allowSetters="false")

https://github.com/FasterXML/jackson-databind/issues/95

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 15, 2015

Ben Madore commented

Also - for read only properties, it'd be great to not only expose that they're 'readOnly' but also to expose the schema for the non-entity object. In general it would be nice to expose a mechanism to provide the schema for non-entity objects as that's a fairly common use case where you might need to return a computed value on your API (e.g. I have a ```
List<DailyHours>

DailyHours getTodaysHours()


Afaict this is not supported - at least, all I get in my schema is:
"todaysHours" : {
  "readOnly" : false
}

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 16, 2015

Oliver Drotbohm commented

As mentioned above @ReadOnlyProperty can be used on such properties. You might have to switch on property access to trigger annotation detection on non-field-backed methods. DATAREST-531 introduced support for that. Let's keep this ticket focused on the request handling with properties submitted that are read-only properties actually

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 5, 2017

Adam Rosini commented

My issue is that when I PATCH an update, the returned response body looks like the read-only fields were updated. After inspecting the database, they are not because they are excluded from SQL via the @Column(updatable = false) annotation. After looking around, it looks like SimpleJpaRepository#save just performs a merge, which does not respect the previously mentioned annotation.

Having an error be thrown would be fine for my use case and I think actually makes sense, I see it as a 400 or 403. So +1 for this

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 25, 2018

Massimiliano Bigatti commented

I'm experiencing the same issue described by Adam: with a PATCH request the response body contains the updated value for the field annotated as updatable = false, but the database still have the old value. I think the operation should return the entity as it is represented in the database, I see this kind of response as inconsistent

@spring-projects-issues spring-projects-issues added status: waiting-for-feedback type: enhancement labels Dec 31, 2020
@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jan 7, 2021

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder label Jan 7, 2021
@gregturn gregturn added status: feedback-provided and removed status: feedback-reminder status: waiting-for-feedback labels Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-provided type: enhancement
Projects
None yet
Development

No branches or pull requests

3 participants