aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorJerome Charaoui <jerome@riseup.net>2016-12-06 22:17:44 -0500
committerJeremy Benoist <jbenoist@20minutes.fr>2017-06-01 09:43:01 +0200
commit7aba665e484c5c36ee029219a999a427d864ff22 (patch)
tree9ac57748d91cee32848b5e961e293e1c9a1fa61f
parent2a0eec07a5630401a9ceb7add65604f79238f10c (diff)
downloadwallabag-7aba665e484c5c36ee029219a999a427d864ff22.tar.gz
wallabag-7aba665e484c5c36ee029219a999a427d864ff22.tar.zst
wallabag-7aba665e484c5c36ee029219a999a427d864ff22.zip
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.
-rw-r--r--src/Wallabag/ApiBundle/Controller/EntryRestController.php4
-rw-r--r--src/Wallabag/CoreBundle/Controller/EntryController.php8
-rw-r--r--src/Wallabag/CoreBundle/Helper/ContentProxy.php4
-rw-r--r--src/Wallabag/ImportBundle/Import/AbstractImport.php4
-rw-r--r--src/Wallabag/ImportBundle/Import/BrowserImport.php2
-rw-r--r--src/Wallabag/ImportBundle/Import/InstapaperImport.php2
-rw-r--r--src/Wallabag/ImportBundle/Import/PinboardImport.php2
-rw-r--r--src/Wallabag/ImportBundle/Import/PocketImport.php2
-rw-r--r--src/Wallabag/ImportBundle/Import/ReadabilityImport.php2
-rw-r--r--src/Wallabag/ImportBundle/Import/WallabagImport.php2
-rw-r--r--tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php30
11 files changed, 30 insertions, 32 deletions
diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
index c3ba1858..d154c18b 100644
--- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php
+++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php
@@ -315,7 +315,7 @@ class EntryRestController extends WallabagRestController
315 } 315 }
316 316
317 try { 317 try {
318 $entry = $this->get('wallabag_core.content_proxy')->updateEntry( 318 $this->get('wallabag_core.content_proxy')->updateEntry(
319 $entry, 319 $entry,
320 $url, 320 $url,
321 [ 321 [
@@ -428,7 +428,7 @@ class EntryRestController extends WallabagRestController
428 $this->validateUserAccess($entry->getUser()->getId()); 428 $this->validateUserAccess($entry->getUser()->getId());
429 429
430 try { 430 try {
431 $entry = $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl()); 431 $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl());
432 } catch (\Exception $e) { 432 } catch (\Exception $e) {
433 $this->get('logger')->error('Error while saving an entry', [ 433 $this->get('logger')->error('Error while saving an entry', [
434 'exception' => $e, 434 'exception' => $e,
diff --git a/src/Wallabag/CoreBundle/Controller/EntryController.php b/src/Wallabag/CoreBundle/Controller/EntryController.php
index 8d2ac6d4..6018dfac 100644
--- a/src/Wallabag/CoreBundle/Controller/EntryController.php
+++ b/src/Wallabag/CoreBundle/Controller/EntryController.php
@@ -53,12 +53,10 @@ class EntryController extends Controller
53 53
54 /** 54 /**
55 * Fetch content and update entry. 55 * Fetch content and update entry.
56 * In case it fails, entry will return to avod loosing the data. 56 * In case it fails, $entry->getContent will return an error message.
57 * 57 *
58 * @param Entry $entry 58 * @param Entry $entry
59 * @param string $prefixMessage Should be the translation key: entry_saved or entry_reloaded 59 * @param string $prefixMessage Should be the translation key: entry_saved or entry_reloaded
60 *
61 * @return Entry
62 */ 60 */
63 private function updateEntry(Entry $entry, $prefixMessage = 'entry_saved') 61 private function updateEntry(Entry $entry, $prefixMessage = 'entry_saved')
64 { 62 {
@@ -68,7 +66,7 @@ class EntryController extends Controller
68 $message = 'flashes.entry.notice.'.$prefixMessage; 66 $message = 'flashes.entry.notice.'.$prefixMessage;
69 67
70 try { 68 try {
71 $entry = $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl()); 69 $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl());
72 } catch (\Exception $e) { 70 } catch (\Exception $e) {
73 $this->get('logger')->error('Error while saving an entry', [ 71 $this->get('logger')->error('Error while saving an entry', [
74 'exception' => $e, 72 'exception' => $e,
@@ -79,8 +77,6 @@ class EntryController extends Controller
79 } 77 }
80 78
81 $this->get('session')->getFlashBag()->add('notice', $message); 79 $this->get('session')->getFlashBag()->add('notice', $message);
82
83 return $entry;
84 } 80 }
85 81
86 /** 82 /**
diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
index 8ba77ca9..c73b8eaf 100644
--- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php
+++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
@@ -40,8 +40,6 @@ class ContentProxy
40 * @param Entry $entry Entry to update 40 * @param Entry $entry Entry to update
41 * @param string $url Url to grab content for 41 * @param string $url Url to grab content for
42 * @param array $content An array with AT LEAST keys title, html, url to skip the fetchContent from the url 42 * @param array $content An array with AT LEAST keys title, html, url to skip the fetchContent from the url
43 *
44 * @return Entry
45 */ 43 */
46 public function updateEntry(Entry $entry, $url, array $content = []) 44 public function updateEntry(Entry $entry, $url, array $content = [])
47 { 45 {
@@ -130,8 +128,6 @@ class ContentProxy
130 'error_msg' => $e->getMessage(), 128 'error_msg' => $e->getMessage(),
131 ]); 129 ]);
132 } 130 }
133
134 return $entry;
135 } 131 }
136 132
137 /** 133 /**
diff --git a/src/Wallabag/ImportBundle/Import/AbstractImport.php b/src/Wallabag/ImportBundle/Import/AbstractImport.php
index a61388c0..fc462c4c 100644
--- a/src/Wallabag/ImportBundle/Import/AbstractImport.php
+++ b/src/Wallabag/ImportBundle/Import/AbstractImport.php
@@ -91,13 +91,11 @@ abstract class AbstractImport implements ImportInterface
91 * @param Entry $entry Entry to update 91 * @param Entry $entry Entry to update
92 * @param string $url Url to grab content for 92 * @param string $url Url to grab content for
93 * @param array $content An array with AT LEAST keys title, html, url, language & content_type to skip the fetchContent from the url 93 * @param array $content An array with AT LEAST keys title, html, url, language & content_type to skip the fetchContent from the url
94 *
95 * @return Entry
96 */ 94 */
97 protected function fetchContent(Entry $entry, $url, array $content = []) 95 protected function fetchContent(Entry $entry, $url, array $content = [])
98 { 96 {
99 try { 97 try {
100 return $this->contentProxy->updateEntry($entry, $url, $content); 98 $this->contentProxy->updateEntry($entry, $url, $content);
101 } catch (\Exception $e) { 99 } catch (\Exception $e) {
102 return $entry; 100 return $entry;
103 } 101 }
diff --git a/src/Wallabag/ImportBundle/Import/BrowserImport.php b/src/Wallabag/ImportBundle/Import/BrowserImport.php
index ef0eeb7e..71e65e59 100644
--- a/src/Wallabag/ImportBundle/Import/BrowserImport.php
+++ b/src/Wallabag/ImportBundle/Import/BrowserImport.php
@@ -201,7 +201,7 @@ abstract class BrowserImport extends AbstractImport
201 $entry->setTitle($data['title']); 201 $entry->setTitle($data['title']);
202 202
203 // update entry with content (in case fetching failed, the given entry will be return) 203 // update entry with content (in case fetching failed, the given entry will be return)
204 $entry = $this->fetchContent($entry, $data['url'], $data); 204 $this->fetchContent($entry, $data['url'], $data);
205 205
206 if (array_key_exists('tags', $data)) { 206 if (array_key_exists('tags', $data)) {
207 $this->tagsAssigner->assignTagsToEntry( 207 $this->tagsAssigner->assignTagsToEntry(
diff --git a/src/Wallabag/ImportBundle/Import/InstapaperImport.php b/src/Wallabag/ImportBundle/Import/InstapaperImport.php
index c8e0cd5b..3aa12f6f 100644
--- a/src/Wallabag/ImportBundle/Import/InstapaperImport.php
+++ b/src/Wallabag/ImportBundle/Import/InstapaperImport.php
@@ -125,7 +125,7 @@ class InstapaperImport extends AbstractImport
125 $entry->setTitle($importedEntry['title']); 125 $entry->setTitle($importedEntry['title']);
126 126
127 // update entry with content (in case fetching failed, the given entry will be return) 127 // update entry with content (in case fetching failed, the given entry will be return)
128 $entry = $this->fetchContent($entry, $importedEntry['url'], $importedEntry); 128 $this->fetchContent($entry, $importedEntry['url'], $importedEntry);
129 129
130 if (!empty($importedEntry['tags'])) { 130 if (!empty($importedEntry['tags'])) {
131 $this->tagsAssigner->assignTagsToEntry( 131 $this->tagsAssigner->assignTagsToEntry(
diff --git a/src/Wallabag/ImportBundle/Import/PinboardImport.php b/src/Wallabag/ImportBundle/Import/PinboardImport.php
index 489b9257..110b0464 100644
--- a/src/Wallabag/ImportBundle/Import/PinboardImport.php
+++ b/src/Wallabag/ImportBundle/Import/PinboardImport.php
@@ -109,7 +109,7 @@ class PinboardImport extends AbstractImport
109 $entry->setTitle($data['title']); 109 $entry->setTitle($data['title']);
110 110
111 // update entry with content (in case fetching failed, the given entry will be return) 111 // update entry with content (in case fetching failed, the given entry will be return)
112 $entry = $this->fetchContent($entry, $data['url'], $data); 112 $this->fetchContent($entry, $data['url'], $data);
113 113
114 if (!empty($data['tags'])) { 114 if (!empty($data['tags'])) {
115 $this->tagsAssigner->assignTagsToEntry( 115 $this->tagsAssigner->assignTagsToEntry(
diff --git a/src/Wallabag/ImportBundle/Import/PocketImport.php b/src/Wallabag/ImportBundle/Import/PocketImport.php
index 8835161b..c1d5b6da 100644
--- a/src/Wallabag/ImportBundle/Import/PocketImport.php
+++ b/src/Wallabag/ImportBundle/Import/PocketImport.php
@@ -192,7 +192,7 @@ class PocketImport extends AbstractImport
192 $entry->setUrl($url); 192 $entry->setUrl($url);
193 193
194 // update entry with content (in case fetching failed, the given entry will be return) 194 // update entry with content (in case fetching failed, the given entry will be return)
195 $entry = $this->fetchContent($entry, $url); 195 $this->fetchContent($entry, $url);
196 196
197 // 0, 1, 2 - 1 if the item is archived - 2 if the item should be deleted 197 // 0, 1, 2 - 1 if the item is archived - 2 if the item should be deleted
198 $entry->setArchived($importedEntry['status'] == 1 || $this->markAsRead); 198 $entry->setArchived($importedEntry['status'] == 1 || $this->markAsRead);
diff --git a/src/Wallabag/ImportBundle/Import/ReadabilityImport.php b/src/Wallabag/ImportBundle/Import/ReadabilityImport.php
index de320d23..002b27f4 100644
--- a/src/Wallabag/ImportBundle/Import/ReadabilityImport.php
+++ b/src/Wallabag/ImportBundle/Import/ReadabilityImport.php
@@ -109,7 +109,7 @@ class ReadabilityImport extends AbstractImport
109 $entry->setTitle($data['title']); 109 $entry->setTitle($data['title']);
110 110
111 // update entry with content (in case fetching failed, the given entry will be return) 111 // update entry with content (in case fetching failed, the given entry will be return)
112 $entry = $this->fetchContent($entry, $data['url'], $data); 112 $this->fetchContent($entry, $data['url'], $data);
113 113
114 $entry->setArchived($data['is_archived']); 114 $entry->setArchived($data['is_archived']);
115 $entry->setStarred($data['is_starred']); 115 $entry->setStarred($data['is_starred']);
diff --git a/src/Wallabag/ImportBundle/Import/WallabagImport.php b/src/Wallabag/ImportBundle/Import/WallabagImport.php
index 0e5382cf..c64ccd64 100644
--- a/src/Wallabag/ImportBundle/Import/WallabagImport.php
+++ b/src/Wallabag/ImportBundle/Import/WallabagImport.php
@@ -108,7 +108,7 @@ abstract class WallabagImport extends AbstractImport
108 $entry->setTitle($data['title']); 108 $entry->setTitle($data['title']);
109 109
110 // update entry with content (in case fetching failed, the given entry will be return) 110 // update entry with content (in case fetching failed, the given entry will be return)
111 $entry = $this->fetchContent($entry, $data['url'], $data); 111 $this->fetchContent($entry, $data['url'], $data);
112 112
113 if (array_key_exists('tags', $data)) { 113 if (array_key_exists('tags', $data)) {
114 $this->tagsAssigner->assignTagsToEntry( 114 $this->tagsAssigner->assignTagsToEntry(
diff --git a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
index 0c715d90..16643938 100644
--- a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
+++ b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
@@ -38,7 +38,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
38 ]); 38 ]);
39 39
40 $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage); 40 $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
41 $entry = $proxy->updateEntry(new Entry(new User()), 'http://user@:80'); 41 $entry = new Entry(new User());
42 $proxy->updateEntry($entry, 'http://user@:80');
42 43
43 $this->assertEquals('http://user@:80', $entry->getUrl()); 44 $this->assertEquals('http://user@:80', $entry->getUrl());
44 $this->assertEmpty($entry->getTitle()); 45 $this->assertEmpty($entry->getTitle());
@@ -72,7 +73,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
72 ]); 73 ]);
73 74
74 $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage); 75 $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
75 $entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0'); 76 $entry = new Entry(new User());
77 $proxy->updateEntry($entry, 'http://0.0.0.0');
76 78
77 $this->assertEquals('http://0.0.0.0', $entry->getUrl()); 79 $this->assertEquals('http://0.0.0.0', $entry->getUrl());
78 $this->assertEmpty($entry->getTitle()); 80 $this->assertEmpty($entry->getTitle());
@@ -111,7 +113,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
111 ]); 113 ]);
112 114
113 $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage); 115 $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
114 $entry = $proxy->updateEntry(new Entry(new User()), 'http://domain.io'); 116 $entry = new Entry(new User());
117 $proxy->updateEntry($entry, 'http://domain.io');
115 118
116 $this->assertEquals('http://domain.io', $entry->getUrl()); 119 $this->assertEquals('http://domain.io', $entry->getUrl());
117 $this->assertEquals('my title', $entry->getTitle()); 120 $this->assertEquals('my title', $entry->getTitle());
@@ -152,7 +155,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
152 ]); 155 ]);
153 156
154 $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage); 157 $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
155 $entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0'); 158 $entry = new Entry(new User());
159 $proxy->updateEntry($entry, 'http://0.0.0.0');
156 160
157 $this->assertEquals('http://1.1.1.1', $entry->getUrl()); 161 $this->assertEquals('http://1.1.1.1', $entry->getUrl());
158 $this->assertEquals('this is my title', $entry->getTitle()); 162 $this->assertEquals('this is my title', $entry->getTitle());
@@ -213,8 +217,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
213 ->method('tag'); 217 ->method('tag');
214 218
215 $proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage); 219 $proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage);
216 $entry = $proxy->updateEntry( 220 $entry = new Entry(new User());
217 new Entry(new User()), 221 $proxy->updateEntry(
222 $entry,
218 'http://0.0.0.0', 223 'http://0.0.0.0',
219 [ 224 [
220 'html' => str_repeat('this is my content', 325), 225 'html' => str_repeat('this is my content', 325),
@@ -251,8 +256,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
251 ->method('tag'); 256 ->method('tag');
252 257
253 $proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage); 258 $proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage);
254 $entry = $proxy->updateEntry( 259 $entry = new Entry(new User());
255 new Entry(new User()), 260 $proxy->updateEntry(
261 $entry,
256 'http://0.0.0.0', 262 'http://0.0.0.0',
257 [ 263 [
258 'html' => str_repeat('this is my content', 325), 264 'html' => str_repeat('this is my content', 325),
@@ -285,8 +291,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
285 $logger->pushHandler($handler); 291 $logger->pushHandler($handler);
286 292
287 $proxy = new ContentProxy((new Graby()), $tagger, $logger, $this->fetchingErrorMessage); 293 $proxy = new ContentProxy((new Graby()), $tagger, $logger, $this->fetchingErrorMessage);
288 $entry = $proxy->updateEntry( 294 $entry = new Entry(new User());
289 new Entry(new User()), 295 $proxy->updateEntry(
296 $entry,
290 'http://0.0.0.0', 297 'http://0.0.0.0',
291 [ 298 [
292 'html' => str_repeat('this is my content', 325), 299 'html' => str_repeat('this is my content', 325),
@@ -326,7 +333,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
326 333
327 $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage); 334 $proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
328 335
329 $entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0', [ 336 $entry = new Entry(new User());
337 $proxy->updateEntry($entry, 'http://0.0.0.0', [
330 'html' => str_repeat('this is my content', 325), 338 'html' => str_repeat('this is my content', 325),
331 'title' => 'this is my title', 339 'title' => 'this is my title',
332 'url' => 'http://1.1.1.1', 340 'url' => 'http://1.1.1.1',