Bug 14939 - OAI Server classes must be modularized
Summary: OAI Server classes must be modularized
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Web services (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Frédéric Demians
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 18794
  Show dependency treegraph
 
Reported: 2015-10-02 06:57 UTC by Frédéric Demians
Modified: 2018-06-04 20:09 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 14939 Modularize OAI Server existing classes (56.10 KB, patch)
2015-10-02 07:25 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 14939 Modularize OAI Server existing classes (56.13 KB, patch)
2015-10-02 15:18 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 14939 Modularize OAI Server existing classes (27.29 KB, patch)
2015-11-04 09:43 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 14939 Modularize OAI Server existing classes (59.20 KB, patch)
2015-11-04 14:03 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 14939 Modularize OAI Server existing classes (59.40 KB, patch)
2015-11-04 19:55 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 14939 Modularize OAI Server existing classes (60.31 KB, patch)
2015-11-14 19:06 UTC, Frédéric Demians
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 14939 Modularize OAI Server existing classes (60.43 KB, patch)
2015-11-16 14:28 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
Bug 14939 Modularize OAI Server existing classes (60.40 KB, patch)
2015-11-17 09:05 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 14939 Modularize OAI Server existing classes (60.49 KB, patch)
2015-11-17 16:23 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 14939 Modularize OAI Server existing classes (60.49 KB, patch)
2015-11-17 16:49 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 14939: Modularize OAI Server existing classes (60.52 KB, patch)
2015-11-17 16:51 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
Bug 14939 Modularize OAI Server existing classes (60.78 KB, patch)
2015-12-02 12:08 UTC, Frédéric Demians
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 14939: Modularize OAI Server existing classes (60.96 KB, patch)
2015-12-03 19:35 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
Bug 14939: Remove the Capture::Tiny dependency (2.23 KB, patch)
2015-12-10 12:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14939: Remove the Capture::Tiny dependency (2.33 KB, patch)
2015-12-10 14:48 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 14939: Modularize OAI Server existing classes (61.00 KB, patch)
2015-12-11 10:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14939: Remove the Capture::Tiny dependency (2.40 KB, patch)
2015-12-11 10:44 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Demians 2015-10-02 06:57:45 UTC
Koha OAI server has been done in one unique .pl file because there wasn't any
object model or rules in the Koha project when it has been coded. Its classes
must be modularized : putting each class in a separate file in a Koha::OAI
namespace.
Comment 1 Frédéric Demians 2015-10-02 07:25:01 UTC Comment hidden (obsolete)
Comment 2 Frédéric Demians 2015-10-02 15:18:26 UTC Comment hidden (obsolete)
Comment 3 Olli-Antti Kivilahti 2015-11-03 09:08:07 UTC
Failing QA because of insufficient testing.
Comment 4 Frédéric Demians 2015-11-03 09:59:12 UTC
(In reply to Olli-Antti Kivilahti from comment #3)
> Failing QA because of insufficient testing.

I disagree.

It's modularizing not refactoring. The existing classes are split from oai.pl into individual .pm files, one per class. It doesn't change a line of code. So coding a full UT for OAI server would be awesome, I can't disagree, but it isn't a requirement for this patch.
Comment 5 Frédéric Demians 2015-11-04 09:43:08 UTC Comment hidden (obsolete)
Comment 6 Frédéric Demians 2015-11-04 09:44:36 UTC
Redefining naming in order not to overlap with bug 10662.
Comment 7 Frédéric Demians 2015-11-04 14:03:40 UTC Comment hidden (obsolete)
Comment 8 Frédéric Demians 2015-11-04 19:55:49 UTC Comment hidden (obsolete)
Comment 9 Héctor Eduardo Castro Avalos 2015-11-14 17:22:41 UTC
Hi Frederick

Test Unit fail because module missing Capture::Tiny

I think you need to include in C4::Installer::PerlDependencies and debian/control (libcapture-tiny-perl)

Regards
Comment 10 Frédéric Demians 2015-11-14 19:05:07 UTC
> Test Unit fail because module missing Capture::Tiny

Thanks Hector. I add it!
Comment 11 Frédéric Demians 2015-11-14 19:06:12 UTC Comment hidden (obsolete)
Comment 12 Héctor Eduardo Castro Avalos 2015-11-15 00:26:21 UTC
Sorry Frédéric
But I got:

#   Failed test 'ListMetadataFormats'
#   at t/db_dependent/OAI/Server.t line 84.
#          got: '<?xml version="1.0" encoding="UTF-8"?>
# 
# <OAI-PMH xmlns='http://www.openarchives.org/OAI/2.0/' xsi:schemaLocation='http://www.openarchives.org/OAI/2.0/ http://www.openarchives.org/OAI/2.0/OAI-PMH.xsd' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'><responseDate>2015-11-15T00:11:11Z</responseDate><request>http://localhost</request><ListMetadataFormats><metadataFormat><metadataPrefix>oai_dc</metadataPrefix><schema>http://www.openarchives.org/OAI/2.0/oai_dc.xsd</schema><metadataNamespace>http://www.openarchives.org/OAI/2.0/oai_dc/</metadataNamespace></metadataFormat><metadataFormat><metadataPrefix>marcxml</metadataPrefix><schema>http://www.loc.gov/MARC21/slim http://www.loc.gov/ standards/marcxml/schema/MARC21slim.xsd</schema><metadataNamespace>http://www.loc.gov/MARC21/slim http://www.loc.gov/ standards/marcxml/schema/MARC21slim</metadataNamespace></metadataFormat></ListMetadataFormats></OAI-PMH>'
#     expected: '<?xml version="1.0" encoding="UTF-8"?>
# 
# <OAI-PMH xmlns='http://www.openarchives.org/OAI/2.0/' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' xsi:schemaLocation='http://www.openarchives.org/OAI/2.0/ http://www.openarchives.org/OAI/2.0/OAI-PMH.xsd'><responseDate>2015-11-15T00:11:11Z</responseDate><request>http://localhost</request><ListMetadataFormats><metadataFormat><metadataPrefix>oai_dc</metadataPrefix><schema>http://www.openarchives.org/OAI/2.0/oai_dc.xsd</schema><metadataNamespace>http://www.openarchives.org/OAI/2.0/oai_dc/</metadataNamespace></metadataFormat><metadataFormat><metadataPrefix>marcxml</metadataPrefix><schema>http://www.loc.gov/MARC21/slim http://www.loc.gov/ standards/marcxml/schema/MARC21slim.xsd</schema><metadataNamespace>http://www.loc.gov/MARC21/slim http://www.loc.gov/ standards/marcxml/schema/MARC21slim</metadataNamespace></metadataFormat></ListMetadataFormats></OAI-PMH>'

#   Failed test 'ListIdentifiers without metadaPrefix argument'
#   at t/db_dependent/OAI/Server.t line 96.
#          got: '<?xml version="1.0" encoding="UTF-8"?>
# 
# <OAI-PMH xsi:schemaLocation='http://www.openarchives.org/OAI/2.0/ http://www.openarchives.org/OAI/2.0/OAI-PMH.xsd' xmlns='http://www.openarchives.org/OAI/2.0/' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'><responseDate>2015-11-15T00:11:11Z</responseDate><request>http://localhost</request><error code='badArgument'>Required argument &apos;metadataPrefix&apos; was undefined</error></OAI-PMH>'
#     expected: '<?xml version="1.0" encoding="UTF-8"?>
# 
# <OAI-PMH xmlns='http://www.openarchives.org/OAI/2.0/' xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance' xsi:schemaLocation='http://www.openarchives.org/OAI/2.0/ http://www.openarchives.org/OAI/2.0/OAI-PMH.xsd'><responseDate>2015-11-15T00:11:11Z</responseDate><request>http://localhost</request><error code='badArgument'>Required argument &apos;metadataPrefix&apos; was undefined</error></OAI-PMH>'
# Looks like you failed 2 tests of 13.

Also i noticed that this happen when you got  OAI-PMH:AutoUpdateSets to "Disable", if you enable all works succefully.

I play with cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:1&metadataPrefix=marcxml

And is working succefully
Comment 13 Héctor Eduardo Castro Avalos 2015-11-15 00:36:35 UTC
Another thing that i could see is: in debian/control when a library is placed, goes in two places and you put it in one. I don't know much about that, tell me if I'm wrong
Comment 14 Frédéric Demians 2015-11-15 07:38:43 UTC
Thanks Hector for testing. Two responses: (1) UT don't work for you because you may use a MARC21 DB. I've tested with Unimarc. I have to change the UT to be marcflavour agnostic. (2) Debian packaging: I will remove my debian/control modification, and let the packaging manager (Robin Sheat) add the correct info.
Comment 15 Frédéric Demians 2015-11-15 07:56:02 UTC
Failing tests seem to be due to HTTP::OAI version variation. Which version do you use? You can get it with:

perl -MHTTP::OAI -e 'print $HTTP::OAI::VERSION, "\n"'
Comment 16 Héctor Eduardo Castro Avalos 2015-11-16 14:26:23 UTC
I have 3.27

I don't know why fail me in my laptop with Debian Jessie. In my work does not fail with Dian wheezy, both Distros have the same version (<https://packages.debian.org/jessie/libhttp-oai-perl>)

Maybe I did something wrong.

Well I will sign-off.
Comment 17 Héctor Eduardo Castro Avalos 2015-11-16 14:28:37 UTC Comment hidden (obsolete)
Comment 18 Frédéric Demians 2015-11-17 09:05:00 UTC Comment hidden (obsolete)
Comment 19 Frédéric Demians 2015-11-17 09:07:35 UTC
(In reply to Héctor Eduardo Castro Avalos from comment #16)

> I don't know why fail me in my laptop with Debian Jessie. In my work does
> not fail with Dian wheezy, both Distros have the same version
> (<https://packages.debian.org/jessie/libhttp-oai-perl>)

I've reproduced your UT error on a Jessie box. I've fixed it, and reattached your signed path, with the corrected UT. Please confirm it also works for you on Jessie.
Comment 20 Frédéric Demians 2015-11-17 16:23:41 UTC Comment hidden (obsolete)
Comment 21 Héctor Eduardo Castro Avalos 2015-11-17 16:49:21 UTC Comment hidden (obsolete)
Comment 22 Héctor Eduardo Castro Avalos 2015-11-17 16:51:18 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2015-12-02 11:20:19 UTC
Comment on attachment 44915 [details] [review]
[SIGNED-OFF]Bug 14939: Modularize OAI Server existing classes

Review of attachment 44915 [details] [review]:
-----------------------------------------------------------------

::: C4/Installer/PerlDependencies.pm
@@ +777,4 @@
>          'required' => '0',
>          'min_ver'  => '0.28',
>      },
> +    'Capture::Tiny' => {

Is this really mandatory?
The module has some known bugs (https://github.com/dagolden/Capture-Tiny/issues).
One is "Capture::Tiny breaks under FCGI" and is not comforting.

::: opac/oai.pl
@@ +1,3 @@
>  #!/usr/bin/perl
>  
> +# Copyright Tamil s.a.r.l. 2008-2015

Why did you remove BibLibre's copyright?
Comment 24 Jonathan Druart 2015-12-02 11:23:09 UTC
 FAIL   Koha/OAI/Server/Identify.pm
   FAIL   critic
                Package declaration must match filename at line 18, column 1. Correct the filename or package statement.
 FAIL   Koha/OAI/Server/Repository.pm
   FAIL   critic
                Code before strictures are enabled at line 20, column 11. See page 429 of PBP.
   FAIL   spelling
                 tranformed  ==> transformed
                 choosed  ==> chose, chosen
   FAIL   valid
                Can't locate package HTTP::OAI::Encapsulation for @HTTP::OAI::Error::ISA 

 FAIL   opac/oai.pl
   FAIL   valid
                Can't locate package HTTP::OAI::Encapsulation for @HTTP::OAI::Error::ISA 


Should I install libhttp-oai-perl?
Comment 25 Jonathan Druart 2015-12-02 11:23:53 UTC
After installing libhttp-oai-perl:

 FAIL   opac/oai.pl
   FAIL   valid
                Can't locate package HTTP::OAI::SAX::Base for @HTTP::OAI::Error::ISA 
                Can't locate package HTTP::OAI::MemberMixin for @HTTP::OAI::Error::ISA
Comment 26 Frédéric Demians 2015-12-02 11:50:06 UTC
(In reply to Jonathan Druart from comment #23)
> Comment on attachment 44915 [details] [review] [review]
> [SIGNED-OFF]Bug 14939: Modularize OAI Server existing classes
> 
> Review of attachment 44915 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: C4/Installer/PerlDependencies.pm
> @@ +777,4 @@
> >          'required' => '0',
> >          'min_ver'  => '0.28',
> >      },
> > +    'Capture::Tiny' => {
> 
> Is this really mandatory?

Is is used in tests to capture STDOUT. Do you have something to suggest to
do the same thing without this module?

> ::: opac/oai.pl
> @@ +1,3 @@
> >  #!/usr/bin/perl
> >  
> > +# Copyright Tamil s.a.r.l. 2008-2015
> 
> Why did you remove BibLibre's copyright?

Not intentional, even if Tamil copyright on this code deserves to come back. I
wrote OAI server back in time, and Tamil copyright disappeared without being
noticed by sharp eyes as yours.
Comment 27 Frédéric Demians 2015-12-02 12:08:22 UTC Comment hidden (obsolete)
Comment 28 Frédéric Demians 2015-12-02 12:11:43 UTC
Jonathan, I've rebased the patch on master, and fixed perlcritic test errors, and copyright stuff. I'm open to any proposal to replace Tiny::Capture.
Comment 29 Jonathan Druart 2015-12-02 12:37:26 UTC
The tests does not pass:
t/db_dependent/OAI/Server.t .. 1/13 Can't call method "start_element" on an undefined value at /usr/share/perl5/HTTP/OAI/Verb.pm line 53.

(In reply to Frédéric Demians from comment #26)

> > > +# Copyright Tamil s.a.r.l. 2008-2015
> > 
> > Why did you remove BibLibre's copyright?
> 
> Not intentional, even if Tamil copyright on this code deserves to come back.
> I
> wrote OAI server back in time, and Tamil copyright disappeared without being
> noticed by sharp eyes as yours.

The idea wasn't to remove the Tamil copyright line, but reintroduce BibLibre one's :)
Comment 30 Frédéric Demians 2015-12-02 12:59:12 UTC
> The tests does not pass:
> t/db_dependent/OAI/Server.t .. 1/13 Can't call method "start_element" on an
> undefined value at /usr/share/perl5/HTTP/OAI/Verb.pm line 53.

I don't get this? What is you test environment?
Comment 31 Héctor Eduardo Castro Avalos 2015-12-02 15:22:38 UTC
(In reply to Jonathan Druart from comment #29)
> The tests does not pass:
> t/db_dependent/OAI/Server.t .. 1/13 Can't call method "start_element" on an
> undefined value at /usr/share/perl5/HTTP/OAI/Verb.pm line 53.

In Debian this doesn't fail

prove -v t/db_dependent/OAI/Server.t
t/db_dependent/OAI/Server.t .. 
1..13
ok 1 - use Koha::OAI::Server::DeletedRecord;
ok 2 - use Koha::OAI::Server::Description;
ok 3 - use Koha::OAI::Server::GetRecord;
ok 4 - use Koha::OAI::Server::Identify;
ok 5 - use Koha::OAI::Server::ListIdentifiers;
ok 6 - use Koha::OAI::Server::ListMetadataFormats;
ok 7 - use Koha::OAI::Server::ListRecords;
ok 8 - use Koha::OAI::Server::ListSets;
ok 9 - use Koha::OAI::Server::Record;
ok 10 - use Koha::OAI::Server::Repository;
ok 11 - use Koha::OAI::Server::ResumptionToken;
ok 12 - ListMetadataFormats
ok 13 - ListIdentifiers without metadaPrefix argument
ok
All tests successful.
Files=1, Tests=13,  8 wallclock secs ( 0.03 usr  0.01 sys +  1.06 cusr  0.08 csys =  1.18 CPU)
Result: PASS
Comment 32 Frédéric Demians 2015-12-02 15:44:00 UTC
(In reply to Jonathan Druart from comment #29)
> The tests does not pass:
> t/db_dependent/OAI/Server.t .. 1/13 Can't call method "start_element" on an
> undefined value at /usr/share/perl5/HTTP/OAI/Verb.pm line 53.

See: 

https://rt.cpan.org/Public/Bug/Display.html?id=102010
Comment 33 David Cook 2015-12-02 23:00:10 UTC
(In reply to Frédéric Demians from comment #32)
> (In reply to Jonathan Druart from comment #29)
> > The tests does not pass:
> > t/db_dependent/OAI/Server.t .. 1/13 Can't call method "start_element" on an
> > undefined value at /usr/share/perl5/HTTP/OAI/Verb.pm line 53.
> 
> See: 
> 
> https://rt.cpan.org/Public/Bug/Display.html?id=102010

Are you sure about that? As Héctor pointed out, it looks like the version in Wheezy and Jessie is the same:

https://packages.debian.org/wheezy/libhttp-oai-perl
https://packages.debian.org/jessie/libhttp-oai-perl

As I pointed out on https://rt.cpan.org/Public/Bug/Display.html?id=102010, someone had force installed 4.03 on the openSUSE machine I was on, and it failed miserably. It worked fine when I downgraded back to 3.28, but Debian should be using 3.27-1 for both Wheezy and Jessie which should be fine...

It does appear that someone has recently done some work to add 4.03 into Debian: https://anonscm.debian.org/cgit/pkg-perl/packages/libhttp-oai-perl.git/

Both Debian Stretch (ie testing) and Debian Sid (ie unstable) are using 4.03-1 which at a glance looks like it doesn't include any fix for that CPAN ticket, so all use of HTTP::OAI would break...:

https://packages.debian.org/stretch/libhttp-oai-perl
https://packages.debian.org/sid/libhttp-oai-perl

--

Héctor: What version of Debian are you using?
Jonathan: What version of Debian are you using?
Frédéric: What OS/version are you using?
Comment 34 David Cook 2015-12-02 23:08:20 UTC
Frédéric: I noticed on https://rt.cpan.org/Public/Bug/Display.html?id=102010 that you asked if there was anything you could do to fix the problem.

I'm guessing that HTTP::OAI itself isn't maintained so well anymore on CPAN, but you could report a bug in Debian perhaps: https://www.debian.org/Bugs/

Robin reported a bug in the idzebra-2.0 package earlier in the year but didn't have much success: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=777515. Although I think that's because the package maintainer didn't see it or isn't active anymore. 

In the case of HTTP::OAI, it seems that the maintainer gregor herrmann is fairly active: https://anonscm.debian.org/cgit/pkg-perl/packages/libhttp-oai-perl.git/, so it might be worth contacting him as well, even if the problems are just in Debian Testing and Debian Unstable...

It would be great if someone could confirm for sure that there are problems in Debian Jessie, although I think Robin has been using Jessie for a while and he was doing quite a bit with HTTP::OAI this year so I would've thought he'd notice an issue during that time...
Comment 35 Héctor Eduardo Castro Avalos 2015-12-03 02:14:41 UTC
(In reply to David Cook from comment #33)

> Héctor: What version of Debian are you using?
> Jonathan: What version of Debian are you using?
> Frédéric: What OS/version are you using?

Hi David

I applied the patch in both Jessie and Wheezy and test passed successfully, maybe fail with Ubuntu.

I Enabled OAI-PMH and play with:
cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:1&metadataPrefix=marcxml

cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:1&metadataPrefix=oai_dc

And works well.
Comment 36 David Cook 2015-12-03 02:24:26 UTC
(In reply to Héctor Eduardo Castro Avalos from comment #35)
> (In reply to David Cook from comment #33)
> 
> > Héctor: What version of Debian are you using?
> > Jonathan: What version of Debian are you using?
> > Frédéric: What OS/version are you using?
> 
> Hi David
> 
> I applied the patch in both Jessie and Wheezy and test passed successfully,
> maybe fail with Ubuntu.
> 

It looks like Ubuntu 15.10 Wily has the 4.03-1 version, but 14.04 has 3.27-1.
http://packages.ubuntu.com/wily/libhttp-oai-perl
Comment 37 Frédéric Demians 2015-12-03 06:20:25 UTC
I confirm that Jessie/Wheezy package 3.2x version. Jonathan who reported the issue should use a CPAN installed version of HTTP::OAI.

I've also have reported the bug on Github:

https://github.com/timbrody/perl-oai-lib

But no response yet. We may give a phone call to Tim Brody...
Comment 38 Jonathan Druart 2015-12-03 09:06:34 UTC
(In reply to Frédéric Demians from comment #37)
> I confirm that Jessie/Wheezy package 3.2x version. Jonathan who reported the
> issue should use a CPAN installed version of HTTP::OAI.
> 
> I've also have reported the bug on Github:
> 
> https://github.com/timbrody/perl-oai-lib
> 
> But no response yet. We may give a phone call to Tim Brody...

So sorry, I was using testing!
Indeed it's 3.27 on my Jessie box.
Comment 39 Frédéric Demians 2015-12-03 09:44:50 UTC
(In reply to Jonathan Druart from comment #38)

> So sorry, I was using testing!
> Indeed it's 3.27 on my Jessie box.

And does it work? Are tests passing? If so, Héctor could re-sign-off the attached patch, which is supposed to fix test failings you pointed out.

It remains that HTTP::OAI v4 is a time bomb...
Comment 40 Jonathan Druart 2015-12-03 16:21:13 UTC
(In reply to Frédéric Demians from comment #39)
> (In reply to Jonathan Druart from comment #38)
> 
> > So sorry, I was using testing!
> > Indeed it's 3.27 on my Jessie box.
> 
> And does it work? Are tests passing? If so, Héctor could re-sign-off the
> attached patch, which is supposed to fix test failings you pointed out.

Yes, all tests pass.
Comment 41 Héctor Eduardo Castro Avalos 2015-12-03 19:35:00 UTC Comment hidden (obsolete)
Comment 42 David Cook 2015-12-03 22:49:00 UTC
(In reply to Frédéric Demians from comment #39)
> It remains that HTTP::OAI v4 is a time bomb...

Indeed it does. I would hope that it wouldn't get into the next stable release of Debian... that they'd notice it fails in Debian Testing and thus continue with the existing stable version, but you never know...

Ubuntu 15.10 Wily was released in October, and it has 4.x, so there is a popular Debian-based OS there with a bad HTTP::OAI package it seems http://packages.ubuntu.com/wily/libhttp-oai-perl. I haven't tested it though as I don't have a Ubuntu 15.10 install at hand...

(Interesting that Robin Sheat is listed as a maintainer... also listed as a maintainer at https://packages.debian.org/source/jessie/libhttp-oai-perl although he's no longer involved with Koha...)
Comment 43 David Cook 2015-12-03 22:51:09 UTC
(In reply to Frédéric Demians from comment #37)
> But no response yet. We may give a phone call to Tim Brody...

That would be great. I'd love to know if he's still maintaining the project, or if he's open to someone taking it over.

Not that I'm necessarily volunteering but it would be good to know.
Comment 44 Jonathan Druart 2015-12-10 12:57:59 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2015-12-10 12:58:55 UTC
(In reply to Frédéric Demians from comment #28)
> I'm open to any proposal to replace Tiny::Capture.

What about this last patch?
Comment 46 Frédéric Demians 2015-12-10 14:48:43 UTC Comment hidden (obsolete)
Comment 47 Frédéric Demians 2015-12-10 14:49:25 UTC
Thanks for the solution.
Comment 48 Jonathan Druart 2015-12-11 10:44:06 UTC
Created attachment 45602 [details] [review]
Bug 14939: Modularize OAI Server existing classes

Koha OAI server has been done in one unique .pl file because there
wasn't any object model or rules in the Koha project when it has been
coded. This patch modularized existing classes, putting each class in a
separate file in Koha::OAI::Server namespace. UT begining.

Add new dependency: Capture::Tiny

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
OAI server moduralized succefully. Works for Debian Jessie and
Wheezy. Test pass successfully

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 49 Jonathan Druart 2015-12-11 10:44:11 UTC
Created attachment 45603 [details] [review]
Bug 14939: Remove the Capture::Tiny dependency

It's possible to redirect the std output to a variable, it will avoid to
add this new dependency.

Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
  OK. + code factorisation with a function call.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 50 Jonathan Druart 2015-12-11 10:44:31 UTC
Nice job Frédéric!
Comment 51 Kyle M Hall 2015-12-31 15:21:50 UTC
Pushed to master, thanks Frédéric!
Comment 52 Jonathan Druart 2016-08-25 13:36:03 UTC
Comment on attachment 45602 [details] [review]
Bug 14939: Modularize OAI Server existing classes

Review of attachment 45602 [details] [review]:
-----------------------------------------------------------------

::: Koha/OAI/Server/ListRecords.pm
@@ +42,5 @@
> +        $set = GetOAISetBySpec($token->{'set'});
> +    }
> +    my $max = $repository->{koha_max_count};
> +    my $sql = "
> +        (SELECT biblioitems.biblionumber, biblioitems.timestamp, marcxml

We have this...

@@ +49,5 @@
> +    $sql .= " JOIN oai_sets_biblios ON biblioitems.biblionumber = oai_sets_biblios.biblionumber " if defined $set;
> +    $sql .= " WHERE timestamp >= ? AND timestamp <= ? ";
> +    $sql .= " AND oai_sets_biblios.set_id = ? " if defined $set;
> +    $sql .= ") UNION
> +        (SELECT deletedbiblio.biblionumber, null as marcxml, timestamp FROM deletedbiblio";

then this (marcxml and timestamp inverted)

@@ +67,5 @@
> +    $sth->execute( @bind_params );
> +
> +    my $count = 0;
> +    my $format = $args{metadataPrefix} || $token->{metadata_prefix};
> +    while ( my ($biblionumber, $timestamp) = $sth->fetchrow ) {

And finally this.

Should not we remove marcxml (as it's unused)?
Comment 53 Frédéric Demians 2016-08-26 07:32:34 UTC
(In reply to Jonathan Druart from comment #52)

> Should not we remove marcxml (as it's unused)?

Maybe. Could you take a look at bug 15108? It improves greatly SQL queries generated by Koha OAI Server, and so performances. You could QA it :-)