Storage Framework

Merge lp:~michihenning/storage-framework/metadata-keys into lp:storage-framework/devel

metadata-keys
Merge into devel

Proposed by Michi Henning on 2016-10-31

Status:	Merged
Approved by:	James Henstridge on 2016-11-02
Approved revision:	92
Merged at revision:	84
Proposed branch:	lp:~michihenning/storage-framework/metadata-keys
Merge into:	lp:storage-framework/devel
Prerequisite:	lp:~michihenning/storage-framework/specify-metadata
Diff against target:	1147 lines (+500/-115) 18 files modified CMakeLists.txt (+1/-1) debian/changelog (+1/-0) demo/provider_test/provider-test.cpp (+5/-5) include/unity/storage/common.h (+19/-0) include/unity/storage/internal/metadata_keys.h (+22/-15) include/unity/storage/qt/Downloader.h (+1/-1) include/unity/storage/qt/Item.h (+1/-3) include/unity/storage/qt/MetadataKeys.h.THIS (+37/-0) include/unity/storage/qt/internal/ItemImpl.h (+1/-2) src/qt/Item.cpp (+5/-10) src/qt/client/internal/remote_client/FileImpl.cpp (+2/-2) src/qt/client/internal/remote_client/ItemImpl.cpp (+3/-3) src/qt/client/internal/remote_client/validate.cpp (+14/-9) src/qt/internal/ItemImpl.cpp (+14/-14) src/qt/internal/validate.cpp (+43/-31) tests/remote-client-v1/MockProvider.cpp (+5/-5) tests/remote-client/MockProvider.cpp (+123/-14) tests/remote-client/remote-client_test.cpp (+203/-0)
To merge this branch:	bzr merge lp:~michihenning/storage-framework/metadata-keys
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
unity-api-1-bot	continuous-integration		Approve on 2016-11-02
James Henstridge		2016-10-31	Approve on 2016-11-02
Review via email: mp+309653@code.launchpad.net

Commit message

Moved metadata key definitions into common.h and move metadata_keys.h to include/unity/storage/internal so both client and server side can see them.
Added commonly supported metadata keys.
Removed free and used space methods (they are metadata now).
No metadata validation for roots because Dropbox can't support that.

Description of the change

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-10-31:

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

review: Approve (continuous-integration)

lp:~michihenning/storage-framework/metadata-keys updated on 2016-10-31

88. By Michi Henning on 2016-10-31: Updated changelog. Bumped API version.
89. By Michi Henning on 2016-10-31: Server-side API version back to 1. Updated changelog accordingly.

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-10-31:

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

review: Approve (continuous-integration)

Revision history for this message

unity-api-1-bot (unity-api-1-bot) wrote on 2016-10-31:

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

review: Approve (continuous-integration)

lp:~michihenning/storage-framework/metadata-keys updated on 2016-11-02

90. By Michi Henning on 2016-11-02: Merged dependent branch.

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/168/rebuild

review: Needs Fixing (continuous-integration)

lp:~michihenning/storage-framework/metadata-keys updated on 2016-11-02

91. By Michi Henning on 2016-11-02: Merged dependent branch.

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/171/rebuild

review: Approve (continuous-integration)

lp:~michihenning/storage-framework/metadata-keys updated on 2016-11-02

92. By Michi Henning on 2016-11-02: Relaxed validation of etag so it applies only to files.

Revision history for this message

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

Looks good.

review: Approve

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/173/rebuild

review: Approve (continuous-integration)

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 'CMakeLists.txt'
 --- CMakeLists.txt	2016-09-29 11:37:05 +0000
 +++ CMakeLists.txt	2016-11-02 06:07:10 +0000
@@ -8,7 +8,7 @@
  project(storage-framework VERSION "0.2" LANGUAGES C CXX)
  # These variables should be incremented when we wish to create a new
--# source incompatible version of the the library where users of the
++# source incompatible version of the library where users of the
  # old API will not compile against the new one.  It is not
  # necessary to increment this for ABI breaks that are source compatible.
  set(SF_CLIENT_API_VERSION "2")
 === modified file 'debian/changelog'
 --- debian/changelog	2016-09-21 04:41:06 +0000
 +++ debian/changelog	2016-11-02 06:07:10 +0000
@@ -2,6 +2,7 @@
    [ Michi Henning ]
    * Added v2 of the client-side API.
++  * Updated server-side API to tell the provider which metadata to return.
   -- Michi Henning <michi.henning@canonical.com>  Wed, 21 Sep 2016 14:36:15 +1000
 === modified file 'demo/provider_test/provider-test.cpp'
 --- demo/provider_test/provider-test.cpp	2016-11-02 06:07:10 +0000
 +++ demo/provider_test/provider-test.cpp	2016-11-02 06:07:10 +0000
@@ -16,9 +16,9 @@
   * Authors: James Henstridge <james.henstridge@canonical.com>
   */
++#include <unity/storage/common.h>
  #include <unity/storage/provider/DownloadJob.h>
  #include <unity/storage/provider/Exceptions.h>
--#include <unity/storage/provider/metadata_keys.h>
  #include <unity/storage/provider/ProviderBase.h>
  #include <unity/storage/provider/Server.h>
  #include <unity/storage/provider/TempfileUploadJob.h>
@@ -128,7 +128,7 @@
+     {
+         {
              "child_id", { "root_id" }, "Child", "etag", ItemType::file,
--            { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
+         }
      };
      boost::promise<tuple<ItemList,string>> p;
@@ -154,7 +154,7 @@
      ItemList children =
+     {
          { "child_id", { "root_id" }, "Child", "etag", ItemType::file,
--          { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } } }
++          { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } } }
      };
      return make_ready_future<ItemList>(children);
+ }
@@ -175,7 +175,7 @@
          Item metadata
+         {
              "child_id", { "root_id" }, "Child", "etag", ItemType::file,
--            { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
          };
          return make_ready_future<Item>(metadata);
+     }
@@ -289,7 +289,7 @@
      Item metadata
+     {
          "some_id", { "root_id" }, "some_upload", "etag", ItemType::file,
--        { { SIZE_IN_BYTES, 10 }, { LAST_MODIFIED_TIME, "2011-04-05T14:30:10.005Z" } }
++        { { metadata::SIZE_IN_BYTES, 10 }, { metadata::LAST_MODIFIED_TIME, "2011-04-05T14:30:10.005Z" } }
      };
      return make_ready_future(metadata);
+ }
 === modified file 'include/unity/storage/common.h'
 --- include/unity/storage/common.h	2016-07-12 02:22:05 +0000
 +++ include/unity/storage/common.h	2016-11-02 06:07:10 +0000
@@ -37,5 +37,24 @@
      overwrite,
  };
++namespace metadata
++{
++
++static char constexpr SIZE_IN_BYTES[] = "size_in_bytes";            // int64_t, >= 0
++static char constexpr CREATION_TIME[] = "creation_time";            // String, ISO 8601 format
++static char constexpr LAST_MODIFIED_TIME[] = "last_modified_time";  // String, ISO 8601 format
++static char constexpr CHILD_COUNT[] = "child_count";                // int64_t, >= 0
++static char constexpr DESCRIPTION[] = "description";                // String
++static char constexpr DISPLAY_NAME[] = "display_name";              // String
++static char constexpr FREE_SPACE_BYTES[] = "free_space_bytes";      // int64_t, >= 0
++static char constexpr USED_SPACE_BYTES[] = "used_space_bytes";      // int64_t, >= 0
++static char constexpr CONTENT_TYPE[] = "content_type";              // String
++static char constexpr WRITABLE[] = "writable";                      // Bool
++static char constexpr MD5[] = "md5";                                // String
++static char constexpr DOWNLOAD_URL[] = "download_url";              // String
++
++static char constexpr ALL[] = "__ALL__";
++
++}  // namespace metadata
  }  // namespace storage
  }  // namespace unity
 === renamed file 'include/unity/storage/provider/metadata_keys.h' => 'include/unity/storage/internal/metadata_keys.h'
 --- include/unity/storage/provider/metadata_keys.h	2016-08-09 02:25:13 +0000
 +++ include/unity/storage/internal/metadata_keys.h	2016-11-02 06:07:10 +0000
@@ -18,28 +18,35 @@
  #pragma once
++#include <unity/storage/common.h>
++
  #include <unordered_map>
  namespace unity
+ {
  namespace storage
+ {
--namespace provider
--{
--
--static char constexpr SIZE_IN_BYTES[] = "size_in_bytes";            // int64_t, >= 0
--static char constexpr CREATION_TIME[] = "creation_time";            // String, ISO 8601 format
--static char constexpr LAST_MODIFIED_TIME[] = "last_modified_time";  // String, ISO 8601 format
--
--enum class MetadataType { int64, iso_8601_date_time };
--
--static std::unordered_map<std::string, MetadataType> known_metadata =
--{
--    { SIZE_IN_BYTES, MetadataType::int64 },
--    { CREATION_TIME, MetadataType::iso_8601_date_time },
--    { LAST_MODIFIED_TIME, MetadataType::iso_8601_date_time }
++namespace metadata
++{
++
++enum class MetadataType { non_zero_pos_int64, iso_8601_date_time, string, boolean };
++
++static std::unordered_map<std::string, MetadataType> const known_metadata =
++{
++    { metadata::SIZE_IN_BYTES, MetadataType::non_zero_pos_int64 },
++    { metadata::CREATION_TIME, MetadataType::iso_8601_date_time },
++    { metadata::LAST_MODIFIED_TIME, MetadataType::iso_8601_date_time },
++    { metadata::CHILD_COUNT, MetadataType::non_zero_pos_int64 },
++    { metadata::DESCRIPTION, MetadataType::string },
++    { metadata::DISPLAY_NAME, MetadataType::string },
++    { metadata::FREE_SPACE_BYTES, MetadataType::non_zero_pos_int64 },
++    { metadata::USED_SPACE_BYTES, MetadataType::non_zero_pos_int64 },
++    { metadata::CONTENT_TYPE, MetadataType::string },
++    { metadata::WRITABLE, MetadataType::boolean },
++    { metadata::MD5, MetadataType::string },
++    { metadata::DOWNLOAD_URL, MetadataType::string }
  };
--}  // namespace provider
++}  // namespace metadata
  }  // namespace storage
  }  // namespace unity
 === modified file 'include/unity/storage/qt/Downloader.h'
 --- include/unity/storage/qt/Downloader.h	2016-10-12 05:25:20 +0000
 +++ include/unity/storage/qt/Downloader.h	2016-11-02 06:07:10 +0000
@@ -54,7 +54,7 @@
      bool isValid() const;        // Not nice, hides QLocalSocket::isValid()
      Status status() const;
      StorageError error() const;  // Not nice, hides QLocalSocket::error()
--    Item item() const;           // TODO: Should we keep this?
++    Item item() const;
      Q_INVOKABLE void finishDownload();
      Q_INVOKABLE void cancel();
 === modified file 'include/unity/storage/qt/Item.h'
 --- include/unity/storage/qt/Item.h	2016-11-02 06:07:10 +0000
 +++ include/unity/storage/qt/Item.h	2016-11-02 06:07:10 +0000
@@ -94,6 +94,7 @@
      QString etag() const;
      Type type() const;
      QVariantMap metadata() const;
++    qint64 sizeInBytes() const;
      QDateTime lastModifiedTime() const;
      QStringList parentIds() const;
@@ -120,9 +121,6 @@
                                       QString const& contentType,
                                       QStringList const& keys = QStringList()) const;
--    Q_INVOKABLE IntJob* freeSpaceBytes() const;
--    Q_INVOKABLE IntJob* usedSpaceBytes() const;
--
      bool operator==(Item const&) const;
      bool operator!=(Item const&) const;
      bool operator<(Item const&) const;
 === added file 'include/unity/storage/qt/MetadataKeys.h.THIS'
 --- include/unity/storage/qt/MetadataKeys.h.THIS	1970-01-01 00:00:00 +0000
 +++ include/unity/storage/qt/MetadataKeys.h.THIS	2016-11-02 06:07:10 +0000
@@ -0,0 +1,37 @@
++/*
++ * 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/internal/ItemImpl.h'
 --- include/unity/storage/qt/internal/ItemImpl.h	2016-11-02 06:07:10 +0000
 +++ include/unity/storage/qt/internal/ItemImpl.h	2016-11-02 06:07:10 +0000
@@ -55,6 +55,7 @@
      QString etag() const;
      Item::Type type() const;
      QVariantMap metadata() const;
++    qint64 sizeInBytes() const;
      QDateTime lastModifiedTime() const;
      QList<QString> parentIds() const;
@@ -73,8 +74,6 @@
                           qint64 sizeInBytes,
                           QString const& contentType,
                           QStringList const& keys) const;
--    IntJob* freeSpaceBytes() const;
--    IntJob* usedSpaceBytes() const;
      bool operator==(ItemImpl const&) const;
      bool operator!=(ItemImpl const&) const;
 === modified file 'src/qt/Item.cpp'
 --- src/qt/Item.cpp	2016-11-02 06:07:10 +0000
 +++ src/qt/Item.cpp	2016-11-02 06:07:10 +0000
@@ -107,6 +107,11 @@
      return p_->metadata();
+ }
++qint64 Item::sizeInBytes() const
++{
++    return p_->sizeInBytes();
++}
++
  QDateTime Item::lastModifiedTime() const
+ {
      return p_->lastModifiedTime();
@@ -171,16 +176,6 @@
      return p_->createFile(name, policy, sizeInBytes, contentType, keys);
+ }
--IntJob* Item::freeSpaceBytes() const
--{
--    return p_->freeSpaceBytes();
--}
--
--IntJob* Item::usedSpaceBytes() const
--{
--    return p_->usedSpaceBytes();
--}
--
  bool Item::operator==(Item const& other) const
+ {
      return p_->operator==(*other.p_);
 === modified file 'src/qt/client/internal/remote_client/FileImpl.cpp'
 --- src/qt/client/internal/remote_client/FileImpl.cpp	2016-11-02 06:07:10 +0000
 +++ src/qt/client/internal/remote_client/FileImpl.cpp	2016-11-02 06:07:10 +0000
@@ -19,7 +19,7 @@
  #include <unity/storage/qt/client/internal/remote_client/FileImpl.h>
  #include "ProviderInterface.h"
--#include <unity/storage/provider/metadata_keys.h>
++#include <unity/storage/common.h>
  #include <unity/storage/qt/client/File.h>
  #include <unity/storage/qt/client/internal/remote_client/Handler.h>
  #include <unity/storage/qt/client/internal/remote_client/DownloaderImpl.h>
@@ -51,7 +51,7 @@
  int64_t FileImpl::size() const
+ {
      throw_if_destroyed("File::size()");
--    return md_.metadata.value(provider::SIZE_IN_BYTES).toLongLong();
++    return md_.metadata.value(metadata::SIZE_IN_BYTES).toLongLong();
+ }
  QFuture<shared_ptr<Uploader>> FileImpl::create_uploader(ConflictPolicy policy, int64_t size)
 === modified file 'src/qt/client/internal/remote_client/ItemImpl.cpp'
 --- src/qt/client/internal/remote_client/ItemImpl.cpp	2016-11-02 06:07:10 +0000
 +++ src/qt/client/internal/remote_client/ItemImpl.cpp	2016-11-02 06:07:10 +0000
@@ -19,7 +19,7 @@
  #include <unity/storage/qt/client/internal/remote_client/ItemImpl.h>
  #include "ProviderInterface.h"
--#include <unity/storage/provider/metadata_keys.h>
++#include <unity/storage/common.h>
  #include <unity/storage/qt/client/Account.h>
  #include <unity/storage/qt/client/internal/remote_client/AccountImpl.h>
  #include <unity/storage/qt/client/internal/remote_client/FileImpl.h>
@@ -72,7 +72,7 @@
  QDateTime ItemImpl::last_modified_time() const
+ {
      throw_if_destroyed("Item::last_modified_time()");
--    return QDateTime::fromString(md_.metadata.value(provider::LAST_MODIFIED_TIME).toString(), Qt::ISODate);
++    return QDateTime::fromString(md_.metadata.value(metadata::LAST_MODIFIED_TIME).toString(), Qt::ISODate);
+ }
  QFuture<shared_ptr<Item>> ItemImpl::copy(shared_ptr<Folder> const& new_parent, QString const& new_name)
@@ -254,7 +254,7 @@
  QDateTime ItemImpl::creation_time() const
+ {
      throw_if_destroyed("Item::creation_time()");
--    return QDateTime::fromString(md_.metadata.value(provider::CREATION_TIME).toString(), Qt::ISODate);
++    return QDateTime::fromString(md_.metadata.value(metadata::CREATION_TIME).toString(), Qt::ISODate);
+ }
  MetadataMap ItemImpl::native_metadata() const
 === modified file 'src/qt/client/internal/remote_client/validate.cpp'
 --- src/qt/client/internal/remote_client/validate.cpp	2016-09-28 10:08:40 +0000
 +++ src/qt/client/internal/remote_client/validate.cpp	2016-11-02 06:07:10 +0000
@@ -18,7 +18,7 @@
  #include <unity/storage/internal/ItemMetadata.h>
--#include <unity/storage/provider/metadata_keys.h>
++#include <unity/storage/internal/metadata_keys.h>
  #include <unity/storage/qt/client/Exceptions.h>
  #include <QDateTime>
@@ -48,9 +48,9 @@
  void validate_type_and_value(QString const& prefix,
                               QMapIterator<QString, QVariant> actual,
--                             unordered_map<string, provider::MetadataType>::const_iterator known)
++                             unordered_map<string, metadata::MetadataType>::const_iterator known)
+ {
--    using namespace unity::storage::provider;
++    using namespace unity::storage::metadata;
      switch (known->second)
+     {
@@ -75,7 +75,7 @@
+             }
              break;
+         }
--        case MetadataType::int64:
++        case MetadataType::non_zero_pos_int64:
+         {
              if (actual.value().type() != QVariant::LongLong)
+             {
@@ -84,6 +84,11 @@
+             }
              break;
+         }
++        case MetadataType::string:
++        case MetadataType::boolean:
++        {
++            break;
++        }
          default:
+         {
              abort();  // Impossible.  // LCOV_EXCL_LINE
@@ -95,7 +100,7 @@
  void validate(QString const& method, ItemMetadata const& md)
+ {
--    using namespace unity::storage::provider;
++    using namespace unity::storage::metadata;
      QString prefix = method + ": received invalid metadata from server: ";
@@ -150,13 +155,13 @@
      // Sanity check metadata to make sure that mandatory fields are present.
      if (md.type == ItemType::file)
+     {
--        if (!md.metadata.contains(SIZE_IN_BYTES))
++        if (!md.metadata.contains(metadata::SIZE_IN_BYTES))
+         {
--            throw LocalCommsException(prefix + "missing key " + SIZE_IN_BYTES + " in metadata for " + md.item_id);
++            throw LocalCommsException(prefix + "missing key " + metadata::SIZE_IN_BYTES + " in metadata for " + md.item_id);
+         }
--        if (!md.metadata.contains(LAST_MODIFIED_TIME))
++        if (!md.metadata.contains(metadata::LAST_MODIFIED_TIME))
+         {
--            throw LocalCommsException(prefix + "missing key " + LAST_MODIFIED_TIME + " in metadata for " + md.item_id);
++            throw LocalCommsException(prefix + "missing key " + metadata::LAST_MODIFIED_TIME + " in metadata for " + md.item_id);
+         }
+     }
+ }
 === modified file 'src/qt/internal/ItemImpl.cpp'
 --- src/qt/internal/ItemImpl.cpp	2016-11-02 06:07:10 +0000
 +++ src/qt/internal/ItemImpl.cpp	2016-11-02 06:07:10 +0000
@@ -19,7 +19,7 @@
  #include <unity/storage/qt/internal/ItemImpl.h>
  #include "ProviderInterface.h"
--#include <unity/storage/provider/metadata_keys.h>
++#include <unity/storage/common.h>
  #include <unity/storage/qt/internal/DownloaderImpl.h>
  #include <unity/storage/qt/internal/ItemJobImpl.h>
  #include <unity/storage/qt/internal/ItemListJobImpl.h>
@@ -96,13 +96,23 @@
  QVariantMap ItemImpl::metadata() const
+ {
--    // TODO: Need to agree on metadata representation.
--    return is_valid_ ? QVariantMap() : QVariantMap();
++    return is_valid_ ? md_.metadata : QVariantMap();
++}
++
++qint64 ItemImpl::sizeInBytes() const
++{
++    if (!is_valid_ || md_.type != ItemType::file)
++    {
++        return 0;
++    }
++    auto variant = md_.metadata.value(metadata::SIZE_IN_BYTES);
++    assert(variant.isValid());
++    return variant.toLongLong();
+ }
  QDateTime ItemImpl::lastModifiedTime() const
+ {
--    return is_valid_ ? QDateTime::fromString(md_.metadata.value(provider::LAST_MODIFIED_TIME).toString(), Qt::ISODate)
++    return is_valid_ ? QDateTime::fromString(md_.metadata.value(metadata::LAST_MODIFIED_TIME).toString(), Qt::ISODate)
                       : QDateTime();
+ }
@@ -425,16 +435,6 @@
      return UploaderImpl::make_job(This, method, reply, validate, policy, sizeInBytes);
+ }
--IntJob* ItemImpl::freeSpaceBytes() const
--{
--    return nullptr;  // TODO
--}
--
--IntJob* ItemImpl::usedSpaceBytes() const
--{
--    return nullptr;  // TODO
--}
--
  bool ItemImpl::operator==(ItemImpl const& other) const
+ {
      if (is_valid_)
 === modified file 'src/qt/internal/validate.cpp'
 --- src/qt/internal/validate.cpp	2016-09-16 06:25:08 +0000
 +++ src/qt/internal/validate.cpp	2016-11-02 06:07:10 +0000
@@ -19,7 +19,7 @@
  #include <unity/storage/qt/internal/validate.h>
  #include <unity/storage/internal/ItemMetadata.h>
--#include <unity/storage/provider/metadata_keys.h>
++#include <unity/storage/internal/metadata_keys.h>
  #include <unity/storage/qt/internal/StorageErrorImpl.h>
  #include <QDateTime>
@@ -45,9 +45,9 @@
  void validate_type_and_value(QString const& prefix,
                               QMapIterator<QString, QVariant> actual,
--                             unordered_map<string, provider::MetadataType>::const_iterator known)
++                             unordered_map<string, metadata::MetadataType>::const_iterator known)
+ {
--    using namespace unity::storage::provider;
++    using namespace unity::storage::metadata;
      switch (known->second)
+     {
@@ -55,7 +55,7 @@
+         {
              if (actual.value().type() != QVariant::String)
+             {
--                QString msg = prefix + actual.key() + ": expected value of type String, but received value of type "
++                QString msg = prefix + actual.key() + ": expected value of type QString, but received value of type "
                                + actual.value().typeName();
                  throw StorageErrorImpl::local_comms_error(msg);
+             }
@@ -75,14 +75,26 @@
+             }
              break;
+         }
--        case MetadataType::int64:
--        {
--            if (actual.value().type() != QVariant::LongLong)
--            {
--                QString msg = prefix + actual.key() + ": expected value of type LongLong, but received value of type "
--                              + actual.value().typeName();
--                throw StorageErrorImpl::local_comms_error(msg);
--            }
++        case MetadataType::non_zero_pos_int64:
++        {
++            auto variant = actual.value();
++            if (variant.type() != QVariant::LongLong)
++            {
++                QString msg = prefix + actual.key() + ": expected value of type qlonglong, but received value of type "
++                              + variant.typeName();
++                throw StorageErrorImpl::local_comms_error(msg);
++            }
++            qint64 val = variant.toLongLong();
++            if (val < 0)
++            {
++                QString msg = prefix + actual.key() + ": expected value >= 0, but received " + QString::number(val);
++                throw StorageErrorImpl::local_comms_error(msg);
++            }
++            break;
++        }
++        case MetadataType::string:
++        case MetadataType::boolean:
++        {
              break;
+         }
          default:
@@ -96,7 +108,7 @@
  void validate(QString const& method, ItemMetadata const& md)
+ {
--    using namespace unity::storage::provider;
++    using namespace unity::storage::metadata;
      QString prefix = method + ": received invalid metadata from provider: ";
@@ -121,15 +133,18 @@
+     }
      if (md.type == ItemType::root && !md.parent_ids.isEmpty())
+     {
--        throw StorageErrorImpl::local_comms_error(prefix + "metadata: parent_ids of root must be empty");
--    }
--    if (md.name.isEmpty())
--    {
--        throw StorageErrorImpl::local_comms_error(prefix + "name cannot be empty");
--    }
--    if (md.etag.isEmpty())
--    {
--        throw StorageErrorImpl::local_comms_error(prefix + "etag cannot be empty");
++        throw StorageErrorImpl::local_comms_error(prefix + "parent_ids of root must be empty");
++    }
++    if (md.type != ItemType::root)  // Dropbox does not support metadata for roots.
++    {
++        if (md.name.isEmpty())
++        {
++            throw StorageErrorImpl::local_comms_error(prefix + "name cannot be empty");
++        }
++    }
++    if (md.type == ItemType::file && md.etag.isEmpty())  // WebDav doesn't do etag for folders.
++    {
++        throw StorageErrorImpl::local_comms_error(prefix + "etag of a file cannot be empty");
+     }
      // Sanity check metadata to make sure only known metadata keys appear.
@@ -140,7 +155,7 @@
          auto known = known_metadata.find(actual.key().toStdString());
          if (known == known_metadata.end())
+         {
--            qWarning() << prefix << "unknown metadata key:" << actual.key();
++            qWarning().noquote().nospace() << prefix << "unknown metadata key: \"" << actual.key() << "\"";
+         }
          else
+         {
@@ -151,14 +166,11 @@
      // Sanity check metadata to make sure that mandatory fields are present.
      if (md.type == ItemType::file)
+     {
--        if (!md.metadata.contains(SIZE_IN_BYTES))
--        {
--            QString msg = prefix + "missing key " + SIZE_IN_BYTES + " in metadata for " + md.item_id;
--            throw StorageErrorImpl::local_comms_error(msg);
--        }
--        if (!md.metadata.contains(LAST_MODIFIED_TIME))
--        {
--            QString msg = prefix + "missing key " + LAST_MODIFIED_TIME + " in metadata for " + md.item_id;
++        if (!md.metadata.contains(metadata::SIZE_IN_BYTES) ||
++            !md.metadata.contains(metadata::LAST_MODIFIED_TIME))
++        {
++            QString msg = prefix + "missing key \"" + metadata::SIZE_IN_BYTES + "\" "
++                          "in metadata for \"" + md.item_id + "\"";
              throw StorageErrorImpl::local_comms_error(msg);
+         }
+     }
 === modified file 'tests/remote-client-v1/MockProvider.cpp'
 --- tests/remote-client-v1/MockProvider.cpp	2016-11-02 06:07:10 +0000
 +++ tests/remote-client-v1/MockProvider.cpp	2016-11-02 06:07:10 +0000
@@ -18,8 +18,8 @@
  #include "MockProvider.h"
++#include <unity/storage/internal/metadata_keys.h>
  #include <unity/storage/provider/Exceptions.h>
--#include <unity/storage/provider/metadata_keys.h>
  #include <boost/thread.hpp>
  #include <boost/thread/future.hpp>
@@ -71,7 +71,7 @@
+     {
+         {
              "child_id", { "root_id" }, "Child", "etag", ItemType::file,
--            { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
+         }
      };
      boost::promise<tuple<ItemList,string>> p;
@@ -95,7 +95,7 @@
      ItemList children =
+     {
          { "child_id", { "root_id" }, "Child", "etag", ItemType::file,
--          { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } } }
++          { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } } }
      };
      return make_ready_future<ItemList>(children);
+ }
@@ -112,7 +112,7 @@
          Item metadata
+         {
              "child_id", { "root_id" }, "Child", "etag", ItemType::file,
--            { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
          };
          return make_ready_future<Item>(metadata);
+     }
@@ -209,7 +209,7 @@
      Item metadata
+     {
          "some_id", { "root_id" }, "some_upload", "etag", ItemType::file,
--        { { SIZE_IN_BYTES, 10 }, { LAST_MODIFIED_TIME, "2011-04-05T14:30:10.005Z" } }
++        { { metadata::SIZE_IN_BYTES, 10 }, { metadata::LAST_MODIFIED_TIME, "2011-04-05T14:30:10.005Z" } }
      };
      return make_ready_future(metadata);
+ }
 === modified file 'tests/remote-client/MockProvider.cpp'
 --- tests/remote-client/MockProvider.cpp	2016-11-02 06:07:10 +0000
 +++ tests/remote-client/MockProvider.cpp	2016-11-02 06:07:10 +0000
@@ -18,8 +18,8 @@
  #include "MockProvider.h"
++#include <unity/storage/internal/metadata_keys.h>
  #include <unity/storage/provider/Exceptions.h>
--#include <unity/storage/provider/metadata_keys.h>
  #include <boost/thread.hpp>
  #include <boost/thread/future.hpp>
@@ -96,7 +96,7 @@
+         {
+             {
                  "child_id", { "root_id" }, "Child", "etag", ItemType::root,
--                { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++                { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
+             }
          };
          boost::promise<tuple<ItemList,string>> p;
@@ -114,7 +114,7 @@
+             {
+                 {
                      "child_id", { "root_id" }, "Child", "etag", ItemType::file,
--                    { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++                    { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
+                 }
              };
+         }
@@ -125,7 +125,7 @@
+             {
+                 {
                      "child2_id", { "root_id" }, "Child2", "etag", ItemType::file,
--                    { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++                    { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
+                 }
              };
+         }
@@ -147,7 +147,7 @@
+     {
+         {
              "child_id", { "root_id" }, "Child", "etag", ItemType::file,
--            { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
+         }
      };
      boost::promise<tuple<ItemList,string>> p;
@@ -171,7 +171,7 @@
      ItemList children =
+     {
          { "child_id", { "root_id" }, "Child", "etag", ItemType::file,
--          { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } } }
++          { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } } }
      };
      return make_ready_future<ItemList>(children);
+ }
@@ -216,24 +216,133 @@
                  return make_ready_future<Item>(metadata);
+             }
+         }
++        if (cmd_ == "root_with_parent")
++        {
++            Item metadata{"root_id", { "this shouldn't be here" }, "Root", "etag", ItemType::root, {}};
++            return make_ready_future<Item>(metadata);
++        }
          Item metadata{"root_id", {}, "Root", "etag", ItemType::root, {}};
          return make_ready_future<Item>(metadata);
+     }
      if (item_id == "child_id")
+     {
++        if (cmd_ == "no_parents")
++        {
++            Item metadata
++            {
++                "child_id", {}, "Child", "etag", ItemType::file,
++                { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            };
++            return make_ready_future<Item>(metadata);
++        }
++        if (cmd_ == "empty_name")
++        {
++            Item metadata
++            {
++                "child_id", { "root_id" }, "", "etag", ItemType::file,
++                { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            };
++            return make_ready_future<Item>(metadata);
++        }
++        if (cmd_ == "empty_etag")
++        {
++            Item metadata
++            {
++                "child_id", { "root_id" }, "Child", "", ItemType::file,
++                { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            };
++            return make_ready_future<Item>(metadata);
++        }
++        if (cmd_ == "unknown_key")
++        {
++            Item metadata
++            {
++                "child_id", { "root_id" }, "Child", "etag", ItemType::file,
++                { { metadata::SIZE_IN_BYTES, 0 },
++                  { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" },
++                  { metadata::DESCRIPTION, "child test file" },  // For coverage
++                  { metadata::WRITABLE, true },                  // For coverage
++                  { "unknown_key", "" }
++                }
++            };
++            return make_ready_future<Item>(metadata);
++        }
++        if (cmd_ == "missing_key")
++        {
++            Item metadata
++            {
++                "child_id", { "root_id" }, "Child", "etag", ItemType::file,
++                { { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            };
++            return make_ready_future<Item>(metadata);
++        }
++        if (cmd_ == "wrong_type_for_time")
++        {
++            Item metadata
++            {
++                "child_id", { "root_id" }, "Child", "etag", ItemType::file,
++                { { metadata::SIZE_IN_BYTES, 10 }, { metadata::LAST_MODIFIED_TIME, true } }
++            };
++            return make_ready_future<Item>(metadata);
++        }
++        if (cmd_ == "bad_parse_for_time")
++        {
++            Item metadata
++            {
++                "child_id", { "root_id" }, "Child", "etag", ItemType::file,
++                { { metadata::SIZE_IN_BYTES, 10 }, { metadata::LAST_MODIFIED_TIME, "xyz" } }
++            };
++            return make_ready_future<Item>(metadata);
++        }
++        if (cmd_ == "missing_timezone")
++        {
++            Item metadata
++            {
++                "child_id", { "root_id" }, "Child", "etag", ItemType::file,
++                { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30" } }
++            };
++            return make_ready_future<Item>(metadata);
++        }
++        if (cmd_ == "wrong_type_for_size")
++        {
++            Item metadata
++            {
++                "child_id", { "root_id" }, "Child", "etag", ItemType::file,
++                { { metadata::SIZE_IN_BYTES, "10" }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            };
++            return make_ready_future<Item>(metadata);
++        }
++        if (cmd_ == "negative_size")
++        {
++            Item metadata
++            {
++                "child_id", { "root_id" }, "Child", "etag", ItemType::file,
++                { { metadata::SIZE_IN_BYTES, -1 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            };
++            return make_ready_future<Item>(metadata);
++        }
++        if (cmd_ == "empty_parent")
++        {
++            Item metadata
++            {
++                "child_id", { "" }, "Child", "etag", ItemType::file,
++                { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            };
++            return make_ready_future<Item>(metadata);
++        }
          if (cmd_ == "two_parents" || cmd_ == "two_parents_throw")
+         {
              Item metadata
+             {
                  "child_id", { "root_id", "child_folder_id" }, "Child", "etag", ItemType::file,
--                { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++                { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
              };
              return make_ready_future<Item>(metadata);
+         }
          Item metadata
+         {
              "child_id", { "root_id" }, "Child", "etag", ItemType::file,
--            { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            { { metadata::SIZE_IN_BYTES, 10 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
          };
          return make_ready_future<Item>(metadata);
+     }
@@ -335,7 +444,7 @@
          Item metadata
+         {
              "root_id", { new_parent_id }, new_name, "etag", ItemType::root,
--            { { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            { { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
          };
          return make_ready_future(metadata);
+     }
@@ -344,14 +453,14 @@
          Item metadata
+         {
              item_id, { new_parent_id }, new_name, "etag", ItemType::folder,
--            { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
          };
          return make_ready_future(metadata);
+     }
      Item metadata
+     {
          item_id, { new_parent_id }, new_name, "etag", ItemType::file,
--        { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++        { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
      };
      return make_ready_future(metadata);
+ }
@@ -365,14 +474,14 @@
          Item metadata
+         {
              "new_item_id", { new_parent_id }, new_name, "etag", ItemType::folder,
--            { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++            { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
          };
          return make_ready_future(metadata);
+     }
      Item metadata
+     {
          "new_item_id", { new_parent_id }, new_name, "etag", ItemType::file,
--        { { SIZE_IN_BYTES, 0 }, { LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
++        { { metadata::SIZE_IN_BYTES, 0 }, { metadata::LAST_MODIFIED_TIME, "2007-04-05T14:30Z" } }
      };
      return make_ready_future(metadata);
+ }
@@ -416,7 +525,7 @@
      Item metadata
+     {
          "child_id", { "root_id" }, "some_upload", "etag", ItemType::file,
--        { { SIZE_IN_BYTES, 10 }, { LAST_MODIFIED_TIME, "2011-04-05T14:30:10.005Z" } }
++        { { metadata::SIZE_IN_BYTES, 10 }, { metadata::LAST_MODIFIED_TIME, "2011-04-05T14:30:10.005Z" } }
      };
      return make_ready_future(metadata);
+ }
 === modified file 'tests/remote-client/remote-client_test.cpp'
 --- tests/remote-client/remote-client_test.cpp	2016-10-17 13:01:16 +0000
 +++ tests/remote-client/remote-client_test.cpp	2016-11-02 06:07:10 +0000
@@ -66,6 +66,7 @@
  class ItemTest : public RemoteClientTest {};
  class ListTest : public RemoteClientTest {};
  class LookupTest : public RemoteClientTest {};
++class MetadataTest : public RemoteClientTest {};
  class MoveTest : public RemoteClientTest {};
  class ParentsTest : public RemoteClientTest {};
  class RootsTest : public RemoteClientTest {};
@@ -345,6 +346,8 @@
      EXPECT_EQ(AccountsJob::Status::Finished, j->status());
      EXPECT_EQ(StorageError::Type::NoError, j->error().type());
++    EXPECT_TRUE(runtime_->connection().isConnected());  // Just for coverage.
++
      auto accounts = j->accounts();
      // We don't check the contents of accounts here because we are using the real online accounts manager
@@ -635,6 +638,203 @@
      EXPECT_EQ("no_such_id", j->error().itemId());
+ }
++TEST_F(MetadataTest, basic)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider()));
++
++    {
++        Item i;
++        EXPECT_EQ(0, i.metadata().size());
++    }
++
++    {
++        unique_ptr<ItemJob> j(acc_.get("root_id"));
++
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        spy.wait(SIGNAL_WAIT_TIME);
++
++        EXPECT_EQ(0, j->item().sizeInBytes());
++        EXPECT_EQ(0, j->item().metadata().size());
++    }
++
++    {
++        unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++        QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++        spy.wait(SIGNAL_WAIT_TIME);
++
++        EXPECT_EQ(10, j->item().sizeInBytes());
++        EXPECT_EQ(2, j->item().metadata().size());
++    }
++}
++
++TEST_F(MetadataTest, no_parents)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("no_parents")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    EXPECT_EQ(ItemJob::Status::Error, j->status());
++    EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
++              "file or folder must have at least one parent ID", j->error().errorString());
++}
++
++TEST_F(MetadataTest, empty_parent)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("empty_parent")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    EXPECT_EQ(ItemJob::Status::Error, j->status());
++    EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
++              "parent_id of file or folder cannot be empty", j->error().errorString());
++}
++
++TEST_F(MetadataTest, root_with_parent)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("root_with_parent")));
++
++    unique_ptr<ItemJob> j(acc_.get("root_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    EXPECT_EQ(ItemJob::Status::Error, j->status());
++    EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
++              "parent_ids of root must be empty", j->error().errorString());
++}
++
++TEST_F(MetadataTest, empty_name)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("empty_name")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    EXPECT_EQ(ItemJob::Status::Error, j->status());
++    EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
++              "name cannot be empty", j->error().errorString());
++}
++
++TEST_F(MetadataTest, empty_etag)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("empty_etag")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    EXPECT_EQ(ItemJob::Status::Error, j->status());
++    EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
++              "etag of a file cannot be empty", j->error().errorString());
++}
++
++TEST_F(MetadataTest, unknown_key)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("unknown_key")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    // We only emit a warning for unknown keys.
++    EXPECT_EQ(ItemJob::Status::Finished, j->status());
++}
++
++TEST_F(MetadataTest, missing_size)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("missing_key")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    EXPECT_EQ(ItemJob::Status::Error, j->status());
++    EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: "
++              "missing key \"size_in_bytes\" in metadata for \"child_id\"", j->error().errorString());
++}
++
++TEST_F(MetadataTest, wrong_type_for_time)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("wrong_type_for_time")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    EXPECT_EQ(ItemJob::Status::Error, j->status());
++    EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: last_modified_time: "
++              "expected value of type QString, but received value of type qlonglong", j->error().errorString());
++}
++
++TEST_F(MetadataTest, bad_parse_for_time)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("bad_parse_for_time")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    EXPECT_EQ(ItemJob::Status::Error, j->status());
++    EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: last_modified_time: "
++              "value \"xyz\" does not parse as ISO-8601 date", j->error().errorString());
++}
++
++TEST_F(MetadataTest, missing_timezone)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("missing_timezone")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    EXPECT_EQ(ItemJob::Status::Error, j->status());
++    EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: last_modified_time: "
++              "value \"2007-04-05T14:30\" lacks a time zone specification", j->error().errorString());
++}
++
++TEST_F(MetadataTest, wrong_type_for_size)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("wrong_type_for_size")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    EXPECT_EQ(ItemJob::Status::Error, j->status());
++    EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: size_in_bytes: "
++              "expected value of type qlonglong, but received value of type QString", j->error().errorString());
++}
++
++TEST_F(MetadataTest, negative_size)
++{
++    set_provider(unique_ptr<provider::ProviderBase>(new MockProvider("negative_size")));
++
++    unique_ptr<ItemJob> j(acc_.get("child_id"));
++
++    QSignalSpy spy(j.get(), &ItemJob::statusChanged);
++    spy.wait(SIGNAL_WAIT_TIME);
++
++    EXPECT_EQ(ItemJob::Status::Error, j->status());
++    EXPECT_EQ("LocalCommsError: Account::get(): received invalid metadata from provider: size_in_bytes: "
++              "expected value >= 0, but received -1", j->error().errorString());
++}
++
  TEST_F(DeleteTest, basic)
+ {
      set_provider(unique_ptr<provider::ProviderBase>(new MockProvider));
@@ -817,6 +1017,8 @@
          EXPECT_EQ("", i.name());
          EXPECT_EQ("", i.etag());
          EXPECT_EQ(Item::Type::File, i.type());
++        EXPECT_EQ(0, i.metadata().size());
++        EXPECT_EQ(0, i.sizeInBytes());
          auto mtime = i.lastModifiedTime();
          EXPECT_FALSE(mtime.isValid());
          auto pids = i.parentIds();
@@ -834,6 +1036,7 @@
          EXPECT_TRUE(i.isValid());
          EXPECT_EQ("child_id", i.itemId());
          EXPECT_EQ("Child", i.name());
++        EXPECT_EQ(10, i.sizeInBytes());
          EXPECT_TRUE(i.account().isValid());
          EXPECT_EQ("etag", i.etag());
          EXPECT_EQ(Item::Type::File, i.type());