diff --git a/api/src/messages/Validation.properties b/api/src/messages/Validation.properties index 9c8254df7b3..1f73b40a553 100644 --- a/api/src/messages/Validation.properties +++ b/api/src/messages/Validation.properties @@ -10,6 +10,6 @@ UniqueViolationError=The value of the {0} field conflicts with another value in typeMismatch=This value could not be converted typeMismatch.int=Please enter a valid integer value typeMismatch.double=Please enter a valid floating point number -tyepMismatch.Date=Please enter a valid date +typeMismatch.Date=Please enter a valid date requiredError=This field is required uniqueConstraint=Value conflicts with existing data. Please enter a unique value. diff --git a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java index 3ea147def9d..e3847e80643 100644 --- a/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java +++ b/api/src/org/labkey/api/assay/actions/AssayRunUploadForm.java @@ -509,7 +509,7 @@ else if (AbstractAssayProvider.PARTICIPANT_VISIT_RESOLVER_PROPERTY_NAME.equals(k ColumnInfo pk = pks.get(0); try { - Object filterValue = ConvertUtils.convert(value, pk.getJavaClass()); + Object filterValue = pk.convert(value); SimpleFilter filter = new SimpleFilter(pk.getFieldKey(), filterValue); Set cols = new HashSet<>(); cols.add(lookupTable.getTitleColumn()); diff --git a/api/src/org/labkey/api/data/AbstractWrappedColumnInfo.java b/api/src/org/labkey/api/data/AbstractWrappedColumnInfo.java index 73165428e45..4c242f2e75f 100644 --- a/api/src/org/labkey/api/data/AbstractWrappedColumnInfo.java +++ b/api/src/org/labkey/api/data/AbstractWrappedColumnInfo.java @@ -848,4 +848,10 @@ public boolean isMultiValued() { return delegate.isMultiValued(); } + + @Override @Transient + public final SimpleConvert getConvertFn() + { + return ColumnRenderProperties.getDefaultConvertFn(this); + } } diff --git a/api/src/org/labkey/api/data/BaseColumnInfo.java b/api/src/org/labkey/api/data/BaseColumnInfo.java index ccd50829965..b230373a374 100644 --- a/api/src/org/labkey/api/data/BaseColumnInfo.java +++ b/api/src/org/labkey/api/data/BaseColumnInfo.java @@ -75,6 +75,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; /** @@ -2229,4 +2230,10 @@ public void setRemapMissingBehavior(SimpleTranslator.RemapMissingBehavior missin { _remapMissingBehavior = missingBehavior; } + + @Override @Transient + public final SimpleConvert getConvertFn() + { + return ColumnRenderProperties.getDefaultConvertFn(this); + } } diff --git a/api/src/org/labkey/api/data/BeanViewForm.java b/api/src/org/labkey/api/data/BeanViewForm.java index 8d7677d991b..e098b951fe4 100644 --- a/api/src/org/labkey/api/data/BeanViewForm.java +++ b/api/src/org/labkey/api/data/BeanViewForm.java @@ -124,12 +124,14 @@ else if (o instanceof Map) } @Override - protected Class getTruePropType(String propName) + protected SimpleConvert getSimpleConvert(String propName) { - var ret = _dynaClass.getTruePropType(propName); - if (null == ret) - ret = super.getTruePropType(propName); - return ret; + var type = _dynaClass.getTruePropType(propName); + if (null != type) + { + return ConvertHelper.getSimpleConvert(type); + } + return super.getSimpleConvert(propName); } // DynaBean diff --git a/api/src/org/labkey/api/data/ColumnInfoTests.jsp b/api/src/org/labkey/api/data/ColumnInfoTests.jsp new file mode 100644 index 00000000000..c2e82a678c1 --- /dev/null +++ b/api/src/org/labkey/api/data/ColumnInfoTests.jsp @@ -0,0 +1,259 @@ +<%@ page import="org.labkey.api.data.JdbcType" %> +<%@ page import="org.junit.Test" %> +<%@ page import="static org.junit.Assert.*" %> +<%@ page import="org.labkey.api.data.BaseColumnInfo" %> +<%@ page import="org.jetbrains.annotations.NotNull" %> +<%@ page import="java.math.BigDecimal" %> +<%@ page import="org.labkey.api.exp.PropertyType" %> +<%@ page import="org.labkey.api.ontology.Quantity" %> +<%@ page import="org.labkey.api.ontology.Unit" %> +<%@ page import="org.labkey.api.ontology.KindOfQuantity" %> +<%@ page import="org.labkey.api.data.MutableColumnInfo" %> +<%@ page import="org.labkey.api.data.WrappedColumnInfo" %> +<%@ page import="org.apache.commons.beanutils.ConversionException" %> +<%@ page import="org.labkey.api.data.ColumnInfo" %> +<%@ page import="org.labkey.api.data.dialect.SqlDialect" %> +<%@ page import="org.labkey.api.data.CoreSchema" %> +<%@ page import="java.nio.ByteBuffer" %> +<%@ page extends="org.labkey.api.jsp.JspTest.BVT" %> +<%-- +This tests uses MockRequest to test some expected Headers and Meta tags for various types of requests. +--%> +<%! + void testConvert(ColumnInfo col, Object expected, Object val) + { + var result = col.convert(val); + assertNotNull(result); + assertEquals(col.getJdbcType().getJavaClass(), result.getClass()); + assertEquals(expected, result); + } + + void testConvertsToNull(ColumnInfo col, Object val) + { + var result = col.convert(val); + assertNull(result); + } + + void testConversionException(ColumnInfo col, Object val) + { + try + { + col.convert(val); + fail(); + } + catch (ConversionException x) + { + return; + } + } + + void testConvert(JdbcType type, Object expected, Object val) + { + var col = new BaseColumnInfo("~", null, type); + testConvert(col.lock(), expected, val); + } + + void testConvertsToNull(JdbcType type, Object val) + { + var col = new BaseColumnInfo("~", null, type); + testConvertsToNull(col.lock(), val); + } + + void testConversionException(JdbcType type, Object val) + { + var col = new BaseColumnInfo("~", null, type); + testConversionException(col.lock(), val); + } + + void testConvert(PropertyType pt, Object expected, @NotNull Object val) + { + var col = new BaseColumnInfo("~", null, pt.getJdbcType()); + col.setPropertyType(pt); + testConvert(col.lock(), expected, val); + } + + void testConvertsToNull(PropertyType pt, Object val) + { + var col = new BaseColumnInfo("~", null, pt.getJdbcType()); + col.setPropertyType(pt); + testConvertsToNull(col.lock(), val); + } + + void testConversionException(PropertyType pt, Object val) + { + var col = new BaseColumnInfo("~", null, pt.getJdbcType()); + col.setPropertyType(pt); + testConversionException(col.lock(), val); + } + + void testQuantity(Unit displayUnit, Quantity expected, Object value) + { + // UNDONE: setDisplayUnit is NYI??? + var col = new BaseColumnInfo("~", null, JdbcType.DOUBLE) + { + @Override + public Unit getDisplayUnit() + { + return displayUnit; + } + + @Override + public KindOfQuantity getKindOfQuantity() + { + return null==displayUnit ? null : displayUnit.getKindOfQuantity(); + } + }; + testConvert(col.lock(), expected, value); + } + + + /** This test is for the integrated ColumnInfo.convert() logic. + *

+ * A lot of this testing is redunant with lower-level unit testing, + * however, his till servers as a basic conversion smoke test. + *

+ * In particualr, the PropertyType conversions are pretty + * redundant with ConvertHelper.convert() and JdbcType.convert() + * (PropertyType predates JdbcType), but there are some differenes + * in implementation. We should try to reconcile these differences. + */ +// @Test + public void testColumnConvert() throws Exception + { + // see also ConvertHelper.testEmpty() + + // w/o propertyType + for (JdbcType type : JdbcType.values()) + { + switch (type) + { + case BIGINT -> + { + testConvert(type, Long.valueOf(5), Integer.valueOf(5)); + testConvert(type, Long.valueOf(5), "5"); + testConvert(type, Long.valueOf(5), Double.valueOf(5.00000)); + testConversionException(type, Double.valueOf(5.00001)); + testConvert(type, Long.valueOf(5), new BigDecimal("5.000")); + testConversionException(type, new BigDecimal("5.001")); + testConversionException(type, "5g"); + testConvertsToNull(type, ""); + testConvertsToNull(type, null); + } + case BINARY, LONGVARBINARY, VARBINARY -> + { + testConvert(type, ByteBuffer.wrap(new byte[] {0x00,0x00,0x00,0x05}), Long.valueOf(5)); + } + case BOOLEAN -> {} + case CHAR,LONGVARCHAR,VARCHAR -> + { + testConvertsToNull(type, null); + // NOTE StandardDataIterator optionally trims, but convert() does not. + // see SimpleTranslator.createConvertColumn() + testConvert(type, " no trim ", " no trim "); + // JdbcType does not convert empty string to null, ColumnInfo.convert() and PropertyType.conver() do + assertEquals("", type.convert("")); + testConvertsToNull(type, ""); + testConvertsToNull(type, null); + } + case DECIMAL -> {} + case DOUBLE -> {} + case INTEGER -> {} + case REAL -> {} + case SMALLINT, TINYINT -> {} + case DATE -> {} + case TIME -> {} + case TIMESTAMP -> {} + case GUID -> {} + case ARRAY, NULL, OTHER -> { /* ignore */ } + default -> fail("We missed a JdbcType: " + type.name()); + } + } + + // testArray() + + // w/ propertyType + for (var type : PropertyType.values()) + { + switch (type) + { + case BOOLEAN -> {} + case STRING -> + { + testConvertsToNull(type, null); + testConvertsToNull(type, ""); + testConvert(type, " no trim ", " no trim "); + } + case MULTI_LINE -> + { + testConvertsToNull(type, null); + testConvertsToNull(type, ""); + testConvert(type, " no trim ", " no trim "); + } + case MULTI_CHOICE -> {} + case RESOURCE -> {} + case INTEGER -> {} + case BIGINT -> + { + testConvert(type, Long.valueOf(5), Integer.valueOf(5)); + testConvert(type, Long.valueOf(5), "5"); + testConvert(type, Long.valueOf(5), new BigDecimal("5.001")); + testConvertsToNull(type, null); + testConvertsToNull(type, ""); + testConversionException(type, "5g"); + } + case BINARY -> {} + case FILE_LINK -> {} + case ATTACHMENT -> {} + case DATE_TIME -> {} + case DATE -> {} + case TIME -> {} + case DOUBLE -> {} + case FLOAT -> {} + case DECIMAL -> {} + case XML_TEXT -> {} + default -> fail("We missed a PropertyType: " + type.name()); + } + } + + // Quantity + Unit unit = Unit.kg; + testQuantity(unit, Quantity.of(5000,unit.getBase()), "5"); + } + + + public void testLocked(MutableColumnInfo col) + { + col.setAlias("!"); + col.lock(); + try + { + col.setAlias("!"); + fail("not locked?"); + } + catch (IllegalStateException x) + { + // success + } + } + + static class _ColumnInfo extends BaseColumnInfo + { + _ColumnInfo() + { + super("~", JdbcType.INTEGER); + } + + @Override + public SqlDialect getSqlDialect() + { + return CoreSchema.getInstance().getSqlDialect(); + } + } + +// @Test + public void testLocked() + { + testLocked(new _ColumnInfo()); + testLocked(WrappedColumnInfo.wrap(new _ColumnInfo())); + } +%> diff --git a/api/src/org/labkey/api/data/ColumnRenderProperties.java b/api/src/org/labkey/api/data/ColumnRenderProperties.java index 6d7c25dce97..fa0ee020d33 100644 --- a/api/src/org/labkey/api/data/ColumnRenderProperties.java +++ b/api/src/org/labkey/api/data/ColumnRenderProperties.java @@ -15,6 +15,7 @@ */ package org.labkey.api.data; +import org.apache.commons.beanutils.ConversionException; import org.apache.commons.beanutils.ConvertUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -41,7 +42,7 @@ import static org.labkey.api.ontology.OntologyService.conceptCodeConceptURI; -public interface ColumnRenderProperties extends ImportAliasable +public interface ColumnRenderProperties extends ImportAliasable, SimpleConvert { void copyTo(ColumnRenderPropertiesImpl to); @@ -235,11 +236,11 @@ default String getSourceOntology() * From an implementation point of view we need a path from the ontology.hierarchy table. From the user's * point of view we only need a concept (assuming hierarchy table is complete and consistent WRT subclass * relationship between concepts). - * + *
* This should be set to an unambiguous path of conceptid (e.g. /NCI:concept1/NCI:concept2). However, we will * accept a simple conceptid and map to a path and hope the hierarchy tree is consistent e.g. {set of concepts * under (path1)/CONCEPT/} == {set of concepts under (alternate path2)/CONCEPT/} - * + *
* Use OntologyManager.resolveSubtreePath(crp.getConceptSubtree()) to get ontology.hierarchy.path. */ default String getConceptSubtree() @@ -319,13 +320,7 @@ default Function getTsvFormatFn() return getDefaultFormatFn(getName(), getJavaObjectClass(), getDisplayUnit(), getTsvFormatString(), DisplayColumn.tsvFormatSymbols); } - @Transient - default Function getConvertFn() - { - return getDefaultConvertFn(this); - } - - static Function getDefaultFormatFn(String colName, Class javaObjectClass, final Unit displayUnit, String formatString, DecimalFormatSymbols dfs) + static Function getDefaultFormatFn(String colName, Class javaObjectClass, final Unit displayUnit, String formatString, DecimalFormatSymbols dfs) { final var format = null==formatString ? null : DisplayColumn.createFormat(formatString, javaObjectClass, dfs); @@ -376,27 +371,28 @@ else if (value instanceof String) }; } - /* empty string -> null */ - static Function getDefaultConvertFn(ColumnRenderProperties col) + /** + * The returned Function<Object,Object> should throw ConversionException (undeclared RuntimeException). + * This method does not handle compound conversions e.g. MissingValues or Out-of-range indicators. + */ + @Override @Transient + SimpleConvert getConvertFn(); + + /** see getConvertFn() */ + @Override + default Object convert(Object o) throws ConversionException { - final Class javaClass = col.getJavaObjectClass(); - final var defaultUnit = col.getDisplayUnit(); - final @NotNull var jdbcType = col.getJdbcType(); + return getConvertFn().convert(o); + } - if (null != defaultUnit) - return defaultUnit::convert; + static SimpleConvert getDefaultConvertFn(ColumnRenderProperties col) + { + if (null != col.getDisplayUnit()) + return col.getDisplayUnit(); - if (PropertyType.MULTI_CHOICE == col.getPropertyType()) - return MultiChoice.Converter.getInstance(); + if (null != col.getPropertyType()) + return col.getPropertyType(); - return (value) -> - { - // quick check for unnecessary conversion - if (value == null || javaClass == value.getClass()) - return value; - if (value instanceof CharSequence) - return ConvertUtils.convert(value.toString(), javaClass); - return jdbcType.convert(value); - }; + return col.getJdbcType(); } } diff --git a/api/src/org/labkey/api/data/CompareType.java b/api/src/org/labkey/api/data/CompareType.java index d9165ae67da..6862e55a9b3 100644 --- a/api/src/org/labkey/api/data/CompareType.java +++ b/api/src/org/labkey/api/data/CompareType.java @@ -827,6 +827,38 @@ protected Collection getCollectionParam(Object value) * * */ + + + public static final CompareType ARRAY_IS_EMPTY = new CompareType("Is Empty", "arrayisempty", "ARRAYISEMPTY", false, null, OperatorType.ARRAYISEMPTY) + { + @Override + public ArrayIsEmptyClause createFilterClause(@NotNull FieldKey fieldKey, Object value) + { + return new ArrayIsEmptyClause(fieldKey); + } + + @Override + public boolean meetsCriteria(ColumnRenderProperties col, Object value, Object[] filterValues) + { + throw new UnsupportedOperationException("Conditional formatting not yet supported for Multi Choices"); + } + }; + + public static final CompareType ARRAY_IS_NOT_EMPTY = new CompareType("Is Not Empty", "arrayisnotempty", "ARRAYISNOTEMPTY", false, null, OperatorType.ARRAYISNOTEMPTY) + { + @Override + public ArrayIsEmptyClause createFilterClause(@NotNull FieldKey fieldKey, Object value) + { + return new ArrayIsNotEmptyClause(fieldKey); + } + + @Override + public boolean meetsCriteria(ColumnRenderProperties col, Object value, Object[] filterValues) + { + throw new UnsupportedOperationException("Conditional formatting not yet supported for Multi Choices"); + } + }; + public static final CompareType ARRAY_CONTAINS_ALL = new CompareType("Contains All", "arraycontainsall", "ARRAYCONTAINSALL", true, null, OperatorType.ARRAYCONTAINSALL) { @Override @@ -981,6 +1013,68 @@ public Pair getSqlFragments(Map columnMap, SqlDialect dialect) + { + ColumnInfo colInfo = columnMap != null ? columnMap.get(_fieldKey) : null; + var alias = SimpleFilter.getAliasForColumnFilter(dialect, colInfo, _fieldKey); + + SQLFragment columnFragment = new SQLFragment().appendIdentifier(alias); + + SQLFragment sql = dialect.array_is_empty(columnFragment); + if (!_negated) + return sql; + return new SQLFragment(" NOT (").append(sql).append(")"); + } + + @Override + public String getLabKeySQLWhereClause(Map columnMap) + { + return "array_is_empty(" + getLabKeySQLColName(_fieldKey) + ")"; + } + + @Override + public void appendFilterText(StringBuilder sb, ColumnNameFormatter formatter) + { + sb.append("is empty"); + } + + } + + private static class ArrayIsNotEmptyClause extends ArrayIsEmptyClause + { + + public ArrayIsNotEmptyClause(@NotNull FieldKey fieldKey) + { + super(fieldKey, CompareType.ARRAY_IS_NOT_EMPTY, true); + } + + @Override + public String getLabKeySQLWhereClause(Map columnMap) + { + return "NOT array_is_empty(" + getLabKeySQLColName(_fieldKey) + ")"; + } + + @Override + public void appendFilterText(StringBuilder sb, ColumnNameFormatter formatter) + { + sb.append("is not empty"); + } + + } + private static class ArrayContainsAllClause extends ArrayClause { diff --git a/api/src/org/labkey/api/data/ConvertHelper.java b/api/src/org/labkey/api/data/ConvertHelper.java index 47c1ee02103..2b3eed5196a 100644 --- a/api/src/org/labkey/api/data/ConvertHelper.java +++ b/api/src/org/labkey/api/data/ConvertHelper.java @@ -175,7 +175,7 @@ protected void register() _register(new NullSafeConverter(new ShortConverter()), Short.class); _register(new ShortConverter(), Short.TYPE); _register(new NullSafeConverter(new ShortArrayConverter()), short[].class); - _register(new NullSafeConverter(new DateFriendlyStringConverter()), String.class); + _register(new DateFriendlyStringConverter(), String.class); _register(new StringArrayConverter(), String[].class); _register(new URLHelper.Converter(), URLHelper.class); _register(new StringExpressionFactory.Converter(), StringExpression.class); @@ -224,18 +224,11 @@ public NullSafeConverter(Converter converter) @Override public T convert(Class clss, Object o) { - if (o instanceof String s) - { - // 2956 : AbstractConvertHelper shouldn't trim Strings - //o = ((String) o).trim(); - if (s.isEmpty()) - return null; - } - - return null == o ? null : _converter.convert(clss, o); + return null == o || "".equals(o) ? null : _converter.convert(clss, o); } } + // Issue 34334: Add 't' and 'f' to BooleanConverter in order to support PostgreSQL array_to_string of booleans array. // For example, array_to_string(array_agg(array[true, false]), '|') ==> returns 't|f' public static class BooleanConverter implements Converter @@ -267,7 +260,6 @@ else if (str.equalsIgnoreCase("f")) */ public static class LenientTimeOnlyConverter implements Converter { - @Override public Object convert(Class clss, Object o) { @@ -277,7 +269,10 @@ public Object convert(Class clss, Object o) if (o instanceof Time || o instanceof TimeOnlyDate) return o; - return new TimeOnlyDate(DateUtil.parseSimpleTime(o).getTime()); + var d = DateUtil.parseSimpleTime(o.toString()); + if (null == d) + throw new ConversionException("Could not convert \"" + o + "\" to time."); + return new TimeOnlyDate(d.getTime()); } } @@ -406,8 +401,13 @@ public static class DateFriendlyStringConverter implements Converter @Override public Object convert(Class clss, Object o) { - if (o instanceof String) - return o; + if (null == o) + return null; + + // MAB I think there's an argument that ""->null conversion should be left to more specific converters + // e.g. ColumnInfo.convert(), PropertyType.convert(), or class SimpleConvertColumn + if (o instanceof String s) + return s.isEmpty() ? null : s; if (o instanceof Container) return ((Container)o).getId(); @@ -601,7 +601,6 @@ public _IntegerConverter(Object defaultValue) this.defaultValue = defaultValue; this.useDefault = true; - } // ----------------------------------------------------- Instance Variables @@ -799,7 +798,23 @@ public static T convert(Object value, Class cl) { return null; } - return (T)ConvertUtils.convert(value.toString(), cl); + return cl.cast(ConvertUtils.convert(value.toString(), cl)); + } + + public static SimpleConvert getSimpleConvert(Class targetType) + { + // ConvertUtils handling String.class and String[].class depends on the source type, + // for these we want the full ConvertUtils.convert(). + Converter converterLookup; + if (targetType == String.class || targetType == String[].class || null == (converterLookup = ConvertUtils.lookup(targetType))) + return (value) -> ConvertUtils.convert(value, targetType); + + // For other usages we can call our registered converter directly. + final Converter converterFinal = converterLookup; + if (converterFinal instanceof SimpleConvert simple) + return simple; + + return (value) -> converterFinal.convert(targetType, value); } public static class ConvertUtilsEditor extends PropertyEditorSupport @@ -1008,7 +1023,11 @@ public static class EnumConverter implements Converter public Object convert(Class type, Object value) { if (!type.isEnum()) + { + if (type == String.class) + return value; throw new IllegalArgumentException(); + } if (value == null) return null; @@ -1221,11 +1240,9 @@ public void testEmpty() assertNull(JdbcType.VARCHAR.convert("")); assertNull(JdbcType.LONGVARCHAR.convert("")); - // PropertyType is used for domain defined tables. - // I would expect these to return null. - assertEquals("", PropertyType.STRING.convert("")); - assertEquals("", PropertyType.MULTI_LINE.convert("")); - assertEquals("", PropertyType.XML_TEXT.convert("")); + assertNull(PropertyType.STRING.convert("")); + assertNull(PropertyType.MULTI_LINE.convert("")); + assertNull(PropertyType.XML_TEXT.convert("")); // Since we often convert "through" string, I'm not sure this low-level // method should modify "". This could potentially mess up @@ -1268,6 +1285,44 @@ public void testTrim() assertEquals(" x ", ConvertUtils.convert(" x ")); assertEquals(" x ", ConvertUtils.convert(" x ", String.class)); } + + @Test + public void testMiscConversions() + { + // Container, java.sql.Time, java.util.Date, java.sql.Clob all had special cases coded into + // the registered String.class converter (DateFriendlyStringConverter). These conversions + // should be handled in the registered converters for each class e.g. LenientTimeConverter for java.sql.Time + // Test that this works after (and before) refactoring. + + // there are date/time converters for : + // java.sql.Date, java.sql.Time, java.sql.Timestamp, + // java.util.Date, + // org.labkey.api.util.TimeOnlyDate, org.labkey.api.util.SimpleTime + + java.util.Date utilDate = (java.util.Date)ConvertUtils.convert("2 Jan 2024 01:02:03.005", java.util.Date.class); + assertEquals("2024-01-02 01:02:03.005", ConvertUtils.convert(utilDate, String.class)); + + Timestamp timestamp = (Timestamp)ConvertUtils.convert("2 Jan 2024 01:02:03.005", Timestamp.class); + assertEquals("2024-01-02 01:02:03.005", ConvertUtils.convert(timestamp, String.class)); + + Time time = (Time)ConvertUtils.convert("01:02:03.500", Time.class); + assertEquals("01:02:03.500", ConvertUtils.convert(time, String.class)); +// BUG handle in TimeOnlyDateCoverter or since this is a LabKey class in TimeOnlyDate.toString() +// assertEquals("01:02:03.500", ConvertUtils.convert(new TimeOnlyDate(time.getTime()), String.class)); + assertEquals("01:02:03.500", ConvertUtils.convert(new SimpleTime(time.getTime()), String.class)); + + time = (Time)ConvertUtils.convert("01:02:03", Time.class); + assertEquals("01:02:03", ConvertUtils.convert(time, String.class)); +// assertEquals("01:02:03", ConvertUtils.convert(new TimeOnlyDate(time.getTime()), String.class)); + assertEquals("01:02:03", ConvertUtils.convert(new SimpleTime(time.getTime()), String.class)); + + java.sql.Date sqlDate = (java.sql.Date)ConvertUtils.convert("2 Jan 2024", java.sql.Date.class); + assertEquals("2024-01-02", ConvertUtils.convert(sqlDate, String.class)); + + Container home = ContainerManager.getHomeContainer(); + assertEquals(home.getRowId(), ((Container)ConvertUtils.convert(home.getId(), Container.class)).getRowId()); + assertEquals(home.getId(), ConvertUtils.convert(home, String.class)); + } } // Note: Keep in sync with LabKeySiteWrapper.getConversionErrorMessage() @@ -1280,6 +1335,6 @@ public static String getStandardConversionErrorMessage(Object value, String fiel if (fieldType.equalsIgnoreCase("date") || fieldType.equalsIgnoreCase("datetime") || fieldType.equalsIgnoreCase("timestamp")) return "'" + value + "' is not a valid " + fieldType + " for " + fieldName + " using " + LookAndFeelProperties.getInstance(ContainerManager.getRoot()).getDateParsingMode().getDisplayString(); - return "Could not convert value '" + value + "' (" + value.getClass().getSimpleName() + ") for " + fieldType + " field '" + fieldName + "'" ; + return "Could not convert value '" + value + "' (" + value.getClass().getSimpleName() + ") for " + fieldType + (null==fieldName ? "" : " field '" + fieldName + "'"); } } diff --git a/api/src/org/labkey/api/data/DataColumn.java b/api/src/org/labkey/api/data/DataColumn.java index 7ae99e64a0e..513c4b53d52 100644 --- a/api/src/org/labkey/api/data/DataColumn.java +++ b/api/src/org/labkey/api/data/DataColumn.java @@ -649,7 +649,7 @@ protected String getStringValue(Object value, Unit unit, boolean disabledInput) } } else - strVal = ConvertUtils.convert(value); + strVal = ConvertHelper.convert(value, String.class); } return strVal; } diff --git a/api/src/org/labkey/api/data/ExcelColumn.java b/api/src/org/labkey/api/data/ExcelColumn.java index 844248a7096..bd869f07578 100644 --- a/api/src/org/labkey/api/data/ExcelColumn.java +++ b/api/src/org/labkey/api/data/ExcelColumn.java @@ -833,8 +833,8 @@ public void testExportFormatting() throws Exception String actual = formatter.formatCellValue(cell); assertEquals("Incorrect Excel Value", expected, actual); - Object expectedObj = ConvertHelper.convert(expected, ci.getJavaClass()); - Object actualObj = ConvertHelper.convert(actual, ci.getJavaClass()); + Object expectedObj = ci.convert(expected); + Object actualObj = ci.convert(actual); assertEquals("Incorrect Parsed Excel Value", expectedObj, actualObj); } } @@ -846,7 +846,7 @@ private String parseAndFormatExpected(Object value, ColumnInfo ci) Format fmt = ci.getDisplayColumnFactory().createRenderer(ci).getFormat(); if (fmt != null) { - value = ConvertHelper.convert(value, ci.getJavaClass()); + value = ci.convert(value); return fmt.format(value); } diff --git a/api/src/org/labkey/api/data/JdbcType.java b/api/src/org/labkey/api/data/JdbcType.java index 1367a5c488e..9226bf9c189 100644 --- a/api/src/org/labkey/api/data/JdbcType.java +++ b/api/src/org/labkey/api/data/JdbcType.java @@ -16,9 +16,6 @@ package org.labkey.api.data; import org.apache.commons.beanutils.ConversionException; -import org.apache.commons.beanutils.ConvertUtils; -import org.apache.commons.beanutils.Converter; -import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.json.JSONArray; @@ -32,6 +29,7 @@ import java.math.BigInteger; import java.nio.ByteBuffer; import java.sql.Array; +import java.sql.SQLException; import java.sql.Time; import java.sql.Timestamp; import java.sql.Types; @@ -42,13 +40,14 @@ import java.util.HashMap; import java.util.LinkedList; +import static org.apache.commons.lang3.StringUtils.isBlank; import static org.labkey.api.util.IntegerUtils.isIntegral; /** * ENUM version of java.sql.Types */ -public enum JdbcType +public enum JdbcType implements SimpleConvert { BIGINT(Types.BIGINT, Long.class, Long.TYPE) { @@ -92,11 +91,23 @@ protected void addSqlTypes(Collection sqlTypes) sqlTypes.add(Types.NCHAR); } + @Override + public boolean isEmpty(Object value) + { + return VARCHAR.isEmpty(value); + } + @Override public Object convert(Object o) throws ConversionException { return VARCHAR.convert(o); } + + @Override + public Object convert(CharSequence cs) + { + return VARCHAR.convert(cs); + } }, DECIMAL(Types.DECIMAL, BigDecimal.class, null, "numberfield") @@ -153,8 +164,15 @@ protected Object _fromNumber(Number n) @Override protected Object _fromString(String s) { - // Be tolerant of trailing decimal zeros like "39.0", which Integer.parseInt() is not - return new BigDecimal(s.trim()).intValueExact(); + try + { + // Be tolerant of trailing decimal zeros like "39.0", which Integer.parseInt() is not + return new BigDecimal(s.trim()).intValueExact(); + } + catch (NumberFormatException x) + { + return null; + } } }, @@ -176,11 +194,23 @@ protected void addSqlTypes(Collection sqlTypes) sqlTypes.add(Types.LONGNVARCHAR); } + @Override + public boolean isEmpty(Object value) + { + return VARCHAR.isEmpty(value); + } + @Override public Object convert(Object o) throws ConversionException { return VARCHAR.convert(o); } + + @Override + public Object convert(CharSequence cs) + { + return VARCHAR.convert(cs); + } }, REAL(Types.REAL, Float.class, Float.TYPE, "numberfield") @@ -284,17 +314,29 @@ protected void addSqlTypes(Collection sqlTypes) sqlTypes.add(Types.NVARCHAR); } + @Override + public boolean isEmpty(Object value) + { + return null==value || "".equals(value); + } + @Override public Object convert(Object o) throws ConversionException { - String s = (String)super.convert(o); - return null==s || s.isEmpty() ? null : s; + if (null == o) + return null; + if (o instanceof String s) + return s.isEmpty() ? null : s; + var ret = converter.convert(o); + return "".equals(ret) ? null : ret; } @Override - protected Object _fromDate(Date d) + public Object convert(CharSequence cs) throws ConversionException { - return DateUtil.toISO(d); // don't shorten + if (null == cs || cs.isEmpty()) + return null; + return cs.toString(); } }, @@ -308,7 +350,34 @@ protected Collection getSqlTypes() } }, - ARRAY(Types.ARRAY, Array.class), + ARRAY(Types.ARRAY, Array.class) + { + @Override + public boolean isEmpty(Object value) + { + if (null == value) + return true; + if (value instanceof String s) + return isBlank(s); + if (value instanceof Object[] arr) + return 0==arr.length; + else if (value instanceof Collection coll) + return coll.isEmpty(); + else if (value instanceof java.sql.Array sqlArray) + { + try + { + Object[] arr = (Object[]) sqlArray.getArray(); + return null == arr || 0==arr.length; + } + catch (SQLException sqlx) + { + throw new RuntimeSQLException(sqlx); + } + } + throw new IllegalArgumentException("illegal value " + value.getClass()); + } + }, NULL(Types.NULL, Object.class), @@ -321,7 +390,7 @@ protected Collection getSqlTypes() public final String json; private final Class typeCls; - private final Converter converter; + protected final SimpleConvert converter; JdbcType(int type, @NotNull Class cls) @@ -344,7 +413,7 @@ protected Collection getSqlTypes() this.typeCls = typeCls; this.xtype = xtype; this.json = DisplayColumn.getJsonTypeName(cls); - this.converter = ConvertUtils.lookup(cls); + this.converter = ConvertHelper.getSimpleConvert(cls); } private static final HashMap, JdbcType> classMap = new HashMap<>(); @@ -458,13 +527,20 @@ public static JdbcType promote(@Nullable JdbcType a, @Nullable JdbcType b) return OTHER; } + /** + * returns true if value should be considered to fail an isRequired() check. + * value should be already be coverted (e.g. via JdbcType.convert()) + */ + public boolean isEmpty(Object value) + { + return null == value; + } public boolean isText() { return this.cls == String.class; } - public boolean isNumeric() { return Number.class.isAssignableFrom(this.cls); @@ -509,6 +585,7 @@ public Class getJavaClass(boolean isNullable) return isNullable || null == typeCls ? cls : typeCls; } + @Override public Object convert(Object o) throws ConversionException { // Unwrap first @@ -537,32 +614,55 @@ public Object convert(Object o) throws ConversionException return r; } - String s = o instanceof String ? (String)o : ConvertUtils.convert(o); - if (cls == String.class) - return s; - if (StringUtils.isEmpty(s)) - return null; - - try + if (o instanceof CharSequence s) { - Object r = _fromString(s); + Object r = _fromString(s.toString()); if (null != r) return r; } - catch (NumberFormatException | ArithmeticException x) - { - throw new ConversionException("Expected decimal value", x); - } - if (converter == null) - { - throw new ConversionException("Unable to find converter for data class " + this.cls + ", unable to convert value: " + s); - } + return converter.convert(o); + } + + /* convert() overrides let the compiler do some of the work */ + public Object convert(Number n) throws ConversionException + { + if (null == n) + return null; + + Object r = _fromNumber(n); + if (null != r) + return r; - // CONSIDER: convert may return default values instead of ConversionException - return converter.convert(cls, s); + return converter.convert(n); } + public Object convert(Date o) throws ConversionException + { + if (null == o) + return null; + + Object r = _fromDate(o); + if (null != r) + return r; + + return converter.convert(o); + } + + + public Object convert(CharSequence cs) throws ConversionException + { + if (null == cs) + return null; + + Object r = _fromString(cs.toString()); + if (null != r) + return r; + + return converter.convert(cs); + } + + public static Object add(@NotNull Object obj1, @NotNull Object obj2, JdbcType type) { switch (type) diff --git a/api/src/org/labkey/api/data/MultiChoice.java b/api/src/org/labkey/api/data/MultiChoice.java index 266fce36ab8..693eaafba9d 100644 --- a/api/src/org/labkey/api/data/MultiChoice.java +++ b/api/src/org/labkey/api/data/MultiChoice.java @@ -48,7 +48,8 @@ public class MultiChoice { public static final String ARRAY_MARKER = "[]"; - public static class DisplayColumn extends DataColumn + + public static class DisplayColumn extends DataColumn implements IMultiValuedDisplayColumn { public DisplayColumn(ColumnInfo col) { @@ -75,12 +76,6 @@ public Object getDisplayValue(RenderContext ctx) return getValue(ctx); } - @Override - protected Object getInputValue(RenderContext ctx) - { - return super.getInputValue(ctx); - } - @Override protected String getStringValue(Object value, Unit unit, boolean disabledInput) { @@ -104,7 +99,7 @@ else if (value instanceof MultiChoice.Array mca) boolean disabledInput = isDisabledInput(ctx); String formFieldName = getFormFieldName(ctx); ColumnInfo boundColumn = getBoundColumn(); - IPropertyValidator textChoiceValidator = PropertyService.get().getValidatorForColumn(boundColumn, PropertyValidatorType.TextChoice); + IPropertyValidator textChoiceValidator = null==boundColumn ? null : PropertyService.get().getValidatorForColumn(boundColumn, PropertyValidatorType.TextChoice); if (textChoiceValidator != null) { @@ -132,7 +127,7 @@ public void renderInputCell(RenderContext ctx, HtmlWriter out) return HtmlString.EMPTY_STRING; return HtmlString.of( - DIV(array.stream().map(v -> SPAN(at(style,"border:solid 1px black; border-radius:3px;"), v)) + DIV(array.stream().map(v -> SPAN(at(style,"border:solid 1px #DDDDDD; border-radius:3px; padding:4px;"), v)) .collect(new JoinRenderable(HtmlString.SP)))); } @@ -158,6 +153,40 @@ public Object getExportCompatibleValue(RenderContext ctx) { return getTsvFormattedValue(ctx); } + + // IMultiValuedDisplayColumn + + @Override + public List getDisplayValues(RenderContext ctx) + { + return (List)getValue(ctx); + } + + @Override + public List renderURLs(RenderContext ctx) + { + List values = getDisplayValues(ctx); + int size = (values == null) ? 0 : values.size(); + return Collections.nCopies(size, ""); + } + + @Override + public List getTsvFormattedValues(RenderContext ctx) + { + return (Array)getValue(ctx); + } + + @Override + public List getFormattedTexts(RenderContext ctx) + { + return (Array)getValue(ctx); + } + + @Override + public List getJsonValues(RenderContext ctx) + { + return (List)getValue(ctx); + } } @@ -341,13 +370,13 @@ public boolean contains(Object o) } @Override - public @NotNull Object[] toArray() + public Object @NotNull [] toArray() { return 0==array.length ? array : array.clone(); } @Override - public @NotNull T[] toArray(@NotNull T[] a) + public T @NotNull [] toArray(T @NotNull [] a) { if (a.length == 0 && a.getClass().getComponentType().isAssignableFrom(String.class)) return (T[])array.clone(); @@ -462,13 +491,13 @@ public void free() throws SQLException } @Override - public String getBaseTypeName() throws SQLException + public String getBaseTypeName() { return "VARCHAR"; } @Override - public int getBaseType() throws SQLException + public int getBaseType() { return Types.VARCHAR; } @@ -540,6 +569,7 @@ public static Converter getInstance() return _converter; } + @SuppressWarnings("unchecked") @Override public T convert(Class aClass, Object o) { @@ -553,8 +583,8 @@ public T convert(Class aClass, Object o) return (T) Array.from((Object[]) o); if (o instanceof org.json.JSONArray json) return (T) Array.from(json); - if (o instanceof List list) - return (T) new Array(list.stream()); + if (o instanceof List list) + return (T) new Array(((List)list).stream()); return (T) Array.from(o.toString()); } diff --git a/api/src/org/labkey/api/data/MultiValuedRenderContext.java b/api/src/org/labkey/api/data/MultiValuedRenderContext.java index 245b1016d9c..6cae8968265 100644 --- a/api/src/org/labkey/api/data/MultiValuedRenderContext.java +++ b/api/src/org/labkey/api/data/MultiValuedRenderContext.java @@ -117,7 +117,7 @@ public Object get(Object key) // Do conversion to switch it back to the expected type. if (value != null && columnInfo != null && !columnInfo.getJavaClass().isInstance(value)) { - value = ConvertUtils.convert(value.toString(), columnInfo.getJavaClass()); + value = columnInfo.convert(value); } } } diff --git a/api/src/org/labkey/api/data/NameGeneratorState.java b/api/src/org/labkey/api/data/NameGeneratorState.java index 123d74df45c..db50e3715af 100644 --- a/api/src/org/labkey/api/data/NameGeneratorState.java +++ b/api/src/org/labkey/api/data/NameGeneratorState.java @@ -697,7 +697,7 @@ else if (value.isEmpty()) { try { - rootValue = ConvertUtils.convert((String)rootValue, pkCol.getJavaClass()); + rootValue = pkCol.convert(rootValue); } catch (ConversionException x) { diff --git a/api/src/org/labkey/api/data/SimpleConvert.java b/api/src/org/labkey/api/data/SimpleConvert.java new file mode 100644 index 00000000000..36530faf3ee --- /dev/null +++ b/api/src/org/labkey/api/data/SimpleConvert.java @@ -0,0 +1,26 @@ +package org.labkey.api.data; + +import org.apache.commons.beanutils.ConversionException; + +/** Implementing classes must implement convert() or getConvertFn() */ +public interface SimpleConvert +{ + Object convert(Object val) throws ConversionException; + + /** + * Some implementations may be able to pass-through or precompute a SimpleConvert, and it may + * be faster to cache and use that implementation. In a loop or dataiterator it may be faster to + * use getConvertFn() e.g. + * var fn = col.getConvertFn(); + * while (...) + * var converted = fb.convert(val); + *

+ * instead of + * while(...) + * var converted = col.convert(val); + */ + default SimpleConvert getConvertFn() + { + return this; + } +} \ No newline at end of file diff --git a/api/src/org/labkey/api/data/SimpleFilter.java b/api/src/org/labkey/api/data/SimpleFilter.java index cdedb78ce2c..1cb5380207e 100644 --- a/api/src/org/labkey/api/data/SimpleFilter.java +++ b/api/src/org/labkey/api/data/SimpleFilter.java @@ -620,7 +620,7 @@ public static abstract class MultiValuedFilterClause extends CompareType.Abstrac public MultiValuedFilterClause(@NotNull FieldKey fieldKey, CompareType comparison, Collection params, boolean negated) { super(fieldKey); - params = new ArrayList<>(params); // possibly immutable + params = params == null ? new ArrayList<>() : new ArrayList<>(params); // possibly immutable if (params.contains(null)) //params.size() == 0 || { _includeNull = true; diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index e8f073b79fe..685dad682d9 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -26,11 +26,11 @@ import org.jetbrains.annotations.Nullable; import org.json.JSONException; import org.json.JSONObject; -import org.labkey.api.action.BaseViewAction; import org.labkey.api.action.HasBindParameters; import org.labkey.api.action.NullSafeBindException; import org.labkey.api.action.SpringActionController; import org.labkey.api.collections.CaseInsensitiveHashMap; +import org.labkey.api.data.validator.RequiredValidator; import org.labkey.api.ontology.Quantity; import org.labkey.api.security.permissions.DeletePermission; import org.labkey.api.security.permissions.InsertPermission; @@ -419,55 +419,32 @@ protected void _populateValues(BindException errors) try { - if (null != bindValue) - { - propType = getTruePropType(propName); - Object val; - if (null != col && null != col.getKindOfQuantity()) - { - // TODO MultiChoice switch to col.getConvertFn().apply(bindValue) - val = Quantity.convert(bindValue, col.getDisplayUnit()); - } - else - { - if (propType != null) - val = ConvertUtils.convert(bindValue, propType); - else - val = bindValue; - } - values.put(propName, val); - } - else if (_validateRequired && null != _tinfo) + Object val; + if (col != null) + val = col.convert(bindValue); + else + val = getSimpleConvert(propName).convert(bindValue); + + boolean requiredError = false; + if (_validateRequired && null != _tinfo && null != col && col.isRequired() && !col.isAutoIncrement()) { - if (null == col || !col.isRequired()) - { - values.put(propName, null); - } - else - { - boolean isError = true; + requiredError =col.getJdbcType().isEmpty(val); - // if the column is mv-enabled and a mv indicator has been specified, don't flag the required - // error - if (col.isMvEnabled() && col.isNullable()) + // if the column is mv-enabled and a mv indicator has been specified, don't flag the required error + if (col.isMvEnabled() && col.isNullable()) + { + ColumnInfo mvCol = _tinfo.getColumn(col.getMvColumnName()); + if (mvCol != null) { - ColumnInfo mvCol = _tinfo.getColumn(col.getMvColumnName()); - if (mvCol != null) - { - String ff_mvName = getFormFieldName(mvCol); - isError = null == getValueToBind(ff_mvName); - } + String ff_mvName = getFormFieldName(mvCol); + requiredError = null == getValueToBind(ff_mvName); } - if (isError) - errors.addError(new FieldError(errors.getObjectName(), propName, this, true, new String[] {SpringActionController.ERROR_REQUIRED}, new String[] {caption}, caption + " must not be empty.")); - else - values.put(propName, null); } } + if (requiredError) + errors.addError(new FieldError(errors.getObjectName(), propName, this, true, new String[] {SpringActionController.ERROR_REQUIRED}, new String[] {caption}, caption + " must not be empty.")); else - { - values.put(propName, null); - } + values.put(propName, val); } catch (ConversionException e) { @@ -733,18 +710,20 @@ public void forceReselect() setDataLoaded(false); } - - protected Class getTruePropType(String propName) + protected SimpleConvert getSimpleConvert(String propName) { ColumnInfo column = getColumnByFormFieldName(propName); if (null == column) - return null; - // TODO MultiChoice : move this to ColumnInfo (it does not belong in this one place) - // TODO MultiChoice : Can we actually assume that the FK column (in this table) is the same type as the lookup column? + return (value) -> value; boolean multiValued = column.getFk() instanceof MultiValuedForeignKey && ((MultiValuedForeignKey)column.getFk()).isMultiSelectInput(); if (multiValued) - return arrayClass(column.getJavaClass()); - return column.getJavaClass(); + { + // TODO This should be reconciled with SimpleTranslator.MultiValueConvertColumn.convert() + // TODO shouldn't this be getFk().createLookupColumn(getLookupDisplayName()).getJavaClass() or something like that? + // I think String is a better guess than column.getJavaClass() + return ConvertHelper.getSimpleConvert(String[].class); + } + return column.getConvertFn(); } private static Class arrayClass(Class k) @@ -878,7 +857,7 @@ else if (orig.getName().startsWith(ARRAY_MARKER) && orig.getValue()!=null) setValueToBind(pv.getName(), pv.getValue()); } - BindException errors = new NullSafeBindException(new BaseViewAction.BeanUtilsPropertyBindingResult(this, "form")); + BindException errors = new NullSafeBindException(this, "form"); validateBind(errors); return errors; } diff --git a/api/src/org/labkey/api/data/dialect/SqlDialect.java b/api/src/org/labkey/api/data/dialect/SqlDialect.java index 85cc60ec933..77e7430d148 100644 --- a/api/src/org/labkey/api/data/dialect/SqlDialect.java +++ b/api/src/org/labkey/api/data/dialect/SqlDialect.java @@ -2200,6 +2200,12 @@ public SQLFragment array_construct(SQLFragment[] elements) throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement"); } + public SQLFragment array_is_empty(SQLFragment a) + { + assert !supportsArrays(); + throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement"); + } + // element a is in array b public SQLFragment element_in_array(SQLFragment a, SQLFragment b) { diff --git a/api/src/org/labkey/api/data/validator/ColumnValidators.java b/api/src/org/labkey/api/data/validator/ColumnValidators.java index 42d0818bbc5..dadb6ecc67f 100644 --- a/api/src/org/labkey/api/data/validator/ColumnValidators.java +++ b/api/src/org/labkey/api/data/validator/ColumnValidators.java @@ -94,11 +94,15 @@ public static RequiredValidator createRequiredValidator(@Nullable ColumnInfo col boolean supportsMV = (null != col && null != col.getMvColumnName()) || (null != dp && dp.isMvEnabled()); boolean notnull = null != col && !col.isNullable(); boolean required = null != dp && dp.isRequired() || null != col && col.isRequired(); + JdbcType jdbcType = null != col ? col.getJdbcType() : null != dp ? dp.getJdbcType(): JdbcType.VARCHAR; + // CONSIDER: RequiredValidator is treating notnull and required the same + // This is a bit confusing for ARRAY, where required clearly means not empty array + // while an empty array would satisfy NOT NULL in the databse. if ((notnull || required) && (col == null || !col.isAutoIncrement())) { String label = col != null ? col.getName() : dp.getName(); - return new RequiredValidator(label, !notnull && supportsMV, allowEmptyString); + return new RequiredValidator(label, jdbcType, !notnull && supportsMV, allowEmptyString); } return null; diff --git a/api/src/org/labkey/api/data/validator/RequiredValidator.java b/api/src/org/labkey/api/data/validator/RequiredValidator.java index 60fde95dc85..6d522f4fc90 100644 --- a/api/src/org/labkey/api/data/validator/RequiredValidator.java +++ b/api/src/org/labkey/api/data/validator/RequiredValidator.java @@ -16,6 +16,7 @@ package org.labkey.api.data.validator; import org.jetbrains.annotations.Nullable; +import org.labkey.api.data.JdbcType; import org.labkey.api.exp.MvFieldWrapper; /** @@ -25,18 +26,20 @@ */ public class RequiredValidator extends AbstractColumnValidator implements UnderstandsMissingValues { + final JdbcType jdbcType; final boolean allowMV; final boolean allowES; final String _message; - public RequiredValidator(String columnName, boolean allowMissingValueIndicators, boolean allowEmptyString) + public RequiredValidator(String columnName, JdbcType jdbcType, boolean allowMissingValueIndicators, boolean allowEmptyString) { - this(columnName, allowMissingValueIndicators, allowEmptyString, null); + this(columnName, jdbcType, allowMissingValueIndicators, allowEmptyString, null); } - public RequiredValidator(String columnName, boolean allowMissingValueIndicators, boolean allowEmptyString, @Nullable String message) + public RequiredValidator(String columnName, JdbcType jdbcType, boolean allowMissingValueIndicators, boolean allowEmptyString, @Nullable String message) { super(columnName); + this.jdbcType = jdbcType; allowMV = allowMissingValueIndicators; allowES = allowEmptyString; _message = message; @@ -47,16 +50,16 @@ protected String _validate(int rowNum, Object value) { checkRequired: { - if (null == value) - break checkRequired; - - if (value instanceof String && ((String) value).isEmpty()) + if ("".equals(value)) { if (allowES) return null; else break checkRequired; } + if (jdbcType.isEmpty(value)) + break checkRequired; + if (!(value instanceof MvFieldWrapper mv)) return null; diff --git a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java b/api/src/org/labkey/api/dataiterator/SimpleTranslator.java index bc8b61b5baa..d6eded05949 100644 --- a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java +++ b/api/src/org/labkey/api/dataiterator/SimpleTranslator.java @@ -49,6 +49,7 @@ import org.labkey.api.data.MultiValuedForeignKey; import org.labkey.api.data.MvUtil; import org.labkey.api.data.NowTimestamp; +import org.labkey.api.data.SimpleConvert; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TableDescription; import org.labkey.api.data.TableInfo; @@ -624,21 +625,37 @@ protected class SimpleConvertColumn implements Supplier final @Nullable Unit defaultUnit; final String fieldName; final boolean _preserveEmptyString; + final SimpleConvert _simpleConvertFn; - SimpleConvertColumn(String fieldName, int indexFrom, @Nullable JdbcType to, @Nullable Unit defaultUnit) + protected SimpleConvertColumn(String fieldName, int indexFrom, @Nullable JdbcType to, @Nullable Unit defaultUnit) { this(fieldName, indexFrom, to, defaultUnit, false); } - public SimpleConvertColumn(String fieldName, int indexFrom, @Nullable JdbcType to, @Nullable Unit defaultUnit, boolean preserveEmptyString) + protected SimpleConvertColumn(String fieldName, int indexFrom, @Nullable JdbcType to, @Nullable Unit defaultUnit, boolean preserveEmptyString) { this.index = indexFrom; this.type = to; this.fieldName = fieldName; this.defaultUnit = (null==type || type.isNumeric()) ? defaultUnit : null; _preserveEmptyString = preserveEmptyString && null != type && type.isText(); + _simpleConvertFn = + null != this.defaultUnit ? this.defaultUnit : + null != this.type ? this.type : + (value) -> value; } + protected SimpleConvertColumn(ColumnInfo col, int indexFrom, boolean preserveEmptyString) + { + this.fieldName = col.getName(); + this.index = indexFrom; + this.type = col.getJdbcType(); + this.defaultUnit = null; + _simpleConvertFn = col.getConvertFn(); + _preserveEmptyString = preserveEmptyString; + } + + @Override final public Object get() { @@ -658,13 +675,10 @@ final public Object get() protected Object simpleConvert(Object o) { + // Consider wrapping _simpleConvertFn to handle _preserveEmptyString so we can remove this "if". if (_preserveEmptyString && "".equals(o)) return ""; - if (null != defaultUnit) - return defaultUnit.convert(o); - if (null != type) - return type.convert(o); - return o; + return _simpleConvertFn.convert(o); } protected Object convert(Object o) @@ -752,7 +766,7 @@ public Object get() } } - public static class ConstantColumn implements Supplier + public static class ConstantColumn implements Supplier { final Object k; @@ -794,10 +808,11 @@ private class MissingValueConvertColumn extends SimpleConvertColumn int indicator; - MissingValueConvertColumn(String fieldName, int index, int indexIndicator, @Nullable JdbcType to, @Nullable Unit defaultUnit) + MissingValueConvertColumn(ColumnInfo col, int index, int indexIndicator, boolean supportsMissingValue) { - super(fieldName, index, to, defaultUnit); - indicator = indexIndicator; + super(col, index, false); + this.supportsMissingValue = supportsMissingValue; + this.indicator = indexIndicator; } @@ -850,33 +865,13 @@ Object innerConvert(Object value) } - private class PropertyConvertColumn extends MissingValueConvertColumn - { - @Nullable PropertyType pt; - - PropertyConvertColumn(String fieldName, int fromIndex, int mvIndex, boolean supportsMissingValue, @Nullable PropertyType pt, @Nullable JdbcType type, Unit defaultUnit) - { - super(fieldName, fromIndex, mvIndex, type != null ? type : pt != null ? pt.getJdbcType() : null, defaultUnit); - this.pt = pt; - this.supportsMissingValue = supportsMissingValue; - } - - @Override - Object innerConvert(Object value) - { - if (null != pt) - return pt.convert(value); - return super.innerConvert(value); - } - } - - private class PropertyConvertAndTrimColumn extends PropertyConvertColumn + private class MissingValueConvertAndTrimColumn extends MissingValueConvertColumn { boolean trimRightOnly; - PropertyConvertAndTrimColumn(String fieldName, int fromIndex, int mvIndex, boolean supportsMissingValue, @Nullable PropertyType pt, @Nullable JdbcType type, @Nullable Unit defaultUnit, boolean trimRightOnly) + MissingValueConvertAndTrimColumn(ColumnInfo col, int fromIndex, int mvIndex, boolean supportsMissingValue, boolean trimRightOnly) { - super(fieldName, fromIndex, mvIndex, supportsMissingValue, pt, type, defaultUnit); + super(col, fromIndex, mvIndex, supportsMissingValue); this.trimRightOnly = trimRightOnly; } @@ -1271,9 +1266,9 @@ public int addConvertColumn(ColumnInfo col, int fromIndex) * @param remapMissingBehavior The behavior desired when remapping fails. If null, indicate an error if the column is required or null if not required. * @param withLookupRemapping Indicates if remapping of lookup columns should be attempted or not */ - private int addConvertColumn(ColumnInfo col, int fromIndex, int mvIndex, @Nullable RemapMissingBehavior remapMissingBehavior, boolean withLookupRemapping) + public int addConvertColumn(ColumnInfo col, int fromIndex, int mvIndex, @Nullable RemapMissingBehavior remapMissingBehavior, boolean withLookupRemapping) { - SimpleConvertColumn c = createConvertColumn(col, fromIndex, mvIndex, null, null, col.getJdbcType(), remapMissingBehavior, withLookupRemapping); + SimpleConvertColumn c = createConvertColumn(col, fromIndex, mvIndex, remapMissingBehavior, withLookupRemapping); return addColumn(col, c); } @@ -1301,7 +1296,7 @@ public int addConvertColumn(String name, int fromIndex, JdbcType toType, @Nullab col.setJdbcType(toType); if (toFk != null) col.setFk(toFk); - if (PropertyType.MULTI_CHOICE.equals(pt)) // TODO: should this be applied to all column types? + if (null != pt) col.setPropertyType(pt); return addConvertColumn(col, fromIndex, fromIndex, remapMissingBehavior, withLookupRemapping); @@ -1319,35 +1314,20 @@ public int addConvertColumn(String name, int fromIndex, JdbcType toType, @Nullab * * @param col Use this column's type to perform conversion. * @param fromIndex Source column to create the output column from and pull data from. - * @param pd PropertyDescriptor used for missing value enabled-ness. - * @param pt Convert the source data values to this type. * @param remapMissingBehavior The behavior desired when remapping fails. If null, indicate an error if the column is required or null if not required. * @param withLookupRemapping Indicates if we should try to remap lookups during the conversion or not. */ - public int addConvertColumn(@NotNull ColumnInfo col, int fromIndex, int mvIndex, @Nullable PropertyDescriptor pd, @Nullable PropertyType pt, @Nullable RemapMissingBehavior remapMissingBehavior, boolean withLookupRemapping) - { - SimpleConvertColumn c = createConvertColumn(col, fromIndex, mvIndex, pd, pt, col.getJdbcType(), remapMissingBehavior, withLookupRemapping); - return addColumn(col, c); - } - - public SimpleConvertColumn createConvertColumn(@NotNull ColumnInfo col, int fromIndex, @Nullable RemapMissingBehavior remapMissingBehavior) - { - return createConvertColumn(col, fromIndex, NO_MV_INDEX, null, col.getPropertyType(), col.getJdbcType(), remapMissingBehavior, true); - } - - private SimpleConvertColumn createConvertColumn(@NotNull ColumnInfo col, int fromIndex, int mvIndex, @Nullable PropertyDescriptor pd, @Nullable PropertyType pt, @Nullable JdbcType type, @Nullable RemapMissingBehavior remapMissingBehavior, boolean withLookupRemapping) + private SimpleConvertColumn createConvertColumn(@NotNull ColumnInfo col, int fromIndex, int mvIndex, @Nullable RemapMissingBehavior remapMissingBehavior, boolean withLookupRemapping) { - final String name = col.getName(); - - boolean mv = null != col.getMvColumnName() || (null != pd && pd.isMvEnabled()); + boolean mv = null != col.getMvColumnName(); boolean trimString = _context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.TrimString); boolean trimStringRight = _context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.TrimStringRight); SimpleConvertColumn c; - if (PropertyType.STRING == pt && (trimString || trimStringRight)) - c = new PropertyConvertAndTrimColumn(name, fromIndex, mvIndex, mv, pt, type, col.getDisplayUnit(), !trimString); + if (PropertyType.STRING == col.getPropertyType() && (trimString || trimStringRight)) + c = new MissingValueConvertAndTrimColumn(col, fromIndex, mvIndex, mv, !trimString); else - c = new PropertyConvertColumn(name, fromIndex, mvIndex, mv, pt, type, col.getDisplayUnit()); + c = new MissingValueConvertColumn(col, fromIndex, mvIndex, mv); ForeignKey fk = col.getFk(); LookupResolutionType lookupResolutionType = _context.getLookupResolutionType(); @@ -1364,11 +1344,17 @@ private SimpleConvertColumn createConvertColumn(@NotNull ColumnInfo col, int fro { // convert input into Collection of jdbcType values c = new MultiValueConvertColumn(c); - } + } final String name = col.getName(); + + return c; } + public SimpleConvertColumn createConvertColumn(@NotNull ColumnInfo col, int fromIndex, @Nullable RemapMissingBehavior remapMissingBehavior) + { + return createConvertColumn(col, fromIndex, NO_MV_INDEX, remapMissingBehavior, true); + } public int addCoalesceColumn(String name, int firstIndex, Supplier second) { diff --git a/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java b/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java index dfbf6b0150d..ccbd5140882 100644 --- a/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java +++ b/api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java @@ -275,6 +275,7 @@ public DataIterator getDataIterator(DataIteratorContext context) { PropertyDescriptor pd = pair.dp == null ? null : pair.dp.getPropertyDescriptor(); PropertyType pt = pd == null ? null : pd.getPropertyType(); + assert null == pt || null == pair.getTarget().getPropertyType() || pair.getTarget().getPropertyType() == pt; boolean isAttachment = pt == PropertyType.ATTACHMENT || pt == PropertyType.FILE_LINK; if (null == pair.target) @@ -282,7 +283,7 @@ public DataIterator getDataIterator(DataIteratorContext context) else if (isAttachment) // Issue 53498: attachment is blank after update from file, if the field name contains underscore convert.addColumn(pair.target, pair.indexFrom); else - convert.addConvertColumn(pair.target, pair.indexFrom, pair.indexMv, pd, pt, pair.target.getRemapMissingBehavior(), context.isWithLookupRemapping()); + convert.addConvertColumn(pair.target, pair.indexFrom, pair.indexMv, pair.target.getRemapMissingBehavior(), context.isWithLookupRemapping()); } // @@ -318,9 +319,9 @@ else if (isAttachment) // Issue 53498: attachment is blank after update from fil { if (additionalRequiredCols.contains(col.getColumnName())) { - List validators = new ArrayList<>(); - validators.add(new RequiredValidator(col.getColumnName(), false, context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.PreserveEmptyString))); - validate.addValidators(index, validators); + var reqd = ColumnValidators.createRequiredValidator(col, null, context.getConfigParameterBoolean(QueryUpdateService.ConfigParameters.PreserveEmptyString)); + if (null != reqd) + validate.addValidator(index, reqd); } continue; } diff --git a/api/src/org/labkey/api/defaults/SetDefaultValuesAction.java b/api/src/org/labkey/api/defaults/SetDefaultValuesAction.java index 2e476cdbc21..77bb5d3f497 100644 --- a/api/src/org/labkey/api/defaults/SetDefaultValuesAction.java +++ b/api/src/org/labkey/api/defaults/SetDefaultValuesAction.java @@ -415,7 +415,7 @@ public boolean handlePost(FormType domainIdForm, BindException errors) throws Ex { try { - Object converted = ConvertUtils.convert(value, type.getJavaType()); + Object converted = type.convert(value); values.put(property, converted); } catch (ConversionException e) diff --git a/api/src/org/labkey/api/exp/PropertyDescriptor.java b/api/src/org/labkey/api/exp/PropertyDescriptor.java index 6150b0393f9..bf2c57832a6 100644 --- a/api/src/org/labkey/api/exp/PropertyDescriptor.java +++ b/api/src/org/labkey/api/exp/PropertyDescriptor.java @@ -22,6 +22,7 @@ import org.labkey.api.data.BaseColumnInfo; import org.labkey.api.data.BeanObjectFactory; import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.ColumnRenderProperties; import org.labkey.api.data.ColumnRenderPropertiesImpl; import org.labkey.api.data.ConditionalFormat; import org.labkey.api.data.Container; @@ -31,6 +32,7 @@ import org.labkey.api.data.ObjectFactory; import org.labkey.api.data.ParameterDescription; import org.labkey.api.data.SQLFragment; +import org.labkey.api.data.SimpleConvert; import org.labkey.api.data.TableInfo; import org.labkey.api.data.Transient; import org.labkey.api.data.dialect.SqlDialect; @@ -590,6 +592,11 @@ public Map getAuditRecordMap(@Nullable String validatorStr, @Nul return map; } + @Override @Transient + public final SimpleConvert getConvertFn() + { + return ColumnRenderProperties.getDefaultConvertFn(this); + } } diff --git a/api/src/org/labkey/api/exp/PropertyType.java b/api/src/org/labkey/api/exp/PropertyType.java index 917492ad1e6..d46b1fbcd7c 100644 --- a/api/src/org/labkey/api/exp/PropertyType.java +++ b/api/src/org/labkey/api/exp/PropertyType.java @@ -28,6 +28,8 @@ import org.labkey.api.data.JdbcType; import org.labkey.api.data.MultiChoice; import org.labkey.api.data.NameGenerator; +import org.labkey.api.data.SimpleConvert; +import org.labkey.api.data.Transient; import org.labkey.api.exp.OntologyManager.PropertyRow; import org.labkey.api.reader.ExcelFactory; import org.labkey.api.util.DateUtil; @@ -55,7 +57,7 @@ /** * TODO: Add more types? Entity, Lsid, User, ... */ -public enum PropertyType +public enum PropertyType implements SimpleConvert { BOOLEAN("http://www.w3.org/2001/XMLSchema#boolean", "Boolean", 'f', JdbcType.BOOLEAN, 10, null, CellType.BOOLEAN, Boolean.class, Boolean.TYPE) { @@ -123,10 +125,11 @@ protected Object convertExcelValue(Cell cell) throws ConversionException @Override public Object convert(Object value) throws ConversionException { - if (value instanceof String) + if (null == value) return value; - else - return ConvertUtils.convert(value); + if (value instanceof CharSequence cs) + return cs.isEmpty() ? null : cs.toString(); + return ConvertUtils.convert(value, String.class); } @Override @@ -171,10 +174,7 @@ protected Object convertExcelValue(Cell cell) throws ConversionException @Override public Object convert(Object value) throws ConversionException { - if (value instanceof String) - return value; - else - return ConvertUtils.convert(value); + return STRING.convert(value); } @Override @@ -478,6 +478,7 @@ protected Object convertExcelValue(Cell cell) throws ConversionException @Override public Object convert(Object value) throws ConversionException { + // Should this use ExpDataFileConverter.convert()??? if (null == value) return null; if (value instanceof File) @@ -934,10 +935,7 @@ protected Object convertExcelValue(Cell cell) throws ConversionException @Override public Object convert(Object value) throws ConversionException { - if (value instanceof String) - return value; - else - return ConvertUtils.convert(value); + return STRING.convert(value); } @Override @@ -1005,6 +1003,10 @@ public Object getPreviewValue(@Nullable String prefix) this.additionalTypes = additionalTypes; } + /** + * The returned Function<Object,Object> should throw ConversionException (undeclared RuntimeException) + */ + public String getTypeUri() { return typeURI; @@ -1182,8 +1184,6 @@ public static PropertyType getFromJdbcTypeName(String typeName) protected abstract Object convertExcelValue(Cell cell) throws ConversionException; - public abstract Object convert(Object value) throws ConversionException; - public static Object getFromExcelCell(Cell cell) throws ConversionException { if (ExcelFactory.isCellNumeric(cell)) diff --git a/api/src/org/labkey/api/exp/property/DomainUtil.java b/api/src/org/labkey/api/exp/property/DomainUtil.java index cf9f540bf25..ad2e65c1d9b 100644 --- a/api/src/org/labkey/api/exp/property/DomainUtil.java +++ b/api/src/org/labkey/api/exp/property/DomainUtil.java @@ -178,7 +178,7 @@ else if (property.getLookup() != null) { ColumnInfo pkColumnInfo = table.getColumn(pkCol); if (!pkColumnInfo.getClass().equals(defaultValue.getClass())) - defaultValue = ConvertUtils.convert(defaultValue.toString(), pkColumnInfo.getJavaClass()); + defaultValue = pkColumnInfo.convert(defaultValue.toString()); if (!validateOnly) { diff --git a/api/src/org/labkey/api/ontology/Unit.java b/api/src/org/labkey/api/ontology/Unit.java index b2872362cb7..3ac94482022 100644 --- a/api/src/org/labkey/api/ontology/Unit.java +++ b/api/src/org/labkey/api/ontology/Unit.java @@ -6,12 +6,12 @@ import org.jetbrains.annotations.Nullable; import org.junit.Assert; import org.junit.Test; -import org.labkey.api.data.ConversionExceptionWithMessage; +import org.labkey.api.data.SimpleConvert; import java.util.HashMap; import java.util.function.Function; -public enum Unit +public enum Unit implements SimpleConvert { unit(KindOfQuantity.Count, null, 1.0, 2, "unit", Quantity.class, @@ -217,6 +217,7 @@ public static double convert(double value, @NotNull Unit from, @NotNull Unit to) return from==to ? value : to.fromBaseUnitValue(from.toBaseUnitValue(value)); } + @Override public Quantity convert(@Nullable Object value) { return Quantity.convert(value, this); @@ -255,6 +256,7 @@ public void testIsCompatible() assertTrue(Unit.cells.isCompatible(Unit.unit)); assertFalse(Unit.unit.isCompatible(Unit.mL)); assertFalse(Unit.bottles.isCompatible(Unit.mL)); + //noinspection ConstantValue assertFalse(Unit.mL.isCompatible(null)); } diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index 1b64835bb42..6a0aea921e6 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -737,15 +737,12 @@ protected Object coerceTypesValue(ColumnInfo col, Map providedVa { try { + if (col.getKindOfQuantity() != null) + providedValues.put(key, value); if (PropertyType.FILE_LINK.equals(col.getPropertyType())) value = ExpDataFileConverter.convert(value); - else if (col.getKindOfQuantity() != null) - { - providedValues.put(key, value); - value = Quantity.convert(value, col.getKindOfQuantity().getStorageUnit()); - } else - value = col.getConvertFn().apply(value); + value = col.convert(value); } catch (ConvertHelper.FileConversionException e) { diff --git a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java index 5538e77a783..594ed785fef 100644 --- a/api/src/org/labkey/api/query/DefaultQueryUpdateService.java +++ b/api/src/org/labkey/api/query/DefaultQueryUpdateService.java @@ -746,7 +746,7 @@ protected Object[] getKeys(Map map, Container container) throws { try { - pkValue = ConvertUtils.convert(pkValue.toString(), pk.getJavaObjectClass()); + pkValue = pk.convert(pkValue); } catch (ConversionException ignored) { /* Maybe the database can do the conversion */ } } @@ -846,7 +846,7 @@ protected Object convertColumnValue(ColumnInfo col, Object value, User user, Con } return ExpDataFileConverter.convert(value); } - return col.getConvertFn().apply(value); + return col.getConvertFn().convert(value); } catch (ConvertHelper.FileConversionException e) { diff --git a/api/src/org/labkey/api/study/assay/ThawListListResolver.java b/api/src/org/labkey/api/study/assay/ThawListListResolver.java index 456a34e23fa..7450146397c 100644 --- a/api/src/org/labkey/api/study/assay/ThawListListResolver.java +++ b/api/src/org/labkey/api/study/assay/ThawListListResolver.java @@ -97,7 +97,7 @@ protected ParticipantVisit resolveParticipantVisit(String specimenID, String par Object convertedID; try { - convertedID = ConvertUtils.convert(specimenID, col.getJavaObjectClass()); + convertedID = col.convert(specimenID); } catch (ConversionException e) { diff --git a/api/src/org/labkey/api/util/DateUtil.java b/api/src/org/labkey/api/util/DateUtil.java index e4db870da6a..20b3003918c 100644 --- a/api/src/org/labkey/api/util/DateUtil.java +++ b/api/src/org/labkey/api/util/DateUtil.java @@ -809,18 +809,17 @@ private static long parseDate(String s, MonthDayOption md, @Nullable String extr } } - public static @Nullable Date parseSimpleTime(@NotNull Object o) + public static @Nullable Date parseSimpleTime(@NotNull String s) { Date duration = null; ParseException parseException = null; - String s = (String) o; boolean hasAMPM = s.toLowerCase().endsWith(" am") || s.toLowerCase().endsWith(" pm"); String[] validFormats = hasAMPM ? SIMPLE_TIME_FORMATS_WITH_AMPM : SIMPLE_TIME_FORMATS_NO_AMPM; for (int i = 0; i < validFormats.length && duration == null; i++) { try { - duration = DateUtil.parseDateTime(o.toString(), validFormats[i]); + duration = DateUtil.parseDateTime(s, validFormats[i]); } catch (ParseException ignore) { @@ -830,7 +829,7 @@ private static long parseDate(String s, MonthDayOption md, @Nullable String extr } } if (duration == null) - throw new ConversionException("Could not convert \"" + o + "\" to duration.", parseException); + throw new ConversionException("Could not convert \"" + s + "\" to time.", parseException); return duration; } diff --git a/api/src/org/labkey/api/util/ReturnURLString.java b/api/src/org/labkey/api/util/ReturnURLString.java index be6e671ee68..a499f789164 100644 --- a/api/src/org/labkey/api/util/ReturnURLString.java +++ b/api/src/org/labkey/api/util/ReturnURLString.java @@ -16,6 +16,7 @@ package org.labkey.api.util; import org.apache.commons.beanutils.ConversionException; +import org.apache.commons.beanutils.converters.StringConverter; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; @@ -190,7 +191,7 @@ public URLHelper getURLHelper(URLHelper defaultURL) public static class Converter implements org.apache.commons.beanutils.Converter { - private static final ConvertHelper.DateFriendlyStringConverter _impl = new ConvertHelper.DateFriendlyStringConverter(); + private static final StringConverter _impl = new StringConverter(); @Override public Object convert(Class type, Object value) diff --git a/api/src/org/labkey/api/util/TimeOnlyDate.java b/api/src/org/labkey/api/util/TimeOnlyDate.java index 4f9679795aa..bd873c35591 100644 --- a/api/src/org/labkey/api/util/TimeOnlyDate.java +++ b/api/src/org/labkey/api/util/TimeOnlyDate.java @@ -21,6 +21,7 @@ * Time: 4:14:31 PM * * Marker class to allow TabLoader and ConvertHelper to identify and parse time/duration objects. + * TODO: Why does this class exists??? What's wrong with java.sql.Time??? (or maybe LocalTime) */ public class TimeOnlyDate extends java.util.Date { diff --git a/api/src/org/labkey/api/view/ViewServlet.java b/api/src/org/labkey/api/view/ViewServlet.java index 9382fc5604b..7b312feabe0 100644 --- a/api/src/org/labkey/api/view/ViewServlet.java +++ b/api/src/org/labkey/api/view/ViewServlet.java @@ -71,7 +71,9 @@ import java.util.Objects; import java.util.concurrent.atomic.AtomicInteger; +import static org.apache.commons.lang3.StringUtils.startsWith; import static org.apache.commons.lang3.StringUtils.trimToEmpty; +import static org.labkey.api.data.MultiChoice.ARRAY_MARKER; /** @@ -773,10 +775,23 @@ public static boolean validChars(HttpServletRequest r) return true; } + // NOTE: The usages of this method seem a bit unorthodox. It might be better to try to align them with more + // usual form-binding code-paths. e.g. use ViewActionParameterPropertyValues and maybe TableViewForm.preprocessPropertyValues() + // // Adapt Map returned by getParameterMap() to match InsertView.setInitialValues(Map). // This makes it possible to upgrade our version of servlet-api.jar without a major overhaul. See #25941. public static Map adaptParameterMap(Map parameterMap) { - return new HashMap<>(parameterMap); + // unwraps single element arrays unless parameter is marked as an array + HashMap ret = new HashMap<>(); + for (var entry : parameterMap.entrySet()) + { + String[] a = entry.getValue(); + Object v = a; + if (null != a && 1 == a.length && !startsWith(entry.getKey(), ARRAY_MARKER)) + v = a[0]; + ret.put(entry.getKey(), v); + } + return ret; } } diff --git a/api/src/org/labkey/api/webdav/WebdavResource.java b/api/src/org/labkey/api/webdav/WebdavResource.java index 5b503d49f1f..44d32720c9e 100644 --- a/api/src/org/labkey/api/webdav/WebdavResource.java +++ b/api/src/org/labkey/api/webdav/WebdavResource.java @@ -25,11 +25,17 @@ import org.labkey.api.view.NavTree; import org.labkey.api.view.ViewContext; import org.labkey.api.writer.ContainerUser; +import org.springframework.util.MimeType; import java.io.File; +import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; import java.util.Collection; +import java.util.Date; import java.util.List; import java.util.Map; @@ -244,4 +250,107 @@ default DavPath toDavPath() { return new DavPath(getPath()); } + + default org.springframework.core.io.Resource getSpringResource(User user) throws IOException + { + WebdavResource r = this; + + final FileStream fs = r.isFile() ? getFileStream(user) : null; + + return new org.springframework.core.io.Resource() + { + @Override + public long contentLength() throws IOException + { + if (null != fs) + return fs.getSize(); + throw new FileNotFoundException(r.getPath().toString()); + } + + @Override + public boolean exists() + { + return r.exists(); + } + + @Override + public URL getURL() throws IOException + { + // Nnot sure why getExceuteHref() takes a view context. We ay not need for the moment. + var href = r.getExecuteHref(null); + if (null == href) + return null; + return new URL(href); + } + + @Override + public URI getURI() throws IOException + { + // Nnot sure why getExceuteHref() takes a view context. We ay not need for the moment. + var href = r.getExecuteHref(null); + if (null == href) + return null; + try + { + return new URI(href); + } + catch (URISyntaxException x) + { + throw new IOException(x.getMessage(), x); + } + } + + @Override + public File getFile() throws IOException + { +// UnsupportedOperationException – if the resource is a file but cannot be exposed as a java.io.File; an alternative to FileNotFoundException +// FileNotFoundException – if the resource cannot be resolved as a file +// IOException – in case of general resolution/reading failures + if (!isFile()) + throw new FileNotFoundException(r.getPath().toString()); + File f = r.getFile(); + if (null == f) + throw new UnsupportedOperationException(r.getPath().toString() + " is not in default file-system"); + return f; + } + + @Override + public long lastModified() throws IOException + { + Date d = null==fs ? null : fs.getLastModified(); + if (null != d) + return d.getTime(); + throw new UnsupportedOperationException(r.getPath().toString() + " does not provide lastModified"); + } + + @Override + public org.springframework.core.io.Resource createRelative(String relativePath) throws IOException + { + // do we need this? + throw new UnsupportedOperationException(); + } + + @Override + public String getFilename() + { + return r.getName(); + } + + @Override + public String getDescription() + { + return r.getDescription(); + } + + @Override + public InputStream getInputStream() throws IOException + { + // FileSTream.openInputStream() enforces one stream per instance, so don't use 'fs' + var filestream = r.getFileStream(user); + if (null != filestream) + return filestream.openInputStream(); + throw new UnsupportedOperationException(r.getPath().toString()); + } + }; + } } diff --git a/api/webapp/WEB-INF/web.xml b/api/webapp/WEB-INF/web.xml index 8825cea45f6..3dbc079f803 100755 --- a/api/webapp/WEB-INF/web.xml +++ b/api/webapp/WEB-INF/web.xml @@ -1,8 +1,8 @@ - + LabKey Server @@ -14,10 +14,12 @@ Set Character Encoding org.labkey.core.filters.SetCharacterEncodingFilter + true Form Authentication Filter org.labkey.api.security.AuthFilter + true Transaction Filter diff --git a/core/src/org/labkey/core/CoreController.java b/core/src/org/labkey/core/CoreController.java index 5837de0c470..3f78eb63b95 100644 --- a/core/src/org/labkey/core/CoreController.java +++ b/core/src/org/labkey/core/CoreController.java @@ -438,7 +438,7 @@ else if (form.getSchemaName() != null && form.getQueryName() != null && form.get try { - Object pkVal = ConvertUtils.convert(form.getPk(), pkCol.getJavaClass()); + Object pkVal = pkCol.convert(form.getPk()); SimpleFilter filter = new SimpleFilter(pkCol.getFieldKey(), pkVal); var select = QueryService.get().getSelectBuilder(table) .columns(col) diff --git a/core/src/org/labkey/core/CoreModule.java b/core/src/org/labkey/core/CoreModule.java index f959bf5aa1e..15e52c386ea 100644 --- a/core/src/org/labkey/core/CoreModule.java +++ b/core/src/org/labkey/core/CoreModule.java @@ -173,6 +173,7 @@ import org.labkey.api.util.FileUtil; import org.labkey.api.util.GUID; import org.labkey.api.util.JobRunner; +import org.labkey.api.util.JspTestCase; import org.labkey.api.util.MimeMap; import org.labkey.api.util.MothershipReport; import org.labkey.api.util.PageFlowUtil; @@ -356,6 +357,7 @@ import java.util.Objects; import java.util.Properties; import java.util.Set; +import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -1453,6 +1455,14 @@ public TabDisplayMode getTabDisplayMode() return testClasses; } + @Override + public @NotNull Collection>> getIntegrationTestFactories() + { + List>> ret = new ArrayList<>(super.getIntegrationTestFactories()); + ret.add(new JspTestCase("/org/labkey/api/data/ColumnInfoTests.jsp")); + return ret; + } + @Override public @NotNull Set> getUnitTests() { diff --git a/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java b/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java index c5fbcad586b..ffdcf014af0 100644 --- a/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java +++ b/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java @@ -1085,6 +1085,12 @@ public SQLFragment array_construct(SQLFragment[] elements) return ret; } + @Override + public SQLFragment array_is_empty(SQLFragment a) + { + return new SQLFragment("(cardinality(").append(a).append(")=0)"); + } + @Override public SQLFragment array_all_in_array(SQLFragment a, SQLFragment b) { diff --git a/experiment/src/org/labkey/experiment/ExpDataIterators.java b/experiment/src/org/labkey/experiment/ExpDataIterators.java index 2d96ea4c1a9..b14dacc468e 100644 --- a/experiment/src/org/labkey/experiment/ExpDataIterators.java +++ b/experiment/src/org/labkey/experiment/ExpDataIterators.java @@ -2545,7 +2545,7 @@ public DataIterator getDataIterator(DataIteratorContext context) if (index != null) { ColumnInfo column = di.getColumnInfo(index); - validate.addValidator(index, new RequiredValidator(column.getColumnName(), false, false, "Sample name cannot be blank")); + validate.addValidator(index, new RequiredValidator(column.getColumnName(), column.getJdbcType(), false, false, "Sample name cannot be blank")); } // Add other column validators here... diff --git a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java index 143dd5c4abf..648152e6067 100644 --- a/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpRunTableImpl.java @@ -1055,7 +1055,7 @@ else if (Column.WorkflowTask.toString().equalsIgnoreCase(columnName)) { try { - value = ConvertUtils.convert(String.valueOf(value), col.getJavaClass()); + value = col.convert(value); } catch (ConversionException e) { @@ -1079,11 +1079,11 @@ else if (Column.WorkflowTask.toString().equalsIgnoreCase(columnName)) Class keyColumnType = lookupColumn.getJavaClass(); if (newValue != null && !keyColumnType.isAssignableFrom(newValue.getClass())) { - newValue = ConvertUtils.convert(newValue.toString(), keyColumnType); + newValue = lookupColumn.convert(newValue); } if (oldValue != null && !keyColumnType.isAssignableFrom(oldValue.getClass())) { - oldValue = ConvertUtils.convert(oldValue.toString(), keyColumnType); + oldValue = lookupColumn.convert(oldValue); } Map oldLookupTarget = new TableSelector(fkTableInfo).getMap(oldValue); if (oldLookupTarget != null) diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 52af868d837..bd8273f8d99 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -1889,7 +1889,7 @@ private void _addConvertColumn(String name, int fromIndex, JdbcType toType, @Nul var col = new BaseColumnInfo(getInput().getColumnInfo(fromIndex)); col.setName(name); col.setJdbcType(toType); - if (PropertyType.MULTI_CHOICE.equals(pt)) // TODO: should this be applied to all column types? + if (null != pt) col.setPropertyType(pt); if (toFk != null) col.setFk(toFk); diff --git a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java index 62390100c31..f6555ee8103 100644 --- a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java @@ -808,7 +808,7 @@ else if (impl.isNew()) impl.save(user, _dd, sortOrder++); // Automatically preserve order String defaultValue = impl.getDefaultValue(); - Object converted = null != defaultValue ? ConvertUtils.convert(defaultValue, impl.getPropertyDescriptor().getJavaClass()) : null; + Object converted = null != defaultValue ? impl.getPropertyType().convert(defaultValue) : null; defaultValueMap.put(impl, converted); if (isImplNew) diff --git a/experiment/src/org/labkey/experiment/api/property/LookupValidator.java b/experiment/src/org/labkey/experiment/api/property/LookupValidator.java index ee33a451f0c..f4183c0354b 100644 --- a/experiment/src/org/labkey/experiment/api/property/LookupValidator.java +++ b/experiment/src/org/labkey/experiment/api/property/LookupValidator.java @@ -234,7 +234,7 @@ public boolean validate(IPropertyValidator validator, assert value != null : "Shouldn't be validating a null value"; if (value != null) - value = ConvertHelper.convert(value, crpField.getJavaObjectClass()); + value = crpField.convert(value); if (crpField instanceof PropertyDescriptor field) { diff --git a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java index d0b22b1366f..0ea06467b31 100644 --- a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java @@ -603,7 +603,7 @@ else if (DefaultType.LAST_ENTERED.equals(xDefaultType)) { try { - Object converted = ConvertUtils.convert(defaultValue, type.getJavaType()); + Object converted = type.convert(defaultValue); defaultValues.put(prop, converted); } catch (ConversionException e) diff --git a/query/src/org/labkey/query/QueryServiceImpl.java b/query/src/org/labkey/query/QueryServiceImpl.java index d44943d4f2a..cc576f9d55d 100644 --- a/query/src/org/labkey/query/QueryServiceImpl.java +++ b/query/src/org/labkey/query/QueryServiceImpl.java @@ -309,6 +309,8 @@ public void moduleChanged(Module module) CompareType.NONBLANK, CompareType.MV_INDICATOR, CompareType.NO_MV_INDICATOR, + CompareType.ARRAY_IS_EMPTY, + CompareType.ARRAY_IS_NOT_EMPTY, CompareType.ARRAY_CONTAINS_ALL, CompareType.ARRAY_CONTAINS_ANY, CompareType.ARRAY_CONTAINS_NONE, diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index 17eab5ba6aa..e32cda8b143 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -3114,7 +3114,7 @@ public boolean handlePost(QueryForm form, BindException errors) for (int idx = 0; idx < numPks; ++idx) { ColumnInfo keyColumn = pks.get(idx); - Object keyValue = keyColumn.getJavaClass() == String.class ? stringValues[idx] : ConvertUtils.convert(stringValues[idx], keyColumn.getJavaClass()); + Object keyValue = keyColumn.getJavaClass() == String.class ? stringValues[idx] : keyColumn.convert(stringValues[idx]); rowKeyValues.put(keyColumn.getName(), keyValue); } keyValues.add(rowKeyValues); diff --git a/query/src/org/labkey/query/sql/Method.java b/query/src/org/labkey/query/sql/Method.java index 77214c9d348..b6377660675 100644 --- a/query/src/org/labkey/query/sql/Method.java +++ b/query/src/org/labkey/query/sql/Method.java @@ -1573,6 +1573,34 @@ public SQLFragment getSQL(SqlDialect dialect, SQLFragment[] arguments) } } + public static class ArrayIsEmptyMethod extends Method + { + ArrayIsEmptyMethod(String name) + { + super(name, JdbcType.BOOLEAN, 1, 1); + } + + @Override + public MethodInfo getMethodInfo() + { + return new AbstractMethodInfo(JdbcType.BOOLEAN) + { + @Override + public JdbcType getJdbcType(JdbcType[] args) + { + if (1 == args.length && args[0] != JdbcType.ARRAY) + throw new QueryParseException(_name + " requires an argument of type ARRAY", null, -1, -1); + return super.getJdbcType(args); + } + + @Override + public SQLFragment getSQL(SqlDialect dialect, SQLFragment[] arguments) + { + return dialect.array_is_empty(arguments[0]); + } + }; + } + } final static Map postgresMethods = Collections.synchronizedMap(new CaseInsensitiveHashMap<>()); @@ -1650,6 +1678,8 @@ private static void addPostgresArrayMethods() // not array_equals() because arrays are ordered, this is an unordered comparison postgresMethods.put("array_is_same", new ArrayOperatorMethod("array_is_same", SqlDialect::array_same_array)); // Use "NOT array_is_same()" instead of something clumsy like "array_is_not_same()" + + postgresMethods.put("array_is_empty", new ArrayIsEmptyMethod("array_is_empty")); }