Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Use immutabledict instead of frozendict #15113

Merged
merged 13 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/15113.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use `immutabledict` instead of `canonicaljson`.
clokep marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ disallow_untyped_defs = False
[mypy-authlib.*]
ignore_missing_imports = True

[mypy-frozendict.*]
ignore_missing_imports = True
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved

[mypy-ijson.*]
ignore_missing_imports = True

Expand Down
125 changes: 16 additions & 109 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 5 additions & 7 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,13 @@ python = "^3.7.1"
# ----------------------
# we use the TYPE_CHECKER.redefine method added in jsonschema 3.0.0
jsonschema = ">=3.0.0"
# frozendict 2.1.2 is broken on Debian 10: https://github.com/Marco-Sulla/python-frozendict/issues/41
# We cannot test our wheels against the 2.3.5 release in CI. Putting in an upper bound for this
# because frozendict has been more trouble than it's worth; we would like to move to immutabledict.
frozendict = ">=1,!=2.1.2,<2.3.5"
# We choose 2.0 as a lower bound: the most recent backwards incompatible release.
# It seems generally available, judging by https://pkgs.org/search/?q=immutabledict
immutabledict = ">=2.0"
# We require 2.1.0 or higher for type hints. Previous guard was >= 1.1.0
unpaddedbase64 = ">=2.1.0"
# We require 1.5.0 to work around an issue when running against the C implementation of
# frozendict: https://github.com/matrix-org/python-canonicaljson/issues/36
canonicaljson = "^1.5.0"
# We require 2.0.0 for immutabledict support.
canonicaljson = "^2.0.0"
# we use the type definitions added in signedjson 1.1.
signedjson = "^1.1.0"
# validating SSL certs for IP addresses requires service_identity 18.1.
Expand Down
39 changes: 0 additions & 39 deletions stubs/frozendict.pyi

This file was deleted.

19 changes: 14 additions & 5 deletions synapse/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
""" This is an implementation of a Matrix homeserver.
"""

import json
import os
import sys
from typing import Any, Dict

from synapse.util.rust import check_rust_lib_up_to_date
from synapse.util.stringutils import strtobool
Expand Down Expand Up @@ -61,11 +61,20 @@
except ImportError:
pass

# Use the standard library json implementation instead of simplejson.
# Teach canonicaljson how to serialise immutabledicts.
try:
from canonicaljson import set_json_library

set_json_library(json)
from canonicaljson import register_preserialisation_callback
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
from immutabledict import immutabledict

def _immutabledict_cb(d: immutabledict) -> Dict[str, Any]:
try:
return d._dict
except Exception:
# Paranoia: fall back to a `dict()` call, in case a future version of
# immutabledict removes `_dict` from the implementation.
return dict(d)
Comment on lines +69 to +75
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a shame that we need this same code here as _handle_immutabledict in utils. I guess we previously had this baked into canonicaljson and Synapse?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also confused by this. It looks like Synapse had/has its own internal version of canonicaljson---did we never fully extract this out to library calls?

I wouldn't object to just using canonicaljson everywhere tbh, but I didn't feel like opening that can of worms here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also confused by this. It looks like Synapse had/has its own internal version of canonicaljson---did we never fully extract this out to library calls?

It has its own JSON serialization code, but that isn't canonicaljson. (It is "just" JSON.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do wonder if we should import the same function though from the utils module?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ISWYM. I can probably sneak that in

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, on second thoughts I'd like to leave that for a follow-up PR---I think it'd be best done alongside the tidy-ups I had in mind in #15113 (comment)


register_preserialisation_callback(immutabledict, _immutabledict_cb)
except ImportError:
pass

Expand Down
2 changes: 1 addition & 1 deletion synapse/crypto/event_signing.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def check_event_content_hash(
# some malformed events lack a 'hashes'. Protect against it being missing
# or a weird type by basically treating it the same as an unhashed event.
hashes = event.get("hashes")
# nb it might be a frozendict or a dict
# nb it might be a immutabledict or a dict
if not isinstance(hashes, collections.abc.Mapping):
raise SynapseError(
400, "Malformed 'hashes': %s" % (type(hashes),), Codes.UNAUTHORIZED
Expand Down
4 changes: 2 additions & 2 deletions synapse/events/snapshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from typing import TYPE_CHECKING, List, Optional, Tuple

import attr
from frozendict import frozendict
from immutabledict import immutabledict

from synapse.appservice import ApplicationService
from synapse.events import EventBase
Expand Down Expand Up @@ -489,4 +489,4 @@ def _decode_state_dict(
if input is None:
return None

return frozendict({(etype, state_key): v for etype, state_key, v in input})
return immutabledict({(etype, state_key): v for etype, state_key, v in input})
Loading