Page MenuHomeSoftware Heritage

Add a directoryEntry entrypoint
ClosedPublic

Authored by jayeshv on Sep 6 2022, 11:28 AM.

Details

Summary

Get a directory entry object with a SWHID and a relative path

Related to T4508

Diff Detail

Repository
rDGQL GraphQL API
Branch
directory-entry
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 31424
Build 49154: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 49153: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D8396 (id=30307)

Rebasing onto 467e0b7e28...

Current branch diff-target is up to date.
Changes applied before test
commit 18e15d91977aeb8f6c9bac8bb41cc62d7a0d40ec
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 6 10:47:27 2022 +0200

    Add a directoryEntry entrypoint
    
    Get a directory entry object with a SWHID and a relative path

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

Build is green

Patch application report for D8396 (id=30308)

Rebasing onto 467e0b7e28...

Current branch diff-target is up to date.
Changes applied before test
commit 919e9ee5c01a1472e42e7b259ac4f14847e0c431
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 6 10:47:27 2022 +0200

    Add a directoryEntry entrypoint
    
    Get a directory entry object with a SWHID and a relative path

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

As this allows nested paths, then the cost calculator needs to take into account the number of / in the path, as each of them requires two joins in the database

As this allows nested paths, then the cost calculator needs to take into account the number of / in the path, as each of them requires two joins in the database

Ok. It will be a bit hard to do in the static cost calculator. But, we can add a higher value for requesting this node irrespective of the path.
The idea of static cost calculator is to reject the query upfront, without even hitting the resolver. That logic is not in master yet. I am still working on some features.

For the dynamic cost calculator (that will also be used for rate limiting in the future), we can have credits depending on the path nesting.

Build has FAILED

Patch application report for D8396 (id=30358)

Rebasing onto 467e0b7e28...

Current branch diff-target is up to date.
Changes applied before test
commit ed900f8a4dcb8499d9df7be116ca741445ab2b25
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 6 10:47:27 2022 +0200

    Add a directoryEntry entrypoint
    
    Get a directory entry object with a SWHID and a relative path
    
    Related to T4508

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

Build is green

Patch application report for D8396 (id=30358)

Rebasing onto 467e0b7e28...

Current branch diff-target is up to date.
Changes applied before test
commit ed900f8a4dcb8499d9df7be116ca741445ab2b25
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 6 10:47:27 2022 +0200

    Add a directoryEntry entrypoint
    
    Get a directory entry object with a SWHID and a relative path
    
    Related to T4508

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

Build is green

Patch application report for D8396 (id=30397)

Rebasing onto 62920c3258...

Current branch diff-target is up to date.
Changes applied before test
commit 713b2d16173c800968ba33dd729b5022eae8a044
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 6 10:47:27 2022 +0200

    Add a directoryEntry entrypoint
    
    Get a directory entry object with a SWHID and a relative path
    
    Related to T4508

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

anlambert added a subscriber: anlambert.
anlambert added inline comments.
swh/graphql/tests/functional/test_directory_entry.py
67–94

This test could be improved to handle all paths reachable from the root directory.

I came up with that code:

def test_get_directory_entry_with_nested_path(client, storage):
    query_str = """
      {
        directoryEntry(swhid: "%s", path: "%s") {
          name {
            text
          }
          targetType
          target {
            ...on Content {
              swhid
            }
            ...on Directory {
              swhid
            }
          }
        }
      }
      """
    directory = get_directories_with_nested_path()[0]
    for entry in storage.directory_ls(directory.id, recursive=True):
        if entry["type"] == "rev":
            # TODO: Handle revision target as directory entry (git submodule)
            continue

        query = query_str % (
            directory.swhid(),
            entry["name"].decode(),
        )

        data, _ = utils.get_query_response(
            client,
            query,
        )
        
        if entry["type"] == "file":
            swhid = CoreSWHID(
                object_type=ObjectType.CONTENT, object_id=entry["sha1_git"]
            )
        elif entry["type"] == "dir":
            swhid = CoreSWHID(
                object_type=ObjectType.DIRECTORY, object_id=entry["target"]
            )
        else:
            swhid = CoreSWHID(
                object_type=ObjectType.REVISION, object_id=entry["target"]
            )

        assert data["directoryEntry"] == {
            "name": {"text": entry["name"].decode()},
            "target": {"swhid": str(swhid)},
            "targetType": entry["type"],
        }

