-
Notifications
You must be signed in to change notification settings - Fork 398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Propel <behavior name="versionable" /> doesn't preserve foreign key relationsships #1983
Comments
Hey @kasparsatke , |
Dear Denis, thanks for picking up on this. Below I provide a shortened (and slightly altered) table scheme.
As you can see, the table is versionable and contains multiple foreign keys. The "versionable" behaviour works like this: Every time I update an object from the original table with different values, a copy of the old object with incremented version counter will be created in the table Now when I call an object of the main table, like e.g.
I can also directly call a related Object and get columns of the related object e.g.
Just normal stuff Propel allows with foreign keys. Now once I try to retrieve on a previous version of the original object like so
this works fine. However, once I try to get related objects of foreign tables like
it fails with an unknown method thrown as the foreign key on the be_user table is not preserved by Propel in the versioned table (invoices_version). Instead I have to go back and call the related table separatedly:
Checking the underlying DB tables, I can confirm that foreign keys are added by propel on the original table, yet not on the versions table. It be would really great if I could call the related objects of table object versions out of the box. To my understanding, @mringler has recently realized this feature on the "archivable" behaviour with two parameters implementing foreign keys on archived table objects. Unfortunately I could not test it yet and hence don't know if this works the way I understand it, since until recently I had to use an older Propel version where the enhancements to the "archivable" behaviour had not been not included yet. I was hoping that (if functioning the way I understand it here) the same or similar parameters as in "archivable" could be implemented for "versionable". |
Hey @kasparsatke, |
Dear Moritz, just wow. This sounds super promising. Also your new base behaviour synced_table has great configuration options.
Are your features All the best, |
Hey Kaspar, thank you for the kind feedback! Yes, I needed it for that auditable behavior. It records changes to the parent table, so you can figure out who did what when. It seems to work well, but there is a huge caveat: I wanted to avoid duplicating every single row, so now each change only records the overridden values (instead of the new values), and the actual audit has to be rebuilt going backwards from the current row. That's fun and interesting, but it is fragile to changes from outside Propel (i.e. direct database access). For a general release, it would probably need a default fool-proof mechanism as well. But since nothing gets merged into Propel anymore, that does not matter, there is no point in creating PRs. Synced table and the changes to versionable are finished, but if you want to use it, you have to get it into your own fork, or copy the files and register them as external behavior. I have completely switched to my own fork, you are welcome to use that too of course (I don't plan on doing something crazy with it). I'd be curious if the changes to versionable work for you though. Let me know if you try it out or if there is something else I can do. Best, |
Hi Moritz, as for the direct database access - I would say there is no bulletproof method. One way to minimize potential impact would be to make the table WORM for a certain (audit) user. A drawback is that you would at least need root access again once the table definition changes I guess. Or, one could prevent changes with triggers on that audit table that prevent UPDATE, DELETE or REPLACE statements to be executed. But then again someone with root access could also tamper/delete these triggers again- regardless if in Propel or directly on the DB. Or, at least in MySQL/MariaDB, as I have just found out, there is a special ARCHIVE storage engine which could is almost WORM (besides the allowed REPLACE statement). But then again you would need to handle a second DB with this engine as you don't want have WORM on all your tables in the main DB. I don't know how easy it would be in Propel to adjust your auditable behaviour to offer the option to automatically copy/redirect changes into the special archive DB. So in general I would say there is alway a risk yet I would still say your auditable behaviour would also be a great addition to Propel! Which brings me to the following question as to why you say nothing gets merged anymore into Propel. I thought @dereuromark has been the previous maintainer and could not be so anymore and now @PhilinTv is the current maintainer. So I thought although progress in the Propel repo is slow, there is something happening. Or do you think otherwise? If indeed no merge and release is happening anymore, I have to think of a way to test your additions with real data in a new test environment. I am not sure if/how fast I can do this, so please don't expect feedback soon. Best, |
Hey Kaspar, oh,protecting the table on DB-level is an interesting idea. But as you say, it seems unfeasible to forbid manual updates completely. The problem with the current solution is that manual updates are not just unaccounted, they give you wrong audits (I have added some description to the PR, it felt out of place here). Not sure if it makes sense to be "published" the way it is, and figuring out how to get there does not seem to make sense at the moment. As to the state of the repo, I had several PRs, from bugfix to feature, doesn't seem like anybody looked at them even once. #1981 is still open, it's a simple bugfix, nine month old, not even a comment. Go figure... No worry about the testing, I just remembered your comment and thought I'd let you know. Best, |
Oh I am sure people are looking, watching and even reading. Maybe even many. Propel is a great project! It just needs people to engage to keep things rolling. So to all you people reading this, engage! Even if you feel you can’t contribute with code reviews or whatnot, reach out to people you know who can. And for whoever is maintaining this repo, think of how responsibility can be shared with multiple capable and trustworthy people who ideally have knowledge of Propel‘s inner workings. Increase the Truck number! There is no such sad thing as a great project depending on a lonely maintainer that has not enough time, means or strength and gets overwhelmed with responsibilities. Recent examples have bitterly shown what might happen, if a lonely maintainer gets swamped: XZ anyone? Saying this, come up with people you trust and that have the right knowledge to easen the burden. Keep up the great work yourself && let trustworthy people help you! Together you are strong! Together the community is strong! 💪 |
I am working on a table that has multiple foreign keys. This table is versionable.
Once I tried to call foreign keys on a version of the table, I got an error like "unknown method".
I then checked the DB and found out that in the versions table foreign keys are not preserved.
I looked a bit around the Propel documentation and found out that the behavior "archivable" does have the option to preserve foreign keys - if I do understand it the right way. The behavior "archivable" has parameters like "inherit_foreign_key_relations" and "inherit_foreign_key_constraints" and it sounds like they would solve my issue. Unfortunately, I did not find any documentation if those parameters are also available for the behaviour "versionable". Quickly looking through the source code I also did not find any evidence.
Saying this, I would like to file a feature request/bug report to enable calling foreign key objects on versioned tables.
The text was updated successfully, but these errors were encountered: