diff --git a/README.md b/README.md index 14148ed..af27e88 100644 --- a/README.md +++ b/README.md @@ -266,7 +266,7 @@ def test_verifier_transport(self): # verify some stuff from the request self.assertEqual(request.container, container_name) - self.assertEqual(request.path, os.path.join(os.sep, container_name, 'some/path')) + self.assertEqual(request.path, url_join(os.sep, container_name, 'some/path')) # return a mocked response return unittest.mock.MagicMock(status_code=200, @@ -276,7 +276,7 @@ def test_verifier_transport(self): # verify some stuff from the request self.assertEqual(request.container, container_name) - self.assertEqual(request.path, os.path.join(os.sep, container_name, 'some/table/path/some_item_key')) + self.assertEqual(request.path, url_join(os.sep, container_name, 'some/table/path/some_item_key')) # prepare and output mock output = unittest.mock.MagicMock(item={ diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..f0a5b4b --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1,14 @@ +# Copyright 2023 Iguazio +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# diff --git a/tests/test_client.py b/tests/test_client.py index 3b46490..d7d83d2 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -22,11 +22,11 @@ import future.utils -import v3io.common.helpers import v3io.dataplane import v3io.dataplane.output import v3io.dataplane.response import v3io.logger +from v3io.common.helpers import url_join class Test(unittest.TestCase): @@ -64,7 +64,7 @@ def _delete_dir(self, path): class TestContainer(Test): def setUp(self): super(TestContainer, self).setUp() - self._path = os.path.join(self._test_parent_dir, "v3io-py-test-container") + self._path = url_join(self._test_parent_dir, "v3io-py-test-container") # clean up self._delete_dir(self._path) @@ -84,13 +84,13 @@ def test_get_container_contents_valid(self): for object_index in range(file_number): self._client.object.put( container=self._container, - path=os.path.join(self._path, "object-{0}.txt".format(object_index)), + path=url_join(self._path, "object-{0}.txt".format(object_index)), body=body, ) for object_index in range(dir_number): self._client.object.put( - container=self._container, path=os.path.join(self._path, "dir-{0}/".format(object_index)) + container=self._container, path=url_join(self._path, "dir-{0}/".format(object_index)) ) response = self._client.container.list( @@ -111,7 +111,7 @@ class TestStream(Test): def setUp(self): super(TestStream, self).setUp() - self._path = os.path.join(self._test_parent_dir, "v3io-py-test-stream") + self._path = url_join(self._test_parent_dir, "v3io-py-test-stream") # clean up self._client.stream.delete(container=self._container, stream_path=self._path, raise_for_status=[200, 204, 404]) @@ -136,7 +136,7 @@ def test_delete_stream_with_cg(self): # write several "consumer group state" files for cg_id in range(3): self._client.object.put( - container=self._container, path=os.path.join(self._path, "cg{}-state.json".format(cg_id)) + container=self._container, path=url_join(self._path, "cg{}-state.json".format(cg_id)) ) # check that the stream doesn't exist @@ -209,7 +209,7 @@ class TestObject(Test): def setUp(self): super(TestObject, self).setUp() - self._object_dir = os.path.join(self._test_parent_dir, "v3io-py-test-object") + self._object_dir = url_join(self._test_parent_dir, "v3io-py-test-object") self._object_path = self._object_dir + "/obj ect.txt" # clean up @@ -315,8 +315,8 @@ class TestSchema(Test): def setUp(self): super(TestSchema, self).setUp() - self._schema_dir = os.path.join(self._test_parent_dir, "v3io-py-test-schema") - self._schema_path = os.path.join(self._schema_dir, ".#schema") + self._schema_dir = url_join(self._test_parent_dir, "v3io-py-test-schema") + self._schema_path = url_join(self._schema_dir, ".#schema") # clean up self._delete_dir(self._schema_dir) @@ -371,7 +371,7 @@ class TestKv(Test): def setUp(self): super(TestKv, self).setUp() - self._path = os.path.join(self._test_parent_dir, "some_dir/v3io-py-test-emd") + self._path = url_join(self._test_parent_dir, "some_dir/v3io-py-test-emd") self._delete_dir(self._path) def test_kv_array(self): @@ -399,7 +399,7 @@ def test_kv_array(self): def test_kv_values(self): def _get_int_array(): - int_array = array.array("l") + int_array = array.array("q") for value in range(10): int_array.append(value) @@ -587,7 +587,7 @@ def _compare_item_values(self, v1, v2): v1 = list(v1) if v1 != v2: - self.fail("Values dont match") + self.fail(f"Values don't match: {v1} vs {v2}") def _compare_item_types(self, v1, v2): if isinstance(v1, array.array): @@ -681,7 +681,7 @@ class TestConnectonErrorRecovery(Test): def setUp(self): super(TestConnectonErrorRecovery, self).setUp() - self._object_dir = os.path.join(self._test_parent_dir, "v3io-py-test-connection-error") + self._object_dir = url_join(self._test_parent_dir, "v3io-py-test-connection-error") self._object_path = self._object_dir + "/object.txt" self._kv_path = "some_dir/v3io-py-test-emd" @@ -761,7 +761,7 @@ def test_verifier_transport(self): def _verify_object_get(request): # verify some stuff from the request self.assertEqual(request.container, container_name) - self.assertEqual(request.path, os.path.join(os.sep, container_name, "some/path")) + self.assertEqual(request.path, url_join("/", container_name, "some/path")) # return a mocked response return unittest.mock.MagicMock(status_code=200, body="some body") @@ -769,7 +769,7 @@ def _verify_object_get(request): def _verify_kv_get(request): # verify some stuff from the request self.assertEqual(request.container, container_name) - self.assertEqual(request.path, os.path.join(os.sep, container_name, "some/table/path/some_item_key")) + self.assertEqual(request.path, url_join("/", container_name, "some/table/path/some_item_key")) # prepare and output mock output = unittest.mock.MagicMock(item={"some_key": "some_value"}) diff --git a/tests/test_client_aio.py b/tests/test_client_aio.py index 71199ac..a2b832b 100644 --- a/tests/test_client_aio.py +++ b/tests/test_client_aio.py @@ -21,6 +21,7 @@ import v3io.aio.dataplane import v3io.dataplane +from v3io.common.helpers import url_join class Test(unittest.IsolatedAsyncioTestCase): @@ -56,7 +57,7 @@ async def _delete_dir(self, path): class TestContainer(Test): async def asyncSetUp(self): await super(TestContainer, self).asyncSetUp() - self._path = os.path.join(self._test_parent_dir, "v3io-py-test-container") + self._path = url_join(self._test_parent_dir, "v3io-py-test-container") # clean up await self._delete_dir(self._path) @@ -74,13 +75,13 @@ async def test_get_container_contents(self): for object_index in range(5): await self._client.object.put( container=self._container, - path=os.path.join(self._path, "object-{0}.txt".format(object_index)), + path=url_join(self._path, "object-{0}.txt".format(object_index)), body=body, ) for object_index in range(5): await self._client.object.put( - container=self._container, path=os.path.join(self._path, "dir-{0}/".format(object_index)) + container=self._container, path=url_join(self._path, "dir-{0}/".format(object_index)) ) response = await self._client.container.list( @@ -103,7 +104,7 @@ class TestStream(Test): async def asyncSetUp(self): await super(TestStream, self).asyncSetUp() - self._path = os.path.join(self._test_parent_dir, "v3io-py-test-stream") + self._path = url_join(self._test_parent_dir, "v3io-py-test-stream") # clean up await self._client.stream.delete( @@ -130,7 +131,7 @@ async def test_delete_stream_with_cg(self): # write several "consumer group state" files for cg_id in range(3): await self._client.object.put( - container=self._container, path=os.path.join(self._path, "cg{}-state.json".format(cg_id)) + container=self._container, path=url_join(self._path, "cg{}-state.json".format(cg_id)) ) # check that the stream doesn't exist @@ -207,7 +208,7 @@ class TestObject(Test): async def asyncSetUp(self): await super(TestObject, self).asyncSetUp() - self._object_dir = os.path.join(self._test_parent_dir, "v3io-py-test-object") + self._object_dir = url_join(self._test_parent_dir, "v3io-py-test-object") self._object_path = self._object_dir + "/obj ect.txt" # clean up @@ -290,7 +291,7 @@ async def test_get_offset(self): # await super(TestSchema, self).asyncSetUp() # # self._schema_dir = "/v3io-py-test-schemaa" -# self._schema_path = os.path.join(self._schema_dir, ".%23schema") +# self._schema_path = url_join(self._schema_dir, ".%23schema") # # # clean up # await self._delete_dir(self._schema_dir) @@ -346,7 +347,7 @@ class TestKv(Test): async def asyncSetUp(self): await super(TestKv, self).asyncSetUp() - self._path = os.path.join(self._test_parent_dir, "some_dir/v3io-py-test-emd") + self._path = url_join(self._test_parent_dir, "some_dir/v3io-py-test-emd") await self._delete_dir(self._path) async def test_kv_array(self): @@ -374,7 +375,7 @@ async def test_kv_array(self): async def test_kv_values(self): def _get_int_array(): - int_array = array.array("l") + int_array = array.array("q") for value in range(10): int_array.append(value) diff --git a/tests/test_client_errors.py b/tests/test_client_errors.py index 0e583be..49fde94 100644 --- a/tests/test_client_errors.py +++ b/tests/test_client_errors.py @@ -1,3 +1,17 @@ +# Copyright 2023 Iguazio +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# import itertools import pytest diff --git a/tests/test_common.py b/tests/test_common.py new file mode 100644 index 0000000..8a39a3e --- /dev/null +++ b/tests/test_common.py @@ -0,0 +1,32 @@ +# Copyright 2023 Iguazio +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +import unittest + +from v3io.common.helpers import url_join + + +class Test(unittest.TestCase): + def test_url_join(self): + self.assertEqual("a/b", url_join("a", "b")) # add just exactly one "/" between parts + self.assertEqual("/a/b", url_join("/", "a", "b")) + self.assertEqual("/a/b", url_join("/", "a", "/b")) + self.assertEqual("/a/b", url_join("/", "/a", "b")) + self.assertEqual("/a/b", url_join("/", "/a", "/b")) + self.assertEqual("/a/b", url_join("/", "/a/", "b")) + self.assertEqual("/a/b", url_join("/", "/a/", "/b")) + self.assertEqual("a/b", url_join("a", "b")) # keep suffix "/" exist/not-exist invariant + self.assertEqual("a/b/", url_join("a", "b/")) + self.assertEqual("a/b//", url_join("a", "b//")) + self.assertEqual("a/b//", url_join("a", "b//", "/")) # suffix "/" count (if > 0) may change (but we don"t care) diff --git a/v3io/aio/dataplane/kv.py b/v3io/aio/dataplane/kv.py index b0047ef..15449fa 100644 --- a/v3io/aio/dataplane/kv.py +++ b/v3io/aio/dataplane/kv.py @@ -12,12 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. # -import os - import v3io.aio.dataplane.kv_cursor import v3io.dataplane.model import v3io.dataplane.output import v3io.dataplane.request +from v3io.common.helpers import url_join class Model(v3io.dataplane.model.Model): @@ -303,7 +302,7 @@ async def delete(self, container, table_path, key, access_key=None, raise_for_st A `Response` object. """ return self._client.delete_object( - container, os.path.join(table_path, key), access_key, raise_for_status, transport_actions + container, url_join(table_path, key), access_key, raise_for_status, transport_actions ) async def create_schema(self, container, table_path, access_key=None, raise_for_status=None, key=None, fields=None): @@ -343,7 +342,7 @@ async def create_schema(self, container, table_path, access_key=None, raise_for_ A `Response` object """ put_object_args = locals() - put_object_args["path"] = os.path.join(put_object_args["table_path"], ".#schema") + put_object_args["path"] = url_join(put_object_args["table_path"], ".#schema") put_object_args["offset"] = 0 put_object_args["append"] = None put_object_args["body"] = self._client._get_schema_contents(key, fields) diff --git a/v3io/aio/dataplane/stream.py b/v3io/aio/dataplane/stream.py index 8b89051..01b27a7 100644 --- a/v3io/aio/dataplane/stream.py +++ b/v3io/aio/dataplane/stream.py @@ -12,12 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. # -import os - import v3io.dataplane.kv_cursor import v3io.dataplane.model import v3io.dataplane.output import v3io.dataplane.request +from v3io.common.helpers import url_join class Model(v3io.dataplane.model.Model): @@ -204,7 +203,7 @@ async def seek( ---------- A `Response` object, whose `output` is `SeekShardOutput`. """ - stream_path = self._ensure_path_ends_with_slash(os.path.join(stream_path, str(shard_id))) + stream_path = self._ensure_path_ends_with_slash(url_join(stream_path, str(shard_id))) return await self._transport.request( container, @@ -307,7 +306,7 @@ async def get_records( ---------- A `Response` object, whose `output` is `GetRecordsOutput`. """ - stream_path = self._ensure_path_ends_with_slash(os.path.join(stream_path, str(shard_id))) + stream_path = self._ensure_path_ends_with_slash(url_join(stream_path, str(shard_id))) return await self._transport.request( container, diff --git a/v3io/common/helpers.py b/v3io/common/helpers.py index 8ad032b..5e303ea 100644 --- a/v3io/common/helpers.py +++ b/v3io/common/helpers.py @@ -12,16 +12,22 @@ # See the License for the specific language governing permissions and # limitations under the License. # -def url_join(base, *parts): - result = base - if result[0] != "/": - result = "/" + base - for part in parts: - if part[0] != "/": +def url_join(*parts): + result = "" + slash_suffix = False + for part_index, part in enumerate(parts): + if part == "": + continue + # add slash prefix before part if: + # 1. slash suffix did not exit in prev part + # 2. slash prefix does not exit in this part + # 3. part is not the first + if not slash_suffix and part[0] != "/" and part_index != 0: result += "/" + part else: - result += part - + # if slash suffix existed in prev trim slash prefix from this part + result += part if not slash_suffix else part.lstrip("/") + slash_suffix = True if part[-1] == "/" else False return result diff --git a/v3io/dataplane/client.py b/v3io/dataplane/client.py index 7440671..a0bce07 100644 --- a/v3io/dataplane/client.py +++ b/v3io/dataplane/client.py @@ -18,7 +18,6 @@ import future.utils import ujson -import v3io.common.helpers import v3io.dataplane.batch import v3io.dataplane.kv_cursor import v3io.dataplane.output @@ -27,6 +26,7 @@ import v3io.dataplane.transport.httpclient import v3io.dataplane.transport.requests import v3io.logger +from v3io.common.helpers import url_join class Client(object): @@ -417,7 +417,7 @@ def put_items(self, container, path, items, access_key=None, raise_for_status=No # create a put item input response = self.put_item( container, - v3io.common.helpers.url_join(path, item_path), + url_join("/", path, item_path), item_attributes, access_key=access_key, condition=condition, @@ -1007,7 +1007,7 @@ def create_schema( A `Response` object """ put_object_args = locals() - put_object_args["path"] = os.path.join(put_object_args["path"], ".#schema") + put_object_args["path"] = url_join(put_object_args["path"], ".#schema") put_object_args["offset"] = 0 put_object_args["append"] = None put_object_args["body"] = self._get_schema_contents(key, fields) diff --git a/v3io/dataplane/kv.py b/v3io/dataplane/kv.py index 9e75704..8a382d0 100644 --- a/v3io/dataplane/kv.py +++ b/v3io/dataplane/kv.py @@ -12,12 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. # -import os - import v3io.dataplane.kv_cursor import v3io.dataplane.model import v3io.dataplane.output import v3io.dataplane.request +from v3io.common.helpers import url_join class Model(v3io.dataplane.model.Model): @@ -328,7 +327,7 @@ def delete(self, container, table_path, key, access_key=None, raise_for_status=N A `Response` object. """ return self._client.delete_object( - container, os.path.join(table_path, key), access_key, raise_for_status, transport_actions + container, url_join(table_path, key), access_key, raise_for_status, transport_actions ) def create_schema( @@ -377,7 +376,7 @@ def create_schema( A `Response` object """ put_object_args = locals() - put_object_args["path"] = os.path.join(put_object_args["table_path"], ".#schema") + put_object_args["path"] = url_join(put_object_args["table_path"], ".#schema") put_object_args["offset"] = 0 put_object_args["append"] = None put_object_args["body"] = self._client._get_schema_contents(key, fields) diff --git a/v3io/dataplane/kv_array.py b/v3io/dataplane/kv_array.py index a1670c2..9644436 100644 --- a/v3io/dataplane/kv_array.py +++ b/v3io/dataplane/kv_array.py @@ -24,7 +24,7 @@ def encode_list(list_value): - typecode = "l" + typecode = "q" if len(list_value) and isinstance(list_value[0], float): typecode = "d" @@ -33,7 +33,7 @@ def encode_list(list_value): def encode_array(array_value, typecode): num_items = len(array_value) - operand_type = OPERAND_TYPE_LONG if typecode == "l" else OPERAND_TYPE_DOUBLE + operand_type = OPERAND_TYPE_LONG if typecode == "q" else OPERAND_TYPE_DOUBLE encoded_array = ITEM_HEADER_MAGIC_AND_VERSION + struct.pack( "II" + typecode * num_items, num_items * 8, operand_type, *array_value @@ -57,7 +57,7 @@ def decode(encoded_array): unpacked_header = struct.unpack("II", header) # get the typecode and number of items - typecode = "l" if unpacked_header[1] == OPERAND_TYPE_LONG else "d" + typecode = "q" if unpacked_header[1] == OPERAND_TYPE_LONG else "d" num_items = int(unpacked_header[0] / 8) # decode the values diff --git a/v3io/dataplane/request.py b/v3io/dataplane/request.py index 1922a34..5a711ff 100644 --- a/v3io/dataplane/request.py +++ b/v3io/dataplane/request.py @@ -15,10 +15,11 @@ import array import base64 import datetime -import os import future.utils +from v3io.common.helpers import url_join + try: from urllib.parse import quote, urlencode except BaseException: @@ -146,7 +147,7 @@ def encode_put_item(container_name, access_key, kwargs): "PUT", container_name, access_key, - kwargs.get("path") or os.path.join(kwargs["table_path"], kwargs["key"]), + kwargs.get("path") or url_join(kwargs["table_path"], kwargs["key"]), None, {"X-v3io-function": "PutItem"}, body, @@ -181,7 +182,7 @@ def encode_update_item(container_name, access_key, kwargs): http_method, container_name, access_key, - kwargs.get("path") or os.path.join(kwargs["table_path"], kwargs["key"]), + kwargs.get("path") or url_join(kwargs["table_path"], kwargs["key"]), None, {"X-v3io-function": function_name}, body, @@ -195,7 +196,7 @@ def encode_get_item(container_name, access_key, kwargs): "PUT", container_name, access_key, - kwargs.get("path") or os.path.join(kwargs["table_path"], kwargs["key"]), + kwargs.get("path") or url_join(kwargs["table_path"], kwargs["key"]), None, {"X-v3io-function": "GetItem"}, body, @@ -385,7 +386,7 @@ def encode_get_records(container_name, access_key, kwargs): def _encode(method, container_name, access_key, path, query, headers, body): if path is not None: - path = v3io.common.helpers.url_join(container_name, path) + path = v3io.common.helpers.url_join("/", container_name, path) else: path = container_name diff --git a/v3io/dataplane/stream.py b/v3io/dataplane/stream.py index 35d8ed5..3acee9a 100644 --- a/v3io/dataplane/stream.py +++ b/v3io/dataplane/stream.py @@ -12,12 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. # -import os - import v3io.dataplane.kv_cursor import v3io.dataplane.model import v3io.dataplane.output import v3io.dataplane.request +from v3io.common.helpers import url_join class Model(v3io.dataplane.model.Model): @@ -217,7 +216,7 @@ def seek( ---------- A `Response` object, whose `output` is `SeekShardOutput`. """ - stream_path = self._ensure_path_ends_with_slash(os.path.join(stream_path, str(shard_id))) + stream_path = self._ensure_path_ends_with_slash(url_join(stream_path, str(shard_id))) return self._transport.request( container, @@ -332,7 +331,7 @@ def get_records( ---------- A `Response` object, whose `output` is `GetRecordsOutput`. """ - stream_path = self._ensure_path_ends_with_slash(os.path.join(stream_path, str(shard_id))) + stream_path = self._ensure_path_ends_with_slash(url_join(stream_path, str(shard_id))) return self._transport.request( container,