Skip to content
This repository was archived by the owner on Feb 2, 2023. It is now read-only.

Ensure data consistency between ASDataController and its delegate while executing update transactions #688

Merged
merged 2 commits into from
Sep 28, 2015
Merged

Ensure data consistency between ASDataController and its delegate while executing update transactions #688

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e619edc
Ensure data consistency between ASDataController and its delegate whi…
nguyenhuy Sep 27, 2015
a94cd9d
Remove externalCompletedNodes getter in ASDataController.
nguyenhuy Sep 27, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 20 additions & 8 deletions AsyncDisplayKit/Details/ASDataController.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@
static void *kASSizingQueueContext = &kASSizingQueueContext;

@interface ASDataController () {
NSMutableArray *_completedNodes; // Main thread only. External data access can immediately query this.
NSMutableArray *_externalCompletedNodes; // Main thread only. External data access can immediately query this if available.
NSMutableArray *_completedNodes; // Main thread only. External data access can immediately query this if _externalCompletedNodes is unavailable.
NSMutableArray *_editingNodes; // Modified on _editingTransactionQueue only. Updates propogated to _completedNodes.

NSMutableArray *_pendingEditCommandBlocks; // To be run on the main thread. Handles begin/endUpdates tracking.
Expand Down Expand Up @@ -347,6 +348,10 @@ - (void)endUpdatesAnimated:(BOOL)animated completion:(void (^)(BOOL))completion

[_editingTransactionQueue addOperationWithBlock:^{
ASDisplayNodePerformBlockOnMainThread(^{
// Deep copy _completedNodes to _externalCompletedNodes.
// Any external queries from now on will be done on _externalCompletedNodes, to guarantee data consistency with the delegate.
_externalCompletedNodes = (NSMutableArray *)ASMultidimensionalArrayDeepMutableCopy(_completedNodes);

LOG(@"endUpdatesWithCompletion - begin updates call to delegate");
[_delegate dataControllerBeginUpdates:self];
});
Expand All @@ -363,6 +368,9 @@ - (void)endUpdatesAnimated:(BOOL)animated completion:(void (^)(BOOL))completion

[_editingTransactionQueue addOperationWithBlock:^{
ASDisplayNodePerformBlockOnMainThread(^{
// Now that the transaction is done, _completedNodes can be accessed externally again.
_externalCompletedNodes = nil;

LOG(@"endUpdatesWithCompletion - calling delegate end");
[_delegate dataController:self endUpdatesAnimated:animated completion:completion];
});
Expand Down Expand Up @@ -617,28 +625,31 @@ - (void)moveRowAtIndexPath:(NSIndexPath *)indexPath toIndexPath:(NSIndexPath *)n
- (NSUInteger)numberOfSections
{
ASDisplayNodeAssertMainThread();
return [_completedNodes count];
return [[self completedNodes] count];
}

- (NSUInteger)numberOfRowsInSection:(NSUInteger)section
{
ASDisplayNodeAssertMainThread();
return [_completedNodes[section] count];
return [[self completedNodes][section] count];
}

- (ASCellNode *)nodeAtIndexPath:(NSIndexPath *)indexPath
{
ASDisplayNodeAssertMainThread();
return _completedNodes[indexPath.section][indexPath.row];
return [self completedNodes][indexPath.section][indexPath.row];
}

- (NSIndexPath *)indexPathForNode:(ASCellNode *)cellNode;
{
ASDisplayNodeAssertMainThread();

NSArray *nodes = [self completedNodes];
NSUInteger numberOfNodes = nodes.count;

// Loop through each section to look for the cellNode
for (NSUInteger i = 0; i < [_completedNodes count]; i++) {
NSArray *sectionNodes = _completedNodes[i];
for (NSUInteger i = 0; i < numberOfNodes; i++) {
NSArray *sectionNodes = nodes[i];
NSUInteger cellIndex = [sectionNodes indexOfObjectIdenticalTo:cellNode];
if (cellIndex != NSNotFound) {
return [NSIndexPath indexPathForRow:cellIndex inSection:i];
Expand All @@ -651,13 +662,14 @@ - (NSIndexPath *)indexPathForNode:(ASCellNode *)cellNode;
- (NSArray *)nodesAtIndexPaths:(NSArray *)indexPaths
{
ASDisplayNodeAssertMainThread();
return ASFindElementsInMultidimensionalArrayAtIndexPaths(_completedNodes, [indexPaths sortedArrayUsingSelector:@selector(compare:)]);
return ASFindElementsInMultidimensionalArrayAtIndexPaths((NSMutableArray *)[self completedNodes], [indexPaths sortedArrayUsingSelector:@selector(compare:)]);
}

/// Returns nodes that can be queried externally. _externalCompletedNodes is used if available, _completedNodes otherwise.
- (NSArray *)completedNodes
{
ASDisplayNodeAssertMainThread();
return _completedNodes;
return _externalCompletedNodes != nil ? _externalCompletedNodes : _completedNodes;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the ASDisplayNodeAssertMainThread() to this method.

I'm wondering if the existing accessor -completedNodes makes sense. I know it is API exposed on ASDataController, I think used by ASRangeController. Right now the names between these don't quite sit right with me, particularly because the external one is mutable, and also is not /actually/ externally exposed at all (at least the method name).

Could we just delete one of these methods, and rename externalCompletedNodes to completedNodes? Not sure if having the header show NSArray as the return type will work seamlessly if the implementation actually returns NSMutableArray.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the main thing is -nodesAtIndexPaths: needs a mutable array (here). Maybe casting it is fine?

Not sure if having the header show NSArray as the return type will work seamlessly if the implementation actually returns NSMutableArray.

I either get a NSArray (and still need to cast), or a "duplicate declaration of method completedNodes" error :(

}

#pragma mark - Dealloc
Expand Down