Hhh 18771 index out of bounds exception by list initializer for list index base #9169
Conversation
There was a problem hiding this comment.
@Selaron thank you for the PR! The initializer change looks reasonable to me, but I would ask you to apply our code style to the test code before approving this.
| public static class Person { | ||
|
|
||
| @Id | ||
| private Long id; | ||
|
|
||
| //tag::collections-customizing-ordered-list-ordinal-mapping-example[] | ||
| @OneToMany(mappedBy = "person", cascade = CascadeType.ALL) | ||
| @OneToMany(fetch = FetchType.EAGER, mappedBy = "person", cascade = CascadeType.ALL) |
There was a problem hiding this comment.
Do we need this to be set to EAGER? I'd be careful about changing this specific mapping since it's used in the user guide (see the //tag comments). You already test the children eager collection association with @ListIndexBase, so I would leave this one be.
There was a problem hiding this comment.
I see your point, I could revert this change but then I had to introduce a new, eager loading @OneToMany that would lead to these "magic" side affects required to reproduce the issue.
There was a problem hiding this comment.
It's not magic 😉 and if you don't fully understand what you're testing I suggest spending a little more time on this issue. I just don't see how another association could affect initialization of the children collection, please make sure this is really needed an simplify the mappings as much as possible.
| @ManyToOne | ||
| @JoinColumn(name = "mother_id") | ||
| private Person mother; |
There was a problem hiding this comment.
Not sure why you need this to-one in this test, if it's superfluous please remove it.
There was a problem hiding this comment.
@mbladel I know I uglified the data model of this test case. My aim was to create a reproducer for the HHH-18771 which I found to be kind of complicated - I had to do a number of try-error cycles to reproduce this. I did not find a speaking, canonical straight forward way to get test coverage on ListInitializer#resolveInstanceSubInitializers. Additionally I believe that also ArrayInitializer#resolveInstanceSubInitializers is affected by the issue but I could not manage to cover this at all. I guess finally you hibernate devs should have deeper knowledge required to create a straight forward test covering both mentioned methods and reproducing the issue. Consider my changes as kind of heuristic. If I remove this Attribute mother or the EAGER loading on on the @OneToMany collection above it won't reproduce and I don't know why.
There was a problem hiding this comment.
Since this changes may affect the original test class, producing different results, please create a separate one which only tests the problem described in the issue you're trying to fix and ensures no regressions will be introduced in the future. I suggest trying to only include properties which are needed to reproduce the bug, unless you really need this additional to-one and / or the EAGERness of another association (don't really see why, but I would need to check).
There was a problem hiding this comment.
Hey @Selaron any news on the separate test? I can try taking a look at your reproducer if you don't have the time to dedicate to it, the fix is easy and looks correct so it would be nice to have.
Selaron
force-pushed
the
HHH-18771_IndexOutOfBoundsException_by_ListInitializer_for_listIndexBase
branch
from
df05eb2 to
bb1fa81
Compare
|
@mbladel I replaced my commits leaving the |
Great, looks good to me now - I'll go ahead and merge this.
Could you please open a new Jira for this? We should keep track of the issue, a reproducer might be similar to your test but using an array-typed associated collection ( |
In complex scenarios eager entity initialization involving multi level hierarchy fetching via JOIN and a List attribute with @ListIndexBase(n) where n >= List.size, an IndexOutOfBoundException may be thrown. This PR tests and fixes this issue which is described in HHH-18771 JIRA bug report.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.