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

Queueing messages, instead of saving only the most recent one #19

Closed
jpmhouston opened this issue Mar 10, 2015 · 11 comments
Closed

Queueing messages, instead of saving only the most recent one #19

jpmhouston opened this issue Mar 10, 2015 · 11 comments

Comments

@jpmhouston
Copy link

Any plans to support storing the sequence of messages in the order they were posted, so if an extension posts many of them while the app is away then they all can be received on the next launch?

If you have no plans to do this, then do you have any suggestions for me while implementing that myself in a way that you'd accept in a pull request? Should I add separate methods queueMessageObject:identifier: queuedMessageWithIdentifier:? Or, what I'm preferring, should there be boolean property that can be turned on for a MWWormhole which changes the semantics of the existing methods?

Regarding those semantics, presumably messageWithIdentifier: would remove the oldest message passed to the wormhole and return it, or return nil if none, so that it can be called repeatedly to get all of them.

The listener block would be called upon notification as before, passing in the message, which gets automatically removed from the queue. But if the other side hasn't pulled out all the preceding messages yet, then this message will be received out of order. I'm thinking that the listener should have the option of making explicit calls to messageWithIdentifier: to catch any that its missed, where the passed-in object would presumably be at the end of those results. (I'm considering the case where the app polls for stored messages on launch then sets up the listener afterwards, and how to ensure any messages sent during that gap are received in order). Its seems that the notification code that calls the listener needs to initially leave its message in the queue, then be wary of it possibly getting removed "behind its back" once the listener returns.

At the moment, I'm not sure what would be expected if once side turns on queueing and the other doesn't, but I'm sure some reasonable behavior can be chosen.

Lastly, I was originally wanting to add support for this just by subclassing MWWormhole, but I believe there's no way to make that work well. If you think I'm wrong about that please let me know.

@cnstoll
Copy link
Contributor

cnstoll commented Mar 11, 2015

I can definitely see the use for something like this, but I'll be honest...I'm skeptical of adding it to the core library. Not opposed, just skeptical.

The first question I'd like to have answered is: how would you envision storing the enqueued messages in a specific order on the file system inside the app group? One of the things I love about MMWormhole is how simple the scheme is. Just a single file, with a specific name, written and read using NSKeyedArchiver. Building a queueing system makes that a lot more complex.

If we do decide to add something like this, I would actually want it to be a completely separate interface and NOT use the main messageWithIdentifier/listenerBlock interface. One of the reasons is that there can be issues if you are updating the UI on the watch app from a listener block and that ends up being called multiple times in succession. That may be unexpected behavior for some people and cause them problems.

Subclassing actually seems like the best way to go about this...if we can find a way to do it. Let me think about that a little bit. What are your thoughts on storage?

@jpmhouston
Copy link
Author

Instead of a file with name equal to the identifier, I was thinking about a directory with the name of the identifier, with numbered files inside. Passing a message involves first getting the list of files within the directory and scanning it to find the largest number, incrementing that and writing file with that name. Receiving a message is similar, except it finds the lowest number and reads that file.

Implementing this in a subclass probably involves exposing some MMWormhole internals, but that could be done in a MMWormhole-private.h header and leave MMWormhole.h as-is.

I was trying to say that messageWithIdentifier/listenerBlock would work as normal by default, but after setting a queueMessages property to YES then it would change their behaviour. But I think I'd agree that a subclass would be preferable.

@patzearfoss
Copy link

I've been following this thread and thought the idea was overkill until literally today when I found I could use something very much like this. I can get by using a series of file to do what I need since the ordering isn't critical, but I'd be interested to see some kind of implementation of this as a subclass or however everyone sees fit.

@jpmhouston
Copy link
Author

Pushed my untested fork to github https://github.com/jpmhouston/MMWormhole in case anyone is interested. I'm probably not going to work on this much in the next little while, since I'm no longer going to be using this in my app.

Anyone who works on this to test it and contribute any fixes, please contact me jpmhouston@gmail.com and I'll give you write access, or feel free to cherry-pick a patch of my commit into your own fork. (Please don't do a fork of this fork)

@cnstoll
Copy link
Contributor

cnstoll commented Mar 12, 2015

I like the idea with a folder named for the identifier with numbered messages inside of it.

Another question I have is what would the expected behavior be for clearing the queue? Would accessing and reading the messages from the queue automatically clear them, or would you be expected to call clearMessageForIdentifier to clear the queue?

@jpmhouston
Copy link
Author

The way I was writing that subclass, reading the messages would clear them. Also, following a call to the listener all preceding messages are cleared too, otherwise I felt it was too easy to write code that inadvertently handled messages out of order. See my additions to the readme for an example.

That's what I intended anyway. I haven't yet gotten a chance to run any tests on it at all.

@burin
Copy link

burin commented Mar 25, 2015

@jpmhouston I looked at your fork and the readme addition that you added, and had some thoughts.

While you and @cnstoll were discussing the usage of it, I was still thinking about the mailbox analogy, where you can deliver multiple things to a particular mailbox and pick up things from the mailbox.

Sending:

In real life, you can keep sending stuff to a mailbox. You don't know if it's been received or not. I think your API for passMessageObject to the same identifier matches my expectations. No changes needed IMO.

