Changes to property change and initialization

[Bryan Matthews]

I recently made some changes to low-level property behavior that could potentially be a breaking change for you (but probably not).

• Property.prototype._setter, Property.prototype._getter, and Property.prototype.init have been removed. These functions should not have been called directly outside of the framework, so if you were calling them you'll need to evaluate the reason for doing so.
• Removed obsolete isInited and wasInited event argument properties in property get and change events.
• Property init is now only called in two situations: 1) when initializing data for existing instances, and 2) when lazily assigning a default value for properties when accessed or setting the property value.  All other scenarios where a value is being assigned are seen as a true property set.
• Throw an error if a setter is called on a ghosted instance or if a non-loaded array property is modified by client code or rules.

Now, for some background...

From the beginning ExoWeb has had a concept of property-level initialization. If you're ever written a client-side calculation and specified "init of" as one of the basedOn paths, then you've taken advantage of property-level initialization (this would have theoretically allowed for property-level loading if we chose to go down that path). In the majority of cases this behavior could be substituted for a type-level init event. For example, if you're calculating an initial value for a client-origin property based on "init of" a server-origin property, then you could just as easily calculate the value when the instance was fully initialized. In fact in some cases it may be more efficient to do so. Also, the concept of property init developed into a strange and confusion combination of "the property is assigned an initial value" and "the instance is being loaded". It isn't easy to reason about whether "change" or "init" events are appropriate for various combinations of new or existing instances, client- or server-origin properties, and list or non-list property types.  ...and if "init" is taken to be synonymous with loading, then "init of this.Person" is somewhat ambiguous.  Ok, so that's a bit of a stretch, but the point is that the concept tends to cause confusion. With the client-side rules refactor that Jamie recently completed, the client-side rules API now more closely matches the server-side rules API.  This means that you write rules to respond to property change, instance init (new and/or existing), and a property rule can also be run when accessed (i.e. property get rules).  This required doing away with the property-level init concept (at least in terms of the public API) and instead using type-level events.  Hopefully this makes things easier and more consistent.

Also as a result, "Property.addChanged" and "Property.addGet" event handlers no longer include the concept of "wasInited" and "isInited", respectively. This is because, in addition to the fact that the property-level init concept no longer exists (in its original form), properties were always inited when a change event handler is fired, and they are always inited when the get event is fired. From what I could tell the "wasInited" flag was only ever used to ensure that change event logic only responded to an actual property change. I apologize wholeheartedly for that headache.

Since init is only ever called when loading data from the server or when assigning a default initial value, this means that you can be certain that calling set_Property (or Property.value, or Entity.set) will always raise property-level change events and it will always result in recording a change event in the change log...or an error.

Which brings us to the last point. As a part of Jamie's rule refactor he changed property getters to raise an error if you access the property for a ghosted (non-loaded) instance. This tended to result in hard-to-track-down errors, so we thought it was best to catch it as early as possible. Along those same lines, setting a property or modifying a list that is not loaded now results in an error as well.

Let me know if you see any problems with these changes or have any questions.

[Matt Hooper]

Hi Bryan,

 

Thanks for the explanation. 

 

We ran into one issue today related to accessing unloaded objects and the commit process.  In this scenario we have a screen that is adding items to a list.  The list however might be quite large so the UI was simply appending items to the list rather than loading the entire list.

 

The code looks something like this:

 

Add button handler:

                var c = new Child();

uiList.add(c);   // UI is bound to this list

parent.get_Children().add(c);  // must add to the model list for the save operation

 

Save button handler:

context.server.save(parent);

 

Now get_Children() fails b/c it has not yet been loaded.  Ideally we would not load the list for performance reasons.

 

Is the fix as simple as passing the uiList to save() and removing the call to get_Children()?  Other ideas?

 

Thanks,

Matt

[Bryan Matthews]

I suspected that might happen. It seemed that in most cases when the client is modifying a list it will ensure that the list is loaded. In general the client modifying a list that it doesn't actually know the value of is a bad idea. But, I think this is the one valid exception (that I can think of at least). In this case the client is adding a new instance to the list, which is the one operation that you can be fairly certain will be valid. I propose allowing the list to be modified if only adding or removing (e.g. if added previously) new instances. Thoughts?

[Bryan Matthews]

Sorry, I just realized that you were actually pointing out that the property access caused the error. This can be circumvented by accessing the value through some mechanism other than the generated get_Children function. This is allowed so that you can do this sort of thing so long as you're considered the implications. Your choices are: 1) Type.$Property.value(obj), or 2) obj.get("Property").  I prefer the first option because it is inverted and looks the least like the standard property getter, in other words it sticks out like a sore thumb.

I'll also add the check for new instances since that would have been an issue down-the-line.

[Matt Hooper]

The special casing of allowing new objects to be added/remove the model list seems a little messy to me but maybe that is the best way at the moment.  Also, on save() the large list would need to be loaded on the server due to the list add events wouldn’t it?  And, we’d need to account for objects being added, the model saved, and then the object being removed without the page refreshing.

 

Wouldn’t the application code be cleaner and clearer if you could save the non-model list (uiList in the example)?  In that case we could remove the get_Children() property access all together.  I’m not sure how hard it is to pull that off however.  Exoweb would need to be able to start a database transaction, save each object, then commit the transaction.  Are those concepts portable across ORMs?  I know we’ve considered removing the argument to save() even.

[Bryan Matthews]

Yes, it would have to be loaded server-side. If its so large that loading the list server-side is a concern then maybe an alternative commit approach is warranted.  I don't know about getting down to the transaction level, but batch saving certainly seems possible. This could be accomplished with a custom event for example.

I agree that checking for new objects is a bit messy. These checks only exist to protect us from ourselves, and non-loaded lists shouldn't be modified in most cases or very often, so checking for an edge case that is valid (supposing that it is) seems like an ok compromise to me.  I'm not married to the idea though. Alternatively, similar to how you can access a property in a roundabout way to bypass property access errors, you could explicitly state that you're going to modify a non-loaded list and suppress the error. I'm not sure that it's any better, just different, except that it does have the advantage that it is explicit. Really the ideal implementation would be that any array member that could modify the array and raise observable events would throw an error. Consider, for example, the case where someone attempts to remove an item from the list, but it doesn't actually remove the item because the list is not loaded and so appears to be a no-op. Also, a developer would theoretically be able to directly call the array manipulation method in question in a less convenient way, bypassing the check like in the other scenarios. I have my doubts about how feasible that will be though.

Can you explain the part about the item being removed without the page refreshing?

[Matt Hooper]

Thanks Bryan.  Regarding the removal statement – what I mean is that the objects would no longer be new after they were background saved. So, if the newly saved object was subsequently removed from the list prior to the page being reloaded, we’d have to realize that it used to be new if it were to be removed without failing validation.