Merge into devel : create-folder : Code : WebDAV Storage Provider

Status:	Merged
Approved by:	James Henstridge on 2016-11-11
Approved revision:	17
Merged at revision:	15
Proposed branch:	lp:~jamesh/storage-provider-webdav/create-folder
Merge into:	lp:storage-provider-webdav/devel
Diff against target:	262 lines (+213/-0) 5 files modified src/CMakeLists.txt (+1/-0) src/CreateFolderHandler.cpp (+55/-0) src/CreateFolderHandler.h (+35/-0) src/DavProvider.cpp (+3/-0) tests/davprovider/davprovider_test.cpp (+119/-0)
To merge this branch:	bzr merge lp:~jamesh/storage-provider-webdav/create-folder
Related bugs:	Link a bug report

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

Commit message

Implement the create_folder() D-Bus method.

Description of the change

Implement the create_folder() D-Bus method.

This is implemented in terms of an MKCOL HTTP request, followed by a PROPFIND on the item to retrieve its metadata if we get a "201 Created" response to the first request.

The test for the non-error case is mostly disabled due to missing ETag problems with storage-framework 0.1. It will need to be reenabled when porting to 0.2.

Revision history for this message

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

#

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

PASSED: Continuous integration, rev:17
https://jenkins.canonical.com/unity-api-1/job/lp-storage-provider-webdav-ci/23/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build/1043
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-0-fetch/1050
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/841
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=vivid+overlay/841/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/841
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=xenial+overlay/841/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/841
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=amd64,release=yakkety/841/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/841
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=vivid+overlay/841/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/841
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=xenial+overlay/841/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/841
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=armhf,release=yakkety/841/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/841
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=vivid+overlay/841/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/841
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=xenial+overlay/841/artifact/output/*zip*/output.zip
    SUCCESS: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/841
        deb: https://jenkins.canonical.com/unity-api-1/job/build-2-binpkg/arch=i386,release=yakkety/841/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

Revision history for this message

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

#

Looks good! (Error handling needs work, but we've decided to that in one go later, rather than piecemeal.)

One other test that would be interesting is to create a folder after deleting its parent folder, but I don't think that's essential right now.

review: Approve

Revision history for this message

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

#

review: Needs Fixing (continuous-integration)

Revision history for this message

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

#

review: Needs Fixing (continuous-integration)

Revision history for this message

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

#

review: Needs Fixing (continuous-integration)

Revision history for this message

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

#

review: Approve (continuous-integration)

WebDAV Storage Provider

Merge lp:~jamesh/storage-provider-webdav/create-folder into lp:storage-provider-webdav/devel

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'src/CMakeLists.txt'
 --- src/CMakeLists.txt	2016-11-08 08:11:31 +0000
 +++ src/CMakeLists.txt	2016-11-10 10:21:24 +0000
@@ -5,6 +5,7 @@
    DavUploadJob.cpp
    MultiStatusParser.cpp
    item_id.cpp
++  CreateFolderHandler.cpp
    PropFindHandler.cpp
    ListHandler.cpp
    LookupHandler.cpp
 === added file 'src/CreateFolderHandler.cpp'
 --- src/CreateFolderHandler.cpp	1970-01-01 00:00:00 +0000
 +++ src/CreateFolderHandler.cpp	2016-11-10 10:21:24 +0000
@@ -0,0 +1,55 @@
++#include "CreateFolderHandler.h"
++#include "RetrieveMetadataHandler.h"
++#include "item_id.h"
++
++using namespace std;
++using namespace unity::storage::provider;
++
++CreateFolderHandler::CreateFolderHandler(DavProvider const& provider,
++                                         string const& parent_id,
++                                         string const& name,
++                                         Context const& ctx)
++    : provider_(provider), item_id_(make_child_id(parent_id, name) + '/'),
++      context_(ctx)
++{
++    QUrl const base_url = provider.base_url(ctx);
++    QNetworkRequest request(id_to_url(item_id_, base_url));
++    mkcol_.reset(provider.send_request(request, QByteArrayLiteral("MKCOL"),
++                                       nullptr, ctx));
++    connect(mkcol_.get(), &QNetworkReply::finished,
++            this, &CreateFolderHandler::onFinished);
++}
++
++CreateFolderHandler::~CreateFolderHandler() = default;
++
++boost::future<Item> CreateFolderHandler::get_future()
++{
++    return promise_.get_future();
++}
++
++void CreateFolderHandler::onFinished()
++{
++    auto status = mkcol_->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
++
++    if (status != 201)
++    {
++        promise_.set_exception(RemoteCommsException("Error from MKCOL: " + to_string(status)));
++        deleteLater();
++        return;
++    }
++
++    metadata_.reset(
++        new RetrieveMetadataHandler(
++            provider_, item_id_, context_,
++            [this](Item const& item, boost::exception_ptr const& error) {
++                if (error)
++                {
++                    promise_.set_exception(error);
++                }
++                else
++                {
++                    promise_.set_value(item);
++                }
++                deleteLater();
++            }));
++}
 === added file 'src/CreateFolderHandler.h'
 --- src/CreateFolderHandler.h	1970-01-01 00:00:00 +0000
 +++ src/CreateFolderHandler.h	2016-11-10 10:21:24 +0000
@@ -0,0 +1,35 @@
++#pragma once
++
++#include <QBuffer>
++#include <QObject>
++#include <QNetworkReply>
++#include <unity/storage/provider/ProviderBase.h>
++
++#include <memory>
++
++class DavProvider;
++class RetrieveMetadataHandler;
++
++class CreateFolderHandler : public QObject {
++    Q_OBJECT
++public:
++    CreateFolderHandler(DavProvider const& provider,
++                        std::string const& parent_id, std::string const& name,
++                        unity::storage::provider::Context const& ctx);
++    ~CreateFolderHandler();
++
++    boost::future<unity::storage::provider::Item> get_future();
++
++private Q_SLOTS:
++    void onFinished();
++
++private:
++    boost::promise<unity::storage::provider::Item> promise_;
++
++    DavProvider const& provider_;
++    std::string const item_id_;
++    unity::storage::provider::Context const context_;
++
++    std::unique_ptr<QNetworkReply> mkcol_;
++    std::unique_ptr<RetrieveMetadataHandler> metadata_;
++};
 === modified file 'src/DavProvider.cpp'
 --- src/DavProvider.cpp	2016-11-08 08:11:31 +0000
 +++ src/DavProvider.cpp	2016-11-10 10:21:24 +0000
@@ -6,6 +6,7 @@
  #include "MetadataHandler.h"
  #include "DavDownloadJob.h"
  #include "DavUploadJob.h"
++#include "CreateFolderHandler.h"
  #include "item_id.h"
  #include <QDateTime>
@@ -62,6 +63,8 @@
  boost::future<Item> DavProvider::create_folder(
      string const& parent_id, string const& name, Context const& ctx)
+ {
++    auto handler = new CreateFolderHandler(*this, parent_id, name, ctx);
++    return handler->get_future();
+ }
  boost::future<unique_ptr<UploadJob>> DavProvider::create_file(
 === modified file 'tests/davprovider/davprovider_test.cpp'
 --- tests/davprovider/davprovider_test.cpp	2016-11-10 07:20:56 +0000
 +++ tests/davprovider/davprovider_test.cpp	2016-11-10 10:21:24 +0000
@@ -309,6 +309,125 @@
      EXPECT_EQ(ItemType::file, item->type());
+ }
++TEST_F(DavProviderTests, create_folder)
++{
++    auto account = get_client();
++
++    shared_ptr<Root> root;
++    {
++        QFutureWatcher<QVector<shared_ptr<Root>>> watcher;
++        QSignalSpy spy(&watcher, &decltype(watcher)::finished);
++        watcher.setFuture(account->roots());
++        if (spy.count() == 0)
++        {
++            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        }
++        auto roots = watcher.result();
++        ASSERT_EQ(1, roots.size());
++        root = roots[0];
++    }
++
++    // FIXME: the test webdav server returns no ETag for folders, so
++    // the client throws away the create_folder() response.  We can
++    // reenable this when porting to storage-framework 0.2.
++    return;
++
++    shared_ptr<Folder> folder;
++    {
++        QFutureWatcher<shared_ptr<Folder>> watcher;
++        QSignalSpy spy(&watcher, &decltype(watcher)::finished);
++        watcher.setFuture(root->create_folder("folder"));
++        if (spy.count() == 0)
++        {
++            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        }
++        folder = watcher.result();
++    }
++
++    EXPECT_EQ("folder/", folder->native_identity());
++    EXPECT_EQ(".", folder->parent_ids().at(0));
++    EXPECT_EQ("folder", folder->name());
++    EXPECT_EQ(ItemType::folder, folder->type());
++}
++
++TEST_F(DavProviderTests, create_folder_overwrite_file)
++{
++    auto account = get_client();
++    make_file("folder");
++
++    shared_ptr<Root> root;
++    {
++        QFutureWatcher<QVector<shared_ptr<Root>>> watcher;
++        QSignalSpy spy(&watcher, &decltype(watcher)::finished);
++        watcher.setFuture(account->roots());
++        if (spy.count() == 0)
++        {
++            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        }
++        auto roots = watcher.result();
++        ASSERT_EQ(1, roots.size());
++        root = roots[0];
++    }
++
++    {
++        QFutureWatcher<shared_ptr<Folder>> watcher;
++        QSignalSpy spy(&watcher, &decltype(watcher)::finished);
++        watcher.setFuture(root->create_folder("folder"));
++        if (spy.count() == 0)
++        {
++            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        }
++        try
++        {
++            watcher.result();
++            FAIL();
++        }
++        catch (RemoteCommsException const& e)
++        {
++            EXPECT_EQ("Error from MKCOL: 405", e.error_message());
++        }
++    }
++}
++
++TEST_F(DavProviderTests, create_folder_overwrite_folder)
++{
++    auto account = get_client();
++    ASSERT_EQ(0, mkdir(local_file("folder").c_str(), 0755));
++
++    shared_ptr<Root> root;
++    {
++        QFutureWatcher<QVector<shared_ptr<Root>>> watcher;
++        QSignalSpy spy(&watcher, &decltype(watcher)::finished);
++        watcher.setFuture(account->roots());
++        if (spy.count() == 0)
++        {
++            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        }
++        auto roots = watcher.result();
++        ASSERT_EQ(1, roots.size());
++        root = roots[0];
++    }
++
++    {
++        QFutureWatcher<shared_ptr<Folder>> watcher;
++        QSignalSpy spy(&watcher, &decltype(watcher)::finished);
++        watcher.setFuture(root->create_folder("folder"));
++        if (spy.count() == 0)
++        {
++            ASSERT_TRUE(spy.wait(SIGNAL_WAIT_TIME));
++        }
++        try
++        {
++            watcher.result();
++            FAIL();
++        }
++        catch (RemoteCommsException const& e)
++        {
++            EXPECT_EQ("Error from MKCOL: 405", e.error_message());
++        }
++    }
++}
++
  TEST_F(DavProviderTests, create_file)
+ {
      int const segments = 50;