Index: perl/modules/Facebook/README
===================================================================
--- perl/modules/Facebook/README	(revision 8b6208883c04d551ae374ef4fca9dec22b98964e)
+++ perl/modules/Facebook/README	(revision 01d186f422ca23eafee3a0850ca9bfc311c32c62)
@@ -22,33 +22,46 @@
 
   (4) Start receiving wall updates in Barnowl!
-      You can post updates with the ":facebook" command.
+  You can post updates with the ":facebook" command.
+
+INFELICITIES
+  * Polling Facebook is pretty slow (on order of a second or more),
+    and blocks the entire BarnOwl interface.  We have a plan for
+    fixing this, which involves creating an async version of
+    Facebook::Graph.  I have been in contact with the original
+    author JT Smith about this.
+
+  * BarnOwl will not receive all comments for news feed items, only
+    comments for items that were recently published.  There is not
+    currently a way to see starks for conversations that you did not
+    participate in, and the only way to see starks for conversations
+    you did participate in are Facebook's email notifications.  (This
+    is a deficiency of the Facebook API, see http://bugs.developers.facebook.net/show_bug.cgi?id=18594.)
+
+  * By default, BarnOwl will not display posts from organizations (such
+    as bands you have liked) or application invites.  This is a
+    judgment of taste from the original author.  This is not currently
+    configurable.
+
+  * Users and applications that you have hidden via the Facebook
+    web interface will show up in the BarnOwl stream.  (This appears to
+    be a deficiency of the Facebook API, see
+    http://stackoverflow.com/questions/6405364/facebook-api-access-hide-posts-from-settings).
+    Users are encouraged to work around this by using traditional Zephyr
+    filters.
+
+  * We are missing support for some notable features, including
+    messaging (Facebook has not publically released the API for this,
+    though we could sign up for the whitelist), events (seeing
+    unresponded to events requires a custom FQL query
+    http://stackoverflow.com/questions/4752967/facebook-api-only-returns-25-events-max),
+    notifications (not supported in Graph API yet).
+
+WISHLIST
+  * Smarter name de-duplication (see Facebook/Handle.pm for details.)
+  * URL minification.
+  * Multiple accounts.  (Does anyone do this? I don't think so...)
+  * Zephyr class mirroring.
 
 POLLING
   Facebook.par polls for normal messages once a minute. To disable
   polling, you can unset the 'facebook:poll' variable in BarnOwl.
-
-TODO
-  * Polling Facebook is pretty slow (on order of a second or more),
-    and blocks the entire BarnOwl interface.  We need to confront
-    Perl's threading demon.
-  * No messaging support. (We'll add it when Facebook makes the new endpoint.)
-  * Smarter name de-duplication (see code comments for details.)
-  * Grep for XXX and TODO for more work items.
-
-TECHNICAL NOTES
-  This module uses 100% undeprecated Facebook Graph API, and should be
-  an interesting case study of how to implement a desktop application in
-  the new Facebook world order.  In particular, we do not use the old
-  infinite session keys trick described in
-  <http://www.emcro.com/blog/2009/01/facebook-infinite-session-keys-no-more/>,
-  instead, we use offline_access to get non-expiring tokens.
-
-  If we decide to extend our permissions to include read_friendlists
-  (for filtering) and rsvp_event (RSVP from BarnOwl), we will need
-  to make sure the UI for upgrading is correct.
-
-  We'll be rolling our own version of Facebook::Graph, as the original
-  uses the synchronous LWP::UserAgent, and we'd like our web requests
-  not to block the user interface.  Furthermore, Facebook::Graph doesn't
-  actually use any of the features of LWP::UserAgent, so we may be able
-  to use a simpler module AnyEvent::HTTP.
Index: perl/modules/Facebook/lib/BarnOwl/Module/Facebook.pm
===================================================================
--- perl/modules/Facebook/lib/BarnOwl/Module/Facebook.pm	(revision 8b6208883c04d551ae374ef4fca9dec22b98964e)
+++ perl/modules/Facebook/lib/BarnOwl/Module/Facebook.pm	(revision 01d186f422ca23eafee3a0850ca9bfc311c32c62)
@@ -24,7 +24,4 @@
 our $facebook_handle = undef;
 
-# did not implement class monitoring
-# did not implement multiple accounts
-
 BarnOwl::new_variable_bool(
     'facebook:poll',
@@ -32,55 +29,24 @@
         default => 1,
         summary => 'Poll Facebook for wall updates',
-        # TODO: Make this configurable
+        # XXX: Make poll time configurable
         description => "If set, will poll Facebook every minute for updates.\n"
      }
  );
 
-sub fail {
-    my $msg = shift;
-    # reset global state here
-    BarnOwl::admin_message('Facebook Error', $msg);
-    die("Facebook Error: $msg\n");
+sub init {
+    my $conffile = BarnOwl::get_config_dir() . "/facebook";
+    my $cfg = {};
+    if (open(my $fh, "<", "$conffile")) {
+        my $raw_cfg = do {local $/; <$fh>};
+        close($fh);
+
+        eval { $cfg = from_json($raw_cfg); };
+        if ($@) { BarnOwl::admin_message('Facebook', "Unable to parse $conffile: $@"); }
+    }
+    eval { $facebook_handle = BarnOwl::Module::Facebook::Handle->new($cfg); };
+    if ($@) { BarnOwl::error($@); }
 }
 
-# We only load up when the conf file is present, to reduce resource
-# usage.  Though, probably not by very much, so maybe a 'facebook-init'
-# command would be more appropriate.
-
-my $conffile = BarnOwl::get_config_dir() . "/facebook";
-
-if (open(my $fh, "<", "$conffile")) {
-    read_config($fh);
-    close($fh);
-}
-
-sub read_config {
-    my $fh = shift;
-    my $raw_cfg = do {local $/; <$fh>};
-    close($fh);
-
-    my $cfg;
-    if ($raw_cfg) {
-        eval { $cfg = from_json($raw_cfg); };
-        if($@) {
-            fail("Unable to parse $conffile: $@");
-        }
-    } else {
-        $cfg = {};
-    }
-
-    eval {
-        $facebook_handle = BarnOwl::Module::Facebook::Handle->new($cfg);
-    };
-    if ($@) {
-        BarnOwl::error($@);
-        next;
-    }
-}
-
-# Ostensibly here as a convenient shortcut for Perl hackery
-sub facebook {
-    $facebook_handle->facebook(@_);
-}
+init();
 
 # Should also add support for posting to other people's walls (this
@@ -89,9 +55,8 @@
 BarnOwl::new_command('facebook' => \&cmd_facebook, {
     summary     => 'Post a status update to your wall from BarnOwl',
-    usage       => 'facebook',
-    description => 'Post a status update to your wall.'
+    usage       => 'facebook [USER]',
+    description => 'Post a status update to your wall, or post on another user\'s wall. Autocomplete is supported.'
 });
 
-# How do we allow people to specify the USER?
 #BarnOwl::new_command('facebook-message' => \&cmd_facebook_direct, {
 #    summary     => 'Send a Facebook message',
@@ -103,5 +68,5 @@
     summary     => 'Comment on a wall post.',
     usage       => 'facebook-comment POST_ID',
-    description => 'Comment on a friend\'s wall post.  Using r is recommended.'
+    description => 'Comment on a friend\'s wall post.'
 });
 
@@ -116,8 +81,6 @@
     summary     => 'Force a poll of Facebook.',
     usage       => 'facebook-poll',
-    description => 'Get updates from Facebook.'
+    description => 'Get updates (news, friends) from Facebook.'
 });
