Add Path-Like Arguments for Querying Files (#87)

* add shortcut-args: directory, path, path-end, path-start, & path-regex

* bump circleci to py3.8 (to match Dockerfile)

* add filename arg (replace start & end)

* "filename" arg fix

* add module block comments to tests

* add test_argbuilder.py

* _handle_path_args() -> _resolve_path_args(): make in-place updates

* "directory" & "filename" wildcard bug fix

* test_00_path_args now passing

* misc

* remove needless variables

* _resolve_path_args RETURNS the resolved path (& only POPS from kwargs)

* add test_13_files_path_like_args() integration test

* simplify directory/filename regex

Author: ric-evans

.circleci/config.yml

@@ -2,7 +2,7 @@ version: 2
 jobs:
     test:
         docker:
-            - image: circleci/python:3.7.2
+            - image: circleci/python:3.8
               environment:
                 # Both TEST_DATABASE_HOST and TEST_DATABASE_PORT must be defined.
                 # Things will break if host is defined and port isn't or vice-versa.

file_catalog/argbuilder.py

@@ -1,11 +1,10 @@
 """Builder utility functions for arg/kwargs dicts."""
 
-from typing import Any, Dict
 
-from tornado.escape import json_decode
+from typing import Any, Dict, Optional, Union
 
-# local imports
 from file_catalog.mongo import AllKeys
+from tornado.escape import json_decode
 
 
 def build_limit(kwargs: Dict[str, Any], config: Dict[str, Any]) -> None:
@@ -31,6 +30,34 @@ def build_start(kwargs: Dict[str, Any]) -> None:
             raise Exception("start is negative")
 
 
+def _resolve_path_args(kwargs: Dict[str, Any]) -> Optional[Union[Dict[str, Any], str]]:
+    """Resolve the path-type shortcut arguments by precedence.
+
+    Pop each key from `kwargs`, even if it's not used.
+    """
+    arg: Optional[Union[Dict[str, Any], str]] = None
+
+    # regex
+    if "path-regex" in kwargs:
+        arg = {"$regex": kwargs.pop("path-regex")}
+
+    # normal path
+    if "path" in kwargs:
+        arg = kwargs.pop("path")
+    if "logical_name" in kwargs:
+        arg = kwargs.pop("logical_name")
+
+    # directory & filename
+    if "directory" in kwargs or "filename" in kwargs:
+        if not (dpath := kwargs.pop("directory", "").rstrip("/")):
+            dpath = r".*"
+        if not (fname := kwargs.pop("filename", "").lstrip("/")):
+            fname = r".*"
+        arg = {"$regex": rf"^{dpath}/(.*/)?{fname}$"}
+
+    return arg
+
+
 def build_files_query(kwargs: Dict[str, Any]) -> None:
     """Build `"query"` dict with formatted/fully-named arguments.
 
@@ -49,8 +76,8 @@ def build_files_query(kwargs: Dict[str, Any]) -> None:
         query["locations.archive"] = None
 
     # shortcut query params
-    if "logical_name" in kwargs:
-        query["logical_name"] = kwargs.pop("logical_name")
+    if path := _resolve_path_args(kwargs):
+        query["logical_name"] = path
     if "run_number" in kwargs:
         query["run.run_number"] = kwargs.pop("run_number")
     if "dataset" in kwargs:

tests/test_argbuilder.py

@@ -0,0 +1,97 @@
+"""Test argbuilder.py functions."""
+
+# pylint: disable=W0212
+
+import pprint
+from typing import Any, Dict, List, Optional, TypedDict, Union
+
+from file_catalog import argbuilder
+
+
+def test_00_path_args() -> None:
+    """Test _handle_path_args."""
+
+    class KwargsTests(TypedDict):  # pylint: disable=C0115
+        kwargs_in: Dict[str, Any]
+        kwargs_after: Dict[str, Any]
+        ret: Optional[Union[Dict[str, Any], str]]
+
+    kwargs_test_dicts: List[KwargsTests] = [
+        # null case
+        {"kwargs_in": {}, "kwargs_after": {}, "ret": None},
+        # no path-args
+        {
+            "kwargs_in": {"an-extra-argument": [12, 34, 56]},
+            "kwargs_after": {"an-extra-argument": [12, 34, 56]},
+            "ret": None,
+        },
+        # only "path-regex"
+        {
+            "kwargs_in": {"path-regex": r"/reg-ex/this.*/(file/)?path"},
+            "kwargs_after": {},
+            "ret": {"$regex": r"/reg-ex/this.*/(file/)?path"},
+        },
+        # only "path"
+        {
+            "kwargs_in": {"an-extra-argument": [12, 34, 56], "path": "PATH"},
+            "kwargs_after": {"an-extra-argument": [12, 34, 56]},
+            "ret": "PATH",
+        },
+        # only "logical_name"
+        {
+            "kwargs_in": {"logical_name": "LOGICAL_NAME"},
+            "kwargs_after": {},
+            "ret": "LOGICAL_NAME",
+        },
+        # only "directory"
+        {
+            "kwargs_in": {"directory": "/path/to/dir/"},
+            "kwargs_after": {},
+            "ret": {"$regex": r"^/path/to/dir/(.*/)?.*$"},
+        },
+        # only "directory" w/o trailing '/'
+        {
+            "kwargs_in": {"directory": "/path/to/dir"},
+            "kwargs_after": {},
+            "ret": {"$regex": r"^/path/to/dir/(.*/)?.*$"},
+        },
+        # only "filename"
+        {
+            "kwargs_in": {"filename": "my-file"},
+            "kwargs_after": {},
+            "ret": {"$regex": r"^.*/(.*/)?my-file$"},
+        },
+        # only "filename" w/ a sub-directory
+        {
+            "kwargs_in": {"filename": "/sub-dir/my-file"},
+            "kwargs_after": {},
+            "ret": {"$regex": r"^.*/(.*/)?sub-dir/my-file$"},
+        },
+        # "directory" & "filename"
+        {
+            "kwargs_in": {"directory": "/path/to/dir/", "filename": "my-file"},
+            "kwargs_after": {},
+            "ret": {"$regex": r"^/path/to/dir/(.*/)?my-file$"},
+        },
+    ]
+
+    for ktd in kwargs_test_dicts:
+        pprint.pprint(ktd)
+        print()
+        assert argbuilder._resolve_path_args(ktd["kwargs_in"]) == ktd["ret"]
+        assert ktd["kwargs_in"] == ktd["kwargs_after"]
+
+    # test multiple path-args (each loop pops the arg of the highest precedence)
+    args = [  # list in decreasing order of precedence
+        ("directory", "/path/to/dir/", {"$regex": r"^/path/to/dir/(.*/)?.*$"}),
+        # not testing "filename" b/c that is equal to "directory" in precedence
+        ("logical_name", "LOGICAL_NAME", "LOGICAL_NAME"),
+        ("path", "PATH", "PATH"),
+        ("path-regex", r"this.*is?a.path", {"$regex": r"this.*is?a.path"}),
+    ]
+    while args:
+        kwargs = {k: v for (k, v, _) in args}
+        pprint.pprint(kwargs)
+        assert argbuilder._resolve_path_args(kwargs) == args[0][2]
+        assert not kwargs  # everything was popped
+        args.pop(0)

tests/test_collections.py

@@ -1,12 +1,18 @@
+"""Test /api/collections."""
+
+# fmt:off
+# pylint: skip-file
+
 from __future__ import absolute_import, division, print_function
 
 import os
 import unittest
 
 from rest_tools.client import RestClient
 
-from .test_server import TestServerAPI
 from .test_files import hex
+from .test_server import TestServerAPI
+
 
 class TestCollectionsAPI(TestServerAPI):
     def test_10_collections(self):
@@ -183,7 +189,7 @@ def test_71_snapshot_find(self):
         self.assertIn('file', data)
         url = data['file']
         file_uid = url.split('/')[-1]
-        
+
         # old snapshot stays empty
         data = r.request_seq('GET', '/api/snapshots/{}/files'.format(snap_uid))
         self.assertEqual(data['files'], [])
@@ -201,7 +207,7 @@ def test_71_snapshot_find(self):
         self.assertEqual(len(data['files']), 1)
         self.assertEqual(data['files'][0]['uuid'], file_uid)
         self.assertEqual(data['files'][0]['checksum'], metadata['checksum'])
-        
+
 
 if __name__ == '__main__':
     suite = unittest.TestLoader().loadTestsFromTestCase(TestStringMethods)

tests/test_files.py

@@ -1,24 +1,28 @@
+"""Test /api/files."""
+
 # fmt:off
+# pylint: skip-file
 
 from __future__ import absolute_import, division, print_function
 
 import hashlib
 import os
 import unittest
+from typing import Any, Dict, List, Optional
 
-from tornado.escape import json_decode, json_encode
-
-# local imports
-from rest_tools.client import RestClient
+from rest_tools.client import RestClient  # type: ignore[import]
+from tornado.escape import json_encode
 
 from .test_server import TestServerAPI
 
 
-def hex(data):
+def hex(data: Any) -> str:
+    """Get sha512."""
     if isinstance(data, str):
-        data = data.encode('utf-8')
+        data = data.encode("utf-8")
     return hashlib.sha512(data).hexdigest()
 
+
 class TestFilesAPI(TestServerAPI):
     def test_10_files(self):
         self.start_server()
@@ -111,7 +115,7 @@ def test_12_files_keys(self):
             "locations",
             "extra",
             "supplemental",
-            "meta_modify_date"
+            "meta_modify_date",
         }
 
         # w/ all-keys = False
@@ -130,7 +134,7 @@ def test_12_files_keys(self):
             "locations",
             "extra",
             "supplemental",
-            "meta_modify_date"
+            "meta_modify_date",
         }
 
         # w/ all-keys = False & keys
@@ -143,6 +147,78 @@ def test_12_files_keys(self):
         data = r.request_seq("GET", "/api/files", args)
         assert set(data["files"][0].keys()) == {"checksum", "file_size"}
 
+    def test_13_files_path_like_args(self):
+        """Test the path-like base/shortcut arguments.
+
+        "logical_name", "directory", "filename", "path", & "path-regex".
+        """
+        self.start_server()  # type: ignore[no-untyped-call]
+        token = self.get_token()  # type: ignore[no-untyped-call]
+        r = RestClient(self.address, token, timeout=1, retries=1)
+
+        metadata_objs = [
+            {
+                "logical_name": "/foo/bar/baz/bat.txt",
+                "checksum": {"sha512": hex("1")},
+                "file_size": 1,
+                "locations": [{"site": "test", "path": "foo/bar/baz/bat.txt"}],
+            },
+            {
+                "logical_name": "/foo/bar/ham.txt",
+                "checksum": {"sha512": hex("2")},
+                "file_size": 2,
+                "locations": [{"site": "test", "path": "/foo/bar/ham.txt"}],
+            },
+            {
+                "logical_name": "/green/eggs/and/ham.txt",
+                "checksum": {"sha512": hex("3")},
+                "file_size": 3,
+                "locations": [{"site": "test", "path": "/green/eggs/and/ham.txt"}],
+            },
+            {
+                "logical_name": "/john/paul/george/ringo/ham.txt",
+                "checksum": {"sha512": hex("4")},
+                "file_size": 4,
+                "locations": [
+                    {"site": "test", "path": "/john/paul/george/ringo/ham.txt"}
+                ],
+            },
+        ]
+        for meta in metadata_objs:
+            r.request_seq("POST", "/api/files", meta)
+
+        def get_paths(args: Optional[Dict[str, str]] = None) -> List[str]:
+            if not args:
+                args = {}
+            ret = r.request_seq("GET", "/api/files", args)
+            print(ret)
+            return [f["logical_name"] for f in ret["files"]]
+
+        assert len(get_paths()) == 4
+        # logical_name
+        assert len(get_paths({"logical_name": "/foo/bar/ham.txt"})) == 1
+        # path
+        assert len(get_paths({"path": "/green/eggs/and/ham.txt"})) == 1
+        # directory
+        paths = get_paths({"directory": "/foo/bar"})
+        assert set(paths) == {"/foo/bar/ham.txt", "/foo/bar/baz/bat.txt"}
+        assert len(get_paths({"directory": "/fo"})) == 0
+        # filename
+        paths = get_paths({"filename": "ham.txt"})
+        assert set(paths) == {
+            "/foo/bar/ham.txt",
+            "/green/eggs/and/ham.txt",
+            "/john/paul/george/ringo/ham.txt",
+        }
+        assert len(get_paths({"filename": ".txt"})) == 0
+        # directory & filename
+        paths = get_paths({"directory": "/foo", "filename": "ham.txt"})
+        assert paths == ["/foo/bar/ham.txt"]
+        # path-regex
+        paths = get_paths({"path-regex": r".*george/ringo.*"})
+        assert paths == ["/john/paul/george/ringo/ham.txt"]
+        assert len(get_paths({"path-regex": r".*"})) == 4
+
     def test_15_files_auth(self):
         self.start_server(config_override={'SECRET':'secret'})
         token = self.get_token()

tests/test_server.py

@@ -1,22 +1,27 @@
+"""Test REST Server."""
+
+# fmt:off
+# pylint: skip-file
+
 from __future__ import absolute_import, division, print_function
 
+import hashlib
 import os
-import time
-import tempfile
-import shutil
 import random
+import shutil
 import subprocess
+import tempfile
+import time
+import unittest
 from functools import partial
 from threading import Thread
-import unittest
-import hashlib
 
-from tornado.escape import json_encode,json_decode
-from tornado.ioloop import IOLoop
 import requests
+from file_catalog.urlargparse import encode as jquery_encode
 from pymongo import MongoClient
+from tornado.escape import json_decode, json_encode
+from tornado.ioloop import IOLoop
 
-from file_catalog.urlargparse import encode as jquery_encode
 
 class TestServerAPI(unittest.TestCase):
     def setUp(self):
@@ -35,7 +40,7 @@ def setUp(self):
                                   '--logpath', dblog])
             self.addCleanup(partial(time.sleep, 0.3))
             self.addCleanup(m.terminate)
-        
+
     def clean_db(self, host, port):
         db = MongoClient(host=host, port=port).file_catalog
         colls = db.list_collection_names()