From e75b9f3f20025d0acf8d8d178aaf60c0f52b8500 Mon Sep 17 00:00:00 2001 From: d3vyce <44915747+d3vyce@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:13:58 +0200 Subject: [PATCH] fix: include positional args in the deduplication fingerprint (#81) --- README.md | 2 +- docs/index.md | 2 +- docs/usage.md | 16 ++++++++++++++-- src/taskiq_deduplication/middleware.py | 11 ++++++++++- tests/conftest.py | 6 ++++-- tests/test_middleware.py | 21 +++++++++++++++++++++ 6 files changed, 51 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 5e345fd..a40d509 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ except DuplicateTaskError: - **Configurable TTL** — set a global default or override per task with the `deduplication_ttl` label. - **Lock heartbeat** — a background task re-extends the lock TTL while the task runs, so long-running tasks keep their lock instead of expiring mid-execution and admitting a duplicate. - **Explicit lock key** — pin any task to a fixed Redis key with `deduplication_key`, bypassing fingerprint computation entirely. -- **Partial fingerprint** — deduplicate on a subset of kwargs with `deduplication_key_fields`, ignoring irrelevant arguments. +- **Partial fingerprint** — deduplicate on a subset of kwargs with `deduplication_key_fields`, ignoring irrelevant arguments (positional arguments are excluded). - **Per-task opt-out** — disable deduplication for individual tasks with the `deduplication` label. - **Startup resilience** — automatic reconnection with exponential backoff if Redis is unavailable at broker startup. diff --git a/docs/index.md b/docs/index.md index 5e345fd..a40d509 100644 --- a/docs/index.md +++ b/docs/index.md @@ -55,7 +55,7 @@ except DuplicateTaskError: - **Configurable TTL** — set a global default or override per task with the `deduplication_ttl` label. - **Lock heartbeat** — a background task re-extends the lock TTL while the task runs, so long-running tasks keep their lock instead of expiring mid-execution and admitting a duplicate. - **Explicit lock key** — pin any task to a fixed Redis key with `deduplication_key`, bypassing fingerprint computation entirely. -- **Partial fingerprint** — deduplicate on a subset of kwargs with `deduplication_key_fields`, ignoring irrelevant arguments. +- **Partial fingerprint** — deduplicate on a subset of kwargs with `deduplication_key_fields`, ignoring irrelevant arguments (positional arguments are excluded). - **Per-task opt-out** — disable deduplication for individual tasks with the `deduplication` label. - **Startup resilience** — automatic reconnection with exponential backoff if Redis is unavailable at broker startup. diff --git a/docs/usage.md b/docs/usage.md index 61c5f02..4393cef 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -147,11 +147,19 @@ await my_task.kicker().with_labels(deduplication_ttl=60).kiq(user_id=42) | `deduplication` | `bool` | Set `False` to opt out of deduplication entirely for this task. | | `deduplication_ttl` | `int` | Lock TTL in seconds. Overrides the middleware `default_ttl`. | | `deduplication_key` | `str` | Explicit lock key. Skips fingerprint computation entirely. | -| `deduplication_key_fields` | `list[str]` | Subset of kwargs to include in the fingerprint. Ignored if `deduplication_key` is set. | +| `deduplication_key_fields` | `list[str]` | Subset of kwargs to include in the fingerprint. Positional arguments are excluded. Ignored if `deduplication_key` is set. | ## Fingerprint and key customisation -By default the lock key is a SHA-256 fingerprint of the task name and all kwargs. +By default the lock key is a SHA-256 fingerprint of the task name, its positional +arguments and all kwargs. + +!!! warning "Positional and keyword calls fingerprint differently" + + taskiq serialises arguments as they were passed, without binding them to the + task signature. `my_task.kiq(42)` and `my_task.kiq(user_id=42)` are therefore + *not* recognised as duplicates of each other. Call a deduplicated task + consistently, preferably always with keyword arguments. ### Explicit key @@ -182,6 +190,10 @@ If a listed field is absent from a task's kwargs, it is dropped from the fingerprint and a warning is logged, since this can make genuinely different calls collide on the same lock. +Positional arguments are excluded from the fingerprint entirely when this label is +set: you asked to deduplicate on named fields, so pass them as keyword arguments. +A warning is logged if the task is called with positional arguments anyway. + ## Opting out per task ```python diff --git a/src/taskiq_deduplication/middleware.py b/src/taskiq_deduplication/middleware.py index 8ad748f..bee30af 100644 --- a/src/taskiq_deduplication/middleware.py +++ b/src/taskiq_deduplication/middleware.py @@ -154,12 +154,21 @@ class RedisDeduplicationMiddleware(TaskiqMiddleware): message.task_name, missing, ) + if message.args: + logger.warning( + "Task %s was called with positional arguments but uses " + "deduplication_key_fields; positional arguments are excluded " + "from the fingerprint.", + message.task_name, + ) kwargs = {k: v for k, v in message.kwargs.items() if k in key_fields} + args: list[Any] = [] else: kwargs = message.kwargs + args = message.args try: payload = json.dumps( - {"task": message.task_name, "kwargs": kwargs}, + {"task": message.task_name, "args": args, "kwargs": kwargs}, sort_keys=True, ) except TypeError: diff --git a/tests/conftest.py b/tests/conftest.py index a78cae5..f80743c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -33,13 +33,15 @@ async def real_redis(): @pytest.fixture def make_message(): - def _make(task_name="my_task", task_id="task-1", labels=None, kwargs=None): + def _make( + task_name="my_task", task_id="task-1", labels=None, kwargs=None, args=None + ): return TaskiqMessage( task_id=task_id, task_name=task_name, labels=labels or {}, labels_types={}, - args=[], + args=args or [], kwargs=kwargs or {}, ) diff --git a/tests/test_middleware.py b/tests/test_middleware.py index c214149..69f90cf 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -86,6 +86,27 @@ class TestDefaultBuildDeduplicationKey: m1 ) == middleware._build_deduplication_key(m2) + def test_different_args_different_key(self, middleware, make_message): + m1 = make_message(args=["a"]) + m2 = make_message(args=["b"]) + assert middleware._build_deduplication_key( + m1 + ) != middleware._build_deduplication_key(m2) + + def test_arg_order_matters(self, middleware, make_message): + m1 = make_message(args=["a", "b"]) + m2 = make_message(args=["b", "a"]) + assert middleware._build_deduplication_key( + m1 + ) != middleware._build_deduplication_key(m2) + + def test_key_fields_ignores_args(self, middleware, make_message): + m1 = make_message(args=["a"], labels={DEDUP_KEY_FIELDS_LABEL: ["x"]}) + m2 = make_message(args=["b"], labels={DEDUP_KEY_FIELDS_LABEL: ["x"]}) + assert middleware._build_deduplication_key( + m1 + ) == middleware._build_deduplication_key(m2) + def test_key_prefix_in_output(self, make_message): mw = RedisDeduplicationMiddleware( redis_url="redis://localhost", key_prefix="myapp:locks"