From 55260ba90a7264611d9621708b4144580a6e9049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20K=C3=BCnzel?= <simonk@fsmpi.rwth-aachen.de> Date: Wed, 9 Oct 2024 01:28:54 +0200 Subject: [PATCH] Rework load options --- src/videoag_common/api_object/__init__.py | 1 + src/videoag_common/api_object/object.py | 17 ++- src/videoag_common/objects/__init__.py | 22 ++- src/videoag_common/objects/changelog.py | 23 ++-- src/videoag_common/objects/course.py | 104 ++++---------- src/videoag_common/objects/medium.py | 128 ++++++++++++++++-- src/videoag_common/objects/site.py | 34 +---- .../objects/view_permissions.py | 12 +- 8 files changed, 201 insertions(+), 140 deletions(-) diff --git a/src/videoag_common/api_object/__init__.py b/src/videoag_common/api_object/__init__.py index 40d0354..8617f6f 100644 --- a/src/videoag_common/api_object/__init__.py +++ b/src/videoag_common/api_object/__init__.py @@ -1,6 +1,7 @@ from .fields import * from .object import ( ApiObject, + RelationshipLoadMarker, DeletableApiObject, VisibilityApiObject, api_mapped, diff --git a/src/videoag_common/api_object/object.py b/src/videoag_common/api_object/object.py index f392e21..1c4f79f 100644 --- a/src/videoag_common/api_object/object.py +++ b/src/videoag_common/api_object/object.py @@ -60,6 +60,10 @@ class ApiValueObject: pass +class RelationshipLoadMarker: + pass + + class ApiObject: id: Mapped[int] = api_mapped( mapped_column(nullable=False, primary_key=True, autoincrement=True, sort_order=-1), @@ -109,8 +113,6 @@ class ApiObject: kwargs should all have default values and are only intended for optimization (e.g. prevent checking from both directions of a relationship) """ - if len(kwargs) > 0: - raise Exception(f"Unknown kwargs: {kwargs}") if isinstance(self, type): return sql.True_() else: @@ -134,8 +136,15 @@ class ApiObject: return sql.select(cls).where(cls.is_active()) @classmethod - def select(cls, is_mod: bool, **kwargs): - return cls.basic_select().where(cls.has_access(is_mod, no_is_active_conditions=True, **kwargs)) + def select(cls, is_mod: bool, to_load: list[RelationshipLoadMarker], **kwargs): + return (cls.basic_select() + .where(cls.has_access(is_mod, no_is_active_conditions=True, **kwargs)) + .options(*cls.load_options(is_mod, to_load, **kwargs)) + ) + + @classmethod + def load_options(cls, is_mod: bool, to_load: list[RelationshipLoadMarker], **kwargs) -> list[ExecutableOption]: + return [] def serialize(self, **kwargs): return self.__api_class__.serialize(self, **kwargs) diff --git a/src/videoag_common/objects/__init__.py b/src/videoag_common/objects/__init__.py index 11b0571..35a93f7 100644 --- a/src/videoag_common/objects/__init__.py +++ b/src/videoag_common/objects/__init__.py @@ -1,5 +1,14 @@ -from .course import Course, Lecture, Chapter -from .site import Announcement, AnnouncementType, AnnouncementPageVisibility, Featured +from .course import ( + Course, + Lecture, + Chapter, + LOAD_COURSE_LECTURES, + LOAD_LECTURE_COURSE, + LOAD_LECTURE_CHAPTERS, + LOAD_LECTURE_PUBLISH_MEDIA, + LOAD_LECTURE_TARGET_MEDIA, +) +from .site import Announcement, AnnouncementType, AnnouncementPageVisibility, Featured, LOAD_FEATURED_COURSE_OR_LECTURE from .user import User from .medium import ( PublishMedium, @@ -8,7 +17,11 @@ from .medium import ( PlainVideoTargetMedium, PlainAudioTargetMedium, ThumbnailTargetMedium, - SourceMedium + SourceMedium, + LOAD_PUBLISH_MEDIUM_LECTURE, + LOAD_PUBLISH_MEDIUM_TARGET_MEDIUM, + LOAD_TARGET_MEDIUM_LECTURE, + LOAD_TARGET_MEDIUM_PUBLISH_MEDIUM ) from .view_permissions import ViewPermissions, ViewPermissionsType, EffectiveViewPermissions from .changelog import ( @@ -17,7 +30,8 @@ from .changelog import ( ChangelogCreationEntry, ChangelogModificationEntry, ChangelogDeletionChangeEntry, - ChangelogUnknownEntry + ChangelogUnknownEntry, + LOAD_CHANGELOG_ENTRY_USER ) from .job import JobState, Job diff --git a/src/videoag_common/objects/changelog.py b/src/videoag_common/objects/changelog.py index 2aaaefb..ae3bafc 100644 --- a/src/videoag_common/objects/changelog.py +++ b/src/videoag_common/objects/changelog.py @@ -16,6 +16,9 @@ class ChangelogEntryType(Enum): _CHANGELOG_ENTRY_TYPE_ENUM = create_enum_type(ChangelogEntryType) +LOAD_CHANGELOG_ENTRY_USER = RelationshipLoadMarker() + + class ChangelogEntry(ApiObject, Base): __mapper_args__ = { "polymorphic_on": "type", @@ -61,22 +64,12 @@ class ChangelogEntry(ApiObject, Base): ) @classmethod - def select(cls, - is_mod: bool, - load_user: bool = False, - **kwargs): - return super().select(is_mod, **kwargs).options(*cls.load_options(load_user=load_user)) - - @staticmethod - def load_options( - load_user=False, - ) -> list[ExecutableOption]: - options = [] + def load_options(cls, is_mod: bool, to_load: list[RelationshipLoadMarker], **kwargs) -> list[ExecutableOption]: + options = super().load_options(is_mod, to_load, **kwargs) + + if LOAD_CHANGELOG_ENTRY_USER in to_load: + options.append(orm.selectinload(ChangelogEntry.modifying_user)) - if load_user: - options.append( - orm.selectinload(ChangelogEntry.modifying_user).options() - ) return options diff --git a/src/videoag_common/objects/course.py b/src/videoag_common/objects/course.py index d31b994..3b83f22 100644 --- a/src/videoag_common/objects/course.py +++ b/src/videoag_common/objects/course.py @@ -1,13 +1,17 @@ from datetime import datetime -from sqlalchemy import UniqueConstraint - from videoag_common.database import * from videoag_common.api_object import * from videoag_common.media_process import * from videoag_common.miscellaneous import * from .view_permissions import EffectiveViewPermissions, ApiViewPermissionsObject, DEFAULT_VIEW_PERMISSIONS +LOAD_COURSE_LECTURES = RelationshipLoadMarker() +LOAD_LECTURE_COURSE = RelationshipLoadMarker() +LOAD_LECTURE_CHAPTERS = RelationshipLoadMarker() +LOAD_LECTURE_PUBLISH_MEDIA = RelationshipLoadMarker() +LOAD_LECTURE_TARGET_MEDIA = RelationshipLoadMarker() + class Chapter(DeletableApiObject, VisibilityApiObject, Base): __api_class__ = ApiObjectClass( @@ -151,7 +155,7 @@ class Lecture(DeletableApiObject, VisibilityApiObject, ApiViewPermissionsObject, from .medium import PublishMedium return sql.and_( PublishMedium.lecture_id == Lecture.id, - PublishMedium.has_access(is_mod=True) + PublishMedium.has_access(is_mod=True, from_lecture=True) ) @staticmethod @@ -159,7 +163,7 @@ class Lecture(DeletableApiObject, VisibilityApiObject, ApiViewPermissionsObject, from .medium import PublishMedium return sql.and_( PublishMedium.lecture_id == Lecture.id, - PublishMedium.has_access(is_mod=False) + PublishMedium.has_access(is_mod=False, from_lecture=True) ) @staticmethod @@ -167,7 +171,7 @@ class Lecture(DeletableApiObject, VisibilityApiObject, ApiViewPermissionsObject, from .medium import TargetMedium return sql.and_( TargetMedium.lecture_id == Lecture.id, - TargetMedium.is_active() + TargetMedium.is_active(from_lecture=True) ) @staticmethod @@ -279,64 +283,33 @@ class Lecture(DeletableApiObject, VisibilityApiObject, ApiViewPermissionsObject, return cond @classmethod - def select(cls, - is_mod: bool, - visibility_check: bool = True, - load_course=False, - load_chapters=False, - load_media=False, - **kwargs): - return ( - super().select(is_mod, visibility_check=visibility_check, **kwargs) - .options(*Lecture.load_options( - is_mod, - False, - load_course=load_course, - load_chapters=load_chapters, - load_media=load_media - )) - ) - - @staticmethod - def load_options( - is_mod: bool, - from_course: bool, - load_course=False, - load_chapters=False, - load_media=False - ) -> list[ExecutableOption]: - """ - - :param is_mod: If false, the public_publish_media, etc. are loaded - :param from_course: Set to True if these options are applied to an orm.selectinload(Course.lectures) (or similar) - :param load_course: Whether to load the course - :param load_chapters: Whether to load the chapters - :param load_media: Whether to load the publish_media - :return: The options - """ - options = [] + def load_options(cls, is_mod: bool, to_load: list[RelationshipLoadMarker], from_course=False, **kwargs) -> list[ExecutableOption]: + options = super().load_options(is_mod, to_load, **kwargs) - if load_chapters: + if LOAD_LECTURE_CHAPTERS in to_load: options.append( orm.selectinload(Lecture.chapters if is_mod else Lecture.public_chapters).options( + # Causes no extra sql, just that the back reference is set (relationship has simple join) orm.immediateload(Chapter.lecture) ) ) - if load_media: - from .medium import PublishMedium, TargetMedium, PlainVideoTargetMedium + if LOAD_LECTURE_PUBLISH_MEDIA in to_load: + from .medium import PublishMedium, TargetMedium options.append( orm.selectinload(Lecture.publish_media if is_mod else Lecture.public_publish_media).options( - orm.immediateload(PublishMedium.lecture), - orm.joinedload(PublishMedium.target_medium) + *PublishMedium.load_options(is_mod, to_load) ) ) if from_course: - # Causes no extra sql, just that the back reference is set + # Causes no extra sql, just that the back reference is set (relationship has simple join) options.append(orm.immediateload(Lecture.course)) - elif load_course: - options.append(orm.joinedload(Lecture.course)) + elif LOAD_LECTURE_COURSE in to_load: + options.append(orm.joinedload(Lecture.course).options( + *Course.load_options(is_mod, to_load) + )) + return options @@ -516,38 +489,15 @@ class Course(DeletableApiObject, VisibilityApiObject, ApiViewPermissionsObject, if not is_mod and visibility_check and not ignore_unlisted: cond &= self.listed return cond - - @classmethod - def select(cls, - is_mod: bool, - visibility_check: bool = True, - ignore_unlisted: bool = False, - load_lectures=False, - load_chapters=False, - load_media=False, - **kwargs): - return ( - super().select(is_mod, visibility_check=visibility_check, ignore_unlisted=ignore_unlisted, **kwargs) - .options(*Course.load_options( - is_mod, - load_lectures=load_lectures, - load_chapters=load_chapters, - load_media=load_media - )) - ) - @staticmethod - def load_options( - is_mod: bool, - load_lectures=False, - load_chapters=False, - load_media=False) -> list[ExecutableOption]: - options = [] + @classmethod + def load_options(cls, is_mod: bool, to_load: list[RelationshipLoadMarker], **kwargs) -> list[ExecutableOption]: + options = super().load_options(is_mod, to_load, **kwargs) - if load_lectures: + if LOAD_COURSE_LECTURES in to_load: options.append( orm.selectinload(Course.lectures if is_mod else Course.public_lectures).options( - *Lecture.load_options(is_mod, True, load_chapters=load_chapters, load_media=load_media) + *Lecture.load_options(is_mod, to_load, from_course=True) ) ) return options diff --git a/src/videoag_common/objects/medium.py b/src/videoag_common/objects/medium.py index 0ab4ba1..1bb0af1 100644 --- a/src/videoag_common/objects/medium.py +++ b/src/videoag_common/objects/medium.py @@ -10,6 +10,12 @@ from .course import Lecture from .job import Job +LOAD_PUBLISH_MEDIUM_LECTURE = RelationshipLoadMarker() +LOAD_PUBLISH_MEDIUM_TARGET_MEDIUM = RelationshipLoadMarker() +LOAD_TARGET_MEDIUM_PUBLISH_MEDIUM = RelationshipLoadMarker() +LOAD_TARGET_MEDIUM_LECTURE = RelationshipLoadMarker() + + # A file which was uploaded and needs to be assigned to a lecture, to be converted to a SourceFileTargetMedium class SourceMedium(DeletableApiObject, Base): __table_args__ = ( @@ -133,6 +139,59 @@ class TargetMedium(DeletableApiObject, Base): def get_default_file_path_no_ending(self): # TODO path return f"{self.process_target_id}.{self.id}" + + @hybrid_method + def is_active(self, usable_check: bool = True, from_lecture: bool = False, from_publish_medium: bool = False, **kwargs): + cond = super().is_active(**kwargs) + if usable_check: + cond &= self.is_produced + if not from_publish_medium and not from_lecture: + cond &= self.hybrid_rel(self.lecture, "is_active", **kwargs) + return cond + + @hybrid_method + def has_access(self, + is_mod: bool, + usable_check: bool = True, + visibility_check: bool = True, + from_lecture: bool = False, + from_publish_medium: bool = False, + **kwargs): + cond = super().has_access( + is_mod, + usable_check=usable_check, + from_lecture=from_lecture, + from_publish_medium=from_publish_medium, + **kwargs + ) + if not from_publish_medium: + cond &= self.hybrid_rel(self.publish_medium, "has_access", is_mod, visibility_check=visibility_check, **kwargs) + return cond + + @classmethod + def load_options(cls, is_mod: bool, to_load: list[RelationshipLoadMarker], from_publish_medium=False, **kwargs) -> list[ExecutableOption]: + options = super().load_options(is_mod, to_load, **kwargs) + + # There is no check for from_publish_medium because even though TargetMedium.publish_medium has a simple join, + # an immediateload would cause an extra sql because it is like the list part of a many-to-one relationship + # (although we use it as a one-to-one) + + if LOAD_TARGET_MEDIUM_PUBLISH_MEDIUM in to_load: + options.append(orm.joinedload(TargetMedium.publish_medium).options( + *PublishMedium.load_options(is_mod, to_load) + )) + + if LOAD_TARGET_MEDIUM_LECTURE in to_load: + if (from_publish_medium or LOAD_TARGET_MEDIUM_PUBLISH_MEDIUM) and LOAD_PUBLISH_MEDIUM_LECTURE in to_load: + # Causes no extra sql, just that the back reference is set (relationship has simple join) + # because the lecture will have been loaded by the publish medium + options.append(orm.immediateload(TargetMedium.lecture)) + else: + options.append(orm.joinedload(TargetMedium.lecture).options( + *Lecture.load_options(is_mod, to_load) + )) + + return options # Note that this usually also includes audio @@ -203,16 +262,24 @@ class PublishMedium(VisibilityApiObject, DeletableApiObject, Base): back_populates="publish_media", lazy="raise_on_sql" ) - target_medium: Mapped["TargetMedium"] = api_mapped( - relationship( - primaryjoin=lambda: TargetMedium.id == PublishMedium.target_medium_id, - back_populates="publish_medium", - lazy="raise_on_sql" + target_medium: Mapped["TargetMedium"] = relationship( + primaryjoin=lambda: sql.and_( + orm.foreign(PublishMedium.target_medium_id) == orm.remote(TargetMedium.id), # foreign is the ForeignKey! + TargetMedium.has_access(is_mod=True, from_publish_medium=True) ), - ApiMany2OneRelationshipField( - include_in_data=True - ) + back_populates="publish_medium", + lazy="raise_on_sql" ) + public_target_medium: Mapped["TargetMedium"] = relationship( + primaryjoin=lambda: sql.and_( + orm.foreign(PublishMedium.target_medium_id) == orm.remote(TargetMedium.id), # foreign is the ForeignKey! + TargetMedium.has_access(is_mod=False, from_publish_medium=True) + ), + back_populates="publish_medium", + lazy="raise_on_sql", + viewonly=True + ) + __table_args__ = ( sql.Index( "check_target_medium_unique", @@ -221,6 +288,51 @@ class PublishMedium(VisibilityApiObject, DeletableApiObject, Base): postgresql_where="NOT deleted" ), ) + + @api_include_in_data( + type_id="target_medium", + data_id="target_medium" + ) + def _data_get_target_media(self, is_mod: bool): + return self.target_medium if is_mod else self.public_target_medium + + @hybrid_method + def is_active(self, from_lecture: bool = False, **kwargs): + cond = super().is_active(**kwargs) + if not from_lecture: + cond &= self.hybrid_rel(self.lecture, "is_active", **kwargs) + return cond + + @hybrid_method + def has_access(self, + is_mod: bool, + visibility_check: bool = True, + from_lecture: bool = False, + **kwargs): + cond = super().has_access(is_mod, visibility_check=visibility_check, from_lecture=from_lecture, **kwargs) + if not from_lecture: + cond &= self.hybrid_rel(self.lecture, "has_access", is_mod, visibility_check=visibility_check, **kwargs) + return cond + + @classmethod + def load_options(cls, is_mod: bool, to_load: list[RelationshipLoadMarker], from_lecture=False, **kwargs) -> list[ExecutableOption]: + options = super().load_options(is_mod, to_load, **kwargs) + + if from_lecture: + # Causes no extra sql, just that the back reference is set (relationship has simple join) + options.append(orm.immediateload(PublishMedium.lecture)) + elif LOAD_PUBLISH_MEDIUM_LECTURE in to_load: + options.append( + orm.joinedload(PublishMedium.lecture).options(*Lecture.load_options(is_mod, to_load)) + ) + + if LOAD_PUBLISH_MEDIUM_TARGET_MEDIUM in to_load: + options.append( + orm.joinedload(PublishMedium.target_medium if is_mod else PublishMedium.public_target_medium).options( + *TargetMedium.load_options(is_mod, to_load, from_publish_medium=True) + )) + + return options class MediaProcessTemplate(ApiObject, Base): diff --git a/src/videoag_common/objects/site.py b/src/videoag_common/objects/site.py index 02bb717..9748d3d 100644 --- a/src/videoag_common/objects/site.py +++ b/src/videoag_common/objects/site.py @@ -70,6 +70,7 @@ class FeaturedType(Enum): FEATURED_TYPE_ENUM = create_enum_type(FeaturedType) +LOAD_FEATURED_COURSE_OR_LECTURE = RelationshipLoadMarker() class Featured(DeletableApiObject, VisibilityApiObject, Base): @@ -112,42 +113,21 @@ class Featured(DeletableApiObject, VisibilityApiObject, Base): ) @classmethod - def select(cls, - is_mod: bool, - load_course_lecture=False, - **kwargs): - return ( - super().select(is_mod, **kwargs) - .options(*cls.load_options(is_mod, load_course_lecture=load_course_lecture)) - ) - - @staticmethod - def load_options( - is_mod: bool, - load_course_lecture=False, - ) -> list[ExecutableOption]: - options = [] + def load_options(cls, is_mod: bool, to_load: list[RelationshipLoadMarker], **kwargs) -> list[ExecutableOption]: + options = super().load_options(is_mod, to_load, **kwargs) - if load_course_lecture: + if LOAD_FEATURED_COURSE_OR_LECTURE in to_load: options.extend( [ orm.selectinload(LectureFeatured.lecture if is_mod else LectureFeatured.public_lecture).options( - *Lecture.load_options( - is_mod, - False, - load_course=True, - load_chapters=True, - load_media=True - ) + *Lecture.load_options(is_mod, to_load) ), orm.selectinload(CourseFeatured.course if is_mod else CourseFeatured.public_course).options( - *Course.load_options( - is_mod, - False - ) + *Course.load_options(is_mod, to_load) ) ] ) + return options diff --git a/src/videoag_common/objects/view_permissions.py b/src/videoag_common/objects/view_permissions.py index d62b79e..3477a16 100644 --- a/src/videoag_common/objects/view_permissions.py +++ b/src/videoag_common/objects/view_permissions.py @@ -20,8 +20,8 @@ class ViewPermissions: def __init__(self, type: ViewPermissionsType, - rwth_authentication: bool = None, - fsmpi_authentication: bool = None, + rwth_authentication: bool = False, + fsmpi_authentication: bool = False, moodle_course_ids: list[int] or None = None, passwords: dict[str, str] or None = None): super().__init__() @@ -35,7 +35,9 @@ class ViewPermissions: if (rwth_authentication and not is_auth) \ or (fsmpi_authentication and not is_auth) \ or ((moodle_course_ids is not None) != is_auth) \ - or ((passwords is not None) != is_auth): + or ((passwords is not None) != is_auth) \ + or rwth_authentication is None \ + or fsmpi_authentication is None: raise TypeError("Invalid view permissions") if is_auth \ and not rwth_authentication \ @@ -136,8 +138,8 @@ def view_permissions_from_json(json: CJsonValue or JsonTypes) -> "ViewPermission type = ViewPermissionsType(type_str) except ValueError: raise json.raise_error(f"Unknown type '{truncate_string(type_str)}'") - rwth_authentication = None - fsmpi_authentication = None + rwth_authentication = False + fsmpi_authentication = False moodle_course_ids = None passwords = None if type == ViewPermissionsType.AUTHENTICATION: -- GitLab