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 PUT operations #2

Closed
datenbrille opened this issue Aug 31, 2017 · 5 comments
Closed

Support for PUT operations #2

datenbrille opened this issue Aug 31, 2017 · 5 comments

Comments

@datenbrille
Copy link

Hi,

we replaced our own handcrafted library with this awesome project. So fare we have not problems at all, except one. We would like to update our entities. I read the comment about problems with PUT operations.

The old library had no problems in updating the entities and the relations. To achieve this we handcrafted the list of URI's for the relations and used the RestTemplate with the HTTP PUT. As I understood from the code, you are doing this in more generic approach. That is the reason why we want to migrate to bowman.

Are there any plans to support PUT? Do you strongly advise against it?

Best Karl

@hdpe
Copy link
Owner

hdpe commented Sep 1, 2017

Hi and thanks for your feedback!

First off, I'd like to say Bowman intends to be a generic HAL client library, and as such PUT support would definitely be desirable. Pull requests would be very welcome for this. However, I am unlikely to work on this myself in the near future as it is not something required in our usual use cases.

Bowman was originally designed to simplify the querying and manipulation of JPA-mapped entities exposed via Spring Data Rest endpoints. PUT support has been omitted for two reasons:

  • The set-up, query and tear-down of data for automated acceptance tests -- our principal use case -- comparatively rarely has a need for the update of data (PUT/PATCH); and
  • The PUT operation, at least on Spring Data JPA repositories, is (or at least, was) in my opinion quite poorly supported in Spring Data Rest.

There were various problems using PUT with SDR when I first found the need for this library, and a cursory look at the latest SDR version showed that similar issues still exist - see for example this JIRA issue and related discussion. There seems to be quite some discussion between the project maintainers and users about how PUT is even supposed or perceived to work.

Consequently, I have found the best workaround is always to define custom SDR endpoints using separate command objects for update operations. You can then define the behaviour of the merge exactly as you like, and avoid problems with PUT on the default endpoints (such as persistent collections being overwritten by the incoming ones and the merge happening outside of a transaction.)

However, as I say, Bowman shouldn't be all about Spring Data REST, and it would be good if PUT did work for other backends. Happy to consider any PRs! If you are using Spring Data REST, I'd be really interested to know how your handcrafted approach got around issues like the ones described above.

@datenbrille
Copy link
Author

Hi and thanks for the answer,

I will have a look at the issues and try to reproduce it in my project. I have roughly implemented the put operation in bowman and I will prepare a PR. I have some troubles with checkstyle and trying to configure IDEA to automatically format the source according to them. Do you prefer tabs or spaces in your code? I have found some parts in spaces and some in tabs.

Thanks for this nice piece of software.

@hdpe
Copy link
Owner

hdpe commented Sep 1, 2017

There's an IDEA code style file you can use here - I will add this to the development guide...

@datenbrille
Copy link
Author

I have looked into the issues and the mentioned links. Indeed we update only the base properties of the entity and not the relations. The relations gets updated in a separate call to SDR repository. Honestly I agree with Oliver Gierke and I like the approach of updating the entity in a two step approach. Can we leave this issue open? I would like to reference it in the PR.

datenbrille referenced this issue in innogames/bowman Sep 1, 2017
datenbrille referenced this issue in innogames/bowman Sep 1, 2017
datenbrille referenced this issue in innogames/bowman Sep 4, 2017
hdpe pushed a commit that referenced this issue Sep 4, 2017
hdpe pushed a commit that referenced this issue Sep 4, 2017
@hdpe
Copy link
Owner

hdpe commented Sep 4, 2017

Closed by #3

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