Adding real-time updates / subscriptions

[jjmaestro]

Hi there!

I have started working on real-time updates / subscriptions in a feature branch: feature/realtime-subscriptions.

So far, it involves a small refactor of GraphAPI into a Base class so that we can segregate functionality into its own container class (FQL, Subscriptions and any other future FB Graph functionality).

I would love if you could have a look and discuss how you see it. If you think everything looks OK, I will add more documentation (and re-write the current doc to accomodate the small changes) and send a pull request when everything is clean an ready.

So far, I added tests for everything (still missing some small branch coverage, I think) and method documentation.

Any ideas / improvements are welcomed :) I will start using this in a little prototype we (@TwoApart) are working on, so hopefully it will be used in real life soon :)

Cheers,

[jgorset]

Hey @jjmaestro,

this is really great! I've made some comments in the codebase that we might consider, but overall I'm really happy about your implementation.

My only concern is that this is perhaps bordering on the level of abstraction and utility you'd expect from a high-level API client, while the rest of the library is clearly very lean and low-level. I'm not sure that's a bad thing, though I have been concentrating my (admittedly limited) efforts towards creating a more abstract client in the facebook library.

[jjmaestro]

Me, I would leave both of them apart since jgorset/facebook is a higher abstraction (namely, mapping the Facebook GraphAPI objects, etc, into Python objects) while jgorset/facepy is the low-level client, "only" implementing the GraphAPI HTTP verbs in a simple and easy way plus interpret the "FB wire protocol" whenever is needed.

With subscriptions, it brings in something a bit more complex since it has a client and a server side (the handlers) but it still remains down at the HTTP level, IMHO.

I will work a bit more on the branch and will issue a pull request when ready for production :)

Thanks so much for your comments!

Cheers,

[jgorset]

I'm torn on this.

I really like your code and I can see how it would make subscriptions easier, but I feel like it probably belongs in facebook if we're keeping facepy really lean. I think a good rule of thumb for a low-level library is that one should need to know nearly nothing to use it. Granted, that is very nearly the case with SubscriptionsAPI, but it's a slippery slope.

I'll think on this, but as I see it we should probably either merge facebook into facepy or apply this pull request to facebook.

[jjmaestro]

Well, I understand your worries but I don't really see any problem here. I'm all with you, 100%, that facepy should remain a really lean and small library. But I can't see how Subscriptions changes the philosophy to start worrying you! :) I mean, it clearly handles HTTP and HTTP only. It doesn't map any of the responses to any data structure, unlike facebook which is all about easy access to Facebook GraphAPI objects through OO encapsulation of data...

For example, our facepy branch right now is still 100% compatible with facebook :) Because facebook only uses GraphAPI to GET some stuff (no POST, no DELETE, etc). And it maps the data it gets into data structures (objects with public parameters). This is all completely orthogonal to facepy.

Please, give me a couple of concrete reasons why you think or feel our branch of facepy is deviating, becoming more complex or going down that slippery slope. We are more than happy to discuss this because we will be using facepy a lot in our application and we want to keep it cool and lean. And at the same time we don't want to deal with a larger library where we only use 50% or less of the features!

That's the reason we discarded software like facebook or fandjango :) We only want an "HTTP facilitator" that deals with the little Facebook quirks when calling the raw HTTP methods in the GraphAPI but just passes along the response in a JSON-mapped dictionary or throws a nice exception. Just like facepy does it :)

Thanks!

[jgorset]

I guess it's not so much about abstraction as it is about documentation, and my concern is that SubscriptionsAPI introduces a documentation overhead that is proportional to the time you'd save by reading it.

# SubscriptionsAPI
subscriptions = SubscriptionsAPI(application_id, application_secret, oauth_token)
subscriptions.post(
    obj='user',
    fields=['activities', 'interests'],
    callback_url='http://example.org'
)

# GraphAPI
graph = GraphAPI(oauth_token)
graph.post(
    path='%s/subscriptions' % application_id,
    object='user',
    fields=['activities', 'interests'],
    callback_url='http://example.org',
    verify_token=application_secret_key
)

I'm sure your branch stems from a more substantial pain in implementing real-time updates with the existing interface, though, and since I haven't really had the opportunity to use them yet I may well be missing it. :-)

[jjmaestro]

Well, with respect to the documentation, if you want to use Facebook's real-time subscriptions... you will have to read their doc (which is not really that bad but it's not great either). I understand Subscriptions are a bit harder to grasp because they require client-to-FB_server and FB_server-to-YOUR_server interactions... but such is life! :)

You got the code part right, initially it seems to not save any typing. That is, the GET part of the subscriptions is just as you wrote. But, as usual, the devil is in the details... and Facebook will issue a GET request to that callback_url to verify the subscription. So, you must have the handler_get() somewhere to facilitate constructing the SubscriptionsCallbackHandler (also, that's why I wrote one plus a little server in examples/subscriptions-server.py :).

All of this confusion is the type of stuff I wanted to avoid in SubscriptionsAPI by implementing the low-level parts of the protocol while only providing callbacks to hook it into any other application (such as the one we need in TwoApart :)

I wanted to encapsulate everything correctly because I think the encapsulation also helps with organising "the rest of the protocol". That is, the handlers for the server that you MUST have to get everything to work (which also deal with the signing...), etc. All of that is still the low-level HTTP part of subscriptions yet (1) if left in one GraphAPI class, it would have been quite messy and (2) without them, the subscriptions would have been half implemented, IMHO.

Finally, the breaking into classes also helps with the doc :) If you don't need real-time subscription... well, don't read that part of the doc! :D We can have an "Advanced" section where more complex interactions, such as Subscriptions, can be explained. Again, if anybody wants to use it, they will have to read and understand the Facebook Developer documentation... and that's quite a lot to read anyway :D

We will keep working on the branch. It's still going to be a while until we use it live, and I'm sure using it in real cases will help us understand better all of the needs, documentation, etc.

However, I think that it would be really good if you add the little refactor of breaking stuff into classes to master because, if that is in mainstream, we could maintain this separate branch for a while without too much effort and we could keep discussing the best way to get it in. Otherwise, it's going to be a bit painful to migrate changes in master.

Plus, having FQL separated (but with backwards compatibility through the fql()method in GraphAPI) will open other opportunities to incorporate batch FQL queries in a cleaner way, IMHO (see "Using fql.query and fql.multiquery in the Batch API" in https://developers.facebook.com/docs/reference/api/batch/).

Cheers!

[jgorset]

However, I think that it would be really good if you add the little refactor of breaking stuff into classes to master because, if that is in mainstream, we could maintain this separate branch for a while without too much effort and we could keep discussing the best way to get it in. Otherwise, it's going to be a bit painful to migrate changes in master.

I agree. In fact, I think segregating FQL from the Graph API would benefit the library regardless of your work on subscriptions. I've created an issue to expedite the inclusion of this part of your branch.

[jgorset]

Well, with respect to the documentation, if you want to use Facebook's real-time subscriptions... you will have to read their doc (which is not really that bad but it's not great either). I understand Subscriptions are a bit harder to grasp because they require client-to-FB_server and FB_server-to-YOUR_server interactions... but such is life! :)

To be sure. I only want to make sure that Facebook's documentation is the only documentation they'd have to read to set up real-time updates with facepy. You could argue that's because I suck at writing documentation, and you'd be mostly right. Seriously, though, I think the greatest boon of low-level libraries is that you don't need to read an additional layer of documentation in order to use them.

I hear you, though. Indeed, I think the brunt of the work involved in real-time subscriptions lies in implementing endpoints for Facebook's API. It makes sense for some of that work to be done in a library since it is largely application-agnostic and terribly boring stuff, but I'm not sure facepy is that library.

It seems to me that these kind of utilities more readily belong in a library which concerns itself with the implementation of servers or a more fully-featured and utilitarian library than a (sometimes painfully) lean library like facepy.

[jgorset]

I've reviewed the codebase and found a number of things (notably the abstraction in signed requests and the utilities for creating test users) that don't really fit with the principles on the grounds of which I declined your impending pull request. I must have been out of touch, because it would appear we're already on that slippery slope.

The question is, are we sliding towards something better or worse?

[jjmaestro]

TL;DR ;)

• Real-time updates is part of the GraphApi specification
• The spec requires both client and server sides and both sides "share state" so they should share helper (private) methods and thus be all in one class.
• It's really low-level, having a 1-to-1 mapping between the spec and the implemented methods, so once you read the Facebook doc you know how it works. Not much additional reading needed.
• The parts of the code that are more interpretations from the spec than actually specifications (basically the generation of the verify_token and the support for other signing algorithms) follow industry standards that almost everybody would want to use.

