Created attachment 135338 [details] [review]
Bug 30848: Add an AuthorizedValues RecordProcessor filter

This patch introduces a RecordProcessor filter for MARC::Record objects
that replaces AV codes with AC descriptions in the MARC::Record passed to the processor.

Target usage:

  my $biblio = Koha::Biblios->find(
      $biblio_id,
      { prefetch => [ metadata ] }
  );

  my $record = $biblio->metadata->record;

  my $processor = Koha::RecordProcessor->new(
    {
        filters => ('AuthorizedValues'),
        options => {
            interface     => 'opac',
            frameworkcode => $biblio->frameworkcode
        }
    }
  );

  $processor->process( $record );

I would call it something like ExpandAuthorizedValues. We just need some tests?

Created attachment 135353 [details] [review]
Bug 30848: Add an AuthorizedValues RecordProcessor filter

This patch introduces a RecordProcessor filter for MARC::Record objects
that replaces AV codes with AC descriptions in the MARC::Record passed to the processor.

Target usage:

  my $biblio = Koha::Biblios->find(
      $biblio_id,
      { prefetch => [ metadata ] }
  );

  my $record = $biblio->metadata->record;

  my $processor = Koha::RecordProcessor->new(
    {
        filters => ('AuthorizedValues'),
        options => {
            interface     => 'opac',
            frameworkcode => $biblio->frameworkcode
        }
    }
  );

  $processor->process( $record );

Unit tests to come.. it hit end of day here so they'll be a tomorrow job now..

Works beautifully in bug 30744 though ;P

I can rename it tomorrow too :)

Created attachment 135363 [details] [review]
Bug 30848: Unit test

Created attachment 135364 [details] [review]
Bug 30848: Add an AuthorizedValues RecordProcessor filter

This patch introduces a RecordProcessor filter for MARC::Record objects
that replaces AV codes with AC descriptions in the MARC::Record passed to the processor.

Target usage:

  my $biblio = Koha::Biblios->find(
      $biblio_id,
      { prefetch => [ metadata ] }
  );

  my $record = $biblio->metadata->record;

  my $processor = Koha::RecordProcessor->new(
    {
        filters => ('ExpandAuthorizedValues'),
        options => {
            interface     => 'opac',
            frameworkcode => $biblio->frameworkcode
        }
    }
  );

  $processor->process( $record );

Created attachment 135386 [details] [review]
Bug 30848: Unit test

Created attachment 135387 [details] [review]
Bug 30848: Add an AuthorizedValues RecordProcessor filter

This patch introduces a RecordProcessor filter for MARC::Record objects
that replaces AV codes with AC descriptions in the MARC::Record passed to the processor.

Target usage:

  my $biblio = Koha::Biblios->find(
      $biblio_id,
      { prefetch => [ metadata ] }
  );

  my $record = $biblio->metadata->record;

  my $processor = Koha::RecordProcessor->new(
    {
        filters => ('ExpandAuthorizedValues'),
        options => {
            interface     => 'opac',
            frameworkcode => $biblio->frameworkcode
        }
    }
  );

  $processor->process( $record );

Created attachment 135388 [details] [review]
Bug 30848: Caching for performance

The cached version seems to be pretty amazing for performance.. orders of magnitude better than the C4::XSLT routine.

However, I've only done the cache populating here.. we'll need to flush this cache appropriately on Library, Itemtype, ClassSource, AV and Framework changes..

Also.. I think we should rename it again from ExpandAuthorizedValues to ExpandKohaCodes.. we're catching more than I first realised here.. it's not just AV's.

Created attachment 135418 [details] [review]
Bug 30848: Ensure we flush the cache as necessary

OK.. I need to consider itemtype description translations.. right now my code is ignorant to that.

As mentioned in bug 28371#c2 I'm starting to think I'm caching in the wrong place.. we could cache the individual parts quite easily and flush less with each update that way..

Would looking up 4 smaller caches be significantly less performance than the single larger cache I wonder?

Seeing several:
Koha::BiblioFrameworks->search()->get_column('frameworkcode')

Any way to get rid of additional DB access in order to flush cache ;) ?

(In reply to Martin Renvoize from comment #15)
> As mentioned in bug 28371#c2 I'm starting to think I'm caching in the wrong
> place.. we could cache the individual parts quite easily and flush less with
> each update that way..
> 
> Would looking up 4 smaller caches be significantly less performance than the
> single larger cache I wonder?

We had a comment clash :)

