Skip to content

Commit a2a017a

Browse files
emilienbevemilienbev
committed
Remove decodeEntityBase logic duplication by using a shared function
Adjust comments Signed-off-by: Emilien Bevierre <emilien.bevierre@couchbase.com>
1 parent a7d84f1 commit a2a017a

1 file changed

Lines changed: 71 additions & 53 deletions

File tree

‎src/main/java/org/springframework/data/couchbase/core/AbstractTemplateSupport.java‎

Lines changed: 71 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
import java.time.Instant;
2020
import java.util.Map;
2121
import java.util.Set;
22+
import java.util.function.BiFunction;
2223

23-
import com.couchbase.client.core.annotation.Stability;
2424
import org.slf4j.Logger;
2525
import org.slf4j.LoggerFactory;
2626
import org.springframework.context.ApplicationContext;
@@ -40,9 +40,9 @@
4040
import org.springframework.data.mapping.model.ConvertingPropertyAccessor;
4141
import org.springframework.util.ClassUtils;
4242

43+
import com.couchbase.client.core.annotation.Stability;
4344
import com.couchbase.client.core.error.CouchbaseException;
4445

45-
4646
/**
4747
* Base shared by Reactive and non-Reactive TemplateSupport
4848
*
@@ -71,82 +71,97 @@ public AbstractTemplateSupport(ReactiveCouchbaseTemplate template, CouchbaseConv
7171

7272
public <T> T decodeEntityBase(Object id, String source, Long cas, Instant expiryTime, Class<T> entityClass,
7373
String scope, String collection, Object txResultHolder, CouchbaseResourceHolder holder) {
74-
CouchbasePersistentEntity persistentEntity = couldBePersistentEntity(entityClass);
75-
76-
if (persistentEntity == null) {
77-
final CouchbaseDocument converted = new CouchbaseDocument(id);
78-
Set<Map.Entry<String, Object>> set = ((CouchbaseDocument) translationService.decode(source, converted))
79-
.getContent().entrySet();
80-
return (T) set.iterator().next().getValue();
81-
}
82-
83-
final CouchbaseDocument converted = prepareConvertedDocument(id, cas, persistentEntity);
84-
T readEntity = converter.read(entityClass, (CouchbaseDocument) translationService.decode(source, converted));
85-
return finalizeEntity(readEntity, id, cas, expiryTime, scope, collection, txResultHolder, holder);
74+
return decodeEntityBase(id, cas, expiryTime, entityClass, scope, collection, txResultHolder, holder,
75+
(ts, converted) -> (CouchbaseDocument) ts.decode(source, converted));
8676
}
8777

8878
public <T> T decodeEntityBase(Object id, byte[] source, Long cas, Instant expiryTime, Class<T> entityClass,
8979
String scope, String collection, Object txResultHolder, CouchbaseResourceHolder holder) {
80+
return decodeEntityBase(id, cas, expiryTime, entityClass, scope, collection, txResultHolder, holder,
81+
(ts, converted) -> (CouchbaseDocument) ts.decode(source, converted));
82+
}
83+
84+
private <T> T decodeEntityBase(Object id, Long cas, Instant expiryTime, Class<T> entityClass, String scope,
85+
String collection, Object txResultHolder, CouchbaseResourceHolder holder,
86+
BiFunction<TranslationService, CouchbaseDocument, CouchbaseDocument> translatorFn) {
9087
CouchbasePersistentEntity persistentEntity = couldBePersistentEntity(entityClass);
9188

9289
if (persistentEntity == null) {
9390
final CouchbaseDocument converted = new CouchbaseDocument(id);
94-
Set<Map.Entry<String, Object>> set = ((CouchbaseDocument) translationService.decode(source, converted))
95-
.getContent().entrySet();
91+
Set<Map.Entry<String, Object>> set = translatorFn.apply(translationService, converted).getContent()
92+
.entrySet();
9693
return (T) set.iterator().next().getValue();
9794
}
9895

9996
final CouchbaseDocument converted = prepareConvertedDocument(id, cas, persistentEntity);
100-
T readEntity = converter.read(entityClass, (CouchbaseDocument) translationService.decode(source, converted));
97+
T readEntity = converter.read(entityClass, translatorFn.apply(translationService, converted));
10198
return finalizeEntity(readEntity, id, cas, expiryTime, scope, collection, txResultHolder, holder);
10299
}
103100

104-
private CouchbaseDocument prepareConvertedDocument(Object id, Long cas, CouchbasePersistentEntity persistentEntity) {
105-
// this is the entity class defined for the repository. It may not be the class of the document that was read
106-
// we will reset it after reading the document
101+
private CouchbaseDocument prepareConvertedDocument(Object id, Long cas,
102+
CouchbasePersistentEntity persistentEntity) {
103+
// persistentEntity is derived from the entityClass declared in the
104+
// repository definition. It may be an abstract class rather than the
105+
// concrete class of the document being read. The concrete type is only
106+
// known after converter.read() is called, therefore version/cas is set again
107+
// on the final entity in finalizeEntity().
107108
//
108-
// This will fail for the case where:
109-
// 1) The version is defined in the concrete class, but not in the abstract class; and
110-
// 2) The constructor takes a "long version" argument resulting in an exception would be thrown if version in
111-
// the source is null.
112-
// We could expose from the MappingCouchbaseConverter determining the persistent entity from the source,
113-
// but that is a lot of work to do every time just for this very rare and avoidable case.
109+
// Pre-populating the CAS/version into the source document (done in getDocument)
110+
// is a best-effort step to avoid a construction failure when the concrete
111+
// class constructor takes a primitive "long version" argument (null is not a
112+
// valid value for a primitive). If the version property is only declared on the
113+
// concrete subclass and not on the abstract base, pre-population is not
114+
// possible here and the issue can be avoided by using "Long" instead of "long".
115+
//
116+
// An alternative would be to resolve the actual concrete type from the source
117+
// document's type metadata before constructing it (see the comment
118+
// below), but that adds overhead for every decode to solve a rare and avoidable
119+
// case.
114120
// TypeInformation<? extends R> typeToUse = typeMapper.readType(source, type);
115121

116122
if (id == null) {
117-
throw new CouchbaseException(TemplateUtils.SELECT_ID + " was null. Either use #{#n1ql.selectEntity} or project "
118-
+ TemplateUtils.SELECT_ID);
119-
}
120-
121-
final CouchbaseDocument converted = new CouchbaseDocument(id);
122-
123-
// if possible, set the version property in the source so that if the constructor has a long version argument,
124-
// it will have a value and not fail (as null is not a valid argument for a long argument). This possible failure
125-
// can be avoid by defining the argument as Long instead of long.
126-
// persistentEntity is still the (possibly abstract) class specified in the repository definition
127-
// it's possible that the abstract class does not have a version property, and this won't be able to set the version
128-
if (persistentEntity.getVersionProperty() != null) {
129-
if (cas == null) {
130-
throw new CouchbaseException("version/cas in the entity but " + TemplateUtils.SELECT_CAS
131-
+ " was not in result. Either use #{#n1ql.selectEntity} or project " + TemplateUtils.SELECT_CAS);
132-
}
133-
if (cas != 0) {
134-
converted.put(persistentEntity.getVersionProperty().getName(), cas);
135-
}
123+
throw new CouchbaseException(
124+
TemplateUtils.SELECT_ID + " was null. Either use #{#n1ql.selectEntity} or project "
125+
+ TemplateUtils.SELECT_ID);
136126
}
137127

138-
return converted;
128+
return getDocument(id, cas, persistentEntity);
139129
}
140130

141-
private <T> T finalizeEntity(T readEntity, Object id, Long cas, Instant expiryTime, String scope, String collection,
131+
private static CouchbaseDocument getDocument(Object id, Long cas, CouchbasePersistentEntity persistentEntity) {
132+
final CouchbaseDocument converted = new CouchbaseDocument(id);
133+
134+
// If possible, set the version property in the source so that if the
135+
// constructor has a long version argument, it will have a value and succeed,
136+
// as null is not a valid argument for a long. This failure can be avoided by
137+
// defining the argument as Long instead of long.
138+
// Note that persistentEntity is still the (possibly abstract) class specified
139+
// in the repository definition, so it's possible that the abstract class does
140+
// not have a version property, in which case this won't be able to set the version.
141+
if (persistentEntity.getVersionProperty() != null) {
142+
if (cas == null) {
143+
throw new CouchbaseException("version/cas in the entity but " + TemplateUtils.SELECT_CAS
144+
+ " was not in result. Either use #{#n1ql.selectEntity} or project "
145+
+ TemplateUtils.SELECT_CAS);
146+
}
147+
if (cas != 0) {
148+
converted.put(persistentEntity.getVersionProperty().getName(), cas);
149+
}
150+
}
151+
return converted;
152+
}
153+
154+
private <T> T finalizeEntity(T readEntity, Object id, Long cas, Instant expiryTime, String scope, String collection,
142155
Object txResultHolder, CouchbaseResourceHolder holder) {
143156
final ConvertingPropertyAccessor<T> accessor = getPropertyAccessor(readEntity);
144157

145158
CouchbasePersistentEntity persistentEntity = couldBePersistentEntity(readEntity.getClass());
146159

147-
// if the constructor has an argument that is long version, then construction will fail if the 'version'
148-
// is not available as 'null' is not a legal value for a long. Changing the arg to "Long version" would solve this.
149-
// (Version doesn't come from 'source', it comes from the cas argument to decodeEntity)
160+
// If the constructor has a long version argument, construction will fail if
161+
// 'version' is not available, as null is not a legal value for a long.
162+
// We therefore use the object-wrapped Long type.
163+
// (Version doesn't come from 'source', it comes from the cas argument to
164+
// decodeEntity.)
150165
if (cas != null && cas != 0 && persistentEntity.getVersionProperty() != null) {
151166
accessor.setProperty(persistentEntity.getVersionProperty(), cas);
152167
}
@@ -155,7 +170,8 @@ private <T> T finalizeEntity(T readEntity, Object id, Long cas, Instant expiryTi
155170
accessor.setProperty(persistentEntity.getExpiryProperty(), expiryTime);
156171
}
157172

158-
N1qlJoinResolver.handleProperties(persistentEntity, accessor, getReactiveTemplate(), id.toString(), scope, collection);
173+
N1qlJoinResolver.handleProperties(persistentEntity, accessor, getReactiveTemplate(), id.toString(), scope,
174+
collection);
159175

160176
if (holder != null) {
161177
holder.transactionResultHolder(txResultHolder, (T) accessor.getBean());
@@ -203,7 +219,8 @@ public <T> T applyResultBase(T entity, CouchbaseDocument converted, Object id, l
203219

204220
public Long getCas(final Object entity) {
205221
final ConvertingPropertyAccessor<Object> accessor = getPropertyAccessor(entity);
206-
final CouchbasePersistentEntity<?> persistentEntity = mappingContext.getRequiredPersistentEntity(entity.getClass());
222+
final CouchbasePersistentEntity<?> persistentEntity = mappingContext
223+
.getRequiredPersistentEntity(entity.getClass());
207224
final CouchbasePersistentProperty versionProperty = persistentEntity.getVersionProperty();
208225
long cas = 0;
209226
if (versionProperty != null) {
@@ -217,7 +234,8 @@ public Long getCas(final Object entity) {
217234

218235
public Object getId(final Object entity) {
219236
final ConvertingPropertyAccessor<Object> accessor = getPropertyAccessor(entity);
220-
final CouchbasePersistentEntity<?> persistentEntity = mappingContext.getRequiredPersistentEntity(entity.getClass());
237+
final CouchbasePersistentEntity<?> persistentEntity = mappingContext
238+
.getRequiredPersistentEntity(entity.getClass());
221239
final CouchbasePersistentProperty idProperty = persistentEntity.getIdProperty();
222240
Object id = null;
223241
if (idProperty != null) {

0 commit comments

Comments
 (0)