Mailing-List: contact commits-help@spamassassin.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "SpamAssassin Dev" <dev@spamassassin.apache.org>
Message-ID: <20060411120328.77270.qmail@minotaur.apache.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r393196 - in
 /spamassassin/branches/bug-3109-shortcircuiting:
 lib/Mail/SpamAssassin/Conf/Parser.pm t/priorities.t
Date: Tue, 11 Apr 2006 12:03:27 -0000
To: commits@spamassassin.apache.org
From: jm@apache.org

Author: jm
Date: Tue Apr 11 05:03:24 2006
New Revision: 393196

URL: http://svn.apache.org/viewcvs?rev=393196&view=rev
Log:
meta_dependencies should list both direct and indirect dependencies, instead of just the direct ones

Modified:
    spamassassin/branches/bug-3109-shortcircuiting/lib/Mail/SpamAssassin/Conf/Parser.pm
    spamassassin/branches/bug-3109-shortcircuiting/t/priorities.t

Modified: spamassassin/branches/bug-3109-shortcircuiting/lib/Mail/SpamAssassin/Conf/Parser.pm
URL: http://svn.apache.org/viewcvs/spamassassin/branches/bug-3109-shortcircuiting/lib/Mail/SpamAssassin/Conf/Parser.pm?rev=393196&r1=393195&r2=393196&view=diff
==============================================================================
--- spamassassin/branches/bug-3109-shortcircuiting/lib/Mail/SpamAssassin/Conf/Parser.pm (original)
+++ spamassassin/branches/bug-3109-shortcircuiting/lib/Mail/SpamAssassin/Conf/Parser.pm Tue Apr 11 05:03:24 2006
@@ -749,29 +749,42 @@
                     == $Mail::SpamAssassin::Conf::TYPE_META_TESTS);
 
     my $deps = [ ];
-
-    # Lex the rule into tokens using a rather simple RE method ...
-    my $rule = $conf->{tests}->{$name};
-    my $lexer = ARITH_EXPRESSION_LEXER;
-    my @tokens = ($rule =~ m/$lexer/g);
-
-    # Go through each token in the meta rule
-    foreach my $token (@tokens) {
-
-      # Numbers can't be rule names
-      next if ($token =~ /^(?:\W+|\d+)$/);
-
-      # If the token is a rule name, add it as a dependency
-      next unless exists $conf->{tests}->{$token};
-      push @{$deps}, $token;
-    }
+    my $alreadydone = { };
+    $self->_meta_deps_recurse($conf, $name, $name, $deps, $alreadydone);
+    $conf->{meta_dependencies}->{$name} = $deps;
 
     # this is too noisy
     # if (scalar @$deps > 0) {
-    # dbg("rules: meta rule $name depends on ".join(' ', @$deps));
+    # dbg("rules: meta dependencies: $name => ".join(' ', @$deps));
     # }
+  }
+}
 
-    $conf->{meta_dependencies}->{$name} = $deps;
+sub _meta_deps_recurse {
+  my ($self, $conf, $toprule, $name, $deps, $alreadydone) = @_;
+
+  # Only do each rule once per top-level meta; avoid infinite recursion
+  return if $alreadydone->{$name};
+  $alreadydone->{$name} = 1;
+
+  # Obviously, don't trace empty or nonexistent rules
+  my $rule = $conf->{tests}->{$name};
+  return unless $rule;
+
+  # Lex the rule into tokens using a rather simple RE method ...
+  my $lexer = ARITH_EXPRESSION_LEXER;
+  my @tokens = ($rule =~ m/$lexer/g);
+
+  # Go through each token in the meta rule
+  foreach my $token (@tokens) {
+    # has to be an alpha+numeric token
+    next if ($token =~ /^(?:\W+|\d+)$/);
+    # and has to be a rule name
+    next unless exists $conf->{tests}->{$token};
+
+    # add and recurse
+    push @{$deps}, $token;
+    $self->_meta_deps_recurse($conf, $toprule, $token, $deps, $alreadydone);
   }
 }
 