To be sure. I only want to make sure that Facebook's documentation is the only documentation they'd have to read to set up real-time updates with facepy. You could argue that's because I suck at writing documentation, and you'd be mostly right. Seriously, though, I think the greatest boon of low-level libraries is that you don't need to read an additional layer of documentation in order to use them.

As I will try to show you further on, the mapping between the implemented methods and the Facebook documentation is 1 to 1 :) That is, once you read the Facebook documentation, you only have to look up the signature of the method to actually know how to use it.

I hear you, though. Indeed, I think the brunt of the work involved in real-time subscriptions lies in implementing endpoints for Facebook's API. It makes sense for some of that work to be done in a library since it is largely application-agnostic and terribly boring stuff, but I'm not sure facepy is that library.

Well, not only in the endpoints. Also in the initial GET. Plus the initial GET and the handle_get server counterpart are deeply linked (it has to verify the token). Thus, IMHO your proposed solution of passing the application secret (or any string for that matter) as the verification_token so that you could simply use the "regular" GraphAPI class was lacking and not really correct.

To ensure a safe functioning and behaviour you would have to:

• use a random string to avoid leaking any secret
• use a timestamp (also helped with the token being random) to limit timing and replay attacks.
• sign the token to have tamper detection while avoiding database lookups to verify authenticity.

which is why I added the _encode_token() and _decode_token() private methods used by get() and handle_get(), etc.

It seems to me that these kind of utilities more readily belong in a library which concerns itself with the implementation of servers or a more fully-featured and utilitarian library than a (sometimes painfully) lean library like facepy.

OK, I understand that you are really worried about keeping everything lean and spot-on, avoiding adding superfluous functionality to the library, but I'm going to give you a few reasons why I think my approach is correct and you should merge it into facepy when the time comes :)

Real-time updates is part of GraphApi:

Unlike FQL, which IMHO is in muddy waters with respect to being part of GraphApi, real-time updates is a first class citizen. It's included in the GraphApi specification like Batching, Pagination, etc. As such, IMHO, it must be implemented in facepy :)

Implementing it, per the specs, implies having a client side and a server side . This is probably what drove you to think that it's over-complicated. Well, it certainly is harder to grasp than simply doing a GET to a GraphApi object and parsing the data... but that's expected :)

low-level:

I think that by low-level, you mean "passthrough". As in "just write the minimum amount of code to avoid having to write boilerplate code to do a raw HTTP call with request".

By such standard, I really think that we are spot-on. It separates the bare common functionality (the HTTP verb wrappers) into a base class and the SubscriptionsAPI simply uses those verbs and adds the server counterparts to Facebook actions in the handle_VERB methods. Nothing else, nothing more.

I mean, look at what has been really added!!

client:
-------
    def get()
    def post(obj, fields, callback_url, verify_token=None)
    def delete(obj=None)

server:
-------
    def handler_get(mode, challenge, verify_token)
    def handler_post(payload, signature)

Actually, if you compare the Facebook specifications for each HTTP verb to the signatures of the methods, you must agree that there is quite a good match. There's a perfect 1-to-1 mapping!! So there is actually zero need to read any other documentation but Facebook :)

lean in LOC:

If I remove all docstrings and empty lines from the SubscriptionsAPI class and the handle_callbacks_and_exceptions() decorator I get something like 106 lines of code (still passing PEP8 :).

This includes client HTTP verbs and the server counterparts (the handlers). Note that I haven't even removed the arguments in multilines added for readability in several HTTP calls.

I really don't think it can be smaller while implementing the real-time subscriptions functionality without sacrificing security or other important feature of the spec (which, BTW, is quite lacking in some of these aspects).

Seriously... I can't really see how parts of this should go into a separate library. If facepy aims to support Facebook GraphApi, this must be in it. I think I really proved my point :) What do you think? :)

Cheers,

[jjmaestro]

With respect to the other issues (#67 and #68) I think you are probably correct in removing most of that. Remember that, when we started using the library, we actually removed SignedRequest and only used the parsing part of the library to get a dictionary.

IMHO, I would remove most of the "OO mapping" leaving only the "HTTP verb" wrappers that interact with the GraphAPI and return a dictionary from parsing JSON.

I can attempt a cleanup in Sunday and we can talk about it in the different issues :)

[jjmaestro]

I am closing this issue since #69 is already including it (well, rather the new modularization branch :)

Thanks!