Skip to content

Prevent unsupported MVFK XML overrides #6793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
labkey-klum merged 4 commits into from
Jun 26, 2025
Merged

Prevent unsupported MVFK XML overrides #6793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c1e6eb9
Don't allow unsupported MVFK XML overrides
labkey-klum Jun 17, 2025
0219677
Merge branch 'develop' into fb_issue_51695
labkey-klum Jun 24, 2025
3ffd894
Merge branch 'develop' into fb_issue_51695
labkey-klum Jun 25, 2025
59c6051
code review feedback
labkey-klum Jun 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions api/src/org/labkey/api/data/AbstractTableInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<QueryParseException> _warnings = new ArrayList<>();
protected Iterable<FieldKey> _defaultVisibleColumns;
Expand Down Expand Up @@ -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
throw new UnsupportedOperationException("Non-junction multi-value columns NYI");
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;
Expand Down
4 changes: 2 additions & 2 deletions api/src/org/labkey/api/data/BaseColumnInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
throw new UnsupportedOperationException("Non-junction multi-value columns NYI");
LOG.warn(String.format("Error in FK configuration for table : \"%s\". The multi value FK type : \"%s\" is not supported.", getParentTable().getName(), type));
}
}

Expand Down
4 changes: 3 additions & 1 deletion api/src/org/labkey/api/data/JsonWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 3 additions & 1 deletion api/src/org/labkey/api/data/ReportingWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,9 @@ public static Map<String, Object> 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);
}
Expand Down
14 changes: 14 additions & 0 deletions query/src/org/labkey/query/controllers/QueryController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1375,6 +1375,7 @@ public void validateForm(SourceForm form, Errors errors)
{
try
{
validateForeignKey(fk, column, errors);
validateLookupFilter(AbstractTableInfo.parseXMLLookupFilters(fk.getFilters()), errors);
}
catch (ValidationException e)
Expand All @@ -1401,6 +1402,19 @@ public void validateForm(SourceForm form, 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 (!AbstractTableInfo.MultiValuedFkType.junction.name().equals(type))
{
errors.reject(ERROR_MSG, String.format("Column : \"%s\" has an invalid fkMultiValued value : \"%s\" is not supported.", column.getColumnName(), type));
}
}
}

private void validateLookupFilter(Map<ForeignKey.FilterOperation, List<FilterType>> filterMap, Errors errors)
{
filterMap.forEach((operation, filters) -> {
Expand Down