Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 8
Issue 52824: Don't mutate domains from the cache#6690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
e2897260d89e53fd3dea8e96be00e9cc42fa5e4c3d8ef82c9b2044c5d456974afdab48924a64448678b568b161800faafed3d31b31b03d594d419063519d58210a8eaa6aca2517f1fa09342e55bb2415ad4c11b42259fceFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -857,6 +857,15 @@ public Domain getDomain() | ||
| return null; | ||
| } | ||
| @Nullable | ||
| @Override | ||
| public Domain getDomain(boolean forUpdate) | ||
| { | ||
| if (forUpdate) | ||
| throw new UnsupportedOperationException("Cannot get domain for update."); | ||
| return getDomain(); | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this should throw ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. | ||
| } | ||
| @Nullable | ||
| @Override | ||
| public DomainKind getDomainKind() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -2441,25 +2441,55 @@ public DomainDescriptor load(@NotNull Integer key, @Nullable Object argument) | ||
| } | ||
| } | ||
| public static DomainDescriptor getDomainDescriptor(int id) | ||
| { | ||
| return getDomainDescriptor(id, false); | ||
| } | ||
| public static DomainDescriptor getDomainDescriptor(int id, boolean forUpdate) | ||
| { | ||
| if (forUpdate) | ||
labkey-susanh marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| return new DomainDescriptorLoader().load(id, null); | ||
| return DOMAIN_DESC_BY_ID_CACHE.get(id); | ||
| } | ||
| @Nullable | ||
| public static DomainDescriptor getDomainDescriptor(String domainURI, Container c) | ||
| { | ||
| return getDomainDescriptor(domainURI, c, false); | ||
| } | ||
| @Nullable | ||
| public static DomainDescriptor getDomainDescriptor(String domainURI, Container c, boolean forUpdate) | ||
| { | ||
| if (c == null) | ||
| return null; | ||
| if (forUpdate) | ||
| return getDomainDescriptorForUpdate(domainURI, c); | ||
| // cache lookup by project. if not found at project level, check to see if global | ||
| DomainDescriptor dd = DOMAIN_DESCRIPTORS_BY_URI_CACHE.get(getCacheKey(domainURI, c)); | ||
| Pair<String, GUID> key = getCacheKey(domainURI, c); | ||
| DomainDescriptor dd = DOMAIN_DESCRIPTORS_BY_URI_CACHE.get(key); | ||
| if (null != dd) | ||
| return dd; | ||
| // Try in the /Shared container too | ||
| return DOMAIN_DESCRIPTORS_BY_URI_CACHE.get(getCacheKey(domainURI, _sharedContainer)); | ||
| key = getCacheKey(domainURI, _sharedContainer); | ||
| return DOMAIN_DESCRIPTORS_BY_URI_CACHE.get(key); | ||
| } | ||
| @Nullable | ||
| private static DomainDescriptor getDomainDescriptorForUpdate(String domainURI, Container c) | ||
| { | ||
| if (c == null) | ||
| return null; | ||
| DomainDescriptor dd = fetchDomainDescriptorFromDB(domainURI, c); | ||
| if (dd == null) | ||
| dd = fetchDomainDescriptorFromDB(domainURI, _sharedContainer); | ||
| return dd; | ||
| } | ||
| /** | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth a comment to say the default
forUpdateis false?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done