diff --git a/api/src/org/labkey/api/ApiModule.java b/api/src/org/labkey/api/ApiModule.java index 910b20e288f..cfd324365d7 100644 --- a/api/src/org/labkey/api/ApiModule.java +++ b/api/src/org/labkey/api/ApiModule.java @@ -82,7 +82,6 @@ import org.labkey.api.data.dialect.JdbcMetaDataTest; import org.labkey.api.data.dialect.ParameterSubstitutionTest; import org.labkey.api.data.dialect.StandardDialectStringHandler; -import org.labkey.api.data.measurement.Measurement; import org.labkey.api.dataiterator.CachingDataIterator; import org.labkey.api.dataiterator.DataIteratorUtil; import org.labkey.api.dataiterator.DiskCachingDataIterator; @@ -411,8 +410,6 @@ public void registerServlets(ServletContext servletCtx) LimitedUser.TestCase.class, MarkableIterator.TestCase.class, MaterializedQueryHelper.TestCase.class, - Measurement.TestCase.class, - Measurement.Unit.TestCase.class, MemTracker.TestCase.class, ModuleContext.TestCase.class, ModuleDependencySorter.TestCase.class, diff --git a/api/src/org/labkey/api/audit/AbstractAuditHandler.java b/api/src/org/labkey/api/audit/AbstractAuditHandler.java index d0d89d05b1f..3f4c3f1c098 100644 --- a/api/src/org/labkey/api/audit/AbstractAuditHandler.java +++ b/api/src/org/labkey/api/audit/AbstractAuditHandler.java @@ -45,16 +45,17 @@ public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryS /** * Create a detailed audit record object so it can be recorded in the audit tables - * @param user making change - * @param c container containing auditable data - * @param tInfo Auditable tableInfo containing auditable record - * @param action being performed - * @param userComment Comment provided by the user explaining reason for change. NOTE: This value is generally not currently supported by many audit logging domains, and may be ignored. - * @param row map of new data values - * @param existingRow map of data values + * @param user making change + * @param c container containing auditable data + * @param tInfo Auditable tableInfo containing auditable record + * @param action being performed + * @param userComment Comment provided by the user explaining reason for change. NOTE: This value is generally not currently supported by many audit logging domains, and may be ignored. + * @param row map of new data values + * @param existingRow map of data values + * @param providedValues map of values provided by the user before conversion (e.g., for quantity values) * @return DetailedAuditTypeEvent object describing audit record (NOTE: not committed to DB yet) */ - protected abstract DetailedAuditTypeEvent createDetailedAuditRecord(User user, Container c, AuditConfigurable tInfo, QueryService.AuditAction action, @Nullable String userComment, @Nullable Map row, Map existingRow); + protected abstract DetailedAuditTypeEvent createDetailedAuditRecord(User user, Container c, AuditConfigurable tInfo, QueryService.AuditAction action, @Nullable String userComment, @Nullable Map row, Map existingRow, Map providedValues); /** * Allow for adding fields that may be present in the updated row but not represented in the original row @@ -69,7 +70,7 @@ protected void addDetailedModifiedFields(Map originalRow, Map> rows, @Nullable List> existingRows, boolean useTransactionAuditCache) + public void addAuditEvent(User user, Container c, TableInfo table, @Nullable AuditBehaviorType auditType, @Nullable String userComment, QueryService.AuditAction action, List> rows, @Nullable List> existingRows, @Nullable List> providedValues, boolean useTransactionAuditCache) { if (table.supportsAuditTracking()) { @@ -119,7 +120,8 @@ public void addAuditEvent(User user, Container c, TableInfo table, @Nullable Aud { Map row = rows.get(i); Map existingRow = null == existingRows ? Collections.emptyMap() : existingRows.get(i); - DetailedAuditTypeEvent event = createDetailedAuditRecord(user, c, auditConfigurable, action, userComment, row, existingRow); + Map providedValueRow = null == providedValues || providedValues.size() <= i ? null : providedValues.get(i); + DetailedAuditTypeEvent event = createDetailedAuditRecord(user, c, auditConfigurable, action, userComment, row, existingRow, providedValueRow); switch (action) { diff --git a/api/src/org/labkey/api/audit/AuditHandler.java b/api/src/org/labkey/api/audit/AuditHandler.java index 1321ea6739d..47eb678980b 100644 --- a/api/src/org/labkey/api/audit/AuditHandler.java +++ b/api/src/org/labkey/api/audit/AuditHandler.java @@ -32,6 +32,9 @@ public interface AuditHandler { + String PROVIDED_DATA_PREFIX = ":::provided:::"; + String DELTA_PROVIDED_DATA_PREFIX = ":::delta_provided:::"; + void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryService.AuditAction action, Integer dataRowCount, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String userComment); default void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryService.AuditAction action, Integer dataRowCount, @Nullable AuditBehaviorType auditBehaviorType, @Nullable String userComment, boolean skipAuditLevelCheck) @@ -40,13 +43,20 @@ default void addSummaryAuditEvent(User user, Container c, TableInfo table, Query } void addAuditEvent(User user, Container c, TableInfo table, @Nullable AuditBehaviorType auditType, @Nullable String userComment, QueryService.AuditAction action, - @Nullable List> rows, @Nullable List> existingRows, boolean useTransactionAuditCache); + @Nullable List> rows, @Nullable List> existingRows, @Nullable List> providedValues, boolean useTransactionAuditCache); + + /* In the case of update the 'existingRows' is the 'before' version of the record. Caller is not expected to provide existingRows without rows. */ + default void addAuditEvent(User user, Container c, TableInfo table, @Nullable AuditBehaviorType auditType, @Nullable String userComment, QueryService.AuditAction action, + @Nullable List> rows, @Nullable List> existingRows) + { + addAuditEvent(user, c, table, auditType, userComment, action, rows, existingRows, null, false); + } /* In the case of update the 'existingRows' is the 'before' version of the record. Caller is not expected to provide existingRows without rows. */ default void addAuditEvent(User user, Container c, TableInfo table, @Nullable AuditBehaviorType auditType, @Nullable String userComment, QueryService.AuditAction action, - @Nullable List> rows, @Nullable List> existingRows) + @Nullable List> rows, @Nullable List> existingRows, @Nullable List> providedValues) { - addAuditEvent(user, c, table, auditType, userComment, action, rows, existingRows, false); + addAuditEvent(user, c, table, auditType, userComment, action, rows, existingRows, providedValues, false); } diff --git a/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java b/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java index dc8306c01f1..eb60d7424b1 100644 --- a/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java +++ b/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java @@ -13,13 +13,22 @@ import java.util.Map; import java.util.Set; +import static org.labkey.api.audit.AuditHandler.DELTA_PROVIDED_DATA_PREFIX; +import static org.labkey.api.audit.AuditHandler.PROVIDED_DATA_PREFIX; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.StoredAmount; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.Units; + public class SampleTimelineAuditEvent extends DetailedAuditTypeEvent { public static final String EVENT_TYPE = "SampleTimelineEvent"; public static final String SAMPLE_TIMELINE_EVENT_TYPE = "SampleTimelineEventType"; + public static final String AMOUNT_AND_UNIT_UPGRADE_COMMENT = "Storage amount unit conversion to base unit during upgrade script."; - public static final Set EXCLUDED_DETAIL_FIELDS = Set.of("AvailableAliquotVolume", "AvailableAliquotCount", "AliquotCount", "AliquotVolume", "AliquotUnit"); + public static final Set EXCLUDED_DETAIL_FIELDS = Set.of( + "AvailableAliquotVolume", "AvailableAliquotCount", "AliquotCount", "AliquotVolume", "AliquotUnit", + PROVIDED_DATA_PREFIX + StoredAmount.name(), PROVIDED_DATA_PREFIX + Units.name(), + DELTA_PROVIDED_DATA_PREFIX + StoredAmount.name() + DELTA_PROVIDED_DATA_PREFIX + Units.name()); public enum SampleTimelineEventType { diff --git a/api/src/org/labkey/api/data/EnumTableInfo.java b/api/src/org/labkey/api/data/EnumTableInfo.java index 8e8aad6f9c2..12a247e33ef 100644 --- a/api/src/org/labkey/api/data/EnumTableInfo.java +++ b/api/src/org/labkey/api/data/EnumTableInfo.java @@ -37,6 +37,7 @@ public class EnumTableInfo> extends VirtualTable { protected final Class _enum; + protected final EnumSet _enumSet; protected final EnumValueGetter _valueGetter; protected final EnumRowIdGetter _rowIdGetter; @Nullable protected String _schemaName; @@ -93,6 +94,11 @@ public EnumTableInfo(Class e, UserSchema schema, EnumRowIdGetter e, UserSchema schema, EnumValueGetter valueGetter, EnumRowIdGetter rowIdGetter, boolean rowIdPK, String description) + { + this(e, EnumSet.allOf(e), schema, valueGetter, rowIdGetter, rowIdPK, description); + } + /** * Exposes an enum as a three column virtual table, using valueGetter to determine its value, rowIdGetter to determine rowId, and ordinal() as ordinal * @param e class of the enum @@ -102,11 +108,12 @@ public EnumTableInfo(Class e, UserSchema schema, EnumRowIdGetter e, UserSchema schema, EnumValueGetter valueGetter, EnumRowIdGetter rowIdGetter, boolean rowIdPK, String description) + public EnumTableInfo(Class e, EnumSet enumSet, UserSchema schema, EnumValueGetter valueGetter, EnumRowIdGetter rowIdGetter, boolean rowIdPK, String description) { super(schema.getDbSchema(), e.getSimpleName(), schema); setDescription(description); _enum = e; + _enumSet = enumSet; _valueGetter = valueGetter; _rowIdGetter = rowIdGetter; @@ -131,8 +138,7 @@ public SQLFragment getFromSQL() checkReadBeforeExecute(); SQLFragment sql = new SQLFragment(); String separator = ""; - EnumSet enumSet = EnumSet.allOf(_enum); - for (EnumType e : enumSet) + for (EnumType e : _enumSet) { sql.append(separator); separator = " UNION "; diff --git a/api/src/org/labkey/api/data/ImportAliasable.java b/api/src/org/labkey/api/data/ImportAliasable.java index 0d529e0954d..def16557b9a 100644 --- a/api/src/org/labkey/api/data/ImportAliasable.java +++ b/api/src/org/labkey/api/data/ImportAliasable.java @@ -16,10 +16,12 @@ package org.labkey.api.data; import org.labkey.api.collections.CaseInsensitiveHashMap; +import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.exp.MvColumn; import org.labkey.api.view.template.PageConfig; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; @@ -45,6 +47,23 @@ public interface ImportAliasable class Helper { + /** + * Creates a set of the different possible aliases for import (name, label, import aliases) for a single column. + */ + public static Set createImportSet(T property) + { + if (property == null) + return Collections.emptySet(); + + Set aliases = new CaseInsensitiveHashSet(); + aliases.add(property.getName()); + if (property.getLabel() != null) + aliases.add(property.getLabel()); + if (!property.getImportAliasSet().isEmpty()) + aliases.addAll(property.getImportAliasSet()); + return aliases; + } + /** * Creates a mapping of many different possible names (actual name, label/caption, property URI, etc). * for a column to the column itself. Useful to provide flexibility in how the data is labeled during imports. diff --git a/api/src/org/labkey/api/data/TableInfo.java b/api/src/org/labkey/api/data/TableInfo.java index 1671fc34dc4..1e4fda92668 100644 --- a/api/src/org/labkey/api/data/TableInfo.java +++ b/api/src/org/labkey/api/data/TableInfo.java @@ -154,7 +154,7 @@ public void addSummaryAuditEvent(User user, Container c, TableInfo table, QueryS } @Override - public void addAuditEvent(User user, Container c, TableInfo table, @Nullable AuditBehaviorType auditType, @Nullable String userComment, QueryService.AuditAction action, List> rows, @Nullable List> updatedRows, boolean useTransactionAuditCache) + public void addAuditEvent(User user, Container c, TableInfo table, @Nullable AuditBehaviorType auditType, @Nullable String userComment, QueryService.AuditAction action, List> rows, @Nullable List> updatedRows, @Nullable List> providedValues, boolean useTransactionAuditCache) { } } diff --git a/api/src/org/labkey/api/data/generator/DataGenerator.java b/api/src/org/labkey/api/data/generator/DataGenerator.java index e4bd025b245..4f1feef13ac 100644 --- a/api/src/org/labkey/api/data/generator/DataGenerator.java +++ b/api/src/org/labkey/api/data/generator/DataGenerator.java @@ -16,7 +16,6 @@ import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; import org.labkey.api.data.dialect.SqlDialect; -import org.labkey.api.data.measurement.Measurement; import org.labkey.api.dataiterator.DataIteratorBuilder; import org.labkey.api.dataiterator.DetailedAuditLogDataIterator; import org.labkey.api.dataiterator.MapDataIterator; @@ -34,6 +33,8 @@ import org.labkey.api.exp.query.SamplesSchema; import org.labkey.api.gwt.client.AuditBehaviorType; import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; +import org.labkey.api.ontology.KindOfQuantity; +import org.labkey.api.ontology.Unit; import org.labkey.api.pipeline.CancelledException; import org.labkey.api.pipeline.PipelineJob; import org.labkey.api.qc.DataState; @@ -76,12 +77,7 @@ public class DataGenerator implements ContainerU private static final String SEARCH_FIELD_NAME = "Search"; private static final String USED_FIELD_NAME = "Used"; private static final List UNITS = List.of("g", "mg", "kg", "uL", "mL", "L", "unit"); - // TODO use KindOfQuantity - private static final Map> UNIT_KINDS = Map.of( - Measurement.Kind.Mass, List.of("g", "mg", "kg"), - Measurement.Kind.Count, List.of("units"), - Measurement.Kind.Volume, List.of("uL", "mL", "L") - ); + private static final List LABEL_COLORS = new ArrayList<>(); static { LABEL_COLORS.add("#800000"); // maroon, @@ -846,8 +842,8 @@ private static String getUnitsValue(ExpSampleType sampleType) { if (!StringUtils.isEmpty(sampleType.getMetricUnit())) { - Measurement.Unit unit = Measurement.Unit.getUnit(sampleType.getMetricUnit()); - units = randomIndex(UNIT_KINDS.get(unit.getKind())); + Unit unit = Unit.fromName(sampleType.getMetricUnit()); + units = randomIndex(unit.getKindOfQuantity().getCommonUnits()).name(); } else units = randomIndex(UNITS); diff --git a/api/src/org/labkey/api/data/measurement/AmountDataColumn.java b/api/src/org/labkey/api/data/measurement/AmountDataColumn.java deleted file mode 100644 index 9f78f822a27..00000000000 --- a/api/src/org/labkey/api/data/measurement/AmountDataColumn.java +++ /dev/null @@ -1,131 +0,0 @@ -package org.labkey.api.data.measurement; - -import org.apache.commons.collections4.MultiValuedMap; -import org.apache.commons.lang3.StringUtils; -import org.apache.commons.math3.util.Precision; -import org.jetbrains.annotations.NotNull; -import org.labkey.api.data.ColumnInfo; -import org.labkey.api.data.DataColumn; -import org.labkey.api.data.DisplayColumn; -import org.labkey.api.data.DisplayColumnFactory; -import org.labkey.api.data.RenderContext; -import org.labkey.api.query.FieldKey; -import org.labkey.api.util.HtmlString; - -import java.util.Set; - -import static org.labkey.api.data.measurement.Measurement.DEFAULT_PRECISION_SCALE; -import static org.labkey.api.data.measurement.UnitsDataColumn.DEFAULT_UNITS_FIELD_PROPERTY_NAME; -import static org.labkey.api.data.measurement.UnitsDataColumn.UNITS_FIELD_PROPERTY_NAME; -import static org.labkey.api.data.measurement.UnitsDataColumn.getFieldKey; - -public class AmountDataColumn extends DataColumn -{ - public static final String AMOUNT_FIELD_PROPERTY_NAME = "amountField"; - - private final MultiValuedMap _properties; - private final FieldKey _unitsField; - private final FieldKey _defaultUnitsField; - private final FieldKey _amountField; - - public static class Factory implements DisplayColumnFactory - { - private final MultiValuedMap _properties; // metadata XML column properties - - // factory for metadata XML loading - public Factory(MultiValuedMap properties) - { - _properties = properties; - } - - @Override - public DisplayColumn createRenderer(ColumnInfo col) - { - if (_properties != null) - return new AmountDataColumn(col, _properties); - else - throw new IllegalArgumentException("Cannot create a renderer from the specified configuration properties"); - } - } - - public AmountDataColumn(ColumnInfo col, MultiValuedMap properties) - { - super(col, false); - _properties = properties; - FieldKey fieldKeyParent = getBoundColumn().getFieldKey().getParent(); - _unitsField = getFieldKey(fieldKeyParent, _properties, UNITS_FIELD_PROPERTY_NAME, "Units"); - _defaultUnitsField = getFieldKey(fieldKeyParent, _properties, DEFAULT_UNITS_FIELD_PROPERTY_NAME, null); - _amountField = getFieldKey(fieldKeyParent, _properties, AMOUNT_FIELD_PROPERTY_NAME, null); - } - - private Object convertToDisplayUnits(RenderContext ctx) - { - Double storedAmount; - Object amountObj = ctx.get(_amountField); - // Issue 48500: For the MultiValuedDisplayColumn this may be an empty string when the actual value is null - if (amountObj instanceof String) - if (StringUtils.isEmpty((String) amountObj)) - storedAmount = null; - else - storedAmount = Double.parseDouble((String) amountObj); - else - storedAmount = (Double) amountObj; - String sampleTypeUnitsStr = (String) ctx.get(_defaultUnitsField); - Measurement.Unit sampleTypeUnit = null; - if (!StringUtils.isEmpty(sampleTypeUnitsStr)) - { - try - { - sampleTypeUnit = Measurement.Unit.valueOf(sampleTypeUnitsStr); - } - catch (IllegalArgumentException e) - { - // do nothing, return unconverted amount - } - } - - if (storedAmount == null) - return null; - - if (ctx.get(_unitsField) == null) - { - int scale = sampleTypeUnit == null ? DEFAULT_PRECISION_SCALE : sampleTypeUnit.getPrecisionScale(); - return Precision.round(storedAmount, scale); - } - - try - { - Measurement.Unit storedUnit = Measurement.Unit.valueOf(ctx.get(_unitsField).toString()); - if (storedUnit.isCompatible(sampleTypeUnit)) - return storedUnit.convertAmountForDisplay(storedAmount, sampleTypeUnit); - else - return storedAmount; - } - catch (IllegalArgumentException e) // units provided are not supported. - { - return storedAmount; - } - } - - @Override - public void addQueryFieldKeys(Set keys) - { - super.addQueryFieldKeys(keys); - keys.add(_unitsField); - if (_defaultUnitsField != null) - keys.add(_defaultUnitsField); - } - - @Override - public @NotNull HtmlString getFormattedHtml(RenderContext ctx) - { - Object convertedValue = convertToDisplayUnits(ctx); - return HtmlString.unsafe(convertedValue == null ? null : convertedValue.toString()); - } - - @Override - public Object getDisplayValue(RenderContext ctx) - { - return convertToDisplayUnits(ctx); - } -} diff --git a/api/src/org/labkey/api/data/measurement/Measurement.java b/api/src/org/labkey/api/data/measurement/Measurement.java deleted file mode 100644 index 1ad683c8a0d..00000000000 --- a/api/src/org/labkey/api/data/measurement/Measurement.java +++ /dev/null @@ -1,506 +0,0 @@ -package org.labkey.api.data.measurement; - -import org.apache.commons.lang3.StringUtils; -import org.apache.commons.math3.util.Precision; -import org.jetbrains.annotations.Nullable; -import org.junit.Assert; -import org.junit.Test; -import org.labkey.api.data.ConversionExceptionWithMessage; - -import java.math.BigDecimal; -import java.util.regex.Matcher; -import java.util.regex.Pattern; - -import static org.labkey.api.util.IntegerUtils.isIntegral; - -public class Measurement -{ - public static final int DEFAULT_PRECISION_SCALE = 6; - - private static final String NUMBER_REGEX = "[+\\-]?\\d+(?:\\.\\d+)?(?:[Ee][+\\-]\\d+)?"; - private static final Pattern AMOUNT_AND_UNITS_PATTERN = Pattern.compile("\\s*(?" + NUMBER_REGEX + ")?\\s*(?\\S*)\\s*"); - private Unit _units; - private Double _amount; - private Unit _normalizingUnits; - - public enum Kind - { - Mass, - Volume, - Count - } - - public enum Unit - { - g("grams", Kind.Mass, 1, 9), - mg("milligrams", Kind.Mass, 0.001, 6), - kg("kilograms", Kind.Mass, 1000, 12), - mL("milliliters", Kind.Volume, 1, 6), - uL("microliters", Kind.Volume, 0.001, 3, "μL"), - L("liters", Kind.Volume, 1000, 9), - kL("kiloliters", Kind.Volume, 100000, 12), - unit("units", Kind.Count, 1, 2); - - private final String _longLabel; - private final Kind _kind; - private final double _ratio; - private final String _alternateName; - private final int _precisionScale; - - Unit(String longLabel, Kind kind, double ratio, int precisionScale) - { - this(longLabel, kind, ratio, precisionScale, null); - } - - Unit(String longLabel, Kind kind, double ratio, int precisionScale, String alternateName) - { - _longLabel = longLabel; - _kind = kind; - _ratio = ratio; - _precisionScale = precisionScale; - _alternateName = alternateName; - } - - public String getLongLabel() - { - return _longLabel; - } - - public Kind getKind() - { - return _kind; - } - - public double getRatio() - { - return _ratio; - } - - public String getAlternateName() - { - return _alternateName; - } - - public int getPrecisionScale() - { - return _precisionScale; - } - - public boolean isCompatible(Unit otherUnit) - { - return otherUnit != null && this._kind == otherUnit._kind; - } - - public static Unit getUnit(String unitString) - { - Unit unit = getUnitFromName(unitString); - if (unit != null) - return unit; - return getUnitFromLabel(unitString); - } - - public static Unit getUnitFromLabel(String label) - { - if (label == null) - return null; - - for (Unit unit : Unit.values()) - { - if (label.equalsIgnoreCase(unit.getLongLabel())) - return unit; - } - return null; - } - - public static Unit getUnitFromName(String name) - { - if (name == null) - return null; - - for (Unit unit : Unit.values()) - { - if (name.equalsIgnoreCase(unit.name()) || name.equalsIgnoreCase(unit.getAlternateName())) - return unit; - } - return null; - } - - public Double convertAmountForDisplay(@Nullable Double amount, @Nullable Measurement.Unit targetUnit) - { - Double converted = convertAmount(amount, targetUnit); - if (converted == null) - return null; - - return Precision.round(converted, targetUnit == null ? DEFAULT_PRECISION_SCALE : targetUnit.getPrecisionScale()); - } - - public Double convertAmount(@Nullable Double amount, @Nullable Measurement.Unit targetUnit) - { - if (amount == null) - return amount; - - if (targetUnit == null || targetUnit == this) - return amount; - - if (!this.isCompatible(targetUnit)) - throw new IllegalArgumentException("The target unit (" + targetUnit + ") is not compatible with the given unit (" + this + ")."); - - return amount * (this.getRatio() / targetUnit.getRatio()); - } - - public static boolean isCompatible(String stringA, String stringB) - { - if (StringUtils.isBlank(stringA) && StringUtils.isBlank(stringB)) - return true; - - if (StringUtils.isBlank(stringA) || StringUtils.isBlank(stringB)) - return false; - - if (stringA.equals(stringB)) - return true; - - Unit unitA = getUnitFromName(stringA); - Unit unitB = getUnitFromName(stringB); - - if (unitA == null ||unitB == null) - return false; - - return unitA.getKind() == unitB.getKind(); - - } - - public static class TestCase extends Assert - { - @Test - public void testIsCompatibleString() - { - assertTrue("empty string should be compatible with null", Unit.isCompatible(" ", null)); - assertTrue("empty string should be compatible with null", Unit.isCompatible(null, "")); - assertTrue("null strings should be compatible", Unit.isCompatible(null, null)); - assertTrue("empty string should be compatible with other empty string", Unit.isCompatible(" ", " ")); - assertFalse("empty string should be not be compatible with non-empty string", Unit.isCompatible("mg", " ")); - assertTrue("same string should be compatible", Unit.isCompatible("mL", "mL")); - assertTrue("units with same base should be compatible", Unit.isCompatible("mL", "L")); - assertTrue("units with same base should be compatible", Unit.isCompatible("kG", "g")); - assertFalse("units with different base should not be compatible", Unit.isCompatible("mg", "units")); - assertFalse("invalid unit should not be compatible with valid unit", Unit.isCompatible("xL", "mL")); - assertFalse("invalid units should not be compatible", Unit.isCompatible("xL", "tsp")); - } - - @Test - public void testConvertAmount() - { - assertEquals("convert to same unit", (Double) 1.0, Unit.g.convertAmount(1.0, Unit.g)); - assertEquals("convert to larger unit", (Double) 0.001, Unit.g.convertAmount(1.0, Unit.kg)); - assertEquals("convert to smaller unit", (Double) 1000.0, Unit.g.convertAmount(1.0, Unit.mg)); - assertNull("convert null amount", Unit.mL.convertAmount(null, Unit.L)); - try { - Unit.g.convertAmount(2.0, Unit.kL); - fail("Conversion to incompatible unit should throw exception."); - } catch (IllegalArgumentException ignore) - { - // do nothing - } - } - - @Test - public void testGetUnit() - { - assertNull("invalid unit label should return null", getUnit("invalid")); - assertNull("invalid unit string should return null", getUnit("inv")); - assertEquals("valid unit should return object", Unit.g, getUnit("g")); - assertEquals("valid unit should return object", Unit.g, getUnit("grams")); - } - } - } - - public Measurement(Object amountObj, String units, @Nullable String normalizingUnits) throws ConversionExceptionWithMessage - { - _amount = convertToAmount(amountObj); - _normalizingUnits = Unit.getUnit(normalizingUnits); - validateUnits(units, _normalizingUnits); - _units = Unit.getUnit(units); - } - - public Measurement(Object amountObj, String units) - { - this(amountObj, units, null); - } - - public Double getNormalizedAmount() - { - // if there's no unit associated, the amount should already be in the normalizing unit - if (_units == null) - return getAmount(); - - if (_units.isCompatible(_normalizingUnits)) - return _units.convertAmount(getAmount(), _normalizingUnits); - else - return getAmount(); - } - - public Unit getNormalizedUnits() - { - return _normalizingUnits != null ? _normalizingUnits : _units; - } - - public Unit getUnits() - { - return _units; - } - - public void setUnits(Unit units) - { - _units = units; - } - - public void setUnits(String unitsStr) - { - _units = Unit.getUnit(unitsStr); - } - - public Double getAmount() - { - return _amount; - } - - public void setAmount(Double amount) - { - _amount = amount; - } - - public Unit getNormalizingUnits() - { - return _normalizingUnits; - } - - public void setNormalizingUnits(Unit normalizingUnits) - { - _normalizingUnits = normalizingUnits; - } - - @Override - public boolean equals(Object obj) - { - if (!(obj instanceof Measurement other)) - return false; - try - { - Double thisNormalized = this.getNormalizedAmount(); - Double otherNormalized = other.getNormalizedAmount(); - if (thisNormalized == null) - { - return otherNormalized == null; - } - else - { - return thisNormalized.equals(otherNormalized); - } - } - catch (IllegalArgumentException e) - { - return false; - } - } - - @Override - public String toString() - { - if (getUnits() == null) - return String.valueOf(getAmount()); - - return String.format("%f %s", getAmount(), getUnits()); - } - - public String toNormalizedString() - { - return String.format("%f %s", getNormalizedAmount(), getNormalizedUnits()); - } - - public static Double convertToAmount(Object amountObj) - { - if (amountObj == null) - return null; - if (amountObj instanceof Double || amountObj instanceof Float || isIntegral(amountObj)) - { - return ((Number)amountObj).doubleValue(); - } - else if (amountObj instanceof BigDecimal) - { - return ((BigDecimal) amountObj).doubleValue(); - } - else if (amountObj instanceof String) - try - { - if (StringUtils.isBlank((String) amountObj)) - return null; - return Double.valueOf((String) amountObj); - } - catch (NumberFormatException e) - { - throw new ConversionExceptionWithMessage("Amount (" + amountObj + ") must be a number."); - } - else - throw new ConversionExceptionWithMessage("Amount (" + amountObj + ") must be a number."); - } - - public static Measurement parse(String stringValue) - { - if (StringUtils.isBlank(stringValue)) - return null; - Matcher matcher = AMOUNT_AND_UNITS_PATTERN.matcher(stringValue); - if (!matcher.matches()) - return null; - return new Measurement(convertToAmount(matcher.group("amount")), matcher.group("units")); - } - - public static void validateUnits(String rawUnits, Unit defaultUnits) - { - if (!StringUtils.isBlank(rawUnits)) - { - rawUnits = rawUnits.trim(); - - Unit mUnit = Unit.getUnitFromName(rawUnits); - if (mUnit == null) - { - mUnit = Unit.getUnitFromLabel(rawUnits); - if (mUnit == null) - throw new ConversionExceptionWithMessage("Unsupported Units value (" + rawUnits + "). Supported values are: " + StringUtils.join(Unit.values(), ", ") + "."); - } - if (defaultUnits != null && mUnit.getKind() != defaultUnits.getKind()) - throw new ConversionExceptionWithMessage("Units value (" + rawUnits + ") cannot be converted to the default units (" + defaultUnits + ")."); - } - } - - public static class TestCase extends Assert - { - - @Test - public void testValidateUnits() - { - try - { - Measurement.validateUnits("g", Unit.mg); - Measurement.validateUnits("g ", Unit.mg); - Measurement.validateUnits(" g ", Unit.mg); - } - catch (ConversionExceptionWithMessage e) - { - fail("Compatible unit should not throw exception."); - } - try - { - Measurement.validateUnits(null, Unit.unit); - } - catch (ConversionExceptionWithMessage e) - { - fail("null units should validate"); - } - try - { - Measurement.validateUnits("", Unit.unit); - } - catch (ConversionExceptionWithMessage e) - { - fail("empty units should validate"); - } - try - { - Measurement.validateUnits("g", Unit.unit); - fail("Units that are not comparable should throw exception."); - } - catch (ConversionExceptionWithMessage ignore) - { - - } - - try - { - Measurement.validateUnits("nonesuch", Unit.unit); - fail("Invalid units should throw exception."); - } - catch (ConversionExceptionWithMessage ignore) - { - - } - - } - - @Test - public void testConvertToAmount() - { - assertNull("null value should convert to null", Measurement.convertToAmount(null)); - assertNull("empty string conversion", Measurement.convertToAmount("")); - assertNull("blank string conversion", Measurement.convertToAmount(" ")); - assertEquals("string conversion", (Double) 32.0, Measurement.convertToAmount("32.0")); - assertEquals("integer conversion", (Double) 18.0, Measurement.convertToAmount(18)); - assertEquals("double conversion", (Double) 819.2, Measurement.convertToAmount(819.2)); - try - { - Measurement.convertToAmount("not-a-number"); - fail("conversion of non-number should throw exception"); - } - catch (ConversionExceptionWithMessage ignore) - { - - } - } - - @Test - public void testEquals() - { - Measurement measurement = new Measurement("43.2", "g", "milligrams"); - assertNotEquals("non-unit object", "43.2 g", measurement); - assertNotEquals("different amounts", new Measurement("23.4", "g", "milligrams"), measurement); - assertEquals("same units", new Measurement("43.2", "g", "mg"), measurement); - assertEquals("different units", new Measurement("43200", "mg", "mg"), measurement); - assertEquals("no normalizing unit", new Measurement("43200", "mg", null), measurement); - assertEquals("case-insensitive", new Measurement("43200", "MilliGRAMS", null), measurement); - } - - @Test - public void testParse() - { - assertNull("Empty string should result in null object", Measurement.parse("")); - assertNull("Null should result in null object", Measurement.parse(null)); - assertNull("Blank string should result in null object", Measurement.parse(" ")); - assertNull("Parse with multiple-word units failed", Measurement.parse("878.8 micro liters")); - - try - { - Measurement.parse("71.9141x"); - fail("Unsupported unit type 'x' not detected"); - } - catch (ConversionExceptionWithMessage ignore) - { - // expected - } - try - { - Measurement.parse("cups"); - fail("Unsupported unit type 'cups' not detected"); - } - catch (ConversionExceptionWithMessage ignore) - { - // expected - } - assertEquals("String with just a number did not parse", new Measurement(43.2431, null), Measurement.parse("43.2431")); - assertEquals("String with just units did not parse", new Measurement(null, "grams"), Measurement.parse("g")); - assertEquals("String with number and units did not parse", new Measurement(43.2, "g"), Measurement.parse("43.2 g")); - assertEquals("String with number and units without a space between did not parse", new Measurement(43.2, "g"), Measurement.parse("43.2g")); - assertEquals("String with number and units without a space between did not parse", new Measurement(43.2, "g"), Measurement.parse("43.2grams")); - assertEquals("String with integer and units did not parse", new Measurement(42, "g"), Measurement.parse("42 g")); - assertEquals("String with space padding before number did not parse", new Measurement(43.2, "g"), Measurement.parse(" 43.2 g")); - assertEquals("String with full unit name did not parse", new Measurement(43.2, "g"), Measurement.parse("43.2 grams")); - assertEquals("String with full unit name and padding did not parse", new Measurement(43.2, "mg"), Measurement.parse(" 43.2 milligrams ")); - assertEquals("String with complex number did not parse", new Measurement(-189140.0, "g"), Measurement.parse("-18.914e+4 g")); - assertEquals("String with leading plus did not parse", new Measurement(183.0, "mL"), Measurement.parse("+183 mL")); - assertEquals("Other complex number did not parse", new Measurement(1.83431, "mL"), Measurement.parse("+183431E-5 units")); - assertEquals("Parse of units not matching case failed", new Measurement(878.8, "g"), Measurement.parse("878.8 G")); - assertEquals("Parse of full unit name not matching case failed", new Measurement(878.8, "g"), Measurement.parse("878.8 GRaMS")); - assertEquals("Parse with alternate name failed", new Measurement(878.8, "uL"), Measurement.parse("878.8 μL")); - } - } - - -} diff --git a/api/src/org/labkey/api/data/measurement/UnitsDataColumn.java b/api/src/org/labkey/api/data/measurement/UnitsDataColumn.java deleted file mode 100644 index db0772459a7..00000000000 --- a/api/src/org/labkey/api/data/measurement/UnitsDataColumn.java +++ /dev/null @@ -1,139 +0,0 @@ -package org.labkey.api.data.measurement; - -import org.apache.commons.collections4.MultiValuedMap; -import org.apache.commons.lang3.StringUtils; -import org.jetbrains.annotations.NotNull; -import org.labkey.api.data.ColumnInfo; -import org.labkey.api.data.DataColumn; -import org.labkey.api.data.DisplayColumn; -import org.labkey.api.data.DisplayColumnFactory; -import org.labkey.api.data.RenderContext; -import org.labkey.api.query.FieldKey; -import org.labkey.api.util.HtmlString; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Set; - -public class UnitsDataColumn extends DataColumn -{ - public static final String UNITS_FIELD_PROPERTY_NAME = "unitsField"; - public static final String DEFAULT_UNITS_FIELD_PROPERTY_NAME = "defaultUnitsField"; - public static final String ALTERNATE_UNITS_FIELD_PROPERTY_NAME = "alternateUnitsField"; - - private final MultiValuedMap _properties; - private final FieldKey _unitsField; - private final FieldKey _defaultUnitsField; - private final FieldKey _alternateUnitsField; - - public static class Factory implements DisplayColumnFactory - { - private final MultiValuedMap _properties; // metadata XML column properties - - // factory for metadata XML loading - public Factory(MultiValuedMap properties) - { - _properties = properties; - } - - @Override - public DisplayColumn createRenderer(ColumnInfo col) - { - return new UnitsDataColumn(col, _properties); - } - } - - UnitsDataColumn(ColumnInfo colInfo, MultiValuedMap properties) - { - super(colInfo, false); - _properties = properties; - - FieldKey fieldKeyParent = getBoundColumn().getFieldKey().getParent(); - _unitsField = getFieldKey(fieldKeyParent, _properties, UNITS_FIELD_PROPERTY_NAME, "Units"); - _defaultUnitsField = getFieldKey(fieldKeyParent, _properties, DEFAULT_UNITS_FIELD_PROPERTY_NAME, null); - _alternateUnitsField = getFieldKey(fieldKeyParent, _properties, ALTERNATE_UNITS_FIELD_PROPERTY_NAME, null); - } - - static FieldKey getFieldKey(FieldKey fieldKeyParent, MultiValuedMap properties, String propertyName, String defaultValue) - { - List keyParts = new ArrayList<>(); - if (fieldKeyParent != null) - keyParts.addAll(fieldKeyParent.getParts()); - int parentPartsSize = keyParts.size(); - if (properties != null) - { - String fieldName = properties.get(propertyName).stream().findFirst().orElse(defaultValue); - if (fieldName != null) - keyParts.addAll(Arrays.asList(fieldName.split("/"))); - } - else if (defaultValue != null) - keyParts.add(defaultValue); - return parentPartsSize == keyParts.size() ? null : FieldKey.fromParts(keyParts); - } - - // Display the units of the converted value - private Object getDisplayUnits(RenderContext ctx) - { - String defaultUnitsStr = _defaultUnitsField != null ? (String) ctx.get(_defaultUnitsField) : null; - Measurement.Unit unit = StringUtils.isEmpty(defaultUnitsStr) ? null : Measurement.Unit.valueOf(defaultUnitsStr); - - String displayUnitStr; - Object units = ctx.get(_unitsField); - if (units == null) - displayUnitStr = defaultUnitsStr; - else - { - String unitsStr = units.toString().trim(); - try - { - // if units is compatible with default unit, use default units otherwise, use unit - Measurement.Unit storedUnit = Measurement.Unit.valueOf(unitsStr); - if (!storedUnit.isCompatible(unit)) - return units; - else - displayUnitStr = defaultUnitsStr; - } - catch (IllegalArgumentException e) - { - displayUnitStr = unitsStr; - } - } - - // if neither default unit nor item unit is available, use alternate unit for display - if (StringUtils.isEmpty(displayUnitStr) && _alternateUnitsField != null) - { - String alternateUnitStr = (String) ctx.get(_alternateUnitsField); - - if (!StringUtils.isEmpty(alternateUnitStr)) - displayUnitStr = alternateUnitStr; - } - - return displayUnitStr; - - } - - @Override - public void addQueryFieldKeys(Set keys) - { - super.addQueryFieldKeys(keys); - if (_defaultUnitsField != null) - keys.add(_defaultUnitsField); - if (_alternateUnitsField != null) - keys.add(_alternateUnitsField); - } - - @Override - public @NotNull HtmlString getFormattedHtml(RenderContext ctx) - { - Object displayUnits = getDisplayUnits(ctx); - return HtmlString.unsafe(displayUnits == null ? null : displayUnits.toString()); - } - - @Override - public Object getDisplayValue(RenderContext ctx) - { - return getDisplayUnits(ctx); - } - -} diff --git a/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java b/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java index 70678f012b6..3f102575ab5 100644 --- a/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/DetailedAuditLogDataIterator.java @@ -16,6 +16,7 @@ package org.labkey.api.dataiterator; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.labkey.api.audit.AuditHandler; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; @@ -29,6 +30,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Map; +import java.util.function.Function; import static org.labkey.api.gwt.client.AuditBehaviorType.DETAILED; @@ -53,12 +55,14 @@ public enum AuditConfigs { final QueryService.AuditAction _auditAction; final AuditHandler _auditHandler; final boolean _useTransactionAuditCache; + Function, Map> _extractProvidedValues; // for batching final ArrayList> _updatedRows = new ArrayList<>(); + final ArrayList> _providedValues = new ArrayList<>(); final ArrayList> _existingRows; - protected DetailedAuditLogDataIterator(DataIterator data, DataIteratorContext context, TableInfo table, QueryService.AuditAction auditAction, User user, Container c) + protected DetailedAuditLogDataIterator(DataIterator data, DataIteratorContext context, TableInfo table, QueryService.AuditAction auditAction, User user, Container c, @Nullable Function, Map> extractProvidedValues) { super(context); _table = table; @@ -69,6 +73,7 @@ protected DetailedAuditLogDataIterator(DataIterator data, DataIteratorContext co _useTransactionAuditCache = !context.getInsertOption().updateOnly && context.isUseTransactionAuditCache(); _auditAction = auditAction; _auditHandler = table.getAuditHandler(DETAILED); + _extractProvidedValues = extractProvidedValues; assert DETAILED == table.getEffectiveAuditBehavior((AuditBehaviorType) context.getConfigParameter(AuditConfigs.AuditBehavior)); assert !context.getInsertOption().mergeRows || _data.supportsGetExistingRecord(); @@ -98,14 +103,20 @@ public boolean next() throws BatchValidationException if (!hasNext || _updatedRows.size() > 1000) { if (!_updatedRows.isEmpty()) - _auditHandler.addAuditEvent(_user, _container, _table, DETAILED, _userComment, _auditAction, _updatedRows, _existingRows, _useTransactionAuditCache); + _auditHandler.addAuditEvent(_user, _container, _table, DETAILED, _userComment, _auditAction, _updatedRows, _existingRows, _providedValues, _useTransactionAuditCache); _updatedRows.clear(); if (null != _existingRows) _existingRows.clear(); } if (hasNext) { - _updatedRows.add(_data.getMap()); + Map map = _data.getMap(); + _updatedRows.add(map); + if (_extractProvidedValues != null) + _providedValues.add(_extractProvidedValues.apply(map)); + else + _providedValues.add(null); + if (null != _existingRows) _existingRows.add(_data.getExistingRecord()); } @@ -124,7 +135,7 @@ public void close() throws IOException _data.close(); } - public static DataIteratorBuilder getDataIteratorBuilder(TableInfo queryTable, @NotNull final DataIteratorBuilder builder, QueryUpdateService.InsertOption insertOption, final User user, final Container container) + public static DataIteratorBuilder getDataIteratorBuilder(TableInfo queryTable, @NotNull final DataIteratorBuilder builder, QueryUpdateService.InsertOption insertOption, final User user, final Container container, @Nullable Function, Map> extractProvidedValues) { return context -> { @@ -137,7 +148,7 @@ public static DataIteratorBuilder getDataIteratorBuilder(TableInfo queryTable, @ { DataIterator it = builder.getDataIterator(context); DataIterator in = DataIteratorUtil.wrapMap(it, true); - return new DetailedAuditLogDataIterator(in, context, queryTable, insertOption.auditAction, user, container); + return new DetailedAuditLogDataIterator(in, context, queryTable, insertOption.auditAction, user, container, extractProvidedValues); } // Nothing to do, so just return input DataIterator return builder.getDataIterator(context); diff --git a/api/src/org/labkey/api/dataiterator/ExistingRecordDataIterator.java b/api/src/org/labkey/api/dataiterator/ExistingRecordDataIterator.java index 09f0cbac7e8..17d143deb40 100644 --- a/api/src/org/labkey/api/dataiterator/ExistingRecordDataIterator.java +++ b/api/src/org/labkey/api/dataiterator/ExistingRecordDataIterator.java @@ -42,8 +42,8 @@ import java.util.function.Supplier; import java.util.stream.Collectors; -import static org.labkey.api.util.IntegerUtils.asInteger; import static org.labkey.api.gwt.client.AuditBehaviorType.DETAILED; +import static org.labkey.api.util.IntegerUtils.asInteger; public abstract class ExistingRecordDataIterator extends WrapperDataIterator { @@ -209,8 +209,6 @@ public Map getExistingRecord() return (Map)get(existingColIndex); } - - public static DataIteratorBuilder createBuilder(DataIteratorBuilder dib, TableInfo target, @Nullable Set keys) { return createBuilder(dib, target, keys, null, false); diff --git a/api/src/org/labkey/api/exp/api/ExpMaterial.java b/api/src/org/labkey/api/exp/api/ExpMaterial.java index 5677762657d..95bde41b9f7 100644 --- a/api/src/org/labkey/api/exp/api/ExpMaterial.java +++ b/api/src/org/labkey/api/exp/api/ExpMaterial.java @@ -70,8 +70,10 @@ public interface ExpMaterial extends ExpRunItem // rollup - begin int getAliquotCount(); - double getAliquotVolume(); + Double getAliquotVolume(); String getAliquotUnit(); + void setAliquotUnit(String unit); + Double getAvailableAliquotVolume(); // rollup - end boolean isOperationPermitted(SampleTypeService.SampleOperations operation); diff --git a/api/src/org/labkey/api/exp/api/ExpSampleType.java b/api/src/org/labkey/api/exp/api/ExpSampleType.java index 3e746633d00..496afe9e2c8 100644 --- a/api/src/org/labkey/api/exp/api/ExpSampleType.java +++ b/api/src/org/labkey/api/exp/api/ExpSampleType.java @@ -24,6 +24,7 @@ import org.labkey.api.exp.Lsid; import org.labkey.api.exp.property.Domain; import org.labkey.api.exp.property.DomainProperty; +import org.labkey.api.ontology.Unit; import org.labkey.api.security.User; import org.labkey.api.util.StringExpressionFactory; import org.labkey.api.view.ActionURL; @@ -134,6 +135,9 @@ public interface ExpSampleType extends ExpObject, ExpSearchable @Nullable String getMetricUnit(); + /** @return the base unit corresponding to the sample type's display unit (metricUnit) */ + @Nullable Unit getBaseUnit(); + /** @return Auto link target container if set. */ @Nullable Container getAutoLinkTargetContainer(); diff --git a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java index 8be8d19a58b..fba5debed21 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java @@ -64,6 +64,7 @@ import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.StringUtilsLabKey; import org.labkey.api.util.UnexpectedException; +import org.labkey.api.util.logging.LogHelper; import org.labkey.api.view.ActionURL; import org.labkey.api.view.NotFoundException; import org.labkey.api.writer.ContainerUser; @@ -82,7 +83,7 @@ public class SampleTypeDomainKind extends AbstractDomainKind { - private static final Logger logger; + private static final Logger logger = LogHelper.getLogger(SampleTypeDomainKind.class, "Sample type domain kind"); public static final String NAME = "SampleSet"; public static final String PROVISIONED_SCHEMA_NAME = "expsampleset"; public static final String SAMPLETYPE_FILE_DIRECTORY = "sampletype"; @@ -156,8 +157,6 @@ public class SampleTypeDomainKind extends AbstractDomainKind, UpdateableTableInfo { enum Column @@ -26,10 +30,10 @@ enum Column Alias, AliquotCount, AliquotUnit, - AliquotVolume, + AliquotVolume(true), AliquotedFromLSID, AvailableAliquotCount, - AvailableAliquotVolume, + AvailableAliquotVolume(true), Created, CreatedBy, Description, @@ -48,7 +52,7 @@ enum Column Properties, Property, QueryableInputs, - RawAmount, + RawAmount(true), RawUnits, RootMaterialRowId, RowId, @@ -60,13 +64,51 @@ enum Column SourceApplicationInput, SourceProtocolApplication, SourceProtocolLSID, - StoredAmount, + StoredAmount(true, "Amount"), Units; + private boolean _hasUnit = false; + private final String _label; + Column() { + _label = ColumnInfo.labelFromName(name()); + } + + Column(boolean hasUnit) + { + this(); + _hasUnit = hasUnit; + } + + Column(boolean hasUnit, String label) + { + _hasUnit = hasUnit; + _label = label; + } + public FieldKey fieldKey() { return FieldKey.fromParts(name()); } + + public boolean hasUnit() + { + return _hasUnit; + } + + public String label() + { + return _label; + } + + public Set namesAndLabels() + { + Set values = new CaseInsensitiveHashSet(); + + values.add(this.name()); + values.add(this.label()); + values.add(this.label().replaceAll("\\s", "")); + return values; + } } default void setSupportTableRules(boolean supportTableRules) diff --git a/api/src/org/labkey/api/exp/query/ExpSchema.java b/api/src/org/labkey/api/exp/query/ExpSchema.java index f302e8ab126..3670576fcd0 100644 --- a/api/src/org/labkey/api/exp/query/ExpSchema.java +++ b/api/src/org/labkey/api/exp/query/ExpSchema.java @@ -28,7 +28,6 @@ import org.labkey.api.data.ForeignKey; import org.labkey.api.data.TableInfo; import org.labkey.api.data.UnionContainerFilter; -import org.labkey.api.data.measurement.Measurement; import org.labkey.api.exp.api.ExpRun; import org.labkey.api.exp.api.ExpSampleType; import org.labkey.api.exp.api.ExperimentService; @@ -37,6 +36,8 @@ import org.labkey.api.exp.property.Lookup; import org.labkey.api.gwt.client.AuditBehaviorType; import org.labkey.api.module.Module; +import org.labkey.api.ontology.KindOfQuantity; +import org.labkey.api.ontology.Unit; import org.labkey.api.query.DefaultSchema; import org.labkey.api.query.FieldKey; import org.labkey.api.query.LookupForeignKey; @@ -55,6 +56,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.EnumSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Objects; @@ -388,7 +390,9 @@ public TableInfo createTable(String name, ContainerFilter cf) if (MEASUREMENT_UNITS_TABLE.equalsIgnoreCase(name)) { - EnumTableInfo table = new EnumTableInfo<>(Measurement.Unit.class, this, Measurement.Unit::name, false, "Contains the list of available units for measurements such as sample stored amounts."); + // Create an EnumSet of the KindOfQuantity getCommonUnits + List commonUnits = KindOfQuantity.getSupportedUnits(); + EnumTableInfo table = new EnumTableInfo<>(Unit.class, EnumSet.copyOf(commonUnits), this, Unit::name, Unit::ordinal, false, "Contains the list of available units for measurements such as sample stored amounts."); table.setPublicSchemaName(this.getName()); table.setName(MEASUREMENT_UNITS_TABLE); return table; diff --git a/api/src/org/labkey/api/inventory/InventoryService.java b/api/src/org/labkey/api/inventory/InventoryService.java index 2203a2f809a..7690e2c8993 100644 --- a/api/src/org/labkey/api/inventory/InventoryService.java +++ b/api/src/org/labkey/api/inventory/InventoryService.java @@ -110,7 +110,7 @@ static InventoryService get() return ServiceRegistry.get().getService(InventoryService.class); } - void addAuditEvent(User user, Container c, TableInfo table, AuditBehaviorType auditBehaviorType, @Nullable String userComment, QueryService.AuditAction action, @Nullable List> rows, @Nullable List> existingRows, boolean useTransactionAuditCache); + void addAuditEvent(User user, Container c, TableInfo table, AuditBehaviorType auditBehaviorType, @Nullable String userComment, QueryService.AuditAction action, @Nullable List> rows, @Nullable List> existingRows, @Nullable List> providedValues, boolean useTransactionAuditCache); Map moveSamples(Collection sampleIds, Container targetContainer, User user); diff --git a/api/src/org/labkey/api/ontology/KindOfQuantity.java b/api/src/org/labkey/api/ontology/KindOfQuantity.java index 3187026c2d5..ad80ae0cd00 100644 --- a/api/src/org/labkey/api/ontology/KindOfQuantity.java +++ b/api/src/org/labkey/api/ontology/KindOfQuantity.java @@ -2,6 +2,7 @@ import org.jetbrains.annotations.NotNull; +import java.util.ArrayList; import java.util.List; @@ -17,28 +18,28 @@ public enum KindOfQuantity Volume("volume", "ml") { @Override - List getCommonUnits() + public List getCommonUnits() { - return List.of(Unit.l, Unit.ml, Unit.ul); + return List.of(Unit.L, Unit.mL, Unit.uL); } }, Mass("mass", "g") { @Override - List getCommonUnits() + public List getCommonUnits() { - return List.of(Unit.kg, Unit.g, Unit.mg ,Unit.ug); + return List.of(Unit.kg, Unit.g, Unit.mg); } }, // Not a real unit per UCUM, but useful for annotation of "storage amount" for instance. - Count("", "unit") + Count("count", "unit") { @Override - List getCommonUnits() + public List getCommonUnits() { - return List.of(Unit.count, Unit.unit); + return List.of(Unit.unit); } }; @@ -64,7 +65,7 @@ List getCommonUnits() } /* unit used for database storage and in-memory representation of Quantity*/ - Unit getStorageUnit() + public Unit getStorageUnit() { if (null == storageUnit) storageUnit = Unit.fromName(storageUnitName); @@ -82,7 +83,7 @@ boolean accept(Unit unit) return getStorageUnit().base == unit.base; } - abstract List getCommonUnits(); + public abstract List getCommonUnits(); static KindOfQuantity getKindOfQuantity(String name) { @@ -99,6 +100,15 @@ static KindOfQuantity getKindOfQuantity(String name) return null; } + public static List getSupportedUnits() + { + List supported = new ArrayList<>(); + supported.addAll(Volume.getCommonUnits()); + supported.addAll(Mass.getCommonUnits()); + supported.addAll(Count.getCommonUnits()); + return supported; + } + // other potentially useful KindOfQuantity // // Fixed Unit e.g. arbitrary UCUM unit w/no conversion supported (basically a column annotation) diff --git a/api/src/org/labkey/api/ontology/Quantity.java b/api/src/org/labkey/api/ontology/Quantity.java index f68f87ddece..ca2c484db96 100644 --- a/api/src/org/labkey/api/ontology/Quantity.java +++ b/api/src/org/labkey/api/ontology/Quantity.java @@ -4,10 +4,14 @@ import org.apache.commons.beanutils.ConvertUtils; import org.apache.commons.beanutils.Converter; import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.junit.Assert; +import org.junit.BeforeClass; import org.junit.Test; import org.labkey.api.util.Formats; +import org.labkey.api.util.logging.LogHelper; import java.math.BigDecimal; import java.text.Format; @@ -55,6 +59,9 @@ */ public class Quantity extends Number implements Comparable { + public static final Logger LOG = LogHelper.getLogger(Quantity.class, "Quantity operations"); + public static final int DEFAULT_PRECISION_SCALE = 6; + public static final String DEFAULT_FORMAT = "0.######"; public final @NotNull KindOfQuantity kind; public final Number value; private final boolean isDouble; @@ -64,11 +71,36 @@ private Quantity() throw new IllegalStateException(); } + @Nullable + public static Quantity of(@Nullable Object value, @Nullable String unitsStr) + { + LOG.debug("Getting quantity of {} for unitsStr {}", value, unitsStr); + if (value == null) + return null; + if (!(value instanceof Number)) + throw new IllegalArgumentException("Value must be a number"); + if (unitsStr != null) + return Quantity.of((Number) value, unitsStr); + else + return Quantity.of((Number) value, Unit.unit); + } + + @NotNull + public static Quantity of(@NotNull Number value, @NotNull String unitStr) + { + Unit unit = Unit.fromName(unitStr); + if (unit == null) + throw new IllegalArgumentException("Unknown unit (" + unitStr + ") for quantity " + value + "."); + return of(value, unit); + } + /* Returns a quantity = value*units * of(1, Kg) -> 1000g */ - public static Quantity of(Number value, Unit unit) + @NotNull + public static Quantity of(@NotNull Number value, @NotNull Unit unit) { + LOG.debug("Creating quantity of {} for unit {}", value, unit); if (value instanceof Quantity q) { if (unit.kindOfQuantity != q.kind) @@ -79,6 +111,7 @@ public static Quantity of(Number value, Unit unit) return new Quantity(unit.kindOfQuantity, bd, unit); if (value instanceof Long l && (l > Integer.MAX_VALUE || l < Integer.MIN_VALUE)) return new Quantity(unit.kindOfQuantity, BigDecimal.valueOf(l), unit); + LOG.debug("Creating new Quantity of kind {}, double {}, unit {}", unit.kindOfQuantity, value, unit); return new Quantity(unit.kindOfQuantity, value.doubleValue(), unit); } @@ -89,7 +122,7 @@ protected Quantity(@NotNull KindOfQuantity kind, BigDecimal value) this.isDouble = false; } - protected Quantity(@NotNull KindOfQuantity kind, BigDecimal value, Unit from) + protected Quantity(@NotNull KindOfQuantity kind, BigDecimal value, @NotNull Unit from) { this.kind = kind; this.value = from.toStorageUnitValue(value); @@ -104,15 +137,29 @@ protected Quantity(@NotNull KindOfQuantity kind, Double value) this.isDouble = true; } - protected Quantity(@NotNull KindOfQuantity kind, Double value, Unit from) + protected Quantity(@NotNull KindOfQuantity kind, Double value, @NotNull Unit from) { + LOG.debug("Quantity constructor with kind {}, double {}, unit {}", kind, value, from); this.kind = kind; this.value = from.toStorageUnitValue(value); this.isDouble = this.value instanceof Double; assert isDouble || this.value instanceof BigDecimal; } - public double doubleValue(Unit unit) + public @NotNull KindOfQuantity getKind() + { + return kind; + } + + public Quantity add(Quantity delta) + { + if (delta.kind != kind) + throw new ConversionException("Cannot add " + delta + " to " + this + "."); + + return new Quantity(this.kind, this.doubleValue() + delta.doubleValue()); + } + + public double doubleValue(@NotNull Unit unit) { if (unit == kind.getStorageUnit()) return value.doubleValue(); @@ -125,7 +172,7 @@ public String format() return format(kind.getDefaultDisplayUnit()); } - public String format(Unit unit) + public String format(@NotNull Unit unit) { return value(unit) + unit.print; } @@ -135,7 +182,7 @@ public String format(Format format) return format(kind.getDefaultDisplayUnit(), format); } - public String format(Unit unit, Format format) + public String format(@NotNull Unit unit, Format format) { return format.format(value(unit)) + unit.print; } @@ -148,7 +195,10 @@ public String format(Unit unit, Format format) public String toString() { var ret = format(kind.getStorageUnit()); - assert this == ConvertUtils.convert(ret, this.getClass()); + // FIXME currently this call to ConvertUtils.convert does not behave as expected. + // When ret is something like "10mL", it is using a converter for Unit.unit + // (The theory is it's either the first or last one that was registered). +// assert this == ConvertUtils.convert(ret, this.getClass()); return ret; } @@ -175,7 +225,7 @@ public Number value() return value; } - public Number value(Unit unit) + public Number value(@NotNull Unit unit) { return unit.fromStorageUnitValue(value); } @@ -219,6 +269,7 @@ public double doubleValue() private static Quantity parse(@NotNull String s) throws ConversionException { + LOG.debug("Parsing quantity {} as Unit.unit", s); return parse(s, Unit.unit); } @@ -226,6 +277,7 @@ private static Quantity parse(@NotNull String s) throws ConversionException /** The defaultUnit has two purposes 1) define the expected KindOfQuantity 2) select a Unit if it is not explicit in the source */ private static Quantity parse(@NotNull String s, @NotNull Unit defaultUnit) throws ConversionException { + LOG.debug("Parsing quantity of {} with default unit {}", s, defaultUnit); // We could probably create a real lexer/parser here, but we only need to be able to parse units we support // FIRST, check if there is a space s = s.trim(); @@ -254,7 +306,7 @@ private static Quantity parse(@NotNull String s, @NotNull Unit defaultUnit) thro } if (StringUtils.isBlank(valuePart)) - throw new ConversionException("Could not parse number"); + throw new ConversionException("Could not parse number from '" + s + "'."); try { @@ -263,14 +315,14 @@ private static Quantity parse(@NotNull String s, @NotNull Unit defaultUnit) thro new BigDecimal(valuePart); var unit = StringUtils.isBlank(unitPart) ? defaultUnit : Unit.fromName(unitPart); if (null == unit) - throw new ConversionException("Could not parse unit: " + unitPart); + throw new ConversionException("Could not parse unit '" + unitPart + "' from '" + s + "'."); if (!defaultUnit.kindOfQuantity.accept(unit)) - throw new ConversionException("Quantity is of wrong type: expected " + defaultUnit.kindOfQuantity.getName() + " found " + unit); + throw new ConversionException("Quantity for value " + value + " is of the wrong type. Expected " + defaultUnit.kindOfQuantity.getName() + ", but found " + unit); return Quantity.of(value, unit); } catch (IllegalArgumentException x) { - throw new ConversionException("could not parse", x); + throw new ConversionException("Could not parse quantity from '" + s + "'.", x); } } @@ -299,8 +351,9 @@ public abstract static class Volume extends Quantity // convert (ala BeanUtils.Converter to Quantity - public static Quantity convert(Object o, Unit unit) + public static Quantity convert(@Nullable Object o, Unit unit) { + LOG.debug("Converting quantity {} to unit {}", o, unit); if (null == o) return null; if (o instanceof Quantity q) @@ -324,6 +377,7 @@ public static Converter converterFor(Unit unit) @Override public T convert(Class aClass, Object o) { + LOG.debug("In convertedFor.convert with object {} and unit {}", o, unit); return (T)Quantity.convert(o, unit); } }; @@ -371,6 +425,33 @@ void failToParse(String s, Unit defaultUnit) } } + @BeforeClass + public static void setup() + { + registerQuantityConverters(); + } + + @Test + public void testAdd() + { + Quantity starting = Quantity.of(1024, Unit.mg); + assertEquals(starting, starting.add(Quantity.of(0, Unit.mg))); + assertEquals(Quantity.of(1034, Unit.mg), starting.add(Quantity.of(10, Unit.mg))); + assertEquals(Quantity.of(1.022, Unit.g), starting.add(Quantity.of(-2, Unit.mg))); + assertEquals(Quantity.of(33024, Unit.mg), starting.add(Quantity.of(32, Unit.g))); + assertEquals(Quantity.of(1.024200, Unit.g), starting.add(Quantity.of(200, Unit.ug))); + assertEquals(Quantity.of(1023800, Unit.ug), starting.add(Quantity.of(-200, Unit.ug))); + try + { + starting.add(Quantity.of(10, "mL")); + fail("Adding quantities of different kinds should throw an error."); + } + catch (ConversionException x) + { + assertEquals("Cannot add 10.0mL to 1.024g.", x.getMessage()); + } + } + @Test public void testParse() { @@ -412,8 +493,8 @@ public void testFailToParseInvalidUnit() @Test public void testFailToParseCantConvert() { - failToParse("1g", Unit.l); - failToParse("1g", Unit.ml); + failToParse("1g", Unit.L); + failToParse("1g", Unit.mL); failToParse("1g", Unit.count); failToParse("1g", Unit.unit); } @@ -462,7 +543,6 @@ public void testPattern() public void testConversion() { Quantity q; - registerQuantityConverters(); q = (Quantity)ConvertUtils.convert("1.234kg", Quantity.Mass_g.class); assertEquals(Quantity.class, q.getClass()); diff --git a/api/src/org/labkey/api/ontology/Unit.java b/api/src/org/labkey/api/ontology/Unit.java index c8caaa05699..7a547dbd156 100644 --- a/api/src/org/labkey/api/ontology/Unit.java +++ b/api/src/org/labkey/api/ontology/Unit.java @@ -3,76 +3,80 @@ import lombok.Getter; import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.junit.Assert; import org.junit.Test; +import org.labkey.api.data.ConversionExceptionWithMessage; import java.util.HashMap; +import java.util.List; import java.util.function.Function; public enum Unit { - unit(KindOfQuantity.Count, null, 1.0, "", + unit(KindOfQuantity.Count, null, 1.0, 2, "unit", Quantity.class, "unit", "units"), - count(KindOfQuantity.Count, unit, 1.0, "", + count(KindOfQuantity.Count, unit, 1.0, 2, "count", Quantity.class, "count", "count"), - ml(KindOfQuantity.Volume, null, 1e0, "ml", + mL(KindOfQuantity.Volume, null, 1e0, 6, "mL", Quantity.Volume_ml.class, "milliliter", "milliliters", - "mL", "millilitre", "millilitres"), + "ml", "millilitre", "millilitres"), // UCUM prefers "l", but "L" is also common and already supported by inventory (sorry Lambert) - l(KindOfQuantity.Volume, ml, 1e3, "l", + L(KindOfQuantity.Volume, mL, 1e3, 9, "L", Quantity.Volume_l.class, "liter", "liters", - "L", "ℓ", "litre", "liters"), + "l", "ℓ", "litre", "liters"), // is it better to include these little used units, to avoid future case-sensitivity problems? - Ml(KindOfQuantity.Volume, ml, 1e9, "Ml", + ML(KindOfQuantity.Volume, mL, 1e9, 12, "ML", Quantity.Volume_Megal.class, "megaliter", "megaliters", - "ML", "megalitre", "megalitres"), - kl(KindOfQuantity.Volume, ml, 1e6, "kl", + "Ml", "megalitre", "megalitres"), + kL(KindOfQuantity.Volume, mL, 1e6, 12, "kL", Quantity.Volume_kl.class, "kiloliter", "kiloliters", - "kL", "kilolitre", "kilolitres"), - ul(KindOfQuantity.Volume, ml, 1e-3, "ul", + "kl", "kilolitre", "kilolitres"), + uL(KindOfQuantity.Volume, mL, 1e-3, 3, "uL", Quantity.Volume_ul.class, "microliter", "microliters", - "uL", "μl", "μL", "microlitre", "microlitres"), - nl(KindOfQuantity.Volume, ml, 1e-6, "nl", + "ul", "μl", "μL", "microlitre", "microlitres"), + nL(KindOfQuantity.Volume, mL, 1e-6, 3, "nL", Quantity.Volume_nl.class, "nanoliter", "nanoliters", - "nL", "nanolitre", "nanolitres"), - pl(KindOfQuantity.Volume, ml, 1e-9, "pl", + "nl", "nanolitre", "nanolitres"), + pL(KindOfQuantity.Volume, mL, 1e-9, 3, "pL", Quantity.Volume_pl.class, "picoliter", "picoliters", - "pL", "picolitre", "picolitres"), + "pl", "picolitre", "picolitres"), - g(KindOfQuantity.Mass, null, 1e0, "g", + g(KindOfQuantity.Mass, null, 1e0, 9, "g", Quantity.Mass_g.class, "gram", "grams"), - Mg(KindOfQuantity.Mass, g, 1e6, "Mg", + Mg(KindOfQuantity.Mass, g, 1e6, 12, "Mg", Quantity.Mass_Megag.class, "megagram", "megagrams", "tonne", "tonnes"), - kg(KindOfQuantity.Mass, g, 1e3, "kg", + kg(KindOfQuantity.Mass, g, 1e3, 12, "kg", Quantity.Mass_kg.class, "kilogram", "kilograms"), - mg(KindOfQuantity.Mass, g, 1e-3, "mg", + mg(KindOfQuantity.Mass, g, 1e-3, 6, "mg", Quantity.Mass_mg.class, "milligram", "milligrams"), - ug(KindOfQuantity.Mass, g, 1e-6, "ug", + ug(KindOfQuantity.Mass, g, 1e-6, 3, "ug", Quantity.Mass_ug.class, "microgram", "micrograms", "μg"), - ng(KindOfQuantity.Mass, g, 1e-9, "ng", + ng(KindOfQuantity.Mass, g, 1e-9, 3, "ng", Quantity.Mass_ng.class, "nanogram", "nanograms"), - pg(KindOfQuantity.Mass, g, 1e-12, "pg", + pg(KindOfQuantity.Mass, g, 1e-12, 3, "pg", 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; @@ -85,15 +89,19 @@ public enum Unit final @NotNull String singular; final @NotNull String plural; final String[] otherNames; + @Getter final double value; + @Getter + final int precisionScale; - Unit(KindOfQuantity kind, Unit base, double value, @NotNull String printName, - Class quantityClass, + Unit(@NotNull KindOfQuantity kind, Unit base, double value, int precisionScale, @NotNull String printName, + @NotNull Class quantityClass, @NotNull String singular, @NotNull String plural, String... otherNames) { this.kindOfQuantity = kind; this.base = null == base ? this : base; this.value = value; + this.precisionScale = precisionScale; this.print = printName; this.quantityClass = quantityClass; this.singular = singular; @@ -108,7 +116,7 @@ public boolean isBase() public boolean isCompatible(Unit other) { - return other.base == base; + return other != null && other.base == base; } public double toBaseUnitValue(double v) @@ -121,14 +129,14 @@ public double fromBaseUnitValue(double v) return v / this.value; } - public Number toStorageUnitValue(Number v) + public Number toStorageUnitValue(@NotNull Number v) { if (this == kindOfQuantity.getStorageUnit()) return v; return convert(v.doubleValue(), this, kindOfQuantity.storageUnit); } - public Number fromStorageUnitValue(Number v) + public Number fromStorageUnitValue(@NotNull Number v) { if (this == kindOfQuantity.getStorageUnit()) return v; @@ -146,6 +154,7 @@ public String toString() { for (Unit unit : Unit.values()) { + unitMap.put(unit.name(), unit); unitMap.put(unit.print, unit); unitMap.put(unit.singular, unit); unitMap.put(unit.plural, unit); @@ -156,7 +165,7 @@ public String toString() } - public static Unit fromName(String unitName) + public static Unit fromName(@Nullable String unitName) { if (StringUtils.isEmpty(unitName)) return null; @@ -176,25 +185,58 @@ static Function convertFn(Unit from, Unit to) return (x) -> to.fromBaseUnitValue(from.toBaseUnitValue(x)); } - public static double convert(double value, Unit from, Unit to) + public static double convert(double value, @NotNull Unit from, @NotNull Unit to) { + Quantity.LOG.debug("Converting value {} from {} to {}", value, from.name(), to.name()); if (from.base != to.base) throw new IllegalArgumentException("Can't convert " + from.name() + " to " + to.name()); return from==to ? value : to.fromBaseUnitValue(from.toBaseUnitValue(value)); } - public Quantity convert(Object value) + 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 public void testIsBase() { - assertTrue(Unit.ml.isBase()); - assertFalse(Unit.l.isBase()); + assertTrue(Unit.mL.isBase()); + assertFalse(Unit.L.isBase()); assertTrue(Unit.g.isBase()); assertFalse(Unit.kg.isBase()); assertTrue(Unit.unit.isBase()); @@ -204,21 +246,22 @@ public void testIsBase() @Test public void testIsCompatible() { - assertTrue(Unit.ml.isCompatible(Unit.ul)); - assertTrue(Unit.ml.isCompatible(Unit.l)); - assertFalse(Unit.ml.isCompatible(Unit.g)); + assertTrue(Unit.mL.isCompatible(Unit.uL)); + assertTrue(Unit.mL.isCompatible(Unit.L)); + assertFalse(Unit.mL.isCompatible(Unit.g)); assertTrue(Unit.g.isCompatible(Unit.mg)); - assertFalse(Unit.g.isCompatible(Unit.ml)); + assertFalse(Unit.g.isCompatible(Unit.mL)); assertTrue(Unit.unit.isCompatible(Unit.count)); - assertFalse(Unit.unit.isCompatible(Unit.ml)); + assertFalse(Unit.unit.isCompatible(Unit.mL)); + assertFalse(Unit.mL.isCompatible(null)); } @Test public void testBaseUnitValue() { - assertEquals(1e0, Unit.ml.toBaseUnitValue(1.0), 0.00001); - assertEquals(1e3, Unit.l.toBaseUnitValue(1.0), 0.00001); - assertEquals(1e-3, Unit.ul.toBaseUnitValue(1.0), 0.00001); + assertEquals(1e0, Unit.mL.toBaseUnitValue(1.0), 0.00001); + assertEquals(1e3, Unit.L.toBaseUnitValue(1.0), 0.00001); + assertEquals(1e-3, Unit.uL.toBaseUnitValue(1.0), 0.00001); assertEquals(1e0, Unit.g.toBaseUnitValue(1.0), 0.00001); assertEquals(1e-3, Unit.mg.toBaseUnitValue(1.0), 0.00001); assertEquals(1e-6, Unit.ug.toBaseUnitValue(1.0), 0.00001); @@ -228,9 +271,9 @@ public void testBaseUnitValue() @Test public void testFromBaseUnitValue() { - assertEquals(1.0, Unit.ml.fromBaseUnitValue(1e0), 0.00001); - assertEquals(1.0, Unit.l.fromBaseUnitValue(1e3), 0.00001); - assertEquals(1.0, Unit.ul.fromBaseUnitValue(1e-3), 0.00001); + assertEquals(1.0, Unit.mL.fromBaseUnitValue(1e0), 0.00001); + assertEquals(1.0, Unit.L.fromBaseUnitValue(1e3), 0.00001); + assertEquals(1.0, Unit.uL.fromBaseUnitValue(1e-3), 0.00001); assertEquals(1.0, Unit.g.fromBaseUnitValue(1e0), 0.00001); assertEquals(1.0, Unit.mg.fromBaseUnitValue(1e-3), 0.00001); assertEquals(1.0, Unit.ug.fromBaseUnitValue(1e-6), 0.00001); @@ -240,9 +283,9 @@ public void testFromBaseUnitValue() @Test public void testToStorageUnitValue() { - assertEquals(1.0, Unit.ml.toStorageUnitValue(1.0).doubleValue(), 0.00001); - assertEquals(1000.0, Unit.l.toStorageUnitValue(1.0).doubleValue(), 0.00001); - assertEquals(0.001, Unit.ul.toStorageUnitValue(1.0).doubleValue(), 0.00001); + assertEquals(1.0, Unit.mL.toStorageUnitValue(1.0).doubleValue(), 0.00001); + assertEquals(1000.0, Unit.L.toStorageUnitValue(1.0).doubleValue(), 0.00001); + assertEquals(0.001, Unit.uL.toStorageUnitValue(1.0).doubleValue(), 0.00001); assertEquals(1.0, Unit.g.toStorageUnitValue(1.0).doubleValue(), 0.00001); assertEquals(0.001, Unit.mg.toStorageUnitValue(1.0).doubleValue(), 0.00001); assertEquals(0.000001, Unit.ug.toStorageUnitValue(1.0).doubleValue(), 0.00001); @@ -252,9 +295,9 @@ public void testToStorageUnitValue() @Test public void testFromStorageUnitValue() { - assertEquals(1.0, Unit.ml.fromStorageUnitValue(1.0).doubleValue(), 0.00001); - assertEquals(0.001, Unit.l.fromStorageUnitValue(1.0).doubleValue(), 0.00001); - assertEquals(1000.0, Unit.ul.fromStorageUnitValue(1.0).doubleValue(), 0.00001); + assertEquals(1.0, Unit.mL.fromStorageUnitValue(1.0).doubleValue(), 0.00001); + assertEquals(0.001, Unit.L.fromStorageUnitValue(1.0).doubleValue(), 0.00001); + assertEquals(1000.0, Unit.uL.fromStorageUnitValue(1.0).doubleValue(), 0.00001); assertEquals(1.0, Unit.g.fromStorageUnitValue(1.0).doubleValue(), 0.00001); assertEquals(1000.0, Unit.mg.fromStorageUnitValue(1.0).doubleValue(), 0.00001); assertEquals(1000000.0, Unit.ug.fromStorageUnitValue(1.0).doubleValue(), 0.00001); @@ -264,22 +307,74 @@ public void testFromStorageUnitValue() @Test public void testFromName() { - assertEquals(Unit.ml, Unit.fromName("ml")); - assertEquals(Unit.ml, Unit.fromName("mL")); - assertEquals(Unit.ml, Unit.fromName("milliliter")); - assertEquals(Unit.ml, Unit.fromName("milliliters")); - assertEquals(Unit.ml, Unit.fromName("millilitre")); - assertEquals(Unit.ml, Unit.fromName("millilitres")); - - assertEquals(Unit.l, Unit.fromName("l")); - assertEquals(Unit.l, Unit.fromName("L")); - assertEquals(Unit.l, Unit.fromName("liter")); - assertEquals(Unit.l, Unit.fromName("liters")); - assertEquals(Unit.l, Unit.fromName("litre")); - assertEquals(Unit.l, Unit.fromName("liters")); + assertEquals(Unit.mL, Unit.fromName("ml")); + assertEquals(Unit.mL, Unit.fromName("mL")); + assertEquals(Unit.mL, Unit.fromName("milliliter")); + assertEquals(Unit.mL, Unit.fromName("milliliters")); + assertEquals(Unit.mL, Unit.fromName("millilitre")); + assertEquals(Unit.mL, Unit.fromName("millilitres")); + + assertEquals(Unit.L, Unit.fromName("l")); + assertEquals(Unit.L, Unit.fromName("L")); + assertEquals(Unit.L, Unit.fromName("liter")); + assertEquals(Unit.L, Unit.fromName("liters")); + assertEquals(Unit.L, Unit.fromName("litre")); + assertEquals(Unit.L, Unit.fromName("liters")); assertNull(Unit.fromName(null)); 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/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 7cd62f644b0..b6d9256fba2 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -16,7 +16,6 @@ package org.labkey.api.query; import org.apache.commons.beanutils.ConversionException; -import org.apache.commons.beanutils.ConvertUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.jetbrains.annotations.NotNull; @@ -78,6 +77,7 @@ import org.labkey.api.files.FileContentService; import org.labkey.api.gwt.client.AuditBehaviorType; import org.labkey.api.ontology.OntologyService; +import org.labkey.api.ontology.Quantity; import org.labkey.api.pipeline.PipeRoot; import org.labkey.api.pipeline.PipelineService; import org.labkey.api.reader.TabLoader; @@ -128,7 +128,7 @@ public abstract class AbstractQueryUpdateService implements QueryUpdateService { - private final TableInfo _queryTable; + protected final TableInfo _queryTable; private boolean _bulkLoad = false; private CaseInsensitiveHashMap _columnImportMap = null; @@ -291,7 +291,7 @@ public DataIteratorBuilder createImportDIB(User user, Container container, DataI dib = ((UpdateableTableInfo) getQueryTable()).persistRows(dib, context); dib = AttachmentDataIterator.getAttachmentDataIteratorBuilder(getQueryTable(), dib, user, context.getInsertOption().batch ? getAttachmentDirectory() : null, container, getAttachmentParentFactory()); - dib = DetailedAuditLogDataIterator.getDataIteratorBuilder(getQueryTable(), dib, context.getInsertOption(), user, container); + dib = DetailedAuditLogDataIterator.getDataIteratorBuilder(getQueryTable(), dib, context.getInsertOption(), user, container, null); return dib; } @@ -580,13 +580,15 @@ protected List> _insertRowsUsingInsertRow(User user, Contain getQueryTable().fireBatchTrigger(container, user, TableInfo.TriggerType.INSERT, true, errors, extraScriptContext); List> result = new ArrayList<>(rows.size()); + List> providedValues = new ArrayList<>(rows.size()); for (int i = 0; i < rows.size(); i++) { Map row = rows.get(i); row = normalizeColumnNames(row); try { - row = coerceTypes(row); + providedValues.add(new CaseInsensitiveHashMap<>()); + row = coerceTypes(row, providedValues.get(i), false); if (hasTableScript) { getQueryTable().fireRowTrigger(container, user, TableInfo.TriggerType.INSERT, true, i, row, null, extraScriptContext); @@ -631,19 +633,19 @@ else if (SqlDialect.isTransactionException(sqlx) && errors.hasErrors()) if (hasTableScript) getQueryTable().fireBatchTrigger(container, user, TableInfo.TriggerType.INSERT, false, errors, extraScriptContext); - addAuditEvent(user, container, QueryService.AuditAction.INSERT, null, result, null); + addAuditEvent(user, container, QueryService.AuditAction.INSERT, null, result, null, providedValues); return result; } - protected void addAuditEvent(User user, Container container, QueryService.AuditAction auditAction, @Nullable Map configParameters, @Nullable List> rows, @Nullable List> existingRows) + protected void addAuditEvent(User user, Container container, QueryService.AuditAction auditAction, @Nullable Map configParameters, @Nullable List> rows, @Nullable List> existingRows, @Nullable List> providedValues) { if (!isBulkLoad()) { AuditBehaviorType auditBehavior = configParameters != null ? (AuditBehaviorType) configParameters.get(AuditBehavior) : null; String userComment = configParameters == null ? null : (String) configParameters.get(AuditUserComment); getQueryTable().getAuditHandler(auditBehavior) - .addAuditEvent(user, container, getQueryTable(), auditBehavior, userComment, auditAction, rows, existingRows); + .addAuditEvent(user, container, getQueryTable(), auditBehavior, userComment, auditAction, rows, existingRows, providedValues); } } @@ -699,7 +701,7 @@ public List> insertRows(User user, Container container, List /** Attempt to make the passed in types match the expected types so the script doesn't have to do the conversion */ @Deprecated - protected Map coerceTypes(Map row) + protected Map coerceTypes(Map row, Map providedValues, boolean isUpdate) { Map result = new CaseInsensitiveHashMap<>(row.size()); Map columnMap = ImportAliasable.Helper.createImportMap(_queryTable.getColumns(), true); @@ -719,6 +721,11 @@ protected Map coerceTypes(Map row) { if (PropertyType.FILE_LINK.equals(col.getPropertyType())) value = ExpDataFileConverter.convert(value); + else if (col.getKindOfQuantity() != null) + { + providedValues.put(entry.getKey(), value); + value = Quantity.convert(value, col.getKindOfQuantity().getStorageUnit()); + } else value = col.getConvertFn().apply(value); } @@ -805,13 +812,15 @@ public List> updateRows(User user, Container container, List List> result = new ArrayList<>(rows.size()); List> oldRows = new ArrayList<>(rows.size()); + List> providedValues = new ArrayList<>(rows.size()); // TODO: Support update/delete without selecting the existing row -- unfortunately, we currently get the existing row to check its container matches the incoming container boolean streaming = false; //_queryTable.canStreamTriggers(container) && _queryTable.getAuditBehavior() != AuditBehaviorType.NONE; for (int i = 0; i < rows.size(); i++) { Map row = rows.get(i); - row = coerceTypes(row); + providedValues.add(new CaseInsensitiveHashMap<>()); + row = coerceTypes(row, providedValues.get(i), true); try { Map oldKey = oldKeys == null ? row : oldKeys.get(i); @@ -857,7 +866,7 @@ public List> updateRows(User user, Container container, List if (errors.hasErrors()) throw errors; - addAuditEvent(user, container, QueryService.AuditAction.UPDATE, configParameters, result, oldRows); + addAuditEvent(user, container, QueryService.AuditAction.UPDATE, configParameters, result, oldRows, providedValues); return result; } @@ -970,7 +979,7 @@ public List> deleteRows(User user, Container container, List // Fire triggers, if any, and also throw if there are any errors getQueryTable().fireBatchTrigger(container, user, TableInfo.TriggerType.DELETE, false, errors, extraScriptContext); - addAuditEvent(user, container, QueryService.AuditAction.DELETE, configParameters, result, null); + addAuditEvent(user, container, QueryService.AuditAction.DELETE, configParameters, result, null, null); return result; } @@ -995,7 +1004,7 @@ public int truncateRows(User user, Container container, @Nullable Map Amount - The amount of this sample currently on hand - - org.labkey.api.data.measurement.AmountDataColumn$Factory - - SampleSet/MetricUnit - StoredAmount - - - - - The units associated with the StoredAmount for this sample - - org.labkey.api.data.measurement.UnitsDataColumn$Factory - - SampleSet/MetricUnit - AliquotUnit - - + Aliquots Created Count false @@ -1069,24 +1052,8 @@ - - - org.labkey.api.data.measurement.AmountDataColumn$Factory - - SampleTypeUnits - StoredAmount - - - - - - org.labkey.api.data.measurement.UnitsDataColumn$Factory - - SampleTypeUnits - AliquotUnit - - - + + diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index e571878f0b2..043b8088885 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -39,6 +39,7 @@ import org.labkey.api.data.CounterDefinition; import org.labkey.api.data.DbScope; import org.labkey.api.data.ExpDataFileConverter; +import org.labkey.api.data.ImportAliasable; import org.labkey.api.data.NameGenerator; import org.labkey.api.data.RemapCache; import org.labkey.api.data.SimpleFilter; @@ -166,6 +167,8 @@ import static org.labkey.api.exp.api.ExpRunItem.INPUTS_PREFIX_LC; import static org.labkey.api.exp.api.ExperimentService.ALIASCOLUMNALIAS; import static org.labkey.api.exp.api.ExperimentService.QueryOptions.SkipBulkRemapCache; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.RawAmount; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.RawUnits; import static org.labkey.api.util.IntegerUtils.asLong; import static org.labkey.api.exp.query.ExpMaterialTable.Column.AliquotCount; import static org.labkey.api.exp.query.ExpMaterialTable.Column.AliquotVolume; @@ -474,7 +477,7 @@ public Object get(int i) return needRecac.second; } - // without existing record, or if existing record is missing root information, we have to be conservative and assume this is a new aliquot, or a amount/status update + // without existing record, or if existing record is missing root information, we have to be conservative and assume this is a new aliquot, or an amount/status update // merge: either a new record, or detailed audit disabled if (!_isUpdate) { @@ -2809,9 +2812,7 @@ private TypeData createSampleHeaderRow(ExpSampleTypeImpl sampleType, Container c samplesTable.getColumns().forEach(column -> { if (!IGNORED_FIELD_NAMES.contains(column.getName())) { - validFields.add(column.getName()); - validFields.addAll(column.getImportAliasSet()); - validFields.add(column.getLabel()); + validFields.addAll(ImportAliasable.Helper.createImportSet(column)); } }); Map aliasMap = sampleType.getImportAliases(); diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 7456235c24b..ebdd80bf14e 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -103,6 +103,7 @@ import org.labkey.api.view.ViewContext; import org.labkey.api.view.WebPartFactory; import org.labkey.api.view.WebPartView; +import org.labkey.api.view.template.WarningService; import org.labkey.api.vocabulary.security.DesignVocabularyPermission; import org.labkey.api.webdav.WebdavResource; import org.labkey.api.webdav.WebdavService; @@ -177,6 +178,9 @@ public class ExperimentModule extends SpringModule { + public static final String AMOUNT_AND_UNIT_UPGRADE_PROP = "AmountAndUnitAudit"; + public static final String TRANSACTION_ID_PROP = "AuditTransactionId"; + public static final String AUDIT_COUNT_PROP = "AuditRecordCount"; private static final String SAMPLE_TYPE_WEB_PART_NAME = "Sample Types"; private static final String PROTOCOL_WEB_PART_NAME = "Protocols"; @@ -191,7 +195,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 25.009; + return 25.010; } @Nullable @@ -273,6 +277,7 @@ protected void init() ExperimentService.get().registerObjectReferencer(ExperimentServiceImpl.get()); addModuleProperty(new LineageMaximumDepthModuleProperty(this)); + WarningService.get().register(new ExperimentWarningProvider()); } @Override diff --git a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java index 7e4153c3f76..cdfe2e821a8 100644 --- a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java +++ b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java @@ -15,23 +15,62 @@ */ package org.labkey.experiment; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; +import org.labkey.api.audit.AbstractAuditTypeProvider; +import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.AuditTypeEvent; +import org.labkey.api.audit.SampleTimelineAuditEvent; +import org.labkey.api.audit.TransactionAuditProvider; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DbScope; +import org.labkey.api.data.JdbcType; +import org.labkey.api.data.Parameter; +import org.labkey.api.data.ParameterMapStatement; +import org.labkey.api.data.PropertyManager; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.Selector; import org.labkey.api.data.SqlExecutor; import org.labkey.api.data.SqlSelector; +import org.labkey.api.data.TableInfo; import org.labkey.api.data.UpgradeCode; +import org.labkey.api.exp.api.ExpSampleType; import org.labkey.api.exp.api.ExperimentService; +import org.labkey.api.exp.api.SampleTypeService; import org.labkey.api.module.ModuleContext; +import org.labkey.api.ontology.Unit; +import org.labkey.api.query.AbstractQueryUpdateService; +import org.labkey.api.query.QueryService; +import org.labkey.api.security.LimitedUser; +import org.labkey.api.security.User; +import org.labkey.api.security.roles.SiteAdminRole; import org.labkey.api.settings.AppProps; import org.labkey.api.util.logging.LogHelper; import org.labkey.experiment.api.ClosureQueryHelper; import org.labkey.experiment.samples.SampleTimelineAuditProvider; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.AliquotUnit; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.AliquotVolume; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.AvailableAliquotVolume; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.Name; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.RowId; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.StoredAmount; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.Units; +import static org.labkey.experiment.ExperimentModule.AMOUNT_AND_UNIT_UPGRADE_PROP; +import static org.labkey.experiment.ExperimentModule.AUDIT_COUNT_PROP; +import static org.labkey.experiment.ExperimentModule.TRANSACTION_ID_PROP; + public class ExperimentUpgradeCode implements UpgradeCode { private static final Logger LOG = LogHelper.getLogger(ExperimentUpgradeCode.class, "Experiment upgrade status"); @@ -46,7 +85,7 @@ public static void addMissingSampleTypeIdsForSampleTimelineAudit(ModuleContext c DbScope scope = ExperimentService.get().getSchema().getScope(); List tableNames = new SqlSelector(scope, "SELECT StorageTableName FROM exp.domainDescriptor WHERE StorageSchemaName='audit' AND name='" + SampleTimelineAuditProvider.SampleTimelineAuditDomainKind.NAME + "'").getArrayList(String.class); if (tableNames.size() > 1) - LOG.warn("Found " + tableNames.size() + " tables for " + SampleTimelineAuditProvider.SampleTimelineAuditDomainKind.NAME); + LOG.warn("Found {} tables for " + SampleTimelineAuditProvider.SampleTimelineAuditDomainKind.NAME, tableNames.size()); try (DbScope.Transaction transaction = scope.ensureTransaction()) { @@ -56,11 +95,11 @@ public static void addMissingSampleTypeIdsForSampleTimelineAudit(ModuleContext c SqlSelector countSelector = new SqlSelector(scope, countSql); long toUpdate = countSelector.getObject(Long.class); - LOG.info("There are " + toUpdate + " audit log entries to be updated in audit." + table + "."); + LOG.info("There are {} audit log entries to be updated in audit.{}.", toUpdate, table); // first update the type id by finding other audit entries that reference the same sample id. if (toUpdate > 0) { - LOG.info("Updating table audit." + table + " via self-join."); + LOG.info("Updating table audit.{} via self-join.", table); SQLFragment updateSql = new SQLFragment("UPDATE audit.").append(table) .append(" SET sampleTypeId = a3.sampleTypeId\n") .append(" FROM\n") @@ -73,7 +112,7 @@ public static void addMissingSampleTypeIdsForSampleTimelineAudit(ModuleContext c SqlExecutor executor = new SqlExecutor(scope); int numRows = executor.execute(updateSql); long elapsed = System.currentTimeMillis() - start; - LOG.info("Updated " + numRows + " rows via self-join for table " + table + " in " + (elapsed / 1000) + " sec"); + LOG.info("Updated {} rows via self-join for table {} in {} sec", numRows, table, elapsed / 1000); } toUpdate = countSelector.getObject(Long.class); @@ -81,7 +120,7 @@ public static void addMissingSampleTypeIdsForSampleTimelineAudit(ModuleContext c { // It may have happened that there's only one audit entry for a sample and that entry has a 0 for the type id, in which case we may be able // to find the type id from the exp.materials table. Since samples may have been deleted, it isn't sufficient to do only this update - LOG.info("Updating table audit." + table + " via exp.materials."); + LOG.info("Updating table audit.{} via exp.materials.", table); SQLFragment updateSql = new SQLFragment("UPDATE audit.").append(table) .append(" SET sampleTypeId = m.materialSourceId\n") .append(" FROM\n") @@ -94,10 +133,10 @@ public static void addMissingSampleTypeIdsForSampleTimelineAudit(ModuleContext c SqlExecutor executor = new SqlExecutor(scope); int numRows = executor.execute(updateSql); long elapsed = System.currentTimeMillis() - start; - LOG.info("Updated " + numRows + " rows from exp.material table join in " + (elapsed / 1000) + " sec"); + LOG.info("Updated {} rows from exp.material table join in {} sec", numRows, elapsed / 1000); } long remaining = countSelector.getObject(Long.class); - LOG.info("There are " + remaining + " rows in audit." + table + " that could not be updated with a proper sample type id."); + LOG.info("There are {} rows in audit.{} that could not be updated with a proper sample type id.", remaining, table); } transaction.commit(); } @@ -153,4 +192,218 @@ private static void ensureBigObjectIds(Selector lastValueSelector, Consumer + auditCount.addAndGet(convertAmountsToBaseUnits(c, admin)) + ); + transaction.commit(); + LOG.info("{} Total audit events expected", auditCount); + if (auditCount.get() > 0) + { + PropertyManager.WritablePropertyMap props = PropertyManager.getWritableProperties(AMOUNT_AND_UNIT_UPGRADE_PROP, true); + props.put(AUDIT_COUNT_PROP, auditCount.toString()); + props.put(TRANSACTION_ID_PROP, String.valueOf(transactionId)); + props.save(); + } + ExperimentService.get().clearCaches(); + } + + } + + private static void getAmountAndUnitUpdates(Map sampleMap, Parameter unitsCol, Set amountCols, Unit currentDisplayUnit, Map oldDataMap, Map newDataMap, Map sampleCounts, boolean aliquotFields) + { + Unit baseUnit = currentDisplayUnit.getBase(); + String unitsStr = (String) sampleMap.get(unitsCol.getName()); + Unit materialUnit = Unit.fromName(unitsStr); + boolean isInBaseUnits = materialUnit == null ? currentDisplayUnit.isBase() : materialUnit.isBase(); + // have a unit value, but it did not convert to a known unit + if (materialUnit == null && !StringUtils.isEmpty(unitsStr)) + { + // invalid unit stored with sample. Leave as is. + LOG.info("Found invalid {} '{}' for sample '{}'. No conversion done.", aliquotFields ? "aliquot unit" : "unit", unitsStr, sampleMap.get(Name.name())); + sampleCounts.put("invalidUnits", sampleCounts.getOrDefault("invalidUnits", 0) + 1); + } + else if (materialUnit != null && !materialUnit.isCompatible(baseUnit)) + { + LOG.info("{} '{}' for sample '{}' is not compatible with the base unit '{}'. No conversion done.", aliquotFields ? "Aliquot unit" : "Unit", materialUnit.name(), sampleMap.get(Name.name()), baseUnit); + sampleCounts.put("invalidUnits", sampleCounts.getOrDefault("invalidUnits", 0) + 1); + } + else if (!isInBaseUnits || materialUnit == null) + { + if (!isInBaseUnits) + { + amountCols.forEach(amountCol -> { + if (sampleMap.get(amountCol.getName()) != null && !(sampleMap.get(amountCol.getName())).equals(0.0)) + { + oldDataMap.put(amountCol.getName(), sampleMap.get(amountCol.getName())); + newDataMap.put(amountCol.getName(), Unit.convert((Double) sampleMap.get(amountCol.getName()), materialUnit == null ? currentDisplayUnit : materialUnit, baseUnit)); + amountCol.setValue(newDataMap.get(amountCol.getName())); + sampleCounts.put("converted", sampleCounts.getOrDefault("converted", 0) + 1); + } + }); + sampleCounts.put("converted", sampleCounts.getOrDefault("converted", 0) + 1); + } + else // in base unit, but not explicitly stored + sampleCounts.put("setUnitsWithoutConvert", sampleCounts.getOrDefault("setUnitsWithoutConvert", 0) + 1); + if (!baseUnit.name().equals(unitsStr)) + { + unitsCol.setValue(baseUnit.name()); + oldDataMap.put(unitsCol.getName(), unitsStr); + newDataMap.put(unitsCol.getName(), baseUnit.name()); + } + } + else if (!unitsStr.equals(baseUnit.name())) + { + oldDataMap.put(unitsCol.getName(), unitsStr); + newDataMap.put(unitsCol.getName(), baseUnit.name()); + unitsCol.setValue(baseUnit.name()); + sampleCounts.put("changeUnitsLabel", sampleCounts.getOrDefault("changeUnitsLabel", 0) + 1); + } + } + + // Converts amounts for all sample types defined in the given container. + // Picks up samples from all containers for each sample type. + private static int convertAmountsToBaseUnits(Container container, User user) + { + DbScope scope = ExperimentService.get().getSchema().getScope(); + TableInfo tInfo = ExperimentService.get().getTinfoMaterial(); + + try (Connection c = scope.getConnection()) + { + AtomicInteger auditCount = new AtomicInteger(); + Parameter rowId = new Parameter("rowId", JdbcType.INTEGER); + Parameter units = new Parameter(Units.name(), JdbcType.VARCHAR); + Parameter amount = new Parameter(StoredAmount.name(), JdbcType.DOUBLE); + Parameter aliquotUnits = new Parameter(AliquotUnit.name(), JdbcType.VARCHAR); + Parameter aliquotAmount = new Parameter(AliquotVolume.name(), JdbcType.DOUBLE); + Parameter availableAliquotAmount = new Parameter(AvailableAliquotVolume.name(), JdbcType.DOUBLE); + ParameterMapStatement updateStmt = new ParameterMapStatement(scope, c, + new SQLFragment("UPDATE ") + .append(tInfo) + .append(" SET Units = ?, StoredAmount = ?, AliquotUnit = ?, AliquotVolume = ?, AvailableAliquotVolume = ? WHERE RowId = ?") + .addAll(units, amount, aliquotUnits, aliquotAmount, availableAliquotAmount, rowId), null); + + for (ExpSampleType sampleType : SampleTypeService.get().getSampleTypes(container, user, false)) + { + LOG.debug("** Starting upgrade for sample type {} in folder {}", sampleType.getName(), container.getPath()); + Map sampleCounts = new HashMap<>(); + Map aliquotCounts = new HashMap<>(); + + Unit currentDisplayUnit = Unit.fromName(sampleType.getMetricUnit()); + boolean hasDisplayUnit = currentDisplayUnit != null; + + AtomicInteger batchCount = new AtomicInteger(); + List auditEvents = new ArrayList<>(); + SQLFragment sql = new SQLFragment("SELECT m.RowId, m.Name, m.StoredAmount, m.Units, m.AliquotVolume, m.AliquotUnit, m.AvailableAliquotVolume, m.container FROM ") + .append(tInfo, "m") + .append(" WHERE cpastype = ?").add(sampleType.getLSID()) + .append(" AND (m.StoredAmount IS NOT NULL OR m.Units IS NOT NULL OR m.AliquotVolume IS NOT NULL OR m.AliquotUnit IS NOT NULL OR m.AvailableAliquotVolume IS NOT NULL)"); + SqlSelector selector = new SqlSelector(scope, sql); + + selector.mapStream().forEach(sampleMap -> { + + Map oldDataMap = new HashMap<>(); + Map newDataMap = new HashMap<>(); + // start out using the data already in the row + rowId.setValue(sampleMap.get(RowId.name())); + units.setValue(sampleMap.get(Units.name())); + amount.setValue(sampleMap.get(StoredAmount.name())); + aliquotUnits.setValue(sampleMap.get(AliquotUnit.name())); + aliquotAmount.setValue(sampleMap.get(AliquotVolume.name())); + availableAliquotAmount.setValue(sampleMap.get(AvailableAliquotVolume.name())); + if (!StringUtils.isEmpty((String) sampleMap.get(Units.name())) && sampleMap.get(StoredAmount.name()) == null) + { + // remove the unit if we had a unit but no amount + oldDataMap.put(Units.name(), sampleMap.get(Units.name())); + newDataMap.put(Units.name(), null); + units.setValue(null); + sampleCounts.put("unitsWithoutAmounts", sampleCounts.getOrDefault("unitsWithoutAmounts", 0) + 1); + } + if (!StringUtils.isEmpty((String) sampleMap.get(AliquotUnit.name())) && sampleMap.get(AliquotVolume.name()) == null && sampleMap.get(AvailableAliquotVolume.name()) == null) + { + // remove the aliquot unit if we had a unit but no amount + oldDataMap.put(AliquotUnit.name(), sampleMap.get(AliquotUnit.name())); + newDataMap.put(AliquotUnit.name(), null); + aliquotUnits.setValue(null); + aliquotCounts.put("unitsWithoutAmounts", aliquotCounts.getOrDefault("unitsWithoutAmounts", 0) + 1); + } + + if (hasDisplayUnit) + { + if (sampleMap.get(StoredAmount.name()) != null) + { + getAmountAndUnitUpdates(sampleMap, units, Set.of(amount), currentDisplayUnit, oldDataMap, newDataMap, sampleCounts, false); + } + if (sampleMap.get(AliquotVolume.name()) != null || sampleMap.get(AvailableAliquotVolume.name()) != null) + { + getAmountAndUnitUpdates(sampleMap, aliquotUnits, Set.of(aliquotAmount, availableAliquotAmount), currentDisplayUnit, oldDataMap, newDataMap, aliquotCounts, true); + } + } + + + if (!newDataMap.isEmpty()) + { + updateStmt.addBatch(); + batchCount.getAndIncrement(); + // All samples default to a 0 for AliquotVolume and a blank AliquotUnit. If the only + // change being made is to replace the blank AliquotUnit with the base unit, we do not + // need to audit that change here since these aliquot values are calculated values anyway. + if (newDataMap.size() > 1 || !newDataMap.containsKey(AliquotUnit.name())) + { + Container sampleContainer = ContainerManager.getForId((String) sampleMap.get("Container")); + SampleTimelineAuditEvent event = new SampleTimelineAuditEvent(sampleContainer != null ? sampleContainer : container, SampleTimelineAuditEvent.AMOUNT_AND_UNIT_UPGRADE_COMMENT); + event.setSampleId((Integer) sampleMap.get(RowId.name())); + event.setSampleName((String) sampleMap.get(Name.name())); + event.setSampleType(sampleType.getName()); + event.setSampleTypeId(sampleType.getRowId()); + event.setLineageUpdate(false); + event.setOldRecordMap(AbstractAuditTypeProvider.encodeForDataMap(oldDataMap)); + event.setNewRecordMap(AbstractAuditTypeProvider.encodeForDataMap(newDataMap)); + auditEvents.add(event); + auditCount.getAndIncrement(); + } + } + if (batchCount.get() > 1000) + { + updateStmt.executeBatch(); + AuditLogService.get().addEvents(user, auditEvents, false); + auditEvents.clear(); + batchCount.set(0); + } + }); + if (batchCount.get() > 0) + { + updateStmt.executeBatch(); + AuditLogService.get().addEvents(user, auditEvents); + } + + LOG.debug(" Sample data update counts {}", sampleCounts); + LOG.debug(" Aliquot data update counts {}", aliquotCounts); + LOG.debug("** Finished upgrade for sample type {} in folder {}", sampleType.getName(), container.getPath()); + } + LOG.debug("{} Audit events expected for container {}", auditCount, container.getPath()); + return auditCount.get(); + } + catch (SQLException e) + { + throw new RuntimeException(e); + } + + } } diff --git a/experiment/src/org/labkey/experiment/ExperimentWarningProvider.java b/experiment/src/org/labkey/experiment/ExperimentWarningProvider.java new file mode 100644 index 00000000000..ae0cd1dc4bf --- /dev/null +++ b/experiment/src/org/labkey/experiment/ExperimentWarningProvider.java @@ -0,0 +1,69 @@ +package org.labkey.experiment; + +import org.apache.commons.lang3.StringUtils; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.labkey.api.action.SpringActionController; +import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.SampleTimelineAuditEvent; +import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.ContainerManager; +import org.labkey.api.data.PropertyManager; +import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.SqlSelector; +import org.labkey.api.data.TableInfo; +import org.labkey.api.query.UserSchema; +import org.labkey.api.security.User; +import org.labkey.api.util.HtmlString; +import org.labkey.api.view.ViewContext; +import org.labkey.api.view.template.WarningProvider; +import org.labkey.api.view.template.Warnings; + +public class ExperimentWarningProvider implements WarningProvider +{ + private Long _actualRecordCount; + + @Override + public void addDynamicWarnings(@NotNull Warnings warnings, @Nullable ViewContext context, boolean showAllWarnings) + { + PropertyManager.WritablePropertyMap props = PropertyManager.getWritableProperties(ExperimentModule.AMOUNT_AND_UNIT_UPGRADE_PROP, false); + if (props == null || props.isEmpty()) + return; + String expectedCount = props.get(ExperimentModule.AUDIT_COUNT_PROP); + String transactionIdStr = props.get(ExperimentModule.TRANSACTION_ID_PROP); + if (StringUtils.isEmpty(expectedCount) || StringUtils.isEmpty(transactionIdStr)) + return; + + if (_actualRecordCount == null) + { + UserSchema auditLogSchema = AuditLogService.getAuditLogSchema(User.getAdminServiceUser(), ContainerManager.getRoot()); + if (auditLogSchema == null) + return; + ContainerFilter cf = ContainerFilter.Type.AllFolders.create(ContainerManager.getRoot(), User.getAdminServiceUser()); + TableInfo timelineTable = auditLogSchema.getTable(SampleTimelineAuditEvent.EVENT_TYPE, cf); + if (timelineTable == null) + return; + SQLFragment sql = new SQLFragment("SELECT COUNT(*) from ").append(timelineTable) + .append(" WHERE transactionId = ?").add(Long.valueOf(transactionIdStr)); + SqlSelector selector = new SqlSelector(auditLogSchema.getDbSchema().getScope(), sql); + _actualRecordCount = selector.getObject(Long.class); + } + + if (Long.valueOf(expectedCount).equals(_actualRecordCount)) + { + try (var ignored = SpringActionController.ignoreSqlUpdates()) + { + props.delete(); + } + _actualRecordCount = null; + } + else + { + String upgradeMessage = "The number of audit logs created during the upgrade of the Experiment Module is not as expected. Expected " + + expectedCount + " but got " + _actualRecordCount + ". The upgrade succeeded but not all audit logs were created, likely due to a premature server shutdown." + + " It is recommended that you restore the DB from backup and rerun the upgrade or contact your account manager."; + warnings.add(HtmlString.of(upgradeMessage)); + } + + } +} diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java index 6ed1a8eeec0..ddb2cafe159 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java @@ -1544,7 +1544,7 @@ public DataIteratorBuilder createImportDIB(User user, Container container, DataI dib = getConceptURIVocabularyDomainDataIteratorBuilder(user, container, dib); - dib = DetailedAuditLogDataIterator.getDataIteratorBuilder(getQueryTable(), dib, context.getInsertOption(), user, container); + dib = DetailedAuditLogDataIterator.getDataIteratorBuilder(getQueryTable(), dib, context.getInsertOption(), user, container, null); return dib; } diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java index f0d2bf354c2..8a59ed07001 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java @@ -240,17 +240,26 @@ public int getAliquotCount() } @Override - public double getAliquotVolume() + public Double getAliquotVolume() { return _object.getAliquotVolume(); } + @Override + public Double getAvailableAliquotVolume() { return _object.getAvailableAliquotVolume(); } + @Override public String getAliquotUnit() { return _object.getAliquotUnit(); } + @Override + public void setAliquotUnit(String units) + { + _object.setAliquotUnit(units); + } + @Override public Date getMaterialExpDate() { diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index 852fcb69578..d6d8a8fdeb8 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -40,6 +40,7 @@ import org.labkey.api.data.DbScope; import org.labkey.api.data.DisplayColumn; import org.labkey.api.data.DisplayColumnFactory; +import org.labkey.api.data.ForeignKey; import org.labkey.api.data.ImportAliasable; import org.labkey.api.data.JdbcType; import org.labkey.api.data.MaterializedQueryHelper; @@ -74,6 +75,8 @@ import org.labkey.api.exp.query.SamplesSchema; import org.labkey.api.gwt.client.AuditBehaviorType; import org.labkey.api.inventory.InventoryService; +import org.labkey.api.ontology.Quantity; +import org.labkey.api.ontology.Unit; import org.labkey.api.qc.SampleStatusService; import org.labkey.api.query.AliasedColumn; import org.labkey.api.query.DetailsURL; @@ -141,6 +144,8 @@ import static org.labkey.api.exp.query.ExpMaterialTable.Column.AliquotVolume; import static org.labkey.api.exp.query.ExpMaterialTable.Column.AvailableAliquotCount; import static org.labkey.api.exp.query.ExpMaterialTable.Column.AvailableAliquotVolume; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.StoredAmount; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.Units; import static org.labkey.api.util.StringExpressionFactory.AbstractStringExpression.NullValueBehavior.NullResult; import static org.labkey.experiment.api.SampleTypeServiceImpl.SampleChangeType.schema; @@ -303,31 +308,78 @@ public StringExpression getURL(ColumnInfo parent) } case RawAmount -> { - return wrapColumn(alias, _rootTable.getColumn(Column.StoredAmount.name())); + var columnInfo = wrapColumn(alias, _rootTable.getColumn(Column.StoredAmount.name())); + columnInfo.setDescription("The amount of this sample, in the base unit for the sample type's display unit (if defined), currently on hand."); + if (columnInfo.getFormat() == null) + columnInfo.setFormat(Quantity.DEFAULT_FORMAT); + columnInfo.setUserEditable(false); + columnInfo.setReadOnly(true); + return columnInfo; } case StoredAmount -> { - var columnInfo = wrapColumn(alias, _rootTable.getColumn(Column.StoredAmount.name())); - columnInfo.setLabel("Amount"); - columnInfo.setImportAliasesSet(Set.of("Amount")); - return columnInfo; + String label = StoredAmount.label(); + Set importAliases = Set.of(label, "Stored Amount"); + Unit typeUnit = getSampleTypeUnit(); + if (typeUnit != null) + { + SampleTypeAmountDisplayColumn columnInfo = new SampleTypeAmountDisplayColumn(this, Column.StoredAmount.name(), Column.Units.name(), label, importAliases, typeUnit); + columnInfo.setDescription("The amount of this sample, in the display unit for the sample type, currently on hand."); + columnInfo.setShownInUpdateView(true); + columnInfo.setShownInInsertView(true); + columnInfo.setUserEditable(true); + columnInfo.setCalculated(false); + return columnInfo; + } + else + { + var columnInfo = wrapColumn(alias, _rootTable.getColumn(Column.StoredAmount.name())); + if (columnInfo.getFormat() == null) + columnInfo.setFormat(Quantity.DEFAULT_FORMAT); + columnInfo.setLabel(label); + columnInfo.setImportAliasesSet(importAliases); + columnInfo.setDescription("The amount of this sample currently on hand."); + return columnInfo; + } } case RawUnits -> { - return wrapColumn(alias, _rootTable.getColumn(Column.Units.name())); + var columnInfo = wrapColumn(alias, _rootTable.getColumn(Column.Units.name())); + columnInfo.setDescription("The units associated with the Stored Amount for this sample."); + columnInfo.setUserEditable(false); + columnInfo.setReadOnly(true); + return columnInfo; } case Units -> { - var columnInfo = wrapColumn(alias, _rootTable.getColumn(Column.Units.name())); - columnInfo.setFk(new LookupForeignKey("Value", "Value") + ForeignKey fk = new LookupForeignKey("Value", "Value") { @Override public @Nullable TableInfo getLookupTableInfo() { return getExpSchema().getTable(ExpSchema.MEASUREMENT_UNITS_TABLE); } - }); - return columnInfo; + }; + + Unit typeUnit = getSampleTypeUnit(); + if (typeUnit != null) + { + SampleTypeUnitDisplayColumn columnInfo = new SampleTypeUnitDisplayColumn(this, Column.Units.name(), typeUnit); + columnInfo.setFk(fk); + columnInfo.setDescription("The sample type display units associated with the Amount for this sample."); + columnInfo.setShownInUpdateView(true); + columnInfo.setShownInInsertView(true); + columnInfo.setUserEditable(true); + columnInfo.setCalculated(false); + return columnInfo; + } + else + { + var columnInfo = wrapColumn(alias, _rootTable.getColumn(Column.Units.name())); + columnInfo.setFk(fk); + columnInfo.setDescription("The units associated with the Stored Amount for this sample."); + return columnInfo; + } } case Description -> { @@ -560,6 +612,14 @@ protected ColumnInfo getPkColumn(TableInfo table) return ret; } + private Unit getSampleTypeUnit() + { + Unit typeUnit = null; + if (_ss != null && _ss.getMetricUnit() != null) + typeUnit = Unit.fromName(_ss.getMetricUnit()); + return typeUnit; + } + private void setSampleType(@Nullable ExpSampleType st) { checkLocked(); @@ -1044,6 +1104,8 @@ private void addSampleTypeColumns(ExpSampleType st, List visibleColumn if (null == selectedColumns) return ALL_COLUMNS; selectedColumns = new TreeSet<>(selectedColumns); + if (selectedColumns.contains(new FieldKey(null, StoredAmount))) + selectedColumns.add(new FieldKey(null, Units)); if (selectedColumns.contains(new FieldKey(null, ExpMaterial.ALIQUOTED_FROM_INPUT))) selectedColumns.add(new FieldKey(null, Column.AliquotedFromLSID.name())); if (selectedColumns.contains(new FieldKey(null, Column.IsAliquot.name()))) @@ -1507,6 +1569,42 @@ protected boolean isDisabledInput(RenderContext ctx) } } + private static class SampleTypeAmountDisplayColumn extends ExprColumn + { + public SampleTypeAmountDisplayColumn(TableInfo parent, String amountFieldName, String unitFieldName, String label, Set importAliases, Unit typeUnit) + { + super(parent, FieldKey.fromParts(amountFieldName), new SQLFragment( + "(CASE WHEN ").append(ExprColumn.STR_TABLE_ALIAS + ".").append(unitFieldName) + .append(" = ? AND ").append(ExprColumn.STR_TABLE_ALIAS + ".").append(amountFieldName) + .append(" IS NOT NULL THEN ROUND(CAST(").append(ExprColumn.STR_TABLE_ALIAS + ".").append(amountFieldName) + .append(" / ? AS ") + .append(parent.getSqlDialect().isPostgreSQL() ? "DECIMAL" : "DOUBLE PRECISION") + .append("), ?) ELSE ").append(ExprColumn.STR_TABLE_ALIAS + ".").append(amountFieldName) + .append(" END)") + .add(typeUnit.getBase().toString()) + .add(typeUnit.getValue()) + .add(typeUnit.getPrecisionScale()), + JdbcType.DOUBLE); + + setLabel(label); + setImportAliasesSet(importAliases); + } + } + + private static class SampleTypeUnitDisplayColumn extends ExprColumn + { + public SampleTypeUnitDisplayColumn(TableInfo parent, String unitFieldName, Unit typeUnit) + { + super(parent, FieldKey.fromParts(Column.Units.name()), new SQLFragment( + "(CASE WHEN ").append(ExprColumn.STR_TABLE_ALIAS + ".").append(unitFieldName) + .append(" = ? THEN ? ELSE ").append(ExprColumn.STR_TABLE_ALIAS + ".").append(unitFieldName) + .append(" END)") + .add(typeUnit.getBase().toString()) + .add(typeUnit.toString()), + JdbcType.VARCHAR); + } + } + @Override public QueryUpdateService getUpdateService() { diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java b/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java index d763a619fb9..c15476b08db 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java @@ -55,6 +55,7 @@ import org.labkey.api.exp.query.ExpMaterialTable; import org.labkey.api.exp.query.ExpSchema; import org.labkey.api.exp.query.SamplesSchema; +import org.labkey.api.ontology.Unit; import org.labkey.api.query.FieldKey; import org.labkey.api.query.QueryRowReference; import org.labkey.api.query.QueryService; @@ -363,6 +364,12 @@ public void setMetricUnit(String metricUnit) return _object.getMetricUnit(); } + @Override + public @Nullable Unit getBaseUnit() + { + return _object.getAmountDisplayUnit() == null ? null : _object.getAmountDisplayUnit().getBase(); + } + public void setAutoLinkTargetContainer(Container autoLinkTargetContainerId) { _object.setAutoLinkTargetContainer(autoLinkTargetContainerId); diff --git a/experiment/src/org/labkey/experiment/api/Material.java b/experiment/src/org/labkey/experiment/api/Material.java index da8419577ab..4cd543ca382 100644 --- a/experiment/src/org/labkey/experiment/api/Material.java +++ b/experiment/src/org/labkey/experiment/api/Material.java @@ -44,6 +44,8 @@ public class Material extends RunItem private Integer aliquotCount; private Double aliquotVolume; private String aliquotUnit; + private Integer availableAliquotCount; + private Double availableAliquotVolume; public Material() { @@ -120,6 +122,26 @@ public Double getAliquotVolume() return aliquotVolume; } + public Integer getAvailableAliquotCount() + { + return availableAliquotCount; + } + + public void setAvailableAliquotCount(Integer availableAliquotCount) + { + this.availableAliquotCount = availableAliquotCount; + } + + public void setAvailableAliquotVolume(Double availableAliquotVolume) + { + this.availableAliquotVolume = availableAliquotVolume; + } + + public Double getAvailableAliquotVolume() + { + return availableAliquotVolume; + } + public String getAliquotUnit() { return aliquotUnit; diff --git a/experiment/src/org/labkey/experiment/api/MaterialSource.java b/experiment/src/org/labkey/experiment/api/MaterialSource.java index 3c30142cc22..d8337a87d9e 100644 --- a/experiment/src/org/labkey/experiment/api/MaterialSource.java +++ b/experiment/src/org/labkey/experiment/api/MaterialSource.java @@ -21,6 +21,7 @@ import org.labkey.api.data.ContainerManager; import org.labkey.api.exp.query.ExpSampleTypeTable; import org.labkey.api.exp.query.ExpSchema; +import org.labkey.api.ontology.Unit; import org.labkey.api.query.FieldKey; import org.labkey.api.query.QueryRowReference; import org.labkey.api.util.GUID; @@ -47,6 +48,7 @@ public class MaterialSource extends IdentifiableEntity implements Comparable { - clearMaterialSourceCache(container); - - if (finalHasMetricUnitChanged) - { - try - { - recomputeSampleTypeRollup(st, container); - } - catch (SQLException e) - { - throw new RuntimeSQLException(e); - } - } - }, DbScope.CommitTaskOption.IMMEDIATE, POSTCOMMIT, POSTROLLBACK); transaction.addCommitTask(() -> SampleTypeServiceImpl.get().indexSampleType(st, SearchService.get().defaultTask().getQueue(container, SearchService.PRIORITY.modified)), POSTCOMMIT); transaction.commit(); refreshSampleTypeMaterializedView(st, SampleChangeType.schema); @@ -1160,9 +1142,9 @@ public String getCommentDetailed(QueryService.AuditAction action, boolean isUpda } @Override - public DetailedAuditTypeEvent createDetailedAuditRecord(User user, Container c, AuditConfigurable tInfo, QueryService.AuditAction action, @Nullable String userComment, @Nullable Map row, Map existingRow) + public DetailedAuditTypeEvent createDetailedAuditRecord(User user, Container c, AuditConfigurable tInfo, QueryService.AuditAction action, @Nullable String userComment, @Nullable Map row, Map existingRow, Map providedValues) { - return createAuditRecord(c, tInfo, getCommentDetailed(action, !existingRow.isEmpty()), userComment, action, row, existingRow); + return createAuditRecord(c, tInfo, getCommentDetailed(action, !existingRow.isEmpty()), userComment, action, row, existingRow, providedValues); } @Override @@ -1173,17 +1155,17 @@ protected AuditTypeEvent createSummaryAuditRecord(User user, Container c, AuditC private SampleTimelineAuditEvent createAuditRecord(Container c, AuditConfigurable tInfo, String comment, String userComment, @Nullable Map row) { - return createAuditRecord(c, tInfo, comment, userComment, null, row, null); + return createAuditRecord(c, tInfo, comment, userComment, null, row, null, null); } private boolean isInputFieldKey(String fieldKey) { int slash = fieldKey.indexOf('/'); - return slash==ExpData.DATA_INPUT_PARENT.length() && StringUtils.startsWithIgnoreCase(fieldKey,ExpData.DATA_INPUT_PARENT) || - slash==ExpMaterial.MATERIAL_INPUT_PARENT.length() && StringUtils.startsWithIgnoreCase(fieldKey,ExpMaterial.MATERIAL_INPUT_PARENT); + return slash==ExpData.DATA_INPUT_PARENT.length() && Strings.CI.startsWith(fieldKey,ExpData.DATA_INPUT_PARENT) || + slash==ExpMaterial.MATERIAL_INPUT_PARENT.length() && Strings.CI.startsWith(fieldKey,ExpMaterial.MATERIAL_INPUT_PARENT); } - private SampleTimelineAuditEvent createAuditRecord(Container c, AuditConfigurable tInfo, String comment, String userComment, @Nullable QueryService.AuditAction action, @Nullable Map row, @Nullable Map existingRow) + private SampleTimelineAuditEvent createAuditRecord(Container c, AuditConfigurable tInfo, String comment, String userComment, @Nullable QueryService.AuditAction action, @Nullable Map row, @Nullable Map existingRow, @Nullable Map providedValues) { SampleTimelineAuditEvent event = new SampleTimelineAuditEvent(c, comment); event.setUserComment(userComment); @@ -1244,14 +1226,29 @@ else if (tInfo != null) } } + // Put the raw amount and units into the stored amount and unit fields to override the conversion to display values that has happened via the expression columns + if (existingRow != null && !existingRow.isEmpty()) + { + if (existingRow.containsKey(RawAmount.name())) + existingRow.put(StoredAmount.name(), existingRow.get(RawAmount.name())); + if (existingRow.containsKey(RawUnits.name())) + existingRow.put(Units.name(), existingRow.get(RawUnits.name())); + } + + // Add providedValues to eventMetadata + Map eventMetadata = new HashMap<>(); + if (providedValues != null) + { + eventMetadata.putAll(providedValues); + } if (action != null) { - Map eventMetadata = new HashMap<>(); SampleTimelineAuditEvent.SampleTimelineEventType timelineEventType = SampleTimelineAuditEvent.SampleTimelineEventType.getTypeFromAction(action); if (timelineEventType != null) eventMetadata.put(SAMPLE_TIMELINE_EVENT_TYPE, action); - event.setMetadata(AbstractAuditTypeProvider.encodeForDataMap(eventMetadata)); } + if (!eventMetadata.isEmpty()) + event.setMetadata(AbstractAuditTypeProvider.encodeForDataMap(eventMetadata)); return event; } @@ -1490,10 +1487,9 @@ public int recomputeSamplesRollup( Long sampleId = sampleAliquotAmounts.getKey(); List aliquotAmounts = sampleAliquotAmounts.getValue(); - AliquotAvailableAmountUnit amountUnit = convertToDisplayUnits(aliquotAmounts, sampleTypeUnit, sampleUnits.get(sampleId)); - if (amountUnit == null) + if (aliquotAmounts == null || aliquotAmounts.isEmpty()) continue; - + AliquotAvailableAmountUnit amountUnit = computeAliquotTotalAmounts(aliquotAmounts, sampleTypeUnit, sampleUnits.get(sampleId)); rowid.setValue(sampleId); amount.setValue(amountUnit.amount); unit.setValue(amountUnit.unit); @@ -1542,37 +1538,31 @@ private Set getRootSampleIdsFromParentNames(String sampleTypeLsid, Set()); } - private AliquotAvailableAmountUnit convertToDisplayUnits(List volumeUnits, String sampleTypeUnitsStr, String sampleItemUnit) + private AliquotAvailableAmountUnit computeAliquotTotalAmounts(List volumeUnits, String sampleTypeUnitsStr, String sampleItemUnitsStr) { if (volumeUnits == null || volumeUnits.isEmpty()) return null; - String totalDisplayUnitStr = sampleTypeUnitsStr; - Measurement.Unit totalDisplayUnit = null; - - if (StringUtils.isEmpty(totalDisplayUnitStr) && (sampleItemUnit != null)) - { - // if sample type lacks unit, but the sample has a unit, use sample's unit - totalDisplayUnitStr = sampleItemUnit; - } - - // if sample unit is empty, use 1st aliquot unit - if (StringUtils.isEmpty(totalDisplayUnitStr)) - { - String aliquotUnit = volumeUnits.get(0).unit; - if (!StringUtils.isEmpty(aliquotUnit)) - totalDisplayUnitStr = aliquotUnit; - } - - if (!StringUtils.isEmpty(totalDisplayUnitStr)) + Unit totalUnit = null; + String totalUnitsStr = null; + if (!StringUtils.isEmpty(sampleTypeUnitsStr)) + totalUnitsStr = sampleTypeUnitsStr; + else if (!StringUtils.isEmpty(sampleItemUnitsStr)) + totalUnitsStr = sampleItemUnitsStr; + else // use the unit of the first aliquot if there are no other indications + totalUnitsStr = volumeUnits.get(0).unit; + if (!StringUtils.isEmpty(totalUnitsStr)) { try { - totalDisplayUnit = Measurement.Unit.valueOf(totalDisplayUnitStr); + if (!StringUtils.isEmpty(sampleTypeUnitsStr)) + totalUnit = Unit.valueOf(totalUnitsStr).getBase(); + else + totalUnit = Unit.valueOf(totalUnitsStr); } catch (IllegalArgumentException e) { - // do nothing; leave unit as null; + // do nothing; leave unit as null } } @@ -1581,7 +1571,7 @@ private AliquotAvailableAmountUnit convertToDisplayUnits(List, Collection> getAliquotParentsForRecalc(String sampleTypeLsid, Container container) throws SQLException @@ -1669,7 +1656,6 @@ private SQLFragment getParentsOfAliquotsSql() private Collection getAliquotParents(String sampleTypeLsid, boolean withAmount, Container container) throws SQLException { DbSchema dbSchema = getExpSchema(); - SqlDialect dialect = dbSchema.getSqlDialect(); SQLFragment sql = withAmount ? getParentsOfAliquotsWithAmountsSql() : getParentsOfAliquotsSql(); @@ -1698,7 +1684,7 @@ private Map> getSampleAliquotCounts(Collection // have run yet. SQLFragment sql = new SQLFragment("SELECT m.RowId as SampleId, m.Units, (SELECT COUNT(*) FROM exp.material a WHERE ") .append(useRootMaterialLSID ? "a.rootMaterialLsid = m.lsid" : "a.rootMaterialRowId = m.rowId") - .append(")-1 AS CreatedAliquotCount FROM exp.material AS m WHERE m.rowid\s"); + .append(")-1 AS CreatedAliquotCount FROM exp.material AS m WHERE m.rowid "); dialect.appendInClauseSql(sql, sampleIds); Map> sampleAliquotCounts = new TreeMap<>(); // Order sample by rowId to reduce probability of deadlock with search indexer @@ -1791,7 +1777,7 @@ private Map> getSampleAliquotAmounts(Collect .append(useRootMaterialLSID ? "parent.lsid = aliquot.rootmateriallsid" : "parent.rowid = aliquot.rootmaterialrowid") .append(" WHERE ") .append(useRootMaterialLSID ? "aliquot.rootmateriallsid <> aliquot.lsid" : "aliquot.rootmaterialrowid <> aliquot.rowid") - .append(" AND parent.rowid\s"); + .append(" AND parent.rowid "); dialect.appendInClauseSql(sql, sampleIds); Map> sampleAliquotAmounts = new LongHashMap<>(); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 126fbe21ca7..43f0ea73d7d 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -17,8 +17,8 @@ import org.apache.commons.beanutils.ConversionException; import org.apache.commons.beanutils.converters.IntegerConverter; -import org.apache.commons.collections4.MapUtils; import org.apache.commons.collections4.ListUtils; +import org.apache.commons.collections4.MapUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.Strings; import org.apache.logging.log4j.Level; @@ -26,6 +26,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.assay.AssayFileWriter; +import org.labkey.api.attachments.AttachmentFile; import org.labkey.api.audit.AuditLogService; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; @@ -39,8 +40,11 @@ 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.ConvertHelper; import org.labkey.api.data.DbScope; import org.labkey.api.data.DbSequence; +import org.labkey.api.data.ExpDataFileConverter; import org.labkey.api.data.Filter; import org.labkey.api.data.ForeignKey; import org.labkey.api.data.ImportAliasable; @@ -55,7 +59,6 @@ import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; import org.labkey.api.data.UpdateableTableInfo; -import org.labkey.api.data.measurement.Measurement; import org.labkey.api.dataiterator.AttachmentDataIterator; import org.labkey.api.dataiterator.CachingDataIterator; import org.labkey.api.dataiterator.DataIterator; @@ -88,6 +91,8 @@ import org.labkey.api.exp.query.SamplesSchema; import org.labkey.api.gwt.client.AuditBehaviorType; import org.labkey.api.inventory.InventoryService; +import org.labkey.api.ontology.Quantity; +import org.labkey.api.ontology.Unit; import org.labkey.api.qc.DataState; import org.labkey.api.qc.SampleStatusService; import org.labkey.api.query.BatchValidationException; @@ -113,6 +118,7 @@ import org.labkey.api.view.UnauthorizedException; import org.labkey.experiment.ExpDataIterators; import org.labkey.experiment.SampleTypeAuditProvider; +import org.springframework.web.multipart.MultipartFile; import java.io.IOException; import java.nio.file.Path; @@ -133,15 +139,19 @@ import java.util.stream.Stream; import static java.util.Collections.emptyMap; +import static org.labkey.api.audit.AuditHandler.DELTA_PROVIDED_DATA_PREFIX; +import static org.labkey.api.audit.AuditHandler.PROVIDED_DATA_PREFIX; import static org.labkey.api.data.TableSelector.ALL_COLUMNS; import static org.labkey.api.dataiterator.DetailedAuditLogDataIterator.AuditConfigs; import static org.labkey.api.dataiterator.SampleUpdateAddColumnsDataIterator.CURRENT_SAMPLE_STATUS_COLUMN_NAME; import static org.labkey.api.exp.api.ExpRunItem.PARENT_IMPORT_ALIAS_MAP_PROP; import static org.labkey.api.exp.api.ExperimentService.QueryOptions.SkipBulkRemapCache; -import static org.labkey.api.util.IntegerUtils.asLong; 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.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; import static org.labkey.api.exp.query.ExpMaterialTable.Column.AliquotedFromLSID; @@ -149,12 +159,15 @@ import static org.labkey.api.exp.query.ExpMaterialTable.Column.AvailableAliquotVolume; import static org.labkey.api.exp.query.ExpMaterialTable.Column.LSID; import static org.labkey.api.exp.query.ExpMaterialTable.Column.Name; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.RawAmount; +import static org.labkey.api.exp.query.ExpMaterialTable.Column.RawUnits; import static org.labkey.api.exp.query.ExpMaterialTable.Column.RootMaterialRowId; import static org.labkey.api.exp.query.ExpMaterialTable.Column.RowId; import static org.labkey.api.exp.query.ExpMaterialTable.Column.SampleState; import static org.labkey.api.exp.query.ExpMaterialTable.Column.StoredAmount; import static org.labkey.api.exp.query.ExpMaterialTable.Column.Units; import static org.labkey.api.exp.query.SamplesSchema.SCHEMA_SAMPLES; +import static org.labkey.api.util.IntegerUtils.asLong; import static org.labkey.experiment.ExpDataIterators.incrementCounts; import static org.labkey.experiment.api.SampleTypeServiceImpl.SampleChangeType.insert; import static org.labkey.experiment.api.SampleTypeServiceImpl.SampleChangeType.rollup; @@ -180,20 +193,20 @@ public class SampleTypeUpdateServiceDI extends DefaultQueryUpdateService public static final Map SAMPLE_ALT_IMPORT_NAME_COLS; - private static final Map ALIQUOT_ROLLUP_FIELDS = Map.of( - AliquotCount.toString(), JdbcType.INTEGER, - AvailableAliquotCount.toString(), JdbcType.INTEGER, - AliquotVolume.toString(), JdbcType.DOUBLE, - AvailableAliquotVolume.toString(), JdbcType.DOUBLE + private static final Map ALIQUOT_ROLLUP_FIELDS = Map.of( + AliquotCount, JdbcType.INTEGER, + AvailableAliquotCount, JdbcType.INTEGER, + AliquotVolume, JdbcType.DOUBLE, + AvailableAliquotVolume, JdbcType.DOUBLE ); - static { SAMPLE_ALT_IMPORT_NAME_COLS = new CaseInsensitiveHashMap<>(); SAMPLE_ALT_IMPORT_NAME_COLS.put("SampleId", "Name"); SAMPLE_ALT_IMPORT_NAME_COLS.put("Sample Id", "Name"); SAMPLE_ALT_IMPORT_NAME_COLS.put("ExpirationDate", "MaterialExpDate"); + SAMPLE_ALT_IMPORT_NAME_COLS.put("Expiration Date", "MaterialExpDate"); SAMPLE_ALT_IMPORT_NAME_COLS.put("Entered Storage", "Stored"); SAMPLE_ALT_IMPORT_NAME_COLS.put("EnteredStorage", "Stored"); } @@ -382,6 +395,33 @@ protected void preImportDIBValidation(@Nullable DataIteratorBuilder in, @Nullabl ExperimentServiceImpl.get().checkDuplicateParentColumns(in, inputColumns, _sampleType); } + @Nullable + protected Map extractProvidedAmountsAndUnits(@NotNull Map dataRow) + { + Map result = new HashMap<>(); + String unitsStr = ""; + String prefix; + if (dataRow.containsKey(DELTA_PROVIDED_DATA_PREFIX + StoredAmount.name())) + prefix = DELTA_PROVIDED_DATA_PREFIX; + else + { + // with no sample type display unit, no conversion will happen + if (_sampleType == null || _sampleType.getMetricUnit() == null) + return null; + prefix = PROVIDED_DATA_PREFIX; + } + Object amountVal = dataRow.get(prefix + StoredAmount.name()); + if (amountVal == null) + return null; + + if (dataRow.get(prefix + Units.name()) != null) + unitsStr = " " + dataRow.get(prefix + Units.name()).toString(); + + result.put(prefix + StoredAmount.label(), amountVal + unitsStr); + + return result; + } + @Override public DataIteratorBuilder createImportDIB(User user, Container container, DataIteratorBuilder data, DataIteratorContext context) { @@ -393,7 +433,7 @@ public DataIteratorBuilder createImportDIB(User user, Container container, DataI dib = ((UpdateableTableInfo) getQueryTable()).persistRows(dib, context); dib = AttachmentDataIterator.getAttachmentDataIteratorBuilder(getQueryTable(), dib, user, context.getInsertOption().batch ? getAttachmentDirectory() : null, container, getAttachmentParentFactory()); - dib = DetailedAuditLogDataIterator.getDataIteratorBuilder(getQueryTable(), dib, context.getInsertOption(), user, container); + dib = DetailedAuditLogDataIterator.getDataIteratorBuilder(getQueryTable(), dib, context.getInsertOption(), user, container, this::extractProvidedAmountsAndUnits); UserSchema userSchema = getQueryTable().getUserSchema(); if (userSchema != null) @@ -480,11 +520,34 @@ public List> insertRows(User user, Container container, List return results; } + /** + * This method is meant to help us ensure that every stored amount also has a unit. This checks only for the + * presence or absence of columns in the incoming data. If both columns are present, no exception is thrown. + * + * @param columns The set of columns in the input + * @param allowsUpdate Whether the type of import supports updates or not + */ + public static void confirmAmountAndUnitsColumns(Collection columns, boolean allowsUpdate) + { + boolean hasUnits = columns.stream().anyMatch(column -> column.equalsIgnoreCase(Units.name())); + boolean hasAmount = columns.stream().anyMatch(column -> StoredAmount.namesAndLabels().contains(column)); + + 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(String.format(MISSING_COLUMN_ERROR_MESSAGE_PATTERN, Units.name(), StoredAmount.label())); + } + @Override public List> updateRows(User user, Container container, List> rows, List> oldKeys, BatchValidationException errors, @Nullable Map configParameters, Map extraScriptContext) throws InvalidKeyException, BatchValidationException, QueryUpdateServiceException, SQLException { assert _sampleType != null : "SampleType required for insert/update, but not required for read/delete"; + if (rows != null && !rows.isEmpty()) + confirmAmountAndUnitsColumns(rows.get(0).keySet(), true); + boolean useDib = false; if (rows != null && !rows.isEmpty() && oldKeys == null) useDib = rows.get(0).containsKey("lsid"); @@ -557,6 +620,91 @@ public List> updateRows(User user, Container container, List return results; } + /** + * Attempt to make the passed in types match the expected types so the script doesn't have to do the conversion + */ + @Deprecated + @Override + protected Map coerceTypes(Map row, Map providedValues, boolean isUpdate) + { + Map result = new CaseInsensitiveHashMap<>(row.size()); + Map columnMap = ImportAliasable.Helper.createImportMap(_queryTable.getColumns(), true); + Object unitsVal = null; + ColumnInfo unitsCol = null; + Object amountVal = null; + ColumnInfo amountCol = null; + if (row.containsKey(ExpMaterialTable.Column.Units.name())) + { + unitsVal = row.get(ExpMaterialTable.Column.Units.name()); + unitsCol = columnMap.get(ExpMaterialTable.Column.Units.name()); + } + for (String colName : ExpMaterialTable.Column.StoredAmount.namesAndLabels()) + { + if (row.containsKey(colName)) + { + amountVal = row.get(colName); + amountCol = columnMap.get(colName); + break; + } + } + if (amountVal != null) + { + String unitsStr = ""; + if (unitsVal != null) + unitsStr = " " + unitsVal; + + providedValues.put(PROVIDED_DATA_PREFIX + StoredAmount.label(), amountVal + unitsStr); + } + + + Unit baseUnit = _sampleType != null ? _sampleType.getBaseUnit() : null; + + for (Map.Entry entry : row.entrySet()) + { + ColumnInfo col = columnMap.get(entry.getKey()); + + Object value = entry.getValue(); + if (col != null && col == unitsCol) + { + value = _SamplesCoerceDataIterator.SampleUnitsConvertColumn.getValue(unitsVal, amountVal, amountCol != null, baseUnit); + } + else if (col != null && col == amountCol) + { + value = _SamplesCoerceDataIterator.SampleAmountConvertColumn.getValue(amountVal, unitsCol != null, unitsVal, baseUnit); + } + else if (col != null && value != null && + !col.getJavaObjectClass().isInstance(value) && + !(value instanceof AttachmentFile) && + !(value instanceof MultipartFile) && + !(value instanceof String[]) && + !(col.getFk() instanceof MultiValuedForeignKey)) + { + try + { + if (PropertyType.FILE_LINK.equals(col.getPropertyType())) + value = ExpDataFileConverter.convert(value); + else if (col.getKindOfQuantity() != null) + { + providedValues.put(entry.getKey(), value); + value = Quantity.convert(value, col.getKindOfQuantity().getStorageUnit()); + } + else + value = col.getConvertFn().apply(value); + } + catch (ConvertHelper.FileConversionException e) + { + throw e; + } + catch (ConversionException e) + { + // That's OK, the transformation script may be able to fix up the value before it gets inserted + } + } + result.put(entry.getKey(), value); + } + return result; + } + @Override public Map moveRows(User user, Container container, Container targetContainer, List> rows, BatchValidationException errors, @Nullable Map configParameters, @Nullable Map extraScriptContext) throws BatchValidationException @@ -728,10 +876,16 @@ protected Map _update(User user, Container c, Map> deleteRows(User user, Container container, List { // NOTE: Not necessary to call onSamplesChanged -- already called by deleteMaterialByRowIds audit(QueryService.AuditAction.DELETE); - addAuditEvent(user, container, QueryService.AuditAction.DELETE, configParameters, result, null); + addAuditEvent(user, container, QueryService.AuditAction.DELETE, configParameters, result, null, null); } return result; } @@ -1116,6 +1270,15 @@ else if (dataColumns.contains(remap.get(column.getColumnName()))) .map(Enum::name) .collect(Collectors.toSet())) .containsAll(includedColumns); + + // only include RawAmount and Raw units if no isAllFromMaterialTable, + // needed to replace converted amount and unit values with raw values so audit difference is accurate + if (!isAllFromMaterialTable) + { + includedColumns.add(RawAmount.name()); + includedColumns.add(RawUnits.name()); + } + TableInfo selectTable = isAllFromMaterialTable ? ExperimentService.get().getTinfoMaterial() : getQueryTable(); if (isAllFromMaterialTable) { @@ -1554,7 +1717,7 @@ private static boolean isReservedHeader(String name) private static boolean isExpMaterialColumn(ExpMaterialTable.Column column, String name) { - return column.name().equalsIgnoreCase(name); + return column.name().equalsIgnoreCase(name) || (column.label() != null && column.label().equalsIgnoreCase(name)); } private static boolean isNameHeader(String name) @@ -1600,7 +1763,7 @@ public static boolean isUnitsHeader(String name) private static boolean isAliquotRollupHeader(String name) { Set rollupFields = new CaseInsensitiveHashSet(); - rollupFields.addAll(ALIQUOT_ROLLUP_FIELDS.keySet()); + rollupFields.addAll(ALIQUOT_ROLLUP_FIELDS.keySet().stream().map(ExpMaterialTable.Column::name).toList()); rollupFields.addAll(ALIQUOT_ROLLUP_FIELD_LABELS); return rollupFields.contains(name); } @@ -1780,13 +1943,13 @@ static class _SamplesCoerceDataIterator extends SimpleTranslator private static final String INVALID_NON_ALIQUOT_PROPERTY = "A sample property [%1$s] value has been ignored for an aliquot."; private final ExpSampleTypeImpl _sampleType; - private final Measurement.Unit _metricUnit; + private final Unit _sampleTypeBaseUnit; public _SamplesCoerceDataIterator(DataIterator source, DataIteratorContext context, ExpSampleTypeImpl sampleType, ExpMaterialTableImpl materialTable) { super(source, context); _sampleType = sampleType; - _metricUnit = _sampleType.getMetricUnit() != null ? Measurement.Unit.valueOf(_sampleType.getMetricUnit()) : null; + _sampleTypeBaseUnit = _sampleType.getBaseUnit(); setDebugName("Coerce before trigger script - samples"); init(materialTable, context.getInsertOption().useImportAliases, !context.getInsertOption().updateOnly); } @@ -1794,6 +1957,8 @@ public _SamplesCoerceDataIterator(DataIterator source, DataIteratorContext conte void init(TableInfo target, boolean useImportAliases, boolean initRollupCounts) { Map targetMap = DataIteratorUtil.createTableMap(target, useImportAliases); + Set amountImportAliasSet = ImportAliasable.Helper.createImportSet(target.getColumn(StoredAmount.name())); + Set unitsImportAliasSet = ImportAliasable.Helper.createImportSet(target.getColumn(Units.name())); DataIterator di = getInput(); int count = di.getColumnCount(); @@ -1814,9 +1979,9 @@ void init(TableInfo target, boolean useImportAliases, boolean initRollupCounts) { if (getAliquotedFromColName().equalsIgnoreCase(from.getName())) aliquotedFromDataColInd = i; - else if (Units.name().equalsIgnoreCase(from.getName())) + else if (unitsImportAliasSet.contains(from.getName())) unitDataColInd = i; - else if (StoredAmount.name().equalsIgnoreCase(from.getName()) || "Amount".equalsIgnoreCase(from.getName())) + else if (amountImportAliasSet.contains(from.getName())) amountDataColInd = i; } } @@ -1857,11 +2022,13 @@ else if (to.getFk() instanceof MultiValuedForeignKey) } else if (Units.name().equalsIgnoreCase(name)) { - addColumn(to, new SampleUnitsConvertColumn(name, i, to.getJdbcType())); + addColumn(PROVIDED_DATA_PREFIX + Units.name(), i); + addColumn(to, new SampleUnitsConvertColumn(name, i, to.getJdbcType(), amountDataColInd, !_context.getInsertOption().allowUpdate)); } else if (StoredAmount.name().equalsIgnoreCase(name)) { - addColumn(to, new SampleAmountConvertColumn(name, i, to.getJdbcType())); + addColumn(PROVIDED_DATA_PREFIX + StoredAmount.name(), i); + addColumn(to, new SampleAmountConvertColumn(name, i, to.getJdbcType(), unitDataColInd)); } else { @@ -1884,13 +2051,13 @@ else if (StoredAmount.name().equalsIgnoreCase(name)) if (initRollupCounts) { - for (Map.Entry entry : ALIQUOT_ROLLUP_FIELDS.entrySet()) + for (Map.Entry entry : ALIQUOT_ROLLUP_FIELDS.entrySet()) { - String fieldName = entry.getKey(); + ExpMaterialTable.Column field = entry.getKey(); JdbcType jdbcType = entry.getValue(); - var col = new BaseColumnInfo(fieldName, jdbcType); + var col = new BaseColumnInfo(field.name(), jdbcType); - addColumn(col, new AliquotRollupConvertColumn(fieldName, jdbcType, aliquotedFromDataColInd)); + addColumn(col, new AliquotRollupConvertColumn(field, jdbcType, aliquotedFromDataColInd)); } } } @@ -1922,32 +2089,99 @@ private void _addConvertColumn(ColumnInfo col, int fromIndex, int derivationData protected class SampleUnitsConvertColumn extends SimpleTranslator.SimpleConvertColumn { - public SampleUnitsConvertColumn(String fieldName, int indexFrom, @Nullable JdbcType to) + final int _storedAmountColInd; + final boolean _isInsert; + + public SampleUnitsConvertColumn(String fieldName, int indexFrom, @Nullable JdbcType to, int storedAmountIdx, boolean isInsert) { - // TODO reconcile unit handling super(fieldName, indexFrom, to, null, true); + _storedAmountColInd = storedAmountIdx; + _isInsert = isInsert; + } + + // This should return the base unit if we have one for the sample type since we are storing all data in the base unit + public static Object getValue(Object o, Object amountObj, boolean haveAmountCol, Unit baseUnit) + { + if (o == null || ((o instanceof String) && ((String) o).isEmpty())) + { + return null; + } + + // when there's a units value but no amount column, this is an error + if (!haveAmountCol) + throw new ConversionExceptionWithMessage(String.format(MISSING_COLUMN_VALUE_ERROR_MESSAGE_PATTERN, StoredAmount.label(), Units.name())); + + // When an amount column is present but no amount value is provided, this is an error + if (amountObj == null || ((amountObj instanceof String) && ((String) amountObj).isEmpty())) + throw new ConversionExceptionWithMessage(String.format(UNPROVIDED_VALUE_ERROR_MESSAGE_PATTERN, StoredAmount.label(), Units.name(), o)); + + + Unit validatedUnit = Unit.getValidatedUnit(o, baseUnit); + // if there's a base unit, return the base unit name otherwise return the name of the given unit + return validatedUnit == null ? null : baseUnit != null ? baseUnit.name() : validatedUnit.name(); } @Override protected Object convert(Object o) { - Measurement.validateUnits((String) o, _metricUnit); - return Measurement.Unit.getUnit((String) o); + return getValue(o, _storedAmountColInd == -1 ? null : _data.get(_storedAmountColInd), _storedAmountColInd != -1, _sampleTypeBaseUnit); } } protected class SampleAmountConvertColumn extends SimpleTranslator.SimpleConvertColumn { - public SampleAmountConvertColumn(String fieldName, int indexFrom, @Nullable JdbcType to) + final int _unitsColInd; + public SampleAmountConvertColumn(String fieldName, int indexFrom, @Nullable JdbcType to, int unitsColInd) { - // TODO reconcile unit handling - super(fieldName, indexFrom, to, null, true); + // should convert from the amount in the given unit into the sample type base unit, if we have one + super(fieldName, indexFrom, to, _sampleTypeBaseUnit, true); + _unitsColInd = unitsColInd; + } + + // This should return a Number in the base units of the sample type. + public static Object getValue(Object amountObj, boolean hasUnitsCol, Object unitsObj, Unit displayUnit) + { + if (amountObj == null || ((amountObj instanceof String) && ((String) amountObj).trim().isEmpty())) + return null; + + // When there is an amount value, if there isn't a units column, this is an error. + if (!hasUnitsCol) + throw new ConversionExceptionWithMessage(String.format(MISSING_COLUMN_VALUE_ERROR_MESSAGE_PATTERN, Units.name(), StoredAmount.label())); + + // Have a units column, but no units value + if (unitsObj == null || ((unitsObj instanceof String) && ((String) unitsObj).trim().isEmpty())) + { + // N.B. It could be that we want to support users providing the amount and unit together in the amount column (e.g., 7g) + // To support that we could try Quantity.convert here. Leaving this out for now, though. + throw new ConversionExceptionWithMessage(String.format(UNPROVIDED_VALUE_ERROR_MESSAGE_PATTERN , Units.name(), StoredAmount.label(), amountObj)); + } + + Unit unit = Unit.getValidatedUnit(unitsObj, displayUnit); + + // Should always be non-null at this point. + if (unit != null && displayUnit != null) + { + if (amountObj instanceof Number) + return Quantity.of((Number) amountObj, unit).doubleValue(); + else if (amountObj instanceof String amountStr) + { + if (StringUtils.isEmpty(amountStr.trim())) + return null; + // If the string value includes the unit (e.g., "7ml"), convert will handle that and should + // ignore the unit value. If the string amount does not have the unit (e.g., "7"), we use either the + // unit from the unit column or the sample type display unit. doubleValue returns the amount in the base unit. + return Quantity.convert(amountObj, unit).doubleValue(); + } + else + throw new ConversionException("Cannot convert " + amountObj + " to " + unit); + } + return amountObj; // have no display unit, so we'll do no conversions } @Override protected Object convert(Object amountObj) { - return Measurement.convertToAmount(amountObj); + return getValue(amountObj, _unitsColInd != -1, _unitsColInd == -1 ? null : _data.get(_unitsColInd), _sampleTypeBaseUnit); } } @@ -1955,10 +2189,9 @@ protected class AliquotRollupConvertColumn extends SimpleConvertColumn { final int aliquotedFromColInd; - public AliquotRollupConvertColumn(String fieldName, @Nullable JdbcType to, int aliquotedFromColInd) + public AliquotRollupConvertColumn(ExpMaterialTable.Column field, @Nullable JdbcType to, int aliquotedFromColInd) { - // TODO reconcile unit handling - super(fieldName, 0, to, null, true); + super(field.name(),0, to, field.hasUnit() ? _sampleTypeBaseUnit : null, true); this.aliquotedFromColInd = aliquotedFromColInd; } diff --git a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java index da23b12f716..545600dcee5 100644 --- a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java @@ -40,6 +40,7 @@ import org.labkey.api.data.ContainerFilter; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DbSchema; +import org.labkey.api.data.ImportAliasable; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.SqlSelector; @@ -738,16 +739,7 @@ public ConceptURIVocabularyDomainProvider getConceptUriVocabularyDomainProvider( @Override public Set getDomainPropertyImportAliases(DomainProperty property) { - if (property == null) - return Collections.emptySet(); - - Set aliases = new CaseInsensitiveHashSet(); - aliases.add(property.getName()); - if (property.getLabel() != null) - aliases.add(property.getLabel()); - if (!property.getImportAliasSet().isEmpty()) - aliases.addAll(property.getImportAliasSet()); - return aliases; + return ImportAliasable.Helper.createImportSet(property); } @Override diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index 5537fab3d14..882a01f66eb 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -4361,14 +4361,17 @@ public ExpSampleTypeImpl getSampleType(Container container) throws NotFoundExcep @RequiresPermission(InsertPermission.class) public static class ImportSamplesAction extends AbstractExpDataImportAction { + ExpSampleTypeImpl _sampleType; + boolean _isCrossTypeImport = false; + @Override public void validateForm(QueryForm queryForm, Errors errors) { _form = queryForm; _insertOption = queryForm.getInsertOption(); - boolean crossTypeImport = getOptionParamValue(Params.crossTypeImport); + _isCrossTypeImport = getOptionParamValue(Params.crossTypeImport); _form.setSchemaName(getTargetSchemaName()); - if (crossTypeImport) + if (_isCrossTypeImport) { _form.setQueryName(getPipelineTargetQueryName()); } @@ -4377,10 +4380,10 @@ public void validateForm(QueryForm queryForm, Errors errors) errors.reject(ERROR_REQUIRED, "Sample type name is required"); else { - if (!crossTypeImport) + if (!_isCrossTypeImport) { - ExpSampleTypeImpl sampleType = SampleTypeServiceImpl.get().getSampleType(getContainer(), getUser(), queryForm.getQueryName()); - if (sampleType == null) + _sampleType = SampleTypeServiceImpl.get().getSampleType(getContainer(), getUser(), queryForm.getQueryName()); + if (_sampleType == null) { errors.reject(ERROR_GENERIC, "Sample type '" + queryForm.getQueryName() + " not found."); } diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index 633e4f0729d..c094ca989ce 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -3118,7 +3118,7 @@ protected AuditTypeEvent createSummaryAuditRecord(User user, Container c, AuditC } @Override - protected DetailedAuditTypeEvent createDetailedAuditRecord(User user, Container c, AuditConfigurable tinfo, AuditAction action, @Nullable String userComment, @Nullable Map updatedRow, Map existingRow) + protected DetailedAuditTypeEvent createDetailedAuditRecord(User user, Container c, AuditConfigurable tinfo, AuditAction action, @Nullable String userComment, @Nullable Map updatedRow, Map existingRow, @Nullable Map providedValues) { DetailedAuditTypeEvent event = createAuditRecord(c, tinfo, action.getCommentDetailed(), updatedRow, existingRow); event.setUserComment(userComment); diff --git a/specimen/src/org/labkey/specimen/query/SpecimenUpdateService.java b/specimen/src/org/labkey/specimen/query/SpecimenUpdateService.java index a95ad0780c6..e9d88406623 100644 --- a/specimen/src/org/labkey/specimen/query/SpecimenUpdateService.java +++ b/specimen/src/org/labkey/specimen/query/SpecimenUpdateService.java @@ -141,7 +141,7 @@ public List> deleteRows(User user, Container container, List getQueryTable().fireBatchTrigger(container, user, TableInfo.TriggerType.DELETE, false, errors, extraScriptContext); - addAuditEvent(user, container, QueryService.AuditAction.DELETE, configParameters, null, null); + addAuditEvent(user, container, QueryService.AuditAction.DELETE, configParameters, null, null, null); return new ArrayList<>(); } @@ -408,7 +408,7 @@ public List> updateRows(User user, Container container, List getQueryTable().fireBatchTrigger(container, user, TableInfo.TriggerType.UPDATE, false, errors, extraScriptContext); - addAuditEvent(user, container, QueryService.AuditAction.UPDATE, configParameters, rows, null); + addAuditEvent(user, container, QueryService.AuditAction.UPDATE, configParameters, rows, null, null); return getRows(user, container, newKeys); } diff --git a/study/src/org/labkey/study/model/DatasetDefinition.java b/study/src/org/labkey/study/model/DatasetDefinition.java index 1cc055e6204..677c88a1ded 100644 --- a/study/src/org/labkey/study/model/DatasetDefinition.java +++ b/study/src/org/labkey/study/model/DatasetDefinition.java @@ -1760,7 +1760,7 @@ public void addAuditEvent(User user, Container c, TableInfo table, @Nullable Aud Map row = rows.get(i); Map existingRow = null==existingRows ? null : existingRows.get(i); // note switched order (oldRecord, newRecord) - var event = createDetailedAuditRecord(user, c, (AuditConfigurable)table, action, userComment, row, existingRow); + var event = createDetailedAuditRecord(user, c, (AuditConfigurable)table, action, userComment, row, existingRow, null); batch.add(event); if (batch.size() > 1000) { @@ -1787,7 +1787,7 @@ protected AuditTypeEvent createSummaryAuditRecord(User user, Container c, AuditC * NOTE: userComment field is not supported for this domain and will be ignored */ @Override - protected DatasetAuditProvider.DatasetAuditEvent createDetailedAuditRecord(User user, Container c, AuditConfigurable tInfo, QueryService.AuditAction action, @Nullable String userComment, @Nullable Map record, Map existingRecord) + protected DatasetAuditProvider.DatasetAuditEvent createDetailedAuditRecord(User user, Container c, AuditConfigurable tInfo, QueryService.AuditAction action, @Nullable String userComment, @Nullable Map record, Map existingRecord, Map providedValues) { String auditComment = switch (action) { @@ -2400,7 +2400,7 @@ public DataIteratorBuilder getInsertDataIterator(User user, Container container, ((TableInsertDataIteratorBuilder) persist).setDontUpdate(dontUpdate); } - DataIteratorBuilder audit = DetailedAuditLogDataIterator.getDataIteratorBuilder(table, persist, context.getInsertOption(), user, target); + DataIteratorBuilder audit = DetailedAuditLogDataIterator.getDataIteratorBuilder(table, persist, context.getInsertOption(), user, target, null); return LoggingDataIterator.wrap(audit); }