-
Notifications
You must be signed in to change notification settings - Fork 1.3k
DATAES-771 - Add after-save entity callbacks support. #414
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
import org.springframework.data.elasticsearch.core.convert.ElasticsearchConverter; | ||
import org.springframework.data.elasticsearch.core.convert.MappingElasticsearchConverter; | ||
import org.springframework.data.elasticsearch.core.document.SearchDocumentResponse; | ||
import org.springframework.data.elasticsearch.core.event.AfterSaveCallback; | ||
import org.springframework.data.elasticsearch.core.event.BeforeConvertCallback; | ||
import org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentEntity; | ||
import org.springframework.data.elasticsearch.core.mapping.ElasticsearchPersistentProperty; | ||
|
@@ -45,6 +46,7 @@ | |
* | ||
* @author Sascha Woo | ||
* @author Peter-Josef Meisch | ||
* @author Roman Puchkovskiy | ||
*/ | ||
public abstract class AbstractElasticsearchTemplate implements ElasticsearchOperations, ApplicationContextAware { | ||
|
||
|
@@ -117,8 +119,13 @@ public <T> T save(T entity, IndexCoordinates index) { | |
Assert.notNull(entity, "entity must not be null"); | ||
Assert.notNull(index, "index must not be null"); | ||
|
||
index(getIndexQuery(entity), index); | ||
return entity; | ||
IndexQuery query = getIndexQuery(entity); | ||
index(query, index); | ||
|
||
// suppressing because it's either entity itself or something of a correct type returned by an entity callback | ||
@SuppressWarnings("unchecked") | ||
T castResult = (T) query.getObject(); | ||
return castResult; | ||
} | ||
|
||
@Override | ||
|
@@ -151,7 +158,10 @@ public <T> Iterable<T> save(Iterable<T> entities, IndexCoordinates index) { | |
}); | ||
} | ||
|
||
return entities; | ||
return indexQueries.stream() | ||
.map(IndexQuery::getObject) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This must not be null, see my comment for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently, this can only be null if some entity in the original Iterable is null. Do you still suggest adding a non-null assertion? |
||
.map(entity -> (T) entity) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
@Override | ||
|
@@ -455,11 +465,39 @@ protected void maybeCallbackBeforeConvertWithQuery(Object query) { | |
} | ||
|
||
// this can be called with either a List<IndexQuery> or a List<UpdateQuery>; these query classes | ||
// don't have a common bas class, therefore the List<?> argument | ||
// don't have a common base class, therefore the List<?> argument | ||
protected void maybeCallbackBeforeConvertWithQueries(List<?> queries) { | ||
queries.forEach(this::maybeCallbackBeforeConvertWithQuery); | ||
} | ||
|
||
protected <T> T maybeCallbackAfterSave(T entity) { | ||
|
||
if (entityCallbacks != null) { | ||
return entityCallbacks.callback(AfterSaveCallback.class, entity); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should make sure that the value returned from the callback is not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at the code of
Also, javadoc for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, I didn't dig this deep into the code for this PR, sorry (wish this were Kotlin stuff!), so that's ok |
||
} | ||
|
||
return entity; | ||
} | ||
|
||
protected void maybeCallbackAfterSaveWithQuery(Object query) { | ||
|
||
if (query instanceof IndexQuery) { | ||
IndexQuery indexQuery = (IndexQuery) query; | ||
Object queryObject = indexQuery.getObject(); | ||
|
||
if (queryObject != null) { | ||
queryObject = maybeCallbackAfterSave(queryObject); | ||
indexQuery.setObject(queryObject); | ||
} | ||
} | ||
} | ||
|
||
// this can be called with either a List<IndexQuery> or a List<UpdateQuery>; these query classes | ||
// don't have a common base class, therefore the List<?> argument | ||
protected void maybeCallbackAfterSaveWithQueries(List<?> queries) { | ||
queries.forEach(this::maybeCallbackAfterSaveWithQuery); | ||
} | ||
|
||
// endregion | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/* | ||
* Copyright 2020 the original author or authors. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.springframework.data.elasticsearch.core.event; | ||
|
||
import org.springframework.data.mapping.callback.EntityCallback; | ||
import org.springframework.data.mapping.callback.EntityCallbacks; | ||
|
||
/** | ||
* Entity callback triggered after save of an entity. | ||
* | ||
* @author Roman Puchkovskiy | ||
* @since 4.0 | ||
* @see EntityCallbacks | ||
*/ | ||
@FunctionalInterface | ||
public interface AfterSaveCallback<T> extends EntityCallback<T> { | ||
|
||
/** | ||
* Entity callback method invoked after a domain object is saved. Can return either the same or a modified | ||
* instance of the domain object. | ||
* | ||
* @param entity the domain object that was saved. | ||
* @return the domain object that was persisted. | ||
*/ | ||
T onAfterSave(T entity); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
/* | ||
* Copyright 2020 the original author or authors. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.springframework.data.elasticsearch.core.event; | ||
|
||
import org.reactivestreams.Publisher; | ||
import org.springframework.data.mapping.callback.EntityCallback; | ||
import org.springframework.data.mapping.callback.ReactiveEntityCallbacks; | ||
|
||
/** | ||
* Entity callback triggered after save of an entity. | ||
* | ||
* @author Roman Puchkovskiy | ||
* @since 4.0 | ||
* @see ReactiveEntityCallbacks | ||
*/ | ||
@FunctionalInterface | ||
public interface ReactiveAfterSaveCallback<T> extends EntityCallback<T> { | ||
|
||
/** | ||
* Entity callback method invoked after a domain object is saved. Can return either the same or a modified | ||
* instance of the domain object. | ||
* | ||
* @param entity the domain object that was saved. | ||
* @return a {@link Publisher} emitting the domain object to be returned to the caller. | ||
*/ | ||
Publisher<T> onAfterSave(T entity); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This must not be null, see my comment for the
maybeCallbackAfterSave(T entity)
method.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
query.getObject()
can be null either ifentity
is null (which is impossible, there is an assertion in the beginning of the method), or ifentityCallbacks.callback()
returns null (which is also impossible).So the
castResult
cannot be null.Would you still prefer to assert that it is not null, just in case?