Storage Framework

Merge lp:~michihenning/storage-framework/more-tests into lp:storage-framework/devel

more-tests
Merge into devel

Proposed by Michi Henning on 2016-09-26

Status:	Merged
Approved by:	James Henstridge on 2016-09-30
Approved revision:	89
Merged at revision:	73
Proposed branch:	lp:~michihenning/storage-framework/more-tests
Merge into:	lp:storage-framework/devel
Prerequisite:	lp:~michihenning/storage-framework/api2
Diff against target:	1420 lines (+796/-151) 18 files modified include/unity/storage/qt/Account.h (+1/-1) include/unity/storage/qt/Item.h (+1/-1) include/unity/storage/qt/VoidJob.h (+20/-4) include/unity/storage/qt/internal/AccountImpl.h (+1/-1) include/unity/storage/qt/internal/ItemImpl.h (+2/-1) include/unity/storage/qt/internal/ItemJobImpl.h (+4/-4) include/unity/storage/qt/internal/ItemListJobImpl.h (+3/-3) include/unity/storage/qt/internal/VoidJobImpl.h (+69/-0) src/qt/CMakeLists.txt (+4/-0) src/qt/VoidJob.cpp (+57/-0) src/qt/internal/AccountImpl.cpp (+16/-7) src/qt/internal/ItemImpl.cpp (+23/-19) src/qt/internal/ItemJobImpl.cpp (+12/-1) src/qt/internal/ItemListJobImpl.cpp (+11/-0) src/qt/internal/RuntimeImpl.cpp (+2/-0) src/qt/internal/VoidJobImpl.cpp (+117/-0) tests/remote-client/MockProvider.cpp (+32/-2) tests/remote-client/remote-client_test.cpp (+421/-107)
To merge this branch:	bzr merge lp:~michihenning/storage-framework/more-tests
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
James Henstridge		2016-09-26	Approve on 2016-09-30
unity-api-1-bot	continuous-integration		Approve on 2016-09-29
Review via email: mp+306744@code.launchpad.net

Commit message

Added VoidJob.
Implemeted get().
Implemeted deleteItem().
Some minor refactoring and tidy-up.
Lots more tests.

Description of the change

Added VoidJob.
Implemeted get().
Implemeted deleteItem().
Some minor refactoring and tidy-up.
Lots more tests.

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-09-26:

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

review: Needs Fixing (continuous-integration)

lp:~michihenning/storage-framework/more-tests updated on 2016-09-26

88. By Michi Henning on 2016-09-26: Fixed whitespace.

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-09-26:

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

review: Approve (continuous-integration)

Revision history for this message

James Henstridge (jamesh) wrote on 2016-09-29:

Just a few minor issues that I've included as inline comments.

review: Needs Fixing

lp:~michihenning/storage-framework/more-tests updated on 2016-09-29

89. By Michi Henning on 2016-09-29: Review comments from James.

Revision history for this message

Michi Henning (michihenning) wrote on 2016-09-29:

Thanks for the review!

I'll go and make the corresponding changes in the follow-up branches.

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-09-29:

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

review: Approve (continuous-integration)

Revision history for this message

James Henstridge (jamesh) wrote on 2016-09-30:

Looks good.

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 'include/unity/storage/qt/Account.h'
 --- include/unity/storage/qt/Account.h	2016-09-26 02:12:30 +0000
 +++ include/unity/storage/qt/Account.h	2016-09-29 13:09:41 +0000
@@ -94,7 +94,7 @@
  template<> struct Q_DECL_EXPORT hash<unity::storage::qt::Account>
+ {
--    std::size_t operator()(unity::storage::qt::Account const& a)
++    std::size_t operator()(unity::storage::qt::Account const& a) const
+     {
          return a.hash();
+     }
 === modified file 'include/unity/storage/qt/Item.h'
 --- include/unity/storage/qt/Item.h	2016-09-26 02:12:30 +0000
 +++ include/unity/storage/qt/Item.h	2016-09-29 13:09:41 +0000
@@ -146,7 +146,7 @@
  template<> struct Q_DECL_EXPORT hash<unity::storage::qt::Item>
+ {
--    std::size_t operator()(unity::storage::qt::Item const& i)
++    std::size_t operator()(unity::storage::qt::Item const& i) const
+     {
          return i.hash();
+     }
 === modified file 'include/unity/storage/qt/VoidJob.h'
 --- include/unity/storage/qt/VoidJob.h	2016-09-20 23:52:45 +0000
 +++ include/unity/storage/qt/VoidJob.h	2016-09-29 13:09:41 +0000
@@ -20,22 +20,29 @@
  #include <QObject>
++#include <memory>
++
  namespace unity
+ {
  namespace storage
+ {
  namespace qt
+ {
++namespace internal
++{
++
++class VoidJobImpl;
++
++}  // namespace internal
  class StorageError;
  class Q_DECL_EXPORT VoidJob final : public QObject
+ {
--    // TODO: notify, CONSTANT where needed
      Q_OBJECT
--    Q_PROPERTY(bool READ isValid FINAL)
--    Q_PROPERTY(unity::storage::qt::VoidJob::Status READ status NOTIFY statusChanged FINAL)
--    Q_PROPERTY(unity::storage::qt::StorageError READ error FINAL)
++    Q_PROPERTY(bool isValid READ isValid NOTIFY statusChanged FINAL)
++    Q_PROPERTY(unity::storage::qt::VoidJob::Status status READ status NOTIFY statusChanged FINAL)
++    Q_PROPERTY(unity::storage::qt::StorageError error READ error NOTIFY statusChanged FINAL)
  public:
      virtual ~VoidJob();
@@ -49,8 +56,17 @@
  Q_SIGNALS:
      void statusChanged(unity::storage::qt::VoidJob::Status status) const;
++
++private:
++    VoidJob(std::unique_ptr<internal::VoidJobImpl> p);
++
++    std::unique_ptr<internal::VoidJobImpl> const p_;
++
++    friend class internal::VoidJobImpl;
  };
  }  // namespace qt
  }  // namespace storage
  }  // namespace unity
++
++Q_DECLARE_METATYPE(unity::storage::qt::VoidJob::Status)
 === modified file 'include/unity/storage/qt/internal/AccountImpl.h'
 --- include/unity/storage/qt/internal/AccountImpl.h	2016-09-20 02:24:36 +0000
 +++ include/unity/storage/qt/internal/AccountImpl.h	2016-09-29 13:09:41 +0000
@@ -63,7 +63,7 @@
      size_t hash() const;
--    //std::shared_ptr<RuntimeImpl> runtime() const;
++    std::shared_ptr<RuntimeImpl> runtime() const;
      std::shared_ptr<ProviderInterface> provider() const;
      static Account make_account(std::shared_ptr<RuntimeImpl> const& runtime,
 === modified file 'include/unity/storage/qt/internal/ItemImpl.h'
 --- include/unity/storage/qt/internal/ItemImpl.h	2016-09-26 02:12:30 +0000
 +++ include/unity/storage/qt/internal/ItemImpl.h	2016-09-29 13:09:41 +0000
@@ -49,7 +49,6 @@
      QString itemId() const;
      QString name() const;
      Account account() const;
--    //Item root() const;
      QString etag() const;
      Item::Type type() const;
      QVariantMap metadata() const;
@@ -82,6 +81,8 @@
                            storage::internal::ItemMetadata const& md,
                            std::shared_ptr<AccountImpl> const& account);
++    std::shared_ptr<RuntimeImpl> runtime() const;
++
  private:
      //std::shared_ptr<RuntimeImpl> get_runtime(QString const& method) const;
 === modified file 'include/unity/storage/qt/internal/ItemJobImpl.h'
 --- include/unity/storage/qt/internal/ItemJobImpl.h	2016-09-26 02:12:30 +0000
 +++ include/unity/storage/qt/internal/ItemJobImpl.h	2016-09-29 13:09:41 +0000
@@ -20,10 +20,10 @@
  #include <unity/storage/qt/ItemJob.h>
--#include <unity/storage/qt/Account.h>
--#include <unity/storage/qt/internal/Handler.h>
  #include <unity/storage/qt/StorageError.h>
++#include <QDBusPendingReply>
++
  namespace unity
+ {
  namespace storage
@@ -33,14 +33,14 @@
  class ItemMetadata;
--}
++}  // namespace internal
  namespace qt
+ {
  namespace internal
+ {
--class RuntimeImpl;
++class AccountImpl;
  class ItemJobImpl : public QObject
+ {
 === modified file 'include/unity/storage/qt/internal/ItemListJobImpl.h'
 --- include/unity/storage/qt/internal/ItemListJobImpl.h	2016-09-26 02:12:30 +0000
 +++ include/unity/storage/qt/internal/ItemListJobImpl.h	2016-09-29 13:09:41 +0000
@@ -20,10 +20,10 @@
  #include <unity/storage/qt/ItemListJob.h>
--#include <unity/storage/qt/Account.h>
--#include <unity/storage/qt/internal/Handler.h>
  #include <unity/storage/qt/StorageError.h>
++#include <QDBusPendingReply>
++
  namespace unity
+ {
  namespace storage
@@ -40,7 +40,7 @@
  namespace internal
+ {
--class RuntimeImpl;
++class AccountImpl;
  class ItemListJobImpl : public QObject
+ {
 === added file 'include/unity/storage/qt/internal/VoidJobImpl.h'
 --- include/unity/storage/qt/internal/VoidJobImpl.h	1970-01-01 00:00:00 +0000
 +++ include/unity/storage/qt/internal/VoidJobImpl.h	2016-09-29 13:09:41 +0000
@@ -0,0 +1,69 @@
++/*
++ * 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/qt/VoidJob.h>
++
++#include <unity/storage/qt/StorageError.h>
++
++#include <QDBusPendingReply>
++
++namespace unity
++{
++namespace storage
++{
++namespace qt
++{
++namespace internal
++{
++
++class ItemImpl;
++
++class VoidJobImpl : public QObject
++{
++    Q_OBJECT
++public:
++    virtual ~VoidJobImpl() = default;
++
++    bool isValid() const;
++    VoidJob::Status status() const;
++    StorageError error() const;
++
++    static VoidJob* make_void_job(std::shared_ptr<ItemImpl> const& item,
++                                  QString const& method,
++                                  QDBusPendingReply<void> const& reply);
++    static VoidJob* make_void_job(StorageError const& e);
++
++private:
++    VoidJobImpl(std::shared_ptr<ItemImpl> const& item,
++                QString const& method,
++                QDBusPendingReply<void> const& reply);
++    VoidJobImpl(StorageError const& e);
++
++    VoidJob* public_instance_;
++    VoidJob::Status status_;
++    StorageError error_;
++    QString method_;
++    std::shared_ptr<ItemImpl> item_;
++};
++
++}  // namespace internal
++}  // namespace qt
++}  // namespace storage
++}  // namespace unity
 === modified file 'src/qt/CMakeLists.txt'
 --- src/qt/CMakeLists.txt	2016-09-16 06:25:08 +0000
 +++ src/qt/CMakeLists.txt	2016-09-29 13:09:41 +0000
@@ -23,6 +23,7 @@
      ItemListJob.cpp
      Runtime.cpp
      StorageError.cpp
++    VoidJob.cpp
      internal/AccountImpl.cpp
      internal/AccountsJobImpl.cpp
      internal/HandlerBase.cpp
@@ -33,16 +34,19 @@
      internal/StorageErrorImpl.cpp
      internal/unmarshal_error.cpp
      internal/validate.cpp
++    internal/VoidJobImpl.cpp
      ${generated_files}
      ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/AccountsJob.h
      ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/Item.h
      ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/ItemJob.h
      ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/ItemListJob.h
      ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/Runtime.h
++    ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/VoidJob.h
      ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/internal/AccountsJobImpl.h
      ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/internal/HandlerBase.h
      ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/internal/ItemJobImpl.h
      ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/internal/ItemListJobImpl.h
++    ${CMAKE_SOURCE_DIR}/include/unity/storage/qt/internal/VoidJobImpl.h
+ )
  add_library(storage-framework-qt-client-v2 SHARED
 === added file 'src/qt/VoidJob.cpp'
 --- src/qt/VoidJob.cpp	1970-01-01 00:00:00 +0000
 +++ src/qt/VoidJob.cpp	2016-09-29 13:09:41 +0000
@@ -0,0 +1,57 @@
++/*
++ * 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>
++ */
++
++#include <unity/storage/qt/VoidJob.h>
++
++#include <unity/storage/qt/internal/VoidJobImpl.h>
++
++using namespace unity::storage::qt;
++using namespace std;
++
++namespace unity
++{
++namespace storage
++{
++namespace qt
++{
++
++VoidJob::VoidJob(unique_ptr<internal::VoidJobImpl> p)
++    : p_(move(p))
++{
++}
++
++VoidJob::~VoidJob() = default;
++
++bool VoidJob::isValid() const
++{
++    return p_->isValid();
++}
++
++VoidJob::Status VoidJob::status() const
++{
++    return p_->status();
++}
++
++StorageError VoidJob::error() const
++{
++    return p_->error();
++}
++
++}  // namespace qt
++}  // namespace storage
++}  // namespace unity
 === modified file 'src/qt/internal/AccountImpl.cpp'
 --- src/qt/internal/AccountImpl.cpp	2016-09-22 01:52:20 +0000
 +++ src/qt/internal/AccountImpl.cpp	2016-09-29 13:09:41 +0000
@@ -24,6 +24,7 @@
  #include <unity/storage/qt/internal/ItemJobImpl.h>
  #include <unity/storage/qt/internal/ItemListJobImpl.h>
  #include <unity/storage/qt/internal/RuntimeImpl.h>
++#include <unity/storage/qt/internal/StorageErrorImpl.h>
  #include <unity/storage/qt/Runtime.h>
  #include <boost/functional/hash.hpp>
@@ -82,24 +83,25 @@
  ItemListJob* AccountImpl::roots() const
+ {
++    QString const method = "Account::roots()";
++
      auto runtime = runtime_.lock();
      if (!runtime || !runtime->isValid())
+     {
--        auto e = StorageErrorImpl::runtime_destroyed_error("Account::roots(): Runtime was destroyed previously");
++        auto e = StorageErrorImpl::runtime_destroyed_error(method + ": Runtime was destroyed previously");
          return ItemListJobImpl::make_item_list_job(e);
+     }
--    auto validate = [](storage::internal::ItemMetadata const& md)
++    auto validate = [method](storage::internal::ItemMetadata const& md)
+     {
          if (md.type != ItemType::root)
+         {
--            QString msg = "provider returned non-root item type: " + QString::number(int(md.type));
++            QString msg = method + ": provider returned non-root item type: " + QString::number(int(md.type));
              qCritical() << msg;
              throw StorageErrorImpl::local_comms_error(msg);
+         }
      };
--    QString method = "Account::roots()";
      auto reply = provider_->Roots();
      auto This = const_pointer_cast<AccountImpl>(shared_from_this());
      return ItemListJobImpl::make_item_list_job(This, method, reply, validate);
@@ -107,19 +109,21 @@
  ItemJob* AccountImpl::get(QString const& itemId) const
+ {
++    QString const method = "Account::get()";
++
      auto runtime = runtime_.lock();
      if (!runtime || !runtime->isValid())
+     {
--        auto e = StorageErrorImpl::runtime_destroyed_error("Account::get(): Runtime was destroyed previously");
++        auto e = StorageErrorImpl::runtime_destroyed_error(method + ": Runtime was destroyed previously");
          return ItemJobImpl::make_item_job(e);
+     }
--    // TODO: use defaulted param?
++    // LCOV_EXCL_START
      auto validate = [](storage::internal::ItemMetadata const&)
+     {
      };
++    // LCOV_EXCL_STOP
--    QString method = "Item::get()";
      auto reply = provider_->Metadata(itemId);
      auto This = const_pointer_cast<AccountImpl>(shared_from_this());
      return ItemJobImpl::make_item_job(This, method, reply, validate);
@@ -187,6 +191,11 @@
      return !operator<(other);
+ }
++shared_ptr<RuntimeImpl> AccountImpl::runtime() const
++{
++    return runtime_.lock();
++}
++
  shared_ptr<ProviderInterface> AccountImpl::provider() const
+ {
      return provider_;
 === modified file 'src/qt/internal/ItemImpl.cpp'
 --- src/qt/internal/ItemImpl.cpp	2016-09-26 02:12:30 +0000
 +++ src/qt/internal/ItemImpl.cpp	2016-09-29 13:09:41 +0000
@@ -23,6 +23,8 @@
  #include <unity/storage/qt/internal/AccountImpl.h>
  #include <unity/storage/qt/internal/ItemJobImpl.h>
  #include <unity/storage/qt/internal/RuntimeImpl.h>
++#include <unity/storage/qt/internal/StorageErrorImpl.h>
++#include <unity/storage/qt/internal/VoidJobImpl.h>
  #include <unity/storage/qt/internal/validate.h>
  #include <boost/functional/hash.hpp>
@@ -70,13 +72,6 @@
      return is_valid_ ? account_ : Account();
+ }
--#if 0
--Item ItemImpl::root() const
--{
--    return is_valid_ ? root_ : Item();
--}
--#endif
--
  QString ItemImpl::etag() const
+ {
      return is_valid_ ? md_.etag : "";
@@ -131,7 +126,25 @@
  VoidJob* ItemImpl::deleteItem() const
+ {
--    return nullptr;  // TODO
++    QString const method = "Item::deleteItem()";
++
++    assert(account_);
++    auto runtime = account_->runtime();
++    if (!runtime || !runtime->isValid())
++    {
++        auto e = StorageErrorImpl::runtime_destroyed_error(method + ": Runtime was destroyed previously");
++        return VoidJobImpl::make_void_job(e);
++    }
++
++    if (md_.type == storage::ItemType::root)
++    {
++        auto e = StorageErrorImpl::logic_error(method + ": cannot delete root");
++        return VoidJobImpl::make_void_job(e);
++    }
++
++    auto reply = account_->provider()->Delete(md_.item_id);
++    auto This = const_pointer_cast<ItemImpl>(shared_from_this());
++    return VoidJobImpl::make_void_job(This, method, reply);
+ }
  Uploader* ItemImpl::createUploader(Item::ConflictPolicy policy, qint64 sizeInBytes) const
@@ -248,19 +261,10 @@
      return Item(p);
+ }
--#if 0
--shared_ptr<RuntimeImpl> ItemImpl::get_runtime(QString const& method) const
++shared_ptr<RuntimeImpl> ItemImpl::runtime() const
+ {
--    auto runtime = account_->runtime_.lock();
--    if (!runtime || !runtime->isValid())
--    {
--        QString msg = method + ": Runtime was destroyed previously";
--        auto This = const_cast<ItemImpl*>(this);
--        This->error_ = StorageErrorImpl::runtime_destroyed_error(msg);
--    }
--    return runtime;
++    return account_->runtime();
+ }
--#endif
  }  // namespace internal
  }  // namespace qt
 === modified file 'src/qt/internal/ItemJobImpl.cpp'
 --- src/qt/internal/ItemJobImpl.cpp	2016-09-26 02:12:30 +0000
 +++ src/qt/internal/ItemJobImpl.cpp	2016-09-29 13:09:41 +0000
@@ -20,8 +20,10 @@
  #include <unity/storage/internal/dbusmarshal.h>
  #include <unity/storage/internal/ItemMetadata.h>
++#include <unity/storage/qt/internal/AccountImpl.h>
  #include <unity/storage/qt/internal/Handler.h>
  #include <unity/storage/qt/internal/ItemImpl.h>
++#include <unity/storage/qt/internal/RuntimeImpl.h>
  using namespace std;
@@ -49,6 +51,15 @@
      auto process_reply = [this](decltype(reply)& r)
+     {
++        auto runtime = account_->runtime();
++        if (!runtime || !runtime->isValid())
++        {
++            error_ = StorageErrorImpl::runtime_destroyed_error(method_ + ": Runtime was destroyed previously");
++            status_ = ItemJob::Error;
++            Q_EMIT public_instance_->statusChanged(status_);
++            return;
++        }
++
          auto metadata = r.value();
          try
+         {
@@ -98,7 +109,7 @@
  Item ItemJobImpl::item() const
+ {
--    return Item();  // TODO
++    return item_;
+ }
  ItemJob* ItemJobImpl::make_item_job(shared_ptr<AccountImpl> const& account,
 === modified file 'src/qt/internal/ItemListJobImpl.cpp'
 --- src/qt/internal/ItemListJobImpl.cpp	2016-09-26 02:12:30 +0000
 +++ src/qt/internal/ItemListJobImpl.cpp	2016-09-29 13:09:41 +0000
@@ -20,8 +20,10 @@
  #include <unity/storage/internal/dbusmarshal.h>
  #include <unity/storage/internal/ItemMetadata.h>
++#include <unity/storage/qt/internal/AccountImpl.h>
  #include <unity/storage/qt/internal/Handler.h>
  #include <unity/storage/qt/internal/ItemImpl.h>
++#include <unity/storage/qt/internal/RuntimeImpl.h>
  using namespace std;
@@ -49,6 +51,15 @@
      auto process_reply = [this](decltype(reply)& r)
+     {
++        auto runtime = account_->runtime();
++        if (!runtime || !runtime->isValid())
++        {
++            error_ = StorageErrorImpl::runtime_destroyed_error(method_ + ": Runtime was destroyed previously");
++            status_ = ItemListJob::Error;
++            Q_EMIT public_instance_->statusChanged(status_);
++            return;
++        }
++
          QList<Item> items;
          auto metadata = r.value();
          for (auto const& md : metadata)
 === modified file 'src/qt/internal/RuntimeImpl.cpp'
 --- src/qt/internal/RuntimeImpl.cpp	2016-09-26 02:12:30 +0000
 +++ src/qt/internal/RuntimeImpl.cpp	2016-09-29 13:09:41 +0000
@@ -26,6 +26,7 @@
  #include <unity/storage/qt/ItemJob.h>
  #include <unity/storage/qt/ItemListJob.h>
  #include <unity/storage/qt/Runtime.h>
++#include <unity/storage/qt/VoidJob.h>
  #include <QDBusError>
  #include <QDBusMetaType>
@@ -50,6 +51,7 @@
      qRegisterMetaType<QList<unity::storage::qt::Item>>();
      qRegisterMetaType<unity::storage::qt::ItemJob::Status>();
      qRegisterMetaType<unity::storage::qt::ItemListJob::Status>();
++    qRegisterMetaType<unity::storage::qt::VoidJob::Status>();
      qDBusRegisterMetaType<unity::storage::internal::ItemMetadata>();
      qDBusRegisterMetaType<QList<unity::storage::internal::ItemMetadata>>();
 === added file 'src/qt/internal/VoidJobImpl.cpp'
 --- src/qt/internal/VoidJobImpl.cpp	1970-01-01 00:00:00 +0000
 +++ src/qt/internal/VoidJobImpl.cpp	2016-09-29 13:09:41 +0000
@@ -0,0 +1,117 @@
++/*
++ * 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>
++ */
++
++#include <unity/storage/qt/internal/VoidJobImpl.h>
++
++#include <unity/storage/qt/internal/Handler.h>
++#include <unity/storage/qt/internal/ItemImpl.h>
++#include <unity/storage/qt/internal/RuntimeImpl.h>
++
++using namespace std;
++
++namespace unity
++{
++namespace storage
++{
++namespace qt
++{
++namespace internal
++{
++
++VoidJobImpl::VoidJobImpl(shared_ptr<ItemImpl> const& item,
++                         QString const& method,
++                         QDBusPendingReply<void> const& reply)
++    : status_(VoidJob::Loading)
++    , method_(method)
++    , item_(item)
++{
++    assert(!method_.isEmpty());
++    assert(item);
++
++    auto process_reply = [this](decltype(reply)&)
++    {
++        auto runtime = item_->runtime();
++        if (!runtime || !runtime->isValid())
++        {
++            error_ = StorageErrorImpl::runtime_destroyed_error(method_ + ": Runtime was destroyed previously");
++            status_ = VoidJob::Error;
++            Q_EMIT public_instance_->statusChanged(status_);
++            return;
++        }
++
++        status_ = VoidJob::Finished;
++        Q_EMIT public_instance_->statusChanged(status_);
++    };
++
++    auto process_error = [this](StorageError const& error)
++    {
++        error_ = error;
++        status_ = VoidJob::Error;
++        Q_EMIT public_instance_->statusChanged(status_);
++    };
++
++    new Handler<void>(this, reply, process_reply, process_error);
++}
++
++VoidJobImpl::VoidJobImpl(StorageError const& error)
++    : status_(VoidJob::Error)
++    , error_(error)
++{
++}
++
++bool VoidJobImpl::isValid() const
++{
++    return status_ != VoidJob::Status::Error;
++}
++
++VoidJob::Status VoidJobImpl::status() const
++{
++    return status_;
++}
++
++StorageError VoidJobImpl::error() const
++{
++    return error_;
++}
++
++VoidJob* VoidJobImpl::make_void_job(shared_ptr<ItemImpl> const& item,
++                                    QString const& method,
++                                    QDBusPendingReply<void> const& reply)
++{
++    unique_ptr<VoidJobImpl> impl(new VoidJobImpl(item, method, reply));
++    auto job = new VoidJob(move(impl));
++    job->p_->public_instance_ = job;
++    return job;
++}
++
++VoidJob* VoidJobImpl::make_void_job(StorageError const& error)
++{
++    unique_ptr<VoidJobImpl> impl(new VoidJobImpl(error));
++    auto job = new VoidJob(move(impl));
++    job->p_->public_instance_ = job;
++    QMetaObject::invokeMethod(job,
++                              "statusChanged",
++                              Qt::QueuedConnection,
++                              Q_ARG(unity::storage::qt::VoidJob::Status, job->p_->status_));
++    return job;
++}
++
++}  // namespace internal
++}  // namespace qt
++}  // namespace storage
++}  // namespace unity
 === modified file 'tests/remote-client/MockProvider.cpp'
 --- tests/remote-client/MockProvider.cpp	2016-09-16 06:25:08 +0000
 +++ tests/remote-client/MockProvider.cpp	2016-09-29 13:09:41 +0000
@@ -46,7 +46,19 @@
  boost::future<ItemList> MockProvider::roots(Context const&)
+ {
--    cerr << "roots CALLED" << endl;
++    if (cmd_ == "slow_roots")
++    {
++        this_thread::sleep_for(chrono::seconds(1));
++    }
++    if (cmd_ == "not_a_root")
++    {
++        ItemList roots =
++        {
++            {"root_id", {}, "Root", "etag", ItemType::file, {}}
++        };
++        return make_ready_future<ItemList>(roots);
++    }
++
      ItemList roots =
+     {
          {"root_id", {}, "Root", "etag", ItemType::root, {}}
@@ -103,6 +115,15 @@
  boost::future<Item> MockProvider::metadata(string const& item_id, Context const&)
+ {
++    if (cmd_ == "slow_metadata")
++    {
++        this_thread::sleep_for(chrono::seconds(1));
++    }
++    if (cmd_ == "empty_id")
++    {
++        Item metadata{"", {}, "Root", "etag", ItemType::root, {}};
++        return make_ready_future<Item>(metadata);
++    }
      if (item_id == "root_id")
+     {
          Item metadata{"root_id", {}, "Root", "etag", ItemType::root, {}};
@@ -168,8 +189,17 @@
+ }
  boost::future<void> MockProvider::delete_item(
--    string const&, Context const&)
++    string const& item_id, Context const&)
+ {
++    if (cmd_ == "slow_delete")
++    {
++        this_thread::sleep_for(chrono::seconds(1));
++    }
++    if (cmd_ == "delete_no_such_item")
++    {
++        string msg = "delete_item(): no such item: " + item_id;
++        return make_exceptional_future<void>(NotExistsException(msg, item_id));
++    }
      return make_ready_future();
+ }
 === modified file 'tests/remote-client/remote-client_test.cpp'
 --- tests/remote-client/remote-client_test.cpp	2016-09-26 02:37:03 +0000
 +++ tests/remote-client/remote-client_test.cpp	2016-09-29 13:09:41 +0000
@@ -24,6 +24,8 @@
  #include <gtest/gtest.h>
  #include <QSignalSpy>
++#include <unordered_set>
++
  using namespace unity::storage;
  using namespace unity::storage::qt;
  using namespace std;
@@ -52,10 +54,12 @@
      Account acc_;
  };
--class RuntimeTest : public ProviderFixture {};
  class AccountTest : public RemoteClientTest {};
++class DeleteTest : public RemoteClientTest {};
++class GetTest : public RemoteClientTest {};
++class ItemTest : public RemoteClientTest {};
  class RootsTest : public RemoteClientTest {};
--class ItemTest : public RemoteClientTest {};
++class RuntimeTest : public ProviderFixture {};
  TEST(Runtime, lifecycle)
+ {
@@ -75,6 +79,7 @@
+ }
  #if 0
++// TODO, how to test this?
  TEST_F(RuntimeTest, init_error)
+ {
      QDBusConnection conn(connection());
@@ -299,9 +304,12 @@
  TEST_F(AccountTest, hash)
+ {
++    unordered_set<Account>();  // Just to show that this works.
++
      Account a1;
++    EXPECT_EQ(0, hash<Account>()(a1));
      EXPECT_EQ(0, a1.hash());
--    EXPECT_EQ(a1.hash(), qHash(a1));
++    EXPECT_EQ(0, qHash(a1));
      auto a2 = runtime_->make_test_account(service_connection_->baseService(), object_path(), "a", "a", "a");
      // Due to different return types (size_t vs uint), hash() and qHash() do not return the same value.
@@ -317,11 +325,11 @@
      EXPECT_EQ(StorageError::NoError, j->error().type());
      EXPECT_EQ(QList<Account>(), j->accounts());  // We haven't waited for the result yet.
--    QSignalSpy spy(j.get(), &unity::storage::qt::AccountsJob::statusChanged);
++    QSignalSpy spy(j.get(), &AccountsJob::statusChanged);
      spy.wait(SIGNAL_WAIT_TIME);
      ASSERT_EQ(1, spy.count());
      auto arg = spy.takeFirst();
--    EXPECT_EQ(AccountsJob::Finished, qvariant_cast<unity::storage::qt::AccountsJob::Status>(arg.at(0)));
++    EXPECT_EQ(AccountsJob::Finished, qvariant_cast<AccountsJob::Status>(arg.at(0)));
      EXPECT_TRUE(j->isValid());
      EXPECT_EQ(AccountsJob::Finished, j->status());
@@ -342,16 +350,15 @@
      EXPECT_FALSE(j->isValid());
      EXPECT_EQ(AccountsJob::Error, j->status());
      EXPECT_EQ(StorageError::RuntimeDestroyed, j->error().type());
--    EXPECT_EQ("Runtime::accounts(): Runtime was destroyed previously",
--              j->error().message()) << j->error().message().toStdString();
++    EXPECT_EQ("Runtime::accounts(): Runtime was destroyed previously", j->error().message());
      EXPECT_EQ(QList<Account>(), j->accounts());
      // Signal must be received.
--    QSignalSpy spy(j, &unity::storage::qt::AccountsJob::statusChanged);
++    QSignalSpy spy(j, &AccountsJob::statusChanged);
      spy.wait(SIGNAL_WAIT_TIME);
      ASSERT_EQ(1, spy.count());
      auto arg = spy.takeFirst();
--    EXPECT_EQ(AccountsJob::Error, qvariant_cast<unity::storage::qt::AccountsJob::Status>(arg.at(0)));
++    EXPECT_EQ(AccountsJob::Error, qvariant_cast<AccountsJob::Status>(arg.at(0)));
+ }
  TEST_F(RootsTest, roots)
@@ -364,19 +371,19 @@
      EXPECT_EQ(StorageError::NoError, j->error().type());
      // Check that we get the statusChanged and itemsReady signals.
--    QSignalSpy ready_spy(j.get(), &unity::storage::qt::ItemListJob::itemsReady);
--    QSignalSpy status_spy(j.get(), &unity::storage::qt::ItemListJob::statusChanged);
++    QSignalSpy ready_spy(j.get(), &ItemListJob::itemsReady);
++    QSignalSpy status_spy(j.get(), &ItemListJob::statusChanged);
      ASSERT_TRUE(ready_spy.wait(SIGNAL_WAIT_TIME));
      ASSERT_EQ(1, ready_spy.count());
      auto arg = ready_spy.takeFirst();
      auto items = qvariant_cast<QList<Item>>(arg.at(0));
--    EXPECT_EQ(1, items.size());
++    ASSERT_EQ(1, items.size());
      ASSERT_EQ(1, status_spy.count());
      arg = status_spy.takeFirst();
--    EXPECT_EQ(ItemListJob::Finished, qvariant_cast<unity::storage::qt::ItemListJob::Status>(arg.at(0)));
++    EXPECT_EQ(ItemListJob::Finished, qvariant_cast<ItemListJob::Status>(arg.at(0)));
      EXPECT_EQ(StorageError::NoError, j->error().type());
      EXPECT_TRUE(j->isValid());
@@ -403,38 +410,349 @@
      EXPECT_FALSE(j->isValid());
      EXPECT_EQ(ItemListJob::Error, j->status());
      EXPECT_EQ(StorageError::RuntimeDestroyed, j->error().type());
--    EXPECT_EQ("Account::roots(): Runtime was destroyed previously",
--              j->error().message()) << j->error().message().toStdString();
--
--    // Signal must be received.
--    QSignalSpy spy(j.get(), &unity::storage::qt::ItemListJob::statusChanged);
--    spy.wait(SIGNAL_WAIT_TIME);
--    ASSERT_EQ(1, spy.count());
--    auto arg = spy.takeFirst();
--    EXPECT_EQ(ItemListJob::Error, qvariant_cast<unity::storage::qt::ItemListJob::Status>(arg.at(0)));
--}
--
--TEST_F(ItemTest, comparison)
++    EXPECT_EQ("Account::roots(): Runtime was destroyed previously", j->error().message());
++
++    // Signal must be received.
++    QSignalSpy spy(j.get(), &ItemListJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++    ASSERT_EQ(1, spy.count());
++    auto arg = spy.takeFirst();
++    EXPECT_EQ(ItemListJob::Error, qvariant_cast<ItemListJob::Status>(arg.at(0)));
++}
++
++TEST_F(RootsTest, runtime_destroyed_while_item_list_job_running)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("slow_roots")));
++
++    unique_ptr<ItemListJob> j(acc_.roots());
++    EXPECT_TRUE(j->isValid());
++    EXPECT_EQ(ItemListJob::Loading, j->status());
++    EXPECT_EQ(StorageError::NoError, j->error().type());
++
++    EXPECT_EQ(StorageError::NoError, runtime_->shutdown().type());  // Destroy runtime, provider still sleeping
++
++    // Signal must be received.
++    QSignalSpy spy(j.get(), &ItemListJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++    ASSERT_EQ(1, spy.count());
++    auto arg = spy.takeFirst();
++    EXPECT_EQ(ItemListJob::Error, qvariant_cast<ItemListJob::Status>(arg.at(0)));
++
++    EXPECT_EQ("Account::roots(): Runtime was destroyed previously", j->error().message());
++}
++
++TEST_F(RootsTest, not_a_root)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("not_a_root")));
++
++    unique_ptr<ItemListJob> j(acc_.roots());
++
++    QSignalSpy ready_spy(j.get(), &ItemListJob::itemsReady);
++    QSignalSpy status_spy(j.get(), &ItemListJob::statusChanged);
++    status_spy.wait(SIGNAL_WAIT_TIME);
++    auto arg = status_spy.takeFirst();
++
++    // Bad metadata is ignored, so status is finished, and itemsReady was never called.
++    EXPECT_EQ(ItemListJob::Finished, qvariant_cast<ItemListJob::Status>(arg.at(0)));
++    EXPECT_EQ(0, status_spy.count());
++}
++
++TEST_F(GetTest, basic)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
++
++    // Get root.
++    {
++        unique_ptr<ItemJob> j(acc_.get("root_id"));
++
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        spy.wait(SIGNAL_WAIT_TIME);
++
++        EXPECT_EQ("root_id", j->item().itemId());
++        EXPECT_EQ("Root", j->item().name());
++        EXPECT_EQ(Item::Root, j->item().type());
++    }
++
++    // Get a file.
++    {
++        unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        spy.wait(SIGNAL_WAIT_TIME);
++
++        EXPECT_EQ("child_id", j->item().itemId());
++        EXPECT_EQ("Child", j->item().name());
++        EXPECT_EQ(Item::File, j->item().type());
++    }
++
++    // Get a folder.
++    {
++        unique_ptr<ItemJob> j(acc_.get("child_folder_id"));
++
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        spy.wait(SIGNAL_WAIT_TIME);
++
++        EXPECT_EQ("child_folder_id", j->item().itemId());
++        EXPECT_EQ("Child_Folder", j->item().name());
++        EXPECT_EQ(Item::Folder, j->item().type());
++    }
++}
++
++TEST_F(GetTest, runtime_destroyed)
++{
++    EXPECT_EQ(StorageError::NoError, runtime_->shutdown().type());  // Destroy runtime.
++
++    unique_ptr<ItemJob> j(acc_.get("root_id"));
++    EXPECT_FALSE(j->isValid());
++    EXPECT_EQ(ItemJob::Error, j->status());
++    EXPECT_EQ(StorageError::RuntimeDestroyed, j->error().type());
++    EXPECT_EQ("Account::get(): Runtime was destroyed previously", j->error().message());
++
++    // Signal must be received.
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++    auto arg = spy.takeFirst();
++    EXPECT_EQ(ItemJob::Error, qvariant_cast<ItemJob::Status>(arg.at(0)));
++
++    EXPECT_EQ("Account::get(): Runtime was destroyed previously", j->error().message());
++}
++
++TEST_F(GetTest, runtime_destroyed_while_item_job_running)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("slow_metadata")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_folder_id"));
++    EXPECT_TRUE(j->isValid());
++
++    EXPECT_EQ(StorageError::NoError, runtime_->shutdown().type());  // Destroy runtime, provider still sleeping
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++    auto arg = spy.takeFirst();
++    EXPECT_EQ(ItemListJob::Error, qvariant_cast<ItemJob::Status>(arg.at(0)));
++
++    EXPECT_EQ("Account::get(): Runtime was destroyed previously", j->error().message());
++}
++
++TEST_F(GetTest, empty_id_from_provider)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("empty_id")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_folder_id"));
++    EXPECT_TRUE(j->isValid());
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++    auto arg = spy.takeFirst();
++    EXPECT_EQ(ItemListJob::Error, qvariant_cast<ItemJob::Status>(arg.at(0)));
++
++    EXPECT_EQ("Account::get(): received invalid metadata from provider: item_id cannot be empty", j->error().message());
++}
++
++TEST_F(GetTest, no_such_id)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
++
++    unique_ptr<ItemJob> j(acc_.get("no_such_id"));
++    EXPECT_TRUE(j->isValid());
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++    auto arg = spy.takeFirst();
++    EXPECT_EQ(ItemListJob::Error, qvariant_cast<ItemJob::Status>(arg.at(0)));
++
++    EXPECT_EQ("metadata(): no such item: no_such_id", j->error().message()) << j->error().message().toStdString();
++    EXPECT_EQ("no_such_id", j->error().itemId());
++}
++
++TEST_F(DeleteTest, basic)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider));
++
++    Item item;
++    {
++        unique_ptr<ItemJob> j(acc_.get("child_id"));
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        item = j->item();
++    }
++
++    unique_ptr<VoidJob> j(item.deleteItem());
++    EXPECT_TRUE(j->isValid());
++    EXPECT_EQ(VoidJob::Loading, j->status());
++    EXPECT_EQ(StorageError::NoError, j->error().type());
++
++    EXPECT_EQ("child_id", item.itemId());
++
++    QSignalSpy spy(j.get(), &VoidJob::statusChanged);
++    ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++
++    EXPECT_EQ(VoidJob::Finished, j->status());
++    EXPECT_TRUE(j->isValid());
++    EXPECT_EQ(StorageError::NoError, j->error().type());
++    EXPECT_EQ(VoidJob::Finished, j->status());
++}
++
++TEST_F(DeleteTest, no_such_item)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("delete_no_such_item")));
++
++    Item item;
++    {
++        unique_ptr<ItemJob> j(acc_.get("child_id"));
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        item = j->item();
++    }
++
++    unique_ptr<VoidJob> j(item.deleteItem());
++    EXPECT_TRUE(j->isValid());
++    EXPECT_EQ(VoidJob::Loading, j->status());
++    EXPECT_EQ(StorageError::NoError, j->error().type());
++
++    EXPECT_EQ("child_id", item.itemId());
++
++    QSignalSpy spy(j.get(), &VoidJob::statusChanged);
++    ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++
++    EXPECT_EQ(VoidJob::Error, j->status());
++    EXPECT_FALSE(j->isValid());
++    EXPECT_EQ(StorageError::NotExists, j->error().type());
++    EXPECT_EQ("delete_item(): no such item: child_id", j->error().message());
++}
++
++TEST_F(DeleteTest, delete_root)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider));
++
++    Item item;
++    {
++        unique_ptr<ItemJob> j(acc_.get("root_id"));
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        item = j->item();
++    }
++
++    unique_ptr<VoidJob> j(item.deleteItem());
++    EXPECT_FALSE(j->isValid());
++    EXPECT_EQ(VoidJob::Error, j->status());
++    EXPECT_EQ(StorageError::LogicError, j->error().type());
++
++    // Signal must be received.
++    QSignalSpy spy(j.get(), &VoidJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++    auto arg = spy.takeFirst();
++    EXPECT_EQ(ItemJob::Error, qvariant_cast<VoidJob::Status>(arg.at(0)));
++
++    EXPECT_EQ("Item::deleteItem(): cannot delete root", j->error().message());
++}
++
++TEST_F(DeleteTest, runtime_destroyed)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider));
++
++    Item item;
++    {
++        unique_ptr<ItemJob> j(acc_.get("child_id"));
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        item = j->item();
++    }
++
++    EXPECT_EQ(StorageError::NoError, runtime_->shutdown().type());  // Destroy runtime.
++
++    unique_ptr<VoidJob> j(item.deleteItem());
++    EXPECT_FALSE(j->isValid());
++    EXPECT_EQ(ItemJob::Error, j->status());
++    EXPECT_EQ(StorageError::RuntimeDestroyed, j->error().type());
++    EXPECT_EQ("Item::deleteItem(): Runtime was destroyed previously", j->error().message());
++
++    // Signal must be received.
++    QSignalSpy spy(j.get(), &VoidJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++    auto arg = spy.takeFirst();
++    EXPECT_EQ(ItemJob::Error, qvariant_cast<VoidJob::Status>(arg.at(0)));
++
++    EXPECT_EQ("Item::deleteItem(): Runtime was destroyed previously", j->error().message());
++}
++
++TEST_F(DeleteTest, runtime_destroyed_while_void_job_running)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("slow_delete")));
++
++    Item item;
++    {
++        unique_ptr<ItemJob> j(acc_.get("child_id"));
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        item = j->item();
++    }
++
++    unique_ptr<VoidJob> j(item.deleteItem());
++    EXPECT_TRUE(j->isValid());
++    EXPECT_EQ(VoidJob::Loading, j->status());
++    EXPECT_EQ(StorageError::NoError, j->error().type());
++
++    EXPECT_EQ(StorageError::NoError, runtime_->shutdown().type());  // Destroy runtime.
++
++    // Signal must be received.
++    QSignalSpy spy(j.get(), &VoidJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++    auto arg = spy.takeFirst();
++    EXPECT_EQ(VoidJob::Error, qvariant_cast<VoidJob::Status>(arg.at(0)));
++
++    EXPECT_EQ("Item::deleteItem(): Runtime was destroyed previously", j->error().message()) << j->error().message().toStdString();
++}
++
++#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, comparison_and_hash)
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider));
+     {
          // Both items invalid.
          Item i1;
--        Item a2;
--        EXPECT_TRUE(i1 == a2);
--        EXPECT_FALSE(i1 != a2);
--        EXPECT_FALSE(i1 < a2);
--        EXPECT_TRUE(i1 <= a2);
--        EXPECT_FALSE(i1 > a2);
--        EXPECT_TRUE(i1 >= a2);
++        Item i2;
++        EXPECT_TRUE(i1 == i2);
++        EXPECT_FALSE(i1 != i2);
++        EXPECT_FALSE(i1 < i2);
++        EXPECT_TRUE(i1 <= i2);
++        EXPECT_FALSE(i1 > i2);
++        EXPECT_TRUE(i1 >= i2);
++
++        unordered_set<Item>();  // Just to show that this works.
++
++        EXPECT_EQ(0, hash<Item>()(i1));
++        EXPECT_EQ(0, i1.hash());
++        EXPECT_EQ(0, qHash(i1));
+     }
--#if 0
+     {
          // i1 valid, i2 invalid
--        auto i1 = runtime_->make_test_account(service_connection_->baseService(), bus_path());
--        Account i2;
++        unique_ptr<ItemListJob> j(acc_.roots());
++
++        QSignalSpy ready_spy(j.get(), &ItemListJob::itemsReady);
++        ASSERT_TRUE(ready_spy.wait(SIGNAL_WAIT_TIME));
++
++        ASSERT_EQ(1, ready_spy.count());
++        auto arg = ready_spy.takeFirst();
++        auto items = qvariant_cast<QList<Item>>(arg.at(0));
++        ASSERT_EQ(1, items.size());
++
++        auto i1 = items[0];
++        Item i2;
          EXPECT_FALSE(i1 == i2);
          EXPECT_TRUE(i1 != i2);
          EXPECT_FALSE(i1 < i2);
@@ -449,75 +767,69 @@
          EXPECT_TRUE(i2 <= i1);
          EXPECT_FALSE(i2 > i1);
          EXPECT_FALSE(i2 >= i1);
--    }
--
--    {
--        // i1 < i2 for owner ID
--        auto i1 = runtime_->make_test_account(service_connection_->baseService(), bus_path(), "a", "x", "x");
--        auto i2 = runtime_->make_test_account(service_connection_->baseService(), bus_path(), "b", "x", "x");
--
--        EXPECT_FALSE(i1 == i2);
--        EXPECT_TRUE(i1 != i2);
--        EXPECT_TRUE(i1 < i2);
--        EXPECT_TRUE(i1 <= i2);
--        EXPECT_FALSE(i1 > i2);
--        EXPECT_FALSE(i1 >= i2);
--
--        // And with swapped operands:
--        EXPECT_FALSE(i2 == i1);
--        EXPECT_TRUE(i2 != i1);
--        EXPECT_FALSE(i2 < i1);
--        EXPECT_FALSE(i2 <= i1);
--        EXPECT_TRUE(i2 > i1);
--        EXPECT_TRUE(i2 >= i1);
--    }
--
--    {
--        // i1 < i2 for owner
--        auto i1 = runtime_->make_test_account(service_connection_->baseService(), bus_path(), "a", "a", "x");
--        auto i2 = runtime_->make_test_account(service_connection_->baseService(), bus_path(), "a", "b", "x");
--
--        EXPECT_FALSE(i1 == i2);
--        EXPECT_TRUE(i1 != i2);
--        EXPECT_TRUE(i1 < i2);
--        EXPECT_TRUE(i1 <= i2);
--        EXPECT_FALSE(i1 > i2);
--        EXPECT_FALSE(i1 >= i2);
--
--        // And with swapped operands:
--        EXPECT_FALSE(i2 == i1);
--        EXPECT_TRUE(i2 != i1);
--        EXPECT_FALSE(i2 < i1);
--        EXPECT_FALSE(i2 <= i1);
--        EXPECT_TRUE(i2 > i1);
--        EXPECT_TRUE(i2 >= i1);
--    }
--
--    {
--        // i1 < i2 for description
--        auto i1 = runtime_->make_test_account(service_connection_->baseService(), bus_path(), "a", "a", "a");
--        auto i2 = runtime_->make_test_account(service_connection_->baseService(), bus_path(), "a", "a", "b");
--
--        EXPECT_FALSE(i1 == i2);
--        EXPECT_TRUE(i1 != i2);
--        EXPECT_TRUE(i1 < i2);
--        EXPECT_TRUE(i1 <= i2);
--        EXPECT_FALSE(i1 > i2);
--        EXPECT_FALSE(i1 >= i2);
--
--        // And with swapped operands:
--        EXPECT_FALSE(i2 == i1);
--        EXPECT_TRUE(i2 != i1);
--        EXPECT_FALSE(i2 < i1);
--        EXPECT_FALSE(i2 <= i1);
--        EXPECT_TRUE(i2 > i1);
--        EXPECT_TRUE(i2 >= i1);
--    }
--
--    {
--        // i1 == i2
--        auto i1 = runtime_->make_test_account(service_connection_->baseService(), bus_path(), "a", "a", "a");
--        auto i2 = runtime_->make_test_account(service_connection_->baseService(), bus_path(), "a", "a", "a");
++
++        EXPECT_NE(0, i1.hash());
++        EXPECT_NE(0, qHash(i1));
++    }
++
++    {
++        // Both items valid with identical metadata, but different accounts (a1 < a2).
++
++        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");
++
++        Item i1;
++        Item i2;
++
++        {
++            unique_ptr<ItemJob> j(a1.get("root_id"));
++            QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++            i1 = j->item();
++        }
++        {
++            unique_ptr<ItemJob> j(a2.get("root_id"));
++            QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++            i2 = j->item();
++        }
++
++        ASSERT_EQ(i1.itemId(), i2.itemId());
++
++        EXPECT_FALSE(i1 == i2);
++        EXPECT_TRUE(i1 != i2);
++        EXPECT_TRUE(i1 < i2);
++        EXPECT_TRUE(i1 <= i2);
++        EXPECT_FALSE(i1 > i2);
++        EXPECT_FALSE(i1 >= i2);
++
++        // And with swapped operands:
++        EXPECT_FALSE(i2 == i1);
++        EXPECT_TRUE(i2 != i1);
++        EXPECT_FALSE(i2 < i1);
++        EXPECT_FALSE(i2 <= i1);
++        EXPECT_TRUE(i2 > i1);
++        EXPECT_TRUE(i2 >= i1);
++    }
++
++    {
++        // Both items valid with identical metadata, but different instances, so we do deep comparison.
++
++        Item i1;
++        Item i2;
++
++        {
++            unique_ptr<ItemJob> j(acc_.get("root_id"));
++            QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++            i1 = j->item();
++        }
++        {
++            unique_ptr<ItemJob> j(acc_.get("root_id"));
++            QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++            i2 = j->item();
++        }
          EXPECT_TRUE(i1 == i2);
          EXPECT_FALSE(i1 != i2);
@@ -533,8 +845,10 @@
          EXPECT_TRUE(i2 <= i1);
          EXPECT_FALSE(i2 > i1);
          EXPECT_TRUE(i2 >= i1);
++
++        EXPECT_EQ(i1.hash(), i2.hash());
++        EXPECT_EQ(qHash(i1), qHash(i2));
+     }
--#endif
+ }
  #if 0
@@ -1290,7 +1604,7 @@
  TEST_F(DestroyedTest, get_destroyed_while_reply_outstanding)
+ {
--    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("metadata slow")));
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("slow_metadata")));
      auto root = call(acc_->roots())[0];
      auto fut = root->get("root_id");
@@ -1328,7 +1642,7 @@
  TEST_F(DestroyedTest, move_destroyed_while_reply_outstanding)
+ {
--    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("move slow")));
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("slow_move")));
      auto root = call(acc_->roots())[0];
      auto file = dynamic_pointer_cast<File>(call(root->get("child_id")));