[Merge] lp:~phablet-team/media-hub/add-tracks-batch into lp:media-hub/stable

[Alfonso Sanchez-Beato]

Diff comments:

> 
> === modified file 'src/core/media/track_list_skeleton.cpp'
> --- src/core/media/track_list_skeleton.cpp	2015-09-28 15:31:46 +0000
> +++ src/core/media/track_list_skeleton.cpp	2015-10-22 19:06:46 +0000
> @@ -113,6 +114,36 @@
>          });
>      }
>  
> +    void handle_add_tracks_with_uri_at(const core::dbus::Message::Ptr& msg)
> +    {
> +        std::cout << "*** " << __PRETTY_FUNCTION__ << std::endl;
> +        request_context_resolver->resolve_context_for_dbus_name_async(msg->sender(), [this, msg](const media::apparmor::ubuntu::Context& context)

It's not only that 80 is the old standard for printers, it is also readability. Newspapers and magazines have columns for a reason: reading long lines is hard for humans. Of course newspapers are antiquated too, but our brain and eyes have not evolved jointly with high resolution monitors ;-). Said this, I am happy with lines <= 100 provided that we enforce that.

> +        {
> +            ContainerURI uris; media::Track::Id after;
> +            msg->reader() >> uris >> after;
> +
> +            media::apparmor::ubuntu::RequestAuthenticator::Result result;
> +            for (const auto uri : uris)
> +            {
> +                // Make sure the client has adequate apparmor permissions to open the URI
> +                result = request_authenticator->authenticate_open_uri_request(context, uri);

Strictly speaking this is not verifying the dbus API correctness, but the semantics of the arguments. Nonetheless this was more of a suggestion, I think we are fine if we do not add any tracks when one of them has an issue with apparmor, provided that we return an error in the DBus reply.

> +                if (not std::get<0>(result))
> +                {
> +                    std::cerr << "Warning: Not adding track " << uri <<
> +                        " to TrackList because of inadequate client apparmor permissions." << std::endl;
> +                    break;
> +                }
> +            }
> +
> +            auto reply = dbus::Message::make_method_return(msg);
> +            // Only add the track to the TrackList if it passes the apparmor permissions check
> +            if (std::get<0>(result))
> +                impl->add_tracks_with_uri_at(uris, after);
> +
> +            bus->send(reply);
> +        });
> +    }
> +
>      void handle_remove_track(const core::dbus::Message::Ptr& msg)
>      {
>          media::Track::Id track;


-- 
https://code.launchpad.net/~phablet-team/media-hub/add-tracks-batch/+merge/275426
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub/stable.