From 910d8e69d5cfbbabd8b9fb2c6fb7b435c4224496 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Tue, 7 Apr 2026 10:28:25 -0700 Subject: [PATCH 01/12] fix getBlanksSql() (#7549) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### Rationale fix getBlankSql() [issue #1033 ](https://github.com/LabKey/internal-issues/issues/1033) #### Related Pull Requests - #### Changes - #### Tasks 📍 - [x] Needs Automation --- api/src/org/labkey/api/exp/property/AbstractDomainKind.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/org/labkey/api/exp/property/AbstractDomainKind.java b/api/src/org/labkey/api/exp/property/AbstractDomainKind.java index e14631cbcbd..5ded66b0c70 100644 --- a/api/src/org/labkey/api/exp/property/AbstractDomainKind.java +++ b/api/src/org/labkey/api/exp/property/AbstractDomainKind.java @@ -206,7 +206,7 @@ protected boolean getBlanksSql(Domain domain, DomainProperty prop, SQLFragment b else { // Issue 29047 - blankRowsSQL.appendIdentifier(columnId).append(" IS NOT NULL"); + blankRowsSQL.appendIdentifier(columnId).append(" IS NULL"); } if (prop.isMvEnabled()) { From ac633c9de1d92e0ea8eaeae120402705d4a06690 Mon Sep 17 00:00:00 2001 From: Karl Lum Date: Tue, 7 Apr 2026 14:20:37 -0700 Subject: [PATCH 02/12] Link samples to study (#7552) #### Rationale https://github.com/LabKey/internal-issues/issues/901 Fixes auto-link from samples when subject and timepoint information are linked through lineage sources. Prior to this change all information could only be sourced from the same container, now for product folders the container filter is generated. Also implement the equivalent to `getContainerFilterForFolder` for the server that exists in our client code. #### Related Pull Requests https://github.com/LabKey/platform/pull/7552 https://github.com/LabKey/limsModules/pull/2095 https://github.com/LabKey/labkey-ui-premium/pull/929 --------- Co-authored-by: cnathe --- .../org/labkey/api/query/QueryService.java | 34 ++++++++++++++++--- .../org/labkey/query/QueryServiceImpl.java | 34 +++++++++++++++++++ .../study/assay/StudyPublishManager.java | 12 +++++-- 3 files changed, 74 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/query/QueryService.java b/api/src/org/labkey/api/query/QueryService.java index f4506494969..1d2205dbdc2 100644 --- a/api/src/org/labkey/api/query/QueryService.java +++ b/api/src/org/labkey/api/query/QueryService.java @@ -16,18 +16,38 @@ package org.labkey.api.query; +import jakarta.servlet.http.HttpSession; import org.apache.commons.collections4.SetValuedMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONObject; import org.labkey.api.audit.AuditHandler; import org.labkey.api.audit.DetailedAuditTypeEvent; +import org.labkey.api.data.ColumnHeaderType; +import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.CompareType; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.DbSchema; +import org.labkey.api.data.DisplayColumn; +import org.labkey.api.data.Filter; +import org.labkey.api.data.JdbcType; +import org.labkey.api.data.MethodInfo; +import org.labkey.api.data.MutableColumnInfo; +import org.labkey.api.data.ParameterDescription; +import org.labkey.api.data.ParameterDescriptionImpl; +import org.labkey.api.data.QueryLogging; +import org.labkey.api.data.Results; +import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.Sort; +import org.labkey.api.data.SqlSelector; +import org.labkey.api.data.TableInfo; +import org.labkey.api.data.TableSelector; +import org.labkey.api.data.dialect.SqlDialect; import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; +import org.labkey.api.module.Module; import org.labkey.api.pipeline.PipelineJob; import org.labkey.api.query.column.ColumnInfoTransformer; -import org.labkey.api.data.*; -import org.labkey.api.data.dialect.SqlDialect; -import org.labkey.api.module.Module; import org.labkey.api.query.column.ConceptURIColumnInfoTransformer; import org.labkey.api.query.snapshot.QuerySnapshotDefinition; import org.labkey.api.security.User; @@ -41,7 +61,6 @@ import org.labkey.data.xml.TableType; import org.springframework.web.servlet.mvc.Controller; -import jakarta.servlet.http.HttpSession; import java.io.IOException; import java.sql.ResultSet; import java.sql.SQLException; @@ -653,6 +672,13 @@ default MutableColumnInfo applyColumnTransformer(MutableColumnInfo col) @Nullable ContainerFilter getContainerFilterForLookups(Container container, User user); + /** + * Provides the configured ContainerFilter to utilize when requesting data that is being read + * within a folder context. Equivalent to the client side function in @labkey/components + */ + @Nullable + ContainerFilter getContainerFilterForFolder(Container container, User user); + interface SelectBuilder { diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index ecf58bc27d8..749bbb656d8 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -3516,6 +3516,40 @@ public ContainerFilter getContainerFilterForLookups(Container container, User us return type.create(container, user); } + @Override + @Nullable + public ContainerFilter getContainerFilterForFolder(Container container, User user) + { + // Check to see if product folders support is enabled. + if (container == null || !container.isProductFoldersEnabled()) + return null; + + if (isProductFoldersDataListingScopedToProject()) + { + // When requesting data from a top-level folder context the ContainerFilter filters + // "down" the folder hierarchy for data. + if (container.isProject()) + { + if (isProductFoldersAllFolderScopeEnabled()) + return ContainerFilter.Type.AllInProjectPlusShared.create(container, user); + return ContainerFilter.Type.CurrentAndSubfoldersPlusShared.create(container, user); + } + + // When listing data in a folder scope return data scoped to the current + // folder when the experimental feature is enabled. + return ContainerFilter.Type.Current.create(container, user); + } + + // When requesting data from a top-level folder context the ContainerFilter filters + // "down" the folder hierarchy for data. + if (container.isProject()) + return ContainerFilter.Type.CurrentAndSubfoldersPlusShared.create(container, user); + + // When requesting data from a sub-folder context the ContainerFilter filters + // "up" the folder hierarchy for data. + return ContainerFilter.Type.CurrentPlusProjectAndShared.create(container, user); + } + @Override @Nullable public ContainerFilter.Type getContainerFilterTypeForLookups(Container container) diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index 8a30d5c44e7..8aaf36c2fe8 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -1126,6 +1126,11 @@ public void autoLinkDerivedSamples(ExpSampleType sampleType, List keys, Co qs.setQueryName(sampleType.getName()); qs.setBaseFilter(new SimpleFilter().addInClause(FieldKey.fromParts("RowId"), keys)); + // GitHub Issue #901 : lineage sourced subject or timepoint information are not resolving in cross folder configurations + ContainerFilter cf = QueryService.get().getContainerFilterForFolder(container, user); + if (cf != null) + qs.setContainerFilterName(cf.getType().name()); + Map fieldKeyMap = StudyPublishService.get().getSamplePublishFieldKeys(user, container, sampleType, qs); UserSchema userSchema = QueryService.get().getUserSchema(user, container, SamplesSchema.SCHEMA_NAME); QueryView view = new QueryView(userSchema, qs, null); @@ -1231,15 +1236,18 @@ public void autoLinkSamples(ExpSampleType sampleType, List for (Map row : results) { Object timePointValue = getTimepointValue(row, publishKeys, visitBased, translateMap); - if (row.containsKey(publishKeys.get(LinkToStudyKeys.ParticipantId)) && timePointValue != null) + Object subjectId = row.get(publishKeys.get(LinkToStudyKeys.ParticipantId)); + if (subjectId != null && timePointValue != null) { dataMaps.add(Map.of( - LinkToStudyKeys.ParticipantId.name(), row.get(publishKeys.get(LinkToStudyKeys.ParticipantId)), + LinkToStudyKeys.ParticipantId.name(), subjectId, timePointPropName, timePointValue, StudyPublishService.ROWID_PROPERTY_NAME, row.get(FieldKey.fromParts(StudyPublishService.ROWID_PROPERTY_NAME)), StudyPublishService.SOURCE_LSID_PROPERTY_NAME, sampleType.getLSID() )); } + else + LOG.error("Failed to auto link samples to study for folder {}. Timepoint and subject ID were both not resolved.", container.getPath()); } } From 6b7e9a5ddd0ce2372f67f8d2a34ab0d901cb7f4b Mon Sep 17 00:00:00 2001 From: Karl Lum Date: Wed, 8 Apr 2026 09:50:17 -0700 Subject: [PATCH 03/12] Update search behavior for restricted issues. (#7529) #### Rationale Restricted issues were previously not getting indexed by the crawler because the search user did not have access to all of the issues. If the index were deleted, these issues would never reappear in the index unless they were resaved by a user with access. This change ensures that all restricted issues can be indexed by the crawler. Additionally any restricted issues that appear in the search summary will not render any comments. Instead, a warning will be displayed to inform the user that they do not have access to that restricted issue. As was the behavior prior, clicking on the details link will generate an unauthorized exception. --- .../org/labkey/issue/model/IssueManager.java | 41 +++++++++++------ .../org/labkey/issue/view/searchSummary.jsp | 46 +++++++++++-------- 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/issues/src/org/labkey/issue/model/IssueManager.java b/issues/src/org/labkey/issue/model/IssueManager.java index a2b4bd33d21..d598e313e94 100644 --- a/issues/src/org/labkey/issue/model/IssueManager.java +++ b/issues/src/org/labkey/issue/model/IssueManager.java @@ -948,6 +948,11 @@ private static class IndexGroup implements Consumer(issue, isRestricted)); } } diff --git a/issues/src/org/labkey/issue/view/searchSummary.jsp b/issues/src/org/labkey/issue/view/searchSummary.jsp index e8a38dff56e..6ebc4acfc5d 100644 --- a/issues/src/org/labkey/issue/view/searchSummary.jsp +++ b/issues/src/org/labkey/issue/view/searchSummary.jsp @@ -23,12 +23,15 @@ <%@ page import="java.util.regex.Matcher" %> <%@ page import="java.util.regex.Pattern" %> <%@ page import="org.apache.commons.lang3.Strings" %> +<%@ page import="org.labkey.api.util.Pair" %> <%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %> <%@ page extends="org.labkey.api.jsp.JspBase" %> <% - JspView me = HttpView.currentView(); - final IssueObject issue = me.getModelBean(); + JspView> me = HttpView.currentView(); + final Pair rec = me.getModelBean(); final User user = getUser(); + final IssueObject issue = rec.first; + final Boolean isRestricted = rec.second; final boolean isClosed = Strings.CI.equals(issue.getStatus(),"closed"); final boolean isOpen = Strings.CI.equals(issue.getStatus(),"open"); %> @@ -47,29 +50,36 @@ <% StringBuilder html = new StringBuilder(); boolean hasTextComment = false; - for (IssueObject.CommentObject comment : issue.getCommentObjects()) + if (isRestricted) { - String s = comment.getHtmlComment().toString(); - String pattern1 = "
"; - String pattern2 = "
"; - String regexString = Pattern.quote(pattern1) + "(?s)(.*?)" + Pattern.quote(pattern2); - Pattern p = Pattern.compile(regexString); - Matcher matcher = p.matcher(s); - while (matcher.find()) + html.append("
Restricted Issue: You do not have access. Contact your administrator for access.
"); + } + else + { + for (IssueObject.CommentObject comment : issue.getCommentObjects()) { - String commentContentText = matcher.group(1); - if (!StringUtils.isEmpty(commentContentText)) + String s = comment.getHtmlComment().toString(); + String pattern1 = "
"; + String pattern2 = "
"; + String regexString = Pattern.quote(pattern1) + "(?s)(.*?)" + Pattern.quote(pattern2); + Pattern p = Pattern.compile(regexString); + Matcher matcher = p.matcher(s); + while (matcher.find()) { - hasTextComment = true; - html.append(commentContentText); - html.append("
"); - if (html.length() > 500) - break; + String commentContentText = matcher.group(1); + if (!StringUtils.isEmpty(commentContentText)) + { + hasTextComment = true; + html.append(commentContentText); + html.append("
"); + if (html.length() > 500) + break; + } } } } - if (hasTextComment) { %> + if (hasTextComment && !isRestricted) { %> <% } %>
<%= unsafe(html.toString()) %>
From a5c73221819334477c39199ce4f1eb6778f33496 Mon Sep 17 00:00:00 2001 From: vagisha Date: Wed, 8 Apr 2026 20:48:14 -0700 Subject: [PATCH 04/12] Add DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE to XmlBeansUtil (#7561) #### Rationale `XmlBeansUtil.DOCUMENT_BUILDER_FACTORY` sets `disallow-doctype-decl=true` for XXE protection, which causes parsers to fail on any XML with a `` declaration. This is a problem for the Panorama Public code that parses NCBI's `esummary.fcgi` response that begins with `` #### Related Pull Requests - https://github.com/LabKey/MacCossLabModules/pull/605 - https://github.com/LabKey/MacCossLabModules/pull/623 #### Changes - Added `DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE` to `XmlBeansUtil`, mirroring the existing `SAX_PARSER_FACTORY_ALLOWING_DOCTYPE`. The DOCTYPE declaration is permitted, but every other XXE mitigation stays in place. - Extracted a private `documentBuilderFactory(boolean allowDocType)` helper, mirroring the existing `saxParserFactory(boolean)` helper. --- api/src/org/labkey/api/util/XmlBeansUtil.java | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index a2617bcd752..f26c2c4c252 100644 --- a/api/src/org/labkey/api/util/XmlBeansUtil.java +++ b/api/src/org/labkey/api/util/XmlBeansUtil.java @@ -132,6 +132,7 @@ public static void addComment(XmlTokenSource doc, String comment) public static final SAXParserFactory SAX_PARSER_FACTORY_ALLOWING_DOCTYPE; public static final XMLInputFactory XML_INPUT_FACTORY; public static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY; + public static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE; static { @@ -145,16 +146,9 @@ public static void addComment(XmlTokenSource doc, String comment) SAX_PARSER_FACTORY = saxParserFactory(false); SAX_PARSER_FACTORY_ALLOWING_DOCTYPE = saxParserFactory(true); - //noinspection XMLInputFactory - DOCUMENT_BUILDER_FACTORY = DocumentBuilderFactory.newInstance(); - DOCUMENT_BUILDER_FACTORY.setNamespaceAware(true); - DOCUMENT_BUILDER_FACTORY.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - DOCUMENT_BUILDER_FACTORY.setFeature("http://xml.org/sax/features/external-general-entities", false); - DOCUMENT_BUILDER_FACTORY.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - DOCUMENT_BUILDER_FACTORY.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - DOCUMENT_BUILDER_FACTORY.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - DOCUMENT_BUILDER_FACTORY.setXIncludeAware(false); - DOCUMENT_BUILDER_FACTORY.setExpandEntityReferences(false); + DOCUMENT_BUILDER_FACTORY = documentBuilderFactory(false); + // Use the ALLOWING_DOCTYPE variant when parsing XML that contains a declaration (e.g. NCBI's eSummary responses) + DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE = documentBuilderFactory(true); } catch (ParserConfigurationException | SAXException e) { @@ -181,4 +175,26 @@ private static SAXParserFactory saxParserFactory(boolean allowDocType) throws SA result.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); return result; } + + private static DocumentBuilderFactory documentBuilderFactory(boolean allowDocType) throws ParserConfigurationException + { + //noinspection XMLInputFactory + DocumentBuilderFactory result = DocumentBuilderFactory.newInstance(); + result.setNamespaceAware(true); + + // Disable features that could lead to XXE or other vulnerabilities. + // When allowDocType is true the DOCTYPE declaration is permitted. External entity + // resolution remains disabled, so XXE protection is still in effect. + if (!allowDocType) + { + result.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + } + result.setFeature("http://xml.org/sax/features/external-general-entities", false); + result.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + result.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + result.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + result.setXIncludeAware(false); + result.setExpandEntityReferences(false); + return result; + } } From 3974e4140bd703246fd7b5ffb365739d869f5155 Mon Sep 17 00:00:00 2001 From: Karl Lum Date: Fri, 10 Apr 2026 08:36:19 -0700 Subject: [PATCH 05/12] Remove error on failed link (#7565) #### Rationale The error is a bit overaggressive. It would be normal to not include subject/timepoint information in sample imports that have autolink configured. Reverting back to ignoring the row. --- study/src/org/labkey/study/assay/StudyPublishManager.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index 8aaf36c2fe8..93a1bac4880 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -1246,8 +1246,6 @@ public void autoLinkSamples(ExpSampleType sampleType, List StudyPublishService.SOURCE_LSID_PROPERTY_NAME, sampleType.getLSID() )); } - else - LOG.error("Failed to auto link samples to study for folder {}. Timepoint and subject ID were both not resolved.", container.getPath()); } } From 7cc712957043609ca514d23b27a0f98cdf6ff286 Mon Sep 17 00:00:00 2001 From: Nick Kerr Date: Fri, 10 Apr 2026 13:37:18 -0700 Subject: [PATCH 06/12] List: delete data in subfolders (#7563) --- .../org/labkey/api/exp/list/ListService.java | 1 + .../experiment/api/ExperimentServiceImpl.java | 15 +- .../labkey/list/model/ListDefinitionImpl.java | 30 +-- .../org/labkey/list/model/ListManager.java | 245 +++++++++++++----- .../list/model/ListQueryUpdateService.java | 44 ++-- .../labkey/list/model/ListServiceImpl.java | 6 + 6 files changed, 214 insertions(+), 127 deletions(-) diff --git a/api/src/org/labkey/api/exp/list/ListService.java b/api/src/org/labkey/api/exp/list/ListService.java index 9d79a027e38..7d924fcff32 100644 --- a/api/src/org/labkey/api/exp/list/ListService.java +++ b/api/src/org/labkey/api/exp/list/ListService.java @@ -51,6 +51,7 @@ static void setInstance(ListService ls) boolean hasLists(Container container, boolean includeProjectAndShared); ListDefinition createList(Container container, String name, ListDefinition.KeyType keyType); ListDefinition createList(Container container, String name, ListDefinition.KeyType keyType, @Nullable TemplateInfo templateInfo, @Nullable ListDefinition.Category category); + void deleteLists(Container container, User user, @Nullable String auditUserComment); @Nullable ListDefinition getList(Container container, int listId); @Nullable ListDefinition getList(Container container, String name); @Nullable ListDefinition getList(Container container, String name, boolean includeProjectAndShared); diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 401357afcc1..9b9ed24af05 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -5497,18 +5497,9 @@ public void deleteAllExpObjInContainer(Container c, User user) throws Experiment { deleteExperimentRunsByRowIds(c, user, runId); } - ListService ls = ListService.get(); - if (ls != null) - { - for (ListDefinition list : ListService.get().getLists(c, null, false).values()) - { - // Temporary fix for Issue 21400: **Deleting workbook deletes lists defined in parent container - if (list.getContainer().equals(c)) - { - list.delete(user); - } - } - } + + // Delete lists (because for some reason lists are under the purview of experiment...) + ListService.get().deleteLists(c, user, null); // Delete DataClasses and their exp.Data members // Need to delete DataClass before SampleTypes since they may be referenced by the DataClass diff --git a/list/src/org/labkey/list/model/ListDefinitionImpl.java b/list/src/org/labkey/list/model/ListDefinitionImpl.java index dc4fbb7e074..d766c2542dd 100644 --- a/list/src/org/labkey/list/model/ListDefinitionImpl.java +++ b/list/src/org/labkey/list/model/ListDefinitionImpl.java @@ -565,37 +565,9 @@ public void delete(User user) throws DomainNotFoundException @Override public void delete(User user, @Nullable String auditUserComment) throws DomainNotFoundException { - TableInfo table = getTable(user); - QueryUpdateService qus = null; - - if (null != table) - qus = table.getUpdateService(); - - // In certain cases we may create a list that is not viable (i.e., one in which a table was never created because - // the metadata wasn't valid). Still allow deleting the list - try (DbScope.Transaction transaction = (table != null) ? table.getSchema().getScope().ensureTransaction() : - ExperimentService.get().ensureTransaction()) - { - // remove related full-text search docs and attachments - ListManager.get().deleteIndexedList(this); - if (qus instanceof ListQueryUpdateService listQus) - listQus.deleteRelatedListData(null); - - // then delete the list itself - ListManager.get().deleteListDef(getContainer(), getListId()); - Domain domain = getDomainOrThrow(); - domain.delete(user, auditUserComment); - - ListManager.get().addAuditEvent(this, user, String.format("The list %s was deleted", _def.getName())); - - transaction.commit(); - } - - SchemaKey schemaPath = SchemaKey.fromParts(ListQuerySchema.NAME); - QueryService.get().fireQueryDeleted(user, getContainer(), null, schemaPath, Collections.singleton(getName())); + ListManager.get().deleteList(user, this, auditUserComment); } - @Override public int insertListItems(User user, Container container, List listItems) { diff --git a/list/src/org/labkey/list/model/ListManager.java b/list/src/org/labkey/list/model/ListManager.java index 497721ba2b4..557eeae8e0a 100644 --- a/list/src/org/labkey/list/model/ListManager.java +++ b/list/src/org/labkey/list/model/ListManager.java @@ -16,16 +16,18 @@ package org.labkey.list.model; +import org.apache.commons.collections4.MapUtils; import org.apache.commons.collections4.MultiValuedMap; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import org.junit.After; +import org.junit.AfterClass; import org.junit.Assert; -import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; import org.labkey.api.attachments.AttachmentService; +import org.labkey.api.attachments.FileAttachmentFile; import org.labkey.api.audit.AuditLogService; import org.labkey.api.cache.Cache; import org.labkey.api.cache.CacheLoader; @@ -35,6 +37,7 @@ import org.labkey.api.data.*; import org.labkey.api.data.Selector.ForEachBlock; import org.labkey.api.exceptions.OptimisticConflictException; +import org.labkey.api.exp.DomainNotFoundException; import org.labkey.api.exp.DomainURIFactory; import org.labkey.api.exp.ImportTypesHelper; import org.labkey.api.exp.OntologyManager.ImportPropertyDescriptor; @@ -49,6 +52,7 @@ import org.labkey.api.exp.property.Domain; import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.exp.property.PropertyService; +import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.FieldKey; import org.labkey.api.query.QueryChangeListener; import org.labkey.api.query.QueryService; @@ -57,7 +61,7 @@ import org.labkey.api.security.User; import org.labkey.api.security.UserManager; import org.labkey.api.util.ExceptionUtil; -import org.labkey.api.util.JunitUtil; +import org.labkey.api.util.FileUtil; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; import org.labkey.api.util.Path; @@ -76,6 +80,7 @@ import org.labkey.list.view.ListItemAttachmentParent; import org.springframework.jdbc.BadSqlGrammarException; +import java.io.File; import java.sql.Timestamp; import java.util.ArrayList; import java.util.Collection; @@ -453,20 +458,76 @@ private void queryChangeUpdate(User user, Container c, String oldName, String up QueryChangeListener.QueryPropertyChange.handleQueryNameChange(oldName, updatedName, new SchemaKey(null, ListQuerySchema.NAME), user, c); } - // CONSIDER: move "list delete" from ListDefinitionImpl.delete() implementation to ListManager for consistency - void deleteListDef(Container c, int listid) + void deleteList(User user, @NotNull ListDefinitionImpl list, @Nullable String auditUserComment) throws DomainNotFoundException + { + var table = list.getTable(user); + var container = list.getContainer(); + var listName = list.getName(); + + // In certain cases we may create a list that is not viable (i.e., one in which a table was never created because + // the metadata wasn't valid). Still allow deleting the list. + try (var tx = (table != null) ? table.getSchema().getScope().ensureTransaction() : ExperimentService.get().ensureTransaction()) + { + // remove related full-text search docs and attachments + if (table != null && table.getUpdateService() instanceof ListQueryUpdateService qus) + qus.deleteRelatedListData(null); + else + deleteIndexedList(list); + + try + { + Table.delete(getListMetadataTable(), new Object[]{container, list.getListId()}); + } + catch (OptimisticConflictException x) + { + // ok + } + _listDefCache.remove(container.getId()); + + list.getDomainOrThrow().delete(user, auditUserComment); + addAuditEvent(list, user, String.format("The list %s was deleted", listName)); + + tx.commit(); + } + + QueryService.get().fireQueryDeleted(user, container, null, SchemaKey.fromParts(ListQuerySchema.NAME), Collections.singleton(listName)); + } + + /** + * Deletes all lists and list data in the given container. + */ + public void deleteLists(Container c, User user, @Nullable String auditUserComment) { - DbScope scope = getListMetadataSchema().getScope(); - assert scope.isTransactionActive(); try { - Table.delete(ListManager.get().getListMetadataTable(), new Object[]{c, listid}); + var containerId = c.getEntityId().toString(); + for (var list : ListManager.get().getLists(c, true)) + { + var listDef = ListDefinitionImpl.of(list); + + // Delete the entire list when the list's container is deleted + if (containerId.equals(list.getContainerId())) + { + deleteList(user, listDef, auditUserComment); + continue; + } + + var table = listDef.getTable(user); + if (table == null) + continue; + + var qus = table.getUpdateService(); + if (qus == null) + continue; + + // Otherwise, truncate the rows in this list in this container + qus.truncateRows(user, c, null, null); + } } - catch (OptimisticConflictException x) + catch (Exception e) { - // ok + throw new RuntimeException(e.getMessage(), e); } - _listDefCache.remove(c.getId()); } public static final SearchService.SearchCategory listCategory = new SearchService.SearchCategory("list", "Lists"); @@ -1264,61 +1325,77 @@ boolean importListSchema( public static class TestCase extends Assert { + private static final String PROJECT_NAME = "BiologicsManagerTest Project"; private static final String LIST_NAME = "Unit Test list"; - private static final String WORKBOOK1_NAME = "Unit Test Workbook 1"; - private static final String WORKBOOK2_NAME = "Unit Test Workbook 2"; private static final String FIELD_NAME = "field"; + private static final String ATTACHMENT_FIELD_NAME = "attachment"; private static final String PARENT_LIST_ITEM = "parentItem"; - private static final String WORKBOOK1_LIST_ITEM = "workbook1Item"; - private static final String WORKBOOK2_LIST_ITEM = "workbook2Item"; - private static final Integer PARENT_LI_KEY = 1; - private static final Integer WB1_LI_KEY = 2; - private static final Integer WB2_LI_KEY = 3; - - private ListDefinitionImpl list; - private Container c; - private User u; - private DomainProperty dp; - - @Before - public void setUp() throws Exception + private static final String CHILD1_LIST_ITEM = "child1Item"; + private static final String CHILD2_LIST_ITEM = "child2Item"; + private static final String KEY_FIELD_NAME = "Unit test list key"; + private static Integer PARENT_LI_KEY; + + private static ListDefinitionImpl list; + private static DomainProperty dp; + private static User u; + private static Container c; + + @BeforeClass + public static void setup() throws Exception { - JunitUtil.deleteTestContainer(); - cleanup(); - c = JunitUtil.getTestContainer(); - TestContext context = TestContext.get(); - u = context.getUser(); + u = TestContext.get().getUser(); + + deleteTestContainer(); + + c = ContainerManager.ensureContainer(PROJECT_NAME, u); list = (ListDefinitionImpl)ListService.get().createList(c, LIST_NAME, ListDefinition.KeyType.AutoIncrementInteger); - list.setKeyName("Unit test list Key"); + list.setKeyName(KEY_FIELD_NAME); dp = list.getDomain().addProperty(); dp.setName(FIELD_NAME); dp.setType(PropertyService.get().getType(c, PropertyType.STRING.getXmlName())); dp.setPropertyURI(ListDomainKind.createPropertyURI(list.getName(), FIELD_NAME, c, list.getKeyType()).toString()); + + var attachmentDp = list.getDomain().addProperty(); + attachmentDp.setName(ATTACHMENT_FIELD_NAME); + attachmentDp.setType(PropertyService.get().getType(c, PropertyType.ATTACHMENT.getXmlName())); + attachmentDp.setPropertyURI(ListDomainKind.createPropertyURI(list.getName(), ATTACHMENT_FIELD_NAME, c, list.getKeyType()).toString()); list.save(u); - addListItem(c, list, PARENT_LIST_ITEM); + PARENT_LI_KEY = addListItem(c, list, PARENT_LIST_ITEM, null); } - private void addListItem(Container scopedContainer, ListDefinition scopedList, String value) + private static int addListItem(Container scopedContainer, ListDefinition scopedList, String value, @Nullable File attachment) throws Exception { - List lis = new ArrayList<>(); - ListItem li = scopedList.createListItem(); - li.setProperty(dp, value); - lis.add(li); - list.insertListItems(u, scopedContainer, lis); + var row = new CaseInsensitiveHashMap<>(); + row.put(FIELD_NAME, value); + if (attachment != null) + row.put(ATTACHMENT_FIELD_NAME, new FileAttachmentFile(attachment)); + + var errors = new BatchValidationException(); + var result = scopedList.getTable(u).getUpdateService().insertRows(u, scopedContainer, List.of(row), errors, null, null); + if (errors.hasErrors()) + throw errors.getLastRowError(); + + return MapUtils.getIntValue(result.getFirst(), KEY_FIELD_NAME); } - @After - public void tearDown() throws Exception + @AfterClass + public static void cleanup() { - cleanup(); + deleteTestContainer(); + + dp = null; + c = null; + u = null; } - private void cleanup() throws Exception + private static void deleteTestContainer() { - //TestContext context = TestContext.get(); - ExperimentService.get().deleteAllExpObjInContainer(c, u); + var project = ContainerManager.getForPath(PROJECT_NAME); + + if (project != null) + ContainerManager.deleteAll(project, TestContext.get().getUser()); } @Test @@ -1326,39 +1403,81 @@ public void testListServiceInOwnFolder() { Map lists = ListService.get().getLists(c); assertTrue("Test List not found in own container", lists.containsKey(LIST_NAME)); - ListItem li = lists.get(LIST_NAME).getListItem(1, u, c); + ListItem li = lists.get(LIST_NAME).getListItem(PARENT_LI_KEY, u, c); assertEquals("Item not found in own container", PARENT_LIST_ITEM, li.getProperty(dp)); } @Test - public void testListServiceInWorkbook() + public void testListServiceInWorkbook() throws Exception { - Container workbook1 = setupWorkbook(WORKBOOK1_NAME); - Container workbook2 = setupWorkbook(WORKBOOK2_NAME); + Container workbook1 = createWorkbook("Unit Test Workbook 1"); + Container workbook2 = createWorkbook("Unit Test Workbook 2"); Map lists = ListService.get().getLists(workbook1); assertTrue("Test List not found in workbook", lists.containsKey(LIST_NAME)); - checkListItemScoping(workbook1, workbook2); + ListDefinition childList1 = ListService.get().getLists(workbook1).get(LIST_NAME); + ListDefinition childList2 = ListService.get().getLists(workbook2).get(LIST_NAME); + + assertEquals("Lists available to each child container are not the same", childList1.toString(), childList2.toString()); + int childKey1 = addListItem(workbook1, childList1, CHILD1_LIST_ITEM, null); + int childKey2 = addListItem(workbook2, childList2, CHILD2_LIST_ITEM, null); + + assertNull("Parent item should not be visible in child container", childList1.getListItem(PARENT_LI_KEY, u, workbook1)); + assertNull("Sibling child item should not be visible in another child container", childList1.getListItem(childKey2, u, workbook1)); + assertEquals("Parent container should be able to see child container item", CHILD1_LIST_ITEM, childList1.getListItem(childKey1, u, c).getProperty(dp)); + assertEquals("Child container should be able to see its own list item", CHILD1_LIST_ITEM, childList1.getListItem(childKey1, u, workbook1).getProperty(dp)); } - private Container setupWorkbook(String title) + /** + * GitHub Issue #1013: Rows added to a list in a subfolder must be cleaned up when that subfolder + * is deleted. Previously, those rows were left orphaned in the database. + */ + @Test + public void testSubfolderDeletionCleansUpListData() throws Exception { - return ContainerManager.createContainer(c, null, title, null, WorkbookContainerType.NAME, u); + Container subfolder1 = createSubfolder("subfolder1"); + Container subfolder2 = createSubfolder("subfolder2"); + + File attachment1 = FileUtil.createTempFile("subfolder1", ".txt"); + attachment1.deleteOnExit(); + File attachment2 = FileUtil.createTempFile("subfolder2", ".txt"); + attachment2.deleteOnExit(); + + int key1 = addListItem(subfolder1, list, "subfolder1Item", attachment1); + int key2 = addListItem(subfolder2, list, "subfolder2Item", attachment2); + + // Precondition: parent can see both subfolder items (setUp inserts key 1; subfolders get keys 2 and 3) + assertNotNull("Subfolder1 item should be visible from subfolder1 before deletion", list.getListItem(key1, u, subfolder1)); + assertNotNull("Subfolder2 item should be visible from subfolder2 before deletion", list.getListItem(key2, u, subfolder2)); + + TableInfo table = list.getTable(u, c, ContainerFilter.getUnsafeEverythingFilter()); + assertNotNull("Expected list table to resolve", table); + + // Preverify rows in the database + long rowCount = new TableSelector(table, Set.of("Container")).getRowCount(); + assertEquals("Expected only two rows (parent, subfolder1, subfolder2)", 3, rowCount); + + // Delete subfolder1 — should delete its list rows and attachments, + // not touch subfolder2 or the parent or the list definition + ListService.get().deleteLists(subfolder1, u, null); + + // Verify row is deleted from underlying table + rowCount = new TableSelector(table, Set.of("Container")).getRowCount(); + assertEquals("Expected only two rows (parent, subfolder2)", 2, rowCount); + + // Items belonging to other containers should be unaffected + assertNotNull("Parent item should survive subfolder1 deletion", list.getListItem(PARENT_LI_KEY, u, c)); + assertNotNull("Subfolder2 item should survive subfolder1 deletion", list.getListItem(key2, u, subfolder2)); } - private void checkListItemScoping(Container wb1, Container wb2) + private Container createSubfolder(String folderName) { - ListDefinition wbList1 = ListService.get().getLists(wb1).get(LIST_NAME); - ListDefinition wbList2 = ListService.get().getLists(wb2).get(LIST_NAME); - - assertEquals("Lists available to each workbook are not the same", wbList1.toString(), wbList2.toString()); - addListItem(wb1, wbList1, WORKBOOK1_LIST_ITEM); - addListItem(wb2, wbList2, WORKBOOK2_LIST_ITEM); + return ContainerManager.createContainer(c, folderName, u); + } - assertNull("Parent item visible in workbook", wbList1.getListItem(PARENT_LI_KEY, u, wb1)); - assertNull("Sibling workbook item visible in another workbook", wbList1.getListItem(WB2_LI_KEY, u, wb1)); - assertEquals("Parent container can not see child workbook item", WORKBOOK1_LIST_ITEM, wbList1.getListItem(WB1_LI_KEY, u, c).getProperty(dp)); - assertEquals("Workbook can not see its own list item", WORKBOOK1_LIST_ITEM, wbList1.getListItem(WB1_LI_KEY, u, wb1).getProperty(dp)); + private Container createWorkbook(String title) + { + return ContainerManager.createContainer(c, null, title, null, WorkbookContainerType.NAME, u); } } } diff --git a/list/src/org/labkey/list/model/ListQueryUpdateService.java b/list/src/org/labkey/list/model/ListQueryUpdateService.java index b4747339634..9d8502155d4 100644 --- a/list/src/org/labkey/list/model/ListQueryUpdateService.java +++ b/list/src/org/labkey/list/model/ListQueryUpdateService.java @@ -35,7 +35,6 @@ import org.labkey.api.data.DbScope; import org.labkey.api.data.LookupResolutionType; import org.labkey.api.data.RuntimeSQLException; -import org.labkey.api.data.Selector.ForEachBatchBlock; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; @@ -91,6 +90,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; import static org.labkey.api.util.IntegerUtils.isIntegral; @@ -761,7 +761,7 @@ protected Map deleteRow(User user, Container container, Map() - { - @Override - public boolean accept(ListRow row) - { - return null != row.entityId(); - } - - @Override - public void exec(List rows) - { - // delete the related attachments for this block - AttachmentService.get().deleteAttachments( - rows.stream() - .map(row -> new ListItemAttachmentParent(row.entityId(), row.container())) - .collect(Collectors.toList()) - ); - } + var filter = container == null ? new SimpleFilter() : SimpleFilter.createContainerFilter(container); + filter.addCondition(FieldKey.fromParts("EntityId"), null, CompareType.NONBLANK); + + // Delete attachments associated with a list in batches + new TableSelector(getDbTable(), new CsvSet("Container, EntityId"), filter, null).forEachBatch(ListRow.class, 1_000, rows -> { + var containerMap = new HashMap(); + + // delete the related attachments for this block + AttachmentService.get().deleteAttachments( + rows.stream() + .map(row -> { + var c = containerMap.computeIfAbsent(row.container(), ContainerManager::getForId); + if (c == null) + return null; + return new ListItemAttachmentParent(row.entityId(), c); + }) + .filter(Objects::nonNull) + .collect(Collectors.toList()) + ); }); } } diff --git a/list/src/org/labkey/list/model/ListServiceImpl.java b/list/src/org/labkey/list/model/ListServiceImpl.java index 20d2f33e905..9d93911d038 100644 --- a/list/src/org/labkey/list/model/ListServiceImpl.java +++ b/list/src/org/labkey/list/model/ListServiceImpl.java @@ -113,6 +113,12 @@ public ListDefinition createList(Container container, String name, ListDefinitio return new ListDefinitionImpl(container, name, keyType, category, templateInfo); } + @Override + public void deleteLists(Container container, User user, @Nullable String auditUserComment) + { + ListManager.get().deleteLists(container, user, auditUserComment); + } + @Override @Nullable public ListDefinition getList(Container container, int listId) From e730d71dae1368126daf2c94234832eaa163a617 Mon Sep 17 00:00:00 2001 From: Xing Yang <5168106+XingY@users.noreply.github.com> Date: Fri, 10 Apr 2026 17:42:20 -0700 Subject: [PATCH 07/12] GitHub Issue 1028: Can't delete a sample when any sample in the sample type has been linked to study (#7572) --- .../study/publish/StudyPublishService.java | 2 +- .../study/assay/ExperimentListenerImpl.java | 55 +++++++++++++------ .../study/assay/StudyPublishManager.java | 7 +-- .../study/controllers/StudyController.java | 4 +- 4 files changed, 45 insertions(+), 23 deletions(-) diff --git a/api/src/org/labkey/api/study/publish/StudyPublishService.java b/api/src/org/labkey/api/study/publish/StudyPublishService.java index 027a78aabf9..45b554d9159 100644 --- a/api/src/org/labkey/api/study/publish/StudyPublishService.java +++ b/api/src/org/labkey/api/study/publish/StudyPublishService.java @@ -156,7 +156,7 @@ ActionURL publishData(User user, Container sourceContainer, @Nullable Container String checkForLockedLinks(Dataset def, @Nullable List rowIds); - void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection> datasetRowLsidAndSourceRowIds); + void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection rowIds); /** * Adds columns to an assay data table, providing a link to any datasets that have diff --git a/study/src/org/labkey/study/assay/ExperimentListenerImpl.java b/study/src/org/labkey/study/assay/ExperimentListenerImpl.java index ece944cc309..ec8c6e027b9 100644 --- a/study/src/org/labkey/study/assay/ExperimentListenerImpl.java +++ b/study/src/org/labkey/study/assay/ExperimentListenerImpl.java @@ -15,6 +15,7 @@ */ package org.labkey.study.assay; +import org.labkey.api.data.CompareType; import org.labkey.api.data.Container; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TableInfo; @@ -30,8 +31,10 @@ import org.labkey.api.query.QueryService; import org.labkey.api.query.UserSchema; import org.labkey.api.query.ValidationException; +import org.labkey.api.security.ElevatedUser; import org.labkey.api.security.User; import org.labkey.api.security.permissions.DeletePermission; +import org.labkey.api.security.roles.ReaderRole; import org.labkey.api.study.Dataset; import org.labkey.api.study.publish.StudyPublishService; import org.labkey.api.view.UnauthorizedException; @@ -41,6 +44,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import static java.util.Collections.singleton; @@ -72,41 +76,58 @@ public void beforeMaterialDelete(List materials, Containe // It's likely that we'll have multiple materials from the same sample type, so group them for efficient processing - Map> typeToMaterials = new HashMap<>(); + Map>> typeToMaterials = new HashMap<>(); for (ExpMaterial material: materials) { ExpSampleType sampleType = material.getSampleType(); if (sampleType != null) { - typeToMaterials.computeIfAbsent(sampleType, x -> new ArrayList<>()).add(material); + Container materialContainer = material.getContainer(); + typeToMaterials. + computeIfAbsent(sampleType, k -> new HashMap<>()). + computeIfAbsent(materialContainer, k -> new ArrayList<>()). + add(material); } } - for (Map.Entry> entry : typeToMaterials.entrySet()) + for (Map.Entry>> entry : typeToMaterials.entrySet()) { for (Dataset dataset: StudyPublishService.get().getDatasetsForPublishSource(entry.getKey().getRowId(), Dataset.PublishSource.SampleType)) { - TableInfo t = dataset.getTableInfo(user); - if (null == t || !t.hasPermission(user, DeletePermission.class)) - { - throw new UnauthorizedException("Cannot delete rows from dataset " + dataset); - } + Map> containerSamples = entry.getValue(); + + // Need Read permission to check for linked samples + User userWithReadPerm = ElevatedUser.getElevatedUser(user, ReaderRole.class); - UserSchema schema = QueryService.get().getUserSchema(user, dataset.getContainer(), "study"); - TableInfo tableInfo = schema.getTable(dataset.getName()); + UserSchema schemaWithReadPerm = QueryService.get().getUserSchema(userWithReadPerm, dataset.getContainer(), "study"); + TableInfo tableInfoForRead = schemaWithReadPerm.getTable(dataset.getName()); + if (null == tableInfoForRead) + throw new UnauthorizedException("Cannot delete rows from dataset " + dataset); - // Future optimization - query for all the materials at once - for (ExpMaterial material : entry.getValue()) + for (Map.Entry> containerEntry : containerSamples.entrySet()) { - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(ExpMaterialTable.Column.RowId.toString()), material.getRowId()); - String lsid = new TableSelector(tableInfo, singleton("LSID"), filter, null).getObject(String.class); + Container sampleContainer = containerEntry.getKey(); + List samples = containerEntry.getValue(); + + // GitHub Issue 1028: Can't delete a sample when any sample in the sample type has been linked to study + // check if samples are linked to the dataset, if not, skip the permission check for DeletePermission since we won't be deleting any rows + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(ExpMaterialTable.Column.RowId.toString()), samples.stream().map(ExpMaterial::getRowId).toList(), CompareType.IN); + Map[] linkedLsidRowIds = new TableSelector(tableInfoForRead, Set.of("LSID", "RowId"), filter, null).getMapArray(); - if (lsid != null) + Set linkedLsids = Arrays.stream(linkedLsidRowIds).map(m -> (String)m.get("LSID")).collect(java.util.stream.Collectors.toSet()); + Set linkedRowIds = Arrays.stream(linkedLsidRowIds).map(m -> ((Integer)m.get("RowId")).longValue()).collect(java.util.stream.Collectors.toSet()); + if (linkedLsids.isEmpty()) + continue; + + TableInfo tableInfo = dataset.getTableInfo(user); + if (null == tableInfo || !tableInfo.hasPermission(user, DeletePermission.class)) { - StudyPublishService.get().addRecallAuditEvent(material.getContainer(), user, dataset, 1, null); - dataset.deleteDatasetRows(user, Arrays.asList(lsid)); + throw new UnauthorizedException("Cannot delete rows from dataset " + dataset); } + + StudyPublishService.get().addRecallAuditEvent(sampleContainer, user, dataset, linkedLsids.size(), linkedRowIds); + dataset.deleteDatasetRows(user, linkedLsids); } } } diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index 93a1bac4880..f0a74f4cbf0 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -1518,7 +1518,7 @@ public String checkForLockedLinks(Dataset def, @Nullable List rowIds) } @Override - public void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection> pairs) + public void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection rowIds) { Dataset.PublishSource sourceType = def.getPublishSource(); if (sourceType != null) @@ -1539,15 +1539,14 @@ public void addRecallAuditEvent(Container sourceContainer, User user, Dataset de AuditLogService.get().addEvent(user, event); // Create sample timeline event for each of the samples - if (sourceType == Dataset.PublishSource.SampleType && pairs != null) + if (sourceType == Dataset.PublishSource.SampleType && rowIds != null && !rowIds.isEmpty()) { var timelineEventType = SampleTimelineAuditEvent.SampleTimelineEventType.RECALL; Map eventMetadata = new HashMap<>(); eventMetadata.put(SAMPLE_TIMELINE_EVENT_TYPE, timelineEventType.name()); String metadata = AbstractAuditTypeProvider.encodeForDataMap(eventMetadata); - List sampleIds = pairs.stream().map(Pair::getValue).collect(toList()); - List samples = ExperimentService.get().getExpMaterials(sampleIds); + List samples = ExperimentService.get().getExpMaterials(rowIds); List events = new ArrayList<>(samples.size()); for (ExpMaterial sample : samples) { diff --git a/study/src/org/labkey/study/controllers/StudyController.java b/study/src/org/labkey/study/controllers/StudyController.java index 83b02c438fe..a4674a3a571 100644 --- a/study/src/org/labkey/study/controllers/StudyController.java +++ b/study/src/org/labkey/study/controllers/StudyController.java @@ -313,6 +313,7 @@ import java.util.concurrent.TimeUnit; import java.util.function.Predicate; import java.util.regex.Pattern; +import java.util.stream.Collectors; import static org.labkey.api.util.IntegerUtils.asInteger; import static org.labkey.study.model.QCStateSet.PUBLIC_STATES_LABEL; @@ -3106,9 +3107,10 @@ public boolean handlePost(DeleteDatasetRowsForm form, BindException errors) { String sourceLsid = entry.getKey(); Collection> pairs = entry.getValue(); + Collection rowIds = pairs.stream().map(Pair::getValue).collect(Collectors.toList()); Container sourceContainer = publishSource.resolveSourceLsidContainer(sourceLsid, _sourceRowId); if (sourceContainer != null) - StudyPublishService.get().addRecallAuditEvent(sourceContainer, getUser(), _def, pairs.size(), pairs); + StudyPublishService.get().addRecallAuditEvent(sourceContainer, getUser(), _def, pairs.size(), rowIds); } } From ef7a44750e95b85688b07aeffbab65d09aaead6c Mon Sep 17 00:00:00 2001 From: Adam Rauch Date: Sat, 11 Apr 2026 07:12:15 -0700 Subject: [PATCH 08/12] Script reordering: support CREATE/ALTER TRIGGER (#7553) --- core/src/org/labkey/core/admin/sql/ScriptReorderer.java | 1 + core/src/org/labkey/core/admin/sql/SqlScriptController.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/org/labkey/core/admin/sql/ScriptReorderer.java b/core/src/org/labkey/core/admin/sql/ScriptReorderer.java index b72cf64d750..7b8110f7f3c 100644 --- a/core/src/org/labkey/core/admin/sql/ScriptReorderer.java +++ b/core/src/org/labkey/core/admin/sql/ScriptReorderer.java @@ -118,6 +118,7 @@ public String getReorderedScript(boolean isHtml) patterns.add(new SqlPattern("DROP INDEX (IF EXISTS )?\\w+ ON " + TABLE_NAME_REGEX + STATEMENT_ENDING_REGEX, Type.Table, Operation.Other)); patterns.add(new SqlPattern("(CREATE|ALTER) PROCEDURE .+?" + STATEMENT_ENDING_REGEX, Type.NonTable, Operation.Other)); + patterns.add(new SqlPattern("(CREATE|ALTER) TRIGGER .+?" + "END GO\\s*$", Type.NonTable, Operation.Other)); patterns.add(new SqlPattern("ALTER TABLE " + TABLE_NAME_REGEX + " CHECK CONSTRAINT " + CONSTRAINT_NAME_REGEX + STATEMENT_ENDING_REGEX, Type.Table, Operation.Other)); } else diff --git a/core/src/org/labkey/core/admin/sql/SqlScriptController.java b/core/src/org/labkey/core/admin/sql/SqlScriptController.java index a7868284993..2fe3959e7de 100644 --- a/core/src/org/labkey/core/admin/sql/SqlScriptController.java +++ b/core/src/org/labkey/core/admin/sql/SqlScriptController.java @@ -1311,10 +1311,10 @@ protected ActionURL getSaveScriptActionURL(SqlScript script, String newContents, - `EXEC core.fn_dropifexists 'MyTable', 'MySchema', 'COLUMN', 'MyColumn` is the same as `ALTER TABLE TableName DROP COLUMN IF EXISTS ColumnName` Please do the following: - - Consolidate all iterative changes (column additions, PK changes, and renames) into the initial CREATE TABLE statements. + - Consolidate all iterative changes (column additions & renames, PK changes, and FK changes) into the initial CREATE TABLE statements. - Remove unnecessary DROP TABLE statements and core.fn_dropifexists calls, for example, those that come before a table has been created. - Remove all intermediate DROP and ALTER statements that are superseded by later logic. - - Remove CREATE TABLE and ALTER TABLE statements followed by DROP TABLE or and core.fn_dropifexists 'TABLE' call on that same table. + - Remove CREATE TABLE and ALTER TABLE statements followed by DROP TABLE or a core.fn_dropifexists 'TABLE' call on that same table. Include a summary of the changes you made at the end. """; From 0f4981de4d9b37731dd5747b9ef49d7efe002408 Mon Sep 17 00:00:00 2001 From: Susan Hert Date: Mon, 13 Apr 2026 06:19:28 -0700 Subject: [PATCH 09/12] Update activemq version (#7575) --- pipeline/gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pipeline/gradle.properties b/pipeline/gradle.properties index 7960fca9127..bdf43c99a8c 100644 --- a/pipeline/gradle.properties +++ b/pipeline/gradle.properties @@ -1,4 +1,4 @@ -activemqVersion=5.19.2 +activemqVersion=5.19.5 geronimoJ2eeConnector15SpecVersion=1.0.1 From f4592345e3d9412898f3242312fe8f2f5f065ee2 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Mon, 13 Apr 2026 12:55:42 -0700 Subject: [PATCH 10/12] MCP tool validateSQL() (#7570) #### Rationale MCP tool validateSql() to go along with the metadata tools #### Related Pull Requests * https://github.com/LabKey/premiumModules/pull/510 #### Changes * --- .../labkey/query/controllers/QueryMcp.java | 52 +++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/query/src/org/labkey/query/controllers/QueryMcp.java b/query/src/org/labkey/query/controllers/QueryMcp.java index b68f2c4ea67..9b11aed43f8 100644 --- a/query/src/org/labkey/query/controllers/QueryMcp.java +++ b/query/src/org/labkey/query/controllers/QueryMcp.java @@ -17,6 +17,9 @@ import org.labkey.api.query.QueryForeignKey; import org.labkey.api.query.QueryKey; import org.labkey.api.query.QueryParseException; +import org.labkey.api.query.QueryParseWarning; +import org.labkey.api.query.QuerySchema; +import org.labkey.api.query.QueryService; import org.labkey.api.query.SchemaKey; import org.labkey.api.query.SimpleSchemaTreeVisitor; import org.labkey.api.query.UserSchema; @@ -78,7 +81,7 @@ String listSchemas(ToolContext toolContext) @Tool(description = "Provide list of tables within the provided schema.") @RequiresPermission(ReadPermission.class) - String listTables(ToolContext toolContext, @ToolParam(description = "Fully qualified schema name as it would appear in SQL e.g. Study or \"Study.Datasets\"") String schemaName) + String listTables(ToolContext toolContext, @ToolParam(description = "Fully qualified schema name as it would appear in SQL e.g. Study or \"Study\".\"Datasets\"") String schemaName) { var json = _listTables(getContext(toolContext), schemaName); return json.toString(); @@ -100,7 +103,7 @@ String listColumns( @RequiresPermission(ReadPermission.class) String getSourceForSavedQuery( ToolContext toolContext, - @ToolParam(description = "Fully qualified schema name as it would appear in SQL e.g. Study or \"Study.Datasets\"") String schemaName, + @ToolParam(description = "Fully qualified schema name as it would appear in SQL e.g. Study or \"Study\".\"Datasets\"") String schemaName, @ToolParam(description = "Table or query name as it would appear in SQL e.g. MyTable, MyQuery, or \"MyTable\"") String queryName ) { @@ -111,6 +114,49 @@ String getSourceForSavedQuery( throw new NotFoundException("Could not find the source for " + schemaName + "." + queryName); } + @Tool(description = "Validate SQL syntax.") + @RequiresPermission(ReadPermission.class) + String validateSQL( + ToolContext toolContext, + @ToolParam(description = "Fully qualified schema name as it would appear in SQL e.g. Study or \"Study\".\"Datasets\"") String schemaName, + @ToolParam(description = "SQL source") String sql + ) + { + var context = getContext(toolContext); + + SchemaKey schemaKey = getSchemaKey(schemaName); + QuerySchema schema = DefaultSchema.get(context.getUser(), context.getContainer(), schemaKey); + + try + { + TableInfo ti = QueryService.get().createTable(schema, sql, null, true); + var warnings = ti.getWarnings(); + if (null != warnings) + { + var warning = warnings.stream().findFirst(); + if (warning.isPresent()) + throw warning.get(); + } +// CONSIDER: add back code to add database validate, but this seems to have stopped working +// if (ti.getSqlDialect().isPostgreSQL()) +// { +// var parameters = ti.getNamedParameters(); +// if (parameters.isEmpty()) +// { +// SQLFragment sqlPrepare = new SQLFragment("PREPARE validate AS SELECT * FROM ").append(ti.getFromSQL("MYVALIDATEQUERY__")); +// new SqlExecutor(ti.getSchema().getScope()).execute(sqlPrepare); +// } +// } + } + catch (Exception x) + { + // CONSIDER remove line line/character information from DB errors as they won't match the LabKey SQL + return "That SQL caused the " + (x instanceof QueryParseWarning ? "warning" : "error") + " below:\n```" + x.getMessage() + "```"; + } + return "success"; + } + + /* For now, list all schemas. CONSIDER support incremental querying. */ public static Map _listAllSchemas(DefaultSchema root) { @@ -309,7 +355,7 @@ static String normalizeIdentifier(String compoundIdentifier) return new SqlParser().parseIdentifier(compoundIdentifier).toSQLString(true).toLowerCase(); } - /** JSON schema example provided by GEMINI, using triple tick-marks to delimit the machine-readable structured data + /* JSON schema example provided by GEMINI, using triple tick-marks to delimit the machine-readable structured data * * Here is the database schema in JSON format: * ```{ From c1ae0b707be985bce1ffd6a40211b5dd2c152018 Mon Sep 17 00:00:00 2001 From: Trey Chadick Date: Mon, 13 Apr 2026 14:08:39 -0700 Subject: [PATCH 11/12] Troubleshoot MothershipTest failure (#7560) #### Changes - Add additional validation to `MothershipTest.testIgnoreExceptionFromDataRegion` --- .../src/org/labkey/test/tests/mothership/MothershipTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mothership/test/src/org/labkey/test/tests/mothership/MothershipTest.java b/mothership/test/src/org/labkey/test/tests/mothership/MothershipTest.java index 26ce1e22c4c..7bd1317d08b 100644 --- a/mothership/test/src/org/labkey/test/tests/mothership/MothershipTest.java +++ b/mothership/test/src/org/labkey/test/tests/mothership/MothershipTest.java @@ -43,6 +43,7 @@ import java.util.Arrays; import java.util.Date; import java.util.List; +import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -137,9 +138,13 @@ public void testIgnoreExceptionFromDataRegion() ShowExceptionsPage showExceptionsPage = ShowExceptionsPage.beginAt(this); ExceptionSummaryDataRegion exceptionSummary = showExceptionsPage.exceptionSummary(); + Supplier> getAssignedToAndIssue = () -> exceptionSummary.getRowDataAsText(exceptionSummary.getRowIndex(String.valueOf(stackTraceId)), "AssignedTo", "Issue"); + assertEquals("Exception %d should be unassigned", List.of(" ", " "), getAssignedToAndIssue.get()); + exceptionSummary.uncheckAllOnPage(); exceptionSummary.checkCheckboxByPrimaryKey(stackTraceId); exceptionSummary.ignoreSelected(); + assertEquals("Exception %d should be ignored (issue -1)", List.of(" ", "-1"), getAssignedToAndIssue.get()); StackTraceDetailsPage detailsPage = exceptionSummary.clickStackTrace(stackTraceId); assertEquals("Ignoring exception should set GitHub Issue", "-1", detailsPage.githubIssue().get()); From c11d7dc4038c52755cadea55db9d23c7d9e6575d Mon Sep 17 00:00:00 2001 From: Trey Chadick Date: Mon, 13 Apr 2026 14:09:32 -0700 Subject: [PATCH 12/12] Prevent memory leak after `ListManager.TestCase` (#7577) - Prevent memory leak after `ListManager.TestCase` - `null` out static variables before deleting test container so that container deletion errors can't cause other leaks --- list/src/org/labkey/list/model/ListManager.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/list/src/org/labkey/list/model/ListManager.java b/list/src/org/labkey/list/model/ListManager.java index 557eeae8e0a..9637309e819 100644 --- a/list/src/org/labkey/list/model/ListManager.java +++ b/list/src/org/labkey/list/model/ListManager.java @@ -1383,11 +1383,12 @@ private static int addListItem(Container scopedContainer, ListDefinition scopedL @AfterClass public static void cleanup() { - deleteTestContainer(); - + list = null; dp = null; c = null; u = null; + + deleteTestContainer(); } private static void deleteTestContainer()