Add after-save entity callbacks support [DATAES-771]

[spring-projects-issues]

Roman Puchkovskiy opened DATAES-771 and commented

At the moment of writing (2020-03-28), spring-data-elasticsearch only supports before-convert callbacks.

after-save callbacks are implemented in spring-data-mongodb. A justification for their usefulness can be found here: https://jira.spring.io/browse/DATAMONGO-2479?focusedCommentId=188194&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-188194

I'm going to publish a pull request implementing after-save callbacks (both reactive and non-reactive flavors)

---

Affects: 4.0 RC1 (Neumann)

Issue Links:

• DATAES-587 Add lifecycle events

Referenced from: pull request #414

[spring-projects-issues]

sothawo commented

This is a duplicate of DATAES-587. The fact that currently only the before-convert callback is implemented, is because this was needed to implement auditing. We should not add single calbacks one by one, but make a complete solution for that

[spring-projects-issues]

Roman Puchkovskiy commented

Ok, while I was publishing the following pull request for this issue https://github.com/spring-projects/spring-data-elasticsearch/pull/414 the issue got closed.

How should we proceed? I'm interested in adding after-save and after-convert callbacks for the use case mentioned above

[spring-projects-issues]

sothawo commented

Please first check Jira if there is already an issue for a problem you have. If not, create it first so that there is time to discuss an issue.

It is always hard to judge an issue and PR, if the issue is submitted together with an already implemented PR

[spring-projects-issues]

sothawo commented

as callbacks process entities - and return possibly modified ones, on what should an after-convert callback operate?

[spring-projects-issues]

Roman Puchkovskiy commented

I'm sorry, it's my bad that I did not search for a Jira ticket.

Actually, I am ready to rename the branch and re-create a PR, if needed, to 'migrate' them to DATAES-587. It's just that I don't know what is the best course of action right now.

 

As for your question on after-convert: I've tried to model callbacks on what events exist in spring-data-mongodb. For the load path, they have after-load (that is given a BSON Document and entity type, but not the entity as it is not yet materialized at this stage) and after-convert (that is given the BSON Document and the materialized entity).

In spring-data-elasticsearch, there is an abstraction also called Document that seems to be similar to the MongoDB's counterpart. For example, the ElasticsearchOperations#get() implementation for Rest template flavor has the following code:

 

GetResponse response = execute(client -> client.get(request, RequestOptions.DEFAULT));
return elasticsearchConverter.mapDocument(DocumentAdapters.from(response), clazz);

 

Document is conceptually a Map (like MongoDB's Document) that contains the document data.

So it seems natural to take the same decisions: give both Document and the entity materialized from it to after-save callback (and index name, probably)

[spring-projects-issues]

sothawo commented

Leave this PR and this ticket, all merge this later. DATAES-587 is a more general one. It probably makes more sense to add new tickets for new callbacks/events, and add them as related to that ticket and then close 587 when all the stuff is done.

As for other callbacks: after-load is ok, as there is an entity loaded that can be passed through the callback.

About callbacks that would be operating on the Document: The Document class is modelled after what is available in mongodb, Mark took it over from there. But in Spring Data Elasticsearch 4 it is an internally used class. One of the biggest problems we had in the pre-4 versions was that Jackson was used to convert an entity to the JSON needed for elasticsearch and people were trying to configure that Jackson instance to do date conversions, field renamings and other stuff. But these customizations could not be taken into account when for example queries were built or the index mappings were created. So now all these tricky conversions are done in the MappingElasticsearchConverter which transforms an entity to a Document and back. The last conversion between Document and JSON is still done by an internal, not accessible Jackson instance.

So for me the question is: why should we expose this document after it is created from an entity? If there is the need to do some customization on it, I'd rather prefer that the entity can be modified (annotations) to produce the desired output than to give the user an interface where she needs to hack a solution into the document.

For the time being I would not add callbacks operating on the document until the need comes up and can be discussed in detail

 

[spring-projects-issues]

Roman Puchkovskiy commented

As you suggested to create a ticket per new callback type, I've created DATAES-772 and I suggest to move there with the discussion of 'after-convert' callback