Skip to content
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

MappingMongoConverter incorrectly processes an object property of type org.bson.Document #3702

Closed
meloujir opened this issue Jul 11, 2021 · 3 comments
Labels
type: regression A regression from a previous release

Comments

@meloujir
Copy link

Hi, we encountered a problem (originates from 3.1.5 > 3.2 update), which occurrs when a property (of some DBO entity) of type org.bson.Document is handled within MappingMongoConverter.ConversionContext#convert. MongoDB driver correctly returns a document, of which all inner object properties (fields) are of type org.bson.Document.

Then MappingMongoConverter comes into play (within process of populating fields , #populateProperties -> #readProperties). During that process, the property value of type org.bson.Document is incorrectly handled as a Map and all properties (fields) with an object value are converted to a LinkedHashMap. Later on, when we, within concrete context, try to convert that object of type org.bson.Document to certain class (using MongoTemplate#getConverter()#read(getEntityClass(), sourceObject), but it is not important), it leads to:

org.springframework.core.convert.ConverterNotFoundException: No converter found capable of converting from type [java.util.LinkedHashMap<?, ?>] to type [...]

That error is only consequence of the way how the object of property of type org.bson.Document is treated. Yes, org.bson.Document is implementing a Map, but it should not be processed as a Map.

If we compare codes of version 3.1.5 and version 3.2.X

First, let's look at version 3.1.5:

MappingMongoConverter#readValue

@Nullable
@SuppressWarnings("unchecked")
<T> T readValue(Object value, TypeInformation<?> type, ObjectPath path) {

	Class<?> rawType = type.getType();

	if (conversions.hasCustomReadTarget(value.getClass(), rawType)) {
		return (T) conversionService.convert(value, rawType);
	} else if (value instanceof DBRef) {
		return potentiallyReadOrResolveDbRef((DBRef) value, type, path, rawType);
	} else if (value instanceof List) {
		return (T) readCollectionOrArray(type, (List<Object>) value, path);
	} else if (value instanceof Document) {
		return (T) read(type, (Document) value, path); // this "if branch" comes into play if a property of type org.bson.Document is read.
	} else if (value instanceof DBObject) {
		return (T) read(type, (BasicDBObject) value, path);
	} else {
		return (T) getPotentiallyConvertedSimpleRead(value, rawType);
	}
}

Then MappingMongoConverter#read is called and the "if (Document.class.isAssignableFrom(rawType))" is chosen as a processing branch).

@Nullable
@SuppressWarnings("unchecked")
private <S extends Object> S read(TypeInformation<S> type, Bson bson, ObjectPath path) {

	Assert.notNull(bson, "Bson must not be null!");

	TypeInformation<? extends S> typeToUse = typeMapper.readType(bson, type);
	Class<? extends S> rawType = typeToUse.getType();

	if (conversions.hasCustomReadTarget(bson.getClass(), rawType)) {
		return conversionService.convert(bson, rawType);
	}

	if (Document.class.isAssignableFrom(rawType)) { // this ensures that a property of type org.bson.Document is not handled at all, which is correct
		return (S) bson;
	}

	if (DBObject.class.isAssignableFrom(rawType)) { // I think that this "if branch" should be also applied within MappingMongoConverter.ConversionContext#convert 

		if (bson instanceof DBObject) {
			return (S) bson;
		}

		if (bson instanceof Document) {
			return (S) new BasicDBObject((Document) bson);
		}

		return (S) bson;
	}

	if (typeToUse.isCollectionLike() && bson instanceof List) {
		return (S) readCollectionOrArray(typeToUse, (List<?>) bson, path);
	}

	if (typeToUse.isMap()) {
		return (S) readMap(typeToUse, bson, path);
	}

	if (bson instanceof Collection) {
		throw new MappingException(String.format(INCOMPATIBLE_TYPES, bson, BasicDBList.class, typeToUse.getType(), path));
	}

	if (typeToUse.equals(ClassTypeInformation.OBJECT)) {
		return (S) bson;
	}
	// Retrieve persistent entity info

	Document target = bson instanceof BasicDBObject ? new Document((BasicDBObject) bson) : (Document) bson;

	MongoPersistentEntity<?> entity = mappingContext.getPersistentEntity(typeToUse);

	if (entity == null) {

		if (codecRegistryProvider != null) {

			Optional<? extends Codec<? extends S>> codec = codecRegistryProvider.getCodecFor(rawType);
			if (codec.isPresent()) {
				return codec.get().decode(new JsonReader(target.toJson()), DecoderContext.builder().build());
			}
		}

		throw new MappingException(String.format(INVALID_TYPE_TO_READ, target, typeToUse.getType()));
	}

	return read((MongoPersistentEntity<S>) entity, target, path);
} 

Now, let's look at MappingMongoConverter.ConversionContext#convert of version 3.2.X, more precisely 3.2.2:

public <S extends Object> S convert(Object source, TypeInformation<? extends S> typeHint) {

	Assert.notNull(typeHint, "TypeInformation must not be null");

	if (conversions.hasCustomReadTarget(source.getClass(), typeHint.getType())) {
		return (S) elementConverter.convert(source, typeHint);
	}
  
  // here is no check whether a document should be handled at all.

	if (source instanceof Collection) {

		Class<?> rawType = typeHint.getType();
		if (!Object.class.equals(rawType)) {
			if (!rawType.isArray() && !ClassUtils.isAssignable(Iterable.class, rawType)) {
				throw new MappingException(
						String.format(INCOMPATIBLE_TYPES, source, source.getClass(), rawType, getPath()));
			}
		}

		if (typeHint.isCollectionLike() || typeHint.getType().isAssignableFrom(Collection.class)) {
			return (S) collectionConverter.convert(this, (Collection<?>) source, typeHint);
		}
	}

	if (typeHint.isMap()) { // this "if branch" is applied
		return (S) mapConverter.convert(this, (Bson) source, typeHint);
	}

	if (source instanceof DBRef) {
		return (S) dbRefConverter.convert(this, (DBRef) source, typeHint);
	}

	if (source instanceof Collection) {
		throw new MappingException(
				String.format(INCOMPATIBLE_TYPES, source, BasicDBList.class, typeHint.getType(), getPath()));
	}

	if (source instanceof Bson) {
		return (S) documentConverter.convert(this, (Bson) source, typeHint);
	}

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

I think, the right way is to let untouched inputs which should be treated (converted into) as org.bson.Document (suggested as typeHint param).

Can you please look at the problem? I believe I provided enough info, but if you need further details, do not hesitate to contact me.

Thank you for your cooperation.

Regards.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 11, 2021
@christophstrobl
Copy link
Member

Thanks for reporting. I might be missing something here but unfortunately, I was not able to reproduce the issue based on the given information.
Could you please share a sample of the raw source document and the desired mapping target type. Ideally a complete minimal sample (something that we can unzip or git clone, build, and deploy) that reproduces the problem.

@christophstrobl christophstrobl added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 12, 2021
@meloujir
Copy link
Author

Hi @christophstrobl,

I'm sorry it wasn't clear, I tried to point to a specific location where is the problem.

To save time, I attach a raw document [1] and screenshots from the debugging of tests with use of both versions of spring data mongodb. Screenshots description is bellow (and all important things are also highlighted on images):

  • Version 3.1.5
    • [2] Describes an instance of org.bson.Document as it was returned from MongoDB driver. Before MappingMongoConverter comes into play.
    • [3] Describes an instance of a class which was created during conversion process incorporating MappingMongoConverter.
  • Version 3.2.2
    • [4] Same as [2].
    • [5] Similar to [3], but all inner object properties of data attribute, which is of type org.bson.Document in the model, are converted into a LinkedHashMap (from org.bson.Document). As I wrote, the consequence is that if we convert the data attribute to a specific type within some application use case, the ConverterNotFoundException error occurs. However, this is only a consequence of the fact that the data attribute (org.bson.Document) was incorrectly modified earlier, I do not want you to try to simulate the given ConverterNotFoundException, the important thing is that an attribute of type org.bson.Document should not be compromised by MappingMongoConverter, where hint type is org.bson.Document.

If it is still not clear what the problem is, I will create a simplified example in my free time, but I hope, it won't be necessary.

[1] https://drive.google.com/file/d/1lTF5p8mRjXbGB7pMzN7uib-Yq8pHAAl_/view
[2] https://drive.google.com/file/d/1qxw5Wb1jf1tDAboMBH4xIIZYIH2Vqh7b/view
[3] https://drive.google.com/file/d/1-_298PxBJhWLIAkFlFPhMbLvLkzXpx3r/view
[4] https://drive.google.com/file/d/1f3E3BzA3cfu3zAuPCJBERiIZRB4ROjGB/view
[5] https://drive.google.com/file/d/1VbTvup6wiVnYGdLgsH07933sPPWNcULt/view

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 12, 2021
@christophstrobl
Copy link
Member

Thanks @meloujir - I see - this seems to happen for Document within a collection property. Good catch - we'll take care of this. So, no need to spend your free time on a reproducer.
We'd appreciate if you'd use code fences instead of images nevertheless.

@christophstrobl christophstrobl added type: regression A regression from a previous release and removed status: feedback-provided Feedback has been provided labels Jul 12, 2021
@mp911de mp911de added this to the 3.2.3 (2021.0.3) milestone Jul 15, 2021
@mp911de mp911de changed the title MappingMongoConverter incorrectly processes an object property of type org.bson.Document MappingMongoConverter incorrectly processes an object property of type org.bson.Document Jul 15, 2021
mp911de added a commit that referenced this issue Jul 15, 2021
Support DBObject and Map that as source for entity materialization and map conversion.

See #3702
Original pull request: #3704.
mp911de pushed a commit that referenced this issue Jul 15, 2021
Along the lines make sure to convert map like structures correctly if they do not come as a Document, eg. cause they got converted to a plain Map in a post load, pre convert event.

Closes #3702
Original pull request: #3704.
mp911de added a commit that referenced this issue Jul 15, 2021
Support DBObject and Map that as source for entity materialization and map conversion.

See #3702
Original pull request: #3704.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: regression A regression from a previous release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants