[Timestampable/Blameable] [RFC] Overhaul of "Trackable" behaviors (Timestampable/Blameable) + inclusion of IpTraceable

[lemoinem]

Hello,

This issue/RFC will include several discussion, but I think it's easier to start with the big picture and break down the discussion if necessary.

I am currently moving away from Stof/Gedmo's Doctrine-Extensions since the bundle doesn't seem to be maintained anymore, and I noticed a few things:

• Timestampable and Blameable are not consistent with each other (Timestampable calls a method in the entity while Blameable relies on a completely external listener)
• They aren't as flexible as in Gedmo (for example the on="change" behavior)
• IpTraceable does not exists in Behaviors (yet?), It's in Gedmo, although the support hasn't been included in Stof's Bundle yet (Adds ip-traceable listener stof/StofDoctrineExtensionsBundle#233)

I would like to propose a complete overhaul (separated in a few PRs) solving these issues. However, before starting to work on it, I would like to know if you are interested in such PRs and, if yes, there are a few design choices I would like to discuss.

Consistency

I propose All "Trackable" behaviors use a single Listener.
The Listener would be provided a set of Callables, each providing an associative array of meta-data.
On prePersist or preUpdate events, the sets of all meta-data would be merged together and injected in a new (custom) LifecycleEvent and given to a method of the behavior Trait. This method would be responsible for actually updating the Entity accordingly.

I believe this scenario to fit the philosophy of other behaviors available in the bundle and to be easily extensible if needed.

Flexibility

Since the whole Behaviors system is based on traits and Annotation would be a major disruption, we need to achieve the flexibility in another way.

Granted the consistency mechanism is accepted, I propose the Trackable Trait provides a method updateConditions which would return a list of tupples: [tag, event, field, value]. The event, field and value are based on Gedmo's system. So event could be either of create, update or change (Maybe update and change could be merged?), field would be a (set of) field(s) to watch and value a value to wait the given field to change to.

The list of conditions would be evaluated by the Listener and appropriate tags would be added to the event meta-data.
The default implementation for each behavior trait would simply provide the default [["create", "create"], ["update", "update"]] (These two cases could be abbreviated to [["create"], ["update"]] or even ["create", "update"].

IP Traceable/Extensivity

Given the consistency proposition is accepted, adding the IP Traceable behavior would be easy enough and rather straight-forward. Same goes for anything else anybody would like to trace (Maybe providing a Symfony Service Tag to annotate callable service could provide easy extension mechanism?)

Conclusion

🏁 Congratulation for making it this far and thanks for your time and attention...

If your are interested by this kind of contributions, I would start working on the PRs as soon as possible. The implementation details of each would be discussed in separated PRs. However, if you have comments on the over all design choices, I would like them to be discussed here.

[docteurklein]

nice proposal of you :-)

my first thoughts:

agreed on current inconsistensy between the different impls. i'd like to see them to "listener only" approach, unless SRP is maintained.

i'm not for copying what gedmo does well already, unless we do it better/differently. but this doesn't mean i don't want to see those kind of features landing :-)
a larger feature set is just harder to maintain.

annotations have been discussed, but i wonder if a pure listener based config wouldn't do the trick. it would fix the SRP problem (define methods in the domain model that makes sense just for an external listener is bad).

now i'd be pleased to see those features land, but i wonder if a refactor to try to apply the ideas above would worth it before.

thanks and aorry for the delay!

[lemoinem]

Hello,

Sorry for my late answer.

Thank you for your feedback.

I will start working on the Consistency part and come back in some time with at least a first PR. I'm doing my best to squeeze it between two projects ;)

On the side, I will keep thinking about the flexibility part, see if there is an easy answer (easy meaning with low implementation and maintenance cost). I completely understand your point about not trying to copy Gedmo over and end up with high maintenance cost because of it.

[TomasVotruba]

I'm going through the old issues/PRs and closing most of them so we can focus on now.
This one is missing any feedback or reasoning so I cannot deduce anything from it.

Thanks for you though, but closing for reasons above.

If the bug still remains, please send failing test case to verify it. Only with test we'll be albe to help.