BUG: Properties of inherited models can be used on different child models#3156
BUG: Properties of inherited models can be used on different child models#3156rPraml wants to merge 1 commit intoebean-orm:masterfrom
Conversation
There was a problem hiding this comment.
PLEASE CHECK:
the idea is, not to call isInstance when no inheritance is used (90% of use cases)
BUT there is no barrier somewhere that prevents the user to use a property from model A on model B.
So the question is, should we pay an additional price for the instance check here to make ebean more robust.
There was a problem hiding this comment.
This seems reasonable.
Q: The only way to use an incorrect property is via io.ebean.plugin.Property correct?
There was a problem hiding this comment.
We sometimes use ebeaninternal.BeanProperty :-) but there is also ExpressionPath.pathGet/Set in the public API
There was a problem hiding this comment.
Technically I don't think this is needed on getValue() but instead on getValueIntercept() only
There was a problem hiding this comment.
Technically I don't think this is needed on setValue() but instead on setValueIntercept() only
There was a problem hiding this comment.
Yes, setValueIntercept should be enough in both cases
|
@rbygrave I've updated this PR. The check is now perfomed in the setIntercept/getIntercept code paths, but is more strict: non inherited beans
inherited beans
|
|
@rbygrave I review all my open PRs. This PR is a small one and IMHO ready to review and merge. |
36f6c91 to
9d85df0
Compare
Hello @rbygrave
we currently are updating to 13.20 and noticed, that #3057 has triggered a bug.
What we do is, that we fetch a property of an inherited root, but the property itself lives in one child node:
As this property seems to be "animal" compatible, we use
prop.value(animal)to read the property value of (any) animal.But if the animal is a dog, and the property comes from cat, you are just reading the same property index from an other bean.
(Note
DB.getDefault().pluginApi().beanType(Animal.class).property("registrationNumber")has the same property index asname)before #3057 this works by luck in our use case, as the property was unloaded and so, it just returns null.
Now as a lazy load occurs, we were wondering, why we get a value, where we do not expect one
The PR provides a test and a suggested fix