Fix an issue with private tags and fix nomarkdown tag

The new bookmark service wasn't handling private tags properly.

nomarkdown tag is now shown only for logged in user in bookmarks, and hidden for everyone in tag clouds/lists.

Fixes #726
This commit is contained in:
ArthurHoaro 2020-01-18 11:33:23 +01:00
parent 3fb29fdda0
commit a39acb2518
13 changed files with 145 additions and 33 deletions

View file

@ -8,6 +8,7 @@
use Shaarli\Bookmark\Exception\BookmarkNotFoundException; use Shaarli\Bookmark\Exception\BookmarkNotFoundException;
use Shaarli\Bookmark\Exception\EmptyDataStoreException; use Shaarli\Bookmark\Exception\EmptyDataStoreException;
use Shaarli\Config\ConfigManager; use Shaarli\Config\ConfigManager;
use Shaarli\Formatter\BookmarkMarkdownFormatter;
use Shaarli\History; use Shaarli\History;
use Shaarli\Legacy\LegacyLinkDB; use Shaarli\Legacy\LegacyLinkDB;
use Shaarli\Legacy\LegacyUpdater; use Shaarli\Legacy\LegacyUpdater;
@ -130,7 +131,7 @@ public function get($id, $visibility = null)
} }
if ($visibility === null) { if ($visibility === null) {
$visibility = $this->isLoggedIn ? 'all' : 'public'; $visibility = $this->isLoggedIn ? BookmarkFilter::$ALL : BookmarkFilter::$PUBLIC;
} }
$bookmark = $this->bookmarks[$id]; $bookmark = $this->bookmarks[$id];
@ -287,9 +288,13 @@ public function bookmarksCountPerTag($filteringTags = [], $visibility = null)
$caseMapping = []; $caseMapping = [];
foreach ($bookmarks as $bookmark) { foreach ($bookmarks as $bookmark) {
foreach ($bookmark->getTags() as $tag) { 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; continue;
} }
// The first case found will be displayed. // The first case found will be displayed.
if (!isset($caseMapping[strtolower($tag)])) { if (!isset($caseMapping[strtolower($tag)])) {
$caseMapping[strtolower($tag)] = $tag; $caseMapping[strtolower($tag)] = $tag;

View file

@ -34,7 +34,7 @@ public function formatDescription($bookmark)
*/ */
protected function formatTagList($bookmark) protected function formatTagList($bookmark)
{ {
return escape($bookmark->getTags()); return escape(parent::formatTagList($bookmark));
} }
/** /**

View file

@ -20,6 +20,9 @@ abstract class BookmarkFormatter
*/ */
protected $conf; protected $conf;
/** @var bool */
protected $isLoggedIn;
/** /**
* @var array Additional parameters than can be used for specific formatting * @var array Additional parameters than can be used for specific formatting
* e.g. index_url for Feed formatting * e.g. index_url for Feed formatting
@ -30,9 +33,10 @@ abstract class BookmarkFormatter
* LinkDefaultFormatter constructor. * LinkDefaultFormatter constructor.
* @param ConfigManager $conf * @param ConfigManager $conf
*/ */
public function __construct(ConfigManager $conf) public function __construct(ConfigManager $conf, bool $isLoggedIn)
{ {
$this->conf = $conf; $this->conf = $conf;
$this->isLoggedIn = $isLoggedIn;
} }
/** /**
@ -172,7 +176,7 @@ protected function formatThumbnail($bookmark)
*/ */
protected function formatTagList($bookmark) protected function formatTagList($bookmark)
{ {
return $bookmark->getTags(); return $this->filterTagList($bookmark->getTags());
} }
/** /**
@ -184,7 +188,7 @@ protected function formatTagList($bookmark)
*/ */
protected function formatTagString($bookmark) protected function formatTagString($bookmark)
{ {
return implode(' ', $bookmark->getTags()); return implode(' ', $this->formatTagList($bookmark));
} }
/** /**
@ -253,4 +257,29 @@ protected function formatUpdatedTimestamp(Bookmark $bookmark)
} }
return 0; 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;
}
} }

View file

@ -36,10 +36,12 @@ class BookmarkMarkdownFormatter extends BookmarkDefaultFormatter
* LinkMarkdownFormatter constructor. * LinkMarkdownFormatter constructor.
* *
* @param ConfigManager $conf instance * @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->parsedown = new \Parsedown();
$this->escape = $conf->get('security.markdown_escape', true); $this->escape = $conf->get('security.markdown_escape', true);
$this->allowedProtocols = $conf->get('security.allowed_protocols', []); $this->allowedProtocols = $conf->get('security.allowed_protocols', []);
@ -79,7 +81,7 @@ public function formatDescription($bookmark)
protected function formatTagList($bookmark) protected function formatTagList($bookmark)
{ {
$out = parent::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]); unset($out[$pos]);
return array_values($out); return array_values($out);
} }

View file

@ -16,14 +16,19 @@ class FormatterFactory
/** @var ConfigManager instance */ /** @var ConfigManager instance */
protected $conf; protected $conf;
/** @var bool */
protected $isLoggedIn;
/** /**
* FormatterFactory constructor. * FormatterFactory constructor.
* *
* @param ConfigManager $conf * @param ConfigManager $conf
* @param bool $isLoggedIn
*/ */
public function __construct(ConfigManager $conf) public function __construct(ConfigManager $conf, bool $isLoggedIn)
{ {
$this->conf = $conf; $this->conf = $conf;
$this->isLoggedIn = $isLoggedIn;
} }
/** /**
@ -33,7 +38,7 @@ public function __construct(ConfigManager $conf)
* *
* @return BookmarkFormatter instance. * @return BookmarkFormatter instance.
*/ */
public function getFormatter($type = null) public function getFormatter(string $type = null)
{ {
$type = $type ? $type : $this->conf->get('formatter', 'default'); $type = $type ? $type : $this->conf->get('formatter', 'default');
$className = '\\Shaarli\\Formatter\\Bookmark'. ucfirst($type) .'Formatter'; $className = '\\Shaarli\\Formatter\\Bookmark'. ucfirst($type) .'Formatter';
@ -41,6 +46,6 @@ public function getFormatter($type = null)
$className = '\\Shaarli\\Formatter\\BookmarkDefaultFormatter'; $className = '\\Shaarli\\Formatter\\BookmarkDefaultFormatter';
} }
return new $className($this->conf); return new $className($this->conf, $this->isLoggedIn);
} }
} }

