From ff5f411de5e3006b49c5f32bb92621e0db1e10f8 Mon Sep 17 00:00:00 2001 From: XingY Date: Thu, 9 Apr 2026 19:11:13 -0700 Subject: [PATCH 1/3] GitHub Issue 1028: Can't delete a sample when any sample in the sample type has been linked to study --- .../study/publish/StudyPublishService.java | 2 +- .../study/assay/ExperimentListenerImpl.java | 51 ++++++++++++------- .../study/assay/StudyPublishManager.java | 7 ++- .../study/controllers/StudyController.java | 4 +- 4 files changed, 41 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..88408679f75 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; @@ -72,41 +75,55 @@ 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)) + Map> containerSamples = entry.getValue(); + + for (Map.Entry> containerEntry : containerSamples.entrySet()) { - throw new UnauthorizedException("Cannot delete rows from dataset " + dataset); - } + Container sampleContainer = containerEntry.getKey(); + List samples = containerEntry.getValue(); - UserSchema schema = QueryService.get().getUserSchema(user, dataset.getContainer(), "study"); - TableInfo tableInfo = schema.getTable(dataset.getName()); + // Need Read permission to check for linked samples + User userWithReadPerm = ElevatedUser.getElevatedUser(user, ReaderRole.class); - // Future optimization - query for all the materials at once - for (ExpMaterial material : entry.getValue()) - { - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(ExpMaterialTable.Column.RowId.toString()), material.getRowId()); - String lsid = new TableSelector(tableInfo, singleton("LSID"), filter, null).getObject(String.class); + UserSchema schemaWithReadPerm = QueryService.get().getUserSchema(userWithReadPerm, dataset.getContainer(), "study"); + TableInfo tableInfoForRead = schemaWithReadPerm.getTable(dataset.getName()); - if (lsid != null) + // 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); + List linkedLsids = new TableSelector(tableInfoForRead, singleton("LSID"), filter, null).getArrayList(String.class); + + 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); } + + List linkedIds = samples.stream().filter(s -> linkedLsids.contains(s.getLSID())).map(ExpMaterial::getRowId).toList(); + StudyPublishService.get().addRecallAuditEvent(sampleContainer, user, dataset, linkedLsids.size(), linkedIds); + 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 8aaf36c2fe8..e0899685b41 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -1520,7 +1520,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) @@ -1541,15 +1541,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 10e7fe687467e834e140e4026c0a79a12a8b6db2 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 10 Apr 2026 11:31:36 -0700 Subject: [PATCH 2/3] code review changes --- .../labkey/study/assay/ExperimentListenerImpl.java | 14 ++++++++------ .../labkey/study/assay/StudyPublishManager.java | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/study/src/org/labkey/study/assay/ExperimentListenerImpl.java b/study/src/org/labkey/study/assay/ExperimentListenerImpl.java index 88408679f75..89f69670c83 100644 --- a/study/src/org/labkey/study/assay/ExperimentListenerImpl.java +++ b/study/src/org/labkey/study/assay/ExperimentListenerImpl.java @@ -96,17 +96,19 @@ public void beforeMaterialDelete(List materials, Containe { Map> containerSamples = entry.getValue(); + // Need Read permission to check for linked samples + User userWithReadPerm = ElevatedUser.getElevatedUser(user, ReaderRole.class); + + 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); + for (Map.Entry> containerEntry : containerSamples.entrySet()) { Container sampleContainer = containerEntry.getKey(); List samples = containerEntry.getValue(); - // Need Read permission to check for linked samples - User userWithReadPerm = ElevatedUser.getElevatedUser(user, ReaderRole.class); - - UserSchema schemaWithReadPerm = QueryService.get().getUserSchema(userWithReadPerm, dataset.getContainer(), "study"); - TableInfo tableInfoForRead = schemaWithReadPerm.getTable(dataset.getName()); - // 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); diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index e0899685b41..841d722430c 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -1541,7 +1541,7 @@ 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 && rowIds != null && rowIds.isEmpty()) + if (sourceType == Dataset.PublishSource.SampleType && rowIds != null && !rowIds.isEmpty()) { var timelineEventType = SampleTimelineAuditEvent.SampleTimelineEventType.RECALL; Map eventMetadata = new HashMap<>(); From 19d77d3115989b9b7aeb5ed225dfbe7317c1d771 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 10 Apr 2026 16:01:16 -0700 Subject: [PATCH 3/3] Fix audit --- .../org/labkey/study/assay/ExperimentListenerImpl.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/study/src/org/labkey/study/assay/ExperimentListenerImpl.java b/study/src/org/labkey/study/assay/ExperimentListenerImpl.java index 89f69670c83..ec8c6e027b9 100644 --- a/study/src/org/labkey/study/assay/ExperimentListenerImpl.java +++ b/study/src/org/labkey/study/assay/ExperimentListenerImpl.java @@ -44,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; @@ -112,8 +113,10 @@ public void beforeMaterialDelete(List materials, Containe // 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); - List linkedLsids = new TableSelector(tableInfoForRead, singleton("LSID"), filter, null).getArrayList(String.class); + Map[] linkedLsidRowIds = new TableSelector(tableInfoForRead, Set.of("LSID", "RowId"), filter, null).getMapArray(); + 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; @@ -123,8 +126,7 @@ public void beforeMaterialDelete(List materials, Containe throw new UnauthorizedException("Cannot delete rows from dataset " + dataset); } - List linkedIds = samples.stream().filter(s -> linkedLsids.contains(s.getLSID())).map(ExpMaterial::getRowId).toList(); - StudyPublishService.get().addRecallAuditEvent(sampleContainer, user, dataset, linkedLsids.size(), linkedIds); + StudyPublishService.get().addRecallAuditEvent(sampleContainer, user, dataset, linkedLsids.size(), linkedRowIds); dataset.deleteDatasetRows(user, linkedLsids); } }