- Notifications
You must be signed in to change notification settings - Fork1.5k
JAVA-3815: Pojo Codec - Detect property models on extended interfaces#563
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Draft
jflorencio wants to merge2 commits intomongodb:mainChoose a base branch fromjflorencio:pojo-codec-interfaces-master
base:main
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
If you have an interface like:```public interface SampleInterface { int getFirst(); String getSecond();}```which is implemented as:```public abstract class SampleImplementor implements SampleInterface { public abstract boolean isThird();}```With a concrete implementation of SampleImplementor. When`PojoBuilderHelper` goes to create the property models, it only checksfor methods on the current class, and super classes - not interfaces. Inthe above example, this means the property model will only have "third",and not "first" or "second". Today, you can manually get around that bycreating a @BsonCreator by hand:```public abstract class SampleImplementor implements SampleInterface { @BsonCreator public static SampleImplementor newInstance( @BsonProperty("first") int first, @BsonProperty("second") String second, @BsonProperty("third") boolean third) { return new SampleImplementorImpl(first, second, third); } public abstract boolean isThird();}```The presence of the `@BsonProperty` on the `@BsonCreator` method willcreate the property models. Conversely though, if you want to leverage aConvention implementation to dynamically create a `InstanceCreator`that knows how to find `SampleImplementorImpl` above, you won't be ableto. `InstanceCreator` only is provided properties for which`PropertyModel` exists, so above, since `PojoBuilderHelper` didn'tdiscover the interface fields, and there is no exposed API to addproperty models, your `InstanceCreator` will never be provided the`first` and `second` fields present on the interface.Simply put, if you provide the pojo codec a class that is not concrete,extends an interface for methods, and does not have a @BsonCreatorannotation, there is no way to implement a InstanceCreatorimplementation that works. We've worked around this problem for yearsand created 700+ hand written @BsonCreator annotations, but, enough isenough :-)This fix is relatively straight forward: Update `PojoBuilderHelper` toscan implementing classes and interfaces, which provides a fullypopulated property model.
Is there anything I can do to help make progress on this? |
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
If we have an interface like:
which is implemented as:
and has a concrete implementation that's called
SampleImplementorImpl
.When
PojoBuilderHelper
goes to create the property models, it only checksfor methods on the current class and super classes - not interfaces. In
the above example, this means the property model will only have "third" entry -
no "first" or "second" property model. Today, you can manually get around that by
creating a @BsonCreator by hand:
The presence of the
@BsonProperty
on the@BsonCreator
method willcreate the property models. Conversely though, if you want to leverage a
Convention
implementation to dynamically create aInstanceCreator
that knows how to find
SampleImplementorImpl
above, you won't be ableto.
InstanceCreator
is only provided properties for whichPropertyModel
exists, so above, sincePojoBuilderHelper
didn'tdiscover the interface fields, and there is no exposed API to add
property models, your
InstanceCreator
will never be provided thefirst
andsecond
fields present on the interface.Simply put, if you provide the pojo codec a class that is not concrete,
extends an interface for methods, and does not have a
@BsonCreator
annotation, there is no way to implement a
InstanceCreator
implementation that works for the non concrete class. You get stuck in
a place where the class can serialize since the concrete implementation
SampleImplementorImpl
is provided at runtime, but then you have no wayto deserialize it since usages in the code only reference
SampleImplementor
.We've worked around this problem for years and created 700+ hand written
@BsonCreator
annotations, so at this point I want to fix actual the problem.This fix is relatively straight forward: Update
PojoBuilderHelper
toscan implementing classes and interfaces, which provides a fully
populated property model.
JAVA-3815