From fbb1191bdc8c0b26e2c38773d988c90749e4e9c4 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Tue, 29 Oct 2024 14:26:23 -0500 Subject: [PATCH 01/11] Allow multiple level of machine design catalog element representation. Closes #1688 --- .../ItemDomainMachineDesignBaseController.java | 6 ------ .../ItemDomainMachineDesignBaseControllerUtility.java | 8 -------- 2 files changed, 14 deletions(-) diff --git a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/ItemDomainMachineDesignBaseController.java b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/ItemDomainMachineDesignBaseController.java index 55de482cf..45c8a990a 100644 --- a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/ItemDomainMachineDesignBaseController.java +++ b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/ItemDomainMachineDesignBaseController.java @@ -3475,12 +3475,6 @@ public void prepareMachineAssignRepresentingAssemblyElement() { } if (parentMachineDesign != null) { - ItemElement representsCatalogElement = parentMachineDesign.getRepresentsCatalogElement(); - if (representsCatalogElement != null) { - SessionUtility.addErrorMessage("Represented Parent", "Parent item is represented by another assembly part, cannot link another level of representation."); - return; - } - Item assignedItem = parentMachineDesign.getAssignedItem(); if (assignedItem != null && assignedItem.getItemElementDisplayList().size() != 0) { diff --git a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignBaseControllerUtility.java b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignBaseControllerUtility.java index e534eedcf..d08d321aa 100644 --- a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignBaseControllerUtility.java +++ b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignBaseControllerUtility.java @@ -729,14 +729,6 @@ private void updateRepresentingAssemblyElementForMachine(ItemDomainMachineDesign throw new InvalidObjectState(errMessage); } } - - ItemDomainMachineDesign parentMachineDesign = node.getParentMachineDesign(); - if (representedElement != null && parentMachineDesign.getRepresentsCatalogElement() != null) { - String errMessage = node.getName(); - errMessage += " has a parent " + parentMachineDesign.getName(); - errMessage += " with represented element reference."; - throw new InvalidObjectState(errMessage); - } List elements = fetchElementsAvaiableForNodeRepresentation(node); From 3172d36ccf73017b356fa9c504f18784493ffc41 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Fri, 30 Oct 2026 14:34:09 -0500 Subject: [PATCH 02/11] Add appropriate foreign key constraint for represented element id. It was missing from prior functioantliy. --- db/sql/create_cdb_tables.sql | 4 +++- ...-dangling-item-element-representations.sql | 23 +++++++++++++++++++ db/sql/updates/updateTo3.19.0.sql | 6 +++++ 3 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 db/sql/scripts/cleanup-dangling-item-element-representations.sql diff --git a/db/sql/create_cdb_tables.sql b/db/sql/create_cdb_tables.sql index 1173f20a0..529ce735f 100644 --- a/db/sql/create_cdb_tables.sql +++ b/db/sql/create_cdb_tables.sql @@ -362,11 +362,13 @@ CREATE TABLE `item_element` ( KEY `item_element_k3` (`contained_item_id2`), KEY `item_element_k4` (`entity_info_id`), KEY `item_element_k5` (`derived_from_item_element_id`), + KEY `item_element_k6` (`represents_item_element_id`), CONSTRAINT `item_element_fk1` FOREIGN KEY (`parent_item_id`) REFERENCES `item` (`id`) ON UPDATE CASCADE, CONSTRAINT `item_element_fk2` FOREIGN KEY (`contained_item_id1`) REFERENCES `item` (`id`) ON UPDATE CASCADE, CONSTRAINT `item_element_fk3` FOREIGN KEY (`contained_item_id2`) REFERENCES `item` (`id`) ON UPDATE CASCADE, CONSTRAINT `item_element_fk4` FOREIGN KEY (`entity_info_id`) REFERENCES `entity_info` (`id`) ON UPDATE CASCADE, - CONSTRAINT `item_element_fk5` FOREIGN KEY (`derived_from_item_element_id`) REFERENCES `item_element` (`id`) ON UPDATE CASCADE ON DELETE SET NULL + CONSTRAINT `item_element_fk5` FOREIGN KEY (`derived_from_item_element_id`) REFERENCES `item_element` (`id`) ON UPDATE CASCADE ON DELETE SET NULL, + CONSTRAINT `item_element_fk6` FOREIGN KEY (`represents_item_element_id`) REFERENCES `item_element` (`id`) ON UPDATE CASCADE ) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8; -- diff --git a/db/sql/scripts/cleanup-dangling-item-element-representations.sql b/db/sql/scripts/cleanup-dangling-item-element-representations.sql new file mode 100644 index 000000000..aea815ea9 --- /dev/null +++ b/db/sql/scripts/cleanup-dangling-item-element-representations.sql @@ -0,0 +1,23 @@ +-- +-- Copyright (c) UChicago Argonne, LLC. All rights reserved. +-- See LICENSE file. +-- +-- Review dangling representation references before clearing them. +-- Execute by running `mysql CDB_DB_NAME -h 127.0.0.1 -u cdb -p < cleanup-dangling-item-element-representations.sql` + +SELECT + representing.id, + representing.parent_item_id, + representing.represents_item_element_id +FROM item_element AS representing +LEFT JOIN item_element AS represented + ON represented.id = representing.represents_item_element_id +WHERE representing.represents_item_element_id IS NOT NULL + AND represented.id IS NULL; + +UPDATE item_element AS representing +LEFT JOIN item_element AS represented + ON represented.id = representing.represents_item_element_id +SET representing.represents_item_element_id = NULL +WHERE representing.represents_item_element_id IS NOT NULL + AND represented.id IS NULL; diff --git a/db/sql/updates/updateTo3.19.0.sql b/db/sql/updates/updateTo3.19.0.sql index ba79ab276..e5aa227bf 100644 --- a/db/sql/updates/updateTo3.19.0.sql +++ b/db/sql/updates/updateTo3.19.0.sql @@ -5,6 +5,12 @@ -- Execute by running `mysql CDB_DB_NAME -h 127.0.0.1 -u cdb -p < updateTo3.19.0.sql` +SELECT 'If adding item_element_fk6 reports dangling references, review and run db/sql/scripts/cleanup-dangling-item-element-representations.sql before retrying this update.' AS recommendation; + +ALTER TABLE item_element + ADD KEY `item_element_k6` (`represents_item_element_id`), + ADD CONSTRAINT `item_element_fk6` FOREIGN KEY (`represents_item_element_id`) REFERENCES `item_element` (`id`) ON UPDATE CASCADE; + DROP VIEW IF EXISTS v_ioc_owner_info; CREATE VIEW v_ioc_owner_info AS From 1f4927326dc587ad43575998147dc4f39416db23 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Fri, 30 Oct 2026 14:42:34 -0500 Subject: [PATCH 03/11] update constraints of item elements --- .../objects/ItemElementConstraintInformation.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/view/objects/ItemElementConstraintInformation.java b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/view/objects/ItemElementConstraintInformation.java index 19b2455da..6793907e7 100644 --- a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/view/objects/ItemElementConstraintInformation.java +++ b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/view/objects/ItemElementConstraintInformation.java @@ -43,6 +43,9 @@ public boolean isSafeToRemove() { } public boolean isSafeToUpdateContainedItem() { + if (isReferencedByRepresentingMachine()) { + return false; + } for (ItemElementConstraintInformation ieci : relatedConstraintInfo) { if (ieci.isPreventUpdateContainedItem()) { return false; @@ -64,6 +67,9 @@ protected boolean isPreventUpdateContainedItem() { } protected boolean isPreventsDelete() { + if (isReferencedByRepresentingMachine()) { + return true; + } // the primary element cannot prevent delete if (isHasProperties()) { return true; @@ -83,6 +89,11 @@ public boolean isHasLogs() { return logList != null && logList.isEmpty() == false; } + public boolean isReferencedByRepresentingMachine() { + List representingElements = itemElement.getRepresentsItemElementList(); + return representingElements != null && !representingElements.isEmpty(); + } + public String getRelatedItemElementToString() { return itemElement.toString(); } From 93c1f38682c80f2a1e71db2615b781fe392c27e9 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Fri, 30 Oct 2026 14:49:15 -0500 Subject: [PATCH 04/11] Prevent removal of represented element and catalog replcement while children representing in machine. --- .../utilities/ItemElementControllerUtility.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemElementControllerUtility.java b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemElementControllerUtility.java index 681eb562c..3485952b8 100644 --- a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemElementControllerUtility.java +++ b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemElementControllerUtility.java @@ -36,12 +36,12 @@ protected void prepareEntityDestroy(ItemElement itemElement, UserInfo userInfo) super.prepareEntityDestroy(itemElement, userInfo); // Verify that item domain allows destroy of item element. + ItemElementConstraintInformation ieci = getFreshItemElementConstraintInformation(itemElement); Item parentItem = itemElement.getParentItem(); - if (!(parentItem instanceof ItemDomainCatalog)) { - ItemElementConstraintInformation ieci = getFreshItemElementConstraintInformation(itemElement); + if (!(parentItem instanceof ItemDomainCatalog) || ieci.isReferencedByRepresentingMachine()) { if (ieci.isSafeToRemove() == false) { String constraintMessage = generateSingleDeleteConstraintMessage(itemElement); - throw new CdbException("Cannot remove item element. Constrains not met. " + constraintMessage); + throw new CdbException("Cannot remove item element. Constraints not met. " + constraintMessage); } } } @@ -56,6 +56,9 @@ private String generateSingleDeleteConstraintMessage(ItemElement itemElement) { ItemElementConstraintInformation constraint = getItemElementConstraintInformation(itemElement); String message = null; + if (constraint.isReferencedByRepresentingMachine()) { + return "Item element is represented by a machine design element. Unlink it before continuing."; + } if (constraint instanceof CatalogItemElementConstraintInformation) { CatalogItemElementConstraintInformation catalogConstraint = (CatalogItemElementConstraintInformation) constraint; message = generateCatalogSpecificPreventDeleteUpdateContainedMessage(catalogConstraint); @@ -137,6 +140,10 @@ protected void prepareEntityUpdate(ItemElement itemElement, UserInfo userInfo) t ItemElementConstraintInformation ieci = getItemElementConstraintInformation(freshDbItemElement); if (ieci.isSafeToUpdateContainedItem() == false) { itemElement.setContainedItem(originalContainedItem); + if (ieci.hasRepresentedMachineChildren()) { + throw new CdbException("Cannot replace the item assigned to element " + itemElement + + " while its representing machine has represented children. Unlink the children before continuing."); + } throw new CdbException("Cannot update item element " + itemElement + " due to constraints not met. Please reload the item details page and try again."); } } From 7fbb97d3861b01a41480cb78ddef690799350a56 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Fri, 30 Oct 2026 14:54:04 -0500 Subject: [PATCH 05/11] Shared validation of represtentation. Prevent chanages that would invalidate represented elements --- ...ainMachineDesignBaseControllerUtility.java | 115 +++++++++++++++--- 1 file changed, 100 insertions(+), 15 deletions(-) diff --git a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignBaseControllerUtility.java b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignBaseControllerUtility.java index 636671c34..ea9a828a6 100644 --- a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignBaseControllerUtility.java +++ b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignBaseControllerUtility.java @@ -723,10 +723,80 @@ private void updateTemplateReferenceForAssignedItem( } public void updateRepresentingAssemblyElementForMachine(ItemDomainMachineDesign node, ItemElement element, boolean matchElementNamesForTemplateInstances) throws InvalidArgument, InvalidObjectState { + preflightRepresentingAssemblyElementUpdate(node, element, matchElementNamesForTemplateInstances, false); node.clearItemsToUpdate(); updateRepresentingAssemblyElementForMachine(node, element, matchElementNamesForTemplateInstances, false); } + private void preflightRepresentingAssemblyElementUpdate(ItemDomainMachineDesign node, + ItemElement representedElement, boolean matchElementNamesForTemplateInstances, + boolean updateFromTemplate) throws InvalidArgument, InvalidObjectState { + ItemElement resolvedElement = resolveRepresentedElement(node, representedElement, + matchElementNamesForTemplateInstances, updateFromTemplate); + Item resultingAssignment = resolvedElement == null + ? node.getSelfElement().getContainedItem2() : resolvedElement.getContainedItem(); + validateRepresentedChildrenForAssignment(node, resultingAssignment); + + if (node.getIsItemTemplate()) { + List instances + = (List) (List) node.getItemsCreatedFromThisTemplateItem(); + for (ItemDomainMachineDesign instance : instances) { + preflightRepresentingAssemblyElementUpdate(instance, representedElement, + matchElementNamesForTemplateInstances, true); + } + } + } + + private ItemElement resolveRepresentedElement(ItemDomainMachineDesign node, + ItemElement representedElement, boolean matchElementNamesForTemplateInstances, + boolean updateFromTemplate) throws InvalidArgument, InvalidObjectState { + if (representedElement == null) { + return null; + } + + List elements = fetchElementsAvaiableForNodeRepresentation(node); + if (elements != null && elements.contains(representedElement)) { + if (node.getSelfElement().getContainedItem2() != null) { + throw new InvalidObjectState(node.getName() + " has an assigned item set."); + } + return representedElement; + } + + if (elements != null && matchElementNamesForTemplateInstances && updateFromTemplate) { + for (ItemElement assemblyElement : elements) { + if (assemblyElement.getName().equals(representedElement.getName())) { + return assemblyElement; + } + } + } + + throw new InvalidArgument("Element provided is not an available element for representation for node " + + node.getName() + "."); + } + + private void validateRepresentedChildrenForAssignment(ItemDomainMachineDesign node, + Item resultingAssignment) throws InvalidObjectState { + Item catalogAssignment = resultingAssignment; + if (catalogAssignment != null && catalogAssignment.getDerivedFromItem() != null) { + catalogAssignment = catalogAssignment.getDerivedFromItem(); + } + List resultingElements = catalogAssignment == null + ? new ArrayList<>() : catalogAssignment.getItemElementDisplayList(); + + for (ItemElement machineChild : node.getItemElementDisplayList()) { + Item containedItem = machineChild.getContainedItem(); + if (containedItem instanceof ItemDomainMachineDesign) { + ItemDomainMachineDesign child = (ItemDomainMachineDesign) containedItem; + ItemElement childRepresentation = child.getRepresentsCatalogElement(); + if (childRepresentation != null && !resultingElements.contains(childRepresentation)) { + throw new InvalidObjectState("Cannot update represented element for " + node.getName() + + " while child " + child.getName() + + " represents an element outside the resulting assembly."); + } + } + } + } + private void updateRepresentingAssemblyElementForMachine(ItemDomainMachineDesign node, ItemElement representedElement, boolean matchElementNamesForTemplateInstances, boolean updateFromTemplate) throws InvalidArgument, InvalidObjectState { // Ensure this can only be applied for standard machine elements and template items. if (node.getEntityTypeList().size() == 1) { @@ -752,16 +822,8 @@ private void updateRepresentingAssemblyElementForMachine(ItemDomainMachineDesign List elements = fetchElementsAvaiableForNodeRepresentation(node); - // Updating of element if (representedElement != null) { - if (elements != null && elements.contains(representedElement)) { - // Valid element. Update the element set. - if (node.getAssignedItem() != null) { - String errMessage = node.getName(); - errMessage += " has an assigned item set."; - throw new InvalidObjectState(errMessage); - } - } else { + if (elements == null || !elements.contains(representedElement)) { boolean found = false; if (elements != null && matchElementNamesForTemplateInstances && updateFromTemplate) { String elementName = representedElement.getName(); @@ -778,8 +840,13 @@ private void updateRepresentingAssemblyElementForMachine(ItemDomainMachineDesign throw new InvalidArgument("Element provided is not an available element for representation for node " + node.getName() + "."); } } - } else { - // No additional validation needed for clearing represented element. + validateElementAvailableForRepresentation(node.getParentMachineDesign(), node, representedElement); + + if (node.getSelfElement().getContainedItem2() != null) { + String errMessage = node.getName(); + errMessage += " has an assigned item set."; + throw new InvalidObjectState(errMessage); + } } node.setRepresentsCatalogElement(representedElement); @@ -796,9 +863,26 @@ private void updateRepresentingAssemblyElementForMachine(ItemDomainMachineDesign } } + public void validateElementAvailableForRepresentation(ItemDomainMachineDesign parentMachineDesign, + ItemDomainMachineDesign node, ItemElement representedElement) throws InvalidArgument { + if (representedElement == null) { + throw new InvalidArgument("Assembly element provided for representation was not found."); + } + + List availableElements = fetchElementsAvaiableForNodeRepresentation(parentMachineDesign, node); + if (availableElements == null || !availableElements.contains(representedElement)) { + String parentName = parentMachineDesign == null ? "N/A" : parentMachineDesign.getName(); + throw new InvalidArgument("Element provided is not an available element for representation under parent machine " + + parentName + "."); + } + } + public List fetchElementsAvaiableForNodeRepresentation(ItemDomainMachineDesign node) { - ItemDomainMachineDesign parentMachineDesign = node.getParentMachineDesign(); + return fetchElementsAvaiableForNodeRepresentation(node.getParentMachineDesign(), node); + } + private List fetchElementsAvaiableForNodeRepresentation(ItemDomainMachineDesign parentMachineDesign, + ItemDomainMachineDesign nodeToIgnore) { List availableElementsForNodeRepresentation = null; if (parentMachineDesign != null) { @@ -812,13 +896,14 @@ public List fetchElementsAvaiableForNodeRepresentation(ItemDomainMa availableElementsForNodeRepresentation = new ArrayList<>(); availableElementsForNodeRepresentation.addAll(assignedCat.getItemElementDisplayList()); - // Remove any already assigned elements. + // Remove elements represented by siblings. List machineChildren = parentMachineDesign.getItemElementDisplayList(); for (ItemElement machineChild : machineChildren) { ItemDomainMachineDesign containedItem = (ItemDomainMachineDesign) machineChild.getContainedItem(); - ItemElement representingElement = containedItem.getRepresentsCatalogElement(); - availableElementsForNodeRepresentation.remove(representingElement); + if (!ObjectUtility.equals(containedItem, nodeToIgnore)) { + availableElementsForNodeRepresentation.remove(containedItem.getRepresentsCatalogElement()); + } } } } From 678abfd7a9a18700d4e233618f0e113d5f74d49c Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Fri, 30 Oct 2026 14:54:47 -0500 Subject: [PATCH 06/11] Utilize the new validation of element functioanlity. --- ...temDomainMachineDesignControllerUtility.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtility.java b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtility.java index 5da14713e..7834548b5 100644 --- a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtility.java +++ b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtility.java @@ -57,6 +57,17 @@ public ItemElement performMachineMoveWithUpdate(ItemDomainMachineDesign newParen */ public ItemDomainMachineDesign createRepresentingMachineForAssemblyElement(ItemDomainMachineDesign parentMachine, ItemElement catalogElement, UserInfo creatorUser) throws InvalidArgument, CdbException { + if (catalogElement == null) { + throw new InvalidArgument("Assembly element provided for representation was not found."); + } + + // Inventory assembly elements represent their source catalog element. + ItemElement derivedFromItemElement = catalogElement.getDerivedFromItemElement(); + if (derivedFromItemElement != null) { + catalogElement = derivedFromItemElement; + } + validateElementAvailableForRepresentation(parentMachine, null, catalogElement); + Item parentItem = catalogElement.getParentItem(); if ((parentItem instanceof ItemDomainCatalog || parentItem instanceof ItemDomainInventory) == false) { @@ -68,12 +79,6 @@ public ItemDomainMachineDesign createRepresentingMachineForAssemblyElement(ItemD machinePlaceholder = prepareMachinePlaceholder(parentMachine, creatorUser); parentMachine.getFullItemElementList().add(machinePlaceholder); - // Ensure to get catalog element - ItemElement derivedFromItemElement = catalogElement.getDerivedFromItemElement(); - if (derivedFromItemElement != null) { - catalogElement = derivedFromItemElement; - } - ItemDomainMachineDesign newMachine = (ItemDomainMachineDesign) machinePlaceholder.getContainedItem(); newMachine.setRepresentsCatalogElement(catalogElement); From f06bd6643d84a01237c4280ab5769b7f3f3651cc Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Fri, 30 Oct 2026 14:55:53 -0500 Subject: [PATCH 07/11] Add standardized way to check if this represented element has represented children. --- .../ItemElementConstraintInformation.java | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/view/objects/ItemElementConstraintInformation.java b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/view/objects/ItemElementConstraintInformation.java index 6793907e7..38d56a46d 100644 --- a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/view/objects/ItemElementConstraintInformation.java +++ b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/view/objects/ItemElementConstraintInformation.java @@ -4,6 +4,7 @@ */ package gov.anl.aps.cdb.portal.view.objects; +import gov.anl.aps.cdb.portal.model.db.entities.ItemDomainMachineDesign; import gov.anl.aps.cdb.portal.model.db.entities.ItemElement; import gov.anl.aps.cdb.portal.model.db.entities.Log; import gov.anl.aps.cdb.portal.model.db.entities.PropertyValue; @@ -43,7 +44,7 @@ public boolean isSafeToRemove() { } public boolean isSafeToUpdateContainedItem() { - if (isReferencedByRepresentingMachine()) { + if (hasRepresentedMachineChildren()) { return false; } for (ItemElementConstraintInformation ieci : relatedConstraintInfo) { @@ -94,6 +95,26 @@ public boolean isReferencedByRepresentingMachine() { return representingElements != null && !representingElements.isEmpty(); } + public boolean hasRepresentedMachineChildren() { + if (!isReferencedByRepresentingMachine()) { + return false; + } + for (ItemElement representingElement : itemElement.getRepresentsItemElementList()) { + if (representingElement.getParentItem() instanceof ItemDomainMachineDesign) { + ItemDomainMachineDesign machine = (ItemDomainMachineDesign) representingElement.getParentItem(); + for (ItemElement childElement : machine.getItemElementDisplayList()) { + if (childElement.getContainedItem() instanceof ItemDomainMachineDesign) { + ItemDomainMachineDesign child = (ItemDomainMachineDesign) childElement.getContainedItem(); + if (child.getRepresentsCatalogElement() != null) { + return true; + } + } + } + } + } + return false; + } + public String getRelatedItemElementToString() { return itemElement.toString(); } From 65097cdd39694532d9a4a518f4b4e8163da2b6b6 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Fri, 30 Oct 2026 15:00:30 -0500 Subject: [PATCH 08/11] Add tests for new functiaonltiy --- ...DomainMachineDesignRepresentationTest.java | 180 ++++++++++++++++++ .../db/utilities/ItemElementUtilityTest.java | 67 +++++++ 2 files changed, 247 insertions(+) create mode 100644 tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignRepresentationTest.java create mode 100644 tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/model/db/utilities/ItemElementUtilityTest.java diff --git a/tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignRepresentationTest.java b/tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignRepresentationTest.java new file mode 100644 index 000000000..6bf3d75ce --- /dev/null +++ b/tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignRepresentationTest.java @@ -0,0 +1,180 @@ +/* + * Copyright (c) UChicago Argonne, LLC. All rights reserved. + * See LICENSE file. + */ +package gov.anl.aps.cdb.portal.controllers.utilities; + +import gov.anl.aps.cdb.common.exceptions.CdbException; +import gov.anl.aps.cdb.common.exceptions.InvalidArgument; +import gov.anl.aps.cdb.common.exceptions.InvalidObjectState; +import gov.anl.aps.cdb.portal.constants.ItemDomainName; +import gov.anl.aps.cdb.portal.model.db.entities.Domain; +import gov.anl.aps.cdb.portal.model.db.entities.Item; +import gov.anl.aps.cdb.portal.model.db.entities.ItemDomainCatalog; +import gov.anl.aps.cdb.portal.model.db.entities.ItemDomainMachineDesign; +import gov.anl.aps.cdb.portal.model.db.entities.ItemElement; +import gov.anl.aps.cdb.portal.view.objects.CatalogItemElementConstraintInformation; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import org.junit.Test; +import static org.junit.Assert.*; + +public class ItemDomainMachineDesignRepresentationTest { + + private final ItemDomainMachineDesignControllerUtility utility + = new ItemDomainMachineDesignControllerUtility(); + + @Test + public void supportsThreeLevelRepresentation() throws Exception { + ItemDomainCatalog assemblyA = catalog(10, "A"); + ItemDomainCatalog assemblyB = catalog(11, "B"); + ItemDomainCatalog assemblyC = catalog(12, "C"); + ItemElement bElement = catalogElement(assemblyA, assemblyB, "B element"); + ItemElement cElement = catalogElement(assemblyB, assemblyC, "C element"); + + ItemDomainMachineDesign root = machine(1, "root", assemblyA); + ItemDomainMachineDesign middle = childMachine(2, "middle", root); + utility.updateRepresentingAssemblyElementForMachine(middle, bElement, false); + ItemDomainMachineDesign leaf = childMachine(3, "leaf", middle); + utility.updateRepresentingAssemblyElementForMachine(leaf, cElement, false); + + assertSame(bElement, middle.getRepresentsCatalogElement()); + assertSame(cElement, leaf.getRepresentsCatalogElement()); + } + + @Test(expected = InvalidObjectState.class) + public void cannotUnlinkParentWhileChildRepresentsItsAssembly() throws Exception { + ItemDomainCatalog assemblyA = catalog(10, "A"); + ItemDomainCatalog assemblyB = catalog(11, "B"); + ItemElement bElement = catalogElement(assemblyA, assemblyB, "B element"); + ItemElement childElement = catalogElement(assemblyB, catalog(12, "C"), "C element"); + ItemDomainMachineDesign root = machine(1, "root", assemblyA); + ItemDomainMachineDesign middle = childMachine(2, "middle", root); + middle.setRepresentsCatalogElement(bElement); + ItemDomainMachineDesign leaf = childMachine(3, "leaf", middle); + leaf.setRepresentsCatalogElement(childElement); + + utility.updateRepresentingAssemblyElementForMachine(middle, null, false); + } + + @Test + public void assemblyRepresentationSwapRequiresCompatibleChildren() throws Exception { + ItemDomainCatalog assemblyA = catalog(10, "A"); + ItemDomainCatalog assemblyB = catalog(11, "B"); + ItemDomainCatalog assemblyD = catalog(12, "D"); + ItemElement originalB = catalogElement(assemblyA, assemblyB, "B original"); + ItemElement replacementB = catalogElement(assemblyA, assemblyB, "B replacement"); + ItemElement replacementD = catalogElement(assemblyA, assemblyD, "D replacement"); + ItemElement childElement = catalogElement(assemblyB, catalog(13, "C"), "C element"); + ItemDomainMachineDesign root = machine(1, "root", assemblyA); + ItemDomainMachineDesign middle = childMachine(2, "middle", root); + middle.setRepresentsCatalogElement(originalB); + ItemDomainMachineDesign leaf = childMachine(3, "leaf", middle); + leaf.setRepresentsCatalogElement(childElement); + + utility.updateRepresentingAssemblyElementForMachine(middle, replacementB, false); + assertSame(replacementB, middle.getRepresentsCatalogElement()); + + try { + utility.updateRepresentingAssemblyElementForMachine(middle, replacementD, false); + fail("Incompatible assembly replacement was accepted."); + } catch (InvalidObjectState expected) { + } + assertSame(replacementB, middle.getRepresentsCatalogElement()); + } + + @Test + public void promotionValidationRejectsForeignAndDuplicateElements() throws Exception { + ItemDomainCatalog assembly = catalog(10, "assembly"); + ItemElement valid = catalogElement(assembly, catalog(11, "part"), "part"); + ItemElement foreign = catalogElement(catalog(12, "foreign"), catalog(13, "other"), "other"); + ItemDomainMachineDesign parent = machine(1, "parent", assembly); + + assertInvalid(parent, foreign); + ItemDomainMachineDesign sibling = childMachine(2, "sibling", parent); + sibling.setRepresentsCatalogElement(valid); + assertInvalid(parent, valid); + assertInvalid(parent, null); + } + + @Test + public void representedCatalogElementRemovalAndReplacementConstraints() { + ItemElement represented = catalogElement(catalog(10, "assembly"), catalog(11, "part"), "part"); + ItemDomainMachineDesign representingMachine = machine(1, "representing", null); + ItemElement machineSelf = representingMachine.getSelfElement(); + machineSelf.setRepresentsItemElement(represented); + represented.setRepresentsItemElementList(new ArrayList<>(Arrays.asList(machineSelf))); + CatalogItemElementConstraintInformation constraint + = new CatalogItemElementConstraintInformation(represented); + + assertFalse(constraint.isSafeToRemove()); + assertTrue(constraint.isSafeToUpdateContainedItem()); + + ItemDomainMachineDesign child = childMachine(2, "child", representingMachine); + child.setRepresentsCatalogElement(catalogElement(catalog(12, "child assembly"), + catalog(13, "child part"), "child part")); + assertFalse(constraint.isSafeToUpdateContainedItem()); + + child.setRepresentsCatalogElement(null); + assertTrue(constraint.isSafeToUpdateContainedItem()); + + represented.getRepresentsItemElementList().clear(); + assertTrue(constraint.isSafeToRemove()); + } + + private void assertInvalid(ItemDomainMachineDesign parent, ItemElement element) throws CdbException { + try { + utility.validateElementAvailableForRepresentation(parent, null, element); + fail("Invalid representation was accepted."); + } catch (InvalidArgument expected) { + } + } + + private static ItemDomainCatalog catalog(int id, String name) { + ItemDomainCatalog item = new ItemDomainCatalog(); + item.setId(id); + item.setName(name); + item.setDomain(new Domain(ItemDomainName.CATALOG_ID, ItemDomainName.catalog.getValue())); + ItemElement self = new ItemElement(); + self.setParentItem(item); + item.setFullItemElementList(new ArrayList<>(Arrays.asList(self))); + return item; + } + + private static ItemElement catalogElement(ItemDomainCatalog parent, Item contained, String name) { + ItemElement element = new ItemElement(); + element.setName(name); + element.setParentItem(parent); + element.setContainedItem(contained); + parent.getFullItemElementList().add(element); + parent.resetItemElementDisplayList(); + return element; + } + + private static ItemDomainMachineDesign machine(int id, String name, Item assigned) { + ItemDomainMachineDesign machine = new ItemDomainMachineDesign(); + machine.setId(id); + machine.setName(name); + machine.setDomain(new Domain(ItemDomainName.MACHINE_DESIGN_ID, ItemDomainName.machineDesign.getValue())); + machine.setEntityTypeListFromApi(new ArrayList<>()); + machine.setItemElementMemberList(new ArrayList<>()); + ItemElement self = new ItemElement(); + self.setParentItem(machine); + self.setContainedItem2(assigned); + machine.setFullItemElementList(new ArrayList<>(Arrays.asList(self))); + return machine; + } + + private static ItemDomainMachineDesign childMachine(int id, String name, ItemDomainMachineDesign parent) { + ItemDomainMachineDesign child = machine(id, name, null); + ItemElement membership = new ItemElement(); + membership.setName(name); + membership.setParentItem(parent); + membership.setContainedItem(child); + parent.getFullItemElementList().add(membership); + parent.resetItemElementDisplayList(); + child.setItemElementMemberList(new ArrayList<>(Collections.singletonList(membership))); + return child; + } +} diff --git a/tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/model/db/utilities/ItemElementUtilityTest.java b/tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/model/db/utilities/ItemElementUtilityTest.java new file mode 100644 index 000000000..a50d353e1 --- /dev/null +++ b/tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/model/db/utilities/ItemElementUtilityTest.java @@ -0,0 +1,67 @@ +/* + * Copyright (c) UChicago Argonne, LLC. All rights reserved. + * See LICENSE file. + */ +package gov.anl.aps.cdb.portal.model.db.utilities; + +import gov.anl.aps.cdb.common.exceptions.CdbException; +import gov.anl.aps.cdb.portal.constants.ItemDomainName; +import gov.anl.aps.cdb.portal.model.db.entities.Domain; +import gov.anl.aps.cdb.portal.model.db.entities.ItemDomainCatalog; +import gov.anl.aps.cdb.portal.model.db.entities.ItemElement; +import java.util.ArrayList; +import java.util.Arrays; +import org.junit.Test; +import static org.junit.Assert.fail; + +public class ItemElementUtilityTest { + + @Test + public void rejectsDirectCatalogAssemblyCycle() throws Exception { + ItemDomainCatalog assembly = catalog(1, "Assembly"); + addChild(assembly, assembly, "self"); + + assertCycleRejected(assembly); + } + + @Test + public void rejectsIndirectCatalogAssemblyCycle() throws Exception { + ItemDomainCatalog first = catalog(1, "First"); + ItemDomainCatalog second = catalog(2, "Second"); + ItemDomainCatalog third = catalog(3, "Third"); + addChild(first, second, "second"); + addChild(second, third, "third"); + addChild(third, first, "first"); + + assertCycleRejected(first); + } + + private static void assertCycleRejected(ItemDomainCatalog item) throws Exception { + try { + ItemElementUtility.createItemElementRoot(item); + fail("Circular catalog assembly was accepted."); + } catch (CdbException expected) { + // Existing tree validation is the canonical cycle check. + } + } + + private static ItemDomainCatalog catalog(int id, String name) { + ItemDomainCatalog item = new ItemDomainCatalog(); + item.setId(id); + item.setName(name); + item.setDomain(new Domain(ItemDomainName.CATALOG_ID, ItemDomainName.catalog.getValue())); + ItemElement self = new ItemElement(); + self.setParentItem(item); + item.setFullItemElementList(new ArrayList<>(Arrays.asList(self))); + return item; + } + + private static void addChild(ItemDomainCatalog parent, ItemDomainCatalog child, String name) { + ItemElement element = new ItemElement(); + element.setName(name); + element.setParentItem(parent); + element.setContainedItem(child); + parent.getFullItemElementList().add(element); + parent.resetItemElementDisplayList(); + } +} From 516e60b880a58739e96677004490bbe2583f7376 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Fri, 30 Oct 2026 15:06:29 -0500 Subject: [PATCH 09/11] Resolve the tests for the new behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Catalog promotion - Old: Any catalog element could be promoted, even if the parent machine was not assigned its assembly. - New: The element must belong to the parent machine’s assigned assembly. - Inventory promotion - Old: Any inventory element could be promoted and normalized to its catalog element. - New: It must belong to the parent’s assigned inventory/catalog assembly before normalization and promotion. - Unsupported-domain promotion - Old: Rejected with a domain-specific error. - New: Still rejected, but currently fails the parent-assembly availability check first; validation should be reordered to preserve the old error. - Nested representation - Old: Always rejected when the parent machine already represented an element. - New: Allowed when the child represents an element from the parent’s effective represented assembly. --- ...mDomainMachineDesignControllerUtility.java | 12 +++---- ...ainMachineDesignControllerUtilityTest.java | 32 +++++++++++++------ 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtility.java b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtility.java index 7834548b5..0dd870cf4 100644 --- a/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtility.java +++ b/src/java/CdbWebPortal/src/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtility.java @@ -61,6 +61,12 @@ public ItemDomainMachineDesign createRepresentingMachineForAssemblyElement(ItemD throw new InvalidArgument("Assembly element provided for representation was not found."); } + Item parentItem = catalogElement.getParentItem(); + if (!(parentItem instanceof ItemDomainCatalog) + && !(parentItem instanceof ItemDomainInventory)) { + throw new InvalidArgument("Cannot create representing machine element for selected item."); + } + // Inventory assembly elements represent their source catalog element. ItemElement derivedFromItemElement = catalogElement.getDerivedFromItemElement(); if (derivedFromItemElement != null) { @@ -68,12 +74,6 @@ public ItemDomainMachineDesign createRepresentingMachineForAssemblyElement(ItemD } validateElementAvailableForRepresentation(parentMachine, null, catalogElement); - Item parentItem = catalogElement.getParentItem(); - if ((parentItem instanceof ItemDomainCatalog - || parentItem instanceof ItemDomainInventory) == false) { - throw new InvalidArgument("Cannot create representing machine element for selected item."); - } - // Create new machine placeholder ItemElement machinePlaceholder = null; machinePlaceholder = prepareMachinePlaceholder(parentMachine, creatorUser); diff --git a/tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtilityTest.java b/tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtilityTest.java index 654676a45..66865f120 100644 --- a/tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtilityTest.java +++ b/tools/developer_tools/code_testing/CdbWebPortalTest/src/test/java/gov/anl/aps/cdb/portal/controllers/utilities/ItemDomainMachineDesignControllerUtilityTest.java @@ -98,6 +98,7 @@ public void createsRepresentingMachineFromCatalogElement() throws Exception { ItemDomainCatalog part = catalog("Part", project); ItemElement represented = child(assembly, part, "Part-1"); ItemDomainMachineDesign parent = machine("Parent"); + parent.setAssignedItem(assembly); parent.setItemProjectList(Collections.singletonList(project(2, "Parent Project"))); ItemDomainMachineDesign created @@ -124,8 +125,10 @@ public void createsRepresentingMachineFromInventoryAndNormalizesDerivedElement() ItemElement inventoryElement = child(inventoryAssembly, inventoryPart, "Inventory E1"); inventoryElement.setDerivedFromItemElement(catalogElement); + ItemDomainMachineDesign parent = machine("Parent"); + parent.setAssignedItem(inventoryAssembly); ItemDomainMachineDesign created = utility.createRepresentingMachineForAssemblyElement( - machine("Parent"), inventoryElement, user); + parent, inventoryElement, user); assertSame(catalogElement, created.getRepresentsCatalogElement()); assertEquals("Catalog E1", created.getName()); @@ -273,14 +276,25 @@ public void rejectsRepresentationForNodeCreatedFromTemplate() { } @Test - public void rejectsNestedRepresentationBelowRepresentedParent() { - RepresentationFixture fixture = representationFixture(); - fixture.parent.setRepresentsCatalogElement(new ItemElement()); - - InvalidObjectState error = expect(InvalidObjectState.class, - () -> utility.updateRepresentingAssemblyElementForMachine( - fixture.node, fixture.available, false)); - assertTrue(error.getMessage().contains("with represented element reference")); + public void allowsNestedRepresentationBelowRepresentedParent() throws Exception { + ItemDomainCatalog rootAssembly = catalog("Root Assembly"); + ItemDomainCatalog nestedAssembly = catalog("Nested Assembly"); + ItemElement nestedAssemblyElement = child(rootAssembly, nestedAssembly, "Nested E1"); + ItemElement nestedPartElement = child(nestedAssembly, catalog("Nested Part"), "Part E1"); + ItemDomainMachineDesign root = machine("Root"); + root.setAssignedItem(rootAssembly); + ItemDomainMachineDesign representedParent = machine("Represented Parent"); + child(root, representedParent, "M1"); + utility.updateRepresentingAssemblyElementForMachine( + representedParent, nestedAssemblyElement, false); + ItemDomainMachineDesign representedChild = machine("Represented Child"); + child(representedParent, representedChild, "M2"); + + utility.updateRepresentingAssemblyElementForMachine( + representedChild, nestedPartElement, false); + + assertSame(nestedAssemblyElement, representedParent.getRepresentsCatalogElement()); + assertSame(nestedPartElement, representedChild.getRepresentsCatalogElement()); } @Test From f0c7da2920978b864263ae9e1ddc3e0dc2c6bdd4 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Fri, 30 Oct 2026 15:06:38 -0500 Subject: [PATCH 10/11] Add updated release notes --- docs/release-notes/3.19.0.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/release-notes/3.19.0.md b/docs/release-notes/3.19.0.md index 022c93270..ccce71d97 100644 --- a/docs/release-notes/3.19.0.md +++ b/docs/release-notes/3.19.0.md @@ -1,3 +1,8 @@ +# Machine Design + +* Added safe multilevel representation of catalog assembly elements in machine hierarchies. +* Prevented unlinking, replacement, promotion, or deletion from leaving invalid or duplicate representation references. + # Inventory * Added housing assignment, reassignment, replacement, and clearing from inventory details. From f4239128d81e817bd9bf10c141305ee4484e418d Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Fri, 30 Oct 2026 15:38:10 -0500 Subject: [PATCH 11/11] Resolve formatting of machine design template --- .../templates/itemDomainMachineDesignPanelGridTemplate.xhtml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java/CdbWebPortal/web/views/itemDomainMachineDesignBase/private/templates/itemDomainMachineDesignPanelGridTemplate.xhtml b/src/java/CdbWebPortal/web/views/itemDomainMachineDesignBase/private/templates/itemDomainMachineDesignPanelGridTemplate.xhtml index 1141e1dec..ac54feade 100644 --- a/src/java/CdbWebPortal/web/views/itemDomainMachineDesignBase/private/templates/itemDomainMachineDesignPanelGridTemplate.xhtml +++ b/src/java/CdbWebPortal/web/views/itemDomainMachineDesignBase/private/templates/itemDomainMachineDesignPanelGridTemplate.xhtml @@ -27,7 +27,7 @@ See LICENSE file. value="Installed QR Id" rendered="#{!entityController.currentViewIsTemplate}" styleClass="entityDataEmphasizedLabel"/> - +