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

Add a docstring & tests for _flatten_dict. #14981

Merged
merged 4 commits into from
Feb 3, 2023
Merged
Show file tree
Hide file tree
Changes from all 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/14981.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add tests for `_flatten_dict`.
23 changes: 23 additions & 0 deletions synapse/push/bulk_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,29 @@ def _flatten_dict(
prefix: Optional[List[str]] = None,
result: Optional[Dict[str, str]] = None,
) -> Dict[str, str]:
"""
Given a JSON dictionary (or event) which might contain sub dictionaries,
flatten it into a single layer dictionary by combining the keys & sub-keys.
clokep marked this conversation as resolved.
Show resolved Hide resolved

clokep marked this conversation as resolved.
Show resolved Hide resolved
Any (non-dictionary), non-string value is dropped.

Transforms:

{"foo": {"bar": "test"}}

To:

{"foo.bar": "test"}
Comment on lines +475 to +481
Copy link
Contributor

Choose a reason for hiding this comment

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

How would (should) the dict

{"foo.bar": 1, "foo": {"bar": 2}}

be transformed? I imagine that 2 will win... but maybe we should just say something like "the result is undefined if there is any kind of conflict between keys"?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is undefined behavior, one of which the pull requests that I'll be implementing will be fixing.

Copy link
Member Author

Choose a reason for hiding this comment

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

MSC3873 for the record.


Args:
d: The event or content to continue flattening.
room_version: The room version object.
prefix: The key prefix (from outer dictionaries).
result: The result to mutate.

Returns:
The resulting dictionary.
"""
if prefix is None:
prefix = []
if result is None:
Expand Down
26 changes: 25 additions & 1 deletion tests/push/test_push_rule_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Dict, List, Optional, Set, Union, cast
from typing import Any, Dict, List, Optional, Set, Union, cast

import frozendict

Expand All @@ -37,6 +37,30 @@
from tests.test_utils.event_injection import create_event, inject_member_event


class FlattenDictTestCase(unittest.TestCase):
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth a test case where this function is given an EventBase---it seems to have bespoke logic for events.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe -- I'm not changing that behavior so I'll try to do that as a follow-up so I can at least avoid the self-conflicts.

def test_simple(self) -> None:
"""Test a dictionary that isn't modified."""
input = {"foo": "abc"}
self.assertEqual(input, _flatten_dict(input))

def test_nested(self) -> None:
"""Nested dictionaries become dotted paths."""
input = {"foo": {"bar": "abc"}}
self.assertEqual({"foo.bar": "abc"}, _flatten_dict(input))

def test_non_string(self) -> None:
"""Non-string items are dropped."""
input: Dict[str, Any] = {
"woo": "woo",
"foo": True,
"bar": 1,
"baz": None,
"fuzz": [],
"boo": {},
}
self.assertEqual({"woo": "woo"}, _flatten_dict(input))


class PushRuleEvaluatorTestCase(unittest.TestCase):
def _get_evaluator(
self,
Expand Down