diff --git a/api/src/org/labkey/api/exp/api/SampleTypeService.java b/api/src/org/labkey/api/exp/api/SampleTypeService.java index 0a5b601ad73..bcfbb85c86c 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeService.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeService.java @@ -30,6 +30,7 @@ import org.labkey.api.gwt.client.model.GWTIndex; import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; import org.labkey.api.lists.permissions.ManagePicklistsPermission; +import org.labkey.api.ontology.Unit; import org.labkey.api.qc.DataState; import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.ValidationException; @@ -54,8 +55,8 @@ public interface SampleTypeService { - String MISSING_COLUMN_ERROR_MESSAGE_PATTERN = "When adding or updating samples, the %s column must be provided when the %s column is."; - String MISSING_COLUMN_VALUE_ERROR_MESSAGE_PATTERN = "When adding or updating samples, a %s value must be provided when there is a value for %s."; + String MISSING_AMOUNT_ERROR_MESSAGE = "An Amount value must be provided when Units are provided."; + String MISSING_UNITS_ERROR_MESSAGE = "A Units value must be provided when Amounts are provided."; String UNPROVIDED_VALUE_ERROR_MESSAGE_PATTERN = "No %s value provided for %s %s."; String NEW_SAMPLE_TYPE_ALIAS_VALUE = "{{this_sample_set}}"; String MATERIAL_INPUTS_PREFIX = "MaterialInputs/"; @@ -114,6 +115,12 @@ static void setInstance(SampleTypeService impl) ServiceRegistry.get().registerService(SampleTypeService.class, impl); } + @NotNull + List getSupportedUnits(); + + @Nullable + Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits, @Nullable String sampleTypeName); + Map getSampleTypesForRoles(Container container, ContainerFilter filter, ExpProtocol.ApplicationType type); /** diff --git a/api/src/org/labkey/api/ontology/KindOfQuantity.java b/api/src/org/labkey/api/ontology/KindOfQuantity.java index ad80ae0cd00..cd51aeb5e6b 100644 --- a/api/src/org/labkey/api/ontology/KindOfQuantity.java +++ b/api/src/org/labkey/api/ontology/KindOfQuantity.java @@ -20,7 +20,7 @@ public enum KindOfQuantity @Override public List getCommonUnits() { - return List.of(Unit.L, Unit.mL, Unit.uL); + return List.of(Unit.kL, Unit.L, Unit.mL, Unit.uL); } }, diff --git a/api/src/org/labkey/api/ontology/Unit.java b/api/src/org/labkey/api/ontology/Unit.java index 7a547dbd156..1cb963687a7 100644 --- a/api/src/org/labkey/api/ontology/Unit.java +++ b/api/src/org/labkey/api/ontology/Unit.java @@ -9,7 +9,6 @@ import org.labkey.api.data.ConversionExceptionWithMessage; import java.util.HashMap; -import java.util.List; import java.util.function.Function; public enum Unit @@ -76,8 +75,6 @@ public enum Unit Quantity.Mass_pg.class, "picogram", "picograms"); - private static final String CONVERSION_EXCEPTION_MESSAGE ="Units value (%s) is not compatible with the display units (%s)."; - @Getter final @NotNull KindOfQuantity kindOfQuantity; @Getter @@ -198,38 +195,6 @@ public Quantity convert(@Nullable Object value) return Quantity.convert(value, this); } - public static Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits) - { - if (rawUnits == null) - return null; - if (rawUnits instanceof Unit u) - { - if (defaultUnits == null) - return u; - else if (u.kindOfQuantity != defaultUnits.kindOfQuantity) - throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); - else - return u; - } - if (!(rawUnits instanceof String rawUnitsString)) - throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); - if (!StringUtils.isBlank(rawUnitsString)) - { - rawUnitsString = rawUnitsString.trim(); - - Unit mUnit = Unit.fromName(rawUnitsString); - if (mUnit == null) - { - List commonUnits = KindOfQuantity.getSupportedUnits(); - throw new ConversionExceptionWithMessage("Unsupported Units value (" + rawUnitsString + "). Supported values are: " + StringUtils.join(commonUnits, ", ") + "."); - } - if (defaultUnits != null && mUnit.kindOfQuantity != defaultUnits.kindOfQuantity) - throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, defaultUnits)); - return mUnit; - } - return null; - } - public static class TestCase extends Assert { @Test @@ -325,56 +290,5 @@ public void testFromName() assertNull(Unit.fromName("")); } - @Test - public void testGetValidatedUnit() - { - try - { - Unit.getValidatedUnit("g", Unit.mg); - Unit.getValidatedUnit("g ", Unit.mg); - Unit.getValidatedUnit(" g ", Unit.mg); - } - catch (ConversionExceptionWithMessage e) - { - fail("Compatible unit should not throw exception."); - } - try - { - assertNull(Unit.getValidatedUnit(null, Unit.unit)); - } - catch (ConversionExceptionWithMessage e) - { - fail("null units should be null"); - } - try - { - assertNull(Unit.getValidatedUnit("", Unit.unit)); - } - catch (ConversionExceptionWithMessage e) - { - fail("empty units should be null"); - } - try - { - Unit.getValidatedUnit("g", Unit.unit); - fail("Units that are not comparable should throw exception."); - } - catch (ConversionExceptionWithMessage ignore) - { - - } - - try - { - Unit.getValidatedUnit("nonesuch", Unit.unit); - fail("Invalid units should throw exception."); - } - catch (ConversionExceptionWithMessage ignore) - { - - } - - } - } } diff --git a/experiment/resources/schemas/exp.xml b/experiment/resources/schemas/exp.xml index 0330b5c5e69..1a3347f50df 100644 --- a/experiment/resources/schemas/exp.xml +++ b/experiment/resources/schemas/exp.xml @@ -223,7 +223,7 @@ /list/grid.view?name=${Name} diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 8529150b446..ddf89a9b3bd 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -1039,6 +1039,7 @@ public Set getIntegrationTests() LineageTest.class, OntologyManager.TestCase.class, PropertyServiceImpl.TestCase.class, + SampleTypeServiceImpl.TestCase.class, StorageNameGenerator.TestCase.class, StorageProvisionerImpl.TestCase.class, UniqueValueCounterTestCase.class diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java b/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java index 85087c513de..0b42122ac78 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java @@ -557,11 +557,11 @@ public void createSampleNames(@NotNull List> maps, { // Failed to generate a name due to some part of the expression not in the row if (hasNameExpression()) - throw new ExperimentException("Failed to generate name for sample on row " + e.getRowNumber(), e); + throw new ExperimentException("Failed to generate name for sample.", e); else if (hasNameAsIdCol()) - throw new ExperimentException("SampleID or Name is required for sample on row " + e.getRowNumber(), e); + throw new ExperimentException("SampleID or Name is required for sample.", e); else - throw new ExperimentException("All id columns are required for sample on row " + e.getRowNumber(), e); + throw new ExperimentException("All id columns are required for sample.", e); } } @@ -609,7 +609,7 @@ public String createSampleName(@NotNull Map rowMap, } catch (NameGenerator.NameGenerationException e) { - throw new ExperimentException("Failed to generate name for Sample", e); + throw new ExperimentException("Failed to generate name for sample.", e); } } diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTableImpl.java index 93af4125c9d..e844b9e20a2 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTableImpl.java @@ -64,11 +64,16 @@ public MutableColumnInfo createColumn(String alias, Column column) case MaterialLSIDPrefix: case Name: case LabelColor: - case MetricUnit: case AutoLinkTargetContainer: case AutoLinkCategory: case RowId: return wrapColumn(alias, _rootTable.getColumn(column.toString())); + case MetricUnit: + { + var columnInfo = wrapColumn(alias, _rootTable.getColumn(column.toString())); + columnInfo.setLabel("Display Units"); + return columnInfo; + } case Created: return wrapColumn(alias, _rootTable.getColumn("Created")); case CreatedBy: diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp index e44db7a69d5..035d1407080 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeTestCase.jsp @@ -390,7 +390,7 @@ public void idColsSet_nameExpressionNull_hasNameProperty() throws Exception errors = new BatchValidationException(); svc.insertRows(user, c, rows, errors, null, null); assertTrue(errors.hasErrors()); - assertTrue(errors.getMessage().contains("SampleID or Name is required for sample on row 1")); + assertTrue(errors.getMessage().contains("SampleID or Name is required for sample")); } // idCols not null, nameExpression not null, 'name' property (not used) -- fail diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index fe5c12767e0..f435673dcbc 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -23,6 +23,8 @@ import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.junit.Assert; +import org.junit.Test; import org.labkey.api.action.ApiUsageException; import org.labkey.api.audit.AbstractAuditHandler; import org.labkey.api.audit.AbstractAuditTypeProvider; @@ -42,6 +44,7 @@ import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.ConversionExceptionWithMessage; import org.labkey.api.data.DatabaseCache; import org.labkey.api.data.DbSchema; import org.labkey.api.data.DbScope; @@ -94,6 +97,7 @@ import org.labkey.api.inventory.InventoryService; import org.labkey.api.miniprofiler.MiniProfiler; import org.labkey.api.miniprofiler.Timing; +import org.labkey.api.ontology.KindOfQuantity; import org.labkey.api.ontology.Quantity; import org.labkey.api.ontology.Unit; import org.labkey.api.qc.DataState; @@ -174,6 +178,16 @@ public class SampleTypeServiceImpl extends AbstractAuditHandler implements Sampl public static final String SAMPLE_COUNT_SEQ_NAME = "org.labkey.api.exp.api.ExpMaterial:sampleCount"; public static final String ROOT_SAMPLE_COUNT_SEQ_NAME = "org.labkey.api.exp.api.ExpMaterial:rootSampleCount"; + public static final List SUPPORTED_UNITS = new ArrayList<>(); + public static final String CONVERSION_EXCEPTION_MESSAGE ="Units value (%s) is not compatible with the %s display units (%s)."; + + static + { + SUPPORTED_UNITS.addAll(KindOfQuantity.Volume.getCommonUnits()); + SUPPORTED_UNITS.addAll(KindOfQuantity.Mass.getCommonUnits()); + SUPPORTED_UNITS.addAll(KindOfQuantity.Count.getCommonUnits()); + } + // columns that may appear in a row when only the sample status is updating. public static final Set statusUpdateColumns = Set.of( ExpMaterialTable.Column.Modified.name().toLowerCase(), @@ -208,6 +222,44 @@ Cache> getMaterialSourceCache() return materialSourceCache; } + @Override @NotNull + public List getSupportedUnits() + { + return SUPPORTED_UNITS; + } + + @Nullable @Override + public Unit getValidatedUnit(@Nullable Object rawUnits, @Nullable Unit defaultUnits, String sampleTypeName) + { + if (rawUnits == null) + return null; + if (rawUnits instanceof Unit u) + { + if (defaultUnits == null) + return u; + else if (u.getKindOfQuantity() != defaultUnits.getKindOfQuantity()) + throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, sampleTypeName == null ? "" : sampleTypeName, defaultUnits)); + else + return u; + } + if (!(rawUnits instanceof String rawUnitsString)) + throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, sampleTypeName == null ? "" : sampleTypeName, defaultUnits)); + if (!StringUtils.isBlank(rawUnitsString)) + { + rawUnitsString = rawUnitsString.trim(); + + Unit mUnit = Unit.fromName(rawUnitsString); + List commonUnits = getSupportedUnits(); + if (mUnit == null || !commonUnits.contains(mUnit)) + { + throw new ConversionExceptionWithMessage("Unsupported Units value (" + rawUnitsString + "). Supported values are: " + StringUtils.join(commonUnits, ", ") + "."); + } + if (defaultUnits != null && mUnit.getKindOfQuantity() != defaultUnits.getKindOfQuantity()) + throw new ConversionExceptionWithMessage(String.format(CONVERSION_EXCEPTION_MESSAGE, rawUnits, sampleTypeName == null ? "" : sampleTypeName, defaultUnits)); + return mUnit; + } + return null; + } public void clearMaterialSourceCache(@Nullable Container c) { @@ -2259,4 +2311,60 @@ public void refreshSampleTypeMaterializedView(@NotNull ExpSampleType st, SampleC { ExpMaterialTableImpl.refreshMaterializedView(st.getLSID(), reason); } + + + public static class TestCase extends Assert + { + @Test + public void testGetValidatedUnit() + { + SampleTypeService service = SampleTypeService.get(); + try + { + service.getValidatedUnit("g", Unit.mg, "Sample Type"); + service.getValidatedUnit("g ", Unit.mg, "Sample Type"); + service.getValidatedUnit(" g ", Unit.mg, "Sample Type"); + } + catch (ConversionExceptionWithMessage e) + { + fail("Compatible unit should not throw exception."); + } + try + { + assertNull(service.getValidatedUnit(null, Unit.unit, "Sample Type")); + } + catch (ConversionExceptionWithMessage e) + { + fail("null units should be null"); + } + try + { + assertNull(service.getValidatedUnit("", Unit.unit, "Sample Type")); + } + catch (ConversionExceptionWithMessage e) + { + fail("empty units should be null"); + } + try + { + service.getValidatedUnit("g", Unit.unit, "Sample Type"); + fail("Units that are not comparable should throw exception."); + } + catch (ConversionExceptionWithMessage ignore) + { + + } + + try + { + service.getValidatedUnit("nonesuch", Unit.unit, "Sample Type"); + fail("Invalid units should throw exception."); + } + catch (ConversionExceptionWithMessage ignore) + { + + } + + } + } } diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 587a1623a24..4dccc7d4fc1 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -148,8 +148,8 @@ import static org.labkey.api.exp.api.SampleTypeDomainKind.ALIQUOT_ROLLUP_FIELD_LABELS; import static org.labkey.api.exp.api.SampleTypeService.ConfigParameters.SkipAliquotRollup; import static org.labkey.api.exp.api.SampleTypeService.ConfigParameters.SkipMaxSampleCounterFunction; -import static org.labkey.api.exp.api.SampleTypeService.MISSING_COLUMN_ERROR_MESSAGE_PATTERN; -import static org.labkey.api.exp.api.SampleTypeService.MISSING_COLUMN_VALUE_ERROR_MESSAGE_PATTERN; +import static org.labkey.api.exp.api.SampleTypeService.MISSING_AMOUNT_ERROR_MESSAGE; +import static org.labkey.api.exp.api.SampleTypeService.MISSING_UNITS_ERROR_MESSAGE; import static org.labkey.api.exp.api.SampleTypeService.UNPROVIDED_VALUE_ERROR_MESSAGE_PATTERN; import static org.labkey.api.exp.query.ExpMaterialTable.Column.AliquotCount; import static org.labkey.api.exp.query.ExpMaterialTable.Column.AliquotVolume; @@ -534,9 +534,9 @@ public static void confirmAmountAndUnitsColumns(Collection columns, bool if (hasUnits == hasAmount) return; // both columns are present or neither is if (!hasAmount) - throw new ConversionExceptionWithMessage(String.format(MISSING_COLUMN_ERROR_MESSAGE_PATTERN, StoredAmount.label(), Units.name())); + throw new ConversionExceptionWithMessage(MISSING_AMOUNT_ERROR_MESSAGE); - throw new ConversionExceptionWithMessage(String.format(MISSING_COLUMN_ERROR_MESSAGE_PATTERN, Units.name(), StoredAmount.label())); + throw new ConversionExceptionWithMessage(MISSING_UNITS_ERROR_MESSAGE); } @Override @@ -665,11 +665,11 @@ protected Map coerceTypes(Map row, Map _update(User user, Container c, Map