aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorThomas Citharel <tcit@tcit.fr>2016-09-25 16:11:06 +0200
committerGitHub <noreply@github.com>2016-09-25 16:11:06 +0200
commit7e98ad962680fac17b3b90ae34b9c6e5afe7636f (patch)
tree451f2c7ec7011f1f0ee8911fb1921b02f158e1dc
parent9d7dd6b0d2480d3efff5b0ab1461f2ef99bfd57a (diff)
parent289875836a09944f5993d33753042abfef13809e (diff)
downloadwallabag-7e98ad962680fac17b3b90ae34b9c6e5afe7636f.tar.gz
wallabag-7e98ad962680fac17b3b90ae34b9c6e5afe7636f.tar.zst
wallabag-7e98ad962680fac17b3b90ae34b9c6e5afe7636f.zip
Merge pull request #2308 from wallabag/tags-duplicate
Fix duplicate tags on import
-rw-r--r--src/Wallabag/ApiBundle/Controller/WallabagRestController.php4
-rw-r--r--src/Wallabag/CoreBundle/Controller/TagController.php28
-rw-r--r--src/Wallabag/CoreBundle/Helper/ContentProxy.php26
-rw-r--r--src/Wallabag/CoreBundle/Repository/TagRepository.php34
-rw-r--r--src/Wallabag/CoreBundle/Twig/WallabagExtension.php31
-rw-r--r--src/Wallabag/ImportBundle/Import/PocketImport.php3
-rw-r--r--src/Wallabag/ImportBundle/Import/WallabagImport.php3
-rw-r--r--tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php23
-rw-r--r--tests/Wallabag/ImportBundle/Import/PocketImportTest.php14
-rw-r--r--tests/Wallabag/ImportBundle/Import/WallabagV1ImportTest.php14
-rw-r--r--tests/Wallabag/ImportBundle/Import/WallabagV2ImportTest.php14
11 files changed, 136 insertions, 58 deletions
diff --git a/src/Wallabag/ApiBundle/Controller/WallabagRestController.php b/src/Wallabag/ApiBundle/Controller/WallabagRestController.php
index fb7c6c1f..dd17ef97 100644
--- a/src/Wallabag/ApiBundle/Controller/WallabagRestController.php
+++ b/src/Wallabag/ApiBundle/Controller/WallabagRestController.php
@@ -322,9 +322,7 @@ class WallabagRestController extends FOSRestController
322 322
323 $tags = $this->getDoctrine() 323 $tags = $this->getDoctrine()
324 ->getRepository('WallabagCoreBundle:Tag') 324 ->getRepository('WallabagCoreBundle:Tag')
325 ->findAllTags($this->getUser()->getId()) 325 ->findAllTagsWithEntries($this->getUser()->getId());
326 ->getQuery()
327 ->getResult();
328 326
329 $json = $this->get('serializer')->serialize($tags, 'json'); 327 $json = $this->get('serializer')->serialize($tags, 'json');
330 328
diff --git a/src/Wallabag/CoreBundle/Controller/TagController.php b/src/Wallabag/CoreBundle/Controller/TagController.php
index bc95a4d3..623a6146 100644
--- a/src/Wallabag/CoreBundle/Controller/TagController.php
+++ b/src/Wallabag/CoreBundle/Controller/TagController.php
@@ -84,16 +84,11 @@ class TagController extends Controller
84 { 84 {
85 $tags = $this->getDoctrine() 85 $tags = $this->getDoctrine()
86 ->getRepository('WallabagCoreBundle:Tag') 86 ->getRepository('WallabagCoreBundle:Tag')
87 ->findAllTags($this->getUser()->getId()) 87 ->findAllTagsWithEntries($this->getUser()->getId());
88 ->getQuery() 88
89 ->getResult(); 89 return $this->render('WallabagCoreBundle:Tag:tags.html.twig', [
90 90 'tags' => $tags,
91 return $this->render( 91 ]);
92 'WallabagCoreBundle:Tag:tags.html.twig',
93 [
94 'tags' => $tags,
95 ]
96 );
97 } 92 }
98 93
99 /** 94 /**
@@ -127,13 +122,10 @@ class TagController extends Controller
127 } 122 }
128 } 123 }
129 124
130 return $this->render( 125 return $this->render('WallabagCoreBundle:Entry:entries.html.twig', [
131 'WallabagCoreBundle:Entry:entries.html.twig', 126 'form' => null,
132 [ 127 'entries' => $entries,
133 'form' => null, 128 'currentPage' => $page,
134 'entries' => $entries, 129 ]);
135 'currentPage' => $page,
136 ]
137 );
138 } 130 }
139} 131}
diff --git a/src/Wallabag/CoreBundle/Helper/ContentProxy.php b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
index 5dd684f2..8019df42 100644
--- a/src/Wallabag/CoreBundle/Helper/ContentProxy.php
+++ b/src/Wallabag/CoreBundle/Helper/ContentProxy.php
@@ -95,14 +95,24 @@ class ContentProxy
95 * Assign some tags to an entry. 95 * Assign some tags to an entry.
96 * 96 *
97 * @param Entry $entry 97 * @param Entry $entry
98 * @param array|string $tags An array of tag or a string coma separated of tag 98 * @param array|string $tags An array of tag or a string coma separated of tag
99 * @param array $entitiesReady Entities from the EntityManager which are persisted but not yet flushed
100 * It is mostly to fix duplicate tag on import @see http://stackoverflow.com/a/7879164/569101
99 */ 101 */
100 public function assignTagsToEntry(Entry $entry, $tags) 102 public function assignTagsToEntry(Entry $entry, $tags, array $entitiesReady = [])
101 { 103 {
102 if (!is_array($tags)) { 104 if (!is_array($tags)) {
103 $tags = explode(',', $tags); 105 $tags = explode(',', $tags);
104 } 106 }
105 107
108 // keeps only Tag entity from the "not yet flushed entities"
109 $tagsNotYetFlushed = [];
110 foreach ($entitiesReady as $entity) {
111 if ($entity instanceof Tag) {
112 $tagsNotYetFlushed[$entity->getLabel()] = $entity;
113 }
114 }
115
106 foreach ($tags as $label) { 116 foreach ($tags as $label) {
107 $label = trim($label); 117 $label = trim($label);
108 118
@@ -111,11 +121,15 @@ class ContentProxy
111 continue; 121 continue;
112 } 122 }
113 123
114 $tagEntity = $this->tagRepository->findOneByLabel($label); 124 if (isset($tagsNotYetFlushed[$label])) {
125 $tagEntity = $tagsNotYetFlushed[$label];
126 } else {
127 $tagEntity = $this->tagRepository->findOneByLabel($label);
115 128
116 if (is_null($tagEntity)) { 129 if (is_null($tagEntity)) {
117 $tagEntity = new Tag(); 130 $tagEntity = new Tag();
118 $tagEntity->setLabel($label); 131 $tagEntity->setLabel($label);
132 }
119 } 133 }
120 134
121 // only add the tag on the entry if the relation doesn't exist 135 // only add the tag on the entry if the relation doesn't exist
diff --git a/src/Wallabag/CoreBundle/Repository/TagRepository.php b/src/Wallabag/CoreBundle/Repository/TagRepository.php
index 41f61607..9d127da7 100644
--- a/src/Wallabag/CoreBundle/Repository/TagRepository.php
+++ b/src/Wallabag/CoreBundle/Repository/TagRepository.php
@@ -7,17 +7,45 @@ use Doctrine\ORM\EntityRepository;
7class TagRepository extends EntityRepository 7class TagRepository extends EntityRepository
8{ 8{
9 /** 9 /**
10 * Find Tags. 10 * Count all tags per user.
11 *
12 * @param int $userId
13 * @param int $cacheLifeTime Duration of the cache for this query
14 *
15 * @return int
16 */
17 public function countAllTags($userId, $cacheLifeTime = null)
18 {
19 $query = $this->createQueryBuilder('t')
20 ->select('t.slug')
21 ->leftJoin('t.entries', 'e')
22 ->where('e.user = :userId')->setParameter('userId', $userId)
23 ->groupBy('t.slug')
24 ->getQuery();
25
26 if (null !== $cacheLifeTime) {
27 $query->useQueryCache(true);
28 $query->useResultCache(true);
29 $query->setResultCacheLifetime($cacheLifeTime);
30 }
31
32 return count($query->getArrayResult());
33 }
34
35 /**
36 * Find all tags with associated entries per user.
11 * 37 *
12 * @param int $userId 38 * @param int $userId
13 * 39 *
14 * @return array 40 * @return array
15 */ 41 */
16 public function findAllTags($userId) 42 public function findAllTagsWithEntries($userId)
17 { 43 {
18 return $this->createQueryBuilder('t') 44 return $this->createQueryBuilder('t')
19 ->leftJoin('t.entries', 'e') 45 ->leftJoin('t.entries', 'e')
20 ->where('e.user = :userId')->setParameter('userId', $userId); 46 ->where('e.user = :userId')->setParameter('userId', $userId)
47 ->getQuery()
48 ->getResult();
21 } 49 }
22 50
23 /** 51 /**
diff --git a/src/Wallabag/CoreBundle/Twig/WallabagExtension.php b/src/Wallabag/CoreBundle/Twig/WallabagExtension.php
index 3780b13e..fb4c7412 100644
--- a/src/Wallabag/CoreBundle/Twig/WallabagExtension.php
+++ b/src/Wallabag/CoreBundle/Twig/WallabagExtension.php
@@ -2,7 +2,6 @@
2 2
3namespace Wallabag\CoreBundle\Twig; 3namespace Wallabag\CoreBundle\Twig;
4 4
5use Doctrine\ORM\Query;
6use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; 5use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
7use Wallabag\CoreBundle\Repository\EntryRepository; 6use Wallabag\CoreBundle\Repository\EntryRepository;
8use Wallabag\CoreBundle\Repository\TagRepository; 7use Wallabag\CoreBundle\Repository\TagRepository;
@@ -85,10 +84,11 @@ class WallabagExtension extends \Twig_Extension implements \Twig_Extension_Globa
85 ->groupBy('e.id') 84 ->groupBy('e.id')
86 ->getQuery(); 85 ->getQuery();
87 86
88 $data = $this->enableCache($query) 87 $query->useQueryCache(true);
89 ->getArrayResult(); 88 $query->useResultCache(true);
89 $query->setResultCacheLifetime($this->lifeTime);
90 90
91 return count($data); 91 return count($query->getArrayResult());
92 } 92 }
93 93
94 /** 94 /**
@@ -104,28 +104,7 @@ class WallabagExtension extends \Twig_Extension implements \Twig_Extension_Globa
104 return 0; 104 return 0;
105 } 105 }
106 106
107 $qb = $this->tagRepository->findAllTags($user->getId()); 107 return $this->tagRepository->countAllTags($user->getId());
108
109 $data = $this->enableCache($qb->getQuery())
110 ->getArrayResult();
111
112 return count($data);
113 }
114
115 /**
116 * Enable cache for a query.
117 *
118 * @param Query $query
119 *
120 * @return Query
121 */
122 private function enableCache(Query $query)
123 {
124 $query->useQueryCache(true);
125 $query->useResultCache(true);
126 $query->setResultCacheLifetime($this->lifeTime);
127
128 return $query;
129 } 108 }
130 109
131 public function getName() 110 public function getName()
diff --git a/src/Wallabag/ImportBundle/Import/PocketImport.php b/src/Wallabag/ImportBundle/Import/PocketImport.php
index e00eb44b..327e2500 100644
--- a/src/Wallabag/ImportBundle/Import/PocketImport.php
+++ b/src/Wallabag/ImportBundle/Import/PocketImport.php
@@ -227,7 +227,8 @@ class PocketImport extends AbstractImport
227 if (isset($importedEntry['tags']) && !empty($importedEntry['tags'])) { 227 if (isset($importedEntry['tags']) && !empty($importedEntry['tags'])) {
228 $this->contentProxy->assignTagsToEntry( 228 $this->contentProxy->assignTagsToEntry(
229 $entry, 229 $entry,
230 array_keys($importedEntry['tags']) 230 array_keys($importedEntry['tags']),
231 $this->em->getUnitOfWork()->getScheduledEntityInsertions()
231 ); 232 );
232 } 233 }
233 234
diff --git a/src/Wallabag/ImportBundle/Import/WallabagImport.php b/src/Wallabag/ImportBundle/Import/WallabagImport.php
index 043bb0a2..3754e4a9 100644
--- a/src/Wallabag/ImportBundle/Import/WallabagImport.php
+++ b/src/Wallabag/ImportBundle/Import/WallabagImport.php
@@ -111,7 +111,8 @@ abstract class WallabagImport extends AbstractImport
111 if (array_key_exists('tags', $data)) { 111 if (array_key_exists('tags', $data)) {
112 $this->contentProxy->assignTagsToEntry( 112 $this->contentProxy->assignTagsToEntry(
113 $entry, 113 $entry,
114 $data['tags'] 114 $data['tags'],
115 $this->em->getUnitOfWork()->getScheduledEntityInsertions()
115 ); 116 );
116 } 117 }
117 118
diff --git a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
index 7abb0737..5d772602 100644
--- a/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
+++ b/tests/Wallabag/CoreBundle/Helper/ContentProxyTest.php
@@ -296,6 +296,29 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
296 $this->assertEquals('tag2', $entry->getTags()[1]->getLabel()); 296 $this->assertEquals('tag2', $entry->getTags()[1]->getLabel());
297 } 297 }
298 298
299 public function testAssignTagsNotFlushed()
300 {
301 $graby = $this->getMockBuilder('Graby\Graby')
302 ->disableOriginalConstructor()
303 ->getMock();
304
305 $tagRepo = $this->getTagRepositoryMock();
306 $tagRepo->expects($this->never())
307 ->method('__call');
308
309 $proxy = new ContentProxy($graby, $this->getTaggerMock(), $tagRepo, $this->getLogger());
310
311 $tagEntity = new Tag();
312 $tagEntity->setLabel('tag1');
313
314 $entry = new Entry(new User());
315
316 $proxy->assignTagsToEntry($entry, 'tag1', [$tagEntity]);
317
318 $this->assertCount(1, $entry->getTags());
319 $this->assertEquals('tag1', $entry->getTags()[0]->getLabel());
320 }
321
299 private function getTaggerMock() 322 private function getTaggerMock()
300 { 323 {
301 return $this->getMockBuilder('Wallabag\CoreBundle\Helper\RuleBasedTagger') 324 return $this->getMockBuilder('Wallabag\CoreBundle\Helper\RuleBasedTagger')
diff --git a/tests/Wallabag/ImportBundle/Import/PocketImportTest.php b/tests/Wallabag/ImportBundle/Import/PocketImportTest.php
index 952521a2..9ec7935c 100644
--- a/tests/Wallabag/ImportBundle/Import/PocketImportTest.php
+++ b/tests/Wallabag/ImportBundle/Import/PocketImportTest.php
@@ -41,6 +41,20 @@ class PocketImportTest extends \PHPUnit_Framework_TestCase
41 ->disableOriginalConstructor() 41 ->disableOriginalConstructor()
42 ->getMock(); 42 ->getMock();
43 43
44 $this->uow = $this->getMockBuilder('Doctrine\ORM\UnitOfWork')
45 ->disableOriginalConstructor()
46 ->getMock();
47
48 $this->em
49 ->expects($this->any())
50 ->method('getUnitOfWork')
51 ->willReturn($this->uow);
52
53 $this->uow
54 ->expects($this->any())
55 ->method('getScheduledEntityInsertions')
56 ->willReturn([]);
57
44 $pocket = new PocketImport( 58 $pocket = new PocketImport(
45 $this->em, 59 $this->em,
46 $this->contentProxy 60 $this->contentProxy
diff --git a/tests/Wallabag/ImportBundle/Import/WallabagV1ImportTest.php b/tests/Wallabag/ImportBundle/Import/WallabagV1ImportTest.php
index 5ab4ad00..82dc4c7e 100644
--- a/tests/Wallabag/ImportBundle/Import/WallabagV1ImportTest.php
+++ b/tests/Wallabag/ImportBundle/Import/WallabagV1ImportTest.php
@@ -26,6 +26,20 @@ class WallabagV1ImportTest extends \PHPUnit_Framework_TestCase
26 ->disableOriginalConstructor() 26 ->disableOriginalConstructor()
27 ->getMock(); 27 ->getMock();
28 28
29 $this->uow = $this->getMockBuilder('Doctrine\ORM\UnitOfWork')
30 ->disableOriginalConstructor()
31 ->getMock();
32
33 $this->em
34 ->expects($this->any())
35 ->method('getUnitOfWork')
36 ->willReturn($this->uow);
37
38 $this->uow
39 ->expects($this->any())
40 ->method('getScheduledEntityInsertions')
41 ->willReturn([]);
42
29 $this->contentProxy = $this->getMockBuilder('Wallabag\CoreBundle\Helper\ContentProxy') 43 $this->contentProxy = $this->getMockBuilder('Wallabag\CoreBundle\Helper\ContentProxy')
30 ->disableOriginalConstructor() 44 ->disableOriginalConstructor()
31 ->getMock(); 45 ->getMock();
diff --git a/tests/Wallabag/ImportBundle/Import/WallabagV2ImportTest.php b/tests/Wallabag/ImportBundle/Import/WallabagV2ImportTest.php
index 12bd6bdd..bea89efb 100644
--- a/tests/Wallabag/ImportBundle/Import/WallabagV2ImportTest.php
+++ b/tests/Wallabag/ImportBundle/Import/WallabagV2ImportTest.php
@@ -26,6 +26,20 @@ class WallabagV2ImportTest extends \PHPUnit_Framework_TestCase
26 ->disableOriginalConstructor() 26 ->disableOriginalConstructor()
27 ->getMock(); 27 ->getMock();
28 28
29 $this->uow = $this->getMockBuilder('Doctrine\ORM\UnitOfWork')
30 ->disableOriginalConstructor()
31 ->getMock();
32
33 $this->em
34 ->expects($this->any())
35 ->method('getUnitOfWork')
36 ->willReturn($this->uow);
37
38 $this->uow
39 ->expects($this->any())
40 ->method('getScheduledEntityInsertions')
41 ->willReturn([]);
42
29 $this->contentProxy = $this->getMockBuilder('Wallabag\CoreBundle\Helper\ContentProxy') 43 $this->contentProxy = $this->getMockBuilder('Wallabag\CoreBundle\Helper\ContentProxy')
30 ->disableOriginalConstructor() 44 ->disableOriginalConstructor()
31 ->getMock(); 45 ->getMock();