mvtc empty #7348

labkey-matthewb · 2026-01-20T20:41:57Z

Rationale

For Array type required does not mean "IS NOT NULL" it means "cardinality() > 0".
Also for Multiple-choice text we prefer that null/"" converts to [] not to null.

To help implement this we consolidate column conversion into ColumnInfo.convert() and required/non-empty checking into JdbcType.isEmpty().

Related Pull Requests

Changes

- JdbcType implements SimpleConvert - PropertyType implements SimpleConvert - ColumnInfo implements SimpleConvert - Unit implements SimpleConvert

- JdbcType implements SimpleConvert - PropertyType implements SimpleConvertmake JdbcType.CHAR.convert() consistent with JdbcType.VARCHAR - ColumnInfo implements SimpleConvert - Unit implements SimpleConvert

Copilot

Pull request overview

This pull request consolidates column value conversion and required/non-empty validation logic across the codebase. The primary goals are to handle Array type validation correctly (where required means "cardinality() > 0" rather than "IS NOT NULL") and to ensure consistent conversion behavior for Multiple-choice text fields (converting null/"" to [] rather than null).

Changes:

Introduces the SimpleConvert interface to unify conversion logic across JdbcType, PropertyType, Unit, and ColumnInfo
Consolidates conversion logic into ColumnInfo.convert() instead of scattered ConvertUtils.convert() calls
Adds JdbcType.isEmpty() method to properly validate required fields based on type-specific semantics
Adds array_is_empty() SQL function for PostgreSQL to check array emptiness using cardinality

Reviewed changes

