Merge lp:~marcustomlinson/unity-scope-vimeo/request_token_from_run into lp:unity-scope-vimeo

[Marcus Tomlinson]

Create one OnlineAccountClient instance and pass it into Query and Preview objects. This instance is then used by an internal Client object to keep its config up to date (by refreshing the service statuses each time the token is needed).

This way the config can be completely encapsulated within Client, and Client can be encapsulated within Query and Preview.

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2014-11-18 17:34:05 +0000
+++ CMakeLists.txt	2014-12-04 12:41:10 +0000
@@ -49,7 +49,7 @@
   ${SCOPE_INCLUDE_DIRS}
 )
 
-set(SCOPE_VERSION "1.0.1-${BZR_REVNO}")
+set(SCOPE_VERSION "1.0.2-${BZR_REVNO}")
 set(CMAKE_INSTALL_PREFIX /)
 set(SCOPE_NAME "com.ubuntu.scopes.vimeo")
 set(SCOPE_INSTALL_NAME "${SCOPE_NAME}_vimeo")
 
=== modified file 'include/vimeo/api/client.h'
--- include/vimeo/api/client.h	2014-11-17 12:44:51 +0000
+++ include/vimeo/api/client.h	2014-12-04 12:41:10 +0000
@@ -23,6 +23,8 @@
 #include <vimeo/api/config.h>
 #include <vimeo/api/video.h>
 
+#include <unity/scopes/OnlineAccountClient.h>
+
 #include <atomic>
 #include <deque>
 #include <future>
@@ -44,7 +46,7 @@
 
     typedef std::deque<Video::Ptr> VideoList;
 
-    Client(Config::Ptr config);
+    Client(std::shared_ptr<unity::scopes::OnlineAccountClient> oa_client);
 
     virtual ~Client() = default;
 
@@ -58,7 +60,7 @@
 
     virtual void cancel();
 
-    virtual Config::Ptr config();
+    virtual bool authenticated();
 
 protected:
     class Priv;
 
=== modified file 'include/vimeo/scope/query.h'
--- include/vimeo/scope/query.h	2014-06-25 14:41:07 +0000
+++ include/vimeo/scope/query.h	2014-12-04 12:41:10 +0000
@@ -30,7 +30,8 @@
 class Query: public unity::scopes::SearchQueryBase {
 public:
     Query(const unity::scopes::CannedQuery &query,
-            const unity::scopes::SearchMetadata &metadata, vimeo::api::Config::Ptr config);
+          const unity::scopes::SearchMetadata &metadata,
+          std::shared_ptr<unity::scopes::OnlineAccountClient> oa_client);
 
     ~Query() = default;
 
 
=== modified file 'include/vimeo/scope/scope.h'
--- include/vimeo/scope/scope.h	2014-11-03 10:04:16 +0000
+++ include/vimeo/scope/scope.h	2014-12-04 12:41:10 +0000
@@ -19,8 +19,6 @@
 #ifndef VIMEO_SCOPE_SCOPE_H_
 #define VIMEO_SCOPE_SCOPE_H_
 
-#include <vimeo/api/config.h>
-
 #include <unity/scopes/ScopeBase.h>
 #include <unity/scopes/OnlineAccountClient.h>
 #include <unity/scopes/QueryBase.h>
@@ -28,8 +26,6 @@
 #include <unity/scopes/QueryBase.h>
 #include <unity/scopes/PreviewQueryBase.h>
 
-#include <condition_variable>
-
 namespace vimeo {
 
 namespace scope {
@@ -48,20 +44,6 @@
             unity::scopes::SearchMetadata const&) override;
 
 protected:
-    void anonymous_login();
-
-    void service_update(unity::scopes::OnlineAccountClient::ServiceStatus const& status);
-
-    void update_config();
-
-    void init_config();
-
-    vimeo::api::Config::Ptr config_;
-
-    std::mutex config_mutex_;
-
-    std::condition_variable config_cond_;
-
     std::shared_ptr<unity::scopes::OnlineAccountClient> oa_client_;
 };
 
 
=== modified file 'src/vimeo/api/client.cpp'
--- src/vimeo/api/client.cpp	2014-11-17 12:54:17 +0000
+++ src/vimeo/api/client.cpp	2014-12-04 12:41:10 +0000
@@ -17,7 +17,10 @@
  */
 
 #include <vimeo/api/client.h>
+#include <vimeo/api/login.h>
 
