Skip to content

Improve handling of immutable classes. #1801

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 3, 2021
Merged

Improve handling of immutable classes. #1801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 32 additions & 10 deletions src/main/java/org/springframework/data/elasticsearch/core/AbstractElasticsearchTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public <T> T save(T entity, IndexCoordinates index) {
IndexQuery query = getIndexQuery(entityAfterBeforeConvert);
doIndex(query, index);

T entityAfterAfterSave = maybeCallbackAfterSave(entityAfterBeforeConvert, index);
T entityAfterAfterSave = (T) maybeCallbackAfterSave(query.getObject(), index);

return entityAfterAfterSave;
}
Expand All @@ -215,13 +215,18 @@ public <T> Iterable<T> save(Iterable<T> entities, IndexCoordinates index) {
List<IndexQuery> indexQueries = Streamable.of(entities).stream().map(this::getIndexQuery)
.collect(Collectors.toList());

if (!indexQueries.isEmpty()) {
List<IndexedObjectInformation> indexedObjectInformations = bulkIndex(indexQueries, index);
Iterator<IndexedObjectInformation> iterator = indexedObjectInformations.iterator();
entities.forEach(entity -> updateIndexedObject(entity, iterator.next()));
if (indexQueries.isEmpty()) {
return Collections.emptyList();
}

return indexQueries.stream().map(IndexQuery::getObject).map(entity -> (T) entity).collect(Collectors.toList());
List<IndexedObjectInformation> indexedObjectInformations = bulkIndex(indexQueries, index);
Iterator<IndexedObjectInformation> iterator = indexedObjectInformations.iterator();

// noinspection unchecked
return indexQueries.stream() //
.map(IndexQuery::getObject) //
.map(entity -> (T) updateIndexedObject(entity, iterator.next())) //
.collect(Collectors.toList()); //
}

@Override
Expand Down Expand Up @@ -419,7 +424,9 @@ public <T> SearchHits<T> search(MoreLikeThisQuery query, Class<T> clazz, IndexCo
Assert.notNull(query.getId(), "No document id defined for MoreLikeThisQuery");

MoreLikeThisQueryBuilder moreLikeThisQueryBuilder = requestFactory.moreLikeThisQueryBuilder(query, index);
return search(new NativeSearchQueryBuilder().withQuery(moreLikeThisQueryBuilder).withPageable(query.getPageable()).build(), clazz, index);
return search(
new NativeSearchQueryBuilder().withQuery(moreLikeThisQueryBuilder).withPageable(query.getPageable()).build(),
clazz, index);
}

@Override
Expand Down Expand Up @@ -611,7 +618,7 @@ protected List<IndexedObjectInformation> checkForBulkOperationFailure(BulkRespon
}).collect(Collectors.toList());
}

