[Jifty-commit] r2827 - in Jifty-DBI: . trunk trunk/lib/Jifty/DBI

jifty-commit at lists.jifty.org jifty-commit at lists.jifty.org
Fri Feb 23 15:25:58 EST 2007


Author: sterling
Date: Fri Feb 23 15:25:57 2007
New Revision: 2827

Modified:
   Jifty-DBI/   (props changed)
   Jifty-DBI/trunk/Makefile.PL
   Jifty-DBI/trunk/lib/Jifty/DBI/Column.pm
   Jifty-DBI/trunk/lib/Jifty/DBI/Record.pm
   Jifty-DBI/trunk/lib/Jifty/DBI/Schema.pm
   Jifty-DBI/trunk/lib/Jifty/DBI/SchemaGenerator.pm
   Jifty-DBI/trunk/t/10schema.t
   Jifty-DBI/trunk/t/testmodels.pl

Log:
 * Added better handling of schema versioning
 * Added _init_methods_for_columns to explicitly handle accessor/mutator
   creation for all columns attached to a record
 * Applications employing JDBI can specify schema_version() in a sub-class of
   record to add better handling of "since" and "till" in both schema and code
   generation
 * Modified columns() on records to only return active columns
 * Added all_columns() to retrieve all columns on a record, even inactive ones
 * Added the active() method to columns to test to see if a column is active for
   the current schema version
 * Added/updated tests for the above
=== Targets to commit (you may delete items from it) ===
M   /Users/andrew/Documents/code/perl/Jifty-DBI/Makefile.PL
M   /Users/andrew/Documents/code/perl/Jifty-DBI/lib/Jifty/DBI/Column.pm
M   /Users/andrew/Documents/code/perl/Jifty-DBI/lib/Jifty/DBI/Record.pm
M   /Users/andrew/Documents/code/perl/Jifty-DBI/lib/Jifty/DBI/Schema.pm
M   /Users/andrew/Documents/code/perl/Jifty-DBI/lib/Jifty/DBI/SchemaGenerator.pm
M   /Users/andrew/Documents/code/perl/Jifty-DBI/t/10schema.t
M   /Users/andrew/Documents/code/perl/Jifty-DBI/t/testmodels.pl


Modified: Jifty-DBI/trunk/Makefile.PL
==============================================================================
--- Jifty-DBI/trunk/Makefile.PL	(original)
+++ Jifty-DBI/trunk/Makefile.PL	Fri Feb 23 15:25:57 2007
@@ -18,6 +18,7 @@
 requires('Exporter::Lite');
 requires('Lingua::EN::Inflect');
 requires('UNIVERSAL::require');
+requires('version');
 build_requires('Test::More' => 0.52);
 build_requires('DBD::SQLite');
 no_index directory => 'ex';

Modified: Jifty-DBI/trunk/lib/Jifty/DBI/Column.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/Column.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/Column.pm	Fri Feb 23 15:25:57 2007
@@ -6,6 +6,7 @@
 our $VERSION = '0.01';
 use base qw/Class::Accessor::Fast Jifty::DBI::HasFilters/;
 use UNIVERSAL::require;
+use version;
 
 __PACKAGE__->mk_accessors qw/
     name
@@ -88,4 +89,47 @@
     $self->till(@_);
 }
 
+=head2 active
+
+Returns the a true value if the column method exists for the current application
+version. The current application version is determined by checking the L<Jifty::DBI::Record/schema_version> of the column's L</record_class>. This method returns a false value if the column is not yet been added or has been dropped.
+
+This method returns a false value under these circumstances:
+
+=over
+
+=item *
+
+Both the C<since> trait and C<schema_version> method are defined and C<schema_version> is less than the version set on C<since>.
+
+=item *
+
+Both the C<till> trait and C<schema_version> method are defined and C<schema_version> is greater than or equal to the version set on C<till>.
+
+=back
+
+Otherwise, this method returns true.
+
+=cut
+
+sub active {
+    my $self    = shift;
+
+    return 1 unless $self->record_class->can('schema_version');
+    return 1 unless defined $self->record_class->schema_version;
+
+    my $version = version->new($self->record_class->schema_version);
+
+    # The application hasn't yet started using this column
+    return 0 if defined $self->since
+            and $version < version->new($self->since);
+
+    # The application stopped using this column
+    return 0 if defined $self->till
+            and $version >= version->new($self->till);
+
+    # The application currently uses this column
+    return 1;
+}
+
 1;

Modified: Jifty-DBI/trunk/lib/Jifty/DBI/Record.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/Record.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/Record.pm	Fri Feb 23 15:25:57 2007
@@ -190,8 +190,41 @@
         $column->mandatory(1);
         $self->_init_methods_for_column($column);
     }
