diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index df052bbe503..e775fd6d834 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -613,9 +613,23 @@ static void validateParentAlias(Map aliasMap, Set reserv throw new IllegalArgumentException(String.format("Parent alias header is reserved: %1$s", trimmedKey)); } - if (updatedDomainDesign != null && !existingAliases.contains(trimmedKey) && updatedDomainDesign.getFieldByName(trimmedKey) != null) + if (updatedDomainDesign != null) { - throw new IllegalArgumentException(String.format("An existing " + dataTypeNoun + " property conflicts with parent alias header: %1$s", trimmedKey)); + if (!existingAliases.contains(trimmedKey)) + { + var field = updatedDomainDesign.getFieldByName(trimmedKey); + if (field != null) + { + throw new IllegalArgumentException(String.format("An existing %1$s property conflicts with parent alias header: %2$s", dataTypeNoun, trimmedKey)); + } + } + // GH Issue 1257: If there are conflicts with import aliases, this should be an error since it produces ambiguity during import + var field = updatedDomainDesign.getFieldByImportAlias(trimmedKey); + if (field != null) + { + throw new IllegalArgumentException(String.format("Field %1$s has an import alias %2$s that conflicts with a parent alias header.", field.getName(), trimmedKey)); + } + } if (!dupes.add(trimmedKey)) diff --git a/api/src/org/labkey/api/exp/property/DomainUtil.java b/api/src/org/labkey/api/exp/property/DomainUtil.java index d10d3d5a854..2f443345214 100644 --- a/api/src/org/labkey/api/exp/property/DomainUtil.java +++ b/api/src/org/labkey/api/exp/property/DomainUtil.java @@ -26,6 +26,7 @@ import org.labkey.api.assay.AbstractAssayProvider; import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.collections.CaseInsensitiveLinkedHashMap; import org.labkey.api.collections.IntHashMap; import org.labkey.api.collections.LongHashMap; import org.labkey.api.data.ColumnInfo; @@ -1563,6 +1564,9 @@ public static ValidationException validateProperties(@Nullable Domain domain, @N Set reservedPrefixes = (null != domain && null != domainKind) ? domainKind.getReservedPropertyNamePrefixes() : updates.getReservedFieldNamePrefixes(); Map namePropertyIdMap = new CaseInsensitiveHashMap<>(); Map altNameMap = new CaseInsensitiveHashMap<>(); + // GH Issue 1257: import alias -> the fields declaring it. + Map> importAliasMap = new CaseInsensitiveLinkedHashMap<>(); + Set fieldNames = new CaseInsensitiveHashSet(); ValidationException exception = new ValidationException(); Map propertyIdNameMap = getOriginalFieldPropertyIdNameMap(orig);//key: orig property id, value : orig field name @@ -1577,6 +1581,15 @@ public static ValidationException validateProperties(@Nullable Domain domain, @N continue; } + fieldNames.add(name); + + // GH Issue 1257: Collect alias to field list mapping + for (String alias : new CaseInsensitiveHashSet(ColumnRenderPropertiesImpl.convertToSet(field.getImportAliases()))) + { + if (!alias.equalsIgnoreCase(name)) // an alias that repeats the field's name is redundant but not ambiguous, so skip it + importAliasMap.computeIfAbsent(alias, k -> new ArrayList<>()).add(field); + } + if (ILLEGAL_PROPERTY_NAMES.contains(name.trim())) { exception.addError(new SimpleValidationError(getDomainErrorMessage(updates, "The field name '" + name + "' is not allowed."))); @@ -1674,9 +1687,28 @@ public static ValidationException validateProperties(@Nullable Domain domain, @N } } + // GH Issue 1257: import aliases must be unique across the domain and must not collide with a field name, since + // ImportAliasable.Helper.createImportMap() resolves names, labels, and aliases into one case-insensitive map. + importAliasMap.forEach((alias, fields) -> { + String names = fields.stream().map(GWTPropertyDescriptor::getName).sorted().collect(Collectors.joining(", ")); + + if (fields.size() > 1) + addImportAliasErrors(exception, updates, fields, "Duplicate import alias " + alias + " for fields " + names + "."); + + if (fieldNames.contains(alias)) + addImportAliasErrors(exception, updates, fields, "Import alias " + alias + " on field" + (fields.size() == 1 ? " " : "s ") + names + " conflicts with a field name."); + }); + return exception; } + /** Anchor an import alias error on every field that declares the offending alias so the designer can highlight them. */ + private static void addImportAliasErrors(ValidationException exception, GWTDomain updates, List fields, String message) + { + for (GWTPropertyDescriptor field : fields) + exception.addError(new PropertyValidationError(getDomainErrorMessage(updates, message), field.getName(), field.getPropertyId())); + } + @Nullable private static Map getOriginalFieldPropertyIdNameMap(@Nullable GWTDomain orig) { diff --git a/api/src/org/labkey/api/gwt/client/model/GWTDomain.java b/api/src/org/labkey/api/gwt/client/model/GWTDomain.java index a0bc62bb98a..58eebeb2b1a 100644 --- a/api/src/org/labkey/api/gwt/client/model/GWTDomain.java +++ b/api/src/org/labkey/api/gwt/client/model/GWTDomain.java @@ -19,6 +19,8 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import lombok.Getter; import lombok.Setter; +import org.labkey.api.collections.CaseInsensitiveHashSet; +import org.labkey.api.data.ColumnRenderPropertiesImpl; import org.labkey.api.gwt.client.DefaultValueType; import java.util.ArrayList; @@ -197,6 +199,17 @@ public FieldType getFieldByName(String name) return null; } + public FieldType getFieldByImportAlias(String alias) + { + for (FieldType field : getFields(true)) + { + Set importAliases = ColumnRenderPropertiesImpl.convertToSet(field.getImportAliases()); + if (new CaseInsensitiveHashSet(importAliases).contains(alias)) + return field; + } + return null; + } + /** * @return Indicates that the property can't be removed from the domain. The property may or may not be nullable. */