DATAMONGO-1819 - Do not create PersistentEntities for simple types contributed by a converter.

[mp911de]

We now check for registered simple types in MongoMappingContext.shouldCreatePersistentEntityFor(…) to prevent PersistentEntity creation for unwanted types. Previously, we only checked against MongoSimpleTypes.HOLDER instead of the simple types contributed by converters.

This change impacts aggregate root type use with top-level converters: These types are no longer represented with a PersistentEntity which removes mapping details and these types cannot be used anymore with a repository and require a collection name if used with the Template API.

---

Related ticket: DATAMONGO-1819.

[odrotbohm]

Do you have details on the side-effect?Should we maybe move the JSR-310 converters more closely to the default converters so that would've been caught by the previous implementation of shouldCreatePersistentEntityFor(…)?

[mp911de]

Consider following code:

@Document("another_collection") class Person { … } class PersonWriteConverter implements Converter<Person, Document> { … } class PersonReadConverter implements Converter<Document, Person> { … }

Then you're required to call MongoTemplate.findAll(Person.class, "another_collection") instead of MongoTemplate.findAll(Person.class) (The latter one works because of the bug).

Creating a repository interface PersonRepository extends Repository<Person, String> fails with:

org.springframework.data.mapping.MappingException: Couldn't find PersistentEntity for type class com.acme.Person!	at org.springframework.data.mapping.context.MappingContext.getRequiredPersistentEntity(MappingContext.java:76)	at org.springframework.data.mongodb.repository.support.MongoRepositoryFactory.getEntityInformation(MongoRepositoryFactory.java:149)	at org.springframework.data.mongodb.repository.support.MongoRepositoryFactory.getTargetRepository(MongoRepositoryFactory.java:123)	at org.springframework.data.repository.core.support.RepositoryFactorySupport.getRepository(RepositoryFactorySupport.java:298)	at org.springframework.data.repository.core.support.RepositoryFactorySupport.getRepository(RepositoryFactorySupport.java:259) 

Moving JSR310 support back to MongoSimpleTypes in would fix this particular issue but that's just a patch, not fixing the actual issue. Another possibility to catch more of the unwanted types could be performing the converter-based simple type check on java.* and org.bson.* packages.

[odrotbohm]

I wonder if we should rather make the ….isEntity() check part of the implementation of getPersistentEntity(PersistentProperty). That would prevent that any other calls to that method with a property that's not an entity in the first place would trigger the addition of the type to the mapping context and the code here could even stay untouched.

[mp911de]

Changing either one is rather a patch than a fix to the actual issue. There's no coherent behavior between PersistentProperty.isEntity() and AbstractMappingContext.shouldCreatePersistentEntityFor(…) as both run different checks.

[odrotbohm]

I generally agree, however, I'd argue it sort of has to be that way (to support the use case you described above). What I am trying to say is, on getPersistentEntity(PersistentProperty), we know that we're in the "nested" case, so that stricter rules might apply and actually can be applied. I.e. in case of the lookup of the PersistentEntity for a PersistentProperty we can apply more strict rules before adding a previously not-known type to the MappingContext than when asking for a PersistentEntity on the MappingContext as that implicitly assumes top-level types to some degree.

I'd also it's inline with the previously implicit assumption that looking up a PersistentEntity for a property, that the latter actually is considered one.

[mp911de]

You're right about the relationship between a property and its reference to a top-level type. We generally want to avoid introspection of types that are considered simple ones.

Let's do it that way. I'll file a separate ticket to address the missing simple type check in shouldCreatePersistentEntityFor(…).