Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down Expand Up @@ -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()));
}
Expand Down
2 changes: 1 addition & 1 deletion api/src/org/labkey/api/assay/DefaultAssayRunCreator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
}
Expand Down
8 changes: 2 additions & 6 deletions api/src/org/labkey/api/data/validator/ColumnValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);

}
10 changes: 8 additions & 2 deletions api/src/org/labkey/api/data/validator/PropertyValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -51,17 +52,22 @@ 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;
String msg = errors.get(0).getMessage();
errors.clear();
return msg;
}

public IPropertyValidator getPropertyValidator()
{
return propertyValidator;
}
}
15 changes: 12 additions & 3 deletions api/src/org/labkey/api/dataiterator/ValidatorIterator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);

Expand Down
4 changes: 4 additions & 0 deletions api/src/org/labkey/api/exp/property/IPropertyValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion api/src/org/labkey/api/exp/property/ValidatorKind.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -40,7 +41,7 @@ public interface ValidatorKind

IPropertyValidator createInstance();
boolean isValid(IPropertyValidator validator, List<ValidationError> errors);
boolean validate(IPropertyValidator validator, ColumnRenderProperties field, @NotNull Object value, List<ValidationError> errors, ValidatorContext validatorCache);
boolean validate(IPropertyValidator validator, ColumnRenderProperties field, @NotNull Object value, List<ValidationError> 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)
Expand Down
10 changes: 5 additions & 5 deletions api/src/org/labkey/api/query/DefaultQueryUpdateService.java
Original file line number Diff line number Diff line change
Expand Up @@ -464,13 +464,13 @@ protected Map<String, Object> updateRow(User user, Container container, Map<Stri
return row;
}

protected void validateValue(ColumnInfo column, Object value) throws ValidationException
protected void validateValue(ColumnInfo column, Object value, Object providedValue) throws ValidationException
{
DomainProperty dp = getDomain() == null ? null : getDomain().getPropertyByName(column.getColumnName());
List<ColumnValidator> 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());
}
Expand All @@ -494,20 +494,20 @@ protected void validateInsertRow(Map<String, Object> row) throws ValidationExcep
}
else
{
validateValue(col, value);
validateValue(col, value, null);
}
}
}

private void validateUpdateRow(Map<String, Object> row) throws ValidationException
protected void validateUpdateRow(Map<String, Object> row) throws ValidationException
{
for (ColumnInfo col : getQueryTable().getColumns())
{
// Only validate incoming values
if (row.containsKey(col.getColumnName()))
{
Object value = row.get(col.getColumnName());
validateValue(col, value);
validateValue(col, value, null);
}
}
}
Expand Down
131 changes: 130 additions & 1 deletion experiment/src/client/test/integration/SampleTypeCrud.ispec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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);

});

});

4 changes: 2 additions & 2 deletions experiment/src/org/labkey/experiment/ExpDataIterators.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading
Loading