Merge lp://staging/~xavi-garcia-mena/mediascanner2/ms-dbus-wal into lp://staging/mediascanner2

Looking quite nice. However one thing that is a bit concerning is that the tests duplicate a lot of code in the dbus and non-dbus cases. Would it be possible to consolidate these so that every test is run with common code. So something along the lines of this:

void testImplementation(MediaStore &writer, MediaStore &queryer) {
  // First set up
  writer.insert(whatevs);

  // Then query
  ASSERT_EQ(queryer.query("..."), result);
}

And then have the actual dbus and non-dbus test cases just do basically this:

void testWithoutDbus() {
  MediaStore local(READWRITE);

  testImplementation(local, local);
}

void testWithDbus() {
  MediaStore writer(READWRITE);
  MediaStore queryer(READONLY);

  testImplementation(writer, queryer);
}

Looks fine. Just a few niggles:

- the new variables have an underscore after their names, whereas the naming convention elsewere is not to have underscores
- when printing the text about error messages, please also print the URL of the bug in question
- in brokenfiles you wrap everything in a try/catch that just prints a generic error, why is this, AFAICR gtest will automatically fail any test that throws and prints the exception's error messag

I think we need to start talking to Jamie about the dbus/apparmor bits that this thing needs (also in music-app).

Hey Jussi,

what file do you mean when you say?
*when printing the text about error messages, please also print the URL of
the bug in question*

What do you mean to print the URL of the bug? When an exception is thrown?

I've erased the try... catch, as it was part of a test I did and changed
(but obviously forgot to erase the try..catch)

Thanks,

Xavi

On Mon, Feb 16, 2015 at 2:40 PM, Jussi Pakkanen <
<email address hidden>> wrote:

> Review: Needs Fixing
>
> Looks fine. Just a few niggles:
>
> - the new variables have an underscore after their names, whereas the
> naming convention elsewere is not to have underscores
> - when printing the text about error messages, please also print the URL
> of the bug in question
> - in brokenfiles you wrap everything in a try/catch that just prints a
> generic error, why is this, AFAICR gtest will automatically fail any test
> that throws and prints the exception's error messag
>
> I think we need to start talking to Jamie about the dbus/apparmor bits
> that this thing needs (also in music-app).
> --
>
> https://code.launchpad.net/~xavi-garcia-mena/mediascanner2/ms-dbus-wal/+merge/249346
> You are the owner of lp:~xavi-garcia-mena/mediascanner2/ms-dbus-wal.
>

Now we need to do the AppArmor bits and for that we need Jamie's help, so I subscribed him.

What we are trying to do here is the following:

Currently users of Mediascanner (media scopes and music-app) have read-only access to Mediascanner's media storage dir ~/.cache/mediascanner-2.0. This MR changes it so that they no longer may access the file directly, instead they call a small dbus server binary instead.

This means that we need the following Apparmor changes:

- remove read access from existing users
- grant them access to the dbus server instead
- create new daemon that is allowed to own the service name and has r/w access to the media directory

Does this seem like a suitable approach, security-wise?

FAILED: Continuous integration, rev:304
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~xavi-garcia-mena/mediascanner2/ms-dbus-wal/+merge/249346/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mediascanner2-ci/188/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-amd64-ci/25/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-armhf-ci/25/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mediascanner2-vivid-i386-ci/25/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mediascanner2-ci/188/rebuild

FAILED: Continuous integration, rev:304
https://jenkins.canonical.com/unity-api-1/job/lp-mediascanner2-ci/3/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/50/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/56/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-mediascanner2-ci/3/rebuild

FAILED: Continuous integration, rev:304
https://jenkins.canonical.com/unity-api-1/job/lp-mediascanner2-ci/4/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/227/console
    FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/235/console

Click here to trigger a rebuild:
https://jenkins.canonical.com/unity-api-1/job/lp-mediascanner2-ci/4/rebuild