Skip to content

Commit

Permalink
Some cleanup & typing improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
jsangmeister committed Jul 25, 2023
1 parent e18431e commit a6866b2
Show file tree
Hide file tree
Showing 11 changed files with 88 additions and 69 deletions.
22 changes: 8 additions & 14 deletions openslides_backend/action/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,8 @@
from ..shared.patterns import (
FullQualifiedId,
collection_from_fqid,
field_from_fqfield,
fqid_and_field_from_fqfield,
fqid_from_collection_and_id,
fqid_from_fqfield,
transform_to_fqids,
)
from ..shared.typing import DeletedModel, HistoryInformation
Expand Down Expand Up @@ -341,33 +340,28 @@ def handle_relation_updates(
relation_updates = self.relation_manager.get_relation_updates(
self.model, instance, self.name
)

return self.handle_relation_updates_helper(relation_updates)

def handle_relation_updates_helper(
self,
relation_updates: RelationUpdates,
) -> Iterable[Event]:
fields: Optional[Dict[str, Any]]
for fqfield, data in relation_updates.items():
list_fields: Optional[ListFields] = None
fields: Dict[str, Any] = {}
list_fields: ListFields = {}
fqid, field = fqid_and_field_from_fqfield(fqfield)
if data["type"] in ("add", "remove"):
data = cast(FieldUpdateElement, data)
fields = {field_from_fqfield(fqfield): data["value"]}
fields[field] = data["value"]
elif data["type"] == "list_update":
data = cast(ListUpdateElement, data)
fields = None
list_fields_tmp = {}
if data["add"]:
list_fields_tmp["add"] = {field_from_fqfield(fqfield): data["add"]}
list_fields["add"] = {field: data["add"]}
if data["remove"]:
list_fields_tmp["remove"] = {
field_from_fqfield(fqfield): data["remove"]
}
list_fields = cast(ListFields, list_fields_tmp)
list_fields["remove"] = {field: data["remove"]}
yield self.build_event(
EventType.Update,
fqid_from_fqfield(fqfield),
fqid,
fields,
list_fields,
)
Expand Down
28 changes: 16 additions & 12 deletions openslides_backend/action/actions/agenda_item/numbering.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from openslides_backend.services.datastore.commands import GetManyRequest

from ....models.models import AgendaItem
from ....permissions.permissions import Permissions
from ....shared.filters import FilterOperator
from ....shared.patterns import fqid_from_collection_and_id
from ...generics.update import UpdateAction
from ...mixins.singular_action_mixin import SingularActionMixin
Expand All @@ -25,22 +26,25 @@ def get_updated_instances(self, action_data: ActionData) -> ActionData:
# Fetch all agenda items for this meeting from datastore.
# Action data is an iterable with exactly one item
instance = next(iter(action_data))
meeting_id = instance["meeting_id"]
agenda_items = self.datastore.filter(
collection=self.model.collection,
filter=FilterOperator("meeting_id", "=", meeting_id),
mapped_fields=["id", "item_number", "parent_id", "weight", "type"],
)

# Build agenda tree and get new numbers
# Fetch data
meeting = self.datastore.get(
fqid_from_collection_and_id("meeting", meeting_id),
["agenda_numeral_system", "agenda_number_prefix"],
lock_result=False,
fqid_from_collection_and_id("meeting", instance["meeting_id"]),
["agenda_item_ids", "agenda_numeral_system", "agenda_number_prefix"],
)
agenda_items = self.datastore.get_many(
[
GetManyRequest(
"agenda_item",
meeting.get("agenda_item_ids", []),
["id", "item_number", "parent_id", "weight", "type"],
)
]
).get("agenda_item", {})
numeral_system = meeting.get("agenda_numeral_system", "arabic")
agenda_number_prefix = meeting.get("agenda_number_prefix")
# Build agenda tree and get new numbers
result = AgendaTree(agenda_items.values()).number_all(
numeral_system=numeral_system, agenda_number_prefix=agenda_number_prefix
numeral_system, agenda_number_prefix
)
return [{"id": key, "item_number": val} for key, val in result.items()]
8 changes: 2 additions & 6 deletions openslides_backend/action/actions/group/delete.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from collections import defaultdict
from typing import Any, Dict, Iterable, List, Optional, Set, Union, cast
from typing import Any, Dict, Iterable, List, Optional, Set

from openslides_backend.shared.exceptions import ActionException

Expand Down Expand Up @@ -102,11 +102,7 @@ def handle_relation_updates(
EventType.Update,
fqid_from_collection_and_id("group", group_id),
list_fields={
"add": {
"mediafile_inherited_access_group_ids": cast(
List[Union[int, str]], mediafile_ids
)
},
"add": {"mediafile_inherited_access_group_ids": mediafile_ids},
"remove": {},
},
)
Expand Down
6 changes: 3 additions & 3 deletions openslides_backend/action/actions/meeting/import_.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import re
import time
from collections import OrderedDict, defaultdict
from typing import Any, Dict, Iterable, List, Optional, Tuple, Union, cast
from typing import Any, Dict, Iterable, List, Optional, Tuple, cast

from openslides_backend.action.actions.meeting.mixins import MeetingPermissionMixin
from openslides_backend.migrations import get_backend_migration_index
Expand Down Expand Up @@ -35,7 +35,7 @@
from openslides_backend.shared.schema import models_map_object
from openslides_backend.shared.util import ONE_ORGANIZATION_FQID

from ....shared.interfaces.event import Event, ListFields
from ....shared.interfaces.event import Event, ListFields, ListFieldsValue
from ....shared.util import ONE_ORGANIZATION_ID
from ...action import RelationUpdates
from ...mixins.singular_action_mixin import SingularActionMixin
Expand Down Expand Up @@ -582,7 +582,7 @@ def create_events(
)

# add meetings to organization if set in meeting
adder: Dict[str, List[Union[int, str]]] = {}
adder: ListFieldsValue = {}
if meeting.get("is_active_in_organization_id"):
adder["active_meeting_ids"] = [meeting_id]
if meeting.get("template_for_organization_id"):
Expand Down
28 changes: 10 additions & 18 deletions openslides_backend/action/relations/single_relation_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def __init__(
self.field = field
self.field_name = field_name
self.instance = instance
self.chained_fields: List[Dict[str, Any]] = []
self.chained_fqids: List[FullQualifiedId] = []

def get_reverse_field(self, collection: Collection) -> BaseRelationField:
"""
Expand Down Expand Up @@ -163,7 +163,7 @@ def perform(self) -> RelationFieldUpdates:
rels,
related_name,
)
for fqfield, rel_update in result.items():
for rel_update in result.values():
# transform fqids back to ids
if not isinstance(related_field, BaseGenericRelationField):
modified_element = rel_update["modified_element"]
Expand Down Expand Up @@ -192,22 +192,19 @@ def perform(self) -> RelationFieldUpdates:
result_template_field = self.prepare_result_template_field(result)
final.update(result_template_field)

for chained_field in self.chained_fields:
handler = self.build_handler_from_chained_field(chained_field)
for fqid in self.chained_fqids:
handler = self.build_handler_from_chained_fqid(fqid)
result = handler.perform()
final.update(result)
return final

def build_handler_from_chained_field(self, chained_field: Dict[str, Any]): # type: ignore
"""
"field": self.field.to,
"fqid": fqid,
"origin_modified_fqid": own_fqid,
"""
collection = collection_from_fqid(chained_field["fqid"])
def build_handler_from_chained_fqid(
self, fqid: FullQualifiedId
) -> "SingleRelationHandler":
collection = collection_from_fqid(fqid)
field_name = self.get_related_name(collection)
field = self.get_reverse_field(collection)
instance = self.datastore.get(chained_field["fqid"], ["id", field_name])
instance = self.datastore.get(fqid, ["id", field_name])
instance[field_name] = None
return SingleRelationHandler(
self.datastore,
Expand Down Expand Up @@ -315,12 +312,7 @@ def prepare_result(
continue
if rel[related_name] and self.get_field_type() in ("1:1", "m:1"):
assert len(rel[related_name]) == 1
self.chained_fields.append(
{
"field": self.field.to,
"fqid": fqid,
}
)
self.chained_fqids.append(fqid)
new_value = [own_fqid]
else:
new_value = rel[related_name] + [own_fqid]
Expand Down
4 changes: 2 additions & 2 deletions openslides_backend/services/auth/adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
from authlib import (
ANONYMOUS_USER,
AUTHENTICATION_HEADER,
AUTHORIZATION_HEADER,
COOKIE_NAME,
AuthenticateException,
AuthHandler,
InvalidCredentialsException,
)
from authlib.constants import AUTHORIZATION_HEADER

from ...shared.exceptions import AuthenticationException
from ...shared.interfaces.logging import LoggingModule
Expand All @@ -35,7 +35,7 @@ def authenticate(
Returns a new access token, too, if one is received from auth service.
"""

access_token = headers.get(AUTHENTICATION_HEADER, None)
access_token = headers.get(AUTHENTICATION_HEADER)
cookie = cookies.get(COOKIE_NAME, "")
self.logger.debug(
f"Start request to authentication service with the following data: access_token: {headers}, cookie: {cookie}"
Expand Down
23 changes: 12 additions & 11 deletions openslides_backend/shared/interfaces/event.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from enum import Enum
from typing import Any, Dict, List, Optional, TypedDict, Union
from typing import Any, Dict, List, TypedDict, Union

from typing_extensions import NotRequired

from ..patterns import FullQualifiedId

Expand All @@ -13,21 +15,20 @@ def __repr__(self) -> str:
return repr(self.value)


ListFields = TypedDict(
"ListFields",
{
"add": Dict[str, List[Union[int, str]]],
"remove": Dict[str, List[Union[int, str]]],
},
)
ListFieldsValue = Dict[str, Union[List[int], List[str]]]


class ListFields(TypedDict):
add: NotRequired[ListFieldsValue]
remove: NotRequired[ListFieldsValue]


class Event(TypedDict, total=False):
class Event(TypedDict):
"""
Event as part of a write request element.
"""

type: EventType
fqid: FullQualifiedId
fields: Optional[Dict[str, Any]]
list_fields: Optional[ListFields]
fields: NotRequired[Dict[str, Any]]
list_fields: NotRequired[ListFields]
4 changes: 4 additions & 0 deletions openslides_backend/shared/patterns.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ def id_from_fqfield(fqfield: FullQualifiedField) -> int:
return int(str(fqfield).split(KEYSEPARATOR)[1])


def fqid_and_field_from_fqfield(fqfield: str) -> Tuple[str, str]:
return cast(Tuple[str, str], fqfield.rsplit(KEYSEPARATOR, 1))


def collectionfield_and_fqid_from_fqfield(fqfield: str) -> Tuple[str, str]:
parts = fqfield.split(KEYSEPARATOR)
return f"{parts[0]}{KEYSEPARATOR}{parts[2]}", f"{parts[0]}{KEYSEPARATOR}{parts[1]}"
Expand Down
30 changes: 29 additions & 1 deletion tests/system/action/agenda_item/test_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ def test_create_parent_weight(self) -> None:
{
"meeting/1": {"is_active_in_organization_id": 1},
"topic/1": {"meeting_id": 1},
"topic/2": {"meeting_id": 1},
"agenda_item/42": {"comment": "test", "meeting_id": 1, "weight": 10},
}
)
Expand All @@ -87,7 +88,7 @@ def test_create_parent_weight(self) -> None:
"parent_id": 42,
},
{
"content_object_id": "topic/1",
"content_object_id": "topic/2",
"parent_id": 42,
},
],
Expand All @@ -100,6 +101,33 @@ def test_create_parent_weight(self) -> None:
self.assertEqual(agenda_item["parent_id"], 42)
self.assertEqual(agenda_item["weight"], 2)

