From b6146e01793def7ca9a239936ee9991d02710d5e Mon Sep 17 00:00:00 2001 From: Matthias Eichner Date: Thu, 22 Oct 2026 15:23:51 +0200 Subject: [PATCH] MCR-3886 skip already deleted children in CLI delete commands "delete all objects of type" and "delete object from .. to .." generate the delete commands up front. Children that were already deleted with their parent are now skipped instead of failing (OCFL) or firing a DELETE event for an empty object (ifs2). - add command "delete existing object {0}" that skips non-existing objects and use it in all commands that generate delete commands - "delete object {0}" is unchanged and still removes leftovers of objects without metadata - add tests --- .../frontend/cli/MCRObjectCommands.java | 38 +++++- .../frontend/cli/MCRObjectCommandsTest.java | 127 ++++++++++++++++++ 2 files changed, 161 insertions(+), 4 deletions(-) create mode 100644 mycore-base/src/test/java/org/mycore/frontend/cli/MCRObjectCommandsTest.java diff --git a/mycore-base/src/main/java/org/mycore/frontend/cli/MCRObjectCommands.java b/mycore-base/src/main/java/org/mycore/frontend/cli/MCRObjectCommands.java index b89fdec8b3..45172705df 100644 --- a/mycore-base/src/main/java/org/mycore/frontend/cli/MCRObjectCommands.java +++ b/mycore-base/src/main/java/org/mycore/frontend/cli/MCRObjectCommands.java @@ -208,7 +208,7 @@ private static void fillWithDescendants(String mcrObjID, List descendant order = 20) public static List deleteAllObjects(String type) { return MCRCommandUtils.getIdsForType(type) - .map(id -> "delete object " + id) + .map(id -> "delete existing object " + id) .collect(Collectors.toList()); } @@ -231,7 +231,7 @@ public static List deleteTopologicalAllObjects() { if (order != null) { //delete in reverse order for (int o = order.length - 1; o >= 0; o--) { - cmds.add("delete object " + ts.getNodeName(order[o])); + cmds.add("delete existing object " + ts.getNodeName(order[o])); } } return cmds; @@ -266,11 +266,41 @@ public static void checkForCircles() { help = "Removes a MCRObject with the MCRObjectID {0}", order = 40) public static void delete(String id) throws MCRPersistenceException, MCRActiveLinkException, MCRAccessException { - MCRObjectID mcrId = MCRObjectID.getInstance(id); + delete(MCRObjectID.getInstance(id)); + } + + private static void delete(MCRObjectID mcrId) + throws MCRPersistenceException, MCRActiveLinkException, MCRAccessException { MCRMetadataManager.deleteMCRObject(mcrId); LOGGER.info("{} deleted.", mcrId); } + /** + * Delete a MCRObject from the datastore if it exists. Used by commands that build a list of delete commands up + * front: children are deleted together with their parent, so their own delete command has to be skipped. + * Unlike {@link #delete(String)}, leftovers of objects without metadata are not removed. + * + * @param id + * the id of the MCRObject that should be deleted + * @throws MCRPersistenceException if a persistence problem is occurred + * @throws MCRAccessException see {@link MCRMetadataManager#deleteMCRObject(MCRObjectID)} + * @throws MCRActiveLinkException if object is referenced by other objects + */ + @MCRCommand( + syntax = "delete existing object {0}", + help = "Removes a MCRObject with the MCRObjectID {0}, skips it if it does not exist (anymore). " + + "Use 'delete object {0}' to remove leftovers of an object without metadata.", + order = 45) + public static void deleteExisting(String id) + throws MCRPersistenceException, MCRActiveLinkException, MCRAccessException { + MCRObjectID mcrId = MCRObjectID.getInstance(id); + if (!MCRMetadataManager.exists(mcrId)) { + LOGGER.info("{} does not exist (anymore), skipping.", mcrId); + return; + } + delete(mcrId); + } + /** * Delete MCRObject's form ID to ID from the datastore. * @@ -286,7 +316,7 @@ public static void delete(String id) throws MCRPersistenceException, MCRActiveLi order = 30) public static List deleteFromTo(String idFrom, String idTo) { return MCRCommandUtils.getIdsFromIdToId(idFrom, idTo) - .map(id -> "delete object " + id) + .map(id -> "delete existing object " + id) .collect(Collectors.toList()); } diff --git a/mycore-base/src/test/java/org/mycore/frontend/cli/MCRObjectCommandsTest.java b/mycore-base/src/test/java/org/mycore/frontend/cli/MCRObjectCommandsTest.java new file mode 100644 index 0000000000..baa1d0dd6f --- /dev/null +++ b/mycore-base/src/test/java/org/mycore/frontend/cli/MCRObjectCommandsTest.java @@ -0,0 +1,127 @@ +/* + * This file is part of *** M y C o R e *** + * See https://www.mycore.de/ for details. + * + * MyCoRe is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * MyCoRe is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with MyCoRe. If not, see . + */ + +package org.mycore.frontend.cli; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; + +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mycore.access.MCRAccessBaseImpl; +import org.mycore.backend.hibernate.MCRHIBLinkTableStore; +import org.mycore.common.MCRTestConfiguration; +import org.mycore.common.MCRTestProperty; +import org.mycore.common.events.MCREvent; +import org.mycore.common.events.MCREvent.ObjectType; +import org.mycore.common.events.MCREventHandlerBase; +import org.mycore.common.events.MCREventManager; +import org.mycore.datamodel.common.MCRLinkTableEventHandler; +import org.mycore.datamodel.common.MCRXMLMetadataEventHandler; +import org.mycore.datamodel.metadata.MCRMetadataManager; +import org.mycore.datamodel.metadata.MCRObject; +import org.mycore.datamodel.metadata.MCRObjectID; +import org.mycore.test.MCRJPAExtension; +import org.mycore.test.MCRMetadataExtension; +import org.mycore.test.MyCoReTest; + +@MyCoReTest +@ExtendWith(MCRJPAExtension.class) +@ExtendWith(MCRMetadataExtension.class) +@MCRTestConfiguration(properties = { + @MCRTestProperty(key = "MCR.Persistence.LinkTable.Store.Class", classNameOf = MCRHIBLinkTableStore.class), + @MCRTestProperty(key = "MCR.Access.Class", classNameOf = MCRAccessBaseImpl.class), + @MCRTestProperty(key = "MCR.Metadata.Type.document", string = "true") +}) +public class MCRObjectCommandsTest { + + private final List deletedObjects = new ArrayList<>(); + + private final MCREventHandlerBase recorder = new MCREventHandlerBase() { + @Override + protected void handleObjectDeleted(MCREvent evt, MCRObject obj) { + deletedObjects.add(obj.getId()); + } + }; + + private MCRObject parent; + + private MCRObject child; + + @BeforeEach + public void setUp() throws Exception { + MCREventManager.getInstance().clear(); + MCREventManager.getInstance().addEventHandler(ObjectType.OBJECT, new MCRXMLMetadataEventHandler()); + MCREventManager.getInstance().addEventHandler(ObjectType.OBJECT, new MCRLinkTableEventHandler()); + MCREventManager.getInstance().addEventHandler(ObjectType.OBJECT, recorder); + parent = createObject("test_document_00000001", null); + child = createObject("test_document_00000002", parent.getId()); + MCRMetadataManager.create(parent); + MCRMetadataManager.create(child); + } + + private MCRObject createObject(String id, MCRObjectID parentId) { + MCRObject object = new MCRObject(); + object.setId(MCRObjectID.getInstance(id)); + object.setSchema("noSchema"); + if (parentId != null) { + object.getStructure().setParent(parentId); + } + return object; + } + + @Test + public void deleteAllObjectsSkipsChildDeletedWithParent() throws Exception { + List commands = MCRObjectCommands.deleteAllObjects("document"); + assertEquals(Set.of("delete existing object " + parent.getId(), "delete existing object " + child.getId()), + Set.copyOf(commands)); + + assertEquals(List.of("delete existing object " + parent.getId(), "delete existing object " + child.getId()), + MCRObjectCommands.deleteFromTo(parent.getId().toString(), child.getId().toString())); + assertEquals(List.of("delete existing object " + child.getId(), "delete existing object " + parent.getId()), + MCRObjectCommands.deleteTopologicalAllObjects()); + + // the child is deleted together with its parent, its own command runs afterwards + MCRObjectCommands.deleteExisting(parent.getId().toString()); + MCRObjectCommands.deleteExisting(child.getId().toString()); + + assertFalse(MCRMetadataManager.exists(parent.getId()), "parent should be deleted"); + assertFalse(MCRMetadataManager.exists(child.getId()), "child should be deleted"); + assertEquals(List.of(child.getId(), parent.getId()), deletedObjects, + "DELETE event should be fired only once for each object"); + } + + @Test + public void deleteRemovesLeftoversOfNonExistingObject() throws Exception { + MCRObjectCommands.delete(parent.getId().toString()); + deletedObjects.clear(); + // only record the event, the stores would log errors for the already deleted object + MCREventManager.getInstance().clear(); + MCREventManager.getInstance().addEventHandler(ObjectType.OBJECT, recorder); + + // "delete object" still fires the DELETE event, so that event handlers can remove leftovers + MCRObjectCommands.delete(child.getId().toString()); + assertEquals(List.of(child.getId()), deletedObjects, "DELETE event should be fired for removed object"); + } + +}