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

Allowing mapping of third party types creates a hole in validation #1520

Closed
evanchooly opened this issue Oct 29, 2020 · 3 comments
Closed

Allowing mapping of third party types creates a hole in validation #1520

evanchooly opened this issue Oct 29, 2020 · 3 comments
Labels
Breaking bug enhancement experimental implementation details subject to change or removal
Milestone

Comments

@evanchooly
Copy link
Member

Describe the bug
Looking at #1516, it's possible to map types without either top level annotation.

Expected behavior
Asking to map that type, and certainly trying to query with it, should have given a meaningful validation error. This bug creeps in via the mechanism intended to support mapping of third party types that are needed for persistence but can't necessarily be annotated appropriately.

The current allowUnannotated boolean is clearly insufficient. Proposal:

  1. Remove that boolean and revert the hard requirements as originally designed.
  2. Add a new method, e.g., mapExternal that takes an explicit Class<?> reference to allow for embedded use.
  3. Consider opening up @Entity to allow for use on method parameters so users can pass in an @Entity instance to define collection mappings, indexes, caps, etc.

This approach requires manually registering each type vs registering a whole package at once but seems a reasonable trade off given the mapping configuration requirements.

@evanchooly evanchooly modified the milestones: 2.2.0, 2.1.0 Oct 29, 2020
@evanchooly evanchooly added the experimental implementation details subject to change or removal label Oct 29, 2020
@Foorcee
Copy link

Foorcee commented Nov 13, 2020

Will it be possible to map (load) objects without Embedded again in the future? The best thing about Morpia was that you could map everything without specifying anything. Or does it have a technical reason?

@evanchooly
Copy link
Member Author

mapExternal() can currently be passed a null in place of the annotation parameter and a default @Embedded will be generated transparently. That API is still slightly awkward as it would be nice to just pass a bare Class and be done with it. In 2.2, I'm going to explore ... essentially deprecating @Embedded (see #1526) in favor of @Entity which would open up new requirements/options for mapExternal() that would make the API a bit cleaner. But for now, just pass null if you want the defaults.

@evanchooly
Copy link
Member Author

Oh, and to answer the other part, @Embedded and @Entity are required to simplify and fix some bugs when scanning packages, e.g.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking bug enhancement experimental implementation details subject to change or removal
Projects
None yet
Development

No branches or pull requests

2 participants