Added album add/remove endpoints

tests/functional/test_albums.py

@@ -1,4 +1,5 @@
 from tests.functional import test_base
+from trovebox.objects.album import Album
 
 class TestAlbums(test_base.TestBase):
     testcase_name = "album API"
@@ -61,20 +62,34 @@ class TestAlbums(test_base.TestBase):
 
     def test_view(self):
         """ Test the album view """
-        album = self.albums[0]
+        # Do a view() with includeElements=False, using a fresh Album object
+        album = Album(self.client, {"id": self.albums[0].id})
+        album.view()
+        # Make sure there are no photos reported
+        self.assertEqual(album.photos, None)
 
-        # Get the photos in the album using the Album object directly
+        # Get the photos with includeElements=True
         album.view(includeElements=True)
         # Make sure all photos are in the album
         for photo in self.photos:
             self.assertIn(photo.id, [p.id for p in album.photos])
 
-    def test_add_photos(self):
+    def test_add_remove(self):
-        """ If album.add_photos gets implemented, write a test! """
+        """ Test that photos can be added and removed from an album """
-        with self.assertRaises(NotImplementedError):
+        # Make sure all photos are in the album
-            self.client.album.add_photos(None, None)
+        album = self.albums[0]
+        album.view(includeElements=True)
+        for photo in self.photos:
+            self.assertIn(photo.id, [p.id for p in album.photos])
 
-    def test_remove_photos(self):
+        # Remove two photos and check that they're gone
-        """ If album.remove_photos gets implemented, write a test! """
+        album.remove(self.photos[:2])
-        with self.assertRaises(NotImplementedError):
+        album.view(includeElements=True)
-            self.client.album.remove_photos(None, None)
+        self.assertEqual([p.id for p in album.photos], [self.photos[2].id])
+
+        # Add a photo and check that it's there
+        album.add(self.photos[1])
+        album.view(includeElements=True)
+        self.assertNotIn(self.photos[0].id, [p.id for p in album.photos])
+        self.assertIn(self.photos[1].id, [p.id for p in album.photos])
+        self.assertIn(self.photos[2].id, [p.id for p in album.photos])

tests/unit/test_actions.py

@@ -66,13 +66,16 @@ class TestActionCreate(TestActions):
 
     @mock.patch.object(trovebox.Trovebox, 'post')
     def test_action_create_invalid_type(self, mock_post):
-        """Check that an exception is raised if an action is created on a non photo object"""
+        """
-        with self.assertRaises(NotImplementedError):
+        Check that an exception is raised if an action is created on an
+        invalid object.
+        """
+        with self.assertRaises(AttributeError):
             self.client.action.create(target=object(), foo="bar")
 
     @mock.patch.object(trovebox.Trovebox, 'post')
     def test_action_create_invalid_return_type(self, mock_post):
-        """Check that an exception is raised if an non photo object is returned"""
+        """Check that an exception is raised if an invalid object is returned"""
         mock_post.return_value = self._return_value({"target": "test",
                                                      "target_type": "invalid"})
         with self.assertRaises(NotImplementedError):

tests/unit/test_albums.py

@@ -9,21 +9,22 @@ import trovebox
 
 class TestAlbums(unittest.TestCase):
     test_host = "test.example.com"
-    test_photo_dict = {"id": "1a", "tags": ["tag1", "tag2"]}
+    test_photos_dict = [{"id": "1a", "tags": ["tag1", "tag2"]},
+                        {"id": "2b", "tags": ["tag3", "tag4"]}]
     test_albums_dict = [{"cover": {"id": "1a", "tags": ["tag1", "tag2"]},
                          "id": "1",
                          "name": "Album 1",
-                         "photos": [test_photo_dict],
+                         "photos": [test_photos_dict[0]],
                          "totalRows": 2},
                         {"cover": {"id": "2b", "tags": ["tag3", "tag4"]},
                          "id": "2",
                          "name": "Album 2",
-                         "photos": [test_photo_dict],
+                         "photos": [test_photos_dict[1]],
                          "totalRows": 2}]
     def setUp(self):
         self.client = trovebox.Trovebox(host=self.test_host)
-        self.test_photo = trovebox.objects.photo.Photo(self.client,
+        self.test_photos = [trovebox.objects.photo.Photo(self.client, photo)
-                                                       self.test_photo_dict)
+                            for photo in self.test_photos_dict]
         self.test_albums = [trovebox.objects.album.Album(self.client, album)
                             for album in self.test_albums_dict]
 
@@ -82,7 +83,7 @@ class TestAlbumUpdateCover(TestAlbums):
         """Check that an album cover can be updated"""
         mock_post.return_value = self._return_value(self.test_albums_dict[1])
         result = self.client.album.cover_update(self.test_albums[0],
-                                                self.test_photo, foo="bar")
+                                                self.test_photos[0], foo="bar")
         mock_post.assert_called_with("/album/1/cover/1a/update.json",
                                      foo="bar")
         self.assertEqual(result.id, "2")
@@ -107,8 +108,8 @@ class TestAlbumUpdateCover(TestAlbums):
         """Check that an album cover can be updated using the album object directly"""
         mock_post.return_value = self._return_value(self.test_albums_dict[1])
         album = self.test_albums[0]
-        album.cover_update(self.test_photo, foo="bar")
+        album.cover_update(self.test_photos[1], foo="bar")
-        mock_post.assert_called_with("/album/1/cover/1a/update.json",
+        mock_post.assert_called_with("/album/1/cover/2b/update.json",
                                      foo="bar")
         self.assertEqual(album.id, "2")
         self.assertEqual(album.name, "Album 2")
@@ -174,44 +175,121 @@ class TestAlbumDelete(TestAlbums):
         with self.assertRaises(trovebox.TroveboxError):
             self.test_albums[0].delete()
 
-class TestAlbumAddPhotos(TestAlbums):
+class TestAlbumAdd(TestAlbums):
     @mock.patch.object(trovebox.Trovebox, 'post')
-    def test_album_add_photos(self, _):
+    def test_album_add(self, mock_post):
-        """ If album.add_photos gets implemented, write a test! """
+        """ Check that photos can be added to an album """
-        with self.assertRaises(NotImplementedError):
+        mock_post.return_value = self._return_value(self.test_albums_dict[1])
-            self.client.album.add_photos(self.test_albums[0], ["Photo Objects"])
+        self.client.album.add(self.test_albums[0], self.test_photos,
+                              foo="bar")
+        mock_post.assert_called_with("/album/1/photo/add.json",
+                                     ids=["1a", "2b"], foo="bar")
 
     @mock.patch.object(trovebox.Trovebox, 'post')
-    def test_album_add_photos_id(self, _):
+    def test_album_add_id(self, mock_post):
-        """ If album.add_photos gets implemented, write a test! """
+        """ Check that photos can be added to an album using IDs """
-        with self.assertRaises(NotImplementedError):
+        mock_post.return_value = self._return_value(self.test_albums_dict[1])
-            self.client.album.add_photos("1", ["Photo Objects"])
+        self.client.album.add(self.test_albums[0].id,
+                              objects=["1a", "2b"],
+                              object_type="photo",
+                              foo="bar")
+        mock_post.assert_called_with("/album/1/photo/add.json",
+                                     ids=["1a", "2b"], foo="bar")
 
     @mock.patch.object(trovebox.Trovebox, 'post')
-    def test_album_object_add_photos(self, _):
+    def test_album_object_add(self, mock_post):
-        """ If album.add_photos gets implemented, write a test! """
+        """
-        with self.assertRaises(NotImplementedError):
+        Check that photos can be added to an album using the
-            self.test_albums[0].add_photos(["Photo Objects"])
+        album object directly
+        """
+        mock_post.return_value = self._return_value(self.test_albums_dict[1])
+        self.test_albums[0].add(self.test_photos, foo="bar")
+        mock_post.assert_called_with("/album/1/photo/add.json",
+                                     ids=["1a", "2b"], foo="bar")
+
+    @mock.patch.object(trovebox.Trovebox, 'post')
+    def test_album_add_single(self, mock_post):
+        """ Check that a single photo can be added to an album """
+        mock_post.return_value = self._return_value(self.test_albums_dict[1])
+        self.test_albums[0].add(self.test_photos[0], foo="bar")
+        mock_post.assert_called_with("/album/1/photo/add.json",
+                                     ids=["1a"], foo="bar")
+
+    @mock.patch.object(trovebox.Trovebox, 'post')
+    def test_album_add_invalid_type(self, _):
+        """
+        Check that an exception is raised if an invalid object is added
+        to an album.
+        """
+        with self.assertRaises(AttributeError):
+            self.test_albums[0].add([object()])
+
+    @mock.patch.object(trovebox.Trovebox, 'post')
+    def test_album_add_multiple_types(self, _):
+        """
+        Check that an exception is raised if multiple types are added
+        to an album.
+        """
+        with self.assertRaises(ValueError):
+            self.test_albums[0].add(self.test_photos+self.test_albums)
 
 class TestAlbumRemovePhotos(TestAlbums):
     @mock.patch.object(trovebox.Trovebox, 'post')
-    def test_album_remove_photos(self, _):
+    def test_album_remove(self, mock_post):
-        """ If album.remove_photos gets implemented, write a test! """
+        """ Check that photos can be removed from an album """
-        with self.assertRaises(NotImplementedError):
+        mock_post.return_value = self._return_value(self.test_albums_dict[1])
-            self.client.album.remove_photos(self.test_albums[0],
+        self.client.album.remove(self.test_albums[0], self.test_photos,
-                                            ["Photo Objects"])
+                              foo="bar")
+        mock_post.assert_called_with("/album/1/photo/remove.json",
+                                     ids=["1a", "2b"], foo="bar")
 
     @mock.patch.object(trovebox.Trovebox, 'post')
-    def test_album_remove_photos_id(self, _):
+    def test_album_remove_id(self, mock_post):
-        """ If album.remove_photos gets implemented, write a test! """
+        """ Check that photos can be removed from an album using IDs """
-        with self.assertRaises(NotImplementedError):
+        mock_post.return_value = self._return_value(self.test_albums_dict[1])
-            self.client.album.remove_photos("1", ["Photo Objects"])
+        self.client.album.remove(self.test_albums[0].id,
+                              objects=["1a", "2b"],
+                              object_type="photo",
+                              foo="bar")
+        mock_post.assert_called_with("/album/1/photo/remove.json",
+                                     ids=["1a", "2b"], foo="bar")
 
     @mock.patch.object(trovebox.Trovebox, 'post')
-    def test_album_object_remove_photos(self, _):
+    def test_album_object_remove(self, mock_post):
-        """ If album.remove_photos gets implemented, write a test! """
+        """
-        with self.assertRaises(NotImplementedError):
+        Check that photos can be removed from an album using the
-            self.test_albums[0].remove_photos(["Photo Objects"])
+        album object directly
+        """
+        mock_post.return_value = self._return_value(self.test_albums_dict[1])
+        self.test_albums[0].remove(self.test_photos, foo="bar")
+        mock_post.assert_called_with("/album/1/photo/remove.json",
+                                     ids=["1a", "2b"], foo="bar")
+
+    @mock.patch.object(trovebox.Trovebox, 'post')
+    def test_album_remove_single(self, mock_post):
+        """ Check that a single photo can be removed from an album """
+        mock_post.return_value = self._return_value(self.test_albums_dict[1])
+        self.test_albums[0].remove(self.test_photos[0], foo="bar")
+        mock_post.assert_called_with("/album/1/photo/remove.json",
+                                     ids=["1a"], foo="bar")
+
+    @mock.patch.object(trovebox.Trovebox, 'post')
+    def test_album_remove_invalid_type(self, _):
+        """
+        Check that an exception is raised if an invalid object is removed
+        from an album.
+        """
+        with self.assertRaises(AttributeError):
+            self.test_albums[0].remove([object()])
+
+    @mock.patch.object(trovebox.Trovebox, 'post')
+    def test_album_remove_multiple_types(self, _):
+        """
+        Check that an exception is raised if multiple types are removed
+        from an album.
+        """
+        with self.assertRaises(ValueError):
+            self.test_albums[0].remove(self.test_photos+self.test_albums)
 
 class TestAlbumUpdate(TestAlbums):
     @mock.patch.object(trovebox.Trovebox, 'post')
@@ -259,7 +337,7 @@ class TestAlbumView(TestAlbums):
         self.assertEqual(result.name, "Album 2")
         self.assertEqual(result.cover.id, "2b")
         self.assertEqual(result.cover.tags, ["tag3", "tag4"])
-        self.assertEqual(result.photos[0].id, self.test_photo.id)
+        self.assertEqual(result.photos[0].id, self.test_photos[1].id)
 
     @mock.patch.object(trovebox.Trovebox, 'get')
     def test_album_view_id(self, mock_get):
@@ -271,7 +349,7 @@ class TestAlbumView(TestAlbums):
         self.assertEqual(result.name, "Album 2")
         self.assertEqual(result.cover.id, "2b")
         self.assertEqual(result.cover.tags, ["tag3", "tag4"])
-        self.assertEqual(result.photos[0].id, self.test_photo.id)
+        self.assertEqual(result.photos[0].id, self.test_photos[1].id)
 
     @mock.patch.object(trovebox.Trovebox, 'get')
     def test_album_object_view(self, mock_get):
@@ -284,4 +362,4 @@ class TestAlbumView(TestAlbums):
         self.assertEqual(album.name, "Album 2")
         self.assertEqual(album.cover.id, "2b")
         self.assertEqual(album.cover.tags, ["tag3", "tag4"])
-        self.assertEqual(album.photos[0].id, self.test_photo.id)
+        self.assertEqual(album.photos[0].id, self.test_photos[1].id)

trovebox/api/api_action.py

@@ -2,7 +2,6 @@
 api_action.py : Trovebox Action API Classes
 """
 from trovebox.objects.action import Action
-from trovebox.objects.photo import Photo
 from .api_base import ApiBase
 
 class ApiAction(ApiBase):
@@ -16,12 +15,10 @@ class ApiAction(ApiBase):
         If a Trovebox object is used, the target type is inferred
         automatically.
         """
+        # Extract the type from the target
         if target_type is None:
-            # Determine the target type
+            target_type = target.get_type()
-            if isinstance(target, Photo):
+
-                target_type = "photo"
-            else:
-                raise NotImplementedError("Unsupported target type")
         # Extract the ID from the target
         try:
             target_id = target.id

trovebox/api/api_activity.py

@@ -8,12 +8,12 @@ from .api_base import ApiBase
 
 class ApiActivities(ApiBase):
     """ Definitions of /activities/ API endpoints """
-    def list(self, filters={}, **kwds):
+    def list(self, filters=None, **kwds):
         """
         Endpoint: /activities/[<filters>]/list.json
 
         Returns a list of Activity objects.
-        The filters parameter can be used to narrow down the returned activities.
+        The filters parameter can be used to narrow down the activities.
         Eg: filters={"type": "photo-upload"}
         """
         filter_string = self._build_filter_string(filters)

trovebox/api/api_album.py

@@ -1,6 +1,9 @@
 """
 api_album.py : Trovebox Album API Classes
 """
+import collections
+
+from trovebox.objects.trovebox_object import TroveboxObject
 from trovebox.objects.album import Album
 from trovebox import http
 from .api_base import ApiBase
@@ -53,15 +56,59 @@ class ApiAlbum(ApiBase):
             album = Album(self._client, {"id": album})
         return album.delete(**kwds)
 
-    # TODO: Should be just "add"
+    def add(self, album, objects, object_type=None, **kwds):
-    def add_photos(self, album, photos, **kwds):
+        """
-        """ Not yet implemented """
+        Endpoint: /album/<id>/<type>/add.json
-        raise NotImplementedError()
 
-    # TODO: Should be just "remove"
+        Add objects (eg. Photos) to an album.
-    def remove_photos(self, album, photos, **kwds):
+        The objects are a list of either IDs or Trovebox objects.
-        """ Not yet implemented """
+        If Trovebox objects are used, the object type is inferred
-        raise NotImplementedError()
+        automatically.
+        Returns True if the album was updated successfully.
+        """
+        return self._add_remove("add", album, objects, object_type,
+                                **kwds)
+
+    def remove(self, album, objects, object_type=None, **kwds):
+        """
+        Endpoint: /album/<id>/<type>/remove.json
+
+        Remove objects (eg. Photos) to an album.
+        The objects are a list of either IDs or Trovebox objects.
+        If Trovebox objects are used, the object type is inferred
+        automatically.
+        Returns True if the album was updated successfully.
+        """
+        return self._add_remove("remove", album, objects, object_type,
+                                **kwds)
+
+    def _add_remove(self, action, album, objects, object_type=None,
+                    **kwds):
+        """Common code for the add and remove endpoints."""
+        # Extract the id of the album
+        if isinstance(album, Album):
+            album = album.id
+
+        # Ensure we have an iterable of objects
+        if not isinstance(objects, collections.Iterable):
+            objects = [objects]
+
+        # Extract the type of the objects
+        if object_type is None:
+            object_type = objects[0].get_type()
+
+        for i, obj in enumerate(objects):
+            if isinstance(obj, TroveboxObject):
+                # Ensure all objects are the same type
+                if obj.get_type() != object_type:
+                    raise ValueError("Not all objects are of type '%s'"
+                                     % object_type)
+                # Extract the ids of the objects
+                objects[i] = obj.id
+
+        return self._client.post("/album/%s/%s/%s.json" %
+                                 (album, object_type, action),
+                                 ids=objects, **kwds)["result"]
 
     def update(self, album, **kwds):
         """

trovebox/api/api_base.py

@@ -3,6 +3,7 @@ api_base.py: Base class for all API classes
 """
 
 class ApiBase(object):
+    """ Base class for all API objects """
     def __init__(self, client):
         self._client = client
 
@@ -13,6 +14,7 @@ class ApiBase(object):
         :returns: filter_string formatted for an API endpoint
         """
         filter_string = ""
-        for filter in filters:
+        if filters is not None:
-            filter_string += "%s-%s/" % (filter, filters[filter])
+            for filt in filters:
+                filter_string += "%s-%s/" % (filt, filters[filt])
         return filter_string

trovebox/objects/action.py

@@ -11,6 +11,7 @@ class Action(TroveboxObject):
         self.target = None
         self.target_type = None
         TroveboxObject.__init__(self, trovebox, json_dict)
+        self._type = "action"
         self._update_fields_with_objects()
 
     def _update_fields_with_objects(self):

trovebox/objects/activity.py

@@ -12,6 +12,7 @@ class Activity(TroveboxObject):
         self.data = None
         self.type = None
         TroveboxObject.__init__(self, trovebox, json_dict)
+        self._type = "activity"
         self._update_fields_with_objects()
 
     def _update_fields_with_objects(self):

trovebox/objects/album.py

@@ -11,18 +11,25 @@ class Album(TroveboxObject):
         self.photos = None
         self.cover = None
         TroveboxObject.__init__(self, trovebox, json_dict)
+        self._type = "album"
         self._update_fields_with_objects()
 
     def _update_fields_with_objects(self):
         """ Convert dict fields into objects, where appropriate """
         # Update the cover with a photo object
+        try:
             if isinstance(self.cover, dict):
                 self.cover = Photo(self._trovebox, self.cover)
+        except AttributeError:
+            pass # No cover
+
         # Update the photo list with photo objects
-        if isinstance(self.photos, list):
+        try:
             for i, photo in enumerate(self.photos):
                 if isinstance(photo, dict):
                     self.photos[i] = Photo(self._trovebox, photo)
+        except (AttributeError, TypeError):
+            pass # No photos, or not a list
 
     def cover_update(self, photo, **kwds):
         """
@@ -60,15 +67,45 @@ class Album(TroveboxObject):
         self._delete_fields()
         return result
 
-    # TODO: Should be just "add"
+    def add(self, objects, object_type=None, **kwds):
-    def add_photos(self, photos, **kwds):
+        """
-        """ Not implemented yet """
+        Endpoint: /album/<id>/<type>/add.json
-        raise NotImplementedError()
 
-    # TODO: Should be just "remove"
+        Add objects (eg. Photos) to this album.
-    def remove_photos(self, photos, **kwds):
+        The objects are a list of either IDs or Trovebox objects.
-        """ Not implemented yet """
+        If Trovebox objects are used, the object type is inferred
-        raise NotImplementedError()
+        automatically.
+        Updates the album's fields with the response.
+        """
+        result = self._trovebox.album.add(self, objects, object_type, **kwds)
+
+        # API currently doesn't return the updated album
+        # (frontend issue #1369)
+        if isinstance(result, bool): # pragma: no cover
+            result = self._trovebox.get("/album/%s/view.json" %
+                                        self.id)["result"]
+        self._replace_fields(result)
+        self._update_fields_with_objects()
+
+    def remove(self, objects, object_type=None, **kwds):
+        """
+        Endpoint: /album/<id>/<type>/remove.json
+
+        Remove objects (eg. Photos) from this album.
+        The objects are a list of either IDs or Trovebox objects.
+        If Trovebox objects are used, the object type is inferred
+        automatically.
+        Updates the album's fields with the response.
+        """
+        result = self._trovebox.album.remove(self, objects, object_type,
+                                             **kwds)
+        # API currently doesn't return the updated album
+        # (frontend issue #1369)
+        if isinstance(result, bool): # pragma: no cover
+            result = self._trovebox.get("/album/%s/view.json" %
+                                        self.id)["result"]
+        self._replace_fields(result)
+        self._update_fields_with_objects()
 
     def update(self, **kwds):
         """

trovebox/objects/photo.py

@@ -6,6 +6,10 @@ from .trovebox_object import TroveboxObject
 
 class Photo(TroveboxObject):
     """ Representation of a Photo object """
+    def __init__(self, trovebox, json_dict):
+        TroveboxObject.__init__(self, trovebox, json_dict)
+        self._type = "photo"
+
     def delete(self, **kwds):
         """
         Endpoint: /photo/<id>/delete.json

trovebox/objects/tag.py

@@ -11,6 +11,10 @@ from .trovebox_object import TroveboxObject
 
 class Tag(TroveboxObject):
     """ Representation of a Tag object """
+    def __init__(self, trovebox, json_dict):
+        TroveboxObject.__init__(self, trovebox, json_dict)
+        self._type = "tag"
+
     def delete(self, **kwds):
         """
         Endpoint: /tag/<id>/delete.json

trovebox/objects/trovebox_object.py

@@ -4,6 +4,7 @@ Base object supporting the storage of custom fields as attributes
 class TroveboxObject(object):
     """ Base object supporting the storage of custom fields as attributes """
     def __init__(self, trovebox, json_dict):
+        self._type = "None"
         self.id = None
         self.name = None
         self._trovebox = trovebox
@@ -48,3 +49,7 @@ class TroveboxObject(object):
     def get_fields(self):
         """ Returns this object's attributes """
         return self._json_dict
+
+    def get_type(self):
+        """ Return this object's type (eg. "photo") """
+        return self._type