From c1e6eb973d5cbe8856627fb19f8ca3fbb0720b57 Mon Sep 17 00:00:00 2001 From: Lum Date: Tue, 17 Jun 2025 16:12:44 -0700 Subject: [PATCH 1/2] Don't allow unsupported MVFK XML overrides --- api/src/org/labkey/api/data/AbstractTableInfo.java | 2 +- api/src/org/labkey/api/data/BaseColumnInfo.java | 2 +- .../labkey/query/controllers/QueryController.java | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/data/AbstractTableInfo.java b/api/src/org/labkey/api/data/AbstractTableInfo.java index 9a8be988497..d8c89b5517e 100644 --- a/api/src/org/labkey/api/data/AbstractTableInfo.java +++ b/api/src/org/labkey/api/data/AbstractTableInfo.java @@ -1204,7 +1204,7 @@ public static ForeignKey makeForeignKey(QuerySchema fromSchema, ContainerFilter if ("junction".equals(type)) ret = new MultiValuedForeignKey(ret, fk.getFkJunctionLookup()); else - throw new UnsupportedOperationException("Non-junction multi-value columns NYI"); + LOG.warn(String.format("Non-junction multi value column type : \"%s\" in schema : \"%s\" is not supported.", type, fromSchema.getSchemaName())); } return ret; diff --git a/api/src/org/labkey/api/data/BaseColumnInfo.java b/api/src/org/labkey/api/data/BaseColumnInfo.java index 5201355a04d..8edd159709a 100644 --- a/api/src/org/labkey/api/data/BaseColumnInfo.java +++ b/api/src/org/labkey/api/data/BaseColumnInfo.java @@ -1185,7 +1185,7 @@ public void loadFromXml(ColumnType xmlCol, boolean merge) if ("junction".equals(type)) _fk = new MultiValuedForeignKey(new SchemaForeignKey(this, key.pkSchemaName, key.pkTableName, key.pkColumnNames.get(0), false), xfk.getFkJunctionLookup()); else - throw new UnsupportedOperationException("Non-junction multi-value columns NYI"); + LOG.warn(String.format("Non-junction multi value column type : \"%s\" in table : \"%s\" is not supported.", type, getParentTable().getName())); } } diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index 225fb3f423f..0ae8f2b72c4 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -1375,6 +1375,7 @@ public void validateForm(SourceForm form, Errors errors) { try { + validateForeignKey(fk, errors); validateLookupFilter(AbstractTableInfo.parseXMLLookupFilters(fk.getFilters()), errors); } catch (ValidationException e) @@ -1401,6 +1402,19 @@ public void validateForm(SourceForm form, Errors errors) } } + private void validateForeignKey(ColumnType.Fk fk, Errors errors) + { + if (fk.isSetFkMultiValued()) + { + // issue 51695 : don't let users create unsupported MVFK types + String type = fk.getFkMultiValued(); + if (!"junction".equals(type)) + { + errors.reject(ERROR_MSG, String.format("Non-junction multi value column type : \"%s\" is not supported.", type)); + } + } + } + private void validateLookupFilter(Map> filterMap, Errors errors) { filterMap.forEach((operation, filters) -> { From 59c6051781a67b4eea5c33d88e2da051aa4b0736 Mon Sep 17 00:00:00 2001 From: Lum Date: Wed, 25 Jun 2025 16:55:05 -0700 Subject: [PATCH 2/2] code review feedback --- api/src/org/labkey/api/data/AbstractTableInfo.java | 9 +++++++-- api/src/org/labkey/api/data/BaseColumnInfo.java | 4 ++-- api/src/org/labkey/api/data/JsonWriter.java | 4 +++- api/src/org/labkey/api/data/ReportingWriter.java | 4 +++- .../org/labkey/query/controllers/QueryController.java | 8 ++++---- 5 files changed, 19 insertions(+), 10 deletions(-) diff --git a/api/src/org/labkey/api/data/AbstractTableInfo.java b/api/src/org/labkey/api/data/AbstractTableInfo.java index d8c89b5517e..b776002aea0 100644 --- a/api/src/org/labkey/api/data/AbstractTableInfo.java +++ b/api/src/org/labkey/api/data/AbstractTableInfo.java @@ -127,6 +127,11 @@ abstract public class AbstractTableInfo implements TableInfo, AuditConfigurable, /** Used as a marker to indicate that a URL (such as insert or update) has been explicitly disabled. Null values get filled in with default URLs in some cases */ public static final DetailsURL LINK_DISABLER = new DetailsURL(LINK_DISABLER_ACTION_URL); + public enum MultiValuedFkType + { + junction, + value, + } private final List _warnings = new ArrayList<>(); protected Iterable _defaultVisibleColumns; @@ -1201,10 +1206,10 @@ public static ForeignKey makeForeignKey(QuerySchema fromSchema, ContainerFilter if (fk.isSetFkMultiValued()) { String type = fk.getFkMultiValued(); - if ("junction".equals(type)) + if (MultiValuedFkType.junction.name().equals(type)) ret = new MultiValuedForeignKey(ret, fk.getFkJunctionLookup()); else - LOG.warn(String.format("Non-junction multi value column type : \"%s\" in schema : \"%s\" is not supported.", type, fromSchema.getSchemaName())); + LOG.warn(String.format("Error in FK configuration for schema : \"%s\". The multi value FK type : \"%s\" is not supported.", fromSchema.getSchemaName(), type)); } return ret; diff --git a/api/src/org/labkey/api/data/BaseColumnInfo.java b/api/src/org/labkey/api/data/BaseColumnInfo.java index 8edd159709a..5d1f2225c2e 100644 --- a/api/src/org/labkey/api/data/BaseColumnInfo.java +++ b/api/src/org/labkey/api/data/BaseColumnInfo.java @@ -1182,10 +1182,10 @@ public void loadFromXml(ColumnType xmlCol, boolean merge) { String type = xfk.getFkMultiValued(); - if ("junction".equals(type)) + if (AbstractTableInfo.MultiValuedFkType.junction.name().equals(type)) _fk = new MultiValuedForeignKey(new SchemaForeignKey(this, key.pkSchemaName, key.pkTableName, key.pkColumnNames.get(0), false), xfk.getFkJunctionLookup()); else - LOG.warn(String.format("Non-junction multi value column type : \"%s\" in table : \"%s\" is not supported.", type, getParentTable().getName())); + LOG.warn(String.format("Error in FK configuration for table : \"%s\". The multi value FK type : \"%s\" is not supported.", getParentTable().getName(), type)); } } diff --git a/api/src/org/labkey/api/data/JsonWriter.java b/api/src/org/labkey/api/data/JsonWriter.java index 9d1e3917200..d23b7818844 100644 --- a/api/src/org/labkey/api/data/JsonWriter.java +++ b/api/src/org/labkey/api/data/JsonWriter.java @@ -418,7 +418,9 @@ public static JSONObject getLookupInfo(ColumnInfo columnInfo, boolean includeDom if (fk instanceof MultiValuedForeignKey mvfk) { String junctionLookup = mvfk.getJunctionLookup(); - lookupInfo.put("multiValued", junctionLookup != null ? "junction" : "value"); + lookupInfo.put("multiValued", junctionLookup != null + ? AbstractTableInfo.MultiValuedFkType.junction.name() + : AbstractTableInfo.MultiValuedFkType.value.name()); if (junctionLookup != null) lookupInfo.put("junctionLookup", junctionLookup); } diff --git a/api/src/org/labkey/api/data/ReportingWriter.java b/api/src/org/labkey/api/data/ReportingWriter.java index 33f61720471..2ca89b8a328 100644 --- a/api/src/org/labkey/api/data/ReportingWriter.java +++ b/api/src/org/labkey/api/data/ReportingWriter.java @@ -315,7 +315,9 @@ public static Map getLookupInfoMap(ColumnInfo columnInfo, boolea if (fk instanceof MultiValuedForeignKey mvfk) { String junctionLookup = mvfk.getJunctionLookup(); - lookupInfo.put("multiValued", junctionLookup != null ? "junction" : "value"); + lookupInfo.put("multiValued", junctionLookup != null + ? AbstractTableInfo.MultiValuedFkType.junction.name() + : AbstractTableInfo.MultiValuedFkType.value.name()); if (junctionLookup != null) lookupInfo.put("junctionLookup", junctionLookup); } diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index 0ae8f2b72c4..99c196a0501 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -1375,7 +1375,7 @@ public void validateForm(SourceForm form, Errors errors) { try { - validateForeignKey(fk, errors); + validateForeignKey(fk, column, errors); validateLookupFilter(AbstractTableInfo.parseXMLLookupFilters(fk.getFilters()), errors); } catch (ValidationException e) @@ -1402,15 +1402,15 @@ public void validateForm(SourceForm form, Errors errors) } } - private void validateForeignKey(ColumnType.Fk fk, Errors errors) + private void validateForeignKey(ColumnType.Fk fk, ColumnType column, Errors errors) { if (fk.isSetFkMultiValued()) { // issue 51695 : don't let users create unsupported MVFK types String type = fk.getFkMultiValued(); - if (!"junction".equals(type)) + if (!AbstractTableInfo.MultiValuedFkType.junction.name().equals(type)) { - errors.reject(ERROR_MSG, String.format("Non-junction multi value column type : \"%s\" is not supported.", type)); + errors.reject(ERROR_MSG, String.format("Column : \"%s\" has an invalid fkMultiValued value : \"%s\" is not supported.", column.getColumnName(), type)); } } }