[rel-DB] Complete checker tests

openslides_backend/action/actions/meeting/clone.py

@@ -94,7 +94,9 @@ def check_permissions(self, instance: dict[str, Any]) -> None:
         MeetingPermissionMixin.check_permissions(self, instance)
 
     def update_instance(self, instance: dict[str, Any]) -> dict[str, Any]:
-        meeting_json = export_meeting(self.datastore, instance["meeting_id"], True)
+        meeting_json = export_meeting(
+            self.datastore, instance["meeting_id"], True, True
+        )
         instance["meeting"] = meeting_json
         additional_user_ids = instance.pop("user_ids", None) or []
         additional_admin_ids = instance.pop("admin_ids", None) or []

openslides_backend/models/checker.py

@@ -1,7 +1,6 @@
-import re
 from collections.abc import Callable, Iterable
 from datetime import datetime
-from decimal import Decimal, InvalidOperation
+from decimal import Decimal
 from math import floor
 from typing import Any, cast
 
@@ -38,6 +37,7 @@
     DECIMAL_PATTERN,
     EXTENSION_REFERENCE_IDS_PATTERN,
     collection_and_id_from_fqid,
+    is_fqid,
 )
 from openslides_backend.shared.schema import (
     models_map_object,
@@ -83,6 +83,15 @@ def check_string(value: Any) -> bool:
     return value is None or isinstance(value, str)
 
 
+def check_fqid(value: Any) -> bool:
+    if value is None:
+        return True
+    if not is_fqid(value):
+        return False
+    collection, _id = collection_and_id_from_fqid(value)
+    return collection in set(model_registry.keys())
+
+
 def check_color(value: Any) -> bool:
     return value is None or bool(isinstance(value, str) and COLOR_PATTERN.match(value))
 
@@ -103,6 +112,10 @@ def check_string_list(value: Any) -> bool:
     return check_x_list(value, check_string)
 
 
+def check_fqid_list(value: Any) -> bool:
+    return check_x_list(value, check_fqid)
+
+
 def check_number_list(value: Any) -> bool:
     return check_x_list(value, check_number)
 
@@ -150,14 +163,14 @@ def check_timestamp(value: Any) -> bool:
     CharField: check_string,
     HTMLStrictField: check_string,
     HTMLPermissiveField: check_string,
-    GenericRelationField: check_string,
+    GenericRelationField: check_fqid,
     IntegerField: check_number,
     TimestampField: check_timestamp,
     RelationField: check_number,
     FloatField: check_float,
     BooleanField: check_boolean,
     CharArrayField: check_string_list,
-    GenericRelationListField: check_string_list,
+    GenericRelationListField: check_fqid_list,
     NumberArrayField: check_number_list,
     RelationListField: check_number_list,
     DecimalField: check_decimal,
@@ -255,8 +268,7 @@ def get_fields(self, collection: str) -> Iterable[Field]:
 
     def run_check(self) -> None:
         self.check_json()
-        # TODO reenable when import migration works
-        # self.check_migration_index()
+        self.check_migration_index()
         self.check_collections()
         for collection, models in self.data.items():
             if collection.startswith("_"):
@@ -304,7 +316,6 @@ def check_normal_fields(self, model: dict[str, Any], collection: str) -> bool:
         all_collection_fields = {
             field.get_own_field_name() for field in self.get_fields(collection)
         }
-        # TODO: remove duplication: required is also checked in check_types
         required_or_default_collection_fields = {
             field.get_own_field_name()
             for field in self.get_fields(collection)
@@ -333,9 +344,6 @@ def check_normal_fields(self, model: dict[str, Any], collection: str) -> bool:
                     error = f"{collection}/{model['id']}/{fieldname}: {str(e)}"
                     self.errors.append(error)
                     errors = True
-                except InvalidOperation:
-                    # invalide decimal json, will be checked at check_types
-                    pass
         return errors
 
     def fix_missing_default_values(
@@ -365,6 +373,7 @@ def check_types(self, model: dict[str, Any], collection: str) -> None:
                     f"TODO implement check for field type {field_type}"
                 )
 
+            # TODO: move the validation logic to `field.validate` methods. Merge with the check from check_normal_fields().
             if not checker(model[field]):
                 error = f"{collection}/{model['id']}/{field}: Type error: Type is not {field_type}"
                 self.errors.append(error)
@@ -412,31 +421,22 @@ def check_special_fields(self, model: dict[str, Any], collection: str) -> None:
                     html, ALLOWED_HTML_TAGS_STRICT
                 ):
                     self.errors.append(msg + f"Invalid html in {key}")
-        if "recommendation_extension" in model:
-            basemsg = (
-                f"{collection}/{model['id']}/recommendation_extension: Relation Error: "
-            )
-            RECOMMENDATION_EXTENSION_REFERENCE_IDS_PATTERN = re.compile(
-                r"\[(?P<fqid>\w+/\d+)\]"
-            )
-            recommendation_extension = model["recommendation_extension"]
-            if recommendation_extension is None:
-                recommendation_extension = ""
 
-            possible_rerids = RECOMMENDATION_EXTENSION_REFERENCE_IDS_PATTERN.findall(
-                recommendation_extension
-            )
-            for fqid_str in possible_rerids:
-                re_collection, re_id_ = collection_and_id_from_fqid(fqid_str)
-                if re_collection != "motion":
-                    self.errors.append(
-                        basemsg + f"Found {fqid_str} but only motion is allowed."
-                    )
-                if not self.find_model(re_collection, int(re_id_)):
-                    self.errors.append(
-                        basemsg
-                        + f"Found {fqid_str} in recommendation_extension but not in models."
-                    )
+        for field_name in ["state_extension", "recommendation_extension"]:
+            basemsg = f"{collection}/{model['id']}/{field_name}: Relation Error:"
+            if value := model.get(field_name):
+                matches = EXTENSION_REFERENCE_IDS_PATTERN.findall(value)
+                for fqid in matches:
+                    re_collection, re_id = collection_and_id_from_fqid(fqid)
+                    if re_collection != "motion":
+                        self.errors.append(
+                            basemsg + f" Found {fqid} but only motion is allowed."
+                        )
+                    if not self.find_model(re_collection, int(re_id)):
+                        self.errors.append(
+                            basemsg
+                            + f" Found {fqid} in {field_name} but not in models."
+                        )
 
     def check_relations(self, model: dict[str, Any], collection: str) -> None:
         for field in model.keys():
@@ -451,7 +451,7 @@ def check_relation(
         self, model: dict[str, Any], collection: str, field: str
     ) -> None:
         field_type = self.get_type_from_collection(field, collection)
-        basemsg = f"{collection}/{model['id']}/{field}: Relation Error: "
+        basemsg = f"{collection}/{model['id']}/{field}: Relation Error:"
 
         if collection == "user" and field == "organization_id":
             return
@@ -478,7 +478,7 @@ def check_relation(
                 )
         elif isinstance(field_type, RelationListField):
             foreign_ids = model[field]
-            if not foreign_ids:
+            if not foreign_ids or not isinstance(foreign_ids, list):
                 return
 
             foreign_collection, foreign_field = self.get_to(field, collection)
@@ -513,6 +513,7 @@ def check_relation(
                     foreign_field,
                     basemsg,
                 )
+            # TODO: cleanup. Unreachable code (mode and collection are checked in split_fqid), but error message there is not too useful
             elif self.mode == "external":
                 self.errors.append(
                     f"{basemsg} points to {foreign_collection}/{foreign_id}, which is not allowed in an external import."
@@ -541,24 +542,6 @@ def check_relation(
                         f"{basemsg} points to {foreign_collection}/{foreign_id}, which is not allowed in an external import."
                     )
 
-        elif collection == "motion":
-            for prefix in ("state", "recommendation"):
-                if field == f"{prefix}_extension" and (
-                    value := model.get(f"{prefix}_extension")
-                ):
-                    matches = EXTENSION_REFERENCE_IDS_PATTERN.findall(value)
-                    for fqid in matches:
-                        re_collection, re_id = collection_and_id_from_fqid(fqid)
-                        if re_collection != "motion":
-                            self.errors.append(
-                                basemsg + f"Found {fqid} but only motion is allowed."
-                            )
-                        if not self.find_model(re_collection, int(re_id)):
-                            self.errors.append(
-                                basemsg
-                                + f"Found {fqid} in {prefix}_extension but not in models."
-                            )
-
     def get_to(self, field: str, collection: str) -> tuple[str, str | None]:
         field_type = cast(
             BaseRelationField, self.get_model(collection).get_field(field)
@@ -586,8 +569,8 @@ def check_calculated_fields(
             source_parent = self.find_model("mediafile", source_model["parent_id"])
             # relations are checked beforehand, so parent always exists
             assert source_parent
-            parent_ids = set(meeting.get("meeting_mediafile_ids", [])).intersection(
-                source_parent.get("meeting_mediafile_ids", [])
+            parent_ids = set(meeting.get("meeting_mediafile_ids") or []).intersection(
+                set(source_parent.get("meeting_mediafile_ids") or [])
             )
             assert len(parent_ids) <= 1
             if len(parent_ids):
@@ -668,21 +651,21 @@ def check_reverse_relation(
         if error:
             self.errors.append(
                 f"{basemsg} points to {foreign_collection}/{foreign_id}/{actual_foreign_field},"
-                " but the reverse relation for it is corrupt"
+                " but the reverse relation for it is corrupt."
             )
 
     def split_fqid(self, fqid: str) -> tuple[str, int]:
         try:
-            collection, _id = fqid.split("/")
-            id = int(_id)
+            collection, _id = collection_and_id_from_fqid(fqid)
+            assert collection
             if self.mode == "external" and collection not in self.allowed_collections:
-                raise CheckException(f"Fqid {fqid} has an invalid collection")
-            return collection, id
-        except (ValueError, AttributeError):
+                raise CheckException(f"Fqid {fqid} has an invalid collection.")
+            return collection, _id
+        except (ValueError, AttributeError, AssertionError, IndexError):
             raise CheckException(f"Fqid {fqid} is malformed")
 
-    def split_collectionfield(self, collectionfield: str) -> tuple[str, str]:
-        collection, field = collectionfield.split("/")
+    def split_collectionfield(self, collectionfield: str) -> tuple[str, int]:
+        collection, field = collection_and_id_from_fqid(collectionfield)
         if collection not in self.allowed_collections:
             raise CheckException(
                 f"Collectionfield {collectionfield} has an invalid collection"
@@ -704,7 +687,7 @@ def get_to_generic_case(
             if foreign_collection not in to.keys():
                 raise CheckException(
                     f"The collection {foreign_collection} is not supported "
-                    "as a reverse relation in {collection}/{field}"
+                    f"as a reverse relation in {collection}/{field}."
                 )
             return to[foreign_collection]
 
@@ -714,5 +697,5 @@ def get_to_generic_case(
                 return f
 
         raise CheckException(
-            f"The collection {foreign_collection} is not supported as a reverse relation in {collection}/{field}"
+            f"The collection {foreign_collection} is not supported as a reverse relation in {collection}/{field}."
         )

openslides_backend/presenter/check_database.py

@@ -36,7 +36,7 @@ def check_meetings(datastore: Database, meeting_id: int | None) -> dict[int, str
 
     errors: dict[int, str] = {}
     for meeting_id in meeting_ids:
-        export = export_meeting(datastore, meeting_id, True)
+        export = export_meeting(datastore, meeting_id, True, True)
         try:
             Checker(
                 data=export,

openslides_backend/presenter/check_database_all.py

@@ -3,6 +3,7 @@
 import fastjsonschema
 
 from openslides_backend.migrations import get_backend_migration_index
+from openslides_backend.shared.export_helper import get_fields_for_export
 from openslides_backend.shared.patterns import is_reserved_field
 
 from ..models.checker import Checker, CheckException
@@ -32,7 +33,11 @@ def check_everything(datastore: Database) -> None:
             str(id): {
                 field: value
                 for field, value in model.items()
-                if not is_reserved_field(field)
+                if (
+                    field in get_fields_for_export(collection)
+                    and not is_reserved_field(field)
+                    and value is not None
+                )
             }
             for id, model in models.items()
         }

openslides_backend/shared/export_helper.py

@@ -3,6 +3,7 @@
 
 from openslides_backend.migrations import get_backend_migration_index
 from openslides_backend.shared.patterns import is_reserved_field
+from openslides_backend.shared.util import ONE_ORGANIZATION_FQID, ONE_ORGANIZATION_ID
 
 from ..models.base import model_registry
 from ..models.fields import (
@@ -23,7 +24,10 @@
 
 
 def export_meeting(
-    datastore: Database, meeting_id: int, internal_target: bool = False
+    datastore: Database,
+    meeting_id: int,
+    internal_target: bool = False,
+    update_mediafiles: bool = False,
 ) -> dict[str, Any]:
     export: dict[str, Any] = {}
 
@@ -56,6 +60,63 @@ def export_meeting(
         results = datastore.get_many(
             get_many_requests, lock_result=False, use_changed_models=False
         )
+        # update_mediafiles_for_internal_calls
+        if update_mediafiles and len(
+            mediafile_ids := results.get("mediafile", {}).keys()
+        ) != len(meeting_mediafiles := results.get("meeting_mediafile", {})):
+            mm_with_unknown_mediafiles = {
+                mm_id: mm_data
+                for mm_id, mm_data in meeting_mediafiles.items()
+                if mm_data["mediafile_id"] not in mediafile_ids
+            }
+            unknown_mediafiles: dict[int, dict[str, Any]] = {}
+            next_file_ids = [
+                mm["mediafile_id"] for mm in mm_with_unknown_mediafiles.values()
+            ]
+            while next_file_ids:
+                unknown_mediafiles.update(
+                    datastore.get_many(
+                        [
+                            GetManyRequest(
+                                "mediafile",
+                                next_file_ids,
+                                [
+                                    "id",
+                                    "owner_id",
+                                    "published_to_meetings_in_organization_id",
+                                    "parent_id",
+                                    "child_ids",
+                                ],
+                            ),
+                        ],
+                        use_changed_models=False,
+                    )["mediafile"]
+                )
+                next_file_ids = list(
+                    {
+                        parent_id
+                        for m in unknown_mediafiles.values()
+                        if (parent_id := m.get("parent_id"))
+                    }
+                    - set(unknown_mediafiles)
+                )
+            for mm_id, mm_data in mm_with_unknown_mediafiles.items():
+                mediafile_id = mm_data["mediafile_id"]
+                mediafile = unknown_mediafiles.get(mediafile_id)
+                if (
+                    mediafile
+                    and mediafile["owner_id"] == ONE_ORGANIZATION_FQID
+                    and mediafile["published_to_meetings_in_organization_id"]
+                    == ONE_ORGANIZATION_ID
+                ):
+                    mediafile["meeting_mediafile_ids"] = [mm_id]
+                    results.setdefault("mediafile", {})[mediafile_id] = mediafile
+                    while (
+                        parent_id := mediafile.get("parent_id")
+                    ) and parent_id not in results["mediafile"]:
+                        mediafile = unknown_mediafiles[parent_id]
+                        results["mediafile"][parent_id] = mediafile
+
     else:
         results = {}
 

tests/system/action/organization/test_initial_import.py

@@ -174,7 +174,7 @@ def test_initial_import_wrong_type(self) -> None:
             response.json["message"],
         )
         self.assertIn(
-            "organization/1/theme_id: Relation Error:  points to theme/test/theme_for_organization_id, but the reverse relation for it is corrupt",
+            "organization/1/theme_id: Relation Error: points to theme/test/theme_for_organization_id, but the reverse relation for it is corrupt.",
             response.json["message"],
         )
 
@@ -186,11 +186,11 @@ def test_initial_import_wrong_relation(self) -> None:
         )
         self.assert_status_code(response, 400)
         self.assertIn(
-            "Relation Error:  points to theme/666/theme_for_organization_id, but the reverse relation for it is corrupt",
+            "Relation Error: points to theme/666/theme_for_organization_id, but the reverse relation for it is corrupt.",
             response.json["message"],
         )
         self.assertIn(
-            "Relation Error:  points to organization/1/theme_id, but the reverse relation for it is corrupt",
+            "Relation Error: points to organization/1/theme_id, but the reverse relation for it is corrupt.",
             response.json["message"],
         )