def test_create_same_content_object(self) -> None:
self.set_models(
{
"meeting/1": {"is_active_in_organization_id": 1},
"topic/1": {"meeting_id": 1},
}
)
response = self.request_multi(
"agenda_item.create",
[
{
"content_object_id": "topic/1",
},
{
"content_object_id": "topic/1",
},
],
)
# This should not work! The relation handling is broken here
self.assert_status_code(response, 200)
agenda_item = self.get_model("agenda_item/1")
self.assertEqual(agenda_item["content_object_id"], "topic/1")
agenda_item = self.get_model("agenda_item/2")
self.assertEqual(agenda_item["content_object_id"], "topic/1")
topic = self.get_model("topic/1")
self.assertEqual(topic["agenda_item_id"], 2)

def test_create_content_object_does_not_exist(self) -> None:
response = self.request("agenda_item.create", {"content_object_id": "topic/1"})
self.assert_status_code(response, 400)
Expand Down
2 changes: 1 addition & 1 deletion tests/system/relations/test_generic_relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ def test_generic_O2O_empty(self) -> None:
assert result == expected

def test_generic_O2O_replace(self) -> None:
self.create_model("fake_model_a/1", {})
self.set_models(
{
"fake_model_a/1": {},
"fake_model_a/2": {"fake_model_b_generic_oo": 3},
"fake_model_b/3": {"fake_model_a_generic_oo": "fake_model_a/2"},
}
Expand Down
2 changes: 1 addition & 1 deletion tests/system/relations/test_relations.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ def test_O2O_empty(self) -> None:
assert result == expected

def test_O2O_replace(self) -> None:
self.create_model("fake_model_a/1", {})
self.set_models(
{
"fake_model_a/1": {},
"fake_model_a/2": {"fake_model_b_oo": 3},
"fake_model_b/3": {"fake_model_a_oo": 2},
}
Expand Down

0 comments on commit a6866b2

Please sign in to comment.