Avoid returning objects passed by reference.

Objects are always passed by reference, so it doesn't make sense to
return an object which is passed by reference as it will always be the
same object. This change makes the code a bit more readable.

diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
index c3ba1858c22d7a3a1d2445e43cae20d4d95d1fb1..d154c18b37d71a623fe28926f8a3cef0d466e452 100644 (file)
--- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php
+++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
@@ -315,7 +315,7 @@ class EntryRestController extends WallabagRestController
         }
 
         try {
-            $entry = $this->get('wallabag_core.content_proxy')->updateEntry(
+            $this->get('wallabag_core.content_proxy')->updateEntry(
                 $entry,
                 $url,
                 [
@@ -428,7 +428,7 @@ class EntryRestController extends WallabagRestController
         $this->validateUserAccess($entry->getUser()->getId());
 
         try {
-            $entry = $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl());
+            $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl());
         } catch (\Exception $e) {
             $this->get('logger')->error('Error while saving an entry', [
                 'exception' => $e,

diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php
index 8d2ac6d4cf144fd515bf347ec1d0dfa12beb725d..6018dfacd999c330378a858a9b50c6bc36d3974b 100644 (file)
--- a/src/Wallabag/CoreBundle/Controller/EntryController.php
+++ b/src/Wallabag/CoreBundle/Controller/EntryController.php
@@ -53,12 +53,10 @@ class EntryController extends Controller
 
     /**
      * Fetch content and update entry.
-     * In case it fails, entry will return to avod loosing the data.
+     * In case it fails, $entry->getContent will return an error message.
      *
      * @param Entry  $entry
      * @param string $prefixMessage Should be the translation key: entry_saved or entry_reloaded
-     *
-     * @return Entry
      */
     private function updateEntry(Entry $entry, $prefixMessage = 'entry_saved')
     {
@@ -68,7 +66,7 @@ class EntryController extends Controller
         $message = 'flashes.entry.notice.'.$prefixMessage;
 
         try {
-            $entry = $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl());
+            $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl());
         } catch (\Exception $e) {
             $this->get('logger')->error('Error while saving an entry', [
                 'exception' => $e,
@@ -79,8 +77,6 @@ class EntryController extends Controller
         }
 
         $this->get('session')->getFlashBag()->add('notice', $message);
-
-        return $entry;
     }
 
     /**

diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
index 8ba77ca921341e60550fb17588980c307a541253..c73b8eafb7aa6de27d45b55794b28ac9dae464ab 100644 (file)
--- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php
+++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
@@ -40,8 +40,6 @@ class ContentProxy
      * @param Entry  $entry   Entry to update
      * @param string $url     Url to grab content for
      * @param array  $content An array with AT LEAST keys title, html, url to skip the fetchContent from the url
-     *
-     * @return Entry
      */
     public function updateEntry(Entry $entry, $url, array $content = [])
     {
@@ -130,8 +128,6 @@ class ContentProxy
                 'error_msg' => $e->getMessage(),
             ]);
         }
-
-        return $entry;
     }
 
     /**

diff --git a/src/Wallabag/ImportBundle/Import/AbstractImport.php b/src/Wallabag/ImportBundle/Import/AbstractImport.php
index a61388c016e256e52edab636b5b55ffe2775aa26..fc462c4cda5cac8acc66629f60bb4f148a75e583 100644 (file)
--- a/src/Wallabag/ImportBundle/Import/AbstractImport.php
+++ b/src/Wallabag/ImportBundle/Import/AbstractImport.php
@@ -91,13 +91,11 @@ abstract class AbstractImport implements ImportInterface
      * @param Entry  $entry   Entry to update
      * @param string $url     Url to grab content for
      * @param array  $content An array with AT LEAST keys title, html, url, language & content_type to skip the fetchContent from the url
-     *
-     * @return Entry
      */
     protected function fetchContent(Entry $entry, $url, array $content = [])
     {
         try {
-            return $this->contentProxy->updateEntry($entry, $url, $content);
+            $this->contentProxy->updateEntry($entry, $url, $content);
         } catch (\Exception $e) {
             return $entry;
         }

diff --git a/src/Wallabag/ImportBundle/Import/BrowserImport.php b/src/Wallabag/ImportBundle/Import/BrowserImport.php
index ef0eeb7e1fb2a80da654efa1770f8db3707cf23d..71e65e5916f04e4216da71c46a2fd272285c2e79 100644 (file)
--- a/src/Wallabag/ImportBundle/Import/BrowserImport.php
+++ b/src/Wallabag/ImportBundle/Import/BrowserImport.php
@@ -201,7 +201,7 @@ abstract class BrowserImport extends AbstractImport
         $entry->setTitle($data['title']);
 
         // update entry with content (in case fetching failed, the given entry will be return)
-        $entry = $this->fetchContent($entry, $data['url'], $data);
+        $this->fetchContent($entry, $data['url'], $data);
 
         if (array_key_exists('tags', $data)) {
             $this->tagsAssigner->assignTagsToEntry(

diff --git a/src/Wallabag/ImportBundle/Import/InstapaperImport.php b/src/Wallabag/ImportBundle/Import/InstapaperImport.php
index c8e0cd5b26a02a6961999c5d9abe6fa4fc83c458..3aa12f6fb6960e39fb19e914109fa1f3af5d7c68 100644 (file)
--- a/src/Wallabag/ImportBundle/Import/InstapaperImport.php
+++ b/src/Wallabag/ImportBundle/Import/InstapaperImport.php
@@ -125,7 +125,7 @@ class InstapaperImport extends AbstractImport
         $entry->setTitle($importedEntry['title']);
 
         // update entry with content (in case fetching failed, the given entry will be return)
-        $entry = $this->fetchContent($entry, $importedEntry['url'], $importedEntry);
+        $this->fetchContent($entry, $importedEntry['url'], $importedEntry);
 
         if (!empty($importedEntry['tags'])) {
             $this->tagsAssigner->assignTagsToEntry(

diff --git a/src/Wallabag/ImportBundle/Import/PinboardImport.php b/src/Wallabag/ImportBundle/Import/PinboardImport.php
index 489b9257e03b3c46019e988cca3ac8e945b01dad..110b046422ae3c25a7f5be34f59c12e4d11938b2 100644 (file)
--- a/src/Wallabag/ImportBundle/Import/PinboardImport.php
+++ b/src/Wallabag/ImportBundle/Import/PinboardImport.php
@@ -109,7 +109,7 @@ class PinboardImport extends AbstractImport
         $entry->setTitle($data['title']);
 
         // update entry with content (in case fetching failed, the given entry will be return)
-        $entry = $this->fetchContent($entry, $data['url'], $data);
+        $this->fetchContent($entry, $data['url'], $data);
 
         if (!empty($data['tags'])) {
             $this->tagsAssigner->assignTagsToEntry(

diff --git a/src/Wallabag/ImportBundle/Import/PocketImport.php b/src/Wallabag/ImportBundle/Import/PocketImport.php
index 8835161b16440a009075adce12038894b01c5a12..c1d5b6da0518c33bef64a621336406c1679ad716 100644 (file)
--- a/src/Wallabag/ImportBundle/Import/PocketImport.php
+++ b/src/Wallabag/ImportBundle/Import/PocketImport.php
@@ -192,7 +192,7 @@ class PocketImport extends AbstractImport
         $entry->setUrl($url);
 
         // update entry with content (in case fetching failed, the given entry will be return)
-        $entry = $this->fetchContent($entry, $url);
+        $this->fetchContent($entry, $url);
 
         // 0, 1, 2 - 1 if the item is archived - 2 if the item should be deleted
         $entry->setArchived($importedEntry['status'] == 1 || $this->markAsRead);

diff --git a/src/Wallabag/ImportBundle/Import/ReadabilityImport.php b/src/Wallabag/ImportBundle/Import/ReadabilityImport.php
index de320d23907070b53e4cdaa45c8909bfdab18be1..002b27f46b5b2ace08be9f89ad402672496a9c8a 100644 (file)
--- a/src/Wallabag/ImportBundle/Import/ReadabilityImport.php
+++ b/src/Wallabag/ImportBundle/Import/ReadabilityImport.php
@@ -109,7 +109,7 @@ class ReadabilityImport extends AbstractImport
         $entry->setTitle($data['title']);
 
         // update entry with content (in case fetching failed, the given entry will be return)
-        $entry = $this->fetchContent($entry, $data['url'], $data);
+        $this->fetchContent($entry, $data['url'], $data);
 
         $entry->setArchived($data['is_archived']);
         $entry->setStarred($data['is_starred']);

diff --git a/src/Wallabag/ImportBundle/Import/WallabagImport.php b/src/Wallabag/ImportBundle/Import/WallabagImport.php
index 0e5382cfe8a7e28a2a76f172add9a46ab6785201..c64ccd64d196095ce8b9e3f913c4f856075f0fa3 100644 (file)
--- a/src/Wallabag/ImportBundle/Import/WallabagImport.php
+++ b/src/Wallabag/ImportBundle/Import/WallabagImport.php
@@ -108,7 +108,7 @@ abstract class WallabagImport extends AbstractImport
         $entry->setTitle($data['title']);
 
         // update entry with content (in case fetching failed, the given entry will be return)
-        $entry = $this->fetchContent($entry, $data['url'], $data);
+        $this->fetchContent($entry, $data['url'], $data);
 
         if (array_key_exists('tags', $data)) {
             $this->tagsAssigner->assignTagsToEntry(

diff --git a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
index 0c715d90a7d82801ef2409ffa6b2d51614da190e..16643938715d62b5d1b9346e28e284c6e4f690d9 100644 (file)
--- a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
+++ b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
@@ -38,7 +38,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
             ]);
 
         $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
-        $entry = $proxy->updateEntry(new Entry(new User()), 'http://user@:80');
+        $entry = new Entry(new User());
+        $proxy->updateEntry($entry, 'http://user@:80');
 
         $this->assertEquals('http://user@:80', $entry->getUrl());
         $this->assertEmpty($entry->getTitle());
@@ -72,7 +73,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
             ]);
 
         $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
-        $entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0');
+        $entry = new Entry(new User());
+        $proxy->updateEntry($entry, 'http://0.0.0.0');
 
         $this->assertEquals('http://0.0.0.0', $entry->getUrl());
         $this->assertEmpty($entry->getTitle());
@@ -111,7 +113,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
             ]);
 
         $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
-        $entry = $proxy->updateEntry(new Entry(new User()), 'http://domain.io');
+        $entry = new Entry(new User());
+        $proxy->updateEntry($entry, 'http://domain.io');
 
         $this->assertEquals('http://domain.io', $entry->getUrl());
         $this->assertEquals('my title', $entry->getTitle());
@@ -152,7 +155,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
             ]);
 
         $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
-        $entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0');
+        $entry = new Entry(new User());
+        $proxy->updateEntry($entry, 'http://0.0.0.0');
 
         $this->assertEquals('http://1.1.1.1', $entry->getUrl());
         $this->assertEquals('this is my title', $entry->getTitle());
@@ -213,8 +217,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
             ->method('tag');
 
         $proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage);
-        $entry = $proxy->updateEntry(
-            new Entry(new User()),
+        $entry = new Entry(new User());
+        $proxy->updateEntry(
+            $entry,
             'http://0.0.0.0',
             [
                 'html' => str_repeat('this is my content', 325),
@@ -251,8 +256,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
             ->method('tag');
 
         $proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage);
-        $entry = $proxy->updateEntry(
-            new Entry(new User()),
+        $entry = new Entry(new User());
+        $proxy->updateEntry(
+            $entry,
             'http://0.0.0.0',
             [
                 'html' => str_repeat('this is my content', 325),
@@ -285,8 +291,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
         $logger->pushHandler($handler);
 
         $proxy = new ContentProxy((new Graby()), $tagger, $logger, $this->fetchingErrorMessage);
-        $entry = $proxy->updateEntry(
-            new Entry(new User()),
+        $entry = new Entry(new User());
+        $proxy->updateEntry(
+            $entry,
             'http://0.0.0.0',
             [
                 'html' => str_repeat('this is my content', 325),
@@ -326,7 +333,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
 
         $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
 
-        $entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0', [
+        $entry = new Entry(new User());
+        $proxy->updateEntry($entry, 'http://0.0.0.0', [
             'html' => str_repeat('this is my content', 325),
             'title' => 'this is my title',
             'url' => 'http://1.1.1.1',