Delete large number of job samples takes a long time and triggers large-collection warning

api/src/org/labkey/api/exp/api/ExperimentService.java

@@ -100,6 +100,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.locks.Lock;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 
@@ -797,6 +798,16 @@ static void validateParentAlias(Map<String, String> aliasMap, Set<String> reserv
     List<? extends ExpData> getAllExpDataByURL(File f, @Nullable Container c);
     List<? extends ExpData> getAllExpDataByURL(Path p, @Nullable Container c);
 
+    /** The per-sample facts an operation confirmation or a status pre-check needs, read straight off exp.material. */
+    record SampleOperationRow(long rowId, Container container, Long sampleState, String cpasType) {}
+
+    /**
+     * Streams the facts an operation confirmation needs about each sample. A confirmation can cover tens of thousands
+     * of samples at once, so nothing is collected here and nothing wider than these four columns is read; callers fold
+     * the rows as they arrive.
+     */
+    void forEachSampleOperationRow(Collection<Long> rowIds, Consumer<SampleOperationRow> block);
+
     TableInfo getTinfoMaterial();
 
     TableInfo getTinfoMaterialAncestors();

api/src/org/labkey/api/exp/api/SampleTypeService.java

@@ -267,6 +267,9 @@ default Map<String, Long> incrementSampleCounts(@Nullable Date counterDate)
 
     String getOperationNotPermittedMessage(Collection<? extends ExpMaterial> samples, SampleOperations operation);
 
+    /** @param totalCount how many samples are blocked, which exceeds the sample list when the caller caps what it collects */
+    String getOperationNotPermittedMessage(Collection<? extends ExpMaterial> samples, int totalCount, SampleOperations operation);
+
     int recomputeSampleTypeRollup(ExpSampleType sampleType, Container container) throws SQLException;
 
     int recomputeSamplesRollup(Collection<Long> sampleIds, String sampleTypeMetricUnit, Container container) throws IllegalStateException, SQLException;

experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java

@@ -133,14 +133,7 @@ public ActionURL detailsURL(Container container, boolean checkForOverride)
     @Override
     public @Nullable ExpSampleTypeImpl getSampleType()
     {
-        String type = _object.getCpasType();
-        if (!ExpMaterial.DEFAULT_CPAS_TYPE.equals(type) && !"Sample".equals(type))
-        {
-            // try current container first (uses cache)
-            return SampleTypeServiceImpl.get().getSampleTypeByType(type, getContainer());
-        }
-
-        return null;
+        return ExperimentServiceImpl.get().getSampleTypeForCpasType(_object.getCpasType(), getContainer());
     }
 
     @Override

experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java

@@ -300,6 +300,7 @@
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
@@ -1052,6 +1053,26 @@ public List<ExpMaterialImpl> getExpMaterials(Collection<Long> rowIds)
         return getExpMaterials(new SimpleFilter().addInClause(FieldKey.fromParts(ExpMaterialTable.Column.RowId.name()), rowIds));
     }
 
+    @Override
+    public void forEachSampleOperationRow(Collection<Long> rowIds, Consumer<SampleOperationRow> block)
+    {
+        if (rowIds.isEmpty())
+            return;
+
+        SimpleFilter filter = new SimpleFilter().addInClause(FieldKey.fromParts(ExpMaterialTable.Column.RowId.name()), rowIds);
+        Set<String> columns = Set.of("RowId", "Container", "SampleState", "CpasType");
+        new TableSelector(getTinfoMaterial(), columns, filter, null).forEach(Material.class,
+                m -> block.accept(new SampleOperationRow(m.getRowId(), m.getContainer(), m.getSampleState(), m.getCpasType())));
+    }
+
+    public @Nullable ExpSampleTypeImpl getSampleTypeForCpasType(@Nullable String cpasType, Container container)
+    {
+        if (cpasType == null || ExpMaterial.DEFAULT_CPAS_TYPE.equals(cpasType) || "Sample".equals(cpasType))
+            return null;
+
+        return SampleTypeServiceImpl.get().getSampleTypeByType(cpasType, container);
+    }
+
     @Override
     @NotNull
     public List<ExpMaterialImpl> getExpMaterialsByLsid(Collection<String> lsids)
