diff --git a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java index 9373ee83ee9..825f84d91f4 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java +++ b/api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java @@ -902,7 +902,7 @@ else if (o != remapped) { try { - String error = validator.validate(rowNum, o, validatorContext); + String error = validator.validate(rowNum, o, validatorContext, null); if (error != null) errors.add(new PropertyValidationError(error, pd.getName())); } @@ -1060,7 +1060,7 @@ else if (validatorMap.containsKey(pd)) { for (ColumnValidator validator : validatorMap.get(pd)) { - String error = validator.validate(rowNum, o, validatorContext); + String error = validator.validate(rowNum, o, validatorContext, null); if (error != null) errors.add(new PropertyValidationError(error, pd.getName())); } diff --git a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java index f7fbae5aaa8..53cd6ffbc4c 100644 --- a/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java +++ b/api/src/org/labkey/api/assay/DefaultAssayRunCreator.java @@ -1229,7 +1229,7 @@ else if (!missing) ValidatorContext validatorContext = new ValidatorContext(context.getContainer(), context.getUser()); for (ColumnValidator validator : validators) { - String msg = validator.validate(rowNum, o, validatorContext); + String msg = validator.validate(rowNum, o, validatorContext, null); if (msg != null) errors.add(new PropertyValidationError(msg, label)); } diff --git a/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java b/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java index aa30afeb5d1..25b989cf011 100644 --- a/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java +++ b/api/src/org/labkey/api/data/ColumnRenderPropertiesImpl.java @@ -48,6 +48,7 @@ public abstract class ColumnRenderPropertiesImpl implements MutableColumnRenderP public static final String STORAGE_UNIQUE_ID_CONCEPT_URI = "http://www.labkey.org/types#storageUniqueId"; public static final String STORAGE_UNIQUE_ID_SEQUENCE_PREFIX = "org.labkey.api.StorageUniqueId"; public static final String TEXT_CHOICE_CONCEPT_URI = "http://www.labkey.org/types#textChoice"; + public static final String NON_NEGATIVE_NUMBER_CONCEPT_URI = "http://www.labkey.org/types#nonNegativeNumber"; protected SortDirection _sortDirection = SortDirection.ASC; protected String _inputType; diff --git a/api/src/org/labkey/api/data/validator/AbstractColumnValidator.java b/api/src/org/labkey/api/data/validator/AbstractColumnValidator.java index d28db959727..a36fd97b5b6 100644 --- a/api/src/org/labkey/api/data/validator/AbstractColumnValidator.java +++ b/api/src/org/labkey/api/data/validator/AbstractColumnValidator.java @@ -15,6 +15,7 @@ */ package org.labkey.api.data.validator; +import org.jetbrains.annotations.Nullable; import org.labkey.api.exp.MvFieldWrapper; import org.labkey.api.exp.property.ValidatorContext; @@ -37,7 +38,7 @@ public String validate(int rowNum, Object o) } @Override - public String validate(int rowNum, Object value, ValidatorContext validatorContext) + public String validate(int rowNum, Object value, ValidatorContext validatorContext, @Nullable Object providedValue) { return validate(rowNum, value); } diff --git a/api/src/org/labkey/api/data/validator/ColumnValidator.java b/api/src/org/labkey/api/data/validator/ColumnValidator.java index 4c0cc8c8626..8a8f1d511fa 100644 --- a/api/src/org/labkey/api/data/validator/ColumnValidator.java +++ b/api/src/org/labkey/api/data/validator/ColumnValidator.java @@ -15,6 +15,7 @@ */ package org.labkey.api.data.validator; +import org.jetbrains.annotations.Nullable; import org.labkey.api.dataiterator.DataIterator; import org.labkey.api.exp.property.ValidatorContext; @@ -25,11 +26,6 @@ public interface ColumnValidator { String validate(int rowNum, Object value); - String validate(int rowNum, Object value, ValidatorContext validatorContext); - - default String validate(int rowNum, Object value, DataIterator data) - { - return validate(rowNum, value); - } + String validate(int rowNum, Object value, ValidatorContext validatorContext, @Nullable Object providedValue); } diff --git a/api/src/org/labkey/api/data/validator/PropertyValidator.java b/api/src/org/labkey/api/data/validator/PropertyValidator.java index fc8a91b8314..83b4fa24c3f 100644 --- a/api/src/org/labkey/api/data/validator/PropertyValidator.java +++ b/api/src/org/labkey/api/data/validator/PropertyValidator.java @@ -15,6 +15,7 @@ */ package org.labkey.api.data.validator; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnRenderProperties; import org.labkey.api.exp.property.IPropertyValidator; import org.labkey.api.exp.property.ValidatorContext; @@ -51,12 +52,12 @@ public String validate(int rowNum, Object value) } @Override - public String validate(int rowNum, Object value, ValidatorContext validatorContext) + public String validate(int rowNum, Object value, ValidatorContext validatorContext, @Nullable Object providedValue) { // Don't validate null values, #15683, #19352 if (null == value) return null; - if (kind.validate(propertyValidator, _columnRenderProperties , value, errors, validatorContext)) + if (kind.validate(propertyValidator, _columnRenderProperties , value, errors, validatorContext, providedValue)) return null; if (errors.isEmpty()) return null; @@ -64,4 +65,9 @@ public String validate(int rowNum, Object value, ValidatorContext validatorConte errors.clear(); return msg; } + + public IPropertyValidator getPropertyValidator() + { + return propertyValidator; + } } diff --git a/api/src/org/labkey/api/dataiterator/ValidatorIterator.java b/api/src/org/labkey/api/dataiterator/ValidatorIterator.java index f729d0ad3d9..e03ea70bb39 100644 --- a/api/src/org/labkey/api/dataiterator/ValidatorIterator.java +++ b/api/src/org/labkey/api/dataiterator/ValidatorIterator.java @@ -166,7 +166,16 @@ public boolean next() throws BatchValidationException for (ColumnValidator v : l) { Object value = _data.get(i); - String msg = validate(v, rowNum, value, _data); + Object providedDataValue = value; + if (v instanceof PropertyValidator pv) + { + if (pv.getPropertyValidator() != null) + { + // Use :::provided:::Amount in non-negative validator message, instead of converted Amount + providedDataValue = pv.getPropertyValidator().getProvidedDataValue(_data); + } + } + String msg = validate(v, rowNum, value, _data, providedDataValue); if (null != msg) { @@ -206,12 +215,12 @@ public boolean next() throws BatchValidationException return true; } - protected String validate(ColumnValidator v, int rowNum, Object value, DataIterator data) + protected String validate(ColumnValidator v, int rowNum, Object value, DataIterator data, @Nullable Object providedValue) { String msg; // CONSIDER: add validatorContext to ColumnValidator.validate() always if (v instanceof PropertyValidator) - msg = v.validate(rowNum, value, validatorContext); + msg = v.validate(rowNum, value, validatorContext, providedValue); else msg = v.validate(rowNum, value); diff --git a/api/src/org/labkey/api/exp/property/IPropertyValidator.java b/api/src/org/labkey/api/exp/property/IPropertyValidator.java index dadd18f8036..11e2e4264f5 100644 --- a/api/src/org/labkey/api/exp/property/IPropertyValidator.java +++ b/api/src/org/labkey/api/exp/property/IPropertyValidator.java @@ -15,7 +15,9 @@ */ package org.labkey.api.exp.property; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; +import org.labkey.api.dataiterator.DataIterator; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.query.ValidationError; import org.labkey.api.query.ValidationException; @@ -50,6 +52,8 @@ public interface IPropertyValidator void setExpressionValue(String expression); void setErrorMessage(String message); void setProperty(String key, String value); + void setColumnNameProvidedData(String columnNameProvidedData); + @Nullable Object getProvidedDataValue(DataIterator dataIterator); IPropertyValidator save(User user, Container container) throws ValidationException; diff --git a/api/src/org/labkey/api/exp/property/ValidatorKind.java b/api/src/org/labkey/api/exp/property/ValidatorKind.java index 6ca092e83ac..4f5eeea17b1 100644 --- a/api/src/org/labkey/api/exp/property/ValidatorKind.java +++ b/api/src/org/labkey/api/exp/property/ValidatorKind.java @@ -16,6 +16,7 @@ package org.labkey.api.exp.property; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnRenderProperties; import org.labkey.api.query.ValidationError; import org.labkey.data.xml.ValidatorPropertyType; @@ -40,7 +41,7 @@ public interface ValidatorKind IPropertyValidator createInstance(); boolean isValid(IPropertyValidator validator, List errors); - boolean validate(IPropertyValidator validator, ColumnRenderProperties field, @NotNull Object value, List errors, ValidatorContext validatorCache); + boolean validate(IPropertyValidator validator, ColumnRenderProperties field, @NotNull Object value, List errors, ValidatorContext validatorCache, @Nullable Object providedValue); // Standard save-validator-to-XML method. ValidatorKind implementations can customize this by overriding. default void convertToXml(IPropertyValidator v, ValidatorsType validatorsXml) diff --git a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java index 46d9fa2d61e..adb096bcb33 100644 --- a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java +++ b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java @@ -464,13 +464,13 @@ protected Map updateRow(User user, Container container, Map validators = ColumnValidators.create(column, dp); for (ColumnValidator v : validators) { - String msg = v.validate(-1, value, _validatorContext); + String msg = v.validate(-1, value, _validatorContext, providedValue); if (msg != null) throw new ValidationException(msg, column.getName()); } @@ -494,12 +494,12 @@ protected void validateInsertRow(Map row) throws ValidationExcep } else { - validateValue(col, value); + validateValue(col, value, null); } } } - private void validateUpdateRow(Map row) throws ValidationException + protected void validateUpdateRow(Map row) throws ValidationException { for (ColumnInfo col : getQueryTable().getColumns()) { @@ -507,7 +507,7 @@ private void validateUpdateRow(Map row) throws ValidationExcepti if (row.containsKey(col.getColumnName())) { Object value = row.get(col.getColumnName()); - validateValue(col, value); + validateValue(col, value, null); } } } diff --git a/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts b/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts index 9be2539267c..a600c34d2b4 100644 --- a/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts +++ b/experiment/src/client/test/integration/SampleTypeCrud.ispec.ts @@ -103,7 +103,7 @@ beforeAll(async () => { name: SAMPLE_ALIQUOT_IMPORT_NO_NAME_PATTERN_NAME, aliquotNameExpression: "", nameExpression: "", - metricUnit: 'mL' + metricUnit: 'g' } }; await server.post('property', 'createDomain', createPayload, {...topFolderOptions, ...designerReaderOptions}).expect((result) => { @@ -958,3 +958,132 @@ describe('Aliquot crud', () => { }); +describe('Amount/Unit CRUD', () => { + it ("Test Amounts/Units validation on insert/import/update/merge", async () => { + const dataType = SAMPLE_ALIQUOT_IMPORT_NO_NAME_PATTERN_NAME; + const NO_UNIT_ERROR = 'A Units value must be provided when Amounts are provided.'; + const NO_AMOUNT_ERROR = 'An Amount value must be provided when Units are provided.'; + const INCOMPATIBLE_ERROR = 'Units value (L) is not compatible with the ' + dataType + ' display units (g).'; + const NEGATIVE_ERROR = "Value '-1.1' for field 'Amount' is invalid. Amounts must be non-negative."; + + const dataName = "S-amountCrud"; + + let errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tStoredAmount\nData1\t1", dataType, "INSERT", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(NO_UNIT_ERROR); + errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tUnits\nData1\tkg", dataType, "INSERT", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(NO_AMOUNT_ERROR); + errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tStoredAmount\tUnits\nData1\t1.1\tL", dataType, "INSERT", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(INCOMPATIBLE_ERROR); + errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tStoredAmount\tUnits\nData1\t-1.1\tkg", dataType, "INSERT", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(NEGATIVE_ERROR); + errorMsg = await ExperimentCRUDUtils.importCrossTypeData(server, "Name\tStoredAmount\tUnits\tSampleType\nData1\t-1.1\tkg\t" + dataType ,'IMPORT', topFolderOptions, adminOptions, true); + expect(errorMsg.text).toContain(NEGATIVE_ERROR); + + await server.post('query', 'insertRows', { + schemaName: 'samples', + queryName: dataType, + rows: [{ + name: dataName, + amount: -1.1, + units: 'kg', + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + const errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toContain(NEGATIVE_ERROR); + }); + const sampleRows = await ExperimentCRUDUtils.insertRows(server, [{ + name: dataName, + amount: 123, + units: 'kg', + }], 'samples', dataType, topFolderOptions, editorUserOptions); + + const sampleRowId = caseInsensitive(sampleRows[0], 'rowId'); + + errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tAmount\n" + dataName + "\t321", dataType, "UPDATE", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(NO_UNIT_ERROR); + errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tStoredAmount\n" + dataName + "\t321", dataType, "MERGE", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(NO_UNIT_ERROR); + await server.post('query', 'updateRows', { + schemaName: 'samples', + queryName: dataType, + rows: [{ + Amount: 321, + rowId: sampleRowId + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + const errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toContain(NO_UNIT_ERROR); + }); + await server.post('query', 'updateRows', { + schemaName: 'samples', + queryName: dataType, + rows: [{ + StoredAmount: 321, + rowId: sampleRowId + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + const errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toContain(NO_UNIT_ERROR); + }); + + + errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tUnits\n" + dataName + "\tg", dataType, "UPDATE", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(NO_AMOUNT_ERROR); + errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tUnits\n" + dataName + "\tg", dataType, "MERGE", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(NO_AMOUNT_ERROR); + await server.post('query', 'updateRows', { + schemaName: 'samples', + queryName: dataType, + rows: [{ + Units: 'kg', + rowId: sampleRowId + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + const errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toContain(NO_AMOUNT_ERROR); + }); + + errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tAmount\tUnits\n" + dataName + "\t321\tL", dataType, "UPDATE", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(INCOMPATIBLE_ERROR); + errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tStoredAmount\tUnits\n" + dataName + "\t321\tL", dataType, "MERGE", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(INCOMPATIBLE_ERROR); + await server.post('query', 'updateRows', { + schemaName: 'samples', + queryName: dataType, + rows: [{ + Amount: 321, + Units: 'L', + rowId: sampleRowId + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + const errorResp = JSON.parse(result.text); + expect(errorResp['exception']).toContain(INCOMPATIBLE_ERROR); + }); + + errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tAmount\tUnits\n" + dataName + "\t-1.1\tkg", dataType, "UPDATE", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(NEGATIVE_ERROR); + errorMsg = await ExperimentCRUDUtils.importSample(server, "Name\tStoredAmount\tUnits\n" + dataName + "\t-1.1\tkg", dataType, "MERGE", topFolderOptions, editorUserOptions); + expect(errorMsg.text).toContain(NEGATIVE_ERROR); + await server.post('query', 'updateRows', { + schemaName: 'samples', + queryName: dataType, + rows: [{ + Amount: -1, + Units: 'kg', + rowId: sampleRowId + }] + }, { ...topFolderOptions, ...editorUserOptions }).expect((result) => { + const errorResp = JSON.parse(result.text); + // Note that the row by row update error is different from DIB. This is OK for now since we are planning to deprecate row by row updates. + expect(errorResp['exception']).toContain("Value '-1000.0 (g)' for field 'Amount' is invalid. Amounts must be non-negative."); + }); + + errorMsg = await ExperimentCRUDUtils.importCrossTypeData(server, "Name\tStoredAmount\tUnits\tSampleType\nData1\t-1.1\tkg\t" + dataType ,'UPDATE', topFolderOptions, adminOptions, true); + expect(errorMsg.text).toContain(NEGATIVE_ERROR); + errorMsg = await ExperimentCRUDUtils.importCrossTypeData(server, "Name\tStoredAmount\tUnits\tSampleType\nData1\t-1.1\tkg\t" + dataType ,'MERGE', topFolderOptions, adminOptions, true); + expect(errorMsg.text).toContain(NEGATIVE_ERROR); + + }); + +}); + diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index 716bda48f0d..f470308fb57 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -303,10 +303,10 @@ else if (isUpdateOnly && columnNameMap.containsKey(AliquotedFromLSID.name())) } @Override - protected String validate(ColumnValidator v, int rowNum, Object value, DataIterator data) + protected String validate(ColumnValidator v, int rowNum, Object value, DataIterator data, Object providedValue) { if (!(v instanceof RequiredValidator) || _aliquotedFromColIdx < 0) - return super.validate(v, rowNum, value, data); + return super.validate(v, rowNum, value, data, providedValue); String aliquotedFromValue = null; Object aliquotedFromObj = data.get(_aliquotedFromColIdx); diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index 80393dc07d5..3843fc1b6c0 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -56,6 +56,7 @@ import org.labkey.api.dataiterator.DataIteratorContext; import org.labkey.api.dataiterator.LoggingDataIterator; import org.labkey.api.dataiterator.SimpleTranslator; +import org.labkey.api.exp.Lsid; import org.labkey.api.exp.MvColumn; import org.labkey.api.exp.OntologyManager; import org.labkey.api.exp.PropertyColumn; @@ -66,15 +67,19 @@ import org.labkey.api.exp.api.ExperimentUrls; import org.labkey.api.exp.api.NameExpressionOptionService; import org.labkey.api.exp.api.StorageProvisioner; +import org.labkey.api.exp.property.DefaultPropertyValidator; import org.labkey.api.exp.property.Domain; import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.exp.property.DomainUtil; +import org.labkey.api.exp.property.IPropertyValidator; +import org.labkey.api.exp.property.PropertyService; import org.labkey.api.exp.query.ExpDataTable; import org.labkey.api.exp.query.ExpMaterialTable; import org.labkey.api.exp.query.ExpSampleTypeTable; import org.labkey.api.exp.query.ExpSchema; import org.labkey.api.exp.query.SamplesSchema; import org.labkey.api.gwt.client.AuditBehaviorType; +import org.labkey.api.gwt.client.model.PropertyValidatorType; import org.labkey.api.inventory.InventoryService; import org.labkey.api.ontology.Quantity; import org.labkey.api.ontology.Unit; @@ -136,6 +141,8 @@ import java.util.stream.Collectors; import static java.util.Objects.requireNonNull; +import static org.labkey.api.audit.AuditHandler.PROVIDED_DATA_PREFIX; +import static org.labkey.api.data.ColumnRenderPropertiesImpl.NON_NEGATIVE_NUMBER_CONCEPT_URI; import static org.labkey.api.exp.api.SampleTypeDomainKind.ALIQUOT_COUNT_LABEL; import static org.labkey.api.exp.api.SampleTypeDomainKind.ALIQUOT_VOLUME_LABEL; import static org.labkey.api.exp.api.SampleTypeDomainKind.AVAILABLE_ALIQUOT_COUNT_LABEL; @@ -158,9 +165,18 @@ public class ExpMaterialTableImpl extends ExpRunItemTableImpl MATERIAL_ALT_MERGE_KEYS; public static final Set MATERIAL_ALT_UPDATE_KEYS; + public static final List AMOUNT_RANGE_VALIDATORS = new ArrayList<>(); static { MATERIAL_ALT_MERGE_KEYS = Set.of(Column.MaterialSourceId.name(), Column.Name.name()); MATERIAL_ALT_UPDATE_KEYS = Set.of(Column.LSID.name()); + + Lsid rangeValidatorLsid = DefaultPropertyValidator.createValidatorURI(PropertyValidatorType.Range); + IPropertyValidator amountValidator = PropertyService.get().createValidator(rangeValidatorLsid.toString()); + amountValidator.setName("SampleAmountNonNegative"); + amountValidator.setExpressionValue("~gte=0"); + amountValidator.setErrorMessage("Amounts must be non-negative."); + amountValidator.setColumnNameProvidedData(PROVIDED_DATA_PREFIX + Column.StoredAmount.name()); + AMOUNT_RANGE_VALIDATORS.add(amountValidator); } public ExpMaterialTableImpl(UserSchema schema, ContainerFilter cf, @Nullable ExpSampleType sampleType) @@ -314,6 +330,8 @@ public StringExpression getURL(ColumnInfo parent) columnInfo.setDescription("The amount of this sample, in the base unit for the sample type's display unit (if defined), currently on hand."); columnInfo.setUserEditable(false); columnInfo.setReadOnly(true); + columnInfo.setConceptURI(NON_NEGATIVE_NUMBER_CONCEPT_URI); + columnInfo.setValidators(AMOUNT_RANGE_VALIDATORS); return columnInfo; } case StoredAmount -> @@ -330,6 +348,8 @@ public StringExpression getURL(ColumnInfo parent) columnInfo.setShownInInsertView(true); columnInfo.setUserEditable(true); columnInfo.setCalculated(false); + columnInfo.setConceptURI(NON_NEGATIVE_NUMBER_CONCEPT_URI); + columnInfo.setValidators(AMOUNT_RANGE_VALIDATORS); return columnInfo; } else @@ -339,6 +359,8 @@ public StringExpression getURL(ColumnInfo parent) columnInfo.setLabel(label); columnInfo.setImportAliasesSet(importAliases); columnInfo.setDescription("The amount of this sample currently on hand."); + columnInfo.setConceptURI(NON_NEGATIVE_NUMBER_CONCEPT_URI); + columnInfo.setValidators(AMOUNT_RANGE_VALIDATORS); return columnInfo; } } diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 9cb45a4f468..a9691bc3f55 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -832,6 +832,29 @@ public static boolean isAliquotStatusChangeNeedRecalc(Collection available return false; } + // Customize negative amount error message when the provided unit doesn't match sample type unit. + // For example, provided value of "-1 kg" would have been converted to "-1000 mg" by now. + // This updateRow (going to be deprecated) inconsistent with the data iterator code path, which use provided value "-1" in error message. + // TODO: remove this override when consolidating sample update method to remove row by row update + @Override + protected void validateUpdateRow(Map row) throws ValidationException + { + for (ColumnInfo col : getQueryTable().getColumns()) + { + if (row.containsKey(col.getColumnName())) + { + // if provided value is present, validate provided + Object value = row.get(col.getColumnName()); + Object providedValue = null; + if (_sampleType != null && _sampleType.getMetricUnit() != null && value != null && (StoredAmount.name().equalsIgnoreCase(col.getColumnName()) || "Amount".equalsIgnoreCase(col.getColumnName()))) + { + providedValue = value + " (" + _sampleType.getMetricUnit() + ")"; + } + validateValue(col, value, providedValue); + } + } + } + @Override protected Map updateRow(User user, Container container, Map row, @NotNull Map oldRow, boolean allowOwner, boolean retainCreation) throws InvalidKeyException, ValidationException, QueryUpdateServiceException, SQLException diff --git a/experiment/src/org/labkey/experiment/api/property/LengthValidator.java b/experiment/src/org/labkey/experiment/api/property/LengthValidator.java index 63af14bd54f..2da37e79bc7 100644 --- a/experiment/src/org/labkey/experiment/api/property/LengthValidator.java +++ b/experiment/src/org/labkey/experiment/api/property/LengthValidator.java @@ -16,6 +16,7 @@ package org.labkey.experiment.api.property; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnRenderProperties; import org.labkey.api.exp.property.DefaultPropertyValidator; import org.labkey.api.exp.property.IPropertyValidator; @@ -68,7 +69,7 @@ public boolean isValid(IPropertyValidator validator, List error @Override public boolean validate(IPropertyValidator validator, ColumnRenderProperties field, @NotNull Object value, - List errors, ValidatorContext validatorCache) + List errors, ValidatorContext validatorCache, @Nullable Object providedValue) { assert value != null : "Shouldn't be validating a null value"; String[] parts = validator.getExpressionValue().split("="); diff --git a/experiment/src/org/labkey/experiment/api/property/LookupValidator.java b/experiment/src/org/labkey/experiment/api/property/LookupValidator.java index 0cd1a54b988..ee33a451f0c 100644 --- a/experiment/src/org/labkey/experiment/api/property/LookupValidator.java +++ b/experiment/src/org/labkey/experiment/api/property/LookupValidator.java @@ -16,6 +16,7 @@ package org.labkey.experiment.api.property; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.ColumnRenderProperties; import org.labkey.api.data.Container; @@ -226,7 +227,8 @@ public boolean validate(IPropertyValidator validator, ColumnRenderProperties crpField, @NotNull Object value, List errors, - ValidatorContext validatorCache) + ValidatorContext validatorCache, + @Nullable Object providedValue) { //noinspection ConstantConditions assert value != null : "Shouldn't be validating a null value"; diff --git a/experiment/src/org/labkey/experiment/api/property/PropertyValidatorImpl.java b/experiment/src/org/labkey/experiment/api/property/PropertyValidatorImpl.java index 9adfefd64f3..ad006489653 100644 --- a/experiment/src/org/labkey/experiment/api/property/PropertyValidatorImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/PropertyValidatorImpl.java @@ -15,11 +15,12 @@ */ package org.labkey.experiment.api.property; -import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; +import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.Table; +import org.labkey.api.dataiterator.DataIterator; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.property.IPropertyValidator; import org.labkey.api.exp.property.PropertyService; @@ -40,6 +41,7 @@ public class PropertyValidatorImpl implements IPropertyValidator private PropertyValidator _validator; private PropertyValidator _validatorOld; private boolean _deleted; + private String columnNameProvidedData; public PropertyValidatorImpl(PropertyValidator validator) { @@ -173,6 +175,35 @@ public ValidatorKind getType() return PropertyService.get().getValidatorKind(getTypeURI()); } + @Override + public void setColumnNameProvidedData(String columnNameProvidedData) + { + this.columnNameProvidedData = columnNameProvidedData; + } + + @Override + public Object getProvidedDataValue(DataIterator dataIterator) + { + if (columnNameProvidedData != null) + { + // Get the value from the provided data column + int providedDataColIndex = -1; + for (int colIndex = 0; colIndex < dataIterator.getColumnCount(); colIndex++) + { + ColumnInfo colInfo = dataIterator.getColumnInfo(colIndex); + if (colInfo != null && columnNameProvidedData.equalsIgnoreCase(colInfo.getName())) + { + providedDataColIndex = colIndex; + break; + } + } + if (providedDataColIndex != -1) + return dataIterator.get(providedDataColIndex); + } + + return null; + } + @Override public IPropertyValidator save(User user, Container container) throws ValidationException { @@ -220,7 +251,7 @@ public boolean validate(PropertyDescriptor prop, Object value, List error @Override public boolean validate(IPropertyValidator validator, ColumnRenderProperties field, @NotNull Object value, - List errors, ValidatorContext validatorCache) + List errors, ValidatorContext validatorCache, @Nullable Object providedValue) { //noinspection ConstantConditions assert value != null : "Shouldn't be validating a null value"; @@ -82,7 +83,7 @@ public boolean validate(IPropertyValidator validator, ColumnRenderProperties fie { if (!isValid(value, constraint)) { - createErrorMessage(validator, field, value, errors); + createErrorMessage(validator, field, providedValue == null ? value : providedValue, errors); return false; } } @@ -113,11 +114,23 @@ private Pair parsePart(String expression) return null; } + /** + * Normalize value for comparison. + * For example -0.0 == 0.0, but: + * Double.doubleToLongBits(-0.0) // returns -9223372036854775808L + * Double.doubleToLongBits(0.0) // returns 0L + */ + private double normalizeForComparison(String strVal) + { + double d = NumberUtils.toDouble(strVal); + return d == 0.0d ? 0.0d : d; + } + private boolean isValid(Object value, Pair constraint) { if (NumberUtils.isCreatable(String.valueOf(value))) { - int comparison = Double.compare(NumberUtils.toDouble(String.valueOf(value)), NumberUtils.toDouble(constraint.getValue())); + int comparison = Double.compare(normalizeForComparison(String.valueOf(value)), normalizeForComparison(constraint.getValue())); return comparisonValid(comparison, constraint.getKey()); } else if (value instanceof Date) diff --git a/experiment/src/org/labkey/experiment/api/property/RegExValidator.java b/experiment/src/org/labkey/experiment/api/property/RegExValidator.java index 9e35c478cd1..5c5f2e32ba7 100644 --- a/experiment/src/org/labkey/experiment/api/property/RegExValidator.java +++ b/experiment/src/org/labkey/experiment/api/property/RegExValidator.java @@ -17,6 +17,7 @@ import org.apache.commons.lang3.BooleanUtils; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnRenderProperties; import org.labkey.api.exp.property.DefaultPropertyValidator; import org.labkey.api.exp.property.IPropertyValidator; @@ -92,7 +93,7 @@ public boolean isValid(IPropertyValidator validator, List error @Override public boolean validate(IPropertyValidator validator, ColumnRenderProperties field, @NotNull Object value, - List errors, ValidatorContext validatorCache) + List errors, ValidatorContext validatorCache, @Nullable Object providedValue) { assert value != null : "Shouldn't be validating a null value"; diff --git a/experiment/src/org/labkey/experiment/api/property/TextChoiceValidator.java b/experiment/src/org/labkey/experiment/api/property/TextChoiceValidator.java index da946740ab8..5c4be86ac7e 100644 --- a/experiment/src/org/labkey/experiment/api/property/TextChoiceValidator.java +++ b/experiment/src/org/labkey/experiment/api/property/TextChoiceValidator.java @@ -16,6 +16,7 @@ package org.labkey.experiment.api.property; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.labkey.api.data.ColumnRenderProperties; import org.labkey.api.data.MultiChoice; import org.labkey.api.exp.property.IPropertyValidator; @@ -53,7 +54,7 @@ public boolean isValid(IPropertyValidator validator, List error @Override public boolean validate(IPropertyValidator validator, ColumnRenderProperties field, @NotNull Object value, - List errors, ValidatorContext validatorCache) + List errors, ValidatorContext validatorCache, @Nullable Object providedValue) { assert value != null : "Shouldn't be validating a null value";