[Bug 39337] New: Use SQL field to apply OpacSuppression in OAI-PMH
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 Bug ID: 39337 Summary: Use SQL field to apply OpacSuppression in OAI-PMH Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@ptfs-europe.com QA Contact: testopia@bugs.koha-community.org Depends on: 37713, 38330 This is a follow-up to bug 37713 and bug 38330. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37713 [Bug 37713] OAI-PMH - Honour OpacSuppression syspref https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38330 [Bug 38330] Make bib-level suppression a biblio table field instead of part of a marc tag -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #1 from David Cook <dcook@prosentient.com.au> --- This would be ideal from both a performance and record visibility perspective. The correct solution on bug 37713 is practical but not ideal and not consistent with the OPAC overall. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- I reckon this can probably be a dupe of bug 37713 really. But we'll tackle it after we're done with bug 38330 I think -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |olivia.reynolds@openfifth.c |ity.org |o.uk -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 Olivia Reynolds <olivia.reynolds@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Comma delimited| |Open Fifth list of Sponsors| | Sponsorship status|--- |Sponsored Patch complexity|--- |Medium patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #3 from Olivia Reynolds <olivia.reynolds@openfifth.co.uk> --- Created attachment 200886 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200886&action=edit Bug 39337: Add is_opac_suppressed helper to C4::Context Previously, OPAC suppression was applied inconsistently, with only some callers taking OpacSuppressionByIPRange into account. This patch adds a new helper to C4::Context to determine whether OPAC suppression should be active for this request, and switches to using it everywhere that was previously manually checking the system preferences. Test plan (specific URLs given for KTD, with thanks to David Cook): 1. Apply patch. 2. Navigate to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=2 3. Set 942$n to "Yes" and save the record. 4. Disable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 5a. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=2 5b. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=2 5c. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=2 5d. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=The+ideas+behind&weight_search=1 5e. Verify you can successfully access http://localhost:8080/api/v1/public/biblios/2 5f. Verify you can successfully access http://localhost:8080/cgi-bin/koha/oai.pl?verb=GetRecord&metadataPrefix=oai_dc&identifier=KOHA-OAI-TEST:2 5g. Verify KOHA-OAI-TEST-2 appears on http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=oai_dc 6. Enable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 7. Repeat steps 5a-5g, verifying that: - the OPAC details pages return "Record blocked". - the OPAC search page returns "No results found". - the API returns "Bibliographic record not found". - the OAI GetRecord and ListRecords show "Record Status: deleted" for KOHA-OAI-TEST-2. 8. Keep "OpacSuppression" enabled, but set "OpacSuppressionByIPRange" to "." (to allow all IPs to bypass suppression). 9. Repeat steps 5a-5g, verifying that every page is successful again. 10. Run `prove t/Context.t`, and verify all tests are successful. Sponsored-by: Open Fifth -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #4 from Olivia Reynolds <olivia.reynolds@openfifth.co.uk> --- Created attachment 200887 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200887&action=edit Bug 39337: Filter out suppressed OAI records in SQL Rather than fetching each biblio before choosing to suppress it, this patch performs this filtering in the initial SQL query that fetches the biblionumbers. Note: for queries including items, this additional filtering is performed in each of the three subqueries, necessitating three quasi-joins against biblio. If we were to hoist this filtering to the outer layer, we would also need to remove the LIMIT from each subquery so as to allow each of them to be fairly weighted. As the preceding comment implies keeping the subqueries separate has been beneficial for performance, I've elected not to hoist the WHERE. Test plan (specific URLs given for KTD, with thanks to David Cook): 1. Apply patch. 2. Navigate to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=2 3. Set 942$n to "Yes" and save the record. 4. Disable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 5a. Verify you can successfully access http://localhost:8080/cgi-bin/koha/oai.pl?verb=GetRecord&metadataPrefix=oai_dc&identifier=KOHA-OAI-TEST:2 5b. Verify KOHA-OAI-TEST-2 appears on http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=oai_dc 6. Enable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 7. Repeat steps 5a-5b, verifying that: - the OAI GetRecord page returns "There is no biblio record with this identifier". - the OAI ListRecords page does not show KOHA-OAI-TEST-2. Sponsored-by: Open Fifth -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200886|0 |1 is obsolete| | Attachment #200887|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 201133 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201133&action=edit Bug 39337: Add is_opac_suppressed helper to C4::Context Previously, OPAC suppression was applied inconsistently, with only some callers taking OpacSuppressionByIPRange into account. This patch adds a new helper to C4::Context to determine whether OPAC suppression should be active for this request, and switches to using it everywhere that was previously manually checking the system preferences. Test plan (specific URLs given for KTD, with thanks to David Cook): 1. Apply patch. 2. Navigate to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=2 3. Set 942$n to "Yes" and save the record. 4. Disable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 5a. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=2 5b. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=2 5c. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=2 5d. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=The+ideas+behind&weight_search=1 5e. Verify you can successfully access http://localhost:8080/api/v1/public/biblios/2 5f. Verify you can successfully access http://localhost:8080/cgi-bin/koha/oai.pl?verb=GetRecord&metadataPrefix=oai_dc&identifier=KOHA-OAI-TEST:2 5g. Verify KOHA-OAI-TEST-2 appears on http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=oai_dc 6. Enable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 7. Repeat steps 5a-5g, verifying that: - the OPAC details pages return "Record blocked". - the OPAC search page returns "No results found". - the API returns "Bibliographic record not found". - the OAI GetRecord and ListRecords show "Record Status: deleted" for KOHA-OAI-TEST-2. 8. Keep "OpacSuppression" enabled, but set "OpacSuppressionByIPRange" to "." (to allow all IPs to bypass suppression). 9. Repeat steps 5a-5g, verifying that every page is successful again. 10. Run `prove t/Context.t`, and verify all tests are successful. Sponsored-by: Open Fifth Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 201134 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201134&action=edit Bug 39337: Filter out suppressed OAI records in SQL Rather than fetching each biblio before choosing to suppress it, this patch performs this filtering in the initial SQL query that fetches the biblionumbers. Note: for queries including items, this additional filtering is performed in each of the three subqueries, necessitating three quasi-joins against biblio. If we were to hoist this filtering to the outer layer, we would also need to remove the LIMIT from each subquery so as to allow each of them to be fairly weighted. As the preceding comment implies keeping the subqueries separate has been beneficial for performance, I've elected not to hoist the WHERE. Test plan (specific URLs given for KTD, with thanks to David Cook): 1. Apply patch. 2. Navigate to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=2 3. Set 942$n to "Yes" and save the record. 4. Disable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 5a. Verify you can successfully access http://localhost:8080/cgi-bin/koha/oai.pl?verb=GetRecord&metadataPrefix=oai_dc&identifier=KOHA-OAI-TEST:2 5b. Verify KOHA-OAI-TEST-2 appears on http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=oai_dc 6. Enable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 7. Repeat steps 5a-5b, verifying that: - the OAI GetRecord page returns "There is no biblio record with this identifier". - the OAI ListRecords page does not show KOHA-OAI-TEST-2. Sponsored-by: Open Fifth Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Keywords| |release-notes-needed --- Comment #7 from David Nind <david@davidnind.com> --- Testing notes (KTD): 1. Enable the OAI-PMH system preference. 2. I applied both patches at once. ==> Test plan for the first patch (Add is_opac_suppressed helper to C4::Context) 3. After Step 6: I did a restart_all and clear browser cache to make sure setting changes were recognised. 4. For Step 5e: With OPAC suppression disabled (Don't hide) (and no IP address range restriction), I get: [ "application/marcxml+xml", "application/marc-in-json", "application/marc", "text/plain" ] 5. For Step 5f (with steps 6 and 7): With OPAC suppression enabled (Hide) (and no IP address range restriction), I get: Error There is no biblio record with this identifier 6. For Step 5g (with steps 6 and 7): With OPAC suppression enabled (Hide) (and no IP address range restriction), I get: there is no record listed for KOHA-OAI-TEST:2 ==> Test plan for the second patch (Filter out suppressed OAI records in SQL) 7. Everything works as per the test plan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |Records marked for OPAC release notes| |suppression (942$n) are now | |correctly hidden from | |OAI-PMH responses when the | |`OpacSuppression` system | |preference is enabled. | |Previously, suppressed | |records could still appear | |in OAI-PMH `ListRecords` | |and `GetRecord` outputs, | |leaking catalogue data to | |harvesters. Suppression is | |now applied consistently | |across the OPAC, REST API, | |and OAI-PMH, and the | |`OpacSuppressionByIPRange` | |preference is also honoured | |by OAI-PMH. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |dcook@prosentient.com.au |y.org | --- Comment #8 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Great work.. love the consistency improvement of the first patch.. 'Today I learned!' I'm going to ask David Cook to QA this one for us :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #9 from David Nind <david@davidnind.com> --- Thanks Martin for the release notes! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #10 from David Cook <dcook@prosentient.com.au> --- Hmm seems like there's been some scope creep here. The bug title is "Use SQL field to apply OpacSuppression in OAI-PMH" but it looks like this is touching a lot of unrelated places: C4/Context.pm | 20 ++++++++++++++++++++ C4/Output.pm | 20 ++++---------------- Koha/OAI/Server/GetRecord.pm | 22 ++++++++++++++-------- Koha/OAI/Server/ListBase.pm | 4 ++++ Koha/OAI/Server/Repository.pm | 5 +---- Koha/REST/V1/Biblios.pm | 2 +- opac/opac-ISBDdetail.pl | 3 +-- opac/opac-MARCdetail.pl | 3 +-- opac/opac-detail.pl | 3 +-- opac/opac-search.pl | 13 +------------ t/Context.t | 32 +++++++++++++++++++++++++++++++- 11 files changed, 79 insertions(+), 48 deletions(-) But... it is all related... ok. Quick code review is looking good. I like the change to Koha::OAI::Server::GetRecord for reusing the WHERE clause. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- Test plan missing the need to turn on system preference "OAI-PMH". -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #12 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize (ashimema) from comment #8)
Great work.. love the consistency improvement of the first patch.. 'Today I learned!'
I'm going to ask David Cook to QA this one for us :)
I think I forgot about that part. Yep, all in all I think this does the trick. It would be interesting to see how much this improves performance for the OAI-PMH server. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #201133|0 |1 is obsolete| | Attachment #201134|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- Created attachment 201687 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201687&action=edit Bug 39337: Add is_opac_suppressed helper to C4::Context Previously, OPAC suppression was applied inconsistently, with only some callers taking OpacSuppressionByIPRange into account. This patch adds a new helper to C4::Context to determine whether OPAC suppression should be active for this request, and switches to using it everywhere that was previously manually checking the system preferences. Test plan (specific URLs given for KTD, with thanks to David Cook): 1. Apply patch. 2. Navigate to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=2 3. Set 942$n to "Yes" and save the record. 4. Disable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 5a. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-detail.pl?biblionumber=2 5b. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=2 5c. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=2 5d. Verify you can successfully access http://localhost:8080/cgi-bin/koha/opac-search.pl?idx=&q=The+ideas+behind&weight_search=1 5e. Verify you can successfully access http://localhost:8080/api/v1/public/biblios/2 5f. Verify you can successfully access http://localhost:8080/cgi-bin/koha/oai.pl?verb=GetRecord&metadataPrefix=oai_dc&identifier=KOHA-OAI-TEST:2 5g. Verify KOHA-OAI-TEST-2 appears on http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=oai_dc 6. Enable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 7. Repeat steps 5a-5g, verifying that: - the OPAC details pages return "Record blocked". - the OPAC search page returns "No results found". - the API returns "Bibliographic record not found". - the OAI GetRecord and ListRecords show "Record Status: deleted" for KOHA-OAI-TEST-2. 8. Keep "OpacSuppression" enabled, but set "OpacSuppressionByIPRange" to "." (to allow all IPs to bypass suppression). 9. Repeat steps 5a-5g, verifying that every page is successful again. 10. Run `prove t/Context.t`, and verify all tests are successful. Sponsored-by: Open Fifth Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #14 from David Cook <dcook@prosentient.com.au> --- Created attachment 201688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=201688&action=edit Bug 39337: Filter out suppressed OAI records in SQL Rather than fetching each biblio before choosing to suppress it, this patch performs this filtering in the initial SQL query that fetches the biblionumbers. Note: for queries including items, this additional filtering is performed in each of the three subqueries, necessitating three quasi-joins against biblio. If we were to hoist this filtering to the outer layer, we would also need to remove the LIMIT from each subquery so as to allow each of them to be fairly weighted. As the preceding comment implies keeping the subqueries separate has been beneficial for performance, I've elected not to hoist the WHERE. Test plan (specific URLs given for KTD, with thanks to David Cook): 1. Apply patch. 2. Navigate to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=2 3. Set 942$n to "Yes" and save the record. 4. Disable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 5a. Verify you can successfully access http://localhost:8080/cgi-bin/koha/oai.pl?verb=GetRecord&metadataPrefix=oai_dc&identifier=KOHA-OAI-TEST:2 5b. Verify KOHA-OAI-TEST-2 appears on http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=oai_dc 6. Enable the system preference "OpacSuppression", and ensure "OpacSuppressionByIPRange" is blank. 7. Repeat steps 5a-5b, verifying that: - the OAI GetRecord page returns "There is no biblio record with this identifier". - the OAI ListRecords page does not show KOHA-OAI-TEST-2. Sponsored-by: Open Fifth Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: David Cook <dcook@prosentient.com.au> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39337 --- Comment #15 from David Cook <dcook@prosentient.com.au> --- Nice one, Olivia. This will be a useful change. Look forward to seeing more from you. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org