[Merge] lp:~ricmm/media-hub/session-reattach-nonfixed into lp:media-hub

Thomas Voß thomas.voss at canonical.com
Fri Apr 17 04:00:48 UTC 2015


On Thu, Apr 16, 2015 at 5:14 PM, Jim Hodapp <jim.hodapp at canonical.com> wrote:
>
>
> Diff comments:
>
>> === modified file 'CMakeLists.txt'
>> --- CMakeLists.txt    2015-01-23 13:00:36 +0000
>> +++ CMakeLists.txt    2015-04-15 18:29:47 +0000
>> @@ -3,7 +3,7 @@
>>  project(ubuntu-media-hub)
>>
>>  set(UBUNTU_MEDIA_HUB_VERSION_MAJOR 3)
>> -set(UBUNTU_MEDIA_HUB_VERSION_MINOR 0)
>> +set(UBUNTU_MEDIA_HUB_VERSION_MINOR 1)
>>  set(UBUNTU_MEDIA_HUB_VERSION_PATCH 0)
>>
>>  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -fPIC -pthread")
>>
>> === modified file 'debian/changelog'
>> --- debian/changelog  2015-04-02 16:43:42 +0000
>> +++ debian/changelog  2015-04-15 18:29:47 +0000
>> @@ -1,3 +1,9 @@
>> +media-hub (3.1.0) UNRELEASED; urgency=medium
>> +
>> +  * Support for signalling player reconnection up the stack.
>> +
>> + -- Ricardo Mendoza <ricardo.mendoza at canonical.com>  Fri, 10 Apr 2015 18:20:32 +0200
>> +
>>  media-hub (3.0.0+15.04.20150402-0ubuntu1) vivid; urgency=medium
>>
>>    [ Jim Hodapp ]
>>
>> === modified file 'include/core/media/player.h'
>> --- include/core/media/player.h       2015-04-15 18:29:47 +0000
>> +++ include/core/media/player.h       2015-04-15 18:29:47 +0000
>> @@ -126,6 +126,9 @@
>>      Player& operator=(const Player&) = delete;
>>      bool operator==(const Player&) const = delete;
>>
>> +    virtual std::string uuid() const = 0;
>
> Yeah I would say this isn't hugely important though as the likely-hood of this happening is very slim. Also, we could just change it if that unlikely event occurred.
>

That's somewhat difficult as this is public facing API. Your call.