Also directory entry targeting a revision is currently not supported, this should also be fixed.

This revision now requires changes to proceed.Sep 8 2022, 6:10 PM

Build has FAILED

Patch application report for D8396 (id=30404)

Rebasing onto 62920c3258...

Current branch diff-target is up to date.
Changes applied before test
commit f51a840ee7023186b283fe0b4d087794c18dbaf7
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 6 10:47:27 2022 +0200

    Add a directoryEntry entrypoint
    
    Get a directory entry object with a SWHID and a relative path
    
    Related to T4508

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

jayeshv added inline comments.
swh/graphql/tests/functional/test_directory_entry.py
67–94

Thanks. I have added these cases in the previous test.
https://forge.softwareheritage.org/D8396#inline-59712:~:text=def%20test_get_directory_entry(client)%3A
Support for revision as a directory-entry target will be fixed in another diff.

jayeshv marked an inline comment as done.

remove wrong code

Build is green

Patch application report for D8396 (id=30405)

Rebasing onto 62920c3258...

Current branch diff-target is up to date.
Changes applied before test
commit cb941a83ccbae1287b54d4657744a6bbe72556fa
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 6 10:47:27 2022 +0200

    Add a directoryEntry entrypoint
    
    Get a directory entry object with a SWHID and a relative path
    
    Related to T4508

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

anlambert added inline comments.
swh/graphql/tests/functional/test_directory_entry.py
67–94

Please remove the test_get_directory_entry_with_nested_path test and use parametrized test instead, see below:

@pytest.mark.parametrize(
    "directory", get_directories() + get_directories_with_nested_path()
)
def test_get_directory_entry(client, storage, directory):
    storage = server.get_storage()
    query_str = """
    {
      directoryEntry(swhid: "%s", path: "%s") {
        name {
          text
        }
        targetType
        target {
          ...on Content {
            swhid
          }
          ...on Directory {
            swhid
          }
        }
      }
    }
    """
    for entry in storage.directory_ls(directory.id, recursive=True):
        if entry["type"] == "rev":
            # FIXME, Revision is not supported as a directory entry target yet
            continue
        query = query_str % (
            directory.swhid(),
            entry["name"].decode(),
        )
        data, _ = utils.get_query_response(
            client,
            query,
        )
        swhid = None
        if entry["type"] == "file" and entry["sha1_git"] is not None:
            swhid = CoreSWHID(
                object_type=ObjectType.CONTENT, object_id=entry["sha1_git"]
            )
        elif entry["type"] == "dir":
            swhid = CoreSWHID(
                object_type=ObjectType.DIRECTORY, object_id=entry["target"]
            )
        assert data["directoryEntry"] == {
            "name": {"text": entry["name"].decode()},
            "target": {"swhid": str(swhid)} if swhid else None,
            "targetType": entry["type"],
        }

I also handled the case where a directory entry targets a non archived content as it is a test case in swh-model tests data.

This revision now requires changes to proceed.Sep 9 2022, 10:37 AM
swh/graphql/tests/functional/test_directory_entry.py
67–94

There is an issue with parametrized tests. Not everything in the test data are valid db objects but just dummy ids. (They are not None either)
eg: https://forge.softwareheritage.org/source/swh-model/browse/master/swh/model/tests/swh_model_data.py$362
GraphQL will try to get the real db objects with an ID and will fail, this will cause an object error.

use parametrize in directory entry tests

Build is green

Patch application report for D8396 (id=30406)

Rebasing onto 62920c3258...

Current branch diff-target is up to date.
Changes applied before test
commit c6052de67686774c2acff352df9f817ff1edc4c7
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 6 10:47:27 2022 +0200

    Add a directoryEntry entrypoint
    
    Get a directory entry object with a SWHID and a relative path
    
    Related to T4508

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

swh/graphql/tests/functional/test_directory_entry.py
67–94

Fixed the issue by explicitly adding a condition to check None as id.

swh/graphql/tests/functional/test_directory_entry.py
67–94