-
-# XXX: UI: probably should bug out immediately if we're not logged in.
 
 sub cmd_facebook {
@@ -125,7 +88,9 @@
     my $user = shift;
 
+    return unless check_ready();
+
     BarnOwl::start_edit_win(
         defined $user ? "Write something to $user..." : "What's on your mind?",
-        sub{ facebook($user, shift) }
+        sub{ $facebook_handle->facebook($user, shift) }
     );
 }
@@ -135,8 +100,8 @@
     my $post_id = shift;
 
+    return unless check_ready();
+
     my $topic = $facebook_handle->get_topic($post_id);
 
-    # XXX UI should give some (better) indication /which/ conversation
-    # is being commented on
     BarnOwl::start_edit_win("Write a comment on '$topic'...",
                             sub { $facebook_handle->facebook_comment($post_id, shift) });
@@ -145,4 +110,6 @@
 sub cmd_facebook_poll {
     my $cmd = shift;
+
+    return unless check_ready();
 
     $facebook_handle->sleep(0);
@@ -154,10 +121,21 @@
     my $url = shift;
 
+    if ($facebook_handle->{logged_in}) {
+        BarnOwl::message("Already logged in. (To force, run ':reload-module Facebook', or deauthorize BarnOwl.)");
+        return;
+    }
+
     $facebook_handle->facebook_auth($url);
 }
 
