@@ -, +, @@ --- Koha/OAI/Server/ListBase.pm | 30 +++++++++---------- Koha/OAI/Server/ResumptionToken.pm | 24 ++++++++------- t/db_dependent/OAI/Server.t | 47 ++++++++++++++++-------------- 3 files changed, 53 insertions(+), 48 deletions(-) --- a/Koha/OAI/Server/ListBase.pm +++ a/Koha/OAI/Server/ListBase.pm @@ -45,13 +45,14 @@ sub GetRecords { if ( defined $token->{'set'} ) { $set = GetOAISetBySpec($token->{'set'}); } - my $offset = $token->{offset}; my $deleted = defined $token->{deleted} ? $token->{deleted} : 0; - my $deleted_count = defined $token->{deleted_count} ? $token->{deleted_count} : 0; my $max = $repository->{koha_max_count}; my $count = 0; my $format = $args{metadataPrefix} || $token->{metadata_prefix}; my $include_items = $repository->items_included( $format ); + my $from = $token->{from_arg}; + my $until = $token->{until_arg}; + my $next_id = $token->{next_id}; # Since creating a union of normal and deleted record tables would be a heavy # operation in a large database, build results in two stages: @@ -62,20 +63,20 @@ sub GetRecords { my $sql = " SELECT biblionumber FROM $table - WHERE (timestamp >= ? AND timestamp <= ?) + WHERE (biblionumber >= ? AND timestamp >= ? AND timestamp <= ?) "; - my @bind_params = ($token->{'from_arg'}, $token->{'until_arg'}); + my @bind_params = ($next_id, $from, $until); if ($include_items) { $sql .= " - OR biblionumber IN (SELECT biblionumber from deleteditems WHERE timestamp >= ? AND timestamp <= ?) + OR biblionumber IN (SELECT biblionumber from deleteditems WHERE biblionumber >= ? AND timestamp >= ? AND timestamp <= ?) "; - push @bind_params, ($token->{'from_arg'}, $token->{'until_arg'}); + push @bind_params, ($next_id, $from, $until); if (!$deleted) { $sql .= " - OR biblionumber IN (SELECT biblionumber from items WHERE timestamp >= ? AND timestamp <= ?) + OR biblionumber IN (SELECT biblionumber from items WHERE biblionumber >= ? AND timestamp >= ? AND timestamp <= ?) "; - push @bind_params, ($token->{'from_arg'}, $token->{'until_arg'}); + push @bind_params, ($next_id, $from, $until); } } @@ -93,9 +94,7 @@ sub GetRecords { push @bind_params, $set->{'id'}; } - $sql .= " - LIMIT " . ($max + 1) . " - OFFSET " . ($offset - $deleted_count); + $sql .= " LIMIT " . ($max + 1); my $sth = $dbh->prepare( $sql ) || die( 'Could not prepare statement: ' . $dbh->errstr ); @@ -131,10 +130,10 @@ sub GetRecords { metadataPrefix => $token->{metadata_prefix}, from => $token->{from}, until => $token->{until}, - offset => $token->{offset} + $max, + cursor => $token->{cursor} + $max, set => $token->{set}, deleted => $deleted, - deleted_count => $deleted_count + next_id => $biblionumber ) ); last STAGELOOP; @@ -172,9 +171,8 @@ sub GetRecords { ) ); } } - # Store offset and deleted record count - $offset += $count; - $deleted_count = $offset if ($deleted); + # Reset $next_id for the next stage + $next_id = 0; } return $count; } --- a/Koha/OAI/Server/ResumptionToken.pm +++ a/Koha/OAI/Server/ResumptionToken.pm @@ -1,6 +1,6 @@ # Copyright Tamil s.a.r.l. 2008-2015 # Copyright Biblibre 2008-2015 -# Copyright The National Library of Finland, University of Helsinki 2016-2017 +# Copyright The National Library of Finland, University of Helsinki 2016-2021 # # This file is part of Koha. # @@ -31,17 +31,20 @@ use base ("HTTP::OAI::ResumptionToken"); # - metadataPrefix # - from # - until -# - offset +# - cursor # - deleted +# - deleted count (not used anymore, but preserved for back-compatibility) +# - nextId sub new { my ($class, %args) = @_; my $self = $class->SUPER::new(%args); - my ($metadata_prefix, $offset, $from, $until, $set, $deleted, $deleted_count); + my ($metadata_prefix, $cursor, $from, $until, $set, $deleted, $next_id); if ( $args{ resumptionToken } ) { - ($metadata_prefix, $offset, $from, $until, $set, $deleted, $deleted_count) + # Note: undef in place of deleted record count for back-compatibility + ($metadata_prefix, $cursor, $from, $until, $set, $deleted, undef, $next_id) = split( '/', $args{resumptionToken} ); } else { @@ -55,25 +58,26 @@ sub new { #Add times to the arguments, when necessary, so they correctly match against the DB timestamps $from .= 'T00:00:00Z' if length($from) == 10; $until .= 'T23:59:59Z' if length($until) == 10; - $offset = $args{ offset } || 0; + $cursor = $args{ cursor } // 0; $set = $args{ set } || ''; $deleted = defined $args{ deleted } ? $args{ deleted } : 1; - $deleted_count = defined $args{ deleted_count } ? $args{ deleted_count } : 0; + $next_id = $args{ next_id } // 0; } $self->{ metadata_prefix } = $metadata_prefix; - $self->{ offset } = $offset; + $self->{ cursor } = $cursor; $self->{ from } = $from; $self->{ until } = $until; $self->{ set } = $set; $self->{ from_arg } = _strip_UTC_designators($from); $self->{ until_arg } = _strip_UTC_designators($until); $self->{ deleted } = $deleted; - $self->{ deleted_count } = $deleted_count; + $self->{ next_id } = $next_id // 0; + # Note: put zero where deleted record count used to be for back-compatibility $self->resumptionToken( - join( '/', $metadata_prefix, $offset, $from, $until, $set, $deleted, $deleted_count ) ); - $self->cursor( $offset ); + join( '/', $metadata_prefix, $cursor, $from, $until, $set, $deleted, 0, $next_id // 0 ) ); + $self->cursor( $cursor ); return $self; } --- a/t/db_dependent/OAI/Server.t +++ a/t/db_dependent/OAI/Server.t @@ -18,6 +18,7 @@ # along with Koha; if not, see . use Modern::Perl; +use Test::Deep qw( cmp_deeply re ); use Test::MockTime qw/set_fixed_time restore_time/; use Test::More tests => 31; @@ -74,12 +75,13 @@ $dbh->do('DELETE FROM oai_sets'); set_fixed_time(CORE::time()); -my $base_datetime = dt_from_string(); +my $base_datetime = dt_from_string(undef, undef, 'UTC'); my $date_added = $base_datetime->ymd . ' ' .$base_datetime->hms . 'Z'; my $date_to = substr($date_added, 0, 10) . 'T23:59:59Z'; my (@header, @marcxml, @oaidc, @marcxml_transformed); my $sth = $dbh->prepare('UPDATE biblioitems SET timestamp=? WHERE biblionumber=?'); my $sth2 = $dbh->prepare('UPDATE biblio_metadata SET timestamp=? WHERE biblionumber=?'); +my $first_bn = 0; # Add biblio records foreach my $index ( 0 .. NUMBER_OF_MARC_RECORDS - 1 ) { @@ -94,6 +96,7 @@ foreach my $index ( 0 .. NUMBER_OF_MARC_RECORDS - 1 ) { $record->append_fields( MARC::Field->new('952', '', '', 'a' => "Code" ) ); } my ($biblionumber) = AddBiblio($record, ''); + $first_bn = $biblionumber unless $first_bn; my $timestamp = $base_datetime->ymd . ' ' .$base_datetime->hms; $sth->execute($timestamp,$biblionumber); $sth2->execute($timestamp,$biblionumber); @@ -174,7 +177,7 @@ sub test_query { } delete $response->{responseDate}; - unless (is_deeply($response, \%full_expected, $test)) { + unless (cmp_deeply($response, \%full_expected, $test)) { diag "PARAM:" . Dump($param) . "EXPECTED:" . Dump(\%full_expected) . @@ -215,7 +218,7 @@ test_query('ListIdentifiers', {verb => 'ListIdentifiers', metadataPrefix => 'mar ListIdentifiers => { header => [ @header[0..2] ], resumptionToken => { - content => "marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0", + content => re( qr{^marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0/\d+$} ), cursor => 3, }, }, @@ -225,7 +228,7 @@ test_query('ListIdentifiers', {verb => 'ListIdentifiers', metadataPrefix => 'mar ListIdentifiers => { header => [ @header[0..2] ], resumptionToken => { - content => "marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0", + content => re( qr{^marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0/\d+$} ), cursor => 3, }, }, @@ -233,12 +236,12 @@ test_query('ListIdentifiers', {verb => 'ListIdentifiers', metadataPrefix => 'mar test_query( 'ListIdentifiers with resumptionToken 1', - { verb => 'ListIdentifiers', resumptionToken => "marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0" }, + { verb => 'ListIdentifiers', resumptionToken => "marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0/" . ($first_bn + 3) }, { ListIdentifiers => { header => [ @header[3..5] ], resumptionToken => { - content => "marcxml/6/1970-01-01T00:00:00Z/$date_to//0/0", + content => re( qr{^marcxml/6/1970-01-01T00:00:00Z/$date_to//0/0/\d+$} ), cursor => 6, }, }, @@ -247,12 +250,12 @@ test_query( test_query( 'ListIdentifiers with resumptionToken 2', - { verb => 'ListIdentifiers', resumptionToken => "marcxml/6/1970-01-01T00:00:00Z/$date_to//0/0" }, + { verb => 'ListIdentifiers', resumptionToken => "marcxml/6/1970-01-01T00:00:00Z/$date_to//0/0/" . ($first_bn + 6) }, { ListIdentifiers => { header => [ @header[6..8] ], resumptionToken => { - content => "marcxml/9/1970-01-01T00:00:00Z/$date_to//0/0", + content => re( qr{^marcxml/9/1970-01-01T00:00:00Z/$date_to//0/0/\d+$} ), cursor => 9, }, }, @@ -261,7 +264,7 @@ test_query( test_query( 'ListIdentifiers with resumptionToken 3, response without resumption', - { verb => 'ListIdentifiers', resumptionToken => "marcxml/9/1970-01-01T00:00:00Z/$date_to//0/0" }, + { verb => 'ListIdentifiers', resumptionToken => "marcxml/9/1970-01-01T00:00:00Z/$date_to//0/0/" . ($first_bn + 9) }, { ListIdentifiers => { header => $header[9], @@ -280,7 +283,7 @@ test_query('ListRecords marcxml', {verb => 'ListRecords', metadataPrefix => 'mar ListRecords => { record => [ @marcxml[0..2] ], resumptionToken => { - content => "marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0", + content => re( qr{^marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0/\d+$} ), cursor => 3, }, }, @@ -288,11 +291,11 @@ test_query('ListRecords marcxml', {verb => 'ListRecords', metadataPrefix => 'mar test_query( 'ListRecords marcxml with resumptionToken 1', - { verb => 'ListRecords', resumptionToken => "marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0" }, + { verb => 'ListRecords', resumptionToken => "marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0/" . ($first_bn + 3) }, { ListRecords => { record => [ @marcxml[3..5] ], resumptionToken => { - content => "marcxml/6/1970-01-01T00:00:00Z/$date_to//0/0", + content => re( qr{^marcxml/6/1970-01-01T00:00:00Z/$date_to//0/0/\d+$} ), cursor => 6, }, }, @@ -300,11 +303,11 @@ test_query( test_query( 'ListRecords marcxml with resumptionToken 2', - { verb => 'ListRecords', resumptionToken => "marcxml/6/1970-01-01T00:00:00Z/$date_to//0/0" }, + { verb => 'ListRecords', resumptionToken => "marcxml/6/1970-01-01T00:00:00Z/$date_to//0/0/" . ($first_bn + 6) }, { ListRecords => { record => [ @marcxml[6..8] ], resumptionToken => { - content => "marcxml/9/1970-01-01T00:00:00Z/$date_to//0/0", + content => re( qr{^marcxml/9/1970-01-01T00:00:00Z/$date_to//0/0/\d+$} ), cursor => 9, }, }, @@ -313,7 +316,7 @@ test_query( # Last record, so no resumption token test_query( 'ListRecords marcxml with resumptionToken 3, response without resumption', - { verb => 'ListRecords', resumptionToken => "marcxml/9/1970-01-01T00:00:00Z/$date_to//0/0" }, + { verb => 'ListRecords', resumptionToken => "marcxml/9/1970-01-01T00:00:00Z/$date_to//0/0/" . ($first_bn + 9) }, { ListRecords => { record => $marcxml[9], }, @@ -323,7 +326,7 @@ test_query('ListRecords oai_dc', {verb => 'ListRecords', metadataPrefix => 'oai_ ListRecords => { record => [ @oaidc[0..2] ], resumptionToken => { - content => "oai_dc/3/1970-01-01T00:00:00Z/$date_to//0/0", + content => re( qr{^oai_dc/3/1970-01-01T00:00:00Z/$date_to//0/0/\d+$} ), cursor => 3, }, }, @@ -331,11 +334,11 @@ test_query('ListRecords oai_dc', {verb => 'ListRecords', metadataPrefix => 'oai_ test_query( 'ListRecords oai_dc with resumptionToken 1', - { verb => 'ListRecords', resumptionToken => "oai_dc/3/1970-01-01T00:00:00Z/$date_to//0/0" }, + { verb => 'ListRecords', resumptionToken => "oai_dc/3/1970-01-01T00:00:00Z/$date_to//0/0/" . ($first_bn + 3) }, { ListRecords => { record => [ @oaidc[3..5] ], resumptionToken => { - content => "oai_dc/6/1970-01-01T00:00:00Z/$date_to//0/0", + content => re( qr{^oai_dc/6/1970-01-01T00:00:00Z/$date_to//0/0/\d+$} ), cursor => 6, }, }, @@ -343,11 +346,11 @@ test_query( test_query( 'ListRecords oai_dc with resumptionToken 2', - { verb => 'ListRecords', resumptionToken => "oai_dc/6/1970-01-01T00:00:00Z/$date_to//0/0" }, + { verb => 'ListRecords', resumptionToken => "oai_dc/6/1970-01-01T00:00:00Z/$date_to//0/0/" . ($first_bn + 6) }, { ListRecords => { record => [ @oaidc[6..8] ], resumptionToken => { - content => "oai_dc/9/1970-01-01T00:00:00Z/$date_to//0/0", + content => re( qr{^oai_dc/9/1970-01-01T00:00:00Z/$date_to//0/0/\d+$} ), cursor => 9, }, }, @@ -356,7 +359,7 @@ test_query( # Last record, so no resumption token test_query( 'ListRecords oai_dc with resumptionToken 3, response without resumption', - { verb => 'ListRecords', resumptionToken => "oai_dc/9/1970-01-01T00:00:00Z/$date_to//0/0" }, + { verb => 'ListRecords', resumptionToken => "oai_dc/9/1970-01-01T00:00:00Z/$date_to//0/0/" . ($first_bn + 9) }, { ListRecords => { record => $oaidc[9], }, @@ -369,7 +372,7 @@ test_query('ListRecords marcxml with xsl transformation', { ListRecords => { record => [ @marcxml_transformed[0..2] ], resumptionToken => { - content => "marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0", + content => re( qr{^marcxml/3/1970-01-01T00:00:00Z/$date_to//0/0/\d+$} ), cursor => 3, } }, --