Koha needs to be able to harvest records from OAI-PMH repositories. This patch implements a cronjob-based OAI harvester, deliberately kept simple and consistent in its configuration with what we already have in Koha for other data sources, such as Z3950 and SRU. It is sponsored by KohaLa, its goals and implementation have been previously discussed on the koha and koha-devel mailing-lists, as well as in a presentation at KohaCon'23 in Helsinki: "Let's add an OAI-PMH harvester in Koha" by Sonia Bouis. Some of the code and ideas are based on a previous implementation of an OAI harvester by David Cook (See BZ 25905 and 10662).
Created attachment 160356 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa
Created attachment 160357 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update
This is great news! Once my January holidays are done, I can look more at these patches! At a glance, you will need to add unit tests for Koha::OAI::Client::Harvester before this can Pass QA (PERL17 in the coding guidelines), and that might require a bit of refactoring. (Missing unit tests are what ultimately bit me with bug 10662.) I think that we're not allowing function declarations in .pl scripts anymore either (for admin/oaiservers.pl) so that might need to be refactored. That said, I can't find a reference to it in the coding guidelines and the patches pass the QA scripts, so I could be wrong there. But other than that the patches are looking pretty good at a glance I think.
I'm having some trouble running /kohadevbox/koha/t/db_dependent/Koha/OAIServers.t as part of the QA scripts, but it might be my install. I'm going to try to refresh my ktd...
I updated my ktd and now I'm getting failures when running the QA tools, although I've got the unit test working. I notice there is a little typo in the pod in Koha/Import/Oaipmh/Authority.pm where it says "type" instead of "_type". Same with Koha/Import/Oaipmh/Biblio.pm and Koha/OaiServers.pm. Koha/OAI/Client/Harvester.pm is missing POD. I think the error for Koha/OaiServer.pm is false though. Not sure why that's getting a pod coverage error... I think the tidiness can also be ignored for the Schema files... -- qa -c 2 -v 2 testing 2 commit(s) (applied to ac02c1d 'ad Bug 35405: Remove noise from MARC:') Processing files before patches The PO directory has not been found. There is a problem in your Koha installation. at /kohadevbox/koha/Koha/I18N.pm line 197. |========================>| 27 / 27 (100.00%) Processing files after patches |========================>| 27 / 27 (100.00%) OK Koha/Import/Oaipmh/Authorities.pm FAIL Koha/Import/Oaipmh/Authority.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 FAIL Koha/Import/Oaipmh/Biblio.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 OK Koha/Import/Oaipmh/Biblios.pm FAIL Koha/OAI/Client/Harvester.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 FAIL Koha/OaiServer.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 OK Koha/OaiServers.pm FAIL Koha/Schema/Result/ImportOaipmhAuthority.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 33) FAIL Koha/Schema/Result/ImportOaipmhBiblio.pm FAIL pod coverage POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666 WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 28) FAIL Koha/Schema/Result/Oaiserver.pm FAIL pod coverage POD is missing for koha_object_class POD is missing for koha_objects_class WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 24) OK admin/oaiservers.pl OK installer/data/mysql/atomicupdate/Bug-35659.pl OK installer/data/mysql/en/mandatory/sample_notices.yml OK installer/data/mysql/kohastructure.sql OK installer/data/mysql/mandatory/sysprefs.sql OK koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc OK koha-tmpl/intranet-tmpl/prog/en/includes/oai-admin-search.inc OK koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc OK koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt OK koha-tmpl/intranet-tmpl/prog/en/modules/admin/oaiservers.tt OK koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl OK koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl OK koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl OK koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl OK koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl OK misc/cronjobs/harvest_oai.pl OK t/db_dependent/Koha/OAIServers.t Processing additional checks * Commit title does not start with 'Bug XXXXX: ' - 44b002f8cc Running tests (1) * Proving /kohadevbox/koha/t/db_dependent/Koha/OAIServers.t OK!
We are still missing supporters for this in the road map: https://wiki.koha-community.org/wiki/Road_map_24.05 *hint*
(In reply to Katrin Fischer from comment #6) > We are still missing supporters for this in the road map: > https://wiki.koha-community.org/wiki/Road_map_24.05 *hint* I've just added myself. Hopefully someone else can jump on board too.
(In reply to Katrin Fischer from comment #6) > We are still missing supporters for this in the road map: > https://wiki.koha-community.org/wiki/Road_map_24.05 *hint* Add myself as supporter :)
Created attachment 160744 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa
Hi David! The typo in Koha/Import/Oaipmh/Authority.pm, Koha/Import/Oaipmh/Biblio.pm and Koha/OaiServers.pm has been fixed. POD has been added to Koha/OAI/Client/Harvester.pm > I think the error for Koha/OaiServer.pm is false though. Not sure why that's getting a pod coverage error... About those, ashimema told me on IRC: 15:07 ashimema if they're new classes you can safely ignore that.. 15:07 there's a bug in the POD detection for new clases > I think the tidiness can also be ignored for the Schema files... That's what I thought too, since they are auto-generated.
I'm in my last week of holiday mode, so I should be more available for this in the coming weeks. Thanks Matthias for looking at that. I'll take another look soon at the QA tools output another time. It looks like Koha::OAI::Client::Harvester is still missing unit tests though. I think that's going to be a QA blocker.
Comment on attachment 160744 [details] [review] Bug 35659: OAI Harvester Review of attachment 160744 [details] [review]: ----------------------------------------------------------------- ::: Koha/OAI/Client/Harvester.pm @@ +127,5 @@ > + printlog("ListMetadataFormats failed"); > + }; > + > + #my $response = $h->Identify; > + #print "Repository supports protocol version ", $response->version, "\n"; Looks like some debugging code that needs to be removed here
(In reply to David Cook from comment #11) > It looks like Koha::OAI::Client::Harvester is still missing unit tests > though. I think that's going to be a QA blocker. At a minimum, I think the code handling "$response->next" should be in separate unit tested functions. (In theory, the other earlier code within the "new" constructor could probably be moved into an "init" function as well. But that is harder to test since it interacts with a real website. We could create a fake website but I think that would be a bit above and beyond.)
Created attachment 161813 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa
(In reply to David Cook from comment #13) > (In reply to David Cook from comment #11) > > It looks like Koha::OAI::Client::Harvester is still missing unit tests > > though. I think that's going to be a QA blocker. > > At a minimum, I think the code handling "$response->next" should be in > separate unit tested functions. > Hi! I've added unit tests for record processing.
(In reply to Matthias Meusburger from comment #15) > Hi! I've added unit tests for record processing. Thanks, Matthias! I've taken a quick glance at the unit tests and they're looking good! I've got a bit on my plate at the moment, but I'll come back to look at this soon! I think it would be good to find someone else to Sign Off on it, so that I can QA it.
Hi, I get an error when I try to apply the patch: Bug 35659 - OAI Harvester 160357 - DO NOT PUSH: Bug 35659 DBIC Update 161813 - Bug 35659: OAI Harvester Apply? [(y)es, (n)o, (i)nteractive] y Applying: DO NOT PUSH: Bug 35659 DBIC Update Applying: Bug 35659: OAI Harvester Using index info to reconstruct a base tree... M installer/data/mysql/mandatory/sysprefs.sql M installer/data/mysql/mandatory/userpermissions.sql M koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc M koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc Auto-merging installer/data/mysql/mandatory/userpermissions.sql Auto-merging installer/data/mysql/mandatory/sysprefs.sql CONFLICT (content): Merge conflict in installer/data/mysql/mandatory/sysprefs.sql error: Failed to merge in the changes.
Created attachment 161960 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa
(In reply to Michal Denar from comment #17) > Hi, > I get an error when I try to apply the patch: > Hi Michal, sorry about that. The patch has been rebased to master, it should be ok now.
Hi, this patch works for me. I don't know why, but I can't get it to unsign the signed patch and switch to SO. I've tried several times, but without success. Harvester works and it is possible to switch to signed off.
I get error: "Failed to update bug 35659, status=200" BZ login credentials are correct.
Created attachment 162004 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: John Doe <you@example.com>
Created attachment 162005 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: John Doe <you@example.com>
Created attachment 162006 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: John Doe <you@example.com>
Created attachment 162007 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: John Doe <you@example.com>
Created attachment 162008 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: Michal <black232gmail.com>
Created attachment 162009 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: John Doe <you@example.com> Signed-off-by: Michal <black232gmail.com>
Created attachment 162010 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: Michal <black23@gmail.com>
Created attachment 162011 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: John Doe <you@example.com> Signed-off-by: Michal <black232gmail.com> Signed-off-by: John Doe <you@example.com>
Created attachment 162527 [details] Screenshot - Accented characters in the OPAC
Just wanted to point out that I tried to harvest bibliographic records from the first repository suggested in the Test plan (http://staroai.theses.fr/OAIHandler, oai_dc data format) in a MARC21 Koha instance (therefore using OAIDCtoMARC21XML.xsl to transform the records prior to importing), and when I ran the OAI harvester script in verbose mode I got many lines similar to the following in the output: no mapping found for [0xC9] at position 0 in Économie politique g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. no mapping found for [0xC9] at position 0 in Église et société g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. The script did actually finish and managed to add 91 records, but I noticed that in the biblio details page in the OPAC some (not all) words containing accented characters did not appear correctly. I have uploaded a screenshot that illustrates this. Upon further investigation, it looks like the script *does* manage to populate database fields such as biblio.title and biblio.abstract with the correctly accented characters, but some MARC fields (such as 245 - Title, and 6xx - Subjects) in the MARCXML representation in biblio_metadata.metadata still contain weird characters wherever accents exist. The fix is to apply Franck Theeten's solution from Bug 16488 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16488#c24) and modify the value of the MARC leader's 10th character to 'a' in the XSLT. Then, the accented characters get imported properly and the records appear correctly in the OPAC.
Created attachment 162528 [details] [review] Bug 35659: (follow-up) Better handling of accented characters If you try to harvest bibliographic records from a UNIMARC OAI repository (using oai_dc data format) in a MARC21 Koha instance and run the OAI harvester script in verbose mode, you may get lines similar to the following in the output: no mapping found for [0xC9] at position 0 in Économie politique g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. no mapping found for [0xC9] at position 0 in Église et société g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. When looking at the imported records' biblio details page in the OPAC, most words containing accented characters will not appear correctly. The fix is to apply Franck Theeten's solution from Bug 16488 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16488#c24) and modify the value of the MARC leader's 10th character to 'a' in the XSLT that transforms the UNIMARC OAI records into MARC21 XML. Then, the accented characters get imported properly and the records appear correctly in the OPAC. Test plan: 0) Without this patch, running the OAI harvesting script in verbose mode produces many warnings, and garbled characters appear in the OPAC biblio details page wherever accented characters are in use. 1) Apply this patch. 2) Re-run the OAI harvesting script in verbose + force mode (force mode is required to ignore record datestamps from previous runs): misc/cronjobs/harvest_oai.pl -v -r <OAI_REPO_ID> -f This time there should be no warnings printed on your screen, and any characters with accents in the updated records should look OK in the OPAC. Thanks-to: Franck Theeten <franck.theeten@africamuseum.be>
Thank you Andreas for your feedback! Not sure if I can sign-off your follow-up myself, since I'm the author of the main patch, but I can confirm that it works as intended: warnings are gone, and encoding is now correct.
Created attachment 162692 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 162693 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: John Doe <you@example.com> Signed-off-by: Michal <black232gmail.com> Signed-off-by: John Doe <you@example.com> Signed-off-by: Michal Denar <black23@gmail.com>
Created attachment 162694 [details] [review] Bug 35659: (follow-up) Better handling of accented characters If you try to harvest bibliographic records from a UNIMARC OAI repository (using oai_dc data format) in a MARC21 Koha instance and run the OAI harvester script in verbose mode, you may get lines similar to the following in the output: no mapping found for [0xC9] at position 0 in Économie politique g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. no mapping found for [0xC9] at position 0 in Église et société g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. When looking at the imported records' biblio details page in the OPAC, most words containing accented characters will not appear correctly. The fix is to apply Franck Theeten's solution from Bug 16488 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16488#c24) and modify the value of the MARC leader's 10th character to 'a' in the XSLT that transforms the UNIMARC OAI records into MARC21 XML. Then, the accented characters get imported properly and the records appear correctly in the OPAC. Test plan: 0) Without this patch, running the OAI harvesting script in verbose mode produces many warnings, and garbled characters appear in the OPAC biblio details page wherever accented characters are in use. 1) Apply this patch. 2) Re-run the OAI harvesting script in verbose + force mode (force mode is required to ignore record datestamps from previous runs): misc/cronjobs/harvest_oai.pl -v -r <OAI_REPO_ID> -f This time there should be no warnings printed on your screen, and any characters with accents in the updated records should look OK in the OPAC. Thanks-to: Franck Theeten <franck.theeten@africamuseum.be> Signed-off-by: Michal Denar <black23@gmail.com>
(obsoleted duplicate old version of a patch)
Adding a server doesn't work due to not complying with CSRF (Bug 34478): > Programming error - op 'add_validated' must start with 'cud-' for POST http://localhost:8081/intranet/admin/oaiservers.pl (referer: http://localhost:8081/cgi-bin/koha/admin/oaiservers.pl?op=add) From Koha/Middleware/CSRF.pm
Created attachment 163527 [details] [review] Bug 35659: (follow-up) Make OAI repositories edition comply with CSRF
Thanks for your feedback Tuxayo!
Comment on attachment 162693 [details] [review] Bug 35659: OAI Harvester Review of attachment 162693 [details] [review]: ----------------------------------------------------------------- Overall this is working really well. OAI repositories mentioned in the test plan can take some time to respond (or fail with a timeout) but this is handled well. However, there are some minor things to fix (see the rest of the review). ::: Koha/OAI/Client/Harvester.pm @@ +79,5 @@ > +C<$force> force harvesting (ignore records datestamps) > + > +=cut > + > +sub new { The `new` method doesn't instantiate an object. Instead it sets global variables and returns the class name. I think it's confusing, as all other `new` methods in Koha instantiate an object. @@ +142,5 @@ > + > + if ($days) { > + > + # Change this to yyyy-mm-dd > + my $dt_today = dt_from_string(); This variable is not used anywhere. @@ +312,5 @@ > + $imported_record->update( > + { > + datestamp => $imported_record->datestamp, > + } > + ); This looks like it does nothing (set "record's datestamp" to "record's datestamp"). Shouldn't it be $oai_record->datestamp (or now if incoming oai record does not have a datestamp) ?
Side note: Koha::OAI::Client::Harvester uses cronlogaction, which makes it tied to the script using it (misc/cronjobs/harvest_oai.pl). As this script is the only way to use the harvester, I think it's okay to keep it like that for now, but it will have to be changed if we want to use the harvester elsewhere (a background job for instance)
It looks like CSRF validation does not work: if I manually change the token in the deletion form and then try to delete the corresponding OAI repository, the repository is deleted.
(In reply to Julian Maurice from comment #43) > It looks like CSRF validation does not work: if I manually change the token > in the deletion form and then try to delete the corresponding OAI > repository, the repository is deleted. Please ignore this comment, it was caused by a bad configuration on my instance
Created attachment 164586 [details] [review] Bug 35659: (QA follow-up)
Created attachment 164590 [details] [review] Bug 35659: (QA follow-up)
Created attachment 164602 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 164603 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 164604 [details] [review] Bug 35659: (follow-up) Better handling of accented characters If you try to harvest bibliographic records from a UNIMARC OAI repository (using oai_dc data format) in a MARC21 Koha instance and run the OAI harvester script in verbose mode, you may get lines similar to the following in the output: no mapping found for [0xC9] at position 0 in Économie politique g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. no mapping found for [0xC9] at position 0 in Église et société g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. When looking at the imported records' biblio details page in the OPAC, most words containing accented characters will not appear correctly. The fix is to apply Franck Theeten's solution from Bug 16488 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16488#c24) and modify the value of the MARC leader's 10th character to 'a' in the XSLT that transforms the UNIMARC OAI records into MARC21 XML. Then, the accented characters get imported properly and the records appear correctly in the OPAC. Test plan: 0) Without this patch, running the OAI harvesting script in verbose mode produces many warnings, and garbled characters appear in the OPAC biblio details page wherever accented characters are in use. 1) Apply this patch. 2) Re-run the OAI harvesting script in verbose + force mode (force mode is required to ignore record datestamps from previous runs): misc/cronjobs/harvest_oai.pl -v -r <OAI_REPO_ID> -f This time there should be no warnings printed on your screen, and any characters with accents in the updated records should look OK in the OPAC. Thanks-to: Franck Theeten <franck.theeten@africamuseum.be> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 164605 [details] [review] Bug 35659: (follow-up) Make OAI repositories edition comply with CSRF Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 164606 [details] [review] Bug 35659: (QA follow-up) Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 164618 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 164619 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 164620 [details] [review] Bug 35659: (follow-up) Better handling of accented characters If you try to harvest bibliographic records from a UNIMARC OAI repository (using oai_dc data format) in a MARC21 Koha instance and run the OAI harvester script in verbose mode, you may get lines similar to the following in the output: no mapping found for [0xC9] at position 0 in Économie politique g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. no mapping found for [0xC9] at position 0 in Église et société g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. When looking at the imported records' biblio details page in the OPAC, most words containing accented characters will not appear correctly. The fix is to apply Franck Theeten's solution from Bug 16488 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16488#c24) and modify the value of the MARC leader's 10th character to 'a' in the XSLT that transforms the UNIMARC OAI records into MARC21 XML. Then, the accented characters get imported properly and the records appear correctly in the OPAC. Test plan: 0) Without this patch, running the OAI harvesting script in verbose mode produces many warnings, and garbled characters appear in the OPAC biblio details page wherever accented characters are in use. 1) Apply this patch. 2) Re-run the OAI harvesting script in verbose + force mode (force mode is required to ignore record datestamps from previous runs): misc/cronjobs/harvest_oai.pl -v -r <OAI_REPO_ID> -f This time there should be no warnings printed on your screen, and any characters with accents in the updated records should look OK in the OPAC. Thanks-to: Franck Theeten <franck.theeten@africamuseum.be> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 164621 [details] [review] Bug 35659: (follow-up) Make OAI repositories edition comply with CSRF Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 164622 [details] [review] Bug 35659: (QA follow-up) Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
+ $dbh->do(q{DROP TABLE IF EXISTS `oaiservers`;}); You shouldn't drop the table but create it only if it does not exist.
Created attachment 165199 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
(In reply to Jonathan Druart from comment #57) > + $dbh->do(q{DROP TABLE IF EXISTS `oaiservers`;}); > > You shouldn't drop the table but create it only if it does not exist. Thanks Joubu, this is fixed.
Some comments: * Why is the `oaiservers` table name used instead of `oai_servers`? (same for oaiservers.{ps|tt}) * Why we sometimes call it `oai` and sometimes `oaipmh`? * OaiServer should either be OAI::Server or OAIServer. * Column names look old-school IMHO (`id`vs `oai_server_id`) * Can we pick names that match biblio_metadata columns for example? Like `format` and `schema` instead of `recordtype` * Is `datestamp` some OAI-PMH specific naming convention? * `repository` varchar(255) NOT NULL COMMENT 'OAI repository'? Shouldn't this be a FK on the `oaiservers` table? This feels important as we are also adding 'record sources' on another bug, and we will eventually want to set rules for OAI-PMH imported records. * Can the OAI harvester be thought as a background job in the mid/long term? In that case, it feels weird that Koha::OAI::Client::Harvester just prints things. It might be better to -for example- accumulate them using Koha::Object->add_message and have the caller decide what to do? (so harvest_oai.pl decides what to do) or it could just be a callback function for dealing with each step's output.
> * Column names look old-school IMHO (`id`vs `oai_server_id`) That's a definitely an issue: https://wiki.koha-community.org/wiki/Coding_Guidelines#SQL7:_Primary_keys «primary key name must be tablename_id (color_id for example) » Also it seems for new columns we are now using snake_case: https://wiki.koha-community.org/wiki/Database_updates#How_to_write_an_atomicupdate_file «Example 2: Adding a new column» Except for foreign keys like biblionumber > * Why is the `oaiservers` table name used instead of `oai_servers`? (same for oaiservers.{ps|tt}) +1 for consistency with how new stuff is usually named. --- Code: > sub ServerSearch This rule should also apply on new all new files outside C4: https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL9:_Subroutine_naming_conventions «subroutines in the Koha namespace should be snake case» --- String changes: #: installer/data/mysql/en/mandatory/sample_notices.yml:letter:1:row:98:mul:4 msgid "Set: %s" #: koha-tmpl/intranet-tmpl/prog/en/modules/admin/oaiservers.tt:92 msgid "Set: " +#: koha-tmpl/intranet-tmpl/prog/en/modules/admin/oaiservers.tt:136 +#, fuzzy, c-format +msgid "Set" They need context for translators: https://wiki.koha-community.org/wiki/Internationalization,_plural_forms,_context,_and_more_RFC#More_on_context And using tp() is an opportunity to deduplicate "Set: " and "Set" +#: koha-tmpl/intranet-tmpl/prog/en/modules/admin/oaiservers.tt:126 +#, c-format +msgid "%s You searched for record %s %s You searched for %s %s " This string should be split. Either via t() or <span> --- I reattached the patches to fix the order that broke applying.
Created attachment 165594 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 165595 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 165596 [details] [review] Bug 35659: (follow-up) Better handling of accented characters If you try to harvest bibliographic records from a UNIMARC OAI repository (using oai_dc data format) in a MARC21 Koha instance and run the OAI harvester script in verbose mode, you may get lines similar to the following in the output: no mapping found for [0xC9] at position 0 in Économie politique g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. no mapping found for [0xC9] at position 0 in Église et société g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. When looking at the imported records' biblio details page in the OPAC, most words containing accented characters will not appear correctly. The fix is to apply Franck Theeten's solution from Bug 16488 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16488#c24) and modify the value of the MARC leader's 10th character to 'a' in the XSLT that transforms the UNIMARC OAI records into MARC21 XML. Then, the accented characters get imported properly and the records appear correctly in the OPAC. Test plan: 0) Without this patch, running the OAI harvesting script in verbose mode produces many warnings, and garbled characters appear in the OPAC biblio details page wherever accented characters are in use. 1) Apply this patch. 2) Re-run the OAI harvesting script in verbose + force mode (force mode is required to ignore record datestamps from previous runs): misc/cronjobs/harvest_oai.pl -v -r <OAI_REPO_ID> -f This time there should be no warnings printed on your screen, and any characters with accents in the updated records should look OK in the OPAC. Thanks-to: Franck Theeten <franck.theeten@africamuseum.be> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 165597 [details] [review] Bug 35659: (follow-up) Make OAI repositories edition comply with CSRF Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 165598 [details] [review] Bug 35659: (QA follow-up) Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 167106 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 167107 [details] [review] Bug 35659: (QA follow-up) Rename Oaipmh to OAI Rename oaiservers to oai_servers Rename OaiServer to OAIServer Rename oai_servers.id to oai_servers.oai_server_id Rename ServerSearch to server_search Use a callback function for logs Use context for string "Set" Split translation string
Thanks for your feedback, Tomas and Victor! Tomas: * Why is the `oaiservers` table name used instead of `oai_servers`? (same for oaiservers.{ps|tt}) * Why we sometimes call it `oai` and sometimes `oaipmh`? * OaiServer should either be OAI::Server or OAIServer. * Column names look old-school IMHO (`id`vs `oai_server_id`) => Done * Can we pick names that match biblio_metadata columns for example? Like `format` and `schema` instead of `recordtype` => I'm not sure I understand what you mean by that. In this situation, recordtype is here to tell if the record is a biblio or an authority, while format and schema are not used for this. Can you elaborate on this? * Is `datestamp` some OAI-PMH specific naming convention? => Yes it is, see https://www.openarchives.org/OAI/openarchivesprotocol.html#SelectiveHarvestingandDatestamps for instance. * `repository` varchar(255) NOT NULL COMMENT 'OAI repository'? Shouldn't this be a FK on the `oaiservers` table? This feels important as we are also adding 'record sources' on another bug, and we will eventually want to set rules for OAI-PMH imported records. => Do you mean a FK from import_oai_biblios/import_oai_authorities.repository to oai_servers.endpoint? * Can the OAI harvester be thought as a background job in the mid/long term? In that case, it feels weird that Koha::OAI::Client::Harvester just prints things. It might be better to -for example- accumulate them using Koha::Object->add_message and have the caller decide what to do? (so harvest_oai.pl decides what to do) or it could just be a callback function for dealing with each step's output. => Done through a callback function. Victor: > sub ServerSearch => Done And using tp() is an opportunity to deduplicate "Set: " and "Set" => Done This string should be split. Either via t() or <span> => Done
Comment on attachment 165598 [details] [review] Bug 35659: (QA follow-up) It's not obsolete, it's colliding with the second commit of QA follow-ups
Created attachment 167171 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 167172 [details] [review] Bug 35659: (follow-up) Better handling of accented characters If you try to harvest bibliographic records from a UNIMARC OAI repository (using oai_dc data format) in a MARC21 Koha instance and run the OAI harvester script in verbose mode, you may get lines similar to the following in the output: no mapping found for [0xC9] at position 0 in Économie politique g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. no mapping found for [0xC9] at position 0 in Église et société g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. When looking at the imported records' biblio details page in the OPAC, most words containing accented characters will not appear correctly. The fix is to apply Franck Theeten's solution from Bug 16488 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16488#c24) and modify the value of the MARC leader's 10th character to 'a' in the XSLT that transforms the UNIMARC OAI records into MARC21 XML. Then, the accented characters get imported properly and the records appear correctly in the OPAC. Test plan: 0) Without this patch, running the OAI harvesting script in verbose mode produces many warnings, and garbled characters appear in the OPAC biblio details page wherever accented characters are in use. 1) Apply this patch. 2) Re-run the OAI harvesting script in verbose + force mode (force mode is required to ignore record datestamps from previous runs): misc/cronjobs/harvest_oai.pl -v -r <OAI_REPO_ID> -f This time there should be no warnings printed on your screen, and any characters with accents in the updated records should look OK in the OPAC. Thanks-to: Franck Theeten <franck.theeten@africamuseum.be> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 167173 [details] [review] Bug 35659: (follow-up) Make OAI repositories edition comply with CSRF Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 167174 [details] [review] Bug 35659: (QA follow-up) Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 167175 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 167176 [details] [review] Bug 35659: (more QA follow-ups) Rename Oaipmh to OAI Rename oaiservers to oai_servers Rename OaiServer to OAIServer Rename oai_servers.id to oai_servers.oai_server_id Rename ServerSearch to server_search Use a callback function for logs Use context for string "Set" Split translation string
> It's not obsolete, it's colliding with the second commit of QA follow-ups And it was preventing the next patches to apply. Changed the name of the second follow-up commit.
Unused import: use C4::Log qw( cronlogaction ); in Koha/OAI/Client/Harvester.pm --- To doublecheck that it was really unused and there wasn't some weird perl feature at play, I removed it and ran the tests. But that might not be reliable if coverage isn't good enough. So I removed this which was used in multiple places: use C4::AuthoritiesMarc qw (AddAuthority GuessAuthTypeCode ModAuthority DelAuthority ); And the tests still passed! [U+1F631] Now checking coverage more seriously with Devel::Cover https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL17:_Unit_tests_are_required from ./cover_db/coverage.html file stmt bran cond sub total Koha/OAI/Client/Harvester.pm 55.6 38.1 66.6 88.8 57.1 That's certainly too low :( The file is clickable to have a very nice view of it's code and colors to see which are the lines covered or not. And it can give details on which values for conditions aren't covered. And all the stuff from C4::AuthoritiesMarc was used in these uncovered parts. Looking at the per files reports showed that these files seem not used in tests vs the biblio one that are: Koha/Import/OAI/Authority.pm Koha/Import/OAI/Authorities.pm Despite their decent scores coming from the imports statements ^^" ---- Found very minor style issues: #use searchfield only in remaining operations [...] #find server(s) by id or name (need a space after #)
> (need a space after #) Some others cases lying around --- Found discrepancies between dbrev and kohastructure oai_servers: `id` int(11) vs `oai_server_id` import_oai_biblios: `id` int(11) vs `import_oai_biblio_id` int(11)
Created attachment 168982 [details] [review] Bug 35659: (more QA follow-ups) Rename Oaipmh to OAI Rename oaiservers to oai_servers Rename OaiServer to OAIServer Rename oai_servers.id to oai_servers.oai_server_id Rename ServerSearch to server_search Use a callback function for logs Use context for string "Set" Split translation string
Created attachment 168983 [details] [review] Bug 35659: (QA follow-up): Auth unit tests & dbrev Add unit tests for authorities and deletion Fix dbrev and kohastructure discrepancies
Hi Tuxayo! Thanks for your feedback. The last QA follow-up: "Auth unit tests & dbrev" fixes discrepancies between kohastructure and atomic update. It also improves coverage by adding unit tests for authorities and record deletion.
> - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version > 3.xx > Version 4.xx is needed. > See Bug 17704 for updating Koha default version to 4.xx Indeed, version 4.xx is needed to enable marc-xml harvesting. However, while testing the patches in this Bug report, I realised that once I upgrade HTTP::OAI::Harvester I can no longer retrieve all records for a specific oai_dc set. I'm not sure if marc-xml harvesting suffers from the same problem as I haven't tested that aspect. In any case, I have filed a GitHub Issue upstream at: https://github.com/LibreCat/perl-oai-lib/issues/10
Tests blow up :o And it's not because the DBIC patch is out of date. It is out of date though, I don't know if these DBIC commits are needed anymore now that the sandboxes have the "Refresh schema" action. Maybe they have another use? If not test plan needs minor update to test to refresh DBIC schema. Back to main issue, even after running `dbic`: kohadev-koha@kohadevbox:koha(testing-SO-QA)$ prove t/db_dependent/Koha/OAIHarvester.t t/db_dependent/Koha/OAIServers.t t/db_dependent/Koha/OAIHarvester.t .. 1/24 DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'me.id' in 'field list' at /kohadevbox/koha/Koha/Objects.pm line 95 # Looks like your test exited with 11 just after 1. t/db_dependent/Koha/OAIHarvester.t .. Dubious, test returned 11 (wstat 2816, 0xb00) Failed 23/24 subtests t/db_dependent/Koha/OAIServers.t .... # Looks like you planned 4 tests but ran 2. t/db_dependent/Koha/OAIServers.t .... 1/2 # Failed test 'new, find and delete tests' # at t/db_dependent/Koha/OAIServers.t line 68. DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'me.id' in 'field list' at /kohadevbox/koha/Koha/Objects.pm line 95 # Looks like your test exited with 255 just after 1. t/db_dependent/Koha/OAIServers.t .... Dubious, test returned 255 (wstat 65280, 0xff00)
Created attachment 169941 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update
Hi Tuxayo! I updated the DBIC patch. It it still needed for the additional koha_object_class and koha_objects_class functions in Koha/Schema/Result/OaiServer.pm, which are not auto-generated. After applying the patches on main, I can successfully run: kohadev-koha@kohadevbox:koha(BZ35659)$ qa -c 7 [...] Running tests (2) * Proving /kohadevbox/koha/t/db_dependent/Koha/OAIHarvester.t OK! * Proving /kohadevbox/koha/t/db_dependent/Koha/OAIServers.t OK! Running 'prove' however shows the following problem: t/00-check-atomic-updates.t ................. 1/? # Failed test 'check for unhandled atomic updates: Bug-35659.pl' This is the following test: unlike( $f, qr/.*pl$/, "check for unhandled atomic updates: $f" ); It checks that the atomic update file ends with .pl, and my file does. What am I missing here?
Oh, my bad, the test checks that the atomic update file does NOT end with .pl Is this test meant to be run after the atomic update file is integrated (and thus, deleted)? If this is the case, once the atomic update file deleted, I can run 'prove' successfully: kohadev-koha@kohadevbox:koha(BZ35659)$ prove [...] All tests successful. Files=77, Tests=5248, 51 wallclock secs
(In reply to Matthias Meusburger from comment #86) > I updated the DBIC patch. It it still needed for the additional > koha_object_class and koha_objects_class functions in > Koha/Schema/Result/OaiServer.pm, which are not auto-generated. Then the not auto-generated parts need to be split in their commit. Otherwise they will be lost because "DO NOT PUSH" when merged in main. So the result won't work. QA note: no relevant issue found with perltidy on the non-auto-generated parts of schema code.
> Is this test meant to be run after the atomic update > file is integrated (and thus, deleted)? It's meant to be ran by the CI to detect a dbrev that wasn't integrated by the RM or RMaint. So it should fail if ran at this stage of the process. (and it doesn't need to be ran) No issue there then :) --------- Thanks for the coverage improvements! stmt bran cond sub total 55.6 38.1 66.6 88.8 57.1 ↓↓↓↓↓ 68.1 64.2 83.3 88.4 70.3 Harvester->init() and it's 120 lines are not covered at all. I just added $harvester->init(); in the tests and coverage went from stmt bran cond sub total 68.1 64.2 83.3 88.4 70.3 to ↓↓↓↓↓ 82.9 71.4 83.3 96.1 82.6 There might be a smarter way to test that ^^" Maybe with a return value of 1 at end we check that? So we can have regression testing of at least it not blowing up ^^ But before that, the execution falls into `if ( $response->is_error )` because I likely forgot some setup. Any idea of what would be missing to have a working run of init()?
*** Bug 10662 has been marked as a duplicate of this bug. ***
Created attachment 171255 [details] [review] Bug 35659: (QA follow-up): Add init function tests by using Koha's OAI server
Created attachment 171258 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update
Created attachment 171260 [details] [review] Bug 35659: (QA follow-up): Add koha_object(s)_class functions for OaiServer.pm
Thanks again tuxayo for your feedback. The init function is now tested using Koha's OAI server. Also, the DBIC patch has been split in two.
Created attachment 171264 [details] [review] Bug 35659: (QA follow-up): Add init function tests by using Koha's OAI server
Created attachment 171265 [details] [review] Bug 35659: (QA follow-up): Add init function tests by using Koha's OAI server
Created attachment 171267 [details] [review] Bug 35659: (QA follow-up): Add init function tests by using Koha's OAI server
Created attachment 171675 [details] Screenshot - Navigation menu wrongly appears on the right I've noticed a couple of issues, please see the attached screenshots. 1) the main navigation menu appears on the right (instead of the left), this is due to the recent changes that upgraded Bootstrap to v5. 2) the label for the "Set:" text input has disappeared, this is due to a missing [% USE %] directive in the template file. 3) the command-line cron script cannot fetch/list the repositories due to changes in the DB schema (`id` got renamed to `oai_server_id`). Follow-up patches are on their way... ;-)
Created attachment 171676 [details] Screenshot - The label for the "Set:" text input is missing
Created attachment 171678 [details] [review] Bug 35659: (follow-up) Use new column name in cron script In an earlier patch, the `id` column of the `oai_servers` table was renamed to `oai_server_id`. However, the code in misc/cronjobs/harvest_oai.pl still uses the old column name, which results in the following error: $ misc/cronjobs/harvest_oai.pl --list DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'id' in 'order clause' at /kohadevbox/koha/Koha/Objects.pm line 403 This patch fixes that. Test plan: 1) $ misc/cronjobs/harvest_oai.pl --list [...you get the DBI Exception...] 2) Apply the patch. 3) $ misc/cronjobs/harvest_oai.pl --list [...list of OAI repositories...]
Created attachment 171679 [details] [review] Bug 35659: (follow-up) Adjust TT code for Bootstrap 5 In Bug 35402 the Bootstrap CSS framework was upgraded to version 5. This patch adjusts the relevant TT code for the OAI repositories administration page, and also adds a missing [% USE %] directive.
Created attachment 172072 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172073 [details] [review] Bug 35659: (follow-up) Better handling of accented characters If you try to harvest bibliographic records from a UNIMARC OAI repository (using oai_dc data format) in a MARC21 Koha instance and run the OAI harvester script in verbose mode, you may get lines similar to the following in the output: no mapping found for [0xC9] at position 0 in Économie politique g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. no mapping found for [0xC9] at position 0 in Église et société g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. When looking at the imported records' biblio details page in the OPAC, most words containing accented characters will not appear correctly. The fix is to apply Franck Theeten's solution from Bug 16488 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16488#c24) and modify the value of the MARC leader's 10th character to 'a' in the XSLT that transforms the UNIMARC OAI records into MARC21 XML. Then, the accented characters get imported properly and the records appear correctly in the OPAC. Test plan: 0) Without this patch, running the OAI harvesting script in verbose mode produces many warnings, and garbled characters appear in the OPAC biblio details page wherever accented characters are in use. 1) Apply this patch. 2) Re-run the OAI harvesting script in verbose + force mode (force mode is required to ignore record datestamps from previous runs): misc/cronjobs/harvest_oai.pl -v -r <OAI_REPO_ID> -f This time there should be no warnings printed on your screen, and any characters with accents in the updated records should look OK in the OPAC. Thanks-to: Franck Theeten <franck.theeten@africamuseum.be> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172074 [details] [review] Bug 35659: (follow-up) Make OAI repositories edition comply with CSRF Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172075 [details] [review] Bug 35659: (QA follow-up) Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172076 [details] [review] Bug 35659: (more QA follow-ups) Rename Oaipmh to OAI Rename oaiservers to oai_servers Rename OaiServer to OAIServer Rename oai_servers.id to oai_servers.oai_server_id Rename ServerSearch to server_search Use a callback function for logs Use context for string "Set" Split translation string Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172077 [details] [review] Bug 35659: (QA follow-up): Auth unit tests & dbrev Add unit tests for authorities and deletion Fix dbrev and kohastructure discrepancies Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172078 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172079 [details] [review] Bug 35659: (QA follow-up): Add koha_object(s)_class functions for OaiServer.pm Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172080 [details] [review] Bug 35659: (QA follow-up): Add init function tests by using Koha's OAI server Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172081 [details] [review] Bug 35659: (follow-up) Use new column name in cron script In an earlier patch, the `id` column of the `oai_servers` table was renamed to `oai_server_id`. However, the code in misc/cronjobs/harvest_oai.pl still uses the old column name, which results in the following error: $ misc/cronjobs/harvest_oai.pl --list DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'id' in 'order clause' at /kohadevbox/koha/Koha/Objects.pm line 403 This patch fixes that. Test plan: 1) $ misc/cronjobs/harvest_oai.pl --list [...you get the DBI Exception...] 2) Apply the patch. 3) $ misc/cronjobs/harvest_oai.pl --list [...list of OAI repositories...] Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172082 [details] [review] Bug 35659: (follow-up) Adjust TT code for Bootstrap 5 In Bug 35402 the Bootstrap CSS framework was upgraded to version 5. This patch adjusts the relevant TT code for the OAI repositories administration page, and also adds a missing [% USE %] directive. Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172083 [details] [review] Bug 35659: (follow-up) fix auth oai to MARC21 Signed-off-by: David Cook <dcook@prosentient.com.au>
Passed QA! I sat with Matthias at Kohacon2024 hackfest and we chatted about it, and tested it side by side. I found a little issue with the provided XSLT for authority oai_dc to marc21, but I've provided a patch for that. I haven't verified the UNIMARC XSLTs, but technically you can provide any XSLT you like, so each instance can do any XSLT they like. I've verified that the OAI harvest mechanism works as intended, and it's all looking good to me. There are probably some additional tweaks I'd like to make in the long-term, but those can be done in the future as follow-ups, if they're really required. The perfect is the enemy of the good, and I think that this is a good change for us to make. Let's get this in, and then we can always polish more as we go. Hurray!
Just noting that the QA tool warnings should all be irrelevant. The ones about tidiness and POD coverage are for generated files or for Koha::Object subclasses which appear to be correct. I recall the POD coverage sometimes being wrong for those. There are some complaints about commit title wording, but I think those are ignorable as well.
Created attachment 172202 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172203 [details] [review] Bug 35659: (follow-up) Better handling of accented characters If you try to harvest bibliographic records from a UNIMARC OAI repository (using oai_dc data format) in a MARC21 Koha instance and run the OAI harvester script in verbose mode, you may get lines similar to the following in the output: no mapping found for [0xC9] at position 0 in Économie politique g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. no mapping found for [0xC9] at position 0 in Église et société g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. When looking at the imported records' biblio details page in the OPAC, most words containing accented characters will not appear correctly. The fix is to apply Franck Theeten's solution from Bug 16488 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16488#c24) and modify the value of the MARC leader's 10th character to 'a' in the XSLT that transforms the UNIMARC OAI records into MARC21 XML. Then, the accented characters get imported properly and the records appear correctly in the OPAC. Test plan: 0) Without this patch, running the OAI harvesting script in verbose mode produces many warnings, and garbled characters appear in the OPAC biblio details page wherever accented characters are in use. 1) Apply this patch. 2) Re-run the OAI harvesting script in verbose + force mode (force mode is required to ignore record datestamps from previous runs): misc/cronjobs/harvest_oai.pl -v -r <OAI_REPO_ID> -f This time there should be no warnings printed on your screen, and any characters with accents in the updated records should look OK in the OPAC. Thanks-to: Franck Theeten <franck.theeten@africamuseum.be> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172204 [details] [review] Bug 35659: (follow-up) Make OAI repositories edition comply with CSRF Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172205 [details] [review] Bug 35659: (QA follow-up) Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172206 [details] [review] Bug 35659: (more QA follow-ups) Rename Oaipmh to OAI Rename oaiservers to oai_servers Rename OaiServer to OAIServer Rename oai_servers.id to oai_servers.oai_server_id Rename ServerSearch to server_search Use a callback function for logs Use context for string "Set" Split translation string Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172207 [details] [review] Bug 35659: (QA follow-up): Auth unit tests & dbrev Add unit tests for authorities and deletion Fix dbrev and kohastructure discrepancies Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172208 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172209 [details] [review] Bug 35659: (QA follow-up): Add koha_object(s)_class functions for OaiServer.pm Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172210 [details] [review] Bug 35659: (QA follow-up): Add init function tests by using Koha's OAI server Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172211 [details] [review] Bug 35659: (follow-up) Use new column name in cron script In an earlier patch, the `id` column of the `oai_servers` table was renamed to `oai_server_id`. However, the code in misc/cronjobs/harvest_oai.pl still uses the old column name, which results in the following error: $ misc/cronjobs/harvest_oai.pl --list DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'id' in 'order clause' at /kohadevbox/koha/Koha/Objects.pm line 403 This patch fixes that. Test plan: 1) $ misc/cronjobs/harvest_oai.pl --list [...you get the DBI Exception...] 2) Apply the patch. 3) $ misc/cronjobs/harvest_oai.pl --list [...list of OAI repositories...] Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172212 [details] [review] Bug 35659: (follow-up) Adjust TT code for Bootstrap 5 In Bug 35402 the Bootstrap CSS framework was upgraded to version 5. This patch adjusts the relevant TT code for the OAI repositories administration page, and also adds a missing [% USE %] directive. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Created attachment 172213 [details] [review] Bug 35659: (follow-up) fix auth oai to MARC21 Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au>
Koha/OAI/Client/Harvester.pm stmt bran cond sub total 68.1 64.2 83.3 88.4 70.3 ↓↓↓↓↓ 93.0 78.5 83.3 96.1 90.9 Thanks a lot, the improvement is great ! Coverage of changes is good 👍 As a bonus, there is even a bit more coverage in the server. As far as I can tell, all good with the coding guidelines. ---- Thanks David for the second look on the technical stuff and a look at the functional side that I couldn't have done. (In reply to David Cook from comment #115) > Just noting that the QA tool warnings should all be irrelevant. Confirmed. ---- Any sponsor for this BTW?
(In reply to Victor Grousset/tuxayo from comment #128) > Any sponsor for this BTW? I think KohaLa sponsored this one.
Yes, sponsored by Kohala association
(In reply to Mathieu Saby from comment #130) > Yes, sponsored by Kohala association Matts, I had started on this, but then got distracted and lost my first batch of notes. :( I will come back to this, but could you already do some smaller fixes maybe? 1) Add the sponsorship lines into the commits? Sponsored-by: ... 2) Fix OAI Harvest/OAI Harvesting to OAI harvest? (capitalization) 3) Check the system preference description so that it forms a proper sentence. That's what I remembered, but I'll be back soon!
Created attachment 173088 [details] [review] Bug 35659: OAI Harvester Test plan: - Run updatedatabase.pl - Apply the DBIC update patch - Setup an OAI repository in Administration -> OAI repositories (admin/oaiservers.pl) If the repository dataformat is in oai_dc, you can use the sample XSLT files: For Unimarc: OAIDCtoUNIMARCXML.xsl for bibliographic records AuthOAIDCtoUNIMARCXML.xsl for authorities For Marc21: OAIDCtoMARC21XML.xsl for bibliographic records AuthOAIDCtoMARC21XML.xsl for authorities If the repository dataformat is in marc-xml, you can use OAIMarcxml2KohaMarcxml.xsl for both Unimarc and Marc21. - Some repositories to test with: Bibliographic records, oai_dc: Endpoint: http://staroai.theses.fr/OAIHandler Set: ddc:260 XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIDCtoMARC21XML.xsl Authorities, oai_dc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoUNIMARCXML.xsl or XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/AuthOAIDCtoMARC21XML.xsl Authorities, marc-xml, Unimarc: Endpoint: https://www.idref.fr/OAI/oai.jsp Set: a XSLT: <path_to_src>/koha-tmpl/intranet-tmpl/prog/en/xslt/OAIMarcxml2KohaMarcxml.xsl - Note: marc-xml harvesting will not work with HTTP::OAI::Harvester version 3.xx Version 4.xx is needed. See Bug 17704 for updating Koha default version to 4.xx - If you want to receive a report email, fill the OAI-PMH:HarvestEmailReport syspref. The model used (OAI_HARVEST_REPORT) can be edited in tools -> Notices and slips. - start a harvest with the following command: misc/cronjobs/harvest_oai.pl This script starts an OAI Harvest This script has the following parameters : -h --help: this message -v --verbose (enable output to stdout) -r --repository: id of the OAI repository -d --days: number of days to harvest from (optional) -l --list: list the OAI repositories -f --force: force harvesting (ignore records datestamps) (optional) Example: misc/cronjobs/harvest_oai.pl -r 1 -d 10 -v -f - Check the logs in the Log Viewer (tools/viewlog.pl) (CronjobLog syspref must be activated) Sponsored-by: KohaLa Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173089 [details] [review] Bug 35659: (follow-up) Better handling of accented characters If you try to harvest bibliographic records from a UNIMARC OAI repository (using oai_dc data format) in a MARC21 Koha instance and run the OAI harvester script in verbose mode, you may get lines similar to the following in the output: no mapping found for [0xC9] at position 0 in Économie politique g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. no mapping found for [0xC9] at position 0 in Église et société g0=ASCII_DEFAULT g1=EXTENDED_LATIN at /usr/share/perl5/MARC/Charset.pm line 308. When looking at the imported records' biblio details page in the OPAC, most words containing accented characters will not appear correctly. The fix is to apply Franck Theeten's solution from Bug 16488 (https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16488#c24) and modify the value of the MARC leader's 10th character to 'a' in the XSLT that transforms the UNIMARC OAI records into MARC21 XML. Then, the accented characters get imported properly and the records appear correctly in the OPAC. Test plan: 0) Without this patch, running the OAI harvesting script in verbose mode produces many warnings, and garbled characters appear in the OPAC biblio details page wherever accented characters are in use. 1) Apply this patch. 2) Re-run the OAI harvesting script in verbose + force mode (force mode is required to ignore record datestamps from previous runs): misc/cronjobs/harvest_oai.pl -v -r <OAI_REPO_ID> -f This time there should be no warnings printed on your screen, and any characters with accents in the updated records should look OK in the OPAC. Thanks-to: Franck Theeten <franck.theeten@africamuseum.be> Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173090 [details] [review] Bug 35659: (follow-up) Make OAI repositories edition comply with CSRF Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173091 [details] [review] Bug 35659: (QA follow-up) Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173092 [details] [review] Bug 35659: (more QA follow-ups) Rename Oaipmh to OAI Rename oaiservers to oai_servers Rename OaiServer to OAIServer Rename oai_servers.id to oai_servers.oai_server_id Rename ServerSearch to server_search Use a callback function for logs Use context for string "Set" Split translation string Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173093 [details] [review] Bug 35659: (QA follow-up): Auth unit tests & dbrev Add unit tests for authorities and deletion Fix dbrev and kohastructure discrepancies Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173094 [details] [review] DO NOT PUSH: Bug 35659 DBIC Update Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173095 [details] [review] Bug 35659: (QA follow-up): Add koha_object(s)_class functions for OaiServer.pm Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173096 [details] [review] Bug 35659: (QA follow-up): Add init function tests by using Koha's OAI server Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173097 [details] [review] Bug 35659: (follow-up) Use new column name in cron script In an earlier patch, the `id` column of the `oai_servers` table was renamed to `oai_server_id`. However, the code in misc/cronjobs/harvest_oai.pl still uses the old column name, which results in the following error: $ misc/cronjobs/harvest_oai.pl --list DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'id' in 'order clause' at /kohadevbox/koha/Koha/Objects.pm line 403 This patch fixes that. Test plan: 1) $ misc/cronjobs/harvest_oai.pl --list [...you get the DBI Exception...] 2) Apply the patch. 3) $ misc/cronjobs/harvest_oai.pl --list [...list of OAI repositories...] Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173098 [details] [review] Bug 35659: (follow-up) Adjust TT code for Bootstrap 5 In Bug 35402 the Bootstrap CSS framework was upgraded to version 5. This patch adjusts the relevant TT code for the OAI repositories administration page, and also adds a missing [% USE %] directive. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173099 [details] [review] Bug 35659: (follow-up) fix auth oai to MARC21 Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: David Cook <dcook@prosentient.com.au> Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173100 [details] [review] Bug 35659: (QA follow-up) Update syspref description to be a sentence Sponsored-by: Association KohaLa - https://koha-fr.org/
Created attachment 173101 [details] [review] Bug 35659: (QA follow-up) Fix capitalisation in notice Sponsored-by: Association KohaLa - https://koha-fr.org/
(In reply to Katrin Fischer from comment #131) > 1) Add the sponsorship lines into the commits? Sponsored-by: ... git rebase --exec 'git commit --amend --trailer "Sponsored-by: Association KohaLa - https://koha-fr.org/" --no-edit' HEAD~12 Boom! (I'm going to add that one to the wiki...) > 2) Fix OAI Harvest/OAI Harvesting to OAI harvest? (capitalization) Whereabouts? In the notice? I've updated the following: installer/data/mysql/en/mandatory/sample_notices.yml installer/data/mysql/atomicupdate/Bug-35659.pl > 3) Check the system preference description so that it forms a proper > sentence. I think I understand. I've updated the following: koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/web_services.pref installer/data/mysql/mandatory/sysprefs.sql installer/data/mysql/atomicupdate/Bug-35659.pl
Also, since some of these follow-ups are pretty trivial, could we do them as a follow-up bug instead?
(In reply to David Cook from comment #147) > Also, since some of these follow-ups are pretty trivial, could we do them as > a follow-up bug instead? As they are so trivial I think follow-ups here should be enough. I didn't stop testing here, it's just a big piece that I need some extra big time slot for. For the capitalization: git grep "OAI-PMH H" or git grep "OAI Harv" is what I would usually do. There were a few occurences. Thanks for the follow-up and the wiki update!
Hi all, thanks for the hard work on this! 1) Position on admin start page (non blocker) Right now the OAI repositories live under "Additional parameters" instead of catalog like the OAI sets. I wonder if we should move them up since this also concerns the catalog :). If we do, let's not forget the module navigation as well. 2) Empty column heading (please follow-up) For some unknown to me reason, this doesn't appear to work as expected: <thead><tr><th>ID</th><th>Repository name</th><th>Endpoint</th><th>[% tp('OAI', 'Set') | html %]</th><th>Data format</th><th>Record type</th><th class="noExport">Actions</th> The column header for Set remains empty in my testing.
Pushed for 24.11! Well done everyone, thank you!
(In reply to Katrin Fischer from comment #149) > Hi all, > > thanks for the hard work on this! > > 1) Position on admin start page (non blocker) > > Right now the OAI repositories live under "Additional parameters" instead of > catalog like the OAI sets. I wonder if we should move them up since this > also concerns the catalog :). If we do, let's not forget the module > navigation as well. > > 2) Empty column heading (please follow-up) > > For some unknown to me reason, this doesn't appear to work as expected: > > <thead><tr><th>ID</th><th>Repository > name</th><th>Endpoint</th><th>[% tp('OAI', 'Set') | html %]</th><th>Data > format</th><th>Record type</th><th class="noExport">Actions</th> > > The column header for Set remains empty in my testing. I solved 2 with [% PROCESS 'i18n.inc' %] after a hint from Joubu. Just 1) to check into :)
Hi! It made sense to me that the OAI repositories should be in the same category as "Z39.50/SRU servers", since they do the same thing: tell Koha where to connect to retrieve records. Hence "Additional parameters".
(In reply to Matthias Meusburger from comment #152) > Hi! > > It made sense to me that the OAI repositories should be in the same category > as "Z39.50/SRU servers", since they do the same thing: tell Koha where to > connect to retrieve records. Hence "Additional parameters". Maybe both should be in catalog :D But I am OK with that explanation, we will see if it comes up again.
This patchset broke t/db_dependent/TestBuilder.t. And t/db_dependent/Koha/OAIHarvester.t is failing on Jenkins Koha_Main/3076
admin/oai_servers.pl and .tt are really old style written, could have been nice to see them follow the current standard. 1. msg_added => should use a @messages array 2. add_form and op => Why 2 variables for the same purpose? 3. No try-catch! 4. type => $op eq 'add' ? lc $type : '' What is "type"? Is not used in template.
(In reply to Jonathan Druart from comment #154) > This patchset broke t/db_dependent/TestBuilder.t. And > t/db_dependent/Koha/OAIHarvester.t is failing on Jenkins > > Koha_Main/3076 Joubu beat me to it, please fix urgently.
Created attachment 173561 [details] [review] Bug 35659: Adjust DT's option names See bug 34913
(In reply to Jonathan Druart from comment #154) > t/db_dependent/Koha/OAIHarvester.t I think I see the problem here. I'll look more at this one in a moment. > t/db_dependent/TestBuilder.t Looking at this one now...
Created attachment 173599 [details] [review] Bug 35659: (QA follow-up) fix t/db_dependent/TestBuilder.t
Created attachment 173600 [details] [review] Bug 35659: (QA follow-up) Fix t/db_dependent/Koha/OAIHarvester.t Previously, this test just checked the last 1 day of bib activity, but on a fresh test system there will be no activity, so it will always fail. Instead, let's just check that a number is returned for the total, even if that total number is 0, since 0 is a valid response too.
Created attachment 173601 [details] [review] Bug 35659: (QA follow-up) Fix t/db_dependent/Koha/OAIHarvester.t Previously, this test just checked the last 1 day of bib activity, but on a fresh test system there will be no activity, so it will always fail. Instead, let's just check that a number is returned for the total, even if that total number is 0, since 0 is a valid response too.
(In reply to Katrin Fischer from comment #156) > (In reply to Jonathan Druart from comment #154) > > This patchset broke t/db_dependent/TestBuilder.t. And > > t/db_dependent/Koha/OAIHarvester.t is failing on Jenkins > > > > Koha_Main/3076 > > Joubu beat me to it, please fix urgently. OK. These should be fixed now. Please see last 2 patches attached
I am sorry, we now have the situation where prove t/db_dependent/Koha/OAIHarvester.t passes for me locally, but fails on Jenkins main/D12 (on dashboard). Can you have another go please?
* 55 my $strp = DateTime::Format::Strptime->new( Not used? * 139 my $dt_start = DateTime->new( year => $date[0], month => $date[1], day => $date[2] ); You are supposed to use dt_from_string, not DateTime directly.
Created attachment 173614 [details] [review] Bug 35659: (QA follow-up) Fix t/db_dependent/Koha/OAIHarvester.t - OPACBaseURL must be set correctly.
Bug 35659: (QA follow-up) Fix t/db_dependent/Koha/OAIHarvester.t pushed to main.
Still 2 important comments here.
This doesn't look quite right to me and will be untranslatable/cannot be searched for: misc/cronjobs/harvest_oai.pl:cronlogaction( { action => 'Start', info => "Starting OAI Harvest" } ); misc/cronjobs/harvest_oai.pl:cronlogaction( { info => "Command line: $command_line_options" } ); misc/cronjobs/harvest_oai.pl:cronlogaction( { action => 'End', info => "Ending OAI Harvest" } ); For comparison: misc/cronjobs/batch_anonymise.pl:use C4::Log qw( cronlogaction ); misc/cronjobs/batch_anonymise.pl:cronlogaction({ info => $command_line_options }); misc/cronjobs/batch_anonymise.pl:cronlogaction({ action => 'End', info => "COMPLETED" }); misc/cronjobs/build_browser_and_cloud.pl:cronlogaction({ info => $command_line_options }); misc/cronjobs/build_browser_and_cloud.pl:cronlogaction({ action => 'End', info => "COMPLETED" }); misc/cronjobs/cart_to_shelf.pl:use C4::Log qw( cronlogaction ); misc/cronjobs/cart_to_shelf.pl:cronlogaction({ info => $command_line_options }); misc/cronjobs/cart_to_shelf.pl:cronlogaction({ action => 'End', info => "COMPLETED" }); misc/cronjobs/cleanup_database.pl:use C4::Log qw( cronlogaction ); misc/cronjobs/cleanup_database.pl:cronlogaction( { info => $command_line_options } ); misc/cronjobs/cleanup_database.pl:cronlogaction( { action => 'End', info => "COMPLETED" } );
New feature, no 24.05.x backport
Created attachment 174790 [details] [review] Bug 35659: (QA follow-up) Fix DateTime and cronlogaction messages
Follow-up "(QA follow-up) Fix DateTime and cronlogaction messages" picked for main.
There is still comment 155 that has been ignored but I am going to stop fighting here...
(In reply to Jonathan Druart from comment #172) > There is still comment 155 that has been ignored but I am going to stop > fighting here... I am moving them into a separate bug because of closeness to release.