+sub check_ready {
+    if (!$facebook_handle->{logged_in}) {
+        BarnOwl::message("Need to login to Facebook first with ':facebook-auth'.");
+        return 0;
+    }
+    return 1;
+}
+
 BarnOwl::filter(qw{facebook type ^facebook$});
-
-# Autocompletion support
 
 sub complete_user { return keys %{$facebook_handle->{friends}}; }
Index: perl/modules/Facebook/lib/BarnOwl/Module/Facebook/Handle.pm
===================================================================
--- perl/modules/Facebook/lib/BarnOwl/Module/Facebook/Handle.pm	(revision 65c2b3c400eaf6b4dbb1d5678be39568c89a6850)
+++ perl/modules/Facebook/lib/BarnOwl/Module/Facebook/Handle.pm	(revision 01d186f422ca23eafee3a0850ca9bfc311c32c62)
@@ -35,4 +35,5 @@
 use Date::Parse;
 use POSIX;
+use Ouch;
 
 use Scalar::Util qw(weaken);
@@ -65,11 +66,4 @@
 #   comments less frequently than polling for new posts.
 
-sub fail {
-    my $self = shift;
-    my $msg  = shift;
-    undef $self->{facebook};
-    die("[Facebook] Error: $msg\n");
-}
-
 sub new {
     my $class = shift;
@@ -83,11 +77,10 @@
         # but we can't assume that the BarnOwl lives on a publically
         # addressable server (XXX maybe we can setup an option for this.)
-        'last_friend_poll' => 0,
         'friend_timer' => undef,
 
         # Initialized with our 'time', but will be synced to Facebook
         # soon enough. (Subtractive amount is just to preseed with some
-        # values.)
-        'last_poll' => time - 60 * 60 * 24 * 2,
+        # values.) XXX Remove subtraction altogether.
+        'last_poll' => time - 60 * 60,
         'timer' => undef,
 
@@ -99,7 +92,11 @@
         # $fb->authorize, but at time of writing (1.0300) they didn't support
         # the response_type parameter.
-        # 'login_url' => 'https://www.facebook.com/dialog/oauth?client_id=235537266461636&scope=read_stream,read_mailbox,publish_stream,offline_access&redirect_uri=http://www.facebook.com/connect/login_success.html&response_type=token',
+        # 'login_url' => 'https://www.facebook.com/dialog/oauth?client_id=235537266461636&scope=read_stream,read_mailbox,publish_stream,offline_access,read_friendlists,rsvp_event,user_events&redirect_uri=http://www.facebook.com/connect/login_success.html&response_type=token',
         # minified to fit in most terminal windows.
-        'login_url' => 'http://goo.gl/yA42G',
+        # Be careful about updating these values, since BarnOwl will not
+        # notice that it is missing necessary permissions until it
+        # attempt to perform an operation which fails due to lack of
+        # permissions.
+        'login_url' => 'http://goo.gl/rcM9s',
 
         'logged_in' => 0,
@@ -164,4 +161,20 @@
 }
 
