s3fs: handle versioned paths in info and exists (#746)

Author: efiop

s3fs/core.py

@@ -967,18 +967,39 @@ async def _ls(self, path, detail=False, refresh=False, versions=False):
                 return files
         return files if detail else sorted([o["name"] for o in files])
 
+    def _exists_in_cache(self, path, bucket, key, version_id):
+        fullpath = "/".join((bucket, key))
+
+        try:
+            entries = self._ls_from_cache(fullpath)
+        except FileNotFoundError:
+            return False
+
+        if entries is None:
+            return None
+
+        if not self.version_aware or version_id is None:
+            return True
+
+        for entry in entries:
+            if entry["name"] == fullpath and entry.get("VersionId") == version_id:
+                return True
+
+        # dircache doesn't support multiple versions, so we really can't tell if
+        # the one we want exists.
+        return None
+
     async def _exists(self, path):
         if path in ["", "/"]:
             # the root always exists, even if anon
             return True
         path = self._strip_protocol(path)
         bucket, key, version_id = self.split_path(path)
         if key:
-            try:
-                if self._ls_from_cache(path):
-                    return True
-            except FileNotFoundError:
-                return False
+            exists_in_cache = self._exists_in_cache(path, bucket, key, version_id)
+            if exists_in_cache is not None:
+                return exists_in_cache
+
             try:
                 await self._info(path, bucket, key, version_id=version_id)
                 return True
@@ -1216,6 +1237,8 @@ async def _open_file(range: int):
     async def _info(self, path, bucket=None, key=None, refresh=False, version_id=None):
         path = self._strip_protocol(path)
         bucket, key, path_version_id = self.split_path(path)
+        fullpath = "/".join((bucket, key))
+
         if version_id is not None:
             if not self.version_aware:
                 raise ValueError(
@@ -1226,15 +1249,15 @@ async def _info(self, path, bucket=None, key=None, refresh=False, version_id=Non
             return {"name": path, "size": 0, "type": "directory"}
         version_id = _coalesce_version_id(path_version_id, version_id)
         if not refresh:
-            out = self._ls_from_cache(path)
+            out = self._ls_from_cache(fullpath)
             if out is not None:
                 if self.version_aware and version_id is not None:
                     # If cached info does not match requested version_id,
                     # fallback to calling head_object
                     out = [
                         o
                         for o in out
-                        if o["name"] == path and version_id == o.get("VersionId")
+                        if o["name"] == fullpath and version_id == o.get("VersionId")
                     ]
                     if out:
                         return out[0]

s3fs/tests/test_s3fs.py

@@ -1508,8 +1508,11 @@ def test_tags(s3):
     assert s3.get_tags(fname) == tagset
 
 
-def test_versions(s3):
-    versioned_file = versioned_bucket_name + "/versioned_file"
+@pytest.mark.parametrize("prefix", ["", "/dir", "/dir/subdir"])
+def test_versions(s3, prefix):
+    parent = versioned_bucket_name + prefix
+    versioned_file = parent + "/versioned_file"
+
     s3 = S3FileSystem(
         anon=False, version_aware=True, client_kwargs={"endpoint_url": endpoint_uri}
     )
@@ -1537,6 +1540,17 @@ def test_versions(s3):
         assert fo.version_id == first_version
         assert fo.read() == b"1"
 
+    versioned_file_v1 = f"{versioned_file}?versionId={first_version}"
+    versioned_file_v2 = f"{versioned_file}?versionId={second_version}"
+
+    assert s3.ls(parent) == [versioned_file]
+    assert set(s3.ls(parent, versions=True)) == {versioned_file_v1, versioned_file_v2}
+
+    assert s3.exists(versioned_file_v1)
+    assert s3.info(versioned_file_v1)
+    assert s3.exists(versioned_file_v2)
+    assert s3.info(versioned_file_v2)
+
 
 def test_list_versions_many(s3):
     # moto doesn't actually behave in the same way that s3 does here so this doesn't test