diff --git a/src/Wallabag/AnnotationBundle/Controller/WallabagAnnotationController.php b/src/Wallabag/AnnotationBundle/Controller/WallabagAnnotationController.php index 883ce4a89..c94c58d84 100644 --- a/src/Wallabag/AnnotationBundle/Controller/WallabagAnnotationController.php +++ b/src/Wallabag/AnnotationBundle/Controller/WallabagAnnotationController.php @@ -3,9 +3,9 @@ namespace Wallabag\AnnotationBundle\Controller; use FOS\RestBundle\Controller\FOSRestController; -use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Wallabag\AnnotationBundle\Entity\Annotation; use Wallabag\AnnotationBundle\Form\EditAnnotationType; use Wallabag\AnnotationBundle\Form\NewAnnotationType; @@ -25,7 +25,7 @@ class WallabagAnnotationController extends FOSRestController $annotationRows = $this ->getDoctrine() ->getRepository('WallabagAnnotationBundle:Annotation') - ->findAnnotationsByPageId($entry->getId(), $this->getUser()->getId()); + ->findByEntryIdAndUserId($entry->getId(), $this->getUser()->getId()); $total = \count($annotationRows); $annotations = ['total' => $total, 'rows' => $annotationRows]; @@ -72,31 +72,35 @@ class WallabagAnnotationController extends FOSRestController * * @see Wallabag\ApiBundle\Controller\WallabagRestController * - * @ParamConverter("annotation", class="WallabagAnnotationBundle:Annotation") - * * @return JsonResponse */ - public function putAnnotationAction(Annotation $annotation, Request $request) + public function putAnnotationAction(Request $request, int $annotation) { - $data = json_decode($request->getContent(), true); + try { + $annotation = $this->validateAnnotation($annotation, $this->getUser()->getId()); - $form = $this->get('form.factory')->createNamed('', EditAnnotationType::class, $annotation, [ - 'csrf_protection' => false, - 'allow_extra_fields' => true, - ]); - $form->submit($data); + $data = json_decode($request->getContent(), true, 512, \JSON_THROW_ON_ERROR); - if ($form->isValid()) { - $em = $this->getDoctrine()->getManager(); - $em->persist($annotation); - $em->flush(); + $form = $this->get('form.factory')->createNamed('', EditAnnotationType::class, $annotation, [ + 'csrf_protection' => false, + 'allow_extra_fields' => true, + ]); + $form->submit($data); - $json = $this->get('jms_serializer')->serialize($annotation, 'json'); + if ($form->isValid()) { + $em = $this->getDoctrine()->getManager(); + $em->persist($annotation); + $em->flush(); - return JsonResponse::fromJsonString($json); + $json = $this->get('jms_serializer')->serialize($annotation, 'json'); + + return JsonResponse::fromJsonString($json); + } + + return $form; + } catch (\InvalidArgumentException $e) { + throw new NotFoundHttpException($e); } - - return $form; } /** @@ -104,18 +108,35 @@ class WallabagAnnotationController extends FOSRestController * * @see Wallabag\ApiBundle\Controller\WallabagRestController * - * @ParamConverter("annotation", class="WallabagAnnotationBundle:Annotation") - * * @return JsonResponse */ - public function deleteAnnotationAction(Annotation $annotation) + public function deleteAnnotationAction(int $annotation) + { + try { + $annotation = $this->validateAnnotation($annotation, $this->getUser()->getId()); + + $em = $this->getDoctrine()->getManager(); + $em->remove($annotation); + $em->flush(); + + $json = $this->get('jms_serializer')->serialize($annotation, 'json'); + + return (new JsonResponse())->setJson($json); + } catch (\InvalidArgumentException $e) { + throw new NotFoundHttpException($e); + } + } + + private function validateAnnotation(int $annotationId, int $userId) { $em = $this->getDoctrine()->getManager(); - $em->remove($annotation); - $em->flush(); - $json = $this->get('jms_serializer')->serialize($annotation, 'json'); + $annotation = $em->getRepository('WallabagAnnotationBundle:Annotation')->findOneByIdAndUserId($annotationId, $userId); - return (new JsonResponse())->setJson($json); + if (null === $annotation) { + throw new NotFoundHttpException(); + } + + return $annotation; } } diff --git a/src/Wallabag/AnnotationBundle/DataFixtures/AnnotationFixtures.php b/src/Wallabag/AnnotationBundle/DataFixtures/AnnotationFixtures.php index 51c79c3d3..5161a4094 100644 --- a/src/Wallabag/AnnotationBundle/DataFixtures/AnnotationFixtures.php +++ b/src/Wallabag/AnnotationBundle/DataFixtures/AnnotationFixtures.php @@ -34,6 +34,15 @@ class AnnotationFixtures extends Fixture implements DependentFixtureInterface $this->addReference('annotation2', $annotation2); + $annotation3 = new Annotation($this->getReference('bob-user')); + $annotation3->setEntry($this->getReference('entry3')); + $annotation3->setText('This is my first annotation !'); + $annotation3->setQuote('content'); + + $manager->persist($annotation3); + + $this->addReference('annotation3', $annotation3); + $manager->flush(); } diff --git a/src/Wallabag/AnnotationBundle/Repository/AnnotationRepository.php b/src/Wallabag/AnnotationBundle/Repository/AnnotationRepository.php index 0de5c934e..560f9eaf2 100644 --- a/src/Wallabag/AnnotationBundle/Repository/AnnotationRepository.php +++ b/src/Wallabag/AnnotationBundle/Repository/AnnotationRepository.php @@ -41,6 +41,24 @@ class AnnotationRepository extends EntityRepository ; } + /** + * Find annotation by id and user. + * + * @param int $annotationId + * @param int $userId + * + * @return Annotation + */ + public function findOneByIdAndUserId($annotationId, $userId) + { + return $this->createQueryBuilder('a') + ->where('a.id = :annotationId')->setParameter('annotationId', $annotationId) + ->andWhere('a.user = :userId')->setParameter('userId', $userId) + ->setMaxResults(1) + ->getQuery() + ->getOneOrNullResult(); + } + /** * Find annotations for entry id. * @@ -49,7 +67,7 @@ class AnnotationRepository extends EntityRepository * * @return array */ - public function findAnnotationsByPageId($entryId, $userId) + public function findByEntryIdAndUserId($entryId, $userId) { return $this->createQueryBuilder('a') ->where('a.entry = :entryId')->setParameter('entryId', $entryId) @@ -66,7 +84,7 @@ class AnnotationRepository extends EntityRepository * * @return array */ - public function findLastAnnotationByPageId($entryId, $userId) + public function findLastAnnotationByUserId($entryId, $userId) { return $this->createQueryBuilder('a') ->where('a.entry = :entryId')->setParameter('entryId', $entryId) diff --git a/src/Wallabag/ApiBundle/Controller/AnnotationRestController.php b/src/Wallabag/ApiBundle/Controller/AnnotationRestController.php index 66693189a..7061845d3 100644 --- a/src/Wallabag/ApiBundle/Controller/AnnotationRestController.php +++ b/src/Wallabag/ApiBundle/Controller/AnnotationRestController.php @@ -3,7 +3,6 @@ namespace Wallabag\ApiBundle\Controller; use Nelmio\ApiDocBundle\Annotation\ApiDoc; -use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Wallabag\AnnotationBundle\Entity\Annotation; @@ -63,11 +62,9 @@ class AnnotationRestController extends WallabagRestController * } * ) * - * @ParamConverter("annotation", class="WallabagAnnotationBundle:Annotation") - * * @return JsonResponse */ - public function putAnnotationAction(Annotation $annotation, Request $request) + public function putAnnotationAction(int $annotation, Request $request) { $this->validateAuthentication(); @@ -86,11 +83,9 @@ class AnnotationRestController extends WallabagRestController * } * ) * - * @ParamConverter("annotation", class="WallabagAnnotationBundle:Annotation") - * * @return JsonResponse */ - public function deleteAnnotationAction(Annotation $annotation) + public function deleteAnnotationAction(int $annotation) { $this->validateAuthentication(); diff --git a/tests/Wallabag/AnnotationBundle/Controller/AnnotationControllerTest.php b/tests/Wallabag/AnnotationBundle/Controller/AnnotationControllerTest.php index 260edd770..2e2ad032c 100644 --- a/tests/Wallabag/AnnotationBundle/Controller/AnnotationControllerTest.php +++ b/tests/Wallabag/AnnotationBundle/Controller/AnnotationControllerTest.php @@ -22,8 +22,6 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase } /** - * Test fetching annotations for an entry. - * * @dataProvider dataForEachAnnotations */ public function testGetAnnotations($prefixUrl) @@ -35,15 +33,7 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase ->findOneByUserName('admin'); $entry = $em ->getRepository('WallabagCoreBundle:Entry') - ->findOneByUsernameAndNotArchived('admin'); - - $annotation = new Annotation($user); - $annotation->setEntry($entry); - $annotation->setText('This is my annotation /o/'); - $annotation->setQuote('content'); - - $em->persist($annotation); - $em->flush(); + ->findByUrlAndUserId('http://0.0.0.0/entry1', $user->getId()); if ('annotations' === $prefixUrl) { $this->logInAs('admin'); @@ -54,23 +44,44 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase $content = json_decode($this->client->getResponse()->getContent(), true); $this->assertGreaterThanOrEqual(1, $content['total']); - $this->assertSame($annotation->getText(), $content['rows'][0]['text']); - - // we need to re-fetch the annotation becase after the flush, it has been "detached" from the entity manager - $annotation = $em->getRepository('WallabagAnnotationBundle:Annotation')->findAnnotationById($annotation->getId()); - $em->remove($annotation); - $em->flush(); } /** - * Test creating an annotation for an entry. - * + * @dataProvider dataForEachAnnotations + */ + public function testGetAnnotationsFromAnOtherUser($prefixUrl) + { + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + + $otherUser = $em + ->getRepository('WallabagUserBundle:User') + ->findOneByUserName('bob'); + $entry = $em + ->getRepository('WallabagCoreBundle:Entry') + ->findByUrlAndUserId('http://0.0.0.0/entry3', $otherUser->getId()); + + if ('annotations' === $prefixUrl) { + $this->logInAs('admin'); + } + + $this->client->request('GET', $prefixUrl . '/' . $entry->getId() . '.json'); + $this->assertSame(200, $this->client->getResponse()->getStatusCode()); + + $content = json_decode($this->client->getResponse()->getContent(), true); + $this->assertGreaterThanOrEqual(0, $content['total']); + } + + /** * @dataProvider dataForEachAnnotations */ public function testSetAnnotation($prefixUrl) { $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + $user = $em + ->getRepository('WallabagUserBundle:User') + ->findOneByUserName('admin'); + if ('annotations' === $prefixUrl) { $this->logInAs('admin'); } @@ -102,7 +113,7 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase /** @var Annotation $annotation */ $annotation = $em ->getRepository('WallabagAnnotationBundle:Annotation') - ->findLastAnnotationByPageId($entry->getId(), 1); + ->findLastAnnotationByUserId($entry->getId(), $user->getId()); $this->assertSame('my annotation', $annotation->getText()); } @@ -195,8 +206,6 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase } /** - * Test editing an existing annotation. - * * @dataProvider dataForEachAnnotations */ public function testEditAnnotation($prefixUrl) @@ -243,8 +252,31 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase } /** - * Test deleting an annotation. - * + * @dataProvider dataForEachAnnotations + */ + public function testEditAnnotationFromAnOtherUser($prefixUrl) + { + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + + $otherUser = $em + ->getRepository('WallabagUserBundle:User') + ->findOneByUserName('bob'); + $entry = $em + ->getRepository('WallabagCoreBundle:Entry') + ->findByUrlAndUserId('http://0.0.0.0/entry3', $otherUser->getId()); + $annotation = $em + ->getRepository('WallabagAnnotationBundle:Annotation') + ->findLastAnnotationByUserId($entry->getId(), $otherUser->getId()); + + $headers = ['CONTENT_TYPE' => 'application/json']; + $content = json_encode([ + 'text' => 'a modified annotation', + ]); + $this->client->request('PUT', $prefixUrl . '/' . $annotation->getId() . '.json', [], [], $headers, $content); + $this->assertSame(404, $this->client->getResponse()->getStatusCode()); + } + + /** * @dataProvider dataForEachAnnotations */ public function testDeleteAnnotation($prefixUrl) @@ -287,4 +319,40 @@ class AnnotationControllerTest extends WallabagAnnotationTestCase $this->assertNull($annotationDeleted); } + + /** + * @dataProvider dataForEachAnnotations + */ + public function testDeleteAnnotationFromAnOtherUser($prefixUrl) + { + $em = $this->client->getContainer()->get('doctrine.orm.entity_manager'); + + $otherUser = $em + ->getRepository('WallabagUserBundle:User') + ->findOneByUserName('bob'); + $entry = $em + ->getRepository('WallabagCoreBundle:Entry') + ->findByUrlAndUserId('http://0.0.0.0/entry3', $otherUser->getId()); + $annotation = $em + ->getRepository('WallabagAnnotationBundle:Annotation') + ->findLastAnnotationByUserId($entry->getId(), $otherUser->getId()); + + $user = $em + ->getRepository('WallabagUserBundle:User') + ->findOneByUserName('admin'); + $entry = $em + ->getRepository('WallabagCoreBundle:Entry') + ->findOneByUsernameAndNotArchived('admin'); + + if ('annotations' === $prefixUrl) { + $this->logInAs('admin'); + } + + $headers = ['CONTENT_TYPE' => 'application/json']; + $content = json_encode([ + 'text' => 'a modified annotation', + ]); + $this->client->request('DELETE', $prefixUrl . '/' . $annotation->getId() . '.json', [], [], $headers, $content); + $this->assertSame(404, $this->client->getResponse()->getStatusCode()); + } } diff --git a/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php b/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php index acd8d58e7..d6a0f0d78 100644 --- a/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php +++ b/tests/Wallabag/CoreBundle/Controller/ConfigControllerTest.php @@ -932,7 +932,7 @@ class ConfigControllerTest extends WallabagCoreTestCase $annotationsReset = $em ->getRepository('WallabagAnnotationBundle:Annotation') - ->findAnnotationsByPageId($entry->getId(), $user->getId()); + ->findByEntryIdAndUserId($entry->getId(), $user->getId()); $this->assertEmpty($annotationsReset, 'Annotations were reset'); @@ -1040,7 +1040,7 @@ class ConfigControllerTest extends WallabagCoreTestCase $annotationsReset = $em ->getRepository('WallabagAnnotationBundle:Annotation') - ->findAnnotationsByPageId($annotationArchived->getId(), $user->getId()); + ->findByEntryIdAndUserId($annotationArchived->getId(), $user->getId()); $this->assertEmpty($annotationsReset, 'Annotations were reset'); } @@ -1097,7 +1097,7 @@ class ConfigControllerTest extends WallabagCoreTestCase $annotationsReset = $em ->getRepository('WallabagAnnotationBundle:Annotation') - ->findAnnotationsByPageId($entry->getId(), $user->getId()); + ->findByEntryIdAndUserId($entry->getId(), $user->getId()); $this->assertEmpty($annotationsReset, 'Annotations were reset'); }