From: ArthurHoaro Date: Sat, 18 Jan 2020 16:59:37 +0000 (+0100) Subject: Fix an issue with private tags and fix nomarkdown tag (#1399) X-Git-Tag: v0.12.0-beta~21 X-Git-Url: https://git.immae.eu/?a=commitdiff_plain;h=1001cc108fec759b076c9f6e12b71dea5d49fe9b;hp=12523aea3458504be91854ce2f37d4f5991ccea2;p=github%2Fshaarli%2FShaarli.git Fix an issue with private tags and fix nomarkdown tag (#1399) Fix an issue with private tags and fix nomarkdown tag --- diff --git a/application/bookmark/BookmarkFileService.php b/application/bookmark/BookmarkFileService.php index a56cc92b..9c59e139 100644 --- a/application/bookmark/BookmarkFileService.php +++ b/application/bookmark/BookmarkFileService.php @@ -8,6 +8,7 @@ use Exception; use Shaarli\Bookmark\Exception\BookmarkNotFoundException; use Shaarli\Bookmark\Exception\EmptyDataStoreException; use Shaarli\Config\ConfigManager; +use Shaarli\Formatter\BookmarkMarkdownFormatter; use Shaarli\History; use Shaarli\Legacy\LegacyLinkDB; use Shaarli\Legacy\LegacyUpdater; @@ -130,7 +131,7 @@ class BookmarkFileService implements BookmarkServiceInterface } if ($visibility === null) { - $visibility = $this->isLoggedIn ? 'all' : 'public'; + $visibility = $this->isLoggedIn ? BookmarkFilter::$ALL : BookmarkFilter::$PUBLIC; } $bookmark = $this->bookmarks[$id]; @@ -287,9 +288,13 @@ class BookmarkFileService implements BookmarkServiceInterface $caseMapping = []; foreach ($bookmarks as $bookmark) { foreach ($bookmark->getTags() as $tag) { - if (empty($tag) || (! $this->isLoggedIn && startsWith($tag, '.'))) { + if (empty($tag) + || (! $this->isLoggedIn && startsWith($tag, '.')) + || $tag === BookmarkMarkdownFormatter::NO_MD_TAG + ) { continue; } + // The first case found will be displayed. if (!isset($caseMapping[strtolower($tag)])) { $caseMapping[strtolower($tag)] = $tag; diff --git a/application/formatter/BookmarkDefaultFormatter.php b/application/formatter/BookmarkDefaultFormatter.php index 7550c556..c6c59064 100644 --- a/application/formatter/BookmarkDefaultFormatter.php +++ b/application/formatter/BookmarkDefaultFormatter.php @@ -34,7 +34,7 @@ class BookmarkDefaultFormatter extends BookmarkFormatter */ protected function formatTagList($bookmark) { - return escape($bookmark->getTags()); + return escape(parent::formatTagList($bookmark)); } /** diff --git a/application/formatter/BookmarkFormatter.php b/application/formatter/BookmarkFormatter.php index c82c3452..a80d83fc 100644 --- a/application/formatter/BookmarkFormatter.php +++ b/application/formatter/BookmarkFormatter.php @@ -20,6 +20,9 @@ abstract class BookmarkFormatter */ protected $conf; + /** @var bool */ + protected $isLoggedIn; + /** * @var array Additional parameters than can be used for specific formatting * e.g. index_url for Feed formatting @@ -30,9 +33,10 @@ abstract class BookmarkFormatter * LinkDefaultFormatter constructor. * @param ConfigManager $conf */ - public function __construct(ConfigManager $conf) + public function __construct(ConfigManager $conf, bool $isLoggedIn) { $this->conf = $conf; + $this->isLoggedIn = $isLoggedIn; } /** @@ -172,7 +176,7 @@ abstract class BookmarkFormatter */ protected function formatTagList($bookmark) { - return $bookmark->getTags(); + return $this->filterTagList($bookmark->getTags()); } /** @@ -184,7 +188,7 @@ abstract class BookmarkFormatter */ protected function formatTagString($bookmark) { - return implode(' ', $bookmark->getTags()); + return implode(' ', $this->formatTagList($bookmark)); } /** @@ -253,4 +257,29 @@ abstract class BookmarkFormatter } return 0; } + + /** + * Format tag list, e.g. remove private tags if the user is not logged in. + * + * @param array $tags + * + * @return array + */ + protected function filterTagList(array $tags): array + { + if ($this->isLoggedIn === true) { + return $tags; + } + + $out = []; + foreach ($tags as $tag) { + if (strpos($tag, '.') === 0) { + continue; + } + + $out[] = $tag; + } + + return $out; + } } diff --git a/application/formatter/BookmarkMarkdownFormatter.php b/application/formatter/BookmarkMarkdownFormatter.php index 7797bfbf..077e5312 100644 --- a/application/formatter/BookmarkMarkdownFormatter.php +++ b/application/formatter/BookmarkMarkdownFormatter.php @@ -36,10 +36,12 @@ class BookmarkMarkdownFormatter extends BookmarkDefaultFormatter * LinkMarkdownFormatter constructor. * * @param ConfigManager $conf instance + * @param bool $isLoggedIn */ - public function __construct(ConfigManager $conf) + public function __construct(ConfigManager $conf, bool $isLoggedIn) { - parent::__construct($conf); + parent::__construct($conf, $isLoggedIn); + $this->parsedown = new \Parsedown(); $this->escape = $conf->get('security.markdown_escape', true); $this->allowedProtocols = $conf->get('security.allowed_protocols', []); @@ -79,7 +81,7 @@ class BookmarkMarkdownFormatter extends BookmarkDefaultFormatter protected function formatTagList($bookmark) { $out = parent::formatTagList($bookmark); - if (($pos = array_search(self::NO_MD_TAG, $out)) !== false) { + if ($this->isLoggedIn === false && ($pos = array_search(self::NO_MD_TAG, $out)) !== false) { unset($out[$pos]); return array_values($out); } diff --git a/application/formatter/FormatterFactory.php b/application/formatter/FormatterFactory.php index 0d2c0466..5f282f68 100644 --- a/application/formatter/FormatterFactory.php +++ b/application/formatter/FormatterFactory.php @@ -16,14 +16,19 @@ class FormatterFactory /** @var ConfigManager instance */ protected $conf; + /** @var bool */ + protected $isLoggedIn; + /** * FormatterFactory constructor. * * @param ConfigManager $conf + * @param bool $isLoggedIn */ - public function __construct(ConfigManager $conf) + public function __construct(ConfigManager $conf, bool $isLoggedIn) { $this->conf = $conf; + $this->isLoggedIn = $isLoggedIn; } /** @@ -33,7 +38,7 @@ class FormatterFactory * * @return BookmarkFormatter instance. */ - public function getFormatter($type = null) + public function getFormatter(string $type = null) { $type = $type ? $type : $this->conf->get('formatter', 'default'); $className = '\\Shaarli\\Formatter\\Bookmark'. ucfirst($type) .'Formatter'; @@ -41,6 +46,6 @@ class FormatterFactory $className = '\\Shaarli\\Formatter\\BookmarkDefaultFormatter'; } - return new $className($this->conf); + return new $className($this->conf, $this->isLoggedIn); } } diff --git a/index.php b/index.php index 013b01af..a39fc762 100644 --- a/index.php +++ b/index.php @@ -70,6 +70,7 @@ use Shaarli\Bookmark\BookmarkFileService; use \Shaarli\Config\ConfigManager; use \Shaarli\Feed\CachedPage; use \Shaarli\Feed\FeedBuilder; +use Shaarli\Formatter\BookmarkMarkdownFormatter; use Shaarli\Formatter\FormatterFactory; use \Shaarli\History; use \Shaarli\Languages; @@ -351,7 +352,7 @@ function showDailyRSS($bookmarkService, $conf, $loginManager) echo 'en-en'; echo ''. $pageaddr .''. PHP_EOL; - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); $formatter = $factory->getFormatter(); $formatter->addContextData('index_url', index_url($_SERVER)); // For each day. @@ -441,7 +442,7 @@ function showDaily($pageBuilder, $bookmarkService, $conf, $pluginManager, $login $linksToDisplay = []; } - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); $formatter = $factory->getFormatter(); // We pre-format some fields for proper output. foreach ($linksToDisplay as $key => $bookmark) { @@ -630,7 +631,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM // Get only bookmarks which have a thumbnail. // Note: we do not retrieve thumbnails here, the request is too heavy. - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); $formatter = $factory->getFormatter(); foreach ($links as $key => $link) { if ($link->getThumbnail() !== false) { @@ -753,7 +754,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM exit; } - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); // Generate data. $feedGenerator = new FeedBuilder( $bookmarkService, @@ -1183,7 +1184,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM $bookmarkService->addOrSet($bookmark, false); // To preserve backward compatibility with 3rd parties, plugins still use arrays - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); $formatter = $factory->getFormatter('raw'); $data = $formatter->format($bookmark); $pluginManager->executeHooks('save_link', $data); @@ -1230,7 +1231,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM if (!count($ids)) { die('no id provided'); } - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); $formatter = $factory->getFormatter('raw'); foreach ($ids as $id) { $id = (int) escape($id); @@ -1286,7 +1287,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM } else { $private = $_GET['newVisibility'] === 'private'; } - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); $formatter = $factory->getFormatter('raw'); foreach ($ids as $id) { $id = (int) escape($id); @@ -1324,14 +1325,18 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM exit; } - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); $formatter = $factory->getFormatter('raw'); $formattedLink = $formatter->format($link); + $tags = $bookmarkService->bookmarksCountPerTag(); + if ($conf->get('formatter') === 'markdown') { + $tags[BookmarkMarkdownFormatter::NO_MD_TAG] = 1; + } $data = array( 'link' => $formattedLink, 'link_is_new' => false, 'http_referer' => (isset($_SERVER['HTTP_REFERER']) ? escape($_SERVER['HTTP_REFERER']) : ''), - 'tags' => $bookmarkService->bookmarksCountPerTag(), + 'tags' => $tags, ); $pluginManager->executeHooks('render_editlink', $data); @@ -1391,17 +1396,21 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM 'private' => $private, ]; } else { - $factory = new FormatterFactory($conf); - $formatter = $factory->getFormatter('raw'); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); + $formatter = $factory->getFormatter('raw'); $link = $formatter->format($bookmark); } + $tags = $bookmarkService->bookmarksCountPerTag(); + if ($conf->get('formatter') === 'markdown') { + $tags[BookmarkMarkdownFormatter::NO_MD_TAG] = 1; + } $data = [ 'link' => $link, 'link_is_new' => $link_is_new, 'http_referer' => (isset($_SERVER['HTTP_REFERER']) ? escape($_SERVER['HTTP_REFERER']) : ''), 'source' => (isset($_GET['source']) ? $_GET['source'] : ''), - 'tags' => $bookmarkService->bookmarksCountPerTag(), + 'tags' => $tags, 'default_private_links' => $conf->get('privacy.default_private_links', false), ]; $pluginManager->executeHooks('render_editlink', $data); @@ -1451,7 +1460,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM } try { - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); $formatter = $factory->getFormatter('raw'); $PAGE->assign( 'links', @@ -1633,7 +1642,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM $bookmark->setThumbnail($thumbnailer->get($bookmark->getUrl())); $bookmarkService->set($bookmark); - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); echo json_encode($factory->getFormatter('raw')->format($bookmark)); exit; } @@ -1655,7 +1664,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM */ function buildLinkList($PAGE, $linkDb, $conf, $pluginManager, $loginManager) { - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, $loginManager->isLoggedIn()); $formatter = $factory->getFormatter(); // Used in templates diff --git a/tests/bookmark/BookmarkFileServiceTest.php b/tests/bookmark/BookmarkFileServiceTest.php index 1b438a7f..4900d41d 100644 --- a/tests/bookmark/BookmarkFileServiceTest.php +++ b/tests/bookmark/BookmarkFileServiceTest.php @@ -12,6 +12,7 @@ use ReflectionClass; use Shaarli; use Shaarli\Bookmark\Exception\BookmarkNotFoundException; use Shaarli\Config\ConfigManager; +use Shaarli\Formatter\BookmarkMarkdownFormatter; use Shaarli\History; /** @@ -1025,6 +1026,46 @@ class BookmarkFileServiceTest extends TestCase $this->assertEquals($expected, $tags, var_export($tags, true)); } + /** + * Test linksCountPerTag public tags with filter. + * Equal occurrences should be sorted alphabetically. + */ + public function testCountTagsNoMarkdown() + { + $expected = [ + 'cartoon' => 3, + 'dev' => 2, + 'tag1' => 1, + 'tag2' => 1, + 'tag3' => 1, + 'tag4' => 1, + 'web' => 4, + 'gnu' => 2, + 'hashtag' => 2, + 'sTuff' => 2, + '-exclude' => 1, + '.hidden' => 1, + 'Mercurial' => 1, + 'css' => 1, + 'free' => 1, + 'html' => 1, + 'media' => 1, + 'newTagToCount' => 1, + 'samba' => 1, + 'software' => 1, + 'stallman' => 1, + 'ut' => 1, + 'w3c' => 1, + ]; + $bookmark = new Bookmark(); + $bookmark->setTags(['newTagToCount', BookmarkMarkdownFormatter::NO_MD_TAG]); + $this->privateLinkDB->add($bookmark); + + $tags = $this->privateLinkDB->bookmarksCountPerTag(); + + $this->assertEquals($expected, $tags, var_export($tags, true)); + } + /** * Allows to test LinkDB's private methods * diff --git a/tests/feed/FeedBuilderTest.php b/tests/feed/FeedBuilderTest.php index a43ff672..54671891 100644 --- a/tests/feed/FeedBuilderTest.php +++ b/tests/feed/FeedBuilderTest.php @@ -51,7 +51,7 @@ class FeedBuilderTest extends \PHPUnit\Framework\TestCase $refLinkDB = new \ReferenceLinkDB(); $refLinkDB->write(self::$testDatastore); $history = new History('sandbox/history.php'); - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, true); self::$formatter = $factory->getFormatter(); self::$bookmarkService = new BookmarkFileService($conf, $history, true); diff --git a/tests/formatter/BookmarkDefaultFormatterTest.php b/tests/formatter/BookmarkDefaultFormatterTest.php index fe42a208..382a560e 100644 --- a/tests/formatter/BookmarkDefaultFormatterTest.php +++ b/tests/formatter/BookmarkDefaultFormatterTest.php @@ -29,7 +29,7 @@ class BookmarkDefaultFormatterTest extends TestCase { copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php'); $this->conf = new ConfigManager(self::$testConf); - $this->formatter = new BookmarkDefaultFormatter($this->conf); + $this->formatter = new BookmarkDefaultFormatter($this->conf, true); } /** @@ -153,4 +153,25 @@ class BookmarkDefaultFormatterTest extends TestCase $link['description'] ); } + + /** + * Make sure that private tags are properly filtered out when the user is logged out. + */ + public function testFormatTagListRemovePrivate(): void + { + $this->formatter = new BookmarkDefaultFormatter($this->conf, false); + + $bookmark = new Bookmark(); + $bookmark->setId($id = 11); + $bookmark->setTags($tags = ['bookmark', '.private', 'othertag']); + + $link = $this->formatter->format($bookmark); + + unset($tags[1]); + $tags = array_values($tags); + + $this->assertSame(11, $link['id']); + $this->assertSame($tags, $link['taglist']); + $this->assertSame(implode(' ', $tags), $link['tags']); + } } diff --git a/tests/formatter/BookmarkMarkdownFormatterTest.php b/tests/formatter/BookmarkMarkdownFormatterTest.php index 0ca7f802..f1f12c04 100644 --- a/tests/formatter/BookmarkMarkdownFormatterTest.php +++ b/tests/formatter/BookmarkMarkdownFormatterTest.php @@ -29,7 +29,7 @@ class BookmarkMarkdownFormatterTest extends TestCase { copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php'); $this->conf = new ConfigManager(self::$testConf); - $this->formatter = new BookmarkMarkdownFormatter($this->conf); + $this->formatter = new BookmarkMarkdownFormatter($this->conf, true); } /** diff --git a/tests/formatter/BookmarkRawFormatterTest.php b/tests/formatter/BookmarkRawFormatterTest.php index ceb6fb73..4491b035 100644 --- a/tests/formatter/BookmarkRawFormatterTest.php +++ b/tests/formatter/BookmarkRawFormatterTest.php @@ -29,7 +29,7 @@ class BookmarkRawFormatterTest extends TestCase { copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php'); $this->conf = new ConfigManager(self::$testConf); - $this->formatter = new BookmarkRawFormatter($this->conf); + $this->formatter = new BookmarkRawFormatter($this->conf, true); } /** diff --git a/tests/formatter/FormatterFactoryTest.php b/tests/formatter/FormatterFactoryTest.php index 317c0b2d..5adf3ffd 100644 --- a/tests/formatter/FormatterFactoryTest.php +++ b/tests/formatter/FormatterFactoryTest.php @@ -28,7 +28,7 @@ class FormatterFactoryTest extends TestCase { copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php'); $this->conf = new ConfigManager(self::$testConf); - $this->factory = new FormatterFactory($this->conf); + $this->factory = new FormatterFactory($this->conf, true); } /** diff --git a/tests/netscape/BookmarkExportTest.php b/tests/netscape/BookmarkExportTest.php index 011d19ac..6c948bba 100644 --- a/tests/netscape/BookmarkExportTest.php +++ b/tests/netscape/BookmarkExportTest.php @@ -46,7 +46,7 @@ class BookmarkExportTest extends \PHPUnit\Framework\TestCase self::$refDb->write(self::$testDatastore); $history = new History('sandbox/history.php'); self::$bookmarkService = new BookmarkFileService($conf, $history, true); - $factory = new FormatterFactory($conf); + $factory = new FormatterFactory($conf, true); self::$formatter = $factory->getFormatter('raw'); }