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/ExperimentUpgradeCode.java b/experiment/src/org/labkey/experiment/ExperimentUpgradeCode.java index e693224a7ec..47690b395a7 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,192 @@ 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"; + + /** + * 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") + public static void migrateDefaultValueLsids(ModuleContext context) + { + if (context.isNewInstall()) + return; + + try (Transaction tx = ExperimentService.get().ensureTransaction()) + { + migrateContainerDefaults(); + reparentUserDefaults(); + tx.commit(); + } + + OntologyManager.clearCaches(); + } + + 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(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 (ContainerDefaultRow row : rows) + { + Domain domain = resolveDomain(row); + if (domain == null) + continue; + + Lsid domainLsid = new Lsid(domain.getTypeURI()); + if (objectIdForm != new Lsid(row.objectURI()).getObjectId().equals(domainLsid.getObjectId())) + continue; + + String newUri = qualifiedLsid(row.objectURI(), domainLsid); + if (newUri == null || newUri.equals(row.objectURI())) + continue; + + if (renameObject(row.objectId(), newUri)) + migrated++; + else + LOG.warn("Leaving default values at {}: {} already exists.", row.objectURI(), 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(UserDefaultRow.class); + + // children of each legacy parent, bucketed by the qualified parent they now belong under + Map>> byLegacyParent = new LinkedHashMap<>(); + for (UserDefaultRow row : rows) + { + Domain domain = resolveDomain(row); + if (domain == null) + continue; + + String newParentUri = qualifiedLsid(row.parentURI(), new Lsid(domain.getTypeURI())); + if (newParentUri == null || newParentUri.equals(row.parentURI())) + continue; + + byLegacyParent.computeIfAbsent(row.parentId(), 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).container()); + if (container == null) + continue; + + long newParentId = OntologyManager.ensureObject(container, group.getKey()); + for (UserDefaultRow row : group.getValue()) + { + SQLFragment update = new SQLFragment("UPDATE ").append(OntologyManager.getTinfoObject()) + .append(" SET OwnerObjectId = ? WHERE ObjectId = ?").addAll(newParentId, row.objectId()); + 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.domainCount() != 1) + { + 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.domainId()); + if (domain == null) + { + LOG.warn("Leaving default values at {}: domain {} no longer exists.", row.objectURI(), row.domainId()); + 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; + } + + private sealed interface DefaultValueRow permits ContainerDefaultRow, UserDefaultRow + { + 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 0ea06467b31..2464dd28d27 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,50 @@ 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); + + 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)); + } + + 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 aabf1fb6965..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; @@ -52,7 +53,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"); @@ -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(@NotNull String lsidPrefix, @NotNull 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'.