Tests are still passing with these changes as the unknown object errors are caught by your implementation,
I did not check the returned errors field but in terms of directoryEntry ones, it seems correct.

(swh) ✔ ~/swh/swh-environment/swh-graphql [arcpatch-D8396_1 L|✚ 1⚑ 3] 
11:18 $ make test
python3 -m pytest  .
================================================================================================================================== test session starts ==================================================================================================================================
platform linux -- Python 3.9.2, pytest-7.1.2, pluggy-1.0.0
rootdir: /home/anlambert/swh/swh-environment/swh-graphql, configfile: pytest.ini
plugins: redis-2.4.0, asyncio-0.19.0, postgresql-3.1.3, forked-1.4.0, flask-1.2.0, anyio-3.6.1, xdist-2.5.0, cov-3.0.0, dash-2.6.1, requests-mock-1.9.3, django-test-migrations-1.2.0, django-4.5.2, mock-3.8.2, hypothesis-6.54.3, swh.core-2.14.1.dev2+g843a010, swh.journal-1.1.0
asyncio: mode=strict
collected 248 items                                                                                                                                                                                                                                                                     

swh/graphql/tests/functional/test_branch_connection.py ...............                                                                                                                                                                                                            [  6%]
swh/graphql/tests/functional/test_content.py .............................................                                                                                                                                                                                        [ 24%]
swh/graphql/tests/functional/test_directory.py .......                                                                                                                                                                                                                            [ 27%]
swh/graphql/tests/functional/test_directory_entry.py ........                                                                                                                                                                                                                     [ 30%]
swh/graphql/tests/functional/test_origin_connection.py ....                                                                                                                                                                                                                       [ 31%]
swh/graphql/tests/functional/test_origin_node.py ...                                                                                                                                                                                                                              [ 33%]
swh/graphql/tests/functional/test_pagination.py .......                                                                                                                                                                                                                           [ 35%]
swh/graphql/tests/functional/test_release_node.py ..........                                                                                                                                                                                                                      [ 39%]
swh/graphql/tests/functional/test_revision.py ........                                                                                                                                                                                                                            [ 43%]
swh/graphql/tests/functional/test_search.py ..                                                                                                                                                                                                                                    [ 43%]
swh/graphql/tests/functional/test_snapshot_node.py ....                                                                                                                                                                                                                           [ 45%]
swh/graphql/tests/functional/test_swhid_resolve.py .....................................                                                                                                                                                                                          [ 60%]
swh/graphql/tests/functional/test_visit_node.py ...                                                                                                                                                                                                                               [ 61%]
swh/graphql/tests/unit/errors/test_errors.py .......                                                                                                                                                                                                                              [ 64%]
swh/graphql/tests/unit/resolvers/test_base_node.py .......                                                                                                                                                                                                                        [ 67%]
swh/graphql/tests/unit/resolvers/test_resolver_factory.py ......................................                                                                                                                                                                                  [ 82%]
swh/graphql/tests/unit/resolvers/test_resolvers.py ................................                                                                                                                                                                                               [ 95%]
swh/graphql/tests/unit/utils/test_utils.py ...........                                                                                                                                                                                                                            [100%]

================================================================================================================================== 248 passed in 2.31s ==================================================================================================================================

Build is green

Patch application report for D8396 (id=30407)

Rebasing onto 62920c3258...

Current branch diff-target is up to date.
Changes applied before test
commit 1af442fd62fd1b9d3c5f0b9c8121e3d1f16a2cd6
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 6 10:47:27 2022 +0200

    Add a directoryEntry entrypoint
    
    Get a directory entry object with a SWHID and a relative path
    
    Related to T4508

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

This revision is now accepted and ready to land.Sep 9 2022, 11:40 AM
swh/graphql/tests/functional/test_directory_entry.py
67–94

Got that. the objects are indeed in the DB and the SWHIDs are generated dynamically
eg: https://forge.softwareheritage.org/source/swh-model/browse/master/swh/model/tests/swh_model_data.py$357
has a SWHID swh:1:cnt:86bc6b377e9d25f9d26777a4a28d08e63e7c5779'

This revision was automatically updated to reflect the committed changes.