fix: Fix path parsing for copy, rename (#672)

* fix: Fix path parsing for copy, list, rename

* Add tests

* bump pyo3 version

Author: kylebarron

.gitignore

@@ -1,3 +1,6 @@
+*.dylib
+*.dylib.yml
+Info.plist
 *.so
 *.whl
 

Cargo.lock

@@ -3,7 +3,7 @@ use std::{future::Future, pin::Pin};
 use object_store::ObjectStoreExt;
 use pyo3::prelude::*;
 use pyo3_async_runtimes::tokio::get_runtime;
-use pyo3_object_store::{PyObjectStore, PyObjectStoreError, PyObjectStoreResult};
+use pyo3_object_store::{PyObjectStore, PyObjectStoreError, PyObjectStoreResult, PyPath};
 
 use crate::utils::PyNone;
 
@@ -12,8 +12,8 @@ use crate::utils::PyNone;
 pub(crate) fn copy(
     py: Python,
     store: PyObjectStore,
-    from_: String,
-    to: String,
+    from_: PyPath,
+    to: PyPath,
     overwrite: bool,
 ) -> PyObjectStoreResult<()> {
     let runtime = get_runtime();
@@ -35,8 +35,8 @@ pub(crate) fn copy(
 pub(crate) fn copy_async(
     py: Python,
     store: PyObjectStore,
-    from_: String,
-    to: String,
+    from_: PyPath,
+    to: PyPath,
     overwrite: bool,
 ) -> PyResult<Bound<PyAny>> {
     let from_ = from_.into();

obstore/src/copy.rs

@@ -346,7 +346,7 @@ pub(crate) fn get(
 ) -> PyObjectStoreResult<PyGetResult> {
     let runtime = get_runtime();
     py.detach(|| {
-        let path = &path.as_ref();
+        let path = path.as_ref();
         let fut = if let Some(options) = options {
             store.as_ref().get_opts(path, options.into())
         } else {
@@ -366,7 +366,7 @@ pub(crate) fn get_async(
     options: Option<PyGetOptions>,
 ) -> PyResult<Bound<PyAny>> {
     pyo3_async_runtimes::tokio::future_into_py(py, async move {
-        let path = &path.as_ref();
+        let path = path.as_ref();
         let fut = if let Some(options) = options {
             store.as_ref().get_opts(path, options.into())
         } else {

obstore/src/get.rs

@@ -8,7 +8,6 @@ use arrow::datatypes::{DataType, Field, Schema, TimeUnit};
 use futures::stream::{BoxStream, Fuse};
 use futures::StreamExt;
 use indexmap::IndexMap;
-use object_store::path::Path;
 use object_store::{ListResult, ObjectMeta, ObjectStore};
 use pyo3::exceptions::{PyImportError, PyStopAsyncIteration, PyStopIteration};
 use pyo3::prelude::*;
@@ -17,7 +16,7 @@ use pyo3::{intern, IntoPyObjectExt};
 use pyo3_arrow::export::{Arro3RecordBatch, Arro3Table};
 use pyo3_arrow::PyTable;
 use pyo3_async_runtimes::tokio::get_runtime;
-use pyo3_object_store::{PyObjectStore, PyObjectStoreError, PyObjectStoreResult};
+use pyo3_object_store::{PyObjectStore, PyObjectStoreError, PyObjectStoreResult, PyPath};
 use tokio::sync::Mutex;
 
 pub(crate) struct PyObjectMeta(ObjectMeta);
@@ -353,8 +352,8 @@ impl<'py> IntoPyObject<'py> for PyListResult {
 pub(crate) fn list(
     py: Python,
     store: PyObjectStore,
-    prefix: Option<String>,
-    offset: Option<String>,
+    prefix: Option<PyPath>,
+    offset: Option<PyPath>,
     chunk_size: usize,
     return_arrow: bool,
 ) -> PyObjectStoreResult<PyListStream> {
@@ -373,7 +372,7 @@ pub(crate) fn list(
     let store = store.into_inner().clone();
     let prefix = prefix.map(|s| s.into());
     let stream = if let Some(offset) = offset {
-        store.list_with_offset(prefix.as_ref(), &offset.into())
+        store.list_with_offset(prefix.as_ref(), offset.as_ref())
     } else {
         store.list(prefix.as_ref())
     };
@@ -385,14 +384,14 @@ pub(crate) fn list(
 pub(crate) fn list_with_delimiter(
     py: Python,
     store: PyObjectStore,
-    prefix: Option<String>,
+    prefix: Option<PyPath>,
     return_arrow: bool,
 ) -> PyObjectStoreResult<PyListResult> {
     let runtime = get_runtime();
     py.detach(|| {
         let out = runtime.block_on(list_with_delimiter_materialize(
             store.into_inner(),
-            prefix.map(|s| s.into()).as_ref(),
+            prefix.as_ref(),
             return_arrow,
         ))?;
         Ok::<_, PyObjectStoreError>(out)
@@ -404,25 +403,24 @@ pub(crate) fn list_with_delimiter(
 pub(crate) fn list_with_delimiter_async(
     py: Python,
     store: PyObjectStore,
-    prefix: Option<String>,
+    prefix: Option<PyPath>,
     return_arrow: bool,
 ) -> PyResult<Bound<PyAny>> {
     pyo3_async_runtimes::tokio::future_into_py(py, async move {
-        let out = list_with_delimiter_materialize(
-            store.into_inner(),
-            prefix.map(|s| s.into()).as_ref(),
-            return_arrow,
-        )
-        .await?;
+        let out =
+            list_with_delimiter_materialize(store.into_inner(), prefix.as_ref(), return_arrow)
+                .await?;
         Ok(out)
     })
 }
 
 async fn list_with_delimiter_materialize(
     store: Arc<dyn ObjectStore>,
-    prefix: Option<&Path>,
+    prefix: Option<&PyPath>,
     return_arrow: bool,
 ) -> PyObjectStoreResult<PyListResult> {
-    let list_result = store.list_with_delimiter(prefix).await?;
+    let list_result = store
+        .list_with_delimiter(prefix.map(|s| s.as_ref()))
+        .await?;
     Ok(PyListResult::new(list_result, return_arrow))
 }

obstore/src/list.rs

@@ -3,7 +3,7 @@ use std::{future::Future, pin::Pin};
 use object_store::ObjectStoreExt;
 use pyo3::prelude::*;
 use pyo3_async_runtimes::tokio::get_runtime;
-use pyo3_object_store::{PyObjectStore, PyObjectStoreError, PyObjectStoreResult};
+use pyo3_object_store::{PyObjectStore, PyObjectStoreError, PyObjectStoreResult, PyPath};
 
 use crate::utils::PyNone;
 
@@ -12,8 +12,8 @@ use crate::utils::PyNone;
 pub(crate) fn rename(
     py: Python,
     store: PyObjectStore,
-    from_: String,
-    to: String,
+    from_: PyPath,
+    to: PyPath,
     overwrite: bool,
 ) -> PyObjectStoreResult<()> {
     let runtime = get_runtime();
@@ -35,8 +35,8 @@ pub(crate) fn rename(
 pub(crate) fn rename_async(
     py: Python,
     store: PyObjectStore,
-    from_: String,
-    to: String,
+    from_: PyPath,
+    to: PyPath,
     overwrite: bool,
 ) -> PyResult<Bound<PyAny>> {
     let from_ = from_.into();

obstore/src/rename.rs

@@ -0,0 +1,23 @@
+from obstore.store import MemoryStore
+
+
+def test_copy_non_ascii():
+    store = MemoryStore()
+
+    name1 = "café.txt"
+    name2 = "ümlaut.txt"
+    name3 = "こんにちは世界.txt"
+    name4 = "你好世界.txt"
+
+    store.put(name1, b"foo")
+    store.put(name3, b"bar")
+
+    store.copy(name1, name2)
+    store.copy(name3, name4)
+
+    result = store.list().collect()
+    assert len(result) == 4
+    assert result[0]["path"] == name1
+    assert result[1]["path"] == name2
+    assert result[2]["path"] == name3
+    assert result[3]["path"] == name4

tests/test_copy.py

@@ -0,0 +1,20 @@
+from obstore.store import MemoryStore
+
+
+def test_head_non_ascii():
+    store = MemoryStore()
+
+    name1 = "café.txt"
+    name2 = "ümlaut.txt"
+    name3 = "こんにちは世界.txt"
+    name4 = "你好世界.txt"
+
+    store.put(name1, b"foo")
+    store.put(name2, b"bar")
+    store.put(name3, b"baz")
+    store.put(name4, b"qux")
+
+    assert store.head(name1)["path"] == name1
+    assert store.head(name2)["path"] == name2
+    assert store.head(name3)["path"] == name3
+    assert store.head(name4)["path"] == name4

tests/test_head.py

@@ -11,11 +11,27 @@ def test_list():
     store.put("file2.txt", b"bar")
     store.put("file3.txt", b"baz")
 
-    stream = store.list()
-    result = stream.collect()
+    result = store.list().collect()
     assert len(result) == 3
 
 
+def test_list_non_ascii():
+    store = MemoryStore()
+
+    name1 = "café.txt"
+    name2 = "ümlaut.txt"
+    name3 = "こんにちは世界.txt"
+    store.put(name1, b"foo")
+    store.put(name2, b"bar")
+    store.put(name3, b"baz")
+
+    result = store.list().collect()
+    assert len(result) == 3
+    assert result[0]["path"] == name1
+    assert result[1]["path"] == name2
+    assert result[2]["path"] == name3
+
+
 def test_list_as_arrow():
     store = MemoryStore()
 
@@ -37,6 +53,23 @@ def test_list_as_arrow():
     assert batch.num_rows == 100
 
 
+def test_list_non_ascii_arrow():
+    store = MemoryStore()
+
+    name1 = "café.txt"
+    name2 = "ümlaut.txt"
+    name3 = "こんにちは世界.txt"
+    store.put(name1, b"foo")
+    store.put(name2, b"bar")
+    store.put(name3, b"baz")
+
+    result = store.list(return_arrow=True).collect()
+    assert result.num_rows == 3
+    assert result["path"][0].as_py() == name1
+    assert result["path"][1].as_py() == name2
+    assert result["path"][2].as_py() == name3
+
+
 @pytest.mark.asyncio
 async def test_list_stream_async():
     store = MemoryStore()

tests/test_list.py

@@ -0,0 +1,21 @@
+from obstore.store import MemoryStore
+
+
+def test_rename_non_ascii():
+    store = MemoryStore()
+
+    name1 = "café.txt"
+    name2 = "ümlaut.txt"
+    name3 = "こんにちは世界.txt"
+    name4 = "你好世界.txt"
+
+    store.put(name1, b"foo")
+    store.put(name3, b"bar")
+
+    store.rename(name1, name2)
+    store.rename(name3, name4)
+
+    result = store.list().collect()
+    assert len(result) == 2
+    assert result[0]["path"] == name2
+    assert result[1]["path"] == name4