+sub check_result {
+    my $self = shift;
+    if (kiss 400) {
+        # Ugh, no easy way of accessing the JSON error type
+        # which is OAuthException.
+        $self->{logged_in} = 0;
+        $self->facebook_do_auth;
+        return 0;
+    } elsif (hug) {
+        my $code = $@->code;
+        warn "Poll failed with $code: $@";
+        return 0;
+    }
+    return 1;
+}
+
 sub poll_friends {
     my $self = shift;
@@ -171,10 +184,6 @@
 
     my $friends = eval { $self->{facebook}->fetch('me/friends'); };
-    if ($@) {
-        warn "Poll failed $@";
-        return;
-    }
-
-    $self->{last_friend_poll} = time;
+    return unless $self->check_result;
+
     $self->{friends} = {};
 
@@ -195,6 +204,9 @@
             # The most recent one.)  Since getting this information
             # involves extra queries, there are also caching and
-            # efficiency concerns.
-            #   We may want a facility for users to specify custom
+            # efficiency concerns (though hopefully you don't have too
+            # many friends with the same name).  Furthermore, accessing
+            # this information requires a pretty hefty extra set of
+            # permissions requests, which we don't currently ask for.
+            #   It may just be better to let users specify custom
             # aliases for Facebook users, which are added into this
             # hash.  See also username support.
@@ -213,5 +225,6 @@
     # query, it would require a rather expensive set of queries. We
     # might try to preserve old data, but all-in-all it's a bit
-    # complicated, so we don't bother.
+    # complicated.  One possible way of fixing this is to construct a
+    # custom FQL query that joins the friends table and the users table.
 }
 
@@ -219,9 +232,6 @@
     my $self = shift;
 
-    #return unless ( time - $self->{last_poll} ) >= 60;
     return unless BarnOwl::getvar('facebook:poll') eq 'on';
     return unless $self->{logged_in};
-
-    #BarnOwl::message("Polling Facebook...");
 
     # XXX Oh no! This blocks the user interface.  Not good.
@@ -237,14 +247,13 @@
              ->from("my_news")
              # Not using this, because we want to pick up comment
-             # updates. We need to manually de-dup, though.
+             # updates. We need to manually de-duplicate, though.
              # ->where_since( "@" . $self->{last_poll} )
+             # Facebook doesn't actually give us that many results.
+             # But it can't hurt to ask!
              ->limit_results( 200 )
-             ->request()
-             ->as_hashref()
+             ->request
+             ->as_hashref
     };
-    if ($@) {
-        warn "Poll failed $@";
-        return;
-    }
+    return unless $self->check_result;
 
     my $new_last_poll = $self->{last_poll};
@@ -262,7 +271,4 @@
         }
 
-        # XXX Need to somehow access Facebook's user hiding
-        # mechanism
-
         # There can be multiple recipients! Strange! Pick the first one.
         my $name    = $post->{to}{data}[0]{name} || $post->{from}{name};
@@ -270,9 +276,11 @@
         my $post_id  = $post->{id};
 
+        my $topic;
         if (defined $old_topics->{$post_id}) {
-            $self->{topics}->{$post_id} = $old_topics->{$post_id};
+            $topic = $old_topics->{$post_id};
+            $self->{topics}->{$post_id} = $topic;
         } else {
             my @keywords = keywords($post->{name} || $post->{message});
-            my $topic = $keywords[0] || 'personal';
+            $topic = $keywords[0] || 'personal';
             $topic =~ s/ /-/g;
             $self->{topics}->{$post_id} = $topic;
@@ -292,5 +300,5 @@
                 body      => $self->format_body($post),
                 post_id   => $post_id,
-                topic     => $self->get_topic($post_id),
+                topic     => $topic,
                 time      => asctime(localtime $created_time),
                 # XXX The intent is to get the 'Comment' link, which also
@@ -302,6 +310,6 @@
         }
 
