From a05b61159e147776f63baee731b5026796e5f7ae Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Fri, 30 Jun 2017 16:54:26 +0200 Subject: Fix PATCH method The PATCH method for the entry should only update what user sent to us and not the whole entry as it was before. Also, sending tags when patching an entry will now remove all current tags & assocatied new ones. --- .../ApiBundle/Controller/EntryRestController.php | 211 ++++++++++++++------- 1 file changed, 142 insertions(+), 69 deletions(-) (limited to 'src/Wallabag/ApiBundle/Controller/EntryRestController.php') 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', ''), + ]; } /** -- cgit v1.2.3 From c18a2476b601bc6b9893462d9be680c2e13c89e8 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Mon, 3 Jul 2017 13:56:39 +0200 Subject: CS --- src/Wallabag/ApiBundle/Controller/EntryRestController.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src/Wallabag/ApiBundle/Controller/EntryRestController.php') diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index a2e913af..8a206124 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php @@ -354,11 +354,11 @@ class EntryRestController extends WallabagRestController ]); } - if (!is_null($data['isArchived'])) { + if (null !== $data['isArchived']) { $entry->setArchived((bool) $data['isArchived']); } - if (!is_null($data['isStarred'])) { + if (null !== $data['isStarred']) { $entry->setStarred((bool) $data['isStarred']); } @@ -366,7 +366,7 @@ class EntryRestController extends WallabagRestController $this->get('wallabag_core.tags_assigner')->assignTagsToEntry($entry, $data['tags']); } - if (!is_null($data['isPublic'])) { + if (null !== $data['isPublic']) { if (true === (bool) $data['isPublic'] && null === $entry->getUid()) { $entry->generateUid(); } elseif (false === (bool) $data['isPublic']) { @@ -457,11 +457,11 @@ class EntryRestController extends WallabagRestController $contentProxy->updatePublishedAt($entry, $data['publishedAt']); } - if (!is_null($data['isArchived'])) { + if (null !== $data['isArchived']) { $entry->setArchived((bool) $data['isArchived']); } - if (!is_null($data['isStarred'])) { + if (null !== $data['isStarred']) { $entry->setStarred((bool) $data['isStarred']); } @@ -470,7 +470,7 @@ class EntryRestController extends WallabagRestController $this->get('wallabag_core.tags_assigner')->assignTagsToEntry($entry, $data['tags']); } - if (!is_null($data['isPublic'])) { + if (null !== $data['isPublic']) { if (true === (bool) $data['isPublic'] && null === $entry->getUid()) { $entry->generateUid(); } elseif (false === (bool) $data['isPublic']) { -- cgit v1.2.3