feat: Make PouchLink querying system compatible with new PouchDB version #1506

Ldoppea · 2024-07-26T14:39:46Z

In previous PRs we modified cozy-client and cozy-pouch-link to make them compatible with the react-native Flagship app

Since we didn't use this link for a long time but also because we use a newer PouchDB version, some queries do not work anymore (i.e. incompatible indexes, bad selectors etc)

This PR improve PouchLink querying system to make our app ecosystem compatible with new PouchDB version

With previous changes, we now expect the CozyPouchLink to be the last link of the chain, so forwarding the query would result to an exception thrown This mean we cannot forward the query when warmup queries are not finished yet So we want to allow CozyPouchLink to ignore the verification step and process the query independently of the warmup queries status To allow this we introduce the `ignoreWarmup` parameter. When set to `true` the CozyPouchLink will process the query even if warmup is not finished yet Related PR: cozy/cozy-client#1506

paultranvan · 2024-08-14T07:45:39Z

packages/cozy-pouch-link/src/CozyPouchLink.js

+   * @param {string} [indexOption.doctype] - doctype
+   * @returns {Promise<import('./types').PouchDbIndex>}
+   */
+  async createIndex(fields, { partialFilter, indexName, doctype } = {}) {


I don't think the doctype should be an option, as the function cannot work without it

paultranvan · 2024-08-14T07:55:19Z

packages/cozy-pouch-link/src/CozyPouchLink.js

+  findExistingIndex(doctype, options, indexName) {
+    const absName = `${doctype}/${indexName}`
+    return this.indexes[absName]
+  }


I know this method name is copied on the one in DocumentCollection, but I feel like it is a bit deceptive here regarding its name: it checks if we have the index in memory, but if it is not found, it does not mean the index does not exist.
And if it's not found, we create the index: about that did we check that pouchdb correctly handle a creation of an existing index (i.e. it won't recreate the index) ? I think we did, but I'm not entirely sure 😄
In any case, I think we should:

either rename this function in something like hasIndexInMemory, either document a bit more the behaviour

add some comment in the index creation to specify that the creation of an exiting index will not recreate it

paultranvan · 2024-08-14T08:03:49Z

packages/cozy-pouch-link/src/CozyPouchLink.spec.js

+          indexName: 'by_myIndex',
+          partial_filter_selector: undefined
+        }
+      })


it might be worth adding a simple test with an actual partialFilter

Done in #1524

paultranvan · 2024-08-14T08:06:31Z

packages/cozy-pouch-link/src/CozyPouchLink.js

+      if (indexedFields) {
+        for (const indexedField of indexedFields) {
+          if (!Object.keys(selector).includes(indexedField)) {
+            selector[indexedField] = {


I feel like we should add a warning here, that we do some magic to please pouchdb but that ideally the query should be fixed

And we probably should extract this logic into a function with a test 😉

Done in #1524

paultranvan · 2024-08-14T08:20:34Z

packages/cozy-pouch-link/src/CozyPouchLink.js

+        findSelector['_id'] = {
+          $gt: null
+        }
+      }


You don't need to force the _id selector if the mechanism on line 566-567 added a predicate.
Thus, we might refacto like this:

let findSelector = selector || {} // ...do the stuff on lines 566... findSelector = Object.keys(selector).length > 0 ? findSelector : { _id: { $gt: null } } // PouchDB does not accept empty selector

And this whole behavior should definitely be tested, as suggested in my previous comment 😉

Done in #1524

paultranvan · 2024-08-14T08:52:11Z

packages/cozy-pouch-link/src/jsonapi.js

  return normalizedDoc
 }

+const normalizeLinks = (docRef, doctype, client) => {
+  if (doctype !== 'io.cozy.apps') {


I guess the function is more normalizeAppsLinks , then?

Done in #1524

paultranvan · 2024-08-14T08:54:50Z

packages/cozy-pouch-link/src/jsonapi.js

 const filterDeletedDocumentsFromRows = doc => !!doc

-export const fromPouchResult = (res, withRows, doctype) => {
+export const fromPouchResult = (res, withRows, doctype, client) => {


nit: When dealing with many params, I prefer a ({param1, param2,...}) API

Done in #1524

paultranvan · 2024-08-14T09:00:14Z

packages/cozy-pouch-link/src/CozyPouchLink.js

@@ -96,6 +96,7 @@ class PouchLink extends CozyLink {
    this.storage = new PouchLocalStorage(
      options.platform?.storage || platformWeb.storage
    )
+    this.ignoreWarmup = ignoreWarmup


If we don't specify any warmup query, this is useless, right?

Done in #1524

With previous changes, we now expect the CozyPouchLink to be the last link of the chain, so forwarding the query would result to an exception thrown This mean we cannot forward the query when warmup queries are not finished yet So we want to allow CozyPouchLink to ignore the verification step and process the query independently of the warmup queries status To allow this we introduce the `ignoreWarmup` parameter. When set to `true` the CozyPouchLink will process the query even if warmup is not finished yet Related PR: cozy/cozy-client#1506

This replies to #1506 (comment)