making the hold targeter (much! about 2x) faster
authormiker <miker@dcc99617-32d9-48b4-a31d-7c20da2025e4>
Fri, 29 Jul 2005 21:15:17 +0000 (21:15 +0000)
committermiker <miker@dcc99617-32d9-48b4-a31d-7c20da2025e4>
Fri, 29 Jul 2005 21:15:17 +0000 (21:15 +0000)
git-svn-id: svn://svn.open-ils.org/ILS/trunk@1579 dcc99617-32d9-48b4-a31d-7c20da2025e4

Open-ILS/src/perlmods/OpenILS/Application/Storage/CDBI.pm
Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher.pm
Open-ILS/src/perlmods/OpenILS/Application/Storage/Publisher/action.pm

index 020bf87..61ad637 100644 (file)
@@ -172,11 +172,13 @@ sub merge {
 
        delete $$arg{$_} for (keys %$search);
 
-       my @objs = $self->search_where($search);
+       my @objs = ($self);
+       @objs = $self->search_where($search) unless (ref $self);
+
        if (@objs == 1) {
-               return $objs[0]->update($arg)->id;
+               return $objs[0]->update($arg);
        } elsif (@objs == 0) {
-               return $self->create($arg)->id;
+               return $self->create($arg);
        } else {
                throw OpenSRF::EX::WARN ("Non-unique search key for merge.  Perhaps you meant to use remote_update?");
        }
@@ -204,7 +206,7 @@ sub create {
 
        $log->debug("\$arg is $arg (".ref($arg).")",DEBUG);
 
-       if (ref($arg)) {
+       if (ref($arg) && UNIVERSAL::isa($arg => 'Fieldmapper')) {
                return $self->create_from_fieldmapper($arg,@_);
        }
 
@@ -221,24 +223,16 @@ sub create_from_fieldmapper {
        my $class = ref($obj) || $obj;
        my ($primary) = $class->columns('Primary');
 
-       if (ref($fm)) {
-               my %hash;
-               if (UNIVERSAL::isa($fm => 'Fieldmapper')) {
-                       %hash = map { defined $fm->$_ ?
-                                               ($_ => $fm->$_) :
-                                               ()
-                                       } grep { $_ ne $primary } $class->columns('All');
-
-                       if ($class->find_column( 'last_xact_id' )) {
-                               my $xact_id = $class->current_xact_id;
-                               throw Error unless ($xact_id);
-                               $hash{last_xact_id} = $xact_id;
-                       }
-               } else {
-                       %hash = map { exists $fm->{$_} ?
-                                               ($_ => $fm->{$_}) :
-                                               ()
-                                       } grep { $_ ne $primary } $class->columns('All');
+       if (ref($fm) &&UNIVERSAL::isa($fm => 'Fieldmapper')) {
+               my %hash = map { defined $fm->$_ ?
+                                       ($_ => $fm->$_) :
+                                       ()
+                               } grep { $_ ne $primary } $class->columns('All');
+
+               if ($class->find_column( 'last_xact_id' )) {
+                       my $xact_id = $class->current_xact_id;
+                       throw Error unless ($xact_id);
+                       $hash{last_xact_id} = $xact_id;
                }
 
                return $class->create( \%hash, @params );
@@ -296,6 +290,7 @@ sub modify_from_fieldmapper {
        $log->debug("Modifying object using fieldmapper", DEBUG);
 
        my $class = ref($obj) || $obj;
+       my ($primary) = $class->columns('Primary');
 
        if (!ref($obj)) {
                $obj = $class->retrieve($fm);
@@ -311,7 +306,7 @@ sub modify_from_fieldmapper {
                %hash = map { defined $fm->$_ ?
                                ($_ => ''.$fm->$_) :
                                ()
-                       } $fm->real_fields;
+                       } grep { $_ ne $primary } $class->columns('All');
        } else {
                %hash = %{$fm};
        }
index b46e361..5f4668e 100644 (file)
@@ -287,7 +287,7 @@ sub merge_node {
 
        my $success = 1;
        try {
-               $success = $cdbi->merge($node);
+               $success = $cdbi->merge($node)->id;
        } catch Error with {
                $success = 0;
        };
@@ -315,6 +315,8 @@ sub batch_call {
        my $client = shift;
        my @nodes = @_;
 
+       my $unwrap = $self->{unwrap};
+
        my $cdbi = $self->{cdbi};
        my $api_name = $self->api_name;
        (my $single_call_api_name = $api_name) =~ s/batch\.//o;
@@ -324,7 +326,7 @@ sub batch_call {
 
        my @success;
        while ( my $node = shift(@nodes) ) {
-               my ($res) = $method->run( $node ); 
+               my ($res) = $method->run( ($unwrap ? (@$node) : ($node)) ); 
                push(@success, 1) if ($res >= 0);
        }
 
@@ -595,6 +597,7 @@ for my $fmclass ( (Fieldmapper->classes) ) {
                        __PACKAGE__->register_method(
                                api_name        => $api_prefix.'.batch.merge',
                                method          => 'batch_call',
+                               unwrap          => 1,
                                api_level       => 1,
                                cdbi            => $cdbi,
                        );
@@ -616,6 +619,7 @@ for my $fmclass ( (Fieldmapper->classes) ) {
                                api_name        => $api_prefix.'.batch.remote_update',
                                method          => 'batch_call',
                                api_level       => 1,
+                               unwrap          => 1,
                                cdbi            => $cdbi,
                        );
                }
index 742aaec..da3d7a7 100644 (file)
@@ -11,10 +11,8 @@ use DateTime::Format::ISO8601;
 my $parser = DateTime::Format::ISO8601->new;
 my $log = 'OpenSRF::Utils::Logger';
 
-sub grab_overdue {
-       my $self = shift;
-       my $client = shift;
-       my $grace = shift || '';
+sub overdue_circs {
+       my $grace = shift;
 
        my $c_t = action::circulation->table;
 
@@ -30,7 +28,16 @@ sub grab_overdue {
        my $sth = action::circulation->db_Main->prepare_cached($sql);
        $sth->execute;
 
-       $client->respond( $_->to_fieldmapper ) for ( map { action::circulation->construct($_) } $sth->fetchall_hash );
+       return ( map { action::circulation->construct($_) } $sth->fetchall_hash );
+
+}
+
+sub grab_overdue {
+       my $self = shift;
+       my $client = shift;
+       my $grace = shift || '';
+
+       $client->respond( $_->to_fieldmapper ) for ( overdue_circs($grace) );
 
        return undef;
 
@@ -270,12 +277,9 @@ sub generate_fines {
        
        my @circs;
        if ($circ) {
-               push @circs,
-                       $self->method_lookup(
-                               'open-ils.storage.direct.action.circulation.search_where'
-                       )->run( { id => $circ, stop_fines => undef } );
+               push @circs, action::circulation->search_where( { id => $circ, stop_fines => undef } );
        } else {
-               push @circs, $self->method_lookup('open-ils.storage.action.circulation.overdue')->run( $grace );
+               push @circs, overdue_circs($grace);
        }
 
        for my $c (@circs) {
@@ -303,8 +307,8 @@ sub generate_fines {
                                " (user ".$c->usr.").\n".
                                "\tItem was due on or before: ".localtime($due)."\n");
        
-                       my ($fine) = $self->method_lookup('open-ils.storage.direct.money.billing.search')->run(
-                               { xact => $c->id, voided => 'f' },
+                       my ($fine) = money::billing->search(
+                               xact => $c->id, voided => 'f',
                                { order_by => 'billing_ts DESC', limit => '1' }
                        );
        
@@ -331,11 +335,10 @@ sub generate_fines {
        
                        for my $bill (1 .. $pending_fine_count) {
        
-                               my ($total) = $self->method_lookup('open-ils.storage.direct.money.billable_transaction_summary.retrieve')->run( $c->id );
+                               my ($total) = money::billable_transaction_summary->retrieve( $c->id );
        
                                if ($total && $total->balance_owed > $c->max_fine) {
-                                       $c->stop_fines('MAXFINES');
-                                       $self->method_lookup('open-ils.storage.direct.action.circulation.update')->run( $c );
+                                       $c->update({stop_fines => 'MAXFINES'});
                                        $client->respond(
                                                "\tMaximum fine level of ".$c->max_fine.
                                                " reached for this circulation.\n".
@@ -343,13 +346,12 @@ sub generate_fines {
                                        last;
                                }
        
-                               my $billing = new Fieldmapper::money::billing;
-                               $billing->xact( $c->id );
-                               $billing->note( "Overdue Fine" );
-                               $billing->amount( $c->recuring_fine );
-       
-                               $billing->billing_ts(
-                                       DateTime->from_epoch( epoch => $last_fine + $fine_interval * $bill )->strftime('%FT%T%z')
+                               my $billing = money::billing->create(
+                                       { xact          => ''.$c->id,
+                                         note          => "Overdue Fine",
+                                         amount        => ''.$c->recuring_fine,
+                                         billing_ts    => DateTime->from_epoch( epoch => $last_fine + $fine_interval * $bill )->strftime('%FT%T%z')
+                                       }
                                );
        
                                $client->respond(
@@ -359,8 +361,6 @@ sub generate_fines {
                                                        clense_ISO8601( $billing->billing_ts )
                                                )->epoch
                                        )."\n" );
-       
-                               $self->method_lookup('open-ils.storage.direct.money.billing.create')->run( $billing );
                        }
                } catch Error with {
                        my $e = shift;
@@ -403,9 +403,9 @@ sub hold_copy_targeter {
        );
 
 
-       ($statuses) = $self->method_lookup('open-ils.storage.direct.config.copy_status.search.holdable.atomic')->run('t');
+       $statuses ||= [ config::copy_status->search(holdable => 't') ];
 
-       ($locations) = $self->method_lookup('open-ils.storage.direct.asset.copy_location.search.holdable.atomic')->run('t');
+       $locations ||= [ asset::copy_location->search(holdable => 't') ];
 
        my $holds;
 
@@ -413,19 +413,16 @@ sub hold_copy_targeter {
 
        try {
                if ($one_hold) {
-                       ($holds) = $self->method_lookup('open-ils.storage.direct.action.hold_request.search.atomic')
-                                               ->run(id => $one_hold);
+                       $holds = [ action::hold_request->search(id => $one_hold) ];
                } else {
-                       ($holds) = $self->method_lookup('open-ils.storage.direct.action.hold_request.search_where.atomic')
-                                               ->run(
+                       $holds = [ action::hold_request->search_where(
                                                        { capture_time => undef,
                                                          prev_check_time => { '<=' => $expire_threshold },
                                                        },
-                                                       { order_by => 'request_time,prev_check_time' } );
-                       push @$holds, $self->method_lookup('open-ils.storage.direct.action.hold_request.search')
-                                               ->run(
-                                                       { capture_time => undef,
-                                                         prev_check_time => undef },
+                                                       { order_by => 'request_time,prev_check_time' } ) ];
+                       push @$holds, action::hold_request->search(
+                                                       capture_time => undef,
+                                                       prev_check_time => undef,
                                                        { order_by => 'request_time' } );
                }
        } catch Error with {
@@ -434,8 +431,8 @@ sub hold_copy_targeter {
        };
 
        for my $hold (@$holds) {
+               action::hold_request->db_Main->begin_work;
                try {
-                       $self->method_lookup( 'open-ils.storage.transaction.begin')->run($client);
                        $client->respond("Processing hold ".$hold->id."...\n");
 
                        my $copies;
@@ -464,13 +461,13 @@ sub hold_copy_targeter {
                        $client->respond("\t".scalar(@good_copies)." (non-current) copies available for targeting...\n");
 
                        my $old_best = $hold->current_copy;
-                       $hold->clear_current_copy;
+                       $hold->update({ current_copy => undef });
        
                        if (!scalar(@good_copies)) {
                                $client->respond("\tNo (non-current) copies available to fill the hold.\n");
                                if ( $old_best && grep {$c->id == $hold->current_copy} @$copies ) {
                                        $client->respond("\tPushing current_copy back onto the targeting list\n");
-                                       push @good_copies, $self->method_lookup('open-ils.storage.direct.asset.copy.retrieve')->run( $old_best );
+                                       push @good_copies, asset::copy->retrieve( $old_best );
                                } else {
                                        $client->respond("\tcurrent_copy is no longer available for targeting... NEXT HOLD, PLEASE!\n");
                                        next;
@@ -492,32 +489,33 @@ sub hold_copy_targeter {
                                # hold wasn't fulfilled, record the fact
                        
                                $client->respond("\tHold was not (but should have been) fulfilled by ".$old_best->id.".\n");
-                               my $ufh = new Fieldmapper::action::unfulfilled_hold_list;
-                               $ufh->hold( $hold->id );
-                               $ufh->current_copy( $old_best->id );
-                               $ufh->circ_lib( $old_best->circ_lib );
-                               $self->method_lookup('open-ils.storage.direct.action.unfulfilled_hold_list.create')->run( $ufh );
+                               action::unfulfilled_hold_list->create(
+                                               { hold => ''.$hold->id,
+                                                 current_copy => ''.$old_best->id,
+                                                 circ_lib => ''.$old_best->circ_lib,
+                                               });
                        }
 
                        if ($best) {
-                               $hold->current_copy( $best->id );
+                               $hold->update( { current_copy => ''.$best->id } );
                                $client->respond("\tTargeting copy ".$best->id." for hold fulfillment.\n");
                        }
 
-                       $hold->prev_check_time( 'now' );
+                       $hold->update( { prev_check_time => 'now' } );
                        $client->respond("\tUpdating hold ".$hold->id." with new 'current_copy' for hold fulfillment.\n");
-                       my ($r) = $self->method_lookup('open-ils.storage.direct.action.hold_request.update')->run( $hold );
 
                        $client->respond("\tProcessing of hold ".$hold->id." complete.\n");
                        $self->method_lookup('open-ils.storage.transaction.commit')->run;
 
+                       action::hold_request->dbi_commit;
+
                } otherwise {
                        my $e = shift;
                        $client->respond("\tProcessing of hold ".$hold->id." failed!.\n\t\t$e\n");
-                       $self->method_lookup('open-ils.storage.transaction.rollback')->run;
+                       action::hold_request->dbi_rollback;
                };
-               $self->method_lookup( 'open-ils.storage.transaction.commit')->run($client);
        }
+
        $self->{user_filter}->disconnect;
        $self->{user_filter}->finish;
        delete $$self{user_filter};
@@ -539,16 +537,14 @@ sub copy_hold_capture {
 
        if (!defined($cps)) {
                try {
-                       ($cps) = $self->method_lookup('open-ils.storage.direct.asset.copy.search.id.atomic')
-                                               ->run( $hold->target );
-       
+                       $cps = [ asset::copy->search( id => $hold->target ) ];
                } catch Error with {
                        my $e = shift;
                        die "Could not retrieve initial volume list:\n\n$e\n";
                };
        }
 
-       my @copies = grep { $_->holdable == 1  and $_->ref == 0 } @$cps;
+       my @copies = grep { $_->holdable and !$_->ref } @$cps;
 
        for (my $i = 0; $i < @copies; $i++) {
                next unless $copies[$i];
@@ -561,33 +557,35 @@ sub copy_hold_capture {
                        !$copies[$i] ||
                        !$self->{user_filter}->request(
                                'open-ils.circ.permit_hold',
-                               $hold => $copies[$i],
-                               { title => $rec, call_number => $cn }
-                       )->gather(1)
+                               $hold->to_fieldmapper, do {
+                                       my $cp_fm = $copies[$i]->to_fieldmapper;
+                                       $cp_fm->circ_lib( $copies[$i]->circ_lib->to_fieldmapper );
+                                       $cp_fm->location( $copies[$i]->location->to_fieldmapper );
+                                       $cp_fm->status( $copies[$i]->status->to_fieldmapper );
+                                       $cp_fm;
+                               },
+                               { title => $rec->to_fieldmapper,
+                                 usr => actor::user->retrieve($hold->usr)->to_fieldmapper,
+                                 requestor => actor::user->retrieve($hold->requestor)->to_fieldmapper,
+                               })->gather(1)
                );
                $self->{client}->status( new OpenSRF::DomainObject::oilsContinueStatus );
        }
 
-       @copies = grep { defined $_ } @copies;
+       @copies = grep { $_ } @copies;
 
        my $count = @copies;
 
        return unless ($count);
        
-       $self->method_lookup('open-ils.storage.direct.action.hold_copy_map.mass_delete')->run( { hold => $hold->id } );
+       action::hold_copy_map->search( { hold => $hold->id } )->delete_all;
        
        my @maps;
+       $self->{client}->respond( "\tMapping ".scalar(@copies)." eligable copies for hold ".$hold->id."\n");
        for my $c (@copies) {
-               $self->{client}->respond( "\tCreating hold-copy map for ".$hold->id." <-> ".$c->id."\n");
-               my $m = new Fieldmapper::action::hold_copy_map;
-               $m->hold( $hold->id );
-               $m->target_copy( $c->id );
-               $m->isnew( 1 );
-               push @maps, $m;
+               push @maps, action::hold_copy_map->create( { hold => ''.$hold->id, target_copy => ''.$c->id } );
        }
 
-       $self->method_lookup('open-ils.storage.direct.action.hold_copy_map.batch.create')->run( @maps );
-
        return \@copies;
 }
 
@@ -626,10 +624,8 @@ sub volume_hold_capture {
 
        if (!defined($vols)) {
                try {
-                       ($vols) = $self->method_lookup('open-ils.storage.direct.asset.call_number.search.id.atomic')->run( $hold->target );
-       
+                       $vols = [ asset::call_number->search( id => $hold->target ) ];
                        $cache{cns}{$_->id} = $_ for (@$vols);
-
                } catch Error with {
                        my $e = shift;
                        die "Could not retrieve initial volume list:\n\n$e\n";
@@ -640,7 +636,7 @@ sub volume_hold_capture {
 
        my $cp_list;
        try {
-               ($cp_list) = $self->method_lookup('open-ils.storage.direct.asset.copy.search.call_number.atomic')->run( \@v_ids );
+               $cp_list = [ asset::copy->search( call_number => \@v_ids ) ];
        
        } catch Error with {
                my $e = shift;
@@ -657,10 +653,8 @@ sub title_hold_capture {
 
        if (!defined($titles)) {
                try {
-                       ($titles) = $self->method_lookup('open-ils.storage.direct.biblio.record_entry.search.id.atomic')->run( $hold->target );
-       
+                       $titles = [ biblio::record_entry->search( id => $hold->target ) ];
                        $cache{titles}{$_->id} = $_ for (@$titles);
-       
                } catch Error with {
                        my $e = shift;
                        die "Could not retrieve initial title list:\n\n$e\n";
@@ -688,7 +682,7 @@ sub metarecord_hold_capture {
 
        my $titles;
        try {
-               ($titles) = $self->method_lookup('open-ils.storage.ordered.metabib.metarecord.records.atomic')->run( $hold->target );
+               $titles = [ metabib::metarecord_source_map->search( metarecord => $hold->target) ];
        
        } catch Error with {
                my $e = shift;
@@ -696,12 +690,9 @@ sub metarecord_hold_capture {
        };
 
        try {
-               my @recs = map {$_->record}
-                               $self->method_lookup('open-ils.storage.direct.metabib.record_descriptor.search')
-                                               ->run( record => $titles, item_type => [split '', $hold->holdable_formats] ); 
+               my @recs = map {$_->record} metabib::record_descriptor->search( record => $titles, item_type => [split '', $hold->holdable_formats] ); 
 
-               $titles = [];
-               ($titles) = $self->method_lookup('open-ils.storage.direct.biblio.record_entry.search.id.atomic')->run( \@recs );
+               $titles = [ biblio::record_entry->search( id => \@recs ) ];
        
        } catch Error with {
                my $e = shift;
@@ -710,7 +701,6 @@ sub metarecord_hold_capture {
 
 
        $cache{titles}{$_->id} = $_ for (@$titles);
-
        $self->title_hold_capture($hold,$titles) if (ref $titles and @$titles);
 }