Avoid losing entry when fetching fail

Instead of just say “Failed to save entry” we’ll save the entry at all cost and try to fetch content. If fetching content failed, the entry will still be saved at least, but without content.
This commit is contained in:
Jeremy Benoist 2016-09-17 07:40:56 +02:00
parent fbb319f064
commit 59b97fae99
No known key found for this signature in database
GPG key ID: BCA73962457ACC3C
18 changed files with 73 additions and 84 deletions

View file

@ -17,26 +17,35 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Cache;
class EntryController extends Controller
{
/**
* Fetch content and update entry.
* In case it fails, entry will return to avod loosing the data.
*
* @param Entry $entry
* @param string $prefixMessage Should be the translation key: entry_saved or entry_reloaded
*
* @return Entry
*/
private function updateEntry(Entry $entry)
private function updateEntry(Entry $entry, $prefixMessage = 'entry_saved')
{
// put default title in case of fetching content failed
$entry->setTitle('No title found');
$message = 'flashes.entry.notice.'.$prefixMessage;
try {
$entry = $this->get('wallabag_core.content_proxy')->updateEntry($entry, $entry->getUrl());
$em = $this->getDoctrine()->getManager();
$em->persist($entry);
$em->flush();
} catch (\Exception $e) {
$this->get('logger')->error('Error while saving an entry', [
'exception' => $e,
'entry' => $entry,
]);
return false;
$message = 'flashes.entry.notice.'.$prefixMessage.'_failed';
}
return true;
$this->get('session')->getFlashBag()->add('notice', $message);
return $entry;
}
/**
@ -66,12 +75,11 @@ class EntryController extends Controller
return $this->redirect($this->generateUrl('view', ['id' => $existingEntry->getId()]));
}
$message = 'flashes.entry.notice.entry_saved';
if (false === $this->updateEntry($entry)) {
$message = 'flashes.entry.notice.entry_saved_failed';
}
$this->updateEntry($entry);
$this->get('session')->getFlashBag()->add('notice', $message);
$em = $this->getDoctrine()->getManager();
$em->persist($entry);
$em->flush();
return $this->redirect($this->generateUrl('homepage'));
}
@ -95,6 +103,10 @@ class EntryController extends Controller
if (false === $this->checkIfEntryAlreadyExists($entry)) {
$this->updateEntry($entry);
$em = $this->getDoctrine()->getManager();
$em->persist($entry);
$em->flush();
}
return $this->redirect($this->generateUrl('homepage'));
@ -316,15 +328,11 @@ class EntryController extends Controller
{
$this->checkUserAction($entry);
$message = 'flashes.entry.notice.entry_reloaded';
if (false === $this->updateEntry($entry)) {
$message = 'flashes.entry.notice.entry_reload_failed';
}
$this->updateEntry($entry, 'entry_reloaded');
$this->get('session')->getFlashBag()->add(
'notice',
$message
);
$em = $this->getDoctrine()->getManager();
$em->persist($entry);
$em->flush();
return $this->redirect($this->generateUrl('view', ['id' => $entry->getId()]));
}

View file

@ -414,10 +414,10 @@ flashes:
notice:
# entry_already_saved: 'Entry already saved on %date%'
# entry_saved: 'Entry saved'
# entry_saved_failed: 'Failed to save entry'
# entry_saved_failed: 'Entry saved but fetching content failed'
# entry_updated: 'Entry updated'
# entry_reloaded: 'Entry reloaded'
# entry_reload_failed: 'Failed to reload entry'
# entry_reload_failed: 'Entry reloaded but fetching content failed'
entry_archived: 'Artikel arkiveret'
entry_unarchived: 'Artikel ikke længere arkiveret'
entry_starred: 'Artikel markeret som favorit'

View file

@ -414,10 +414,10 @@ flashes:
notice:
entry_already_saved: 'Eintrag bereits am %date% gespeichert'
entry_saved: 'Eintrag gespeichert'
# entry_saved_failed: 'Failed to save entry'
# entry_saved_failed: 'Entry saved but fetching content failed'
entry_updated: 'Eintrag aktualisiert'
entry_reloaded: 'Eintrag neugeladen'
entry_reload_failed: 'Neuladen des Eintrags fehlgeschlagen'
# entry_reload_failed: 'Entry reloaded but fetching content failed'
entry_archived: 'Artikel archiviert'
entry_unarchived: 'Artikel dearchiviert'
entry_starred: 'Artikel favorisiert'

View file

@ -416,10 +416,10 @@ flashes:
notice:
entry_already_saved: 'Entry already saved on %date%'
entry_saved: 'Entry saved'
entry_saved_failed: 'Failed to save entry'
entry_saved_failed: 'Entry saved but fetching content failed'
entry_updated: 'Entry updated'
entry_reloaded: 'Entry reloaded'
entry_reload_failed: 'Failed to reload entry'
entry_reload_failed: 'Entry reloaded but fetching content failed'
entry_archived: 'Entry archived'
entry_unarchived: 'Entry unarchived'
entry_starred: 'Entry starred'

View file

@ -414,10 +414,10 @@ flashes:
notice:
entry_already_saved: 'Entrada ya guardada por %fecha%'
entry_saved: 'Entrada guardada'
# entry_saved_failed: 'Failed to save entry'
# entry_saved_failed: 'Entry saved but fetching content failed'
entry_updated: 'Entrada actualizada'
entry_reloaded: 'Entrada recargada'
entry_reload_failed: 'Entrada recargada reprobada'
# entry_reload_failed: 'Entry reloaded but fetching content failed'
entry_archived: 'Artículo archivado'
entry_unarchived: 'Artículo desarchivado'
entry_starred: 'Artículo guardado en los favoritos'

View file

@ -414,10 +414,10 @@ flashes:
notice:
entry_already_saved: 'این مقاله در تاریخ %date% ذخیره شده بود'
entry_saved: 'مقاله ذخیره شد'
# entry_saved_failed: 'Failed to save entry'
# entry_saved_failed: 'Entry saved but fetching content failed'
entry_updated: 'مقاله به‌روز شد'
entry_reloaded: 'مقاله به‌روز شد'
entry_reload_failed: 'به‌روزرسانی مقاله شکست خورد'
# entry_reload_failed: 'Entry reloaded but fetching content failed'
entry_archived: 'مقاله بایگانی شد'
entry_unarchived: 'مقاله از بایگانی درآمد'
entry_starred: 'مقاله برگزیده شد'

View file

@ -416,10 +416,10 @@ flashes:
notice:
entry_already_saved: 'Article déjà sauvergardé le %date%'
entry_saved: 'Article enregistré'
entry_saved_failed: "L'enregistrement a échoué"
entry_saved_failed: 'Article enregistré mais impossible de récupérer le contenu'
entry_updated: 'Article mis à jour'
entry_reloaded: 'Article rechargé'
entry_reload_failed: "Le rechargement de l'article a échoué"
entry_reload_failed: "Article mis à jour mais impossible de récupérer le contenu"
entry_archived: 'Article marqué comme lu'
entry_unarchived: 'Article marqué comme non lu'
entry_starred: 'Article ajouté dans les favoris'

View file

@ -413,10 +413,10 @@ flashes:
notice:
entry_already_saved: 'Contenuto già salvato in data %date%'
entry_saved: 'Contenuto salvato'
# entry_saved_failed: 'Failed to save entry'
# entry_saved_failed: 'Entry saved but fetching content failed'
entry_updated: 'Contenuto aggiornato'
entry_reloaded: 'Contenuto ricaricato'
entry_reload_failed: 'Errore nel ricaricamento del contenuto'
# entry_reload_failed: 'Entry reloaded but fetching content failed'
entry_archived: 'Contenuto archiviato'
entry_unarchived: 'Contenuto dis-archiviato'
entry_starred: 'Contenuto segnato come preferito'

View file

@ -414,10 +414,10 @@ flashes:
notice:
entry_already_saved: 'Article ja salvargardat lo %date%'
entry_saved: 'Article enregistrat'
entry_saved_failed: "Fracàs de l'enregistrament de l'entrada"
# entry_saved_failed: 'Entry saved but fetching content failed'
entry_updated: 'Article mes a jorn'
entry_reloaded: 'Article recargat'
entry_reload_failed: "Fracàs de l'actualizacion de l'article"
# entry_reload_failed: 'Entry reloaded but fetching content failed'
entry_archived: 'Article marcat coma legit'
entry_unarchived: 'Article marcat coma pas legit'
entry_starred: 'Article apondut dins los favorits'

View file

@ -414,10 +414,10 @@ flashes:
notice:
entry_already_saved: 'Wpis już został dodany %date%'
entry_saved: 'Wpis zapisany'
entry_saved_failed: 'Zapis artykułu się nie powiódł'
# entry_saved_failed: 'Entry saved but fetching content failed'
entry_updated: 'Wpis zaktualizowany'
entry_reloaded: 'Wpis ponownie załadowany'
entry_reload_failed: 'Błąd ponownego załadowania'
# entry_reload_failed: 'Entry reloaded but fetching content failed'
entry_archived: 'Wpis dodany do archiwum'
entry_unarchived: 'Wpis usunięty z archiwum'
entry_starred: 'Wpis oznaczony gwiazdką'

View file

@ -414,10 +414,10 @@ flashes:
notice:
# entry_already_saved: 'Entry already saved on %date%'
# entry_saved: 'Entry saved'
# entry_saved_failed: 'Failed to save entry'
# entry_saved_failed: 'Entry saved but fetching content failed'
# entry_updated: 'Entry updated'
# entry_reloaded: 'Entry reloaded'
# entry_reload_failed: 'Failed to reload entry'
# entry_reload_failed: 'Entry reloaded but fetching content failed'
entry_archived: 'Articol arhivat'
entry_unarchived: 'Articol dezarhivat'
entry_starred: 'Articol adăugat la favorite'

View file

@ -414,10 +414,10 @@ flashes:
notice:
entry_already_saved: 'Entry already saved on %date%'
entry_saved: 'Makale kaydedildi'
# entry_saved_failed: 'Failed to save entry'
# entry_saved_failed: 'Entry saved but fetching content failed'
# entry_updated: 'Entry updated'
entry_reloaded: 'Makale içeriği yenilendi'
# entry_reload_failed: 'Failed to reload entry'
# entry_reload_failed: 'Entry reloaded but fetching content failed'
entry_archived: 'Makale arşivlendi'
entry_unarchived: 'Makale arşivden çıkartıldı'
entry_starred: 'Makale favorilere eklendi'

View file

@ -79,20 +79,20 @@ abstract class AbstractImport implements ImportInterface
/**
* Fetch content from the ContentProxy (using graby).
* If it fails return false instead of the updated entry.
* If it fails return the given entry to be saved in all case (to avoid user to loose the content).
*
* @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|false
* @return Entry
*/
protected function fetchContent(Entry $entry, $url, array $content = [])
{
try {
return $this->contentProxy->updateEntry($entry, $url, $content);
} catch (\Exception $e) {
return false;
return $entry;
}
}

View file

@ -199,24 +199,16 @@ class PocketImport extends AbstractImport
}
$entry = new Entry($this->user);
$entry->setUrl($url);
// update entry with content (in case fetching failed, the given entry will be return)
$entry = $this->fetchContent($entry, $url);
// jump to next entry in case of problem while getting content
if (false === $entry) {
++$this->skippedEntries;
return;
}
// 0, 1, 2 - 1 if the item is archived - 2 if the item should be deleted
if ($importedEntry['status'] == 1 || $this->markAsRead) {
$entry->setArchived(true);
}
$entry->setArchived($importedEntry['status'] == 1 || $this->markAsRead);
// 0 or 1 - 1 If the item is starred
if ($importedEntry['favorite'] == 1) {
$entry->setStarred(true);
}
$entry->setStarred($importedEntry['favorite'] == 1);
$title = 'Untitled';
if (isset($importedEntry['resolved_title']) && $importedEntry['resolved_title'] != '') {
@ -226,7 +218,6 @@ class PocketImport extends AbstractImport
}
$entry->setTitle($title);
$entry->setUrl($url);
// 0, 1, or 2 - 1 if the item has images in it - 2 if the item is an image
if (isset($importedEntry['has_image']) && $importedEntry['has_image'] > 0 && isset($importedEntry['images'][1])) {

View file

@ -103,18 +103,12 @@ class ReadabilityImport extends AbstractImport
'created_at' => $importedEntry['date_added'],
];
$entry = $this->fetchContent(
new Entry($this->user),
$data['url'],
$data
);
$entry = new Entry($this->user);
$entry->setUrl($data['url']);
$entry->setTitle($data['title']);
// jump to next entry in case of problem while getting content
if (false === $entry) {
++$this->skippedEntries;
return;
}
// update entry with content (in case fetching failed, the given entry will be return)
$entry = $this->fetchContent($entry, $data['url'], $data);
$entry->setArchived($data['is_archived']);
$entry->setStarred($data['is_starred']);

