fix(dependencies): update minio client to adjust for changed exceptions

Minio in the newest version changed some API sturucture. Mainly, the
specific exceptions got wrapped into the `S3Error` exception.

While we're at it, also remove some duplicate code and move it into
a decorator (Any other place would imply more boilerplate).

caluma/caluma_form/storage_clients.py

@@ -1,8 +1,43 @@
 from datetime import timedelta
+from functools import wraps
+from logging import getLogger
 
 import minio
 import urllib3
 from django.conf import settings
+from minio.error import S3Error
+
+log = getLogger(__name__)
+
+
+class Retry(BaseException):
+    pass
+
+
+def _retry_on_missing_bucket(fn):
+    """Create missing bucket if needed (decorator).
+
+    If enabled in the settings, try to create the bucket if it
+    doesn't exist yet, then retry.
+    """
+
+    @wraps(fn)
+    def wrapper(self, *args, **kwargs):
+        try:
+            return fn(self, *args, **kwargs)
+        except S3Error as exc:
+            if (
+                exc.code == "NoSuchBucket"
+                and settings.MINIO_STORAGE_AUTO_CREATE_MEDIA_BUCKET
+            ):
+                log.warning(
+                    b"Minio bucket '{self.bucket}' missing, trying to create it"
+                )
+                self.client.make_bucket(self.bucket)
+                return fn(self, *args, **kwargs)
+            raise
+
+    return wrapper
 
 
 class Minio:
@@ -34,6 +69,7 @@ def __init__(self):
         )
         self.bucket = settings.MINIO_STORAGE_MEDIA_BUCKET_NAME
 
+    @_retry_on_missing_bucket
     def stat_object(self, object_name):
         """
         Get stat of object in bucket.
@@ -43,39 +79,25 @@ def stat_object(self, object_name):
         """
         try:
             return self.client.stat_object(self.bucket, object_name)
-        except minio.error.NoSuchKey:  # pragma: no cover
-            # object does not exist
-            pass
-        except minio.error.NoSuchBucket:  # pragma: no cover
-            if settings.MINIO_STORAGE_AUTO_CREATE_MEDIA_BUCKET:
-                self.client.make_bucket(self.bucket)
-                return self.stat_object(object_name)
-        except minio.error.ResponseError:  # pragma: no cover
-            pass
+        except S3Error as exc:
+            log.error(f"Minio error, cannot stat object: {exc.code}")
+            return None
 
+    @_retry_on_missing_bucket
     def download_url(self, object_name):
-        try:
-            return self.client.presigned_get_object(
-                self.bucket,
-                object_name,
-                timedelta(minutes=settings.MINIO_PRESIGNED_TTL_MINUTES),
-            )
-        except minio.error.NoSuchBucket:  # pragma: no cover
-            if settings.MINIO_STORAGE_AUTO_CREATE_MEDIA_BUCKET:
-                self.client.make_bucket(self.bucket)
-                return self.download_url(object_name)
+        return self.client.presigned_get_object(
+            self.bucket,
+            object_name,
+            timedelta(minutes=settings.MINIO_PRESIGNED_TTL_MINUTES),
+        )
 
+    @_retry_on_missing_bucket
     def upload_url(self, object_name):
-        try:
-            return self.client.presigned_put_object(
-                self.bucket,
-                object_name,
-                timedelta(minutes=settings.MINIO_PRESIGNED_TTL_MINUTES),
-            )
-        except minio.error.NoSuchBucket:  # pragma: no cover
-            if settings.MINIO_STORAGE_AUTO_CREATE_MEDIA_BUCKET:
-                self.client.make_bucket(self.bucket)
-                return self.upload_url(object_name)
+        return self.client.presigned_put_object(
+            self.bucket,
+            object_name,
+            timedelta(minutes=settings.MINIO_PRESIGNED_TTL_MINUTES),
+        )
 
     def remove_object(self, object_name):
         self.client.remove_object(self.bucket, object_name)

caluma/caluma_form/tests/test_minio.py

@@ -1,3 +1,4 @@
+import minio.error
 import pytest
 import urllib3
 
@@ -28,3 +29,66 @@ def test_minio_disable_cert_checks(db, settings, disable_cert_checks, debug):
         with pytest.raises(urllib3.exceptions.RequestError):
             # This should fail, as we are in fact verifying certificates
             client.client._http.urlopen("get", "https://self-signed.badssl.com/")
+
+
+def _put_side_effect(*_, **__):
+    # first call, raise an exception, second time, succeed
+    yield minio.error.S3Error(
+        "NoSuchBucket",
+        "Bucket does not exist",
+        resource="adsf",
+        request_id="fake",
+        host_id="fake",
+        response="fake",
+    )
+    yield "upload_url_successful"
+
+
+@pytest.fixture
+def patched_minio(mocker):
+    make_bucket = mocker.patch("minio.api.Minio.make_bucket")
+    put_object = mocker.patch(
+        "minio.api.Minio.presigned_put_object", side_effect=_put_side_effect()
+    )
+    return make_bucket, put_object
+
+
+def test_minio_auto_create_bucket_enabled(db, settings, patched_minio):
+    settings.MINIO_STORAGE_AUTO_CREATE_MEDIA_BUCKET = True
+    client = storage_clients.Minio()
+
+    make_bucket, _ = patched_minio
+
+    assert client.upload_url("asdf") == "upload_url_successful"
+    assert make_bucket.call_count == 1
+
+
+def test_minio_auto_create_bucket_disabled(db, settings, patched_minio):
+    settings.MINIO_STORAGE_AUTO_CREATE_MEDIA_BUCKET = False
+    client = storage_clients.Minio()
+    make_bucket, _ = patched_minio
+
+    with pytest.raises(minio.error.S3Error):
+        client.upload_url("asdf")
+
+    assert make_bucket.call_count == 0
+
+
+@pytest.mark.parametrize("exc_code", ["NoSuchBucket", "NoSuchKey"])
+def test_minio_handle_exceptions(exc_code, caplog, mocker):
+    # The "happy path" is tested in various places already
+    mocker.patch(
+        "minio.api.Minio.stat_object",
+        side_effect=minio.error.S3Error(
+            code=exc_code,
+            message="",
+            resource="test_object",
+            request_id="",
+            host_id="",
+            response=None,
+        ),
+    )
+    client = storage_clients.Minio()
+    stat = client.stat_object("test_object")
+    assert stat is None
+    assert caplog.messages == [f"Minio error, cannot stat object: {exc_code}"]