From c2a0ed8077ef5723338e61d99f3f453c46c3648e Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Thu, 3 Jan 2008 12:36:40 -0600 Subject: [PATCH] item rework: replaced AddBiblioAndItems Replace C4::Biblio::AddBiblioAndItems with two things: * An option to C4::Biblio::AddBiblio to defer writing biblioitems.marc and biblioitems.marcxml. This option was created to give a significant speed boost to bulkmarcimport.pl, but is *not* recommended for general use. * C4::Items::AddItemBatchFromMarc This refactoring removes the need to have functions in C4::Biblio and C4::Items that call each other's private functions. Signed-off-by: Chris Cormack Signed-off-by: Joshua Ferraro --- C4/Biblio.pm | 179 +++++-------------------- C4/Items.pm | 133 ++++++++++++++++++ misc/migration_tools/bulkmarcimport.pl | 22 ++- 3 files changed, 178 insertions(+), 156 deletions(-) diff --git a/C4/Biblio.pm b/C4/Biblio.pm index 6949c9d374..3871b128af 100755 --- a/C4/Biblio.pm +++ b/C4/Biblio.pm @@ -40,8 +40,10 @@ use vars qw($VERSION @ISA @EXPORT); # EXPORTED FUNCTIONS. -# to add biblios or items -push @EXPORT, qw( &AddBiblio &AddBiblioAndItems ); +# to add biblios +push @EXPORT, qw( + &AddBiblio +); # to get something push @EXPORT, qw( @@ -180,14 +182,37 @@ When modifying a biblio or an item, the behaviour is quite similar. =over 4 ($biblionumber,$biblioitemnumber) = AddBiblio($record,$frameworkcode); -Exported function (core API) for adding a new biblio to koha. =back +Exported function (core API) for adding a new biblio to koha. + +The first argument is a C object containing the +bib to add, while the second argument is the desired MARC +framework code. + +This function also accepts a third, optional argument: a hashref +to additional options. The only defined option is C, +which if present and mapped to a true value, causes C +to omit the call to save the MARC in C +and C This option is provided B +for the use of scripts such as C that may need +to do some manipulation of the MARC record for item parsing before +saving it and which cannot afford the performance hit of saving +the MARC record twice. Consequently, do not use that option +unless you can guarantee that C will be called. + =cut sub AddBiblio { - my ( $record, $frameworkcode ) = @_; + my $record = shift; + my $frameworkcode = shift; + my $options = @_ ? shift : undef; + my $defer_marc_save = 0; + if (defined $options and exists $options->{'defer_marc_save'} and $options->{'defer_marc_save'}) { + $defer_marc_save = 1; + } + my ($biblionumber,$biblioitemnumber,$error); my $dbh = C4::Context->dbh; # transform the data into koha-table style data @@ -199,7 +224,7 @@ sub AddBiblio { _koha_marc_update_bib_ids($record, $frameworkcode, $biblionumber, $biblioitemnumber); # now add the record - $biblionumber = ModBiblioMarc( $record, $biblionumber, $frameworkcode ); + $biblionumber = ModBiblioMarc( $record, $biblionumber, $frameworkcode ) unless $defer_marc_save; &logaction(C4::Context->userenv->{'number'},"CATALOGUING","ADD",$biblionumber,"biblio") if C4::Context->preference("CataloguingLog"); @@ -207,150 +232,6 @@ sub AddBiblio { return ( $biblionumber, $biblioitemnumber ); } -=head2 AddBiblioAndItems - -=over 4 - -($biblionumber,$biblioitemnumber, $itemnumber_ref, $error_ref) = AddBiblioAndItems($record, $frameworkcode); - -=back - -Efficiently add a biblio record and create item records from its -embedded item fields. This routine is suitable for batch jobs. - -The goal of this API is to have a similar effect to using AddBiblio -and AddItems in succession, but without inefficient repeated -parsing of the MARC XML bib record. - -One functional difference is that the duplicate item barcode -check is implemented in this API, instead of relying on -the caller to do it, like AddItem does. - -This function returns the biblionumber and biblioitemnumber of the -new bib, an arrayref of new itemsnumbers, and an arrayref of item -errors encountered during the processing. Each entry in the errors -list is a hashref containing the following keys: - -=over 2 - -=item item_sequence - -Sequence number of original item tag in the MARC record. - -=item item_barcode - -Item barcode, provide to assist in the construction of -useful error messages. - -=item error_condition - -Code representing the error condition. Can be 'duplicate_barcode', -'invalid_homebranch', or 'invalid_holdingbranch'. - -=item error_information - -Additional information appropriate to the error condition. - -=back - -=cut - -sub AddBiblioAndItems { - my ( $record, $frameworkcode ) = @_; - my ($biblionumber,$biblioitemnumber,$error); - my @itemnumbers = (); - my @errors = (); - my $dbh = C4::Context->dbh; - - # transform the data into koha-table style data - # FIXME - this paragraph copied from AddBiblio - my $olddata = TransformMarcToKoha( $dbh, $record, $frameworkcode ); - ($biblionumber,$error) = _koha_add_biblio( $dbh, $olddata, $frameworkcode ); - $olddata->{'biblionumber'} = $biblionumber; - ($biblioitemnumber,$error) = _koha_add_biblioitem( $dbh, $olddata ); - - # FIXME - this paragraph copied from AddBiblio - _koha_marc_update_bib_ids($record, $frameworkcode, $biblionumber, $biblioitemnumber); - - # now we loop through the item tags and start creating items - my @bad_item_fields = (); - my ($itemtag, $itemsubfield) = &GetMarcFromKohaField("items.itemnumber",''); - my $item_sequence_num = 0; - ITEMFIELD: foreach my $item_field ($record->field($itemtag)) { - $item_sequence_num++; - # we take the item field and stick it into a new - # MARC record -- this is required so far because (FIXME) - # TransformMarcToKoha requires a MARC::Record, not a MARC::Field - # and there is no TransformMarcFieldToKoha - my $temp_item_marc = MARC::Record->new(); - $temp_item_marc->append_fields($item_field); - - # add biblionumber and biblioitemnumber - my $item = TransformMarcToKoha( $dbh, $temp_item_marc, $frameworkcode, 'items' ); - $item->{'biblionumber'} = $biblionumber; - $item->{'biblioitemnumber'} = $biblioitemnumber; - - # check for duplicate barcode - my %item_errors = C4::Items::CheckItemPreSave($item); - if (%item_errors) { - push @errors, _repack_item_errors($item_sequence_num, $item, \%item_errors); - push @bad_item_fields, $item_field; - next ITEMFIELD; - } - - C4::Items::_set_defaults_for_add($item); - C4::Items::_set_derived_columns_for_add($item); - my ( $itemnumber, $error ) = C4::Items::_koha_new_item( $dbh, $item, $item->{barcode} ); - warn $error if $error; - push @itemnumbers, $itemnumber; # FIXME not checking error - $item->{'itemnumber'} = $itemnumber; - - &logaction(C4::Context->userenv->{'number'},"CATALOGUING","ADD",$itemnumber,"item") - if C4::Context->preference("CataloguingLog"); - - my $new_item_marc = C4::Items::_marc_from_item_hash($item, $frameworkcode); - $item_field->replace_with($new_item_marc->field($itemtag)); - } - - # remove any MARC item fields for rejected items - foreach my $item_field (@bad_item_fields) { - $record->delete_field($item_field); - } - - # now add the record - # FIXME - this paragraph copied from AddBiblio -- however, moved since - # since we need to create the items row and plug in the itemnumbers in the - # MARC - $biblionumber = ModBiblioMarc( $record, $biblionumber, $frameworkcode ); - - # FIXME - when using this API, do we log both bib and item add, or just - # bib - &logaction(C4::Context->userenv->{'number'},"CATALOGUING","ADD",$biblionumber,"biblio") - if C4::Context->preference("CataloguingLog"); - - return ( $biblionumber, $biblioitemnumber, \@itemnumbers, \@errors); - -} - -sub _repack_item_errors { - my $item_sequence_num = shift; - my $item_ref = shift; - my $error_ref = shift; - - my @repacked_errors = (); - - foreach my $error_code (sort keys %{ $error_ref }) { - my $repacked_error = {}; - $repacked_error->{'item_sequence'} = $item_sequence_num; - $repacked_error->{'item_barcode'} = exists($item_ref->{'barcode'}) ? $item_ref->{'barcode'} : ''; - $repacked_error->{'error_code'} = $error_code; - $repacked_error->{'error_information'} = $error_ref->{$error_code}; - push @repacked_errors, $repacked_error; - } - - return @repacked_errors; -} - =head2 ModBiblio ModBiblio( $record,$biblionumber,$frameworkcode); diff --git a/C4/Items.pm b/C4/Items.pm index 39b24f20a3..602f3dfac1 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -42,6 +42,7 @@ my $VERSION = 3.00; GetItem AddItemFromMarc AddItem + AddItemBatchFromMarc ModItemFromMarc ModItem ModDateLastSeen @@ -210,6 +211,112 @@ sub AddItem { return ($item->{biblionumber}, $item->{biblioitemnumber}, $itemnumber); } +=head2 AddItemBatchFromMarc + +=over 4 + +($itemnumber_ref, $error_ref) = AddItemBatchFromMarc($record, $biblionumber, $biblioitemnumber, $frameworkcode); + +=back + +Efficiently create item records from a MARC biblio record with +embedded item fields. This routine is suitable for batch jobs. + +This API assumes that the bib record has already been +saved to the C and C tables. It does +not expect that C and C +are populated, but it will do so via a call to ModBibiloMarc. + +The goal of this API is to have a similar effect to using AddBiblio +and AddItems in succession, but without inefficient repeated +parsing of the MARC XML bib record. + +This function returns an arrayref of new itemsnumbers and an arrayref of item +errors encountered during the processing. Each entry in the errors +list is a hashref containing the following keys: + +=over 2 + +=item item_sequence + +Sequence number of original item tag in the MARC record. + +=item item_barcode + +Item barcode, provide to assist in the construction of +useful error messages. + +=item error_condition + +Code representing the error condition. Can be 'duplicate_barcode', +'invalid_homebranch', or 'invalid_holdingbranch'. + +=item error_information + +Additional information appropriate to the error condition. + +=back + +=cut + +sub AddItemBatchFromMarc { + my ($record, $biblionumber, $biblioitemnumber, $frameworkcode) = @_; + my $error; + my @itemnumbers = (); + my @errors = (); + my $dbh = C4::Context->dbh; + + # loop through the item tags and start creating items + my @bad_item_fields = (); + my ($itemtag, $itemsubfield) = &GetMarcFromKohaField("items.itemnumber",''); + my $item_sequence_num = 0; + ITEMFIELD: foreach my $item_field ($record->field($itemtag)) { + $item_sequence_num++; + # we take the item field and stick it into a new + # MARC record -- this is required so far because (FIXME) + # TransformMarcToKoha requires a MARC::Record, not a MARC::Field + # and there is no TransformMarcFieldToKoha + my $temp_item_marc = MARC::Record->new(); + $temp_item_marc->append_fields($item_field); + + # add biblionumber and biblioitemnumber + my $item = TransformMarcToKoha( $dbh, $temp_item_marc, $frameworkcode, 'items' ); + $item->{'biblionumber'} = $biblionumber; + $item->{'biblioitemnumber'} = $biblioitemnumber; + + # check for duplicate barcode + my %item_errors = CheckItemPreSave($item); + if (%item_errors) { + push @errors, _repack_item_errors($item_sequence_num, $item, \%item_errors); + push @bad_item_fields, $item_field; + next ITEMFIELD; + } + + _set_defaults_for_add($item); + _set_derived_columns_for_add($item); + my ( $itemnumber, $error ) = _koha_new_item( $dbh, $item, $item->{barcode} ); + warn $error if $error; + push @itemnumbers, $itemnumber; # FIXME not checking error + $item->{'itemnumber'} = $itemnumber; + + &logaction(C4::Context->userenv->{'number'},"CATALOGUING","ADD",$itemnumber,"item") + if C4::Context->preference("CataloguingLog"); + + my $new_item_marc = _marc_from_item_hash($item, $frameworkcode); + $item_field->replace_with($new_item_marc->field($itemtag)); + } + + # remove any MARC item fields for rejected items + foreach my $item_field (@bad_item_fields) { + $record->delete_field($item_field); + } + + # update the MARC biblio + $biblionumber = ModBiblioMarc( $record, $biblionumber, $frameworkcode ); + + return (\@itemnumbers, \@errors); +} + =head2 ModItemFromMarc =over 4 @@ -1724,4 +1831,30 @@ sub _replace_item_field_in_biblio { ModBiblioMarc($completeRecord, $biblionumber, $frameworkcode); } +=head2 _repack_item_errors + +Add an error message hash generated by C +to a list of errors. + +=cut + +sub _repack_item_errors { + my $item_sequence_num = shift; + my $item_ref = shift; + my $error_ref = shift; + + my @repacked_errors = (); + + foreach my $error_code (sort keys %{ $error_ref }) { + my $repacked_error = {}; + $repacked_error->{'item_sequence'} = $item_sequence_num; + $repacked_error->{'item_barcode'} = exists($item_ref->{'barcode'}) ? $item_ref->{'barcode'} : ''; + $repacked_error->{'error_code'} = $error_code; + $repacked_error->{'error_information'} = $error_ref->{$error_code}; + push @repacked_errors, $repacked_error; + } + + return @repacked_errors; +} + 1; diff --git a/misc/migration_tools/bulkmarcimport.pl b/misc/migration_tools/bulkmarcimport.pl index c41baf1a94..73833e5753 100755 --- a/misc/migration_tools/bulkmarcimport.pl +++ b/misc/migration_tools/bulkmarcimport.pl @@ -215,7 +215,6 @@ $commitnum = $commit; } -my $dbh = C4::Context->dbh(); $dbh->{AutoCommit} = 0; RECORD: while ( my $record = $batch->next() ) { $i++; @@ -230,13 +229,22 @@ RECORD: while ( my $record = $batch->next() ) { } unless ($test_parameter) { - my ( $bibid, $oldbibitemnum, $itemnumbers_ref, $errors_ref ); - eval { ( $bibid, $oldbibitemnum, $itemnumbers_ref, $errors_ref ) = AddBiblioAndItems( $record, '' ); }; + my ( $biblionumber, $biblioitemnumber, $itemnumbers_ref, $errors_ref ); + eval { ( $biblionumber, $biblioitemnumber ) = AddBiblio($record, '', { defer_marc_save => 1 }) }; if ( $@ ) { - warn "ERROR: Adding biblio and or items $bibid failed: $@\n"; + warn "ERROR: Adding biblio $biblionumber failed: $@\n"; + next RECORD; + } + eval { ( $itemnumbers_ref, $errors_ref ) = AddItemBatchFromMarc( $record, $biblionumber, $biblioitemnumber, '' ); }; + if ( $@ ) { + warn "ERROR: Adding items to bib $biblionumber failed: $@\n"; + # if we failed because of an exception, assume that + # the MARC columns in biblioitems were not set. + ModBiblioMarc( $record, $biblionumber, '' ); + next RECORD; } if ($#{ $errors_ref } > -1) { - report_item_errors($bibid, $errors_ref); + report_item_errors($biblionumber, $errors_ref); } $dbh->commit() if (0 == $i % $commitnum); @@ -259,11 +267,11 @@ print "$i MARC records done in $timeneeded seconds\n"; exit 0; sub report_item_errors { - my $bibid = shift; + my $biblionumber = shift; my $errors_ref = shift; foreach my $error (@{ $errors_ref }) { - my $msg = "Item not added (bib $bibid, item tag #$error->{'item_sequence'}, barcode $error->{'item_barcode'}): "; + my $msg = "Item not added (bib $biblionumber, item tag #$error->{'item_sequence'}, barcode $error->{'item_barcode'}): "; my $error_code = $error->{'error_code'}; $error_code =~ s/_/ /g; $msg .= "$error_code $error->{'error_information'}"; -- 2.39.5