Bug 25457 - The ViewPolicy filter should cache the MARC framework structure
Summary: The ViewPolicy filter should cache the MARC framework structure
Status: CLOSED WORKSFORME
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords:
Depends on: 24458
Blocks:
  Show dependency treegraph
 
Reported: 2020-05-11 13:30 UTC by Tomás Cohen Arazi
Modified: 2021-06-14 21:29 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 25457: Regression tests (3.01 KB, patch)
2020-05-11 18:49 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25457: Make ViewPolicy cache MARC frameworks (1.64 KB, patch)
2020-05-11 18:50 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25457: Regression tests (3.07 KB, patch)
2020-05-13 11:34 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 25457: Make ViewPolicy cache MARC frameworks (1.69 KB, patch)
2020-05-13 11:34 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2020-05-11 13:30:38 UTC
If a record processor is to be used inside a loop (as in searchResults on bug 24458) it should not retrieve the frameworks information on each call.
Comment 1 Tomás Cohen Arazi 2020-05-11 18:49:57 UTC
Created attachment 104712 [details] [review]
Bug 25457: Regression tests

This patch implements tests on the ViewPolicy RecordProcessor filter, to
assess it caches the calculated MARC subfield structure between calls.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2020-05-11 18:50:02 UTC
Created attachment 104713 [details] [review]
Bug 25457: Make ViewPolicy cache MARC frameworks

This patch implements an implicit cache of the calculated MARC subfield
structures on each use of the filter. Moreover, if different frameworks
are set between calls (using $processor->options) all of them are
cached.

To test:
1. Apply the tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Filter_MARC_ViewPolicy.t
=> FAIL: The ViewPolicy filter doesn't cache anything
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Nick Clemens (kidclamp) 2020-05-13 11:34:01 UTC
Created attachment 104836 [details] [review]
Bug 25457: Regression tests

This patch implements tests on the ViewPolicy RecordProcessor filter, to
assess it caches the calculated MARC subfield structure between calls.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 4 Nick Clemens (kidclamp) 2020-05-13 11:34:04 UTC
Created attachment 104837 [details] [review]
Bug 25457: Make ViewPolicy cache MARC frameworks

This patch implements an implicit cache of the calculated MARC subfield
structures on each use of the filter. Moreover, if different frameworks
are set between calls (using $processor->options) all of them are
cached.

To test:
1. Apply the tests patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Filter_MARC_ViewPolicy.t
=> FAIL: The ViewPolicy filter doesn't cache anything
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 5 Jonathan Druart 2020-06-09 09:05:50 UTC
I don't see how it could be useful, you are calling GetMarcStructure that is doing exactly that.
Comment 6 Tomás Cohen Arazi 2020-06-09 12:35:09 UTC
(In reply to Jonathan Druart from comment #5)
> I don't see how it could be useful, you are calling GetMarcStructure that is
> doing exactly that.

Is it storing the structure in L1?
Comment 7 Jonathan Druart 2020-06-09 12:42:25 UTC
yes of course.