[jifty-devel] [Jifty-commit] r5229 - in jifty/trunk: lib/Jifty

[Dobrica Pavlinusic]

On Tue, Mar 18, 2008 at 02:51:21PM -0400, Jesse Vincent wrote:
> Hm. I'm a little worried about canonicalizing on load. (I'm not saying  
> it's wrong or that I want it reverted, but that it raised a red flag)
>
> Can you explain what motivated the change?

After short discussion on #jifty, I reverted this patch until we get
more opinions on it.

My use case follows: I have canonicalization rule on date column which turns
string "none" into date in future. This works fine with create, but when
I replace it with load_or_create I get SQL error because load doesn't
originally do canonicalization.

I think of create and load_or_create as symmetrical operations, so I
would expect that this just works (by magic :-)

Possible solution outside jifty code include plugin (which would require
new hook) and filters.

Original idea was to add before_load_by_cols hook, but that would introduce
canonicalization overhead (which can be noticeable if there are external
calls, to LDAP for example) to every load which I don't like.

Filter seems like insufficient magic, since I already do have
canonicalize for that column, why would I have to have additional
filter?

Currently best solution seems to be adding before_load_or_create hook,
and writing plugin which does canonicalization on load_or_create.

I will pursue this path if there are no objections to it.

> On Mar 18, 2008, at 2:39 PM, Jifty commits wrote:
>> Author: dpavlin
>> Date: Tue Mar 18 14:39:52 2008
>> New Revision: 5229
>>
>> Added:
>>   jifty/trunk/t/TestApp/t/22-canonicalize-load_or_create.t
>> Modified:
>>   jifty/trunk/lib/Jifty/Record.pm
>>
>> Log:
>> run canonicalization before load in load_or_create
>>
>> Modified: jifty/trunk/lib/Jifty/Record.pm
>> ======== 
>> ======================================================================
>> --- jifty/trunk/lib/Jifty/Record.pm	(original)
>> +++ jifty/trunk/lib/Jifty/Record.pm	Tue Mar 18 14:39:52 2008
>> @@ -143,6 +143,13 @@
>>
>>     my %args = (@_);
>>
>> +    foreach my $key ( keys %args ) {
>> +        $args{$key} = $self->run_canonicalization_for_column(
>> +            column => $key,
>> +            value  => $args{$key}
>> +        );
>> +    }
>> +
>>     my ( $id, $msg ) = $self->load_by_cols(%args);
>>     unless ( $self->id ) {
>>         return $self->create(%args);
>>
>> Added: jifty/trunk/t/TestApp/t/22-canonicalize-load_or_create.t
>> ======== 
>> ======================================================================
>> --- (empty file)
>> +++ jifty/trunk/t/TestApp/t/22-canonicalize-load_or_create.t	Tue Mar  
>> 18 14:39:52 2008
>> @@ -0,0 +1,38 @@
>> +#!/usr/bin/env perl
>> +use warnings;
>> +use strict;
>> +
>> +=head1 DESCRIPTION
>> +
>> +Test canonicalize on load_or_create
>> +
>> +=cut
>> +
>> +use lib 't/lib';
>> +use Jifty::SubTest;
>> +
>> +use Jifty::Test tests => 9;
>> +use_ok('TestApp::Model::CanonTest');
>> +
>> +# Grab a system use
>> +my $system_user = TestApp::CurrentUser->superuser;
>> +ok($system_user, "Found a system user");
>> +
>> +# Try testing a create
>> +my $o = TestApp::Model::CanonTest->new(current_user => $system_user);
>> +my ($id) = $o->create( column_1 => 'foo#$bar' );
>> +ok($id, "CanonTest create returned success");
>> +ok($o->id, "New CanonTest has valid id set");
>> +is($o->id, $id, "Create returned the right id");
>> +is($o->column_1, 'foobar', "Created object has the right column_1");
>> +
>> +# And another
>> +$o->load_or_create( column_1 => 'foo()bar' );
>> +ok($o->id, "CanonTest create returned value");
>> +is($o->id, $id, "And it is same from the previous one");
>> +
>> +# Searches in general
>> +my $collection =  TestApp::Model::CanonTestCollection- 
>> >new(current_user => $system_user);
>> +$collection->unlimit;
>> +is($collection->count, 1, "Finds one records");
>> +
>> _______________________________________________
>> Jifty-commit mailing list
>> Jifty-commit at lists.jifty.org
>> http://lists.jifty.org/cgi-bin/mailman/listinfo/jifty-commit
>>
>



> _______________________________________________
> jifty-devel mailing list
> jifty-devel at lists.jifty.org
> http://lists.jifty.org/cgi-bin/mailman/listinfo/jifty-devel


-- 
Dobrica Pavlinusic               2share!2flame            dpavlin at rot13.org
Unix addict. Internet consultant.             http://www.rot13.org/~dpavlin