Backport 2.7: Fix slug id used in blockstore

newsfragments/2154.bugfix.rst

@@ -0,0 +1 @@
+Fix regression in Parsec server introduced in version 2.7.0 leading to block being stored and fetched with an incorrect ID

parsec/backend/s3_blockstore.py

@@ -13,6 +13,13 @@
 from parsec.backend.block import BlockAlreadyExistsError, BlockNotFoundError, BlockTimeoutError
 
 
+def build_s3_slug(organization_id: OrganizationID, id: BlockID):
+    # The slug uses the UUID canonical textual representation (eg.
+    # `CoolOrg/3b917792-35ac-409f-9af1-fe6de8d2b905`) where `BlockID.__str__`
+    # uses the short textual representation (eg. `3b91779235ac409f9af1fe6de8d2b905`)
+    return f"{organization_id}/{id.uuid}"
+
+
 class S3BlockStoreComponent(BaseBlockStoreComponent):
     def __init__(self, s3_region, s3_bucket, s3_key, s3_secret, s3_endpoint_url=None):
         self._s3 = None
@@ -28,7 +35,7 @@ def __init__(self, s3_region, s3_bucket, s3_key, s3_secret, s3_endpoint_url=None
         self._s3.head_bucket(Bucket=s3_bucket)
 
     async def read(self, organization_id: OrganizationID, id: BlockID) -> bytes:
-        slug = f"{organization_id}/{id}"
+        slug = build_s3_slug(organization_id=organization_id, id=id)
         try:
             obj = self._s3.get_object(Bucket=self._s3_bucket, Key=slug)
 
@@ -45,7 +52,7 @@ async def read(self, organization_id: OrganizationID, id: BlockID) -> bytes:
         return obj["Body"].read()
 
     async def create(self, organization_id: OrganizationID, id: BlockID, block: bytes) -> None:
-        slug = f"{organization_id}/{id}"
+        slug = build_s3_slug(organization_id=organization_id, id=id)
         try:
             await trio.to_thread.run_sync(
                 partial(self._s3.head_object, Bucket=self._s3_bucket, Key=slug)

parsec/backend/swift_blockstore.py

@@ -26,6 +26,13 @@ def side_effect(key):
 from parsec.backend.block import BlockAlreadyExistsError, BlockNotFoundError, BlockTimeoutError
 
 
+def build_swift_slug(organization_id: OrganizationID, id: BlockID):
+    # The slug uses the UUID canonical textual representation (eg.
+    # `CoolOrg/3b917792-35ac-409f-9af1-fe6de8d2b905`) where `BlockID.__str__`
+    # uses the short textual representation (eg. `3b91779235ac409f9af1fe6de8d2b905`)
+    return f"{organization_id}/{id.uuid}"
+
+
 class SwiftBlockStoreComponent(BaseBlockStoreComponent):
     def __init__(self, auth_url, tenant, container, user, password):
         self.swift_client = swiftclient.Connection(
@@ -35,7 +42,7 @@ def __init__(self, auth_url, tenant, container, user, password):
         self.swift_client.head_container(container)
 
     async def read(self, organization_id: OrganizationID, id: BlockID) -> bytes:
-        slug = f"{organization_id}/{id}"
+        slug = build_swift_slug(organization_id=organization_id, id=id)
         try:
             headers, obj = await trio.to_thread.run_sync(
                 self.swift_client.get_object, self._container, slug
@@ -51,11 +58,9 @@ async def read(self, organization_id: OrganizationID, id: BlockID) -> bytes:
         return obj
 
     async def create(self, organization_id: OrganizationID, id: BlockID, block: bytes) -> None:
-        slug = f"{organization_id}/{id}"
+        slug = build_swift_slug(organization_id=organization_id, id=id)
         try:
-            _, obj = await trio.to_thread.run_sync(
-                self.swift_client.get_object, self._container, slug
-            )
+            await trio.to_thread.run_sync(self.swift_client.get_object, self._container, slug)
 
         except ClientException as exc:
             if exc.http_status == 404:

tests/backend/test_s3_blockstore.py

@@ -9,12 +9,16 @@
 )
 import pytest
 
+from parsec.api.protocol import OrganizationID, BlockID
 from parsec.backend.s3_blockstore import S3BlockStoreComponent
 from parsec.backend.block import BlockAlreadyExistsError, BlockNotFoundError, BlockTimeoutError
 
 
 @pytest.mark.trio
 async def test_s3_read():
+    org_id = OrganizationID("org42")
+    block_id = BlockID.from_hex("0694a21176354e8295e28a543e5887f9")
+
     with mock.patch("boto3.client") as client_mock:
         client_mock.return_value = Mock()
         client_mock().head_bucket.return_value = True
@@ -23,27 +27,34 @@ async def test_s3_read():
         response_mock = Mock()
         response_mock.read.return_value = "content"
         client_mock().get_object.return_value = {"Body": response_mock}
-        assert await blockstore.read("org42", 123) == "content"
+        assert await blockstore.read(org_id, block_id) == "content"
+        client_mock().get_object.assert_called_once_with(
+            Bucket="parsec", Key="org42/0694a211-7635-4e82-95e2-8a543e5887f9"
+        )
+        client_mock().get_object.reset_mock()
         # Not found
         client_mock().get_object.side_effect = S3ClientError(
             error_response={"Error": {"Code": "404"}}, operation_name="GET"
         )
         with pytest.raises(BlockNotFoundError):
-            assert await blockstore.read("org42", 123)
+            assert await blockstore.read(org_id, block_id)
         # Connection error
         client_mock().get_object.side_effect = S3EndpointConnectionError(endpoint_url="url")
         with pytest.raises(BlockTimeoutError):
-            assert await blockstore.read("org42", 123)
+            assert await blockstore.read(org_id, block_id)
         # Unknown exception
         client_mock().get_object.side_effect = S3ClientError(
             error_response={"Error": {"Code": "401"}}, operation_name="GET"
         )
         with pytest.raises(BlockTimeoutError):
-            assert await blockstore.read("org42", 123)
+            assert await blockstore.read(org_id, block_id)
 
 
 @pytest.mark.trio
 async def test_s3_create():
+    org_id = OrganizationID("org42")
+    block_id = BlockID.from_hex("0694a21176354e8295e28a543e5887f9")
+
     with mock.patch("boto3.client") as client_mock:
         client_mock.return_value = Mock()
         client_mock().head_container.return_value = True
@@ -52,37 +63,37 @@ async def test_s3_create():
         client_mock().head_object.side_effect = S3ClientError(
             error_response={"Error": {"Code": "404"}}, operation_name="HEAD"
         )
-        await blockstore.create("org42", 123, "content")
+        await blockstore.create(org_id, block_id, "content")
         client_mock().put_object.assert_called_with(
-            Bucket="parsec", Key="org42/123", Body="content"
+            Bucket="parsec", Key="org42/0694a211-7635-4e82-95e2-8a543e5887f9", Body="content"
         )
         client_mock().put_object.reset_mock()
         # Already exist
         client_mock().head_object.side_effect = None
         with pytest.raises(BlockAlreadyExistsError):
-            await blockstore.create("org42", 123, "content")
+            await blockstore.create(org_id, block_id, "content")
         client_mock().put_object.assert_not_called()
         # Connection error at HEAD
         client_mock().head_object.side_effect = S3EndpointConnectionError(endpoint_url="url")
         with pytest.raises(BlockTimeoutError):
-            await blockstore.create("org42", 123, "content")
+            await blockstore.create(org_id, block_id, "content")
         client_mock().put_object.assert_not_called()
         # Unknown exception at HEAD
         client_mock().head_object.side_effect = S3ClientError(
             error_response={"Error": {"Code": "401"}}, operation_name="HEAD"
         )
         with pytest.raises(BlockTimeoutError):
-            await blockstore.create("org42", 123, "content")
+            await blockstore.create(org_id, block_id, "content")
         # Connection error at PUT
         client_mock().head_object.side_effect = S3ClientError(
             error_response={"Error": {"Code": "404"}}, operation_name="HEAD"
         )
         client_mock().put_object.side_effect = S3EndpointConnectionError(endpoint_url="url")
         with pytest.raises(BlockTimeoutError):
-            await blockstore.create("org42", 123, "content")
+            await blockstore.create(org_id, block_id, "content")
         # Unknown exception at PUT
         client_mock().put_object.side_effect = S3ClientError(
             error_response={"Error": {"Code": "401"}}, operation_name="PUT"
         )
         with pytest.raises(BlockTimeoutError):
-            await blockstore.create("org42", 123, "content")
+            await blockstore.create(org_id, block_id, "content")

tests/backend/test_swift_blockstore.py

@@ -2,52 +2,76 @@
 
 from unittest.mock import Mock
 from unittest import mock
-import swiftclient
 from swiftclient.exceptions import ClientException
 import pytest
 
+from parsec.api.protocol import OrganizationID, BlockID
 from parsec.backend.block import BlockAlreadyExistsError, BlockNotFoundError, BlockTimeoutError
 from parsec.backend.swift_blockstore import SwiftBlockStoreComponent
 
-import swiftclient  # noqa
-from swiftclient.exceptions import ClientException  # noqa
-
 
 @pytest.mark.trio
 async def test_swift_get():
+    org_id = OrganizationID("org42")
+    block_id = BlockID.from_hex("0694a21176354e8295e28a543e5887f9")
+
     with mock.patch("swiftclient.Connection") as connection_mock:
         connection_mock.return_value = Mock()
         connection_mock().head_container.return_value = True
         blockstore = SwiftBlockStoreComponent("http://url", "scille", "parsec", "john", "secret")
         # Ok
         connection_mock().get_object.return_value = True, "content"
-        assert await blockstore.read("org42", 123) == "content"
+        assert await blockstore.read(org_id, block_id) == "content"
+        connection_mock().get_object.assert_called_once_with(
+            "parsec", "org42/0694a211-7635-4e82-95e2-8a543e5887f9"
+        )
+        connection_mock().get_object.reset_mock()
         # Not found
         connection_mock().get_object.side_effect = ClientException(http_status=404, msg="")
         with pytest.raises(BlockNotFoundError):
-            assert await blockstore.read("org42", 123)
+            assert await blockstore.read(org_id, block_id)
         # Other exception
         connection_mock().get_object.side_effect = ClientException(http_status=500, msg="")
         with pytest.raises(BlockTimeoutError):
-            assert await blockstore.read("org42", 123)
+            assert await blockstore.read(org_id, block_id)
 
 
 @pytest.mark.trio
-async def test_swift_post():
+async def test_swift_create():
+    org_id = OrganizationID("org42")
+    block_id = BlockID.from_hex("0694a21176354e8295e28a543e5887f9")
+
     with mock.patch("swiftclient.Connection") as connection_mock:
         connection_mock.return_value = Mock()
         connection_mock().head_container.return_value = True
         blockstore = SwiftBlockStoreComponent("http://url", "scille", "parsec", "john", "secret")
         # Ok
         connection_mock().get_object.side_effect = ClientException(http_status=404, msg="")
-        await blockstore.create("org42", 123, "content")
-        connection_mock().put_object.assert_called_with("parsec", "org42/123", "content")
-        # Already exists
+        await blockstore.create(org_id, block_id, "content")
+        connection_mock().put_object.assert_called_with(
+            "parsec", "org42/0694a211-7635-4e82-95e2-8a543e5887f9", "content"
+        )
+        connection_mock().put_object.reset_mock()
+        # Already exist
         connection_mock().get_object.side_effect = None
-        connection_mock().get_object.return_value = True, "content"
         with pytest.raises(BlockAlreadyExistsError):
-            await blockstore.create("org42", 123, "content")
-        # Other exception
+            await blockstore.create(org_id, block_id, "content")
+        connection_mock().put_object.assert_not_called()
+        # Connection error at HEAD
+        connection_mock().get_object.side_effect = ClientException(msg="Connection error")
+        with pytest.raises(BlockTimeoutError):
+            await blockstore.create(org_id, block_id, "content")
+        connection_mock().put_object.assert_not_called()
+        # Unknown exception at HEAD
         connection_mock().get_object.side_effect = ClientException(http_status=500, msg="")
         with pytest.raises(BlockTimeoutError):
-            await blockstore.create("org42", 123, "content")
+            await blockstore.create(org_id, block_id, "content")
+        # Connection error at PUT
+        connection_mock().get_object.side_effect = ClientException(msg="Connection error")
+        connection_mock().put_object.side_effect = ClientException(msg="Connection error")
+        with pytest.raises(BlockTimeoutError):
+            await blockstore.create(org_id, block_id, "content")
+        # Unknown exception at PUT
+        connection_mock().put_object.side_effect = ClientException(http_status=500, msg="")
+        with pytest.raises(BlockTimeoutError):
+            await blockstore.create(org_id, block_id, "content")