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

Add lifecycle events [DATAES-587] #1159

Closed
spring-projects-issues opened this issue Jun 6, 2019 · 8 comments
Closed

Add lifecycle events [DATAES-587] #1159

spring-projects-issues opened this issue Jun 6, 2019 · 8 comments
Labels
in: core Issues in core support type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link

sothawo opened DATAES-587 and commented

spring-data-mongodb has [lifecycle events|https://docs.spring.io/spring-data/mongodb/docs/current/reference/html/#mongodb.mapping-usage.events.] spring-data-elasticsearch should have something similar


Reference URL: https://stackoverflow.com/questions/56426467/is-it-possible-to-intercept-a-save-or-load-event-in-spring-data-elasticsearch

Issue Links:

  • DATAES-68 Add support for auditing annotations

  • DATAES-771 Add after-save entity callbacks support

  • DATAES-772 Add after-convert entity callbacks support

@spring-projects-issues
Copy link
Author

sothawo commented

Mark Paluch, Christoph Strobl what do you think? To implement this would probably mean to extract some {AbstractEventListener} and event classes into spring-data-commons to reuse it?

@spring-projects-issues
Copy link
Author

Mark Paluch commented

Having lifecycle events makes sense. Typically, each store defines its own events because there are store-specifics in each event. You might want to look at MongoDB to get an idea how it is done there.
Right now, we're quite late (RC1 is due next week) and we should not add major features after RC1. Also, with the intended rewrite of ES4 it's likely that the effort required to add event support is now higher than after the rewrite

@spring-projects-issues
Copy link
Author

sothawo commented

I wouldn't have started working on this before Moore is out and the work is on 4. I created this issue because it came up on SO today and I think it might be a good addition so it gets not lost and just was curious what you are thinking about it ;)

@spring-projects-issues
Copy link
Author

Roman Puchkovskiy commented

Ok, while I was publishing a pull request for DATAES-771 it was closed as a duplicate of this one. Still, I believe the PR may be of use: #414

Please let me know how we should proceed from here

@spring-projects-issues
Copy link
Author

Roman Puchkovskiy commented

Hi Peter!

  1. In spring-data-mongodb, AfterSaveCallback's method accepts 3 parameters: the saved entity, the document returned by MongoDB upon saving, and collection name. Elasticsearch does not return any document upon indexing, so no document is needed in spring-data-elasticsearch's AfterSaveCallback. But collection name's analogue (IndexCoordinates) is available everywhere [Reactive]AfterSaveCallback is invoked. My suggestion is to add IndexCoordinates as a second parameter to the callback method. What do you think?

  2. Same question about before-convert. In spring-data-mongodb, collection name is passed, here we can pass IndexCoordinates as well. Should I make this change?

  3. If answers to 1 and 2 are yes, should I file separate tickets for them? If not, what ticket should be used?

  4. Also, it looks like I found a bug in before-convert implementation (for batch saves  in reactive case, the callback is  not called). Should I file a separate ticket in Jira to work on it, just work on it in this ticket?

 

Sorry for questions 3 and 4, I don't want to mess this up :)

@spring-projects-issues
Copy link
Author

sothawo commented

Adding the IndexCoordinate is ok,

please make one ticket for all of these fixes.

 

@spring-projects-issues
Copy link
Author

Roman Puchkovskiy commented

I've added DATAES-785

@spring-projects-issues
Copy link
Author

sothawo commented

marking this as resolved, as we have the implementations for the basic events. If more events are needed, new tickets should be created for them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core support type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

1 participant