Refactored album code in photosdb to fix issue #169

CHANGELOG.md

@@ -4,6 +4,23 @@ All notable changes to this project will be documented in this file. Dates are d
 
 Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
 
+#### [v0.29.23](https://github.com/RhetTbull/osxphotos/compare/v0.29.22...v0.29.23)
+
+> 20 June 2020
+
+- Fixed PhotoInfo.albums, album_info for issue #169 [`1212fad`](https://github.com/RhetTbull/osxphotos/commit/1212fad4adde0b4c6b2887392eed829d8d96d61d)
+
+#### [v0.29.22](https://github.com/RhetTbull/osxphotos/compare/v0.29.19...v0.29.22)
+
+> 19 June 2020
+
+- Don't raise KeyError when SystemLibraryPath is absent [`#168`](https://github.com/RhetTbull/osxphotos/pull/168)
+- Added check for export db in directory branch, closes #164 [`#164`](https://github.com/RhetTbull/osxphotos/issues/164)
+- Added OSXPhotosDB.get_db_connection() [`43d28e7`](https://github.com/RhetTbull/osxphotos/commit/43d28e78f394fa33f8d88f64b56b7dc7258cd454)
+- Added show() to photos_repl.py [`e98c3fe`](https://github.com/RhetTbull/osxphotos/commit/e98c3fe42912ac16d13675bf14154981089d41ea)
+- Fixed get_last_library_path and get_system_library_path to not raise KeyError [`5a83218`](https://github.com/RhetTbull/osxphotos/commit/5a832181f73e082927c80864f2063e554906b06b)
+- Don't raise KeyError when SystemLibraryPath is absent [`1fd0f96`](https://github.com/RhetTbull/osxphotos/commit/1fd0f96b14f0bc38e47bddb4cae12e19406324fb)
+
 #### [v0.29.19](https://github.com/RhetTbull/osxphotos/compare/v0.29.18...v0.29.19)
 
 > 14 June 2020

osxphotos/_version.py

@@ -1,3 +1,3 @@
 """ version info """
 
-__version__ = "0.29.22"
+__version__ = "0.29.24"

osxphotos/photoinfo/photoinfo.py

@@ -21,12 +21,15 @@ import yaml
 from .._constants import (
     _MOVIE_TYPE,
     _PHOTO_TYPE,
+    _PHOTOS_4_ALBUM_KIND,
     _PHOTOS_4_VERSION,
+    _PHOTOS_5_ALBUM_KIND,
+    _PHOTOS_5_SHARED_ALBUM_KIND,
     _PHOTOS_5_SHARED_PHOTO_PATH,
 )
 from ..albuminfo import AlbumInfo
-from ..placeinfo import PlaceInfo4, PlaceInfo5
 from ..phototemplate import PhotoTemplate
+from ..placeinfo import PlaceInfo4, PlaceInfo5
 from ..utils import _debug, _get_resource_loc, findfiles, get_preferred_uti_extension
 
 
@@ -341,21 +344,40 @@ class PhotoInfo:
     @property
     def albums(self):
         """ list of albums picture is contained in """
-        albums = []
-        for album in self._info["albums"]:
-            if not self._db._dbalbum_details[album]["intrash"]:
-                albums.append(self._db._dbalbum_details[album]["title"])
-        return albums
+        try:
+            return self._albums
+        except AttributeError:
+            self._albums = []
+            album_kinds = (
+                [_PHOTOS_4_ALBUM_KIND]
+                if self._db._db_version <= _PHOTOS_4_VERSION
+                else [_PHOTOS_5_ALBUM_KIND, _PHOTOS_5_SHARED_ALBUM_KIND]
+            )
+
+            for album in self._info["albums"]:
+                detail = self._db._dbalbum_details[album]
+                if detail["kind"] in album_kinds and not detail["intrash"]:
+                    self._albums.append(detail["title"])
+            return self._albums
 
     @property
     def album_info(self):
         """ list of AlbumInfo objects representing albums the photos is contained in """
-        albums = []
-        for album in self._info["albums"]:
-            if not self._db._dbalbum_details[album]["intrash"]:
-                albums.append(AlbumInfo(db=self._db, uuid=album))
+        try:
+            return self._album_info
+        except AttributeError:
+            self._album_info = []
+            album_kinds = (
+                [_PHOTOS_4_ALBUM_KIND]
+                if self._db._db_version <= _PHOTOS_4_VERSION
+                else [_PHOTOS_5_ALBUM_KIND, _PHOTOS_5_SHARED_ALBUM_KIND]
+            )
 
-        return albums
+            for album in self._info["albums"]:
+                detail = self._db._dbalbum_details[album]
+                if detail["kind"] in album_kinds and not detail["intrash"]:
+                    self._album_info.append(AlbumInfo(db=self._db, uuid=album))
+            return self._album_info
 
     @property
     def keywords(self):
@@ -772,12 +794,18 @@ class PhotoInfo:
         }
         return json.dumps(pic)
 
-    # compare two PhotoInfo objects for equality
     def __eq__(self, other):
+        """ Compare two PhotoInfo objects for equality """
+        # Can't just compare the two __dicts__ because some methods (like albums)
+        # memoize their value once called in an instance variable (e.g. self._albums)
         if isinstance(other, self.__class__):
-            return self.__dict__ == other.__dict__
-
+            return (
+                self._db.db_path == other._db.db_path
+                and self.uuid == other.uuid
+                and self._info == other._info
+            )
         return False
 
     def __ne__(self, other):
+        """ Compare two PhotoInfo objects for inequality """
         return not self.__eq__(other)

osxphotos/photosdb/photosdb.py

@@ -311,18 +311,16 @@ class PhotosDB:
     def albums_as_dict(self):
         """ return albums as dict of albums, count in reverse sorted order (descending) """
         albums = {}
-        album_keys = [
-            k
-            for k in self._dbalbums_album.keys()
-            if self._dbalbum_details[k]["cloudownerhashedpersonid"] is None
-            and not self._dbalbum_details[k]["intrash"]
-        ]
-        for k in album_keys:
-            title = self._dbalbum_details[k]["title"]
-            if title in albums:
-                albums[title] += len(self._dbalbums_album[k])
+        album_keys = self._get_album_uuids(shared=False)
+        for album in album_keys:
+            title = self._dbalbum_details[album]["title"]
+            if album in self._dbalbums_album:
+                try:
+                    albums[title] += len(self._dbalbums_album[album])
+                except KeyError:
+                    albums[title] = len(self._dbalbums_album[album])
             else:
-                albums[title] = len(self._dbalbums_album[k])
+                albums[title] = 0  # empty album
         albums = dict(sorted(albums.items(), key=lambda kv: kv[1], reverse=True))
         return albums
 
@@ -331,25 +329,17 @@ class PhotosDB:
         """ returns shared albums as dict of albums, count in reverse sorted order (descending)
             valid only on Photos 5; on Photos <= 4, prints warning and returns empty dict """
 
-        # if _dbalbum_details[key]["cloudownerhashedpersonid"] is not None, then it's a shared album
-        if self._db_version <= _PHOTOS_4_VERSION:
-            logging.warning(
-                f"albums_shared not implemented for Photos versions < {_PHOTOS_5_VERSION}"
-            )
-            return {}
-
         albums = {}
-        album_keys = [
-            k
-            for k in self._dbalbums_album.keys()
-            if self._dbalbum_details[k]["cloudownerhashedpersonid"] is not None
-        ]
-        for k in album_keys:
-            title = self._dbalbum_details[k]["title"]
-            if title in albums:
-                albums[title] += len(self._dbalbums_album[k])
+        album_keys = self._get_album_uuids(shared=True)
+        for album in album_keys:
+            title = self._dbalbum_details[album]["title"]
+            if album in self._dbalbums_album:
+                try:
+                    albums[title] += len(self._dbalbums_album[album])
+                except KeyError:
+                    albums[title] = len(self._dbalbums_album[album])
             else:
-                albums[title] = len(self._dbalbums_album[k])
+                albums[title] = 0  # empty album
         albums = dict(sorted(albums.items(), key=lambda kv: kv[1], reverse=True))
         return albums
 
@@ -410,32 +400,28 @@ class PhotosDB:
     @property
     def album_info(self):
         """ return list of AlbumInfo objects for each album in the photos database """
-
-        return [
-            AlbumInfo(db=self, uuid=album)
-            for album in self._dbalbums_album.keys()
-            if self._dbalbum_details[album]["cloudownerhashedpersonid"] is None
-            and not self._dbalbum_details[album]["intrash"]
-        ]
+        try:
+            return self._album_info
+        except AttributeError:
+            self._album_info = [
+                AlbumInfo(db=self, uuid=album)
+                for album in self._get_album_uuids(shared=False)
+            ]
+            return self._album_info
 
     @property
     def album_info_shared(self):
         """ return list of AlbumInfo objects for each shared album in the photos database
             only valid for Photos 5; on Photos <= 4, prints warning and returns empty list """
         # if _dbalbum_details[key]["cloudownerhashedpersonid"] is not None, then it's a shared album
-
-        if self._db_version <= _PHOTOS_4_VERSION:
-            logging.warning(
-                f"albums_shared not implemented for Photos versions < {_PHOTOS_5_VERSION}"
-            )
-            return []
-
-        return [
-            AlbumInfo(db=self, uuid=album)
-            for album in self._dbalbums_album.keys()
-            if self._dbalbum_details[album]["cloudownerhashedpersonid"] is not None
-            and not self._dbalbum_details[album]["intrash"]
-        ]
+        try:
+            return self._album_info_shared
+        except AttributeError:
+            self._album_info_shared = [
+                AlbumInfo(db=self, uuid=album)
+                for album in self._get_album_uuids(shared=True)
+            ]
+            return self._album_info_shared
 
     @property
     def albums(self):
@@ -444,13 +430,11 @@ class PhotosDB:
         # Could be more than one album with same name
         # Right now, they are treated as same album and photos are combined from albums with same name
 
-        albums = {
-            self._dbalbum_details[album]["title"]
-            for album in self._dbalbums_album.keys()
-            if self._dbalbum_details[album]["cloudownerhashedpersonid"] is None
-            and not self._dbalbum_details[album]["intrash"]
-        }
-        return list(albums)
+        try:
+            return self._albums
+        except AttributeError:
+            self._albums = self._get_albums(shared=False)
+            return self._albums
 
     @property
     def albums_shared(self):
@@ -462,19 +446,11 @@ class PhotosDB:
 
         # if _dbalbum_details[key]["cloudownerhashedpersonid"] is not None, then it's a shared album
 
-        if self._db_version <= _PHOTOS_4_VERSION:
-            logging.warning(
-                f"album_names_shared not implemented for Photos versions < {_PHOTOS_5_VERSION}"
-            )
-            return []
-
-        albums = {
-            self._dbalbum_details[album]["title"]
-            for album in self._dbalbums_album.keys()
-            if self._dbalbum_details[album]["cloudownerhashedpersonid"] is not None
-            and not self._dbalbum_details[album]["intrash"]
-        }
-        return list(albums)
+        try:
+            return self._albums_shared
+        except AttributeError:
+            self._albums_shared = self._get_albums(shared=True)
+            return self._albums_shared
 
     @property
     def db_version(self):
@@ -634,6 +610,8 @@ class PhotosDB:
                 "folderUuid": album[5],
                 "albumType": album[6],
                 "albumSubclass": album[7],
+                # for compatability with Photos 5 where album kind is ZKIND
+                "kind": album[7],
             }
 
         # get details about folders
@@ -2098,6 +2076,65 @@ class PhotosDB:
         hierarchy = _recurse_folder_hierarchy(folders)
         return hierarchy
 
+    def _get_album_uuids(self, shared=False):
+        """ Return list of album UUIDs found in photos database
+        
+            Filters out albums in the trash and any special album types
+        
+        Args:
+            shared: boolean; if True, returns shared albums, else normal albums
+        
+        Returns: list of album names
+        """
+        if self._db_version <= _PHOTOS_4_VERSION:
+            version4 = True
+            if shared:
+                logging.warning(
+                    f"Shared albums not implemented for Photos library version {self._db_version}"
+                )
+                return []  # not implemented for _PHOTOS_4_VERSION
+            else:
+                album_kind = _PHOTOS_4_ALBUM_KIND
+        else:
+            version4 = False
+            album_kind = _PHOTOS_5_SHARED_ALBUM_KIND if shared else _PHOTOS_5_ALBUM_KIND
+
+        album_list = []
+        # look through _dbalbum_details because _dbalbums_album won't have empty albums it
+        for album, detail in self._dbalbum_details.items():
+            if (
+                detail["kind"] == album_kind
+                and not detail["intrash"]
+                and (
+                    (shared and detail["cloudownerhashedpersonid"] is not None)
+                    or (not shared and detail["cloudownerhashedpersonid"] is None)
+                )
+                and (
+                    # in Photos 4, special albums like "printAlbum" have kind _PHOTOS_4_ALBUM_KIND
+                    # but should not be listed here; they can be distinguished by looking
+                    # for folderUuid of _PHOTOS_4_ROOT_FOLDER as opposed to _PHOTOS_4_TOP_LEVEL_ALBUM
+                    (version4 and detail["folderUuid"] != _PHOTOS_4_ROOT_FOLDER)
+                    or not version4
+                )
+            ):
+                album_list.append(album)
+        return album_list
+
+    def _get_albums(self, shared=False):
+        """ Return list of album titles found in photos database
+            Albums may have duplicate titles -- these will be treated as a single album.
+        
+            Filters out albums in the trash and any special album types
+
+        Args:
+            shared: boolean; if True, returns shared albums, else normal albums
+        
+        Returns: list of album names
+        """
+
+        album_uuids = self._get_album_uuids(shared=shared)
+        return list({self._dbalbum_details[album]["title"] for album in album_uuids})
+
     def photos(
         self,
         keywords=None,

tests/test_10_12_6.py

@@ -16,7 +16,7 @@ KEYWORDS = [
     "United Kingdom",
 ]
 PERSONS = ["Katie", "Suzy", "Maria"]
-ALBUMS = ["Pumpkin Farm", "Last Import", "AlbumInFolder"]
+ALBUMS = ["Pumpkin Farm", "AlbumInFolder"]
 KEYWORDS_DICT = {
     "Kids": 4,
     "wedding": 2,
@@ -29,7 +29,7 @@ KEYWORDS_DICT = {
     "United Kingdom": 1,
 }
 PERSONS_DICT = {"Katie": 3, "Suzy": 2, "Maria": 1}
-ALBUM_DICT = {"Pumpkin Farm": 3, "Last Import": 1, "AlbumInFolder": 1}
+ALBUM_DICT = {"Pumpkin Farm": 3, "AlbumInFolder": 1}
 
 
 def test_init():

tests/test_albums_folders_catalina_10_15_4.py

@@ -229,6 +229,7 @@ def test_albums_photos():
 
 
 def test_photoinfo_albums():
+    """ Test PhotoInfo.albums """
     import osxphotos
 
     photosdb = osxphotos.PhotosDB(dbfile=PHOTOS_DB)
@@ -238,7 +239,20 @@ def test_photoinfo_albums():
     assert "Pumpkin Farm" in albums
 
 
+def test_photoinfo_albums_2():
+    """ Test that PhotoInfo.albums returns only number albums expected """
+    import osxphotos
+
+    photosdb = osxphotos.PhotosDB(dbfile=PHOTOS_DB)
+    photos = photosdb.photos(uuid=[UUID_DICT["two_albums"]])
+
+    albums = photos[0].albums
+    assert len(albums) == 2
+
+
 def test_photoinfo_album_info():
+    """ test PhotoInfo.album_info """
+
     import osxphotos
 
     photosdb = osxphotos.PhotosDB(dbfile=PHOTOS_DB)
@@ -249,4 +263,4 @@ def test_photoinfo_album_info():
     assert album_info[0].title in ["Pumpkin Farm", "Test Album"]
     assert album_info[1].title in ["Pumpkin Farm", "Test Album"]
 
-    assert photos[0] in album_info[0].photos
+    assert photos[0].uuid in [photo.uuid for photo in album_info[0].photos]

tests/test_albums_folders_mojave_10_14_6.py

@@ -244,4 +244,4 @@ def test_photoinfo_album_info():
     assert album_info[0].title in ["Pumpkin Farm", "Test Album"]
     assert album_info[1].title in ["Pumpkin Farm", "Test Album"]
 
-    assert photos[0] in album_info[0].photos
+    assert photos[0].uuid in [photo.uuid for photo in album_info[0].photos]

tests/test_catalina_10_15_5.py

@@ -28,6 +28,7 @@ ALBUMS = [
     "AlbumInFolder",
     "Raw",
     "I have a deleted twin",  # there's an empty album with same name that has been deleted
+    "EmptyAlbum",
 ]
 KEYWORDS_DICT = {
     "Kids": 4,
@@ -47,6 +48,7 @@ ALBUM_DICT = {
     "AlbumInFolder": 2,
     "Raw": 4,
     "I have a deleted twin": 1,
+    "EmptyAlbum": 0,
 }  # Note: there are 2 albums named "Test Album" for testing duplicate album names
 
 UUID_DICT = {
@@ -63,6 +65,7 @@ UUID_DICT = {
     "no_external_edit": "E9BC5C36-7CD1-40A1-A72B-8B8FAC227D51",
     "export": "D79B8D77-BFFC-460B-9312-034F2877D35B",  # "Pumkins2.jpg"
     "export_tif": "8846E3E6-8AC8-4857-8448-E3D025784410",
+    "in_album": "D79B8D77-BFFC-460B-9312-034F2877D35B",  # "Pumkins2.jpg"
 }
 
 UUID_PUMPKIN_FARM = [
@@ -797,11 +800,29 @@ def test_export_14(caplog):
 
 
 def test_eq():
+    """ Test equality of two PhotoInfo objects """
     import osxphotos
 
-    photosdb = osxphotos.PhotosDB(dbfile=PHOTOS_DB)
-    photos1 = photosdb.photos(uuid=[UUID_DICT["export"]])
-    photos2 = photosdb.photos(uuid=[UUID_DICT["export"]])
+    photosdb1 = osxphotos.PhotosDB(dbfile=PHOTOS_DB)
+    photosdb2 = osxphotos.PhotosDB(dbfile=PHOTOS_DB)
+    photos1 = photosdb1.photos(uuid=[UUID_DICT["export"]])
+    photos2 = photosdb2.photos(uuid=[UUID_DICT["export"]])
+    assert photos1[0] == photos2[0]
+
+
+def test_eq_2():
+    """ Test equality of two PhotoInfo objects when one has memoized property """
+    import osxphotos
+
+    photosdb1 = osxphotos.PhotosDB(dbfile=PHOTOS_DB)
+    photosdb2 = osxphotos.PhotosDB(dbfile=PHOTOS_DB)
+    photos1 = photosdb1.photos(uuid=[UUID_DICT["in_album"]])
+    photos2 = photosdb2.photos(uuid=[UUID_DICT["in_album"]])
+
+    # memoize a value
+    albums = photos1[0].albums
+    assert albums
+
     assert photos1[0] == photos2[0]
 
 

tests/test_cli.py

@@ -261,6 +261,7 @@ ALBUMS_JSON = {
         "AlbumInFolder": 2,
         "Test Album": 2,
         "I have a deleted twin": 1,
+        "EmptyAlbum": 0,
     },
     "shared albums": {},
 }