-        # This will have funky interleaving of times (they'll all be
-        # sorted linearly), but since we don't expect too many updates between
+        # This will interleave times (they'll all be organized by parent
+        # post), but since we don't expect too many updates between
         # polls this is pretty acceptable.
         my $updated_time = str2time($post->{updated_time});
@@ -320,6 +328,6 @@
                     direction => 'in',
                     body      => $comment->{message},
-                    post_id    => $post_id,
-                    topic     => $self->get_topic($post_id),
+                    post_id   => $post_id,
+                    topic     => $topic,
                     time      => asctime(localtime $comment_time),
                    );
@@ -355,4 +363,8 @@
 }
 
+# Invariant: we don't become logged out between entering text field
+# and actually processing the request.  XXX I don't think this actually
+# holds, but such a case would rarely happen.
+
 sub facebook {
     my $self = shift;
@@ -361,13 +373,11 @@
     my $msg = shift;
 
-    if (!defined $self->{facebook} || !$self->{logged_in}) {
-        BarnOwl::admin_message('Facebook', 'You are not currently logged into Facebook.');
-        return;
-    }
     if (defined $user) {
         $user = $self->{friends}{$user} || $user;
-        $self->{facebook}->add_post( $user )->set_message( $msg )->publish;
+        eval { $self->{facebook}->add_post( $user )->set_message( $msg )->publish; };
+        return unless $self->check_result;
     } else {
-        $self->{facebook}->add_post->set_message( $msg )->publish;
+        eval { $self->{facebook}->add_post->set_message( $msg )->publish; };
+        return unless $self->check_result;
     }
     $self->sleep(0);
@@ -380,5 +390,6 @@
     my $msg = shift;
 
-    $self->{facebook}->add_comment( $post_id )->set_message( $msg )->publish;
+    eval { $self->{facebook}->add_comment( $post_id )->set_message( $msg )->publish; };
+    return unless $self->check_result;
     $self->sleep(0);
 }
@@ -388,6 +399,12 @@
 
     my $url = shift;
+
     # http://www.facebook.com/connect/login_success.html#access_token=TOKEN&expires_in=0
     $url =~ /access_token=([^&]+)/; # XXX Ew regex
+
+    if (!defined $1) {
+        BarnOwl::message("Invalid URL.");
+        return;
+    }
 
     $self->{cfg}->{token} = $1;
@@ -402,10 +419,16 @@
     if ( ! defined $self->{cfg}->{token} ) {
         BarnOwl::admin_message('Facebook', "Login to Facebook at ".$self->{login_url}
-            . "\nand run command ':facebook-auth URL' with the URL you are redirected to.");
+            . "\nand run command ':facebook-auth URL' with the URL you are redirected to."
+            . "\n\nWhat does Barnowl use these permissions for?  As a desktop"
+            . "\nmessaging application, we need persistent read/write access to your"
+            . "\nnews feed and your inbox.  Other permissions are for pending"
+            . "\nfeatures: we intend on adding support for event streaming, RSVP,"
+            . "\nand BarnOwl filtering on friend lists."
+        );
         return 0;
     }
     $self->{facebook}->access_token($self->{cfg}->{token});
     # Do a quick check to see if things are working
-    my $result = eval { $self->{facebook}->fetch('me'); };
+    my $result = eval { $self->{facebook}->query()->find('me')->select_fields('name')->request->as_hashref; };
     if ($@) {
         BarnOwl::admin_message('Facebook', "Failed to authenticate! Login to Facebook at ".$self->{login_url}
@@ -421,11 +444,3 @@
 }
 
-sub get_topic {
-    my $self = shift;
-
-    my $post_id = shift;
-
-    return $self->{topics}->{$post_id} || 'personal';
-}
-
 1;
