From 2163a425fd813e6dffd7f7590dcec857b018f5db Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 21:52:57 +0100 Subject: [PATCH 01/15] [dependencies] add structlog packages, also for django and pytest --- requirements/dev.in | 1 + requirements/dev.txt | 5 ++++- requirements/prod.in | 2 ++ requirements/prod.txt | 6 +++++- requirements/test.in | 1 + requirements/test.txt | 5 +++-- 6 files changed, 16 insertions(+), 4 deletions(-) diff --git a/requirements/dev.in b/requirements/dev.in index 4b7cb9f..8c26e50 100644 --- a/requirements/dev.in +++ b/requirements/dev.in @@ -4,5 +4,6 @@ django-debug-toolbar django-stubs ipython pip-compile-multi +rich ruff uv diff --git a/requirements/dev.txt b/requirements/dev.txt index 5b4dab8..bd3692d 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -1,4 +1,4 @@ -# SHA1:8a2089b98bf40ef6e9739b167a6c3bdc35a7c180 +# SHA1:dabc24f4b9a41d5fae3314c9a05a98434dd126e3 # # This file is autogenerated by pip-compile-multi # To update, run: @@ -17,7 +17,9 @@ executing==2.2.0 ipython==9.0.2 ipython-pygments-lexers==1.1.1 jedi==0.19.2 +markdown-it-py==3.0.0 matplotlib-inline==0.1.7 +mdurl==0.1.2 parso==0.8.4 pexpect==4.9.0 pip==25.0.1 @@ -28,6 +30,7 @@ ptyprocess==0.7.0 pure-eval==0.2.3 pygments==2.19.1 pyproject-hooks==1.2.0 +rich==13.9.4 ruff==0.11.0 setuptools==76.0.0 stack-data==0.6.3 diff --git a/requirements/prod.in b/requirements/prod.in index c360703..4058e05 100644 --- a/requirements/prod.in +++ b/requirements/prod.in @@ -1,4 +1,6 @@ django django-environ django-extensions +django-structlog[commands] psycopg[binary,pool] +structlog>24,<25 diff --git a/requirements/prod.txt b/requirements/prod.txt index 19971fe..493a11f 100644 --- a/requirements/prod.txt +++ b/requirements/prod.txt @@ -1,4 +1,4 @@ -# SHA1:0534e06bf8de6836b424e914cfbef002223d6f48 +# SHA1:48c3af154036b224a6b21f175ea0a949febaf6f5 # # This file is autogenerated by pip-compile-multi # To update, run: @@ -9,8 +9,12 @@ asgiref==3.8.1 django==5.1.7 django-environ==0.12.0 django-extensions==3.2.3 +django-ipware==7.0.1 +django-structlog==9.0.1 psycopg==3.2.6 psycopg-binary==3.2.6 psycopg-pool==3.2.6 +python-ipware==3.0.0 sqlparse==0.5.3 +structlog==24.4.0 typing-extensions==4.12.2 diff --git a/requirements/test.in b/requirements/test.in index 2c801fa..f1e3b52 100644 --- a/requirements/test.in +++ b/requirements/test.in @@ -3,3 +3,4 @@ hypothesis[django] pytest pytest-django +logot[pytest,structlog] diff --git a/requirements/test.txt b/requirements/test.txt index 2f8d5ca..7aa87fa 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -1,4 +1,4 @@ -# SHA1:9bf147356ad56dbe073ef0973141bdfb94631c9b +# SHA1:40e4b481355fc16525f8944d39904264b6e382a6 # # This file is autogenerated by pip-compile-multi # To update, run: @@ -7,8 +7,9 @@ # -r prod.txt attrs==25.3.0 -hypothesis==6.129.1 +hypothesis==6.129.2 iniconfig==2.0.0 +logot==1.3.0 packaging==24.2 pluggy==1.5.0 pytest==8.3.5 From f27dc5a0d337275fbc0953231b93e248c184c980 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 21:53:41 +0100 Subject: [PATCH 02/15] [test] tell pytest about the logot capturer ...which we do not yet use at this point --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 63bbc12..9c7d16a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -23,6 +23,7 @@ addopts = [ "--no-migrations", "--capture=sys", ] +logot_capturer = "logot.structlog.StructlogCapturer" [build-system] requires = ["pdm-backend"] From 2f8db3c67dad0313d6b16955018f7fa6ba21f6e2 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 21:54:11 +0100 Subject: [PATCH 03/15] [codestyle] taplo wants "pythonpath" to be a list of strings --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 9c7d16a..c60b9ff 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,7 @@ authors = [ ] [tool.pytest.ini_options] -pythonpath = "." +pythonpath = ["."] python_files = [ "test_*.py", "tests.py", From e175071d3bfc6d53635d2be323015ad52257d810 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 21:54:37 +0100 Subject: [PATCH 04/15] [urls] move urls of collector up to top level --- config/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/urls.py b/config/urls.py index f42570b..b02a46c 100644 --- a/config/urls.py +++ b/config/urls.py @@ -20,7 +20,7 @@ from django.contrib import admin from django.urls import include, path urlpatterns = [ - path("collector/", include("collector.urls")), + path("", include("collector.urls")), path("admin/", admin.site.urls), ] From 4d96f60b46f2106ee3005f515c142034e13453e8 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 21:55:43 +0100 Subject: [PATCH 05/15] [settings] more robust detection of deployment Retrieve it only once from env, make it an uppercase string --- config/settings.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/config/settings.py b/config/settings.py index bde6d3d..3c440cb 100644 --- a/config/settings.py +++ b/config/settings.py @@ -15,7 +15,7 @@ BASE_DIR = Path(__file__).resolve().parent.parent env = environ.Env( DEBUG=(bool, False), - DEPLOYMENT=(str, "Dev"), + DEPLOYMENT=(str, "DEV"), ) env.read_env(env.str("ENV_PATH", BASE_DIR / ".env")) # pyright: ignore[reportArgumentType] @@ -23,6 +23,7 @@ env.read_env(env.str("ENV_PATH", BASE_DIR / ".env")) # pyright: ignore[reportAr SECRET_KEY = env("SECRET_KEY") DEBUG = env("DEBUG") +DEPLOYMENT = str(env("DEPLOYMENT")).upper() ALLOWED_HOSTS = [] SESSION_COOKIE_SECURE = True @@ -39,7 +40,7 @@ INSTALLED_APPS = [ "django.contrib.staticfiles", "collector.apps.CollectorConfig", ] -if env("DEPLOYMENT") == "Dev": +if DEPLOYMENT == "DEV": INSTALLED_APPS += [ "django_extensions", "debug_toolbar", @@ -55,7 +56,7 @@ MIDDLEWARE = [ "django.contrib.messages.middleware.MessageMiddleware", "django.middleware.clickjacking.XFrameOptionsMiddleware", ] -if env("DEPLOYMENT") == "Dev": +if DEPLOYMENT == "DEV": MIDDLEWARE += [ "debug_toolbar.middleware.DebugToolbarMiddleware", ] From 2e31cf80471e87c5828493baa584fb8a26b4c838 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 21:56:21 +0100 Subject: [PATCH 06/15] [settings] remove authenticator settings: we don't have users --- config/settings.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/config/settings.py b/config/settings.py index 3c440cb..e9c707c 100644 --- a/config/settings.py +++ b/config/settings.py @@ -90,23 +90,6 @@ DATABASES = { "default": env.db(), } -# Password validation -# https://docs.djangoproject.com/en/5.1/ref/settings/#auth-password-validators -AUTH_PASSWORD_VALIDATORS = [ - { - "NAME": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator", - }, - { - "NAME": "django.contrib.auth.password_validation.MinimumLengthValidator", - }, - { - "NAME": "django.contrib.auth.password_validation.CommonPasswordValidator", - }, - { - "NAME": "django.contrib.auth.password_validation.NumericPasswordValidator", - }, -] - # Internationalization # https://docs.djangoproject.com/en/5.1/topics/i18n/ LANGUAGE_CODE = "en-us" From 86ccdd58ee03b453d0a1fb6dec8eb6818101b305 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 21:57:13 +0100 Subject: [PATCH 07/15] [logs] set up and use structlog in our app --- collector/views.py | 8 ++++--- config/settings.py | 54 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/collector/views.py b/collector/views.py index cc6023d..13d7a09 100644 --- a/collector/views.py +++ b/collector/views.py @@ -1,4 +1,4 @@ -import logging +import structlog from typing import Any from django.db import transaction @@ -9,7 +9,7 @@ from django.views import generic from .models import Teil -logger = logging.getLogger(__name__) +logger = structlog.get_logger(__name__) DEFAULT_TEILE_NUMBER = 10 @@ -47,8 +47,10 @@ class DetailView(generic.DetailView): def enter(request: HttpRequest) -> HttpResponse: try: with transaction.atomic(): - Teil.objects.create(name=request.POST["new_name"]) + teil = Teil.objects.create(name=request.POST["new_name"]) except Exception: logger.warning("Teil already existed") + else: + logger.info("New Teil entered", teil_id=teil.pk) return HttpResponseRedirect(reverse("collector:list")) diff --git a/config/settings.py b/config/settings.py index e9c707c..febcbb7 100644 --- a/config/settings.py +++ b/config/settings.py @@ -8,9 +8,12 @@ For the full list of settings and their values, see https://docs.djangoproject.com/en/5.1/ref/settings/ """ -import environ +import os from pathlib import Path +import environ +import structlog + BASE_DIR = Path(__file__).resolve().parent.parent env = environ.Env( @@ -32,6 +35,7 @@ CSRF_COOKIE_SECURE = True # Application definition INSTALLED_APPS = [ + "django_structlog", "django.contrib.admin", "django.contrib.auth", "django.contrib.contenttypes", @@ -48,6 +52,7 @@ if DEPLOYMENT == "DEV": SHELL_PLUS = "ipython" MIDDLEWARE = [ + "django_structlog.middlewares.RequestMiddleware", "django.middleware.security.SecurityMiddleware", "django.contrib.sessions.middleware.SessionMiddleware", "django.middleware.common.CommonMiddleware", @@ -104,3 +109,50 @@ STATIC_URL = "static/" # Default primary key field type # https://docs.djangoproject.com/en/5.1/ref/settings/#default-auto-field DEFAULT_AUTO_FIELD = "django.db.models.BigAutoField" + +LOGGING = { + "version": 1, + "disable_existing_loggers": True, + "formatters": { + "logftm_formatter": { + "()": structlog.stdlib.ProcessorFormatter, + "processor": structlog.processors.LogfmtRenderer(sort_keys=True, key_order=["level", "event"]), + }, + }, + "handlers": { + "console": { + "class": "logging.StreamHandler", + "formatter": "logftm_formatter", + }, + }, + "loggers": { + "django_structlog": { + "handlers": ["console"], + "level": "DEBUG", + }, + "root": { + "handlers": ["console"], + "level": "DEBUG", + }, + }, +} + +structlog.configure( + processors=[ + structlog.contextvars.merge_contextvars, + structlog.stdlib.filter_by_level, + structlog.processors.TimeStamper(fmt="iso"), + structlog.stdlib.add_logger_name, + structlog.stdlib.add_log_level, + structlog.stdlib.PositionalArgumentsFormatter(), + structlog.processors.StackInfoRenderer(), + structlog.processors.format_exc_info, + structlog.processors.UnicodeDecoder(), + structlog.stdlib.ProcessorFormatter.wrap_for_formatter, + ], + logger_factory=structlog.stdlib.LoggerFactory(), + cache_logger_on_first_use=True, +) +os.makedirs(BASE_DIR / "logs", exist_ok=True) + +# DJANGO_STRUCTLOG_COMMAND_LOGGING_ENABLED = True From 051f0b964e56951f7e163e5aaa0d787eab175963 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 22:25:13 +0100 Subject: [PATCH 08/15] [test] add new fixture to generate random strings of given length --- collector/conftest.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/collector/conftest.py b/collector/conftest.py index a925f11..6266efd 100644 --- a/collector/conftest.py +++ b/collector/conftest.py @@ -1,3 +1,7 @@ +import random +import string +from typing import Callable + import pytest from django.test import Client @@ -10,3 +14,12 @@ def enable_db_access_for_all_tests(db): @pytest.fixture(scope="session", name="session") def fixture_longlived_client() -> Client: return Client() + +@pytest.fixture(name="random_name") +def fixture_random_name_generator() -> Callable[[int], str]: + def name_generator(length: int = 7) -> str: + name = "".join(random.choices(string.ascii_letters, k=length)) + return name + + return name_generator + From 4605dedc690c1fb2d71ae5228b85e9efbaf3edc4 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 22:26:01 +0100 Subject: [PATCH 09/15] [codestyle] compact test functions a little bit --- collector/tests.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/collector/tests.py b/collector/tests.py index 9c2c77f..3a97ee6 100644 --- a/collector/tests.py +++ b/collector/tests.py @@ -9,12 +9,10 @@ names = st.text(alphabet=st.characters(exclude_categories=["C"]), min_size=1) @given(data=names) def test_submitted_data_ends_up_in_database(data, session: Client): - with pytest.raises(Teil.DoesNotExist): Teil.objects.get(name=data) response = session.post(reverse("collector:enter"), data={"new_name": data}) - assert response.status_code == 302 assert Teil.objects.get(name=data) @@ -25,11 +23,9 @@ def test_entering_same_name_twice_does_not_change_database_entry(data, session: response = session.post(reverse("collector:enter"), data={"new_name": data}) assert response.status_code == 302 - assert Teil.objects.filter(name=data).count() == 1 response = session.post(reverse("collector:enter"), data={"new_name": data}) assert response.status_code == 302 - assert Teil.objects.filter(name=data).count() == 1 From aa5d19c5e5de54224b0e9641d847ba31698208a0 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 22:26:36 +0100 Subject: [PATCH 10/15] [collector] ensure endpoint only accepts POST requests --- collector/tests.py | 20 ++++++++++++++++++++ collector/views.py | 2 ++ 2 files changed, 22 insertions(+) diff --git a/collector/tests.py b/collector/tests.py index 3a97ee6..c23355f 100644 --- a/collector/tests.py +++ b/collector/tests.py @@ -7,6 +7,7 @@ from .models import Teil names = st.text(alphabet=st.characters(exclude_categories=["C"]), min_size=1) + @given(data=names) def test_submitted_data_ends_up_in_database(data, session: Client): with pytest.raises(Teil.DoesNotExist): @@ -29,3 +30,22 @@ def test_entering_same_name_twice_does_not_change_database_entry(data, session: assert response.status_code == 302 assert Teil.objects.filter(name=data).count() == 1 + +@pytest.mark.parametrize( + "http_method,expected_status", + [ + ("GET", 405), + ("PATCH", 405), + ("POST", 302), + ("PUT", 405), + ], +) +def test_enter_endpoint_accepts_only_post_requests( + client: Client, http_method: str, expected_status: int, random_name +): + request_method = getattr(client, http_method.lower()) + + response = request_method( + reverse("collector:enter"), data={"new_name": random_name(8)} + ) + assert response.status_code == expected_status diff --git a/collector/views.py b/collector/views.py index 13d7a09..5e25b6b 100644 --- a/collector/views.py +++ b/collector/views.py @@ -6,6 +6,7 @@ from django.db.models import QuerySet from django.http import HttpRequest, HttpResponse, HttpResponseRedirect from django.urls import reverse from django.views import generic +from django.views.decorators.http import require_http_methods from .models import Teil @@ -44,6 +45,7 @@ class DetailView(generic.DetailView): return context +@require_http_methods(["POST"]) def enter(request: HttpRequest) -> HttpResponse: try: with transaction.atomic(): From 246c22ce3943311f760e98bf6db8279a2be71c22 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 22:28:46 +0100 Subject: [PATCH 11/15] [codestyle] shortly explain why we wrap `.create()` in a transaction --- collector/views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/collector/views.py b/collector/views.py index 5e25b6b..a273c68 100644 --- a/collector/views.py +++ b/collector/views.py @@ -48,6 +48,8 @@ class DetailView(generic.DetailView): @require_http_methods(["POST"]) def enter(request: HttpRequest) -> HttpResponse: try: + # if .create() failed, the transaction is rolled back and we are in a + # clean (database) state to handle exceptions with transaction.atomic(): teil = Teil.objects.create(name=request.POST["new_name"]) except Exception: From 16503bea7a9305f49b85ed2da54bec6d328dd9a7 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 22:30:34 +0100 Subject: [PATCH 12/15] [codestyle] add type hint to fixture, indicating it is a function that we need to call --- collector/tests.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/collector/tests.py b/collector/tests.py index c23355f..1b2208a 100644 --- a/collector/tests.py +++ b/collector/tests.py @@ -1,3 +1,4 @@ +from typing import Callable import pytest from django.test import Client from django.urls import reverse @@ -41,7 +42,7 @@ def test_entering_same_name_twice_does_not_change_database_entry(data, session: ], ) def test_enter_endpoint_accepts_only_post_requests( - client: Client, http_method: str, expected_status: int, random_name + client: Client, http_method: str, expected_status: int, random_name: Callable[[int], str] ): request_method = getattr(client, http_method.lower()) From 7c825e15cfd96170d95dd370aa825a9756900ee0 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 22:33:14 +0100 Subject: [PATCH 13/15] [test] widen scope of random-name fixture --- collector/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/collector/conftest.py b/collector/conftest.py index 6266efd..7f0989f 100644 --- a/collector/conftest.py +++ b/collector/conftest.py @@ -15,7 +15,7 @@ def enable_db_access_for_all_tests(db): def fixture_longlived_client() -> Client: return Client() -@pytest.fixture(name="random_name") +@pytest.fixture(name="random_name", scope="session") def fixture_random_name_generator() -> Callable[[int], str]: def name_generator(length: int = 7) -> str: name = "".join(random.choices(string.ascii_letters, k=length)) From a0a2b1ea53b361934cd25756bca03bb36dd12456 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 22:48:43 +0100 Subject: [PATCH 14/15] [codestyle] rename variable --- collector/conftest.py | 2 +- collector/tests.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/collector/conftest.py b/collector/conftest.py index 7f0989f..fadbcd5 100644 --- a/collector/conftest.py +++ b/collector/conftest.py @@ -15,6 +15,7 @@ def enable_db_access_for_all_tests(db): def fixture_longlived_client() -> Client: return Client() + @pytest.fixture(name="random_name", scope="session") def fixture_random_name_generator() -> Callable[[int], str]: def name_generator(length: int = 7) -> str: @@ -22,4 +23,3 @@ def fixture_random_name_generator() -> Callable[[int], str]: return name return name_generator - diff --git a/collector/tests.py b/collector/tests.py index 1b2208a..44ae259 100644 --- a/collector/tests.py +++ b/collector/tests.py @@ -44,9 +44,9 @@ def test_entering_same_name_twice_does_not_change_database_entry(data, session: def test_enter_endpoint_accepts_only_post_requests( client: Client, http_method: str, expected_status: int, random_name: Callable[[int], str] ): - request_method = getattr(client, http_method.lower()) + client_method = getattr(client, http_method.lower()) - response = request_method( + response = client_method( reverse("collector:enter"), data={"new_name": random_name(8)} ) assert response.status_code == expected_status From f0f727fd995fdd3970e38743d9af8e2677c866c1 Mon Sep 17 00:00:00 2001 From: bronsen Date: Sat, 15 Mar 2025 23:14:27 +0100 Subject: [PATCH 15/15] [tests] WIP re #3 --- collector/tests.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/collector/tests.py b/collector/tests.py index 44ae259..69eba73 100644 --- a/collector/tests.py +++ b/collector/tests.py @@ -3,6 +3,7 @@ import pytest from django.test import Client from django.urls import reverse from hypothesis import given, strategies as st +from logot import Logot, logged from .models import Teil @@ -42,7 +43,10 @@ def test_entering_same_name_twice_does_not_change_database_entry(data, session: ], ) def test_enter_endpoint_accepts_only_post_requests( - client: Client, http_method: str, expected_status: int, random_name: Callable[[int], str] + client: Client, + http_method: str, + expected_status: int, + random_name: Callable[[int], str], ): client_method = getattr(client, http_method.lower()) @@ -50,3 +54,18 @@ def test_enter_endpoint_accepts_only_post_requests( reverse("collector:enter"), data={"new_name": random_name(8)} ) assert response.status_code == expected_status + + +@pytest.mark.xfail("WIP, kein Plan warum nichts gecaptured zu werden scheint") +def test_enter_endpoints_emits_expected_logs( + client: Client, logot: Logot, random_name: Callable[[int], str] +): + same_name = random_name(10) + + response = client.post(reverse("collector:enter"), data={"new_name": same_name}) + assert response.status_code == 302 + logot.assert_logged(logged.info("New Teil entered")) + + response = client.post(reverse("collector:enter"), data={"new_name": same_name}) + assert response.status_code == 302 + logot.assert_logged(logged.warning("Teil already existed"))