Page MenuHomeSoftware Heritage

Change targettype in directory entry; add directory entry filter by name
ClosedPublic

Authored by jayeshv on Sep 20 2022, 3:08 PM.

Details

Summary

targetType is called "dir", "file" and "rev" in DirectoryEntry and is
different from other target types. Change that to "directory", "content"
and "revision"

Add a nameInclude GraphQL level filter for directory entries.

Diff Detail

Repository
rDGQL GraphQL API
Branch
dir-entry-improve
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 31629
Build 49469: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 49468: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D8511 (id=30647)

Rebasing onto ebc6e44534...

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

    Change targettype in directory entry; add directory entry filter by name
    
    Target type is called "dir", "file" and "rev" in directory entry it is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL lavel filter for directory entries.

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

add generic filter for connections

Build is green

Patch application report for D8511 (id=30650)

Rebasing onto ebc6e44534...

Current branch diff-target is up to date.
Changes applied before test
commit 5d1c1d87589cf13f16667788c435a1d9f267523b
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 20 15:02:47 2022 +0200

    Change targettype in directory entry; add directory entry filter by name
    
    Target type is called "dir", "file" and "rev" in directory entry it is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL lavel filter for directory entries.

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

Build is green

Patch application report for D8511 (id=30652)

Rebasing onto ebc6e44534...

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

    Change targettype in directory entry; add directory entry filter by name
    
    Target type is called "dir", "file" and "rev" in directory entry it is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL lavel filter for directory entries.

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

add extra comment; update commit mesasge

Build is green

Patch application report for D8511 (id=30660)

Rebasing onto ebc6e44534...

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

    Change value of targetType in DirectoryEntry; add filter by entry name
    
    targetType is called "dir", "file" and "rev" in DirectoryEntry and is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL lavel filter for directory entries.

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

Build is green

Patch application report for D8511 (id=30661)

Rebasing onto ebc6e44534...

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

    Change value of targetType in DirectoryEntry; add filter by entry name
    
    targetType is called "dir", "file" and "rev" in DirectoryEntry and is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL level filter for directory entries.

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

Using .lower() on byte strings of unknown encoding is going to have surprising results.

What is the intended use of making this case-insensitive?

Using .lower() on byte strings of unknown encoding is going to have surprising results.

What is the intended use of making this case-insensitive?

The intention is to have some kind of a search for contents. Do you think a case sensitive filter is good enough, or should we explicitly handle the error here?

anlambert added a subscriber: anlambert.

Using .lower() on byte strings of unknown encoding is going to have surprising results.

What is the intended use of making this case-insensitive?

The intention is to have some kind of a search for contents. Do you think a case sensitive filter is good enough, or should we explicitly handle the error here?

I also think search should be case insensitive here, this is quite a common practice.

swh/graphql/resolvers/directory_entry.py
65

you should call lower before encode here

swh/graphql/tests/functional/test_directory_entry.py
126–149

You should try to make your tests cover more cases and more dynamic by avoiding to hard code test inputs, something like:

@pytest.mark.parametrize("directory", get_directories())
def test_directory_entry_connection_filter_by_name(client, directory):
    storage = server.get_storage()
    for dir_entry in storage.directory_ls(directory.id):
        name_include = dir_entry["name"][:-1].decode()
        query_str = """
      {
        directory(swhid: "%s") {
          swhid
          entries(nameInclude: "%s") {
            nodes {
              targetType
              name {
                text
              }
            }
          }
        }
      }
      """ % (
            directory.swhid(),
            name_include,
        )

        data, _ = utils.get_query_response(client, query_str)

        for entry in data["directory"]["entries"]["nodes"]:
            assert name_include in entry["name"]["text"]
            assert entry["targetType"] == get_target_type(dir_entry["type"])
This revision now requires changes to proceed.Sep 21 2022, 2:09 PM

Build is green

Patch application report for D8511 (id=30667)

Rebasing onto ad9602edd7...

First, rewinding head to replay your work on top of it...
Applying: Change value of targetType in DirectoryEntry; add filter by entry name
Changes applied before test
commit afb80b1563e20b04f6ca087ac30f0292fc225d09
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Sep 20 15:02:47 2022 +0200

    Change value of targetType in DirectoryEntry; add filter by entry name
    
    targetType is called "dir", "file" and "rev" in DirectoryEntry and is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL level filter for directory entries.

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

This revision is now accepted and ready to land.Sep 21 2022, 2:57 PM

Build is green

Patch application report for D8511 (id=30669)

Rebasing onto ad9602edd7...

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

    Change value of targetType in DirectoryEntry; add filter by entry name
    
    targetType is called "dir", "file" and "rev" in DirectoryEntry and is
    different from other target types. Change that to "directory", "content"
    and "revision"
    
    Add a nameInclude GraphQL level filter for directory entries.

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