(In reply to Marcel de Rooy from comment #16)
> Seeing several:
> Koha::BiblioFrameworks->search()->get_column('frameworkcode')
> 
> Any way to get rid of additional DB access in order to flush cache ;) ?

Not easily the way it's currently written ;P

Created attachment 135421 [details] [review]
Bug 30848: Unit test

Created attachment 135422 [details] [review]
Bug 30848: Add an AuthorizedValues RecordProcessor filter

This patch introduces a RecordProcessor filter for MARC::Record objects
that replaces AV codes with AC descriptions in the MARC::Record passed to the processor.

Target usage:

  my $biblio = Koha::Biblios->find(
      $biblio_id,
      { prefetch => [ metadata ] }
  );

  my $record = $biblio->metadata->record;

  my $processor = Koha::RecordProcessor->new(
    {
        filters => ('ExpandAuthorizedValues'),
        options => {
            interface     => 'opac',
            frameworkcode => $biblio->frameworkcode
        }
    }
  );

  $processor->process( $record );

Created attachment 135423 [details] [review]
Bug 30848: Caching for performance

Created attachment 135424 [details] [review]
Bug 30848: Ensure we flush the cache as necessary

Created attachment 135425 [details] [review]
Bug 30848: Move caching level

Caching moved.. now I need to follow through and fix the 'Ensure we flush the cache' patch to flush appropriately now we have multiple small caches instead of a single larger structure per framework.

Created attachment 135707 [details] [review]
Bug 30848: Unit test

Created attachment 135708 [details] [review]
Bug 30848: Add an AuthorizedValues RecordProcessor filter

This patch introduces a RecordProcessor filter for MARC::Record objects
that replaces AV codes with AC descriptions in the MARC::Record passed to the processor.

Target usage:

  my $biblio = Koha::Biblios->find(
      $biblio_id,
      { prefetch => [ metadata ] }
  );

  my $record = $biblio->metadata->record;

  my $processor = Koha::RecordProcessor->new(
    {
        filters => ('ExpandAuthorizedValues'),
        options => {
            interface     => 'opac',
            frameworkcode => $biblio->frameworkcode
        }
    }
  );

  $processor->process( $record );

Created attachment 135709 [details] [review]
Bug 30848: Caching for performance

Created attachment 135710 [details] [review]
Bug 30848: Ensure we flush the cache as necessary

Created attachment 135711 [details] [review]
Bug 30848: Move caching level

Created attachment 135712 [details] [review]
Bug 30848: Ensure we flush the cache as necessary

Created attachment 135713 [details] [review]
Bug 30848: Rename to ExpandCodedFields

Created attachment 135714 [details] [review]
Bug 30848: Unit test

Created attachment 135715 [details] [review]
Bug 30848: Add an AuthorizedValues RecordProcessor filter

This patch introduces a RecordProcessor filter for MARC::Record objects
that replaces AV codes with AC descriptions in the MARC::Record passed to the processor.

Target usage:

  my $biblio = Koha::Biblios->find(
      $biblio_id,
      { prefetch => [ metadata ] }
  );

  my $record = $biblio->metadata->record;

  my $processor = Koha::RecordProcessor->new(
    {
        filters => ('ExpandAuthorizedValues'),
        options => {
            interface     => 'opac',
            frameworkcode => $biblio->frameworkcode
        }
    }
  );

  $processor->process( $record );

Created attachment 135716 [details] [review]
Bug 30848: Caching for performance

This patch adds caching to ExpandAuthorizedValues for performance.

We cache each hash involved in C4::Biblio::GetAuthorisedValueDesc as
well as caching the reduced marc subfield structure that filter down to
just those marc fields with coded substitutions.

Created attachment 135717 [details] [review]
Bug 30848: Rename to ExpandCodedFields

OK, squashed and refined as discussed.. ready for review.

This patchset does two things, we introduce a new Koha::Filter::MARC::ExpandCodedValues record processor to allow Marc records to have their Koha coded fields expanded from codes to descriptions/values. (We can use this to replace C4::XSLT::transformMARCXML4XSLT but also add it in wherever we're already calling record processor to get a proper representative record for display).

Additionally, we add caching to improve performance.. after working through this a few times to pick the appropriate cache level, I think we've got it now.