From dd852ccfdf78480dbc76be92a04af423a07e0c96 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Mon, 31 Aug 2026 14:28:54 -0500 Subject: [PATCH 1/5] Expose full by addign a view all logbooks button. --- .../ItemDomainLogbookController.java | 57 +++++++++++++++++++ .../model/ItemDomainLogbookLazyDataModel.java | 20 ++++++- .../ItemDomainLogbookQueryBuilder.java | 29 +++++++--- .../db/beans/builder/ItemQueryBuilder.java | 2 +- .../templates/portalViewMenubarTemplate.xhtml | 6 ++ .../views/itemDomainLogbook/fullList.xhtml | 27 +++++++++ .../itemDomainLogbookFullListDataTable.xhtml | 27 +++++++++ 7 files changed, 159 insertions(+), 9 deletions(-) create mode 100644 src/java/LogrPortal/web/views/itemDomainLogbook/fullList.xhtml create mode 100644 src/java/LogrPortal/web/views/itemDomainLogbook/private/itemDomainLogbookFullListDataTable.xhtml diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java index b390de6b7..eb4ce3d8f 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java @@ -101,6 +101,7 @@ public class ItemDomainLogbookController extends ItemController logResults; @@ -144,6 +145,11 @@ public class ItemDomainLogbookController extends ItemController public final String getCurrentListPermalink() { + if (fullListMode) { + String redirect = String.format("%s/list?et=%d", getDomainPath(), FULL_LIST_ET_ID); + return String.format("%s%s", contextRootPermanentUrl, redirect); + } + if (currentEntityType != null) { String redirect = getListRedirectForEntityType(currentEntityType, true, true); String viewPath = String.format("%s%s", contextRootPermanentUrl, redirect); diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/ItemDomainLogbookLazyDataModel.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/ItemDomainLogbookLazyDataModel.java index 0e2a7335e..215cc0510 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/ItemDomainLogbookLazyDataModel.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/ItemDomainLogbookLazyDataModel.java @@ -21,6 +21,8 @@ */ public class ItemDomainLogbookLazyDataModel extends ItemLazyDataModel { + private boolean allLogbookTypes = false; + public ItemDomainLogbookLazyDataModel(ItemDomainLogbookFacade facade, Domain itemDomain, ItemSettings settings) { super(facade, itemDomain, settings); @@ -39,10 +41,26 @@ private void addDefaultSortOrder() { @Override protected ItemDomainLogbookQueryBuilder getQueryBuilder(Map filterMap, String sortField, SortOrder sortOrder) { - return new ItemDomainLogbookQueryBuilder(itemDomain.getId(), filterMap, sortField, sortOrder, settings); + return new ItemDomainLogbookQueryBuilder(itemDomain.getId(), filterMap, sortField, sortOrder, settings, allLogbookTypes); + } + + /** + * Show log documents of every logbook type instead of a single type. + */ + public void setAllLogbookTypes() { + allLogbookTypes = true; + setCurrentEntityType(null); + } + + public boolean isAllLogbookTypes() { + return allLogbookTypes; } public void setCurrentEntityType(String entityType) { + if (entityType != null) { + allLogbookTypes = false; + } + if (entityType == null) { setDefaultFilterBy(null); } else { diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/builder/ItemDomainLogbookQueryBuilder.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/builder/ItemDomainLogbookQueryBuilder.java index 3ab333000..b7037b6a4 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/builder/ItemDomainLogbookQueryBuilder.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/builder/ItemDomainLogbookQueryBuilder.java @@ -4,6 +4,7 @@ */ package gov.anl.aps.logr.portal.model.db.beans.builder; +import gov.anl.aps.logr.portal.constants.EntityTypeName; import gov.anl.aps.logr.portal.controllers.settings.ItemSettings; import java.util.Map; import org.primefaces.model.SortOrder; @@ -13,21 +14,35 @@ * @author djarosz */ public class ItemDomainLogbookQueryBuilder extends ItemQueryBuilder { - + + private final boolean allLogbookTypes; + public ItemDomainLogbookQueryBuilder(Integer domainId, Map filterMap, String sortField, SortOrder sortOrder, ItemSettings scopeSettings) { + this(domainId, filterMap, sortField, sortOrder, scopeSettings, false); + } + + public ItemDomainLogbookQueryBuilder(Integer domainId, Map filterMap, String sortField, SortOrder sortOrder, ItemSettings scopeSettings, boolean allLogbookTypes) { super(domainId, filterMap, sortField, sortOrder, scopeSettings); - } + this.allLogbookTypes = allLogbookTypes; + } @Override protected void generateWhereString() { - super.generateWhereString(); - + super.generateWhereString(); + + if (allLogbookTypes) { + // Show top level log documents of every logbook type. Templates are + // excluded since their only entity type is the template entity type. + appendRawWhere("i.itemElementMemberList IS EMPTY"); + appendRawWhere(ENTITY_TYPE_LIST_JOIN_NAME + ".name <> '" + EntityTypeName.template.getValue() + "'"); + include_etl = true; + return; + } + if (filterMap == null || filterMap.isEmpty()) { appendRawWhere("i.itemElementMemberList IS EMPTY"); appendRawWhere("i.entityTypeList IS EMPTY"); } } - - - + } diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/builder/ItemQueryBuilder.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/builder/ItemQueryBuilder.java index b6c539c7b..9e77a4c78 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/builder/ItemQueryBuilder.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/builder/ItemQueryBuilder.java @@ -42,7 +42,7 @@ public abstract class ItemQueryBuilder extends CdbQueryBuilder { private static final String QUERY_STRING_START = "SELECT DISTINCT(i) FROM Item i "; private static final String ITEM_ELEMENTS_LIST_JOIN_NAME = "fiel"; private static final String ITEM_PROJECT_LIST_JOIN_NAME = "ipl"; - private static final String ENTITY_TYPE_LIST_JOIN_NAME = "etl"; + protected static final String ENTITY_TYPE_LIST_JOIN_NAME = "etl"; private static final String ITEM_CATEGORY_LIST_JOIN_NAME = "icl"; private static final String ITEM_TYPE_LIST_JOIN_NAME = "itl"; private static final String ITEM_SOURCE_LIST_JOIN_NAME = "isl"; diff --git a/src/java/LogrPortal/web/templates/portalViewMenubarTemplate.xhtml b/src/java/LogrPortal/web/templates/portalViewMenubarTemplate.xhtml index 6f1d4c3b6..5b330bfca 100644 --- a/src/java/LogrPortal/web/templates/portalViewMenubarTemplate.xhtml +++ b/src/java/LogrPortal/web/templates/portalViewMenubarTemplate.xhtml @@ -54,6 +54,12 @@ See LICENSE file. + + diff --git a/src/java/LogrPortal/web/views/itemDomainLogbook/fullList.xhtml b/src/java/LogrPortal/web/views/itemDomainLogbook/fullList.xhtml new file mode 100644 index 000000000..36d5c4bf4 --- /dev/null +++ b/src/java/LogrPortal/web/views/itemDomainLogbook/fullList.xhtml @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/src/java/LogrPortal/web/views/itemDomainLogbook/private/itemDomainLogbookFullListDataTable.xhtml b/src/java/LogrPortal/web/views/itemDomainLogbook/private/itemDomainLogbookFullListDataTable.xhtml new file mode 100644 index 000000000..abe660d0e --- /dev/null +++ b/src/java/LogrPortal/web/views/itemDomainLogbook/private/itemDomainLogbookFullListDataTable.xhtml @@ -0,0 +1,27 @@ + + + + + + + + + + #{itemListObject.longEntityTypeString} + + + + From 2aa71167451d30e5efbe0bf9d5efa23896a12cbf Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Mon, 31 Aug 2026 14:50:58 -0500 Subject: [PATCH 2/5] Resolve filtering of logbook --- .../itemDomainLogbookFullListDataTable.xhtml | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/java/LogrPortal/web/views/itemDomainLogbook/private/itemDomainLogbookFullListDataTable.xhtml b/src/java/LogrPortal/web/views/itemDomainLogbook/private/itemDomainLogbookFullListDataTable.xhtml index abe660d0e..a2d8cdfa9 100644 --- a/src/java/LogrPortal/web/views/itemDomainLogbook/private/itemDomainLogbookFullListDataTable.xhtml +++ b/src/java/LogrPortal/web/views/itemDomainLogbook/private/itemDomainLogbookFullListDataTable.xhtml @@ -9,17 +9,15 @@ See LICENSE file. xmlns:p="http://primefaces.org/ui" template="../../item/private/templates/itemListDataTableTemplate.xhtml"> - + + + sortBy="#{itemListObject.entityTypeString}" + filterBy="#{itemListObject.entityTypeString}" + filterMatchMode="contains" + filterOptions="#{entityController.entityTypesForDataTableFilterSelectOne}"> #{itemListObject.longEntityTypeString} From 9b60bd11a8a417383ff9d98d12a376722e2ee9c2 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Mon, 31 Aug 2026 14:51:07 -0500 Subject: [PATCH 3/5] Concise comments --- .../portal/controllers/ItemDomainLogbookController.java | 9 ++------- .../portal/model/ItemDomainLogbookLazyDataModel.java | 4 +--- .../db/beans/builder/ItemDomainLogbookQueryBuilder.java | 3 +-- .../web/views/itemDomainLogbook/fullList.xhtml | 2 +- 4 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java index eb4ce3d8f..2a3b52763 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java @@ -145,7 +145,7 @@ public class ItemDomainLogbookController extends ItemController '" + EntityTypeName.template.getValue() + "'"); include_etl = true; diff --git a/src/java/LogrPortal/web/views/itemDomainLogbook/fullList.xhtml b/src/java/LogrPortal/web/views/itemDomainLogbook/fullList.xhtml index 36d5c4bf4..933c80e9a 100644 --- a/src/java/LogrPortal/web/views/itemDomainLogbook/fullList.xhtml +++ b/src/java/LogrPortal/web/views/itemDomainLogbook/fullList.xhtml @@ -16,7 +16,7 @@ See LICENSE file. - + From 4b90ee7cb5c066470415c5ac7ae2552c0734b494 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Mon, 31 Aug 2026 15:22:30 -0500 Subject: [PATCH 4/5] Resolve the previous and next document for all documents selection. --- .../ItemDomainLogbookController.java | 21 +++++++++++--- .../db/beans/ItemDomainLogbookFacade.java | 29 +++++++++++++++++++ .../logr/portal/model/db/entities/Item.java | 4 +++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java index 2a3b52763..541e45bd4 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/controllers/ItemDomainLogbookController.java @@ -1036,6 +1036,11 @@ public String getItemListPageTitle() { } public void navigateToLogDocumentList() { + if (fullListMode) { + redirectToFullList(); + return; + } + EntityType entityType = getCurrent().getEntityTypeList().get(0); redirectToEntityTypeList(entityType); @@ -1047,8 +1052,12 @@ public ItemDomainLogbook getNextLogDocument() { boolean nextDocLoaded = currentDoc.getNextDocLoaded(); if (!nextDocLoaded) { Integer logId = currentDoc.getId(); - String entityTypeName = currentDoc.getEntityTypeList().get(0).getName(); - nextDoc = itemDomainLogbookFacade.getNextLogDocument(entityTypeName, logId); + if (fullListMode) { + nextDoc = itemDomainLogbookFacade.getNextLogDocument(logId); + } else { + String entityTypeName = currentDoc.getEntityTypeList().get(0).getName(); + nextDoc = itemDomainLogbookFacade.getNextLogDocument(entityTypeName, logId); + } currentDoc.setNextDoc(nextDoc); currentDoc.setNextDocLoaded(true); } else { @@ -1063,8 +1072,12 @@ public ItemDomainLogbook getPrevLogDocument() { boolean prevDocLoaded = currentDoc.getPrevDocLoaded(); if (!prevDocLoaded) { Integer logId = currentDoc.getId(); - String entityTypeName = currentDoc.getEntityTypeList().get(0).getName(); - prevDoc = itemDomainLogbookFacade.getPreviousLogDocument(entityTypeName, logId); + if (fullListMode) { + prevDoc = itemDomainLogbookFacade.getPreviousLogDocument(logId); + } else { + String entityTypeName = currentDoc.getEntityTypeList().get(0).getName(); + prevDoc = itemDomainLogbookFacade.getPreviousLogDocument(entityTypeName, logId); + } currentDoc.setPrevDoc(prevDoc); currentDoc.setPrevDocLoaded(true); } else { diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/ItemDomainLogbookFacade.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/ItemDomainLogbookFacade.java index d15b49cd4..4115fd51e 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/ItemDomainLogbookFacade.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/beans/ItemDomainLogbookFacade.java @@ -4,6 +4,7 @@ */ package gov.anl.aps.logr.portal.model.db.beans; +import gov.anl.aps.logr.portal.constants.EntityTypeName; import gov.anl.aps.logr.portal.constants.ItemDomainName; import gov.anl.aps.logr.portal.model.db.entities.ItemDomainLogbook; import gov.anl.aps.logr.portal.utilities.SessionUtility; @@ -57,6 +58,34 @@ public ItemDomainLogbook getNextLogDocument(String entityTypeName, Integer curre } + // Previous top level log document across every logbook type, excluding templates. + public ItemDomainLogbook getPreviousLogDocument(Integer currentId) { + try { + return (ItemDomainLogbook) em.createNamedQuery("Item.findByDomainNameAndTopLevelBeforeId") + .setParameter("domainName", getDomain().getValue()) + .setParameter("excludeEntityTypeName", EntityTypeName.template.getValue()) + .setParameter("itemId", currentId) + .setMaxResults(1) + .getSingleResult(); + } catch (NoResultException ex) { + } + return null; + } + + // Next top level log document across every logbook type, excluding templates. + public ItemDomainLogbook getNextLogDocument(Integer currentId) { + try { + return (ItemDomainLogbook) em.createNamedQuery("Item.findByDomainNameAndTopLevelAfterId") + .setParameter("domainName", getDomain().getValue()) + .setParameter("excludeEntityTypeName", EntityTypeName.template.getValue()) + .setParameter("itemId", currentId) + .setMaxResults(1) + .getSingleResult(); + } catch (NoResultException ex) { + } + return null; + } + public static ItemDomainLogbookFacade getInstance() { return (ItemDomainLogbookFacade) SessionUtility.findFacade(ItemDomainLogbookFacade.class.getSimpleName()); } diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/entities/Item.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/entities/Item.java index 36a580fbd..c44e3256c 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/entities/Item.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/portal/model/db/entities/Item.java @@ -155,6 +155,10 @@ query = "SELECT DISTINCT(i) FROM Item i JOIN i.entityTypeList etl WHERE i.domain.name = :domainName and etl.name = :entityTypeName and i.itemElementMemberList IS EMPTY AND i.itemElementMemberList2 IS EMPTY AND i.id > :itemId ORDER BY i.id ASC"), @NamedQuery(name = "Item.findByDomainNameAndEntityTypeAndTopLevelBeforeId", query = "SELECT DISTINCT(i) FROM Item i JOIN i.entityTypeList etl WHERE i.domain.name = :domainName and etl.name = :entityTypeName and i.itemElementMemberList IS EMPTY AND i.itemElementMemberList2 IS EMPTY AND i.id < :itemId ORDER BY i.id DESC"), + @NamedQuery(name = "Item.findByDomainNameAndTopLevelAfterId", + query = "SELECT DISTINCT(i) FROM Item i JOIN i.entityTypeList etl WHERE i.domain.name = :domainName and etl.name <> :excludeEntityTypeName and i.itemElementMemberList IS EMPTY AND i.itemElementMemberList2 IS EMPTY AND i.id > :itemId ORDER BY i.id ASC"), + @NamedQuery(name = "Item.findByDomainNameAndTopLevelBeforeId", + query = "SELECT DISTINCT(i) FROM Item i JOIN i.entityTypeList etl WHERE i.domain.name = :domainName and etl.name <> :excludeEntityTypeName and i.itemElementMemberList IS EMPTY AND i.itemElementMemberList2 IS EMPTY AND i.id < :itemId ORDER BY i.id DESC"), @NamedQuery(name = "Item.findByDomainNameAndEntityTypeAndTopLevelExcludeEntityType", query = "SELECT DISTINCT(i) FROM Item i JOIN i.entityTypeList etl WHERE i.domain.name = :domainName and etl.name = :entityTypeName and (i.id not in (SELECT DISTINCT(i.id) FROM Item i JOIN i.entityTypeList etl WHERE i.domain.name = :domainName and etl.name = :excludeEntityTypeName)) and i.itemElementMemberList IS EMPTY AND i.itemElementMemberList2 IS EMPTY"), @NamedQuery(name = "Item.findByDomainNameAndEntityTypeAndTopLevelOrderByDerivedFromItem", From d2635082a8a436c7a1fb116c88f09eec62c536a0 Mon Sep 17 00:00:00 2001 From: Dariusz Jarosz Date: Tue, 1 Sep 2026 08:19:05 -0500 Subject: [PATCH 5/5] Standardize fetching logbooks and logbook systems between API and MCP --- .../anl/aps/logr/rest/mcp/McpConstants.java | 1 + .../logr/rest/mcp/tools/McpToolContext.java | 20 +- .../aps/logr/rest/routes/LogbookRoute.java | 24 +- .../anl/aps/logr/rest/routes/SearchRoute.java | 26 +- .../rest/utilities/LogbookDomainUtility.java | 69 +++++ .../utilities/LogbookDomainUtilityTest.java | 273 ++++++++++++++++++ 6 files changed, 358 insertions(+), 55 deletions(-) create mode 100644 src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/utilities/LogbookDomainUtility.java create mode 100644 src/java/LogrPortal/test/gov/anl/aps/logr/rest/utilities/LogbookDomainUtilityTest.java diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/mcp/McpConstants.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/mcp/McpConstants.java index a4b21d803..a5284954c 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/mcp/McpConstants.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/mcp/McpConstants.java @@ -61,6 +61,7 @@ private McpConstants() { + "once you know its id. All tools are read-only."; public static final String PROP_ENABLED = "cdb.portal.mcp.enabled"; + // Defaults to false by design: all MCP tools are read-only and the equivalent REST reads are unauthenticated too. Set true to require a valid token header. public static final String PROP_REQUIRE_AUTH = "cdb.portal.mcp.requireAuth"; public static final String PROP_ALLOWED_ORIGINS = "cdb.portal.mcp.allowedOrigins"; } diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/mcp/tools/McpToolContext.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/mcp/tools/McpToolContext.java index f7c66cb62..1828e97cf 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/mcp/tools/McpToolContext.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/mcp/tools/McpToolContext.java @@ -4,18 +4,15 @@ */ package gov.anl.aps.logr.rest.mcp.tools; -import gov.anl.aps.logr.portal.constants.EntityTypeName; -import gov.anl.aps.logr.portal.constants.ItemDomainName; import gov.anl.aps.logr.portal.model.db.beans.DomainFacade; import gov.anl.aps.logr.portal.model.db.beans.ItemDomainLogbookFacade; import gov.anl.aps.logr.portal.model.db.beans.ItemFacade; import gov.anl.aps.logr.portal.model.db.beans.UserGroupFacade; import gov.anl.aps.logr.portal.model.db.beans.UserInfoFacade; -import gov.anl.aps.logr.portal.model.db.entities.Domain; import gov.anl.aps.logr.portal.model.db.entities.EntityType; import gov.anl.aps.logr.portal.model.db.entities.ItemType; import gov.anl.aps.logr.portal.model.db.entities.UserInfo; -import java.util.ArrayList; +import gov.anl.aps.logr.rest.utilities.LogbookDomainUtility; import java.util.List; /** Facade bundle plus the resolved current user, built once per MCP request and passed to every {@link McpTool#call}. */ @@ -63,20 +60,13 @@ public UserInfo getCurrentUser() { return currentUser; } - private Domain getLogbookDomain() { - return domainFacade.find(ItemDomainName.LOGBOOK_ID); - } - - // Copies before filtering — SearchRoute/LogbookRoute's equivalent helper mutates the shared, JPA-managed list in place. + // Delegates to the shared helper so REST and MCP filter logbook types identically. public List getLogbookTypes() { - Domain domain = getLogbookDomain(); - List logbookTypes = new ArrayList<>(domain.getAllowedEntityTypeList()); - logbookTypes.removeIf(t -> t.getName().equals(EntityTypeName.template.getValue())); - return logbookTypes; + return LogbookDomainUtility.getLogbookTypes(domainFacade); } + // Delegates to the shared helper so REST and MCP resolve systems identically. public List getLogbookSystems() { - Domain domain = getLogbookDomain(); - return domain.getItemTypeList(); + return LogbookDomainUtility.getLogbookSystems(domainFacade); } } diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/LogbookRoute.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/LogbookRoute.java index b34ee0949..0964f5845 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/LogbookRoute.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/LogbookRoute.java @@ -16,7 +16,6 @@ import gov.anl.aps.logr.portal.model.db.beans.ItemDomainLogbookFacade; import gov.anl.aps.logr.portal.model.db.beans.LogFacade; import gov.anl.aps.logr.portal.model.db.entities.Attachment; -import gov.anl.aps.logr.portal.model.db.entities.Domain; import gov.anl.aps.logr.portal.model.db.entities.EntityInfo; import gov.anl.aps.logr.portal.model.db.entities.EntityType; import gov.anl.aps.logr.portal.model.db.entities.Item; @@ -32,6 +31,7 @@ import gov.anl.aps.logr.rest.entities.LogDocumentSection; import gov.anl.aps.logr.rest.entities.LogEntry; import gov.anl.aps.logr.rest.entities.LogEntryAttachment; +import gov.anl.aps.logr.rest.utilities.LogbookDomainUtility; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Parameter; import io.swagger.v3.oas.annotations.responses.ApiResponse; @@ -77,38 +77,24 @@ public class LogbookRoute extends ItemBaseRoute { @EJB LogFacade logFacade; - private Domain getLogbookDomain() { - return domainFacade.find(ItemDomainName.LOGBOOK_ID); - } - + // Delegates to the shared helper, which copies before removing the template type instead of filtering the domain's managed list in place. @GET @Path("/LogbookTypes") @Operation(responses = { @ApiResponse(responseCode = "200", description = "OK", useReturnTypeSchema = true)}) @Produces(MediaType.APPLICATION_JSON) public List getLogbookTypes() { - Domain domain = getLogbookDomain(); - List logbookTypes = domain.getAllowedEntityTypeList(); - // Remove template - for (EntityType logbookType : logbookTypes) { - if (logbookType.getName().equals(EntityTypeName.template.getValue())) { - logbookTypes.remove(logbookType); - break; - } - } - - return logbookTypes; + return LogbookDomainUtility.getLogbookTypes(domainFacade); } + // Delegates to the shared helper; behavior is unchanged, the item type list is returned unfiltered. @GET @Path("/LogbookSystems") @Operation(responses = { @ApiResponse(responseCode = "200", description = "OK", useReturnTypeSchema = true)}) @Produces(MediaType.APPLICATION_JSON) public List getLogbookSystems() { - Domain domain = getLogbookDomain(); - - return domain.getItemTypeList(); + return LogbookDomainUtility.getLogbookSystems(domainFacade); } @GET diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/SearchRoute.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/SearchRoute.java index 5437640c4..681a45520 100644 --- a/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/SearchRoute.java +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/routes/SearchRoute.java @@ -8,8 +8,6 @@ import gov.anl.aps.logr.common.exceptions.InvalidRequest; import gov.anl.aps.logr.common.mqtt.constants.CallSource; import gov.anl.aps.logr.common.mqtt.model.entities.LogbookSearchOptions; -import gov.anl.aps.logr.portal.constants.EntityTypeName; -import gov.anl.aps.logr.portal.constants.ItemDomainName; import gov.anl.aps.logr.portal.controllers.utilities.ItemCategoryControllerUtility; import gov.anl.aps.logr.portal.controllers.utilities.ItemDomainLogbookControllerUtility; import gov.anl.aps.logr.portal.controllers.utilities.ItemElementControllerUtility; @@ -22,7 +20,6 @@ import gov.anl.aps.logr.portal.controllers.utilities.UserInfoControllerUtility; import gov.anl.aps.logr.portal.model.db.beans.DomainFacade; import gov.anl.aps.logr.portal.model.db.beans.UserInfoFacade; -import gov.anl.aps.logr.portal.model.db.entities.Domain; import gov.anl.aps.logr.portal.model.db.entities.EntityType; import gov.anl.aps.logr.portal.model.db.entities.ItemType; import gov.anl.aps.logr.portal.model.db.entities.UserInfo; @@ -30,6 +27,7 @@ import gov.anl.aps.logr.rest.entities.LogbookSearchResults; import gov.anl.aps.logr.rest.entities.SearchEntitiesOptions; import gov.anl.aps.logr.rest.entities.SearchEntitiesResults; +import gov.anl.aps.logr.rest.utilities.LogbookDomainUtility; import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Parameter; import io.swagger.v3.oas.annotations.responses.ApiResponse; @@ -195,28 +193,14 @@ public SearchEntitiesResults genericSearch(@RequestBody(required = true) SearchE return results; } - private Domain getLogbookDomain() { - return domainFacade.find(ItemDomainName.LOGBOOK_ID); - } - + // Delegates to the shared helper, which copies before removing the template type instead of filtering the domain's managed list in place. private List getLogbookTypes() { - Domain domain = getLogbookDomain(); - List logbookTypes = domain.getAllowedEntityTypeList(); - // Remove template - for (EntityType logbookType : logbookTypes) { - if (logbookType.getName().equals(EntityTypeName.template.getValue())) { - logbookTypes.remove(logbookType); - break; - } - } - - return logbookTypes; + return LogbookDomainUtility.getLogbookTypes(domainFacade); } + // Delegates to the shared helper; behavior is unchanged, the item type list is returned unfiltered. private List getLogbookSystems() { - Domain domain = getLogbookDomain(); - - return domain.getItemTypeList(); + return LogbookDomainUtility.getLogbookSystems(domainFacade); } private List resolveEntityTypeList(List idList) throws InvalidArgument { diff --git a/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/utilities/LogbookDomainUtility.java b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/utilities/LogbookDomainUtility.java new file mode 100644 index 000000000..b502c9f88 --- /dev/null +++ b/src/java/LogrPortal/src/java/gov/anl/aps/logr/rest/utilities/LogbookDomainUtility.java @@ -0,0 +1,69 @@ +/* + * Copyright (c) UChicago Argonne, LLC. All rights reserved. + * See LICENSE file. + */ +package gov.anl.aps.logr.rest.utilities; + +import gov.anl.aps.logr.portal.constants.EntityTypeName; +import gov.anl.aps.logr.portal.constants.ItemDomainName; +import gov.anl.aps.logr.portal.model.db.beans.DomainFacade; +import gov.anl.aps.logr.portal.model.db.entities.Domain; +import gov.anl.aps.logr.portal.model.db.entities.EntityType; +import gov.anl.aps.logr.portal.model.db.entities.ItemType; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +// Shared logbook domain lookups for the REST and MCP layers; copies before filtering so the domain's managed collection is never modified. +public final class LogbookDomainUtility { + + private LogbookDomainUtility() { + } + + // There is only ever one logbook domain, so every caller resolves it the same way. + public static Domain getLogbookDomain(DomainFacade domainFacade) { + return domainFacade == null ? null : domainFacade.find(ItemDomainName.LOGBOOK_ID); + } + + // Convenience overload so callers holding only the facade need no domain lookup of their own. + public static List getLogbookTypes(DomainFacade domainFacade) { + return getLogbookTypes(getLogbookDomain(domainFacade)); + } + + // Convenience overload so callers holding only the facade need no domain lookup of their own. + public static List getLogbookSystems(DomainFacade domainFacade) { + return getLogbookSystems(getLogbookDomain(domainFacade)); + } + + // Allowed logbook types minus the template type, as a new list; never returns the managed collection. + public static List getLogbookTypes(Domain domain) { + // Callers may hold no domain; return an empty, modifiable list rather than throwing. + if (domain == null) { + return new ArrayList<>(); + } + + List allowedEntityTypeList = domain.getAllowedEntityTypeList(); + // A domain with no configured allowed types is valid, not an error. + if (allowedEntityTypeList == null) { + return new ArrayList<>(); + } + + // Copy before filtering; never mutate the domain's managed collection. + List logbookTypes = new ArrayList<>(allowedEntityTypeList); + String templateName = EntityTypeName.template.getValue(); + // Null-tolerant exact-name match, preserving the original filtering semantics. + logbookTypes.removeIf(t -> t != null && templateName.equals(t.getName())); + + return logbookTypes; + } + + // Systems configured for the logbook domain; unfiltered, empty when unavailable. + public static List getLogbookSystems(Domain domain) { + // Nothing filters this list, so the managed collection can be returned as-is. + if (domain == null || domain.getItemTypeList() == null) { + return Collections.emptyList(); + } + + return domain.getItemTypeList(); + } +} diff --git a/src/java/LogrPortal/test/gov/anl/aps/logr/rest/utilities/LogbookDomainUtilityTest.java b/src/java/LogrPortal/test/gov/anl/aps/logr/rest/utilities/LogbookDomainUtilityTest.java new file mode 100644 index 000000000..bb6b5b375 --- /dev/null +++ b/src/java/LogrPortal/test/gov/anl/aps/logr/rest/utilities/LogbookDomainUtilityTest.java @@ -0,0 +1,273 @@ +/* + * Copyright (c) UChicago Argonne, LLC. All rights reserved. + * See LICENSE file. + */ +package gov.anl.aps.logr.rest.utilities; + +import gov.anl.aps.logr.portal.constants.EntityTypeName; +import gov.anl.aps.logr.portal.constants.ItemDomainName; +import gov.anl.aps.logr.portal.model.db.beans.DomainFacade; +import gov.anl.aps.logr.portal.model.db.entities.Domain; +import gov.anl.aps.logr.portal.model.db.entities.EntityType; +import gov.anl.aps.logr.portal.model.db.entities.ItemType; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +// Plain-Java checks for LogbookDomainUtility; testSourceListIsNotMutated is the regression guard against filtering the domain's managed list in place. +public class LogbookDomainUtilityTest { + + // Reflection-based runner mirroring the existing MCP tests; no JUnit on the classpath. + public static void main(String[] args) throws Exception { + int passed = 0; + int failed = 0; + for (java.lang.reflect.Method m : LogbookDomainUtilityTest.class.getDeclaredMethods()) { + if (m.getName().startsWith("test") && m.getParameterCount() == 0) { + try { + m.setAccessible(true); + m.invoke(null); + System.out.println("PASS " + m.getName()); + passed++; + } catch (java.lang.reflect.InvocationTargetException e) { + System.out.println("FAIL " + m.getName() + ": " + e.getCause()); + failed++; + } + } + } + System.out.println(passed + " passed, " + failed + " failed"); + if (failed > 0) { + System.exit(1); + } + } + + static void check(boolean condition, String message) { + if (!condition) { + throw new AssertionError(message); + } + } + + private static final String TEMPLATE = EntityTypeName.template.getValue(); + + // Builds a detached Domain; these entities construct fine without a container. + private static Domain domainWithAllowedTypes(EntityType... types) { + Domain domain = new Domain(); + domain.setAllowedEntityTypeList(new ArrayList<>(Arrays.asList(types))); + return domain; + } + + private static boolean containsName(List types, String name) { + for (EntityType t : types) { + if (name.equals(t.getName())) { + return true; + } + } + return false; + } + + // Core behavior: the template type is excluded, everything else survives. + static void testTemplateIsFilteredOut() { + Domain domain = domainWithAllowedTypes( + new EntityType(1, "Ops-Shift"), + new EntityType(EntityTypeName.TEMPLATE_ID, TEMPLATE), + new EntityType(2, "Maintenance")); + + List result = LogbookDomainUtility.getLogbookTypes(domain); + + check(!containsName(result, TEMPLATE), "template must be filtered out of the logbook types"); + check(result.size() == 2, "the two non-template types must survive, got " + result.size()); + check(containsName(result, "Ops-Shift") && containsName(result, "Maintenance"), + "non-template types must be preserved"); + } + + // The actual regression guard for the in-place mutation bug. + static void testSourceListIsNotMutated() { + Domain domain = domainWithAllowedTypes( + new EntityType(1, "Ops-Shift"), + new EntityType(EntityTypeName.TEMPLATE_ID, TEMPLATE)); + List managedList = domain.getAllowedEntityTypeList(); + + LogbookDomainUtility.getLogbookTypes(domain); + + check(managedList.size() == 2, + "the domain's managed allowed-entity-type list must not be modified, size is " + managedList.size()); + check(containsName(managedList, TEMPLATE), + "template must still be present on the domain after filtering a copy"); + check(domain.getAllowedEntityTypeList() == managedList, + "the domain must still reference the same underlying list instance"); + } + + // The returned copy must be independent, so callers cannot corrupt the domain. + static void testReturnedListIsNotTheManagedList() { + Domain domain = domainWithAllowedTypes(new EntityType(1, "Ops-Shift")); + + List result = LogbookDomainUtility.getLogbookTypes(domain); + + check(result != domain.getAllowedEntityTypeList(), + "callers must receive a copy, never the managed collection itself"); + result.clear(); + check(domain.getAllowedEntityTypeList().size() == 1, + "mutating the returned list must not affect the domain"); + } + + // Under the old in-place logic, repeated calls degraded the shared list. + static void testRepeatedCallsAreStable() { + Domain domain = domainWithAllowedTypes( + new EntityType(1, "Ops-Shift"), + new EntityType(EntityTypeName.TEMPLATE_ID, TEMPLATE)); + + List first = LogbookDomainUtility.getLogbookTypes(domain); + List second = LogbookDomainUtility.getLogbookTypes(domain); + List third = LogbookDomainUtility.getLogbookTypes(domain); + + check(first.size() == second.size() && second.size() == third.size(), + "repeated calls on the same domain must return identically sized results"); + check(first.size() == 1, "each call must drop exactly the template entry"); + } + + // No template entry present is a normal case, not an error. + static void testNoTemplatePresentIsANoOp() { + Domain domain = domainWithAllowedTypes(new EntityType(1, "Ops-Shift"), new EntityType(2, "Maintenance")); + + List result = LogbookDomainUtility.getLogbookTypes(domain); + + check(result.size() == 2, "a domain without a template entry must be returned intact"); + } + + static void testEmptyAllowedListReturnsEmpty() { + Domain domain = domainWithAllowedTypes(); + + List result = LogbookDomainUtility.getLogbookTypes(domain); + + check(result != null && result.isEmpty(), "an empty allowed list must yield an empty result, not null"); + } + + static void testNullAllowedListReturnsEmpty() { + Domain domain = new Domain(); + domain.setAllowedEntityTypeList(null); + + List result = LogbookDomainUtility.getLogbookTypes(domain); + + check(result != null && result.isEmpty(), "a null allowed list must yield an empty result, not throw"); + } + + static void testNullDomainReturnsEmptyTypes() { + List result = LogbookDomainUtility.getLogbookTypes((Domain) null); + + check(result != null && result.isEmpty(), "a null domain must yield an empty result, not throw"); + } + + // Defensive: a null element must not blow up the filter. + static void testNullEntryInAllowedListIsTolerated() { + Domain domain = new Domain(); + domain.setAllowedEntityTypeList(new ArrayList<>(Arrays.asList( + new EntityType(1, "Ops-Shift"), null, new EntityType(EntityTypeName.TEMPLATE_ID, TEMPLATE)))); + + List result = LogbookDomainUtility.getLogbookTypes(domain); + + check(!containsNull(result) || result.size() == 2, "a null entry must not cause a NullPointerException"); + check(!containsName(stripNulls(result), TEMPLATE), "template must still be filtered alongside a null entry"); + } + + private static boolean containsNull(List types) { + for (EntityType t : types) { + if (t == null) { + return true; + } + } + return false; + } + + private static List stripNulls(List types) { + List result = new ArrayList<>(); + for (EntityType t : types) { + if (t != null) { + result.add(t); + } + } + return result; + } + + // Systems are passed through unfiltered, in their configured order. + static void testGetLogbookSystemsReturnsItemTypeList() { + Domain domain = new Domain(); + List itemTypes = new ArrayList<>(Arrays.asList( + new ItemType(1, "Storage-Ring"), new ItemType(2, "Linac"))); + domain.setItemTypeList(itemTypes); + + List result = LogbookDomainUtility.getLogbookSystems(domain); + + check(result.size() == 2, "all configured systems must be returned"); + check(result.get(0).getName().equals("Storage-Ring"), "system ordering must be preserved"); + } + + static void testGetLogbookSystemsNullSafe() { + check(LogbookDomainUtility.getLogbookSystems((Domain) null).isEmpty(), + "a null domain must yield an empty system list, not throw"); + check(LogbookDomainUtility.getLogbookSystems(new Domain()).isEmpty(), + "a domain with a null item type list must yield an empty list, not throw"); + } + + // Records which id the facade was asked for, so the lookup constant can be asserted. + private static class RecordingDomainFacade extends DomainFacade { + + private final Domain domain; + private Integer requestedId; + + RecordingDomainFacade(Domain domain) { + this.domain = domain; + } + + @Override + public Domain find(Object id) { + requestedId = (Integer) id; + return domain; + } + } + + // The facade overload must look up the one logbook domain by its well-known id. + static void testFacadeOverloadResolvesLogbookDomain() { + Domain domain = domainWithAllowedTypes( + new EntityType(1, "Ops-Shift"), + new EntityType(EntityTypeName.TEMPLATE_ID, TEMPLATE)); + RecordingDomainFacade facade = new RecordingDomainFacade(domain); + + List result = LogbookDomainUtility.getLogbookTypes(facade); + + check(facade.requestedId != null && facade.requestedId == ItemDomainName.LOGBOOK_ID, + "the facade must be queried with the logbook domain id, got " + facade.requestedId); + check(result.size() == 1, "the facade overload must apply the same template filtering"); + } + + // The facade overload must not mutate the domain it resolves either. + static void testFacadeOverloadDoesNotMutateDomain() { + Domain domain = domainWithAllowedTypes( + new EntityType(1, "Ops-Shift"), + new EntityType(EntityTypeName.TEMPLATE_ID, TEMPLATE)); + List managedList = domain.getAllowedEntityTypeList(); + + LogbookDomainUtility.getLogbookTypes(new RecordingDomainFacade(domain)); + + check(managedList.size() == 2, + "the facade overload must leave the managed list intact, size is " + managedList.size()); + } + + // Systems resolve through the facade the same way types do. + static void testFacadeOverloadReturnsSystems() { + Domain domain = new Domain(); + domain.setItemTypeList(new ArrayList<>(Arrays.asList(new ItemType(1, "Storage-Ring")))); + + List result = LogbookDomainUtility.getLogbookSystems(new RecordingDomainFacade(domain)); + + check(result.size() == 1, "the facade overload must return the domain's systems"); + } + + // A null facade is treated like a null domain rather than throwing. + static void testNullFacadeIsSafe() { + check(LogbookDomainUtility.getLogbookDomain(null) == null, + "a null facade must resolve to a null domain, not throw"); + check(LogbookDomainUtility.getLogbookTypes((DomainFacade) null).isEmpty(), + "a null facade must yield empty logbook types, not throw"); + check(LogbookDomainUtility.getLogbookSystems((DomainFacade) null).isEmpty(), + "a null facade must yield empty systems, not throw"); + } +}