+
+}
+
+=head2 _init_methods_for_columns
+
+This is an internal method responsible for calling L</_init_methods_for_column> for each column that has been configured.
+
+=cut
+
+sub _init_methods_for_columns {
+    my $self = shift;
+
+    for my $column (sort keys %{ $self->COLUMNS || {} }) {
+        $self->_init_methods_for_column($self->COLUMNS->{ $column });
+    }
 }
 
+=head2 schema_version
+
+If present, this method must return a string in '1.2.3' format to be used to determine which columns are currently active in the schema. That is, this value is used to determine which columns are defined, based upon comparison to values set in C<till> and C<since>.
+
+If no implementation is present, the "latest" schema version is assumed, meaning that any column defining a C<till> is not active and all others are.
+
+=head2 _init_methods_for_column COLUMN
+
+This method is used internally to update the symbol table for the record class to include an accessor and mutator for each column based upon the column's name.
+
+In addition, if your record class defines the method L</schema_version>, it will automatically generate methods according to whether the column currently exists for the current application schema version returned by that method. The C<schema_version> method must return a value in the same form used by C<since> and C<till>.
+
+If the column doesn't currently exist, it will create the methods, but they will die with an error message stating that the column does not exist for the current version of the application. If it does exist, a normal accessor and mutator will be created.
+
+See also L<Jifty::DBI::Column/active>, L<Jifty::DBI::Schema/since>, L<Jifty::DBI::Schema/till> for more information.
+
+=cut
+
 sub _init_methods_for_column {
     my $self   = $_[0];
     my $column = $_[1];
@@ -208,25 +241,35 @@
     if ( not $self->can($column_name) ) {
         # Accessor
         my $subref;
-        if ( $column->readable ) {
-            if ( UNIVERSAL::isa( $column->refers_to, "Jifty::DBI::Record" ) )
-            {
-                $subref = sub {
-                    $_[0]->_to_record( $column_name,
-                        $_[0]->__value($column_name) );
-                };
-            } elsif (
-                UNIVERSAL::isa(
-                    $column->refers_to, "Jifty::DBI::Collection"
-                )
-                )
-            {
-                $subref = sub { $_[0]->_collection_value($column_name) };
+        if ( $column->active ) {
+            
+
+            if ( $column->readable ) {
+                if ( UNIVERSAL::isa( $column->refers_to, "Jifty::DBI::Record" ) )
+                {
+                    $subref = sub {
+                        $_[0]->_to_record( $column_name,
+                            $_[0]->__value($column_name) );
+                    };
+                } elsif (
+                    UNIVERSAL::isa(
+                        $column->refers_to, "Jifty::DBI::Collection"
+                    )
+                    )
+                {
+                    $subref = sub { $_[0]->_collection_value($column_name) };
+                } else {
+                    $subref = sub { return ( $_[0]->_value($column_name) ) };
+                }
             } else {
-                $subref = sub { return ( $_[0]->_value($column_name) ) };
+                $subref = sub { return '' }
             }
-        } else {
-            $subref = sub { return '' }
+        }
+        else {
+            # XXX sterling: should this be done with Class::ReturnValue instead
+            $subref = sub {
+                Carp::croak("column $column_name is not available for $package for schema version ".$self->schema_version);
+            };
         }
         *{ $package . "::" . $column_name } = $subref;
 
@@ -235,27 +278,45 @@
     if ( not $self->can( "set_" . $column_name ) ) {
         # Mutator
         my $subref;
-        if ( $column->writable ) {
-            if ( UNIVERSAL::isa( $column->refers_to, "Jifty::DBI::Record" ) )
-            {
-                $subref = sub {
-                    my $self = shift;
-                    my $val  = shift;
-
-                    $val = $val->id
-                        if UNIVERSAL::isa( $val, 'Jifty::DBI::Record' );
-                    return (
-                        $self->_set( column => $column_name, value => $val )
+        if ( $column->active ) {
+            if ( $column->writable ) {
+                if ( UNIVERSAL::isa( $column->refers_to, "Jifty::DBI::Record" ) )
+                {
+                    $subref = sub {
+                        my $self = shift;
+                        my $val  = shift;
+
+                        $val = $val->id
+                            if UNIVERSAL::isa( $val, 'Jifty::DBI::Record' );
+                        return (
+                            $self->_set( column => $column_name, value => $val )
+                        );
+                    };
+                } elsif (
+                    UNIVERSAL::isa(
+                        $column->refers_to, "Jifty::DBI::Collection"
+                    )
+                    )
+                {    # XXX elw: collections land here, now what?
+                    my $ret     = Class::ReturnValue->new();
+                    my $message = "Collection column '$column_name' not writable";
+                    $ret->as_array( 0, $message );
+                    $ret->as_error(
+                        errno        => 3,
+                        do_backtrace => 0,
+                        message      => $message
                     );
-                };
-            } elsif (
-                UNIVERSAL::isa(
-                    $column->refers_to, "Jifty::DBI::Collection"
-                )
-                )
-            {    # XXX elw: collections land here, now what?
+                    $subref = sub { return ( $ret->return_value ); };
+                } else {
+                    $subref = sub {
+                        return (
+                            $_[0]->_set( column => $column_name, value => $_[1] )
+                        );
+                    };
+                }
+            } else {
                 my $ret     = Class::ReturnValue->new();
-                my $message = "Collection column '$column_name' not writable";
+                my $message = 'Immutable column';
                 $ret->as_array( 0, $message );
                 $ret->as_error(
                     errno        => 3,
@@ -263,23 +324,13 @@
                     message      => $message
                 );
                 $subref = sub { return ( $ret->return_value ); };
-            } else {
-                $subref = sub {
-                    return (
-                        $_[0]->_set( column => $column_name, value => $_[1] )
-                    );
-                };
             }
-        } else {
-            my $ret     = Class::ReturnValue->new();
-            my $message = 'Immutable column';
-            $ret->as_array( 0, $message );
-            $ret->as_error(
-                errno        => 3,
-                do_backtrace => 0,
-                message      => $message
-            );
-            $subref = sub { return ( $ret->return_value ); };
+        }
+        else {
+            # XXX sterling: should this be done with Class::ReturnValue instead
+            $subref = sub {
+                Carp::croak("column $column_name is not available for $package for schema version ".$self->schema_version);
+            };
         }
         *{ $package . "::" . "set_" . $column_name } = $subref;
     }
@@ -407,11 +458,29 @@
                 <=> ( ( $a->type || '' ) eq 'serial' ) )
                 or ( ($a->sort_order || 0) <=> ($b->sort_order || 0))
                 or ( $a->name cmp $b->name )
-            } values %{ $self->COLUMNS || {} }
+            } grep { $_->active } values %{ $self->COLUMNS || {} }
+	])}
+}
 
