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 12 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 `frozendict`.
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