Merge lp:~unity-api-team/unity-scope-soundcloud/request_token_from_run into lp:~unity-api-team/unity-scope-soundcloud/c++

[Marcus Tomlinson]

Invoke OA on each query

[lp:~unity-api-team/unity-scope-soundcloud/request_token_from_run]

47. By Marcus Tomlinson on 2014-12-04

Increment version number

[Pete Woods (pete-woods)]

Looks good to me

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2014-11-18 17:36:21 +0000
+++ CMakeLists.txt	2014-12-04 15:01:59 +0000
@@ -55,7 +55,7 @@
 set(UBUNTU_PROJECT_TYPE "Scope" CACHE INTERNAL "Tells QtCreator this is a Scope project")
 
 # Important project paths
-set(SCOPE_VERSION "1.0.0-${BZR_REVNO}")
+set(SCOPE_VERSION "1.0.1-${BZR_REVNO}")
 set(CMAKE_INSTALL_PREFIX /)
 set(SCOPE_INSTALL_DIR "/soundcloud")
 set(GETTEXT_PACKAGE "unity-scope-soundcloud")

=== modified file 'include/api/client.h'
--- include/api/client.h	2014-11-17 11:50:50 +0000
+++ include/api/client.h	2014-12-04 15:01:59 +0000
@@ -22,6 +22,8 @@
 #include <api/config.h>
 #include <api/track.h>
 
+#include <unity/scopes/OnlineAccountClient.h>
+
 #include <atomic>
 #include <deque>
 #include <future>
@@ -50,7 +52,7 @@
  */
 class Client {
 public:
-    Client(Config::Ptr config);
+    Client(std::shared_ptr<unity::scopes::OnlineAccountClient> oa_client);
 
     virtual ~Client() = default;
 
@@ -65,7 +67,9 @@
      */
     virtual void cancel();
 
-    virtual Config::Ptr config();
+    virtual std::string client_id();
+
+    virtual bool authenticated();
 
 protected:
     class Priv;

=== modified file 'include/api/config.h'
--- include/api/config.h	2014-11-10 11:54:32 +0000
+++ include/api/config.h	2014-12-04 15:01:59 +0000
@@ -36,8 +36,6 @@
     std::string access_token { };
 
     bool authenticated = false;
-
-    std::string directory {};
 };
 
 }

=== modified file 'include/scope/query.h'
--- include/scope/query.h	2014-11-12 12:02:30 +0000
+++ include/scope/query.h	2014-12-04 15:01:59 +0000
@@ -36,8 +36,8 @@
 class Query: public unity::scopes::SearchQueryBase {
 public:
     Query(const unity::scopes::CannedQuery &query,
-            const unity::scopes::SearchMetadata &metadata,
-            api::Config::Ptr config);
+          const unity::scopes::SearchMetadata &metadata,
+          std::shared_ptr<unity::scopes::OnlineAccountClient> oa_client);
 
     ~Query() = default;
 

=== modified file 'include/scope/scope.h'
--- include/scope/scope.h	2014-11-12 10:49:13 +0000
+++ include/scope/scope.h	2014-12-04 15:01:59 +0000
@@ -19,11 +19,6 @@
 #ifndef SCOPE_SCOPE_H_
 #define SCOPE_SCOPE_H_
 
-#include <mutex>
-#include <condition_variable>
-
-#include <api/config.h>
-
 #include <unity/scopes/ScopeBase.h>
 #include <unity/scopes/OnlineAccountClient.h>
 #include <unity/scopes/QueryBase.h>
@@ -68,13 +63,6 @@
 
 protected:
     std::shared_ptr<unity::scopes::OnlineAccountClient> oa_client_;
-
-    std::mutex config_mutex_;
-    std::condition_variable config_cond_;
-    api::Config::Ptr config_;
-
-    void init_config();
-    void update_config();
 };
 
 }

=== modified file 'src/api/client.cpp'
--- src/api/client.cpp	2014-11-17 11:50:50 +0000
+++ src/api/client.cpp	2014-12-04 15:01:59 +0000
@@ -74,9 +74,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() {
@@ -90,27 +90,32 @@
 
     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);
-        if (config_->authenticated) {
+        if (config_.authenticated) {
             complete_parameters.emplace_back(
-                "oauth_token", config_->access_token);
+                "oauth_token", config_.access_token);
         } else {
-            complete_parameters.emplace_back("client_id", config_->client_id);
+            complete_parameters.emplace_back("client_id", config_.client_id);
         }
 
-        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);
-        configuration.header.add("User-Agent", config_->user_agent + " (gzip)");
+        configuration.header.add("User-Agent", config_.user_agent + " (gzip)");
         configuration.header.add("Accept-Encoding", "gzip");
 
         auto request = client_->head(configuration);
@@ -170,10 +175,60 @@
 
         return prom->get_future();
     }
+
+    std::string client_id() {
+        std::lock_guard<std::mutex> lock(config_mutex_);
+        update_config();
+        return config_.client_id;
+    }
+
+    bool authenticated() {
+        std::lock_guard<std::mutex> lock(config_mutex_);
+        update_config();
+        return config_.authenticated;
+    }
+
+    void update_config() {
+        config_ = Config();
+
+        if (getenv("NETWORK_SCOPE_APIROOT")) {
+            config_.apiroot = getenv("NETWORK_SCOPE_APIROOT");
+        }
+
+        if (getenv("SOUNDCLOUD_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_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;
+                break;
+            }
+        }
+
+        if (!config_.authenticated) {
+            std::cerr << "SoundCloud scope is unauthenticated" << std::endl;
+        } else {
+            std::cerr << "SoundCloud 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<deque<Track>> Client::search_tracks(const std::deque<std::pair<SP, std::string>> &parameters) {
@@ -226,7 +281,11 @@
     p->cancelled_ = true;
 }
 
-Config::Ptr Client::config() {
-    return p->config_;
+std::string Client::client_id() {
+    return p->client_id();
+}
+
+bool Client::authenticated() {
+    return p->authenticated();
 }
 

=== modified file 'src/scope/query.cpp'
--- src/scope/query.cpp	2014-11-17 11:50:50 +0000
+++ src/scope/query.cpp	2014-12-04 15:01:59 +0000
@@ -182,8 +182,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() {
@@ -203,7 +204,7 @@
         future<deque<Track>> stream_future;
         bool reading_stream = false;
         if (query_string.empty() && query.department_id().empty()) {
-            if (client_.config()->authenticated) {
+            if (client_.authenticated()) {
                 first_cat = reply->register_category(
                     "stream", _("Stream"), "",
                     sc::CategoryRenderer(SEARCH_CATEGORY_TEMPLATE));
@@ -271,7 +272,7 @@
 
     res["label"] = track.label_name();
     res["streamable"] = track.streamable();
-    res["stream-url"] = track.stream_url() + "?client_id=" + client_.config()->client_id;
+    res["stream-url"] = track.stream_url() + "?client_id=" + client_.client_id();
     res["purchase-url"] = track.purchase_url();
     res["video-url"] = track.video_url();
     res["waveform"] = track.waveform();

=== modified file 'src/scope/scope.cpp'
--- src/scope/scope.cpp	2014-11-13 09:25:33 +0000
+++ src/scope/scope.cpp	2014-12-04 15:01:59 +0000
@@ -21,46 +21,11 @@
 #include <scope/query.h>
 #include <scope/scope.h>
 
-#include <iostream>
-#include <sstream>
-#include <fstream>
-
 namespace sc = unity::scopes;
 using namespace std;
 using namespace api;
 using namespace scope;
 
-void Scope::init_config() {
-    config_ = make_shared<Config>();
-    config_->directory = scope_directory();
-    // Under test we set a different API root
-    char *apiroot = getenv("NETWORK_SCOPE_APIROOT");
-    if (apiroot) {
-        config_->apiroot = apiroot;
-    }
-}
-
-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;
-            break;
-        }
-    }
-
-    if (config_->authenticated) {
-        cerr << "SoundCloud scope is authenticated" << endl;
-    } else {
-        cerr << "SoundCloud scope is unauthenticated" << endl;
-    }
-    config_cond_.notify_all();
-}
-
 void Scope::start(string const&) {
     setlocale(LC_ALL, "");
     string translation_directory = ScopeBase::scope_directory()
@@ -70,24 +35,6 @@
     if (getenv("SOUNDCLOUD_SCOPE_IGNORE_ACCOUNTS") == nullptr) {
         oa_client_.reset(new sc::OnlineAccountClient(
             SCOPE_NAME, "sharing", SCOPE_ACCOUNTS_NAME));
-        oa_client_->set_service_update_callback(
-            [this](sc::OnlineAccountClient::ServiceStatus const &) {
-                this->update_config();
-            });
-
-        ///! TODO: We should only be waiting here if we know that
-        ///        there is at least one SoundCloud 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) {
-        init_config();
-        cerr << "SoundCloud scope is unauthenticated" << endl;
     }
 }
 
@@ -96,12 +43,7 @@
 
 sc::SearchQueryBase::UPtr Scope::search(const sc::CannedQuery &query,
                                         const sc::SearchMetadata &metadata) {
-    shared_ptr<Config> config;
-    {
-        std::lock_guard<std::mutex> lock(config_mutex_);
-        config = config_;
-    }
-    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,