protected void updateIndexedObject(Object entity, IndexedObjectInformation indexedObjectInformation) {
protected <T> T updateIndexedObject(T entity, IndexedObjectInformation indexedObjectInformation) {

ElasticsearchPersistentEntity<?> persistentEntity = elasticsearchConverter.getMappingContext()
.getPersistentEntity(entity.getClass());
Expand All @@ -621,22 +628,30 @@ protected void updateIndexedObject(Object entity, IndexedObjectInformation index
ElasticsearchPersistentProperty idProperty = persistentEntity.getIdProperty();

// Only deal with text because ES generated Ids are strings!
if (idProperty != null && idProperty.getType().isAssignableFrom(String.class)) {
if (indexedObjectInformation.getId() != null && idProperty != null
&& idProperty.getType().isAssignableFrom(String.class)) {
propertyAccessor.setProperty(idProperty, indexedObjectInformation.getId());
}

if (indexedObjectInformation.getSeqNo() != null && indexedObjectInformation.getPrimaryTerm() != null
&& persistentEntity.hasSeqNoPrimaryTermProperty()) {
ElasticsearchPersistentProperty seqNoPrimaryTermProperty = persistentEntity.getSeqNoPrimaryTermProperty();
// noinspection ConstantConditions
propertyAccessor.setProperty(seqNoPrimaryTermProperty,
new SeqNoPrimaryTerm(indexedObjectInformation.getSeqNo(), indexedObjectInformation.getPrimaryTerm()));
}

if (indexedObjectInformation.getVersion() != null && persistentEntity.hasVersionProperty()) {
ElasticsearchPersistentProperty versionProperty = persistentEntity.getVersionProperty();
// noinspection ConstantConditions
propertyAccessor.setProperty(versionProperty, indexedObjectInformation.getVersion());
}

// noinspection unchecked
T updatedEntity = (T) propertyAccessor.getBean();
return updatedEntity;
}
return entity;
}

ElasticsearchPersistentEntity<?> getRequiredPersistentEntity(Class<?> clazz) {
Expand Down Expand Up @@ -807,13 +822,16 @@ protected <T> T maybeCallbackAfterConvert(T entity, Document document, IndexCoor

protected void updateIndexedObjectsWithQueries(List<?> queries,
List<IndexedObjectInformation> indexedObjectInformations) {

for (int i = 0; i < queries.size(); i++) {
Object query = queries.get(i);

if (query instanceof IndexQuery) {
IndexQuery indexQuery = (IndexQuery) query;
Object queryObject = indexQuery.getObject();

if (queryObject != null) {
updateIndexedObject(queryObject, indexedObjectInformations.get(i));
indexQuery.setObject(updateIndexedObject(queryObject, indexedObjectInformations.get(i)));
}
}
}
Expand Down Expand Up @@ -848,6 +866,10 @@ public T doWith(@Nullable Document document) {
}

T entity = reader.read(type, document);
IndexedObjectInformation indexedObjectInformation = IndexedObjectInformation.of(
document.hasId() ? document.getId() : null, document.getSeqNo(), document.getPrimaryTerm(),
document.getVersion());
entity = updateIndexedObject(entity, indexedObjectInformation);
return maybeCallbackAfterConvert(entity, document, index);
}
}
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchRestTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,10 @@ public String doIndex(IndexQuery query, IndexCoordinates index) {
IndexResponse indexResponse = execute(client -> client.index(request, RequestOptions.DEFAULT));

Object queryObject = query.getObject();

if (queryObject != null) {
updateIndexedObject(queryObject, IndexedObjectInformation.of(indexResponse.getId(), indexResponse.getSeqNo(),
indexResponse.getPrimaryTerm(), indexResponse.getVersion()));
query.setObject(updateIndexedObject(queryObject, IndexedObjectInformation.of(indexResponse.getId(),
indexResponse.getSeqNo(), indexResponse.getPrimaryTerm(), indexResponse.getVersion())));
}

return indexResponse.getId();
Expand All @@ -168,6 +169,7 @@ public String doIndex(IndexQuery query, IndexCoordinates index) {
@Override
@Nullable
public <T> T get(String id, Class<T> clazz, IndexCoordinates index) {

GetRequest request = requestFactory.getRequest(id, routingResolver.getRouting(), index);
GetResponse response = execute(client -> client.get(request, RequestOptions.DEFAULT));

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/springframework/data/elasticsearch/core/ElasticsearchTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@ public String doIndex(IndexQuery query, IndexCoordinates index) {

Object queryObject = query.getObject();
if (queryObject != null) {
updateIndexedObject(queryObject, IndexedObjectInformation.of(documentId, response.getSeqNo(),
response.getPrimaryTerm(), response.getVersion()));
query.setObject(updateIndexedObject(queryObject, IndexedObjectInformation.of(documentId, response.getSeqNo(),
response.getPrimaryTerm(), response.getVersion())));
}

return documentId;
Expand Down
9 changes: 6 additions & 3 deletions src/main/java/org/springframework/data/elasticsearch/core/EntityOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,6 @@ public ElasticsearchPersistentEntity<?> getPersistentEntity() {
*/
private static class AdaptibleMappedEntity<T> extends MappedEntity<T> implements AdaptibleEntity<T> {

private final T bean;
private final ElasticsearchPersistentEntity<?> entity;
private final ConvertingPropertyAccessor<T> propertyAccessor;
private final IdentifierAccessor identifierAccessor;
Expand All @@ -490,7 +489,6 @@ private AdaptibleMappedEntity(T bean, ElasticsearchPersistentEntity<?> entity,

super(entity, identifierAccessor, propertyAccessor);

this.bean = bean;
this.entity = entity;
this.propertyAccessor = propertyAccessor;
this.identifierAccessor = identifierAccessor;
Expand All @@ -510,6 +508,11 @@ static <T> AdaptibleEntity<T> of(T bean,
new ConvertingPropertyAccessor<>(propertyAccessor, conversionService), conversionService, routingResolver);
}

@Override
public T getBean() {
return propertyAccessor.getBean();
}

@Nullable
@Override
public T populateIdIfNecessary(@Nullable Object id) {
Expand Down Expand Up @@ -584,7 +587,7 @@ public T incrementVersion() {
@Override
public String getRouting() {

String routing = routingResolver.getRouting(bean);
String routing = routingResolver.getRouting(propertyAccessor.getBean());

if (routing != null) {
return routing;
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/org/springframework/data/elasticsearch/core/IndexedObjectInformation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,25 @@
* @since 4.1
*/
public class IndexedObjectInformation {
private final String id;
@Nullable private final String id;
@Nullable private final Long seqNo;
@Nullable private final Long primaryTerm;
@Nullable private final Long version;

private IndexedObjectInformation(String id, @Nullable Long seqNo, @Nullable Long primaryTerm,
private IndexedObjectInformation(@Nullable String id, @Nullable Long seqNo, @Nullable Long primaryTerm,
@Nullable Long version) {
this.id = id;
this.seqNo = seqNo;
this.primaryTerm = primaryTerm;
this.version = version;
}

public static IndexedObjectInformation of(String id, @Nullable Long seqNo, @Nullable Long primaryTerm,
public static IndexedObjectInformation of(@Nullable String id, @Nullable Long seqNo, @Nullable Long primaryTerm,
@Nullable Long version) {
return new IndexedObjectInformation(id, seqNo, primaryTerm, version);
}

@Nullable
public String getId() {
return id;
}
Expand Down
31 changes: 25 additions & 6 deletions src/main/java/org/springframework/data/elasticsearch/core/ReactiveElasticsearchTemplate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,27 +275,42 @@ public <T> Flux<T> saveAll(Mono<? extends Collection<? extends T>> entitiesPubli
}

private <T> T updateIndexedObject(T entity, IndexedObjectInformation indexedObjectInformation) {
AdaptibleEntity<T> adaptibleEntity = operations.forEntity(entity, converter.getConversionService(),
routingResolver);
adaptibleEntity.populateIdIfNecessary(indexedObjectInformation.getId());

ElasticsearchPersistentEntity<?> persistentEntity = getPersistentEntityFor(entity.getClass());
ElasticsearchPersistentEntity<?> persistentEntity = converter.getMappingContext()
.getPersistentEntity(entity.getClass());

if (persistentEntity != null) {
PersistentPropertyAccessor<Object> propertyAccessor = persistentEntity.getPropertyAccessor(entity);
ElasticsearchPersistentProperty idProperty = persistentEntity.getIdProperty();

// Only deal with text because ES generated Ids are strings!
if (indexedObjectInformation.getId() != null && idProperty != null
&& idProperty.getType().isAssignableFrom(String.class)) {
propertyAccessor.setProperty(idProperty, indexedObjectInformation.getId());
}

if (indexedObjectInformation.getSeqNo() != null && indexedObjectInformation.getPrimaryTerm() != null
&& persistentEntity.hasSeqNoPrimaryTermProperty()) {
ElasticsearchPersistentProperty seqNoPrimaryTermProperty = persistentEntity.getSeqNoPrimaryTermProperty();
// noinspection ConstantConditions
propertyAccessor.setProperty(seqNoPrimaryTermProperty,
new SeqNoPrimaryTerm(indexedObjectInformation.getSeqNo(), indexedObjectInformation.getPrimaryTerm()));
}

if (indexedObjectInformation.getVersion() != null && persistentEntity.hasVersionProperty()) {
ElasticsearchPersistentProperty versionProperty = persistentEntity.getVersionProperty();
// noinspection ConstantConditions
propertyAccessor.setProperty(versionProperty, indexedObjectInformation.getVersion());
}
}

// noinspection unchecked
T updatedEntity = (T) propertyAccessor.getBean();
return updatedEntity;
} else {
AdaptibleEntity<T> adaptibleEntity = operations.forEntity(entity, converter.getConversionService(),
routingResolver);
adaptibleEntity.populateIdIfNecessary(indexedObjectInformation.getId());
}
return entity;
}

Expand Down Expand Up @@ -457,7 +472,7 @@ public <T> Mono<T> get(String id, Class<T> entityType, IndexCoordinates index) {

DocumentCallback<T> callback = new ReadDocumentCallback<>(converter, entityType, index);

return doGet(id, index).flatMap(it -> callback.toEntity(DocumentAdapters.from(it)));
return doGet(id, index).flatMap(response -> callback.toEntity(DocumentAdapters.from(response)));
}

private Mono<GetResult> doGet(String id, IndexCoordinates index) {
Expand Down Expand Up @@ -1097,6 +1112,10 @@ public Mono<T> toEntity(@Nullable Document document) {
}

T entity = reader.read(type, document);
IndexedObjectInformation indexedObjectInformation = IndexedObjectInformation.of(
document.hasId() ? document.getId() : null, document.getSeqNo(), document.getPrimaryTerm(),
document.getVersion());
entity = updateIndexedObject(entity, indexedObjectInformation);
return maybeCallAfterConvert(entity, document, index);
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/springframework/data/elasticsearch/core/join/JoinField.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.springframework.data.elasticsearch.core.join;

import org.springframework.data.annotation.PersistenceConstructor;
import org.springframework.lang.Nullable;

/**
Expand All @@ -35,6 +36,7 @@ public JoinField(String name) {
this(name, null);
}

@PersistenceConstructor
public JoinField(String name, @Nullable ID parent) {
this.name = name;
this.parent = parent;
Expand Down
14 changes: 0 additions & 14 deletions .../springframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentEntity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.springframework.data.mapping.PropertyHandler;
import org.springframework.data.mapping.model.BasicPersistentEntity;
import org.springframework.data.mapping.model.FieldNamingStrategy;
import org.springframework.data.mapping.model.PersistentPropertyAccessorFactory;
import org.springframework.data.spel.ExpressionDependencies;
import org.springframework.data.util.Lazy;
import org.springframework.data.util.TypeInformation;
Expand Down Expand Up @@ -238,19 +237,6 @@ private void warnAboutBothSeqNoPrimaryTermAndVersionProperties() {
getType());
}

/*
* (non-Javadoc)
* @see org.springframework.data.mapping.model.BasicPersistentEntity#setPersistentPropertyAccessorFactory(org.springframework.data.mapping.model.PersistentPropertyAccessorFactory)
*/
@SuppressWarnings("SpellCheckingInspection")
@Override
public void setPersistentPropertyAccessorFactory(PersistentPropertyAccessorFactory factory) {

// Do nothing to avoid the usage of ClassGeneratingPropertyAccessorFactory for now
// DATACMNS-1322 switches to proper immutability behavior which Spring Data Elasticsearch
// cannot yet implement
}

@Nullable
@Override
public ElasticsearchPersistentProperty getPersistentPropertyWithFieldName(String fieldName) {
Expand Down
5 changes: 0 additions & 5 deletions ...pringframework/data/elasticsearch/core/mapping/SimpleElasticsearchPersistentProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,6 @@ protected Association<ElasticsearchPersistentProperty> createAssociation() {
throw new UnsupportedOperationException();
}

@Override
public boolean isImmutable() {
return false;
}

@Override
public boolean isSeqNoPrimaryTermProperty() {
return isSeqNoPrimaryTerm;
Expand Down
Loading