Storage Framework

Merge lp:~michihenning/storage-framework/qml-improvements into lp:storage-framework/devel

qml-improvements
Merge into devel

Proposed by Michi Henning on 2016-11-02

Status:	Merged
Approved by:	Michi Henning on 2016-11-03
Approved revision:	86
Merged at revision:	88
Proposed branch:	lp:~michihenning/storage-framework/qml-improvements
Merge into:	lp:storage-framework/devel
Diff against target:	665 lines (+167/-193) 14 files modified demo/demo.qml (+3/-1) include/unity/storage/qt/Account.h (+9/-8) include/unity/storage/qt/AccountsJob.h (+3/-1) include/unity/storage/qt/Item.h (+22/-22) include/unity/storage/qt/MetadataKeys.h.THIS (+0/-37) include/unity/storage/qt/Runtime.h (+3/-3) include/unity/storage/qt/internal/AccountImpl.h (+12/-12) include/unity/storage/qt/internal/AccountsJobImpl.h (+1/-0) plugins/Ubuntu/StorageFramework/plugin.cpp (+1/-1) src/qt/Account.cpp (+13/-13) src/qt/AccountsJob.cpp (+7/-0) src/qt/internal/AccountImpl.cpp (+46/-46) src/qt/internal/AccountsJobImpl.cpp (+11/-1) tests/remote-client/remote-client_test.cpp (+36/-48)
To merge this branch:	bzr merge lp:~michihenning/storage-framework/qml-improvements
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Michi Henning (community)			Approve on 2016-11-03
unity-api-1-bot	continuous-integration		Approve on 2016-11-03
James Henstridge		2016-11-02	Approve on 2016-11-03
Review via email: mp+309809@code.launchpad.net

Commit message

Renamed Account accessors to reflect what we get from online accounts.
A few fixes for QML:
- accounts property is no tied to a method that returns a QVariantList
- Changed to fully-qualified type names for Q_INVOKABLE methods, so QML
doesn't complain about an unknown return type.

Description of the change

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-02:

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

review: Approve (continuous-integration)

Revision history for this message

James Henstridge (jamesh) wrote on 2016-11-02:

This all looks a bit messed up.

In AccountImpl, we have:

     static Account make_account(std::shared_ptr<RuntimeImpl> const& runtime_impl,
                                 QString const& bus_name,
                                 QString const& object_path,
                                 QString const& id,
                                 QString const& service_id,
                                 QString const& display_name);

Which is being invoked in AccountJobImpl as:

            accounts_.append(AccountImpl::make_account(runtime,
                                                       bus_name,
                                                       object_path,
                                                       a->serviceId(),
                                                       "",
                                                       a->displayName()));

That is, it is setting id=a->serviceId(), service_id="", display_name=a->displayName(). This would explain my confusion when discussing this with you yesterday.

This looks like it is a transposition of the 4th and 5th arguments relative to the v1 API. So before merging this, it'd be good to decide what information exactly we're actually trying expose here.

review: Needs Information

Revision history for this message

Michi Henning (michihenning) wrote on 2016-11-02:

I fixed that after the hangout last night. Are you sure that you were looking at the correct branch? Looking through the diff, I'm seeing the correct call:

            accounts_.append(AccountImpl::make_account(runtime,
                                                       bus_name,
                                                       object_path,
                                                       QString::number(a->id()),
                                                       a->serviceId(),
                                                       a->displayName()));

The change is around line 441 of the diff.

In terms of what to expose, the ID is not particularly useful, so we could drop that. The ID isn't stable anyway (for example, if I delete an account and then re-add it).

The service ID and display name we have to keep, I think.

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-03:

FAILED: Continuous integration, rev:83
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-ci/177/
Executed test runs:
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/969/console
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/976/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-03:

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

review: Approve (continuous-integration)

Revision history for this message

James Henstridge (jamesh) wrote on 2016-11-03:

There are a few small issues I've noted inline, but this is looking almost ready to merge.

