AfterSetContentEvent's source have been updated ,but repoInvoker'invokeSave still save the stale domainObj

[lmtoo]

hi, @paulcwarren , I defined a DocumentStoreEventHandler class which annotated with @StoreEventHandler, and a afterSetContent methd which annotated with @HandleAfterSetContent, like this

    @HandleAfterSetContent
    @Order(Ordered.HIGHEST_PRECEDENCE)
    public void afterSetContent(AfterSetContentEvent event) {
        if (event.getSource() != null && event.getSource() instanceof Document) {
            Document document = (Document) event.getSource();
            documentContentPostProcessors.orderedStream().forEach(it -> it.postProcessAfterSetContent(document, event.getStore().getResource(document)));
        }
    }

postProcessAfterSetContent have changed the Document ,which create a new workingCopy, also , invoked DocumentRepository's saveAndFlush method, but ContentServiceHandlerMethodArgumentResolver's setContent still to save stale domainObj which have been changed in persist Context or db,

Object updatedDomainObj = ReflectionUtils.invokeMethod(methodToUse, targetObj, (embeddedProperty == null ? domainObj : embeddedProperty), contentArg);
repoInvoker.invokeSave(embeddedProperty == null ? updatedDomainObj : domainObj);

and will lead the following exception:

2020-09-28 12:35:19.784 ERROR 95748 --- [ XNIO-1 task-1] c.l.r.s.SpringFrameworkExceptionHandler : Object of class [com.lubansoft.infrastructure.document.Document] with identifier [1426]: optimistic locking failed; nested exception is org.hibernate.StaleObjectStateException: Row was updated or deleted by another transaction (or unsaved-value mapping was incorrect) : [com.lubansoft.infrastructure.document.Document#1426]

org.springframework.orm.ObjectOptimisticLockingFailureException: Object of class [com.lubansoft.infrastructure.document.Document] with identifier [1426]: optimistic locking failed; nested exception is org.hibernate.StaleObjectStateException: Row was updated or deleted by another transaction (or unsaved-value mapping was incorrect) : [com.lubansoft.infrastructure.document.Document#1426]

[paulcwarren]

Thanks for the report @lmtoo.

So, in general, you need support in the event handlers for changing the associated entity.

Can I clarify, when you say workingCopy. Are you referring to a LockingAndVersioningRepository.workingCopy?

[lmtoo]

There are something I did in @ HandleAfterSetContent:

1. if this eneity is a pdf , I will calculate total pages of this eneity, and call JpaRepository's saveAndFlush .
2. if this entity is a word, I will use LockingAndVersioningRepository.workingCopy create a newCopy and call JpaRepository's saveAndFlush for wordToPdf conversion task .

[paulcwarren]

Hi @lmtoo,

I spent some time trying to repro this but was unable to. I also inspected the code and it seems like the entity flows through the eventhandlers and setcontent operation as I would expect (to be able to support entity updates). I put together a test that registers an aftersetcontent event handler that saves and flushes the entity and creates a new working copy. All seemed to work ok.

One thing I wonder if you code is somehow performing the save and flushes/workingCopy in a different thread, or something like that?

If that draws a blank I will check in my same project and we can collaborate on it to see if we can figure it out together.

[lmtoo]

ok, I will try to reproduce this later

[paulcwarren]

OK @lmtoo. Let's work on it together. I will commit my sample project so you dont have to start from scratch.

[paulcwarren]

Created this repo to collaborate on.

Check SpringBootApplication class and you should see a registered after set content event handler.

Run Issue373Test and it should be green.

If you can diff this against your repo and figure out what is different we'll probably find the issue.

[lmtoo]

hi @paulcwarren what is JpaLockingAndVersioningProxyFactoryImpl used for. in your demo, OptimisticLockingInterceptor is always invoked ,but in my application OptimisticLockingInterceptor is always dones't invoke

[lmtoo]

I reproduced this bug, It's here https://github.com/paulcwarren/issue-373/pulls

[paulcwarren]

Thanks @lmtoo. That's great.

Of course you are using mongo. I should have thought about that (perhaps I should add an Issue template to capture these details).

The JpaLockingandVersioningProxyFactoryImpl add actually add the pessimistic and optimistic locking semantics to a store. So, if your entities are stored in mongo then we basically need a spring-versions-mongo implementation.

I will look into this.

[lmtoo]

FilesystemStoreFactoryBean make use of LockingAndVersioningProxyFactory ,but MongoStoreFactoryBean doesn't

[paulcwarren]

@lmtoo

I pulled in your PRs and I see that spring-versions-jpa is still a dependency.

So is it your intention to use mongo to store entities/content but a relational db for the locking sub-system? In which case you want spring-versions-jpa integrated into the spring-content-mongo?

Or do you expect the locking sub-system to also be persisted in mongo? In which case you would eventually expect to swap out spring-versions-jpa replacing it with spring-versions-mongo?

[lmtoo]

@paulcwarren entity persist to mysql by using spring-data-jpa, so I introduce spring-versions-jpa to, but entity's content use mongodb to persist, so I intruduce spring-versions-jpa to make use LockingAndVersioningRepository.
I don't want locking sub-system to be persisted in mongo for now, as entity is persist in mysql already.

[lmtoo]

Hi, @paulcwarren when I config with putAndPostPreferResource, will case the same problem , OptimisticLockingInterceptor's static field setContentMethod is not equals to S setContent(S property, Resource resourceContent);

[paulcwarren]

Which version of Spring Content are you trying @lmtoo? I recently added a fix to the REST layer to lazy load stores from the application context to ensure that we get the proxy wrapped in the right set of proxy objects, including the one for optimistic locking. That fix was in 1.1.0.M5. Are you trying that one?

[lmtoo]

yes, I'm trying 1.1.0.M5 @paulcwarren

[lmtoo]

I created a pull request for 1.1.0.M5 paulcwarren/issue-373#3

[paulcwarren]

Thanks. I am able to repro the issue with that. Looks like the event handler is updating the entity but that updated entity is being ignored by the framework.

[paulcwarren]

Hi @lmtoo,

I discovered that lock, unlock and workingCopy were all making unnecessary calls to em.merge (sometimes via LockingAndVersioningRepository.save). I removed these calls.

This makes the tests on our test project now work. I accepted your PR and made a second commit that updated it to use Spring Content 1.2.0-SNAPSHOT.

But I committed this on the new 1.2.x branch (supporting Spring Boot 2.4). Let me know if you are not able to upgrade to that as I should be able to backport this fix to 1.1.x branch.

[lmtoo]

Hi, @paulcwarren I upgrade to new version ,but still can't config restConfiguration.forDomainType(File.class).putAndPostPreferResource();
paulcwarren/issue-373#4

[paulcwarren]

Are you getting this error again when you run the test?

org.springframework.orm.ObjectOptimisticLockingFailureException: Object of class [gettingstarted.File] with identifier [1]: optimistic locking failed; nested exception is org.hibernate.StaleObjectStateException: Row was updated or deleted by another transaction (or unsaved-value mapping was incorrect) : [gettingstarted.File#1]

[lmtoo]

yes

[paulcwarren]

Yeah, I re-opened this bug because I had to revert some of the commits that originally addressed this as they ended up being bad commits (causing a bunch of issues elsewhere).

The actual problem here is that the event handler modifies the subject object of the event (locks and creates a working copy of) and because the entity is @Versioned that increments its @Version number making the event object stale. Later on when we try to save the event object it therefore throws this error.

I am not sure what to do about this atm.

The standard event mechanism treats the event object as a read-only so I am reluctant to modify that to add a setter (allowing event handlers to reset the event object). That feels like it runs counter to Spring's original design for the event mechanism.

Spring Data REST event handling has always been up at the REST layer but moving SCs to the same layer would be both problematic and I am not sure would actually solves the problem.

This might be one of those occasions where the right thing to do is create a custom Controller for the setContent operation and add you event handler logically directly to your Controller method.

[lmtoo]

if I config restConfiguration.forDomainType(File.class).putAndPostPreferResource(); the test don't pass, but if I delete this config the test is passed