5 files changed, 270 insertions, 121 deletions

diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
index ca460c84..a2e913af 100644
--- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php
+++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
@@ -299,8 +299,8 @@ class EntryRestController extends WallabagRestController
      *          {"name"="url", "dataType"="string", "required"=true, "format"="http://www.test.com/article.html", "description"="Url for the entry."},
      *          {"name"="title", "dataType"="string", "required"=false, "description"="Optional, we'll get the title from the page."},
      *          {"name"="tags", "dataType"="string", "required"=false, "format"="tag1,tag2,tag3", "description"="a comma-separated list of tags."},
-     *          {"name"="starred", "dataType"="integer", "required"=false, "format"="1 or 0", "description"="entry already starred"},
      *          {"name"="archive", "dataType"="integer", "required"=false, "format"="1 or 0", "description"="entry already archived"},
+     *          {"name"="starred", "dataType"="integer", "required"=false, "format"="1 or 0", "description"="entry already starred"},
      *          {"name"="content", "dataType"="string", "required"=false, "description"="Content of the entry"},
      *          {"name"="language", "dataType"="string", "required"=false, "description"="Language of the entry"},
      *          {"name"="preview_picture", "dataType"="string", "required"=false, "description"="Preview picture of the entry"},
@@ -328,7 +328,58 @@ class EntryRestController extends WallabagRestController
             $entry->setUrl($url);
         }
 
-        $this->upsertEntry($entry, $request);
+        $data = $this->retrieveValueFromRequest($request);
+
+        try {
+            $this->get('wallabag_core.content_proxy')->updateEntry(
+                $entry,
+                $entry->getUrl(),
+                [
+                    'title' => !empty($data['title']) ? $data['title'] : $entry->getTitle(),
+                    'html' => !empty($data['content']) ? $data['content'] : $entry->getContent(),
+                    'url' => $entry->getUrl(),
+                    'language' => !empty($data['language']) ? $data['language'] : $entry->getLanguage(),
+                    'date' => !empty($data['publishedAt']) ? $data['publishedAt'] : $entry->getPublishedAt(),
+                    // faking the open graph preview picture
+                    'open_graph' => [
+                        'og_image' => !empty($data['picture']) ? $data['picture'] : $entry->getPreviewPicture(),
+                    ],
+                    'authors' => is_string($data['authors']) ? explode(',', $data['authors']) : $entry->getPublishedBy(),
+                ]
+            );
+        } catch (\Exception $e) {
+            $this->get('logger')->error('Error while saving an entry', [
+                'exception' => $e,
+                'entry' => $entry,
+            ]);
+        }
+
+        if (!is_null($data['isArchived'])) {
+            $entry->setArchived((bool) $data['isArchived']);
+        }
+
+        if (!is_null($data['isStarred'])) {
+            $entry->setStarred((bool) $data['isStarred']);
+        }
+
+        if (!empty($data['tags'])) {
+            $this->get('wallabag_core.tags_assigner')->assignTagsToEntry($entry, $data['tags']);
+        }
+
+        if (!is_null($data['isPublic'])) {
+            if (true === (bool) $data['isPublic'] && null === $entry->getUid()) {
+                $entry->generateUid();
+            } elseif (false === (bool) $data['isPublic']) {
+                $entry->cleanUid();
+            }
+        }
+
+        $em = $this->getDoctrine()->getManager();
+        $em->persist($entry);
+        $em->flush();
+
+        // entry saved, dispatch event about it!
+        $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry));
 
         return $this->sendResponse($entry);
     }
@@ -361,7 +412,78 @@ class EntryRestController extends WallabagRestController
         $this->validateAuthentication();
         $this->validateUserAccess($entry->getUser()->getId());
 
