From 6f371701e40efaf2e6ed08a3c6976e196bcfc1b9 Mon Sep 17 00:00:00 2001 From: Ethan Dalool Date: Tue, 9 Jan 2018 18:59:15 -0800 Subject: [PATCH] Fix digest second-scan bug; Break down into smaller functions. There was a bug where moving an album out of its determined parent caused future digests to fail because I wanted to use the GroupExists for control flow, but due to the @transaction decorator it was rolling back the changes. Moved some of the logical pieces of the function into subfunctions as a visual and readability aid. --- README.md | 3 +- etiquette/photodb.py | 88 ++++++++++++++++++++++---------------------- 2 files changed, 47 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index eb12c2a..294e43f 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ Etiquette is a tag-based file organization system with a web front-end. Documentation is still a work in progress. In general, -- You must make the `etiquette` package importable by placing it in one of your lib paths. I use filesystem junctions for this purpose. +- You must make the `etiquette` package importable by placing it in one of your lib paths because I have not made a setup.py yet. Rather than actually moving the folder I just use filesystem junctions. - Run `python etiquette_flask_launch.py [port]` to launch the flask server. Port defaults to 5000 if not provided. - Run `python -i etiquette_repl.py` to launch the Python interpreter with the PhotoDB pre-loaded into a variable called `P`. Try things like `P.new_photo` or `P.digest_directory`. @@ -81,6 +81,7 @@ If you are interested in helping, please raise an issue before making any pull r - Perhaps instead of actually deleting objects, they should just have a `deleted` flag, to make easy restoration possible. Also consider regrouping the children of restored Groupables if those children haven't already been reassigned somewhere else. - Add a new table to store permanent history of add/remove of tags on photos, so that accidents or trolling can be reversed. - Currently, the photo clipboard only stores IDs and therefore when we construct the clipboard tray elements we cannot provide more rich information like filename, the user is only presented with a list of IDs which they probably don't care about. Should the localstorage cache some other more user-friendly information? +- Improve transaction rollbacking. I'm not satisfied with the @transaction decorator because sometimes I want to use exceptions as control flow without them rolling things back. Context managers are good but it's a matter of how abstracted they should be. ### To do list: User permissions Here are some thoughts about the kinds of features that need to exist within the permission system. I don't know how I'll actually manage it just yet. Possibly a `permissions` table in the database with `user_id | permission` where `permission` is some reliably-formatted string. diff --git a/etiquette/photodb.py b/etiquette/photodb.py index 86da2c6..647d6bc 100644 --- a/etiquette/photodb.py +++ b/etiquette/photodb.py @@ -1232,9 +1232,41 @@ class PhotoDB(PDBAlbumMixin, PDBBookmarkMixin, PDBPhotoMixin, PDBTagMixin, PDBUs If a Photo object already exists for a file, it will be added to the correct album. ''' + def create_or_fetch_photos(files): + photos = [] + for filepath in files: + try: + photo = self.get_photo_by_path(filepath) + except exceptions.NoSuchPhoto: + photo = self.new_photo(filepath.absolute_path, commit=False, **new_photo_kwargs) + + photos.append(photo) + return photos + + def create_or_fetch_current_album(albums_by_path, current_directory): + current_album = albums_by_path.get(current_directory.absolute_path, None) + if current_album is None: + try: + current_album = self.get_album_by_path(current_directory.absolute_path) + except exceptions.NoSuchAlbum: + current_album = self.new_album( + associated_directory=current_directory.absolute_path, + commit=False, + title=current_directory.basename, + ) + albums_by_path[current_directory.absolute_path] = current_album + return current_album + + def orphan_join_parent_album(albums_by_path, current_album, current_directory): + if current_album.parent() is None: + parent = albums_by_path.get(current_directory.parent.absolute_path, None) + if parent is not None: + parent.add_child(current_album, commit=False) + directory = pathclass.Path(directory) if not directory.is_dir: raise ValueError('Not a directory: %s' % directory) + directory.correct_case() if exclude_directories is None: exclude_directories = self.config['digest_exclude_dirs'] @@ -1246,15 +1278,6 @@ class PhotoDB(PDBAlbumMixin, PDBBookmarkMixin, PDBPhotoMixin, PDBTagMixin, PDBUs if 'filepath' in new_photo_kwargs: new_photo_kwargs.pop('filepath') - directory.correct_case() - generator = spinal.walk_generator( - directory, - exclude_directories=exclude_directories, - exclude_filenames=exclude_filenames, - recurse=recurse, - yield_style='nested', - ) - if make_albums: try: album = self.get_album_by_path(directory.absolute_path) @@ -1264,47 +1287,26 @@ class PhotoDB(PDBAlbumMixin, PDBBookmarkMixin, PDBPhotoMixin, PDBTagMixin, PDBUs commit=False, title=directory.basename, ) - albums = {directory.absolute_path: album} + albums_by_path = {directory.absolute_path: album} - for (current_location, directories, files) in generator: - # Create the photo object - new_photos = [] - for filepath in files: - try: - photo = self.get_photo_by_path(filepath) - except exceptions.NoSuchPhoto: - makenew = True - else: - makenew = False + walk_generator = spinal.walk_generator( + directory, + exclude_directories=exclude_directories, + exclude_filenames=exclude_filenames, + recurse=recurse, + yield_style='nested', + ) - if makenew: - photo = self.new_photo(filepath.absolute_path, commit=False, **new_photo_kwargs) - - new_photos.append(photo) + for (current_directory, subdirectories, files) in walk_generator: + photos = create_or_fetch_photos(files) if not make_albums: continue - # Ensure that the current directory is an album. - current_album = albums.get(current_location.absolute_path, None) - if current_album is None: - try: - current_album = self.get_album_by_path(current_location.absolute_path) - except exceptions.NoSuchAlbum: - current_album = self.new_album( - associated_directory=current_location.absolute_path, - commit=False, - title=current_location.basename, - ) - albums[current_location.absolute_path] = current_album + current_album = create_or_fetch_current_album(albums_by_path, current_directory) + orphan_join_parent_album(albums_by_path, current_album, current_directory) - parent = albums.get(current_location.parent.absolute_path, None) - if parent is not None: - try: - parent.add_child(current_album, commit=False) - except exceptions.GroupExists: - pass - for photo in new_photos: + for photo in photos: current_album.add_photo(photo, commit=False) if commit: