Pull out sync related methods and move them to separate repository

[TimBeyer]

We are planning to use ampersand-model on the server, with the requirement that different users need different authentication tokens added to the headers without using some magic to propagate them down to all models that need to know how to fetch themselves.

To solve this problem, the idea was to remove a model's ability to fetch itself, and use a different component that knows everything about how to fetch models.
The idea is similar to this:

var syncManager = require('sync-manager')({
    authToken: 1243
});
var Model = require('our-model');

var m = new Model({
    id: 123,
    urlRoot: '/foo'
});

var fetched = syncManager.fetch(m);
var changed = fetched.then(function (model) {
    model.set('foo', 'bar');
    return model;
});
var saved = changed.then(syncManager.save);

Our model implementation will not contain any of the save, fetch and destroy methods.
That way we enforce that updates need to go through the syncManager and can slowly work our way towards single-directional data flow. We also don't need to propagate user data down into randomly required models and collections, something quite painful to do in event-loop based async programming on node. We currently use https://github.com/othiym23/node-continuation-local-storage instead to magically create something similar to thread locals which allow us to basically treat the auth data as globals over the lifetime of the request.

Of course we do not want to copy-paste all the code from ampersand-model over and create maintainence and compatibility overhead. At first I wanted to delegate through to ampersand-model's prototype method, but there's one annoying problem: sync aka ampersand-sync is accessed from the closure scope in ampersand-model, making it necessary to create a proxy object passing through all this.XXX and model.XXX data so we can have our models be clean of the sync related methods.

I would propose instead to turn the code into a mixin factory which you can then put on the prototype of Model and we can use in our sync-manager.

// ampersand-model.js
var syncMethodFactory = require('ampersand-sync-methods');
var sync = require('ampersand-sync');

...

_.extend(Model.prototype, syncMethodFactory(sync));

// sync-manager.js
var syncMethodFactory = require('ampersand-sync-methods');
var sync = require('our-isomorphic-sync');

var syncManager = function (options) {
    var customizedSync = sync({
        authToken: options.authToken
    });

    var syncMethods = syncMethodFactory(customizedSync);

    return {
        save: function (model, key, val, options) {
            return syncMethods.save.call(model, key, val, options);
        }
    };
};

This way it's very easy to create customized model classes that still rely on the sync code maintained by the project, and also allowing developers to replace sync inside of ampersand-model without having to rely on monkey patching the prototype.

[claudioc]

👍

[dazld]

We also don't need to propagate user data down into randomly required models and collections,
something quite painful to do in event-loop based async programming on node.

This is the really killer requirement!

[latentflip]

Hey @TimBeyer! There's a lot here, that I'm trying to get my head around, so bear with me if I'm misunderstanding you.

sync aka ampersand-sync is accessed from the closure scope in ampersand-model

This is true, however, it's only ever accessed by the sync method, so if you want to not use ampersand-sync, and instead use something else, there's no reason you can't do:

var BaseModel = AmpersandModel.extend({
    sync: function(method, model, options) {
      //do whatever you want in here
    }
});

If that's not enough, and you really don't want any of the sync stuff from ampersand-model, have you seen ampersand-state](https://github.com/AmpersandJS/ampersand-state)? It's basically ampersand-model but without any of the restful syncing stuff - which sounds like what you want, I think?

As to your original problem of how to handle user-specific tokens, we solve this at &yet by using the ajaxConfig options, and setting a base model up like this, as per ajaxConfig docs.

//some global-ish object to store your accessToken in:
app.me.accessToken = "{wherever you got the access token}";

var BaseModel = AmpersandModel.extend({
    ajaxConfig: function () {
        return { 
            headers: {
                "Authorization": app.me.accessToken
            }
        }
    }
});

But it's quite possible I've misunderstood what you're trying to do?

[TimBeyer]

So, let me try to explain:

I wanted to do exactly what you proposed:
use the raw ampersand-state, add a url method to it and then use external sync methods.

To re-use your code for fetch, save and destroy, my plan was to just do

var Model = require('ampersand-model');
var fetch = Model.prototype.fetch;

var fetchState = function (state) {
    return fetch.call(state);
};

But then it also needs this.sync, so I cannot just call it on state alone.
I'd need an object that delegates to the state methods and properties (state.set, state.parse, state.trigger etc etc) for everything except sync, which needs to be overridden.

Basically what's really in the way is Model.prototype.sync, which directly delegates to sync - and looking back at my PR, I am actually no longer sure it would solve this problem. I'll close the PR.

The only solution I currently see is to not look for sync on the prototype, but to call it based on the dependency passed in, similar to what the PR suggested.

Regarding using ajaxConfig on a BaseModel:
This does not work on the server, unless you create new classes for every request, because many requests come in and are being worked on asynchronously, and they all need different authentication tokens for API requests. If you have a base class, everyone would share the same tokens.
If you create a new class for every request you have a new problem: If some other module wants to import this class and extend it, it cannot easily do that. It needs to wait for the 'personalized' class to be available, then export the subclass based on that.

All of this is a lot of overhead, and continuation-local-storage, while being very elegant, kind of is a hack. The most solid and non-hacky, non-magicy way seems to be to just have a single component created per request that takes care of making all http calls for the models, and not allowing models to be instantiated just anywhere in the application, but only at the central sync manager.

[latentflip]

Regarding using ajaxConfig on a BaseModel:

Oh, sorry, I missed the point about being on the server.

Basically what's really in the way is Model.prototype.sync, which directly delegates to sync - and looking back at my PR, I am actually no longer sure it would solve this problem.

In that case can't you do something like:

var sync = require('ampersand-sync');
var AmpersandModel = require('ampersand-model');

var BaseModel = AmpersandModel.extend({
    sync: function (method, model, options) {
        //do whatever you need to
        //...
        // call original sync
        sync(/*args*/)
    }
});

Again, I'm sure I'm still not quite following you correctly, sorry! :)

[TimBeyer]

The thing is that I do not want a sync method on the model, I want to basically use ampersand-state only, in addition to the url method. And I want to reuse the fetch, save and destroy methods from ampersand-model, but instead of having them on the instances / their prototype, I want an external component that calls them like syncMethods.fetch.apply(state, arguments), because I really don't want developers to even think about being able to directly save, fetch or delete a model.

The way it currently works, I end up with some monstrosity like this:

var Promise = require('bluebird');
var Model = require('ampersand-model');

var sync = function (method, model, options) {
    // Imagine this actually does something
    return Promise.resolve();
};

module.exports = function dataManager(options) {

    var getSyncDelegator = function (sync, state) {
        var syncMethod = function () {
            return sync.apply(state, arguments);
        };

        var deleteSync = function () {
            delete state.sync;
            return state;
        };

        var syncable = {
            save: function (key, val, options) {
                state.sync = syncMethod;
                return Model.prototype.save.call(state, key, val, options).then(deleteSync);
            },
            fetch: function (options) {
                state.sync = syncMethod;
                return Model.prototype.fetch.call(state, options).then(deleteSync);
            },
            destroy: function (options) {
                state.sync = syncMethod;
                return Model.prototype.destroy.call(state, options).then(deleteSync);
            }
        };

        return syncable;
    };

    return {
        save: function (state, key, val, options) {
            var syncable = getSyncDelegator(sync, state);
            return syncable.save(key, val, options);
        },
        fetch: function (state, options) {
            var syncable = getSyncDelegator(sync, state);
            return syncable.fetch(options);
        },
        destroy: function (state, options) {
            var syncable = getSyncDelegator(sync, state);
            return syncable.destroy(options);
        }
    };
};

Not relying on Model.prototype.sync would turn this code into just this:

var Promise = require('bluebird');
var syncMethodsFactory = require('ampersand-sync-methods');

var sync = function (method, model, options) {
    // Imagine this actually does something
    return Promise.resolve();
};

module.exports = function dataManager(options) {

    var syncMethods = syncMethodsFactory(sync);

    return {
        save: function (state, key, val, options) {
            return syncMethods.save.call(state, key, val, options);
        },
        fetch: function (state, options) {
            return syncMethods.fetch.call(state, options);
        },
        destroy: function (state, options) {
            return syncMethods.destroy.call(state, options);
        }
    }
}