-        $this->upsertEntry($entry, $request, true);
+        $contentProxy = $this->get('wallabag_core.content_proxy');
+
+        $data = $this->retrieveValueFromRequest($request);
+
+        // this is a special case where user want to manually update the entry content
+        // the ContentProxy will only cleanup the html
+        // and also we force to not re-fetch the content in case of error
+        if (!empty($data['content'])) {
+            try {
+                $contentProxy->updateEntry(
+                    $entry,
+                    $entry->getUrl(),
+                    [
+                        'html' => $data['content'],
+                    ],
+                    true
+                );
+            } catch (\Exception $e) {
+                $this->get('logger')->error('Error while saving an entry', [
+                    'exception' => $e,
+                    'entry' => $entry,
+                ]);
+            }
+        }
+
+        if (!empty($data['title'])) {
+            $entry->setTitle($data['title']);
+        }
+
+        if (!empty($data['language'])) {
+            $contentProxy->updateLanguage($entry, $data['language']);
+        }
+
+        if (!empty($data['authors']) && is_string($data['authors'])) {
+            $entry->setPublishedBy(explode(',', $data['authors']));
+        }
+
+        if (!empty($data['picture'])) {
+            $contentProxy->updatePreviewPicture($entry, $data['picture']);
+        }
+
+        if (!empty($data['publishedAt'])) {
+            $contentProxy->updatePublishedAt($entry, $data['publishedAt']);
+        }
+
+        if (!is_null($data['isArchived'])) {
+            $entry->setArchived((bool) $data['isArchived']);
+        }
+
+        if (!is_null($data['isStarred'])) {
+            $entry->setStarred((bool) $data['isStarred']);
+        }
+
+        if (!empty($data['tags'])) {
+            $entry->removeAllTags();
+            $this->get('wallabag_core.tags_assigner')->assignTagsToEntry($entry, $data['tags']);
+        }
+
+        if (!is_null($data['isPublic'])) {
+            if (true === (bool) $data['isPublic'] && null === $entry->getUid()) {
+                $entry->generateUid();
+            } elseif (false === (bool) $data['isPublic']) {
+                $entry->cleanUid();
+            }
+        }
+
+        $em = $this->getDoctrine()->getManager();
+        $em->persist($entry);
+        $em->flush();
+
+        // entry saved, dispatch event about it!
+        $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry));
 
         return $this->sendResponse($entry);
     }
@@ -634,76 +756,27 @@ class EntryRestController extends WallabagRestController
     }
 
     /**
-     * Update or Insert a new entry.
+     * Retrieve value from the request.
+     * Used for POST & PATCH on a an entry.
      *
-     * @param Entry   $entry
      * @param Request $request
-     * @param bool    $disableContentUpdate If we don't want the content to be update by fetching the url (used when patching instead of posting)
+     *
+     * @return array
      */
