From: Vsevolod (S. I. <si...@cs...> - 2004-12-01 18:49:08
|
Chris, Please review our subsequent discussion with Ray titled "has_many progress". He has pretty much converted me to his point of view. He proposes 1) keeping the cache optional (so I'll have to test two configuration), 2) storing dependent objects in hashes in objects instead of retrieving them every time (thus, with cache enabled, fetch_many can skip trips to the database), and most importantly, 3) using his syntax for defining relationships. I can still support the current syntax, of course, but forcing the new configs follow the old syntax results in awkward rules. I looked at some other frameworks, and what Ray proposes turns out to be fairly standard. I am sorry it's not as straightforward. So even though I already have the code for the currently-out-of-favor syntax, I'll have to change it to use Ray's syntax. Please let us know what you think. Simon Chris Winters wrote: > Terribly sorry it took so long to respond. > > >>Now that Chris has some time (hopefully) to consider the changes, I'd >>like to submit a patch that would add has_many, auto-save/auto-delete >>(optional) and full cache support (which is necessary for the >>consistency circular references). Before I do that, I'd like to get >>Chris's approval on the following architectural decisions: >> >>1. There will be a basic cache which will always return the cached >>object, not its copy. > > > Ok. > > >>2. The fetch_many() now will also use cache. It will still make a call >>to the database, because it won't know which ids to request from the >>cache, though. > > > Ok -- although I'd like to have the current behavior available if the > cache is not enabled. (Where it fetches the actual data from the database, > not just the IDs.) > > >>3. Auto-save and auto-delete are off by default. This is not what Ray >>requested, but it's the only way to stay backwards compatible. With >>auto-save on, the graph of object in memory will be traversed to >>determine what has been changed. (Thus, I have deep_changed() instead of >>changed().) > > > I'm for backward compatibility. > > >>4. It will be possible to use a separate class that corresponds to the >>linking table in links_to (think ClubMembership). The syntax I came up >>with is: >> Club => { >> class => 'Club', >> ... >> links_to => >> { 'Person' => >> { table => 'ClubMembership', >> link_class => 'ClubMembership', >> link_class_alias => 'memberships', >> alias => 'members', >> to_id_field => 'member_id', >> from_id_field => 'club_id' }, > > > This looks good. > > >>The ClubMembership class can have extra attributes (like date), but they >>will be specified in a regular way in the config file. The new attribute >>'link_class_alias' will return an arrayref of the ClubMembership >>instances. Again, this is not as elegant as Ray proposed, but backwards >>compatible. > > > Ok. > > >>An open question is what to do with has_a. Currently it's implemented in >>ClassFactory, not ClassFactory/DBI, but adding auto-save and auto-delete >>functionality requires the has_a code to be DBI-aware. I can add extra >>functionality into ClassFactory/DBI, but I am not sure which effect it >>would have on LDAP storage, for example. > > > We can tell ClassFactory::DBI to override the has_a from ClassFactory. (I > don't remember how to do it off the top of my head, but I'm 98% sure it's > already built-in.) > > >>Oh, and a style question. I have marked all of my changes with "tags": >>#Simon >>#/Simon >>and I commented out, not deleted, all replaced code, for ease of >>reference. Is this necessary? > > > No -- that's why we have CVS :-) Just use good comments when you commit. > > Chris > -- Simon (Vsevolod ILyushchenko) si...@cs... http://www.simonf.com Terrorism is a tactic and so to declare war on terrorism is equivalent to Roosevelt's declaring war on blitzkrieg. Zbigniew Brzezinski, U.S. national security advisor, 1977-81 |