meteor-collection2 icon indicating copy to clipboard operation
meteor-collection2 copied to clipboard

Add latency compensation for autoValue fields

Open mjgallag opened this issue 10 years ago • 19 comments

When I call collection2's overridden version of collection's insert it's inserting a copy of the object into client side collection without the autoValue calculated. Once the insert runs on the server, where autoValues are calculated, the client side collection is patched with the autoValue from the server as expected. I think it makes more sense to run it in both places so as to avoid a delay that is visible to the user. Thoughts @aldeed?

mjgallag avatar Feb 26 '14 20:02 mjgallag

I don't think it's possible. The problem is that if we add auto values on the client, then they're in the doc that's sent to the server. Then, when we rerun on the server, we can't know whether a value was entered by the original user or by the client-side autovalue function, so we can't possibly evaluate based on this.value, this.isSet, etc.

If anyone has ideas about how we could work around this, chime in.

aldeed avatar Feb 27 '14 00:02 aldeed

If we want latency compensation for autoValue fields, we need to run both server and client computation in parallel, ie don't wait for the autoValue fields on the client before sending the document to the server. The normal way of doing this in meteor is by using methods.

So maybe we could overwrite the internals Meteor.Collection methods do to this computation? Or create a "proxy" method that only add autofields and then call the normal internal method?

mquandalle avatar Feb 27 '14 01:02 mquandalle

Both of my uses of autoValue don't require user override. What is a use case for an autoValue that can be overridden by the user? If you wanted a field to be overrideable by the user would not defaultValue cover most of those use cases. I think a user overrideable autoValue may be an edge case. Am I missing a common use case here?

mjgallag avatar Feb 27 '14 02:02 mjgallag

@mjgallag, now that there is defaultValue, there is less need, but there are still cases where you need to calculate the default value based on the values of other fields, which requires using autoValue.

@mquandalle, it's true that we could overwrite pieces of collection internals to accomplish this, but it might not be easy. There's actually a lot of code related to this, and the more we mess with the private API, the more we have to carefully test things and potentially rewrite with each new Meteor release. I would definitely look at a PR because it would be nice to have the compensation, but I don't see myself having time to tackle it anytime soon.

Just thinking as I'm typing... maybe it's possible to do an autovalues update against the LocalCollection only, immediately after the real insert/update?

aldeed avatar Feb 27 '14 04:02 aldeed

Ah, I haven't looked closely at defaultValue yet, didn't realize it had that limitation.

What about a configurable option of some sort then to distinguish user overridable autoValues?

I wonder if that update against the local collection wouldn't create additional noise because it will still kinda flash undefined or null after the insert and before the local update.

mjgallag avatar Feb 27 '14 04:02 mjgallag

FYI, for now I've worked around this issue by calling clean before inserts & updates on the client so the autoValue is present before being inserted into the client collection, that way I get my latency compensation.

mjgallag avatar Feb 27 '14 19:02 mjgallag

IIRC the current behavior (autoValue is computed on both but committed only on server) was also suggested as workaround for several use-cases, where things are to be done only by server code.

For example, for having a "commited on server" field that shows the user when his changes have successfully made it to the server (and back).

So, this issue depends on a solution for on isServer, changeByServerCode, ... conditions?

testbird avatar Mar 07 '14 11:03 testbird

@testbird, even if we manage to add latency compensation, you could still get "commited on server" flagging by returning your autovalue only when Meteor.isServer, else call this.unset().

I do plan to add this.isTrusted to autovalue, but that is for slightly different purposes, when you want to allow the server to insert whatever it wants but force certain values when coming from untrusted code.

aldeed avatar Mar 07 '14 20:03 aldeed

I could be missing something here but wouldn't this.isTrusted === Meteor.isServer? If so, what's the point?

mjgallag avatar Mar 07 '14 22:03 mjgallag

No they're not equal because inserts and updates from untrusted code are re-cleaned and re-validated on the server. So when Meteor.isClient, then this.isTrusted is always false, but when Meteor.isServer, then this.isTrusted will be true when the call to insert or update happened on the server but will be false when the call happened on the client.

It's similar to being in an allow or deny function. They run on the server, but only for untrusted actions.

aldeed avatar Mar 07 '14 22:03 aldeed

Thanks for clarifying!

Maybe this.fromTrustedCode could be more self-explaining?

testbird avatar Mar 08 '14 08:03 testbird

this.isFromTrustedCode is added in the latest release. There were also a couple issues with setting userId and isUpdate properly, which I added tests for and are now fixed.

aldeed avatar Mar 17 '14 00:03 aldeed

#66 is related to this issue. Both will require overwriting or proxying the core mutation methods for insert and update.

aldeed avatar Mar 25 '14 15:03 aldeed

Is there any update ?

I just had a case where my autoValue was missing on client and it throw an error because I needed them.

darkship avatar Jan 07 '15 08:01 darkship

No update. This isn't very high on my list since it's a complete package rewrite. It is more feasible now that Meteor has a stable 1.0 API, though.

aldeed avatar Jan 07 '15 15:01 aldeed

Coming into Meteor brand new and have to ask if the moral of this story is simply "don't use autovalues" or "autovalues do not have latency compensation". Are either of these accurate currently? This concerns me a little in making a determination to use Meteor based on the "too much magic" critique v.s. alternatives and which to teach my students (v.s. MEAN-ish stack for example). Thanks for any feedback.

rwxrob avatar Jan 26 '15 13:01 rwxrob

You can use them if you use them correctly, but no, you won't have latency compensation. In a lot of cases, latency compensation isn't necessary. If you need it in one or two cases for the user experience, then you can always set the auto value yourself in some client code that does have latency compensation.

aldeed avatar Jan 26 '15 18:01 aldeed

autovalues only being generated on the server is, by definition, a problem for me when running an app I need to work in offline mode (using ground:db)

My workaround

I call my schema's autoValue method manually on the client. In my autoValue definitions I have to watch out to not use the Collection2 API like this.isInsert as those fields won't be populated when I call it manually.

At least this way I get a single definition for autoValue, keep the code in my model, and a more secure server side verification of a correct autoValue (I'm using it to set owner fields). It's also a nice bonus that I'm getting latency compensation free.

AnthonyAstige avatar Sep 17 '15 18:09 AnthonyAstige

@AnthonyAstige I'm using https://github.com/matb33/meteor-collection-hooks, to do it.

darkship avatar Oct 08 '15 07:10 darkship