From d6e784cb1fb78d041c7b27e9155946d03f26d477 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Thu, 3 Aug 2023 16:43:12 +0200 Subject: [PATCH 01/38] utilizing sentry --- .../qfieldcloud/core/middleware/requests.py | 17 +++++++++++++++++ docker-app/qfieldcloud/core/utils2/sentry.py | 10 +++++++++- .../qfieldcloud/core/views/files_views.py | 1 + 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index e470877b2..48552e89c 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -1,3 +1,7 @@ +import io +import shutil + + def attach_keys(get_response): """ QF-2540 @@ -6,12 +10,25 @@ def attach_keys(get_response): """ def middleware(request): + input_stream = io.BufferedReader(io.BytesIO(request.body)) + request_attributes = { "file_key": str(request.FILES.keys()), "meta": str(request.META), } + + # only report rawbody of less-than-10MBs multipart requests + if ( + "multipart/form-data;boundary" in request.headers["Content-Type"] + and input_stream.tell() < 10000000 + ): + output_stream = io.BytesIO() + shutil.copyfileobj(input_stream, output_stream) + request.body_stream = output_stream + if "file" in request.FILES.keys(): request_attributes["files"] = request.FILES.getlist("file") + request.attached_keys = str(request_attributes) response = get_response(request) return response diff --git a/docker-app/qfieldcloud/core/utils2/sentry.py b/docker-app/qfieldcloud/core/utils2/sentry.py index 00d14bd75..1e72324e9 100644 --- a/docker-app/qfieldcloud/core/utils2/sentry.py +++ b/docker-app/qfieldcloud/core/utils2/sentry.py @@ -1,5 +1,5 @@ import logging -from io import StringIO +from io import BytesIO, StringIO import sentry_sdk @@ -11,6 +11,7 @@ def report_serialization_diff_to_sentry( pre_serialization: str, post_serialization: str, buffer: StringIO, + body_stream: BytesIO, capture_message=False, ) -> bool: """ @@ -20,10 +21,12 @@ def report_serialization_diff_to_sentry( pre_serialization: str representing the request `files` keys and meta information before serialization and middleware. post_serialization: str representing the request `files` keys and meta information after serialization and middleware. buffer: StringIO buffer from which to extract traceback capturing callstack ahead of the calling function. + bodystream: BytesIO buffer capturing the request's raw body. capture_message: bool used as a flag by the caller to create an extra event against Sentry to attach the files to. """ with sentry_sdk.configure_scope() as scope: try: + filename = f"{name}_contents.txt" scope.add_attachment( bytes=bytes( @@ -38,9 +41,14 @@ def report_serialization_diff_to_sentry( bytes=bytes(buffer.getvalue(), encoding="utf8"), filename=filename, ) + + filename = f"{name}_rawbody.txt" + scope.add_attachment(bytes=body_stream.getvalue(), filename=filename) + if capture_message: sentry_sdk.capture_message("Sending to Sentry...", scope=scope) return True + except Exception as error: sentry_sdk.capture_exception(error) logging.error(f"Unable to send file to Sentry: failed on {error}") diff --git a/docker-app/qfieldcloud/core/views/files_views.py b/docker-app/qfieldcloud/core/views/files_views.py index f89c3f296..c72c783ff 100644 --- a/docker-app/qfieldcloud/core/views/files_views.py +++ b/docker-app/qfieldcloud/core/views/files_views.py @@ -222,6 +222,7 @@ def post(self, request, projectid, filename, format=None): pre_serialization=request.attached_keys, post_serialization=str(request_attributes), buffer=buffer, + body_stream=request.body_stream, ) raise exceptions.EmptyContentError() From 6104c172567ecb7fa9fd27e60e239216ef443ce6 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Thu, 3 Aug 2023 17:57:28 +0200 Subject: [PATCH 02/38] content-type --- docker-app/qfieldcloud/core/middleware/requests.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index 48552e89c..7473bd558 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -17,9 +17,14 @@ def middleware(request): "meta": str(request.META), } + content_type = request.headers.get("Content-Type") or request.headers.get( + "content-type" + ) + # only report rawbody of less-than-10MBs multipart requests if ( - "multipart/form-data;boundary" in request.headers["Content-Type"] + content_type + and "multipart/form-data;boundary" in content_type and input_stream.tell() < 10000000 ): output_stream = io.BytesIO() From db7dc14b466a1fa73d0c3470aed1164c64f129b4 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Fri, 4 Aug 2023 08:35:54 +0200 Subject: [PATCH 03/38] optional body_stream --- docker-app/qfieldcloud/core/utils2/sentry.py | 7 ++++--- docker-app/qfieldcloud/core/views/files_views.py | 4 +++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docker-app/qfieldcloud/core/utils2/sentry.py b/docker-app/qfieldcloud/core/utils2/sentry.py index 1e72324e9..152329a31 100644 --- a/docker-app/qfieldcloud/core/utils2/sentry.py +++ b/docker-app/qfieldcloud/core/utils2/sentry.py @@ -11,7 +11,7 @@ def report_serialization_diff_to_sentry( pre_serialization: str, post_serialization: str, buffer: StringIO, - body_stream: BytesIO, + body_stream: BytesIO | None, capture_message=False, ) -> bool: """ @@ -42,8 +42,9 @@ def report_serialization_diff_to_sentry( filename=filename, ) - filename = f"{name}_rawbody.txt" - scope.add_attachment(bytes=body_stream.getvalue(), filename=filename) + if body_stream: + filename = f"{name}_rawbody.txt" + scope.add_attachment(bytes=body_stream.getvalue(), filename=filename) if capture_message: sentry_sdk.capture_message("Sending to Sentry...", scope=scope) diff --git a/docker-app/qfieldcloud/core/views/files_views.py b/docker-app/qfieldcloud/core/views/files_views.py index c72c783ff..74b0c3747 100644 --- a/docker-app/qfieldcloud/core/views/files_views.py +++ b/docker-app/qfieldcloud/core/views/files_views.py @@ -222,7 +222,9 @@ def post(self, request, projectid, filename, format=None): pre_serialization=request.attached_keys, post_serialization=str(request_attributes), buffer=buffer, - body_stream=request.body_stream, + body_stream=request.body_stream + if hasattr(request, "body_stream") + else None, ) raise exceptions.EmptyContentError() From ac0d51dd95845d3113a1c0672599d499b1f1697d Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Fri, 4 Aug 2023 09:32:47 +0200 Subject: [PATCH 04/38] updated unit test --- docker-app/qfieldcloud/core/tests/test_sentry.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/core/tests/test_sentry.py b/docker-app/qfieldcloud/core/tests/test_sentry.py index ed782c346..0771ac255 100644 --- a/docker-app/qfieldcloud/core/tests/test_sentry.py +++ b/docker-app/qfieldcloud/core/tests/test_sentry.py @@ -42,7 +42,8 @@ def test_logging_with_sentry(self): } ), "buffer": StringIO("The traceback of the exception to raise"), - "capture_message": True, # so that sentry receives attachments even when there's no exception/event + "capture_message": True, # so that sentry receives attachments even when there's no exception/event, + "body_stream": None } will_be_sent = report_serialization_diff_to_sentry(**mock_payload) self.assertTrue(will_be_sent) From 91c44e3a9c20a627fd54d861b269706172d78e13 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Fri, 4 Aug 2023 11:40:34 +0200 Subject: [PATCH 05/38] investigating failure of 1 test --- docker-app/qfieldcloud/core/middleware/requests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index 7473bd558..71210f058 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -10,7 +10,7 @@ def attach_keys(get_response): """ def middleware(request): - input_stream = io.BufferedReader(io.BytesIO(request.body)) + input_stream = io.BytesIO(request.body) request_attributes = { "file_key": str(request.FILES.keys()), @@ -27,6 +27,7 @@ def middleware(request): and "multipart/form-data;boundary" in content_type and input_stream.tell() < 10000000 ): + request_attributes["content_type"] = content_type output_stream = io.BytesIO() shutil.copyfileobj(input_stream, output_stream) request.body_stream = output_stream From 92afd5e11738946f85daae50289fe5696f45925b Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Fri, 4 Aug 2023 11:45:32 +0200 Subject: [PATCH 06/38] cleanup --- docker-app/qfieldcloud/core/middleware/requests.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index 71210f058..68f4acbb4 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -10,8 +10,6 @@ def attach_keys(get_response): """ def middleware(request): - input_stream = io.BytesIO(request.body) - request_attributes = { "file_key": str(request.FILES.keys()), "meta": str(request.META), @@ -21,13 +19,13 @@ def middleware(request): "content-type" ) - # only report rawbody of less-than-10MBs multipart requests + # only report raw body multipart requests if ( content_type and "multipart/form-data;boundary" in content_type - and input_stream.tell() < 10000000 ): request_attributes["content_type"] = content_type + input_stream = io.BytesIO(request.body) output_stream = io.BytesIO() shutil.copyfileobj(input_stream, output_stream) request.body_stream = output_stream From 2cd970bcb5c34b0a719dfee33a300586baf1c5a0 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Fri, 4 Aug 2023 11:45:32 +0200 Subject: [PATCH 07/38] cleanup --- docker-app/qfieldcloud/core/middleware/requests.py | 5 +---- docker-app/qfieldcloud/core/tests/test_sentry.py | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index 68f4acbb4..a9fd15ac0 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -20,10 +20,7 @@ def middleware(request): ) # only report raw body multipart requests - if ( - content_type - and "multipart/form-data;boundary" in content_type - ): + if content_type and "multipart/form-data;boundary" in content_type: request_attributes["content_type"] = content_type input_stream = io.BytesIO(request.body) output_stream = io.BytesIO() diff --git a/docker-app/qfieldcloud/core/tests/test_sentry.py b/docker-app/qfieldcloud/core/tests/test_sentry.py index 0771ac255..0a103fa7e 100644 --- a/docker-app/qfieldcloud/core/tests/test_sentry.py +++ b/docker-app/qfieldcloud/core/tests/test_sentry.py @@ -43,7 +43,7 @@ def test_logging_with_sentry(self): ), "buffer": StringIO("The traceback of the exception to raise"), "capture_message": True, # so that sentry receives attachments even when there's no exception/event, - "body_stream": None + "body_stream": None, } will_be_sent = report_serialization_diff_to_sentry(**mock_payload) self.assertTrue(will_be_sent) From 4fba42fcffa0a278e140c233dd2b0290cb0d7d91 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Fri, 4 Aug 2023 15:33:01 +0200 Subject: [PATCH 08/38] only report less than 10 MBs --- .../qfieldcloud/core/middleware/requests.py | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index a9fd15ac0..0d92ec2a8 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -1,33 +1,28 @@ import io import shutil +from typing import Any def attach_keys(get_response): """ QF-2540 - Annotate request with a str representation of relevant fields, so as to obtain a diff - by comparing with the post-serialized request later in the callstack. + Annotate request with: + - a `str` representation of relevant fields, so as to obtain a diff by comparing with the post-serialized request later in the callstack; + - a byte-for-byte, non stealing copy of the raw body to inspect multipart boundaries. """ - def middleware(request): + input_stream = io.BytesIO(request.body) + request_attributes = { "file_key": str(request.FILES.keys()), "meta": str(request.META), } - content_type = request.headers.get("Content-Type") or request.headers.get( - "content-type" - ) - - # only report raw body multipart requests - if content_type and "multipart/form-data;boundary" in content_type: - request_attributes["content_type"] = content_type - input_stream = io.BytesIO(request.body) + # only report raw body of POST requests with a < 10MBs content length + if request.META["REQUEST_METHOD"] == "POST" and int(request.META["CONTENT_LENGTH"]) < 10000000: output_stream = io.BytesIO() shutil.copyfileobj(input_stream, output_stream) request.body_stream = output_stream - - if "file" in request.FILES.keys(): request_attributes["files"] = request.FILES.getlist("file") request.attached_keys = str(request_attributes) From a3772c0a58acbf53ebc47a9d3387944404c373e2 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Fri, 4 Aug 2023 15:46:36 +0200 Subject: [PATCH 09/38] format, suggestion --- docker-app/qfieldcloud/core/middleware/requests.py | 9 ++++++--- docker-app/qfieldcloud/core/views/files_views.py | 4 +--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index 0d92ec2a8..ea9980440 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -1,15 +1,15 @@ import io import shutil -from typing import Any def attach_keys(get_response): """ QF-2540 - Annotate request with: + Annotate request with: - a `str` representation of relevant fields, so as to obtain a diff by comparing with the post-serialized request later in the callstack; - a byte-for-byte, non stealing copy of the raw body to inspect multipart boundaries. """ + def middleware(request): input_stream = io.BytesIO(request.body) @@ -19,7 +19,10 @@ def middleware(request): } # only report raw body of POST requests with a < 10MBs content length - if request.META["REQUEST_METHOD"] == "POST" and int(request.META["CONTENT_LENGTH"]) < 10000000: + if ( + request.META["REQUEST_METHOD"] == "POST" + and int(request.META["CONTENT_LENGTH"]) < 10000000 + ): output_stream = io.BytesIO() shutil.copyfileobj(input_stream, output_stream) request.body_stream = output_stream diff --git a/docker-app/qfieldcloud/core/views/files_views.py b/docker-app/qfieldcloud/core/views/files_views.py index 74b0c3747..14f6df520 100644 --- a/docker-app/qfieldcloud/core/views/files_views.py +++ b/docker-app/qfieldcloud/core/views/files_views.py @@ -222,9 +222,7 @@ def post(self, request, projectid, filename, format=None): pre_serialization=request.attached_keys, post_serialization=str(request_attributes), buffer=buffer, - body_stream=request.body_stream - if hasattr(request, "body_stream") - else None, + body_stream=getattr(request, "body_stream", None), ) raise exceptions.EmptyContentError() From c00e4e0c86d8d01360b7fb9939206da10246b848 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Sun, 6 Aug 2023 20:51:01 +0200 Subject: [PATCH 10/38] Subclassing MultiPartParser to report on missing file --- .../qfieldcloud/core/views/files_views.py | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/docker-app/qfieldcloud/core/views/files_views.py b/docker-app/qfieldcloud/core/views/files_views.py index 14f6df520..5010a12c2 100644 --- a/docker-app/qfieldcloud/core/views/files_views.py +++ b/docker-app/qfieldcloud/core/views/files_views.py @@ -21,7 +21,7 @@ ) from rest_framework import permissions, serializers, status, views from rest_framework.exceptions import NotFound -from rest_framework.parsers import MultiPartParser +from rest_framework.parsers import DataAndFiles, MultiPartParser from rest_framework.request import Request from rest_framework.response import Response @@ -141,6 +141,23 @@ def has_permission(self, request, view): return False +class QfcMultiPartSerializer(MultiPartParser): + + errors: list[str] = [] + + # QF-2540 + def parse(self, stream, media_type=None, parser_context=None) -> DataAndFiles: + """Substitute to MultiPartParser for debugging `EmptyContentError`""" + parsed: DataAndFiles = super().parse(stream, media_type, parser_context) + + if "file" not in parsed.files or not parsed.files["file"]: + self.errors.append( + f"QfcMultiPartParser was able to obtain `DataAndFiles` from the request's input stream, but `MultiValueDict` either lacks a `file` key or a value at `file`! parser_context: {parser_context}. `EmptyContentError` expected." + ) + + return parsed + + @extend_schema_view( get=extend_schema( description="Download a file from a project", @@ -165,7 +182,7 @@ def has_permission(self, request, view): class DownloadPushDeleteFileView(views.APIView): # TODO: swagger doc # TODO: docstring - parser_classes = [MultiPartParser] + parser_classes = [QfcMultiPartSerializer] permission_classes = [ permissions.IsAuthenticated, DownloadPushDeleteFileViewPermissions, @@ -220,7 +237,8 @@ def post(self, request, projectid, filename, format=None): # using the 'X-Request-Id' added to the request by RequestIDMiddleware name=f"{request.META.get('X-Request-Id')}_{projectid}", pre_serialization=request.attached_keys, - post_serialization=str(request_attributes), + post_serialization=",".join(QfcMultiPartSerializer.errors) + + str(request_attributes), buffer=buffer, body_stream=getattr(request, "body_stream", None), ) From f0cd69eba4f9c272f64731c6dd62cd864cbb4d8a Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Mon, 7 Aug 2023 09:03:03 +0200 Subject: [PATCH 11/38] only taking an initial chunk instead of the entire body --- docker-app/qfieldcloud/core/utils2/sentry.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/core/utils2/sentry.py b/docker-app/qfieldcloud/core/utils2/sentry.py index 152329a31..dbd5199d9 100644 --- a/docker-app/qfieldcloud/core/utils2/sentry.py +++ b/docker-app/qfieldcloud/core/utils2/sentry.py @@ -43,8 +43,9 @@ def report_serialization_diff_to_sentry( ) if body_stream: + initial_chunk = body_stream.read(500) filename = f"{name}_rawbody.txt" - scope.add_attachment(bytes=body_stream.getvalue(), filename=filename) + scope.add_attachment(bytes=initial_chunk, filename=filename) if capture_message: sentry_sdk.capture_message("Sending to Sentry...", scope=scope) From cae8c6dd17bef98e8a20551171a156faf5ea0e14 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Mon, 7 Aug 2023 11:56:19 +0200 Subject: [PATCH 12/38] fixed erroneous accessor to get 'CONTENT_TYPE` header --- docker-app/qfieldcloud/core/middleware/requests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index ea9980440..320920306 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -21,7 +21,7 @@ def middleware(request): # only report raw body of POST requests with a < 10MBs content length if ( request.META["REQUEST_METHOD"] == "POST" - and int(request.META["CONTENT_LENGTH"]) < 10000000 + and int(request.headers["CONTENT_LENGTH"]) < 10000000 ): output_stream = io.BytesIO() shutil.copyfileobj(input_stream, output_stream) From 0f6381b3f4d8706216953bacc09c7594806f4ceb Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Mon, 7 Aug 2023 12:28:58 +0200 Subject: [PATCH 13/38] improved the previous fix --- docker-app/qfieldcloud/core/middleware/requests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index 320920306..ecee75490 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -17,11 +17,11 @@ def middleware(request): "file_key": str(request.FILES.keys()), "meta": str(request.META), } - # only report raw body of POST requests with a < 10MBs content length if ( request.META["REQUEST_METHOD"] == "POST" - and int(request.headers["CONTENT_LENGTH"]) < 10000000 + and "Content-Length" in request.headers + and int(request.headers["Content-Length"]) < 10000000 ): output_stream = io.BytesIO() shutil.copyfileobj(input_stream, output_stream) From 70ce049696ddfe1ab775a4094d734d1fff58ff9a Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Mon, 7 Aug 2023 14:04:31 +0200 Subject: [PATCH 14/38] fixed ordering issue --- .../qfieldcloud/core/middleware/requests.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index ecee75490..7bb17297e 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -11,23 +11,22 @@ def attach_keys(get_response): """ def middleware(request): - input_stream = io.BytesIO(request.body) - - request_attributes = { - "file_key": str(request.FILES.keys()), - "meta": str(request.META), - } # only report raw body of POST requests with a < 10MBs content length if ( - request.META["REQUEST_METHOD"] == "POST" + request.method == "POST" and "Content-Length" in request.headers and int(request.headers["Content-Length"]) < 10000000 ): + input_stream = io.BytesIO(request.body) output_stream = io.BytesIO() shutil.copyfileobj(input_stream, output_stream) request.body_stream = output_stream - request_attributes["files"] = request.FILES.getlist("file") + request_attributes = { + "file_key": str(request.FILES.keys()), + "meta": str(request.META), + "files": request.FILES.getlist("file") + } request.attached_keys = str(request_attributes) response = get_response(request) return response From 33baf79098782e0e66cdb8ce3b0498aa7ffbf97c Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Mon, 7 Aug 2023 14:05:47 +0200 Subject: [PATCH 15/38] format --- docker-app/qfieldcloud/core/middleware/requests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index 7bb17297e..21560113e 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -25,7 +25,7 @@ def middleware(request): request_attributes = { "file_key": str(request.FILES.keys()), "meta": str(request.META), - "files": request.FILES.getlist("file") + "files": request.FILES.getlist("file"), } request.attached_keys = str(request_attributes) response = get_response(request) From 458263986653fecd6848ff65afb23ef6b9139cb6 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Tue, 8 Aug 2023 09:24:39 +0200 Subject: [PATCH 16/38] attach entire body , not just initial chunk --- docker-app/qfieldcloud/core/utils2/sentry.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docker-app/qfieldcloud/core/utils2/sentry.py b/docker-app/qfieldcloud/core/utils2/sentry.py index dbd5199d9..152329a31 100644 --- a/docker-app/qfieldcloud/core/utils2/sentry.py +++ b/docker-app/qfieldcloud/core/utils2/sentry.py @@ -43,9 +43,8 @@ def report_serialization_diff_to_sentry( ) if body_stream: - initial_chunk = body_stream.read(500) filename = f"{name}_rawbody.txt" - scope.add_attachment(bytes=initial_chunk, filename=filename) + scope.add_attachment(bytes=body_stream.getvalue(), filename=filename) if capture_message: sentry_sdk.capture_message("Sending to Sentry...", scope=scope) From 9002a6c3b7fa0dcdc9babb27a864c2fd4367f664 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Tue, 8 Aug 2023 09:42:51 +0200 Subject: [PATCH 17/38] added flag to control whether the full budy should be sent to Sentry --- docker-app/qfieldcloud/core/utils2/sentry.py | 3 ++- docker-app/qfieldcloud/settings.py | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/core/utils2/sentry.py b/docker-app/qfieldcloud/core/utils2/sentry.py index 152329a31..f6651e835 100644 --- a/docker-app/qfieldcloud/core/utils2/sentry.py +++ b/docker-app/qfieldcloud/core/utils2/sentry.py @@ -2,6 +2,7 @@ from io import BytesIO, StringIO import sentry_sdk +from django.conf import settings logger = logging.getLogger(__name__) @@ -42,7 +43,7 @@ def report_serialization_diff_to_sentry( filename=filename, ) - if body_stream: + if body_stream and settings.SENTRY_REPORT_FULL_BODY: filename = f"{name}_rawbody.txt" scope.add_attachment(bytes=body_stream.getvalue(), filename=filename) diff --git a/docker-app/qfieldcloud/settings.py b/docker-app/qfieldcloud/settings.py index ed92b7ef5..c9016c58a 100644 --- a/docker-app/qfieldcloud/settings.py +++ b/docker-app/qfieldcloud/settings.py @@ -320,6 +320,10 @@ def before_send(event, hint): # environment=ENVIRONMENT, ) +# QF-2704 +# Flag to turn on/off byte-for-byte copy of request's body +# Only requests with a < 10MB body will be reported +SENTRY_REPORT_FULL_BODY = True # Django allauth configurations # https://django-allauth.readthedocs.io/en/latest/configuration.html From aecaef1d32634706ad1079e2c9e8b5f85e395d27 Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Tue, 8 Aug 2023 10:05:12 +0200 Subject: [PATCH 18/38] adjusted unit test to make use of the new body_stream --- .../qfieldcloud/core/tests/test_sentry.py | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/docker-app/qfieldcloud/core/tests/test_sentry.py b/docker-app/qfieldcloud/core/tests/test_sentry.py index 0a103fa7e..518be3cf4 100644 --- a/docker-app/qfieldcloud/core/tests/test_sentry.py +++ b/docker-app/qfieldcloud/core/tests/test_sentry.py @@ -1,13 +1,29 @@ -from io import StringIO +import shutil +from io import BytesIO, StringIO from os import environ from unittest import skipIf -from rest_framework.test import APITestCase +from django.test import Client, TestCase from ..utils2.sentry import report_serialization_diff_to_sentry -class QfcTestCase(APITestCase): +class QfcTestCase(TestCase): + @classmethod + def setUpClass(cls) -> None: + super().setUpClass() + # Let's set up a WSGI request the body of which we'll extract + response = Client().post( + "test123", + data={"file": BytesIO(b"Hello World")}, + format="multipart", + ) + request = response.wsgi_request + input_stream = BytesIO(request.body) + output_stream = BytesIO() + shutil.copyfileobj(input_stream, output_stream) + cls.body_stream = output_stream + @skipIf( environ.get("SENTRY_DSN", False), "Do not run this test when Sentry's DSN is not set.", @@ -42,8 +58,8 @@ def test_logging_with_sentry(self): } ), "buffer": StringIO("The traceback of the exception to raise"), + "body_stream": self.body_stream, "capture_message": True, # so that sentry receives attachments even when there's no exception/event, - "body_stream": None, } will_be_sent = report_serialization_diff_to_sentry(**mock_payload) self.assertTrue(will_be_sent) From a0c2686f16c5d9c92478bb120e07595571bb43fc Mon Sep 17 00:00:00 2001 From: why-not-try-calmer Date: Tue, 8 Aug 2023 10:26:41 +0200 Subject: [PATCH 19/38] better use of settings and better phrasing of test skip condition --- docker-app/qfieldcloud/core/tests/test_sentry.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docker-app/qfieldcloud/core/tests/test_sentry.py b/docker-app/qfieldcloud/core/tests/test_sentry.py index 518be3cf4..60595ad36 100644 --- a/docker-app/qfieldcloud/core/tests/test_sentry.py +++ b/docker-app/qfieldcloud/core/tests/test_sentry.py @@ -1,12 +1,14 @@ import shutil from io import BytesIO, StringIO -from os import environ from unittest import skipIf +from django.conf import settings from django.test import Client, TestCase from ..utils2.sentry import report_serialization_diff_to_sentry +sentry_is_configured = all([settings.SENTRY_DSN, settings.SENTRY_REPORT_FULL_BODY]) + class QfcTestCase(TestCase): @classmethod @@ -25,7 +27,7 @@ def setUpClass(cls) -> None: cls.body_stream = output_stream @skipIf( - environ.get("SENTRY_DSN", False), + not sentry_is_configured, "Do not run this test when Sentry's DSN is not set.", ) def test_logging_with_sentry(self): @@ -63,3 +65,4 @@ def test_logging_with_sentry(self): } will_be_sent = report_serialization_diff_to_sentry(**mock_payload) self.assertTrue(will_be_sent) + print(f"Body sent to Sentry: {self.body_stream.getvalue()}") From a8682778b84900224d92eb7ec5cc7cf805046f7f Mon Sep 17 00:00:00 2001 From: faebebin Date: Tue, 15 Aug 2023 15:20:57 +0200 Subject: [PATCH 20/38] Cleanup @deprecated that affect only tests --- docker-app/qfieldcloud/core/exceptions.py | 10 -- docker-app/qfieldcloud/core/models.py | 20 --- .../qfieldcloud/core/permissions_utils.py | 69 ---------- docker-app/qfieldcloud/subscription/models.py | 9 -- .../subscription/tests/test_package.py | 124 +++++++++--------- 5 files changed, 62 insertions(+), 170 deletions(-) diff --git a/docker-app/qfieldcloud/core/exceptions.py b/docker-app/qfieldcloud/core/exceptions.py index 70a7e7758..d27b5421d 100644 --- a/docker-app/qfieldcloud/core/exceptions.py +++ b/docker-app/qfieldcloud/core/exceptions.py @@ -1,4 +1,3 @@ -from deprecated import deprecated from rest_framework import status @@ -177,12 +176,3 @@ class ProjectAlreadyExistsError(QFieldCloudException): code = "project_already_exists" message = "This user already owns a project with the same name." status_code = status.HTTP_400_BAD_REQUEST - - -@deprecated("moved to subscription") -class ReachedMaxOrganizationMembersError(QFieldCloudException): - """Raised when an organization has exhausted its quota of members""" - - code = "organization_has_max_number_of_members" - message = "Cannot add new organization members, account limit has been reached." - status_code = status.HTTP_403_FORBIDDEN diff --git a/docker-app/qfieldcloud/core/models.py b/docker-app/qfieldcloud/core/models.py index 78a722161..efd03cf5d 100644 --- a/docker-app/qfieldcloud/core/models.py +++ b/docker-app/qfieldcloud/core/models.py @@ -415,11 +415,6 @@ def current_subscription(self): Subscription = get_subscription_model() return Subscription.get_or_create_current_subscription(self) - @property - @deprecated("Use `current_subscription` instead") - def active_subscription(self): - return self.current_subscription() - @property def upcoming_subscription(self): from qfieldcloud.subscription.models import get_subscription_model @@ -437,13 +432,6 @@ def avatar_url(self): else: return None - @property - @deprecated("Use `UserAccount().storage_used_bytes` instead") - # TODO delete this method after refactoring tests so it's no longer used there - def storage_used_mb(self) -> float: - """Returns the storage used in MB""" - return self.storage_used_bytes / 1000 / 1000 - @property def storage_used_bytes(self) -> float: """Returns the storage used in bytes""" @@ -457,14 +445,6 @@ def storage_used_bytes(self) -> float: return used_quota - @property - @deprecated("Use `UserAccount().storage_free_bytes` instead") - # TODO delete this method after refactoring tests so it's no longer used there - def storage_free_mb(self) -> float: - """Returns the storage quota left in MB (quota from account and packages minus storage of all owned projects)""" - - return self.storage_free_bytes / 1000 / 1000 - @property def storage_free_bytes(self) -> float: """Returns the storage quota left in bytes (quota from account and packages minus storage of all owned projects)""" diff --git a/docker-app/qfieldcloud/core/permissions_utils.py b/docker-app/qfieldcloud/core/permissions_utils.py index 1b704cf6d..b79479677 100644 --- a/docker-app/qfieldcloud/core/permissions_utils.py +++ b/docker-app/qfieldcloud/core/permissions_utils.py @@ -1,6 +1,5 @@ from typing import List, Literal, Union -from deprecated import deprecated from django.utils.translation import gettext as _ from qfieldcloud.authentication.models import AuthToken from qfieldcloud.core.models import ( @@ -336,33 +335,6 @@ def can_apply_pending_deltas_for_project(user: QfcUser, project: Project) -> boo ) -@deprecated("Use `can_set_delta_status_for_project` instead") -def can_apply_deltas(user: QfcUser, project: Project) -> bool: - return user_has_project_roles( - user, - project, - [ - ProjectCollaborator.Roles.ADMIN, - ProjectCollaborator.Roles.MANAGER, - ProjectCollaborator.Roles.EDITOR, - ProjectCollaborator.Roles.REPORTER, - ], - ) - - -@deprecated("Use `can_set_delta_status_for_project` instead") -def can_overwrite_deltas(user: QfcUser, project: Project) -> bool: - return user_has_project_roles( - user, - project, - [ - ProjectCollaborator.Roles.ADMIN, - ProjectCollaborator.Roles.MANAGER, - ProjectCollaborator.Roles.EDITOR, - ], - ) - - def can_set_delta_status_for_project(user: QfcUser, project: Project) -> bool: return user_has_project_roles( user, @@ -414,47 +386,6 @@ def can_create_delta(user: QfcUser, delta: Delta) -> bool: return False -@deprecated("Use `can_set_delta_status` instead") -def can_retry_delta(user: QfcUser, delta: Delta) -> bool: - if not can_apply_deltas(user, delta.project): - return False - - if delta.last_status not in ( - Delta.Status.CONFLICT, - Delta.Status.NOT_APPLIED, - Delta.Status.ERROR, - ): - return False - - return True - - -@deprecated("Use `can_set_delta_status` instead") -def can_overwrite_delta(user: QfcUser, delta: Delta) -> bool: - if not can_overwrite_deltas(user, delta.project): - return False - - if delta.last_status not in (Delta.Status.CONFLICT): - return False - - return True - - -@deprecated("Use `can_set_delta_status` instead") -def can_ignore_delta(user: QfcUser, delta: Delta) -> bool: - if not can_apply_deltas(user, delta.project): - return False - - if delta.last_status not in ( - Delta.Status.CONFLICT, - Delta.Status.NOT_APPLIED, - Delta.Status.ERROR, - ): - return False - - return True - - def can_read_jobs(user: QfcUser, project: Project) -> bool: return user_has_project_roles( user, diff --git a/docker-app/qfieldcloud/subscription/models.py b/docker-app/qfieldcloud/subscription/models.py index fd3f3c7c3..2386c22fb 100644 --- a/docker-app/qfieldcloud/subscription/models.py +++ b/docker-app/qfieldcloud/subscription/models.py @@ -368,10 +368,6 @@ def current(self): return qs - @deprecated("Use `current` instead. Remove this once parent repo uses current") - def active(self): - return self.current() - def managed_by(self, user_id: int): """Returns all subscriptions that are managed by given `user_id`. It means the owner personal account and all organizations they own. @@ -698,11 +694,6 @@ def get_or_create_current_subscription(cls, account: UserAccount) -> "Subscripti return subscription - @property - @deprecated("Use `get_or_create_current_subscription` instead") - def get_or_create_active_subscription(cls, account: UserAccount) -> "Subscription": - return cls.get_or_create_current_subscription(account) - @classmethod def get_upcoming_subscription(cls, account: UserAccount) -> "Subscription": result = ( diff --git a/docker-app/qfieldcloud/subscription/tests/test_package.py b/docker-app/qfieldcloud/subscription/tests/test_package.py index 0aa23f102..133dc170e 100644 --- a/docker-app/qfieldcloud/subscription/tests/test_package.py +++ b/docker-app/qfieldcloud/subscription/tests/test_package.py @@ -64,8 +64,8 @@ def assertStorage( future_storage_package_quantity, future_storage_package_mb, future_storage_package_changed_mb, - storage_used_mb, - storage_free_mb, + storage_used_bytes, + storage_free_bytes, user=None, ): if user is None: @@ -107,8 +107,8 @@ def assertStorage( user.useraccount.current_subscription.future_storage_package_changed_mb, future_storage_package_changed_mb, ) - self.assertEqual(user.useraccount.storage_used_mb, storage_used_mb) - self.assertEqual(user.useraccount.storage_free_mb, storage_free_mb) + self.assertEqual(user.useraccount.storage_used_bytes, storage_used_bytes) + self.assertEqual(user.useraccount.storage_free_bytes, storage_free_bytes) def test_get_storage_package_type(self): PackageType.objects.all().delete() @@ -139,8 +139,8 @@ def test_storage_on_default_plan(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=self.plan_default.storage_mb, + storage_used_bytes=0, + storage_free_bytes=self.plan_default.storage_mb * 1000 * 1000, ) def test_default_plan_raises_when_adding_active_storage(self): @@ -163,8 +163,8 @@ def test_default_plan_raises_when_adding_active_storage(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=self.plan_default.storage_mb, + storage_used_bytes=0, + storage_free_bytes=self.plan_default.storage_mb * 1000 * 1000, ) def test_default_plan_ignores_active_package(self): @@ -190,8 +190,8 @@ def test_default_plan_ignores_active_package(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=self.plan_default.storage_mb, + storage_used_bytes=0, + storage_free_bytes=self.plan_default.storage_mb * 1000 * 1000, ) def test_premium_plan_without_additional_storage(self): @@ -205,8 +205,8 @@ def test_premium_plan_without_additional_storage(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=1, + storage_used_bytes=0, + storage_free_bytes=1 * 1000 * 1000, ) def test_premium_plan_with_active_storage(self): @@ -228,8 +228,8 @@ def test_premium_plan_with_active_storage(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=-1000, - storage_used_mb=0, - storage_free_mb=1001, + storage_used_bytes=0, + storage_free_bytes=1001 * 1000 * 1000, ) def test_premium_plan_with_active_storage_without_end_date(self): @@ -251,8 +251,8 @@ def test_premium_plan_with_active_storage_without_end_date(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=1001, + storage_used_bytes=0, + storage_free_bytes=1001 * 1000 * 1000, ) def test_premium_plan_with_expired_storage(self): @@ -274,8 +274,8 @@ def test_premium_plan_with_expired_storage(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=1, + storage_used_bytes=0, + storage_free_bytes=1 * 1000 * 1000, ) def test_premium_plan_with_future_storage(self): @@ -297,8 +297,8 @@ def test_premium_plan_with_future_storage(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=1000, - storage_used_mb=0, - storage_free_mb=1, + storage_used_bytes=0, + storage_free_bytes=1 * 1000 * 1000, ) def test_premium_plan_with_active_and_future_and_expired_storage(self): @@ -335,8 +335,8 @@ def test_premium_plan_with_active_and_future_and_expired_storage(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=1001, + storage_used_bytes=0, + storage_free_bytes=1001 * 1000 * 1000, ) def test_premium_plan_can_add_active_storage(self): @@ -354,8 +354,8 @@ def test_premium_plan_can_add_active_storage(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=1001, + storage_used_bytes=0, + storage_free_bytes=1001 * 1000 * 1000, ) def test_premium_plan_can_add_active_storage_with_quantity_of_42(self): @@ -373,8 +373,8 @@ def test_premium_plan_can_add_active_storage_with_quantity_of_42(self): future_storage_package_quantity=42, future_storage_package_mb=42000, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=42001, + storage_used_bytes=0, + storage_free_bytes=42001 * 1000 * 1000, ) def test_premium_plan_can_add_future_storage(self): @@ -394,8 +394,8 @@ def test_premium_plan_can_add_future_storage(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=1000, - storage_used_mb=0, - storage_free_mb=1, + storage_used_bytes=0, + storage_free_bytes=1 * 1000 * 1000, ) def test_premium_plan_can_replace_active_storage(self): @@ -417,8 +417,8 @@ def test_premium_plan_can_replace_active_storage(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=1001, + storage_used_bytes=0, + storage_free_bytes=1001 * 1000 * 1000, ) ( @@ -443,8 +443,8 @@ def test_premium_plan_can_replace_active_storage(self): future_storage_package_quantity=2, future_storage_package_mb=2000, future_storage_package_changed_mb=1000, - storage_used_mb=0, - storage_free_mb=1001, + storage_used_bytes=0, + storage_free_bytes=1001 * 1000 * 1000, ) def test_storage_with_custom_additional_storage(self): @@ -475,8 +475,8 @@ def test_storage_with_custom_additional_storage(self): future_storage_package_quantity=1, future_storage_package_mb=2, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=3, + storage_used_bytes=0, + storage_free_bytes=3 * 1000 * 1000, ) def test_storage_cannot_have_packages_with_time_overlaps(self): @@ -499,8 +499,8 @@ def test_storage_cannot_have_packages_with_time_overlaps(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=-1000, - storage_used_mb=0, - storage_free_mb=1001, + storage_used_bytes=0, + storage_free_bytes=1001 * 1000 * 1000, ) with self.assertRaises(django.db.utils.IntegrityError): @@ -523,8 +523,8 @@ def test_storage_cannot_have_packages_with_time_overlaps(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=-1000, - storage_used_mb=0, - storage_free_mb=1001, + storage_used_bytes=0, + storage_free_bytes=1001 * 1000 * 1000, ) def test_storage_cannot_add_new_package_when_active_until_is_null(self): @@ -546,8 +546,8 @@ def test_storage_cannot_add_new_package_when_active_until_is_null(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=1001, + storage_used_bytes=0, + storage_free_bytes=1001 * 1000 * 1000, ) with self.assertRaises(django.db.utils.IntegrityError): @@ -569,8 +569,8 @@ def test_storage_cannot_add_new_package_when_active_until_is_null(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=1001, + storage_used_bytes=0, + storage_free_bytes=1001 * 1000 * 1000, ) def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(self): @@ -584,8 +584,8 @@ def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(sel future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=1, + storage_used_bytes=0, + storage_free_bytes=1 * 1000 * 1000, ) p1 = Project.objects.create(name="p1", owner=self.u1) @@ -604,8 +604,8 @@ def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(sel future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0.3, - storage_free_mb=0.7, + storage_used_bytes=0.3 * 1000 * 1000, + storage_free_bytes=0.7 * 1000 * 1000, ) bucket.upload_fileobj(get_random_file(mb=0.1), storage_path) @@ -621,8 +621,8 @@ def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(sel future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0.4, - storage_free_mb=0.6, + storage_used_bytes=0.4 * 1000 * 1000, + storage_free_bytes=0.6 * 1000 * 1000, ) version = list(list_versions(bucket, storage_path))[0] @@ -639,8 +639,8 @@ def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(sel future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0.1, - storage_free_mb=0.9, + storage_used_bytes=0.1 * 1000 * 1000, + storage_free_bytes=0.9 * 1000 * 1000, ) delete_project_file_permanently(p1, "test.data") @@ -656,8 +656,8 @@ def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(sel future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=1, + storage_used_bytes=0 * 1000 * 1000, + storage_free_bytes=1 * 1000 * 1000, ) def test_api_enforces_storage_limit(self): @@ -711,8 +711,8 @@ def test_api_enforces_storage_limit_when_owner_changes(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=10, + storage_used_bytes=0, + storage_free_bytes=10 * 1000 * 1000, user=u10mb, ) self.assertStorage( @@ -725,8 +725,8 @@ def test_api_enforces_storage_limit_when_owner_changes(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=20, + storage_used_bytes=0, + storage_free_bytes=20 * 1000 * 1000, user=u20mb, ) @@ -749,8 +749,8 @@ def test_api_enforces_storage_limit_when_owner_changes(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_mb=15, - storage_free_mb=5, + storage_used_bytes=15 * 1000 * 1000, + storage_free_bytes=5 * 1000 * 1000, user=u20mb, ) @@ -779,8 +779,8 @@ def test_api_enforces_storage_limit_when_owner_changes(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_mb=0, - storage_free_mb=1010, + storage_used_bytes=0, + storage_free_bytes=1010 * 1000 * 1000, user=u10mb, ) @@ -799,7 +799,7 @@ def test_api_enforces_storage_limit_when_owner_changes(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_mb=15, - storage_free_mb=995, + storage_used_bytes=15 * 1000 * 1000, + storage_free_bytes=995 * 1000 * 1000, user=u10mb, ) From a1e7d8e54b43661a7551162c5663d348f117e6db Mon Sep 17 00:00:00 2001 From: faebebin Date: Wed, 16 Aug 2023 07:44:26 +0200 Subject: [PATCH 21/38] Fix settings link in notifications email --- docker-app/qfieldcloud/notifs/cron.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/notifs/cron.py b/docker-app/qfieldcloud/notifs/cron.py index d3c022806..b73394260 100644 --- a/docker-app/qfieldcloud/notifs/cron.py +++ b/docker-app/qfieldcloud/notifs/cron.py @@ -1,4 +1,5 @@ import logging +import os from django.conf import settings from django.core.mail import send_mail @@ -43,12 +44,14 @@ def do(self): logging.warning(f"{user} has notifications, but no email set !") continue + QFIELDCLOUD_HOST = os.environ.get("QFIELDCLOUD_HOST", None) + logging.debug(f"Sending an email to {user} !") context = { "notifs": notifs, "username": user.username, - "hostname": settings.ALLOWED_HOSTS[0], + "hostname": QFIELDCLOUD_HOST, } subject = render_to_string( From 69ac0e9dcc2b8d9971af99acbc211fee1ec2478d Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Wed, 16 Aug 2023 13:41:30 +0300 Subject: [PATCH 22/38] Retire useless print --- docker-app/qfieldcloud/core/tests/test_sentry.py | 1 - 1 file changed, 1 deletion(-) diff --git a/docker-app/qfieldcloud/core/tests/test_sentry.py b/docker-app/qfieldcloud/core/tests/test_sentry.py index 60595ad36..de787acef 100644 --- a/docker-app/qfieldcloud/core/tests/test_sentry.py +++ b/docker-app/qfieldcloud/core/tests/test_sentry.py @@ -65,4 +65,3 @@ def test_logging_with_sentry(self): } will_be_sent = report_serialization_diff_to_sentry(**mock_payload) self.assertTrue(will_be_sent) - print(f"Body sent to Sentry: {self.body_stream.getvalue()}") From 3ece691ca2d5ca859e7a0c387f4714327dcd25e2 Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Wed, 16 Aug 2023 13:41:52 +0300 Subject: [PATCH 23/38] Simpler inline check whether to run the test or not --- docker-app/qfieldcloud/core/tests/test_sentry.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docker-app/qfieldcloud/core/tests/test_sentry.py b/docker-app/qfieldcloud/core/tests/test_sentry.py index de787acef..b12613f03 100644 --- a/docker-app/qfieldcloud/core/tests/test_sentry.py +++ b/docker-app/qfieldcloud/core/tests/test_sentry.py @@ -7,8 +7,6 @@ from ..utils2.sentry import report_serialization_diff_to_sentry -sentry_is_configured = all([settings.SENTRY_DSN, settings.SENTRY_REPORT_FULL_BODY]) - class QfcTestCase(TestCase): @classmethod @@ -27,7 +25,7 @@ def setUpClass(cls) -> None: cls.body_stream = output_stream @skipIf( - not sentry_is_configured, + not settings.SENTRY_DSN, "Do not run this test when Sentry's DSN is not set.", ) def test_logging_with_sentry(self): From 989c7b66db65f09ff9ad17dc400aef8548922e9c Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Wed, 16 Aug 2023 13:42:33 +0300 Subject: [PATCH 24/38] Remove obsolete copy of the already copied bytes stream --- docker-app/qfieldcloud/core/tests/test_sentry.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/docker-app/qfieldcloud/core/tests/test_sentry.py b/docker-app/qfieldcloud/core/tests/test_sentry.py index b12613f03..512fa999e 100644 --- a/docker-app/qfieldcloud/core/tests/test_sentry.py +++ b/docker-app/qfieldcloud/core/tests/test_sentry.py @@ -1,4 +1,3 @@ -import shutil from io import BytesIO, StringIO from unittest import skipIf @@ -19,10 +18,7 @@ def setUpClass(cls) -> None: format="multipart", ) request = response.wsgi_request - input_stream = BytesIO(request.body) - output_stream = BytesIO() - shutil.copyfileobj(input_stream, output_stream) - cls.body_stream = output_stream + cls.body_stream = BytesIO(request.read()) @skipIf( not settings.SENTRY_DSN, From 6f689f833c1e7c84a67c62beee3da2c3d5f82c6a Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Wed, 16 Aug 2023 13:44:06 +0300 Subject: [PATCH 25/38] Add DEBUG setting in `constance` to control whether to send the request body to Sentry --- docker-app/qfieldcloud/core/middleware/requests.py | 5 ++++- docker-app/qfieldcloud/settings.py | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index 21560113e..d8d8623a1 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -1,6 +1,8 @@ import io import shutil +from constance import config + def attach_keys(get_response): """ @@ -15,7 +17,8 @@ def middleware(request): if ( request.method == "POST" and "Content-Length" in request.headers - and int(request.headers["Content-Length"]) < 10000000 + and int(request.headers["Content-Length"]) + < config.SENTRY_REQUEST_MAX_SIZE_TO_SEND ): input_stream = io.BytesIO(request.body) output_stream = io.BytesIO() diff --git a/docker-app/qfieldcloud/settings.py b/docker-app/qfieldcloud/settings.py index c9016c58a..9f543ed97 100644 --- a/docker-app/qfieldcloud/settings.py +++ b/docker-app/qfieldcloud/settings.py @@ -461,6 +461,10 @@ def before_send(event, hint): "1000m", "Maximum memory for each QGIS worker container.", ), + "SENTRY_REQUEST_MAX_SIZE_TO_SEND": ( + 0, + "Maximum request size to send the raw request to Sentry. Value 0 disables the raw request copy.", + ), "WORKER_QGIS_CPU_SHARES": ( 512, "Share of CPUs for each QGIS worker container. By default all containers have value 1024 set by docker.", @@ -481,6 +485,7 @@ def before_send(event, hint): "WORKER_QGIS_MEMORY_LIMIT", "WORKER_QGIS_CPU_SHARES", ), + "Debug": ("SENTRY_REQUEST_MAX_SIZE_TO_SEND",), "Subscription": ("TRIAL_PERIOD_DAYS",), } From 42fb8e43998a06aab4cd531a7fb72c8bee98e56e Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Wed, 16 Aug 2023 17:20:28 +0300 Subject: [PATCH 26/38] Make sure the SENTRY_DSN is configured when making a copy and add some logging --- docker-app/qfieldcloud/core/middleware/requests.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index d8d8623a1..4c7571433 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -1,7 +1,11 @@ import io +import logging import shutil from constance import config +from django.conf import settings + +logger = logging.getLogger(__name__) def attach_keys(get_response): @@ -17,9 +21,14 @@ def middleware(request): if ( request.method == "POST" and "Content-Length" in request.headers - and int(request.headers["Content-Length"]) - < config.SENTRY_REQUEST_MAX_SIZE_TO_SEND + and ( + int(request.headers["Content-Length"]) + < config.SENTRY_REQUEST_MAX_SIZE_TO_SEND + ) + and settings.SENTRY_DSN ): + logger.info("Making a temporary copy for request body.") + input_stream = io.BytesIO(request.body) output_stream = io.BytesIO() shutil.copyfileobj(input_stream, output_stream) From 5b9f3ecb377b2e62129e275538437283581e0c86 Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Wed, 16 Aug 2023 17:22:12 +0300 Subject: [PATCH 27/38] This setting has been long dropped --- docker-app/qfieldcloud/core/utils2/sentry.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docker-app/qfieldcloud/core/utils2/sentry.py b/docker-app/qfieldcloud/core/utils2/sentry.py index f6651e835..152329a31 100644 --- a/docker-app/qfieldcloud/core/utils2/sentry.py +++ b/docker-app/qfieldcloud/core/utils2/sentry.py @@ -2,7 +2,6 @@ from io import BytesIO, StringIO import sentry_sdk -from django.conf import settings logger = logging.getLogger(__name__) @@ -43,7 +42,7 @@ def report_serialization_diff_to_sentry( filename=filename, ) - if body_stream and settings.SENTRY_REPORT_FULL_BODY: + if body_stream: filename = f"{name}_rawbody.txt" scope.add_attachment(bytes=body_stream.getvalue(), filename=filename) From a5bd5047f276fd8cfc250773aff2914b9ee6254c Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Wed, 16 Aug 2023 17:22:42 +0300 Subject: [PATCH 28/38] Improve logging and sentry message --- docker-app/qfieldcloud/core/utils2/sentry.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docker-app/qfieldcloud/core/utils2/sentry.py b/docker-app/qfieldcloud/core/utils2/sentry.py index 152329a31..707e0078b 100644 --- a/docker-app/qfieldcloud/core/utils2/sentry.py +++ b/docker-app/qfieldcloud/core/utils2/sentry.py @@ -26,6 +26,7 @@ def report_serialization_diff_to_sentry( """ with sentry_sdk.configure_scope() as scope: try: + logger.info("Sending explicit sentry report!") filename = f"{name}_contents.txt" scope.add_attachment( @@ -47,10 +48,10 @@ def report_serialization_diff_to_sentry( scope.add_attachment(bytes=body_stream.getvalue(), filename=filename) if capture_message: - sentry_sdk.capture_message("Sending to Sentry...", scope=scope) + sentry_sdk.capture_message("Explicit Sentry report!", scope=scope) return True except Exception as error: + logger.error(f"Unable to send file to Sentry: failed on {error}") sentry_sdk.capture_exception(error) - logging.error(f"Unable to send file to Sentry: failed on {error}") return False From 5ba29ade75f19712744539375e20aa3dc5590b38 Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Thu, 17 Aug 2023 11:26:42 +0300 Subject: [PATCH 29/38] Update comment and reorder the `if` conditions --- docker-app/qfieldcloud/core/middleware/requests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docker-app/qfieldcloud/core/middleware/requests.py b/docker-app/qfieldcloud/core/middleware/requests.py index 4c7571433..7a29f2f9c 100644 --- a/docker-app/qfieldcloud/core/middleware/requests.py +++ b/docker-app/qfieldcloud/core/middleware/requests.py @@ -17,15 +17,15 @@ def attach_keys(get_response): """ def middleware(request): - # only report raw body of POST requests with a < 10MBs content length + # add a copy of the request body to the request if ( - request.method == "POST" + settings.SENTRY_DSN + and request.method == "POST" and "Content-Length" in request.headers and ( int(request.headers["Content-Length"]) < config.SENTRY_REQUEST_MAX_SIZE_TO_SEND ) - and settings.SENTRY_DSN ): logger.info("Making a temporary copy for request body.") From 7daa94b40d2e66e747ff73cc1bf737e21185f1dc Mon Sep 17 00:00:00 2001 From: Fabian Binder Date: Thu, 17 Aug 2023 12:36:17 +0200 Subject: [PATCH 30/38] Remove obsolete default None Co-authored-by: Adrien --- docker-app/qfieldcloud/notifs/cron.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/notifs/cron.py b/docker-app/qfieldcloud/notifs/cron.py index b73394260..baeaedcce 100644 --- a/docker-app/qfieldcloud/notifs/cron.py +++ b/docker-app/qfieldcloud/notifs/cron.py @@ -44,7 +44,7 @@ def do(self): logging.warning(f"{user} has notifications, but no email set !") continue - QFIELDCLOUD_HOST = os.environ.get("QFIELDCLOUD_HOST", None) + QFIELDCLOUD_HOST = os.environ.get("QFIELDCLOUD_HOST") logging.debug(f"Sending an email to {user} !") From c30325f2e4b001617311864b20b6e6f889847d18 Mon Sep 17 00:00:00 2001 From: faebebin Date: Thu, 17 Aug 2023 12:53:58 +0200 Subject: [PATCH 31/38] Use sotrage_used_mb instead in test --- .../subscription/tests/test_package.py | 124 +++++++++--------- 1 file changed, 62 insertions(+), 62 deletions(-) diff --git a/docker-app/qfieldcloud/subscription/tests/test_package.py b/docker-app/qfieldcloud/subscription/tests/test_package.py index 133dc170e..9f7022bfc 100644 --- a/docker-app/qfieldcloud/subscription/tests/test_package.py +++ b/docker-app/qfieldcloud/subscription/tests/test_package.py @@ -64,8 +64,8 @@ def assertStorage( future_storage_package_quantity, future_storage_package_mb, future_storage_package_changed_mb, - storage_used_bytes, - storage_free_bytes, + storage_used_mb, + storage_free_mb, user=None, ): if user is None: @@ -107,8 +107,8 @@ def assertStorage( user.useraccount.current_subscription.future_storage_package_changed_mb, future_storage_package_changed_mb, ) - self.assertEqual(user.useraccount.storage_used_bytes, storage_used_bytes) - self.assertEqual(user.useraccount.storage_free_bytes, storage_free_bytes) + self.assertEqual(user.useraccount.storage_used_bytes, storage_used_mb * 1000 * 1000) + self.assertEqual(user.useraccount.storage_free_bytes, storage_free_mb * 1000 * 1000) def test_get_storage_package_type(self): PackageType.objects.all().delete() @@ -139,8 +139,8 @@ def test_storage_on_default_plan(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=self.plan_default.storage_mb * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=self.plan_default.storage_mb, ) def test_default_plan_raises_when_adding_active_storage(self): @@ -163,8 +163,8 @@ def test_default_plan_raises_when_adding_active_storage(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=self.plan_default.storage_mb * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=self.plan_default.storage_mb, ) def test_default_plan_ignores_active_package(self): @@ -190,8 +190,8 @@ def test_default_plan_ignores_active_package(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=self.plan_default.storage_mb * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=self.plan_default.storage_mb, ) def test_premium_plan_without_additional_storage(self): @@ -205,8 +205,8 @@ def test_premium_plan_without_additional_storage(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=1 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1, ) def test_premium_plan_with_active_storage(self): @@ -228,8 +228,8 @@ def test_premium_plan_with_active_storage(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=-1000, - storage_used_bytes=0, - storage_free_bytes=1001 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1001, ) def test_premium_plan_with_active_storage_without_end_date(self): @@ -251,8 +251,8 @@ def test_premium_plan_with_active_storage_without_end_date(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=1001 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1001, ) def test_premium_plan_with_expired_storage(self): @@ -274,8 +274,8 @@ def test_premium_plan_with_expired_storage(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=1 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1, ) def test_premium_plan_with_future_storage(self): @@ -297,8 +297,8 @@ def test_premium_plan_with_future_storage(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=1000, - storage_used_bytes=0, - storage_free_bytes=1 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1, ) def test_premium_plan_with_active_and_future_and_expired_storage(self): @@ -335,8 +335,8 @@ def test_premium_plan_with_active_and_future_and_expired_storage(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=1001 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1001, ) def test_premium_plan_can_add_active_storage(self): @@ -354,8 +354,8 @@ def test_premium_plan_can_add_active_storage(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=1001 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1001, ) def test_premium_plan_can_add_active_storage_with_quantity_of_42(self): @@ -373,8 +373,8 @@ def test_premium_plan_can_add_active_storage_with_quantity_of_42(self): future_storage_package_quantity=42, future_storage_package_mb=42000, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=42001 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=42001, ) def test_premium_plan_can_add_future_storage(self): @@ -394,8 +394,8 @@ def test_premium_plan_can_add_future_storage(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=1000, - storage_used_bytes=0, - storage_free_bytes=1 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1, ) def test_premium_plan_can_replace_active_storage(self): @@ -417,8 +417,8 @@ def test_premium_plan_can_replace_active_storage(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=1001 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1001, ) ( @@ -443,8 +443,8 @@ def test_premium_plan_can_replace_active_storage(self): future_storage_package_quantity=2, future_storage_package_mb=2000, future_storage_package_changed_mb=1000, - storage_used_bytes=0, - storage_free_bytes=1001 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1001, ) def test_storage_with_custom_additional_storage(self): @@ -475,8 +475,8 @@ def test_storage_with_custom_additional_storage(self): future_storage_package_quantity=1, future_storage_package_mb=2, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=3 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=3, ) def test_storage_cannot_have_packages_with_time_overlaps(self): @@ -499,8 +499,8 @@ def test_storage_cannot_have_packages_with_time_overlaps(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=-1000, - storage_used_bytes=0, - storage_free_bytes=1001 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1001, ) with self.assertRaises(django.db.utils.IntegrityError): @@ -523,8 +523,8 @@ def test_storage_cannot_have_packages_with_time_overlaps(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=-1000, - storage_used_bytes=0, - storage_free_bytes=1001 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1001, ) def test_storage_cannot_add_new_package_when_active_until_is_null(self): @@ -546,8 +546,8 @@ def test_storage_cannot_add_new_package_when_active_until_is_null(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=1001 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1001, ) with self.assertRaises(django.db.utils.IntegrityError): @@ -569,8 +569,8 @@ def test_storage_cannot_add_new_package_when_active_until_is_null(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=1001 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1001, ) def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(self): @@ -584,8 +584,8 @@ def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(sel future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=1 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1, ) p1 = Project.objects.create(name="p1", owner=self.u1) @@ -604,8 +604,8 @@ def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(sel future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0.3 * 1000 * 1000, - storage_free_bytes=0.7 * 1000 * 1000, + storage_used_mb=0.3, + storage_free_mb=0.7, ) bucket.upload_fileobj(get_random_file(mb=0.1), storage_path) @@ -621,8 +621,8 @@ def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(sel future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0.4 * 1000 * 1000, - storage_free_bytes=0.6 * 1000 * 1000, + storage_used_mb=0.4, + storage_free_mb=0.6, ) version = list(list_versions(bucket, storage_path))[0] @@ -639,8 +639,8 @@ def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(sel future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0.1 * 1000 * 1000, - storage_free_bytes=0.9 * 1000 * 1000, + storage_used_mb=0.1, + storage_free_mb=0.9, ) delete_project_file_permanently(p1, "test.data") @@ -656,8 +656,8 @@ def test_used_storage_changes_when_uploading_and_deleting_files_and_versions(sel future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0 * 1000 * 1000, - storage_free_bytes=1 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1, ) def test_api_enforces_storage_limit(self): @@ -711,8 +711,8 @@ def test_api_enforces_storage_limit_when_owner_changes(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=10 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=10, user=u10mb, ) self.assertStorage( @@ -725,8 +725,8 @@ def test_api_enforces_storage_limit_when_owner_changes(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=20 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=20, user=u20mb, ) @@ -749,8 +749,8 @@ def test_api_enforces_storage_limit_when_owner_changes(self): future_storage_package_quantity=0, future_storage_package_mb=0, future_storage_package_changed_mb=0, - storage_used_bytes=15 * 1000 * 1000, - storage_free_bytes=5 * 1000 * 1000, + storage_used_mb=15, + storage_free_mb=5, user=u20mb, ) @@ -779,8 +779,8 @@ def test_api_enforces_storage_limit_when_owner_changes(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_bytes=0, - storage_free_bytes=1010 * 1000 * 1000, + storage_used_mb=0, + storage_free_mb=1010, user=u10mb, ) @@ -799,7 +799,7 @@ def test_api_enforces_storage_limit_when_owner_changes(self): future_storage_package_quantity=1, future_storage_package_mb=1000, future_storage_package_changed_mb=0, - storage_used_bytes=15 * 1000 * 1000, - storage_free_bytes=995 * 1000 * 1000, + storage_used_mb=15, + storage_free_mb=995, user=u10mb, ) From a2affe9b760bebc0a2ff5b9e10866dca0e898842 Mon Sep 17 00:00:00 2001 From: faebebin Date: Thu, 17 Aug 2023 12:55:56 +0200 Subject: [PATCH 32/38] fix formatting --- docker-app/qfieldcloud/subscription/tests/test_package.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docker-app/qfieldcloud/subscription/tests/test_package.py b/docker-app/qfieldcloud/subscription/tests/test_package.py index 9f7022bfc..fdb83cdb8 100644 --- a/docker-app/qfieldcloud/subscription/tests/test_package.py +++ b/docker-app/qfieldcloud/subscription/tests/test_package.py @@ -107,8 +107,12 @@ def assertStorage( user.useraccount.current_subscription.future_storage_package_changed_mb, future_storage_package_changed_mb, ) - self.assertEqual(user.useraccount.storage_used_bytes, storage_used_mb * 1000 * 1000) - self.assertEqual(user.useraccount.storage_free_bytes, storage_free_mb * 1000 * 1000) + self.assertEqual( + user.useraccount.storage_used_bytes, storage_used_mb * 1000 * 1000 + ) + self.assertEqual( + user.useraccount.storage_free_bytes, storage_free_mb * 1000 * 1000 + ) def test_get_storage_package_type(self): PackageType.objects.all().delete() From d3776428e02feea84c4e56cb75dff293daab122f Mon Sep 17 00:00:00 2001 From: Ivan Ivanov Date: Thu, 17 Aug 2023 11:27:44 +0300 Subject: [PATCH 33/38] Save a few precious milliseconds by preparing sentry report data only if needed --- .../qfieldcloud/core/views/files_views.py | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/docker-app/qfieldcloud/core/views/files_views.py b/docker-app/qfieldcloud/core/views/files_views.py index 5010a12c2..ced767712 100644 --- a/docker-app/qfieldcloud/core/views/files_views.py +++ b/docker-app/qfieldcloud/core/views/files_views.py @@ -214,23 +214,25 @@ def post(self, request, projectid, filename, format=None): # QF-2540 # Getting traceback in case the traceback provided by Sentry is too short # Add post-serialization keys for diff-ing with pre-serialization keys - buffer = io.StringIO() - print_stack(limit=50, file=buffer) - request_attributes = { - "data": str(copy.copy(self.request.data).keys()), - "files": str(self.request.FILES.keys()), - "meta": str(self.request.META), - } - missing_error = "" + if "file" not in request.data and not request.FILES.getlist("file"): + if "file" not in request.data: + logger.warning( + 'The key "file" was not found in `request.data`. Sending report to Sentry.' + ) - if "file" not in request.data: - missing_error = 'The key "file" was not found in `request.data`. Sending report to Sentry.' + if not request.FILES.getlist("file"): + logger.warning( + 'The key "file" occurs in `request.data` but maps to an empty list. Sending report to Sentry.' + ) - if not request.FILES.getlist("file"): - missing_error = 'The key "file" occurs in `request.data` but maps to an empty list. Sending report to Sentry.' + callstack_buffer = io.StringIO() + print_stack(limit=50, file=callstack_buffer) - if missing_error: - logging.warning(missing_error) + request_attributes = { + "data": str(copy.copy(self.request.data).keys()), + "files": str(self.request.FILES.keys()), + "meta": str(self.request.META), + } # QF-2540 report_serialization_diff_to_sentry( @@ -239,7 +241,7 @@ def post(self, request, projectid, filename, format=None): pre_serialization=request.attached_keys, post_serialization=",".join(QfcMultiPartSerializer.errors) + str(request_attributes), - buffer=buffer, + buffer=callstack_buffer, body_stream=getattr(request, "body_stream", None), ) raise exceptions.EmptyContentError() From e6a036fb554057a51f0401a1293c27c6f8bf4ef0 Mon Sep 17 00:00:00 2001 From: Adrien Date: Fri, 18 Aug 2023 14:28:00 +0200 Subject: [PATCH 34/38] Slight readability improvement on etag preparation for files (#771) --- docker-app/qfieldcloud/core/views/files_views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docker-app/qfieldcloud/core/views/files_views.py b/docker-app/qfieldcloud/core/views/files_views.py index 15ca9f406..a564a6d74 100644 --- a/docker-app/qfieldcloud/core/views/files_views.py +++ b/docker-app/qfieldcloud/core/views/files_views.py @@ -83,10 +83,11 @@ def get(self, request: Request, projectid: str) -> Response: path = PurePath(version.key) filename = str(path.relative_to(*path.parts[:3])) last_modified = version.last_modified.strftime("%d.%m.%Y %H:%M:%S %Z") + md5sum = version.e_tag.replace('"', "") version_data = { "size": version.size, - "md5sum": version.e_tag.replace('"', ""), + "md5sum": md5sum, "version_id": version.version_id, "last_modified": last_modified, "is_latest": version.is_latest, @@ -119,7 +120,7 @@ def get(self, request: Request, projectid: str) -> Response: files[version.key]["name"] = filename files[version.key]["size"] = version.size - files[version.key]["md5sum"] = version.e_tag.replace('"', "") + files[version.key]["md5sum"] = md5sum files[version.key]["last_modified"] = last_modified files[version.key]["is_attachment"] = is_attachment From f697bf3fdd707112095d01143f96d370eaa0c3fe Mon Sep 17 00:00:00 2001 From: Adrien Date: Fri, 18 Aug 2023 14:32:57 +0200 Subject: [PATCH 35/38] Cannot add organization owner as collaborator to org project (#773) --- docker-app/qfieldcloud/core/models.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docker-app/qfieldcloud/core/models.py b/docker-app/qfieldcloud/core/models.py index efd03cf5d..20b5ae6ae 100644 --- a/docker-app/qfieldcloud/core/models.py +++ b/docker-app/qfieldcloud/core/models.py @@ -1478,7 +1478,12 @@ def clean(self) -> None: if self.collaborator.is_person: members_qs = organization.members.filter(member=self.collaborator) - if not members_qs.exists(): + # for organizations-owned projects, the candidate collaborator + # must be a member of the organization or the organization's owner + if not ( + members_qs.exists() + or self.collaborator == organization.organization_owner + ): raise ValidationError( _( "Cannot add a user who is not a member of the organization as a project collaborator." From 051e9b9d22feb9e0fbe7795cc61cdae74d4ad63e Mon Sep 17 00:00:00 2001 From: Adrien Date: Fri, 18 Aug 2023 14:35:18 +0200 Subject: [PATCH 36/38] =?UTF-8?q?Make=20Project=20file=20deletion=20via=20?= =?UTF-8?q?Django=20Admin=20to=20ask=20for=20filename=20instead=20of=20?= =?UTF-8?q?=F0=9F=90=89=20#770?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../core/templates/admin/project_files_widget.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker-app/qfieldcloud/core/templates/admin/project_files_widget.html b/docker-app/qfieldcloud/core/templates/admin/project_files_widget.html index 32c561a78..a14858198 100644 --- a/docker-app/qfieldcloud/core/templates/admin/project_files_widget.html +++ b/docker-app/qfieldcloud/core/templates/admin/project_files_widget.html @@ -247,8 +247,8 @@ }); $deleteBtn.addEventListener('click', () => { - const passPhrase = 'Here be dragons!'; - const confirmation = prompt(`Are you sure you want to delete file "${file.name}"? This operation is irreversible, the file is deleted forever and the project may be damaged forever! Type "${passPhrase}" to confirm your destructive action!`); + const passPhrase = file.name; + const confirmation = prompt(`Are you sure you want to delete file "${passPhrase}"? This operation is irreversible, the file is deleted forever and the project may be damaged forever! Type "${passPhrase}" to confirm your destructive action!`); if (confirmation !== passPhrase) { $dialog.close(); From ffd1d885153ca045f6cf085262a69a5022fe5451 Mon Sep 17 00:00:00 2001 From: Adrien Date: Wed, 23 Aug 2023 09:03:56 +0200 Subject: [PATCH 37/38] Use a user's type name when adding as collaborator (#777) --- .../qfieldcloud/core/utils2/projects.py | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/docker-app/qfieldcloud/core/utils2/projects.py b/docker-app/qfieldcloud/core/utils2/projects.py index 14e49d606..bd150838b 100644 --- a/docker-app/qfieldcloud/core/utils2/projects.py +++ b/docker-app/qfieldcloud/core/utils2/projects.py @@ -1,26 +1,25 @@ -from typing import Tuple - from django.db.models import Q from django.utils.translation import gettext as _ from qfieldcloud.core import invitations_utils as invitation from qfieldcloud.core import permissions_utils as perms -from qfieldcloud.core.models import Person, Project, ProjectCollaborator, Team, User +from qfieldcloud.core.models import Person, Project, ProjectCollaborator, Team def create_collaborator( - project: Project, user: User, created_by: Person -) -> Tuple[bool, str]: + project: Project, user: Person | Team, created_by: Person +) -> tuple[bool, str]: """Creates a new collaborator (qfieldcloud.core.ProjectCollaborator) if possible Args: project (Project): the project to add collaborator to - user (User): the user to be added as collaborator + user (Person | Team): the user to be added as collaborator created_by (Person): the user that initiated the collaborator creation Returns: - Tuple[bool, str]: success, message - whether the collaborator creation was success and explanation message of the outcome + tuple[bool, str]: success, message - whether the collaborator creation was success and explanation message of the outcome """ success, message = False, "" + user_type_name = "Team" if isinstance(user, Team) else "User" try: perms.check_can_become_collaborator(user, project) @@ -34,12 +33,14 @@ def create_collaborator( updated_by=created_by, ) success = True - message = _('User "{}" has been invited to the project.').format(user.username) + message = _( + f'{user_type_name} "{user.username}" has been invited to the project.' + ) except perms.UserOrganizationRoleError: message = _( - "User '{}' is not a member of the organization that owns the project. " - "Please add this user to the organization first." - ).format(user.username) + f"{user_type_name} '{user.username}' is not a member of the organization that owns the project. " + f"Please add this {user_type_name.lower()} to the organization first." + ) except ( perms.AlreadyCollaboratorError, perms.ReachedCollaboratorLimitError, From 5885c49b896de633881edc0b0315bac7902cac35 Mon Sep 17 00:00:00 2001 From: Adrien Date: Fri, 25 Aug 2023 10:57:05 +0200 Subject: [PATCH 38/38] Type hints in worker_wrapper (#778) --- docker-app/worker_wrapper/wrapper.py | 26 +++++++++++++------------- docker-qgis/entrypoint.py | 14 +++++++------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/docker-app/worker_wrapper/wrapper.py b/docker-app/worker_wrapper/wrapper.py index a9c5eeb44..f70e97538 100644 --- a/docker-app/worker_wrapper/wrapper.py +++ b/docker-app/worker_wrapper/wrapper.py @@ -7,7 +7,7 @@ import uuid from datetime import timedelta from pathlib import Path -from typing import Any, Dict, Iterable, List, Tuple +from typing import Any, Iterable import docker import requests @@ -45,6 +45,7 @@ DOCKER_SIGKILL_EXIT_CODE = 137 QGIS_CONTAINER_NAME = os.environ.get("QGIS_CONTAINER_NAME", None) QFIELDCLOUD_HOST = os.environ.get("QFIELDCLOUD_HOST", None) +TMP_FILE = Path("/tmp") assert QGIS_CONTAINER_NAME assert QFIELDCLOUD_HOST @@ -63,9 +64,9 @@ def __init__(self, job_id: str) -> None: try: self.job_id = job_id self.job = self.job_class.objects.select_related().get(id=job_id) - self.shared_tempdir = Path(tempfile.mkdtemp(dir="/tmp")) + self.shared_tempdir = Path(tempfile.mkdtemp(dir=TMP_FILE)) except Exception as err: - feedback: Dict[str, Any] = {} + feedback: dict[str, Any] = {} (_type, _value, tb) = sys.exc_info() feedback["error"] = str(err) feedback["error_origin"] = "worker_wrapper" @@ -82,7 +83,7 @@ def __init__(self, job_id: str) -> None: else: logger.critical(msg, exc_info=err) - def get_context(self) -> Dict[str, Any]: + def get_context(self) -> dict[str, Any]: context = model_to_dict(self.job) for key, value in model_to_dict(self.job.project).items(): @@ -92,10 +93,9 @@ def get_context(self) -> Dict[str, Any]: return context - def get_command(self) -> List[str]: - return [ - p % self.get_context() for p in ["python3", "entrypoint.py", *self.command] - ] + def get_command(self) -> list[str]: + context = self.get_context() + return [p % context for p in ["python3", "entrypoint.py", *self.command]] def before_docker_run(self) -> None: pass @@ -231,8 +231,8 @@ def run(self): ) def _run_docker( - self, command: List[str], volumes: List[str], run_opts: Dict[str, Any] = {} - ) -> Tuple[int, bytes]: + self, command: list[str], volumes: list[str], run_opts: dict[str, Any] = {} + ) -> tuple[int, bytes]: QGIS_CONTAINER_NAME = os.environ.get("QGIS_CONTAINER_NAME", None) QFIELDCLOUD_HOST = os.environ.get("QFIELDCLOUD_HOST", None) QFIELDCLOUD_WORKER_QFIELDCLOUD_URL = os.environ.get( @@ -421,7 +421,7 @@ def __init__(self, job_id: str) -> None: if self.job.overwrite_conflicts: self.command = [*self.command, "--overwrite-conflicts"] - def _prepare_deltas(self, deltas: Iterable[Delta]): + def _prepare_deltas(self, deltas: Iterable[Delta]) -> dict[str, Any]: delta_contents = [] delta_client_ids = [] @@ -533,7 +533,7 @@ class ProcessProjectfileJobRun(JobRun): "%(project__project_filename)s", ] - def get_context(self, *args) -> Dict[str, Any]: + def get_context(self, *args) -> dict[str, Any]: context = super().get_context(*args) if not context.get("project__project_filename"): @@ -573,7 +573,7 @@ def after_docker_exception(self) -> None: def cancel_orphaned_workers(): client: DockerClient = docker.from_env() - running_workers: List[Container] = client.containers.list( + running_workers: list[Container] = client.containers.list( filters={"label": f"app={settings.ENVIRONMENT}_worker"}, ) diff --git a/docker-qgis/entrypoint.py b/docker-qgis/entrypoint.py index 3189a4bda..06ec2c7d4 100755 --- a/docker-qgis/entrypoint.py +++ b/docker-qgis/entrypoint.py @@ -4,7 +4,7 @@ import logging import os from pathlib import Path -from typing import Dict, Union +from typing import Union import qfieldcloud.qgis.apply_deltas import qfieldcloud.qgis.process_projectfile @@ -135,13 +135,13 @@ def _call_qfieldsync_packager(project_filename: Path, package_dir: Path) -> str: return packaged_project_filename -def _extract_layer_data(project_filename: Union[str, Path]) -> Dict: +def _extract_layer_data(project_filename: Union[str, Path]) -> dict: logging.info("Extracting QGIS project layer data…") project_filename = str(project_filename) project = QgsProject.instance() project.read(project_filename) - layers_by_id = get_layers_data(project) + layers_by_id: dict = get_layers_data(project) logging.info( f"QGIS project layer data\n{layers_data_to_string(layers_by_id)}", @@ -150,7 +150,7 @@ def _extract_layer_data(project_filename: Union[str, Path]) -> Dict: return layers_by_id -def cmd_package_project(args): +def cmd_package_project(args: argparse.Namespace): workflow = Workflow( id="package_project", name="Package Project", @@ -228,7 +228,7 @@ def cmd_package_project(args): ) -def cmd_apply_deltas(args): +def cmd_apply_deltas(args: argparse.Namespace): workflow = Workflow( id="apply_changes", name="Apply Changes", @@ -286,7 +286,7 @@ def cmd_apply_deltas(args): ) -def cmd_process_projectfile(args): +def cmd_process_projectfile(args: argparse.Namespace): workflow = Workflow( id="process_projectfile", name="Process Projectfile", @@ -399,5 +399,5 @@ def cmd_process_projectfile(args): ) parser_process_projectfile.set_defaults(func=cmd_process_projectfile) - args = parser.parse_args() + args: argparse.Namespace = parser.parse_args() args.func(args)