Breeze / breeze.js.labs

OFFICIAL Breeze.js Labs package of extensions and utilities for Breeze.js client apps.
MIT License
43 stars 44 forks source link

Bug on DataService for Azure Mobile Services.js #18

Open akaztp opened 9 years ago

akaztp commented 9 years ago

On the function "_createErrorFromResponse" the before-last line is:

proto._catchNoConnectionError(err);

which gave me undefined error for "proto" in case of bad communications. In fact, I don't find any variable "proto" in scope of this function or its parent. I've changed this to: ctor.prototype._catchNoConnectionError(err);

And now it works fine. This bug is also on the ShaprePoint DataService, although I did not test, since I do not use that. But it seems like the same problem and solution applies.

Regards ztp

wardbell commented 9 years ago

I think I fixed it (albeit in a different way than you suggested). Would you mind confirming for your app by pulling the latest source file and creating the connection error?

When you confirm it, I'll update the bower and npm packages

akaztp commented 9 years ago

Yeap, it's good. thank you. Meanwhile I've got another problem, and maybe it is related to these files. Before I open another issue, there is one thing I need to know. If it confirms my suspictions, you may want to hold on those updates to the packages. When saving changes for an entity (EntityManager.saveChanges()), a PATCH request is sent to the Azure Mobile Services. This request only includes the key and the changed properties of the entity. The question is: in this case, the "OK" response from the server should have values for all properties or can Breeze handle a response with only the key and the changed properties' names and values? thx ztp

wardbell commented 9 years ago

I believe that the adapter was written with that in mind. It's been a long time.

Have you tried the azure mobile services sample (todo-zumo) in the breeze.js.samples repo? It does full CRUD and I'm sure I would have noticed if I had neglected that. Would help me if you can confirm or deny.

Thanks

akaztp commented 9 years ago

I've just run the todo-zumo demo app. It does not show "my" problem because it gets all data from the server right after is saves the changes. Network inspection also shows that the response from the server for the PATCH command only returns the uploaded properties for the entity. I've modified the todo-zumo, thus disabling the "refresh all" after the save and "my" problem now appears exactly. For the local entity, it resets to the defaults all properties that were not saved due to no changes: since it cannot find them on the server response. I don't think it has anything to do with the Azure and abstract adapters.

Is it mandatory (by design of breeze) that I make a GET after a PATCH for some entity? In high concurrent systems, I can see the reason, but I can see many cases where it shouldn't be mandatory.

Should I open an issue on the main breeze repo?

wardbell commented 9 years ago

No. This is a breeze labs concern given that breeze core has no zumo support at all. This is the right place to deal with it.

I don't think of this as a bug. It's a feature question.

Let's think about what would be desirable. Then we (you and I) can create an adapter to meet that need. I know how and even know what I would do. But I'm more interested in what you think.

So lets take the time to hash out a proposal here. Maybe you'd like to start a design doc on Google docs? We can reference it here. Your call.

akaztp commented 9 years ago

Ward, thank you one more time for spending some time on this.

Either me or you are not understanding each other. Or both! :)

I don’t see this as a feature question/request. Since I've progressed a lot in knowing what is going on, I will re-state the problem I’ve found and we will go from there.

The problem happens when the Azure Mobile Services dataservice sends a PATCH request with only the modified changes to an entity AND, afterwards, the client application, by design, does not go GET the entity (with all its properties). The problem arises because the Breeze core does not expect a partial result for an entity. More precisely the code on EntityType._updateTargetFromRaw() and DataProperty.getRawValueFromServer() expect all properties to be on the Raw object or else they will set them to their default values. And their default values will override the cached ones.

I don’t know what the right solution is, mainly because I don’t know the side effects of changing updateTargetFromRaw() so it would check for the presence of each property on the raw data.

One solution is to send all properties in the PATCH (except for the key). I had success, modifying the Azure Mobile Services dataservice, namely on the _createChangeRequest(), line 161:

Another one, I think, would be to add server side code to the Azure Mobile Services to not return the changed values on a PATCH request, or return all of them. But I don’t even know if this is possible to code there.

A little aside from this problem, I would like to see support to the concurrency problem (in your sample solved by the GET after a PATCH) by examining a serial/timestamp property and comparing it to the cached version. But surely there would be a lot more to say about this.

Regards ztp

wardbell commented 9 years ago

I'm looking for what you would like to do, not the solution. Ideally you wouldn't change a thing on the server nor do any special gymnastics on the client.

I realize that updateRaw... etc are unsuitable for blending a few values from the server with unchanged values on the client. That's where I ride in on my white horse.so don't worry about how. Just think about what.

Your comment on optimistic concurrency is along those lines. Do you have a property for that? Shouldn't we use the etag? Or does that not actual work in zumo? That's the stuff I need help with.

Cheers, W

akaztp commented 9 years ago

My current project is not that much OCC prone, so I just wanted the zumo dataservice to work as (I think it is) expected. But one of the goals of my project is to make me get experience in SPAs and make a kind-of proof of capabilities for my freelance developer venture. In fact, there is one aspect of my application that would be nice to support updating of some entities from different clients at the same time. I think now I understand where you want to get at and I’m pleased to be able to make part of it.

The Azure Mobile Services implements OCC with the ETag, as this article explains: http://blogs.msdn.com/b/carlosfigueira/archive/2013/12/02/accessing-optimistic-concurrency-features-in-azure-mobile-services-client-sdks.aspx In summary, provide in the querystring “systemproperties=version” and the ETag mechanism will be activated.

What would be neat is breeze to interface to that accordingly:

I’m aware that Breeze:

It would be desirable to:

Is this what you were thinking? Regards ztp