[freeside-commits] branch FREESIDE_3_BRANCH updated. 1552ee615a8f73ee8a72cc85c6e1a2fcfc601dda

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


The branch, FREESIDE_3_BRANCH has been updated
       via  1552ee615a8f73ee8a72cc85c6e1a2fcfc601dda (commit)
       via  70cda99de667abcafbe9f96d9d000098f3410ae9 (commit)
       via  fadbf7963db877d3c37028805cbb656ba1d634ec (commit)
      from  dd0bf7c0db2d97a33bedd324788ca550b54572fc (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 1552ee615a8f73ee8a72cc85c6e1a2fcfc601dda
Author: Mark Wells <mark at freeside.biz>
Date:   Mon Jan 13 18:55:06 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 70cda99de667abcafbe9f96d9d000098f3410ae9
Author: Mark Wells <mark at freeside.biz>
Date:   Mon Jan 13 17:36:28 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 778c124..77ac334 100755
--- a/FS/FS/addr_block.pm
+++ b/FS/FS/addr_block.pm
@@ -260,7 +260,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 fadbf7963db877d3c37028805cbb656ba1d634ec
Author: Mark Wells <mark at freeside.biz>
Date:   Mon Jan 13 15:52:50 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