View file

@ -70,6 +70,7 @@
use \Shaarli\Config\ConfigManager; use \Shaarli\Config\ConfigManager;
use \Shaarli\Feed\CachedPage; use \Shaarli\Feed\CachedPage;
use \Shaarli\Feed\FeedBuilder; use \Shaarli\Feed\FeedBuilder;
use Shaarli\Formatter\BookmarkMarkdownFormatter;
use Shaarli\Formatter\FormatterFactory; use Shaarli\Formatter\FormatterFactory;
use \Shaarli\History; use \Shaarli\History;
use \Shaarli\Languages; use \Shaarli\Languages;
@ -351,7 +352,7 @@ function showDailyRSS($bookmarkService, $conf, $loginManager)
echo '<language>en-en</language>'; echo '<language>en-en</language>';
echo '<copyright>'. $pageaddr .'</copyright>'. PHP_EOL; echo '<copyright>'. $pageaddr .'</copyright>'. PHP_EOL;
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
$formatter = $factory->getFormatter(); $formatter = $factory->getFormatter();
$formatter->addContextData('index_url', index_url($_SERVER)); $formatter->addContextData('index_url', index_url($_SERVER));
// For each day. // For each day.
@ -441,7 +442,7 @@ function showDaily($pageBuilder, $bookmarkService, $conf, $pluginManager, $login
$linksToDisplay = []; $linksToDisplay = [];
} }
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
$formatter = $factory->getFormatter(); $formatter = $factory->getFormatter();
// We pre-format some fields for proper output. // We pre-format some fields for proper output.
foreach ($linksToDisplay as $key => $bookmark) { foreach ($linksToDisplay as $key => $bookmark) {
@ -630,7 +631,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
// Get only bookmarks which have a thumbnail. // Get only bookmarks which have a thumbnail.
// Note: we do not retrieve thumbnails here, the request is too heavy. // 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(); $formatter = $factory->getFormatter();
foreach ($links as $key => $link) { foreach ($links as $key => $link) {
if ($link->getThumbnail() !== false) { if ($link->getThumbnail() !== false) {
@ -753,7 +754,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
exit; exit;
} }
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
// Generate data. // Generate data.
$feedGenerator = new FeedBuilder( $feedGenerator = new FeedBuilder(
$bookmarkService, $bookmarkService,
@ -1183,7 +1184,7 @@ function renderPage($conf, $pluginManager, $bookmarkService, $history, $sessionM
$bookmarkService->addOrSet($bookmark, false); $bookmarkService->addOrSet($bookmark, false);
// To preserve backward compatibility with 3rd parties, plugins still use arrays // 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'); $formatter = $factory->getFormatter('raw');
$data = $formatter->format($bookmark); $data = $formatter->format($bookmark);
$pluginManager->executeHooks('save_link', $data); $pluginManager->executeHooks('save_link', $data);
@ -1230,7 +1231,7 @@ function ($item) {
if (!count($ids)) { if (!count($ids)) {
die('no id provided'); die('no id provided');
} }
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
$formatter = $factory->getFormatter('raw'); $formatter = $factory->getFormatter('raw');
foreach ($ids as $id) { foreach ($ids as $id) {
$id = (int) escape($id); $id = (int) escape($id);
@ -1286,7 +1287,7 @@ function ($item) {
} else { } else {
$private = $_GET['newVisibility'] === 'private'; $private = $_GET['newVisibility'] === 'private';
} }
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
$formatter = $factory->getFormatter('raw'); $formatter = $factory->getFormatter('raw');
foreach ($ids as $id) { foreach ($ids as $id) {
$id = (int) escape($id); $id = (int) escape($id);
@ -1324,14 +1325,18 @@ function ($item) {
exit; exit;
} }
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
$formatter = $factory->getFormatter('raw'); $formatter = $factory->getFormatter('raw');
$formattedLink = $formatter->format($link); $formattedLink = $formatter->format($link);
$tags = $bookmarkService->bookmarksCountPerTag();
if ($conf->get('formatter') === 'markdown') {
$tags[BookmarkMarkdownFormatter::NO_MD_TAG] = 1;
}
$data = array( $data = array(
'link' => $formattedLink, 'link' => $formattedLink,
'link_is_new' => false, 'link_is_new' => false,
'http_referer' => (isset($_SERVER['HTTP_REFERER']) ? escape($_SERVER['HTTP_REFERER']) : ''), 'http_referer' => (isset($_SERVER['HTTP_REFERER']) ? escape($_SERVER['HTTP_REFERER']) : ''),
'tags' => $bookmarkService->bookmarksCountPerTag(), 'tags' => $tags,
); );
$pluginManager->executeHooks('render_editlink', $data); $pluginManager->executeHooks('render_editlink', $data);
@ -1391,17 +1396,21 @@ function ($item) {
'private' => $private, 'private' => $private,
]; ];
} else { } else {
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
$formatter = $factory->getFormatter('raw'); $formatter = $factory->getFormatter('raw');
$link = $formatter->format($bookmark); $link = $formatter->format($bookmark);
} }
$tags = $bookmarkService->bookmarksCountPerTag();
if ($conf->get('formatter') === 'markdown') {
$tags[BookmarkMarkdownFormatter::NO_MD_TAG] = 1;
}
$data = [ $data = [
'link' => $link, 'link' => $link,
'link_is_new' => $link_is_new, 'link_is_new' => $link_is_new,
'http_referer' => (isset($_SERVER['HTTP_REFERER']) ? escape($_SERVER['HTTP_REFERER']) : ''), 'http_referer' => (isset($_SERVER['HTTP_REFERER']) ? escape($_SERVER['HTTP_REFERER']) : ''),
'source' => (isset($_GET['source']) ? $_GET['source'] : ''), 'source' => (isset($_GET['source']) ? $_GET['source'] : ''),
'tags' => $bookmarkService->bookmarksCountPerTag(), 'tags' => $tags,
'default_private_links' => $conf->get('privacy.default_private_links', false), 'default_private_links' => $conf->get('privacy.default_private_links', false),
]; ];
$pluginManager->executeHooks('render_editlink', $data); $pluginManager->executeHooks('render_editlink', $data);
@ -1451,7 +1460,7 @@ function ($item) {
} }
try { try {
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
$formatter = $factory->getFormatter('raw'); $formatter = $factory->getFormatter('raw');
$PAGE->assign( $PAGE->assign(
'links', 'links',
@ -1633,7 +1642,7 @@ function ($a, $b) {
$bookmark->setThumbnail($thumbnailer->get($bookmark->getUrl())); $bookmark->setThumbnail($thumbnailer->get($bookmark->getUrl()));
$bookmarkService->set($bookmark); $bookmarkService->set($bookmark);
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
echo json_encode($factory->getFormatter('raw')->format($bookmark)); echo json_encode($factory->getFormatter('raw')->format($bookmark));
exit; exit;
} }
@ -1655,7 +1664,7 @@ function ($a, $b) {
*/ */
function buildLinkList($PAGE, $linkDb, $conf, $pluginManager, $loginManager) function buildLinkList($PAGE, $linkDb, $conf, $pluginManager, $loginManager)
{ {
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, $loginManager->isLoggedIn());
$formatter = $factory->getFormatter(); $formatter = $factory->getFormatter();
// Used in templates // Used in templates

View file

@ -12,6 +12,7 @@
use Shaarli; use Shaarli;
use Shaarli\Bookmark\Exception\BookmarkNotFoundException; use Shaarli\Bookmark\Exception\BookmarkNotFoundException;
use Shaarli\Config\ConfigManager; use Shaarli\Config\ConfigManager;
use Shaarli\Formatter\BookmarkMarkdownFormatter;
use Shaarli\History; use Shaarli\History;
/** /**
@ -1025,6 +1026,46 @@ public function testCountLinkPerTagPrivateWithFilter()
$this->assertEquals($expected, $tags, var_export($tags, true)); $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 * Allows to test LinkDB's private methods
* *

View file

@ -51,7 +51,7 @@ public static function setUpBeforeClass()
$refLinkDB = new \ReferenceLinkDB(); $refLinkDB = new \ReferenceLinkDB();
$refLinkDB->write(self::$testDatastore); $refLinkDB->write(self::$testDatastore);
$history = new History('sandbox/history.php'); $history = new History('sandbox/history.php');
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, true);
self::$formatter = $factory->getFormatter(); self::$formatter = $factory->getFormatter();
self::$bookmarkService = new BookmarkFileService($conf, $history, true); self::$bookmarkService = new BookmarkFileService($conf, $history, true);

View file

@ -29,7 +29,7 @@ public function setUp()
{ {
copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php'); copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php');
$this->conf = new ConfigManager(self::$testConf); $this->conf = new ConfigManager(self::$testConf);
$this->formatter = new BookmarkDefaultFormatter($this->conf); $this->formatter = new BookmarkDefaultFormatter($this->conf, true);
} }
/** /**
@ -153,4 +153,25 @@ public function testFormatNoteWithIndexUrl()
$link['description'] $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']);
}
} }

View file

@ -29,7 +29,7 @@ public function setUp()
{ {
copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php'); copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php');
$this->conf = new ConfigManager(self::$testConf); $this->conf = new ConfigManager(self::$testConf);
$this->formatter = new BookmarkMarkdownFormatter($this->conf); $this->formatter = new BookmarkMarkdownFormatter($this->conf, true);
} }
/** /**

View file

@ -29,7 +29,7 @@ public function setUp()
{ {
copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php'); copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php');
$this->conf = new ConfigManager(self::$testConf); $this->conf = new ConfigManager(self::$testConf);
$this->formatter = new BookmarkRawFormatter($this->conf); $this->formatter = new BookmarkRawFormatter($this->conf, true);
} }
/** /**

View file

@ -28,7 +28,7 @@ public function setUp()
{ {
copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php'); copy('tests/utils/config/configJson.json.php', self::$testConf .'.json.php');
$this->conf = new ConfigManager(self::$testConf); $this->conf = new ConfigManager(self::$testConf);
$this->factory = new FormatterFactory($this->conf); $this->factory = new FormatterFactory($this->conf, true);
} }
/** /**

View file

@ -46,7 +46,7 @@ public static function setUpBeforeClass()
self::$refDb->write(self::$testDatastore); self::$refDb->write(self::$testDatastore);
$history = new History('sandbox/history.php'); $history = new History('sandbox/history.php');
self::$bookmarkService = new BookmarkFileService($conf, $history, true); self::$bookmarkService = new BookmarkFileService($conf, $history, true);
$factory = new FormatterFactory($conf); $factory = new FormatterFactory($conf, true);
self::$formatter = $factory->getFormatter('raw'); self::$formatter = $factory->getFormatter('raw');
} }