mirrors/wallabag
1
0
Fork 0
mirror of https://github.com/wallabag/wallabag.git synced 2024-05-31 15:38:04 +00:00
Code Issues Projects Releases Wiki Activity

Fix PATCH method

Browse source 
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.
This commit is contained in:
Jeremy Benoist 2017-06-30 16:54:26 +02:00
parent 71e1cbc8eb
commit a05b61159e
No known key found for this signature in database
GPG key ID: BCA73962457ACC3C
5 changed files with 270 additions and 121 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

211
src/Wallabag/ApiBundle/Controller/EntryRestController.php
View file

@ -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', ''),
];
}
/**

16
src/Wallabag/CoreBundle/Entity/Entry.php
View file

@ -593,6 +593,11 @@ class Entry
$tag->addEntry($this);
}
/**
* Remove the given tag from the entry (if the tag is associated).
*
* @param Tag $tag
*/
public function removeTag(Tag $tag)
{
if (!$this->tags->contains($tag)) {
@ -603,6 +608,17 @@ class Entry
$tag->removeEntry($this);
}
/**
* Remove all assigned tags from the entry.
*/
public function removeAllTags()
{
foreach ($this->tags as $tag) {
$this->tags->removeElement($tag);
$tag->removeEntry($this);
}
}
/**
* Set previewPicture.
*

89
src/Wallabag/CoreBundle/Helper/ContentProxy.php
View file

@ -75,9 +75,17 @@ class ContentProxy
*/
private function stockEntry(Entry $entry, array $content)
{
$title = $content['title'];
if (!$title && !empty($content['open_graph']['og_title'])) {
$title = $content['open_graph']['og_title'];
$entry->setUrl($content['url']);
$domainName = parse_url($entry->getUrl(), PHP_URL_HOST);
if (false !== $domainName) {
$entry->setDomainName($domainName);
}
if (!empty($content['title'])) {
$entry->setTitle($content['title']);
} elseif (!empty($content['open_graph']['og_title'])) {
$entry->setTitle($content['open_graph']['og_title']);
}
$html = $content['html'];
@ -90,24 +98,11 @@ class ContentProxy
}
}
$entry->setUrl($content['url']);
$entry->setTitle($title);
$entry->setContent($html);
$entry->setHttpStatus(isset($content['status']) ? $content['status'] : '');
$entry->setReadingTime(Utils::getReadingTime($html));
if (!empty($content['date'])) {
$date = $content['date'];
// is it a timestamp?
if (filter_var($date, FILTER_VALIDATE_INT) !== false) {
$date = '@' . $content['date'];
}
try {
$entry->setPublishedAt(new \DateTime($date));
} catch (\Exception $e) {
$this->logger->warning('Error while defining date', ['e' => $e, 'url' => $content['url'], 'date' => $content['date']]);
}
if (!empty($content['status'])) {
$entry->setHttpStatus($content['status']);
}
if (!empty($content['authors']) && is_array($content['authors'])) {
@ -118,15 +113,17 @@ class ContentProxy
$entry->setHeaders($content['all_headers']);
}
$this->validateAndSetLanguage(
$entry,
isset($content['language']) ? $content['language'] : null
);
if (!empty($content['date'])) {
$this->updatePublishedAt($entry, $content['date']);
}
$this->validateAndSetPreviewPicture(
$entry,
isset($content['open_graph']['og_image']) ? $content['open_graph']['og_image'] : null
);
if (!empty($content['language'])) {
$this->updateLanguage($entry, $content['language']);
}
if (!empty($content['open_graph']['og_image'])) {
$this->updatePreviewPicture($entry, $content['open_graph']['og_image']);
}
// if content is an image, define it as a preview too
if (!empty($content['content_type']) && in_array($this->mimeGuesser->guess($content['content_type']), ['jpeg', 'jpg', 'gif', 'png'], true)) {
@ -136,12 +133,8 @@ class ContentProxy
);
}
$entry->setMimetype(isset($content['content_type']) ? $content['content_type'] : '');
$entry->setReadingTime(Utils::getReadingTime($html));
$domainName = parse_url($entry->getUrl(), PHP_URL_HOST);
if (false !== $domainName) {
$entry->setDomainName($domainName);
if (!empty($content['content_type'])) {
$entry->setMimetype($content['content_type']);
}
try {
@ -170,9 +163,9 @@ class ContentProxy
* Use a Symfony validator to ensure the language is well formatted.
*
* @param Entry $entry
* @param string $value Language to validate
* @param string $value Language to validate and save
*/
private function validateAndSetLanguage($entry, $value)
public function updateLanguage(Entry $entry, $value)
{
// some lang are defined as fr-FR, es-ES.
// replacing - by _ might increase language support
@ -196,9 +189,9 @@ class ContentProxy
* Use a Symfony validator to ensure the preview picture is a real url.
*
* @param Entry $entry
* @param string $value URL to validate
* @param string $value URL to validate and save
*/
private function validateAndSetPreviewPicture($entry, $value)
public function updatePreviewPicture(Entry $entry, $value)
{
$errors = $this->validator->validate(
$value,
@ -213,4 +206,26 @@ class ContentProxy
$this->logger->warning('PreviewPicture validation failed. ' . (string) $errors);
}
/**
* Update date.
*
* @param Entry $entry
* @param string $value Date to validate and save
*/
public function updatePublishedAt(Entry $entry, $value)
{
$date = $value;
// is it a timestamp?
if (filter_var($date, FILTER_VALIDATE_INT) !== false) {
$date = '@'.$value;
}
try {
$entry->setPublishedAt(new \DateTime($date));
} catch (\Exception $e) {
$this->logger->warning('Error while defining date', ['e' => $e, 'url' => $entry->getUrl(), 'date' => $value]);
}
}
}

68
tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php
View file

@ -519,10 +519,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->markTestSkipped('No content found in db.');
}
// hydrate the tags relations
$nbTags = count($entry->getTags());
$this->client->request('PATCH', '/api/entries/' . $entry->getId() . '.json', [
$this->client->request('PATCH', '/api/entries/'.$entry->getId().'.json', [
'title' => 'New awesome title',
'tags' => 'new tag ' . uniqid(),
'starred' => '1',
@ -532,6 +529,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
'authors' => 'bob,sponge',
'content' => 'awesome',
'public' => 0,
'published_at' => 1488833381,
]);
$this->assertSame(200, $this->client->getResponse()->getStatusCode());
@ -541,7 +539,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->assertSame($entry->getId(), $content['id']);
$this->assertSame($entry->getUrl(), $content['url']);
$this->assertSame('New awesome title', $content['title']);
$this->assertGreaterThan($nbTags, count($content['tags']));
$this->assertGreaterThanOrEqual(1, count($content['tags']), 'We force only one tag');
$this->assertSame(1, $content['user_id']);
$this->assertSame('de_AT', $content['language']);
$this->assertSame('http://preview.io/picture.jpg', $content['preview_picture']);
@ -549,6 +547,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->assertContains('bob', $content['published_by']);
$this->assertSame('awesome', $content['content']);
$this->assertFalse($content['is_public'], 'Entry is no more shared');
$this->assertContains('2017-03-06', $content['published_at']);
}
public function testPatchEntryWithoutQuotes()
@ -562,8 +561,8 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->markTestSkipped('No content found in db.');
}
// hydrate the tags relations
$nbTags = count($entry->getTags());
$previousContent = $entry->getContent();
$previousLanguage = $entry->getLanguage();
$this->client->request('PATCH', '/api/entries/' . $entry->getId() . '.json', [
'title' => 'New awesome title',
@ -579,9 +578,11 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->assertSame($entry->getId(), $content['id']);
$this->assertSame($entry->getUrl(), $content['url']);
$this->assertSame('New awesome title', $content['title']);
$this->assertGreaterThanOrEqual(1, count($content['tags']), 'We force only one tag');
$this->assertGreaterThan($nbTags, count($content['tags']));
$this->assertEmpty($content['published_by'], 'Authors were not saved because of an array instead of a string');
$this->assertEquals($previousContent, $content['content'], 'Ensure content has not moved');
$this->assertEquals($previousLanguage, $content['language'], 'Ensure language has not moved');
}
public function testGetTagsEntry()
@ -727,8 +728,10 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->markTestSkipped('No content found in db.');
}
$this->client->request('PATCH', '/api/entries/' . $entry->getId() . '.json', [
'title' => $entry->getTitle() . '++',
$previousTitle = $entry->getTitle();
$this->client->request('PATCH', '/api/entries/'.$entry->getId().'.json', [
'title' => $entry->getTitle().'++',
]);
$this->assertSame(200, $this->client->getResponse()->getStatusCode());
@ -736,6 +739,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
$content = json_decode($this->client->getResponse()->getContent(), true);
$this->assertSame(1, $content['is_archived']);
$this->assertEquals($previousTitle.'++', $content['title']);
}
public function testSaveIsStarredAfterPatch()
@ -907,6 +911,17 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->assertCount(4, $tags);
}
public function testPostEntriesTagsListActionNoList()
{
$this->client->request('POST', '/api/entries/tags/lists?list='.json_encode([]));
$this->assertEquals(200, $this->client->getResponse()->getStatusCode());
$content = json_decode($this->client->getResponse()->getContent(), true);
$this->assertEmpty($content);
}
public function testDeleteEntriesTagsListAction()
{
$em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
@ -933,6 +948,17 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->assertCount(0, $entry->getTags());
}
public function testDeleteEntriesTagsListActionNoList()
{
$this->client->request('DELETE', '/api/entries/tags/list?list='.json_encode([]));
$this->assertEquals(200, $this->client->getResponse()->getStatusCode());
$content = json_decode($this->client->getResponse()->getContent(), true);
$this->assertEmpty($content);
}
public function testPostEntriesListAction()
{
$list = [
@ -953,6 +979,17 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->assertSame('http://0.0.0.0/entry2', $content[1]['url']);
}
public function testPostEntriesListActionWithNoUrls()
{
$this->client->request('POST', '/api/entries/lists?urls='.json_encode([]));
$this->assertEquals(200, $this->client->getResponse()->getStatusCode());
$content = json_decode($this->client->getResponse()->getContent(), true);
$this->assertEmpty($content);
}
public function testDeleteEntriesListAction()
{
$em = $this->client->getContainer()->get('doctrine.orm.entity_manager');
@ -978,6 +1015,17 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->assertSame('http://0.0.0.0/test-entry-not-exist', $content[1]['url']);
}
public function testDeleteEntriesListActionWithNoUrls()
{
$this->client->request('DELETE', '/api/entries/list?urls='.json_encode([]));
$this->assertEquals(200, $this->client->getResponse()->getStatusCode());
$content = json_decode($this->client->getResponse()->getContent(), true);
$this->assertEmpty($content);
}
public function testLimitBulkAction()
{
$list = [

7
tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
View file

@ -221,12 +221,9 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
->method('tag');
$validator = $this->getValidator();
$validator->expects($this->exactly(2))
$validator->expects($this->once())
->method('validate')
->will($this->onConsecutiveCalls(
new ConstraintViolationList([new ConstraintViolation('oops', 'oops', [], 'oops', 'language', 'dontexist')]),
new ConstraintViolationList()
));
->willReturn(new ConstraintViolationList([new ConstraintViolation('oops', 'oops', [], 'oops', 'language', 'dontexist')]));
$graby = $this->getMockBuilder('Graby\Graby')
->setMethods(['fetchContent'])