View file

@ -101,18 +101,12 @@ abstract class WallabagImport extends AbstractImport
$data = $this->prepareEntry($importedEntry);
$entry = $this->fetchContent(
new Entry($this->user),
$importedEntry['url'],
$data
);
$entry = new Entry($this->user);
$entry->setUrl($data['url']);
$entry->setTitle($data['title']);
// jump to next entry in case of problem while getting content
if (false === $entry) {
++$this->skippedEntries;
return;
}
// update entry with content (in case fetching failed, the given entry will be return)
$entry = $this->fetchContent($entry, $data['url'], $data);
if (array_key_exists('tags', $data)) {
$this->contentProxy->assignTagsToEntry(

View file

@ -566,6 +566,8 @@ JSON;
"status": 1,
"list": {
"229279689": {
"status": "1",
"favorite": "1",
"resolved_url": "http://www.grantland.com/blog/the-triangle/post/_/id/38347/ryder-cup-preview"
}
}
@ -603,6 +605,6 @@ JSON;
$res = $pocketImport->import();
$this->assertTrue($res);
$this->assertEquals(['skipped' => 1, 'imported' => 0, 'queued' => 0], $pocketImport->getSummary());
$this->assertEquals(['skipped' => 0, 'imported' => 1, 'queued' => 0], $pocketImport->getSummary());
}
}

View file

@ -256,6 +256,6 @@ class WallabagV2ImportTest extends \PHPUnit_Framework_TestCase
$res = $wallabagV2Import->import();
$this->assertTrue($res);
$this->assertEquals(['skipped' => 24, 'imported' => 0, 'queued' => 0], $wallabagV2Import->getSummary());
$this->assertEquals(['skipped' => 22, 'imported' => 2, 'queued' => 0], $wallabagV2Import->getSummary());
}
}