Fetching the MARCXML via the OAI-PMH interface doesn't include item data, however for harvesting purposes, it should do.
Created attachment 28228 [details] [review] Bug 12252 - OAI-PMH result includes item data GetRecord for OAI-PMH was pulling the MARCXML directly from the database. Now it uses GetMarcBiblio and includes the item data with it, making it more generally useful. Test plan: * Run an OAI-PMH query, for example: http://koha/cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:52&metadataPrefix=marcxml to fetch biblionumber 52 * Note that it doesn't include the 952 data * Apply the patch * Do the same thing, but this time see that the 952 data is at the bottom of the MARCXML. Note: * This patch also includes a small tidy-up in C4::Biblios to group things semantically a bit better, so I don't spend ages looking for a function that was staring me in the face all along again.
Created attachment 28229 [details] [review] Bug 12252 - OAI-PMH result includes item data GetRecord for OAI-PMH was pulling the MARCXML directly from the database. Now it uses GetMarcBiblio and includes the item data with it, making it more generally useful. Test plan: * Run an OAI-PMH query, for example: http://koha/cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:52&metadataPrefix=marcxml to fetch biblionumber 52 * Note that it doesn't include the 952 data * Apply the patch * Do the same thing, but this time see that the 952 data is at the bottom of the MARCXML. Note: * This patch also includes a small tidy-up in C4::Biblios to group things semantically a bit better, so I don't spend ages looking for a function that was staring me in the face all along again. Signed-off-by: David Cook <dcook@prosentient.com.au> Works as described. Simple yet useful patch.
Created attachment 28243 [details] [review] Bug 12252 - OAI-PMH result includes item data GetRecord for OAI-PMH was pulling the MARCXML directly from the database. Now it uses GetMarcBiblio and includes the item data with it, making it more generally useful. Test plan: * Run an OAI-PMH query, for example: http://koha/cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:52&metadataPrefix=marcxml to fetch biblionumber 52 * Note that it doesn't include the 952 data * Apply the patch * Do the same thing, but this time see that the 952 data is at the bottom of the MARCXML. Note: * This patch also includes a small tidy-up in C4::Biblios to group things semantically a bit better, so I don't spend ages looking for a function that was staring me in the face all along again. Signed-off-by: David Cook <dcook@prosentient.com.au> Works as described. Simple yet useful patch. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
It looks like a bug, isn't it? I changed from enh to normal.
I wasn't sure, but I don't disagree :-)
Setting to in discussion -- arguably, it would be a bug that the proposed change does not respect the OpacHiddenItems syspref.
Also, changing back to an enhancement, as it's not clear to me that folks wanting to run Koha as an OAI-PMH provider would *always* want to include item data.
(In reply to Galen Charlton from comment #6) > Setting to in discussion -- arguably, it would be a bug that the proposed > change does not respect the OpacHiddenItems syspref. It's probable that supporting it wouldn't be too hard. I think support would have to be added at the GetMarcBiblio level. (In reply to Galen Charlton from comment #7) > Also, changing back to an enhancement, as it's not clear to me that folks > wanting to run Koha as an OAI-PMH provider would *always* want to include > item data. On the flip side, I'm going to argue that not having item data was a regression caused by the removal of item data from the stored MARC record, because it would have worked then. This means it's indeed a bug! :)
(In reply to Galen Charlton from comment #6) > Setting to in discussion -- arguably, it would be a bug that the proposed > change does not respect the OpacHiddenItems syspref. I think that this one can be tricky. On one hand, I would want the OAI-PMH server to honor OpacHiddenItems and the MARC frameworks, so that we weren't sending records with private/hidden information (whether stored at the item-level or bib-level). On the other hand, I wonder if there are times where you do want to serve complete (i.e. staff side) records. I suppose we could assume the former case of treating the OAI-PMH server as "public", and if there arose a need for a "private" stream, we could add authentication. I've seen a few OAI-PMH servers out there that require username and password to access them.
Created attachment 28344 [details] [review] Bug 12252 - OAI-PMH items respect OpacHiddenItems This allows the OAI-PMH service to not provide item information when there is a rule that would supress it in OpacHiddenItems. Test plan: * Find an OAI-PMH URL that shows you some items. * Add an entry to OpacHiddenItems that would block that. * Check that it's blocked.
Created attachment 28345 [details] [review] Bug 12252 - OAI-PMH items respect OpacHiddenItems This allows the OAI-PMH service to not provide item information when there is a rule that would supress it in OpacHiddenItems. Test plan: * Find an OAI-PMH URL that shows you some items. * Add an entry to OpacHiddenItems that would block that. * Check that it's blocked.
I suppose I'm a bit hesitant to sign off on this one at the moment because of Mark's bug 11592. That said, I'm not going to sign off on Mark's patch either, as I'm not sold on the code there...
Assuming Mark's code is backwards compatible, it should work anyway.
Created attachment 29159 [details] [review] Bug 12252 - include item data in OAI-PMH ListRecords result Same in spirit to the other patch, this also includes the item detail in ListRecords. Test plan: * Fetch a URL like: http://koha/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml * Verify that there are 952 entries in the returned records where appropriate.
Created attachment 29160 [details] [review] Bug 12252 - include item data in OAI-PMH ListRecords result Same in spirit to the other patch, this also includes the item detail in ListRecords. Test plan: * Fetch a URL like: http://koha/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml * Verify that there are 952 entries in the returned records where appropriate.
...I forgot the OpacHiddenItems thing again...
Created attachment 33120 [details] [review] Bug 12252 - OAI-PMH result includes item data GetRecord for OAI-PMH was pulling the MARCXML directly from the database. Now it uses GetMarcBiblio and includes the item data with it, making it more generally useful. Test plan: * Run an OAI-PMH query, for example: http://koha/cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:52&metadataPrefix=marcxml to fetch biblionumber 52 * Note that it doesn't include the 952 data * Apply the patch * Do the same thing, but this time see that the 952 data is at the bottom of the MARCXML. Note: * This patch also includes a small tidy-up in C4::Biblios to group things semantically a bit better, so I don't spend ages looking for a function that was staring me in the face all along again. Signed-off-by: David Cook <dcook@prosentient.com.au> Works as described. Simple yet useful patch. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Frederic Demians <f.demians@tamil.fr> 952/995 item fields are back in response to GetRecord OAI verb.
Created attachment 33121 [details] [review] Bug 12252 - OAI-PMH items respect OpacHiddenItems This allows the OAI-PMH service to not provide item information when there is a rule that would supress it in OpacHiddenItems. Test plan: * Find an OAI-PMH URL that shows you some items. * Add an entry to OpacHiddenItems that would block that. * Check that it's blocked. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Tested, playing with OpacHiddenItems. GetRecord OAI verb returns a record complying with OpacHiddenItems rules, for example without items from a specific library.
Created attachment 33122 [details] [review] Bug 12252 - include item data in OAI-PMH ListRecords result Same in spirit to the other patch, this also includes the item detail in ListRecords. Test plan: * Fetch a URL like: http://koha/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml * Verify that there are 952 entries in the returned records where appropriate. Signed-off-by: Frederic Demians <f.demians@tamil.fr> ListRecords OAI verb returns a list of records including items in 952/995 which are not hidden based on OpacHiddenItems syspref.
*** Bug 13475 has been marked as a duplicate of this bug. ***
Comment on attachment 33121 [details] [review] Bug 12252 - OAI-PMH items respect OpacHiddenItems Review of attachment 33121 [details] [review]: ----------------------------------------------------------------- ::: C4/Biblio.pm @@ +1263,5 @@ > +=item C<$embeditems> > + > +set to true to include item information. > + > +=item C<$opac> This new parameter will be used in all opac scripts where we want to embed items. @@ +2928,5 @@ > my @item_fields; > my ( $itemtag, $itemsubfield ) = GetMarcFromKohaField( "items.itemnumber", $frameworkcode ); > + my @items; > + my $opachiddenitems = $opac > + && ( C4::Context->preference('OpacHiddenItems') !~ /^\s*$/ ); It would have been better to use /\S/ to match the existing test. ::: opac/oai.pl @@ +280,4 @@ > > # We fetch it using this method, rather than the database directly, > # so it'll include the item data > + my $record = GetMarcBiblio($biblionumber, 1, 1); We really want to avoid this kind of calls, it's not readable.
I would like to get other QA point of view here. The changes in GetMarcBiblio does not provided any tests. But none of GetMarcBiblio and EmbedItemsInMarcBiblio have tests! It would be very good nice to see these 2 subroutines tested, but should we block this patch set for this reason?
(In reply to Jonathan Druart from comment #22) > I would like to get other QA point of view here. > > The changes in GetMarcBiblio does not provided any tests. But none of > GetMarcBiblio and EmbedItemsInMarcBiblio have tests! > It would be very good nice to see these 2 subroutines tested, but should we > block this patch set for this reason? From a glance, it looks like adding some tests for at least EmbedItems would not be a bad thing here.
Created attachment 34890 [details] [review] Bug 12252: Add tests for EmbedItemsInMarcBiblio Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 35006 [details] [review] Bug 12252 - OAI-PMH result includes item data GetRecord for OAI-PMH was pulling the MARCXML directly from the database. Now it uses GetMarcBiblio and includes the item data with it, making it more generally useful. Test plan: * Run an OAI-PMH query, for example: http://koha/cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:52&metadataPrefix=marcxml to fetch biblionumber 52 * Note that it doesn't include the 952 data * Apply the patch * Do the same thing, but this time see that the 952 data is at the bottom of the MARCXML. Note: * This patch also includes a small tidy-up in C4::Biblios to group things semantically a bit better, so I don't spend ages looking for a function that was staring me in the face all along again. Signed-off-by: David Cook <dcook@prosentient.com.au> Works as described. Simple yet useful patch. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Frederic Demians <f.demians@tamil.fr> 952/995 item fields are back in response to GetRecord OAI verb. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 35007 [details] [review] Bug 12252 - OAI-PMH items respect OpacHiddenItems This allows the OAI-PMH service to not provide item information when there is a rule that would supress it in OpacHiddenItems. Test plan: * Find an OAI-PMH URL that shows you some items. * Add an entry to OpacHiddenItems that would block that. * Check that it's blocked. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Tested, playing with OpacHiddenItems. GetRecord OAI verb returns a record complying with OpacHiddenItems rules, for example without items from a specific library. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 35008 [details] [review] Bug 12252 - include item data in OAI-PMH ListRecords result Same in spirit to the other patch, this also includes the item detail in ListRecords. Test plan: * Fetch a URL like: http://koha/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml * Verify that there are 952 entries in the returned records where appropriate. Signed-off-by: Frederic Demians <f.demians@tamil.fr> ListRecords OAI verb returns a list of records including items in 952/995 which are not hidden based on OpacHiddenItems syspref. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 35009 [details] [review] Bug 12252: Add tests for EmbedItemsInMarcBiblio Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 35010 [details] [review] Bug 12252: (qa-follow-up) Remove perlcritic error
Created attachment 35011 [details] [review] Bug 12252: (qa-follow-up) Remove perlcritic error
Didn't test yet, but wondering: how are we dealing with the subfields marked as 'don't show in OPAC'? Thinking of the internal notes especially.
I might be arriving late to the party but, if i am not mistaken, OAI in Koha can work in extended mode, with a custom xslt. I think that would be the right way of filtering things out in the exported data. Otherwise we might have situations where we need to hide certain fields on the opac, but want to have them in the exported data, which wouldn't be possible then.
Hm, not sure I agree. I think internal note holds the promise that noone outside the library can see it and afaik the extended mode is not documented in a way that it would be easy to add those constraints for a library.
Maybe better documentation and a switch to include/not include? The export feature in the staff client allows to list fields to 'not export'.
That's more complicated than i first thought indeed. My understanding at first was that the library would want to filter out some fields to import in the OAI harvester, but Jonathan pointed out that anyone can query the koha OAI provider and that the real issue is data that would be considered confidential and that should never be exported. I would say we have two separate issues though: - Having items data in the OAI export. If we don't have it, this is a problem for harvesting purposes, which even defeats the point of being an OAI provider in some situations. - Some confidential data can be exposed through this. But Jonathan and i just tried it, and it's not just the OAI interface, if you save the record as marc or marcxml from the opac, this data will be in there. So we don't respect this confidentiality here either. I would say this ticket should remain about adding item data, and we should create another bug report, that would tackle the issue of exporting sensitive data everywhere this could happen. what do you think?
I am not sure if the current problem is something that 'used to work'. I filed a bug recently about the plain MARC view in OPAC where I was fairly sure that the fields would not be included - but they show. So this is indeed a wider issue, just not sure if it's new or things we broke with some other changes over time :( If we just quietly add the items and expose the data, this seems problematic to me. Could it be a switch with a warning for now?
OPAC visibility turns out to be quite a mess. Not blocking this, but still wondering if we can document this somewhere. Maybe add a note to the pref that it will include all item data?
Adding a note to the OAI-PMH preference so that it reads like the following would be ok for me: able/disable Koha's OAI-PMH server. The data exposed will not include items hidden through the OpacHiddenItems system preference, but everything else will be available for harvest. If you need to hide some data, look into extended mode and the OAI-PMH:ConfFile system preference. (It does look to me as enabling extended mode with a configuration file allowing only one type of metadata that would result from a custom xslt removing the confidential fields would be enough.)
(In reply to Gaetan Boisson from comment #35) > I would say this ticket should remain about adding item data, and we should > create another bug report, that would tackle the issue of exporting > sensitive data everywhere this could happen. what do you think? Yes, this! :) I kinda think that internal notes shouldn't ever go into the MARC anyway, it should perhaps be a separate table. Otherwise the chance of leak is quite high. But this is definitely a separate issue.
I like Gaetan's suggestion - we discussed this some more on IRC earlier today, the 'visibility' problem is indeed too big for this bug. But apart from how it should be... We have an 'internal note' in the item and the configuration options to hide in OPAC ... but we are not doing it right.
(In reply to Robin Sheat from comment #39) > (In reply to Gaetan Boisson from comment #35) > > I would say this ticket should remain about adding item data, and we should > > create another bug report, that would tackle the issue of exporting > > sensitive data everywhere this could happen. what do you think? > > Yes, this! :) > > I kinda think that internal notes shouldn't ever go into the MARC anyway, it > should perhaps be a separate table. Otherwise the chance of leak is quite > high. But this is definitely a separate issue. +1 Gaetan: There's been a bit of work done on the opac detail page (see http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11592), but I'd definitely like to see a more consistent approach that respects the "visibility settings" everywhere in Koha (i.e. tools export, OAI, cart, search results, detail page, etc.) I think Tomas had an idea about this... about using the frameworks to generate an "exclusion" XSLT which MARCXML would be run through after retrieving a record from Zebra or MySQL. I suppose the only time that you might want to not use the "exclusion" XSLT would be for... well perhaps a few scenarios. Most places in the staff client would probably want to show it, and only hide it if you're sending the record elsewhere... (unless you're exporting it to a cataloguing tool where you want to re-import afteward). Hmm, food for thought!
(In reply to Katrin Fischer from comment #40) > I like Gaetan's suggestion - we discussed this some more on IRC earlier > today, the 'visibility' problem is indeed too big for this bug. > > But apart from how it should be... We have an 'internal note' in the item > and the configuration options to hide in OPAC ... but we are not doing it > right. Agreed. Definitely too big for this bug... but perhaps the "visibility" problem blocks this bug? Yeah, an "internal note" in the item and ineffectual configuration options to hide in the OPAC :/.
I definitely think this might not be a feature for some institutions and we should make it opt-in. Because as Galen mentioned, it is not obvious that everyone will want to include items data.
(In reply to Tomás Cohen Arazi from comment #43) > I definitely think this might not be a feature for some institutions and we > should make it opt-in. Because as Galen mentioned, it is not obvious that > everyone will want to include items data. Well, it is there for harvesting. The reason I added it is because the harvesting required the item data so it could be presented.
(In reply to Tomás Cohen Arazi from comment #43) > I definitely think this might not be a feature for some institutions and we > should make it opt-in. Because as Galen mentioned, it is not obvious that > everyone will want to include items data. I am pretty sure it's a bug and a regression introduced by bug 5579.
(In reply to Gaetan Boisson from comment #38) > Adding a note to the OAI-PMH preference so that it reads like the following > would be ok for me: > > able/disable Koha's OAI-PMH server. The data exposed will not include items > hidden through the OpacHiddenItems system preference, but everything else > will be available for harvest. If you need to hide some data, look into > extended mode and the OAI-PMH:ConfFile system preference. > > (It does look to me as enabling extended mode with a configuration file > allowing only one type of metadata that would result from a custom xslt > removing the confidential fields would be enough.) I liked this idea from Gaetan. But thinking about it more I am still torn: This is a change in behaviour. People might have built things assuming the item data is not there and might suddenly expose more data, than they wanted to. I know that Jonathan is probably right in that this was initially a regression from bug 5579, but we missed to fix this for a really long time now :( Would it be really bad to make the items a configuration option? I know some people hate new prefs... but the Web Services tab still has plenty of room...
> This is a change in behaviour. People might have built things assuming the > item data is not there and might suddenly expose more data, than they wanted > to. I agree but they will still be exposing this data, even if they switch it off for OAI, through the marc view, the marc downloads, etc. It's true i think we have enough sysprefs, but especially here, i feel we will add one to do something that can already be achieved using extended mode, and that it will only very partially fix an issue we have in a lot of other places.
Maybe we should add an option to control this on the YAML configuration for OAI sets, so it is set-specific.
(In reply to Tomás Cohen Arazi from comment #48) > Maybe we should add an option to control this on the YAML configuration for > OAI sets, so it is set-specific. Something like: format: with_items: metadataPrefix: with_items metadataNamespace: http://koha-community.org/marcxml/with_items schema: http://koha-community.org/marcxml/with_items/xsd xsl_file: /usr/local/koha/xslt/marcxml.xsl include_items: yes
Hm, I am not sure it works setting this on Set level as a lot of libraries might not be using sets - they are an optional feature and not needed if you want to get harvest the full catalog I think. Maybe Gaetan is right - I am really not sure what's best here.
(In reply to Katrin Fischer from comment #50) > Hm, I am not sure it works setting this on Set level as a lot of libraries > might not be using sets - they are an optional feature and not needed if you > want to get harvest the full catalog I think. Yeah, doing it at the set level wouldn't make sense. I don't know anything about Koha's "OAI-PMH:ConfFile" or "extended mode", so can't really comment there... I certainly get the idea of it being optional though and having the default be not to include items...
Any decision on the horizon? As said by Johnathan, this patch solves a regression introduced by bug 5579. The data visibility breach is a real issue. But most of the libraries using Koha OAI Server that I know need to expose item data, at least call number, and location. Without item data, their OAI server is almost useless... Couldn't we agree with Thomas solution? In Normal mode, no item data. In Extended mode, optional item data. This way, item will only appear back on purpose.
I agree with Thomas' solution, although, to be picky, i would do it the other way around and would make exposing all the data the default, and filtering out things the specific behaviour ;) But i would also like to see this solved soon, so i really don't mind what the default is in the end.
Bug 3206 prevents this patch to apply anymore.
> Bug 3206 prevents this patch to apply anymore. and Bug 13568
Created attachment 41135 [details] [review] Bug 12252 - OAI-PMH result includes item data GetRecord for OAI-PMH was pulling the MARCXML directly from the database. Now it uses GetMarcBiblio and includes the item data with it, making it more generally useful. Test plan: * Run an OAI-PMH query, for example: http://koha/cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:52&metadataPrefix=marcxml to fetch biblionumber 52 * Note that it doesn't include the 952 data * Apply the patch * Do the same thing, but this time see that the 952 data is at the bottom of the MARCXML. Note: * This patch also includes a small tidy-up in C4::Biblios to group things semantically a bit better, so I don't spend ages looking for a function that was staring me in the face all along again. Signed-off-by: David Cook <dcook@prosentient.com.au> Works as described. Simple yet useful patch. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Frederic Demians <f.demians@tamil.fr> 952/995 item fields are back in response to GetRecord OAI verb. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 41136 [details] [review] Bug 12252 - OAI-PMH items respect OpacHiddenItems This allows the OAI-PMH service to not provide item information when there is a rule that would supress it in OpacHiddenItems. Test plan: * Find an OAI-PMH URL that shows you some items. * Add an entry to OpacHiddenItems that would block that. * Check that it's blocked. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Tested, playing with OpacHiddenItems. GetRecord OAI verb returns a record complying with OpacHiddenItems rules, for example without items from a specific library. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 41137 [details] [review] Bug 12252 - include item data in OAI-PMH ListRecords result Same in spirit to the other patch, this also includes the item detail in ListRecords. Test plan: * Fetch a URL like: http://koha/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml * Verify that there are 952 entries in the returned records where appropriate. Signed-off-by: Frederic Demians <f.demians@tamil.fr> ListRecords OAI verb returns a list of records including items in 952/995 which are not hidden based on OpacHiddenItems syspref. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 41138 [details] [review] Bug 12252: Add tests for EmbedItemsInMarcBiblio Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 41139 [details] [review] Bug 12252: (qa-follow-up) Remove perlcritic error
Created attachment 41140 [details] [review] Bug 12252 Include item data only in extended mode Previous patches attached to this bug have been refactored to merge bug 3206 and bug 13568 features. So OAI server must be carrefully tested to ensure that there is no regression in this area: deleted records and resumption token. This last patch fixed the way items are returned. They are returned only if OAI server operates in extended mode, and specifically for format having the parameter include_item set to 1 (true). For example this configuration file set via OAI-PMH:ConfFile syspref will return items: --- format: marcxml: metadataPrefix: marxml metadataNamespace: http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim schema: http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd include_items: 1 oai_dc: metadataNamespace: http://www.openarchives.org/OAI/2.0/oai_dc/ metadataPrefix: oai_dc schema: http://www.openarchives.org/OAI/2.0/oai_dc.xsd xsl_file: /home/tamil/client/devel/kohaclone/koha-tmpl/intranet-tmpl/prog/en/xslt/UNIMARCslim2OAIDC.xsl To test: 1 Call the oai server with metadataprefix=marcxml, verbs ListRecords and ListIdentifiers 2 No items are returned 3 Create an oai.conf file as above, and fill OAI-PMH:ConfFile syspref 4 Repeate 1. Items are returned.
Created attachment 41175 [details] [review] Bug 12252 - OAI-PMH result includes item data [SIGNED-OFF] GetRecord for OAI-PMH was pulling the MARCXML directly from the database. Now it uses GetMarcBiblio and includes the item data with it, making it more generally useful. Test plan: * Run an OAI-PMH query, for example: http://koha/cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:52&metadataPrefix=marcxml to fetch biblionumber 52 * Note that it doesn't include the 952 data * Apply the patch * Do the same thing, but this time see that the 952 data is at the bottom of the MARCXML. Note: * This patch also includes a small tidy-up in C4::Biblios to group things semantically a bit better, so I don't spend ages looking for a function that was staring me in the face all along again. Signed-off-by: David Cook <dcook@prosentient.com.au> Works as described. Simple yet useful patch. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Frederic Demians <f.demians@tamil.fr> 952/995 item fields are back in response to GetRecord OAI verb. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Created attachment 41176 [details] [review] Bug 12252 - OAI-PMH items respect OpacHiddenItems [SIGNED-OFF] This allows the OAI-PMH service to not provide item information when there is a rule that would supress it in OpacHiddenItems. Test plan: * Find an OAI-PMH URL that shows you some items. * Add an entry to OpacHiddenItems that would block that. * Check that it's blocked. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Tested, playing with OpacHiddenItems. GetRecord OAI verb returns a record complying with OpacHiddenItems rules, for example without items from a specific library. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Created attachment 41177 [details] [review] Bug 12252 - include item data in OAI-PMH ListRecords result [SIGNED-OFF] Same in spirit to the other patch, this also includes the item detail in ListRecords. Test plan: * Fetch a URL like: http://koha/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml * Verify that there are 952 entries in the returned records where appropriate. Signed-off-by: Frederic Demians <f.demians@tamil.fr> ListRecords OAI verb returns a list of records including items in 952/995 which are not hidden based on OpacHiddenItems syspref. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Created attachment 41178 [details] [review] Bug 12252: Add tests for EmbedItemsInMarcBiblio [SIGNED-OFF] Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Created attachment 41179 [details] [review] Bug 12252: (qa-follow-up) Remove perlcritic error [SIGNED-OFF] Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Created attachment 41180 [details] [review] Bug 12252 Include item data only in extended mode [SIGNED-OFF] Previous patches attached to this bug have been refactored to merge bug 3206 and bug 13568 features. So OAI server must be carrefully tested to ensure that there is no regression in this area: deleted records and resumption token. This last patch fixed the way items are returned. They are returned only if OAI server operates in extended mode, and specifically for format having the parameter include_item set to 1 (true). For example this configuration file set via OAI-PMH:ConfFile syspref will return items: Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Works great, items will not be shown unless you set up a specific configuration file with include_items: 1 in the options for the marcxml metadata prefix.
When applying the patch on master, I get the following error when trying to display an existing record: Request: /cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:10&metadataPrefix=oai_dc Response: Can't use an undefined value as a subroutine reference at /home/koha/versions/kohacommunity/opac/oai.pl line 337.
Indeed, that was sloppy testing on my part: i tested the listrecord verb, verified deleted records and resumption token where properly handled, but it turns out the getrecord verb is broken. Marking as failed.
Created attachment 42018 [details] [review] Bug 12252 - OAI-PMH result includes item data [SIGNED-OFF] GetRecord for OAI-PMH was pulling the MARCXML directly from the database. Now it uses GetMarcBiblio and includes the item data with it, making it more generally useful. Test plan: * Run an OAI-PMH query, for example: http://koha/cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:52&metadataPrefix=marcxml to fetch biblionumber 52 * Note that it doesn't include the 952 data * Apply the patch * Do the same thing, but this time see that the 952 data is at the bottom of the MARCXML. Note: * This patch also includes a small tidy-up in C4::Biblios to group things semantically a bit better, so I don't spend ages looking for a function that was staring me in the face all along again. Signed-off-by: David Cook <dcook@prosentient.com.au> Works as described. Simple yet useful patch. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Frederic Demians <f.demians@tamil.fr> 952/995 item fields are back in response to GetRecord OAI verb. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Created attachment 42019 [details] [review] Bug 12252 - OAI-PMH items respect OpacHiddenItems [SIGNED-OFF] This allows the OAI-PMH service to not provide item information when there is a rule that would supress it in OpacHiddenItems. Test plan: * Find an OAI-PMH URL that shows you some items. * Add an entry to OpacHiddenItems that would block that. * Check that it's blocked. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Tested, playing with OpacHiddenItems. GetRecord OAI verb returns a record complying with OpacHiddenItems rules, for example without items from a specific library. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Created attachment 42020 [details] [review] Bug 12252 - include item data in OAI-PMH ListRecords result [SIGNED-OFF] Same in spirit to the other patch, this also includes the item detail in ListRecords. Test plan: * Fetch a URL like: http://koha/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml * Verify that there are 952 entries in the returned records where appropriate. Signed-off-by: Frederic Demians <f.demians@tamil.fr> ListRecords OAI verb returns a list of records including items in 952/995 which are not hidden based on OpacHiddenItems syspref. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Created attachment 42021 [details] [review] Bug 12252: Add tests for EmbedItemsInMarcBiblio [SIGNED-OFF] Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Created attachment 42022 [details] [review] Bug 12252: (qa-follow-up) Remove perlcritic error [SIGNED-OFF] Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Created attachment 42023 [details] [review] Bug 12252 Include item data only in extended mode [SIGNED-OFF] Previous patches attached to this bug have been refactored to merge bug 3206 and bug 13568 features. So OAI server must be carrefully tested to ensure that there is no regression in this area: deleted records and resumption token. This last patch fixed the way items are returned. They are returned only if OAI server operates in extended mode, and specifically for format having the parameter include_item set to 1 (true). For example this configuration file set via OAI-PMH:ConfFile syspref will return items: Signed-off-by: Admin User Koha <koha@matrice_wheezy>
Created attachment 42024 [details] [review] Bug 12252: (follow-up) Fix error raised on comment 69 http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12252#c69
I have rebased this patch because i didn't manage to apply them (with or without git-bz) and added a follow up to fix the error on comment 69. However, i didn't understand the way the OAI server's config file works. I created a YAML file containing only "include_items: 1" and now i get an error while retrieving a record: <error code="cannotDisseminateFormat">Dissemination as 'oai_dc' is not supported</error>
The conf file needs to be more complete, an example conf file would be (note there was a small typo in Frederic's example in the metadataPrefix, but this one should work): --- format: marcxml: metadataPrefix: marcxml metadataNamespace: http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim schema: http://www.loc.gov/MARC21/slim http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd include_items: 1 oai_dc: metadataNamespace: http://www.openarchives.org/OAI/2.0/oai_dc/ metadataPrefix: oai_dc schema: http://www.openarchives.org/OAI/2.0/oai_dc.xsd xsl_file: /home/koha/src/koha-tmpl/intranet-tmpl/prog/en/xslt/UNIMARCslim2OAIDC.xsl
There is a problem I think in "OAI-PMH items respect OpacHiddenItems" : This patch changes GetMarcBiblio adding "opac" arg. But there is no : $opac = shift
(In reply to Fridolin SOMERS from comment #80) > There is a problem I think in "OAI-PMH items respect OpacHiddenItems" : > > This patch changes GetMarcBiblio adding "opac" arg. > But there is no : $opac = shift Oups sorry, its corrected by patch : "Include item data only in extended mode"
Thx Gaetan, it works fine with this configuration file. The QA script hasn't found any issues so the last patch could be checked for singing off.
Created attachment 42314 [details] [review] [SIGNED-OFF] Bug 12252: (follow-up) Fix error raised on comment 69 http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12252#c69 Signed-off-by: Gaetan Boisson <gaetan.boisson@biblibre.com>
Checked all the OAI verbs presented on this page : http://re.cs.uct.ac.za/ everything went smoothly.
apologies for my last commit message.
One question: How librarians will know this feature exists (No, they don't read release notes)? There is no way to know about it. Gaetan: Please don't add the "[SIGNED-OFF]" string in the commit messages :)
I think it should go in the manual, in the section that documents the OAI-PMH:ConfFile syspref. (Somewhere in this section : http://translate.koha-community.org/manual/master/en/html-desktop/#webserviceprefs )
Created attachment 42575 [details] [review] Bug 12252 - OAI-PMH result includes item data GetRecord for OAI-PMH was pulling the MARCXML directly from the database. Now it uses GetMarcBiblio and includes the item data with it, making it more generally useful. Test plan: * Run an OAI-PMH query, for example: http://koha/cgi-bin/koha/oai.pl?verb=GetRecord&identifier=KOHA-OAI-TEST:52&metadataPrefix=marcxml to fetch biblionumber 52 * Note that it doesn't include the 952 data * Apply the patch * Do the same thing, but this time see that the 952 data is at the bottom of the MARCXML. Note: * This patch also includes a small tidy-up in C4::Biblios to group things semantically a bit better, so I don't spend ages looking for a function that was staring me in the face all along again. Signed-off-by: David Cook <dcook@prosentient.com.au> Works as described. Simple yet useful patch. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Frederic Demians <f.demians@tamil.fr> 952/995 item fields are back in response to GetRecord OAI verb. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Signed-off-by: Gaetan Boisson <gaetan.boisson@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 42576 [details] [review] Bug 12252 - OAI-PMH items respect OpacHiddenItems This allows the OAI-PMH service to not provide item information when there is a rule that would supress it in OpacHiddenItems. Test plan: * Find an OAI-PMH URL that shows you some items. * Add an entry to OpacHiddenItems that would block that. * Check that it's blocked. Signed-off-by: Frederic Demians <f.demians@tamil.fr> Tested, playing with OpacHiddenItems. GetRecord OAI verb returns a record complying with OpacHiddenItems rules, for example without items from a specific library. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Signed-off-by: Gaetan Boisson <gaetan.boisson@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 42577 [details] [review] Bug 12252 - include item data in OAI-PMH ListRecords result Same in spirit to the other patch, this also includes the item detail in ListRecords. Test plan: * Fetch a URL like: http://koha/cgi-bin/koha/oai.pl?verb=ListRecords&metadataPrefix=marcxml * Verify that there are 952 entries in the returned records where appropriate. Signed-off-by: Frederic Demians <f.demians@tamil.fr> ListRecords OAI verb returns a list of records including items in 952/995 which are not hidden based on OpacHiddenItems syspref. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Signed-off-by: Gaetan Boisson <gaetan.boisson@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 42578 [details] [review] Bug 12252: Add tests for EmbedItemsInMarcBiblio Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Signed-off-by: Signed-off-by: Gaetan Boisson <gaetan.boisson@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 42579 [details] [review] Bug 12252: (qa-follow-up) Remove perlcritic error Signed-off-by: Signed-off-by: Gaetan Boisson <gaetan.boisson@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 42580 [details] [review] Bug 12252 Include item data only in extended mode Previous patches attached to this bug have been refactored to merge bug 3206 and bug 13568 features. So OAI server must be carrefully tested to ensure that there is no regression in this area: deleted records and resumption token. This last patch fixed the way items are returned. They are returned only if OAI server operates in extended mode, and specifically for format having the parameter include_item set to 1 (true). For example this configuration file set via OAI-PMH:ConfFile syspref will return items: Signed-off-by: Signed-off-by: Gaetan Boisson <gaetan.boisson@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 42581 [details] [review] Bug 12252: (follow-up) Include item data only in extended mode Fix perlcritic error. Signed-off-by: Signed-off-by: Gaetan Boisson <gaetan.boisson@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Patches pushed to master. Thanks Robin and everyone involved.
Do folk have a preferred place where they store this file? I'm storing it based on my web server configuration at the moment, but it could be good to have an official place to put "extra" configuration like this.
(In reply to David Cook from comment #96) > Do folk have a preferred place where they store this file? > > I'm storing it based on my web server configuration at the moment, but it > could be good to have an official place to put "extra" configuration like > this. I put it in /etc/koha/sites/instance/is It would be good to move it to a yaml syspref.
(In reply to Tomás Cohen Arazi from comment #97) > I put it in /etc/koha/sites/instance/is > > It would be good to move it to a yaml syspref. I was actually surprised to discover that it was a filepath syspref. I would have sworn it used to be a yaml syspref, but I think I might just be imagining that. I find filepath sysprefs a bit awkward because they expose the underlying file system to web users, so I think I'd prefer a yaml syspref. But then the yaml syspref could have filepaths in it too :/. How would you propose changing this syspref? Making a second one and prioritize the second one over the existing one? Use the upgrade script to read the file into a yaml syspref? I admit I don't really want to manage the oai config in configuration files on the file system so I'm pretty interested in your idea >_>.
I notice two issues regarding this bug, I wonder if it's the intended behavior: 1. `include_items` works great, but it makes public all non-public notes (952$x). IMO this is a bug. Meanwhile, it can be removed using a XSLT (`xsl_file`)? 2. OAI-PMH:ConfFile is a filepath syspref but doesn't let you set an URL (returns 500 Internal Server Error). Shouldn't this syspref allow URLs also? Just like OPACXSLTDetailsDisplay. Maybe all sysprefs of this kind should handle both types of references. This is particularly useful to point to raw files hosted on Github's like sites.
(In reply to Pablo AB from comment #99) > I notice two issues regarding this bug, I wonder if it's the intended > behavior: > > 1. `include_items` works great, but it makes public all non-public notes > (952$x). IMO this is a bug. Meanwhile, it can be removed using a XSLT > (`xsl_file`)? Agreed, this is a bug and you can take care of this with a custom XSLT in the meantime. File a bug report! > 2. OAI-PMH:ConfFile is a filepath syspref but doesn't let you set an URL > (returns 500 Internal Server Error). Shouldn't this syspref allow URLs also? > Just like OPACXSLTDetailsDisplay. Maybe all sysprefs of this kind should > handle both types of references. This is particularly useful to point to raw > files hosted on Github's like sites. Agreed.