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

5 files changed, 53 insertions, 12 deletions

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);
     }
 }