@@ -, +, @@ --- Koha/OAI/Server/ListBase.pm | 194 ++++++++++++++++--------------------- Koha/OAI/Server/ResumptionToken.pm | 15 +-- t/db_dependent/OAI/Server.t | 34 +++---- 3 files changed, 102 insertions(+), 141 deletions(-) --- a/Koha/OAI/Server/ListBase.pm +++ a/Koha/OAI/Server/ListBase.pm @@ -1,6 +1,6 @@ package Koha::OAI::Server::ListBase; -# Copyright The National Library of Finland, University of Helsinki 2016-2017 +# Copyright The National Library of Finland, University of Helsinki 2016-2018 # # This file is part of Koha. # @@ -46,136 +46,104 @@ sub GetRecords { $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 ); - # Since creating a union of normal and deleted record tables would be a heavy - # operation in a large database, build results in two stages: - # first deleted records ($deleted == 1), then normal records ($deleted == 0) - STAGELOOP: - for ( ; $deleted >= 0; $deleted-- ) { - my $table = $deleted ? 'deletedbiblio_metadata' : 'biblio_metadata'; - my $sql = " - SELECT biblionumber - FROM $table - WHERE (timestamp >= ? AND timestamp <= ?) - "; - my @bind_params = ($token->{'from_arg'}, $token->{'until_arg'}); - - if ($include_items) { - $sql .= " - OR biblionumber IN (SELECT biblionumber from items WHERE timestamp >= ? AND timestamp <= ? AND deleted_at IS NOT NULL) - "; - push @bind_params, ($token->{'from_arg'}, $token->{'until_arg'}); - if (!$deleted) { - $sql .= " - OR biblionumber IN (SELECT biblionumber from items WHERE timestamp >= ? AND timestamp <= ? AND deleted_at IS NULL) - "; - push @bind_params, ($token->{'from_arg'}, $token->{'until_arg'}); - } - } + my $sql = " + SELECT biblionumber, deleted_at + FROM biblio_metadata + WHERE (timestamp >= ? AND timestamp <= ?) + "; + my @bind_params = ($token->{'from_arg'}, $token->{'until_arg'}); + if ($include_items) { $sql .= " - ORDER BY biblionumber + OR biblionumber IN (SELECT biblionumber from items WHERE timestamp >= ? AND timestamp <= ?) "; + push @bind_params, ($token->{'from_arg'}, $token->{'until_arg'}); + } - # Use a subquery for sets since it allows us to use an index in - # biblioitems table and is quite a bit faster than a join. - if (defined $set) { - $sql = " - SELECT bi.* FROM ($sql) bi - WHERE bi.biblionumber in (SELECT osb.biblionumber FROM oai_sets_biblios osb WHERE osb.set_id = ?) - "; - push @bind_params, $set->{'id'}; - } + $sql .= " + ORDER BY biblionumber + "; - $sql .= " - LIMIT " . ($max + 1) . " - OFFSET " . ($offset - $deleted_count); - - my $sth = $dbh->prepare( $sql ) || die( 'Could not prepare statement: ' . $dbh->errstr ); - - if ( $deleted ) { - $sql = " - SELECT MAX(timestamp) - FROM ( - SELECT timestamp FROM deletedbiblio_metadata WHERE biblionumber = ? - UNION - SELECT timestamp FROM items WHERE biblionumber = ? AND deleted_at IS NOT NULL - ) bis - "; - } else { - $sql = " - SELECT MAX(timestamp) - FROM ( - SELECT timestamp FROM biblio_metadata WHERE biblionumber = ? - UNION - SELECT timestamp FROM items WHERE biblionumber = ? AND deleted_at IS NULL - UNION - SELECT timestamp FROM items WHERE biblionumber = ? AND deleted_at IS NOT NULL - ) bi - "; + # Use a subquery for sets since it allows us to use an index in + # biblioitems table and is quite a bit faster than a join. + if (defined $set) { + $sql = " + SELECT bi.* FROM ($sql) bi + WHERE bi.biblionumber in (SELECT osb.biblionumber FROM oai_sets_biblios osb WHERE osb.set_id = ?) + "; + push @bind_params, $set->{'id'}; + } + + $sql .= " + LIMIT " . ($max + 1) . " + OFFSET $offset"; + + my $sth = $dbh->prepare( $sql ) || die( 'Could not prepare statement: ' . $dbh->errstr ); + + $sql = " + SELECT MAX(timestamp) + FROM ( + SELECT timestamp FROM biblio_metadata WHERE biblionumber = ? + UNION + SELECT timestamp FROM items WHERE biblionumber = ? + ) bi + "; + my $record_sth = $dbh->prepare( $sql ) || die( 'Could not prepare statement: ' . $dbh->errstr ); + + $sth->execute( @bind_params ) || die( 'Could not execute statement: ' . $sth->errstr ); + while ( my ($biblionumber, $deleted_at) = $sth->fetchrow ) { + $count++; + if ( $count > $max ) { + $self->resumptionToken( + new Koha::OAI::Server::ResumptionToken( + metadataPrefix => $token->{metadata_prefix}, + from => $token->{from}, + until => $token->{until}, + offset => $token->{offset} + $max, + set => $token->{set}, + ) + ); + last; } - my $record_sth = $dbh->prepare( $sql ) || die( 'Could not prepare statement: ' . $dbh->errstr ); - - $sth->execute( @bind_params ) || die( 'Could not execute statement: ' . $sth->errstr ); - while ( my ($biblionumber) = $sth->fetchrow ) { - $count++; - if ( $count > $max ) { - $self->resumptionToken( - new Koha::OAI::Server::ResumptionToken( - metadataPrefix => $token->{metadata_prefix}, - from => $token->{from}, - until => $token->{until}, - offset => $token->{offset} + $max, - set => $token->{set}, - deleted => $deleted, - deleted_count => $deleted_count - ) - ); - last STAGELOOP; - } - my @params = $deleted ? ( $biblionumber, $biblionumber ) : ( $biblionumber, $biblionumber, $biblionumber ); - $record_sth->execute( @params ) || die( 'Could not execute statement: ' . $sth->errstr ); + $record_sth->execute( ( $biblionumber, $biblionumber ) ) || die( 'Could not execute statement: ' . $sth->errstr ); - my ($timestamp) = $record_sth->fetchrow; + my ($timestamp) = $record_sth->fetchrow; - my $oai_sets = GetOAISetsBiblio($biblionumber); - my @setSpecs; - foreach ( @$oai_sets ) { - push @setSpecs, $_->{spec}; - } - if ( $metadata ) { - my $marcxml = !$deleted ? $repository->get_biblio_marcxml($biblionumber, $format) : undef; - if ( $marcxml ) { - $self->record( Koha::OAI::Server::Record->new( - $repository, $marcxml, $timestamp, \@setSpecs, - identifier => $repository->{ koha_identifier } . ':' . $biblionumber, - metadataPrefix => $token->{metadata_prefix} - ) ); - } else { - $self->record( Koha::OAI::Server::DeletedRecord->new( - $timestamp, \@setSpecs, identifier => $repository->{ koha_identifier } . ':' . $biblionumber - ) ); - } + my $oai_sets = GetOAISetsBiblio($biblionumber); + my @setSpecs; + foreach ( @$oai_sets ) { + push @setSpecs, $_->{spec}; + } + if ( $metadata ) { + my $marcxml = !$deleted_at ? $repository->get_biblio_marcxml($biblionumber, $format) : undef; + if ( $marcxml ) { + $self->record( Koha::OAI::Server::Record->new( + $repository, $marcxml, $timestamp, \@setSpecs, + identifier => $repository->{ koha_identifier } . ':' . $biblionumber, + metadataPrefix => $token->{metadata_prefix} + ) ); } else { - $timestamp =~ s/ /T/; - $timestamp .= 'Z'; - $self->identifier( new HTTP::OAI::Header( - identifier => $repository->{ koha_identifier} . ':' . $biblionumber, - datestamp => $timestamp, - status => $deleted ? 'deleted' : undef + $self->record( Koha::OAI::Server::DeletedRecord->new( + $timestamp, \@setSpecs, identifier => $repository->{ koha_identifier } . ':' . $biblionumber ) ); } + } else { + $timestamp =~ s/ /T/; + $timestamp .= 'Z'; + $self->identifier( new HTTP::OAI::Header( + identifier => $repository->{ koha_identifier} . ':' . $biblionumber, + datestamp => $timestamp, + status => $deleted_at ? 'deleted' : undef + ) ); } - # Store offset and deleted record count - $offset += $count; - $deleted_count = $offset if ($deleted); } + # Store offset + $offset += $count; 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-2018 # # This file is part of Koha. # @@ -32,17 +32,15 @@ use base ("HTTP::OAI::ResumptionToken"); # - from # - until # - offset -# - deleted sub new { my ($class, %args) = @_; my $self = $class->SUPER::new(%args); - my ($metadata_prefix, $offset, $from, $until, $set, $deleted, $deleted_count); + my ($metadata_prefix, $offset, $from, $until, $set); if ( $args{ resumptionToken } ) { - ($metadata_prefix, $offset, $from, $until, $set, $deleted, $deleted_count) - = split( '/', $args{resumptionToken} ); + ($metadata_prefix, $offset, $from, $until, $set) = split( '/', $args{resumptionToken} ); } else { $metadata_prefix = $args{ metadataPrefix }; @@ -57,8 +55,6 @@ sub new { $until .= 'T23:59:59Z' if length($until) == 10; $offset = $args{ offset } || 0; $set = $args{ set } || ''; - $deleted = defined $args{ deleted } ? $args{ deleted } : 1; - $deleted_count = defined $args{ deleted_count } ? $args{ deleted_count } : 0; } $self->{ metadata_prefix } = $metadata_prefix; @@ -68,11 +64,8 @@ sub new { $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->resumptionToken( - join( '/', $metadata_prefix, $offset, $from, $until, $set, $deleted, $deleted_count ) ); + $self->resumptionToken( join( '/', $metadata_prefix, $offset, $from, $until, $set ) ); $self->cursor( $offset ); return $self; --- a/t/db_dependent/OAI/Server.t +++ a/t/db_dependent/OAI/Server.t @@ -201,7 +201,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 => "marcxml/3/1970-01-01T00:00:00Z/$date_to/", cursor => 3, }, }, @@ -211,7 +211,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 => "marcxml/3/1970-01-01T00:00:00Z/$date_to/", cursor => 3, }, }, @@ -224,7 +224,7 @@ test_query( ListIdentifiers => { header => [ @header[3..5] ], resumptionToken => { - content => "marcxml/6/1970-01-01T00:00:00Z/$date_to//0/0", + content => "marcxml/6/1970-01-01T00:00:00Z/$date_to/", cursor => 6, }, }, @@ -238,7 +238,7 @@ test_query( ListIdentifiers => { header => [ @header[6..8] ], resumptionToken => { - content => "marcxml/9/1970-01-01T00:00:00Z/$date_to//0/0", + content => "marcxml/9/1970-01-01T00:00:00Z/$date_to/", cursor => 9, }, }, @@ -247,7 +247,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/" }, { ListIdentifiers => { header => $header[9], @@ -266,7 +266,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 => "marcxml/3/1970-01-01T00:00:00Z/$date_to/", cursor => 3, }, }, @@ -274,11 +274,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/" }, { ListRecords => { record => [ @marcxml[3..5] ], resumptionToken => { - content => "marcxml/6/1970-01-01T00:00:00Z/$date_to//0/0", + content => "marcxml/6/1970-01-01T00:00:00Z/$date_to/", cursor => 6, }, }, @@ -286,11 +286,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/" }, { ListRecords => { record => [ @marcxml[6..8] ], resumptionToken => { - content => "marcxml/9/1970-01-01T00:00:00Z/$date_to//0/0", + content => "marcxml/9/1970-01-01T00:00:00Z/$date_to/", cursor => 9, }, }, @@ -299,7 +299,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/" }, { ListRecords => { record => $marcxml[9], }, @@ -309,7 +309,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 => "oai_dc/3/1970-01-01T00:00:00Z/$date_to/", cursor => 3, }, }, @@ -317,11 +317,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/" }, { ListRecords => { record => [ @oaidc[3..5] ], resumptionToken => { - content => "oai_dc/6/1970-01-01T00:00:00Z/$date_to//0/0", + content => "oai_dc/6/1970-01-01T00:00:00Z/$date_to/", cursor => 6, }, }, @@ -329,11 +329,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/" }, { ListRecords => { record => [ @oaidc[6..8] ], resumptionToken => { - content => "oai_dc/9/1970-01-01T00:00:00Z/$date_to//0/0", + content => "oai_dc/9/1970-01-01T00:00:00Z/$date_to/", cursor => 9, }, }, @@ -342,7 +342,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/" }, { ListRecords => { record => $oaidc[9], }, --