review: Needs Fixing

Revision history for this message

Michi Henning (michihenning) wrote on 2016-11-03:

Thanks for your eagle eye! Old habits die hard :-(

Revision history for this message

James Henstridge (jamesh) wrote on 2016-11-03:

Looks good.

review: Approve

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-03:

FAILED: Autolanding.
More details in the following jenkins job:
https://jenkins.canonical.com/unity-api-1/job/lp-storage-framework-autoland/64/
Executed test runs:
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build/979/console
FAILURE: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/986/console

review: Needs Fixing (continuous-integration)

lp:~michihenning/storage-framework/qml-improvements updated on 2016-11-03

86. By Michi Henning on 2016-11-03: Merged devel and resolved conflict.

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-03:

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

review: Approve (continuous-integration)

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-11-03:

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

review: Approve (continuous-integration)

Revision history for this message

Michi Henning (michihenning) wrote on 2016-11-03:

Re-approving after fixing merge conflict.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Michi Henning

 === modified file 'demo/demo.qml'
 --- demo/demo.qml	2016-09-29 12:34:18 +0000
 +++ demo/demo.qml	2016-11-03 07:36:55 +0000
@@ -27,7 +27,9 @@
          var accounts = root.accountsjob.accounts;
          console.log("Got accounts " + accounts);
          for (var i = 0; i < accounts.length; i++) {
--          console.log("Account " + i + " owner = " + accounts[i].owner);
++          console.log("Account " + i + ": busName =     " + accounts[i].busName());
++          console.log("Account " + i + ": objectPath =  " + accounts[i].objectPath());
++          console.log("Account " + i + ": displayName = " + accounts[i].displayName);
+         }
+       }
+     }
 === modified file 'include/unity/storage/qt/Account.h'
 --- include/unity/storage/qt/Account.h	2016-11-02 04:27:52 +0000
 +++ include/unity/storage/qt/Account.h	2016-11-03 07:36:55 +0000
@@ -44,9 +44,9 @@
+ {
      Q_GADGET
      Q_PROPERTY(bool isValid READ isValid FINAL)
--    Q_PROPERTY(QString owner READ owner FINAL)
--    Q_PROPERTY(QString ownerId READ ownerId FINAL)
--    Q_PROPERTY(QString description READ description FINAL)
++    Q_PROPERTY(QString busName READ busName FINAL)
++    Q_PROPERTY(QString objectPath READ objectPath FINAL)
++    Q_PROPERTY(QString displayName READ displayName FINAL)
  public:
      Account();
@@ -57,12 +57,13 @@
      Account& operator=(Account&&);
      bool isValid() const;
--    QString owner() const;
--    QString ownerId() const;
--    QString description() const;
++    QString busName() const;
++    QString objectPath() const;
++    QString displayName() const;
++    // TODO: add more methods to retrieve additional info, such as description, icon ID, and service name.
--    Q_INVOKABLE ItemListJob* roots(QStringList const& keys = QStringList()) const;
--    Q_INVOKABLE ItemJob* get(QString const& itemId, QStringList const& keys = QStringList()) const;
++    Q_INVOKABLE unity::storage::qt::ItemListJob* roots(QStringList const& keys = QStringList()) const;
++    Q_INVOKABLE unity::storage::qt::ItemJob* get(QString const& itemId, QStringList const& keys = QStringList()) const;
      bool operator==(Account const&) const;
      bool operator!=(Account const&) const;
 === modified file 'include/unity/storage/qt/AccountsJob.h'
 --- include/unity/storage/qt/AccountsJob.h	2016-10-10 04:07:07 +0000
 +++ include/unity/storage/qt/AccountsJob.h	2016-11-03 07:36:55 +0000
@@ -48,7 +48,7 @@
      Q_PROPERTY(bool isValid READ isValid NOTIFY statusChanged FINAL)
      Q_PROPERTY(unity::storage::qt::AccountsJob::Status status READ status NOTIFY statusChanged FINAL)
      Q_PROPERTY(unity::storage::qt::StorageError error READ error NOTIFY statusChanged FINAL)
--    Q_PROPERTY(QList<unity::storage::qt::Account> accounts READ accounts NOTIFY statusChanged FINAL)
++    Q_PROPERTY(QVariantList accounts READ accountsAsVariantList NOTIFY statusChanged FINAL)
  public:
      enum Status { Loading, Finished, Error };
@@ -68,6 +68,8 @@
      AccountsJob(std::shared_ptr<internal::RuntimeImpl> const& runtime);
      AccountsJob(StorageError const& error);
++    QVariantList accountsAsVariantList() const;
++
      std::unique_ptr<internal::AccountsJobImpl> const p_;
      friend class internal::RuntimeImpl;
 === modified file 'include/unity/storage/qt/Item.h'
 --- include/unity/storage/qt/Item.h	2016-11-03 03:41:24 +0000
 +++ include/unity/storage/qt/Item.h	2016-11-03 07:36:55 +0000
@@ -98,28 +98,28 @@
      QDateTime lastModifiedTime() const;
      QStringList parentIds() const;
--    Q_INVOKABLE ItemListJob* parents(QStringList const& keys = QStringList()) const;
--    Q_INVOKABLE ItemJob* copy(Item const& newParent,
--                              QString const& newName,
--                              QStringList const& keys = QStringList()) const;
--    Q_INVOKABLE ItemJob* move(Item const& newParent,
--                              QString const& newName,
--                              QStringList const& keys = QStringList()) const;
--    Q_INVOKABLE VoidJob* deleteItem() const;
--
--    Q_INVOKABLE Uploader* createUploader(ConflictPolicy policy,
--                                         qint64 sizeInBytes,
--                                         QStringList const& keys = QStringList()) const;
--    Q_INVOKABLE Downloader* createDownloader(ConflictPolicy policy) const;
--
--    Q_INVOKABLE ItemListJob* list(QStringList const& keys = QStringList()) const;
--    Q_INVOKABLE ItemListJob* lookup(QString const& name, QStringList const& = QStringList()) const;
--    Q_INVOKABLE ItemJob* createFolder(QString const& name, QStringList const& = QStringList()) const;
--    Q_INVOKABLE Uploader* createFile(QString const& name,
--                                     ConflictPolicy policy,
--                                     qint64 sizeInBytes,
--                                     QString const& contentType,
--                                     QStringList const& keys = QStringList()) const;
++    Q_INVOKABLE unity::storage::qt::ItemListJob* parents(QStringList const& keys = QStringList()) const;
++    Q_INVOKABLE unity::storage::qt::ItemJob* copy(Item const& newParent,
++                              QString const& newName,
++                              QStringList const& keys = QStringList()) const;
++    Q_INVOKABLE unity::storage::qt::ItemJob* move(Item const& newParent,
++                              QString const& newName,
++                              QStringList const& keys = QStringList()) const;
++    Q_INVOKABLE unity::storage::qt::VoidJob* deleteItem() const;
++
++    Q_INVOKABLE unity::storage::qt::Uploader* createUploader(ConflictPolicy policy,
++                                                             qint64 sizeInBytes,
++                                                             QStringList const& keys = QStringList()) const;
++    Q_INVOKABLE unity::storage::qt::Downloader* createDownloader(ConflictPolicy policy) const;
++
++    Q_INVOKABLE unity::storage::qt::ItemListJob* list(QStringList const& keys = QStringList()) const;
++    Q_INVOKABLE unity::storage::qt::ItemListJob* lookup(QString const& name, QStringList const& = QStringList()) const;
++    Q_INVOKABLE unity::storage::qt::ItemJob* createFolder(QString const& name, QStringList const& = QStringList()) const;
++    Q_INVOKABLE unity::storage::qt::Uploader* createFile(QString const& name,
++                                                         ConflictPolicy policy,
++                                                         qint64 sizeInBytes,
++                                                         QString const& contentType,
++                                                         QStringList const& keys = QStringList()) const;
      bool operator==(Item const&) const;
      bool operator!=(Item const&) const;
 === removed file 'include/unity/storage/qt/MetadataKeys.h.THIS'
 --- include/unity/storage/qt/MetadataKeys.h.THIS	2016-11-02 03:27:23 +0000
 +++ include/unity/storage/qt/MetadataKeys.h.THIS	1970-01-01 00:00:00 +0000
@@ -1,37 +0,0 @@
--/*
-- * Copyright (C) 2016 Canonical Ltd
-- *
-- * This program is free software: you can redistribute it and/or modify
-- * it under the terms of the GNU Lesser General Public License version 3 as
-- * published by the Free Software Foundation.
-- *
-- * This program is distributed in the hope that it will be useful,
-- * but WITHOUT ANY WARRANTY; without even the implied warranty of
-- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-- * GNU Lesser General Public License for more details.
-- *
-- * You should have received a copy of the GNU Lesser General Public License
-- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
-- *
-- * Authors: Michi Henning <michi.henning@canonical.com>
-- */
--
--#pragma once
--
--#include <unity/storage/common.h>
--
--#include <QList>
--#include <QString>
--
--namespace unity
--{
--namespace storage
--{
--namespace qt
--{
--
--static QStringList const ALL_METADATA = { metadata::ALL };
--
--}  // namespace qt
--}  // namespace storage
--}  // namespace unity
 === modified file 'include/unity/storage/qt/Runtime.h'
 --- include/unity/storage/qt/Runtime.h	2016-09-29 12:28:46 +0000
 +++ include/unity/storage/qt/Runtime.h	2016-11-03 07:36:55 +0000
@@ -66,9 +66,9 @@
      Account make_test_account(QString const& bus_name,
                                QString const& object_path,
--                              QString const& owner_id = "",
--                              QString const& owner = "",
--                              QString const& description = "") const;
++                              QString const& id = "",
++                              QString const& serviceId = "",
++                              QString const& displayName = "") const;
  private:
      std::shared_ptr<internal::RuntimeImpl> p_;
 === modified file 'include/unity/storage/qt/internal/AccountImpl.h'
 --- include/unity/storage/qt/internal/AccountImpl.h	2016-11-02 03:20:03 +0000
 +++ include/unity/storage/qt/internal/AccountImpl.h	2016-11-03 07:36:55 +0000
@@ -47,9 +47,9 @@
      AccountImpl& operator=(AccountImpl const&) = default;
      AccountImpl& operator=(AccountImpl&&) = default;
--    QString ownerId() const;
--    QString owner() const;
--    QString description() const;
++    QString busName() const;
++    QString objectPath() const;
++    QString displayName() const;
      ItemListJob* roots(QStringList const& keys) const;
      ItemJob* get(QString const& itemId, QStringList const& keys) const;
@@ -69,24 +69,24 @@
      static Account make_account(std::shared_ptr<RuntimeImpl> const& runtime_impl,
                                  QString const& bus_name,
                                  QString const& object_path,
--                                QString const& owner_id,
--                                QString const& owner,
--                                QString const& description);
++                                QString const& id,
++                                QString const& service_id,
++                                QString const& display_name);
  private:
      AccountImpl(std::shared_ptr<RuntimeImpl> const& runtime_impl,
                  QString const& bus_name,
                  QString const& object_path,
--                QString const& owner_id,
--                QString const& owner,
--                QString const& description);
++                QString const& id,
++                QString const& service_id,
++                QString const& display_name);
      bool is_valid_;
      QString bus_name_;
      QString object_path_;
--    QString owner_id_;
--    QString owner_;
--    QString description_;
++    QString id_;
++    QString service_id_;
++    QString display_name_;
      std::weak_ptr<RuntimeImpl> runtime_impl_;
      std::shared_ptr<ProviderInterface> provider_;
 === modified file 'include/unity/storage/qt/internal/AccountsJobImpl.h'
 --- include/unity/storage/qt/internal/AccountsJobImpl.h	2016-10-12 05:25:20 +0000
 +++ include/unity/storage/qt/internal/AccountsJobImpl.h	2016-11-03 07:36:55 +0000
@@ -43,6 +43,7 @@
      AccountsJob::Status status() const;
      StorageError error() const;
      QList<Account> accounts() const;
++    QVariantList accountsAsVariantList() const;
  private Q_SLOTS:
      void manager_ready();
 === modified file 'plugins/Ubuntu/StorageFramework/plugin.cpp'
 --- plugins/Ubuntu/StorageFramework/plugin.cpp	2016-09-29 12:28:46 +0000
 +++ plugins/Ubuntu/StorageFramework/plugin.cpp	2016-11-03 07:36:55 +0000
@@ -36,8 +36,8 @@
  void StorageFrameworkPlugin::registerTypes(const char* uri)
+ {
      qmlRegisterType<Runtime>(uri, 0, 1, "Runtime");
++    qmlRegisterUncreatableType<Account>(uri, 0, 1, "Account", "");
      qmlRegisterUncreatableType<AccountsJob>(uri, 0, 1, "AccountsJob", "Use Runtime to create AccountsJob");
--    qmlRegisterUncreatableType<Account>(uri, 0, 1, "Account", "");
      qmlRegisterUncreatableType<Item>(uri, 0, 1, "Item", "");
      qmlRegisterUncreatableType<ItemJob>(uri, 0, 1, "ItemJob", "Use Account or another item to access items");
      qmlRegisterUncreatableType<ItemListJob>(uri, 0, 1, "ItemListJob", "Use Account or another item to access items");
 === modified file 'src/qt/Account.cpp'
 --- src/qt/Account.cpp	2016-11-02 03:20:03 +0000
 +++ src/qt/Account.cpp	2016-11-03 07:36:55 +0000
@@ -78,19 +78,19 @@
      return p_->is_valid_;
+ }
--QString Account::owner() const
--{
--    return p_->owner();
--}
--
--QString Account::ownerId() const
--{
--    return p_->ownerId();
--}
--
--QString Account::description() const
--{
--    return p_->description();
++QString Account::busName() const
++{
++    return p_->busName();
++}
++
++QString Account::objectPath() const
++{
++    return p_->objectPath();
++}
++
++QString Account::displayName() const
++{
++    return p_->displayName();
+ }
  ItemListJob* Account::roots(QStringList const& keys) const
 === modified file 'src/qt/AccountsJob.cpp'
 --- src/qt/AccountsJob.cpp	2016-09-16 06:25:08 +0000
 +++ src/qt/AccountsJob.cpp	2016-11-03 07:36:55 +0000
@@ -21,6 +21,8 @@
  #include <unity/storage/qt/Account.h>
  #include <unity/storage/qt/internal/AccountsJobImpl.h>
++#include <QVariant>
++
  using namespace unity::storage::qt;
  using namespace std;
@@ -63,6 +65,11 @@
      return p_->accounts();
+ }
++QVariantList AccountsJob::accountsAsVariantList() const
++{
++    return p_->accountsAsVariantList();
++}
++
  }  // namespace qt
  }  // namespace storage
  }  // namespace unity
 === modified file 'src/qt/internal/AccountImpl.cpp'
 --- src/qt/internal/AccountImpl.cpp	2016-11-02 03:20:03 +0000
 +++ src/qt/internal/AccountImpl.cpp	2016-11-03 07:36:55 +0000
@@ -50,15 +50,15 @@
  AccountImpl::AccountImpl(shared_ptr<RuntimeImpl> const& runtime_impl,
                           QString const& bus_name,
                           QString const& object_path,
--                         QString const& owner_id,
--                         QString const& owner,
--                         QString const& description)
++                         QString const& id,
++                         QString const& service_id,
++                         QString const& display_name)
      : is_valid_(true)
      , bus_name_(bus_name)
      , object_path_(object_path)
--    , owner_id_(owner_id)
--    , owner_(owner)
--    , description_(description)
++    , id_(id)
++    , service_id_(service_id)
++    , display_name_(display_name)
      , runtime_impl_(runtime_impl)
      , provider_(new ProviderInterface(bus_name, object_path, runtime_impl->connection()))
+ {
@@ -66,19 +66,19 @@
      assert(!object_path.isEmpty());
+ }
--QString AccountImpl::owner() const
--{
--    return is_valid_ ? owner_ : "";
--}
--
--QString AccountImpl::ownerId() const
--{
--    return is_valid_ ? owner_id_ : "";
--}
--
--QString AccountImpl::description() const
--{
--    return is_valid_ ? description_ : "";
++QString AccountImpl::busName() const
++{
++    return is_valid_ ? bus_name_ : "";
++}
++
++QString AccountImpl::objectPath() const
++{
++    return is_valid_ ? object_path_ : "";
++}
++
++QString AccountImpl::displayName() const
++{
++    return is_valid_ ? display_name_ : "";
+ }
  ItemListJob* AccountImpl::roots(QStringList const& keys) const
@@ -142,9 +142,9 @@
      if (is_valid_)
+     {
          return    other.is_valid_
--               && owner_ == other.owner_
--               && owner_id_ == other.owner_id_
--               && description_ == other.description_;
++               && id_ == other.id_
++               && service_id_ == other.service_id_
++               && display_name_ == other.display_name_;
+     }
      return !other.is_valid_;
+ }
@@ -165,23 +165,23 @@
          return false;
+     }
      assert(is_valid_ && other.is_valid_);
--    if (owner_id_ < other.owner_id_)
--    {
--        return true;
--    }
--    if (owner_id_ > other.owner_id_)
--    {
--        return false;
--    }
--    if (owner_ < other.owner_)
--    {
--        return true;
--    }
--    if (owner_ > other.owner_)
--    {
--        return false;
--    }
--    return description_ < other.description_;
++    if (id_ < other.id_)
++    {
++        return true;
++    }
++    if (id_ > other.id_)
++    {
++        return false;
++    }
++    if (service_id_ < other.service_id_)
++    {
++        return true;
++    }
++    if (service_id_ > other.service_id_)
++    {
++        return false;
++    }
++    return display_name_ < other.display_name_;
+ }
  bool AccountImpl::operator<=(AccountImpl const& other) const
@@ -216,20 +216,20 @@
          return 0;
+     }
      size_t hash = 0;
--    boost::hash_combine(hash, qHash(owner_));
--    boost::hash_combine(hash, qHash(owner_id_));
--    boost::hash_combine(hash, qHash(description_));
++    boost::hash_combine(hash, qHash(service_id_));
++    boost::hash_combine(hash, qHash(id_));
++    boost::hash_combine(hash, qHash(display_name_));
      return hash;
+ }
  Account AccountImpl::make_account(shared_ptr<RuntimeImpl> const& runtime,
                                    QString const& bus_name,
                                    QString const& object_path,
--                                  QString const& owner_id,
--                                  QString const& owner,
--                                  QString const& description)
++                                  QString const& id,
++                                  QString const& service_id,
++                                  QString const& display_name)
+ {
--    shared_ptr<AccountImpl> p(new AccountImpl(runtime, bus_name, object_path, owner_id, owner, description));
++    shared_ptr<AccountImpl> p(new AccountImpl(runtime, bus_name, object_path, id, service_id, display_name));
      return Account(p);
+ }
 === modified file 'src/qt/internal/AccountsJobImpl.cpp'
 --- src/qt/internal/AccountsJobImpl.cpp	2016-11-02 09:32:53 +0000
 +++ src/qt/internal/AccountsJobImpl.cpp	2016-11-03 07:36:55 +0000
@@ -103,6 +103,16 @@
      return accounts_;
+ }
++QVariantList AccountsJobImpl::accountsAsVariantList() const
++{
++    QVariantList account_list;
++    for (auto const& a : accounts())
++    {
++        account_list.append(QVariant::fromValue(a));
++    }
++    return account_list;
++}
++
  void AccountsJobImpl::manager_ready()
+ {
      timer_.stop();
@@ -171,8 +181,8 @@
              accounts_.append(AccountImpl::make_account(runtime,
                                                         bus_name,
                                                         object_path,
++                                                       QString::number(a->id()),
                                                         a->serviceId(),
--                                                       "",
                                                         a->displayName()));
+         }
+     }
 === modified file 'tests/remote-client/remote-client_test.cpp'
 --- tests/remote-client/remote-client_test.cpp	2016-11-03 03:41:24 +0000
 +++ tests/remote-client/remote-client_test.cpp	2016-11-03 07:36:55 +0000
@@ -114,84 +114,84 @@
          // Default constructor.
          Account a;
          EXPECT_FALSE(a.isValid());
--        EXPECT_EQ("", a.ownerId());
--        EXPECT_EQ("", a.owner());
--        EXPECT_EQ("", a.description());
++        EXPECT_EQ("", a.busName());
++        EXPECT_EQ("", a.objectPath());
++        EXPECT_EQ("", a.displayName());
+     }
+     {
          auto acc = runtime_->make_test_account(service_connection_->baseService(), object_path(),
--                                               "id", "owner", "description");
++                                               "id", "sid", "displayName");
          EXPECT_TRUE(acc.isValid());
--        EXPECT_EQ("id", acc.ownerId());
--        EXPECT_EQ("owner", acc.owner());
--        EXPECT_EQ("description", acc.description());
++        EXPECT_EQ(service_connection_->baseService(), acc.busName());
++        EXPECT_EQ(object_path(), acc.objectPath());
++        EXPECT_EQ("displayName", acc.displayName());
          // Copy constructor
          Account a2(acc);
          EXPECT_TRUE(a2.isValid());
--        EXPECT_EQ("id", a2.ownerId());
--        EXPECT_EQ("owner", a2.owner());
--        EXPECT_EQ("description", a2.description());
++        EXPECT_EQ(service_connection_->baseService(), a2.busName());
++        EXPECT_EQ(object_path(), a2.objectPath());
++        EXPECT_EQ("displayName", a2.displayName());
          // Move constructor
          Account a3(move(a2));
          EXPECT_TRUE(a3.isValid());
--        EXPECT_EQ("id", a3.ownerId());
--        EXPECT_EQ("owner", a3.owner());
--        EXPECT_EQ("description", a3.description());
++        EXPECT_EQ(service_connection_->baseService(), a3.busName());
++        EXPECT_EQ(object_path(), a3.objectPath());
++        EXPECT_EQ("displayName", a3.displayName());
          // Moved-from object must be invalid
          EXPECT_FALSE(a2.isValid());
          // Moved-from object must be assignable
          auto a4 = runtime_->make_test_account(service_connection_->baseService(), object_path(),
--                                              "id4", "owner4", "description4");
++                                              "id4", "sid4", "displayName4");
          a2 = a4;
          EXPECT_TRUE(a2.isValid());
--        EXPECT_EQ("id4", a2.ownerId());
--        EXPECT_EQ("owner4", a2.owner());
--        EXPECT_EQ("description4", a2.description());
++        EXPECT_EQ(service_connection_->baseService(), a2.busName());
++        EXPECT_EQ(object_path(), a2.objectPath());
++        EXPECT_EQ("displayName4", a2.displayName());
+     }
+     {
--        auto a1 = runtime_->make_test_account(service_connection_->baseService(), object_path(), "id", "owner", "description");
--        auto a2 = runtime_->make_test_account(service_connection_->baseService(), object_path(), "id2", "owner2", "description2");
++        auto a1 = runtime_->make_test_account(service_connection_->baseService(), object_path(), "id", "sid", "dn");
++        auto a2 = runtime_->make_test_account(service_connection_->baseService(), object_path(), "id2", "sid2", "dn2");
          // Copy assignment
          a1 = a2;
          EXPECT_TRUE(a2.isValid());
--        EXPECT_EQ("id2", a1.ownerId());
--        EXPECT_EQ("owner2", a2.owner());
--        EXPECT_EQ("description2", a1.description());
++        EXPECT_EQ(service_connection_->baseService(), a1.busName());
++        EXPECT_EQ(object_path(), a1.objectPath());
++        EXPECT_EQ("dn2", a1.displayName());
          // Self-assignment
          a2 = a2;
          EXPECT_TRUE(a2.isValid());
--        EXPECT_EQ("id2", a1.ownerId());
--        EXPECT_EQ("owner2", a2.owner());
--        EXPECT_EQ("description2", a1.description());
++        EXPECT_EQ(service_connection_->baseService(), a1.busName());
++        EXPECT_EQ(object_path(), a1.objectPath());
++        EXPECT_EQ("dn2", a1.displayName());
          // Move assignment
          auto a3 = runtime_->make_test_account(service_connection_->baseService(), object_path(),
--                                              "id3", "owner3", "description3");
++                                              "id3", "sid3", "dn3");
          a1 = move(a3);
          EXPECT_TRUE(a1.isValid());
--        EXPECT_EQ("id3", a1.ownerId());
--        EXPECT_EQ("owner3", a1.owner());
--        EXPECT_EQ("description3", a1.description());
++        EXPECT_EQ(service_connection_->baseService(), a1.busName());
++        EXPECT_EQ(object_path(), a1.objectPath());
++        EXPECT_EQ("dn3", a1.displayName());
          // Moved-from object must be invalid
          EXPECT_FALSE(a3.isValid());
          // Moved-from object must be assignable
          auto a4 = runtime_->make_test_account(service_connection_->baseService(), object_path(),
--                                              "id4", "owner4", "description4");
++                                              "id4", "sid4", "dn4");
          a2 = a4;
          EXPECT_TRUE(a2.isValid());
--        EXPECT_EQ("id4", a2.ownerId());
--        EXPECT_EQ("owner4", a2.owner());
--        EXPECT_EQ("description4", a2.description());
++        EXPECT_EQ(service_connection_->baseService(), a2.busName());
++        EXPECT_EQ(object_path(), a2.objectPath());
++        EXPECT_EQ("dn4", a2.displayName());
+     }
+ }
@@ -230,7 +230,7 @@
+     }
+     {
--        // a1 < a2 for owner ID
++        // a1 < a2 for ID
          auto a1 = runtime_->make_test_account(service_connection_->baseService(), object_path(), "a", "x", "x");
          auto a2 = runtime_->make_test_account(service_connection_->baseService(), object_path(), "b", "x", "x");
@@ -251,7 +251,7 @@
+     }
+     {
--        // a1 < a2 for owner
++        // a1 < a2 for service ID
          auto a1 = runtime_->make_test_account(service_connection_->baseService(), object_path(), "a", "a", "x");
          auto a2 = runtime_->make_test_account(service_connection_->baseService(), object_path(), "a", "b", "x");
@@ -272,7 +272,7 @@
+     }
+     {
--        // a1 < a2 for description
++        // a1 < a2 for display name
          auto a1 = runtime_->make_test_account(service_connection_->baseService(), object_path(), "a", "a", "a");
          auto a2 = runtime_->make_test_account(service_connection_->baseService(), object_path(), "a", "a", "b");
@@ -994,18 +994,6 @@
      EXPECT_EQ("Item::deleteItem(): cannot create job from invalid item", j->error().message());
+ }
--#if 0
--// TODO: need to make internal symbols available for testing.
--TEST_F(ValidateTest, basic)
--{
--    using namespace unity::storage::qt::internal;
--
--    unity::storage::internal::ItemMetadata md;
--
--    validate("foo", md);
--}
--#endif
--
  TEST_F(ItemTest, basic)
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));