search

spring-projects / spring-data-rest

Simplifies building hypermedia-driven REST web services on top of Spring Data repositories
https://spring.io/projects/spring-data-rest
Apache License 2.0
905 stars 558 forks source link

Concurrency issue with JPA Lists [DATAREST-895] #1264

Open spring-projects-issues opened 7 years ago

spring-projects-issues commented 7 years ago

Laurent Bovet opened DATAREST-895 and commented

I have a OneToMany association with a JoinTable: https://github.com/lbovet/bugtik/blob/jointable-concurrency-issue/src/main/java/li/chee/bugtik/Project.java#L20

When creating associations in parallel (POST URIs to the collection resource concurrently), There is a race condition where the previous collection content overwrites a more recent one. This results in not all the associations created.

Note that if I model the association with a JoinColumn, the problem does not arises: https://github.com/lbovet/bugtik/blob/joincolumn-concurrency-ok/src/main/java/li/chee/bugtik/Project.java#L20

I think it is a bug that Spring Data Rest operations are not performed atomically in the JoinTable case. If not, is there a configurative way to solve this?

You can checkout the sample project (https://github.com/lbovet/bugtik.git) and try both branches: mvn spring-boot:run http://localhost:8080/concurrency-test.html

Affects: 2.4.4 (Gosling SR4), 2.5.2 (Hopper SR2)

Reference URL: https://github.com/lbovet/spring-data-rest/tree/issue/DATAREST-895

spring-projects-issues commented 7 years ago

Oliver Drotbohm commented

mvn clean test succeeds on the root project. Am I missing anything?

spring-projects-issues commented 7 years ago

Laurent Bovet commented

The test is written in javascript and runs in the browser. You need to start the application:

mvn spring-boot:run

And open the html jasmine test runner:

http://localhost:8080/concurrency-test.html

(be sure to checkout branch jointable-concurrency-issue)

spring-projects-issues commented 7 years ago

Oliver Drotbohm commented

Well, no. I won't dive into this unless it's a plain integration test or even a unit one. We have to focus on a minimal reproducing example as everything added unnecessarily is both time consuming and opens up additional vectors for failure. Also, if there's a fix needed I will have to add test to Spring Data REST itself. And theres no way we're gonna add anything beyond plain Spring MVC tests.

That said, I'd completely understand, if you don't want / can't invest more time to simplify things. Happy to look into it my self entirely. :)

spring-projects-issues commented 7 years ago

Laurent Bovet commented

Ok, I'll try to find time to rewrite the jasmine test as a springmvc test. Is there any integration test somewhere I can base upon? I need JPA and spring-data-rest-webmvc. Also note that as it happens only with concurrent requests, the reproducibility is not 100% guaranteed.

If you still have the five minutes needed to look at the jasmine test in your browser, that may help to figure out if I was simply misguided in how I use spring data rest or if it is something to be fixed.

spring-projects-issues commented 7 years ago

Laurent Bovet commented

I rewrote as an integration test, directly mergeable into spring-data-rest-tests:

https://github.com/lbovet/spring-data-rest/tree/issue/DATAREST-895

This highlights that the race condition occurs when using List for collections (instead of Set). I suspect that the ordering constraint provokes a rewrite of the whole collection, overwriting.

Note: I signed the CLA, so you can merge this into spring data codebase

spring-projects-issues commented 7 years ago

Laurent Bovet commented

Any news on this?