[Bug 29135] New: OAI should not include biblionumbers from deleteditems when determining deletedbiblios
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Bug ID: 29135 Summary: OAI should not include biblionumbers from deleteditems when determining deletedbiblios Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org When including items in OAI we add the biblionumbers of deleted items into the deleted list This is from this line: 93 if ( $include_items ) { 94 $sql = "($sql) UNION (SELECT DISTINCT(biblionumber) FROM deleteditems $criteria)"; 95 push @bind_params, @part_bind_params; This is wrong because deletion of a single item does not indicate a record is deleted To recreate: 1 - Create /var/lib/koha/kohadev/OAI.yaml with: format: marcxml: metadataPrefix: marcxml metadataNamespace: http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim schema: http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd include_items: 1 2 - Set OAI-PMH:ConfFile to '/var/lib/koha/kohadev/OAI.yaml' 3 - Delete an item from a record 4 - View the page: http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml 5 - Note the record is now listed as deleted -- 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=29135 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27584 -- 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=29135 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_21_11_candidate CC| |jonathan.druart+koha@gmail. | |com -- 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=29135 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #1 from David Cook <dcook@prosentient.com.au> --- Good catch. I think there's some additional relevant lines lower down in Koha/OAI/Server/ListBase.pm as well when the max timestamp is fetched. $sql = " SELECT MAX(timestamp) FROM ( SELECT timestamp FROM deletedbiblio_metadata WHERE biblionumber = ? UNION SELECT timestamp FROM deleteditems WHERE biblionumber = ? ) bis "; I mean it should be impossible to deleted an item after the bib has been deleted. But this SQL would return a timestamp where there is a deleted item but not a deleted bib. -- 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=29135 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #1)
Good catch.
I think there's some additional relevant lines lower down in Koha/OAI/Server/ListBase.pm as well when the max timestamp is fetched.
Koha/OAI/Server/GetRecord.pm has that same weird logic, although it would just be a performance degradation rather than a wrong classification... -- 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=29135 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |27584 See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=27584 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27584 [Bug 27584] Improve OAI-PMH provider performance -- 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=29135 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |ere.maijala@helsinki.fi -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |ere.maijala@helsinki.fi |ity.org | Status|NEW |ASSIGNED --- Comment #3 from Ere Maijala <ere.maijala@helsinki.fi> --- Oh no. It seems that I made a terrible mistake here. However, it's not that simple. The timestamp of a deleted item is still relevant for a live biblio. The latest timestamp of any change, be that change or deletion, must be used as the timestamp since everything affects what's in the returned record. So it's not as simple as removing the statements for non-deleted records. I'll take this with a high priority. -- 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=29135 --- Comment #4 from Ere Maijala <ere.maijala@helsinki.fi> --- And yes, you're both right about the deleted records. I had missed the most logical shortcut that a deleted biblio can't have items deleted later. I think I have a fix which mostly just makes things more simple, but I'm still working on adding tests to cover all cases. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 --- Comment #5 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 125570 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125570&action=edit Bug 29135: Fix handling of deleted items in OAI-PMH provider Test plan: 1. Make sure this scenario now works properly: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135#c0 2. Run prove -v t/db_dependent/OAI/ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 --- Comment #6 from Ere Maijala <ere.maijala@helsinki.fi> --- This should fix the issues. I haven't had a chance to manually test much yet, but at least the tests added failed miserably without the changes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Thanks a lot, Ere, for your reactivity! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125570|0 |1 is obsolete| | --- Comment #8 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 125576 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125576&action=edit Bug 29135: Fix handling of deleted items in OAI-PMH provider Test plan: 1. Make sure this scenario now works properly: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135#c0 2. Run prove -v t/db_dependent/OAI/ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 --- Comment #9 from Ere Maijala <ere.maijala@helsinki.fi> --- Jonathan, no problem, it was my bad anyway.. I've added a couple more tests. I'll run a harvesting job to verify. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125576|0 |1 is obsolete| | --- Comment #10 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 125591 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125591&action=edit Bug 29135: Fix handling of deleted items in OAI-PMH provider Test plan: 1. Make sure this scenario now works properly: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135#c0 2. Run prove -v t/db_dependent/OAI/ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 --- Comment #11 from Ere Maijala <ere.maijala@helsinki.fi> --- Test harvesting completed successfully and record counts match. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |david@davidnind.com --- Comment #12 from David Nind <david@davidnind.com> --- For me the tests fail: not ok 33 - Tests for timestamp handling # Failed test 'Tests for timestamp handling' # at t/db_dependent/OAI/Server.t line 849. # Looks like you failed 1 test of 33. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/33 subtests Testing notes (koha-testing-docker started with ku): - Enable OAI-PMH - Before the patch is replied there is an incomplete item record listed at the end of the metadata section for the item deleted, after the patch nothing is shown for the deleted item, for example: <datafield ind2="" tag="952" ind1=""> <subfield code="0">0</subfield> <subfield code="1">0</subfield> <subfield code="4">0</subfield> <subfield code="6">_</subfield> <subfield code="7">0</subfield> <subfield code="8">REF</subfield> <subfield code="9">313</subfield> <subfield code="a">FPL</subfield> <subfield code="b">FPL</subfield> <subfield code="c">GEN</subfield> <subfield code="d">2014-09-04</subfield> <subfield code="p">39999000006117</subfield> <subfield code="r">2014-09-04</subfield> <subfield code="w">2014-09-04</subfield> <subfield code="y">BK</subfield> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125591|0 |1 is obsolete| | --- Comment #13 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 125688 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125688&action=edit Bug 29135: Fix handling of deleted items in OAI-PMH provider Test plan: 1. Make sure this scenario now works properly: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135#c0 2. Run prove -v t/db_dependent/OAI/ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 --- Comment #14 from Ere Maijala <ere.maijala@helsinki.fi> --- David, Oops, silly me. I somehow thought it was clever that I made the timestamps start from an hour ago. This was fine until you created a dev box and ran the tests in an hour.. Should be fixed now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 --- Comment #15 from David Nind <david@davidnind.com> --- 8-) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 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=29135 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125688|0 |1 is obsolete| | --- Comment #16 from David Nind <david@davidnind.com> --- Created attachment 125718 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125718&action=edit Bug 29135: Fix handling of deleted items in OAI-PMH provider Test plan: 1. Make sure this scenario now works properly: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135#c0 2. Run prove -v t/db_dependent/OAI/ 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=29135 Nick Clemens <nick@bywatersolutions.com> 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=29135 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125718|0 |1 is obsolete| | --- Comment #17 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125955 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125955&action=edit Bug 29135: Fix handling of deleted items in OAI-PMH provider Test plan: 1. Make sure this scenario now works properly: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135#c0 2. Run prove -v t/db_dependent/OAI/ Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Can we get a proper commit message please? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125955|0 |1 is obsolete| | --- Comment #19 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 126029 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=126029&action=edit Bug 29135: Fix handling of deleted items in OAI-PMH provider When the OAI-PMH provider was configured to include item information in the returned records, deleting an item would cause the record to be reported as deleted. The provider also did some useless checks when determining the timestamp of a deleted record. These checks only had a performance impact. To test: 1) Create /var/lib/koha/kohadev/OAI.yaml with: format: marcxml: metadataPrefix: marcxml metadataNamespace: http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim schema: http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd include_items: 1 2) Set preference OAI-PMH to Enable 3) Set preference OAI-PMH:ConfFile to /var/lib/koha/kohadev/OAI.yaml 4) Delete an item from a record 5) View the page: http://localhost:8080/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml 6) Note the record is now listed as deleted 7) Run prove -v t/db_dependent/OAI/ Sponsored-by: The National Library of Finland Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Change sponsored?|--- |Sponsored --- Comment #20 from Ere Maijala <ere.maijala@helsinki.fi> --- Commit message revised. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 --- Comment #21 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Pushed to master for 21.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed, | |rel_21_11_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.11.00 |21.11.00,21.05.05 released in| | CC| |kyle@bywatersolutions.com Status|Pushed to master |Pushed to stable --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #23 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 27584 not in 20.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21105 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=30143 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29135 Joonas Kylmälä <joonas.kylmala@iki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |30143 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30143 [Bug 30143] OAI-PMH provider may end up in an eternal loop due to missing sort -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org