Page MenuHomeSoftware Heritage

common/identifiers: Fix content SWHID with anchor revision browse URL
ClosedPublic

Authored by anlambert on Tue, Apr 20, 4:57 PM.

Details

Summary

An invalid value was generated for the path query parameter of the
revision view, leading to a 404 error.

Related to T3279

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build has FAILED

Patch application report for D5564 (id=19874)

Rebasing onto 4a6d884a74...

Current branch diff-target is up to date.
Changes applied before test
commit d733ae37eb5e20051526f4cf117667718bd3a1a4
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 20 16:54:01 2021 +0200

    common/identifiers: Fix content SWHID with anchor revision browse URL
    
    An invalid value was generated for the path query parameter of the
    revision view, leading to a 404 error.
    
    Related to T3279

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/700/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/700/console

Harbormaster returned this revision to the author for changes because remote builds failed.Tue, Apr 20, 5:05 PM
Harbormaster failed remote builds in B20885: Diff 19874!

Build is green

Patch application report for D5564 (id=19874)

Rebasing onto 4a6d884a74...

Current branch diff-target is up to date.
Changes applied before test
commit d733ae37eb5e20051526f4cf117667718bd3a1a4
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 20 16:54:01 2021 +0200

    common/identifiers: Fix content SWHID with anchor revision browse URL
    
    An invalid value was generated for the path query parameter of the
    revision view, leading to a 404 error.
    
    Related to T3279

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

Why use hypothesis for this? According to Jenkins, one of the two important branches are not covered.

Why use hypothesis for this? According to Jenkins, one of the two important branches are not covered.

It seems a test is missing for that special case that should only occurs for a SWHID without origin and visit qualifier and a non anchor revision, for instance:
swh:1:cnt:546792d176a4801b7b4f7659ec3953a948a85ba0;anchor=swh:1:dir:94c2b67787a2a40c08831b5a61eb31b7fce267c4;path=/hexagon/kernel/dma.c

Let's add it before landing this then.

Rebase and add tests for content/directory SWHID with directory anchor (missing execution branch to cover).

Build is green

Patch application report for D5564 (id=19892)

Rebasing onto 2f83416d9f...

Current branch diff-target is up to date.
Changes applied before test
commit 461aa57ec4e83a281e5cca9e7533990a954bc93e
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 20 16:54:01 2021 +0200

    common/identifiers: Fix content SWHID with anchor revision browse URL
    
    An invalid value was generated for the path query parameter of the
    revision view, leading to a 404 error.
    
    Related to T3279

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

vlorentz added inline comments.
swh/web/tests/common/test_identifiers.py
701–710

still not covered

This revision now requires changes to proceed.Thu, Apr 22, 11:12 AM

Build is green

Patch application report for D5564 (id=19961)

Rebasing onto 3a0ffc0879...

Current branch diff-target is up to date.
Changes applied before test
commit 82bcc274fc799acb1602ecd92cc8d34f68cf5fe4
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 20 16:54:01 2021 +0200

    common/identifiers: Fix content SWHID with anchor revision browse URL
    
    An invalid value was generated for the path query parameter of the
    revision view, leading to a 404 error.
    
    Also add missing tests for some SWHIDs resolving.
    
    Related to T3279

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

Thanks. I still don't get why we use Hypothesis for this though.

This revision is now accepted and ready to land.Fri, Apr 23, 12:37 PM

Thanks. I still don't get why we use Hypothesis for this though.

Because the swh-web tests need to use objects stored into its sample archive and not fake ones.

If you look at the code of resolve_swhid, you will see some checks are done on objects existence,
for instance to ensure an anchor really exists into the archive.

Build is green

Patch application report for D5564 (id=19965)

Rebasing onto 00cc0c4dae...

Current branch diff-target is up to date.
Changes applied before test
commit 1ca5105c4e8748c4efb80a1ed0c67f4830d0039a
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 20 16:54:01 2021 +0200

    common/identifiers: Fix content SWHID with anchor revision browse URL
    
    An invalid value was generated for the path query parameter of the
    revision view, leading to a 404 error.
    
    Also add missing tests for some SWHIDs resolving.
    
    Related to T3279

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