From 8068a006f9f29d38baa66f676fd3d3b35358e429 Mon Sep 17 00:00:00 2001 From: bbimber Date: Tue, 23 Jun 2026 15:58:32 -0700 Subject: [PATCH 1/4] Use UpdateService instead of raw DbSchema layer for assay_template edits --- .../laboratory/assay/DefaultAssayParser.java | 27 ++++----- .../labkey/laboratory/assay/AssayHelper.java | 57 ++++++++++++++++--- 2 files changed, 63 insertions(+), 21 deletions(-) diff --git a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java index bbcd2ac4..f4e27df3 100644 --- a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java +++ b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java @@ -20,18 +20,18 @@ import org.apache.commons.beanutils.ConversionException; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Level; -import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.poi.openxml4j.exceptions.InvalidFormatException; import org.json.JSONArray; import org.json.JSONObject; +import org.labkey.api.assay.AssayProvider; +import org.labkey.api.assay.AssayService; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerType; import org.labkey.api.data.ConvertHelper; -import org.labkey.api.data.DbSchema; -import org.labkey.api.data.RuntimeSQLException; import org.labkey.api.data.SimpleFilter; -import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; import org.labkey.api.exp.PropertyDescriptor; @@ -44,14 +44,14 @@ import org.labkey.api.laboratory.LaboratoryService; import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.FieldKey; +import org.labkey.api.query.QueryService; +import org.labkey.api.query.UserSchema; import org.labkey.api.query.ValidationException; import org.labkey.api.reader.ColumnDescriptor; import org.labkey.api.reader.ExcelFactory; import org.labkey.api.reader.Readers; import org.labkey.api.reader.TabLoader; import org.labkey.api.security.User; -import org.labkey.api.assay.AssayProvider; -import org.labkey.api.assay.AssayService; import org.labkey.api.util.FileType; import org.labkey.api.util.JsonUtil; import org.labkey.api.util.Pair; @@ -467,9 +467,10 @@ protected void saveTemplate(ViewContext ctx, int templateId, int runId) throws B try { //validate the template exists - TableInfo ti = DbSchema.get("laboratory").getTable("assay_run_templates"); + UserSchema us = QueryService.get().getUserSchema(ctx.getUser(), ctx.getContainer().getContainerFor(ContainerType.DataType.tabParent), "laboratory"); + TableInfo ti = us.getTable("assay_run_templates"); TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString("rowid"), templateId), null); - if (ts.getRowCount() == 0) + if (!ts.exists()) { throw new BatchValidationException(Collections.singletonList(new ValidationException("Unknown template: " + templateId)), null); } @@ -478,11 +479,11 @@ protected void saveTemplate(ViewContext ctx, int templateId, int runId) throws B row.put("runid", runId); row.put("status", "Complete"); - Table.update(ctx.getUser(), ti, row, templateId); + ti.getUpdateService().updateRows(ctx.getUser(), ctx.getContainer(), Arrays.asList(row), Arrays.asList(Map.of("rowId", templateId)), null, null); } - catch (RuntimeSQLException e) + catch (Exception e) { - throw new BatchValidationException(Collections.singletonList(new ValidationException(e.getSQLException().getMessage())), null); + throw new BatchValidationException(Collections.singletonList(new ValidationException(e.getMessage())), null); } } @@ -584,8 +585,8 @@ protected Map> getTemplateRowMap(ImportContext conte if (templateId == null) return ret; - TableInfo ti = DbSchema.get("laboratory").getTable("assay_run_templates"); - + UserSchema us = QueryService.get().getUserSchema(context.getViewContext().getUser(), context.getViewContext().getContainer().getContainerFor(ContainerType.DataType.tabParent), "laboratory"); + TableInfo ti = us.getTable("assay_run_templates"); TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString("rowid"), templateId), null); Map[] maps = ts.getMapArray(); if (maps.length == 0) diff --git a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java index ee4a9816..3076ea5c 100644 --- a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java +++ b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java @@ -32,9 +32,9 @@ import org.labkey.api.collections.CollectionUtils; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; -import org.labkey.api.data.RuntimeSQLException; +import org.labkey.api.data.ContainerType; +import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TSVMapWriter; -import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; import org.labkey.api.exp.ChangePropertyDescriptorException; @@ -51,8 +51,12 @@ import org.labkey.api.laboratory.assay.AssayDataProvider; import org.labkey.api.laboratory.assay.AssayImportMethod; import org.labkey.api.query.BatchValidationException; +import org.labkey.api.query.FieldKey; +import org.labkey.api.query.QueryService; +import org.labkey.api.query.UserSchema; import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; +import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.util.FileUtil; import org.labkey.api.util.Pair; import org.labkey.api.view.ActionURL; @@ -66,6 +70,7 @@ import java.io.File; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -125,27 +130,27 @@ public Map saveTemplate(User u, Container c, ExpProtocol protoco { validateTemplate(u, c, protocol, templateId, title, importMethod, json); - TableInfo ti = LaboratorySchema.getInstance().getSchema().getTable(LaboratorySchema.TABLE_ASSAY_RUN_TEMPLATES); + UserSchema us = QueryService.get().getUserSchema(u, c, "laboratory"); + TableInfo ti = us.getTable(LaboratorySchema.TABLE_ASSAY_RUN_TEMPLATES); Map row = new HashMap<>(); row.put("assayId", protocol.getRowId()); row.put("title", title); row.put("importMethod", importMethod); row.put("json", json.toString()); - row.put("container", c.getId()); if (templateId == null) { - row = Table.insert(u, ti, row); + ti.getUpdateService().insertRows(u, c, Arrays.asList(row), null, null, null); } else { row.put("rowid", templateId); - row = Table.update(u, ti, row, templateId); + ti.getUpdateService().updateRows(u, c, Arrays.asList(row), Arrays.asList(Map.of("rowId", templateId)), null, null); } return row; } - catch (RuntimeSQLException e) + catch (Exception e) { _log.error(e.getMessage(), e); errors.addRowError(new ValidationException(e.getMessage())); @@ -153,7 +158,7 @@ public Map saveTemplate(User u, Container c, ExpProtocol protoco } } - public void validateTemplate(User u, Container c, ExpProtocol protocol, Integer templateId, String title, String importMethod, JSONObject json) throws BatchValidationException + public void validateTemplate(User u, Container c, ExpProtocol protocol, @Nullable Integer templateId, String title, String importMethod, JSONObject json) throws BatchValidationException { BatchValidationException errors = new BatchValidationException(); @@ -178,6 +183,42 @@ public void validateTemplate(User u, Container c, ExpProtocol protocol, Integer throw errors; } + // Verify if this template exists and permissions: + if (templateId != null) + { + UserSchema us = QueryService.get().getUserSchema(u, c.getContainerFor(ContainerType.DataType.tabParent), "laboratory"); + TableInfo ti = us.getTable("assay_run_templates"); + TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString("rowId"), templateId), null); + if (ts.exists()) + { + Container rowContainer = ContainerManager.getForId(ts.getObject(String.class)); + if (rowContainer == null) + { + errors.addRowError(new ValidationException("Unable to determine the container for template: " + templateId)); + throw errors; + } + + if (!rowContainer.hasPermission("AssayHelper.validateTemplate()", u, UpdatePermission.class)) + { + errors.addRowError(new ValidationException("The current user does not have permission to edit template: " + templateId)); + throw errors; + } + } + else + { + errors.addRowError(new ValidationException("Unable to find template with ID: " + templateId)); + throw errors; + } + } + else + { + if (!c.hasPermission("AssayHelper.validateTemplate()", u, UpdatePermission.class)) + { + errors.addRowError(new ValidationException("The current user does not have permission to creates templates in folder: " + c.getName())); + throw errors; + } + } + method.validateTemplate(u, c, protocol, templateId, title, json, errors); if (errors.hasErrors()) From 9a96634db3e8ded48592eb15578b750753cc3ccd Mon Sep 17 00:00:00 2001 From: bbimber Date: Wed, 24 Jun 2026 08:10:29 -0700 Subject: [PATCH 2/4] Code review --- .../laboratory/assay/DefaultAssayParser.java | 13 +++++++++-- .../labkey/laboratory/assay/AssayHelper.java | 22 ++++++++++++++----- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java index f4e27df3..aef4b1b0 100644 --- a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java +++ b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java @@ -466,8 +466,13 @@ protected void saveTemplate(ViewContext ctx, int templateId, int runId) throws B { try { - //validate the template exists - UserSchema us = QueryService.get().getUserSchema(ctx.getUser(), ctx.getContainer().getContainerFor(ContainerType.DataType.tabParent), "laboratory"); + //validate the template exists. Note: this should always act against the current container, even if the container is a workbook (e.g., it should not allow cross-workbook actions) + UserSchema us = QueryService.get().getUserSchema(ctx.getUser(), ctx.getContainer(), "laboratory"); + if (us == null) + { + throw new IllegalStateException("The laboratory schema is not available in container: " + ctx.getContainer().getPath()); + } + TableInfo ti = us.getTable("assay_run_templates"); TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString("rowid"), templateId), null); if (!ts.exists()) @@ -586,6 +591,10 @@ protected Map> getTemplateRowMap(ImportContext conte return ret; UserSchema us = QueryService.get().getUserSchema(context.getViewContext().getUser(), context.getViewContext().getContainer().getContainerFor(ContainerType.DataType.tabParent), "laboratory"); + if (us == null) + { + throw new IllegalStateException("Could not find the laboratory schema in container: " + context.getViewContext().getContainer().getContainerFor(ContainerType.DataType.tabParent).getPath()); + } TableInfo ti = us.getTable("assay_run_templates"); TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString("rowid"), templateId), null); Map[] maps = ts.getMapArray(); diff --git a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java index 3076ea5c..b83e353e 100644 --- a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java +++ b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java @@ -58,6 +58,7 @@ import org.labkey.api.security.User; import org.labkey.api.security.permissions.UpdatePermission; import org.labkey.api.util.FileUtil; +import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.Pair; import org.labkey.api.view.ActionURL; import org.labkey.api.view.ViewContext; @@ -131,6 +132,11 @@ public Map saveTemplate(User u, Container c, ExpProtocol protoco validateTemplate(u, c, protocol, templateId, title, importMethod, json); UserSchema us = QueryService.get().getUserSchema(u, c, "laboratory"); + if (us == null) + { + throw new IllegalStateException("Could not find the laboratory schema in container: " + c.getPath()); + } + TableInfo ti = us.getTable(LaboratorySchema.TABLE_ASSAY_RUN_TEMPLATES); Map row = new HashMap<>(); row.put("assayId", protocol.getRowId()); @@ -140,15 +146,21 @@ public Map saveTemplate(User u, Container c, ExpProtocol protoco if (templateId == null) { - ti.getUpdateService().insertRows(u, c, Arrays.asList(row), null, null, null); + BatchValidationException bve = new BatchValidationException(); + List> rows = ti.getUpdateService().insertRows(u, c, Arrays.asList(row), bve, null, null); + if (bve.hasErrors()) + { + throw bve; + } + + return rows.getFirst(); } else { row.put("rowid", templateId); - ti.getUpdateService().updateRows(u, c, Arrays.asList(row), Arrays.asList(Map.of("rowId", templateId)), null, null); + List> rows = ti.getUpdateService().updateRows(u, c, Arrays.asList(row), Arrays.asList(Map.of("rowId", templateId)), null, null); + return rows.getFirst(); } - - return row; } catch (Exception e) { @@ -188,7 +200,7 @@ public void validateTemplate(User u, Container c, ExpProtocol protocol, @Nullabl { UserSchema us = QueryService.get().getUserSchema(u, c.getContainerFor(ContainerType.DataType.tabParent), "laboratory"); TableInfo ti = us.getTable("assay_run_templates"); - TableSelector ts = new TableSelector(ti, new SimpleFilter(FieldKey.fromString("rowId"), templateId), null); + TableSelector ts = new TableSelector(ti, PageFlowUtil.set("container"), new SimpleFilter(FieldKey.fromString("rowId"), templateId), null); if (ts.exists()) { Container rowContainer = ContainerManager.getForId(ts.getObject(String.class)); From d8e37ffe1e72eeb18dd37e96a445a3504aebb1b0 Mon Sep 17 00:00:00 2001 From: bbimber Date: Wed, 24 Jun 2026 09:25:03 -0700 Subject: [PATCH 3/4] Tighten behavior around cross-container actions --- .../api/laboratory/assay/DefaultAssayParser.java | 13 +++++++++++++ .../org/labkey/laboratory/assay/AssayHelper.java | 9 ++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java index aef4b1b0..6093d98b 100644 --- a/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java +++ b/laboratory/api-src/org/labkey/api/laboratory/assay/DefaultAssayParser.java @@ -29,6 +29,7 @@ import org.labkey.api.assay.AssayService; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; import org.labkey.api.data.ContainerType; import org.labkey.api.data.ConvertHelper; import org.labkey.api.data.SimpleFilter; @@ -605,6 +606,18 @@ protected Map> getTemplateRowMap(ImportContext conte Map map = maps[0]; JSONObject templateJson = new JSONObject((String)map.get("json")); + + // This enforces that the request and existing record are from the same container, including for workbook/parents: + Container rowContainer = ContainerManager.getForId(String.valueOf(map.get("container"))); + if (rowContainer == null) + { + throw new IllegalStateException("Unable to determine the container for template: " + templateId); + } + else if (!rowContainer.equals(context.getViewContext().getContainer())) + { + throw new IllegalStateException("Template is from the wrong container: " + templateId); + } + JSONArray rows = templateJson.getJSONArray("ResultRows"); for (JSONObject row : JsonUtil.toJSONObjectList(rows)) { diff --git a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java index b83e353e..47379b29 100644 --- a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java +++ b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java @@ -195,9 +195,10 @@ public void validateTemplate(User u, Container c, ExpProtocol protocol, @Nullabl throw errors; } - // Verify if this template exists and permissions: + // Verify if this template exists and permissions. This expects any existing row to be present in the current container: if (templateId != null) { + // This queries the current container+workbooks to identify the existence of rows in other reasonable containers: UserSchema us = QueryService.get().getUserSchema(u, c.getContainerFor(ContainerType.DataType.tabParent), "laboratory"); TableInfo ti = us.getTable("assay_run_templates"); TableSelector ts = new TableSelector(ti, PageFlowUtil.set("container"), new SimpleFilter(FieldKey.fromString("rowId"), templateId), null); @@ -215,6 +216,12 @@ public void validateTemplate(User u, Container c, ExpProtocol protocol, @Nullabl errors.addRowError(new ValidationException("The current user does not have permission to edit template: " + templateId)); throw errors; } + + if (!c.equals(rowContainer)) + { + errors.addRowError(new ValidationException("Template " + templateId + " is not from this folder")); + throw errors; + } } else { From e448eaf91db00af47c7892cb28c5975e5ecc6c30 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Wed, 24 Jun 2026 12:06:51 -0600 Subject: [PATCH 4/4] Use List.get(0) instead of getFirst() so this compiles on Java 17 This module sets sourceCompatibility=17, but List.getFirst() was added in Java 21 (SequencedCollection), so it failed to resolve. Replaced both calls with List.get(0). --- laboratory/src/org/labkey/laboratory/assay/AssayHelper.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java index 47379b29..72c4671a 100644 --- a/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java +++ b/laboratory/src/org/labkey/laboratory/assay/AssayHelper.java @@ -153,13 +153,13 @@ public Map saveTemplate(User u, Container c, ExpProtocol protoco throw bve; } - return rows.getFirst(); + return rows.get(0); } else { row.put("rowid", templateId); List> rows = ti.getUpdateService().updateRows(u, c, Arrays.asList(row), Arrays.asList(Map.of("rowId", templateId)), null, null); - return rows.getFirst(); + return rows.get(0); } } catch (Exception e)