Last sync support for Indexed Fixtures #842
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
shubham1g5
added
bug
ready for review
product/internal-or-flagged
enhancement
and removed
bug
labels
ctsims
requested changes
Mar 22, 2019
| @Override | ||
| public AbstractTreeElement getAttribute(String namespace, String name) { | ||
| TreeElement attr = attrHolder.getAttribute(namespace, name); | ||
| attr.setParent(this); |
Member
There was a problem hiding this comment.
Can you confirm that this method has no side effects that affect the current object (this)?
Contributor
Author
There was a problem hiding this comment.
The only affect is that invalidates the reference cache for this. Is this something to be worried about ?
Member
There was a problem hiding this comment.
No, invalidating the reference cache is fine, I just wanted to make sure that this wouldn't result in a "Backlink" being created where "this" (the treelement part of the object) now has an independent reference to attr
Contributor
Author
There was a problem hiding this comment.
got it. Doesn't look like that's happening.
src/main/java/org/commcare/cases/instance/StorageBackedChildElement.java
Outdated
Show resolved
Hide resolved
src/main/java/org/commcare/modern/database/IndexedFixturePathsConstants.java
Show resolved
Hide resolved
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.
Refactoring to support root atrributes lookup for Android indexed fixture.
IndexedFixtureIdentifierto hold basic/meta properties for an indexed fixture.IndexedFixtureInstanceTreeElementto have hold attributes inTreeElement attrHolderT getAttribute(String namespace, String name)toAbstractTreeElement getAttribute(String namespace, String name)to be able to returnlast_syncas an instance ofAbstractTreeElementrather thanIndexedFixtureChildElement. It's possible that it's not the right soultion here but I could not think of any alternates.cross-request: dimagi/commcare-android#2085