+#include <boost/filesystem/operations.hpp>
+#include <boost/filesystem/path.hpp>
 #include <boost/iostreams/filtering_stream.hpp>
 #include <boost/iostreams/filter/gzip.hpp>
 #include <core/net/error.h>
@@ -26,7 +29,10 @@
 #include <core/net/http/response.h>
 #include <json/json.h>
 
+#include <fstream>
+
 namespace http = core::net::http;
+namespace fs = boost::filesystem;
 namespace io = boost::iostreams;
 namespace json = Json;
 namespace net = core::net;
@@ -34,6 +40,9 @@
 using namespace vimeo::api;
 using namespace std;
 
+static const char* CLIENT_ID = "b6758ff9f929cdb9f45a8477732bdbc4c6a89c7e";
+static const char* CLIENT_SECRET = "a3222f38f799b3b528e29418fe062c02c677a249";
+
 namespace {
 
 template<typename T>
@@ -50,9 +59,9 @@
 
 class Client::Priv {
 public:
-    Priv(Config::Ptr config) :
-            client_(http::make_client()), worker_ { [this]() {client_->run();} }, config_(
-                    config), cancelled_(false) {
+    Priv(std::shared_ptr<unity::scopes::OnlineAccountClient> oa_client) :
+            client_(http::make_client()), worker_ { [this]() {client_->run();} },
+            oa_client_(oa_client), cancelled_(false) {
     }
 
     ~Priv() {
@@ -66,31 +75,36 @@
 
     std::thread worker_;
 
-    Config::Ptr config_;
+    Config config_;
+    std::mutex config_mutex_;
+
+    std::shared_ptr<unity::scopes::OnlineAccountClient> oa_client_;
 
     std::atomic<bool> cancelled_;
 
     void get(const net::Uri::Path &path,
             const net::Uri::QueryParameters &parameters,
             http::Request::Handler &handler) {
+        std::lock_guard<std::mutex> lock(config_mutex_);
+        update_config();
 
         http::Request::Configuration configuration;
         net::Uri::QueryParameters complete_parameters(parameters);
 
-        net::Uri uri = net::make_uri(config_->apiroot, path,
+        net::Uri uri = net::make_uri(config_.apiroot, path,
                 complete_parameters);
         configuration.uri = client_->uri_to_string(uri);
-        if (!config_->access_token.empty()) {
+        if (!config_.access_token.empty()) {
             configuration.header.add("Authorization",
-                    "bearer " + config_->access_token);
-        } else if (!config_->client_id.empty() && !config_->client_secret.empty()) {
+                    "bearer " + config_.access_token);
+        } else if (!config_.client_id.empty() && !config_.client_secret.empty()) {
             string auth = "basic "
                     + client_->base64_encode(
-                            config_->client_id + ":" + config_->client_secret);
+                            config_.client_id + ":" + config_.client_secret);
             configuration.header.add("Authorization", auth);
         }
-        configuration.header.add("Accept", config_->accept);
-        configuration.header.add("User-Agent", config_->user_agent + " (gzip)");
+        configuration.header.add("Accept", config_.accept);
+        configuration.header.add("User-Agent", config_.user_agent + " (gzip)");
         configuration.header.add("Accept-Encoding", "gzip");
 
         auto request = client_->head(configuration);
@@ -151,10 +165,93 @@
 
         return prom->get_future();
     }
+
+    bool authenticated() {
+        std::lock_guard<std::mutex> lock(config_mutex_);
+        update_config();
+        return config_.authenticated;
+    }
+
+    void anonymous_login() {
+        fs::path saved_token_dir = fs::path(getenv("HOME"))
+                / ".local" / "share" / "unity-scopes" / "leaf-net" / SCOPE_NAME;
+        fs::path saved_token_path = saved_token_dir
+                / "anonymous_auth_token";
+
+        bool save_auth_token = getenv("VIMEO_SCOPE_IGNORE_ACCOUNTS") == nullptr;
+
+        config_.client_id = CLIENT_ID;
+        config_.client_secret = CLIENT_SECRET;
+
+        if (fs::exists(saved_token_path) && save_auth_token) {
+            ifstream in(saved_token_path.native(), ios::in | ios::binary);
+            if (in) {
+                ostringstream contents;
+                contents << in.rdbuf();
+                in.close();
+                config_.access_token = contents.str();
+            }
+            cerr << "  re-using saved auth_token" << endl;
+        } else {
+            config_.access_token = unauthenticated(CLIENT_ID, CLIENT_SECRET,
+                    config_.apiroot);
+            if (save_auth_token) {
+                fs::create_directories(saved_token_dir);
+                ofstream out(saved_token_path.native(), ios::out | ios::binary);
+                if (out) {
+                    out << config_.access_token;
+                    out.close();
+                }
+            }
+            cerr << "  new auth_token" << endl;
+        }
+    }
+
+    void update_config() {
+        if (getenv("VIMEO_SCOPE_APIROOT")) {
+            config_.apiroot = getenv("VIMEO_SCOPE_APIROOT");
+        }
+
+        if (getenv("VIMEO_SCOPE_IGNORE_ACCOUNTS") != nullptr) {
+            return;
+        }
+
+        /// TODO: The code commented out below should be uncommented as soon as
+        /// OnlineAccountClient::refresh_service_statuses() is fixed (Bug #1398813).
+        /// For now we have to re-instantiate a new OnlineAccountClient each time.
+
+        ///if (oa_client_ == nullptr) {
+            oa_client_.reset(
+                    new unity::scopes::OnlineAccountClient(SCOPE_INSTALL_NAME,
+                            "sharing", SCOPE_ACCOUNTS_NAME));
+        ///} else {
+        ///    oa_client_->refresh_service_statuses();
+        ///}
+
+        for (auto const& status : oa_client_->get_service_statuses()) {
+            if (status.service_authenticated) {
+                config_.authenticated = true;
+                config_.access_token = status.access_token;
+                config_.client_id = status.client_id;
+                config_.client_secret = status.client_secret;
+                break;
+            }
+        }
+
+        if (!config_.authenticated) {
+            config_.access_token = "";
+            config_.client_id = "";
+            config_.client_secret = "";
+            std::cerr << "Vimeo scope is unauthenticated" << std::endl;
+            anonymous_login();
+        } else {
+            std::cerr << "Vimeo scope is authenticated" << std::endl;
+        }
+    }
 };
 
-Client::Client(Config::Ptr config) :
-        p(new Priv(config)) {
+Client::Client(std::shared_ptr<unity::scopes::OnlineAccountClient> oa_client) :
+        p(new Priv(oa_client)) {
 }
 
 future<Client::VideoList> Client::videos(const string &query) {
@@ -190,7 +287,7 @@
     p->cancelled_ = true;
 }
 
-Config::Ptr Client::config() {
-    return p->config_;
+bool Client::authenticated() {
+    return p->authenticated();
 }
 
 
=== modified file 'src/vimeo/scope/query.cpp'
--- src/vimeo/scope/query.cpp	2014-11-17 12:44:51 +0000
+++ src/vimeo/scope/query.cpp	2014-12-04 12:41:10 +0000
@@ -88,8 +88,9 @@
 }
 
 Query::Query(const sc::CannedQuery &query, const sc::SearchMetadata &metadata,
-        Config::Ptr config) :
-        sc::SearchQueryBase(query, metadata), client_(config) {
+             std::shared_ptr<sc::OnlineAccountClient> oa_client) :
+        sc::SearchQueryBase(query, metadata),
+        client_(oa_client) {
 }
 
 void Query::cancelled() {
@@ -132,7 +133,7 @@
                 "My Feed");
         sc::Department::SPtr dummy_dept;
 
-        bool include_login_nag = !client_.config()->authenticated;
+        bool include_login_nag = !client_.authenticated();
 
         if (query_string.empty()) {
             channels_future = client_.channels();
@@ -147,7 +148,7 @@
                 videos_future = client_.channels_videos("staffpicks");
             } else if (!query.department_id().empty()) {
                 videos_future = client_.channels_videos(query.department_id());
-            } else if (client_.config()->authenticated) {
+            } else if (client_.authenticated()) {
                 videos_future = client_.feed();
             } else {
                 videos_future = client_.channels_videos("staffpicks");
 
=== modified file 'src/vimeo/scope/scope.cpp'
--- src/vimeo/scope/scope.cpp	2014-11-07 18:54:12 +0000
+++ src/vimeo/scope/scope.cpp	2014-12-04 12:41:10 +0000
@@ -16,102 +16,16 @@
  * Author: Pete Woods <[e-mail]>
  */
 
-#include <vimeo/api/login.h>
+#include <vimeo/scope/localisation.h>
 #include <vimeo/scope/scope.h>
 #include <vimeo/scope/query.h>
 #include <vimeo/scope/preview.h>
 
-#include <boost/filesystem/path.hpp>
-#include <boost/filesystem/operations.hpp>
-
-#include <iostream>
-#include <sstream>
-#include <fstream>
-
 namespace sc = unity::scopes;
 using namespace std;
-using namespace boost;
 using namespace vimeo::scope;
 using namespace vimeo::api;
 
-static const char* CLIENT_ID = "b6758ff9f929cdb9f45a8477732bdbc4c6a89c7e";
-static const char* CLIENT_SECRET = "a3222f38f799b3b528e29418fe062c02c677a249";
-
-void Scope::anonymous_login() {
-    filesystem::path saved_token_dir = filesystem::path(getenv("HOME"))
-            / ".local" / "share" / "unity-scopes" / "leaf-net" / SCOPE_NAME;
-    filesystem::path saved_token_path = saved_token_dir
-            / "anonymous_auth_token";
-
-    bool save_auth_token = getenv("VIMEO_SCOPE_IGNORE_ACCOUNTS") == nullptr;
-
-    config_->client_id = CLIENT_ID;
-    config_->client_secret = CLIENT_SECRET;
-
-    if (filesystem::exists(saved_token_path) && save_auth_token) {
-        ifstream in(saved_token_path.native(), ios::in | ios::binary);
-        if (in) {
-            ostringstream contents;
-            contents << in.rdbuf();
-            in.close();
-            config_->access_token = contents.str();
-        }
-        cerr << "  re-using saved auth_token" << endl;
-    } else {
-        config_->access_token = unauthenticated(CLIENT_ID, CLIENT_SECRET,
-                config_->apiroot);
-        if (save_auth_token) {
-            filesystem::create_directories(saved_token_dir);
-            ofstream out(saved_token_path.native(), ios::out | ios::binary);
-            if (out) {
-                out << config_->access_token;
-                out.close();
-            }
-        }
-        cerr << "  new auth_token" << endl;
-    }
-}
-
-void Scope::service_update(sc::OnlineAccountClient::ServiceStatus const&)
-{
-    update_config();
-}
-
-void Scope::update_config()
-{
-    std::lock_guard<std::mutex> lock(config_mutex_);
-    init_config();
-
-    for (auto const& status : oa_client_->get_service_statuses())
-    {
-        if (status.service_authenticated)
-        {
-            config_->authenticated = true;
-            config_->access_token = status.access_token;
-            config_->client_id = status.client_id;
-            config_->client_secret = status.client_secret;
-            break;
-        }
-    }
-
-    if (!config_->authenticated) {
-        cerr << "Vimeo scope is unauthenticated" << endl;
-        anonymous_login();
-    } else {
-        cerr << "Vimeo scope is authenticated" << endl;
-    }
-
-    config_cond_.notify_all();
-}
-
-void Scope::init_config()
-{
-    config_ = make_shared<Config>();
-    if (getenv("VIMEO_SCOPE_APIROOT")) {
-        config_->apiroot = getenv("VIMEO_SCOPE_APIROOT");
-    }
-}
-
 void Scope::start(string const&) {
     setlocale(LC_ALL, "");
     string translation_directory = ScopeBase::scope_directory()
@@ -122,23 +36,6 @@
         oa_client_.reset(
                 new sc::OnlineAccountClient(SCOPE_INSTALL_NAME,
                         "sharing", SCOPE_ACCOUNTS_NAME));
-        oa_client_->set_service_update_callback(
-                std::bind(&Scope::service_update, this, std::placeholders::_1));
-
-        ///! TODO: We should only be waiting here if we know that there is at least one Google account enabled.
-        ///        OnlineAccountClient needs to expose some functionality for us to determine that.
-
-        // Allow 1 second for the callback to initialize config_
-        std::unique_lock<std::mutex> lock(config_mutex_);
-        config_cond_.wait_for(lock, std::chrono::seconds(1), [this] { return config_ != nullptr; });
-    }
-
-    if (config_ == nullptr)
-    {
-        // If the callback was not invoked, default initialize config_
-        init_config();
-        cerr << "Vimeo scope is unauthenticated" << endl;
-        anonymous_login();
     }
 }
 
@@ -147,7 +44,7 @@
 
 sc::SearchQueryBase::UPtr Scope::search(const sc::CannedQuery &query,
         const sc::SearchMetadata &metadata) {
-    return sc::SearchQueryBase::UPtr(new Query(query, metadata, config_));
+    return sc::SearchQueryBase::UPtr(new Query(query, metadata, oa_client_));
 }
 
 sc::PreviewQueryBase::UPtr Scope::preview(sc::Result const& result,