| Before I dive into the test failures themselves and discuss those, I wanted to take a brief moment and discuss your change. It would seem your goal is to specifically target a unique condition as it pertains to @ElementCollection mappings. Might I suggest in order to preserve backward compatibility with legacy mappings that we alter your change slightly? In your original PR at line #362 of AbstractCollectionPersister, you change this from
elementColumnIsSettable[j] = true;
to
elementColumnIsSettable[j] = oneToMany || columnInsertability[j];
Instead I would suggest the following:
if ( elementType.isComponentType() ) {
elementColumnIsSettable[j] = oneToMany || columnInsertability[j];
}
else {
elementColumnIsSettable[j] = true;
}
With this very small change to your fix; all the existing ORM and Envers tests pass including your test for this issue. The tests I noticed which were initially impacted by this change were the following:
BasicSametable
BasicBiowned
ValidityAuditStrategyRevEndTsTest
ValidityAuditStrategyRevEndTestCustomRevEnt
The common theme across all these tests is the concept of a bidirectional many-to-many relationship that is owned by both sides of the mapping rather than one side being considered an inverse. And as you pointed out, the join column mappings are specifically annotated with insertable=false, updatable=false. I truly don't grok why the mappings are written as they are, but I am also skeptical to change them. Gail Badner, do you recall if there was a specific reason why they'd be done this way in a legacy version of ORM? Anyway, my suggestion would be to make the small nugget change I proposed to specifically narrow the behavior change solely to @ElementCollection support and believe your fix would be reasonable. |