+=head2 all_columns
 
-	])}
+  my @all_columns = $record->all_columns;
 
+Returns all the columns for the table, even those that are inactive.
+
+=cut
+
+sub all_columns {
+    my $self = shift;
+
+    # Not cached because it's not expected to be used often
+    return
+        sort {
+            ( ( ( $b->type || '' ) eq 'serial' )
+                <=> ( ( $a->type || '' ) eq 'serial' ) )
+                or ( ($a->sort_order || 0) <=> ($b->sort_order || 0))
+                or ( $a->name cmp $b->name )
+            } values %{ $self->COLUMNS || {} }
 }
 
 # sub {{{ readable_attributes
@@ -589,8 +658,8 @@
 sub as_hash {
     my $self = shift;
     my %values;
-     map {$values{$_} = $self->$_()} $self->readable_attributes  ;
-     return %values;
+    $values{$_} = $self->$_() for $self->readable_attributes;
+    return %values;
 }
 
 

Modified: Jifty-DBI/trunk/lib/Jifty/DBI/Schema.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/Schema.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/Schema.pm	Fri Feb 23 15:25:57 2007
@@ -81,10 +81,7 @@
                 if \&{"$from\::$sym"} == \&$sym;
         }
 
-        # Then initialize all columns
-        foreach my $column (sort keys %{$from->COLUMNS||{}}) {
-            $from->_init_methods_for_column($from->COLUMNS->{$column});
-        }
+        $from->_init_methods_for_columns;
     };
 
     return('-base' => $new_code);
@@ -479,8 +476,10 @@
 
 =head2 till
 
-What application version this column was last supported.  Correct usage
-is C<till '0.2.5'>.
+The version after this column was supported. The column is not available in
+the version named, but would have been in the version immediately prior.
+
+Correct usage is C<till '0.2.5'>. This indicates that the column is not available in version C<0.2.5>, but was available in C<0.2.4>. The value specified for L</since> must be less than this version.
 
 =cut
 

Modified: Jifty-DBI/trunk/lib/Jifty/DBI/SchemaGenerator.pm
==============================================================================
--- Jifty-DBI/trunk/lib/Jifty/DBI/SchemaGenerator.pm	(original)
+++ Jifty-DBI/trunk/lib/Jifty/DBI/SchemaGenerator.pm	Fri Feb 23 15:25:57 2007
@@ -8,6 +8,7 @@
 use DBIx::DBSchema::Column;
 use DBIx::DBSchema::Table;
 use Class::ReturnValue;
