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

SimpleTypeHolder.isSimpleType(…) returns different results for enums implementing interfaces [DATACMNS-1278] #1717

Closed
spring-projects-issues opened this issue Mar 9, 2018 · 4 comments
Assignees
Labels
type: bug

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Mar 9, 2018

Tim B opened DATACMNS-1278 and commented

We have the following usage of SimpleTypeHolder (through MappingMongoConverter and MongoCustomConversions):

  • An interface
  • An enum implementing that interface
  • A class with a field of the interface

When we persist an object of the class (with Spring Data MongoDB) and retrieve it we (sometimes) get the following exception:

Document contains: name, ordinal, _class resulting in ReflectionEntityInstantiator being used resulting in 
org.springframework.data.mapping.model.MappingInstantiationException: Failed to instantiate xxx.VersionReasons using constructor private xxx.VersionReasons(java.lang.String,int) with arguments null,null

I debugged and came to the following conclusion: when SimpleTypeHolder is being asked whether the interface is simple it returns false (this seems logical). When we ask SimpleTypeHolder wether the enum implementing the interface is simple the answer depends. Depending the order of the keys of the weak HashSet we get a different result:

  • If the interface key is before the enum key the result is false not simple and the field is stored inside MongoDB with _class, ordinal and value fields.
  • If the interface key is after the enum key the result is true and the field is stored key value in MongoDB.

I tried recreating this behavior in a unit test but so far I was unsuccessful.


Affects: 2.0.5 (Kay SR5)

Issue Links:

  • DATAMONGO-1900 Add section to reference documentation about mapping of enums with interfaces

  • DATAMONGO-1898 Verify handling of enums in MappingMongoConverter

Backported to: 2.0.6 (Kay SR6)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 11, 2018

Tim B commented

Solved this now by providing a Converter from the interface to String.
In version 1.10.x of Spring Data MongoDB this was not needed but with the implementation change of SimpleTypeHolder it seems necessary.

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 11, 2018

Oliver Drotbohm commented

I guess the interesting question is what'd be a correct answer for ….isSimpleType(…) fo an enum implementing an interface. I am not sure we should treat those types as complex ones as otherwise an introduction of an interface on an enum at a point in time where values already were persisted would all of a sudden change the way those values get persisted. That makes me think we should always treat enums and write enum names by default.

If you want to use an interface with the enum and keep fields of that interface type around, you'll have to resort to registering a custom @ReadingConverter Converter<String, InterfaceType> to make sure the values can be read properly.

Thoughts?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 12, 2018

Tim B commented

I forgot this in the description but indeed there already was a Converter<String, InterfaceType> for the read mapping before upgrading Spring Data MongoDB to v2.0.X.
After upgrading it was not used anymore because a _class field was present and Reflection was used to instantiate an instance of the enum type.

Following the logic that Enums are always simple types:
In the case where the Interface would be implemented by both an Enum and a Class and a field exists of the interface type I'm not sure what would happen.
The Enum would be mapped by the Converter.
And the Class objects would be stored with a _class value so Reflection will be used to instantiate them?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 12, 2018

Oliver Drotbohm commented

In SimpleTypeHolder.isSimpleType(…), we now explicitly check for the assignability of the given type so that enums are treated as simple types consistently.

As for your question: on the writing side, everything would work as it did before. A complex implementation would be written as nested document carrying type information in _class. Enums implementing that interface would get written as plain String value. For the reading side that means that class based values can be instantiated as they carry enough type information. Simple String instances would cause a ConverterNotFoundException as we cannot make an educated connection between a plain String and the target interface. Registering a converter for that combination will thus be required for this case. If you already have that, you should be fine.

Feel free to give the snapshots a try

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug
Projects
None yet
Development

No branches or pull requests

2 participants