Updating b7eac04c..717ee249
Fast-forward
 docs/metadata.rst                                  |  30 +++-
 docs/specs/metadata_example.xml                    |   3 +
 docs/specs/spec-loading.rst                        |   3 +-
 docs/specs/swh.xsd                                 |  13 ++
 docs/user-manual.rst                               |   8 +-
 swh/deposit/api/collection.py                      |   1 +
 swh/deposit/api/common.py                          |  80 +++++++--
 swh/deposit/tests/api/test_collection.py           | 131 ++++++++++++--
 swh/deposit/tests/api/test_collection_post_atom.py | 190 +++++++++++++++++----
 .../tests/api/test_collection_post_metadata.py     |   2 +-
 .../api/test_deposit_private_read_metadata.py      |   5 +-
 swh/deposit/tests/api/test_deposit_update.py       |  98 ++++++++++-
 swh/deposit/tests/conftest.py                      |   5 +-
 swh/deposit/tests/data/atom/codemeta-sample.xml    |   6 +
 .../tests/data/atom/entry-data-no-origin-url.xml   |  26 +++
 .../data/atom/entry-data-with-add-to-origin.xml    |  13 ++
 ...in.xml => entry-data-with-origin-reference.xml} |   0
 swh/deposit/tests/data/atom/entry-data0.xml        |   9 +-
 swh/deposit/tests/data/atom/entry-data2.xml        |   9 +-
 .../tests/data/atom/entry-only-create-origin.xml   |  10 ++
 swh/deposit/tests/data/atom/error-with-decimal.xml |  10 +-
 ...-with-external-identifier-and-create-origin.xml |  14 ++
 .../data/atom/error-with-external-identifier.xml   |   3 +-
 .../error-with-reference-and-create-origin.xml     |  16 ++
 swh/deposit/tests/data/atom/metadata.xml           |  10 +-
 25 files changed, 625 insertions(+), 70 deletions(-)
 create mode 100644 swh/deposit/tests/data/atom/entry-data-no-origin-url.xml
 create mode 100644 swh/deposit/tests/data/atom/entry-data-with-add-to-origin.xml
 rename swh/deposit/tests/data/atom/{entry-data-with-origin.xml => entry-data-with-origin-reference.xml} (100%)
 create mode 100644 swh/deposit/tests/data/atom/entry-only-create-origin.xml
 create mode 100644 swh/deposit/tests/data/atom/error-with-external-identifier-and-create-origin.xml
 create mode 100644 swh/deposit/tests/data/atom/error-with-reference-and-create-origin.xml

Changes applied before test

commit 717ee249859749d3c1be0f03978db719d0c38472
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Nov 26 13:10:15 2020 +0100

    Add tag <swh:add_to_origin>, to replace the Slug header for parent relationships.
    
    Computing origin urls based on provider url + slug is brittle.
    
    Additionally, the Slug is not designed to define relationships between deposits.

commit b75ebd1a74ae6b07bfa299c9aab74ef5d48258df
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 19:14:26 2020 +0100

    Check origin urls start with the user's provider_url
    
    To prevent any user from creating any origin url.

commit 441cfe10a4b636ab3ed0311ceeb8e780a90a7e4f
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 19:06:33 2020 +0100

    Add tag <swh:create_origin>, to replace the Slug header.
    
    Computing origin urls based on provider url + slug is brittle.
    
    Additionally, it meant basing parenting relationship on reusing
    values of the slug across deposit, which is clearly not in the
    spirit of the SWORD specification.

See https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/468/ for more details.

Harbormaster completed remote builds in B17667: Diff 16481.Dec 2 2020, 12:37 PM

Couple of remarks, questions inline

swh/deposit/api/common.py
757	missing test scenario
792	same, missing scnenario
swh/deposit/tests/data/atom/entry-data-with-add-to-origin.xml
11	I'm a bit surprised here, do we rename from swh:reference to swh:add_to_origin? that file with `swh:reference` is used for the metadata only deposit...

vlorentz added inline comments.Dec 2 2020, 4:56 PM

swh/deposit/tests/data/atom/entry-data-with-add-to-origin.xml
11	I blame Phabricator. I copied the file, not move it. See https://forge.softwareheritage.org/D4645#116053

Missing test cases but otherwise, lgtm.

Request changes for the sake of those missing scenarios.

swh/deposit/api/collection.py
113	So as far as i could tell, for now, we keep the retro-compatibilty, right?
swh/deposit/tests/data/atom/entry-data-with-add-to-origin.xml
11	wow, yeah, that's confusing. Thanks for lifting the doubt, i did not think of checking the diff stats. (And also, I asked because i thought i missed something earlier...) ok then.

This revision now requires changes to proceed.Dec 2 2020, 7:14 PM

vlorentz added inline comments.Dec 3 2020, 11:43 AM

swh/deposit/api/collection.py
113	yes
swh/deposit/tests/data/atom/entry-data-with-add-to-origin.xml
11	it confused me as well, as I forgot about that change.

vlorentz added a task: T2824: Add tag to the deposit protocol to allow client to specify a parent deposit.Dec 7 2020, 10:56 AM

add tests

Build is green

Patch application report for D4645 (id=16591)

Could not rebase; Attempt merge onto b7eac04c53...

Updating b7eac04c..b66cfff9
Fast-forward
 docs/metadata.rst                                  |  30 ++-
 docs/specs/metadata_example.xml                    |   3 +
 docs/specs/spec-loading.rst                        |   3 +-
 docs/specs/swh.xsd                                 |  13 ++
 docs/user-manual.rst                               |   8 +-
 swh/deposit/api/collection.py                      |   1 +
 swh/deposit/api/common.py                          |  80 ++++++-
 swh/deposit/tests/api/test_collection.py           | 131 +++++++++++-
 swh/deposit/tests/api/test_collection_post_atom.py | 234 ++++++++++++++++++---
 .../tests/api/test_collection_post_metadata.py     |   2 +-
 .../api/test_deposit_private_read_metadata.py      |   5 +-
 swh/deposit/tests/api/test_deposit_update.py       |  98 ++++++++-
 swh/deposit/tests/conftest.py                      |   5 +-
 swh/deposit/tests/data/atom/codemeta-sample.xml    |   6 +
 .../tests/data/atom/entry-data-no-origin-url.xml   |  26 +++
 .../data/atom/entry-data-with-add-to-origin.xml    |  13 ++
 ...ata-with-both-add-to-origin-and-external-id.xml |  14 ++
 ...a-with-both-create-origin-and-add-to-origin.xml |  16 ++
 ...in.xml => entry-data-with-origin-reference.xml} |   0
 swh/deposit/tests/data/atom/entry-data0.xml        |   9 +-
 swh/deposit/tests/data/atom/entry-data2.xml        |   9 +-
 .../tests/data/atom/entry-only-create-origin.xml   |  10 +
 swh/deposit/tests/data/atom/error-with-decimal.xml |  10 +-
 ...-with-external-identifier-and-create-origin.xml |  14 ++
 .../data/atom/error-with-external-identifier.xml   |   3 +-
 .../error-with-reference-and-create-origin.xml     |  16 ++
 swh/deposit/tests/data/atom/metadata.xml           |  10 +-
 27 files changed, 699 insertions(+), 70 deletions(-)
 create mode 100644 swh/deposit/tests/data/atom/entry-data-no-origin-url.xml
 create mode 100644 swh/deposit/tests/data/atom/entry-data-with-add-to-origin.xml
 create mode 100644 swh/deposit/tests/data/atom/entry-data-with-both-add-to-origin-and-external-id.xml
 create mode 100644 swh/deposit/tests/data/atom/entry-data-with-both-create-origin-and-add-to-origin.xml
 rename swh/deposit/tests/data/atom/{entry-data-with-origin.xml => entry-data-with-origin-reference.xml} (100%)
 create mode 100644 swh/deposit/tests/data/atom/entry-only-create-origin.xml
 create mode 100644 swh/deposit/tests/data/atom/error-with-external-identifier-and-create-origin.xml
 create mode 100644 swh/deposit/tests/data/atom/error-with-reference-and-create-origin.xml

Changes applied before test

commit b66cfff9e8d59156155140b8a61bc72558a9558b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Nov 26 13:10:15 2020 +0100

    Add tag <swh:add_to_origin>, to replace the Slug header for parent relationships.
    
    Computing origin urls based on provider url + slug is brittle.
    
    Additionally, the Slug is not designed to define relationships between deposits.

commit b75ebd1a74ae6b07bfa299c9aab74ef5d48258df
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 19:14:26 2020 +0100

    Check origin urls start with the user's provider_url
    
    To prevent any user from creating any origin url.

commit 441cfe10a4b636ab3ed0311ceeb8e780a90a7e4f
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Nov 25 19:06:33 2020 +0100

    Add tag <swh:create_origin>, to replace the Slug header.
    
    Computing origin urls based on provider url + slug is brittle.
    
    Additionally, it meant basing parenting relationship on reusing
    values of the slug across deposit, which is clearly not in the
    spirit of the SWORD specification.

See https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/471/ for more details.

Harbormaster completed remote builds in B17771: Diff 16591.Dec 7 2020, 5:18 PM

ardumont accepted this revision.Dec 7 2020, 6:01 PM

This revision is now accepted and ready to land.Dec 7 2020, 6:01 PM

Closed by commit rDDEPb66cfff9e8d5: Add tag <swh:add_to_origin>, to replace the Slug header for parent… (authored by vlorentz). · Explain WhyDec 8 2020, 11:40 AM

This revision was automatically updated to reflect the committed changes.

vlorentz added a commit: rDDEPb66cfff9e8d5: Add tag <swh:add_to_origin>, to replace the Slug header for parent….