+use version;
 
 our $VERSION = '0.01';
 
@@ -259,6 +260,24 @@
         next if $column->virtual;
         next if defined $column->alias_for_column;
 
+        # If schema_version is defined, make sure columns are for that version
+        if ($model->can('schema_version') and defined $model->schema_version) {
+
+            # Skip it if the app version is earlier than the column version
+            next if defined $column->since 
+                and $model->schema_version <  version->new($column->since);
+
+            # Skip it if the app version is the same as or later than the 
+            # column version
+            next if defined $column->till
+                and $model->schema_version >= version->new($column->till);
+
+        }
+
+        # Otherwise, assume the latest version and eliminate till columns
+        next if (!$model->can('schema_version') or !defined $model->schema_version)
+            and defined $column->till;
+
         push @cols,
             DBIx::DBSchema::Column->new(
             {   name     => $column->name,

Modified: Jifty-DBI/trunk/t/10schema.t
==============================================================================
--- Jifty-DBI/trunk/t/10schema.t	(original)
+++ Jifty-DBI/trunk/t/10schema.t	Fri Feb 23 15:25:57 2007
@@ -3,8 +3,9 @@
 use strict;
 use warnings;
 use Test::More;
+use version;
 
-use constant TESTS_PER_DRIVER => 18;
+use constant TESTS_PER_DRIVER => 48;
 our @available_drivers;
 
 BEGIN {
@@ -91,6 +92,40 @@
                        $manually_make_text, 
                        'create_table_sql_text is the statements in create_table_sql_statements');
 
+    my $version_024_min = version->new('0.2.4');
+    my $version_024_max = version->new('0.2.8');
+
+    for my $version (qw/ 0.2.0 0.2.4 0.2.6 0.2.8 0.2.9 /) {
+
+        Sample::Address->schema_version($version);
+
+        my $SG = Jifty::DBI::SchemaGenerator->new($handle, $version);
+        $SG->add_model('Sample::Address');
+
+        my $street_added
+            = version->new($version) >= $version_024_min
+           && version->new($version) <  $version_024_max;
+
+        ok(Sample::Address->COLUMNS->{id}->active, 'id active');
+        ok(Sample::Address->COLUMNS->{employee_id}->active, 'employee_id active');
+        ok(Sample::Address->COLUMNS->{name}->active, 'name active');
+        ok(Sample::Address->COLUMNS->{phone}->active, 'phone active');
+        if ($street_added) {
+            ok(Sample::Address->COLUMNS->{street}->active, 'street active');
+        }
+
+        else {
+            ok(!Sample::Address->COLUMNS->{street}->active, 'street not active');
+        }
+
+        my $address_version_schema = $street_added ? "${address_schema}_024"
+            :                                         $address_schema;
+
+        is_ignoring_space($SG->create_table_sql_text,
+                        Sample::Address->$address_version_schema,
+                        "got the right Address schema for $d version $version");
+    }
+
     cleanup_schema( 'TestApp', $handle );
     disconnect_handle( $handle );
 }

Modified: Jifty-DBI/trunk/t/testmodels.pl
==============================================================================
--- Jifty-DBI/trunk/t/testmodels.pl	(original)
+++ Jifty-DBI/trunk/t/testmodels.pl	Fri Feb 23 15:25:57 2007
@@ -53,10 +53,23 @@
 column phone =>
   type is 'varchar';
 
+column street =>
+  type is 'varchar',
+  since '0.2.4',
+  till '0.2.8';
+
 };
 
 sub validate_name { 1 }
 
+my $schema_version = undef;
+sub schema_version {
+    my $class = shift;
+    my $new_schema_version = shift;
+    $schema_version = $new_schema_version if defined $new_schema_version;
+    return $schema_version;
+}
+
 sub schema_sqlite {
     return q{
     CREATE TABLE addresses (
@@ -68,6 +81,18 @@
     }
 }
 
+sub schema_sqlite_024 {
+    return q{
+    CREATE TABLE addresses (
+     id INTEGER PRIMARY KEY NOT NULL  ,
+     employee_id integer   ,
+     name varchar  DEFAULT 'Frank' ,
+     phone varchar ,
+     street varchar
+    ) ;
+    }
+}
+
 sub schema_pg {
     return q{
     CREATE TABLE addresses ( 
@@ -80,4 +105,17 @@
     };
 }
 
+sub schema_pg_024 {
+    return q{
+    CREATE TABLE addresses ( 
+      id serial NOT NULL , 
+      employee_id integer  ,
+      name varchar DEFAULT 'Frank' ,
+      phone varchar ,
+      street varchar ,
+      PRIMARY KEY (id)
+    ) ;
+    };
+}
+
 1;


More information about the Jifty-commit mailing list