This should help reduce the current overhead, and by using the features introduced by bug 22454, probably give a performance boost.
Created attachment 93971 [details] [review] Bug 23793: Add an EmbedItems RecordProcessor filter for MARC::Record objects This patch is work in progress. I submit early with the aim to provide people thinking about performance on the biblio/records/items front an idea of what I'm thinking. The synopsis on the new filter is self explanatory. The idea is that we calculate as much as we can outside the loops. And this simpe implementation is clean and light as well. Problems: - Filters are supposed to support passing multiple MARC records, this one doesn't, beacuse I didn't think enough of a way to pass all the items and biblionumbers in a way that is safe...
I paste the synopsis for easier analysis: my $biblio = Koha::Biblios->find( $biblio_id, { prefetch => [ items, metadata ] } ); my $opachiddenitems_rules; eval { my $yaml = C4::Context->preference('OpacHiddenItems') . "\n\n"; $opachiddenitems_rules = YAML::Load($yaml); }; my @items = grep { $_->hidden_in_opac({ rules => $opachiddenitems_rules }) @{$biblio->items}; my $record = $biblio->metadata->record; my $processor = Koha::RecordProcessor->new( { filters => ('EmbedItems'), options => { items => \@items, item_tag => $item_tag, biblionumber => $biblionumber } } ); $processor->process( $record );
Created attachment 93980 [details] [review] Bug 23793: Add an EmbedItems RecordProcessor filter for MARC::Record objects This patch is work in progress. I submit early with the aim to provide people thinking about performance on the biblio/records/items front an idea of what I'm thinking. The synopsis on the new filter is self explanatory. The idea is that we calculate as much as we can outside the loops. And this simpe implementation is clean and light as well. Problems: - Filters are supposed to support passing multiple MARC records, this one doesn't, beacuse I didn't think enough of a way to pass all the items and biblionumbers in a way that is safe...
Created attachment 94030 [details] [review] Bug 23793: Add an EmbedItems RecordProcessor filter for MARC::Record objects This patch is work in progress. I submit early with the aim to provide people thinking about performance on the biblio/records/items front an idea of what I'm thinking. The synopsis on the new filter is self explanatory. The idea is that we calculate as much as we can outside the loops. And this simpe implementation is clean and light as well. Problems: - Filters are supposed to support passing multiple MARC records, this one doesn't, beacuse I didn't think enough of a way to pass all the items and biblionumbers in a way that is safe...
Simplified version, based on 23807. Update synopsis here: my $biblio = Koha::Biblios->find( $biblio_id, { prefetch => [ items, metadata ] } ); my $opachiddenitems_rules; eval { my $yaml = C4::Context->preference('OpacHiddenItems') . "\n\n"; $opachiddenitems_rules = YAML::Load($yaml); }; my @items = grep { !$_->hidden_in_opac({ rules => $opachiddenitems_rules }) @{$biblio->items}; my $record = $biblio->metadata->record; my $processor = Koha::RecordProcessor->new( { filters => ('EmbedItems'), options => { items => \@items } } ); $processor->process( $record );
Created attachment 94095 [details] [review] Bug 23793: Add an EmbedItems RecordProcessor filter for MARC::Record objects This patch introduces a RecordProcessor filter for MARC::Record objects that embeds MARC::Field's for each item that is passed to the processor. Target usage: my $biblio = Koha::Biblios->find( $biblio_id, { prefetch => [ items, metadata ] } ); my $opachiddenitems_rules; eval { my $yaml = C4::Context->preference('OpacHiddenItems') . "\n\n"; $opachiddenitems_rules = YAML::Load($yaml); }; my @items = grep { !$_->hidden_in_opac({ rules => $opachiddenitems_rules }) @{$biblio->items->as_list}; my $record = $biblio->metadata->record; my $processor = Koha::RecordProcessor->new( { filters => ('EmbedItems'), options => { items => \@items } } ); $processor->process( $record );
Created attachment 94096 [details] [review] Bug 23793: Unit tests This patch introduces full test coverage for the EmbedItems filter. To test: 1. Apply this patchset 2. Run: $ kshell k$ prove t/db_dependent/Koha/Filter/EmbedItems.t => SUCCESS: Tests pass! 3. Sign off :-D 4. Use it to improve Koha!
This looks great, but wouldn't it be nice to put it into use right away?
(In reply to Ere Maijala from comment #8) > This looks great, but wouldn't it be nice to put it into use right away? Yes, but I wanted your opinion first!
+1 for this, I'm going to close my old optimization bug. :)
(In reply to Ere Maijala from comment #10) > +1 for this, I'm going to close my old optimization bug. :) Can we talk you into a sign-off? :)
Created attachment 94599 [details] [review] Bug 23793: Add an EmbedItems RecordProcessor filter for MARC::Record objects This patch introduces a RecordProcessor filter for MARC::Record objects that embeds MARC::Field's for each item that is passed to the processor. Target usage: my $biblio = Koha::Biblios->find( $biblio_id, { prefetch => [ items, metadata ] } ); my $opachiddenitems_rules; eval { my $yaml = C4::Context->preference('OpacHiddenItems') . "\n\n"; $opachiddenitems_rules = YAML::Load($yaml); }; my @items = grep { !$_->hidden_in_opac({ rules => $opachiddenitems_rules }) @{$biblio->items->as_list}; my $record = $biblio->metadata->record; my $processor = Koha::RecordProcessor->new( { filters => ('EmbedItems'), options => { items => \@items } } ); $processor->process( $record ); Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Created attachment 94600 [details] [review] Bug 23793: Unit tests This patch introduces full test coverage for the EmbedItems filter. To test: 1. Apply this patchset 2. Run: $ kshell k$ prove t/db_dependent/Koha/Filter/EmbedItems.t => SUCCESS: Tests pass! 3. Sign off :-D 4. Use it to improve Koha! Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Ok, fine. :) I trust that there will be something using this soon. I'm generally not in favour of adding functionality without anything using it since there's always a risk that something happens and it ends up never being used.
Created attachment 94602 [details] [review] Bug 23793: Add an EmbedItems RecordProcessor filter for MARC::Record objects This patch introduces a RecordProcessor filter for MARC::Record objects that embeds MARC::Field's for each item that is passed to the processor. Target usage: my $biblio = Koha::Biblios->find( $biblio_id, { prefetch => [ items, metadata ] } ); my $opachiddenitems_rules; eval { my $yaml = C4::Context->preference('OpacHiddenItems') . "\n\n"; $opachiddenitems_rules = YAML::Load($yaml); }; my @items = grep { !$_->hidden_in_opac({ rules => $opachiddenitems_rules }) @{$biblio->items->as_list}; my $record = $biblio->metadata->record; my $processor = Koha::RecordProcessor->new( { filters => ('EmbedItems'), options => { items => \@items } } ); $processor->process( $record ); Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 94603 [details] [review] Bug 23793: Unit tests This patch introduces full test coverage for the EmbedItems filter. To test: 1. Apply this patchset 2. Run: $ kshell k$ prove t/db_dependent/Koha/Filter/EmbedItems.t => SUCCESS: Tests pass! 3. Sign off :-D 4. Use it to improve Koha! Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Passes QA scripts, Passes tests.. Passing QA. I trust Tomas to followup with uses of this in new bugs soon :)
Nice work! Pushed to master for 19.11.00
(In reply to Ere Maijala from comment #14) > Ok, fine. :) I trust that there will be something using this soon. I'm > generally not in favour of adding functionality without anything using it > since there's always a risk that something happens and it ends up never > being used. I don't think there's any core functionality that is using it today unfortunately.
(In reply to David Cook from comment #19) > (In reply to Ere Maijala from comment #14) > > Ok, fine. :) I trust that there will be something using this soon. I'm > > generally not in favour of adding functionality without anything using it > > since there's always a risk that something happens and it ends up never > > being used. > > I don't think there's any core functionality that is using it today > unfortunately. opac/opac-ISBDdetail.pl
(In reply to Tomás Cohen Arazi from comment #20) > > opac/opac-ISBDdetail.pl Cool. I'm going to use it in some local code now. The sample code in the documentation doesn't work but working out the right way of doing it...
(In reply to David Cook from comment #21) > (In reply to Tomás Cohen Arazi from comment #20) > > > > opac/opac-ISBDdetail.pl > > Cool. I'm going to use it in some local code now. The sample code in the > documentation doesn't work but working out the right way of doing it... I fixed the POD on bug 29764. Please test. The only change is parenthesis for square brackets in the filters list.