Give user registered types priority when encoding / decoding JSON by soceanainn · Pull Request #2188 · celery/kombu

soceanainn · 2024-11-07T12:41:38Z

User registered types should take priority over default types in Kombu.

Although users can currently override the encoder/decoder for a Kombu registered type by calling register_type (for example calling register_type(Decimal, ...) in their own code), this doesn't work well when it comes to subclassing. If users currently need to pass subclasses of any Celery registered types (datetime, date, time, Decimal or UUID) they would be forced to either:

Override implementation of superclass with a subclass aware encoder / decoder implementation, or
Access the 'protected' _encoders dictionary in the Kombu json module, pop the value for the superclass, add their subclass, and then re-add the superclass.

By separating user registered types and Kombu registered types into separate dictionaries, we can always give priority to user registered types instead, which simplifies this process for users (although technically it is a breaking change from existing behaviour).

Solves #1895

codecov · 2024-11-07T13:31:44Z

Codecov Report

❌ Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.46%. Comparing base (33d4eba) to head (ccbc8df).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
kombu/utils/json.py	95.65%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2188   +/-   ##
=======================================
  Coverage   82.45%   82.46%           
=======================================
  Files          79       79           
  Lines       10150    10161   +11     
  Branches     1167     1171    +4     
=======================================
+ Hits         8369     8379   +10     
  Misses       1579     1579           
- Partials      202      203    +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

auvipy

please fix the lint errors as well

Nusnus

I briefly reviewed the PR and it looks nice but I wonder if anything might break due to the split, even though it makes sense.

auvipy · 2024-12-28T08:35:56Z

I briefly reviewed the PR and it looks nice but I wonder if anything might break due to the split, even though it makes sense.

technically the change is a breaking change

auvipy · 2024-12-29T08:52:09Z

we have to release in in a new major version like 5.6/5.7

Copilot

Pull request overview

This PR changes Kombu’s JSON type registration mechanism so user-registered encoders/decoders are evaluated before Kombu’s built-in default type handlers, improving behavior when users register subclasses of Kombu’s default types.

Changes:

Split JSON type registries into user (_encoders/_decoders) vs default (_default_encoders/_default_decoders) registries.
Update encoding/decoding to consult user registries first, then fall back to defaults.
Add a unit test intended to validate user-registered type priority.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
`kombu/utils/json.py`	Introduces separate user/default registries and updates encoder/decoder lookup order to prefer user registrations.
`t/unit/utils/test_json.py`	Adds a test case intended to validate priority behavior for user-registered types.

Comments suppressed due to low confidence (1)

kombu/utils/json.py:79

The ValueError raised for unknown __type__ currently includes the builtin type object, not the marker from the payload. This makes the error misleading; use o["__type__"] (and possibly include available markers) instead of type.

        decoder = _decoders.get(o["__type__"]) or _default_decoders.get(o["__type__"])
        if decoder:
            return decoder(o["__value__"])
        else:
            raise ValueError("Unsupported type", type, o)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T07:38:32Z

 def _register_default_types():
    # NOTE: datetime should be registered before date,
    # because datetime is also instance of date.
-    register_type(datetime, "datetime", datetime.isoformat,
-                  datetime.fromisoformat)
-    register_type(
+    _register_type(datetime, "datetime", datetime.isoformat,
+                   datetime.fromisoformat)
+    _register_type(


_register_default_types() is used as a reset mechanism in tests, but after splitting defaults into _default_encoders/_default_decoders it no longer clears user-registered _encoders/_decoders. This means a test that calls register_type (e.g. overriding uuid.UUID) can leak that override into subsequent tests. Consider clearing _encoders/_decoders (and reinitializing _default_*) at the start of _register_default_types, or adding a dedicated reset helper for tests.

Copilot · 2026-02-12T07:38:32Z

    def default(self, o):
+        for t, (marker, encoder) in _encoders.items():
+            if isinstance(o, t):
+                return (
+                    encoder(o) if marker is None else _as(marker, encoder(o))
+                )
+


This change gives priority to user-registered types within JSONEncoder.default, but stdlib json does not call default() for JSON primitives (e.g. str/int/float/bool/None) or their subclasses. If the intent is to solve #1895's SafeString (a str subclass) example, this implementation likely won’t affect that case; it would require intercepting encoding before the primitive fast-path (e.g. overriding iterencode/preprocessing).

@soceanainn please cross check this and other suggestions

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

auvipy

can you please revisit this? some review comments need your attention

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-29T09:59:16Z

+        serialized_str = dumps(original)
+        # Ensure our custom marker is used instead of the default Decimal handler
+        assert '"mydecimal"' in serialized_str
+        loaded_value = loads(serialized_str)


assert '"mydecimal"' in serialized_str is a brittle check (it can match unrelated string content). It’s more robust to parse the JSON and assert on the __type__ field for md, which also makes the intent clearer.

Copilot · 2026-03-29T09:59:17Z

    def default(self, o):
+        for t, (marker, encoder) in _encoders.items():
+            if isinstance(o, t):
+                return (
+                    encoder(o) if marker is None else _as(marker, encoder(o))
+                )
+
        reducer = getattr(o, "__json__", None)
        if reducer is not None:
            return reducer()


JSONEncoder.default now applies user-registered encoders before checking __json__. This changes existing precedence: objects that implement __json__ and also match a registered type will no longer use their __json__ reducer. Consider keeping the __json__ check first (as before) and only prioritizing user encoders over default encoders.

Copilot · 2026-03-29T09:59:17Z

 def _register_default_types():
    # NOTE: datetime should be registered before date,
    # because datetime is also instance of date.
-    register_type(datetime, "datetime", datetime.isoformat,
-                  datetime.fromisoformat)
-    register_type(
+    _register_type(datetime, "datetime", datetime.isoformat,
+                   datetime.fromisoformat)
+    _register_type(
        date,
        "date",
        lambda o: o.isoformat(),
-        lambda o: datetime.fromisoformat(o).date(),
+        lambda o: datetime.fromisoformat(o).date()
    )
-    register_type(time, "time", lambda o: o.isoformat(), time.fromisoformat)
-    register_type(Decimal, "decimal", str, Decimal)
-    register_type(
+    _register_type(time, "time", lambda o: o.isoformat(), time.fromisoformat)
+    _register_type(Decimal, "decimal", str, Decimal)
+    _register_type(


_register_default_types() no longer resets user overrides registered via register_type(). In the test suite, the autouse fixture calls _register_default_types() to reset state between tests, but user-registered encoders/decoders will now persist and can make tests order-dependent (e.g. after overriding the UUID handler). Consider clearing _encoders/_decoders as part of the test reset, or adding an explicit reset helper that restores a clean default state.

Copilot · 2026-03-29T09:59:17Z

+    def test_register_type_takes_priority(self):
+        class MyDecimal(Decimal):
+            pass
+
+        register_type(MyDecimal, "mydecimal", str, MyDecimal)
+        original = {'md': MyDecimal('3314132.13363235235324234123213213214134')}
+        serialized_str = dumps(original)
+        # Ensure our custom marker is used instead of the default Decimal handler
+        assert '"mydecimal"' in serialized_str
+        loaded_value = loads(serialized_str)
+        # Ensure the decoded value is of the registered subclass, not just equal
+        assert isinstance(loaded_value['md'], MyDecimal)
+        assert original == loaded_value


This test registers a new custom type via register_type(), which mutates module-level global registries. With the new split between user/default registries, the existing _register_default_types() fixture no longer clears user registrations, so this (and other register_type tests) can leak state into subsequent tests and make the suite order-dependent. Consider resetting the user registries in the autouse fixture (or adding teardown in this test) to keep tests isolated.

soceanainn force-pushed the seamus/give-custom-json-encoders-priority branch from 52b2f86 to 247cf3e Compare November 7, 2024 14:45

auvipy requested changes Nov 18, 2024

View reviewed changes

soceanainn force-pushed the seamus/give-custom-json-encoders-priority branch 2 times, most recently from e9cfe80 to 21430db Compare November 18, 2024 21:41

soceanainn requested a review from auvipy November 18, 2024 21:42

Nusnus reviewed Dec 1, 2024

View reviewed changes

Nusnus force-pushed the seamus/give-custom-json-encoders-priority branch from 21430db to b3d4bb7 Compare December 1, 2024 23:28

Give user registered types priority when encoding / decoding JSON

7a20cbd

Nusnus force-pushed the seamus/give-custom-json-encoders-priority branch from b3d4bb7 to 7a20cbd Compare December 26, 2024 22:42

Merge branch 'main' into seamus/give-custom-json-encoders-priority

b6c080b

auvipy added this to the 5.7.0 milestone May 12, 2025

auvipy requested a review from Copilot February 12, 2026 07:33

Merge branch 'main' into seamus/give-custom-json-encoders-priority

c4b3ff6

Copilot started reviewing on behalf of auvipy February 12, 2026 07:33 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

auvipy and others added 3 commits February 12, 2026 15:33

Update t/unit/utils/test_json.py

bd37e27

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge branch 'main' into seamus/give-custom-json-encoders-priority

fdd48db

Merge branch 'main' into seamus/give-custom-json-encoders-priority

238e6a1

auvipy requested changes Mar 29, 2026

View reviewed changes

Merge branch 'main' into seamus/give-custom-json-encoders-priority

f6a7719

auvipy requested a review from Copilot March 29, 2026 09:55

Copilot started reviewing on behalf of auvipy March 29, 2026 09:55 View session

Copilot AI reviewed Mar 29, 2026

View reviewed changes

Merge branch 'main' into seamus/give-custom-json-encoders-priority

ccbc8df

celery deleted a comment from thedrow Apr 18, 2026

Uh oh!

Conversation

soceanainn commented Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

Nusnus left a comment

Choose a reason for hiding this comment

Uh oh!

auvipy commented Dec 28, 2024

Uh oh!

auvipy commented Dec 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

auvipy Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

auvipy left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

soceanainn commented Nov 7, 2024 •

edited

Loading

codecov Bot commented Nov 7, 2024 •

edited

Loading

auvipy commented Dec 29, 2024 •

edited

Loading