remove subscriptions sync if timeout is zero, to preserve add/remove call order

[alexcorre]

We have a few features that require unsub messages to be sent over the wire before sub message to the same subscription, with different parameters. Even with a timeout of zero, in the current code on master branch, the unsub would go after the sub.

This is caused by the following:

METSubscription *subscription1 = [_subscriptionManager addSubscriptionWithName:@"todos" parameters:@[@"bla"] completionHandler:nil];

// later ...

[_subscriptionManager removeSubscription:subscription1];
METSubscription *subscription2 = [_subscriptionManager addSubscriptionWithName:@"todos" parameters:@[@"XXXXXXXX"] completionHandler:nil];

if the METSubscriptionManager was configured with a default timeout of 0, or the METSubscription had a timeout configured of zero, the call to removeSubscription would actually cause the remove to happen after [subscription.reuseTimer startWithTimeInterval:subscription.notInUseTimeout];. Even with timeout 0 the sub for subscription 2 would happen first.

[alexcorre]

@martijnwalraven what do you think?

[martijnwalraven]

I remember considering it a feature not to unsubscribe immediately even with a timeout of 0, because other parts of the app might add the same subscription within the current run loop cycle and it would be a waste to unnecessarily unsubscribe and subscribe again.

I'm ok with making this change because you can always set the notInUseTimeout > 0 to get reuse behavior back. But just out of curiosity, why do you require an unsub to be sent before the new sub message?

[alexcorre]

It may seem unusual but our search feature is built reactively. This is done in such a way that we use temporary collections of data to hold results to subscriptions. We are almost emulating a method call with a subscription, so that we can react to changes after the call is made. We are using subscriptions in a fairly non traditional way, but i think it is pushing the limits of what can be achieved, with less code running client side for complicated things like search.

In the UI the a complicated search query can be created, with searchTerm, location w/ distance sort, sort by , and various filters. The client the subscribes to pages of results:

Meteor.subscribe('jobSearch', <SOME CRAZY QUERY HERE>, <limit/offset stuff here>);

Search results are then published to a collection called jobSearchResults

{"msg":"added","collection":"jobSearchResults","id":"1","fields":{"createdAt...
{"msg":"added","collection":"jobSearchResults","id":"2","fields":{"createdAt...
{"msg":"added","collection":"jobSearchResults","id":"3","fields":{"createdAt...
{"msg":"added","collection":"jobSearchResults","id":"4","fields":{"createdAt...

At any given time, the jobSearchResults array must contain ONLY results of the current search query. We use this collection to allow the client to reactively see changes to their results without having to support all filters locally by observing changes to that collection as a whole. If a new job was added, within your location vicinity, or unpublished, ddp will inform us, but removing the job from the jobSearchResults collection.

When the query changes, we unsubscribe to all pages of the previous query, and wait for ready message of the first page of the new query to display the results, now conveniently located in the same collection, jobSearchResults. This allows us to build a reactive search, without supporting complex queries in our local store. Leaves the client, significantly dumber than the server, which is what we are currently going for.

Before my change, if the search results for the new query were added before, the old one was removed, the user would see the results from both searches simultaneously for a short period of time.

[alexcorre]

There is probably a way to support both what you want, reuse if resubscribed during the same run loop, and immediate unsub as we desire.

Ideally, there needs to be a lower level subscriptions API that is not limited by the logic of METSubscriptionManager so we can achieve more non-traditional features like ours. ObjectiveDDP provides only this lower level API and intelligent management of subs is limited. Perhaps your library is not intended for more custom uses like ours? How do you envision it being used?

Another possibility is there can be a separate flag besides the notInUseTimeout property to determine if a particular subscription should be unsub synchronously.

Any ideas on this stuff?

[martijnwalraven]

Ah, no I understand where this is coming from. The search subscription seems like a pretty neat solution for live results updates. Another way to avoid mixing up results might be to include some kind of queryId in jobSearchResults documents so you can filter them on the client. That wouldn't require an immediate unsub and would also make it possible to run multiple concurrent live queries. That would require changes on the server though.

I'm a bit preoccupied with another project right now, so I haven't thought this through, but it seems what you're looking for is a way to add a subscription that is never shared and reused (and so can be unsubscribed immediately). I'm not sure about a lower level API because METSubscriptionManager is also responsible for reviving subscriptions after reconnect. But perhaps an addSubscriptionWithName:@"bla" reusable:NO would make sense?