[PLUGIN][Pinboard] Refactor and cleanup code

components/Link/Link.php

@@ -35,6 +35,22 @@ use InvalidArgumentException;
 
 class Link extends Component
 {
+    /**
+     * Note that this persists both a Link and a NoteToLink
+     *
+     * @return [Entity\Link, NoteToLink]
+     */
+    public static function maybeCreateLink(string $url, int $note_id): array
+    {
+        try {
+            $link                  = Entity\Link::getOrCreate($url);
+            DB::persist($note_link = NoteToLink::create(['link_id' => $link->getId(), 'note_id' => $note_id]));
+            return ['link' => $link, 'note_to_link' => $note_link];
+        } catch (InvalidArgumentException) {
+            return ['link' => null, 'note_to_link' => null];
+        }
+    }
+
     /**
      * Extract URLs from $content and create the appropriate Link and NoteToLink entities
      */
@@ -49,12 +65,7 @@ class Link extends Component
                 if (\in_array($match, $ignore)) {
                     continue;
                 }
-                try {
-                    $link_id = Entity\Link::getOrCreate($match)->getId();
-                    DB::persist(NoteToLink::create(['link_id' => $link_id, 'note_id' => $note->getId()]));
-                } catch (InvalidArgumentException) {
-                    continue;
-                }
+                self::maybeCreateLink($match, $note_id);
             }
         }
         return Event::next;

components/Tag/Tag.php

@@ -67,6 +67,54 @@ class Tag extends Component
         return Event::next;
     }
 
+    public static function maybeCreateTag(string $tag, int $note_id, ?int $lang_id): ?NoteTag
+    {
+        if (!self::validate($tag)) {
+            return null; // Ignore invalid tag candidates
+        }
+        $canonical_tag        = self::canonicalTag($tag, \is_null($lang_id) ? null : Language::getById($lang_id)->getLocale());
+        DB::persist($note_tag = NoteTag::create([
+            'tag'           => $tag,
+            'canonical'     => $canonical_tag,
+            'note_id'       => $note_id,
+            'use_canonical' => $extra_args['tag_use_canonical'] ?? false,
+            'language_id'   => $lang_id,
+        ]));
+        foreach (self::cacheKeys($canonical_tag) as $key) {
+            Cache::delete($key);
+        }
+        return $note_tag;
+    }
+
+    /**
+     * @return NoteTag[]
+     */
+    public static function getNoteTags(int $actor_id, ?string $note_type): array
+    {
+        $query = <<<'EOF'
+            select nt from \App\Entity\Note n
+            join \Component\Tag\Entity\NoteTag nt with n.id = nt.note_id
+            where n.actor_id = :id
+            EOF;
+        if (\is_null($note_type)) {
+            return Cache::getList(
+                Actor::cacheKeys($actor_id, 'any')['note-tags'],
+                fn () => DB::dql(
+                    $query,
+                    ['id' => $actor_id],
+                ),
+            );
+        } else {
+            return Cache::getList(
+                Actor::cacheKeys($actor_id, $note_type)['note-tags'],
+                fn () => DB::dql(
+                    $query . ' and n.type = :type',
+                    ['id' => $actor_id, 'type' => $note_type],
+                ),
+            );
+        }
+    }
+
     /**
      * Process note by extracting any tags present
      */
@@ -82,21 +130,7 @@ class Tag extends Component
         $matched_tags = array_unique(F\map($matched_tags, fn ($m) => $m[2]));
         foreach ($matched_tags as $match) {
             $tag = self::extract($match);
-            if (!self::validate($tag)) {
-                continue; // Ignore invalid tag candidates
-            }
-            $canonical_tag = self::canonicalTag($tag, \is_null($lang_id = $note->getLanguageId()) ? null : Language::getById($lang_id)->getLocale());
-            DB::persist(NoteTag::create([
-                'tag'           => $tag,
-                'canonical'     => $canonical_tag,
-                'note_id'       => $note->getId(),
-                'use_canonical' => $extra_args['tag_use_canonical'] ?? false,
-                'language_id'   => $lang_id,
-            ]));
-            Cache::listPushLeft("tag-{$canonical_tag}", $note);
-            foreach (self::cacheKeys($canonical_tag) as $key) {
-                Cache::delete($key);
-            }
+            self::maybeCreateTag(tag: $tag, note_id: $note->getId(), lang_id: $note->getLanguageId());
         }
         return Event::next;
     }

