From: Peter Rabbitson Date: Mon, 16 Sep 2013 17:16:48 +0000 (+0200) Subject: Reorder relationship sanity checks, run them only on known loaded classes X-Git-Tag: v0.08260~149 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=9e7525a2;p=dbsrgits%2FDBIx-Class.git Reorder relationship sanity checks, run them only on known loaded classes This avoids load-order issues in contrived situations where column definitions are split across class inheritance (which gets rather fun doe to the prorotypical inheritance of resultsource instances themselves) While not an immediate problem in code in the wild, the hash randomization in 5.18 now leads to random load order, and as a result more load-order combinations are tried and can eventually fail. This should reduce these situations to a minimum, and 79061be1 should keep us appraised of any problems while DBICT:Schema evolves Ideally the checks will be lifted up to schema-composition time, but we are not there yet, one yak at a time. --- diff --git a/Changes b/Changes index 1333f38..d42f06f 100644 --- a/Changes +++ b/Changes @@ -4,6 +4,9 @@ Revision history for DBIx::Class - A new zero-to-DBIC style manual: DBIx::Class::Manual::QuickStart * Fixes + - More robust handling of circular relationship declarations by loading + foreign classes less frequently (should resolve issues like + http://lists.scsys.co.uk/pipermail/dbix-class/2013-June/011374.html) - Setting quote_names propagates to SQL::Translator when producing SQLite DDL (it is one of the few producers *NOT* quoting by default) - Back out self-cleaning from DBIx::Class::Carp for the time being diff --git a/lib/DBIx/Class/Relationship/BelongsTo.pm b/lib/DBIx/Class/Relationship/BelongsTo.pm index cbc4484..b594d3a 100644 --- a/lib/DBIx/Class/Relationship/BelongsTo.pm +++ b/lib/DBIx/Class/Relationship/BelongsTo.pm @@ -25,9 +25,6 @@ sub belongs_to { # no join condition or just a column name if (!ref $cond) { - $class->ensure_class_loaded($f_class); - - my $pri = $f_class->result_source_instance->_single_pri_col_or_die; my ($f_key, $guess); if (defined $cond and length $cond) { @@ -43,6 +40,19 @@ sub belongs_to { "No such column '$f_key' declared yet on ${class} ($guess)" ) unless $class->has_column($f_key); + $class->ensure_class_loaded($f_class); + my $f_rsrc = try { + $f_class->result_source_instance; + } + catch { + $class->throw_exception( + "Foreign class '$f_class' does not seem to be a Result class " + . "(or it simply did not load entirely due to a circular relation chain)" + ); + }; + + my $pri = $f_rsrc->_single_pri_col_or_die; + $cond = { "foreign.${pri}" => "self.${f_key}" }; } diff --git a/lib/DBIx/Class/Relationship/HasMany.pm b/lib/DBIx/Class/Relationship/HasMany.pm index 5707c93..664f4fd 100644 --- a/lib/DBIx/Class/Relationship/HasMany.pm +++ b/lib/DBIx/Class/Relationship/HasMany.pm @@ -15,7 +15,6 @@ sub has_many { my ($class, $rel, $f_class, $cond, $attrs) = @_; unless (ref $cond) { - $class->ensure_class_loaded($f_class); my $pri = $class->result_source_instance->_single_pri_col_or_die; @@ -29,10 +28,12 @@ sub has_many { $guess = "using our class name '$class' as foreign key source"; } - my $f_class_loaded = try { $f_class->columns }; - $class->throw_exception( - "No such column '$f_key' on foreign class ${f_class} ($guess)" - ) if $f_class_loaded && !$f_class->has_column($f_key); + # only perform checks if the far side appears already loaded + if (my $f_rsrc = try { $f_class->result_source_instance } ) { + $class->throw_exception( + "No such column '$f_key' on foreign class ${f_class} ($guess)" + ) if !$f_rsrc->has_column($f_key); + } $cond = { "foreign.${f_key}" => "self.${pri}" }; } diff --git a/lib/DBIx/Class/Relationship/HasOne.pm b/lib/DBIx/Class/Relationship/HasOne.pm index 57885b3..f1c9ccc 100644 --- a/lib/DBIx/Class/Relationship/HasOne.pm +++ b/lib/DBIx/Class/Relationship/HasOne.pm @@ -23,28 +23,44 @@ sub has_one { sub _has_one { my ($class, $join_type, $rel, $f_class, $cond, $attrs) = @_; unless (ref $cond) { - $class->ensure_class_loaded($f_class); - my $pri = $class->result_source_instance->_single_pri_col_or_die; - my $f_class_loaded = try { $f_class->columns }; - my ($f_key,$guess); + my ($f_key,$guess,$f_rsrc); if (defined $cond && length $cond) { $f_key = $cond; $guess = "caller specified foreign key '$f_key'"; - } elsif ($f_class_loaded && $f_class->has_column($rel)) { - $f_key = $rel; - $guess = "using given relationship name '$rel' as foreign key column name"; - } elsif ($f_class_loaded and my $f_pri = try { - $f_class->result_source_instance->_single_pri_col_or_die - }) { - $f_key = $f_pri; - $guess = "using primary key of foreign class for foreign key"; } + else { + # at this point we need to load the foreigner, expensive or not + $class->ensure_class_loaded($f_class); + + $f_rsrc = try { + $f_class->result_source_instance; + } + catch { + $class->throw_exception( + "Foreign class '$f_class' does not seem to be a Result class " + . "(or it simply did not load entirely due to a circular relation chain)" + ); + }; - $class->throw_exception( - "No such column '$f_key' on foreign class ${f_class} ($guess)" - ) if $f_class_loaded && !$f_class->has_column($f_key); + if ($f_rsrc->has_column($rel)) { + $f_key = $rel; + $guess = "using given relationship name '$rel' as foreign key column name"; + } + else { + $f_key = $f_rsrc->_single_pri_col_or_die; + $guess = "using primary key of foreign class for foreign key"; + } + } + + # only perform checks if the far side was not preloaded above *AND* + # appears to have been loaded by something else (has a rsrc_instance) + if (! $f_rsrc and $f_rsrc = try { $f_class->result_source_instance }) { + $class->throw_exception( + "No such column '$f_key' on foreign class ${f_class} ($guess)" + ) if !$f_rsrc->has_column($f_key); + } $cond = { "foreign.${f_key}" => "self.${pri}" }; }