mirror of
https://github.com/wallabag/wallabag.git
synced 2024-11-26 19:11:07 +00:00
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.
This commit is contained in:
parent
2a0eec07a5
commit
7aba665e48
11 changed files with 30 additions and 32 deletions
|
@ -315,7 +315,7 @@ class EntryRestController extends WallabagRestController
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
$entry = $this->get('wallabag_core.content_proxy')->updateEntry(
|
$this->get('wallabag_core.content_proxy')->updateEntry(
|
||||||
$entry,
|
$entry,
|
||||||
$url,
|
$url,
|
||||||
[
|
[
|
||||||
|
@ -428,7 +428,7 @@ class EntryRestController extends WallabagRestController
|
||||||
$this->validateUserAccess($entry->getUser()->getId());
|
$this->validateUserAccess($entry->getUser()->getId());
|
||||||
|
|
||||||
try {
|
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) {
|
} catch (\Exception $e) {
|
||||||
$this->get('logger')->error('Error while saving an entry', [
|
$this->get('logger')->error('Error while saving an entry', [
|
||||||
'exception' => $e,
|
'exception' => $e,
|
||||||
|
|
|
@ -53,12 +53,10 @@ class EntryController extends Controller
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Fetch content and update entry.
|
* 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 Entry $entry
|
||||||
* @param string $prefixMessage Should be the translation key: entry_saved or entry_reloaded
|
* @param string $prefixMessage Should be the translation key: entry_saved or entry_reloaded
|
||||||
*
|
|
||||||
* @return Entry
|
|
||||||
*/
|
*/
|
||||||
private function updateEntry(Entry $entry, $prefixMessage = 'entry_saved')
|
private function updateEntry(Entry $entry, $prefixMessage = 'entry_saved')
|
||||||
{
|
{
|
||||||
|
@ -68,7 +66,7 @@ class EntryController extends Controller
|
||||||
$message = 'flashes.entry.notice.'.$prefixMessage;
|
$message = 'flashes.entry.notice.'.$prefixMessage;
|
||||||
|
|
||||||
try {
|
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) {
|
} catch (\Exception $e) {
|
||||||
$this->get('logger')->error('Error while saving an entry', [
|
$this->get('logger')->error('Error while saving an entry', [
|
||||||
'exception' => $e,
|
'exception' => $e,
|
||||||
|
@ -79,8 +77,6 @@ class EntryController extends Controller
|
||||||
}
|
}
|
||||||
|
|
||||||
$this->get('session')->getFlashBag()->add('notice', $message);
|
$this->get('session')->getFlashBag()->add('notice', $message);
|
||||||
|
|
||||||
return $entry;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -40,8 +40,6 @@ class ContentProxy
|
||||||
* @param Entry $entry Entry to update
|
* @param Entry $entry Entry to update
|
||||||
* @param string $url Url to grab content for
|
* @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
|
* @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 = [])
|
public function updateEntry(Entry $entry, $url, array $content = [])
|
||||||
{
|
{
|
||||||
|
@ -130,8 +128,6 @@ class ContentProxy
|
||||||
'error_msg' => $e->getMessage(),
|
'error_msg' => $e->getMessage(),
|
||||||
]);
|
]);
|
||||||
}
|
}
|
||||||
|
|
||||||
return $entry;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -91,13 +91,11 @@ abstract class AbstractImport implements ImportInterface
|
||||||
* @param Entry $entry Entry to update
|
* @param Entry $entry Entry to update
|
||||||
* @param string $url Url to grab content for
|
* @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
|
* @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 = [])
|
protected function fetchContent(Entry $entry, $url, array $content = [])
|
||||||
{
|
{
|
||||||
try {
|
try {
|
||||||
return $this->contentProxy->updateEntry($entry, $url, $content);
|
$this->contentProxy->updateEntry($entry, $url, $content);
|
||||||
} catch (\Exception $e) {
|
} catch (\Exception $e) {
|
||||||
return $entry;
|
return $entry;
|
||||||
}
|
}
|
||||||
|
|
|
@ -201,7 +201,7 @@ abstract class BrowserImport extends AbstractImport
|
||||||
$entry->setTitle($data['title']);
|
$entry->setTitle($data['title']);
|
||||||
|
|
||||||
// update entry with content (in case fetching failed, the given entry will be return)
|
// 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)) {
|
if (array_key_exists('tags', $data)) {
|
||||||
$this->tagsAssigner->assignTagsToEntry(
|
$this->tagsAssigner->assignTagsToEntry(
|
||||||
|
|
|
@ -125,7 +125,7 @@ class InstapaperImport extends AbstractImport
|
||||||
$entry->setTitle($importedEntry['title']);
|
$entry->setTitle($importedEntry['title']);
|
||||||
|
|
||||||
// update entry with content (in case fetching failed, the given entry will be return)
|
// 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'])) {
|
if (!empty($importedEntry['tags'])) {
|
||||||
$this->tagsAssigner->assignTagsToEntry(
|
$this->tagsAssigner->assignTagsToEntry(
|
||||||
|
|
|
@ -109,7 +109,7 @@ class PinboardImport extends AbstractImport
|
||||||
$entry->setTitle($data['title']);
|
$entry->setTitle($data['title']);
|
||||||
|
|
||||||
// update entry with content (in case fetching failed, the given entry will be return)
|
// 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'])) {
|
if (!empty($data['tags'])) {
|
||||||
$this->tagsAssigner->assignTagsToEntry(
|
$this->tagsAssigner->assignTagsToEntry(
|
||||||
|
|
|
@ -192,7 +192,7 @@ class PocketImport extends AbstractImport
|
||||||
$entry->setUrl($url);
|
$entry->setUrl($url);
|
||||||
|
|
||||||
// update entry with content (in case fetching failed, the given entry will be return)
|
// 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
|
// 0, 1, 2 - 1 if the item is archived - 2 if the item should be deleted
|
||||||
$entry->setArchived($importedEntry['status'] == 1 || $this->markAsRead);
|
$entry->setArchived($importedEntry['status'] == 1 || $this->markAsRead);
|
||||||
|
|
|
@ -109,7 +109,7 @@ class ReadabilityImport extends AbstractImport
|
||||||
$entry->setTitle($data['title']);
|
$entry->setTitle($data['title']);
|
||||||
|
|
||||||
// update entry with content (in case fetching failed, the given entry will be return)
|
// 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->setArchived($data['is_archived']);
|
||||||
$entry->setStarred($data['is_starred']);
|
$entry->setStarred($data['is_starred']);
|
||||||
|
|
|
@ -108,7 +108,7 @@ abstract class WallabagImport extends AbstractImport
|
||||||
$entry->setTitle($data['title']);
|
$entry->setTitle($data['title']);
|
||||||
|
|
||||||
// update entry with content (in case fetching failed, the given entry will be return)
|
// 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)) {
|
if (array_key_exists('tags', $data)) {
|
||||||
$this->tagsAssigner->assignTagsToEntry(
|
$this->tagsAssigner->assignTagsToEntry(
|
||||||
|
|
|
@ -38,7 +38,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
|
||||||
]);
|
]);
|
||||||
|
|
||||||
$proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
|
$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->assertEquals('http://user@:80', $entry->getUrl());
|
||||||
$this->assertEmpty($entry->getTitle());
|
$this->assertEmpty($entry->getTitle());
|
||||||
|
@ -72,7 +73,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
|
||||||
]);
|
]);
|
||||||
|
|
||||||
$proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
|
$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->assertEquals('http://0.0.0.0', $entry->getUrl());
|
||||||
$this->assertEmpty($entry->getTitle());
|
$this->assertEmpty($entry->getTitle());
|
||||||
|
@ -111,7 +113,8 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
|
||||||
]);
|
]);
|
||||||
|
|
||||||
$proxy = new ContentProxy($graby, $tagger, $this->getLogger(), $this->fetchingErrorMessage);
|
$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('http://domain.io', $entry->getUrl());
|
||||||
$this->assertEquals('my title', $entry->getTitle());
|
$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);
|
$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('http://1.1.1.1', $entry->getUrl());
|
||||||
$this->assertEquals('this is my title', $entry->getTitle());
|
$this->assertEquals('this is my title', $entry->getTitle());
|
||||||
|
@ -213,8 +217,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
|
||||||
->method('tag');
|
->method('tag');
|
||||||
|
|
||||||
$proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage);
|
$proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage);
|
||||||
$entry = $proxy->updateEntry(
|
$entry = new Entry(new User());
|
||||||
new Entry(new User()),
|
$proxy->updateEntry(
|
||||||
|
$entry,
|
||||||
'http://0.0.0.0',
|
'http://0.0.0.0',
|
||||||
[
|
[
|
||||||
'html' => str_repeat('this is my content', 325),
|
'html' => str_repeat('this is my content', 325),
|
||||||
|
@ -251,8 +256,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
|
||||||
->method('tag');
|
->method('tag');
|
||||||
|
|
||||||
$proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage);
|
$proxy = new ContentProxy((new Graby()), $tagger, $this->getLogger(), $this->fetchingErrorMessage);
|
||||||
$entry = $proxy->updateEntry(
|
$entry = new Entry(new User());
|
||||||
new Entry(new User()),
|
$proxy->updateEntry(
|
||||||
|
$entry,
|
||||||
'http://0.0.0.0',
|
'http://0.0.0.0',
|
||||||
[
|
[
|
||||||
'html' => str_repeat('this is my content', 325),
|
'html' => str_repeat('this is my content', 325),
|
||||||
|
@ -285,8 +291,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
|
||||||
$logger->pushHandler($handler);
|
$logger->pushHandler($handler);
|
||||||
|
|
||||||
$proxy = new ContentProxy((new Graby()), $tagger, $logger, $this->fetchingErrorMessage);
|
$proxy = new ContentProxy((new Graby()), $tagger, $logger, $this->fetchingErrorMessage);
|
||||||
$entry = $proxy->updateEntry(
|
$entry = new Entry(new User());
|
||||||
new Entry(new User()),
|
$proxy->updateEntry(
|
||||||
|
$entry,
|
||||||
'http://0.0.0.0',
|
'http://0.0.0.0',
|
||||||
[
|
[
|
||||||
'html' => str_repeat('this is my content', 325),
|
'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);
|
$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),
|
'html' => str_repeat('this is my content', 325),
|
||||||
'title' => 'this is my title',
|
'title' => 'this is my title',
|
||||||
'url' => 'http://1.1.1.1',
|
'url' => 'http://1.1.1.1',
|
||||||
|
|
Loading…
Reference in a new issue