From 397942ab9eb4ef9d7a21d0ea28d851af1f9bed36 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 9 Jul 2025 16:32:48 -0700 Subject: [PATCH 01/11] Issue 52504 and 52886: WIP to see if changing where we add the container filter for lookups is viable --- .../org/labkey/experiment/ExpDataIterators.java | 6 ------ .../experiment/api/ExpDataClassDataTableImpl.java | 8 ++++++++ .../experiment/api/ExpMaterialTableImpl.java | 12 +++++++++++- .../controllers/exp/ExperimentController.java | 14 ++++++++++++-- .../labkey/query/controllers/QueryController.java | 2 -- 5 files changed, 31 insertions(+), 11 deletions(-) diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index 1bdb4385312..013a34f6285 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -2587,8 +2587,6 @@ private int _importPartition(TypeData typeData) Container splitContainer = ContainerManager.getForRowId(containerSplitFile.getKey()); AbstractExpSchema schema = _isSamples ? new SamplesSchema(_user, splitContainer) : new DataClassUserSchema(splitContainer, _user); QueryDefinition qDef = schema.getQueryDefForTable(typeData.dataType.getName()); - // Issue 52504: For lookup validation, we need to use the proper lookup container filter on the table - qDef.setContainerFilter(QueryService.get().getContainerFilterForLookups(splitContainer, _user)); TableInfo dataTable = qDef.getTable(schema, new ArrayList<>(), true); if (dataTable == null) @@ -2843,8 +2841,6 @@ private TypeData createDataClassHeaderRow(ExpDataClass dataClass, Container cont List qpe = new ArrayList<>(); DataClassUserSchema schema = new DataClassUserSchema(container, _user); QueryDefinition qDef = schema.getQueryDefForTable(dataClass.getName()); - // Issue 52504: For lookup validation, we need to use the proper lookup container filter on the table - qDef.setContainerFilter(QueryService.get().getContainerFilterForLookups(container, _user)); TableInfo dataTable = qDef.getTable(schema, qpe, true); if (dataTable == null) { @@ -2874,8 +2870,6 @@ private TypeData createSampleHeaderRow(ExpSampleTypeImpl sampleType, Container c List qpe = new ArrayList<>(); SamplesSchema schema = new SamplesSchema(_user, container); QueryDefinition qDef = schema.getQueryDefForTable(sampleType.getName()); - // Issue 52504: For lookup validation, we need to use the proper lookup container filter on the table - qDef.setContainerFilter(QueryService.get().getContainerFilterForLookups(container, _user)); TableInfo samplesTable = qDef.getTable(schema, qpe, true); if (samplesTable == null) { diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 90408498d8b..7ef5ff60b89 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -455,6 +455,14 @@ protected void populateColumns() PropertyDescriptor pd = (null == dp) ? null : dp.getPropertyDescriptor(); if (dp != null && pd != null) { + // Issue 52504: For lookup validation, we need to use the proper lookup container filter on the table + if (pd.isLookup()) + { + var fk = QueryForeignKey.from(this.getUserSchema(), QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser())) + .schema(ExpSchema.SCHEMA_NAME, getContainer()) + .to(pd.getLookup().getQueryName(), null, null); + wrapped.setFk(fk); + } defaultsSupplier = PropertyColumn.copyAttributes(_userSchema.getUser(), wrapped, dp, getContainer(), lsidFieldKey, getContainerFilter(), defaultsSupplier); wrapped.setFieldKey(FieldKey.fromParts(dp.getName())); diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index 8e80573a917..b903894b501 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -971,9 +971,18 @@ private void addSampleTypeColumns(ExpSampleType st, List visibleColumn continue; } + var wrapped = wrapColumnFromJoinedTable(dbColumn.getName(), dbColumn); + if (dbColumn.isLookup()) + { + var fk = QueryForeignKey.from(this.getUserSchema(), QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser())) + .schema(ExpSchema.SCHEMA_NAME, getContainer()) + .to(dbColumn.getFk().getLookupTableName(), dbColumn.getFk().getLookupColumnName(), null); + wrapped.setFk(fk); + } + // TODO missing values? comments? flags? DomainProperty dp = domain.getPropertyByURI(dbColumn.getPropertyURI()); - var propColumn = copyColumnFromJoinedTable(null==dp?dbColumn.getName():dp.getName(), dbColumn); + var propColumn = copyColumnFromJoinedTable(null==dp?dbColumn.getName():dp.getName(), wrapped); if (propColumn.getName().equalsIgnoreCase("genid")) { propColumn.setHidden(true); @@ -1020,6 +1029,7 @@ private void addSampleTypeColumns(ExpSampleType st, List visibleColumn if (!mvColumns.contains(propColumn.getFieldKey())) addColumn(propColumn); + } setDefaultVisibleColumns(visibleColumns); diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index 8702a5e0d69..c5951ea516c 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -4528,8 +4528,18 @@ public void validateForm(QueryForm form, Errors errors) protected void initRequest(QueryForm form) throws ServletException { QueryDefinition query = form.getQueryDef(); - // Issue 52504: For lookup validation, we need to use the proper lookup container filter on the table - query.setContainerFilter(QueryService.get().getContainerFilterForLookups(getContainer(), getUser())); + if (getContainer().isProductFoldersEnabled()) + { + ContainerFilter cf; + // Note that this is slightly different from our treatment of lookups: + // - when in a project, we allow import or update to all subfolders, + // - when in a folder, we only allow references to data up the folder tree + if (getContainer().isProject()) + cf = new ContainerFilter.AllInProjectPlusShared(getContainer(), getUser()); + else + cf = new ContainerFilter.CurrentPlusProjectAndShared(getContainer(), getUser()); + query.setContainerFilter(cf); + } List qpe = new ArrayList<>(); TableInfo t = query.getTable(form.getSchema(), qpe, true); diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index 99c196a0501..d8249bebac9 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -4122,8 +4122,6 @@ protected void initRequest(QueryForm form) throws ServletException _insertOption = form.getInsertOption(); QueryDefinition query = form.getQueryDef(); - // Issue 52504: For lookup validation, we need to use the proper lookup container filter on the table - query.setContainerFilter(QueryService.get().getContainerFilterForLookups(getContainer(), getUser())); List qpe = new ArrayList<>(); TableInfo t = query.getTable(form.getSchema(), qpe, true); if (!qpe.isEmpty()) From d1908d42935f17c15e6497f50e4e8f7c61b490fc Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Tue, 15 Jul 2025 13:57:37 -0700 Subject: [PATCH 02/11] For exp.data table, update _select to use user schema filtered query instead of db query (again) and set proper container filter when copying properties for a lookup instead of setting for the full query table. --- .../org/labkey/api/data/FieldKeyRowMap.java | 2 +- .../api/query/AbstractQueryUpdateService.java | 2 +- .../org/labkey/assay/TSVProtocolSchema.java | 4 +- .../labkey/assay/plate/query/WellTable.java | 4 +- .../api/ExpDataClassDataTableImpl.java | 86 +++++++++++-------- .../api/ExpDataClassDataTestCase.jsp | 2 +- .../experiment/api/ExpMaterialTableImpl.java | 2 +- list/src/org/labkey/list/model/ListTable.java | 3 +- 8 files changed, 63 insertions(+), 42 deletions(-) diff --git a/api/src/org/labkey/api/data/FieldKeyRowMap.java b/api/src/org/labkey/api/data/FieldKeyRowMap.java index b315ab1f824..24e596af970 100644 --- a/api/src/org/labkey/api/data/FieldKeyRowMap.java +++ b/api/src/org/labkey/api/data/FieldKeyRowMap.java @@ -27,7 +27,7 @@ import java.util.Map; import java.util.Set; -class FieldKeyRowMap implements Map +public class FieldKeyRowMap implements Map { private final Results _results; diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 3dbaa08b6cb..0c8fbc655ba 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -203,7 +203,7 @@ public Map> getExistingRows(User user, Container co Map keyValues = key.getValue(); Map row = getRow(user, container, keyValues, verifyNoCrossFolderData); boolean hasValidExisting = false; - if (row != null) + if (row != null && !row.isEmpty()) { result.put(key.getKey(), row); if (verifyNoCrossFolderData) diff --git a/assay/src/org/labkey/assay/TSVProtocolSchema.java b/assay/src/org/labkey/assay/TSVProtocolSchema.java index 936575e61ea..b9077569dfc 100644 --- a/assay/src/org/labkey/assay/TSVProtocolSchema.java +++ b/assay/src/org/labkey/assay/TSVProtocolSchema.java @@ -47,6 +47,7 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.query.FilteredTable; import org.labkey.api.query.QueryForeignKey; +import org.labkey.api.query.QueryService; import org.labkey.api.query.QueryUpdateService; import org.labkey.api.security.User; import org.labkey.api.security.UserPrincipal; @@ -285,7 +286,8 @@ public _AssayPlateReplicateStatsTable(@NotNull Domain domain, @NotNull AssayProt PropertyDescriptor pd = dp.getPropertyDescriptor(); if (pd != null) { - defaultsSupplier = PropertyColumn.copyAttributes(userSchema.getUser(), columnInfo, dp, getContainer(), null, containerFilter, defaultsSupplier); + var cf = pd.isLookup() ? QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser()) : containerFilter; + defaultsSupplier = PropertyColumn.copyAttributes(userSchema.getUser(), columnInfo, dp, getContainer(), null, cf, defaultsSupplier); columnInfo.setFieldKey(FieldKey.fromParts(dp.getName())); } } diff --git a/assay/src/org/labkey/assay/plate/query/WellTable.java b/assay/src/org/labkey/assay/plate/query/WellTable.java index 92a277c303a..a7035a13ad7 100644 --- a/assay/src/org/labkey/assay/plate/query/WellTable.java +++ b/assay/src/org/labkey/assay/plate/query/WellTable.java @@ -39,6 +39,7 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.query.InvalidKeyException; import org.labkey.api.query.QueryForeignKey; +import org.labkey.api.query.QueryService; import org.labkey.api.query.QueryUpdateService; import org.labkey.api.query.QueryUpdateServiceException; import org.labkey.api.query.SimpleUserSchema; @@ -250,7 +251,8 @@ private void addWellMetadataColumns() PropertyDescriptor pd = dp.getPropertyDescriptor(); if (pd != null) { - defaultsSupplier = PropertyColumn.copyAttributes(getUserSchema().getUser(), wrapped, dp, getContainer(), lsidFieldKey, getContainerFilter(), defaultsSupplier); + var cf = pd.isLookup() ? QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser()) : getContainerFilter(); + defaultsSupplier = PropertyColumn.copyAttributes(getUserSchema().getUser(), wrapped, dp, getContainer(), lsidFieldKey, cf, defaultsSupplier); wrapped.setFieldKey(FieldKey.fromParts(dp.getName())); } } diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 5d6e504c100..bf75b57851a 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -40,6 +40,7 @@ import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DbScope; +import org.labkey.api.data.FieldKeyRowMap; import org.labkey.api.data.JdbcType; import org.labkey.api.data.MutableColumnInfo; import org.labkey.api.data.PHI; @@ -456,14 +457,8 @@ protected void populateColumns() if (dp != null && pd != null) { // Issue 52504: For lookup validation, we need to use the proper lookup container filter on the table - if (pd.isLookup()) - { - var fk = QueryForeignKey.from(this.getUserSchema(), QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser())) - .schema(ExpSchema.SCHEMA_NAME, getContainer()) - .to(pd.getLookup().getQueryName(), null, null); - wrapped.setFk(fk); - } - defaultsSupplier = PropertyColumn.copyAttributes(_userSchema.getUser(), wrapped, dp, getContainer(), lsidFieldKey, getContainerFilter(), defaultsSupplier); + ContainerFilter cf = pd.isLookup() ? QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser()) : getContainerFilter(); + defaultsSupplier = PropertyColumn.copyAttributes(_userSchema.getUser(), wrapped, dp, getContainer(), lsidFieldKey, cf, defaultsSupplier); wrapped.setFieldKey(FieldKey.fromParts(dp.getName())); if (pd.getPropertyType() == PropertyType.ATTACHMENT) @@ -1249,14 +1244,14 @@ public List> insertRows(User user, Container container, List } @Override - protected Map getRow(User user, Container container, Map keys) throws InvalidKeyException + protected Map getRow(User user, Container container, Map keys) throws InvalidKeyException, SQLException { return getRow(user, container, keys, false); } /* This class overrides getRow() in order to support getRow() using "rowid" or "lsid" */ @Override - protected Map getRow(User user, Container container, Map keys, boolean allowCrossContainer) throws InvalidKeyException + protected Map getRow(User user, Container container, Map keys, boolean allowCrossContainer) throws InvalidKeyException, SQLException { aliasColumns(_columnMapping, keys); @@ -1273,15 +1268,15 @@ protected Map getRow(User user, Container container, Map row = _select(container, rowId, lsid, name, classId, allowCrossContainer); - //PostgreSQL includes a column named _row for the row index, but since this is selecting by - //primary key, it will always be 1, which is not only unnecessary, but confusing, so strip it - if (null != row) - { - if (row instanceof ArrayListMap arrayListMap) - arrayListMap.getFindMap().remove("_row"); - else - row.remove("_row"); - } +// //PostgreSQL includes a column named _row for the row index, but since this is selecting by +// //primary key, it will always be 1, which is not only unnecessary, but confusing, so strip it +// if (null != row) +// { +// if (row instanceof ArrayListMap arrayListMap) +// arrayListMap.getFindMap().remove("_row"); +// else +// row.remove("_row"); +// } return row; } @@ -1292,32 +1287,53 @@ protected Map _select(Container container, Object[] keys) throws throw new IllegalStateException(); } - protected Map _select(Container container, Integer rowid, String lsid, String name, Integer classId, boolean allowCrossContainer) throws ConversionException + protected Map _select(Container container, Integer rowid, String lsid, String name, Integer classId, boolean allowCrossContainer) throws SQLException { if (null == rowid && null == lsid && (null == name || null == classId)) return null; // FIXME Issue 52886: This retrieves raw db column names, which doesn't work well for comparing existing and new audit records if the name doesn't match the field key - TableInfo d = getDbTable(); - TableInfo t = _dataClassDataTableSupplier.get(); - - SQLFragment sql = new SQLFragment() - .append("SELECT t.*, d.RowId, d.Name, d.ClassId, d.Container, d.Description, d.CreatedBy, d.Created, d.ModifiedBy, d.Modified") - .append(" FROM ").append(d, "d") - .append(" LEFT OUTER JOIN ").append(t, "t") - .append(" ON d.lsid = t.lsid WHERE "); - + SimpleFilter filter = new SimpleFilter(); if (null != rowid) - sql.append("d.rowid=?").add(rowid); + filter.addCondition(FieldKey.fromParts("rowId"), rowid); else if (null != lsid) - sql.append("d.lsid=?").add(lsid); + filter.addCondition(FieldKey.fromParts("lsid"), lsid); else - sql.append("d.classid=? AND d.name=?").add(classId).add(name); - + filter.addCondition(FieldKey.fromParts("classid"), classId) + .addCondition(FieldKey.fromParts("name"), name); if (!allowCrossContainer) - sql.append(" AND d.Container=?").add(container.getEntityId()); + filter.addCondition(FieldKey.fromParts("Folder"), container.getEntityId()); + + TableInfo queryTable = getQueryTable(); + TableSelector selector = new TableSelector(queryTable, filter, null); - return new SqlSelector(getDbTable().getSchema(), sql).getMap(); + try (var results = selector.getResults()) { + if (results.next()) + { + return FieldKeyRowMap.toNameMap(results.getFieldKeyRowMap()); + } + } + return null; +// TableInfo d = getDbTable(); +// TableInfo t = _dataClassDataTableSupplier.get(); +// +// SQLFragment sql = new SQLFragment() +// .append("SELECT t.*, d.RowId, d.Name, d.ClassId, d.Container, d.Description, d.CreatedBy, d.Created, d.ModifiedBy, d.Modified") +// .append(" FROM ").append(d, "d") +// .append(" LEFT OUTER JOIN ").append(t, "t") +// .append(" ON d.lsid = t.lsid WHERE "); +// +// if (null != rowid) +// sql.append("d.rowid=?").add(rowid); +// else if (null != lsid) +// sql.append("d.lsid=?").add(lsid); +// else +// sql.append("d.classid=? AND d.name=?").add(classId).add(name); +// +// if (!allowCrossContainer) +// sql.append(" AND d.Container=?").add(container.getEntityId()); +// +// return new SqlSelector(getDbTable().getSchema(), sql).getMap(); } @Override diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTestCase.jsp b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTestCase.jsp index a5c5e1ec711..7e9ff9333b4 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTestCase.jsp +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTestCase.jsp @@ -1181,7 +1181,7 @@ private @NotNull TableInfo getDataClassTable(String dataClassName) return schema.getTableOrThrow(dataClassName); } -// @Test // Issue 52886 +@Test // Issue 52886 public void testUpdateAuditForLongField() throws Exception { User user = TestContext.get().getUser(); diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index 504c0b99766..eb4319ee5b0 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -982,7 +982,7 @@ private void addSampleTypeColumns(ExpSampleType st, List visibleColumn // TODO missing values? comments? flags? DomainProperty dp = domain.getPropertyByURI(dbColumn.getPropertyURI()); - var propColumn = copyColumnFromJoinedTable(null==dp?dbColumn.getName():dp.getName(), wrapped); + var propColumn = copyColumnFromJoinedTable(null==dp ? dbColumn.getName() : dp.getName(), wrapped); if (propColumn.getName().equalsIgnoreCase("genid")) { propColumn.setHidden(true); diff --git a/list/src/org/labkey/list/model/ListTable.java b/list/src/org/labkey/list/model/ListTable.java index 07ef3f2d493..fd229228b3e 100644 --- a/list/src/org/labkey/list/model/ListTable.java +++ b/list/src/org/labkey/list/model/ListTable.java @@ -239,7 +239,8 @@ else if (!baseColumn.isMvIndicatorColumn()) if (null != pd) { col.setFieldKey(new FieldKey(null,pd.getName())); - defaultsSupplier = PropertyColumn.copyAttributes(schema.getUser(), col, dp, schema.getContainer(), FieldKey.fromParts("EntityId"), getContainerFilter(), defaultsSupplier); + var propertyCf = pd.isLookup() ? QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser()) : getContainerFilter(); + defaultsSupplier = PropertyColumn.copyAttributes(schema.getUser(), col, dp, schema.getContainer(), FieldKey.fromParts("EntityId"), propertyCf, defaultsSupplier); if (pd.isMvEnabled()) { From 1a6434c82ff4fbd89a8ae80d6136b4cbeade151c Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 16 Jul 2025 13:58:16 -0700 Subject: [PATCH 03/11] Update error message expectations. Set the container filter during cross-type/cross-container imports. --- .../test/integration/DataClassCrud.ispec.ts | 8 ++--- .../labkey/experiment/ExpDataIterators.java | 34 +++++++++++++++++++ .../controllers/exp/ExperimentController.java | 14 ++------ 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/experiment/src/client/test/integration/DataClassCrud.ispec.ts b/experiment/src/client/test/integration/DataClassCrud.ispec.ts index 31cd3362e1b..1b53f0fb06f 100644 --- a/experiment/src/client/test/integration/DataClassCrud.ispec.ts +++ b/experiment/src/client/test/integration/DataClassCrud.ispec.ts @@ -234,7 +234,7 @@ describe('Import with update / merge', () => { const BLANK_KEY_UPDATE_ERROR_NO_EXPRESSION = 'Missing value for required property: Name'; const BLANK_KEY_UPDATE_ERROR_WITH_EXPRESSION = 'Name value not provided on row '; const BOGUS_KEY_UPDATE_ERROR = 'Data not found: '; - const CROSS_FOLDER_UPDATE_NOT_SUPPORTED_ERROR = "Data doesn't belong to folder "; + const DUPLICATE_KEY_ERROR = 'duplicate key value'; const dataType = "NoExpressionNameRequired52922"; const createPayload = { @@ -306,9 +306,9 @@ describe('Import with update / merge', () => { // cross folder update not supported when folder type is "Collaboration" let crossFolderErrorResp = await ExperimentCRUDUtils.importData(server, "Name\tDescription\nData1\tNotblank\n\tisBlank", dataTypeWithExpression, "MERGE", subfolder1Options, editorUserOptions); - expect(crossFolderErrorResp.text.indexOf(CROSS_FOLDER_UPDATE_NOT_SUPPORTED_ERROR) > -1).toBeTruthy(); + expect(crossFolderErrorResp.text.indexOf(DUPLICATE_KEY_ERROR) > -1).toBeTruthy(); crossFolderErrorResp = await ExperimentCRUDUtils.importData(server, "Name\tDescription\nData1\tNotblank", dataTypeWithExpression, "UPDATE", subfolder1Options, editorUserOptions); - expect(crossFolderErrorResp.text.indexOf(CROSS_FOLDER_UPDATE_NOT_SUPPORTED_ERROR) > -1).toBeTruthy(); + expect(crossFolderErrorResp.text.indexOf(BOGUS_KEY_UPDATE_ERROR) > -1).toBeTruthy(); // bogus name bogusKeyProvidedError = await ExperimentCRUDUtils.importData(server, "Name\tDescription\nbogus\tisBogus", dataTypeWithExpression, "UPDATE", topFolderOptions, editorUserOptions); @@ -577,4 +577,4 @@ describe('Duplicate IDs', () => { }); -}); \ No newline at end of file +}); diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index 01d42597814..8cb811c046a 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -805,6 +805,29 @@ public boolean next() throws BatchValidationException } } + /** + * Chooses a container filter that is approprate for import, merge or update actions in the face of product folders. + * Note that this is slightly different from our treatment of lookups: + * - when in a project, we allow import or update to all subfolders, + * - when in a folder, we only allow references to data up the folder tree + * @param qDef The QueryDefinition in use for the import action + * @param container The container that is the target of the import or update + * @param user The user doing the action + */ + public static void setContainerFilterForImport(QueryDefinition qDef, Container container, User user) + { + if (container.isProductFoldersEnabled()) + { + ContainerFilter cf; + + if (container.isProject()) + cf = new ContainerFilter.AllInProjectPlusShared(container, user); + else + cf = new ContainerFilter.CurrentPlusProjectAndShared(container, user); + qDef.setContainerFilter(cf); + } + } + /* setup mini dataiterator pipeline to process lineage */ public static void derive(User user, Container container, DataIterator di, boolean isSample, ExpObject dataType, boolean skipAliquot) throws BatchValidationException { @@ -2741,6 +2764,7 @@ private TypeData createDataClassHeaderRow(ExpDataClass dataClass, Container cont List qpe = new ArrayList<>(); DataClassUserSchema schema = new DataClassUserSchema(container, _user); QueryDefinition qDef = schema.getQueryDefForTable(dataClass.getName()); + setContainerFilterForImport(qDef, container, _user); TableInfo dataTable = qDef.getTable(schema, qpe, true); if (dataTable == null) { @@ -2770,6 +2794,16 @@ private TypeData createSampleHeaderRow(ExpSampleTypeImpl sampleType, Container c List qpe = new ArrayList<>(); SamplesSchema schema = new SamplesSchema(_user, container); QueryDefinition qDef = schema.getQueryDefForTable(sampleType.getName()); + setContainerFilterForImport(qDef, container, _user); + ContainerFilter cf; + // Note that this is slightly different from our treatment of lookups: + // - when in a project, we allow import or update to all subfolders, + // - when in a folder, we only allow references to data up the folder tree + if (container.isProject()) + cf = new ContainerFilter.AllInProjectPlusShared(container, _user); + else + cf = new ContainerFilter.CurrentPlusProjectAndShared(container, _user); + qDef.setContainerFilter(cf); TableInfo samplesTable = qDef.getTable(schema, qpe, true); if (samplesTable == null) { diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index 0d319209c22..7a61149cbe2 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -366,6 +366,7 @@ import static org.labkey.api.util.DOM.UL; import static org.labkey.api.util.DOM.at; import static org.labkey.api.util.DOM.cl; +import static org.labkey.experiment.ExpDataIterators.setContainerFilterForImport; import static org.labkey.experiment.api.SampleTypeServiceImpl.SampleChangeType.update; public class ExperimentController extends SpringActionController @@ -4528,18 +4529,7 @@ public void validateForm(QueryForm form, Errors errors) protected void initRequest(QueryForm form) throws ServletException { QueryDefinition query = form.getQueryDef(); - if (getContainer().isProductFoldersEnabled()) - { - ContainerFilter cf; - // Note that this is slightly different from our treatment of lookups: - // - when in a project, we allow import or update to all subfolders, - // - when in a folder, we only allow references to data up the folder tree - if (getContainer().isProject()) - cf = new ContainerFilter.AllInProjectPlusShared(getContainer(), getUser()); - else - cf = new ContainerFilter.CurrentPlusProjectAndShared(getContainer(), getUser()); - query.setContainerFilter(cf); - } + setContainerFilterForImport(query, getContainer(), getUser()); List qpe = new ArrayList<>(); TableInfo t = query.getTable(form.getSchema(), qpe, true); From 70bc0b975447a1128b64f991ec5912473ea8f372 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Thu, 17 Jul 2025 06:27:25 -0700 Subject: [PATCH 04/11] Don't throw exception for multi-part field key --- api/src/org/labkey/api/data/FieldKeyRowMap.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/data/FieldKeyRowMap.java b/api/src/org/labkey/api/data/FieldKeyRowMap.java index 24e596af970..9f686b46e9b 100644 --- a/api/src/org/labkey/api/data/FieldKeyRowMap.java +++ b/api/src/org/labkey/api/data/FieldKeyRowMap.java @@ -131,8 +131,8 @@ public static Map toNameMap(Map rowMap) { Map map = new CaseInsensitiveHashMap<>(); rowMap.forEach((key, value) -> { - if (key.getParent() != null) - throw new IllegalArgumentException("Multi-part field key '" + key + "' cannot be used as key in string map since it may not be unique."); +// if (key.getParent() != null) +// throw new IllegalArgumentException("Multi-part field key '" + key + "' cannot be used as key in string map since it may not be unique."); if (map.containsKey(key.getName())) throw new IllegalArgumentException("Duplicate key '" + key + "' found in fieldKey map."); map.put(key.getName(), value); From 885029fa7a2316e2c7e030737346b1de31e6aa6d Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Thu, 17 Jul 2025 13:00:31 -0700 Subject: [PATCH 05/11] Exclude ostensible changes to multivalued foreign key fields from audit details --- .../org/labkey/api/audit/AuditHandler.java | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/api/src/org/labkey/api/audit/AuditHandler.java b/api/src/org/labkey/api/audit/AuditHandler.java index b7bbdcc57be..b2eac34a642 100644 --- a/api/src/org/labkey/api/audit/AuditHandler.java +++ b/api/src/org/labkey/api/audit/AuditHandler.java @@ -4,6 +4,7 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; +import org.labkey.api.data.MultiValuedForeignKey; import org.labkey.api.data.TableInfo; import org.labkey.api.dataiterator.DataIterator; import org.labkey.api.dataiterator.ExistingRecordDataIterator; @@ -67,13 +68,21 @@ static Pair, Map> getOldAndNewRecordForMerge String key = entry.getKey(); // getDatasetRows() (at least) should return key==column.getName(), expect getColumn(name) to work ColumnInfo col = null==table ? null : table.getColumn(key); - String nameFromAlias = null != col - ? col.getName() - : columns.stream() - .filter(column -> column.getAlias().getId().equalsIgnoreCase(key)) - .map((ColumnInfo::getName)) - .findFirst() - .orElse(key); + // Issue 52886: Skip multivalued foreign key values since they are always stored as the junction table id + // but the new row value will be the data referenced via the junction table entries and thus a difference will be recorded. + // The differences here should be audited in the junction table instead. + if (col != null && col.getFk() instanceof MultiValuedForeignKey) + continue; + + String nameFromAlias = key; + if (null != col) + nameFromAlias = col.getName(); + else + { + ColumnInfo aliasColumn = columns.stream().filter(c -> c.getAlias().getId().equalsIgnoreCase(key) && !(c.getFk() instanceof MultiValuedForeignKey)).findFirst().orElse(null); + if (aliasColumn != null) + nameFromAlias = aliasColumn.getName(); + } String lcName = nameFromAlias.toLowerCase(); // Preserve casing of inputs so we can show the names properly boolean isExpInput = false; From dff0c11651b18ec1319519cc41a1b069110b45ec Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Fri, 18 Jul 2025 07:26:54 -0700 Subject: [PATCH 06/11] Tidy the code --- .../org/labkey/api/audit/AuditHandler.java | 31 ++++++++++---- .../org/labkey/api/data/FieldKeyRowMap.java | 2 - .../org/labkey/api/exp/PropertyColumn.java | 8 +++- .../labkey/assay/plate/query/WellTable.java | 3 +- .../labkey/experiment/ExpDataIterators.java | 12 +----- .../api/ExpDataClassDataTableImpl.java | 41 ++----------------- .../experiment/api/ExpMaterialTableImpl.java | 7 ---- list/src/org/labkey/list/model/ListTable.java | 3 +- 8 files changed, 39 insertions(+), 68 deletions(-) diff --git a/api/src/org/labkey/api/audit/AuditHandler.java b/api/src/org/labkey/api/audit/AuditHandler.java index b2eac34a642..6767eccbdcd 100644 --- a/api/src/org/labkey/api/audit/AuditHandler.java +++ b/api/src/org/labkey/api/audit/AuditHandler.java @@ -65,23 +65,26 @@ static Pair, Map> getOldAndNewRecordForMerge // and we won't convert sample type and data class names into lower case. for (Map.Entry entry : existingRow.entrySet()) { + boolean isMultiValued = false; String key = entry.getKey(); // getDatasetRows() (at least) should return key==column.getName(), expect getColumn(name) to work ColumnInfo col = null==table ? null : table.getColumn(key); - // Issue 52886: Skip multivalued foreign key values since they are always stored as the junction table id - // but the new row value will be the data referenced via the junction table entries and thus a difference will be recorded. - // The differences here should be audited in the junction table instead. if (col != null && col.getFk() instanceof MultiValuedForeignKey) - continue; + isMultiValued = true; String nameFromAlias = key; if (null != col) nameFromAlias = col.getName(); else { - ColumnInfo aliasColumn = columns.stream().filter(c -> c.getAlias().getId().equalsIgnoreCase(key) && !(c.getFk() instanceof MultiValuedForeignKey)).findFirst().orElse(null); + ColumnInfo aliasColumn = columns.stream().filter(c -> c.getAlias().getId().equalsIgnoreCase(key)).findFirst().orElse(null); + if (aliasColumn != null) + { + if (aliasColumn.getFk() != null && aliasColumn.getFk() instanceof MultiValuedForeignKey) + isMultiValued = true; nameFromAlias = aliasColumn.getName(); + } } String lcName = nameFromAlias.toLowerCase(); // Preserve casing of inputs so we can show the names properly @@ -140,8 +143,22 @@ else if (newValue instanceof Number && oldValue != null) } else if (!Objects.equals(oldValue, newValue) || isExtraAuditField) { - originalRow.put(nameFromAlias, oldValue); - modifiedRow.put(nameFromAlias, newValue); + // If multivalued columns change, the value in this table will remain the key to the junction table + // but at this point newValue will look like the newly chosen values not that key. So we skip + // this in the diff unless the value changes from non-null to null or vice versa. + if (isMultiValued) + { + if ((oldValue == null && newValue != null) || (newValue == null && oldValue != null)) + { + originalRow.put(nameFromAlias, oldValue); + modifiedRow.put(nameFromAlias, newValue); + } + } + else + { + originalRow.put(nameFromAlias, oldValue); + modifiedRow.put(nameFromAlias, newValue); + } } } else if (isExtraAuditField) diff --git a/api/src/org/labkey/api/data/FieldKeyRowMap.java b/api/src/org/labkey/api/data/FieldKeyRowMap.java index 9f686b46e9b..12d882b1e3d 100644 --- a/api/src/org/labkey/api/data/FieldKeyRowMap.java +++ b/api/src/org/labkey/api/data/FieldKeyRowMap.java @@ -131,8 +131,6 @@ public static Map toNameMap(Map rowMap) { Map map = new CaseInsensitiveHashMap<>(); rowMap.forEach((key, value) -> { -// if (key.getParent() != null) -// throw new IllegalArgumentException("Multi-part field key '" + key + "' cannot be used as key in string map since it may not be unique."); if (map.containsKey(key.getName())) throw new IllegalArgumentException("Duplicate key '" + key + "' found in fieldKey map."); map.put(key.getName(), value); diff --git a/api/src/org/labkey/api/exp/PropertyColumn.java b/api/src/org/labkey/api/exp/PropertyColumn.java index e9c15740013..d898bbe324b 100644 --- a/api/src/org/labkey/api/exp/PropertyColumn.java +++ b/api/src/org/labkey/api/exp/PropertyColumn.java @@ -35,6 +35,7 @@ import org.labkey.api.exp.property.PropertyService; import org.labkey.api.query.FieldKey; import org.labkey.api.query.PdLookupForeignKey; +import org.labkey.api.query.QueryService; import org.labkey.api.query.SchemaKey; import org.labkey.api.security.User; import org.labkey.api.study.assay.FileLinkDisplayColumn; @@ -182,7 +183,12 @@ public static void copyAttributes( } if (user != null && ((pd.getLookupSchema() != null && pd.getLookupQuery() != null) || pd.getConceptURI() != null)) - to.setFk(PdLookupForeignKey.create(to.getParentTable().getUserSchema(), user, container, pd, cf)); + { + // Issue 52504: Use proper container filter for lookups + var _cf = pd.isLookup() ? QueryService.get().getContainerFilterForLookups(container, user) : cf; + + to.setFk(PdLookupForeignKey.create(to.getParentTable().getUserSchema(), user, container, pd, _cf)); + } to.setDefaultValueType(pd.getDefaultValueTypeEnum()); to.setConditionalFormats(PropertyService.get().getConditionalFormats(pd)); diff --git a/assay/src/org/labkey/assay/plate/query/WellTable.java b/assay/src/org/labkey/assay/plate/query/WellTable.java index a7035a13ad7..9369e90fbe4 100644 --- a/assay/src/org/labkey/assay/plate/query/WellTable.java +++ b/assay/src/org/labkey/assay/plate/query/WellTable.java @@ -251,8 +251,7 @@ private void addWellMetadataColumns() PropertyDescriptor pd = dp.getPropertyDescriptor(); if (pd != null) { - var cf = pd.isLookup() ? QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser()) : getContainerFilter(); - defaultsSupplier = PropertyColumn.copyAttributes(getUserSchema().getUser(), wrapped, dp, getContainer(), lsidFieldKey, cf, defaultsSupplier); + defaultsSupplier = PropertyColumn.copyAttributes(getUserSchema().getUser(), wrapped, dp, getContainer(), lsidFieldKey, getContainerFilter(), defaultsSupplier); wrapped.setFieldKey(FieldKey.fromParts(dp.getName())); } } diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index 8cb811c046a..30e506a1726 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -806,7 +806,7 @@ public boolean next() throws BatchValidationException } /** - * Chooses a container filter that is approprate for import, merge or update actions in the face of product folders. + * Issue 52504 (sort of): Chooses a container filter that is appropriate for import, merge or update actions in the face of product folders. * Note that this is slightly different from our treatment of lookups: * - when in a project, we allow import or update to all subfolders, * - when in a folder, we only allow references to data up the folder tree @@ -2510,6 +2510,7 @@ private int _importPartition(TypeData typeData) Container splitContainer = ContainerManager.getForRowId(containerSplitFile.getKey()); AbstractExpSchema schema = _isSamples ? new SamplesSchema(_user, splitContainer) : new DataClassUserSchema(splitContainer, _user); QueryDefinition qDef = schema.getQueryDefForTable(typeData.dataType.getName()); + setContainerFilterForImport(qDef, splitContainer, _user); TableInfo dataTable = qDef.getTable(schema, new ArrayList<>(), true); if (dataTable == null) @@ -2795,15 +2796,6 @@ private TypeData createSampleHeaderRow(ExpSampleTypeImpl sampleType, Container c SamplesSchema schema = new SamplesSchema(_user, container); QueryDefinition qDef = schema.getQueryDefForTable(sampleType.getName()); setContainerFilterForImport(qDef, container, _user); - ContainerFilter cf; - // Note that this is slightly different from our treatment of lookups: - // - when in a project, we allow import or update to all subfolders, - // - when in a folder, we only allow references to data up the folder tree - if (container.isProject()) - cf = new ContainerFilter.AllInProjectPlusShared(container, _user); - else - cf = new ContainerFilter.CurrentPlusProjectAndShared(container, _user); - qDef.setContainerFilter(cf); TableInfo samplesTable = qDef.getTable(schema, qpe, true); if (samplesTable == null) { diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 02b3855d599..467faab196e 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -456,9 +456,7 @@ protected void populateColumns() PropertyDescriptor pd = (null == dp) ? null : dp.getPropertyDescriptor(); if (dp != null && pd != null) { - // Issue 52504: For lookup validation, we need to use the proper lookup container filter on the table - ContainerFilter cf = pd.isLookup() ? QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser()) : getContainerFilter(); - defaultsSupplier = PropertyColumn.copyAttributes(_userSchema.getUser(), wrapped, dp, getContainer(), lsidFieldKey, cf, defaultsSupplier); + defaultsSupplier = PropertyColumn.copyAttributes(_userSchema.getUser(), wrapped, dp, getContainer(), lsidFieldKey, getContainerFilter(), defaultsSupplier); wrapped.setFieldKey(FieldKey.fromParts(dp.getName())); if (pd.getPropertyType() == PropertyType.ATTACHMENT) @@ -1266,19 +1264,7 @@ protected Map getRow(User user, Container container, Map row = _select(container, rowId, lsid, name, classId, allowCrossContainer); - -// //PostgreSQL includes a column named _row for the row index, but since this is selecting by -// //primary key, it will always be 1, which is not only unnecessary, but confusing, so strip it -// if (null != row) -// { -// if (row instanceof ArrayListMap arrayListMap) -// arrayListMap.getFindMap().remove("_row"); -// else -// row.remove("_row"); -// } - - return row; + return _select(container, rowId, lsid, name, classId, allowCrossContainer); } @Override @@ -1292,7 +1278,8 @@ protected Map _select(Container container, Integer rowid, String if (null == rowid && null == lsid && (null == name || null == classId)) return null; - // FIXME Issue 52886: This retrieves raw db column names, which doesn't work well for comparing existing and new audit records if the name doesn't match the field key + // Issue 52886: Use queryTable here, not raw database table, so the rows are from the user schema with names + // as expected to match row inserts and other querySchema data SimpleFilter filter = new SimpleFilter(); if (null != rowid) filter.addCondition(FieldKey.fromParts("rowId"), rowid); @@ -1314,26 +1301,6 @@ else if (null != lsid) } } return null; -// TableInfo d = getDbTable(); -// TableInfo t = _dataClassDataTableSupplier.get(); -// -// SQLFragment sql = new SQLFragment() -// .append("SELECT t.*, d.RowId, d.Name, d.ClassId, d.Container, d.Description, d.CreatedBy, d.Created, d.ModifiedBy, d.Modified") -// .append(" FROM ").append(d, "d") -// .append(" LEFT OUTER JOIN ").append(t, "t") -// .append(" ON d.lsid = t.lsid WHERE "); -// -// if (null != rowid) -// sql.append("d.rowid=?").add(rowid); -// else if (null != lsid) -// sql.append("d.lsid=?").add(lsid); -// else -// sql.append("d.classid=? AND d.name=?").add(classId).add(name); -// -// if (!allowCrossContainer) -// sql.append(" AND d.Container=?").add(container.getEntityId()); -// -// return new SqlSelector(getDbTable().getSchema(), sql).getMap(); } @Override diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index eb4319ee5b0..0d5e54cb947 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -972,13 +972,6 @@ private void addSampleTypeColumns(ExpSampleType st, List visibleColumn } var wrapped = wrapColumnFromJoinedTable(dbColumn.getName(), dbColumn); - if (dbColumn.isLookup()) - { - var fk = QueryForeignKey.from(this.getUserSchema(), QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser())) - .schema(ExpSchema.SCHEMA_NAME, getContainer()) - .to(dbColumn.getFk().getLookupTableName(), dbColumn.getFk().getLookupColumnName(), null); - wrapped.setFk(fk); - } // TODO missing values? comments? flags? DomainProperty dp = domain.getPropertyByURI(dbColumn.getPropertyURI()); diff --git a/list/src/org/labkey/list/model/ListTable.java b/list/src/org/labkey/list/model/ListTable.java index 2e6ad0e3d0e..c6feb8cd2bd 100644 --- a/list/src/org/labkey/list/model/ListTable.java +++ b/list/src/org/labkey/list/model/ListTable.java @@ -240,8 +240,7 @@ else if (!baseColumn.isMvIndicatorColumn()) if (null != pd) { col.setFieldKey(new FieldKey(null,pd.getName())); - var propertyCf = pd.isLookup() ? QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser()) : getContainerFilter(); - defaultsSupplier = PropertyColumn.copyAttributes(schema.getUser(), col, dp, schema.getContainer(), FieldKey.fromParts("EntityId"), propertyCf, defaultsSupplier); + defaultsSupplier = PropertyColumn.copyAttributes(schema.getUser(), col, dp, schema.getContainer(), FieldKey.fromParts("EntityId"), getContainerFilter(), defaultsSupplier); if (pd.isMvEnabled()) { From 24c5adea334b732dbb3492d985892835aa93c7dc Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Fri, 18 Jul 2025 07:37:20 -0700 Subject: [PATCH 07/11] Remove redundant logic --- assay/src/org/labkey/assay/TSVProtocolSchema.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/assay/src/org/labkey/assay/TSVProtocolSchema.java b/assay/src/org/labkey/assay/TSVProtocolSchema.java index b9077569dfc..17d74d93a8d 100644 --- a/assay/src/org/labkey/assay/TSVProtocolSchema.java +++ b/assay/src/org/labkey/assay/TSVProtocolSchema.java @@ -286,8 +286,7 @@ public _AssayPlateReplicateStatsTable(@NotNull Domain domain, @NotNull AssayProt PropertyDescriptor pd = dp.getPropertyDescriptor(); if (pd != null) { - var cf = pd.isLookup() ? QueryService.get().getContainerFilterForLookups(getContainer(), _userSchema.getUser()) : containerFilter; - defaultsSupplier = PropertyColumn.copyAttributes(userSchema.getUser(), columnInfo, dp, getContainer(), null, cf, defaultsSupplier); + defaultsSupplier = PropertyColumn.copyAttributes(userSchema.getUser(), columnInfo, dp, getContainer(), null, containerFilter, defaultsSupplier); columnInfo.setFieldKey(FieldKey.fromParts(dp.getName())); } } From 69d61bd895ee0aa02f06938dd8ae9aa525bb7bc3 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Fri, 18 Jul 2025 07:37:57 -0700 Subject: [PATCH 08/11] Unused import --- assay/src/org/labkey/assay/TSVProtocolSchema.java | 1 - 1 file changed, 1 deletion(-) diff --git a/assay/src/org/labkey/assay/TSVProtocolSchema.java b/assay/src/org/labkey/assay/TSVProtocolSchema.java index 17d74d93a8d..936575e61ea 100644 --- a/assay/src/org/labkey/assay/TSVProtocolSchema.java +++ b/assay/src/org/labkey/assay/TSVProtocolSchema.java @@ -47,7 +47,6 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.query.FilteredTable; import org.labkey.api.query.QueryForeignKey; -import org.labkey.api.query.QueryService; import org.labkey.api.query.QueryUpdateService; import org.labkey.api.security.User; import org.labkey.api.security.UserPrincipal; From 56e52995fa057e69339b66690753cb2256b3e4e8 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Fri, 18 Jul 2025 07:38:48 -0700 Subject: [PATCH 09/11] Unused import --- assay/src/org/labkey/assay/plate/query/WellTable.java | 1 - 1 file changed, 1 deletion(-) diff --git a/assay/src/org/labkey/assay/plate/query/WellTable.java b/assay/src/org/labkey/assay/plate/query/WellTable.java index 9369e90fbe4..a9746c5ca86 100644 --- a/assay/src/org/labkey/assay/plate/query/WellTable.java +++ b/assay/src/org/labkey/assay/plate/query/WellTable.java @@ -39,7 +39,6 @@ import org.labkey.api.query.FieldKey; import org.labkey.api.query.InvalidKeyException; import org.labkey.api.query.QueryForeignKey; -import org.labkey.api.query.QueryService; import org.labkey.api.query.QueryUpdateService; import org.labkey.api.query.QueryUpdateServiceException; import org.labkey.api.query.SimpleUserSchema; From 0caaeff9aada4ccb4c610a4ca6c2687495a8281e Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Fri, 18 Jul 2025 09:46:18 -0700 Subject: [PATCH 10/11] better capitalization --- .../api/ExpDataClassDataTableImpl.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 467faab196e..860fefe10a1 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1273,23 +1273,23 @@ protected Map _select(Container container, Object[] keys) throws throw new IllegalStateException(); } - protected Map _select(Container container, Integer rowid, String lsid, String name, Integer classId, boolean allowCrossContainer) throws SQLException + protected Map _select(Container container, Integer rowId, String lsid, String name, Integer classId, boolean allowCrossContainer) throws SQLException { - if (null == rowid && null == lsid && (null == name || null == classId)) + if (null == rowId && null == lsid && (null == name || null == classId)) return null; // Issue 52886: Use queryTable here, not raw database table, so the rows are from the user schema with names // as expected to match row inserts and other querySchema data SimpleFilter filter = new SimpleFilter(); - if (null != rowid) - filter.addCondition(FieldKey.fromParts("rowId"), rowid); + if (null != rowId) + filter.addCondition(Column.RowId.fieldKey(), rowId); else if (null != lsid) - filter.addCondition(FieldKey.fromParts("lsid"), lsid); + filter.addCondition(Column.LSID.fieldKey(), lsid); else - filter.addCondition(FieldKey.fromParts("classid"), classId) - .addCondition(FieldKey.fromParts("name"), name); + filter.addCondition(Column.ClassId.fieldKey(), classId) + .addCondition(Column.Name.fieldKey(), name); if (!allowCrossContainer) - filter.addCondition(FieldKey.fromParts("Folder"), container.getEntityId()); + filter.addCondition(Column.Folder.fieldKey(), container.getEntityId()); TableInfo queryTable = getQueryTable(); TableSelector selector = new TableSelector(queryTable, filter, null); From e219d07ae77406401b75f7d43fc58df5adf3a4d4 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Fri, 18 Jul 2025 09:49:10 -0700 Subject: [PATCH 11/11] Space --- assay/src/org/labkey/assay/plate/query/WellTable.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assay/src/org/labkey/assay/plate/query/WellTable.java b/assay/src/org/labkey/assay/plate/query/WellTable.java index a9746c5ca86..92a277c303a 100644 --- a/assay/src/org/labkey/assay/plate/query/WellTable.java +++ b/assay/src/org/labkey/assay/plate/query/WellTable.java @@ -250,7 +250,7 @@ private void addWellMetadataColumns() PropertyDescriptor pd = dp.getPropertyDescriptor(); if (pd != null) { - defaultsSupplier = PropertyColumn.copyAttributes(getUserSchema().getUser(), wrapped, dp, getContainer(), lsidFieldKey, getContainerFilter(), defaultsSupplier); + defaultsSupplier = PropertyColumn.copyAttributes(getUserSchema().getUser(), wrapped, dp, getContainer(), lsidFieldKey, getContainerFilter(), defaultsSupplier); wrapped.setFieldKey(FieldKey.fromParts(dp.getName())); } }