>> +    virtual void reconnect() = 0;
>> +
>>      virtual std::shared_ptr<TrackList> track_list() = 0;
>>      virtual PlayerKey key() const = 0;
>>
>>
>> === modified file 'include/core/media/service.h'
>> --- include/core/media/service.h      2014-09-30 06:51:15 +0000
>> +++ include/core/media/service.h      2015-04-15 18:29:47 +0000
>> @@ -42,9 +42,25 @@
>>      Service& operator=(const Service&) = delete;
>>      bool operator==(const Service&) const = delete;
>>
>> +    /** @brief Creates a session with the media-hub service. */
>>      virtual std::shared_ptr<Player> create_session(const Player::Configuration&) = 0;
>> +
>> +    /** @brief Detaches a UUID-identified session for later resuming. */
>> +    virtual void detach_session(const std::string& uuid, const Player::Configuration&) = 0;
>> +
>> +    /** @brief Reattaches to a UUID-identified session that is in detached state. */
>> +    virtual std::shared_ptr<Player> reattach_session(const std::string& uuid, const Player::Configuration&) = 0;
>> +
>> +    /** @brief Asks the service to destroy a session. The session is destroyed when the client exits. */
>> +    virtual void destroy_session(const std::string& uuid, const Player::Configuration&) = 0;
>> +
>> +    /** @brief Creates a fixed-named session with the media-hub service. */
>>      virtual std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&) = 0;
>> +
>> +    /** @brief Resumes a fixed-name session directly by player key. */
>>      virtual std::shared_ptr<Player> resume_session(Player::PlayerKey) = 0;
>> +
>> +    /** @brief Pauses sessions other than the supplied one. */
>>      virtual void pause_other_sessions(Player::PlayerKey) = 0;
>>
>>    protected:
>>
>> === modified file 'src/core/media/mpris/service.h'
>> --- src/core/media/mpris/service.h    2014-09-30 06:51:15 +0000
>> +++ src/core/media/mpris/service.h    2015-04-15 18:29:47 +0000
>> @@ -48,6 +48,42 @@
>>              }
>>          };
>>
>> +        struct DetachingSession
>> +        {
>> +            static const std::string& name()
>> +            {
>> +                static const std::string s
>> +                {
>> +                    "core.ubuntu.media.Service.Error.DetachingSession"
>> +                };
>> +                return s;
>> +            }
>> +        };
>> +
>> +        struct ReattachingSession
>> +        {
>> +            static const std::string& name()
>> +            {
>> +                static const std::string s
>> +                {
>> +                    "core.ubuntu.media.Service.Error.ReattachingSession"
>> +                };
>> +                return s;
>> +            }
>> +        };
>> +
>> +        struct DestroyingSession
>> +        {
>> +            static const std::string& name()
>> +            {
>> +                static const std::string s
>> +                {
>> +                    "core.ubuntu.media.Service.Error.DestroyingSession"
>> +                };
>> +                return s;
>> +            }
>> +        };
>> +
>>          struct CreatingFixedSession
>>          {
>>              static const std::string& name()
>> @@ -74,6 +110,9 @@
>>      };
>>
>>      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateSession, Service, 1000)
>> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(DetachSession, Service, 1000)
>> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(ReattachSession, Service, 1000)
>> +    DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(DestroySession, Service, 1000)
>>      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(CreateFixedSession, Service, 1000)
>>      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(ResumeSession, Service, 1000)
>>      DBUS_CPP_METHOD_WITH_TIMEOUT_DEF(PauseOtherSessions, Service, 1000)
>>
>> === modified file 'src/core/media/player_configuration.h'
>> --- src/core/media/player_configuration.h     2014-11-26 16:23:19 +0000
>> +++ src/core/media/player_configuration.h     2015-04-15 18:29:47 +0000
>> @@ -27,7 +27,7 @@
>>  // to the implementation in a way that is opaque to the client.
>>  struct core::ubuntu::media::Player::Configuration
>>  {
>> -    // Unique key for identifying the session.
>> +    // Unique key for identifying the session path.
>>      core::ubuntu::media::Player::PlayerKey key;
>>      // The bus connection to expose objects on.
>>      std::shared_ptr<core::dbus::Bus> bus;
>>
>> === modified file 'src/core/media/player_implementation.cpp'
>> --- src/core/media/player_implementation.cpp  2015-04-15 18:29:47 +0000
>> +++ src/core/media/player_implementation.cpp  2015-04-15 18:29:47 +0000
>> @@ -512,6 +512,18 @@
>>  }
>>
>>  template<typename Parent>
>> +std::string media::PlayerImplementation<Parent>::uuid() const
>> +{
>> +    // No impl for now, as not needed internally.
>> +}
>> +
>> +template<typename Parent>
>> +void media::PlayerImplementation<Parent>::reconnect()
>> +{
>> +    d->config.client_death_observer->register_for_death_notifications_with_key(d->config.key);
>> +}
>> +
>> +template<typename Parent>
>>  std::shared_ptr<media::TrackList> media::PlayerImplementation<Parent>::track_list()
>>  {
>>      return d->track_list;
>>
>> === modified file 'src/core/media/player_implementation.h'
>> --- src/core/media/player_implementation.h    2015-04-15 18:29:47 +0000
>> +++ src/core/media/player_implementation.h    2015-04-15 18:29:47 +0000
>> @@ -55,6 +55,9 @@
>>      PlayerImplementation(const Configuration& configuration);
>>      ~PlayerImplementation();
>>
>> +    virtual std::string uuid() const;
>> +    virtual void reconnect();
>> +
>>      virtual std::shared_ptr<TrackList> track_list();
>>      virtual Player::PlayerKey key() const;
>>
>>
>> === modified file 'src/core/media/player_stub.cpp'
>> --- src/core/media/player_stub.cpp    2015-04-15 18:29:47 +0000
>> +++ src/core/media/player_stub.cpp    2015-04-15 18:29:47 +0000
>> @@ -43,10 +43,12 @@
>>  struct media::PlayerStub::Private
>>  {
>>      Private(const std::shared_ptr<Service>& parent,
>> -            const std::shared_ptr<core::dbus::Object>& object
>> +            const std::shared_ptr<core::dbus::Object>& object,
>> +            const std::string uuid
>>              ) : parent(parent),
>>                  object(object),
>>                  key(object->invoke_method_synchronously<mpris::Player::Key, media::Player::PlayerKey>().value()),
>> +                uuid(uuid),
>>                  sink_factory(media::video::make_platform_default_sink_factory(key)),
>>                  properties
>>                  {
>> @@ -93,6 +95,7 @@
>>      std::shared_ptr<TrackList> track_list;
>>      dbus::Object::Ptr object;
>>      media::Player::PlayerKey key;
>> +    std::string uuid;
>>      media::video::SinkFactory sink_factory;
>>      struct
>>      {
>> @@ -209,8 +212,9 @@
>>
>>  media::PlayerStub::PlayerStub(
>>      const std::shared_ptr<Service>& parent,
>> -    const std::shared_ptr<core::dbus::Object>& object)
>> -        : d(new Private{parent, object})
>> +    const std::shared_ptr<core::dbus::Object>& object,
>> +    std::string uuid)
>> +        : d(new Private{parent, object, uuid})
>>  {
>>  }
>>
>> @@ -218,6 +222,16 @@
>>  {
>>  }
>>
>> +std::string media::PlayerStub::uuid() const
>> +{
>> +    return d->uuid;
>> +}
>> +
>> +void media::PlayerStub::reconnect()
>> +{
>> +    // No implementation
>> +}
>> +
>>  std::shared_ptr<media::TrackList> media::PlayerStub::track_list()
>>  {
>>      if (!d->track_list)
>>
>> === modified file 'src/core/media/player_stub.h'
>> --- src/core/media/player_stub.h      2015-04-15 18:29:47 +0000
>> +++ src/core/media/player_stub.h      2015-04-15 18:29:47 +0000
>> @@ -39,10 +39,14 @@
>>    public:
>>      explicit PlayerStub(
>>          const std::shared_ptr<Service>& parent,
>> -        const std::shared_ptr<core::dbus::Object>& object);
>> +        const std::shared_ptr<core::dbus::Object>& object,
>> +        std::string uuid = "");
>
> Indeed.
>
>>
>>      ~PlayerStub();
>>
>> +    virtual std::string uuid() const;
>> +    virtual void reconnect();
>> +
>>      virtual std::shared_ptr<TrackList> track_list();
>>      virtual PlayerKey key() const;
>>
>>
>> === modified file 'src/core/media/server/server.cpp'
>> --- src/core/media/server/server.cpp  2014-12-15 14:43:44 +0000
>> +++ src/core/media/server/server.cpp  2015-04-15 18:29:47 +0000
>> @@ -123,7 +123,7 @@
>>      {
>>          impl,
>>          player_store,
>> -
>> +        external_services
>>      });
>>
>>      std::thread service_worker
>>
>> === modified file 'src/core/media/service_skeleton.cpp'
>> --- src/core/media/service_skeleton.cpp       2015-04-15 18:29:47 +0000
>> +++ src/core/media/service_skeleton.cpp       2015-04-15 18:29:47 +0000
>> @@ -35,6 +35,10 @@
>>
>>  #include <core/posix/this_process.h>
>>
>> +#include <boost/uuid/uuid.hpp>
>> +#include <boost/uuid/uuid_generators.hpp>
>> +#include <boost/uuid/uuid_io.hpp>
>> +
>>  #include <map>
>>  #include <regex>
>>  #include <sstream>
>> @@ -50,17 +54,33 @@
>>  struct media::ServiceSkeleton::Private
>>  {
>>      Private(media::ServiceSkeleton* impl, const ServiceSkeleton::Configuration& config)
>> -        : impl(impl),
>> +        : request_context_resolver(media::apparmor::ubuntu::make_platform_default_request_context_resolver(config.external_services)),
>> +          impl(impl),
>>            object(impl->access_service()->add_object_for_path(
>>                       dbus::traits::Service<media::Service>::object_path())),
>> -          exported(impl->access_bus(), config.cover_art_resolver),
>> -          configuration(config)
>> +          configuration(config),
>> +          exported(impl->access_bus(), config.cover_art_resolver)
>>      {
>>          object->install_method_handler<mpris::Service::CreateSession>(
>>                      std::bind(
>>                          &Private::handle_create_session,
>>                          this,
>>                          std::placeholders::_1));
>> +        object->install_method_handler<mpris::Service::DetachSession>(
>> +                    std::bind(
>> +                        &Private::handle_detach_session,
>> +                        this,
>> +                        std::placeholders::_1));
>> +        object->install_method_handler<mpris::Service::ReattachSession>(
>> +                    std::bind(
>> +                        &Private::handle_reattach_session,
>> +                        this,
>> +                        std::placeholders::_1));
>> +        object->install_method_handler<mpris::Service::DestroySession>(
>> +                    std::bind(
>> +                        &Private::handle_destroy_session,
>> +                        this,
>> +                        std::placeholders::_1));
>>          object->install_method_handler<mpris::Service::CreateFixedSession>(
>>                      std::bind(
>>                          &Private::handle_create_fixed_session,
>> @@ -78,24 +98,26 @@
>>                          std::placeholders::_1));
>>      }
>>
>> -    std::pair<std::string, media::Player::PlayerKey> create_session_info()
>> +    std::tuple<std::string, media::Player::PlayerKey, std::string> create_session_info()
>>      {
>>          static unsigned int session_counter = 0;
>>
>>          unsigned int current_session = session_counter++;
>> +        boost::uuids::uuid uuid = gen();
>
> Right, it's not available directly, but uuid() is, which in my opinion would be good enough. And I don't think we need to mock the generator as it's available anywhere we need it and can safely rely on it being safe.
>

Again, your call. It's less a question if the generator is safe, but
more a question if it is actually called.

>>
>>          std::stringstream ss;
>>          ss << "/core/ubuntu/media/Service/sessions/" << current_session;
>>
>> -        return std::make_pair(ss.str(), media::Player::PlayerKey(current_session));
>> +        return std::make_tuple(ss.str(), media::Player::PlayerKey(current_session), to_string(uuid));
>>      }
>>
>>      void handle_create_session(const core::dbus::Message::Ptr& msg)
>>      {
>> -        auto  session_info = create_session_info();
>> +        auto session_info = create_session_info();
>>
>> -        dbus::types::ObjectPath op{session_info.first};
>> -        media::Player::PlayerKey key{session_info.second};
>> +        dbus::types::ObjectPath op{std::get<0>(session_info)};
>> +        media::Player::PlayerKey key{std::get<1>(session_info)};
>> +        std::string uuid{std::get<2>(session_info)};
>>
>>          media::Player::Configuration config
>>          {
>> @@ -104,11 +126,21 @@
>>              impl->access_service()->add_object_for_path(op)
>>          };
>>
>> +        std::cout << "Session created by request of: " << msg->sender() << ", uuid: " << uuid << ", path:" << op << std::endl;
>> +
>>          try
>>          {
>>              configuration.player_store->add_player_for_key(key, impl->create_session(config));
>> +            uuid_player_map.insert(std::make_pair(uuid, key));
>> +
>> +            request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, key, msg](const media::apparmor::ubuntu::Context& context)
>> +            {
>> +                fprintf(stderr, "%s():%d -- app_name='%s', attached\n", __func__, __LINE__, context.str().c_str());
>> +                player_owner_map.insert(std::make_pair(key, std::make_tuple(context.str(), true, msg->sender())));
>> +            });
>> +
>>              auto reply = dbus::Message::make_method_return(msg);
>> -            reply->writer() << op;
>> +            reply->writer() << std::make_tuple(op, uuid);
>>
>>              impl->access_bus()->send(reply);
>>          } catch(const std::runtime_error& e)
>> @@ -121,6 +153,171 @@
>>          }
>>      }
>>
>> +    void handle_detach_session(const core::dbus::Message::Ptr& msg)
>> +    {
>> +        try
>> +        {
>> +            std::string uuid;
>> +            msg->reader() >> uuid;
>> +
>> +            auto key = uuid_player_map.at(uuid);
>> +
>> +            if (player_owner_map.count(key) != 0) {
>> +                auto info = player_owner_map.at(key);
>> +                // Check if session is attached(1) and that the detachment
>> +                // request comes from the same peer(2) that created the session.
>> +                if (std::get<1>(info) && (std::get<2>(info) == msg->sender())) { // Player is attached
>> +                    std::get<1>(info) = false; // Detached
>> +                    std::get<2>(info).clear(); // Clear registered sender/peer
>> +                    auto player = configuration.player_store->player_for_key(key);
>> +                    player->lifetime().set(media::Player::Lifetime::resumable);
>> +                }
>> +            }
>> +
>> +            auto reply = dbus::Message::make_method_return(msg);
>> +            impl->access_bus()->send(reply);
>> +
>> +        } catch(const std::runtime_error& e)
>> +        {
>> +            auto reply = dbus::Message::make_error(
>> +                        msg,
>> +                        mpris::Service::Errors::DetachingSession::name(),
>> +                        e.what());
>> +            impl->access_bus()->send(reply);
>> +        }
>> +    }
>> +
>> +    void handle_reattach_session(const core::dbus::Message::Ptr& msg)
>> +    {
>> +        try
>> +        {
>> +            std::string uuid;
>> +            msg->reader() >> uuid;
>> +
>> +            if (uuid_player_map.count(uuid) != 0) {
>> +                auto key = uuid_player_map.at(uuid);
>> +                if (not configuration.player_store->has_player_for_key(key)) {
>> +                    auto reply = dbus::Message::make_error(
>> +                                msg,
>> +                                mpris::Service::Errors::ReattachingSession::name(),
>> +                                "Unable to locate player session");
>> +                    impl->access_bus()->send(reply);
>> +                    return;
>> +                }
>> +                std::stringstream ss;
>> +                ss << "/core/ubuntu/media/Service/sessions/" << key;
>> +                dbus::types::ObjectPath op{ss.str()};
>> +
>> +                request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, msg, key, op](const media::apparmor::ubuntu::Context& context)
>> +                {
>> +                    auto info = player_owner_map.at(key);
>> +                    fprintf(stderr, "%s():%d -- reattach app_name='%s', info='%s', '%s'\n", __func__, __LINE__, context.str().c_str(), std::get<0>(info).c_str(), std::get<2>(info).c_str());
>> +                    if (std::get<0>(info) == context.str()) {
>> +                        std::get<1>(info) = true; // Set to Attached
>> +                        std::get<2>(info) = msg->sender(); // Register new owner
>> +
>> +                        // Signal player reconnection
>> +                        auto player = configuration.player_store->player_for_key(key);
>> +                        player->reconnect();
>> +
>> +                        auto reply = dbus::Message::make_method_return(msg);
>> +                        reply->writer() << op;
>> +
>> +                        impl->access_bus()->send(reply);
>> +                    }
>> +                    else {
>> +                        auto reply = dbus::Message::make_error(
>> +                                    msg,
>> +                                    mpris::Service::Errors::ReattachingSession::name(),
>> +                                    "Invalid permissions for the requested session");
>> +                        impl->access_bus()->send(reply);
>> +                        return;
>> +                    }
>> +                });
>> +            }
>> +            else {
>> +               auto reply = dbus::Message::make_error(
>> +                           msg,
>> +                           mpris::Service::Errors::ReattachingSession::name(),
>> +                           "Invalid session");
>> +               impl->access_bus()->send(reply);
>> +               return;
>> +            }
>> +        } catch(const std::runtime_error& e)
>> +        {
>> +            auto reply = dbus::Message::make_error(
>> +                        msg,
>> +                        mpris::Service::Errors::ReattachingSession::name(),
>> +                        e.what());
>> +            impl->access_bus()->send(reply);
>> +        }
>> +    }
>> +
>> +    void handle_destroy_session(const core::dbus::Message::Ptr& msg)
>> +    {
>> +
>> +        try
>> +        {
>> +            std::string uuid;
>> +            msg->reader() >> uuid;
>> +
>> +            if (uuid_player_map.count(uuid) != 0) {
>> +                auto key = uuid_player_map.at(uuid);
>> +                if (not configuration.player_store->has_player_for_key(key)) {
>> +                    auto reply = dbus::Message::make_error(
>> +                                msg,
>> +                                mpris::Service::Errors::DestroyingSession::name(),
>> +                                "Unable to locate player session");
>> +                    impl->access_bus()->send(reply);
>> +                    return;
>> +                }
>> +
>> +                // Remove control entries from the map, at this point
>> +                // the session is no longer usable.
>> +                uuid_player_map.erase(uuid);
>> +
>> +                request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, msg, key](const media::apparmor::ubuntu::Context& context)
>> +                {
>> +                    auto info = player_owner_map.at(key);
>> +                    fprintf(stderr, "%s():%d -- Destroying app_name='%s', info='%s', '%s'\n", __func__, __LINE__, context.str().c_str(), std::get<0>(info).c_str(), std::get<2>(info).c_str());
>> +                    if (std::get<0>(info) == context.str()) {
>> +                        player_owner_map.erase(key);
>> +
>> +                        // Reset lifecycle to non-resumable on the now-abandoned session
>> +                        auto player = configuration.player_store->player_for_key(key);
>> +                        player->lifetime().set(media::Player::Lifetime::normal);
>> +
>> +                        auto reply = dbus::Message::make_method_return(msg);
>> +                        impl->access_bus()->send(reply);
>> +                    }
>> +                    else {
>> +                        auto reply = dbus::Message::make_error(
>> +                                    msg,
>> +                                    mpris::Service::Errors::DestroyingSession::name(),
>> +                                    "Invalid permissions for the requested session");
>> +                        impl->access_bus()->send(reply);
>> +                        return;
>> +                    }
>> +                });
>> +            }
>> +            else {
>> +               auto reply = dbus::Message::make_error(
>> +                           msg,
>> +                           mpris::Service::Errors::DestroyingSession::name(),
>> +                           "Invalid session");
>> +               impl->access_bus()->send(reply);
>> +               return;
>> +            }
>> +        } catch(const std::runtime_error& e)
>> +        {
>> +            auto reply = dbus::Message::make_error(
>> +                        msg,
>> +                        mpris::Service::Errors::DestroyingSession::name(),
>> +                        e.what());
>> +            impl->access_bus()->send(reply);
>> +        }
>> +    }
>> +
>>      void handle_create_fixed_session(const core::dbus::Message::Ptr& msg)
>>      {
>>          try
>> @@ -132,8 +329,8 @@
>>                  // Create new session
>>                  auto  session_info = create_session_info();
>>
>> -                dbus::types::ObjectPath op{session_info.first};
>> -                media::Player::PlayerKey key{session_info.second};
>> +                dbus::types::ObjectPath op{std::get<0>(session_info)};
>> +                media::Player::PlayerKey key{std::get<1>(session_info)};
>>
>>                  media::Player::Configuration config
>>                  {
>> @@ -147,7 +344,6 @@
>>
>>                  configuration.player_store->add_player_for_key(key, session);
>>
>> -
>>                  named_player_map.insert(std::make_pair(name, key));
>>
>>                  auto reply = dbus::Message::make_method_return(msg);
>> @@ -231,6 +427,7 @@
>>          impl->access_bus()->send(reply);
>>      }
>>
>> +    media::apparmor::ubuntu::RequestContextResolver::Ptr request_context_resolver;
>>      media::ServiceSkeleton* impl;
>>      dbus::Object::Ptr object;
>>
>> @@ -238,6 +435,14 @@
>>      ServiceSkeleton::Configuration configuration;
>>      // We map named/fixed player instances to their respective keys.
>>      std::map<std::string, media::Player::PlayerKey> named_player_map;
>> +    // We map UUIDs to their respective keys.
>> +    std::map<std::string, media::Player::PlayerKey> uuid_player_map;
>> +    // We keep a list of keys and their respective owners and states.
>> +    // value: (owner context, attached state, attached dbus name)
>> +    std::map<media::Player::PlayerKey, std::tuple<std::string, bool, std::string>> player_owner_map;
>> +
>> +    boost::uuids::random_generator gen;
>> +
>>      // We expose the entire service as an MPRIS player.
>>      struct Exported
>>      {
>>
>> === modified file 'src/core/media/service_skeleton.h'
>> --- src/core/media/service_skeleton.h 2014-12-15 14:43:44 +0000
>> +++ src/core/media/service_skeleton.h 2015-04-15 18:29:47 +0000
>> @@ -19,6 +19,8 @@
>>  #ifndef CORE_UBUNTU_MEDIA_SERVICE_SKELETON_H_
>>  #define CORE_UBUNTU_MEDIA_SERVICE_SKELETON_H_
>>
>> +#include "apparmor/ubuntu.h"
>> +
>>  #include <core/media/service.h>
>>
>>  #include "cover_art_resolver.h"
>> @@ -43,6 +45,7 @@
>>      {
>>          std::shared_ptr<Service> impl;
>>          KeyedPlayerStore::Ptr player_store;
>> +        helper::ExternalServices& external_services;
>>          CoverArtResolver cover_art_resolver;
>>      };
>>
>>
>> === modified file 'src/core/media/service_stub.cpp'
>> --- src/core/media/service_stub.cpp   2014-10-23 14:42:59 +0000
>> +++ src/core/media/service_stub.cpp   2015-04-15 18:29:47 +0000
>> @@ -58,7 +58,7 @@
>>  std::shared_ptr<media::Player> media::ServiceStub::create_session(const media::Player::Configuration&)
>>  {
>>      auto op = d->object->invoke_method_synchronously<mpris::Service::CreateSession,
>> -         dbus::types::ObjectPath>();
>> +         std::tuple<dbus::types::ObjectPath, std::string>>();
>>
>>      if (op.is_error())
>>          throw std::runtime_error("Problem creating session: " + op.error());
>> @@ -66,8 +66,43 @@
>>      return std::shared_ptr<media::Player>(new media::PlayerStub
>>      {
>>          shared_from_this(),
>> -        access_service()->object_for_path(op.value())
>> -    });
>> +        access_service()->object_for_path(std::get<0>(op.value())),
>> +        std::get<1>(op.value())
>> +    });
>> +}
>> +
>> +void media::ServiceStub::detach_session(const std::string& uuid, const media::Player::Configuration&)
>> +{
>> +    auto op = d->object->invoke_method_synchronously<mpris::Service::DetachSession,
>> +         void>(uuid);
>> +
>> +    if (op.is_error())
>> +        throw std::runtime_error("Problem detaching session: " + op.error());
>> +}
>> +
>> +std::shared_ptr<media::Player> media::ServiceStub::reattach_session(const std::string& uuid, const media::Player::Configuration&)
>> +{
>> +    auto op = d->object->invoke_method_synchronously<mpris::Service::ReattachSession,
>> +         dbus::types::ObjectPath>(uuid);
>> +
>> +    if (op.is_error())
>> +        throw std::runtime_error("Problem reattaching session: " + op.error());
>> +
>> +    return std::shared_ptr<media::Player>(new media::PlayerStub
>> +    {
>> +        shared_from_this(),
>> +        access_service()->object_for_path(op.value()),
>> +        uuid
>> +    });
>> +}
>> +
>> +void media::ServiceStub::destroy_session(const std::string& uuid, const media::Player::Configuration&)
>> +{
>> +    auto op = d->object->invoke_method_synchronously<mpris::Service::DestroySession,
>> +         void>(uuid);
>> +
>> +    if (op.is_error())
>> +        throw std::runtime_error("Problem destroying session: " + op.error());
>>  }
>>
>>  std::shared_ptr<media::Player> media::ServiceStub::create_fixed_session(const std::string& name, const media::Player::Configuration&)
>>
>> === modified file 'src/core/media/service_stub.h'
>> --- src/core/media/service_stub.h     2014-10-23 14:42:59 +0000
>> +++ src/core/media/service_stub.h     2015-04-15 18:29:47 +0000
>> @@ -40,6 +40,9 @@
>>      ~ServiceStub();
>>
>>      std::shared_ptr<Player> create_session(const Player::Configuration&);
>> +    void detach_session(const std::string& uuid, const Player::Configuration&);
>> +    std::shared_ptr<Player> reattach_session(const std::string& uuid, const Player::Configuration&);
>> +    void destroy_session(const std::string& uuid, const Player::Configuration&);
>>      std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&);
>>      std::shared_ptr<Player> resume_session(Player::PlayerKey key);
>>      void pause_other_sessions(Player::PlayerKey key);
>>
>> === modified file 'tests/test-track-list/test_track_list.cpp'
>> --- tests/test-track-list/test_track_list.cpp 2015-04-15 18:29:47 +0000
>> +++ tests/test-track-list/test_track_list.cpp 2015-04-15 18:29:47 +0000
>> @@ -44,7 +44,7 @@
>>  {
>>  }
>>
>> -void media::TestTrackList::create_new_player_session()
>> +std::string media::TestTrackList::create_new_player_session()
>>  {
>>      try {
>>          m_hubPlayerSession = m_hubService->create_session(media::Player::Client::default_configuration());
>> @@ -59,9 +59,51 @@
>>      catch (std::runtime_error &e) {
>>          cerr << "FATAL: Failed to retrieve the current player's TrackList: " << e.what() << endl;
>>      }
>> -}
>> -
>> -void media::TestTrackList::destroy_player_session()
>> +
>> +    std::string uuid;
>> +    try {
>> +        uuid.assign(m_hubPlayerSession->uuid());
>> +    }
>> +    catch (std::runtime_error &e) {
>> +        cerr << "FATAL: Failed to retrieve the current player's uuid: " << e.what() << endl;
>> +    }
>> +
>> +    cout << "Connected to session " << uuid << endl;
>> +    return uuid;
>> +}
>> +
>> +void media::TestTrackList::detach_player_session(const std::string &uuid)
>> +{
>> +    try {
>> +        m_hubService->detach_session(uuid, media::Player::Client::default_configuration());
>> +    }
>> +    catch (std::runtime_error &e) {
>> +        cerr << "FATAL: Failed to detach the media-hub player session: " << e.what() << endl;
>> +    }
>> +
>> +    cout << "Detached session " << uuid << endl;
>> +}
>> +
>> +void media::TestTrackList::reattach_player_session(const std::string &uuid)
>> +{
>> +    try {
>> +        m_hubPlayerSession = m_hubService->reattach_session(uuid, media::Player::Client::default_configuration());
>> +    }
>> +    catch (std::runtime_error &e) {
>> +        cerr << "FATAL: Failed to reattach the media-hub player session: " << e.what() << endl;
>> +    }
>> +
>> +    try {
>> +        m_hubTrackList = m_hubPlayerSession->track_list();
>> +    }
>> +    catch (std::runtime_error &e) {
>> +        cerr << "FATAL: Failed to retrieve the current player's TrackList: " << e.what() << endl;
>> +    }
>> +
>> +    cout << "Re-connected to session " << uuid << endl;
>> +}
>> +
>> +void media::TestTrackList::destroy_player_session(const std::string &uuid)
>>  {
>>      // TODO: explicitly add a destroy session to the Service class after ricmm lands his new creation_session
>>      // that returns a session ID. This will allow me to clear the tracklist after each test.
>> @@ -121,8 +163,28 @@
>>      //destroy_player_session();
>>  }
>>
>> +void media::TestTrackList::test_tracklist_resume(const std::string &uri1, const std::string &uri2, const std::string &uuid)
>> +{
>> +    cout << "--> Running test: test_tracklist_resume" << std::endl;
>> +
>> +    add_track(uri1);
>> +    add_track(uri2);
>> +
>> +    int initial_size = m_hubTrackList->tracks().get().size();
>> +    cout << "Tracklist size: " << initial_size << endl;
>> +    detach_player_session(uuid);
>> +    reattach_player_session(uuid);
>> +    cout << "Tracklist size: " << m_hubTrackList->tracks().get().size() << endl;
>> +
>> +    m_hubPlayerSession->play();
>> +
>> +    if (initial_size != m_hubTrackList->tracks().get().size())
>> +        cout << "Tracklist sizes are different, error in resuming" << endl;
>> +}
>> +
>>  void media::TestTrackList::test_ensure_tracklist_is_not_empty(const std::string &uri1, const std::string &uri2)
>>  {
>> +    //create_new_player_session();
>>      cout << "--> Running test: test_ensure_tracklist_is_not_empty" << std::endl;
>>
>>      add_track(uri1);
>> @@ -335,6 +397,11 @@
>>          tracklist->test_shuffle(argv[1], argv[2], argv[3]);
>>          tracklist->test_remove_track(argv[1], argv[2], argv[3]);
>>      }
>> +    else if (argc == 5)
>> +    {
>> +        std::string uuid = tracklist->create_new_player_session();
>> +        tracklist->test_tracklist_resume(argv[1], argv[2], uuid);
>> +    }
>>      else
>>      {
>>          cout << "Can't test TrackList, no track(s) specified." << endl;
>>
>> === modified file 'tests/test-track-list/test_track_list.h'
>> --- tests/test-track-list/test_track_list.h   2015-04-15 18:29:47 +0000
>> +++ tests/test-track-list/test_track_list.h   2015-04-15 18:29:47 +0000
>> @@ -45,14 +45,19 @@
>>      TestTrackList();
>>      ~TestTrackList();
>>
>> -    void create_new_player_session();
>> -    void destroy_player_session();
>> +    std::string create_new_player_session();
>> +    void detach_player_session(const std::string &uuid);
>> +    void reattach_player_session(const std::string &uuid);
>> +    void destroy_player_session(const std::string &uuid);
>>
>>      void add_track(const std::string &uri, bool make_current = false);
>>
>>      // Takes in one or two files for playback, adds it/them to the TrackList, and plays
>>      void test_basic_playback(const std::string &uri1, const std::string &uri2 = std::string{});
>>
>> +    // Takes two uris and confirms that they remain after a detach/reattach
>> +    void test_tracklist_resume(const std::string &uri1, const std::string &uri2, const std::string &uuid);
>> +
>>      void test_ensure_tracklist_is_not_empty(const std::string &uri1, const std::string &uri2 = std::string{});
>>
>>      // Takes in one or two files for playback, adds it/them to the TrackList, plays and makes sure
>>
>
>
> --
> https://code.launchpad.net/~ricmm/media-hub/session-reattach-nonfixed/+merge/256101
> You are reviewing the proposed merge of lp:~ricmm/media-hub/session-reattach-nonfixed into lp:media-hub.

-- 
https://code.launchpad.net/~ricmm/media-hub/session-reattach-nonfixed/+merge/256101
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/media-hub/tracklist-cli.



More information about the Ubuntu-reviews mailing list