Retrieving stuff already there:

This is where I think the API could be improved.

The proposed way is to keep checking until it's empty (messageWithIdentifier over and over). And every time you check, it's a new thing inside of it, until it's empty. If you think about that scenario happening in real life, it's kinda weird IMO.

If you think about real life, you go to the mailbox, and you just get whatever is in it. There could be a stack of different things in there. You check once, and you take everything out.

Maybe messageWithIdentifier should be something like messageCollectionWithIdentifier. And it's an array that you can loop through, and maybe it's stamped with when it was sent.

Listening for messages

In the listen scenario, going back to "real life" again: you can sit outside of the mailbox and wait for the mail man to come with packages. It has nothing to do with the stuff that's already in the mailbox. Maybe it should be that way for this too? Another question is whether it goes into the mailbox or if you "intercept it", and it doesn't go in there.

All together

[self.queuedWormhole passMessageObject:@(1) identifier:@"test"];
[self.queuedWormhole passMessageObject:@(2) identifier:@"test"];
[self.queuedWormhole listenForMessageWithIdentifier:@"test" listener:^(id messageObject) { NSLog(@"%@", messageObject); }];
[self.queuedWormhole passMessageObject:@(3) identifier:@"test"]; // lister logs: 3, nothing happened to the messages already there. maybe it logs [1,2,3] ?
messageObject = [self.queuedWormhole messageCollectionWithIdentifier:@"test"]; // [1, 2], or [1, 2, 3] ?
[self.queuedWormhole stopListeningForMessageWithIdentifier:@"test"];

FYI: I'm thinking about a solution like this for analytics. Tracking the activity in extensions and sticking it in a queue, and then sending the data when the iPhone app starts up.

@jpmhouston
Copy link
Author

Somewhat finished my fork (after all). Tested with XCTest cases I wrote, but not with either example extension yet. @burin nice idea, to be able to receive all the queued messages in one go, however I didn't see your post soon enough to influence what I did.

I simplified my plan so that the listener block doesn't have to iterate over manual calls to messageWithIdentifier, instead MMQueuedWormhole just calls the block repeatedly for each undelivered message up to the one from the notification. I think this fits best with @cnstoll's original API.

[self.queuedWormhole passMessageObject:@(1) identifier:@"test"];
[self.queuedWormhole passMessageObject:@(2) identifier:@"test"];
[self.queuedWormhole listenForMessageWithIdentifier:@"test" listener:^(id messageObject) {...}]; // called with message 1, then 2
[self.queuedWormhole passMessageObject:@(3) identifier:@"test"]; // lister block called with message 3
id messageObject = [self.queuedWormhole messageWithIdentifier:@"test"]; // nil
[self.queuedWormhole stopListeningForMessageWithIdentifier:@"test"];

When originally using this with an actual extension, I saw how an outstanding Darwin Notification gets delivered to the callback upon my (main) app returning to the foreground. I haven't yet seen anything documenting the exact behavior - whether this is guaranteed, and perhaps only the most recent notification is delivered. The scheme I settled on for the listener should work best if those are both true.

Note that if the most recent notification isn't guaranteed to be delivered when the app returns to the foreground, then there's nothing to make the listener fire, and it won't catch up until the next message is passed. If someone who is actively working on an extension would like to test this, I'd appreciate it.

@cnstoll - sorry for commondeering your project's issues for discussions of my fork. Let me know if you think this effort should end in a pull request, or should instead be packaged as its own pod that has a dependency on yours, or what. Please feel free to close this issue and direct any further conversations to https://github.com/jpmhouston/MMWormhole/issues.

@kellytk
Copy link

kellytk commented Apr 10, 2015

I found this discussion after I had already begun implementing my own solution, which uses MMWormhole. (KTKPinhole) I'm leaving this note for future reference to anyone that needs message queueing and order preservation.

Thank you for making MMWormhole.

@jpmhouston
Copy link
Author

@burin I indeed see the benefits of returning all queued messages in an array. For example, if the messages affect the app's UI, then it's then easier to batch apply all the messages and update the UI only once.

I added to MMQueuedWormhole the methods messagesWithIdentifier:, listenForMessagesWithIdentifier:listener:, and stopListeningForMessagesWithIdentifier: that do this. Note plural "messages" instead of "message" (is this too subtle? I might still change).

Also, tested somewhat and I now believe its ready for use. I created release 1.2-jpmh-beta.1 in my fork https://github.com/jpmhouston/MMWormhole. Any issues with it should be posted there instead of here.

I expect this to be my last post to this issue. @cnstoll if you are happy with this state of affairs, with the feature left to be implemented by my fork (plus KTKPinhole by @kellytk), then I'd suggest closing it.

@cnstoll
Copy link
Contributor

cnstoll commented May 27, 2015

I'm going to go ahead and close this thread. I really appreciate you guys taking the time to create a subclass that implements this behavior.

I wanted to also let you know that I started a PR for adding support for first class subclassing of MMWormhole's message passing implementation that I believe could be used for implementing this a little more cleanly in the future. If you have any thoughts or want to comment on that, the link is here below.

#40

Thanks,

  • Conrad

@cnstoll cnstoll closed this as completed May 27, 2015
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

No branches or pull requests

5 participants