doctrine-extensions / DoctrineExtensions

Doctrine2 behavioral extensions, Translatable, Sluggable, Tree-NestedSet, Timestampable, Loggable, Sortable
MIT License
4.06k stars 1.27k forks source link

SoftDeletable #128

Closed Moinax closed 12 years ago

Moinax commented 13 years ago

Hi,

Are you thinking about implementing a SoftDeletable extension for Doctrine or aware of someone doing it ?

I'm trying to create one based on the code you did for Timestampable with a PreRemove event handler.

Is it the good way to implement such a feature ? What are your advices/recommandations ?

l3pp4rd commented 13 years ago

Hi, Yes it is a good way for the start. And so far I remember someone asked me about it, but I do not think someone is doing it right now. So it would be nice to have a contribution for it. You should know that the preRemove event will be triggered only in case if object is being removed using the manager, if you use delete query you will need later to implement the query hint, and I think it will also need to skip the softdeleted ones in select statement also. so the same query hint can be updated to add where statement or something. I think @jwage had implemented it for ODM

Garfield-fr commented 13 years ago

+1 for this behavior

ghost commented 13 years ago

+1

comfortablynumb commented 12 years ago

@Moinax Are you still working on this? I started to work on this extension today but let me know if you're so I help you instead.

There're some points that I'd like to discuss here with you guys:

Now this is the easy part. But I'm having some doubts regarding how to add the condition to skip soft-deleted objects at query time. Which would be the best way to implement this in the ORM and ODM? In ORM I could write a custom tree walker to add the condition, but I'd like this extension to be transparent. With this I mean, if I use a "find" method of a repository, or if I create a query of my own, I'd like to receive objects that are not soft-deleted, without the need of modifying manually the query specifying, for example, a query hint. So, if I have an app with some entities, just adding to them the @Gedmo\SoftDeleteable annotation, it would start to work, without having to modify anything else.

I didn't find a way to do this. Maybe there's no one, but if you know how, please let me know (maybe an event fired just before the execution of any query allowing to modify it would help a lot in this case).

In the meantime, the only way I see to implement this is to create a new manager with remove and restore methods, and a custom tree walker to be used in the ORM, to add the condition to skip soft-deleteable objects in a SELECT query, and to soft-delete objects in a DELETE query.

I'd like to hear your comments and advices on this guys.

Thanks.

Moinax commented 12 years ago

@comfortablynumb I didn't have time to implement it for the project I was working on as the deadline was to small. I think you are a few steps further as I was, so don't hesitate to start. Let me know if and when you do it, so I can fork your project and help you on time to time.

I didn't find any event that could help with queries, and the query hint seems to be the best way.

I thought about a wrapper for the query builder and repositories which would add the filter (same as your manager), but I don't really like this solution. It should be transparent for the developer and he should not be forced to use a dedicated manager (like FosUserBundle do with his UserManager), because if each bundle implements his own manager, it will become complex to use and even impossible to work together.

l3pp4rd commented 12 years ago

I personally also do not like managers, its enough to have EntityManager all logic regarding custom functionality should be in repository: Restore, persistAs or whatever. Conserning queries, the best option is hints, though, you cannot add objects in hints, like $entityManager, query cache will not work. In one hand, for select statement it will be very easy to do, etc. adding the condition, but on the other hand, for delete queries, I do not advice you to select entities and update their timestamps, I advice to convert delete query into update query in the hint, in general it is the same, expected result is scalar. In that case all entities in database would be updated instead. Also you would need to iterate through IdentityMap of unitOfWork to update values or remove entities which were in memory.

glen-84 commented 12 years ago

What about an SQLFilter?

https://github.com/asm89/doctrine2/blob/DDC-551/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php#L638

l3pp4rd commented 12 years ago

yes, very interesting @glen-84 nice one

comfortablynumb commented 12 years ago

This seems to solve every issue that I had in mind. I'll take a deeper look at it but it looks like a really nice approach.

l3pp4rd commented 12 years ago

279