Feature/odm

[wh1pp3rz]

Hey Roni,

I created a PR with a bunch of refactored changed, but I realise there are a lot conflicts, before I go about resolving, can you take a look to see if you're in agreement with the general changes?

In summary, here are the major changes I did

1. decoupled orm by renaming anything with entity to doctrine
2. Use doctrine common classes where applicable instead of orm
3. added new config option doctrine_driver which should be either orm or odm
4. Modified the registered services according to the value of doctrine_driver to register the correct doctrine listeners
5. Updated the tests to reflect orm and odm test cases

I await your feedback.

[ronisaha]

1. decoupled orm by renaming anything with entity to doctrine

OK

2. Use doctrine common classes where applicable instead of orm

OK

3. added new config option doctrine_driver which should be either orm or odm
4. Modified the registered services according to the value of doctrine_driver to register the correct doctrine listeners

I guess we do not need to add driver option, what are the difference in orm and odm changes in our case. Could you please try the odm branch with your project how much code you need to change to support it. if only adding doctrine_mongodb.odm.event_subscriber tag do the work. we can add both tag or some more tag if needed. like now I'm trying to test doctrine/phpcr-odm If we can support all document implementation by doctrine, we do not need to implement each driver.

Let me know what you think.

[wh1pp3rz]

I tried a fresh copy of the feature/odm branch on my odm projects, here are my findings

1. adding the tag doctrine_mongodb.odm.event_subscriber to the xiidea.easy_audit.doctrine_subscriber service definition is enough to make it work for both orm and odm
2. The Logger constructor currently accepts in an instance of Doctrine\Bundle\DoctrineBundle\Registry, it needs to be Doctrine\Common\Persistence\ManagerRegistry so it works for both orm and odm
3. Inside the DoctrineObjectEventResolver, there's a call to $this->getUnitOfWork()->getEntityChangeSet($entity), this somehow needs to be aware of whether orm or odm is being used, if odm the call has to $this->getUnitOfWork()->getDocumentChangeSet($entity)

I refactored the method DoctrineObjectEventResolver::getChangeSets to:

protected function getChangeSets($entity)
    {
        if (!$this->isUpdateEvent()) {
            return null;
        }

        $unitOfWork = $this->getUnitOfWork();

        if ($unitOfWork instanceof \Doctrine\ODM\MongoDB\UnitOfWork) {
            return $unitOfWork->getDocumentChangeSet($entity);
        }

        if ($unitOfWork instanceof \Doctrine\ORM\UnitOfWork) {
            return $unitOfWork->getEntityChangeSet($entity);
        }

        return null;
    }

Everything worked fine after that. Those changes are enough to support ODM

[wh1pp3rz]

There's also one other small issue I can't quite figure out. In my odm project I'm using a custom user object (not FosUserBundle), in the audit log database entry, the user column correctly shows the user property set in the config, in this case username. However in the log file the context data shows the entire user object. I tried it on my orm project which uses fosuserbundle and it correctly shows only the username in the log context. I don't know if's because I'm using orm or fosuserbundle why it works for that use case. Do you have any idea why the context would get the entire user object and not just the property?

[ronisaha]

There's also one other small issue I can't quite figure out. In my odm project I'm using a custom user object (not FosUserBundle), in the audit log database entry, the user column correctly shows the user property set in the config, in this case username. However in the log file the context data shows the entire user object. I tried it on my orm project which uses fosuserbundle and it correctly shows only the username in the log context. I don't know if's because I'm using orm or fosuserbundle why it works for that use case. Do you have any idea why the context would get the entire user object and not just the property?

Can you share your ODM project so that i can try it out? The bundle has no hard dependency with fosuserbundle. It will get the accessible property from current security context using PropertyAccessor component. So if your user object has username field, and you have configured the user_property option, this should work. I'm assuming you have not configured the user_property and you do not have __toString function in your User class. fosuserbundle has

[wh1pp3rz]

OK the user_property is what was missing, it all works now and I updated my PR. In my opinion it's good to be merged, I tested with both orm and odm.