Issue 52824: Don't mutate domains from the cache #353
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e types more similar
labkey-jeckels
approved these changes
May 29, 2025
labkey-jeckels
requested changes
May 29, 2025
Comment on lines
204
to
207
| DomainProperty objectId1 = d1.getDomain().addProperty(); | ||
| DomainProperty objectId1 = d1.getDomain(true).addProperty(); | ||
| objectId1.setName("objectId"); | ||
| objectId1.setPropertyURI(AuditSummaryUserSchema.class.getName() + ":ObjectId"); | ||
| d1.getDomain().save(TestContext.get().getUser()); | ||
| d1.getDomain(true).save(TestContext.get().getUser()); |
There was a problem hiding this comment.
Actually, this looks incorrect. Won't the two calls to getDomain(true) return two different objects now? This should hold onto the Domain as a local variable and reuse it.
labkey-jeckels
approved these changes
May 30, 2025
bbimber
pushed a commit
that referenced
this pull request
Nov 5, 2025
…r to delete VCFTabixAdapter (#353) * Add subadapter + reflection pattern to ExtendedVariantAdapter in order to delete VCFTabixAdapter * Add API wrapper for getMetadata and getHeader --------- Co-authored-by: Sebastian Benjamin <sebastiancbenjamin@gmail.com>
bbimber
added a commit
that referenced
this pull request
Nov 25, 2025
* Update view sort * Add wrapper for IdentifyAndStoreActiveClonotypes * Error checking for CommonFilters * Update defaults * Set baseUrl in script * Do not storeStimLevelData for TRA * Do not force seurat object name to match readset * Do not force seurat object name to match readset * Drop CS-Core and Tricycle * Expand data cleanup * Minor code cleanup * Add subadapter + reflection pattern to ExtendedVariantAdapter in order to delete VCFTabixAdapter (#353) * Add subadapter + reflection pattern to ExtendedVariantAdapter in order to delete VCFTabixAdapter * Add API wrapper for getMetadata and getHeader --------- Co-authored-by: Sebastian Benjamin <sebastiancbenjamin@gmail.com> * Correct HTML syntax * Expand study triggers and update cohort fields * Expose getter for StudiesTriggerFactory * Clean up trigger/customizer layer code * Create fields to coalesce name/label for studies * Add null check * Switch default * Bugfix to StudiesTriggerFactory * Update default * Improve SnpEff index check * Switch sequence init tasks to use webserver-high-priority * Build short delay into github triggers to aid cross-repo commits * Switch ETLs to log row count discrepancies * Update dependencies * Support sawfish --sample-csv arg * Option to create readsets from SRA (#355) * Option to create readsets from SRA * Test fix * Error check * Add nimble/bulk step * Allow nimble step to use cached barcodes * Bugfix to NimbleAlignmentStep * Bugfix to NimbleAlignmentStep * Bugfix to NimbleAlignmentStep * Bugfix to NimbleAlignmentStep * Add CD4_Activation_Axis * Better support readsets created directly from SRA * Expand BAM header * Bugfix to RestoreSraDataHandler * Bugfix to RestoreSraDataHandler for new SRA datasets * Bugfix to RestoreSraDataHandler for new SRA datasets * Bugfix to RestoreSraDataHandler for new SRA datasets * Bugfix to RestoreSraDataHandler for new SRA datasets * Reduce logging * Update sawfish install * Better error handling * Updates to Save10xBarcodes * Expand StudyMetadata cohorts * Throw exception when existing file present * Improve resume for ReadsetInitTask * Improve resume for ReadsetInitTask * Bugfix to Save10xBarcodes * Bugfix to handling of 10x barcodes * Switch nimble/CR barcodes to CB alone * Omit writing to 10x barcodes * Add another coalesce() term in case name and label are blank * Bugfix to study import * Fix typo * Add manage freezer button and expand TagPcrSummaryStep * Change the pattern of nimble fastq-to-bam * Add check for integers * Expand ETL/presentation of SIV data * Expand ETL/presentation of SIV data * Int -> long * npm dependency updates * Build fix --------- Co-authored-by: hextraza <hex@hextraza.moe> Co-authored-by: Sebastian Benjamin <sebastiancbenjamin@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rationale
Issue 52824
See related PR.
Related Pull Requests
Changes
forUpdateparameter in various calls to get domains.