Recomputing validation function (and methods) on every validating call

[tohagan]

I've just had chance to walk through all the code in this plugin and if I read the code correctly I think the current implementation will incur an unnecessarily large performance penalty on every validating call as on every call it reloads all the _design objects from the local database and recompiles the validation functions. Of smaller performance impact is that it also calls method() to rebuild the set of method on the database when it really only need one of them.

In reality, validation functions in the database are very rarely modified (except during development) so it would make more sense to cache a db.validation() function with each database instance. The plugin would then rebuild this cached function if unset for each PouchDb instance. If required the app can also watch db.changes() and clear this cache function if a design doc is updated. I think this would significantly improve validation performance.

I suspect that you might have the same issue for the other related plugins.

[tohagan]

Not tested but I think all you need to do is to save db.validationFuncs in getValidationFunctions():

function getValidationFunctions(db, callback) {
  if (db.validationFuncs) return Promise.resolve(db.validationFuncs)
  return db.allDocs({
    startkey: "_design/",
    endkey: "_design0",
    include_docs: true
  }).then(function(designDocs) {
    return db.validationFuncs = parseValidationFunctions(designDocs);
  });
}

if you're concerned about backward compatibility just add an option and send options to getValidationFunctions():

function getValidationFunctions(db, options, callback) {
  if (options.useCache && db.validationFuncs) return Promise.resolve(db.validationFuncs)
  return db.allDocs({
    startkey: "_design/",
    endkey: "_design0",
    include_docs: true
  }).then(function(designDocs) {
    return db.validationFuncs = parseValidationFunctions(designDocs);
  });
}

[marten-de-vries]

Thanks for the feedback! The nicest solution would be to invalidate the cache when the relevant design doc is updated, but that'd require a wrapper over put/post/bulkdocs. I'd accept a PR for this using the useCache option too, though.

[tohagan]

Actually I think we'd have use db.changes() as ddocs can be synced from server.
Am I missing something? Perhaps your suggestion is more efficient that my approach in that replication calls bulkdocs()?

Personally I'd not be that keen to have multiple db.changes() callbacks firing on every synced doc. I'd rather just have a way to clear the cache in a single app level db.changes() that watches other stuff in my app and clears the validation cache when a ddoc is observed. A code sample in the README for this plugin could show how to do this. So in short, we'd add a clearValidationCache() method to the plugin. Not as elegant an API but I suspect more efficient. What do you think?

[tohagan]

Another nice pattern I learned recently would be to just cache the promise object :) .

This example includes many other features we don't need but it shows the pattern.

[marten-de-vries]

Pretty sure wrapping catches replication as well, changes() would work as well.

The advantage of such an approach would be that e.g. express-pouchdb immediately profits. So it has my preference, but an 'useCache' option would do. Having to manually invalidate using a method makes this technically couchdb incompatible, so I prefer not to do that.

[tohagan]

OK ... Sounds good. The wrapping wins 👍 if that works best for all cases. I have an urgent deadline this week for my Pouch project so won't be able to do a PR this week - but I'd like to contribute.

[marten-de-vries]

I just opened a rather substantial PR which re-adds the test suite (bringing coverage to 100%), and documentation. If you plan to open a PR at some point, please use it as the base instead of master: #5

[marten-de-vries]

The disadvantage to wrapping is that you can only use it when using installValidationMethods(), not when calling the validating* methods. But that might very well be enough.

[tohagan]

Roger that. Thanks Marten.