tiesselune / reindeer-rs

A thin entity layer around Sled using Bincode for serialization
MIT License
12 stars 2 forks source link

Ability to add `pre_remove_hook` in the derive macro #16

Open dzervas opened 5 months ago

dzervas commented 5 months ago

I'd like to add a hook that checks the expiry of the rest of the records (I have a chrono field that describes expiry) and deletes them. Essentially it's cleanup during every delete - since I'm rarely deleting stuff it makes sense

tiesselune commented 5 months ago

Hum, that would make sense, what do you have in mind for the derive syntax ? Providing the name of a free function that would take an instance of the entity as an argument?

tiesselune commented 5 months ago

(by the way be careful not to create infinite recursions, since pre_remove_hook will likely be called on other related records before deleting them which could in turn call it again on the source record, and so on)

dzervas commented 5 months ago

oh damn I forgot about infinite recursion!

I'd expect something in the sense of pre_remove_hook = "my_awesome_remove_hook", like serde does

btw why do you use use_pre_remove_hook and not inline the pre_remove_hook { Ok(()) }? I expect it to get completely optimized away

tiesselune commented 5 months ago

Hmm, yes you're probably right. It's been a while, the use_pre_remove_hook might indeed not be useful at all. The pre-remove hook's default implementation is already what you suggested, btw.

For the macro itself, I'm moving houses at the moment and have a ton of way-overdue work so I hope you're not in a hurry; in the meantime you can implement the Entity trait the hard way, it should work!

dzervas commented 5 months ago

I'm not in a hurry at all - my project is "finished" in terms of features, I'm just discussing what would be nice. I like it very much when people interact with my projects (issues/ideas/etc.) so it's some kind of a weird "thank you" :P

take your time and I wish you the best