@@ -4808,7 +4829,7 @@ private void deleteProtocolInputs(@NotNull Protocol protocol, Collection<? exten
 
     public static Map<String, Collection<Map<String, Object>>> partitionRequestedOperationObjects(User user, Collection<Long> requestIds, Collection<Long> notAllowedIds, List<? extends ExpRunItem> allData)
     {
-        List<Long> allowedIds = new LongArrayList(requestIds);
+        Set<Long> allowedIds = new HashSet<>(requestIds);
         allowedIds.removeAll(notAllowedIds);
         List<Map<String, Object>> allowedRows = new ArrayList<>();
         List<Map<String, Object>> notAllowedRows = new ArrayList<>();
@@ -4833,13 +4854,12 @@ public static Map<String, Collection<Map<String, Object>>> partitionRequestedOpe
      * For set of rows selected for deletion, find get all linked-to-study datasets that will be affected by the delete
      * and warn user
      */
-    public static ArrayList<Map<String, Object>> includeLinkedToStudyText(List<? extends ExpMaterial> allMaterials, Set<Long> deletable, User user, Container container)
+    public static ArrayList<Map<String, Object>> includeLinkedToStudyText(@Nullable ExpSampleType sampleType, Set<Long> deletable, User user, Container container)
     {
         ArrayList<Map<String, Object>> associatedDatasets = new ArrayList<>();
         StudyPublishService studyPublishService = StudyPublishService.get();
-        if (studyPublishService != null && !allMaterials.isEmpty())
+        if (studyPublishService != null && sampleType != null)
         {
-            ExpSampleType sampleType = allMaterials.getFirst().getSampleType();
             UserSchema userSchema = QueryService.get().getUserSchema(user, container, SamplesSchema.SCHEMA_NAME);
             TableInfo tableInfo = userSchema.getTable(sampleType.getName());
             if (tableInfo == null)

experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java

@@ -1479,18 +1479,25 @@ public Collection<? extends ExpMaterial> getSamplesNotPermitted(Collection<? ext
 
     @Override
     public String getOperationNotPermittedMessage(Collection<? extends ExpMaterial> samples, SampleOperations operation)
+    {
+        return getOperationNotPermittedMessage(samples, samples.size(), operation);
+    }
+
+    @Override
+    public String getOperationNotPermittedMessage(Collection<? extends ExpMaterial> samples, int totalCount, SampleOperations operation)
     {
         String message;
-        if (samples.size() == 1)
+        if (totalCount == 1 && samples.size() == 1)
         {
             ExpMaterial sample = samples.iterator().next();
             message = "Sample " + sample.getName() + " has status " + sample.getStateLabel() + ", which prevents";
         }
         else
         {
-            message = samples.size() + " samples (";
-            message += samples.stream().limit(10).map(ExpMaterial::getNameAndStatus).collect(Collectors.joining(", "));
-            if (samples.size() > 10)
+            int named = Math.min(samples.size(), 10);
+            message = totalCount + " samples (";
+            message += samples.stream().limit(named).map(ExpMaterial::getNameAndStatus).collect(Collectors.joining(", "));
+            if (totalCount > named)
                 message += " ...";
             message += ") have statuses that prevent";
         }

experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java

@@ -340,6 +340,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.TreeSet;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -3786,30 +3787,52 @@ public Object execute(MaterialOperationConfirmationForm form, BindException erro
         {
             Set<Long> requestIds = form.getIds(false);
             ExperimentServiceImpl service = ExperimentServiceImpl.get();
-            List<? extends ExpMaterial> allMaterials = service.getExpMaterials(requestIds);
+            User user = getUser();
+            SampleTypeService.SampleOperations operation = form.getSampleOperation();
+            Class<? extends Permission> permClass = operation.getPermissionClass();
+            boolean statusSupported = SampleStatusService.get().supportsSampleStatus();
 
             Set<Long> notAllowedIds = new HashSet<>();
             // We prevent deletion if a sample is used as a parent, has assay data, is used in a job, etc.
-            if (form.getSampleOperation() == SampleTypeService.SampleOperations.Delete)
+            if (operation == SampleTypeService.SampleOperations.Delete)
                 service.getObjectReferencers().forEach(referencer ->
                         notAllowedIds.addAll(referencer.getItemsWithReferences(requestIds, "samples")));
 
-            if (SampleStatusService.get().supportsSampleStatus())
-                notAllowedIds.addAll(service.findIdsNotPermittedForOperation(allMaterials, form.getSampleOperation()));
-
-            Map<String, Collection<Map<String, Object>>> response = ExperimentServiceImpl.partitionRequestedOperationObjects(getUser(), requestIds, notAllowedIds, allMaterials);
-
             Collection<Container> containers = new HashSet<>();
             Collection<Long> notPermittedIds = new ArrayList<>();
-            Class<? extends Permission> permClass = form.getSampleOperation().getPermissionClass();
-            for (ExpMaterial material : allMaterials)
-            {
-                Container c = material.getContainer();
-                if (c.hasPermission(getUser(), ReadPermission.class))
+            List<Map<String, Object>> rows = new ArrayList<>();
+            AtomicReference<ExpSampleType> sampleType = new AtomicReference<>();
+
+            service.forEachSampleOperationRow(requestIds, row -> {
+                Container c = row.container();
+                boolean canRead = c.hasPermission(user, ReadPermission.class);
+                if (canRead)
                     containers.add(c);
-                if (permClass != null && !c.hasPermission(getUser(), permClass))
-                    notPermittedIds.add(material.getRowId());
-            }
+                if (permClass != null && !c.hasPermission(user, permClass))
+                    notPermittedIds.add(row.rowId());
+                if (statusSupported && !SampleStatusService.get().isOperationPermitted(c, row.sampleState(), operation))
+                    notAllowedIds.add(row.rowId());
+                if (rows.isEmpty())
+                    sampleType.set(service.getSampleTypeForCpasType(row.cpasType(), c));
+
+                Map<String, Object> rowMap = new HashMap<>();
+                rowMap.put("RowId", row.rowId());
+                if (canRead)
+                    rowMap.put("ContainerPath", c.getPath());
+                rows.add(rowMap);
+            });
+
+            // the status check above feeds notAllowedIds, so the split waits until every row has been seen
+            Set<Long> allowedIds = new HashSet<>(requestIds);
+            allowedIds.removeAll(notAllowedIds);
+            List<Map<String, Object>> allowedRows = new ArrayList<>();
+            List<Map<String, Object>> notAllowedRows = new ArrayList<>();
+            for (Map<String, Object> rowMap : rows)
+                (allowedIds.contains((Long) rowMap.get("RowId")) ? allowedRows : notAllowedRows).add(rowMap);
+
+            Map<String, Collection<Map<String, Object>>> response = new HashMap<>();
+            response.put("allowed", allowedRows);
+            response.put("notAllowed", notAllowedRows);
 
             NameExpressionOptionService svc = NameExpressionOptionService.get();
 
@@ -3824,7 +3847,7 @@ public Object execute(MaterialOperationConfirmationForm form, BindException erro
 
             if (form.getSampleOperation() == SampleTypeService.SampleOperations.Delete)
                 // String 'associatedDatasets' must be synced to its handling in confirmDelete.js, confirmDelete()
-                response.put("associatedDatasets", ExperimentServiceImpl.includeLinkedToStudyText(allMaterials, requestIds, getUser(), getContainer()));
+                response.put("associatedDatasets", ExperimentServiceImpl.includeLinkedToStudyText(sampleType.get(), requestIds, getUser(), getContainer()));
 
             return success(response);
         }