From a5ce44bd735c8c714f22ff48df2304b622861a1e Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 11 Sep 2026 15:35:29 -0500 Subject: [PATCH 1/6] GitHub Issue #1569: Qualify default value LSIDs by domain kind - Domain URI objectIds repeat across kinds and an assay design's domains share one outright, so unrelated domains collided on a single default-value object and each save destroyed the other's values --- .../defaults/DefaultValueServiceImpl.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java b/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java index aabf1fb6965..58d76cc60a2 100644 --- a/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java +++ b/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java @@ -64,7 +64,7 @@ private String getContainerDefaultsLSID(Container container, Domain domain) if (kind != null && kind.isUserCreatedType()) { Lsid domainLsid = new Lsid(domain.getTypeURI()); - return (new Lsid(DOMAIN_DEFAULT_VALUE_LSID_PREFIX, suffix, domainLsid.getObjectId())).toString(); + return (new Lsid(qualifyByDomainKind(DOMAIN_DEFAULT_VALUE_LSID_PREFIX, domainLsid), suffix, domainLsid.getObjectId())).toString(); } else // for internal domains (such as audit domains), the domain name and objectId are often not the same. return (new Lsid(DOMAIN_DEFAULT_VALUE_LSID_PREFIX, suffix, domain.getName())).toString(); @@ -78,18 +78,26 @@ private String getUserDefaultsParentLSID(Container container, User user, Domain if (kind != null && kind.isUserCreatedType()) { Lsid domainLsid = new Lsid(domain.getTypeURI()); - return (new Lsid(USER_DEFAULT_VALUE_DOMAIN_PARENT, suffix, domainLsid.getObjectId())).toString(); + return (new Lsid(qualifyByDomainKind(USER_DEFAULT_VALUE_DOMAIN_PARENT, domainLsid), suffix, domainLsid.getObjectId())).toString(); } else // for internal domains (such as audit domains), the domain name and objectId are often not the same. return (new Lsid(USER_DEFAULT_VALUE_DOMAIN_PARENT, suffix, domain.getName())).toString(); } + // GitHub Issue #1569: Qualifying LSID by domain kind since multiple data types have distinct domain kinds + // Ex: assay designs have AssayDomain-Batch, AssayDomain-Run, AssayDomain-Result + private String qualifyByDomainKind(String lsidPrefix, Lsid domainLsid) + { + return lsidPrefix + "-" + domainLsid.getNamespacePrefix(); + } + private static final String WILD_CARD_PLACEHOLDER = "WILDCARD"; private String getUserDefaultsWildcardLSID(Container container, Domain domain, boolean parentObject) { - String suffix = "Folder-" + container.getRowId() + ".User-" + WILD_CARD_PLACEHOLDER + (!parentObject ? "." + WILD_CARD_PLACEHOLDER : ""); - String objectId = domain.getName(); + String suffix = "Folder-" + container.getRowId() + ".User-" + WILD_CARD_PLACEHOLDER; + // getUserDefaultsLSID() appends the scope to the objectId, not to the namespace, so the wildcard has to match there + String objectId = domain.getName() + (!parentObject ? "." + WILD_CARD_PLACEHOLDER : ""); String lsid = (new Lsid(USER_DEFAULT_VALUE_LSID_PREFIX, suffix, objectId)).toString(); // this hack is to include '%' characters in an LSID-like string. The '%' character can't be part of the // lsid components passed to the Lsid constructor, or it will be encoded as '%25'. From fc3ef00e080ff04abd6551cc3412f6f0c630cfa8 Mon Sep 17 00:00:00 2001 From: cnathe Date: Fri, 11 Sep 2026 15:37:54 -0500 Subject: [PATCH 2/6] GitHub Issue #1569: Migrate default value LSIDs to the kind-qualified form - Re-keys existing folder-level and per-user parent objects so defaults saved before the fix are found again, resolving each object's owning domain from the properties it holds since several domains could previously share one object. --- .../experiment/ExperimentUpgradeCode.java | 206 ++++++++++++++++++ 1 file changed, 206 insertions(+) diff --git a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java index e693224a7ec..7fa2c4d1989 100644 --- a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java +++ b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java @@ -54,6 +54,7 @@ import org.labkey.api.data.UpgradeCode; import org.labkey.api.data.dialect.BasePostgreSqlDialect; import org.labkey.api.data.dialect.PostgreSqlService; +import org.labkey.api.exp.Lsid; import org.labkey.api.exp.OntologyManager; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.api.ExpSampleType; @@ -62,6 +63,7 @@ import org.labkey.api.exp.api.SampleTypeService; import org.labkey.api.exp.api.StorageProvisioner; import org.labkey.api.exp.property.Domain; +import org.labkey.api.exp.property.DomainKind; import org.labkey.api.exp.property.DomainUtil; import org.labkey.api.exp.property.PropertyService; import org.labkey.api.files.FileContentService; @@ -97,6 +99,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -816,4 +819,207 @@ public static void shortenAllStorageNames(ModuleContext context) if (!badColumnNames.isEmpty()) LOG.error("Some storage column names are still too long!! {}", badColumnNames); } + + // Legacy prefixes, frozen at their historical values so later renames of the service constants can't change what this upgrade matches. + private static final String LEGACY_CONTAINER_DEFAULTS_PREFIX = "DomainDefaultValue"; + private static final String LEGACY_USER_DEFAULTS_PARENT_PREFIX = "UserDefaultValueParent"; + + /** + * Called from exp-26.005-26.006.sql + * GitHub Issue #1569: re-key folder-level default values by domain kind. The old key was the container plus the + * domain typeURI objectId, which repeats across kinds and is shared outright by an assay design's domains, so + * unrelated domains silently overwrote each other's defaults. Rows written before 25.7 are keyed by domain name + * instead; both forms are handled here. The owning domain is resolved from the properties the object actually + * holds, which is what disambiguates rows that several domains once shared. + */ + @SuppressWarnings("unused") + @DeferredUpgrade + public static void migrateDefaultValueLsids(ModuleContext context) + { + if (context.isNewInstall()) + return; + + try (Transaction tx = ExperimentService.get().ensureTransaction()) + { + migrateContainerDefaults(); + reparentUserDefaults(); + tx.commit(); + } + } + + private static void migrateContainerDefaults() + { + SQLFragment sql = new SQLFragment() + .append("SELECT o.ObjectId, o.ObjectURI, MIN(pdm.DomainId) AS DomainId, COUNT(DISTINCT pdm.DomainId) AS DomainCount\n") + .append("FROM ").append(OntologyManager.getTinfoObject(), "o").append("\n") + .append("INNER JOIN ").append(OntologyManager.getTinfoObjectProperty(), "op").append(" ON op.ObjectId = o.ObjectId\n") + .append("INNER JOIN ").append(OntologyManager.getTinfoPropertyDomain(), "pdm").append(" ON pdm.PropertyId = op.PropertyId\n") + // the legacy prefix is followed by '.'; the qualified form written below inserts '-' before it + .append("WHERE o.ObjectURI LIKE ?\n").add("%:" + LEGACY_CONTAINER_DEFAULTS_PREFIX + ".Folder-%") + .append("GROUP BY o.ObjectId, o.ObjectURI"); + + List rows = new SqlSelector(ExperimentService.get().getSchema(), sql).getArrayList(DefaultValueRow.class); + int migrated = 0; + + // Both legacy forms can map onto the same target. The objectId form was written by 25.7 or later, so it is the + // newer of the two and is moved first; the name form then loses the NOT EXISTS race below, as it should. + for (boolean objectIdForm : new boolean[] {true, false}) + { + for (DefaultValueRow row : rows) + { + Domain domain = resolveDomain(row); + if (domain == null) + continue; + + Lsid domainLsid = new Lsid(domain.getTypeURI()); + if (objectIdForm != new Lsid(row.getObjectURI()).getObjectId().equals(domainLsid.getObjectId())) + continue; + + String newUri = qualifiedLsid(row.getObjectURI(), domainLsid); + if (newUri == null || newUri.equals(row.getObjectURI())) + continue; + + if (renameObject(row.getObjectId(), newUri)) + migrated++; + else + LOG.warn("Leaving default values at {}: {} already exists.", row.getObjectURI(), newUri); + } + } + + LOG.info("Folder-level default values re-keyed by domain kind: {} of {} moved.", migrated, rows.size()); + } + + /** + * The per-user parent object groups a domain's scopes so they can be deleted together. A colliding parent collected + * children from several domains, so only one of them can keep it -- the rest get a parent of their own. + */ + private static void reparentUserDefaults() + { + SQLFragment sql = new SQLFragment() + .append("SELECT child.ObjectId, child.ObjectURI, parent.ObjectId AS ParentId, parent.ObjectURI AS ParentURI, parent.Container,\n") + .append(" MIN(pdm.DomainId) AS DomainId, COUNT(DISTINCT pdm.DomainId) AS DomainCount\n") + .append("FROM ").append(OntologyManager.getTinfoObject(), "parent").append("\n") + .append("INNER JOIN ").append(OntologyManager.getTinfoObject(), "child").append(" ON child.OwnerObjectId = parent.ObjectId\n") + .append("INNER JOIN ").append(OntologyManager.getTinfoObjectProperty(), "op").append(" ON op.ObjectId = child.ObjectId\n") + .append("INNER JOIN ").append(OntologyManager.getTinfoPropertyDomain(), "pdm").append(" ON pdm.PropertyId = op.PropertyId\n") + .append("WHERE parent.ObjectURI LIKE ?\n").add("%:" + LEGACY_USER_DEFAULTS_PARENT_PREFIX + ".Folder-%") + .append("GROUP BY child.ObjectId, child.ObjectURI, parent.ObjectId, parent.ObjectURI, parent.Container"); + + List rows = new SqlSelector(ExperimentService.get().getSchema(), sql).getArrayList(DefaultValueRow.class); + + // children of each legacy parent, bucketed by the qualified parent they now belong under + Map>> byLegacyParent = new LinkedHashMap<>(); + for (DefaultValueRow row : rows) + { + Domain domain = resolveDomain(row); + if (domain == null) + continue; + + String newParentUri = qualifiedLsid(row.getParentURI(), new Lsid(domain.getTypeURI())); + if (newParentUri == null || newParentUri.equals(row.getParentURI())) + continue; + + byLegacyParent.computeIfAbsent(row.getParentId(), k -> new LinkedHashMap<>()) + .computeIfAbsent(newParentUri, k -> new ArrayList<>()).add(row); + } + + int reparented = 0; + + for (Map.Entry>> legacyParent : byLegacyParent.entrySet()) + { + boolean legacyParentReused = false; + + for (Map.Entry> group : legacyParent.getValue().entrySet()) + { + // Renaming the legacy parent for the first group leaves nothing behind; its children still point at it + if (!legacyParentReused && renameObject(legacyParent.getKey(), group.getKey())) + { + legacyParentReused = true; + reparented += group.getValue().size(); + continue; + } + + Container container = ContainerManager.getForId(group.getValue().get(0).getContainer()); + if (container == null) + continue; + + long newParentId = OntologyManager.ensureObject(container, group.getKey()); + for (DefaultValueRow row : group.getValue()) + { + SQLFragment update = new SQLFragment("UPDATE ").append(OntologyManager.getTinfoObject()) + .append(" SET OwnerObjectId = ? WHERE ObjectId = ?").addAll(newParentId, row.getObjectId()); + new SqlExecutor(ExperimentService.get().getSchema()).execute(update); + reparented++; + } + } + } + + LOG.info("Per-user default values re-parented by domain kind: {} of {} moved.", reparented, rows.size()); + } + + /** The owning domain, or null when the object's properties don't identify exactly one resolvable user-created domain. */ + private static Domain resolveDomain(DefaultValueRow row) + { + if (row.getDomainCount() != 1) + { + LOG.warn("Leaving default values at {}: properties span {} domains, so the owner is ambiguous.", row.getObjectURI(), row.getDomainCount()); + return null; + } + + Domain domain = PropertyService.get().getDomain(row.getDomainId()); + if (domain == null) + { + LOG.warn("Leaving default values at {}: domain {} no longer exists.", row.getObjectURI(), row.getDomainId()); + return null; + } + + // Domains whose kind can't be resolved have always used the name-based key, so they are already correct + DomainKind kind = domain.getDomainKind(); + return kind != null && kind.isUserCreatedType() ? domain : null; + } + + /** Rebuild a default-value LSID with the domain kind folded into the namespace prefix, preserving Folder-/User- scope. */ + private static String qualifiedLsid(String objectURI, Lsid domainLsid) + { + Lsid existing = new Lsid(objectURI); + if (existing.getNamespaceSuffix() == null) + return null; + return new Lsid(existing.getNamespacePrefix() + "-" + domainLsid.getNamespacePrefix(), existing.getNamespaceSuffix(), domainLsid.getObjectId()).toString(); + } + + /** A row already at the target was written under the new scheme and is newer, so it wins. */ + private static boolean renameObject(long objectId, String newUri) + { + SQLFragment update = new SQLFragment("UPDATE ").append(OntologyManager.getTinfoObject()) + .append(" SET ObjectURI = ? WHERE ObjectId = ? AND NOT EXISTS (SELECT 1 FROM ") + .append(OntologyManager.getTinfoObject(), "existing").append(" WHERE existing.ObjectURI = ?)") + .addAll(newUri, objectId, newUri); + return new SqlExecutor(ExperimentService.get().getSchema()).execute(update) > 0; + } + + public static class DefaultValueRow + { + private long _objectId; + private String _objectURI; + private long _parentId; + private String _parentURI; + private String _container; + private int _domainId; + private int _domainCount; + + public long getObjectId() { return _objectId; } + public void setObjectId(long objectId) { _objectId = objectId; } + public String getObjectURI() { return _objectURI; } + public void setObjectURI(String objectURI) { _objectURI = objectURI; } + public long getParentId() { return _parentId; } + public void setParentId(long parentId) { _parentId = parentId; } + public String getParentURI() { return _parentURI; } + public void setParentURI(String parentURI) { _parentURI = parentURI; } + public String getContainer() { return _container; } + public void setContainer(String container) { _container = container; } + public int getDomainId() { return _domainId; } + public void setDomainId(int domainId) { _domainId = domainId; } + public int getDomainCount() { return _domainCount; } + public void setDomainCount(int domainCount) { _domainCount = domainCount; } + } } From db1ca13dd1e5a7c7fdc12a01d0fcd394f6501cb8 Mon Sep 17 00:00:00 2001 From: cnathe Date: Wed, 16 Sep 2026 10:48:23 -0500 Subject: [PATCH 3/6] remove comment referencing a SQL upgrade script file --- experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java | 1 - 1 file changed, 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java index 7fa2c4d1989..8367b5ac361 100644 --- a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java +++ b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java @@ -825,7 +825,6 @@ public static void shortenAllStorageNames(ModuleContext context) private static final String LEGACY_USER_DEFAULTS_PARENT_PREFIX = "UserDefaultValueParent"; /** - * Called from exp-26.005-26.006.sql * GitHub Issue #1569: re-key folder-level default values by domain kind. The old key was the container plus the * domain typeURI objectId, which repeats across kinds and is shared outright by an assay design's domains, so * unrelated domains silently overwrote each other's defaults. Rows written before 25.7 are keyed by domain name From 08c41a57ce13fbde4595231c9ea6891661f02cfc Mon Sep 17 00:00:00 2001 From: cnathe Date: Wed, 16 Sep 2026 13:11:59 -0500 Subject: [PATCH 4/6] Add metric for count of saved default value property values per domain kind and type --- .../api/defaults/DefaultValueService.java | 1 + .../api/property/PropertyServiceImpl.java | 43 ++++++++++++++++++- .../defaults/DefaultValueServiceImpl.java | 1 - 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/defaults/DefaultValueService.java b/api/src/org/labkey/api/defaults/DefaultValueService.java index f2b5e93eb62..e2b9a975aa8 100644 --- a/api/src/org/labkey/api/defaults/DefaultValueService.java +++ b/api/src/org/labkey/api/defaults/DefaultValueService.java @@ -41,6 +41,7 @@ public interface DefaultValueService { String DOMAIN_DEFAULT_VALUE_LSID_PREFIX = "DomainDefaultValue"; + String USER_DEFAULT_VALUE_LSID_PREFIX = "UserDefaultValue"; static DefaultValueService get() { diff --git a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java index 0ea06467b31..6ef6a0203b0 100644 --- a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java @@ -48,6 +48,7 @@ import org.labkey.api.data.Table; import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; +import org.labkey.api.defaults.DefaultValueService; import org.labkey.api.exceptions.OptimisticConflictException; import org.labkey.api.exp.ChangePropertyDescriptorException; import org.labkey.api.exp.DomainDescriptor; @@ -730,11 +731,51 @@ public Map getUsageMetrics() "propertyCountsByConcept", stripUriPrefixes(new SqlSelector(schema, new SQLFragment("SELECT CASE WHEN ConceptURI IS NULL THEN 'null' ELSE ConceptURI END, COUNT(*) AS Count FROM exp.PropertyDescriptor GROUP BY ConceptURI") ).getValueMap(String.class)), - "conditionalFormattingFields", new SqlSelector(schema, new SQLFragment("SELECT COUNT (DISTINCT propertyid) from exp.conditionalformat")).getObject(Long.class), + "defaultValuePropertyCounts", Map.of( + "folder", savedDefaultValuePropertyCounts(schema, "%:" + DefaultValueService.DOMAIN_DEFAULT_VALUE_LSID_PREFIX + "%.Folder-%"), + "user", savedDefaultValuePropertyCounts(schema, "%:" + DefaultValueService.USER_DEFAULT_VALUE_LSID_PREFIX + ".Folder-%") + ), + "conditionalFormattingFields", new SqlSelector(schema, new SQLFragment("SELECT COUNT (DISTINCT propertyid) from exp.conditionalformat")).getObject(Long.class), "storageColumnNameMismatches", storageColumnNameMismatches ); } + /** + * Properties that a user has actually saved a default value for, counted by the property's default value type and + * by the namespace prefix of the domain that owns it. + */ + private Map savedDefaultValuePropertyCounts(DbSchema schema, String lsidPattern) + { + Map> byDefaultValueType = new HashMap<>(); + Map> byDomainKind = new HashMap<>(); + + // One row per property/domain pair, so a property shared by two domains counts toward both kinds + SQLFragment sql = new SQLFragment(""" + SELECT DISTINCT OP.PropertyId, PD.DefaultValueType, DD.DomainURI + FROM exp.ObjectProperty OP + INNER JOIN exp.PropertyDescriptor PD ON PD.PropertyId = OP.PropertyId + INNER JOIN exp.PropertyDomain PDM ON PDM.PropertyId = OP.PropertyId + INNER JOIN exp.DomainDescriptor DD ON DD.DomainId = PDM.DomainId + WHERE OP.ObjectId IN (SELECT ObjectId FROM exp.Object WHERE ObjectURI LIKE ?)""").add(lsidPattern); + + for (Map row : new SqlSelector(schema, sql).getMapCollection()) + { + Integer propertyId = ((Number) row.get("PropertyId")).intValue(); + Object defaultValueType = row.get("DefaultValueType"); + Lsid domainLsid = new Lsid((String) row.get("DomainURI")); + + byDefaultValueType.computeIfAbsent(defaultValueType == null ? "null" : defaultValueType.toString(), k -> new HashSet<>()).add(propertyId); + byDomainKind.computeIfAbsent(domainLsid.isValid() ? domainLsid.getNamespacePrefix() : "null", k -> new HashSet<>()).add(propertyId); + } + + return Map.of("byDefaultValueType", countDistinct(byDefaultValueType), "byDomainKind", countDistinct(byDomainKind)); + } + + private static Map countDistinct(Map> propertyIds) + { + return propertyIds.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().size())); + } + /** * @return a map where the URI keys have been stripped of everything before the hash or xsd: prefix. * http://www.labkey.org/exp/xml#attachment -> attachment diff --git a/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java b/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java index 58d76cc60a2..685635fb438 100644 --- a/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java +++ b/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java @@ -52,7 +52,6 @@ public class DefaultValueServiceImpl implements DefaultValueService { - private static final String USER_DEFAULT_VALUE_LSID_PREFIX = "UserDefaultValue"; private static final String USER_DEFAULT_VALUE_DOMAIN_PARENT = "UserDefaultValueParent"; private final Lock _lock = new ReentrantLockWithName(DefaultValueServiceImpl.class, "_lock"); From 9b0f560abd08c9c172e86ee6ffb01218b4ae8ff6 Mon Sep 17 00:00:00 2001 From: cnathe Date: Wed, 16 Sep 2026 13:46:56 -0500 Subject: [PATCH 5/6] Clear caches after migration --- experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java index 8367b5ac361..825af55dd1d 100644 --- a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java +++ b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java @@ -844,6 +844,8 @@ public static void migrateDefaultValueLsids(ModuleContext context) reparentUserDefaults(); tx.commit(); } + + OntologyManager.clearCaches(); } private static void migrateContainerDefaults() From 5834da1420feb168860aef6c3c100a8a6348ebd9 Mon Sep 17 00:00:00 2001 From: cnathe Date: Thu, 17 Sep 2026 15:09:41 -0500 Subject: [PATCH 6/6] Code review feedback: add and remove some annotations, use Record in ExperimentUpgradeCode, --- .../experiment/ExperimentUpgradeCode.java | 76 ++++++++----------- .../api/property/PropertyServiceImpl.java | 5 +- .../defaults/DefaultValueServiceImpl.java | 3 +- 3 files changed, 34 insertions(+), 50 deletions(-) diff --git a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java index 825af55dd1d..47690b395a7 100644 --- a/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java +++ b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java @@ -832,7 +832,6 @@ public static void shortenAllStorageNames(ModuleContext context) * holds, which is what disambiguates rows that several domains once shared. */ @SuppressWarnings("unused") - @DeferredUpgrade public static void migrateDefaultValueLsids(ModuleContext context) { if (context.isNewInstall()) @@ -859,31 +858,31 @@ private static void migrateContainerDefaults() .append("WHERE o.ObjectURI LIKE ?\n").add("%:" + LEGACY_CONTAINER_DEFAULTS_PREFIX + ".Folder-%") .append("GROUP BY o.ObjectId, o.ObjectURI"); - List rows = new SqlSelector(ExperimentService.get().getSchema(), sql).getArrayList(DefaultValueRow.class); + List rows = new SqlSelector(ExperimentService.get().getSchema(), sql).getArrayList(ContainerDefaultRow.class); int migrated = 0; // Both legacy forms can map onto the same target. The objectId form was written by 25.7 or later, so it is the // newer of the two and is moved first; the name form then loses the NOT EXISTS race below, as it should. for (boolean objectIdForm : new boolean[] {true, false}) { - for (DefaultValueRow row : rows) + for (ContainerDefaultRow row : rows) { Domain domain = resolveDomain(row); if (domain == null) continue; Lsid domainLsid = new Lsid(domain.getTypeURI()); - if (objectIdForm != new Lsid(row.getObjectURI()).getObjectId().equals(domainLsid.getObjectId())) + if (objectIdForm != new Lsid(row.objectURI()).getObjectId().equals(domainLsid.getObjectId())) continue; - String newUri = qualifiedLsid(row.getObjectURI(), domainLsid); - if (newUri == null || newUri.equals(row.getObjectURI())) + String newUri = qualifiedLsid(row.objectURI(), domainLsid); + if (newUri == null || newUri.equals(row.objectURI())) continue; - if (renameObject(row.getObjectId(), newUri)) + if (renameObject(row.objectId(), newUri)) migrated++; else - LOG.warn("Leaving default values at {}: {} already exists.", row.getObjectURI(), newUri); + LOG.warn("Leaving default values at {}: {} already exists.", row.objectURI(), newUri); } } @@ -906,31 +905,31 @@ private static void reparentUserDefaults() .append("WHERE parent.ObjectURI LIKE ?\n").add("%:" + LEGACY_USER_DEFAULTS_PARENT_PREFIX + ".Folder-%") .append("GROUP BY child.ObjectId, child.ObjectURI, parent.ObjectId, parent.ObjectURI, parent.Container"); - List rows = new SqlSelector(ExperimentService.get().getSchema(), sql).getArrayList(DefaultValueRow.class); + List rows = new SqlSelector(ExperimentService.get().getSchema(), sql).getArrayList(UserDefaultRow.class); // children of each legacy parent, bucketed by the qualified parent they now belong under - Map>> byLegacyParent = new LinkedHashMap<>(); - for (DefaultValueRow row : rows) + Map>> byLegacyParent = new LinkedHashMap<>(); + for (UserDefaultRow row : rows) { Domain domain = resolveDomain(row); if (domain == null) continue; - String newParentUri = qualifiedLsid(row.getParentURI(), new Lsid(domain.getTypeURI())); - if (newParentUri == null || newParentUri.equals(row.getParentURI())) + String newParentUri = qualifiedLsid(row.parentURI(), new Lsid(domain.getTypeURI())); + if (newParentUri == null || newParentUri.equals(row.parentURI())) continue; - byLegacyParent.computeIfAbsent(row.getParentId(), k -> new LinkedHashMap<>()) + byLegacyParent.computeIfAbsent(row.parentId(), k -> new LinkedHashMap<>()) .computeIfAbsent(newParentUri, k -> new ArrayList<>()).add(row); } int reparented = 0; - for (Map.Entry>> legacyParent : byLegacyParent.entrySet()) + for (Map.Entry>> legacyParent : byLegacyParent.entrySet()) { boolean legacyParentReused = false; - for (Map.Entry> group : legacyParent.getValue().entrySet()) + for (Map.Entry> group : legacyParent.getValue().entrySet()) { // Renaming the legacy parent for the first group leaves nothing behind; its children still point at it if (!legacyParentReused && renameObject(legacyParent.getKey(), group.getKey())) @@ -940,15 +939,15 @@ private static void reparentUserDefaults() continue; } - Container container = ContainerManager.getForId(group.getValue().get(0).getContainer()); + Container container = ContainerManager.getForId(group.getValue().get(0).container()); if (container == null) continue; long newParentId = OntologyManager.ensureObject(container, group.getKey()); - for (DefaultValueRow row : group.getValue()) + for (UserDefaultRow row : group.getValue()) { SQLFragment update = new SQLFragment("UPDATE ").append(OntologyManager.getTinfoObject()) - .append(" SET OwnerObjectId = ? WHERE ObjectId = ?").addAll(newParentId, row.getObjectId()); + .append(" SET OwnerObjectId = ? WHERE ObjectId = ?").addAll(newParentId, row.objectId()); new SqlExecutor(ExperimentService.get().getSchema()).execute(update); reparented++; } @@ -961,16 +960,16 @@ private static void reparentUserDefaults() /** The owning domain, or null when the object's properties don't identify exactly one resolvable user-created domain. */ private static Domain resolveDomain(DefaultValueRow row) { - if (row.getDomainCount() != 1) + if (row.domainCount() != 1) { - LOG.warn("Leaving default values at {}: properties span {} domains, so the owner is ambiguous.", row.getObjectURI(), row.getDomainCount()); + LOG.warn("Leaving default values at {}: properties span {} domains, so the owner is ambiguous.", row.objectURI(), row.domainCount()); return null; } - Domain domain = PropertyService.get().getDomain(row.getDomainId()); + Domain domain = PropertyService.get().getDomain(row.domainId()); if (domain == null) { - LOG.warn("Leaving default values at {}: domain {} no longer exists.", row.getObjectURI(), row.getDomainId()); + LOG.warn("Leaving default values at {}: domain {} no longer exists.", row.objectURI(), row.domainId()); return null; } @@ -998,29 +997,14 @@ private static boolean renameObject(long objectId, String newUri) return new SqlExecutor(ExperimentService.get().getSchema()).execute(update) > 0; } - public static class DefaultValueRow + private sealed interface DefaultValueRow permits ContainerDefaultRow, UserDefaultRow { - private long _objectId; - private String _objectURI; - private long _parentId; - private String _parentURI; - private String _container; - private int _domainId; - private int _domainCount; - - public long getObjectId() { return _objectId; } - public void setObjectId(long objectId) { _objectId = objectId; } - public String getObjectURI() { return _objectURI; } - public void setObjectURI(String objectURI) { _objectURI = objectURI; } - public long getParentId() { return _parentId; } - public void setParentId(long parentId) { _parentId = parentId; } - public String getParentURI() { return _parentURI; } - public void setParentURI(String parentURI) { _parentURI = parentURI; } - public String getContainer() { return _container; } - public void setContainer(String container) { _container = container; } - public int getDomainId() { return _domainId; } - public void setDomainId(int domainId) { _domainId = domainId; } - public int getDomainCount() { return _domainCount; } - public void setDomainCount(int domainCount) { _domainCount = domainCount; } + String objectURI(); + int domainId(); + int domainCount(); } + + private record ContainerDefaultRow(long objectId, String objectURI, int domainId, int domainCount) implements DefaultValueRow {} + + private record UserDefaultRow(long objectId, String objectURI, long parentId, String parentURI, String container, int domainId, int domainCount) implements DefaultValueRow {} } diff --git a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java index 6ef6a0203b0..2464dd28d27 100644 --- a/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/PropertyServiceImpl.java @@ -758,15 +758,14 @@ private Map savedDefaultValuePropertyCounts(DbSchema schema, Str INNER JOIN exp.DomainDescriptor DD ON DD.DomainId = PDM.DomainId WHERE OP.ObjectId IN (SELECT ObjectId FROM exp.Object WHERE ObjectURI LIKE ?)""").add(lsidPattern); - for (Map row : new SqlSelector(schema, sql).getMapCollection()) - { + new SqlSelector(schema, sql).forEachMap(row -> { Integer propertyId = ((Number) row.get("PropertyId")).intValue(); Object defaultValueType = row.get("DefaultValueType"); Lsid domainLsid = new Lsid((String) row.get("DomainURI")); byDefaultValueType.computeIfAbsent(defaultValueType == null ? "null" : defaultValueType.toString(), k -> new HashSet<>()).add(propertyId); byDomainKind.computeIfAbsent(domainLsid.isValid() ? domainLsid.getNamespacePrefix() : "null", k -> new HashSet<>()).add(propertyId); - } + }); return Map.of("byDefaultValueType", countDistinct(byDefaultValueType), "byDomainKind", countDistinct(byDomainKind)); } diff --git a/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java b/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java index 685635fb438..5707b2c1515 100644 --- a/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java +++ b/experiment/src/org/labkey/experiment/defaults/DefaultValueServiceImpl.java @@ -17,6 +17,7 @@ import org.apache.commons.beanutils.ConversionException; import org.apache.logging.log4j.LogManager; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.labkey.api.data.Container; import org.labkey.api.data.DbScope; @@ -85,7 +86,7 @@ private String getUserDefaultsParentLSID(Container container, User user, Domain // GitHub Issue #1569: Qualifying LSID by domain kind since multiple data types have distinct domain kinds // Ex: assay designs have AssayDomain-Batch, AssayDomain-Run, AssayDomain-Result - private String qualifyByDomainKind(String lsidPrefix, Lsid domainLsid) + private String qualifyByDomainKind(@NotNull String lsidPrefix, @NotNull Lsid domainLsid) { return lsidPrefix + "-" + domainLsid.getNamespacePrefix(); }