diff --git a/openslides_backend/action/actions/meeting/import_.py b/openslides_backend/action/actions/meeting/import_.py index f73ad9e6b..8763b254b 100644 --- a/openslides_backend/action/actions/meeting/import_.py +++ b/openslides_backend/action/actions/meeting/import_.py @@ -88,10 +88,9 @@ def update_instance(self, instance: Dict[str, Any]) -> Dict[str, Any]: # save blobs from mediafiles self.mediadata = [] for entry in meeting_json.get("mediafile", {}).values(): - if "blob" in entry: - self.mediadata.append( - (entry.pop("blob"), entry["id"], entry["mimetype"]) - ) + # mediafiles have "blob": None + if blob := entry.pop("blob", None): + self.mediadata.append((blob, entry["id"], entry["mimetype"])) # check datavalidation checker = Checker(data=meeting_json, mode="external") diff --git a/openslides_backend/models/checker.py b/openslides_backend/models/checker.py index fc427fd65..93db814d8 100644 --- a/openslides_backend/models/checker.py +++ b/openslides_backend/models/checker.py @@ -677,29 +677,29 @@ def check_calculated_fields( ) -> None: if collection != "mediafile": return - if model["is_directory"] and not model.get("parent_id"): - return + access_group_ids = model["access_group_ids"] parent_is_public = None parent_inherited_access_group_ids = None if model.get("parent_id"): - parent = self.find_model(collection, model.get("parent_id", 0)) - if parent: - parent_is_public = parent.get("is_public") - parent_inherited_access_group_ids = parent.get( - "inherited_access_group_ids" - ) - is_public, inherited_access_group_ids = calculate_inherited_groups_helper( - access_group_ids, parent_is_public, parent_inherited_access_group_ids + parent = self.find_model(collection, model["parent_id"]) + # relations are checked beforehand, so parent always exists + assert parent + parent_is_public = parent["is_public"] + parent_inherited_access_group_ids = parent["inherited_access_group_ids"] + is_public, inherited_access_group_ids = calculate_inherited_groups_helper( + access_group_ids, parent_is_public, parent_inherited_access_group_ids + ) + if is_public != model["is_public"]: + self.errors.append( + f"{collection}/{model['id']}: is_public is wrong. {is_public} != {model['is_public']}" + ) + if set(inherited_access_group_ids) != set( + model["inherited_access_group_ids"] or [] + ): + self.errors.append( + f"{collection}/{model['id']}: inherited_access_group_ids is wrong" ) - if is_public != model["is_public"]: - self.errors.append( - f"{collection}/{model['id']}: is_public is wrong. {is_public} != {model['is_public']}" - ) - if inherited_access_group_ids != model["inherited_access_group_ids"]: - self.errors.append( - f"{collection}/{model['id']}: inherited_access_group_ids is wrong" - ) def find_model(self, collection: str, id: int) -> Optional[Dict[str, Any]]: return self.data.get(collection, {}).get(str(id)) diff --git a/openslides_backend/models/helper.py b/openslides_backend/models/helper.py index f64170b71..093d084ca 100644 --- a/openslides_backend/models/helper.py +++ b/openslides_backend/models/helper.py @@ -1,25 +1,21 @@ -from typing import Any, Dict, List, Optional, Tuple +from typing import List, Optional, Tuple def calculate_inherited_groups_helper( access_group_ids: Optional[List[int]], parent_is_public: Optional[bool], parent_inherited_access_group_ids: Optional[List[int]], -) -> Tuple[bool, Optional[List[int]]]: - - parent: Dict[str, Any] = dict() - parent["inherited_access_group_ids"] = parent_inherited_access_group_ids - parent["is_public"] = parent_is_public - - if not parent["inherited_access_group_ids"]: - inherited_access_group_ids = access_group_ids - elif not access_group_ids: - inherited_access_group_ids = parent.get("inherited_access_group_ids", []) - else: +) -> Tuple[bool, List[int]]: + inherited_access_group_ids: List[int] + if parent_inherited_access_group_ids and access_group_ids: inherited_access_group_ids = [ - id_ - for id_ in access_group_ids - if id_ in parent.get("inherited_access_group_ids", []) + id_ for id_ in access_group_ids if id_ in parent_inherited_access_group_ids ] - is_public = not bool(inherited_access_group_ids) and bool(parent["is_public"]) + elif access_group_ids: + inherited_access_group_ids = access_group_ids + elif parent_inherited_access_group_ids: + inherited_access_group_ids = parent_inherited_access_group_ids + else: + inherited_access_group_ids = [] + is_public = not bool(inherited_access_group_ids) and parent_is_public is not False return is_public, inherited_access_group_ids diff --git a/openslides_backend/services/media/adapter.py b/openslides_backend/services/media/adapter.py index 0d1b167ce..b3d622470 100644 --- a/openslides_backend/services/media/adapter.py +++ b/openslides_backend/services/media/adapter.py @@ -19,7 +19,7 @@ def __init__(self, media_url: str, logging: LoggingModule) -> None: def _upload(self, file: str, id: int, mimetype: str, subpath: str) -> None: url = self.media_url + subpath + "/" payload = {"file": file, "id": id, "mimetype": mimetype} - self.logger.debug("Starting upload of file") + self.logger.debug(f"Starting upload of mediafile/{id} (mimetype: {mimetype})") self._handle_upload(url, payload, description="Upload of file: ") self.logger.debug("File successfully uploaded to the media service") @@ -42,8 +42,8 @@ def _handle_upload( ) -> None: try: response = requests.post(url, json=payload) - except requests.exceptions.ConnectionError: - msg = "Connect to mediaservice failed." + except requests.exceptions.ConnectionError as e: + msg = f"Connect to mediaservice failed. {e}" self.logger.debug(description + msg) raise MediaServiceException(msg) diff --git a/tests/system/action/meeting/test_clone.py b/tests/system/action/meeting/test_clone.py index e6a338acc..da16bcf8e 100644 --- a/tests/system/action/meeting/test_clone.py +++ b/tests/system/action/meeting/test_clone.py @@ -193,6 +193,7 @@ def test_clone_with_mediafile(self) -> None: "used_as_font_$_in_meeting_id": [], "used_as_logo_$_in_meeting_id": [], "mimetype": "text/plain", + "is_public": True, }, } ) diff --git a/tests/system/action/meeting/test_import.py b/tests/system/action/meeting/test_import.py index 1d5047f7a..c666eb31e 100644 --- a/tests/system/action/meeting/test_import.py +++ b/tests/system/action/meeting/test_import.py @@ -209,25 +209,14 @@ def create_request_data(self, datapart: Dict[str, Any]) -> Dict[str, Any]: ), }, "group": { - "1": { - "id": 1, - "meeting_id": 1, - "name": "testgroup", - "user_ids": [1], - "admin_group_for_meeting_id": 1, - "default_group_for_meeting_id": 1, - "permissions": [], - "mediafile_access_group_ids": [], - "mediafile_inherited_access_group_ids": [], - "read_comment_section_ids": [], - "write_comment_section_ids": [], - "read_chat_group_ids": [], - "write_chat_group_ids": [], - "poll_ids": [], - "used_as_motion_poll_default_id": None, - "used_as_assignment_poll_default_id": None, - "used_as_poll_default_id": None, - } + "1": self.get_group_data( + 1, + { + "user_ids": [1], + "admin_group_for_meeting_id": 1, + "default_group_for_meeting_id": 1, + }, + ) }, "motion_workflow": { "1": { @@ -294,9 +283,14 @@ def create_request_data(self, datapart: Dict[str, Any]) -> Dict[str, Any]: "used_as_default_$_in_meeting_id": [], } }, - **datapart, }, } + for collection, models in datapart.items(): + if collection not in data["meeting"]: + data["meeting"][collection] = models + else: + data["meeting"][collection].update(models) + needed_collections = ( "user", "meeting", @@ -382,6 +376,28 @@ def get_user_data(self, obj_id: int, data: Dict[str, Any] = {}) -> Dict[str, Any **data, } + def get_group_data(self, obj_id: int, data: Dict[str, Any] = {}) -> Dict[str, Any]: + return { + "id": obj_id, + "meeting_id": 1, + "name": "testgroup", + "user_ids": [], + "admin_group_for_meeting_id": None, + "default_group_for_meeting_id": None, + "permissions": [], + "mediafile_access_group_ids": [], + "mediafile_inherited_access_group_ids": [], + "read_comment_section_ids": [], + "write_comment_section_ids": [], + "read_chat_group_ids": [], + "write_chat_group_ids": [], + "poll_ids": [], + "used_as_motion_poll_default_id": None, + "used_as_assignment_poll_default_id": None, + "used_as_poll_default_id": None, + **data, + } + def get_motion_data(self, obj_id: int, data: Dict[str, Any] = {}) -> Dict[str, Any]: return { "id": obj_id, @@ -430,6 +446,34 @@ def get_motion_data(self, obj_id: int, data: Dict[str, Any] = {}) -> Dict[str, A **data, } + def get_mediafile_data( + self, obj_id: int, data: Dict[str, Any] = {} + ) -> Dict[str, Any]: + file_content = base64.b64encode(b"testtesttest").decode() + return { + "id": obj_id, + "meeting_id": 1, + "blob": file_content, + "title": "A.txt", + "is_directory": False, + "filesize": 3, + "filename": "A.txt", + "mimetype": "text/plain", + "pdf_information": {}, + "create_timestamp": 1584513771, + "is_public": True, + "access_group_ids": [], + "inherited_access_group_ids": [], + "parent_id": None, + "child_ids": [], + "list_of_speakers_id": None, + "projection_ids": [], + "attachment_ids": [], + "used_as_logo_$_in_meeting_id": [], + "used_as_font_$_in_meeting_id": [], + **data, + } + def test_no_meeting_collection(self) -> None: response = self.request( "meeting.import", {"committee_id": 1, "meeting": {"meeting": {}}} @@ -706,30 +750,7 @@ def test_use_blobs(self) -> None: file_content = base64.b64encode(b"testtesttest").decode() request_data = self.create_request_data( { - "mediafile": { - "1": { - "id": 1, - "meeting_id": 1, - "blob": file_content, - "title": "A.txt", - "is_directory": False, - "filesize": 3, - "filename": "A.txt", - "mimetype": "text/plain", - "pdf_information": {}, - "create_timestamp": 1584513771, - "is_public": True, - "access_group_ids": [], - "inherited_access_group_ids": [], - "parent_id": None, - "child_ids": [], - "list_of_speakers_id": None, - "projection_ids": [], - "attachment_ids": [], - "used_as_logo_$_in_meeting_id": [], - "used_as_font_$_in_meeting_id": [], - } - }, + "mediafile": {"1": self.get_mediafile_data(1)}, } ) request_data["meeting"]["meeting"]["1"]["mediafile_ids"] = [1] @@ -739,6 +760,68 @@ def test_use_blobs(self) -> None: assert mediafile.get("blob") is None self.media.upload_mediafile.assert_called_with(file_content, 1, "text/plain") + def test_inherited_access_group_ids_none(self) -> None: + request_data = self.create_request_data( + { + "mediafile": { + "1": self.get_mediafile_data( + 1, + { + "access_group_ids": None, + "inherited_access_group_ids": None, + }, + ), + }, + } + ) + request_data["meeting"]["meeting"]["1"]["mediafile_ids"] = [1] + response = self.request("meeting.import", request_data) + self.assert_status_code(response, 200) + + def test_inherited_access_group_ids_wrong_order(self) -> None: + request_data = self.create_request_data( + { + "group": { + "1": self.get_group_data( + 1, + { + "user_ids": [1], + "admin_group_for_meeting_id": 1, + "default_group_for_meeting_id": 1, + "mediafile_access_group_ids": [1], + "mediafile_inherited_access_group_ids": [1], + }, + ), + "2": self.get_group_data( + 2, + { + "mediafile_access_group_ids": [1], + "mediafile_inherited_access_group_ids": [1], + }, + ), + }, + "mediafile": { + "1": self.get_mediafile_data( + 1, + { + "is_public": False, + "access_group_ids": [1, 2], + "inherited_access_group_ids": [1, 2], + }, + ), + }, + } + ) + request_data["meeting"]["meeting"]["1"]["mediafile_ids"] = [1] + request_data["meeting"]["meeting"]["1"]["group_ids"] = [1, 2] + # try both orders, both should work + response = self.request("meeting.import", request_data) + self.assert_status_code(response, 200) + # other order + request_data["meeting"]["mediafile"]["1"]["inherited_access_group_ids"] = [2, 1] + response = self.request("meeting.import", request_data) + self.assert_status_code(response, 200) + def test_meeting_user_ids(self) -> None: # Calculated field. # User/1 is in user_ids, because calling user is added @@ -848,29 +931,13 @@ def test_logo_dollar_id(self) -> None: request_data = self.create_request_data( { "mediafile": { - "3": { - "id": 3, - "meeting_id": 1, - "used_as_logo_$_in_meeting_id": ["web_header"], - "used_as_logo_$web_header_in_meeting_id": 1, - "title": "A.txt", - "is_directory": False, - "filesize": 3, - "filename": "A.txt", - "mimetype": "text/plain", - "pdf_information": {}, - "create_timestamp": 1584513771, - "is_public": True, - "access_group_ids": [], - "inherited_access_group_ids": [], - "parent_id": None, - "child_ids": [], - "list_of_speakers_id": None, - "projection_ids": [], - "attachment_ids": [], - "used_as_font_$_in_meeting_id": [], - "blob": "bla", - } + "3": self.get_mediafile_data( + 3, + { + "used_as_logo_$_in_meeting_id": ["web_header"], + "used_as_logo_$web_header_in_meeting_id": 1, + }, + ) } } ) @@ -888,50 +955,26 @@ def test_is_public_error(self) -> None: request_data = self.create_request_data( { "mediafile": { - "1": { - "id": 3, - "meeting_id": 1, - "used_as_logo_$_in_meeting_id": ["web_header"], - "used_as_logo_$web_header_in_meeting_id": 1, - "title": "A.txt", - "is_directory": False, - "filesize": 3, - "filename": "A.txt", - "mimetype": "text/plain", - "pdf_information": {}, - "create_timestamp": 1584513771, - "is_public": True, - "access_group_ids": [], - "inherited_access_group_ids": [], - "parent_id": 2, - "child_ids": [], - "list_of_speakers_id": None, - "projection_ids": [], - "attachment_ids": [], - "used_as_font_$_in_meeting_id": [], - "blob": "bla", - }, - "2": { - "id": 2, - "meeting_id": 1, - "used_as_logo_$_in_meeting_id": [], - "title": "A.txt", - "is_directory": True, - "filesize": 3, - "filename": "A.txt", - "mimetype": "text/plain", - "pdf_information": {}, - "create_timestamp": 1584513771, - "is_public": False, - "access_group_ids": [], - "inherited_access_group_ids": [], - "parent_id": None, - "child_ids": [3], - "list_of_speakers_id": None, - "projection_ids": [], - "attachment_ids": [], - "used_as_font_$_in_meeting_id": [], - }, + "3": self.get_mediafile_data( + 3, + { + "used_as_logo_$_in_meeting_id": ["web_header"], + "used_as_logo_$web_header_in_meeting_id": 1, + "parent_id": 2, + }, + ), + "2": self.get_mediafile_data( + 2, + { + "title": "dir", + "is_directory": True, + "filesize": 0, + "filename": None, + "mimetype": None, + "is_public": False, + "child_ids": [3], + }, + ), } } ) @@ -1046,29 +1089,12 @@ def test_field_check(self) -> None: request_data = self.create_request_data( { "mediafile": { - "1": { - "id": 1, - "foobar": "test this", - "meeting_id": 1, - "title": "A.txt", - "is_directory": False, - "filesize": 3, - "filename": "A.txt", - "mimetype": "text/plain", - "pdf_information": {}, - "create_timestamp": 1584513771, - "is_public": True, - "access_group_ids": [], - "inherited_access_group_ids": [], - "parent_id": None, - "child_ids": [], - "list_of_speakers_id": None, - "projection_ids": [], - "attachment_ids": [], - "used_as_logo_$_in_meeting_id": [], - "used_as_font_$_in_meeting_id": [], - "blob": "bla", - } + "1": self.get_mediafile_data( + 1, + { + "foobar": "test this", + }, + ) } } )