Creating Video/Archive get_by_id/find_by_id methods. find_by_id will raise an exception when not found, get_by_id will not.

modules/archive/lib.py

@@ -304,9 +304,7 @@ def get_title_from_html(html: str, url: str = None) -> str:
 @optional_session
 def get_archive(session, archive_id: int) -> Archive:
     """Get an Archive."""
-    archive = session.query(Archive).filter_by(id=archive_id).one_or_none()
-    if not archive:
-        raise UnknownArchive(f'Could not find archive with id: {archive_id}')
+    archive = Archive.find_by_id(archive_id, session=session)
     return archive
 
 

modules/archive/models.py

@@ -11,6 +11,8 @@ from sqlalchemy.orm.collections import InstrumentedList
 
 from wrolpi.common import ModelHelper, Base, logger
 from wrolpi.dates import TZDateTime
+from wrolpi.db import optional_session
+from wrolpi.errors import UnknownArchive
 from wrolpi.files.models import FileGroup
 from wrolpi.media_path import MediaPathType
 from wrolpi.tags import Tag, TagFile
@@ -43,6 +45,24 @@ class Archive(Base, ModelHelper):
                    f'domain={self.domain.domain}>'
         return f'<Archive id={self.id} url={self.url} singlefile={repr(str(self.singlefile_path))}>'
 
+    @staticmethod
+    @optional_session
+    def get_by_id(id_: int, session: Session = None) -> Optional[Base]:
+        """Attempt to find an Archive with the provided id.  Returns None if it cannot be found."""
+        archive = session.query(Archive).filter_by(id=id_).one_or_none()
+        return archive
+
+    @staticmethod
+    @optional_session
+    def find_by_id(id_: int, session: Session = None) -> Base:
+        """Find an Archive with the provided id, raises an exception when no Archive is found.
+
+        @raise UnknownArchive: if the Archive can not be found"""
+        archive = Archive.get_by_id(id_, session=session)
+        if not archive:
+            raise UnknownArchive(f'Cannot find Archive with id {id_}')
+        return archive
+
     def my_paths(self, *mimetypes: str) -> List[pathlib.Path]:
         return self.file_group.my_paths(*mimetypes)
 

modules/videos/channel/lib.py

@@ -172,17 +172,13 @@ def create_channel(session: Session, data: schema.ChannelPostRequest, return_dic
 
 
 @run_after(save_channels_config)
-@optional_session
+@optional_session(commit=True)
 def delete_channel(session: Session, *, channel_id: int):
-    try:
-        channel = session.query(Channel).filter_by(id=channel_id).one()
-        channel_dict = channel.dict()
-    except NoResultFound:
-        raise UnknownChannel()
+    channel = Channel.find_by_id(channel_id, session=session)
 
+    channel_dict = channel.dict()
     channel.delete_with_videos()
 
-    session.commit()
     return channel_dict
 
 

modules/videos/channel/test/test_api.py

@@ -38,7 +38,9 @@ def test_get_video(test_client, test_session, simple_channel, video_factory):
     # Test that a 404 is returned when no video exists
     _, response = test_client.get('/api/videos/video/10')
     assert response.status_code == HTTPStatus.NOT_FOUND, response.json
-    assert response.json == {'code': 'UNKNOWN_VIDEO', 'summary': 'The video could not be found.', 'error': ''}
+    assert response.json == {'code': 'UNKNOWN_VIDEO',
+                             'error': 'Cannot find Video with id 10',
+                             'summary': 'The video could not be found.'}
 
     # Get the video info we inserted
     _, response = test_client.get('/api/videos/video/1')

modules/videos/conftest.py

@@ -81,7 +81,7 @@ def simple_video(test_session, test_directory, simple_channel, video_file) -> Vi
     video = Video.from_paths(test_session, video_path)
     video.channel = simple_channel
     test_session.commit()
-    video = test_session.query(Video).filter_by(id=video.id).one()
+    video = Video.find_by_id(video.id, session=test_session)
     return video
 
 

modules/videos/downloader.py

@@ -505,9 +505,7 @@ def update_channel_catalog(channel: Channel, info: dict):
 def _find_all_missing_videos(channel_id: id) -> List[Tuple]:
     """Get all Video entries which don't have the required media files (i.e. hasn't been downloaded)."""
     with get_db_session() as session:
-        channel: Channel = Channel.get_by_id(channel_id)
-        if not channel:
-            raise UnknownChannel(f'Cannot find channel with id {channel_id}')
+        channel: Channel = Channel.find_by_id(channel_id)
 
         def resolve_url(entry: dict) -> str:
             parsed = urlparse(entry.get('webpage_url') or entry['url'])

modules/videos/lib.py

@@ -475,7 +475,7 @@ def import_channels_config():
                     # Favorite in the config is the name of the video_path.  Add the channel directory onto this
                     # video_path, so we can match the complete path for the Video.
                     video_path = channel_dir / video_path
-                    file_group: FileGroup = FileGroup.find_by_path(video_path, session)
+                    file_group: FileGroup = FileGroup.get_by_path(video_path, session)
                     if file_group and file_group.tag_files:
                         # Video is already tagged or favorited.
                         pass

modules/videos/models.py

@@ -8,7 +8,7 @@ from sqlalchemy import Column, Integer, String, Boolean, JSON, Date, ARRAY, Fore
 from sqlalchemy.orm import relationship, Session, deferred
 from sqlalchemy.orm.collections import InstrumentedList
 
-from modules.videos.errors import UnknownVideo
+from modules.videos.errors import UnknownVideo, UnknownChannel
 from wrolpi.captions import read_captions
 from wrolpi.common import Base, ModelHelper, logger, get_media_directory, background_task
 from wrolpi.dates import now, TZDateTime
@@ -192,8 +192,8 @@ class Video(ModelHelper, Base):
                 break
 
         # Fetch the videos by id, if they exist.
-        previous_video = session.query(Video).filter_by(id=previous_id).one() if previous_id else None
-        next_video = session.query(Video).filter_by(id=next_id).one() if next_id else None
+        previous_video = Video.find_by_id(previous_id, session) if previous_id else None
+        next_video = Video.find_by_id(next_id, session) if next_id else None
 
         return previous_video, next_video
 
@@ -285,7 +285,7 @@ class Video(ModelHelper, Base):
 
     @staticmethod
     @optional_session
-    def find_by_path(path, session: Session) -> Optional['Video']:
+    def get_by_path(path, session: Session) -> Optional['Video']:
         video = session.query(Video) \
             .join(FileGroup, FileGroup.id == Video.file_group_id) \
             .filter(FileGroup.primary_path == path).one_or_none()
@@ -293,10 +293,22 @@ class Video(ModelHelper, Base):
 
     @staticmethod
     @optional_session
-    def find_by_id(id_: int, session: Session) -> Optional['Video']:
+    def get_by_id(id_: int, session: Session = None) -> Optional['Video']:
+        """Attempt to find a Video with the provided id.  Returns None if it cannot be found."""
         video = session.query(Video).filter(Video.id == id_).one_or_none()
         return video
 
+    @staticmethod
+    @optional_session
+    def find_by_id(id_: int, session: Session = None) -> 'Video':
+        """Find a Video with the provided id, raises an exception if it cannot be found.
+
+        @raise UnknownVideo: if the Video can not be found"""
+        video = Video.get_by_id(id_, session)
+        if not video:
+            raise UnknownVideo(f'Cannot find Video with id {id_}')
+        return video
+
     def add_tag(self, tag_or_tag_name: Union[Tag, str]) -> TagFile:
         tag = Tag.find_by_name(tag_or_tag_name) if isinstance(tag_or_tag_name, str) else tag_or_tag_name
         return self.file_group.add_tag(tag)
@@ -535,5 +547,17 @@ class Channel(ModelHelper, Base):
     @staticmethod
     @optional_session
     def get_by_id(id_: int, session: Session = None) -> Optional['Channel']:
+        """Attempt to find a Channel with the provided id.  Returns None if it cannot be found."""
         channel = session.query(Channel).filter_by(id=id_).one_or_none()
         return channel
+
+    @staticmethod
+    @optional_session
+    def find_by_id(id_: int, session: Session = None) -> 'Channel':
+        """Find a Channel with the provided id, raises an exception when no Channel is found.
+
+        @raise UnknownChannel: if the channel can not be found"""
+        channel = Channel.get_by_id(id_, session=session)
+        if not channel:
+            raise UnknownChannel(f'Cannot find channel with id {id_}')
+        return channel

modules/videos/test/test_api.py

@@ -104,7 +104,7 @@ def test_refresh_videos(test_client, test_session, test_directory, simple_channe
     video1_video_path = str(video1.video_path)
     video1.poster_path.unlink()
     test_client.post('/api/files/refresh')
-    video1 = Video.find_by_path(video1_video_path, test_session)
+    video1 = Video.get_by_path(video1_video_path, test_session)
     assert not video1.poster_path
 
 
@@ -136,9 +136,9 @@ def test_channels_with_videos(test_session, test_client, test_directory, channel
     assert test_session.query(Video).count() == 3, 'Did not find correct amount of video files.'
     assert {i[0] for i in test_session.query(Channel.name)} == {'channel1', 'channel2'}, 'Channels were changed.'
 
-    vid1 = Video.find_by_path(vid1_path)
-    vid2 = Video.find_by_path(vid2_path)
-    vid3 = Video.find_by_path(vid3_path)
+    vid1 = Video.get_by_path(vid1_path)
+    vid2 = Video.get_by_path(vid2_path)
+    vid3 = Video.get_by_path(vid3_path)
 
     assert vid1.channel == channel1
     assert vid2.channel == channel2

modules/videos/test/test_video.py

@@ -79,8 +79,8 @@ async def test_video_channel_refresh(test_session, test_directory, channel_facto
     assert test_session.query(Video).count() == 2
 
     # The video is associated with `simple_channel` because its file is in the Channel's directory.
-    video1: Video = Video.find_by_path(video1_path, test_session)
-    video2: Video = Video.find_by_path(video2_path, test_session)
+    video1: Video = Video.get_by_path(video1_path, test_session)
+    video2: Video = Video.get_by_path(video2_path, test_session)
     assert video1.channel == channel
     assert not video2.channel
     assert video1.__json__()['video']['channel']

modules/videos/video/lib.py

@@ -1,32 +1,23 @@
 from typing import Tuple, Optional, List, Dict
 
 from sqlalchemy.orm import Session
-from sqlalchemy.orm.exc import NoResultFound
 
 from modules.videos.models import Video
 from wrolpi.common import run_after, logger
 from wrolpi.db import get_db_session, optional_session
-from ..errors import UnknownVideo
 from wrolpi.files.lib import handle_file_group_search_results, tag_names_to_file_group_sub_select
+from ..errors import UnknownVideo
 from ..lib import save_channels_config
 
 logger.getChild(__name__)
 
 
-def get_video(session: Session, video_id: int) -> Video:
-    try:
-        video = session.query(Video).filter_by(id=video_id).one()
-        return video
-    except NoResultFound:
-        raise UnknownVideo()
-
-
 def get_video_for_app(video_id: int) -> Tuple[dict, Optional[dict], Optional[dict]]:
     """
     Get a Video, with it's prev/next videos.  Mark the Video as viewed.
     """
     with get_db_session(commit=True) as session:
-        video = get_video(session, video_id)
+        video = Video.find_by_id(video_id, session=session)
         video.set_viewed()
         previous_video, next_video = video.get_surrounding_videos()
 

wrolpi/common.py

@@ -155,7 +155,7 @@ class ModelHelper:
         raise NotImplementedError('This model has not defined this method.')
 
     @staticmethod
-    def find_by_path(path, session) -> Base:
+    def get_by_path(path, session) -> Base:
         raise NotImplementedError('This model has not defined this method.')
 
     @staticmethod
@@ -164,6 +164,12 @@ class ModelHelper:
 
     @staticmethod
     def get_by_id(id_: int, session: Session = None) -> Optional[Base]:
+        """Attempts to get a model instance by its id.  Returns None if no instance can be found."""
+        raise NotImplementedError('This model has not defined this method.')
+
+    @staticmethod
+    def find_by_id(id_: int, session: Session = None) -> Base:
+        """Get a model instance by its id, raise an error if no instance is found."""
         raise NotImplementedError('This model has not defined this method.')
 
     def flush(self):

wrolpi/files/downloader.py

@@ -77,7 +77,7 @@ async def save_and_tag(file: pathlib.Path, tag_names: List[str] = None):
     tag_names = tag_names or list()
 
     with get_db_session(commit=True) as session:
-        file_group = FileGroup.find_by_path(file)
+        file_group = FileGroup.get_by_path(file)
         if not file_group:
             file_group = FileGroup.from_paths(session, file)
             session.commit()

wrolpi/files/models.py

@@ -260,7 +260,7 @@ class FileGroup(ModelHelper, Base):
 
     @staticmethod
     @optional_session
-    def find_by_path(path, session) -> Optional['FileGroup']:
+    def get_by_path(path, session) -> Optional['FileGroup']:
         file_group = session.query(FileGroup).filter(FileGroup.primary_path == str(path)).one_or_none()
         return file_group
 

wrolpi/test/test_tags.py

@@ -67,8 +67,8 @@ async def test_tags_file_group(test_session, make_files_structure, tag_factory,
 async def test_tags_config_(test_session, test_directory, tag_factory, example_pdf, video_file, test_tags_config):
     """Test that the config is updated when a FileGroup is tagged."""
     await files_lib.refresh_files()
-    pdf: FileGroup = FileGroup.find_by_path(example_pdf, test_session)
-    video: FileGroup = FileGroup.find_by_path(video_file, test_session)
+    pdf: FileGroup = FileGroup.get_by_path(example_pdf, test_session)
+    video: FileGroup = FileGroup.get_by_path(video_file, test_session)
     tag1 = tag_factory()
     tag2 = tag_factory()
     test_session.commit()