Skip to content

Commit 1e7dc7c

Browse files
christophstroblmp911de
christophstrobl
authored and
mp911de
committed
Fix non-association mapping when id value matches already resolved instance of same type.
This commit ensures to fully resolve non association values from the given source document instead of trying attempt a by id lookup in already resolved instances. Closes: #4098 Original pull request: #4133.
1 parent 234783f commit 1e7dc7c

File tree

2 files changed

+132
-32
lines changed

2 files changed

+132
-32
lines changed

Diff for: ‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java

+113-27
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import org.bson.conversions.Bson;
4040
import org.bson.json.JsonReader;
4141
import org.bson.types.ObjectId;
42-
4342
import org.springframework.beans.BeansException;
4443
import org.springframework.beans.factory.BeanClassLoaderAware;
4544
import org.springframework.context.ApplicationContext;
@@ -188,7 +187,7 @@ protected ConversionContext getConversionContext(ObjectPath path) {
188187

189188
Assert.notNull(path, "ObjectPath must not be null");
190189

191-
return new ConversionContext(this, conversions, path, this::readDocument, this::readCollectionOrArray,
190+
return new DefaultConversionContext(this, conversions, path, this::readDocument, this::readCollectionOrArray,
192191
this::readMap, this::readDBRef, this::getPotentiallyConvertedSimpleRead);
193192
}
194193

@@ -385,7 +384,46 @@ private Object doReadOrProject(ConversionContext context, Bson source, TypeInfor
385384
return readDocument(context, source, typeHint);
386385
}
387386

388-
class ProjectingConversionContext extends ConversionContext {
387+
static class AssociationConversionContext implements ConversionContext {
388+
389+
private final ConversionContext delegate;
390+
391+
public AssociationConversionContext(ConversionContext delegate) {
392+
this.delegate = delegate;
393+
}
394+
395+
@Override
396+
public <S> S convert(Object source, TypeInformation<? extends S> typeHint, ConversionContext context) {
397+
return delegate.convert(source, typeHint, context);
398+
}
399+
400+
@Override
401+
public ConversionContext withPath(ObjectPath currentPath) {
402+
return new AssociationConversionContext(delegate.withPath(currentPath));
403+
}
404+
405+
@Override
406+
public ObjectPath getPath() {
407+
return delegate.getPath();
408+
}
409+
410+
@Override
411+
public CustomConversions getCustomConversions() {
412+
return delegate.getCustomConversions();
413+
}
414+
415+
@Override
416+
public MongoConverter getSourceConverter() {
417+
return delegate.getSourceConverter();
418+
}
419+
420+
@Override
421+
public boolean resolveIdsInContext() {
422+
return true;
423+
}
424+
}
425+
426+
class ProjectingConversionContext extends DefaultConversionContext {
389427

390428
private final EntityProjection<?, ?> returnedTypeDescriptor;
391429

@@ -401,20 +439,21 @@ class ProjectingConversionContext extends ConversionContext {
401439
}
402440

403441
@Override
404-
public ConversionContext forProperty(String name) {
442+
public DefaultConversionContext forProperty(String name) {
405443

406444
EntityProjection<?, ?> property = returnedTypeDescriptor.findProperty(name);
407445
if (property == null) {
408-
return new ConversionContext(sourceConverter, conversions, path, MappingMongoConverter.this::readDocument,
409-
collectionConverter, mapConverter, dbRefConverter, elementConverter);
446+
return new DefaultConversionContext(sourceConverter, conversions, path,
447+
MappingMongoConverter.this::readDocument, collectionConverter, mapConverter, dbRefConverter,
448+
elementConverter);
410449
}
411450

412451
return new ProjectingConversionContext(sourceConverter, conversions, path, collectionConverter, mapConverter,
413452
dbRefConverter, elementConverter, property);
414453
}
415454

416455
@Override
417-
public ConversionContext withPath(ObjectPath currentPath) {
456+
public DefaultConversionContext withPath(ObjectPath currentPath) {
418457
return new ProjectingConversionContext(sourceConverter, conversions, currentPath, collectionConverter,
419458
mapConverter, dbRefConverter, elementConverter, returnedTypeDescriptor);
420459
}
@@ -529,7 +568,7 @@ private <S> S read(ConversionContext context, MongoPersistentEntity<S> entity, D
529568
SpELExpressionEvaluator evaluator = new DefaultSpELExpressionEvaluator(bson, spELContext);
530569
DocumentAccessor documentAccessor = new DocumentAccessor(bson);
531570

532-
if (hasIdentifier(bson)) {
571+
if (context.resolveIdsInContext() && hasIdentifier(bson)) {
533572
S existing = findContextualEntity(context, entity, bson);
534573
if (existing != null) {
535574
return existing;
@@ -632,7 +671,7 @@ private void readProperties(ConversionContext context, MongoPersistentEntity<?>
632671
continue;
633672
}
634673

635-
ConversionContext propertyContext = context.forProperty(prop.getName());
674+
ConversionContext propertyContext = context.forProperty(prop);
636675
MongoDbPropertyValueProvider valueProviderToUse = valueProvider.withContext(propertyContext);
637676

638677
if (prop.isAssociation() && !entity.isConstructorArgument(prop)) {
@@ -706,7 +745,7 @@ private void readAssociation(Association<MongoPersistentProperty> association, P
706745
accessor.setProperty(property,
707746
dbRefResolver.resolveReference(property,
708747
new DocumentReferenceSource(documentAccessor.getDocument(), documentAccessor.get(property)),
709-
referenceLookupDelegate, context::convert));
748+
referenceLookupDelegate, context.forProperty(property)::convert));
710749
}
711750
return;
712751
}
@@ -1935,13 +1974,13 @@ public <T> T getPropertyValue(MongoPersistentProperty property) {
19351974
return null;
19361975
}
19371976

1938-
CustomConversions conversions = context.conversions;
1977+
CustomConversions conversions = context.getCustomConversions();
19391978
if (conversions.hasValueConverter(property)) {
19401979
return (T) conversions.getPropertyValueConversions().getValueConverter(property).read(value,
1941-
new MongoConversionContext(property, context.sourceConverter));
1980+
new MongoConversionContext(property, context.getSourceConverter()));
19421981
}
19431982

1944-
ConversionContext contextToUse = context.forProperty(property.getName());
1983+
ConversionContext contextToUse = context.forProperty(property);
19451984

19461985
return (T) contextToUse.convert(value, property.getTypeInformation());
19471986
}
@@ -2158,13 +2197,49 @@ public TypeDescriptor toTypeDescriptor() {
21582197
}
21592198
}
21602199

2200+
interface ConversionContext {
2201+
2202+
default <S extends Object> S convert(Object source, TypeInformation<? extends S> typeHint) {
2203+
return convert(source, typeHint, this);
2204+
}
2205+
2206+
<S extends Object> S convert(Object source, TypeInformation<? extends S> typeHint, ConversionContext context);
2207+
2208+
ConversionContext withPath(ObjectPath currentPath);
2209+
2210+
ObjectPath getPath();
2211+
2212+
default ConversionContext forProperty(String name) {
2213+
return this;
2214+
}
2215+
2216+
default ConversionContext forProperty(@Nullable PersistentProperty property) {
2217+
2218+
if (property != null) {
2219+
if (property.isAssociation()) {
2220+
return new AssociationConversionContext(forProperty(property.getName()));
2221+
}
2222+
return forProperty(property.getName());
2223+
}
2224+
return this;
2225+
}
2226+
2227+
default boolean resolveIdsInContext() {
2228+
return false;
2229+
}
2230+
2231+
CustomConversions getCustomConversions();
2232+
2233+
MongoConverter getSourceConverter();
2234+
}
2235+
21612236
/**
21622237
* Conversion context holding references to simple {@link ValueConverter} and {@link ContainerValueConverter}.
21632238
* Entrypoint for recursive conversion of {@link Document} and other types.
21642239
*
21652240
* @since 3.2
21662241
*/
2167-
protected static class ConversionContext {
2242+
protected static class DefaultConversionContext implements ConversionContext {
21682243

21692244
final MongoConverter sourceConverter;
21702245
final org.springframework.data.convert.CustomConversions conversions;
@@ -2175,7 +2250,7 @@ protected static class ConversionContext {
21752250
final ContainerValueConverter<DBRef> dbRefConverter;
21762251
final ValueConverter<Object> elementConverter;
21772252

2178-
ConversionContext(MongoConverter sourceConverter,
2253+
DefaultConversionContext(MongoConverter sourceConverter,
21792254
org.springframework.data.convert.CustomConversions customConversions, ObjectPath path,
21802255
ContainerValueConverter<Bson> documentConverter, ContainerValueConverter<Collection<?>> collectionConverter,
21812256
ContainerValueConverter<Bson> mapConverter, ContainerValueConverter<DBRef> dbRefConverter,
@@ -2199,7 +2274,8 @@ protected static class ConversionContext {
21992274
* @return the converted object.
22002275
*/
22012276
@SuppressWarnings("unchecked")
2202-
public <S extends Object> S convert(Object source, TypeInformation<? extends S> typeHint) {
2277+
public <S extends Object> S convert(Object source, TypeInformation<? extends S> typeHint,
2278+
ConversionContext context) {
22032279

22042280
Assert.notNull(source, "Source must not be null");
22052281
Assert.notNull(typeHint, "TypeInformation must not be null");
@@ -2219,26 +2295,26 @@ public <S extends Object> S convert(Object source, TypeInformation<? extends S>
22192295
}
22202296

22212297
if (typeHint.isCollectionLike() || typeHint.getType().isAssignableFrom(Collection.class)) {
2222-
return (S) collectionConverter.convert(this, (Collection<?>) source, typeHint);
2298+
return (S) collectionConverter.convert(context, (Collection<?>) source, typeHint);
22232299
}
22242300
}
22252301

22262302
if (typeHint.isMap()) {
22272303

22282304
if (ClassUtils.isAssignable(Document.class, typeHint.getType())) {
2229-
return (S) documentConverter.convert(this, BsonUtils.asBson(source), typeHint);
2305+
return (S) documentConverter.convert(context, BsonUtils.asBson(source), typeHint);
22302306
}
22312307

22322308
if (BsonUtils.supportsBson(source)) {
2233-
return (S) mapConverter.convert(this, BsonUtils.asBson(source), typeHint);
2309+
return (S) mapConverter.convert(context, BsonUtils.asBson(source), typeHint);
22342310
}
22352311

22362312
throw new IllegalArgumentException(
22372313
String.format("Expected map like structure but found %s", source.getClass()));
22382314
}
22392315

22402316
if (source instanceof DBRef) {
2241-
return (S) dbRefConverter.convert(this, (DBRef) source, typeHint);
2317+
return (S) dbRefConverter.convert(context, (DBRef) source, typeHint);
22422318
}
22432319

22442320
if (source instanceof Collection) {
@@ -2247,31 +2323,41 @@ public <S extends Object> S convert(Object source, TypeInformation<? extends S>
22472323
}
22482324

22492325
if (BsonUtils.supportsBson(source)) {
2250-
return (S) documentConverter.convert(this, BsonUtils.asBson(source), typeHint);
2326+
return (S) documentConverter.convert(context, BsonUtils.asBson(source), typeHint);
22512327
}
22522328

22532329
return (S) elementConverter.convert(source, typeHint);
22542330
}
22552331

2332+
@Override
2333+
public CustomConversions getCustomConversions() {
2334+
return conversions;
2335+
}
2336+
2337+
@Override
2338+
public MongoConverter getSourceConverter() {
2339+
return sourceConverter;
2340+
}
2341+
22562342
/**
2257-
* Create a new {@link ConversionContext} with {@link ObjectPath currentPath} applied.
2343+
* Create a new {@link DefaultConversionContext} with {@link ObjectPath currentPath} applied.
22582344
*
22592345
* @param currentPath must not be {@literal null}.
2260-
* @return a new {@link ConversionContext} with {@link ObjectPath currentPath} applied.
2346+
* @return a new {@link DefaultConversionContext} with {@link ObjectPath currentPath} applied.
22612347
*/
2262-
public ConversionContext withPath(ObjectPath currentPath) {
2348+
public DefaultConversionContext withPath(ObjectPath currentPath) {
22632349

22642350
Assert.notNull(currentPath, "ObjectPath must not be null");
22652351

2266-
return new ConversionContext(sourceConverter, conversions, currentPath, documentConverter, collectionConverter,
2267-
mapConverter, dbRefConverter, elementConverter);
2352+
return new DefaultConversionContext(sourceConverter, conversions, currentPath, documentConverter,
2353+
collectionConverter, mapConverter, dbRefConverter, elementConverter);
22682354
}
22692355

22702356
public ObjectPath getPath() {
22712357
return path;
22722358
}
22732359

2274-
public ConversionContext forProperty(String name) {
2360+
public DefaultConversionContext forProperty(String name) {
22752361
return this;
22762362
}
22772363

Diff for: ‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java

+19-5
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,6 @@
4545
import org.mockito.Mock;
4646
import org.mockito.Mockito;
4747
import org.mockito.junit.jupiter.MockitoExtension;
48-
4948
import org.springframework.aop.framework.ProxyFactory;
5049
import org.springframework.beans.ConversionNotSupportedException;
5150
import org.springframework.beans.factory.annotation.Autowired;
@@ -2823,6 +2822,15 @@ public org.bson.Document write(@Nullable String domainValue, MongoConversionCont
28232822
assertThat(read.viaRegisteredConverter).isEqualTo("spring");
28242823
}
28252824

2825+
@Test // GH-4098
2826+
void resolvesCyclicNonAssociationValueFromSource/* and does not attempt to be smart and look up id values in context */() {
2827+
2828+
org.bson.Document source = new org.bson.Document("_id", "id-1").append("value", "v1").append("cycle",
2829+
new org.bson.Document("_id", "id-1").append("value", "v2"));
2830+
2831+
assertThat(converter.read(Cyclic.class, source).cycle.value).isEqualTo("v2");
2832+
}
2833+
28262834
static class GenericType<T> {
28272835
T content;
28282836
}
@@ -3546,12 +3554,10 @@ static class WithFieldWrite {
35463554
@org.springframework.data.mongodb.core.mapping.Field(
35473555
write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Integer writeAlways;
35483556

3549-
@org.springframework.data.mongodb.core.mapping.DBRef
3550-
@org.springframework.data.mongodb.core.mapping.Field(
3557+
@org.springframework.data.mongodb.core.mapping.DBRef @org.springframework.data.mongodb.core.mapping.Field(
35513558
write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPerson;
35523559

3553-
@org.springframework.data.mongodb.core.mapping.DBRef
3554-
@org.springframework.data.mongodb.core.mapping.Field(
3560+
@org.springframework.data.mongodb.core.mapping.DBRef @org.springframework.data.mongodb.core.mapping.Field(
35553561
write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPerson;
35563562

35573563
}
@@ -3665,4 +3671,12 @@ static class Author {
36653671

36663672
}
36673673

3674+
@Data
3675+
static class Cyclic {
3676+
3677+
@Id String id;
3678+
String value;
3679+
Cyclic cycle;
3680+
}
3681+
36683682
}

0 commit comments

Comments
 (0)