Merge pull request #1699 from wallabag/v2-assign-tag

Move assignTagsToEntry in ContentProxy helper
This commit is contained in:
Nicolas Lœuillet 2016-02-19 21:21:47 +01:00
commit 0cde91cc6f
7 changed files with 158 additions and 92 deletions

View file

@ -15,31 +15,6 @@ use Wallabag\CoreBundle\Entity\Tag;
class WallabagRestController extends FOSRestController
{
/**
* @param Entry $entry
* @param string $tags
*/
private function assignTagsToEntry(Entry $entry, $tags)
{
foreach (explode(',', $tags) as $label) {
$label = trim($label);
$tagEntity = $this
->getDoctrine()
->getRepository('WallabagCoreBundle:Tag')
->findOneByLabel($label);
if (is_null($tagEntity)) {
$tagEntity = new Tag();
$tagEntity->setLabel($label);
}
// only add the tag on the entry if the relation doesn't exist
if (!$entry->getTags()->contains($tagEntity)) {
$entry->addTag($tagEntity);
}
}
}
private function validateAuthentication()
{
if (false === $this->get('security.authorization_checker')->isGranted('IS_AUTHENTICATED_FULLY')) {
@ -140,7 +115,7 @@ class WallabagRestController extends FOSRestController
$tags = $request->request->get('tags', '');
if (!empty($tags)) {
$this->assignTagsToEntry($entry, $tags);
$this->get('wallabag_core.content_proxy')->assignTagsToEntry($entry, $tags);
}
$em = $this->getDoctrine()->getManager();
@ -192,7 +167,7 @@ class WallabagRestController extends FOSRestController
$tags = $request->request->get('tags', '');
if (!empty($tags)) {
$this->assignTagsToEntry($entry, $tags);
$this->get('wallabag_core.content_proxy')->assignTagsToEntry($entry, $tags);
}
$em = $this->getDoctrine()->getManager();
@ -270,7 +245,7 @@ class WallabagRestController extends FOSRestController
$tags = $request->request->get('tags', '');
if (!empty($tags)) {
$this->assignTagsToEntry($entry, $tags);
$this->get('wallabag_core.content_proxy')->assignTagsToEntry($entry, $tags);
}
$em = $this->getDoctrine()->getManager();

View file

@ -5,7 +5,9 @@ namespace Wallabag\CoreBundle\Helper;
use Graby\Graby;
use Psr\Log\LoggerInterface as Logger;
use Wallabag\CoreBundle\Entity\Entry;
use Wallabag\CoreBundle\Entity\Tag;
use Wallabag\CoreBundle\Tools\Utils;
use Wallabag\CoreBundle\Repository\TagRepository;
/**
* This kind of proxy class take care of getting the content from an url
@ -16,12 +18,14 @@ class ContentProxy
protected $graby;
protected $tagger;
protected $logger;
protected $tagRepository;
public function __construct(Graby $graby, RuleBasedTagger $tagger, Logger $logger)
public function __construct(Graby $graby, RuleBasedTagger $tagger, TagRepository $tagRepository, Logger $logger)
{
$this->graby = $graby;
$this->tagger = $tagger;
$this->logger = $logger;
$this->tagRepository = $tagRepository;
}
/**
@ -75,4 +79,38 @@ class ContentProxy
return $entry;
}
/**
* Assign some tags to an entry.
*
* @param Entry $entry
* @param array|string $tags An array of tag or a string coma separated of tag
*/
public function assignTagsToEntry(Entry $entry, $tags)
{
if (!is_array($tags)) {
$tags = explode(',', $tags);
}
foreach ($tags as $label) {
$label = trim($label);
// avoid empty tag
if (0 === strlen($label)) {
continue;
}
$tagEntity = $this->tagRepository->findOneByLabel($label);
if (is_null($tagEntity)) {
$tagEntity = new Tag();
$tagEntity->setLabel($label);
}
// only add the tag on the entry if the relation doesn't exist
if (false === $entry->getTags()->contains($tagEntity)) {
$entry->addTag($tagEntity);
}
}
}
}

View file

@ -50,6 +50,7 @@ services:
arguments:
- "@wallabag_core.graby"
- "@wallabag_core.rule_based_tagger"
- "@wallabag_core.tag_repository"
- "@logger"
wallabag_core.rule_based_tagger:

View file

@ -3,8 +3,9 @@
namespace Wallabag\CoreBundle\Tests\Helper;
use Psr\Log\NullLogger;
use Wallabag\CoreBundle\Entity\Entry;
use Wallabag\CoreBundle\Helper\ContentProxy;
use Wallabag\CoreBundle\Entity\Entry;
use Wallabag\CoreBundle\Entity\Tag;
use Wallabag\UserBundle\Entity\User;
class ContentProxyTest extends \PHPUnit_Framework_TestCase
@ -30,7 +31,7 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
'language' => '',
));
$proxy = new ContentProxy($graby, $tagger, $this->getLogger());
$proxy = new ContentProxy($graby, $tagger, $this->getTagRepositoryMock(), $this->getLogger());
$entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0');
$this->assertEquals('http://0.0.0.0', $entry->getUrl());
@ -68,7 +69,7 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
),
));
$proxy = new ContentProxy($graby, $tagger, $this->getLogger());
$proxy = new ContentProxy($graby, $tagger, $this->getTagRepositoryMock(), $this->getLogger());
$entry = $proxy->updateEntry(new Entry(new User()), 'http://domain.io');
$this->assertEquals('http://domain.io', $entry->getUrl());
@ -107,7 +108,7 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
),
));
$proxy = new ContentProxy($graby, $tagger, $this->getLogger());
$proxy = new ContentProxy($graby, $tagger, $this->getTagRepositoryMock(), $this->getLogger());
$entry = $proxy->updateEntry(new Entry(new User()), 'http://0.0.0.0');
$this->assertEquals('http://1.1.1.1', $entry->getUrl());
@ -120,6 +121,96 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
$this->assertEquals('1.1.1.1', $entry->getDomainName());
}
public function testAssignTagsWithArrayAndExtraSpaces()
{
$graby = $this->getMockBuilder('Graby\Graby')
->disableOriginalConstructor()
->getMock();
$tagRepo = $this->getTagRepositoryMock();
$proxy = new ContentProxy($graby, $this->getTaggerMock(), $tagRepo, $this->getLogger());
$entry = new Entry(new User());
$proxy->assignTagsToEntry($entry, array(' tag1', 'tag2 '));
$this->assertCount(2, $entry->getTags());
$this->assertEquals('tag1', $entry->getTags()[0]->getLabel());
$this->assertEquals('tag2', $entry->getTags()[1]->getLabel());
}
public function testAssignTagsWithString()
{
$graby = $this->getMockBuilder('Graby\Graby')
->disableOriginalConstructor()
->getMock();
$tagRepo = $this->getTagRepositoryMock();
$proxy = new ContentProxy($graby, $this->getTaggerMock(), $tagRepo, $this->getLogger());
$entry = new Entry(new User());
$proxy->assignTagsToEntry($entry, 'tag1, tag2');
$this->assertCount(2, $entry->getTags());
$this->assertEquals('tag1', $entry->getTags()[0]->getLabel());
$this->assertEquals('tag2', $entry->getTags()[1]->getLabel());
}
public function testAssignTagsWithEmptyArray()
{
$graby = $this->getMockBuilder('Graby\Graby')
->disableOriginalConstructor()
->getMock();
$tagRepo = $this->getTagRepositoryMock();
$proxy = new ContentProxy($graby, $this->getTaggerMock(), $tagRepo, $this->getLogger());
$entry = new Entry(new User());
$proxy->assignTagsToEntry($entry, array());
$this->assertCount(0, $entry->getTags());
}
public function testAssignTagsWithEmptyString()
{
$graby = $this->getMockBuilder('Graby\Graby')
->disableOriginalConstructor()
->getMock();
$tagRepo = $this->getTagRepositoryMock();
$proxy = new ContentProxy($graby, $this->getTaggerMock(), $tagRepo, $this->getLogger());
$entry = new Entry(new User());
$proxy->assignTagsToEntry($entry, '');
$this->assertCount(0, $entry->getTags());
}
public function testAssignTagsAlreadyAssigned()
{
$graby = $this->getMockBuilder('Graby\Graby')
->disableOriginalConstructor()
->getMock();
$tagRepo = $this->getTagRepositoryMock();
$proxy = new ContentProxy($graby, $this->getTaggerMock(), $tagRepo, $this->getLogger());
$tagEntity = new Tag();
$tagEntity->setLabel('tag1');
$entry = new Entry(new User());
$entry->addTag($tagEntity);
$proxy->assignTagsToEntry($entry, 'tag1, tag2');
$this->assertCount(2, $entry->getTags());
$this->assertEquals('tag1', $entry->getTags()[0]->getLabel());
$this->assertEquals('tag2', $entry->getTags()[1]->getLabel());
}
private function getTaggerMock()
{
return $this->getMockBuilder('Wallabag\CoreBundle\Helper\RuleBasedTagger')
@ -128,6 +219,13 @@ class ContentProxyTest extends \PHPUnit_Framework_TestCase
->getMock();
}
private function getTagRepositoryMock()
{
return $this->getMockBuilder('Wallabag\CoreBundle\Repository\TagRepository')
->disableOriginalConstructor()
->getMock();
}
private function getLogger()
{
return new NullLogger();

View file

@ -9,7 +9,6 @@ use GuzzleHttp\Client;
use GuzzleHttp\Exception\RequestException;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Wallabag\CoreBundle\Entity\Entry;
use Wallabag\CoreBundle\Entity\Tag;
use Wallabag\CoreBundle\Helper\ContentProxy;
use Craue\ConfigBundle\Util\Config;
@ -177,26 +176,6 @@ class PocketImport implements ImportInterface
$this->client = $client;
}
private function assignTagsToEntry(Entry $entry, $tags)
{
foreach ($tags as $tag) {
$label = trim($tag['tag']);
$tagEntity = $this->em
->getRepository('WallabagCoreBundle:Tag')
->findOneByLabel($label);
if (is_object($tagEntity)) {
$entry->addTag($tagEntity);
} else {
$newTag = new Tag();
$newTag->setLabel($label);
$entry->addTag($newTag);
}
$this->em->flush();
}
}
/**
* @see https://getpocket.com/developer/docs/v3/retrieve
*
@ -246,7 +225,10 @@ class PocketImport implements ImportInterface
}
if (isset($pocketEntry['tags']) && !empty($pocketEntry['tags'])) {
$this->assignTagsToEntry($entry, $pocketEntry['tags']);
$this->contentProxy->assignTagsToEntry(
$entry,
array_keys($pocketEntry['tags'])
);
}
$this->em->persist($entry);

View file

@ -6,7 +6,6 @@ use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Doctrine\ORM\EntityManager;
use Wallabag\CoreBundle\Entity\Entry;
use Wallabag\CoreBundle\Entity\Tag;
use Wallabag\UserBundle\Entity\User;
use Wallabag\CoreBundle\Tools\Utils;
use Wallabag\CoreBundle\Helper\ContentProxy;
@ -144,6 +143,7 @@ class WallabagV1Import implements ImportInterface
// @see ContentProxy->updateEntry
$entry = new Entry($this->user);
$entry->setUrl($importedEntry['url']);
if (in_array($importedEntry['title'], $untitled)) {
$entry = $this->contentProxy->updateEntry($entry, $importedEntry['url']);
} else {
@ -152,10 +152,14 @@ class WallabagV1Import implements ImportInterface
$entry->setReadingTime(Utils::getReadingTime($importedEntry['content']));
$entry->setDomainName(parse_url($importedEntry['url'], PHP_URL_HOST));
}
if (array_key_exists('tags', $importedEntry) && $importedEntry['tags'] != '') {
$tags = explode(',', $importedEntry['tags']);
$this->assignTagsToEntry($entry, $tags);
$this->contentProxy->assignTagsToEntry(
$entry,
$importedEntry['tags']
);
}
$entry->setArchived($importedEntry['is_read']);
$entry->setStarred($importedEntry['is_fav']);
@ -171,22 +175,4 @@ class WallabagV1Import implements ImportInterface
$this->em->flush();
}
private function assignTagsToEntry(Entry $entry, $tags)
{
foreach ($tags as $tag) {
$label = trim($tag);
$tagEntity = $this->em
->getRepository('WallabagCoreBundle:Tag')
->findOneByLabel($label);
if (is_object($tagEntity)) {
$entry->addTag($tagEntity);
} else {
$newTag = new Tag();
$newTag->setLabel($label);
$entry->addTag($newTag);
}
$this->em->flush();
}
}
}

View file

@ -260,24 +260,10 @@ class PocketImportTest extends \PHPUnit_Framework_TestCase
->method('findByUrlAndUserId')
->will($this->onConsecutiveCalls(false, true));
$tag = $this->getMockBuilder('Wallabag\CoreBundle\Entity\Tag')
->disableOriginalConstructor()
->getMock();
$tagRepo = $this->getMockBuilder('Wallabag\CoreBundle\Repository\TagRepository')
->disableOriginalConstructor()
->getMock();
$tagRepo->expects($this->exactly(2))
// the method `findOneByLabel` doesn't exist, EntityRepository will then call `_call` method
// to magically call the `findOneBy` with ['label' => 'foo']
->method('__call')
->will($this->onConsecutiveCalls(false, $tag));
$this->em
->expects($this->any())
->expects($this->exactly(2))
->method('getRepository')
->will($this->onConsecutiveCalls($entryRepo, $tagRepo, $tagRepo, $entryRepo));
->willReturn($entryRepo);
$entry = $this->getMockBuilder('Wallabag\CoreBundle\Entity\Entry')
->disableOriginalConstructor()