prevent find from corrupting dircache (#488)

Author: dtrifiro

gcsfs/core.py

@@ -1086,54 +1086,55 @@ async def _isdir(self, path):
     async def _find(self, path, withdirs=False, detail=False, prefix="", **kwargs):
         path = self._strip_protocol(path)
         bucket, key = self.split_path(path)
-        out, _ = await self._do_list_objects(
-            path,
-            delimiter=None,
-            prefix=prefix,
-        )
-        if not prefix and not out and key:
-            try:
-                out = [
-                    await self._get_object(
-                        path,
-                    )
-                ]
-            except FileNotFoundError:
-                out = []
-        dirs = []
-        sdirs = set()
+
+        if prefix:
+            _path = "" if not key else key.rstrip("/") + "/"
+            _prefix = f"{_path}{prefix}"
+        else:
+            _prefix = key
+
+        objects, _ = await self._do_list_objects(bucket, delimiter="", prefix=_prefix)
+
+        dirs = {}
         cache_entries = {}
-        for o in out:
-            par = o["name"]
-            while par:
-                par = self._parent(par)
-                if par not in sdirs:
-                    if len(par) < len(path):
-                        break
-                    sdirs.add(par)
-                    dirs.append(
-                        {
-                            "Key": self.split_path(par)[1],
-                            "Size": 0,
-                            "name": par,
-                            "StorageClass": "DIRECTORY",
-                            "type": "directory",
-                            "size": 0,
-                        }
-                    )
-                # Don't cache "folder-like" objects (ex: "Create Folder" in GCS console) to prevent
-                # masking subfiles in subsequent requests.
-                if not o["name"].endswith("/"):
-                    cache_entries.setdefault(par, []).append(o)
+
+        for obj in objects:
+            parent = self._parent(obj["name"])
+            previous = obj
+
+            while parent:
+                dir_key = self.split_path(parent)[1]
+                if not dir_key:
+                    break
+
+                dirs[parent] = {
+                    "Key": dir_key,
+                    "Size": 0,
+                    "name": parent,
+                    "StorageClass": "DIRECTORY",
+                    "type": "directory",
+                    "size": 0,
+                }
+
+                if len(parent) < len(path):
+                    # don't go above the requested level
+                    break
+
+                cache_entries.setdefault(parent, []).append(previous)
+
+                previous = dirs[parent]
+                parent = self._parent(parent)
+
         if not prefix:
             self.dircache.update(cache_entries)
 
         if withdirs:
-            out = sorted(out + dirs, key=lambda x: x["name"])
+            objects = sorted(objects + list(dirs.values()), key=lambda x: x["name"])
 
         if detail:
-            return {o["name"]: o for o in out}
-        return [o["name"] for o in out]
+            return {o["name"]: o for o in objects}
+
+        return [o["name"] for o in objects]
 
     @retry_request(retries=retries)
     async def _get_file_request(

gcsfs/tests/test_core.py

@@ -15,11 +15,12 @@
 
 from gcsfs.tests.settings import TEST_BUCKET, TEST_PROJECT, TEST_REQUESTER_PAYS_BUCKET
 from gcsfs.tests.conftest import (
-    files,
-    csv_files,
-    text_files,
     a,
+    allfiles,
     b,
+    csv_files,
+    files,
+    text_files,
 )
 from gcsfs.tests.utils import tempdir, tmpfile
 from gcsfs.core import GCSFileSystem, quote_plus
@@ -982,26 +983,42 @@ def test_zero_cache_timeout(gcs):
         gcs.dircache[f"{TEST_BUCKET}/a"]
 
 
-def test_find_with_prefix_partial_cache(gcs):
+@pytest.mark.parametrize("with_cache", (False, True))
+def test_find_with_prefix_partial_cache(gcs, with_cache):
     base_dir = f"{TEST_BUCKET}/test_find_with_prefix"
     gcs.touch(base_dir + "/test_1")
     gcs.touch(base_dir + "/test_2")
 
-    for with_cache in (True, False):
-        # Test once with cached, and once with no cache
-        gcs.invalidate_cache()
-        if with_cache:
-            gcs.ls(base_dir)
-        precache = dict(gcs.dircache)
-        assert gcs.find(base_dir, prefix="non_existent_") == []
-        assert gcs.find(base_dir, prefix="test_") == [
-            base_dir + "/test_1",
-            base_dir + "/test_2",
-        ]
-        assert dict(gcs.dircache) == precache  # find qwith prefix shouldn't touch cache
-        assert gcs.find(base_dir + "/test_1") == [base_dir + "/test_1"]
-        assert gcs.find(base_dir + "/non_existent") == []
-        assert gcs.find(base_dir + "/non_existent", prefix="more_non_existent") == []
+    gcs.invalidate_cache()
+    if with_cache:
+        gcs.ls(base_dir)
+    precache = dict(gcs.dircache)
+    assert gcs.find(base_dir, prefix="non_existent_") == []
+    assert gcs.find(base_dir, prefix="test_") == [
+        base_dir + "/test_1",
+        base_dir + "/test_2",
+    ]
+    assert dict(gcs.dircache) == precache  # find qwith prefix shouldn't touch cache
+    assert gcs.find(base_dir + "/test_1") == [base_dir + "/test_1"]
+    assert gcs.find(base_dir + "/non_existent") == []
+    assert gcs.find(base_dir + "/non_existent", prefix="more_non_existent") == []
+
+
+def test_find_dircache(gcs):
+    """Running `ls` after find should not corrupt the dir cache"""
+    assert set(gcs.find(TEST_BUCKET)) == {f"{TEST_BUCKET}/{path}" for path in allfiles}
+    assert set(gcs.ls(TEST_BUCKET)) == {
+        f"{TEST_BUCKET}/test",
+        f"{TEST_BUCKET}/nested",
+        f"{TEST_BUCKET}/2014-01-01.csv",
+        f"{TEST_BUCKET}/2014-01-02.csv",
+        f"{TEST_BUCKET}/2014-01-03.csv",
+    }
+    assert set(gcs.ls(f"{TEST_BUCKET}/nested")) == {
+        f"{TEST_BUCKET}/nested/file1",
+        f"{TEST_BUCKET}/nested/file2",
+        f"{TEST_BUCKET}/nested/nested2",
+    }
 
 
 def test_percent_file_name(gcs):