Copilot reviewed 43 out of 43 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
api/src/org/labkey/api/data/SimpleConvert.java	New interface defining conversion contract with optional performance optimization method
api/src/org/labkey/api/data/JdbcType.java	Implements SimpleConvert, adds isEmpty() method with Array-specific logic, refactors convert() methods
api/src/org/labkey/api/exp/PropertyType.java	Implements SimpleConvert, updates STRING/MULTI_LINE/XML_TEXT to convert empty strings to null
api/src/org/labkey/api/ontology/Unit.java	Implements SimpleConvert for Quantity conversion
api/src/org/labkey/api/data/ColumnRenderProperties.java	Adds SimpleConvert interface, refactors getConvertFn() to use new pattern
api/src/org/labkey/api/data/BaseColumnInfo.java	Implements getConvertFn() using ColumnRenderProperties.getDefaultConvertFn()
api/src/org/labkey/api/data/AbstractWrappedColumnInfo.java	Implements getConvertFn() delegation
api/src/org/labkey/api/exp/PropertyDescriptor.java	Implements getConvertFn() delegation
api/src/org/labkey/api/data/validator/RequiredValidator.java	Updated to use JdbcType parameter and isEmpty() method for validation
api/src/org/labkey/api/data/validator/ColumnValidators.java	Updated createRequiredValidator to pass JdbcType to RequiredValidator
api/src/org/labkey/api/dataiterator/SimpleTranslator.java	Refactored conversion columns to use ColumnInfo.convert(), removed PropertyConvertColumn classes
api/src/org/labkey/api/dataiterator/StandardDataIteratorBuilder.java	Updated to use simplified addConvertColumn signature
api/src/org/labkey/api/data/ConvertHelper.java	Added getSimpleConvert() factory method, updated DateFriendlyStringConverter to handle empty strings
api/src/org/labkey/api/data/TableViewForm.java	Refactored to use col.convert() instead of ConvertUtils.convert()
api/src/org/labkey/api/data/MultiChoice.java	Updated DisplayColumn to implement IMultiValuedDisplayColumn, improved rendering
api/src/org/labkey/api/query/*.java	Multiple query service classes updated to use col.convert()
experiment/src/org/labkey/experiment/api/*.java	Multiple experiment classes updated to use col.convert() or type.convert()
query/src/org/labkey/query/sql/Method.java	Added ArrayIsEmptyMethod for array_is_empty SQL function
api/src/org/labkey/api/webdav/WebdavResource.java	Added getSpringResource() helper method
api/webapp/WEB-INF/web.xml	Updated to Servlet 3.0 spec, added async-supported to filters
api/src/org/labkey/api/data/ColumnInfoTests.jsp	New test file for ColumnInfo.convert() integration testing
api/src/messages/Validation.properties	Fixed typo: "tyepMismatch" -> "typeMismatch"
core/src/org/labkey/core/CoreModule.java	Added ColumnInfoTests.jsp as integration test

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-20T20:57:07Z

api/src/org/labkey/api/webdav/WebdavResource.java

+            @Override
+            public URI getURI() throws IOException
+            {
+                // Nnot sure why getExceuteHref() takes a view context. We ay not need for the moment.


There are spelling errors in the comment. "Nnot" should be "Not", "getExceuteHref" should be "getExecuteHref", and "We ay" should be "We may".

Copilot · 2026-01-20T20:57:07Z

api/src/org/labkey/api/webdav/WebdavResource.java

+            @Override
+            public InputStream getInputStream() throws IOException
+            {
+                // FileSTream.openInputStream() enforces one stream per instance, so don't use 'fs'


There is a typo in the comment. "FileSTream" should be "FileStream".

Copilot · 2026-01-20T20:57:07Z

api/src/org/labkey/api/data/ColumnInfoTests.jsp

+     * A lot of this testing is redunant with lower-level unit testing,
+     * however, his till servers as a basic conversion smoke test.
+     * <p></p>
+     * In particualr, the PropertyType conversions are pretty
+     * redundant with ConvertHelper.convert() and JdbcType.convert()
+     * (PropertyType predates JdbcType), but there are some differenes


There are multiple spelling errors in the comment: "redunant" should be "redundant", "his till servers" should be "this still serves", "particualr" should be "particular", and "differenes" should be "differences".

Suggested change

* A lot of this testing is redunant with lower-level unit testing,

* however, his till servers as a basic conversion smoke test.

* <p></p>

* In particualr, the PropertyType conversions are pretty

* redundant with ConvertHelper.convert() and JdbcType.convert()

* (PropertyType predates JdbcType), but there are some differenes

* A lot of this testing is redundant with lower-level unit testing,

* however, this still serves as a basic conversion smoke test.

* <p></p>

* In particular, the PropertyType conversions are pretty

* redundant with ConvertHelper.convert() and JdbcType.convert()

* (PropertyType predates JdbcType), but there are some differences

Copilot · 2026-01-20T20:57:08Z

api/src/org/labkey/api/dataiterator/SimpleTranslator.java

+        }        final String name = col.getName();
+
+



There appears to be orphaned code on line 1347. The variable declaration final String name = col.getName(); is defined but never used in this scope, and appears to be a remnant from refactoring. This dead code should be removed.

Suggested change

} final String name = col.getName();

}

Copilot · 2026-01-20T20:57:08Z

api/src/org/labkey/api/data/SimpleConvert.java

+     * use getConvertFn() e.g.
+     *      var fn = col.getConvertFn();
+     *      while (...)
+     *          var converted = fb.convert(val);


There's a typo in the example code comment. Line 16 shows fb.convert(val) but should be fn.convert(val) to match the variable name defined on line 14.

Suggested change

* var converted = fb.convert(val);

* var converted = fn.convert(val);

Copilot · 2026-01-20T20:57:08Z

api/src/org/labkey/api/webdav/WebdavResource.java

+            @Override
+            public URL getURL() throws IOException
+            {
+                // Nnot sure why getExceuteHref() takes a view context. We ay not need for the moment.


There are spelling errors in the comment. "Nnot" should be "Not", "getExceuteHref" should be "getExecuteHref", and "We ay" should be "We may".

XingY and others added 30 commits January 8, 2026 12:39

Multi choices

685f9ed

Multi value text choices

322bd91

fix empty array

672a750

crlf

11f042c

support filters

65538dc

Merge remote-tracking branch 'origin/develop' into fb_mvtc

5a017c6

clean

9801a9c

fix

afbfda0

order inputs

c628cc8

Merge remote-tracking branch 'origin/develop' into fb_mvtc

3283986

order elements of a MultiChoice.Array

16150fc

explicit null/isBlank() tests

3013578

bug fixes

74f54e4

fix assay import

9e711cc

fix assay import

21cc32b

code review changes

4aabc6b

show advanced settings

defb3c1

Fix study dataset multi value crud

0edb7ec

code review changes

c34b984

Merge remote-tracking branch 'origin/develop' into fb_mvtc

6630ad0

code review changes

a96ac32

Merge remote-tracking branch 'origin/develop' into fb_mvtc_empty

61693b3

SimpleConvert

3059b18

- JdbcType implements SimpleConvert - PropertyType implements SimpleConvert - ColumnInfo implements SimpleConvert - Unit implements SimpleConvert

SimpleConvert

421a026

- JdbcType implements SimpleConvert - PropertyType implements SimpleConvertmake JdbcType.CHAR.convert() consistent with JdbcType.VARCHAR - ColumnInfo implements SimpleConvert - Unit implements SimpleConvert

JdbcType.isEmpty()

b446305

Merge remote-tracking branch 'origin/develop' into fb_mvtc_empty

e3e491b

add to the right branch

e7b3df9

Merge remote-tracking branch 'origin/develop' into fb_mvtc_empty

b1a9acd

merge from develop

6740d1a

fix selectRows api

03c2f64

XingY requested review from XingY and Copilot January 20, 2026 20:46

Copilot started reviewing on behalf of XingY January 20, 2026 20:47 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

XingY and others added 2 commits January 20, 2026 13:30

add compare type for array is null

b2c1fca

fix check

e4b9f51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

mvtc empty #7348

mvtc empty #7348

Uh oh!

labkey-matthewb commented Jan 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Copilot AI Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	* var converted = fb.convert(val);
	* var converted = fn.convert(val);

mvtc empty #7348

Are you sure you want to change the base?

mvtc empty #7348

Uh oh!

Conversation

labkey-matthewb commented Jan 20, 2026

Rationale

Related Pull Requests

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants