Merge lp:~marcustomlinson/unity-scopes-api/remove_oa_pub_sub into lp:unity-scopes-api/devel

[Marcus Tomlinson]

1. Removed SIGNON_SESSION_DATA_UI_POLICY specification (use default always) as this is actually a bug in OA not scopes (Bug #1374394).

2. Removed pub-sub network for authentication as this is no longer needed when using the default UI policy.

3. Updated test.

[lp:~marcustomlinson/unity-scopes-api/remove_oa_pub_sub]

268. By Marcus Tomlinson on 2014-10-09

Reverted staging specific change

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:267
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/1211/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-amd64-ci/769
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/764
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/764/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-i386-ci/763

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/1211/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:268
http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-ci/1212/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-amd64-ci/770
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/765
        deb: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-armhf-ci/765/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-scopes-api-devel-utopic-i386-ci/764

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scopes-api-devel-ci/1212/rebuild

review: Approve (continuous-integration)

[Paweł Stołowski (stolowski)]

LGTM.

review: Approve

=== modified file 'include/unity/scopes/OnlineAccountClient.h'
--- include/unity/scopes/OnlineAccountClient.h	2014-10-06 07:53:41 +0000
+++ include/unity/scopes/OnlineAccountClient.h	2014-10-09 16:20:29 +0000
@@ -80,8 +80,6 @@
     {
         RunInExternalMainLoop,    ///< An external main loop already exists and is running.
         CreateInternalMainLoop,   ///< An external main loop does not exist.
-        RunInExternalUiMainLoop,  ///< An external UI main loop exists and is running (Intended for
-                                  ///  shell use only. A scope should not be running a UI main loop).
     };
 
     /**

=== modified file 'include/unity/scopes/internal/OnlineAccountClientImpl.h'
--- include/unity/scopes/internal/OnlineAccountClientImpl.h	2014-10-06 07:53:41 +0000
+++ include/unity/scopes/internal/OnlineAccountClientImpl.h	2014-10-09 16:20:29 +0000
@@ -19,7 +19,6 @@
 #ifndef UNITY_SCOPES_INTERNAL_ONLINEACCOUNTCLIENTIMPL_H
 #define UNITY_SCOPES_INTERNAL_ONLINEACCOUNTCLIENTIMPL_H
 
-#include <unity/scopes/internal/MiddlewareBase.h>
 #include <unity/scopes/OnlineAccountClient.h>
 
 #include <libaccounts-glib/accounts-glib.h>
@@ -88,7 +87,6 @@
 
     std::shared_ptr<AgManager> manager();
     std::string service_name();
-    OnlineAccountClient::MainLoopSelect main_loop_select();
     std::shared_ptr<GMainContext> main_loop_context();
 
     void callback(AccountInfo const* info, std::string const& error = "");
@@ -119,12 +117,7 @@
     std::shared_ptr<AgManager> manager_;
     std::map<AgAccountId, std::shared_ptr<AccountInfo>> accounts_;
 
-    MiddlewareBase::SPtr mw_;
-    MWPublisher::UPtr auth_publisher_;
-    MWSubscriber::UPtr auth_subscriber_;
-
     void main_loop_thread();
-    void auth_callback(std::string const& details_json);
 };
 
 } // namespace internal

=== modified file 'src/scopes/internal/OnlineAccountClientImpl.cpp'
--- src/scopes/internal/OnlineAccountClientImpl.cpp	2014-10-06 07:53:41 +0000
+++ src/scopes/internal/OnlineAccountClientImpl.cpp	2014-10-09 16:20:29 +0000
@@ -18,10 +18,6 @@
 
 #include <unity/scopes/internal/OnlineAccountClientImpl.h>
 
-#include <unity/scopes/internal/JsonCppNode.h>
-#include <unity/scopes/internal/MiddlewareFactory.h>
-#include <unity/scopes/internal/RuntimeConfig.h>
-#include <unity/scopes/internal/RuntimeImpl.h>
 #include <unity/UnityExceptions.h>
 
 #include <iostream>
@@ -93,40 +89,6 @@
     return service_status;
 }
 
-static std::string details_to_json(OnlineAccountClient::ServiceStatus const& details)
-{
-    VariantMap vm;
-    vm["account_id"] = details.account_id;
-    vm["service_enabled"] = details.service_enabled;
-    vm["service_authenticated"] = details.service_authenticated;
-    vm["client_id"] = details.client_id;
-    vm["client_secret"] = details.client_secret;
-    vm["access_token"] = details.access_token;
-    vm["token_secret"] = details.token_secret;
-    vm["error"] = details.error;
-
-    Variant var(vm);
-    JsonCppNode node(var);
-    return node.to_json_string();
-}
-
-static OnlineAccountClient::ServiceStatus json_to_details(std::string const& json)
-{
-    OnlineAccountClient::ServiceStatus details;
-    JsonCppNode node(json);
-
-    details.account_id = node.get_node("account_id")->as_int();
-    details.service_enabled = node.get_node("service_enabled")->as_bool();
-    details.service_authenticated = node.get_node("service_authenticated")->as_bool();
-    details.client_id = node.get_node("client_id")->as_string();
-    details.client_secret = node.get_node("client_secret")->as_string();
-    details.access_token = node.get_node("access_token")->as_string();
-    details.token_secret = node.get_node("token_secret")->as_string();
-    details.error = node.get_node("error")->as_string();
-
-    return details;
-}
-
 static void clear_session(AccountInfo* info)
 {
     if (info->session)
@@ -198,24 +160,8 @@
         }
 
         // Get authorization parameters then attempt to signon
-        GVariantBuilder builder;
-        g_variant_builder_init(&builder, G_VARIANT_TYPE_VARDICT);
-
-        if (info->account_client->main_loop_select() == OnlineAccountClient::RunInExternalUiMainLoop)
-        {
-            g_variant_builder_add(&builder, "{sv}",
-                                  SIGNON_SESSION_DATA_UI_POLICY,
-                                  g_variant_new_int32(SIGNON_POLICY_DEFAULT));  // LCOV_EXCL_LINE
-        }
-        else
-        {
-            g_variant_builder_add(&builder, "{sv}",
-                                  SIGNON_SESSION_DATA_UI_POLICY,
-                                  g_variant_new_int32(SIGNON_POLICY_NO_USER_INTERACTION));
-        }
-
         info->auth_params.reset(
-            g_variant_ref_sink(ag_auth_data_get_login_parameters(auth_data.get(), g_variant_builder_end(&builder))), free_variant);
+            g_variant_ref_sink(ag_auth_data_get_login_parameters(auth_data.get(), nullptr)), free_variant);
 
         // Start signon process
         signon_auth_session_process_async(info->session.get(),
@@ -290,25 +236,6 @@
     , main_loop_select_(main_loop_select)
     , main_loop_is_running_(main_loop_select != OnlineAccountClient::CreateInternalMainLoop)
 {
-    // Set up authentication pub/sub
-    std::string oa_id = provider_name + ":" + service_type + ":" + service_name;
-    RuntimeImpl::UPtr rt = RuntimeImpl::create(oa_id);
-    MiddlewareFactory mw_factory(rt.get());
-    RuntimeConfig rt_config("");
-
-    mw_ = mw_factory.create(oa_id, rt_config.default_middleware(), rt_config.default_middleware_configfile());
-    if (main_loop_select == OnlineAccountClient::RunInExternalUiMainLoop)
-    {
-        // If this client was created by the shell, set up as the publisher
-        auth_publisher_ = mw_->create_publisher(oa_id);
-    }
-    else
-    {
-        // If this client was created by a scope, set up as a subscriber
-        auth_subscriber_ = mw_->create_subscriber(oa_id);
-        auth_subscriber_->message_received().connect(std::bind(&OnlineAccountClientImpl::auth_callback, this, std::placeholders::_1));
-    }
-
     // If we are responsible for the main loop
     if (main_loop_select_ == OnlineAccountClient::CreateInternalMainLoop)
     {
@@ -576,11 +503,6 @@
     return service_name_;
 }
 
-OnlineAccountClient::MainLoopSelect OnlineAccountClientImpl::main_loop_select()
-{
-    return main_loop_select_;
-}
-
 std::shared_ptr<GMainContext> OnlineAccountClientImpl::main_loop_context()
 {
     return main_loop_context_;
@@ -589,14 +511,9 @@
 void OnlineAccountClientImpl::callback(AccountInfo const* info, std::string const& error)
 {
     std::lock_guard<std::mutex> lock(callback_mutex_);
-    auto service_status = info_to_details(info, error);
-    if (service_status.service_authenticated && auth_publisher_)
-    {
-        auth_publisher_->send_message(details_to_json(service_status));
-    }
     if (callback_)
     {
-        callback_(service_status);
+        callback_(info_to_details(info, error));
     }
 }
 
@@ -667,31 +584,6 @@
     // LCOV_EXCL_STOP
 }
 
-void OnlineAccountClientImpl::auth_callback(std::string const& details_json)
-{
-    OnlineAccountClient::ServiceStatus details = json_to_details(details_json);
-
-    // Update account info
-    {
-        std::lock_guard<std::mutex> lock(mutex_);
-        auto info_it = accounts_.find(details.account_id);
-        if (info_it == accounts_.end())
-        {
-            return;
-        }
-        auto info = info_it->second;
-
-        GVariantDict dict;
-        g_variant_dict_init(&dict, nullptr);
-        g_variant_dict_insert(&dict, "AccessToken", "s", details.access_token.c_str());
-        g_variant_dict_insert(&dict, "TokenSecret", "s", details.token_secret.c_str());
-        info->session_data.reset(g_variant_ref_sink(g_variant_dict_end(&dict)), free_variant);
-    }
-
-    std::lock_guard<std::mutex> lock(callback_mutex_);
-    callback_(details);
-}
-
 }  // namespace internal
 
 }  // namespace scopes

=== modified file 'test/gtest/scopes/OnlineAccountClient/OnlineAccountClient_test.cpp'
--- test/gtest/scopes/OnlineAccountClient/OnlineAccountClient_test.cpp	2014-10-09 01:16:53 +0000
+++ test/gtest/scopes/OnlineAccountClient/OnlineAccountClient_test.cpp	2014-10-09 16:20:29 +0000
@@ -521,17 +521,8 @@
     EXPECT_TRUE(statuses[0].service_enabled);
 }
 
-TEST_F(OnlineAccountClientTestNoMainLoop, pub_sub_authentication)
+TEST_F(OnlineAccountClientTestNoMainLoop, authentication)
 {
-    std::shared_ptr<OnlineAccountClient> shell_oa_client;
-    shell_oa_client.reset(new OnlineAccountClient("TestService", "sharing", "TestProvider", OnlineAccountClient::RunInExternalUiMainLoop));
-
-    std::shared_ptr<OnlineAccountClient> scope_oa_client;
-    scope_oa_client.reset(new OnlineAccountClient("TestService", "sharing", "TestProvider", OnlineAccountClient::RunInExternalMainLoop));
-
-    create_account();
-    scope_oa_client->refresh_service_statuses();
-
     std::shared_ptr<AccountInfo> info(new AccountInfo);
     info->account_id = 1;
     info->service_enabled = true;
@@ -550,9 +541,9 @@
         info->session_data.reset(g_variant_ref_sink(g_variant_dict_end(&dict)), safe_g_variant_free_);
     }
 
-    // Invoke callback on shell_oa_client which should invoke a callback on scope_oa_client via pub/sub
-    scope_oa_client->set_service_update_callback(std::bind(&OnlineAccountClientTest::service_update_auth, this, std::placeholders::_1));
-    invoke_callback(shell_oa_client, info.get(), "not really an error, but just to test");
+    // Manually invoke the callback with a valid access token, which should result in service_authenticated = true
+    oa_client()->set_service_update_callback(std::bind(&OnlineAccountClientTest::service_update_auth, this, std::placeholders::_1));
+    invoke_callback(oa_client(), info.get(), "not really an error, but just to test");
     wait_for_service_update();
 }