-    private function upsertEntry(Entry $entry, Request $request, $disableContentUpdate = false)
+    private function retrieveValueFromRequest(Request $request)
     {
-        $title = $request->request->get('title');
-        $tags = $request->request->get('tags', []);
-        $isArchived = $request->request->get('archive');
-        $isStarred = $request->request->get('starred');
-        $isPublic = $request->request->get('public');
-        $content = $request->request->get('content');
-        $language = $request->request->get('language');
-        $picture = $request->request->get('preview_picture');
-        $publishedAt = $request->request->get('published_at');
-        $authors = $request->request->get('authors', '');
-
-        try {
-            $this->get('wallabag_core.content_proxy')->updateEntry(
-                $entry,
-                $entry->getUrl(),
-                [
-                    'title' => !empty($title) ? $title : $entry->getTitle(),
-                    'html' => !empty($content) ? $content : $entry->getContent(),
-                    'url' => $entry->getUrl(),
-                    'language' => !empty($language) ? $language : $entry->getLanguage(),
-                    'date' => !empty($publishedAt) ? $publishedAt : $entry->getPublishedAt(),
-                    // faking the open graph preview picture
-                    'open_graph' => [
-                        'og_image' => !empty($picture) ? $picture : $entry->getPreviewPicture(),
-                    ],
-                    'authors' => is_string($authors) ? explode(',', $authors) : $entry->getPublishedBy(),
-                ],
-                $disableContentUpdate
-            );
-        } catch (\Exception $e) {
-            $this->get('logger')->error('Error while saving an entry', [
-                'exception' => $e,
-                'entry' => $entry,
-            ]);
-        }
-
-        if (null !== $isArchived) {
-            $entry->setArchived((bool) $isArchived);
-        }
-
-        if (null !== $isStarred) {
-            $entry->setStarred((bool) $isStarred);
-        }
-
-        if (!empty($tags)) {
-            $this->get('wallabag_core.tags_assigner')->assignTagsToEntry($entry, $tags);
-        }
-
-        if (null !== $isPublic) {
-            if (true === (bool) $isPublic && null === $entry->getUid()) {
-                $entry->generateUid();
-            } elseif (false === (bool) $isPublic) {
-                $entry->cleanUid();
-            }
-        }
-
-        $em = $this->getDoctrine()->getManager();
-        $em->persist($entry);
-        $em->flush();
-
-        // entry saved, dispatch event about it!
-        $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry));
+        return [
+            'title' => $request->request->get('title'),
+            'tags' => $request->request->get('tags', []),
+            'isArchived' => $request->request->get('archive'),
+            'isStarred' => $request->request->get('starred'),
+            'isPublic' => $request->request->get('public'),
+            'content' => $request->request->get('content'),
+            'language' => $request->request->get('language'),
+            'picture' => $request->request->get('preview_picture'),
+            'publishedAt' => $request->request->get('published_at'),
+            'authors' => $request->request->get('authors', ''),
+        ];
     }
 
     /**
diff --git a/src/Wallabag/CoreBundle/Entity/Entry.php b/src/Wallabag/CoreBundle/Entity/Entry.php
index 581e8906..cba72d31 100644
--- a/src/Wallabag/CoreBundle/Entity/Entry.php
+++ b/src/Wallabag/CoreBundle/Entity/Entry.php
@@ -593,6 +593,11 @@ class Entry
         $tag->addEntry($this);
     }
 
+    /**
+     * Remove the given tag from the entry (if the tag is associated).
+     *
+     * @param Tag $tag
+     */
     public function removeTag(Tag $tag)
     {
         if (!$this->tags->contains($tag)) {
@@ -604,6 +609,17 @@ class Entry
     }
 
     /**
+     * Remove all assigned tags from the entry.
+     */
+    public function removeAllTags()
+    {
+        foreach ($this->tags as $tag) {
+            $this->tags->removeElement($tag);
+            $tag->removeEntry($this);
+        }
+    }
+
+    /**
      * Set previewPicture.
      *
      * @param string $previewPicture
diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
index ddecd6f4..2a650e97 100644
--- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php
+++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
@@ -75,9 +75,17 @@ class ContentProxy
      */
     private function stockEntry(Entry $entry, array $content)
     {
-        $title = $content['title'];
-        if (!$title && !empty($content['open_graph']['og_title'])) {
-            $title = $content['open_graph']['og_title'];
+        $entry->setUrl($content['url']);
+
+        $domainName = parse_url($entry->getUrl(), PHP_URL_HOST);
+        if (false !== $domainName) {
+            $entry->setDomainName($domainName);
+        }
+
+        if (!empty($content['title'])) {
+            $entry->setTitle($content['title']);
+        } elseif (!empty($content['open_graph']['og_title'])) {
+            $entry->setTitle($content['open_graph']['og_title']);
         }
 
         $html = $content['html'];
@@ -90,24 +98,11 @@ class ContentProxy
             }
         }
 
-        $entry->setUrl($content['url']);
-        $entry->setTitle($title);
         $entry->setContent($html);
-        $entry->setHttpStatus(isset($content['status']) ? $content['status'] : '');
-
-        if (!empty($content['date'])) {
-            $date = $content['date'];
-
-            // is it a timestamp?
-            if (filter_var($date, FILTER_VALIDATE_INT) !== false) {
-                $date = '@' . $content['date'];
-            }
+        $entry->setReadingTime(Utils::getReadingTime($html));
 
-            try {
-                $entry->setPublishedAt(new \DateTime($date));
-            } catch (\Exception $e) {
-                $this->logger->warning('Error while defining date', ['e' => $e, 'url' => $content['url'], 'date' => $content['date']]);
-            }
+        if (!empty($content['status'])) {
+            $entry->setHttpStatus($content['status']);
         }
 
         if (!empty($content['authors']) && is_array($content['authors'])) {
@@ -118,15 +113,17 @@ class ContentProxy
             $entry->setHeaders($content['all_headers']);
         }
 
-        $this->validateAndSetLanguage(
-            $entry,
-            isset($content['language']) ? $content['language'] : null
-        );
+        if (!empty($content['date'])) {
+            $this->updatePublishedAt($entry, $content['date']);
+        }
 
-        $this->validateAndSetPreviewPicture(
-            $entry,
-            isset($content['open_graph']['og_image']) ? $content['open_graph']['og_image'] : null
-        );
+        if (!empty($content['language'])) {
+            $this->updateLanguage($entry, $content['language']);
+        }
+
+        if (!empty($content['open_graph']['og_image'])) {
+            $this->updatePreviewPicture($entry, $content['open_graph']['og_image']);
+        }
 
         // if content is an image, define it as a preview too
         if (!empty($content['content_type']) && in_array($this->mimeGuesser->guess($content['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) {
@@ -136,12 +133,8 @@ class ContentProxy
             );
         }
 
-        $entry->setMimetype(isset($content['content_type']) ? $content['content_type'] : '');
-        $entry->setReadingTime(Utils::getReadingTime($html));
-
-        $domainName = parse_url($entry->getUrl(), PHP_URL_HOST);
-        if (false !== $domainName) {
-            $entry->setDomainName($domainName);
+        if (!empty($content['content_type'])) {
+            $entry->setMimetype($content['content_type']);
         }
 
         try {
@@ -170,9 +163,9 @@ class ContentProxy
      * Use a Symfony validator to ensure the language is well formatted.
      *
      * @param Entry  $entry
-     * @param string $value Language to validate
+     * @param string $value Language to validate and save
      */
-    private function validateAndSetLanguage($entry, $value)
+    public function updateLanguage(Entry $entry, $value)
     {
         // some lang are defined as fr-FR, es-ES.
         // replacing - by _ might increase language support
@@ -196,9 +189,9 @@ class ContentProxy
      * Use a Symfony validator to ensure the preview picture is a real url.
      *
      * @param Entry  $entry
-     * @param string $value URL to validate
+     * @param string $value URL to validate and save
      */
-    private function validateAndSetPreviewPicture($entry, $value)
+    public function updatePreviewPicture(Entry $entry, $value)
     {
         $errors = $this->validator->validate(
             $value,
@@ -213,4 +206,26 @@ class ContentProxy
 
         $this->logger->warning('PreviewPicture validation failed. ' . (string) $errors);
     }
+
+    /**
+     * Update date.
+     *
+     * @param Entry  $entry
+     * @param string $value Date to validate and save
+     */
+    public function updatePublishedAt(Entry $entry, $value)
+    {
+        $date = $value;
+
+        // is it a timestamp?
+        if (filter_var($date, FILTER_VALIDATE_INT) !== false) {
+            $date = '@'.$value;
+        }
+
+        try {
+            $entry->setPublishedAt(new \DateTime($date));
+        } catch (\Exception $e) {
+            $this->logger->warning('Error while defining date', ['e' => $e, 'url' => $entry->getUrl(), 'date' => $value]);
+        }
+    }
 }
diff --git a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
index ae4af4cd..0647bb23 100644
--- a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
+++ b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
@@ -519,10 +519,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
             $this->markTestSkipped('No content found in db.');
         }
 
-        // hydrate the tags relations
-        $nbTags = count($entry->getTags());
-
-        $this->client->request('PATCH', '/api/entries/' . $entry->getId() . '.json', [
+        $this->client->request('PATCH', '/api/entries/'.$entry->getId().'.json', [
             'title' => 'New awesome title',
             'tags' => 'new tag ' . uniqid(),
             'starred' => '1',
@@ -532,6 +529,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
             'authors' => 'bob,sponge',
             'content' => 'awesome',
             'public' => 0,
+            'published_at' => 1488833381,
         ]);
 
         $this->assertSame(200, $this->client->getResponse()->getStatusCode());
@@ -541,7 +539,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $this->assertSame($entry->getId(), $content['id']);
         $this->assertSame($entry->getUrl(), $content['url']);
         $this->assertSame('New awesome title', $content['title']);
-        $this->assertGreaterThan($nbTags, count($content['tags']));
+        $this->assertGreaterThanOrEqual(1, count($content['tags']), 'We force only one tag');
         $this->assertSame(1, $content['user_id']);
         $this->assertSame('de_AT', $content['language']);
         $this->assertSame('http://preview.io/picture.jpg', $content['preview_picture']);
@@ -549,6 +547,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $this->assertContains('bob', $content['published_by']);
         $this->assertSame('awesome', $content['content']);
         $this->assertFalse($content['is_public'], 'Entry is no more shared');
+        $this->assertContains('2017-03-06', $content['published_at']);
     }
 
     public function testPatchEntryWithoutQuotes()
@@ -562,8 +561,8 @@ class EntryRestControllerTest extends WallabagApiTestCase
             $this->markTestSkipped('No content found in db.');
         }
 
-        // hydrate the tags relations
-        $nbTags = count($entry->getTags());
+        $previousContent = $entry->getContent();
+        $previousLanguage = $entry->getLanguage();
 
         $this->client->request('PATCH', '/api/entries/' . $entry->getId() . '.json', [
             'title' => 'New awesome title',
@@ -579,9 +578,11 @@ class EntryRestControllerTest extends WallabagApiTestCase
 
         $this->assertSame($entry->getId(), $content['id']);
         $this->assertSame($entry->getUrl(), $content['url']);
-        $this->assertSame('New awesome title', $content['title']);
+        $this->assertGreaterThanOrEqual(1, count($content['tags']), 'We force only one tag');
         $this->assertGreaterThan($nbTags, count($content['tags']));
         $this->assertEmpty($content['published_by'], 'Authors were not saved because of an array instead of a string');
+        $this->assertEquals($previousContent, $content['content'], 'Ensure content has not moved');
+        $this->assertEquals($previousLanguage, $content['language'], 'Ensure language has not moved');
     }
 
     public function testGetTagsEntry()
@@ -727,8 +728,10 @@ class EntryRestControllerTest extends WallabagApiTestCase
             $this->markTestSkipped('No content found in db.');
         }
 
-        $this->client->request('PATCH', '/api/entries/' . $entry->getId() . '.json', [
-            'title' => $entry->getTitle() . '++',
+        $previousTitle = $entry->getTitle();
+
+        $this->client->request('PATCH', '/api/entries/'.$entry->getId().'.json', [
+            'title' => $entry->getTitle().'++',
         ]);
 
         $this->assertSame(200, $this->client->getResponse()->getStatusCode());
@@ -736,6 +739,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $content = json_decode($this->client->getResponse()->getContent(), true);
 
         $this->assertSame(1, $content['is_archived']);
+        $this->assertEquals($previousTitle.'++', $content['title']);
     }
 
     public function testSaveIsStarredAfterPatch()
@@ -907,6 +911,17 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $this->assertCount(4, $tags);
     }
 
+    public function testPostEntriesTagsListActionNoList()
+    {
+        $this->client->request('POST', '/api/entries/tags/lists?list='.json_encode([]));
+
+        $this->assertEquals(200, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertEmpty($content);
+    }
+
     public function testDeleteEntriesTagsListAction()
     {
         $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
@@ -933,6 +948,17 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $this->assertCount(0, $entry->getTags());
     }
 
+    public function testDeleteEntriesTagsListActionNoList()
+    {
+        $this->client->request('DELETE', '/api/entries/tags/list?list='.json_encode([]));
+
+        $this->assertEquals(200, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertEmpty($content);
+    }
+
     public function testPostEntriesListAction()
     {
         $list = [
@@ -953,6 +979,17 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $this->assertSame('http://0.0.0.0/entry2', $content[1]['url']);
     }
 
+    public function testPostEntriesListActionWithNoUrls()
+    {
+        $this->client->request('POST', '/api/entries/lists?urls='.json_encode([]));
+
+        $this->assertEquals(200, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertEmpty($content);
+    }
+
     public function testDeleteEntriesListAction()
     {
         $em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
@@ -978,6 +1015,17 @@ class EntryRestControllerTest extends WallabagApiTestCase
         $this->assertSame('http://0.0.0.0/test-entry-not-exist', $content[1]['url']);
     }
 
+    public function testDeleteEntriesListActionWithNoUrls()
+    {
+        $this->client->request('DELETE', '/api/entries/list?urls='.json_encode([]));
+
+        $this->assertEquals(200, $this->client->getResponse()->getStatusCode());
+
+        $content = json_decode($this->client->getResponse()->getContent(), true);
+
+        $this->assertEmpty($content);
+    }
+
     public function testLimitBulkAction()
     {
         $list = [
diff --git a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
index c63671c4..f394b947 100644
--- a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
+++ b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
@@ -221,12 +221,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
             ->method('tag');
 
         $validator = $this->getValidator();
-        $validator->expects($this->exactly(2))
+        $validator->expects($this->once())
             ->method('validate')
-            ->will($this->onConsecutiveCalls(
-                new ConstraintViolationList([new ConstraintViolation('oops', 'oops', [], 'oops', 'language', 'dontexist')]),
-                new ConstraintViolationList()
-            ));
+            ->willReturn(new ConstraintViolationList([new ConstraintViolation('oops', 'oops', [], 'oops', 'language', 'dontexist')]));
 
         $graby = $this->getMockBuilder('Graby\Graby')
             ->setMethods(['fetchContent'])