@@ -781,43 +794,27 @@
 
   die unless $conf->{meta_dependencies};    # order requirement
   my $pri = $conf->{priority};
-  my $alreadydone = { };
 
   # sort into priority order, lowest first -- this way we ensure that if we
-  # rearrange the pri of a rule deep in recursion here, then later iterate in
-  # *this* loop to that rule, we cannot accidentally increase its priority.
-
+  # rearrange the pri of a rule early on, we cannot accidentally increase its
+  # priority later.
   foreach my $rule (sort {
             $pri->{$a} <=> $pri->{$b}
           } keys %{$pri})
   {
-    $self->_fix_pri_recurse($conf, $rule, $rule, $alreadydone);
-  }
-}
-
-sub _fix_pri_recurse {
-  my ($self, $conf, $rule, $shorter, $alreadydone) = @_;
-
-  # avoid infinite recursion by only looking at each rule once
-  return if $alreadydone->{$rule};
-  $alreadydone->{$rule} = 1;
-
-  # we only need to worry about meta rules -- they are the
-  # only type of rules who depend on other rules
-  my $deps = $conf->{meta_dependencies}->{$rule};
-  return unless (defined $deps);
-
-  my $basepri = $conf->{priority}->{$rule};
-
-  foreach my $dep (@$deps) {
-    my $deppri = $conf->{priority}->{$dep};
-    if ($deppri > $basepri) {
-      dbg("rules: meta $shorter (pri $basepri) depends on $dep (pri $deppri): fixing");
-      $conf->{priority}->{$dep} = $basepri;
+    # we only need to worry about meta rules -- they are the
+    # only type of rules which depend on other rules
+    my $deps = $conf->{meta_dependencies}->{$rule};
+    next unless (defined $deps);
+
+    my $basepri = $pri->{$rule};
+    foreach my $dep (@$deps) {
+      my $deppri = $pri->{$dep};
+      if ($deppri > $basepri) {
+        dbg("rules: $rule (pri $basepri) requires $dep (pri $deppri): fixed");
+        $pri->{$dep} = $basepri;
+      }
     }
-
-    # and recurse
-    $self->_fix_pri_recurse($conf, $dep, $shorter, $alreadydone);
   }
 }
 

Modified: spamassassin/branches/bug-3109-shortcircuiting/t/priorities.t
URL: http://svn.apache.org/viewcvs/spamassassin/branches/bug-3109-shortcircuiting/t/priorities.t?rev=393196&r1=393195&r2=393196&view=diff
==============================================================================
--- spamassassin/branches/bug-3109-shortcircuiting/t/priorities.t (original)
+++ spamassassin/branches/bug-3109-shortcircuiting/t/priorities.t Tue Apr 11 05:03:24 2006
@@ -1,5 +1,7 @@
 #!/usr/bin/perl
 
+use constant NUM_TESTS => 10;
+
 BEGIN {
   if (-e 't/test_dir') { # if we are running "t/priorities.t", kluge around ...
     chdir 't';
@@ -17,7 +19,7 @@
 
 use SATest; sa_t_init("priorities");
 use strict;
-use Test; BEGIN { plan tests => 8 };
+use Test; BEGIN { plan tests => NUM_TESTS };
 
 use Mail::SpamAssassin;
 
@@ -58,6 +60,14 @@
   shortcircuit DIGEST_MULTIPLE       spam
   priority DIGEST_MULTIPLE           -300
 
+  meta FOO1 (FOO2 && FOO3)
+  meta FOO2 (1)
+  meta FOO3 (FOO4 && FOO5)
+  meta FOO4 (1)
+  meta FOO5 (1)
+  priority FOO5 -23
+  priority FOO1 -28
+
 });
 
 my $sa = create_saobj({
@@ -81,6 +91,8 @@
 # SC_URIBL_BAYES will have overridden its base priority setting
 ok assert_rule_pri 'BAYES_99', -510;
 
+ok assert_rule_pri 'FOO5', -28;
+ok assert_rule_pri 'FOO1', -28;
 
 # ---------------------------------------------------------------------------