Skip to content
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

Multiple Message Passing Implementation #100

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

msmollin
Copy link

Hello folks! Seems like this library is mostly unmaintained but I figured I'd open this PR for posterity sake incase anyone else needs this.

Issue

The default MMWormholeFileTransiting and its subclass MMWormholeCoordinatedFileTransiting both have an inherent issue where they support enqueuing multiple messages across the wormhole, BUT every time a message is enqueued, it overwrites the message payload. So, when attempting to transit N message payloads across the wormhole (similar to how NSNotificationCenter or any other message bus construct works), you get N messages but only the latest written payload.

Solution

This doesn't work for my use case unfortunately, so thus I present MMWormholeManifestFileTransiting for your inspection and approval. It utilizes the more modern NSFileCoordinator-based approach from MMWormholeCoordinatedFileTransiting but instead of just writing the message to disk and thus overwriting whatever is currently there, it manages a stack (NSArray) of messages that need to be sent across the wormhole which are then dispatched FILO style across the wormhole via pushing and popping the stack.

There's a few potential improvements / optimizations to be made here:

  • Implement a FIFO queue management in addition to the existing FILO system. For my use case this would change nothing but I could see it being useful if ordering mattered if delivery is delayed (as my use case of sending data from an ActionExtension is)
  • Cleanup synchronization between messages on disk and notifications sent to the app. There's potential for drift here if for some reason the CF notification fails to deliver, as then we'll have messages enqueued in the array with no matching notification. The framework could expect the calling code to handle this case as well if that's an expected problem, which in my testing hasn't happened.


#pragma mark - MMWormholeTransiting Methods

- (BOOL)writeMessageObject:(id<NSCoding>)messageObject forIdentifier:(NSString *)identifier {
Copy link

Choose a reason for hiding this comment

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

Would you want to pass in an error pointer to find out how it might have failed?

} else {
storageArray = [[NSMutableArray alloc] init];
}
[storageArray addObject:messageObject];
Copy link

Choose a reason for hiding this comment

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

Is there a concern that we are storing a reference to the messageObject and someone might later mutate the message? Do we want to prefer a copy of the object here?

if ([storageArray count] == 0) {
return nil;
}
id messageObject = storageArray[0];
Copy link

Choose a reason for hiding this comment

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

Seems curious that we are making the assumption the array returned for this identifier is always a single item collection. Why are we writing and reading arrays from the file system and not the message object itself?

NSError *error = nil;
__block NSData *data = nil;

[fileCoordinator
Copy link

Choose a reason for hiding this comment

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

Seems expensive to read from disk for every call, though I'd probably just make a note of the consideration until it's a measurable issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants