Skip to content

Comments

drop corresponding records from metadataStatus when deleting a metadata#8693

Open
landryb wants to merge 1 commit intogeonetwork:mainfrom
landryb:fix/deletemdstatus
Open

drop corresponding records from metadataStatus when deleting a metadata#8693
landryb wants to merge 1 commit intogeonetwork:mainfrom
landryb:fix/deletemdstatus

Conversation

@landryb
Copy link
Contributor

@landryb landryb commented Mar 13, 2025

for some reason, on a catalog that went through several upgrades without a reinstall, i ended up with strong foreign key constraints between the metadata and metadatastatus tables (and other extra FK constraints that apparently shouldnt be here anymore either but that's another story):

$ \d metadata
Referenced by:
...
      TABLE "metadatastatus" CONSTRAINT "metadatastatus_metadataid_fkey" FOREIGN KEY (metadataid) REFERENCES metadata(id)

$\d metadatastatus

Foreign-key constraints:
...
    "metadatastatus_metadataid_fkey" FOREIGN KEY (metadataid) REFERENCES metadata(id)

in that current state of things, that prevents me from deleting a metadata, using 4.2.8. When going through the teardown of all the records in all related tables, the records in the metadatastatus table are left untouched

2025-03-13T13:25:26,913 DEBUG [org.hibernate.SQL] - delete from OperationAllowed where metadataId=?
2025-03-13T13:25:26,922 DEBUG [org.hibernate.SQL] - delete from MetadataRating where metadataId=?
2025-03-13T13:25:26,925 DEBUG [org.hibernate.SQL] - delete from Validation where metadataId=?
2025-03-13T13:25:26,939 DEBUG [org.hibernate.SQL] - delete from UserSavedSelections where metadataUuid=?
2025-03-13T13:25:27,244 DEBUG [org.hibernate.SQL] - delete from MetadataCateg where metadataId=?
2025-03-13T13:25:27,256 DEBUG [org.hibernate.SQL] - delete from Metadata where id=?

the DELETE /geonetwork/srv/api/records/<id> REST call returns a 204 code, leaving one think the MD has been deleted.

but in fact the metadata ends up being only removed from the ES index, left in the database tables. reindex the MD in ES, and the metadata you think you've just removed is back.

trying to manually delete the record in the metadata table clearly shows that the FK constraint is preventing the deletion:

[db3:5432] geonetwork@geonetwork=> delete from metadata where id=71673598;
ERROR:  update or delete on table "metadata" violates foreign key constraint "metadatastatus_metadataid_fkey" on table "metadatastatus"

so maybe the FK constraint shouldn't be here, but there will still be leftover records in metadataStatus.

with that PR on top of 4.2.8, the records are dropped in the metadataStatus table too:

2025-03-13T14:48:36,465 DEBUG [org.hibernate.SQL] - delete from OperationAllowed where metadataId=?
2025-03-13T14:48:36,479 DEBUG [org.hibernate.SQL] - delete from MetadataRating where metadataId=?
2025-03-13T14:48:36,482 DEBUG [org.hibernate.SQL] - delete from Validation where metadataId=?
2025-03-13T14:48:36,485 DEBUG [org.hibernate.SQL] - delete from MetadataStatus where metadataId=?
2025-03-13T14:48:36,500 DEBUG [org.hibernate.SQL] - delete from UserSavedSelections where metadataUuid=?
2025-03-13T14:48:36,656 DEBUG [org.hibernate.SQL] - delete from Metadata where id=?

and the metadata is then completely removed from the DB.

the issue is probably present in 4.4 or main, just 'hidden' if you dont have the foreign constraint key. STR:

  • delete an existing MD which has entries in the metadataStatus table
  • check that the corresponding entries are still in the metadataStatus table, while the record has been dropped from the other tables.

Checklist

  • I have read the contribution guidelines
  • Pull request provided for main branch, backports managed with label
  • Good housekeeping of code, cleaning up comments, tests, and documentation
  • Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
  • Clean commit messages, longer verbose messages are encouraged
  • API Changes are identified in commit messages
  • Testing provided for features or enhancements using automatic tests
  • User documentation provided for new features or enhancements in manual
  • Build documentation provided for development instructions in README.md files
  • Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

Funded by @gipcraig

@CLAassistant
Copy link

CLAassistant commented Mar 13, 2025

CLA assistant check
All committers have signed the CLA.

@landryb
Copy link
Contributor Author

landryb commented Mar 13, 2025

not 100% sure but the same issue of 'leftover behind' records might also happen if a category was assigned to a metadata, deleting the metadata wont drop the entry from the metadataCategory table.

i also have a fk constraint there, and i cant delete a metadata record if a metadatacateg is linked to it.. which is strange, since in my initial debugging/testing i saw the delete from MetadataCateg where metadataId=? in the SQL.

@jahow
Copy link
Member

jahow commented Mar 21, 2025

Thank you! At first sight this change looks good; could you try to come up with a test demonstrating this fix and preventing future regressions?

I will try to do a more in-depth review

@landryb
Copy link
Contributor Author

landryb commented Mar 21, 2025

i think creating a metadata, changing its status and deleting it, with that it should leave a record in the metadataStatus table.

@jahow
Copy link
Member

jahow commented Mar 21, 2025

Right! But I meant a unit test in the code; it would make this easier to merge

@landryb
Copy link
Contributor Author

landryb commented Mar 24, 2025

Right! But I meant a unit test in the code; it would make this easier to merge

well, i dont think thats something i can do easily in a short timeframe. never wrote tests for GN (and i dunno if thats the case for GN, but when a test setup requires docker i give up).. so dont wait for me on that. might be able to look on the long term..

@josegar74 josegar74 added this to the 4.4.9 milestone May 30, 2025
Copy link
Member

@josegar74 josegar74 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested with a new database and without this change and the metadata can be deleted successfully. The records in the metadata status table are preserved, so the metadata deleted can be restored.

I think with this change, as the metadata status table entries for the metadata are removed, it will not be possible to restore a deleted metadata.

It seems a database migration issue. I would suggest to check the table configuration for metadatastatus of your database and compare it with a new database so can be identified the difference.

Probably the pull request should remove the current change and add a migration script to remove a FK from metadatastatus to metadata table. With a new database I see these 2 FK, please compare with the table in your current database:

metadatastatus-fk

@landryb
Copy link
Contributor Author

landryb commented Jun 2, 2025

i have this table, as said it's been through.. a ton of migrations.

[db:5432] geonetwork@geonetwork=> \d metadatastatus 
                           Table "public.metadatastatus"
         Column          |          Type           | Collation | Nullable | Default 
-------------------------+-------------------------+-----------+----------+---------
 metadataid              | integer                 |           | not null | 
 statusid                | integer                 |           | not null | 0
 userid                  | integer                 |           | not null | 
 changedate              | character varying(30)   |           | not null | 
 changemessage           | character varying(2048) |           | not null | 
 closedate               | character varying(30)   |           |          | 
 currentstate            | text                    |           |          | 
 duedate                 | character varying(30)   |           |          | 
 owner                   | integer                 |           |          | 
 previousstate           | text                    |           |          | 
 titles                  | text                    |           |          | 
 relatedmetadatastatusid | integer                 |           |          | 
 id                      | integer                 |           | not null | 
 uuid                    | character varying(255)  |           | not null | 
Indexes:
    "metadatastatuspk" PRIMARY KEY, btree (id)
    "idx_metadatastatus_changedate" btree (changedate)
    "idx_metadatastatus_metadataid" btree (metadataid)
    "idx_metadatastatus_owner" btree (owner)
    "idx_metadatastatus_statusid" btree (statusid)
    "idx_metadatastatus_userid" btree (userid)
Foreign-key constraints:
    "fkgf12fblfohqs5f8ln9ymj8bph" FOREIGN KEY (statusid) REFERENCES statusvalues(id)
    "fktih0k768u8kuwxsp8dnm9kru3" FOREIGN KEY (relatedmetadatastatusid) REFERENCES metadatastatus(id)
    "metadatastatus_metadataid_fkey" FOREIGN KEY (metadataid) REFERENCES metadata(id)
    "metadatastatus_statusid_fkey" FOREIGN KEY (statusid) REFERENCES statusvalues(id)
    "metadatastatus_userid_fkey" FOREIGN KEY (userid) REFERENCES users(id)
    "metadatastatusrelmdstatusidfk" FOREIGN KEY (relatedmetadatastatusid) REFERENCES metadatastatus(id)
Referenced by:
    TABLE "metadatastatus" CONSTRAINT "fktih0k768u8kuwxsp8dnm9kru3" FOREIGN KEY (relatedmetadatastatusid) REFERENCES metadatastatus(id)
    TABLE "metadatastatus" CONSTRAINT "metadatastatusrelmdstatusidfk" FOREIGN KEY (relatedmetadatastatusid) REFERENCES metadatastatus(id)

dont ask me where those FK constraints come from, and which upgrade should have removed them and when, because upgrades have always been a rollercoaster...

@josegar74
Copy link
Member

josegar74 commented Jun 2, 2025

@landryb it seems that at a certain point all but the first 2 FKs were removed. Also in your table, the 1st and 4th FK are the same, as well as the 2nd and last FK.

I think this was changed in #4817, but I'm not totally sure.

@jahow jahow modified the milestones: 4.4.9, 4.4.10 Oct 7, 2025
landryb added a commit to landryb/georchestra that referenced this pull request Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants