Page MenuHomeSoftware Heritage
Differential D4645

Add tag <swh:add_to_origin>, to replace the Slug header for parent relationships.
ClosedPublic
Actions

Authored by vlorentz on Dec 2 2020, 12:35 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T2824: Add tag to the deposit protocol to allow client to specify a parent deposit
Commits
rDDEPb66cfff9e8d5: Add tag <swh:add_to_origin>, to replace the Slug header for parent…
Summary

Computing origin urls based on provider url + slug is brittle.

Additionally, the Slug is not designed to define relationships between deposits.

Diff Detail

Repository
rDDEP Push deposit
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Dec 2 2020, 12:35 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptDec 2 2020, 12:35 PM
swh-public-ci added a comment.Dec 2 2020, 12:37 PM

Build is green

Patch application report for D4645 (id=16481)

Could not rebase; Attempt merge onto b7eac04c53...

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
ardumont added a subscriber: ardumont.Dec 2 2020, 1:30 PM

Couple of remarks, questions inline

swh/deposit/api/common.py
756

missing test scenario

791

same, missing scnenario

swh/deposit/tests/data/atom/entry-data-with-add-to-origin.xml
10

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
10

I blame Phabricator. I copied the file, not move it. See https://forge.softwareheritage.org/D4645#116053

ardumont requested changes to this revision.Dec 2 2020, 7:14 PM

Missing test cases but otherwise, lgtm.

Request changes for the sake of those missing scenarios.

swh/deposit/api/collection.py
113–114

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
10

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–114

yes

swh/deposit/tests/data/atom/entry-data-with-add-to-origin.xml
10

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
vlorentz updated this revision to Diff 16591.Dec 7 2020, 5:14 PM

add tests

swh-public-ci added a comment.Dec 7 2020, 5:17 PM

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….

Revision Contents
Changeset List

PathSize
docs/
specs/
7 lines
swh/
deposit/
api/
1 line
50 lines
tests/
api/
96 lines
73 lines
2 lines
data/
atom/
entry-data-with-add-to-origin.xml
entry-data-with-origin.xml
6 lines
7 lines
9 lines

Diff 16613

View Options

docs/specs/swh.xsd

Loading...
View Options

swh/deposit/api/collection.py

Loading...
View Options

swh/deposit/api/common.py

Loading...
View Options

swh/deposit/tests/api/test_collection.py

Loading...
View Options

swh/deposit/tests/api/test_collection_post_atom.py

Loading...
View Options

swh/deposit/tests/api/test_collection_post_metadata.py

Loading...
View Options

swh/deposit/tests/data/atom/entry-data-with-add-to-origin.xml

Loading...
View Options

swh/deposit/tests/data/atom/entry-data-with-both-add-to-origin-and-external-id.xml

Loading...
View Options

swh/deposit/tests/data/atom/entry-data-with-both-create-origin-and-add-to-origin.xml

Loading...
View Options

swh/deposit/tests/data/atom/entry-data-with-origin.xml

Loading...
About · Developer information · Legal