From 486a821fb0e1cf56e02ddda6a157121a13de1697 Mon Sep 17 00:00:00 2001 From: Matthias Meusburger <matthias.meusburger@biblibre.com> Date: Wed, 10 Jul 2024 14:06:15 +0000 Subject: [PATCH] Bug 35659: (QA follow-up): Auth unit tests & dbrev Add unit tests for authorities and deletion Fix dbrev and kohastructure discrepancies --- Koha/Import/OAI/Authorities.pm | 2 +- Koha/Import/OAI/Authority.pm | 2 +- Koha/OAI/Client/Harvester.pm | 3 - admin/oai_servers.pl | 6 +- .../data/mysql/atomicupdate/Bug-35659.pl | 6 +- installer/data/mysql/kohastructure.sql | 12 +- .../prog/en/modules/admin/oai_servers.tt | 16 +- t/db_dependent/Koha/OAIHarvester.t | 153 +++++++++++++++++- 8 files changed, 170 insertions(+), 30 deletions(-) diff --git a/Koha/Import/OAI/Authorities.pm b/Koha/Import/OAI/Authorities.pm index c66daa6816..cdff207854 100644 --- a/Koha/Import/OAI/Authorities.pm +++ b/Koha/Import/OAI/Authorities.pm @@ -36,7 +36,7 @@ This object represents a collection of OAI-PMH records being imported as authori =cut sub _type { - return 'ImportOAIAuthority'; + return 'ImportOaiAuthority'; } =head3 object_class diff --git a/Koha/Import/OAI/Authority.pm b/Koha/Import/OAI/Authority.pm index 8f44b235f4..f4e3a0d31f 100644 --- a/Koha/Import/OAI/Authority.pm +++ b/Koha/Import/OAI/Authority.pm @@ -35,7 +35,7 @@ This object represents an OAI-PMH record being imported as an authority =cut sub _type { - return 'ImportOAIAuthority'; + return 'ImportOaiAuthority'; } 1; diff --git a/Koha/OAI/Client/Harvester.pm b/Koha/OAI/Client/Harvester.pm index 462f56e06f..779632824d 100644 --- a/Koha/OAI/Client/Harvester.pm +++ b/Koha/OAI/Client/Harvester.pm @@ -32,7 +32,6 @@ use open qw( :std :utf8 ); use C4::Biblio qw( AddBiblio GetFrameworkCode ModBiblio DelBiblio ); use C4::AuthoritiesMarc qw (AddAuthority GuessAuthTypeCode ModAuthority DelAuthority ); -use C4::Log qw( cronlogaction ); use HTTP::OAI; use HTTP::OAI::Metadata::OAI_DC; use Koha::DateUtils qw( dt_from_string ); @@ -276,14 +275,12 @@ sub processRecord { "Record " . $oai_record->identifier . " not deleted, biblionumber: $biblionumber ($error)" ); $status = 'in_error'; } else { - $imported_record->delete; $self->printlog( "Record " . $oai_record->identifier . " deleted, biblionumber: $biblionumber" ); $status = 'deleted'; } } else { my $authid = $imported_record->authid; DelAuthority( { authid => $authid } ); - $imported_record->delete; $self->printlog( "Record " . $oai_record->identifier . " deleted, authid: $authid" ); $status = 'deleted'; } diff --git a/admin/oai_servers.pl b/admin/oai_servers.pl index af35b14110..3edb2e118c 100755 --- a/admin/oai_servers.pl +++ b/admin/oai_servers.pl @@ -83,7 +83,7 @@ if ( $op eq 'cud-delete_confirmed' && $id ) { } } elsif ( $op eq 'search' ) { - #use searchfield only in remaining operations + # use searchfield only in remaining operations $searchfield = $input->param('searchfield') || ''; } @@ -108,11 +108,11 @@ output_html_with_http_headers $input, $cookie, $template->output; # End of main code -sub server_search { #find server(s) by id or name +sub server_search { # find server(s) by id or name my ( $schema, $id, $searchstring ) = @_; return Koha::OAIServers->search( - $id ? { id => $id } : { servername => { like => $searchstring . '%' } }, + $id ? { oai_server_id => $id } : { servername => { like => $searchstring . '%' } }, )->unblessed; } diff --git a/installer/data/mysql/atomicupdate/Bug-35659.pl b/installer/data/mysql/atomicupdate/Bug-35659.pl index c637027d45..584d5c4869 100755 --- a/installer/data/mysql/atomicupdate/Bug-35659.pl +++ b/installer/data/mysql/atomicupdate/Bug-35659.pl @@ -16,7 +16,7 @@ return { `dataformat` enum('oai_dc','marc-xml', 'marcxml') NOT NULL DEFAULT 'oai_dc' COMMENT 'data format', `recordtype` enum('authority','biblio') NOT NULL DEFAULT 'biblio' COMMENT 'server contains bibliographic or authority records', `add_xslt` longtext DEFAULT NULL COMMENT 'zero or more paths to XSLT files to be processed on the search results', - PRIMARY KEY (`id`) + PRIMARY KEY (`oai_server_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; } ); @@ -32,7 +32,7 @@ return { `recordtype` enum('authority','biblio') NOT NULL DEFAULT 'biblio' COMMENT 'is the record bibliographic or authority', `datestamp` varchar(255) DEFAULT NULL COMMENT 'OAI set to harvest', `last_modified` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, - PRIMARY KEY (id), + PRIMARY KEY (`import_oai_biblio_id`), KEY biblionumber (biblionumber), CONSTRAINT FK_import_oai_biblios_1 FOREIGN KEY (biblionumber) REFERENCES biblio (biblionumber) ON DELETE CASCADE ON UPDATE NO ACTION ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; @@ -49,7 +49,7 @@ return { `recordtype` enum('authority','biblio') NOT NULL DEFAULT 'biblio' COMMENT 'is the record bibliographic or authority', `datestamp` varchar(255) DEFAULT NULL COMMENT 'OAI set to harvest', `last_modified` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, - PRIMARY KEY (id), + PRIMARY KEY (`import_oai_authority_id`), KEY authid (authid), CONSTRAINT FK_import_oai_authorities_1 FOREIGN KEY (authid) REFERENCES auth_header (authid) ON DELETE CASCADE ON UPDATE NO ACTION ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 0a43745c97..cccf7fe3f7 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -4859,14 +4859,14 @@ DROP TABLE IF EXISTS `oai_servers`; /*!40101 SET @saved_cs_client = @@character_set_client */; /*!40101 SET character_set_client = utf8 */; CREATE TABLE `oai_servers` ( - `id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'unique identifier assigned by Koha', + `oai_server_id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'unique identifier assigned by Koha', `endpoint` varchar(255) NOT NULL COMMENT 'OAI endpoint (host + port + path)', `oai_set` varchar(255) DEFAULT NULL COMMENT 'OAI set to harvest', `servername` longtext NOT NULL COMMENT 'name given to the target by the library', `dataformat` enum('oai_dc','marc-xml', 'marcxml') NOT NULL DEFAULT 'oai_dc' COMMENT 'data format', `recordtype` enum('authority','biblio') NOT NULL DEFAULT 'biblio' COMMENT 'server contains bibliographic or authority records', `add_xslt` longtext DEFAULT NULL COMMENT 'zero or more paths to XSLT files to be processed on the search results', - PRIMARY KEY (`id`) + PRIMARY KEY (`oai_server_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; -- @@ -4877,14 +4877,14 @@ DROP TABLE IF EXISTS `import_oai_biblios`; /*!40101 SET @saved_cs_client = @@character_set_client */; /*!40101 SET character_set_client = utf8 */; CREATE TABLE `import_oai_biblios` ( - `id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'unique identifier assigned by Koha', + `import_oai_biblio_id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'unique identifier assigned by Koha', `biblionumber` int(11) NOT NULL COMMENT 'unique identifier assigned to each koha record', `identifier` varchar(255) NOT NULL COMMENT 'OAI record identifier', `repository` varchar(255) NOT NULL COMMENT 'OAI repository', `recordtype` enum('authority','biblio') NOT NULL DEFAULT 'biblio' COMMENT 'is the record bibliographic or authority', `datestamp` varchar(255) DEFAULT NULL COMMENT 'OAI set to harvest', `last_modified` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, - PRIMARY KEY (id), + PRIMARY KEY (import_oai_biblio_id), KEY biblionumber (biblionumber), CONSTRAINT FK_import_oai_biblios_1 FOREIGN KEY (biblionumber) REFERENCES biblio (biblionumber) ON DELETE CASCADE ON UPDATE NO ACTION ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; @@ -4897,14 +4897,14 @@ DROP TABLE IF EXISTS `import_oai_authorities`; /*!40101 SET @saved_cs_client = @@character_set_client */; /*!40101 SET character_set_client = utf8 */; CREATE TABLE `import_oai_authorities` ( - `id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'unique identifier assigned by Koha', + `import_oai_authority_id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'unique identifier assigned by Koha', `authid` bigint(20) unsigned NOT NULL COMMENT 'unique identifier assigned to each koha record', `identifier` varchar(255) NOT NULL COMMENT 'OAI record identifier', `repository` varchar(255) NOT NULL COMMENT 'OAI repository', `recordtype` enum('authority','biblio') NOT NULL DEFAULT 'biblio' COMMENT 'is the record bibliographic or authority', `datestamp` varchar(255) DEFAULT NULL COMMENT 'OAI set to harvest', `last_modified` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, - PRIMARY KEY (id), + PRIMARY KEY (import_oai_authority_id), KEY authid (authid), CONSTRAINT FK_import_oai_authorities_1 FOREIGN KEY (authid) REFERENCES auth_header (authid) ON DELETE CASCADE ON UPDATE NO ACTION ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/oai_servers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/oai_servers.tt index 07751b636e..8603e83ccc 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/oai_servers.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/oai_servers.tt @@ -70,7 +70,7 @@ [% INCLUDE 'csrf-token.inc' %] [% IF op == 'edit' %] <h1>Modify OAI repository</h1> - <input type="hidden" name="id" value="[% server.id | html %]" /> + <input type="hidden" name="id" value="[% server.oai_server_id | html %]" /> [% ELSE %] <h1>New OAI repository</h1> [% END %] @@ -138,8 +138,8 @@ <tbody> [% FOREACH loo IN loop %] <tr> - <td>[% loo.id | html %]</td> - <td><a href="/cgi-bin/koha/admin/oai_servers.pl?op=edit&id=[% loo.id | uri %]">[% loo.servername | html %]</a></td><td>[% loo.endpoint | html %]</td><td>[% loo.oai_set | html %]</td> + <td>[% loo.oai_server_id | html %]</td> + <td><a href="/cgi-bin/koha/admin/oai_servers.pl?op=edit&id=[% loo.oai_server_id | uri %]">[% loo.servername | html %]</a></td><td>[% loo.endpoint | html %]</td><td>[% loo.oai_set | html %]</td> <td>[% loo.dataformat | html %]</td> <td>[% IF ( loo.recordtype == 'biblio' ) %] <span>Bibliographic</span> @@ -153,14 +153,14 @@ Actions <b class="caret"></b> </a> <ul class="dropdown-menu pull-right" role="menu" aria-labelledby="reportactions[% savedreport.id | html %]"> - <li><a href="/cgi-bin/koha/admin/oai_servers.pl?op=edit&id=[% loo.id | uri %]"><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit</a></li> - <li><a href="/cgi-bin/koha/admin/oai_servers.pl?op=add&id=[% loo.id | uri %]"><i class="fa fa-copy"></i> Copy</a></li> + <li><a href="/cgi-bin/koha/admin/oai_servers.pl?op=edit&id=[% loo.oai_server_id | uri %]"><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit</a></li> + <li><a href="/cgi-bin/koha/admin/oai_servers.pl?op=add&id=[% loo.oai_server_id | uri %]"><i class="fa fa-copy"></i> Copy</a></li> <li> - <a href="#" class="delete" data-formid="server_row_[% loo.id | uri %]" data-servername="[% loo.servername | html %]"><i class="fa fa-trash-can"></i> Delete</a> - <form id="server_row_[% loo.id | uri %]" action="/cgi-bin/koha/admin/oai_servers.pl" method="post"> + <a href="#" class="delete" data-formid="server_row_[% loo.oai_server_id | uri %]" data-servername="[% loo.servername | html %]"><i class="fa fa-trash-can"></i> Delete</a> + <form id="server_row_[% loo.oai_server_id | uri %]" action="/cgi-bin/koha/admin/oai_servers.pl" method="post"> [% INCLUDE 'csrf-token.inc' %] <input type="hidden" name="op" value="cud-delete_confirmed"> - <input type="hidden" name="id" value="[% loo.id | uri %]"> + <input type="hidden" name="id" value="[% loo.oai_server_id | uri %]"> </form> </li> diff --git a/t/db_dependent/Koha/OAIHarvester.t b/t/db_dependent/Koha/OAIHarvester.t index 57c1d30be5..c8ecd701b4 100755 --- a/t/db_dependent/Koha/OAIHarvester.t +++ b/t/db_dependent/Koha/OAIHarvester.t @@ -18,8 +18,9 @@ # along with Koha; if not, see <http://www.gnu.org/licenses>. use Modern::Perl; -use Test::More tests => 9; +use Test::More tests => 24; use Test::Exception; +use File::Temp qw/tempfile/; use t::lib::TestBuilder; use HTTP::OAI; @@ -36,7 +37,7 @@ my $builder = t::lib::TestBuilder->new; $schema->storage->txn_begin; -my $new_oai_1 = Koha::OAIServer->new( +my $new_oai_biblio = Koha::OAIServer->new( { endpoint => 'my_host1.org', oai_set => 'set1', @@ -47,7 +48,7 @@ my $new_oai_1 = Koha::OAIServer->new( } )->store; -my $harvester = Koha::OAI::Client::Harvester->new( { server => $new_oai_1, verbose => 1, days => 1, force => 1 } ); +my $harvester = Koha::OAI::Client::Harvester->new( { server => $new_oai_biblio, verbose => 1, days => 1, force => 1 } ); my $record = '<metadata xmlns="http://www.openarchives.org/OAI/2.0/"><oai_dc:dc xmlns:oai_dc="http://www.openarchives.org/OAI/2.0/oai_dc/" xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.openarchives.org/OAI/2.0/oai_dc/ http://www.openarchives.org/OAI/2.0/oai_dc.xsd"><dc:title>Pièces diverses </dc:title><dc:identifier>ARCH/0320 [cote]</dc:identifier><dc:relation>FR-920509801 [RCR établissement]</dc:relation><dc:relation>Central obrera boliviana [Fonds ou collection]</dc:relation><dc:format>1 carton</dc:format><dc:date>1971/2000</dc:date><dc:type>Archives et manuscrits</dc:type></oai_dc:dc></metadata>'; @@ -77,7 +78,7 @@ is( $status, 'added', 'Record is added' ); $status = $harvester->processRecord($r); is( $status, 'updated', 'When force is used, record is updated' ); -$harvester = Koha::OAI::Client::Harvester->new( { server => $new_oai_1, verbose => 1, days => 1, force => 0 } ); +$harvester = Koha::OAI::Client::Harvester->new( { server => $new_oai_biblio, verbose => 1, days => 1, force => 0 } ); $status = $harvester->processRecord($r); is( $status, 'skipped', 'When force is not used, record is skipped (already up to date)' ); @@ -86,7 +87,13 @@ $status = $harvester->processRecord($r); is( $status, 'updated', 'When force is not used, record is updated if newer' ); my $imported_record = Koha::Import::OAI::Biblios->find( { identifier => 'oai:myarchive.org:oid-234' } ); -my $added_datestamp = $imported_record->datestamp; +my $added_datestamp = '2017-05-10T09:18:13Z'; +$imported_record->update( + { + datestamp => $added_datestamp, + } +); + $r->header->datestamp(undef); $status = $harvester->processRecord($r); $imported_record = Koha::Import::OAI::Biblios->find( { identifier => 'oai:myarchive.org:oid-234' } ); @@ -96,4 +103,140 @@ isnt( 'local datestamp is updated even if there is no datestamp in incoming record' ); +$r->header->status('deleted'); +$status = $harvester->processRecord($r); +is( $status, 'deleted', 'When a record is marked to be deleted, status is deleted' ); + +$imported_record = Koha::Import::OAI::Biblios->find( { identifier => 'oai:myarchive.org:oid-234' } ); +is( $imported_record, undef, 'Record has been deleted' ); + +$status = $harvester->processRecord($r); +is( $status, 'skipped', 'Status is skipped for already deleted record' ); + +# Authorities +my $file = xsl_file(); +my $new_oai_auth = Koha::OAIServer->new( + { + endpoint => 'my_host1.org', + oai_set => 'set1', + servername => 'my_test_1', + dataformat => 'oai_dc', + recordtype => 'authority', + add_xslt => $file, + } +)->store; + +$harvester = Koha::OAI::Client::Harvester->new( { server => $new_oai_auth, verbose => 1, days => 1, force => 1 } ); + +my $auth = + '<metadata xmlns="http://www.openarchives.org/OAI/2.0/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"><oai_dc:dc xmlns:oai_dc="http://www.openarchives.org/OAI/2.0/oai_dc/" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:dc="http://purl.org/dc/elements/1.1/" xsi:schemaLocation="http://www.openarchives.org/OAI/2.0/oai_dc/ http://www.openarchives.org/OAI/2.0/oai_dc.xsd"><dc:FamilyName>Emerson</dc:FamilyName><dc:GivenName>Everett H.</dc:GivenName></oai_dc:dc></metadata>'; + +$r = HTTP::OAI::Record->new(); +$r->metadata( HTTP::OAI::Metadata->new( dom => $auth ) ); + +$status = $harvester->processRecord($r); +is( $status, 'skipped', 'Authority with no identifier is skipped' ); + +$r->header->identifier('oai:myarchive.org:oid-161'); +$status = $harvester->processRecord($r); +is( $status, 'added', 'Authority with no date is added' ); + +$status = $harvester->processRecord($r); +is( $status, 'updated', 'Authority with no date is updated' ); + +$status = $harvester->processRecord($r); +is( $status, 'updated', 'Authority with no date is updated even without force' ); + +$r->header->identifier('oai:myarchive.org:oid-162'); +$r->header->datestamp('2017-05-10T09:18:13Z'); + +$status = $harvester->processRecord($r); +is( $status, 'added', 'Authority is added' ); + +$status = $harvester->processRecord($r); +is( $status, 'updated', 'When force is used, authority is updated' ); + +$harvester = Koha::OAI::Client::Harvester->new( { server => $new_oai_auth, verbose => 1, days => 1, force => 0 } ); +$status = $harvester->processRecord($r); +is( $status, 'skipped', 'When force is not used, authority is skipped (already up to date)' ); + +$r->header->datestamp('2018-05-10T09:18:13Z'); +$status = $harvester->processRecord($r); +is( $status, 'updated', 'When force is not used, authority is updated if newer' ); + +my $imported_authority = Koha::Import::OAI::Authorities->find( { identifier => 'oai:myarchive.org:oid-162' } ); +$imported_authority->update( + { + datestamp => $added_datestamp, + } +); + +$r->header->datestamp(undef); + +$status = $harvester->processRecord($r); +$imported_authority = Koha::Import::OAI::Authorities->find( { identifier => 'oai:myarchive.org:oid-162' } ); +$updated_datestamp = $imported_authority->datestamp; +isnt( + $added_datestamp, $updated_datestamp, + 'local datestamp is updated even if there is no datestamp in incoming authority' +); + +$r->header->status('deleted'); +$status = $harvester->processRecord($r); +is( $status, 'deleted', 'When an authority is marked to be deleted, status is deleted' ); + +$imported_record = Koha::Import::OAI::Biblios->find( { identifier => 'oai:myarchive.org:oid-162' } ); +is( $imported_record, undef, 'Authority has been deleted' ); + +$status = $harvester->processRecord($r); +is( $status, 'skipped', 'Status is skipped for already deleted authority' ); + $schema->storage->txn_rollback; + +sub xsl_file { + return mytempfile( + q{<?xml version="1.0" encoding="UTF-8"?> +<xsl:stylesheet version="1.0" + xmlns:dc="http://purl.org/dc/elements/1.1/" + xmlns:dcterms="http://purl.org/dc/terms/1.1" + xmlns:oai_dc="http://www.openarchives.org/OAI/2.0/oai_dc/" + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation="http://www.openarchives.org/OAI/2.0/oai_dc/ + http://www.openarchives.org/OAI/2.0/oai_dc.xsd" + xmlns:xsl="http://www.w3.org/1999/XSL/Transform" + xmlns="http://www.loc.gov/MARC21/slim" exclude-result-prefixes="dc dcterms oai_dc"> + + <xsl:output method="xml" encoding="UTF-8" indent="yes"/> + + <xsl:template match="/"> + <collection xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd" > + <xsl:apply-templates /> + </collection> + </xsl:template> + + <xsl:template match="oai_dc:dc"> + <record> + + <xsl:variable name="FamilyName" select="dc:FamilyName"/> + <xsl:variable name="GivenName" select="dc:GivenName"/> + + <datafield tag="100" ind1="0" ind2=" "> + <subfield code="a"> + <xsl:value-of select="concat($FamilyName,', ',$GivenName)"/> + </subfield> + </datafield> + + </record> + </xsl:template> + +</xsl:stylesheet> + } + ); +} + +sub mytempfile { + my ( $fh, $fn ) = tempfile( SUFFIX => '.xsl', UNLINK => 1 ); + print $fh $_[0] // ''; + close $fh; + return $fn; +} -- 2.39.2