coen-hyde / Shanty-Mongo

Shanty Mongo is a mongodb library for the Zend Framework. Its intention is to make working with mongodb documents as natural and as simple as possible. In particular allowing embedded documents to also have custom document classes.
Other
200 stars 52 forks source link

Hooks not firing when parent document is saved #35

Open tholder opened 13 years ago

tholder commented 13 years ago

First of all, fantastic work. Very simple, very natural.

Just noticed one thing, not really sure this is an issue as such, just slightly confusing about the way it works.

If I have classes defined as:

<? class Contact extends Shanty_Mongo_Document { protected static $_requirements = array( 'name' => array('Document:Name', 'Required') );

}

class Name extends Shanty_Mongo_Document {

protected function preSave()
{
    $this->fullName = $this->forename . ' ' . $this->surname;
}

} ?>

When I create a new contact I want it to build the full name using the preSave hook.

Doing this:

<? $c = new Contact(); $c->name->forename = 'Tom'; $c->name->forename = 'Surname'; $c->save(); ?>

That doesn't trigger the hook.

This however does:

<? $c = new Contact(); $c->name->forename = 'Tom'; $c->name->forename = 'Surname'; $c->name->save(); $c->save(); ?>

I might look at forking this to see if it's possible to change it, but I'm not entirely sure this is the wrong behavior.

Also, one question, can you get MongoID of the parent from within a child document?

Thanks Tom

coen-hyde commented 13 years ago

Hi Tom,

This was intentional, but only because i didn't really think it though. Yes it makes sense to fire the hooks on child documents when parent documents are saved.

There is currently a hack way to get the parent id from the child document (in the criteria attribute of the config array attached to sub docs) but IMO the root document id should be stored in a config array under the attribute root_id or something similar.

Cheers, Coen

tholder commented 13 years ago

Cool, I've had a quick look and can't immediately see how I would implementing it cascading. For now I actually don't need this so I will leave it.

Hope you don't mind but I created a couple of Wiki page. One was for querying, took me a while to get the syntax of $or right.

Cheers

coen-hyde commented 13 years ago

I'd say the best way to implement it would be to have a Shanty_Mongo_Document::fireHook method which would take care of recursively calling the hooks.

so something like this:

//fire a hook:
$this->fireHook('preSave');

public function fireHook($hook) {
  foreach ($this->_data as $property => $document) {
    if (!($document instanceof Shanty_Mongo_Document)) continue;

    if (!$this->isReference($document) && !$this->hasRequirement($property, 'AsReference')) {
      $document->fireHook($hook);
    }
  }

  call_user_func(array($this, $hook));
}

Oh and go for you life on the wiki. There are definitely some lacking documentation.

tholder commented 13 years ago

This is very odd!

I implemented what you suggested and it appears to work. However, when I came to write test I used the same method of incrementing a hook counter as you, just put it in to the name document.

The really strange thing is the assert fails, it doesn't seem to keep the value of hookCounter['preSave'];

I've tried to track this down but I'm completely stumped.

The hook definitely seems to fire because as well as a simple die(), I've change the first name property and then read it back from mongo.

You might want to check it out:

https://github.com/simpleweb/Shanty-Mongo/commit/b2d1a265a4ac3684d95ad848c821550f78ef14c1

Is this possibly a bigger issue to do with sub documents?

Thanks, Tom

coen-hyde commented 13 years ago

Hey Tom,

I think you might just have a typo bug.

$this->assertEquals(1, $user->_hookCounter['preInsert']); $this->assertEquals(1, $user->_hookCounter['postInsert']); $this->assertEquals(1, $user->_hookCounter['preUpdate']); $this->assertEquals(1, $user->_hookCounter['postUpdate']); $this->assertEquals(2, $user->_hookCounter['preSave']); $this->assertEquals(2, $user->_hookCounter['postSave']);

All the above look for the _hookCounter array on the user document when in reality it's only on the name document.

On Sun, Oct 2, 2011 at 7:12 AM, Tom Holder reply@reply.github.com wrote:

This is very odd!

I implemented what you suggested and it appears to work. However, when I came to write test I used the same method of incrementing a hook counter as you, just put it in to the name document.

The really strange thing is the assert fails, it doesn't seem to keep the value of hookCounter['preSave'];

I've tried to track this down but I'm completely stumped.

The hook definitely seems to fire because as well as a simple die(), I've change the first name property and then read it back from mongo.

You might want to check it out:

https://github.com/simpleweb/Shanty-Mongo/commit/b2d1a265a4ac3684d95ad848c821550f78ef14c1

Is this possibly a bigger issue to do with sub documents?

Thanks, Tom

Reply to this email directly or view it on GitHub: https://github.com/coen-hyde/Shanty-Mongo/issues/35#issuecomment-2258902

tholder commented 13 years ago

Sorry I left those in because I just copied your test and extended it by adding the hooks to the name document and then added this line below:

$this->assertEquals(1, $user->name->_hookCounter['preSave']);

That should surely work?

I've been having one of those proper wtf moments but it doesn't seem to be behaving as I'd expect, it's always returning zero despite firing.

coen-hyde commented 13 years ago

That's weird dude, it looks like it should work to me. Infact $user->name->_hookCounter['preSave'] should equal 2

tholder commented 13 years ago

Yeah thought so. Could you pull mine and run the tests? All the other tests pass fine for me.