Conversation
- 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
… 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.
| 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<String, Object> row : new SqlSelector(schema, sql).getMapCollection()) |
There was a problem hiding this comment.
Could consider using a stream of results here instead.
There was a problem hiding this comment.
I don't expect this to return a very large set of results so I didn't initially go with the stream approach. Seems fine to make the suggested change though. Done.
|
|
||
| // 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) |
There was a problem hiding this comment.
@NotNull for these parameters?
There was a problem hiding this comment.
I wasn't going to add those annotations given this was a private method only in this class, but it doesn't hurt. Done.
| * holds, which is what disambiguates rows that several domains once shared. | ||
| */ | ||
| @SuppressWarnings("unused") | ||
| @DeferredUpgrade |
There was a problem hiding this comment.
Assuming you verified that this needs to be deferred (to resolve domains?)
There was a problem hiding this comment.
Yes, DeferredUpgrade because of resolving domains. However technically since we decided that this upgrade code isn't going to run via a SQL upgrade script as part of 26.3 (and only run on demand by the site admin), I'll remove this for now and it will get added back when we make the develop PR for the SQL upgrade script to call this code.
| return new SqlExecutor(ExperimentService.get().getSchema()).execute(update) > 0; | ||
| } | ||
|
|
||
| public static class DefaultValueRow |
There was a problem hiding this comment.
Would a Record have worked in this place.
There was a problem hiding this comment.
not a single record since DefaultValueRow is used by two queries that have slightly different shaped row responses. But it can be two records that share some implementation. Done.
…ExperimentUpgradeCode,
3 participants
Rationale
https://github.com/LabKey/internal-issues/issues/1569
Default-value objects are keyed by an LSID built from the container plus the domain typeURI's objectId. Every domain of one assay design shares that objectId (the assay name), so Batch/Run/Result/Sample defaults all collapsed onto a single exp.Object, and saving one domain's defaults destroyed the others. The change folds the domain typeURI's namespace prefix (AssayDomain-Run, AssayDomain-SampleWellGroup, …) into the default-value LSID's namespace prefix to make the key unique per domain, adds a deferred upgrade to re-key existing folder-level objects and per-user parent objects onto the new form, and fixes the user-defaults wildcard LSID that previously matched nothing for scoped objects.
Related Pull Requests
Changes