CenterForOpenScience / waterbutler

WaterButler is a Python web application for interacting with various file storage services via a single RESTful API, developed at Center for Open Science.
Apache License 2.0
62 stars 76 forks source link

[CR][ENG-2533] Add write support to OneDrive provider #396

Closed felliott closed 3 years ago

felliott commented 3 years ago

Ticket

ENG-2533

Purpose

Add read-write support to the OneDrive provider. Based off previously-submitted PRs from Ryan Casey (@caseyrygt) & Oleksandr Melnykov (@alexandr-melnikov-dev-pro).

Changes

SO MANY! Copying commit message from HEAD^^^:

    make OneDrive provider read-write

     * Add read/write (rw) support to the OneDrive provider.  This
       provider was originally developed with rw support in mind, but we
       opted to release a simpler version without write support.  Since
       then we haven't had many requests for write support for Personal
       OneDrive, but there are entities with an interest in rw support for
       OneDrive for Business (OD4B).  This branch resurrects the original
       rw support for the modern (and greatly-diverged) codebase.

     * Annotate readonly (ro) and rw method groups by adding comment
       dividers.

     * Replace default implementations of rw methods with actual
       implementations.  These were adapted from the work of Ryan Casey
       (@caseyrygt) & Oleksandr Melnykov (@alexandr-melnikov-dev-pro).

       * Add `has_real_root()` method.

       * Update `_build_/foo/_url` utility methods.  Old code uses
         `_build_drive_url` and `_build_content_url`.  New code uses
         `_build_item_url` and `_build_root_url`.  Update both provider
         and tests to use these new methods.

       * Update paths & identifiers:

         * OneDrive path identifiers are either `root` (for the root
           folder) or of the format `${foo}!${bar}`, where `$foo` is the
           root id and `$bar` is a monotonically-increasing number.  There
           was some legacy code that expected that the root path
           identifier could be `'0'`.  This was probably carryover from
           the Box provider, which this provider was based off of.

         * Some code was calling a OneDrivePath method, `api_identifier`,
           to get the relevant url segments.  Why should OneDrivePath know
           about OneDrive API urls?  It shouldn't!  That's a job for the
           provider.  Remove it.

         * Expand `OneDrivePath` test coverage; add test for alternate
           exception branch test.

       * Update `revalidate_path()` for read-write support

         * Propagate path's parent id in return val.

         * Fix `revalidate_path` tests since R/W support means
           non-existent paths should still validate.  Instead of returning
           404s, non-existing paths should return a new ODPath object
           without an identifier for the last path part.

       * Remove untestable "is deleted" condition in `metadata()` method.

       * Add `shares_storage_root()` method.

       * Update `can_intra_{move,copy}()` methods for rw support.

       * Implement `upload()` method.

       * Implement `create_folder()` method.  Changes from prior work:

         * Propagate path's parent id in return val.

         * `create_folder()` log creation was broken b/c the original
           implementation was passing the wrong path when building the
           metadata object for the new folder.  Additionaly, as an
           ID-based provider, it needs to explicitly set the new path's
           id.

       * Implement delete() method.  Changes from prior work:

         * Move root deletion logic into its own method,
           `_delete_folder_contents`.

       * Implement intra_{copy,move} methods. Changes from prior work:

         * Update `intra_move`'s returned path to use `new_from_response`
           instead of non-existent `update_from_response`.

         * Fix return val of `intra_{move,copy}()`.  It had been returning
           the same data as `construct_metadata()`, which turns folder
           requests into a list of the folder's children.
           `intra_{move,copy}()` should return an actual
           `OneDriveFolderMetadata` object with the list of children as an
           attribute.

       * Update provider tests:

         * Ditch TestReadOnlyProvider.

         * TestOperations: Update existing and add
           `test_shares_storage_root`.

         * TestCreateFolder: adapted from prior code; updated for new code
           changes and expanded coverage

         * TestUpload: adapted from prior code; updated for new code
           changes

         * TestDelete: adapted from prior code; rewrite delete tests and
           update fixtures

         * Comment out unimplemented tests for now.

     * metadata.py: remove some conditional gets from the metadata
       processors.  So far it looks like these fields are always present.

     * Add test for `metadata()` on empty folder to improve coverage.

Side effects

None expected, but bugs may exist!

QA Notes

Requires a full r/w regression test after deploy. This has been QA'd in a staging environment by @felliott and @DougCorrell.

Deployment Notes

No changes needed from the prior read-write version.

This branch pins pip and cryptography to fix docker image build breakage.

felliott commented 3 years ago

Superseded by #399