Docs with rev greater than 1-* not transforming when replicating from remote

[czarcismok]

Hi everyone!
Working with PouchDB and transform-pouch for some time now and come across this problem (original issue: pouchdb/pouchdb#7034)

Issue

Documents with revision greater than 1-* are not transforming when replicating from remote-CouchDB.

_changes?style=all_docs&seq_interval=100&since=0&limit=100 gets all changes, but in
_all_docs?conflicts=true&include_docs=true request payload are only keys of docs with rev = 1.*

Info

• Environment: browser
• Platform: Chrome
• Adapter: IndexedDB
• Server: CouchDB

Reproduce

1. Put document in remote (rev 1).
2. Modify doc and put it in remote (rev 2).
3. Set some transformation on an outgoing docs (ie.: doc.local = true; doc.remote = false;).
4. Start replication to local PouchDB.
5. List local docs (should not contain changes from transformation).

Can't get glitch to work with transform-pouch (any ideas?):
https://glitch.com/edit/#!/understood-ear

I've been struggling with this for almost a week now and don't know if I'm doing something awfully wrong or this is a bug / desired behavior. Help, please? :)

[czarcismok]

Looking at the response I think that something like this should handle bulkGet:

  handlers.bulkGet = function (orig) {
    return orig().then(function (res) {
      var none = {};
      return utils.Promise.all(res.results.map(function (result) {
        if (result.docs && result.docs[0] && result.docs[0].ok) {
          return outgoing(result.docs[0].ok);
        }
        return none;
      })).then(function (resp) {
        resp.forEach(function (doc, i) {
          if (doc === none) {
            return;
          }
          res.results[i].docs[0].ok = doc;
        });
        return res;
      });
    });
  };

I'll try this and share result.

[marten-de-vries]

Thanks for your bug report! I think the problem is that bulkGet did not exist when this plugin was written. It would not surprise me if pouchdb-wrappers, which is used by this library to wrap PouchDB functions, does not even support it yet either.

[czarcismok]

It doesn't. Don't know what this function supposed to do, but with this (copied from wrapper for allDocs):

wrapperBuilders.bulkGet = function (db, bulkGet, handlers) {
  return function (options, callback) {
    var args = parseBaseArgs(db, this, options, callback);
    return callHandlers(handlers, args, makeCallWithOptions(bulkGet, args));
  };
};

it works.

Test case with modified transform-pouch:
https://tasteful-lunch.glitch.me/

[marten-de-vries]

Nice! I'll have to read the surrounding code for both to check your snippets for correctness (it's been long since I touched that code), but on first look I think they can serve as a patch.

[marten-de-vries]

I'm not sure if your patch for transform-pouch is correct. It seems like pouchdb will never violate your docs[0] assumption, but I'm not sure if other implementations also won't. Perhaps to be sure it's better to send every array item through outgoing? I'd appreciate you opinion, @czarcismok.

I opened a PR for a pouchdb-wrappers fix. I'm going to focus on pouchdb/pouchdb-server#290 now, though. I realised that bug exists due to this one.

[czarcismok]

You might be right @marten-de-vries. I couldn't find how open_revs response look like with multiple docs but for some reason that array is there. Based on PouchDB api reference I came up with something that looks exactly like this:

  handlers.bulkGet = function (orig) {
    return orig().then(function (res) {
      var none = {};
      return utils.Promise.all(res.results.map(function (result) {
        if (result.id && result.docs && Array.isArray(result.docs)) {
          return {
            docs: result.docs.map(function(doc) {
              if (doc.ok) {
                return {ok: outgoing(doc.ok)};
              } else {
                return doc;
              }
            }),
            id: result.id
          };
        } else {
          return none;
        }
      })).then(function (results) {
        return {results: results};
      });
    });
  };

I know it doesn't return original res object but do it have to? Or is it for performance purpose?
I didn't test it with Promise-like transformation functions.

[marten-de-vries]

pouchdb-wrappers does not require returning the original object, so that should be ok. I guess it requires node to do garbage collection a bit more, but let's not obsess over performance without observing a slowdown. I'll check if I can find the CouchDB implementation of _bulk_get to see how that array is used before merging, but I expect your patch to fix the problem.

[marten-de-vries]

Oh, this does depend on the fix to pouchdb-wrappers (which I just merged) being released. As it's part of the monorepo of pouchdb-server, we'll have to wait for a release of that. That's in the works, though.

[marten-de-vries]

Well, I still don't fully understand it, but it seems like CouchDB's internal handling does use lists: https://github.com/apache/couchdb-couch/pull/18/files

So let's go with your last snippet for transform-pouch. I'll make a PR, but it'll be blocked on a new pouchdb-wrappers release.

[AdrianoFerrari]

Any updates on this @marten-de-vries?

Replicating from remote is my only use case for this package, and it isn't working for me (bug reproduced as per original comment).

As far as I can tell, this has been updated upstream, and committed here, and is just waiting for publish. When I pull this repo locally (and manually upgrade pouchdb-wrappers to latest 4.2.0), it seems to work 😄.

I'd make a PR, but I've never published to npm and I don't want to break anything. (willing to try though, if that's what you recommend).

[marten-de-vries]

@AdrianoFerrari I just published 1.1.5, and hope that fixes the problem. This project hasn't been updated for a while and it seems like most (dev) dependencies are out of date, but this should take care of the most pressing problem.