HasMany and ComposeMany not updated when items added #14

Closed
a-musing-moose opened this Issue May 23, 2011 · 4 comments

Comments

Projects
None yet
2 participants
@a-musing-moose
Owner

a-musing-moose commented May 23, 2011

When doing the following:

<?php
$o = \morph\Query::instance()->fetchById(new SomeObject, $anId);
$o->aHasManyField[] = new SomethingElse();
$o->save();

The new SomethingElse is not persisted.

The magic __get function used to serve up morph object properties is the only function called. It returns an instance of an object with array access. This array object is then updated but __set is not call on the parent morph object. As a result the morph object still has a clean (unchanged) status and is not checked for changes on save().

@mgrandi

This comment has been minimized.

Show comment Hide comment
@mgrandi

mgrandi Jun 1, 2011

Contributor

I managed to get around this for ComposeMany by creating a new array, putting all of the old entries in the old composemany array and then saying aComposeManyField = $newArray, and since you are changing the entire property, then it gets its state set to dirty and it works fine. it doesn't seem to be working for has many however

Contributor

mgrandi commented Jun 1, 2011

I managed to get around this for ComposeMany by creating a new array, putting all of the old entries in the old composemany array and then saying aComposeManyField = $newArray, and since you are changing the entire property, then it gets its state set to dirty and it works fine. it doesn't seem to be working for has many however

@a-musing-moose

This comment has been minimized.

Show comment Hide comment
@a-musing-moose

a-musing-moose Jun 2, 2011

Owner

Hey Mark,

Yep that is a one way around it. The actual issue is around how Morph keeps
track of the status of an object.

Whenever a property is set on an object the __set() function is called.
Part of this function is to set the state to DIRTY (unless is is already
NEW). When it comes to storing the object in Mongo. It first checks the
status. If the status is NEW it does an insert, DIRTY results in an update
and nothing is done if the status is CLEAN.

When dealing with Has/Compose Many type fields what is actually happening
when you do something like:

$myObject->aHasManyField[] = new Something();

is that the __get() function is called on $myObject to retrieve the HasMany
Property (which acts like an array). The new Something() is then appended
to that property. e.g. __set is never called and therefore status remains
CLEAN.

The complete solution to this I think is to change how status is determined.
Instead of having it as a private property of morh\Object I think that is
will be maintained in each property. When status() is called on the Object
then it will simply go through the various properties object to determine
the status. That way I can easily determine what has changed.

It also paves the way for a process allowing only partial updates of changed
fields rather than having to pass the entire object back to Mongo. Which
hopefully would improve performance by quite a bit.

Regards,
Jon

On 2 June 2011 05:38, mgrandi <
reply@reply.github.com>wrote:

I managed to get around this for ComposeMany by creating a new array,
putting all of the old entries in the old composemany array and then saying
aComposeManyField = $newArray, and since you are changing the entire
property, then it gets its state set to dirty and it works fine. it doesn't
seem to be working for has many however

Reply to this email directly or view it on GitHub:
#14 (comment)

Owner

a-musing-moose commented Jun 2, 2011

Hey Mark,

Yep that is a one way around it. The actual issue is around how Morph keeps
track of the status of an object.

Whenever a property is set on an object the __set() function is called.
Part of this function is to set the state to DIRTY (unless is is already
NEW). When it comes to storing the object in Mongo. It first checks the
status. If the status is NEW it does an insert, DIRTY results in an update
and nothing is done if the status is CLEAN.

When dealing with Has/Compose Many type fields what is actually happening
when you do something like:

$myObject->aHasManyField[] = new Something();

is that the __get() function is called on $myObject to retrieve the HasMany
Property (which acts like an array). The new Something() is then appended
to that property. e.g. __set is never called and therefore status remains
CLEAN.

The complete solution to this I think is to change how status is determined.
Instead of having it as a private property of morh\Object I think that is
will be maintained in each property. When status() is called on the Object
then it will simply go through the various properties object to determine
the status. That way I can easily determine what has changed.

It also paves the way for a process allowing only partial updates of changed
fields rather than having to pass the entire object back to Mongo. Which
hopefully would improve performance by quite a bit.

Regards,
Jon

On 2 June 2011 05:38, mgrandi <
reply@reply.github.com>wrote:

I managed to get around this for ComposeMany by creating a new array,
putting all of the old entries in the old composemany array and then saying
aComposeManyField = $newArray, and since you are changing the entire
property, then it gets its state set to dirty and it works fine. it doesn't
seem to be working for has many however

Reply to this email directly or view it on GitHub:
#14 (comment)

@mgrandi

This comment has been minimized.

Show comment Hide comment
@mgrandi

mgrandi Jun 2, 2011

Contributor

But i feel even if every property (string, has many, compose many, etc) has its own status, you are still going to run into the problem where the code isn't able to detect when something is added to the 'composeMany/hasMany array' because __set isn't called when you use the array syntax. I asked around about this (still kinda new at php) and they mentioned the interface ArrayAccess. HasMany and ComposeMany can just implement ArrayAccess, and then we will have methods that get called whenever we add something to the array (offsetSet) so then we can set the state as dirty. As well as the added bonus of still treating HasMany and ComposeMany as arrays with the array syntax.

I feel what you are talking about is good though and should be implemented , i just feel these are two different issues =P

Contributor

mgrandi commented Jun 2, 2011

But i feel even if every property (string, has many, compose many, etc) has its own status, you are still going to run into the problem where the code isn't able to detect when something is added to the 'composeMany/hasMany array' because __set isn't called when you use the array syntax. I asked around about this (still kinda new at php) and they mentioned the interface ArrayAccess. HasMany and ComposeMany can just implement ArrayAccess, and then we will have methods that get called whenever we add something to the array (offsetSet) so then we can set the state as dirty. As well as the added bonus of still treating HasMany and ComposeMany as arrays with the array syntax.

I feel what you are talking about is good though and should be implemented , i just feel these are two different issues =P

@a-musing-moose

This comment has been minimized.

Show comment Hide comment
@a-musing-moose

a-musing-moose Jun 2, 2011

Owner

Hey Mark.

Within Compose/Has Many type properties the actual values are now held in an instance of \morph\properties\StatefulCollection. Which does pretty much what you are talking about. It extends \morph\Collection (which is itself an ArrayObject) but it knows who it's parent property object is and will let it know whenever an element is appended or an offset set.

implementing the ArrayAccess interface directly within the property class might actually be cleaner but you loose a lot of the interesting functionality that \morph\Collection brings. I shall think on it and may switch to using the interface if it makes sense.

Owner

a-musing-moose commented Jun 2, 2011

Hey Mark.

Within Compose/Has Many type properties the actual values are now held in an instance of \morph\properties\StatefulCollection. Which does pretty much what you are talking about. It extends \morph\Collection (which is itself an ArrayObject) but it knows who it's parent property object is and will let it know whenever an element is appended or an offset set.

implementing the ArrayAccess interface directly within the property class might actually be cleaner but you loose a lot of the interesting functionality that \morph\Collection brings. I shall think on it and may switch to using the interface if it makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment