diff --git a/experiment/src/org/labkey/experiment/SpecialCharacterMetricsMaintenanceTask.java b/experiment/src/org/labkey/experiment/SpecialCharacterMetricsMaintenanceTask.java index 08af9595693..5491c8c59bc 100644 --- a/experiment/src/org/labkey/experiment/SpecialCharacterMetricsMaintenanceTask.java +++ b/experiment/src/org/labkey/experiment/SpecialCharacterMetricsMaintenanceTask.java @@ -22,7 +22,10 @@ import org.junit.Before; import org.junit.Test; import org.labkey.api.collections.CaseInsensitiveHashMap; +import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.data.Container; +import org.labkey.api.data.DbSchema; +import org.labkey.api.data.DbSchemaType; import org.labkey.api.data.DbScope; import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SqlSelector; @@ -54,6 +57,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; public class SpecialCharacterMetricsMaintenanceTask implements MaintenanceTask { @@ -182,7 +186,7 @@ private void collectTextFieldMetrics(DbScope scope, SqlDialect dialect, Map new ArrayList<>()).add(new Col(rs.getString("storagecolumnname"), fieldType)); + byTable.computeIfAbsent(key, k -> new ArrayList<>()).add(new Col(rs.getString("storagecolumnname"), rs.getString("name"), fieldType)); }); // Sample type and data class provisioned tables have a base "name" column that is not a domain property (so it @@ -224,8 +228,8 @@ private void addBaseNameColumns(DbScope scope, Map> byTable) new SqlSelector(scope, sql).forEach(rs -> { TableKey key = new TableKey(rs.getString("storageschemaname"), rs.getString("storagetablename")); List cols = byTable.computeIfAbsent(key, k -> new ArrayList<>()); - if (cols.stream().noneMatch(col -> "name".equalsIgnoreCase(col.storageName()))) - cols.add(new Col("name", TYPE_DATA_NAME)); + if (cols.stream().noneMatch(col -> "name".equalsIgnoreCase(col.storageColumnName()))) + cols.add(new Col("name", "name", TYPE_DATA_NAME)); }); } @@ -234,9 +238,43 @@ private void scanTable(DbScope scope, SqlDialect dialect, TableKey table, List aliasMeta = new LinkedHashMap<>(); boolean first = true; + + Map remappedCols = new CaseInsensitiveHashMap<>(); + boolean needsRemap = false; + for (Col col : cols) + { + // GitHub Issue 1449: A legacy descriptor can carry a storage name that is the field name uniquified with a numeric suffix (e.g. "Container1") but never became a physical column + String storageColumnName = col.storageColumnName(); + if (isNumericSuffixOf(storageColumnName, col.colName())) + { + needsRemap = true; + break; + } + } + + if (needsRemap) + { + TableInfo ti = DbSchema.get(table.schema(), DbSchemaType.Provisioned).getTable(table.table()); + if (ti != null) + { + Set columnNames = new CaseInsensitiveHashSet(ti.getColumnNameSet()); + for (Col col : cols) + { + String storageColumnName = col.storageColumnName(); + if (isNumericSuffixOf(storageColumnName, col.colName()) && !columnNames.contains(storageColumnName) && columnNames.contains(col.colName())) + remappedCols.put(storageColumnName, col.colName()); + } + } + } + for (int i = 0; i < cols.size(); i++) { - SQLFragment colRef = PropertyDescriptor.getLegalSelectNameFromStorageName(dialect, cols.get(i).storageName()).getSql(); + Col col = cols.get(i); + String storageColumnName = col.storageColumnName(); + if (remappedCols.containsKey(storageColumnName)) + storageColumnName = remappedCols.get(storageColumnName); + + SQLFragment colRef = PropertyDescriptor.getLegalSelectNameFromStorageName(dialect, storageColumnName).getSql(); for (String ck : CHAR_KEYS) { String alias = "c" + i + "_" + ck.toLowerCase(); @@ -246,7 +284,7 @@ private void scanTable(DbScope scope, SqlDialect dialect, TableKey table, List newRecordMap, @Nullable List calculatedFields) throws ChangePropertyDescriptorException { - save(user, false, false, null, null, null, newRecordMap, null, calculatedFields); + save(user, false, null, null, null, newRecordMap, null, calculatedFields); } @Override @@ -602,10 +602,10 @@ public void save(User user, @Nullable String auditComment, @Nullable String audi @Nullable Map oldRecordMap, @Nullable Map newRecordMap, @Nullable List oldCalculatedFields, @Nullable List newCalculatedFields) throws ChangePropertyDescriptorException { - save(user, false, false, auditComment, auditUserComment, oldRecordMap, newRecordMap, oldCalculatedFields, newCalculatedFields); + save(user, false, auditComment, auditUserComment, oldRecordMap, newRecordMap, oldCalculatedFields, newCalculatedFields); } - public void save(User user, boolean allowAddBaseProperty, boolean saveOnlyIfNotExists, @Nullable String auditComment, @Nullable String auditUserComment, + public void save(User user, boolean saveOnlyIfNotExists, @Nullable String auditComment, @Nullable String auditUserComment, @Nullable Map oldRecordMap, @Nullable Map newRecordMap, @Nullable List oldCalculatedFields, @Nullable List newCalculatedFields) throws ChangePropertyDescriptorException { @@ -718,7 +718,7 @@ public void save(User user, boolean allowAddBaseProperty, boolean saveOnlyIfNotE // make sure all properties have storageColumnName if (null == impl._pd.getStorageColumnName()) { - if (!allowAddBaseProperty && baseProperties.contains(newPropName)) + if (baseProperties.contains(newPropName)) impl._pd.setStorageColumnName(newPropName); // Issue 29047: if we allow base property (like "date"), we're later going to use the base property name for storage else generateStorageColumnName(impl._pd); @@ -845,7 +845,7 @@ else if (null != pdOld) { if (!propsAdded.isEmpty()) { - StorageProvisionerImpl.get().addProperties(this, propsAdded, allowAddBaseProperty); + StorageProvisionerImpl.get().addProperties(this, propsAdded); try { ensureUniqueIdValues(propsAdded); diff --git a/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java b/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java index ad584e9bfaa..a755f3e3dd5 100644 --- a/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java @@ -364,7 +364,7 @@ public void addStorageProperties(Domain domain, Collection change.execute(); } - public void addProperties(Domain domain, Collection properties, boolean allowAddBaseProperty) + public void addProperties(Domain domain, Collection properties) { DomainKind kind = domain.getDomainKind(); DbScope scope = kind.getScope(); @@ -389,7 +389,7 @@ public void addProperties(Domain domain, Collection properties, if (prop.getName() == null || prop.getName().isEmpty()) throw new IllegalArgumentException("Can't add property with no name: " + prop.getPropertyURI()); - if (!allowAddBaseProperty && base.contains(prop.getName())) + if (base.contains(prop.getName())) { // apparently this is a case where the domain allows a propertydescriptor to be defined with the same // name as a built-in column. e.g. to allow setting overrides?