[freeside-commits] branch master updated. 130b0b14727bafe247719a4005c527bb8b852c1d

Mark Wells mark at 420.am
Mon Jan 13 18:55:55 PST 2014


The branch, master has been updated
       via  130b0b14727bafe247719a4005c527bb8b852c1d (commit)
       via  6356a7168ec51ce98145b77b095a7bc8dffb3880 (commit)
       via  0a160304352bc27dde67f55e03e0069f7716ad95 (commit)
      from  608cb226935dcc6d96ebafe24683136256895e18 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 130b0b14727bafe247719a4005c527bb8b852c1d
Author: Mark Wells <mark at freeside.biz>
Date:   Mon Jan 13 18:55:45 2014 -0800

    debug

diff --git a/httemplate/graph/cust_bill_pkg.cgi b/httemplate/graph/cust_bill_pkg.cgi
index 44498c8..57fb81b 100644
--- a/httemplate/graph/cust_bill_pkg.cgi
+++ b/httemplate/graph/cust_bill_pkg.cgi
@@ -129,7 +129,6 @@ if ( $cgi->param('class_agg_break') eq 'aggregate' or
     # The new way:
     # Actually break down all subsets of the (selected) report classes.
     my @subsets = FS::part_pkg_report_option->subsets(@classnums);
-    warn "SUBSETS:\n".Dumper(\@subsets)."\n\n";
     my @classnum_space = @classnums;
     @classnums = @classnames = ();
     while(@subsets) {
@@ -145,7 +144,6 @@ if ( $cgi->param('class_agg_break') eq 'aggregate' or
       push @not_classnums, $not_these;
       push @classnames, shift @subsets;
     } #while subsets
-    warn "COMPLEMENTS:\n".Dumper(\@not_classnums)."\n\n";
   }
   # else it's 'pkg', i.e. part_pkg.classnum, which is singular on pkgpart
   # and much simpler

commit 6356a7168ec51ce98145b77b095a7bc8dffb3880
Author: Mark Wells <mark at freeside.biz>
Date:   Mon Jan 13 18:55:37 2014 -0800

    fix auto-assignment when forbidden range is very far from selected block, #26868, from #25530

diff --git a/FS/FS/addr_block.pm b/FS/FS/addr_block.pm
index 8dd09ab..3e62a68 100755
--- a/FS/FS/addr_block.pm
+++ b/FS/FS/addr_block.pm
@@ -242,7 +242,7 @@ sub next_free_addr {
     # also make sure it's not blocked from assignment by an address range
     if ( !$used{$freeaddr->addr } ) {
       my ($range) = grep { !$_->allow_use }
-                  FS::addr_range->any_contains($freeaddr);
+                  FS::addr_range->any_contains($freeaddr->addr);
       if ( !$range ) {
         # then we've found a free address
         return $freeaddr;
diff --git a/FS/FS/addr_range.pm b/FS/FS/addr_range.pm
index 18ae1e2..3cf746f 100644
--- a/FS/FS/addr_range.pm
+++ b/FS/FS/addr_range.pm
@@ -149,7 +149,10 @@ sub end {
       $self->set('start', $end);
       ($end, $start) = ($start, $end);
     }
-    $self->set('length', $end - $start + 1);
+    # bigints are PROBABLY not needed here...but if someone wants to exclude
+    # all the address space not assigned to them, for example, that could be
+    # a pretty large part of IPv4.
+    $self->set('length', $end->bigint - $start->bigint + 1);
     return $end->addr;
   }
   my $end = $start + $self->get('length') - 1;
@@ -165,13 +168,13 @@ Checks whether IPADDR (a dotted-quad IPv4 address) is within the range.
 sub contains {
   my $self = shift;
   my $addr = shift;
-  $addr = NetAddr::IP->new($addr, 0)
-    unless ref($addr) and UNIVERSAL::isa($addr, 'NetAddr::IP');
+  $addr = NetAddr::IP->new($addr, 0);
   return 0 unless $addr;
 
   my $start = NetAddr::IP->new($self->start, 0);
 
-  return ($addr >= $start and $addr - $start < $self->length) ? 1 : 0;
+  return ($addr >= $start and $addr->bigint - $start->bigint < $self->length) 
+          ? 1 : 0;
 } 
 
 =item as_string
@@ -242,10 +245,14 @@ sub any_contains {
 
 =head1 DEVELOPER NOTE
 
-L<NetAddr::IP> objects have netmasks.  When using them to represent 
-range endpoints, be sure to set the netmask to I<zero> so that math on 
-the address doesn't stop at the subnet boundary.  (The default is /32, 
-which doesn't work very well.  Address ranges ignore subnet boundaries.)
+L<NetAddr::IP> objects have netmasks.  They also have overloaded operators
+for addition and subtraction, but those have range limitations when comparing
+addresses.  (An IPv4 address is effectively a uint32; the difference
+between two IPv4 addresses is the same range, but signed.)  Therefore,
+the distance between two addresses should be calculated using the 
+C<bigint> method ($addr2->bigint - $addr1->bigint), which returns the 
+address as a L<Math::BigInt> object, and also conveniently discards the 
+netmask.
 
 =head1 BUGS
 

commit 0a160304352bc27dde67f55e03e0069f7716ad95
Author: Mark Wells <mark at freeside.biz>
Date:   Mon Jan 13 18:55:29 2014 -0800

    query optimization for #25459

diff --git a/FS/FS/part_pkg_report_option.pm b/FS/FS/part_pkg_report_option.pm
index 16a4c98..372b119 100644
--- a/FS/FS/part_pkg_report_option.pm
+++ b/FS/FS/part_pkg_report_option.pm
@@ -2,7 +2,7 @@ package FS::part_pkg_report_option;
 
 use strict;
 use base qw( FS::Record );
-use FS::Record qw( qsearch qsearchs );
+use FS::Record qw( qsearch qsearchs dbh );
 
 =head1 NAME
 
@@ -111,6 +111,48 @@ sub check {
 
 =back
 
+=head1 CLASS METHODS
+
+=over 4
+
+=item subsets OPTIONNUM, ...
+
+Given a list of report_option numbers, determines all combinations of those
+numbers that exist on actual package definitions.  For each such combination,
+returns an arrayref of report_option numbers, followed by an arrayref of
+corresponding report class names.  This is used for a search optimization.
+
+=cut
+
+# probably doesn't belong here, but there's not a better place for it
+# and optimizations are, by nature, hackish
+
+sub subsets {
+  my ($self, @nums) = @_;
+  my @optionnames = map { "'report_option_$_'" } @nums;
+  my $where = "WHERE optionname IN(".join(',', at optionnames).")"
+    if @nums;
+  my $subselect =
+    "SELECT pkgpart, replace(optionname, 'report_option_', '')::int AS num ".
+    "FROM part_pkg_option $where ".
+    "ORDER BY pkgpart, num";
+  my $select =
+    "SELECT DISTINCT array_to_string(array_agg(num), ','), ".
+    "array_to_string(array_agg(name), ',') ".
+    "FROM ($subselect) AS x JOIN part_pkg_report_option USING (num) ".
+    "GROUP BY pkgpart";
+  my $dbh = dbh;
+  my $sth = $dbh->prepare($select)
+    or die $dbh->errstr; # seriously, this should never happen
+  $sth->execute
+    or die $sth->errstr;
+  # return the first (only) column
+  map { [ split(',',$_->[0]) ],
+        [ split(',',$_->[1]) ] } @{ $sth->fetchall_arrayref };
+}
+
+=back
+
 =head1 BUGS
 
 Overlaps somewhat with pkg_class and pkg_category
diff --git a/httemplate/graph/cust_bill_pkg.cgi b/httemplate/graph/cust_bill_pkg.cgi
index 1b31955..44498c8 100644
--- a/httemplate/graph/cust_bill_pkg.cgi
+++ b/httemplate/graph/cust_bill_pkg.cgi
@@ -103,7 +103,7 @@ if ( $cgi->param('class_mode') eq 'report' ) {
   $value_col = 'classnum';
 }
 
-my @classnums = grep /^\d+$/, $cgi->param($class_param);
+my @classnums = sort {$a <=> $b} grep /^\d+$/, $cgi->param($class_param);
 my @classnames = map { if ( $_ ) {
                          my $class = qsearchs($class_table, {$value_col=>$_} );
                          $class->$name_col;
@@ -128,22 +128,24 @@ if ( $cgi->param('class_agg_break') eq 'aggregate' or
   if ( $cgi->param('class_mode') eq 'report' ) {
     # The new way:
     # Actually break down all subsets of the (selected) report classes.
-    my $powerset = sub {
-      my @set = [];
-      foreach my $x (@_) {
-        @set = map { $_, [ @$_, $x ] } @set;
+    my @subsets = FS::part_pkg_report_option->subsets(@classnums);
+    warn "SUBSETS:\n".Dumper(\@subsets)."\n\n";
+    my @classnum_space = @classnums;
+    @classnums = @classnames = ();
+    while(@subsets) {
+      my $these = shift @subsets;
+      # applied topology!
+      my $not_these = [ @classnum_space ];
+      my $i = 0;
+      foreach (@$these) {
+        $i++ until $not_these->[$i] == $_;
+        splice($not_these, $i, 1);
       }
-      @set;
-    };
-    @classnums = $powerset->(@classnums);
-    @classnames = $powerset->(@classnames);
-    # this is pairwise complementary to @classnums, because math
-    @not_classnums = reverse(@classnums);
-warn Dumper(\@classnums, \@classnames, \@not_classnums);
-    # remove the null set
-    shift @classnums;
-    shift @classnames;
-    shift @not_classnums;
+      push @classnums, $these;
+      push @not_classnums, $not_these;
+      push @classnames, shift @subsets;
+    } #while subsets
+    warn "COMPLEMENTS:\n".Dumper(\@not_classnums)."\n\n";
   }
   # else it's 'pkg', i.e. part_pkg.classnum, which is singular on pkgpart
   # and much simpler

-----------------------------------------------------------------------

Summary of changes:
 FS/FS/addr_block.pm                |    2 +-
 FS/FS/addr_range.pm                |   23 ++++++++++++------
 FS/FS/part_pkg_report_option.pm    |   44 +++++++++++++++++++++++++++++++++++-
 httemplate/graph/cust_bill_pkg.cgi |   32 +++++++++++++-------------
 4 files changed, 75 insertions(+), 26 deletions(-)




More information about the freeside-commits mailing list