UriHolder: Make uri & uri_set proper properties

- to avoid direct DB access
- to allow for getter & setter functions
- makes consistency checks on each lookup unneccesary
- propose async consistency checks (TODO)

server/app.py

@@ -1,3 +1,7 @@
+# TODO check that some queries check DB integrity like:
+# SELECT e.uri FROM mediaelment e LEFT JOIN mediaurimapping m ON e.uri = m.uri WHERE m.uri IS NULL;
+# SELECT e.uri FROM mediacollection e LEFT JOIN collectionurimapping m ON e.uri = m.uri WHERE m.uri IS NULL;
+
 ####
 # Imports
 ####
@@ -730,7 +734,7 @@ def refresh_collections() -> ResponseReturnValue:
                     "collection": {
                         "id": coll.id,
                         "title": coll.title,
-                        "uri": coll.uri,
+                        "uri": coll.primary_uri,
                     },
                     "error": {
                         "args": repr(e.args),

server/entertainment_decider/extractors/collection/__init__.py

@@ -43,7 +43,7 @@ def collection_update(
     collection: MediaCollection,
     check_cache_expired: bool = True,
 ) -> ChangedReport:
-    ex = collection_expect_extractor(collection.uri)
+    ex = collection_expect_extractor(collection.primary_uri)
     return ex.update_object(
         object=collection,
         check_cache_expired=check_cache_expired,

server/entertainment_decider/extractors/collection/aggregated.py

@@ -48,7 +48,7 @@ class AggregatedCollectionExtractor(CollectionExtractor[DataType]):
         return True
 
     def _cache_expired(self, object: MediaCollection) -> bool:
-        colls = self.__get_collections(object.uri)
+        colls = self.__get_collections(object.primary_uri)
         for c in colls:
             if c.last_updated is None or object.last_updated <= c.last_updated:
                 return True
@@ -86,7 +86,7 @@ class AggregatedCollectionExtractor(CollectionExtractor[DataType]):
         data: DataType,
     ) -> ChangedReport:
         if object.title is None or "[aggregated]" not in object.title:
-            object.title = f"[aggregated] {object.uri}"
+            object.title = f"[aggregated] {object.primary_uri}"
         object.creator = None
         object.set_watch_in_order_auto(True)
         all_links: Set[int] = set(
@@ -101,6 +101,5 @@ class AggregatedCollectionExtractor(CollectionExtractor[DataType]):
                     episode=episode + 1,
                 )
         orm.delete(link for link in object.media_links if link.element.id in all_links)
-        for uri_link in list(object.uris):
+        object.set_as_only_uri(object.primary_uri)
-            uri_link.delete()
         return ChangedReport.ChangedSome  # TODO improve

server/entertainment_decider/extractors/collection/base.py

@@ -37,14 +37,6 @@ class CollectionExtractor(GeneralExtractor[MediaCollection, T]):
         mapping: CollectionUriMapping = CollectionUriMapping.get(uri=uri)
         if mapping:
             return mapping.element
-        elem: MediaCollection = MediaCollection.get(uri=uri)
-        if elem:
-            logging.warning(
-                f"Add missing URI mapping entry for uri {uri!r}, "
-                + "this should not happen at this point and is considered a bug"
-            )
-            elem.add_single_uri(uri)
-            return elem
         return None
 
     @staticmethod
@@ -116,7 +108,7 @@ class CollectionExtractor(GeneralExtractor[MediaCollection, T]):
             element = extractor.inject_object(data)
         except ExtractionError:
             logging.warning(
-                f"Failed while extracting media {data.object_uri!r} while injecting from {collection.uri!r}",
+                f"Failed while extracting media {data.object_uri!r} while injecting from {collection.primary_uri!r}",
                 exc_info=True,
             )
             return None

server/entertainment_decider/extractors/collection/rss.py

@@ -76,7 +76,7 @@ class RssCollectionExtractor(CollectionExtractor[RSSFeed]):
         object.description = data.description
         object.set_watch_in_order_auto(True)
         object.add_single_uri(
-            self.__get_uri(object.uri)
+            self.__get_uri(object.primary_uri)
         )  # add url without prefix if required
         for item in data.feed:
             element = self._add_episode(

server/entertainment_decider/extractors/collection/tt_rss.py

@@ -80,11 +80,11 @@ class TtRssCollectionExtractor(CollectionExtractor[HeadlineList]):
         data: HeadlineList,
     ) -> ChangedReport:
         if not object.title:
-            object.title = object.uri
+            object.title = object.primary_uri
         object.creator = None
         object.set_watch_in_order_auto(True)
         logging.debug(f"Got {len(data)} headlines")
-        rss_uri = self.__decode_uri(object.uri)
+        rss_uri = self.__decode_uri(object.primary_uri)
         readed_headlines = list[int]()
         for headline in data:
             elem = self._add_episode(collection=object, uri=headline.url)

server/entertainment_decider/extractors/generic.py

@@ -77,14 +77,14 @@ class ExtractedDataLight:
     object_key: str
 
     def create_media(self) -> MediaElement:
-        return MediaElement(
+        return MediaElement.new(
             uri=self.object_uri,
             extractor_name=self.extractor_name,
             extractor_key=self.object_key,
         )
 
     def create_collection(self) -> MediaCollection:
-        return MediaCollection(
+        return MediaCollection.new(
             uri=self.object_uri,
             extractor_name=self.extractor_name,
             extractor_key=self.object_key,
@@ -219,7 +219,7 @@ class GeneralExtractor(Generic[E, T]):
         return self._extract_online(data.object_uri)
 
     def _update_object(self, object: E, data: ExtractedDataOnline[T]) -> ChangedReport:
-        object.uri = data.object_uri
+        object.primary_uri = data.object_uri
         object.tag_list.add(self._get_extractor_tag())
         self._update_object_raw(object, data.data)
         self._update_hook(object, data)
@@ -240,7 +240,7 @@ class GeneralExtractor(Generic[E, T]):
                 f"Skip info for element as already extracted and cache valid: {object.title!r}"
             )
             return ChangedReport.StayedSame
-        data = self._extract_online(object.uri)
+        data = self._extract_online(object.primary_uri)
         logging.debug(f"Updating info for media: {data!r}")
         return self._update_object(object, data)
 

server/entertainment_decider/extractors/media/__init__.py

@@ -28,7 +28,7 @@ def media_expect_extractor(uri: str) -> MediaExtractor:
 
 
 def media_update(element: MediaElement, check_cache_expired: bool = True) -> None:
-    ex = media_expect_extractor(element.uri)
+    ex = media_expect_extractor(element.primary_uri)
     ex.update_object(
         object=element,
         check_cache_expired=check_cache_expired,

server/entertainment_decider/extractors/media/base.py

@@ -30,14 +30,6 @@ class MediaExtractor(GeneralExtractor[MediaElement, T]):
         mapping: MediaUriMapping = MediaUriMapping.get(uri=uri)
         if mapping:
             return mapping.element
-        elem: MediaElement = MediaElement.get(uri=uri)
-        if elem:
-            logging.warning(
-                f"Add missing URI mapping entry for uri {uri!r}, "
-                + "this should not happen at this point and is considered a bug"
-            )
-            elem.add_single_uri(uri)
-            return elem
         return None
 
     def _create_object(self, data: ExtractedDataOffline[T]) -> MediaElement:

server/entertainment_decider/extractors/media/tmdb.py

@@ -81,7 +81,7 @@ class TmdbMovieMediaExtractor(MediaExtractor[TmdbMovieData]):
         # sanity check
         if not data.was_released:
             raise ExtractionError(
-                f"Could not extract {object.uri!r} because of missing data probably due to not being released yet"
+                f"Could not extract {object.primary_uri!r} because of missing data probably due to not being released yet"
             )
         # extract data
         object.title = data.title

server/entertainment_decider/extractors/media/tvmaze.py

@@ -106,7 +106,7 @@ class TvmazeMediaExtractor(MediaExtractor[TvmazeEpisodeEmbedded]):
         airstamp = data.get("airstamp")
         if airstamp is None:  # not released yet
             raise ExtractionError(
-                f"Could not extract {object.uri!r} because of missing data probably due to not being released yet"
+                f"Could not extract {object.primary_uri!r} because of missing data probably due to not being released yet"
             )
         # extract data
         show = data["_embedded"]["show"]

server/entertainment_decider/extractors/media/youtube.py

@@ -112,7 +112,7 @@ class YoutubeMediaExtractor(MediaExtractor[YoutubeVideoData]):
         object.length = int(data["duration"]["secondsText"])
         for tag in get_video_tags(data):
             object.tag_list.add(tag)
-        object.uri = f"https://www.youtube.com/watch?v={data['id']}"
+        object.primary_uri = f"https://www.youtube.com/watch?v={data['id']}"
         object.add_uris(
             (
                 f"https://youtu.be/{data['id']}",

server/entertainment_decider/models/entities.py

@@ -330,8 +330,9 @@ class MediaElement(db.Entity, UriHolder, Tagable):
         int,
         auto=True,
     )
-    uri: str = orm.Required(
+    __uri: str = orm.Required(
         str,
+        column="uri",
         unique=True,
     )
 
@@ -377,7 +378,7 @@ class MediaElement(db.Entity, UriHolder, Tagable):
     tag_list: Iterable[Tag] = orm.Set(
         lambda: Tag,
     )
-    uris: Iterable[MediaUriMapping] = orm.Set(
+    __uri_list: Iterable[MediaUriMapping] = orm.Set(
         lambda: MediaUriMapping,
     )
     collection_links: Iterable[MediaCollectionLink] = orm.Set(
@@ -393,21 +394,35 @@ class MediaElement(db.Entity, UriHolder, Tagable):
         reverse="blocked_by",
     )
 
+    @classmethod
+    def new(
+        cls,
+        *,
+        extractor_name: str,
+        extractor_key: str,
+        uri: str,
+    ) -> MediaElement:
+        return cls(
+            extractor_name=extractor_name,
+            extractor_key=extractor_key,
+            _MediaElement__uri=uri,  # manual mangling for MediaElement
+        )
+
     ### for UriHolder
 
     @property
     def _primary_uri(self) -> str:
-        return self.uri
+        return self.__uri
 
     def _set_primary_uri(self, uri: str) -> None:
-        self.uri = uri
+        self.__uri = uri
 
     @property
-    def _get_uri_set(self) -> Set[str]:
+    def _uri_set(self) -> Set[str]:
-        return {m.uri for m in self.uris}
+        return {m.uri for m in self.__uri_list}
 
     def _set_uri_set(self, uri_set: Set[str]) -> None:
-        self.uris = set()
+        self.__uri_list = set()
         self.add_uris(uri_set)
 
     ### for Tagable
@@ -530,7 +545,7 @@ class MediaElement(db.Entity, UriHolder, Tagable):
             other.ignored = True
         if self.progress >= 0 and other.progress <= 0:
             other.progress = self.progress
-        for uri_map in self.uris:
+        for uri_map in self.__uri_list:
             uri_map.element = other
         for link in self.collection_links:
             if not MediaCollectionLink.get(collection=link.collection, element=other):
@@ -557,7 +572,7 @@ class MediaElement(db.Entity, UriHolder, Tagable):
         self.before_update()
 
     def before_update(self) -> None:
-        self.add_single_uri(self.uri)
+        self.add_single_uri(self.__uri)
 
 
 class MediaThumbnail(db.Entity):
@@ -640,8 +655,9 @@ class MediaCollection(db.Entity, UriHolder, Tagable):
         int,
         auto=True,
     )
-    uri: str = orm.Required(
+    __uri: str = orm.Required(
         str,
+        column="uri",
         unique=True,
     )
 
@@ -697,7 +713,7 @@ class MediaCollection(db.Entity, UriHolder, Tagable):
     tag_list: Iterable[Tag] = orm.Set(
         lambda: Tag,
     )
-    uris: Iterable[CollectionUriMapping] = orm.Set(
+    __uri_set: Iterable[CollectionUriMapping] = orm.Set(
         lambda: CollectionUriMapping,
     )
     media_links: Iterable[MediaCollectionLink] = orm.Set(
@@ -707,21 +723,35 @@ class MediaCollection(db.Entity, UriHolder, Tagable):
         lambda: MediaCollection,
     )
 
+    @classmethod
+    def new(
+        cls,
+        *,
+        extractor_name: str,
+        extractor_key: str,
+        uri: str,
+    ) -> MediaCollection:
+        return cls(
+            extractor_name=extractor_name,
+            extractor_key=extractor_key,
+            _MediaCollection__uri=uri,  # manual mangling for MediaCollection
+        )
+
     ### for UriHolder
 
     @property
     def _primary_uri(self) -> str:
-        return self.uri
+        return self.__uri
 
     def _set_primary_uri(self, uri: str) -> None:
-        self.uri = uri
+        self.__uri = uri
 
     @property
-    def _get_uri_set(self) -> Set[str]:
+    def _uri_set(self) -> Set[str]:
-        return {m.uri for m in self.uris}
+        return {m.uri for m in self.__uri_set}
 
     def _set_uri_set(self, uri_set: Set[str]) -> None:
-        self.uris = set()
+        self.__uri_set = set()
         self.add_uris(uri_set)
 
     ### for Tagable
@@ -917,7 +947,7 @@ class MediaCollection(db.Entity, UriHolder, Tagable):
         self.before_update()
 
     def before_update(self) -> None:
-        self.add_single_uri(self.uri)
+        self.add_single_uri(self.__uri)
 
 
 class CollectionUriMapping(db.Entity):

server/entertainment_decider/models/extras/uris.py

@@ -17,7 +17,7 @@ class UriHolder:
         """Sets the primary uri of this object in a naive way."""
 
     @abstractproperty
-    def _get_uri_set(self) -> Set[str]:
+    def _uri_set(self) -> Set[str]:
         """Returns the uri set of this object in a naive way."""
 
     @abstractmethod
@@ -45,6 +45,17 @@ class UriHolder:
         """Returns the current primary uri of this object."""
         return self._primary_uri
 
+    @primary_uri.setter
+    def primary_uri(self, uri: str) -> None:
+        self.set_primary_uri(uri)
+
+    @property
+    def uri_set(self) -> Set[str]:
+        return self._uri_set
+
+    # uri_set has no setter due to the problem which uri then becomes primary
+    # instead, set_as_only_uri & add_uris should be used so the primary becomes obvious
+
     def is_primary_uri(self, compare_uri: str) -> bool:
         """Returns True if the given uri is equal to the current primary uri."""
         return self.primary_uri == compare_uri
@@ -54,6 +65,8 @@ class UriHolder:
 
         It will also add the uri to the uri set.
         Returns True if the uri was not in the uri set before.
+
+        You may also just write the primary_uri property if you do not need the return value.
         """
         ret = self._add_uri_to_set(uri)  # might fail, so try first
         self._set_primary_uri(uri)
@@ -68,3 +81,9 @@ class UriHolder:
 
     def add_uris(self, uri_list: Iterable[Optional[str]]) -> bool:
         return any([self.add_single_uri(uri) for uri in set(uri_list) if uri])
+
+    def remove_single_uri(self, uri: str) -> bool:
+        return self._remove_uri_from_set(uri)
+
+    def remove_uris(self, uri_list: Iterable[Optional[str]]) -> bool:
+        return any([self.remove_single_uri(uri) for uri in set(uri_list) if uri])

server/templates/collection_element.htm

@@ -88,8 +88,8 @@
         <pre>{{ collection.notes or "" }}</pre>
         <h2>Links</h2>
         <ul>
-            {% for link in collection.uris|sort(attribute="uri") %}
+            {% for link in collection.uri_set|sort %}
-                <li>{{ link.uri | as_link }} {% if collection.uri == link.uri %}*{% endif %}</li>
+                <li>{{ link | as_link }} {% if collection.is_primary_uri(link) %}*{% endif %}</li>
             {% endfor %}
         </ul>
         {% if collection.created_collections %}

server/templates/macros.htm

@@ -358,7 +358,7 @@
     {# TODO do not hardcode certain extractors here #}
     {% if element.extractor_name in ["ytdl", "youtube"] %}
         {%- set opts = {
-            "video_uri": element.uri,
+            "video_uri": element.primary_uri,
             "start": element.progress,
         } -%}
         <a class="button play_button" href="entertainment-decider:///player/play?{{ opts | encode_options }}">{{ symbol | safe }}</a>

server/templates/media_element.htm

@@ -77,8 +77,8 @@
                 </ul>
                 <h2>Links</h2>
                 <ul>
-                    {% for link in element.uris|sort(attribute="uri") %}
+                    {% for link in element.uri_set|sort %}
-                        <li>{{ link.uri | as_link }} {% if element.uri == link.uri %}*{% endif %}</li>
+                        <li>{{ link | as_link }} {% if element.is_primary_uri(link) %}*{% endif %}</li>
                     {% endfor %}
                 </ul>
                 <h2>Blocked By</h2>