Created attachment 58348 [details] [review] Bug 17807: Add format parameter to XSLT_Handler->transform Format may be chars (default), bytes or xmldoc. Note: xmldoc is a XML::LibXML document object. Since the default is chars, this does not affect current use. Note: The format parameter (xmldoc) will be used later in one of the OAI modules to prevent duplicated xml parsing. Test plan: Run t/db_dependent/XSLT_Handler.t
Created attachment 58349 [details] [review] Bug 17807: Use XSLT_Handler in Koha::OAI Replacing some code in the OAI modules by a call to an existing module. Note that the xmldoc format is used in the transform call to get a xml document object. The stylesheet method of Repository now only returns the name of the xsl file to be used instead of a cached xslt object. Similar functionality inside XSLT_Handler is used when calling transform. Note: We still lack unit tests in this area. I did not see the need for adding something for stylesheet since it only returns a simple string. The other change is made in Record::new; there are no tests for this module yet and the heart of the change here is actually tested already in XSLT_Handler.t. Note: I benchmarked calls to Repository in the old and the new situation and did not see significant changes. Test plan: [1] Run t/db_dependent/Koha/OAI/Server.t [2] Run oai.pl with ListRecords and marcxml. [3] Run oai.pl with ListRecords and oai_dc.
Created attachment 63463 [details] [review] Bug 17807: Add format parameter to XSLT_Handler->transform Format may be chars (default), bytes or xmldoc. Note: xmldoc is a XML::LibXML document object. Since the default is chars, this does not affect current use. Note: The format parameter (xmldoc) will be used later in one of the OAI modules to prevent duplicated xml parsing. Test plan: Run t/db_dependent/XSLT_Handler.t
Created attachment 63464 [details] [review] Bug 17807: Use XSLT_Handler in Koha::OAI Replacing some code in the OAI modules by a call to an existing module. Note that the xmldoc format is used in the transform call to get a xml document object. The stylesheet method of Repository now only returns the name of the xsl file to be used instead of a cached xslt object. Similar functionality inside XSLT_Handler is used when calling transform. Note: We still lack unit tests in this area. I did not see the need for adding something for stylesheet since it only returns a simple string. The other change is made in Record::new; there are no tests for this module yet and the heart of the change here is actually tested already in XSLT_Handler.t. Note: I benchmarked calls to Repository in the old and the new situation and did not see significant changes. Test plan: [1] Run t/db_dependent/Koha/OAI/Server.t [2] Run oai.pl with ListRecords and marcxml. [3] Run oai.pl with ListRecords and oai_dc.
Rebased (waiting since Dec 21)
Created attachment 63935 [details] [review] Bug 17807: Use XSLT_Handler in Koha::OAI Replacing some code in the OAI modules by a call to an existing module. Note that the xmldoc format is used in the transform call to get a xml document object. The stylesheet method of Repository now only returns the name of the xsl file to be used instead of a cached xslt object. Similar functionality inside XSLT_Handler is used when calling transform. Note: We still lack unit tests in this area. I did not see the need for adding something for stylesheet since it only returns a simple string. The other change is made in Record::new; there are no tests for this module yet and the heart of the change here is actually tested already in XSLT_Handler.t. Note: I benchmarked calls to Repository in the old and the new situation and did not see significant changes. Test plan: [1] Run t/db_dependent/OAI/Server.t [2] Run oai.pl with ListRecords and marcxml. [3] Run oai.pl with ListRecords and oai_dc.
Marcel, when I try to get record in marc21 metadata format, I am getting the oai_dc record.... I should get marcxml....
(In reply to Josef Moravec from comment #7) > Marcel, when I try to get record in marc21 metadata format, I am getting the > oai_dc record.... I should get marcxml.... I tested again and I see DC output and marcxml. Did you restart Plack? Can you show the URLs you are using?
/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=oai_dc --> oai_dc, that's OK /cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml --> marcxml, that's OK /cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marc21 --> oai_dc, that's NOT OK
Created attachment 64045 [details] [review] Bug 17807: [Follow-up] Use marcxml for marc21 prefix Not sure what happened here, but this is an easy fix. Test plan: Try oai.pl with marcxml, marc21 and oai_dc again.
Created attachment 64046 [details] [review] [SIGNED-OFF] Bug 17807: Add format parameter to XSLT_Handler->transform Format may be chars (default), bytes or xmldoc. Note: xmldoc is a XML::LibXML document object. Since the default is chars, this does not affect current use. Note: The format parameter (xmldoc) will be used later in one of the OAI modules to prevent duplicated xml parsing. Test plan: Run t/db_dependent/XSLT_Handler.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 64047 [details] [review] [SIGNED-OFF] Bug 17807: Use XSLT_Handler in Koha::OAI Replacing some code in the OAI modules by a call to an existing module. Note that the xmldoc format is used in the transform call to get a xml document object. The stylesheet method of Repository now only returns the name of the xsl file to be used instead of a cached xslt object. Similar functionality inside XSLT_Handler is used when calling transform. Note: We still lack unit tests in this area. I did not see the need for adding something for stylesheet since it only returns a simple string. The other change is made in Record::new; there are no tests for this module yet and the heart of the change here is actually tested already in XSLT_Handler.t. Note: I benchmarked calls to Repository in the old and the new situation and did not see significant changes. Test plan: [1] Run t/db_dependent/OAI/Server.t [2] Run oai.pl with ListRecords and marcxml. [3] Run oai.pl with ListRecords and oai_dc. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 64048 [details] [review] [SIGNED-OFF] Bug 17807: [Follow-up] Use marcxml for marc21 prefix Not sure what happened here, but this is an easy fix. Test plan: Try oai.pl with marcxml, marc21 and oai_dc again. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
(In reply to Marcel de Rooy from comment #10) > Created attachment 64045 [details] [review] [review] > Bug 17807: [Follow-up] Use marcxml for marc21 prefix > > Not sure what happened here, but this is an easy fix. > > Test plan: > Try oai.pl with marcxml, marc21 and oai_dc again. Thanks for quick followup, now it's working perfect, so I signing off ;)
(In reply to Josef Moravec from comment #14) > Thanks for quick followup, now it's working perfect, so I signing off ;) Same for you !
Everything seems to work fine here, however, errors are really noisy if the file is not found - this shouldn't be an issue really since file isn't configurable, but should it be a little quieter? Waiting for answer before PQA
(In reply to Nick Clemens from comment #16) > Everything seems to work fine here, however, errors are really noisy if the > file is not found - this shouldn't be an issue really since file isn't > configurable, but should it be a little quieter? Waiting for answer before > PQA File should be there obviously, but you are right. We could reduce the noise a bit. Opened a new report for that.
Created attachment 65534 [details] [review] Bug 17807: Add format parameter to XSLT_Handler->transform Format may be chars (default), bytes or xmldoc. Note: xmldoc is a XML::LibXML document object. Since the default is chars, this does not affect current use. Note: The format parameter (xmldoc) will be used later in one of the OAI modules to prevent duplicated xml parsing. Test plan: Run t/db_dependent/XSLT_Handler.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 65535 [details] [review] Bug 17807: Use XSLT_Handler in Koha::OAI Replacing some code in the OAI modules by a call to an existing module. Note that the xmldoc format is used in the transform call to get a xml document object. The stylesheet method of Repository now only returns the name of the xsl file to be used instead of a cached xslt object. Similar functionality inside XSLT_Handler is used when calling transform. Note: We still lack unit tests in this area. I did not see the need for adding something for stylesheet since it only returns a simple string. The other change is made in Record::new; there are no tests for this module yet and the heart of the change here is actually tested already in XSLT_Handler.t. Note: I benchmarked calls to Repository in the old and the new situation and did not see significant changes. Test plan: [1] Run t/db_dependent/OAI/Server.t [2] Run oai.pl with ListRecords and marcxml. [3] Run oai.pl with ListRecords and oai_dc. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 65536 [details] [review] Bug 17807: [Follow-up] Use marcxml for marc21 prefix Not sure what happened here, but this is an easy fix. Test plan: Try oai.pl with marcxml, marc21 and oai_dc again. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 66635 [details] [review] Bug 17807: Add format parameter to XSLT_Handler->transform Format may be chars (default), bytes or xmldoc. Note: xmldoc is a XML::LibXML document object. Since the default is chars, this does not affect current use. Note: The format parameter (xmldoc) will be used later in one of the OAI modules to prevent duplicated xml parsing. Test plan: Run t/db_dependent/XSLT_Handler.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 66636 [details] [review] Bug 17807: Use XSLT_Handler in Koha::OAI Replacing some code in the OAI modules by a call to an existing module. Note that the xmldoc format is used in the transform call to get a xml document object. The stylesheet method of Repository now only returns the name of the xsl file to be used instead of a cached xslt object. Similar functionality inside XSLT_Handler is used when calling transform. Note: We still lack unit tests in this area. I did not see the need for adding something for stylesheet since it only returns a simple string. The other change is made in Record::new; there are no tests for this module yet and the heart of the change here is actually tested already in XSLT_Handler.t. Note: I benchmarked calls to Repository in the old and the new situation and did not see significant changes. Test plan: [1] Run t/db_dependent/OAI/Server.t [2] Run oai.pl with ListRecords and marcxml. [3] Run oai.pl with ListRecords and oai_dc. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 66637 [details] [review] Bug 17807: [Follow-up] Use marcxml for marc21 prefix Not sure what happened here, but this is an easy fix. Test plan: Try oai.pl with marcxml, marc21 and oai_dc again. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to master for 17.11, thanks to everybody involved!
Enhancement not pushed to 17.05.x