From 290d475c4032ba56c6cf085c313d8aa9b5d09f57 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Mon, 23 Feb 2026 18:52:56 -0800 Subject: [PATCH 1/3] test isMultiValued() in ColumnInfo.getDefaultConvertFn(). fixes update of non-string MVFK --- .../api/data/AbstractWrappedColumnInfo.java | 6 ---- .../org/labkey/api/data/BaseColumnInfo.java | 7 ---- api/src/org/labkey/api/data/ColumnInfo.java | 33 +++++++++++++++++++ 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/api/src/org/labkey/api/data/AbstractWrappedColumnInfo.java b/api/src/org/labkey/api/data/AbstractWrappedColumnInfo.java index 4c242f2e75f..73165428e45 100644 --- a/api/src/org/labkey/api/data/AbstractWrappedColumnInfo.java +++ b/api/src/org/labkey/api/data/AbstractWrappedColumnInfo.java @@ -848,10 +848,4 @@ 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 d3228c1459c..d6c4a12cfb4 100644 --- a/api/src/org/labkey/api/data/BaseColumnInfo.java +++ b/api/src/org/labkey/api/data/BaseColumnInfo.java @@ -75,7 +75,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; /** @@ -2233,10 +2232,4 @@ 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/ColumnInfo.java b/api/src/org/labkey/api/data/ColumnInfo.java index ce718f58b83..671638f3dab 100644 --- a/api/src/org/labkey/api/data/ColumnInfo.java +++ b/api/src/org/labkey/api/data/ColumnInfo.java @@ -15,6 +15,7 @@ */ package org.labkey.api.data; +import org.apache.commons.beanutils.ConversionException; import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -457,6 +458,38 @@ static String toString(ColumnInfo col) return sb.toString(); } + + /** + * 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. + * Also, converting the pieces of a MVFK column is handled by QUS. + */ + @Override + @Transient + default SimpleConvert getConvertFn() + { + return getDefaultConvertFn(this); + } + + /** see getConvertFn() */ + @Override + default Object convert(Object o) throws ConversionException + { + return getConvertFn().convert(o); + } + + /* NOTE: This could be folded into ColumnRenderProperties if we moved .isMultiValued() and .getFk(). */ + static SimpleConvert getDefaultConvertFn(ColumnInfo col) + { + if (col.isMultiValued() || col.getFk() instanceof MultiValuedForeignKey) + { + // See CoerceDataIterator.init() which just passes value unmodified. + // We could consider explicit convert to String[], but there is some argument for not doing nested + // conversion here (convert to Quatnity[]). + return (v) -> v; + } + return ColumnRenderProperties.getDefaultConvertFn(col); + } } From 0ddc248eaf6b05bb7108d960c261d94c9f8880c3 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Tue, 24 Feb 2026 10:53:18 -0800 Subject: [PATCH 2/3] SimpleConvert.identity --- api/src/org/labkey/api/data/ColumnInfo.java | 19 ++++++++----------- api/src/org/labkey/api/data/MultiChoice.java | 4 ++-- .../org/labkey/api/data/SimpleConvert.java | 2 ++ 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/api/src/org/labkey/api/data/ColumnInfo.java b/api/src/org/labkey/api/data/ColumnInfo.java index 671638f3dab..e1878a89b1a 100644 --- a/api/src/org/labkey/api/data/ColumnInfo.java +++ b/api/src/org/labkey/api/data/ColumnInfo.java @@ -459,11 +459,9 @@ static String toString(ColumnInfo col) return sb.toString(); } - /** - * 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. - * Also, converting the pieces of a MVFK column is handled by QUS. - */ + /// The returned Function should throw ConversionException (undeclared RuntimeException). + /// This method does not handle compound conversions e.g. MissingValues or Out-of-range indicators. + /// Also, converting the pieces of a MVFK column is handled by QUS. @Override @Transient default SimpleConvert getConvertFn() @@ -471,22 +469,21 @@ default SimpleConvert getConvertFn() return getDefaultConvertFn(this); } - /** see getConvertFn() */ @Override default Object convert(Object o) throws ConversionException { return getConvertFn().convert(o); } - /* NOTE: This could be folded into ColumnRenderProperties if we moved .isMultiValued() and .getFk(). */ + /// NOTE: This could be folded into ColumnRenderProperties if we moved .isMultiValued() and .getFk(). static SimpleConvert getDefaultConvertFn(ColumnInfo col) { if (col.isMultiValued() || col.getFk() instanceof MultiValuedForeignKey) { - // See CoerceDataIterator.init() which just passes value unmodified. - // We could consider explicit convert to String[], but there is some argument for not doing nested - // conversion here (convert to Quatnity[]). - return (v) -> v; + // The type for these columns is the element type (String, not String[]), and we do not want to convert to the element type. + // We could consider explicit convert to String[]. but MVFK handling is always a special-case in the UpdateService anyway. + // See also: CoerceDataIterator.init() which also just passes the value unmodified. + return SimpleConvert.identity; } return ColumnRenderProperties.getDefaultConvertFn(col); } diff --git a/api/src/org/labkey/api/data/MultiChoice.java b/api/src/org/labkey/api/data/MultiChoice.java index f3ed665143c..7612d13fc1b 100644 --- a/api/src/org/labkey/api/data/MultiChoice.java +++ b/api/src/org/labkey/api/data/MultiChoice.java @@ -525,7 +525,7 @@ public ResultSet getResultSet(long index, int count, Map> map) } - public static class Converter implements org.apache.commons.beanutils.Converter, Function + public static class Converter implements org.apache.commons.beanutils.Converter, SimpleConvert { private Converter() { @@ -558,7 +558,7 @@ public T convert(Class aClass, Object o) } @Override - final public Object apply(Object o) + final public Object convert(Object o) { return convert(MultiChoice.Array.class, o); } diff --git a/api/src/org/labkey/api/data/SimpleConvert.java b/api/src/org/labkey/api/data/SimpleConvert.java index ef19e199fb9..4918a7a3b32 100644 --- a/api/src/org/labkey/api/data/SimpleConvert.java +++ b/api/src/org/labkey/api/data/SimpleConvert.java @@ -23,4 +23,6 @@ default SimpleConvert getConvertFn() { return this; } + + SimpleConvert identity = (v) -> v; } \ No newline at end of file From 97d952a4b34825d554fb4b43a23e20cee926fcf6 Mon Sep 17 00:00:00 2001 From: labkey-matthewb Date: Tue, 24 Feb 2026 15:18:36 -0800 Subject: [PATCH 3/3] use TableViewForm.getSimpleConvert() which knows about MVFK --- api/src/org/labkey/api/data/ColumnInfo.java | 11 +++-------- api/src/org/labkey/api/data/TableViewForm.java | 9 +-------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/api/src/org/labkey/api/data/ColumnInfo.java b/api/src/org/labkey/api/data/ColumnInfo.java index e1878a89b1a..420f64f1b63 100644 --- a/api/src/org/labkey/api/data/ColumnInfo.java +++ b/api/src/org/labkey/api/data/ColumnInfo.java @@ -475,16 +475,11 @@ default Object convert(Object o) throws ConversionException return getConvertFn().convert(o); } - /// NOTE: This could be folded into ColumnRenderProperties if we moved .isMultiValued() and .getFk(). static SimpleConvert getDefaultConvertFn(ColumnInfo col) { - if (col.isMultiValued() || col.getFk() instanceof MultiValuedForeignKey) - { - // The type for these columns is the element type (String, not String[]), and we do not want to convert to the element type. - // We could consider explicit convert to String[]. but MVFK handling is always a special-case in the UpdateService anyway. - // See also: CoerceDataIterator.init() which also just passes the value unmodified. - return SimpleConvert.identity; - } + // NOTE for MultiValued columns e.g. col.isMultiValued() || col.getFk() instanceof MultiValuedForeignKey + // MultiValuedDisplayColumn currently relies on the parent fk column hanldling convert from String to the element type (see MultiValuedRenderContext.get()) + // However, This behavior is obviously wrong for MVFC that support insert/update, callers must be aware of this unfortunately (e.g. TableViewForm and CoerceDataIterator) return ColumnRenderProperties.getDefaultConvertFn(col); } } diff --git a/api/src/org/labkey/api/data/TableViewForm.java b/api/src/org/labkey/api/data/TableViewForm.java index 37e75fb333a..430646e1cf0 100644 --- a/api/src/org/labkey/api/data/TableViewForm.java +++ b/api/src/org/labkey/api/data/TableViewForm.java @@ -417,16 +417,9 @@ protected void _populateValues(BindException errors) try { - Object val; if (col != null) - { propType = col.getJavaClass(); - val = col.convert(bindValue); - } - else - { - val = getSimpleConvert(propName).convert(bindValue); - } + Object val = getSimpleConvert(propName).convert(bindValue); boolean requiredError = false; if (_validateRequired && null != _tinfo && null != col && col.isRequired() && !col.isAutoIncrement())