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
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Testopia
URL:
Keywords:
Depends on: 24458
Blocks:
  Show dependency tree / graph
 
Reported: 2020-05-11 13:30 UTC by Tomás Cohen Arazi (tcohen)
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:
Circulation function:

Attachments
Bug 25457: Regression tests (3.01 KB, patch)
2020-05-11 18:49 UTC, Tomás Cohen Arazi (tcohen) 		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 (tcohen) 		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
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 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 (tcohen) 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 (tcohen) 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 (tcohen) 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.