[native] feat: Support tags on Android and iOS

.github/workflows/android-appium.yml

@@ -89,7 +89,7 @@ jobs:
 
   android-appium:
     runs-on: ubuntu-latest
-    timeout-minutes: 60
+    timeout-minutes: 80
     needs: init
 
     env:

_locales/en/messages.json

@@ -960,6 +960,15 @@
   "LabelParentfolder": {
     "message": "Parent folder"
   },
+  "LabelTags": {
+    "message": "Tags"
+  },
+  "DescriptionTags": {
+    "message": "Type a tag and press enter"
+  },
+  "LabelSearchbytag": {
+    "message": "Search bookmarks tagged {0}"
+  },
   "LabelHome": {
     "message": "Home"
   },

doc/Adapters.md

@@ -109,6 +109,21 @@ class Resource {
    */
   async removeFolder(id: int)
 
+  /**
+   * @return Promise<ICapabilities> what this resource can do:
+   *   {
+   *     preserveOrder: boolean,          // can store the order of children
+   *     hashFn: ('sha256'|'murmur3'|'xxhash3')[], // supported hash functions
+   *     supportsTags: boolean,           // can store and return Bookmark#tags
+   *   }
+   *
+   * Tags are only synced if *both* resources of a sync report `supportsTags`.
+   * If they do, `Bookmark#tags` takes part in hashing, so tag-only changes bump
+   * folder hashes and aren't skipped over -- servers that compute folder hashes
+   * themselves have to include tags in them too (see NextcloudBookmarks#_getFolderHash).
+   */
+  async getCapabilities() : ICapabilities
+
   /**
    * ------
    * The following methods are optional
@@ -143,8 +158,12 @@ class Bookmark {
   public parentId: int
   public url: string
   public title: string
+  // Only meaningful for adapters that report `supportsTags: true`. `undefined`
+  // means "this resource has nothing to say about tags" and is never treated as
+  // "no tags", so an adapter without tag support can't wipe the other side's.
+  public tags: string[]|undefined
 
-  constructor({ id: int, parentId: int, url: string, title: string })
+  constructor({ id: int, parentId: int, url: string, title: string, tags?: string[] })
 
   clone() : Bookmark
 }

doiuse-report.baseline.txt

@@ -2,6 +2,8 @@
 
 
 
+108:1: CSS overflow property only partially supported by: Safari on iOS (11.0-11.2), QQ Browser (14.9) (css-overflow)
+112:1: CSS scrollbar styling not supported by: Safari on iOS (16.6-16.7,18.5-18.7) and only partially supported by: Chrome (103-104,109,111-112,116-118,120), Safari on iOS (11.0-11.2), Android Browser (149), QQ Browser (14.9) (css-scrollbar)
 1:10179: CSS caret-color not supported by: Safari on iOS (11.0-11.2) (css-caret-color)
 1:10389: CSS caret-color not supported by: Safari on iOS (11.0-11.2) (css-caret-color)
 1:10599: CSS caret-color not supported by: Safari on iOS (11.0-11.2) (css-caret-color)

src/lib/LocalTabs.ts

@@ -607,6 +607,7 @@ export default class LocalTabs implements OrderFolderResource<typeof ItemLocatio
     return {
       preserveOrder: true,
       hashFn: ['xxhash3', 'murmur3', 'sha256'],
+      supportsTags: false,
     }
   }
 

src/lib/Scanner.ts

@@ -1,6 +1,6 @@
 import * as Parallel from 'async-parallel'
 import Diff, { ActionType, CreateAction, MoveAction, RemoveAction, ReorderAction, UpdateAction } from './Diff'
-import { Bookmark, Folder, ItemLocation, ItemType, TItem, TItemLocation } from './Tree'
+import { Bookmark, Folder, ItemLocation, ItemType, tagsEqual, TItem, TItemLocation } from './Tree'
 import Logger from './Logger'
 import { IHashSettings } from './interfaces/Resource'
 import { yieldToEventLoop } from './yieldToEventLoop'
@@ -150,9 +150,13 @@ export default class Scanner<L1 extends TItemLocation, L2 extends TItemLocation>
   async diffBookmark(oldBookmark:Bookmark<L1>, newBookmark:Bookmark<L2>):Promise<void> {
     let hasChanged
     if (this.checkHashes) {
+      // With tag syncing on, the hash covers tags as well
       hasChanged = await this.bookmarkHasChanged(oldBookmark, newBookmark)
     } else {
-      hasChanged = oldBookmark.title !== newBookmark.title || oldBookmark.url !== newBookmark.url
+      hasChanged =
+        oldBookmark.title !== newBookmark.title ||
+        oldBookmark.url !== newBookmark.url ||
+        Boolean(this.hashSettings?.syncTags && !tagsEqual(oldBookmark.tags, newBookmark.tags))
     }
     if (hasChanged) {
       this.result.UPDATE.commit({ type: ActionType.UPDATE, payload: newBookmark, oldItem: oldBookmark })

src/lib/Tree.ts

@@ -32,6 +32,73 @@ interface IItemIndex<L extends TItemLocation> {
 
 let HASH_ITERATIONS = 0
 
+/**
+ * Bring a bookmark's tags into canonical form: strings only, trimmed, no empty
+ * entries, no duplicates, sorted.
+ *
+ * Sorting is what makes tags behave like the set they are: the order a server
+ * hands them back in is its own business (Linkwarden orders by tag identity,
+ * Nextcloud by whatever the database feels like), so without a canonical order
+ * a pure reordering would hash differently and register as a change.
+ *
+ * The sort has to agree byte for byte with Nextcloud Bookmarks, which sorts
+ * with `sort($tags, SORT_STRING)` before hashing `{title, url, tags}` for its
+ * server-side folder hashes. Plain `.sort()` is UTF-16 code-unit order, which
+ * matches that across the BMP. Do NOT switch this to `localeCompare` -- it is
+ * locale-dependent and would silently stop matching, leaving folders looking
+ * changed on every sync. (Astral characters do diverge from PHP's byte order;
+ * the cost is a needlessly re-fetched folder, never wrong data.)
+ *
+ * `undefined` means "this resource didn't tell us anything about tags" and is
+ * preserved as such, so we never mistake a silent adapter for "all tags removed".
+ */
+export function normalizeTags(tags?: string[]): string[] | undefined {
+  if (!Array.isArray(tags)) {
+    return undefined
+  }
+  const seen = new Set<string>()
+  const normalized = []
+  for (const tag of tags) {
+    if (typeof tag !== 'string') {
+      continue
+    }
+    const trimmed = tag.trim()
+    if (!trimmed || seen.has(trimmed)) {
+      continue
+    }
+    seen.add(trimmed)
+    normalized.push(trimmed)
+  }
+  return normalized.sort()
+}
+
+/**
+ * Cache slot for a memoized hash. Every setting that changes the hashed bytes
+ * has to be part of it, or a sync that negotiated different settings would read
+ * back a stale value.
+ */
+function hashCacheKey({ preserveOrder, hashFn, syncTags }: IHashSettings): string {
+  return `${preserveOrder}-${hashFn}-${Boolean(syncTags)}`
+}
+
+/**
+ * Compare two tag lists as sets: tags are unordered by nature, so a mere
+ * reordering must not count as a change.
+ */
+export function tagsEqual(tags1?: string[], tags2?: string[]): boolean {
+  const set1 = new Set(tags1 || [])
+  const set2 = new Set(tags2 || [])
+  if (set1.size !== set2.size) {
+    return false
+  }
+  for (const tag of set1) {
+    if (!set2.has(tag)) {
+      return false
+    }
+  }
+  return true
+}
+
 export class Bookmark<L extends TItemLocation> {
   public type = ItemType.BOOKMARK
   public id: string | number
@@ -62,7 +129,7 @@ export class Bookmark<L extends TItemLocation> {
     this.id = id
     this.parentId = parentId
     this.title = title
-    this.tags = tags
+    this.tags = normalizeTags(tags)
     // eslint-disable-next-line @typescript-eslint/ban-ts-comment
     // @ts-ignore
     this.location = location || ItemLocation.LOCAL
@@ -101,33 +168,40 @@ export class Bookmark<L extends TItemLocation> {
   }
 
   setHashCacheValue(hashSettings: IHashSettings, value: string): void {
-    const cacheKey = `${hashSettings.preserveOrder}-${hashSettings.hashFn}`
+    const cacheKey = hashCacheKey(hashSettings)
     if (!this.hashValue) this.hashValue = {}
     this.hashValue[cacheKey] = value
   }
 
   async hash(
-    { preserveOrder = false, hashFn = 'sha256' }: IHashSettings = {
+    { preserveOrder = false, hashFn = 'sha256', syncTags = false }: IHashSettings = {
       preserveOrder: false,
       hashFn: 'sha256',
     }
   ): Promise<string> {
+    const cacheKey = hashCacheKey({ preserveOrder, hashFn, syncTags })
     if (!this.hashValue) {
       this.hashValue = {}
     }
-    if (typeof this.hashValue[hashFn] === 'undefined' || this.hashValue[hashFn] === null) {
-      const json = JSON.stringify({ title: this.title, url: this.url })
+    if (typeof this.hashValue[cacheKey] === 'undefined' || this.hashValue[cacheKey] === null) {
+      // Nextcloud Bookmarks hashes the very same JSON server-side, with the
+      // fields in exactly this order (`fields[]=title&fields[]=url&fields[]=tags`)
+      // and the tags sorted the same way (see normalizeTags), so don't reorder
+      // or add keys here lightly.
+      const json = syncTags
+        ? JSON.stringify({ title: this.title, url: this.url, tags: this.tags || [] })
+        : JSON.stringify({ title: this.title, url: this.url })
       if (hashFn === 'sha256') {
-        this.hashValue[hashFn] = await Crypto.sha256(json)
+        this.hashValue[cacheKey] = await Crypto.sha256(json)
       } else if (hashFn === 'xxhash3') {
-        this.hashValue[hashFn] = await Crypto.xxhash32(json)
+        this.hashValue[cacheKey] = await Crypto.xxhash32(json)
       } else if (hashFn === 'murmur3') {
-        this.hashValue[hashFn] = await Crypto.murmurHash3(json)
+        this.hashValue[cacheKey] = await Crypto.murmurHash3(json)
       } else {
         throw new Error('Unsupported hash function specified')
       }
     }
-    return this.hashValue[hashFn]
+    return this.hashValue[cacheKey]
   }
 
   clone(withHash?: boolean): Bookmark<L> {
@@ -433,18 +507,18 @@ export class Folder<L extends TItemLocation> {
   }
 
   setHashCacheValue(hashSettings: IHashSettings, value: string): void {
-    const cacheKey = `${hashSettings.preserveOrder}-${hashSettings.hashFn}`
+    const cacheKey = hashCacheKey(hashSettings)
     if (!this.hashValue) this.hashValue = {}
     this.hashValue[cacheKey] = value
   }
 
   async hash(
-    { preserveOrder = false, hashFn = 'sha256' }: IHashSettings = {
+    { preserveOrder = false, hashFn = 'sha256', syncTags = false }: IHashSettings = {
       preserveOrder: false,
       hashFn: 'sha256',
     }
   ): Promise<string> {
-    const cacheKey = `${preserveOrder}-${hashFn}`
+    const cacheKey = hashCacheKey({ preserveOrder, hashFn, syncTags })
     if (this.hashValue && typeof this.hashValue[cacheKey] !== 'undefined') {
       return this.hashValue[cacheKey]
     }
@@ -475,7 +549,7 @@ export class Folder<L extends TItemLocation> {
       title: this.title,
       children: await Parallel.map(
         children,
-        (child) => child.hash({ preserveOrder, hashFn }),
+        (child) => child.hash({ preserveOrder, hashFn, syncTags }),
         1
       ),
     })

src/lib/adapters/Caching.ts

@@ -93,6 +93,11 @@ export default class CachingAdapter implements Adapter, BulkImportResource<TItem
     }
     foundBookmark.url = newBm.url
     foundBookmark.title = newBm.title
+    // `undefined` means the caller doesn't know about tags (e.g. it came from a
+    // resource that doesn't support them) -- don't take that as "no tags".
+    if (typeof newBm.tags !== 'undefined') {
+      foundBookmark.tags = newBm.tags.slice()
+    }
     if (String(foundBookmark.parentId) === String(newBm.parentId)) {
       return
     }
@@ -284,6 +289,10 @@ export default class CachingAdapter implements Adapter, BulkImportResource<TItem
     return {
       preserveOrder: true,
       hashFn: ['xxhash3', 'murmur3', 'sha256'],
+      // The in-memory tree could hold tags just fine, but the file-based
+      // adapters built on top of it serialize through formats that drop them,
+      // so subclasses opt in explicitly.
+      supportsTags: false,
     }
   }
 

src/lib/adapters/Fake.js

@@ -23,4 +23,12 @@ export default class FakeAdapter extends CachingAdapter {
   getLabel() {
     return 'Fake account (floccus)'
   }
+
+  async getCapabilities() {
+    return {
+      ...(await super.getCapabilities()),
+      // The fake server stands in for tag-capable servers in the test suite
+      supportsTags: true,
+    }
+  }
 }

src/lib/adapters/Karakeep.ts

@@ -152,9 +152,57 @@ export default class KarakeepAdapter implements Adapter, IResource<typeof ItemLo
       true,
       bookmark
     )
+    if (typeof bookmark.tags !== 'undefined') {
+      if (response.alreadyExists) {
+        await this.setBookmarkTags(response.id, bookmark.tags, bookmark)
+      } else {
+        await this.attachTags(response.id, bookmark.tags, bookmark)
+      }
+    }
     return `${response.id};${bookmark.parentId}`
   }
 
+  async attachTags(id: string | number, tags: string[], item: TItem<TItemLocation> = null): Promise<void> {
+    if (!tags.length) {
+      return
+    }
+    await this.sendRequest(
+      'POST',
+      `/api/v1/bookmarks/${id}/tags`,
+      'application/json',
+      { tags: tags.map((tagName) => ({ tagName })) },
+      false,
+      item
+    )
+  }
+
+  async detachTags(id: string | number, tags: string[], item: TItem<TItemLocation> = null): Promise<void> {
+    if (!tags.length) {
+      return
+    }
+    await this.sendRequest(
+      'DELETE',
+      `/api/v1/bookmarks/${id}/tags`,
+      'application/json',
+      { tags: tags.map((tagName) => ({ tagName })) },
+      false,
+      item
+    )
+  }
+
+  /**
+   * Karakeep has no "replace all tags" call, so reconcile against what's there.
+   */
+  async setBookmarkTags(id: string | number, tags: string[], item: TItem<TItemLocation> = null): Promise<void> {
+    const response = await this.sendRequest(
+      'GET',
+      `/api/v1/bookmarks/${id}?includeContent=false`
+    )
+    const currentTags = (response.tags || []).map((tag) => tag.name)
+    await this.attachTags(id, tags.filter((tag) => !currentTags.includes(tag)), item)
+    await this.detachTags(id, currentTags.filter((tag) => !tags.includes(tag)), item)
+  }
+
   async updateBookmark(bookmark: Bookmark<TItemLocation>): Promise<void> {
     Logger.log('(karakeep)UPDATE', { bookmark })
     const [id, oldParentId] = this.parseBookmarkId(bookmark.id)
@@ -170,6 +218,10 @@ export default class KarakeepAdapter implements Adapter, IResource<typeof ItemLo
       bookmark
     )
 
+    if (typeof bookmark.tags !== 'undefined') {
+      await this.setBookmarkTags(id, bookmark.tags, bookmark)
+    }
+
     if (oldParentId !== bookmark.parentId) {
       await Promise.all([
         this.sendRequest(
@@ -417,6 +469,7 @@ export default class KarakeepAdapter implements Adapter, IResource<typeof ItemLo
               title: b.title ?? b.content.title,
               parentId: listId,
               url: b.content.url,
+              tags: (b.tags || []).map((tag) => tag.name),
               location: ItemLocation.SERVER,
             })
         )
@@ -617,6 +670,7 @@ export default class KarakeepAdapter implements Adapter, IResource<typeof ItemLo
     return {
       preserveOrder: false,
       hashFn: ['xxhash3', 'murmur3', 'sha256'],
+      supportsTags: true,
     }
   }