plugins/Pinboard/Controller/APIv1.php

@@ -17,7 +17,9 @@ use App\Util\Exception\ClientException;
 use App\Util\Exception\ServerException;
 use App\Util\Formatting;
 use Component\Conversation\Conversation;
+use Component\Link\Link;
 use Component\Tag\Entity\NoteTag;
+use Component\Tag\Tag;
 use Datetime;
 use DateTimeImmutable;
 use DateTimeInterface;
@@ -34,7 +36,7 @@ class APIv1 extends Controller
 {
     public const SOURCE = 'Pinboard API v1';
 
-    private function before()
+    private function preCheck()
     {
         $format     = $this->string('format');
         $auth_token = $this->string('auth_token');
@@ -79,14 +81,18 @@ class APIv1 extends Controller
         if (!str_contains($input, ':')) {
             return null;
         }
-        [$nickame, $token] = explode(':', $input);
-        return Token::get($nickame, $token)?->getUser();
+        [$id, $token] = explode(':', $input);
+        if (filter_var($id, \FILTER_VALIDATE_INT) !== false) {
+            return Token::get((int) $id, $token)?->getUser();
+        } else {
+            return null;
+        }
     }
 
     private function deleteNoteAndMaybePin(LocalUser $user, Note $note, ?Pin $pin): void
     {
         $note->delete($user->getActor(), self::SOURCE);
-        if (!\is_null($note)) {
+        if (!\is_null($pin)) {
             DB::remove($pin);
         }
         DB::flush();
@@ -101,6 +107,15 @@ class APIv1 extends Controller
         return $tags;
     }
 
+    /**
+     * @param Pin[]    $pins
+     * @param string[] $tags
+     */
+    private function filterByTags(array $pins, array $tags): array
+    {
+        return F\select($pins, fn (Pin $pin) => array_intersect(F\map($pin->getTags(), fn (NoteTag $tag) => $tag->getTag()), $tags) !== []);
+    }
+
     private function getLatestModified(LocalUser $user): string
     {
         return Cache::get(
@@ -147,12 +162,17 @@ class APIv1 extends Controller
 
     /**
      * Returns the most recent time a bookmark was added, updated or
-     * deleted. Use this before calling posts/all to see if the data
+     * deleted. Use this preCheck calling posts/all to see if the data
      * has changed since the last fetch
      */
     public function posts_update(Request $request)
     {
-        $user = self::before();
+        $check = self::preCheck();
+        if (!$check instanceof LocalUser) {
+            return $check;
+        } else {
+            $user = $check;
+        }
 
         return self::respond(['update_time' => self::getLatestModified($user)]);
     }
@@ -162,7 +182,12 @@ class APIv1 extends Controller
      */
     public function posts_add(Request $request)
     {
-        $user = self::before();
+        $check = self::preCheck();
+        if (!$check instanceof LocalUser) {
+            return $check;
+        } else {
+            $user = $check;
+        }
 
         if (\is_null($url = $this->string('url'))) {
             throw new ClientException('URL must be provided');
@@ -199,28 +224,33 @@ class APIv1 extends Controller
             if (!$replace) {
                 $result_code = 'item already exists';
             } else {
-                throw new ServerException('Updating is unimplemented'); // TODO delete old note, create new one
                 $this->deleteNoteAndMaybePin($user, $pin->getNote(), pin: null);
+                // Continue below
             }
-        } else {
-            DB::persist($note = Note::create([
-                'actor_id'     => $user->getId(),
-                'content'      => $url,
-                'content_type' => 'text/uri-list',
-                'rendered'     => Formatting::twigRenderFile('pinboard/render.html.twig', ['url' => $url, 'title' => $title, 'description' => $description]),
-                'reply_to'     => null,
-                'is_local'     => true,
-                'source'       => self::SOURCE,
-                'scope'        => $public ? VisibilityScope::EVERYWHERE->value : VisibilityScope::ADDRESSEE->value,
-                'language_id'  => $user->getActor()->getTopLanguage()->getId(),
-                'type'         => 'page',
-                'title'        => $title,
-            ]));
-            $note->setUrl(Router::url('note_view', ['id' => $note->getId()], Router::ABSOLUTE_URL));
-            $pin->setNoteId($note->getId());
-            Conversation::assignLocalConversation($note, null);
-            DB::persist($pin);
-            // TODO handle tags
+        }
+        DB::persist($note = Note::create([
+            'actor_id'     => $user->getId(),
+            'content'      => "Bookmark: {$url}\nTitle: {$title}\nDescription: {$description}",
+            'content_type' => 'text/plain',
+            'rendered'     => Formatting::twigRenderFile('pinboard/render.html.twig', ['url' => $url, 'title' => $title, 'description' => $description]),
+            'reply_to'     => null,
+            'is_local'     => true,
+            'source'       => self::SOURCE,
+            'scope'        => $public ? VisibilityScope::EVERYWHERE->value : VisibilityScope::ADDRESSEE->value,
+            'language_id'  => $user->getActor()->getTopLanguage()->getId(),
+            'type'         => Pin::note_type,
+            'title'        => $title,
+        ]));
+        $note->setUrl(Router::url('note_view', ['id' => $note->getId()], Router::ABSOLUTE_URL));
+        $pin->setNoteId($note->getId());
+        Conversation::assignLocalConversation($note, null);
+        DB::persist($pin);
+        foreach ($tags as $tag) {
+            if (!\is_null($nt = Tag::maybeCreateTag(tag: $tag, note_id: $note->getId(), lang_id: $note->getLanguageId()))) {
+                DB::persist($nt);
+            }
+        }
+        if (array_values(Link::maybeCreateLink($url, $note->getId())) !== [null, null]) {
             DB::flush();
             $result_code = 'done';
         }
@@ -233,8 +263,14 @@ class APIv1 extends Controller
      */
     public function posts_delete(Request $request)
     {
-        $user = self::before($request);
-        $url  = $this->string('url');
+        $check = self::preCheck();
+        if (!$check instanceof LocalUser) {
+            return $check;
+        } else {
+            $user = $check;
+        }
+
+        $url = $this->string('url');
         if (\is_null($url)) {
             throw new ClientException('URL must be provided');
         }
@@ -255,16 +291,18 @@ class APIv1 extends Controller
      */
     public function posts_get(Request $request)
     {
-        $user = self::before($request);
+        $check = self::preCheck();
+        if (!$check instanceof LocalUser) {
+            return $check;
+        } else {
+            $user = $check;
+        }
+
         $tags = self::parseTags();
         $day  = $this->string('dt');
         $url  = $this->string('url');
         $meta = $this->bool('meta');
 
-        if (!\is_null($tags) && $tags !== []) {
-            throw new ClientException(_m('tags attribute not implemented'));
-        }
-
         if (\is_null($url) && \is_null($day)) {
             $pins = DB::findBy(Pin::class, ['actor_id' => $user->getId(), 'gte' => ['modified' => new DateTimeImmutable('today')]]);
         } elseif (!\is_null($day)) {
@@ -276,6 +314,8 @@ class APIv1 extends Controller
             throw new BugFoundException('Wonky logic in pinboard/posts/get');
         }
 
+        $pins = self::filterByTags($pins, $tags);
+
         return self::respond(self::formatPins($user, $pins));
     }
 
@@ -284,15 +324,18 @@ class APIv1 extends Controller
      */
     public function posts_recent(Request $request)
     {
-        $user  = self::before($request);
+        $check = self::preCheck();
+        if (!$check instanceof LocalUser) {
+            return $check;
+        } else {
+            $user = $check;
+        }
+
         $tags  = self::parseTags();
         $limit = min($this->int('count') ?? 15, 100);
 
-        if (!\is_null($tags) && $tags !== []) {
-            throw new ClientException('tags attribute not implemented');
-        }
-
         $pins = DB::findBy(Pin::class, ['actor_id' => $user->getId()], order_by: ['modified' => 'asc'], limit: $limit);
+        $pins = self::filterByTags($pins, $tags);
 
         return self::respond(self::formatPins($user, $pins));
     }
@@ -310,7 +353,13 @@ class APIv1 extends Controller
      */
     public function posts_all(Request $request)
     {
-        $user       = self::before($request);
+        $check = self::preCheck();
+        if (!$check instanceof LocalUser) {
+            return $check;
+        } else {
+            $user = $check;
+        }
+
         $tags       = self::parseTags();
         $offset     = $this->int('start');
         $limit      = $this->int('results');
@@ -318,10 +367,6 @@ class APIv1 extends Controller
         $end_time   = $this->string('todt');
         $meta       = $this->bool('meta');
 
-        if (!\is_null($tags) && $tags !== []) {
-            throw new ClientException('tags attribute not implemented');
-        }
-
         $criteria = ['actor_id' => $user->getId()];
         if (!\is_null($start_time)) {
             $criteria['gte'] = ['modified' => new DateTimeImmutable($start_time)];
@@ -331,6 +376,48 @@ class APIv1 extends Controller
         }
 
         $pins = DB::findBy(Pin::class, $criteria, order_by: ['modified' => 'asc'], offset: $offset, limit: $limit);
+        $pins = self::filterByTags($pins, $tags);
+
         return self::respond(self::formatPins($user, $pins)['posts']);
     }
+
+    /**
+     * Returns a full list of the user's tags along with the number of times they were used
+     */
+    public function tags_get(Request $request)
+    {
+        $check = self::preCheck();
+        if (!$check instanceof LocalUser) {
+            return $check;
+        } else {
+            $user = $check;
+        }
+
+        $tags_freq = [];
+        foreach ($user->getActor()->getNoteTags(Pin::note_type) as $tag) {
+            if (!isset($tags_freq[$tag->getCanonical()])) {
+                $tags_freq[$tag->getCanonical()] = [];
+            }
+            $tags_freq[$tag->getCanonical()][] = $tag->getTag();
+        }
+
+        foreach ($tags_freq as $canon => $variations) {
+            $freqs = array_count_values($variations);
+            arsort($freqs);
+            $tags_freq[$canon] = $freqs[array_key_first($freqs)];
+        }
+
+        return self::respond($tags_freq);
+    }
+
+    public function unimplmented(Request $request)
+    {
+        $check = self::preCheck();
+        if (!$check instanceof LocalUser) {
+            return $check;
+        } else {
+            $user = $check;
+        }
+        return self::respond(['result_code' => 'something went wrong', 'reason' => 'Unimplemented']);
+    }
 }

plugins/Pinboard/Controller/Settings.php

@@ -25,7 +25,7 @@ class Settings extends Controller
     public static function setup()
     {
         $user    = Common::ensureLoggedIn();
-        $token   = Token::get(nickname: null, token: null, user: $user);
+        $token   = Token::get(id: null, token: null, user: $user);
         $enabled = ($token?->getEnabled() ?? false);
         $form    = Form::create([
             ['token', TextType::class, ['label' => _m('The token used to authenticate you via the Pinboard-compatible API'), 'data' => $token?->getUserTokenString(), 'disabled' => true]],
@@ -79,7 +79,7 @@ class Settings extends Controller
             } else {
                 throw new ClientException(_m('Invalid form submission'));
             }
-            Cache::set(Token::cacheKeys($user->getNickname())['user-token'], $token);
+            Cache::set(Token::cacheKeys($user->getId())['user-token'], $token);
             DB::flush();
             return Form::forceRedirect($form, $request);
         }

plugins/Pinboard/Entity/Pin.php

@@ -139,6 +139,8 @@ class Pin extends Entity
     // @codeCoverageIgnoreEnd
     // }}} Autocode
 
+    public const note_type = 'page';
+
     public static function cacheKeys(int|LocalUser|Actor $user): array
     {
         $id = \is_int($user) ? $user : $user->getId();
@@ -157,7 +159,7 @@ class Pin extends Entity
      */
     public function getTags(): array
     {
-        return [];
+        return Note::getById($this->getNoteId())->getTags();
     }
 
     public static function schemaDef(): array

plugins/Pinboard/Entity/Token.php

@@ -67,10 +67,10 @@ class Token extends Entity
     // @codeCoverageIgnoreEnd
     // }}} Autocode
 
-    public static function cacheKeys(string $nickname): array
+    public static function cacheKeys(int $id): array
     {
         return [
-            'user-token' => "pinboard-token-{$nickname}",
+            'user-token' => "pinboard-token-{$id}",
         ];
     }
 
@@ -80,31 +80,30 @@ class Token extends Entity
     }
 
     /**
-     * Get a token for a $nickname and $token pair, unless given a $user, in which case the token field is not validated
+     * Get a token for a $id and $token pair, unless given a $user, in which case the token field is not validated
      *
      * XXX: may need to verify it's timing safe
      */
-    public static function get(?string $nickname, ?string $token, ?LocalUser $user = null): ?self
+    public static function get(?int $id, ?string $token, ?LocalUser $user = null): ?self
     {
         if (!\is_null($user)) {
             return Cache::get(
-                self::cacheKeys($user->getNickname())['user-token'],
+                self::cacheKeys($user->getId())['user-token'],
                 fn () => DB::dql(
                     'select t from \Plugin\Pinboard\Entity\Token t where t.actor_id = :id',
                     ['id' => $user->getId()],
                     options: ['limit' => 1],
                 ),
             );
-        } elseif (!\is_null($nickname) && !\is_null($token)) {
+        } elseif (!\is_null($id) && !\is_null($token)) {
             return Cache::get(
-                self::cacheKeys($nickname)['user-token'],
+                self::cacheKeys($id)['user-token'],
                 fn () => DB::dql(
                     <<<'EOF'
-                        select lu from \App\Entity\LocalUser lu
-                        join \Plugin\Pinboard\Entity\Token t on t.actor_id = lu.actor_id
-                        where lu.nickname = :nickname and t.token = :token and t.enabled = true
+                        select t from \Plugin\Pinboard\Entity\Token t
+                        where t.actor_id = :id and t.token = :token and t.enabled = true
                         EOF,
-                    ['nickname' => $nickame, 'token' => $token],
+                    ['id' => $id, 'token' => $token],
                     options: ['limit' => 1],
                 ),
             );
@@ -113,7 +112,7 @@ class Token extends Entity
 
     public function getUserTokenString()
     {
-        return LocalUser::getById($this->getActorId())->getNickname() . ':' . $this->getToken();
+        return $this->getActorId() . ':' . $this->getToken();
     }
 
     public static function generateTokenString(): string

plugins/Pinboard/Pinboard.php

@@ -35,10 +35,7 @@ namespace Plugin\Pinboard;
 use App\Core\Event;
 use App\Core\Modules\Plugin;
 use App\Core\Router;
-use App\Entity\Actor;
-use App\Entity\LocalUser;
 use Plugin\Pinboard\Controller as C;
-use Plugin\Pinboard\Entity\Token;
 use Symfony\Component\HttpFoundation\Request;
 
 class Pinboard extends Plugin
@@ -47,48 +44,26 @@ class Pinboard extends Plugin
 
     public function onAddRoute(Router $r): bool
     {
-        $r->connect(
-            self::controller_route,
-            '/pinboard/settings',
-            C\Settings::class,
-            options: ['method' => 'post'],
-        );
+        $r->connect(self::controller_route, '/pinboard/settings', C\Settings::class, options: ['method' => 'post']);
 
-        $r->connect(
-            'pinboard_posts_update',
-            '/pinboard/v1/posts/update',
-            [C\APIv1::class, 'posts_update'],
-        );
+        $r->connect('pinboard_posts_update', '/pinboard/v1/posts/update', [C\APIv1::class, 'posts_update']);
+        $r->connect('pinboard_posts_add', '/pinboard/v1/posts/add', [C\APIv1::class, 'posts_add']);
+        $r->connect('pinboard_posts_delete', '/pinboard/v1/posts/delete', [C\APIv1::class, 'posts_delete']);
+        $r->connect('pinboard_posts_get', '/pinboard/v1/posts/get', [C\APIv1::class, 'posts_get']);
+        $r->connect('pinboard_posts_recent', '/pinboard/v1/posts/recent', [C\APIv1::class, 'posts_recent']);
+        $r->connect('pinboard_posts_all', '/pinboard/v1/posts/all', [C\APIv1::class, 'posts_all']);
 
-        $r->connect(
-            'pinboard_posts_add',
-            '/pinboard/v1/posts/add',
-            [C\APIv1::class, 'posts_add'],
-        );
+        $r->connect('pinboard_posts_dates', '/pinboard/v1/posts/dates', [C\APIv1::class, 'unimplemented']);
+        $r->connect('pinboard_posts_suggest', '/pinboard/v1/posts/suggest', [C\APIv1::class, 'unimplemented']);
 
-        $r->connect(
-            'pinboard_posts_delete',
-            '/pinboard/v1/posts/delete',
-            [C\APIv1::class, 'posts_delete'],
-        );
+        $r->connect('pinboard_tags_get', '/pinboard/v1/tags/get', [C\APIv1::class, 'tags_get']);
 
-        $r->connect(
-            'pinboard_posts_get',
-            '/pinboard/v1/posts/get',
-            [C\APIv1::class, 'posts_get'],
-        );
-
-        $r->connect(
-            'pinboard_posts_recent',
-            '/pinboard/v1/posts/recent',
-            [C\APIv1::class, 'posts_recent'],
-        );
-
-        $r->connect(
-            'pinboard_posts_all',
-            '/pinboard/v1/posts/all',
-            [C\APIv1::class, 'posts_all'],
-        );
+        $r->connect('pinboard_tags_delete', '/pinboard/v1/tags/delete', [C\APIv1::class, 'unimplemented']);
+        $r->connect('pinboard_tags_rename', '/pinboard/v1/tags/rename', [C\APIv1::class, 'unimplemented']);
+        $r->connect('pinboard_user_secret', '/pinboard/v1/user/secret', [C\APIv1::class, 'unimplemented']);
+        $r->connect('pinboard_user_api_token', '/pinboard/v1/user/api_token', [C\APIv1::class, 'unimplemented']);
+        $r->connect('pinboard_notes_list', '/pinboard/v1/notes/list', [C\APIv1::class, 'unimplemented']);
+        $r->connect('pinboard_notes_id', '/pinboard/v1/notes/{id}', [C\APIv1::class, 'unimplemented']);
 
         return Event::next;
     }
@@ -105,17 +80,4 @@ class Pinboard extends Plugin
         }
         return Event::next;
     }
-
-    public function onActorFormInvalidateRelated(Actor $actor, ?LocalUser $user)
-    {
-        $user ??= $actor->getLocal();
-        if (!$user instanceof LocalUser) {
-            return Event::next;
-        }
-        Cache::delete(Token::cacheKeys($user->getNickname())['user-token']);
-        DB::remove(DB::refetch(Token::get(nickname: null, token: null, user: $user)));
-        DB::flush();
-        // TODO notify user that their token got invalidated
-        return Event::next;
-    }
 }

src/Entity/Actor.php

@@ -42,6 +42,7 @@ use Component\Group\Entity\LocalGroup;
 use Component\Language\Entity\ActorLanguage;
 use Component\Language\Entity\Language;
 use Component\Subscription\Entity\ActorSubscription;
+use Component\Tag\Tag;
 use Functional as F;
 
 /**
@@ -263,7 +264,8 @@ class Actor extends Entity
             'id' => "actor-id-{$actor_id}",
             'nickname' => "actor-nickname-id-{$actor_id}",
             'fullname' => "actor-fullname-id-{$actor_id}",
-            'self-tags' =>  "actor-self-tags-{$actor_id}",
+            'self-tags' => "actor-self-tags-{$actor_id}",
+            'note-tags' => "actor-note-tags-{$actor_id}-{$other}", // $other is note type
             'circles' => "actor-circles-{$actor_id}",
             'subscribers' => "subscribers-{$actor_id}",
             'subscribed' => "subscribed-{$actor_id}",
@@ -347,6 +349,14 @@ class Actor extends Entity
         );
     }
 
+    /**
+     * @return NoteTag[]
+     */
+    public function getNoteTags(?string $note_type = null): array
+    {
+        return Tag::getNoteTags($this->getId(), $note_type);
+    }
+
     private function getSubCount(string $which, string $column): int
     {
         return Cache::get(