Repository navigation
chore: DataSet annotation mapping - #25014
Merged
Merged
Conversation
…g orphan-removal exception CI reported HibernateException "A collection with cascade=all-delete-orphan was no longer referenced by the owning entity instance" across DataSet.dataSetElements and DataSet.dataInputPeriods, breaking basic create/patch flows. Two compounding root causes: 1. DataInputPeriod was never migrated to JPA - it was still purely HBM-mapped (DataInputPeriod.hbm.xml) while DataSet.dataInputPeriods already declared a JPA @onetomany @joincolumn targeting it. A JPA-owned @onetomany requires its target to be a genuine @entity; the DB schema already had the datasetid column ready, this was a pure Java-mapping gap. Converted DataInputPeriod to @entity and removed its HBM file. 2. setDataSetElements/setDataInputPeriods/setCompulsoryDataElementOperands had been changed to clear-and-repopulate the existing collection instead of a plain field assignment. Hibernate's own PojoEntityTuplizer calls these setters internally via reflection during session.save()/flush() to install its tracked PersistentCollection wrapper - a setter that doesn't accept the new reference leaves the entity's field pointing at the old collection while Hibernate believes its wrapper was installed, throwing the orphan-removal exception on the next flush, even for a brand-new entity with an empty collection. Reverted to plain assignment; the dataSetElements back-reference wiring (needed since it's the mappedBy/inverse side) now happens in a separate loop alongside the assignment, not instead of it. All 4 originally-failing tests pass (AbstractCrudControllerTest.testPatchRemoveById, testPatchSharingUserGroups, DataSetControllerTest.responseTypeTest, MetadataImportExportControllerTest.removeDataSetIndicatorTest). Full dhis-test-web-api H2-backed suite passes; remaining Testcontainers/Postgres-backed test failures in that run are a pre-existing local Docker-unavailability issue, unrelated to this change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
jbee
approved these changes
Oct 6, 2026
| private int id; | ||
|
|
||
| /** Period data must belong to */ | ||
| @ManyToOne(fetch = FetchType.LAZY) |
Contributor
Author
There was a problem hiding this comment.
I can change to EAGER, so when Hibernate loads that collection, it builds the collection query from the mapping and outer-joins an EAGER many-to-one. So you get the input periods and their Periods in one SQL statement, instead of one query per proxy. This could improve query time a bit.
netroms
approved these changes
Oct 6, 2026
|
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
Sign up for free
to 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.



https://dhis2.atlassian.net/browse/DHIS2-20117
Migrates
DataSetandDataInputPeriodfrom HBM XML to JPA annotations.@JsonSerializenow targetsIdentifiableObjectinstead ofBaseIdentifiableObject/BaseNameableObject.AbstractHibernateListener: audit re-fetches through the proxy only for singular embedded properties or unloaded collections, so embedded collections aren't lost.DataSetStoreTest; added an import test forDataSetElements without back-references.