Page MenuHomeSoftware Heritage

api/origin: Do not attempt to lookup similar origin URLs
Needs ReviewPublic

Authored by anlambert on Tue, Jun 14, 3:01 PM.

Details

Reviewers
None
Group Reviewers
Reviewers
Summary

Web API should be as dumb as possible and should not attempt to
lookup an origin with and without trailing slash in its URL.

That regression was introduced in rDWAPPS56fb869644ad45e0648f4ad45c07619d468019b4 but that process should
only be performed in a Web UI context.

Related to D7980

Diff Detail

Repository
rDWAPPS Web applications
Branch
api-origin-no-url-processing
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 29908
Build 46752: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 46751: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D7988 (id=28777)

Rebasing onto 662ce6eafc...

Current branch diff-target is up to date.
Changes applied before test
commit b92ae62dfc66a937a5f678addd121ef15ab03f56
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jun 14 13:45:53 2022 +0200

    api/origin: Do not attempt to lookup similar origin URLs
    
    Web API should be as dumb as possible and should not attempt to
    lookup an origin with and without trailing slash in its URL.
    
    That regression was introduced in 56fb869 but that process should
    only be performed in a Web UI context.
    
    Related to D7980

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

Build is green

Patch application report for D7988 (id=28778)

Rebasing onto 662ce6eafc...

Current branch diff-target is up to date.
Changes applied before test
commit 9e82cd6ee59e3817c18a8b3a05af4c2259f1d642
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jun 14 13:45:53 2022 +0200

    api/origin: Do not attempt to lookup similar origin URLs
    
    Web API should be as dumb as possible and should not attempt to
    lookup an origin with and without trailing slash in its URL.
    
    rDWAPPS56fb869644ad45e0648f4ad45c07619d468019b4 introduced that
    regression but that process  should only be performed in a Web
    UI context.
    
    Related to D7980

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

douardda added inline comments.
swh/web/api/views/origin.py
351–353

I don't get why this (and similar snippets below) is related to this diff's description.

Also, this will "break the API". I am not opposed to this change, but some care should be taken when changing public's API behavior like this. (attempt to check if there are users of the behavior to be modified, document the new behavior, display/return a warning or something, etc)

Also, this will "break the API". I am not opposed to this change, but some care should be taken when changing public's API behavior like this. (attempt to check if there are users of the behavior to be modified, document the new behavior, display/return a warning or something, etc)

I looked at the stats collected by piwik and the origin endpoint does not seem to be queried a lot in the current year so I do not think that change will impact a lot of users.

swh/web/api/views/origin.py
351–353

All API endpoints looking for an origin should have the same behavior, see added test.

But I will move that origin existence check in the archive module instead.

Rebase and centralize similar URLs processing in common/archive.py module.

Build is green

Patch application report for D7988 (id=28822)

Rebasing onto d5f020c02b...

Current branch diff-target is up to date.
Changes applied before test
commit d2cdd09d6f50ff3e18978d40f180eb55a023dda4
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Tue Jun 14 13:45:53 2022 +0200

    api/origin: Do not attempt to lookup similar origin URLs
    
    Web API should be as dumb as possible and should not attempt to
    lookup an origin with and without trailing slash in its URL.
    
    rDWAPPS56fb869644ad45e0648f4ad45c07619d468019b4 introduced that
    regression but that process should only be performed in a Web
    UI context.
    
    Related to D7980

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