If we are altering the original schema, the database will need to keep a
schema version. This helps us with debugging remote installations.

http://blog.cherouvim.com/a-table-that-should-exist-in-all-projects-with-a-database/

Please add this to your pull request.

On 9 July 2012 20:09, Denis Washington <
reply@reply.github.com

wrote:

Adds support for marking a Pub-Sub subscription as temporary (see
http://xmpp.org/extensions/xep-0060.html#impl-tempsub). Such
subscriptions are removed as soon as the client goes offline.

You can merge this Pull Request by running:

git pull https://github.com/denisw/buddycloud-server tempsub

Or you can view, comment on it, or merge it online at:

#64

-- Commit Summary --

• Implement temporary subscriptions

-- File Changes --

M postgres.sql (1)
M src/local/model_postgres.coffee (9)
M src/local/operations.coffee (10)
M src/router.coffee (5)
M src/xmpp/pubsub_server.coffee (31)

-- Patch Links --

https://github.com/buddycloud/buddycloud-server/pull/64.patch
https://github.com/buddycloud/buddycloud-server/pull/64.diff

---

Reply to this email directly or view it on GitHub:
#64

Simon Tennant | buddycloud.com | +49 17 8545 0880

Yes. We need some safe way to upgrade the DB schema.

Something like adding a ensureSchemaVersion (version) function called from dbIsAvailable (in src/local/model_postgres.coffee) that fails if no schema_version table is present or if the latest version is less than the required version.

At first it could just tell the user to run the upgrade scripts manually; maybe later we can make the upgrade automatic.

Other than that, your patch looks good! (Maybe just replace (err) => cb err with just cb? Not sure, not tested...)
Did you test it yet?

If you like the DB schema version to be part of the pull request, I'll try to add that to the pull request today.

@Schnouki I did test the changes; otherwise I would never come across the idea to make a pull request. :) Using some test code that subscribes to a node, makes the subscription temporary and logs out, I could verify with psql that the subscription was marked temporary and was deleted afterwards. (And, naturally, that only that subscription was deleted.)

@denisw Great :)

A few more issues and questions:

• temporary subscriptions to private channels should not be allowed (as discussed in the gsoc2012 channel)
• maybe temporary subscriptions should not be listed by getSubscriptions? Not sure about that, but I don't think we want temporary subscribers to show up in the webclient.
• what happens if a clients does a temporary subscription to a (public) channel, then decides to subscribe to it? Right now AFAICT it's not possible because setSubscription does not set temporary to false.
• when a temporary subscription to a remote channel expires, the remote server may need to be notified (if the temp. subscription was the only subscription from the local server). It does not happen with the current code.

(Don't hesitate to ask if you need help for any of these!)

From my testing, it seems that the webclient uses getAffiliations rather than getSubscriptions to compile the list of displayed followers, which operates on the affiliations table. Is it OK if I augment getAffiliations with an ugly sub-SELECT to filter based on subscriptions.temporary?

@Schnouki I fixed two of the issues you listed in your last comment, but have questions about the other two:

• Where should I check whether the channel is private? In the operation's transaction method? And which methods are preferred to return error stanzas to the client?
• What do you mean exactly with "the remote server may need to be notified"? As in, a stanza signalling that a user has unsubscribed?

Are you sure about getAffiliations? From src/views/channel/details/index.coffee in the webclient I'd say followers are determined by getSubscribers... @dodo to the rescue?
If so, we should filter them there (and in getSubscriptions). Or maybe in the Retrieve{Node,User}Subscriptions operations. Your call :)

Which raise another issue: remote servers must know which subscriptions are temporary so they can do the same filtering with their clients.

I think we should also only allow temporary subscriptions if both the subscription request and the pubsub#expire option are in the same stanza (XEP-0060 6.3.7). It would then be easier to deny these temporary subscriptions for private channels, and to notify federated servers.

Suddenly temporary subscriptions seem much more complicated to do :)

• You chan check if a channel is private using if @nodeConfig.accessModel is 'authorize' in privilegedTransaction (or call @fetchNodeConfig first in transaction).
• The remote server must be notified of the unsubscription with an appropriate stanza, probably with something similar to the notification method of the Unsubscribe operation.

@Schnouki I tied making a subscription temporary to the <subscribe/> IQ as you suggested. I would have done this before, but I didn't know that XEP-0060 allows combined <subscribe/> / <options> queries. Thanks for the pointer.

As to notifications: do remote servers need to know about temporary subscriptions? If not, it might be a good idea to avoid the notifications completely in this case.

Yes, remote servers do need to know about temporary subscriptions: they will only send PubSub notifications to servers they know about. If test.com does not know that a user from example.net has subscribed to whatever@test.com, it won't send PubSub notifications to example.net when something happens in the whatever@test.com channel. So at the very least they need to know about subscriptions.
Remote servers wouldn't necessarily need to know about temporary subscriptions. test.com will notify example.net even if the subscription from user@example.net to whatever@test.com is temporary. It's only needed if you want to be able to hide these subscriptions from clients.

The problem I see is that there doesn't seem to be any standard way of notifying Pub-Sub servers of subscription options. I guess if we want to federate the temporary status, we need to invent some new <subscription/> attribute or another element for this.

Right. We could just add a temporary attribute to <subscription/>:

<message
    from='pubsub.shakespeare.lit'
    to='horatio@denmark.lit'
    id='approvalnotify1'>
  <event xmlns='http://jabber.org/protocol/pubsub#event'>
    <subscription node='princely_musings' jid='horatio@denmark.lit' subscription='subscribed' temporary='1'/>
  </event>
</message>

We also probably need to add it to items in */subscriptions nodes:

<iq from="channeluser@example.com/ChannelCompatibleClient" to="channelserver.example.com" type="set" id="publish:20">
  <pubsub xmlns="http://jabber.org/protocol/pubsub">
     <publish node="/user/channeluser@example.com/subscriptions">
        <item id="koski@buddycloud.com">
          <query xmlns="http://jabber.org/protocol/disco#items" xmlns:pubsub="http://jabber.org/protocol/pubsub" xmlns:atom="http://www.w3.org/2005/Atom">
            <item jid="sandbox.buddycloud.com"
                  node="/user/koski@buddycloud.com/posts"
                  pubsub:affiliation="publisher"
                  pubsub:temporary="0">
              <atom:updated>2010-12-26T17:30:00Z</atom:updated>
            </item>
            ...
            <item jid="sandbox.buddycloud.com"
                  node="/user/koski@buddycloud.com/mood"
                  pubsub:affiliation="member"
                  pubsub:temporary="0"/>
          </query>
        </item>
     </publish>
  </pubsub>
</iq>

...but this would expose temporary subscriptions to clients (which should use this anyway).

@denisw I think this is becoming too complicated to do everything in a pull request :) I started a wiki page (on bc.org) so we can design it correctly: https://buddycloud.org/wiki/Buddycloud_server_WIP. Feel free to contribute there.

@Schnouki I agree with (and have implemented) most of what you have written on the wiki page, but still have some questions / concerns. I added a "Discussion" section to the page for this.

Implemented in e84c384 and following commits; will be merged soon. Thanks for your help, code and input!

Excellent work. Thanks @Schnouki