Project

General

Profile

Bug #2048

Find namespace for poster fra lånerstatus

Added by Steen Larsen about 3 years ago. Updated about 2 years ago.

Status:
Resolved (tag version)
Priority:
Urgent
Assignee:
Estimated time:
URL med eksempel:
Kategorier:
Min konto - Lån, Min konto - Mellemværende, Min konto - Reserveringer

Description

Når ddbcms henter data fra provideren (fbs, alma) til lånerstatus fås kun faustnummer på de enkelte poster.

For at benytte posterne i det videre forløb skal namespace sættes på, så man får et pid.
Hvis ikke eller der benyttes et forkert namespace kan posten ikke søges frem og resultatet vil være tom post eller manglende oplysninger.

For namespace er der to muligheder: "870970-basis" eller "AGENCYID-katalog", hvor AGENCYID er bibliotekets agencyid.

Hvad der skal benyttes afhænger af opensearch – version 3.0 eller 3.5:

Før 3.0_4.2 kan dette bestemmes ud fra faustnummer - lokale faust starter med 9 og her er det "AGENCYID-katalog" ellers er det "870970-basis" og metoden bliver allerede brugt i ddbcms.

For 3.0_4.2 og fremefter er namespace AGENCYID-katalog

For 3.5_4.x og fremefter afhænger namespace af posten – for en lokal post eller en påhængspost er det "AGENCYID-katalog" ellers er det en basispost med "870970-basis". Typen kan IKKE afgøres ud fra faustnummeret.

En løsning på dette er at lave søgningen: "rec.id=870970-basis:FAUST or rec.id=AGENCYID-katalog:FAUST"
I FBS skal der kombineres med holdingsitem som øvrige søgninger i FBS.

Resultatet, dvs identifier i posten afgør hvilket namespace der skal benyttes og derved hvilket pid der herefter skal bruges i link m.m.

I FBS kan en post godt skifte mellem at være en påhængspost og en basispost - det afhænger af om personalet opdaterer posten via katalogiseringen - men jeg gætter på at det ikke vil ske så ofte.
Dette for at sige at en vis form for caching burde være mulig.

Problemet med manglende/forkert namespace ses i sagerne: #1270 #1271 #1677 #1698

2048-loan-list-with-namespace-fix.PNG (43.3 KB) 2048-loan-list-with-namespace-fix.PNG Simon Holt, 08/09/2017 05:20 PM
2048-test-79-loans.txt (3.05 KB) 2048-test-79-loans.txt Simon Holt, 09/22/2017 03:03 PM
2048-test-49-loans.txt (1.85 KB) 2048-test-49-loans.txt Simon Holt, 09/22/2017 03:03 PM
2048-test-263-loans.txt (9.57 KB) 2048-test-263-loans.txt Simon Holt, 09/22/2017 03:03 PM

Related issues

Related to DDB CMS - Bug #1271: FBS integration - Materiale titel vises ikke i oversigten MellemværendeResolved (tag version)
Related to DDB CMS - Bug #1948: Poster vises ikke korrekt i DDB CMSResolved
Related to DDB CMS - Bug #1849: getObject er tilsyneladende ligeglad med søgeprofilResolved (tag version)
Related to DDB CMS - Bug #2452: Manglende titler på lån i låneroversigten DDB CMS version 2.5.1Resolved (tag version)

History

#1 Updated by Rolf Madsen about 3 years ago

  • Status changed from New to Ready for development
  • Assignee set to Martin Cording
  • Target version set to Release 27 - Bugfixes (2017 2. opgradering) (7.x-4.2.1)

#2 Updated by Rolf Madsen about 3 years ago

  • Priority changed from High to Urgent

#3 Updated by Rolf Madsen about 3 years ago

  • Target version changed from Release 27 - Bugfixes (2017 2. opgradering) (7.x-4.2.1) to Release 27 - Bugfixes (Inlead)

#4 Updated by Steen Larsen about 3 years ago

  • Description updated (diff)

#5 Updated by Jesper K. Lund about 3 years ago

Mht. om skifte mellem påhængspost og basispost i FBS vil ske ofte eller ej, vil dette afhænge af muligheden for at søge DBC's rettelsesposter frem.
Pt. kan vi ikke men det er meningen at vi skal kunne og så vil der oftere ske et skifte fra en påhængspost til en basispost (i hvert fald hvis man ønsker at holde sin "lokale base" opdateret)

#6 Updated by Steen Larsen almost 3 years ago

Et alternativ forslag til at finde namespace for en række faustnumre kunne være denne - her med nogle tilfældige faustnumre fra vores agencyid:

rec.id any "106278369 106278377 106278385 41638435 41691050 52666341 52666562 52668921" and facet.acsource=bibliotekskatalog and holdingsitem.agencyid=775100

Systemet skal jo så kunne håndtere matchet mellem et givet faustnummer og et af de to mulige PID'ere
Jeg ved ikke om disse fundne poster kan bruges i den sædvanlige cache-håndtering eller om man skal starte forfra med at hente data.

#7 Updated by Martin Cording almost 3 years ago

  • Status changed from Ready for development to Needs code review

#8 Updated by Gitte Barlach almost 3 years ago

  • Status changed from Needs code review to Need more info

#9 Updated by Martin Cording almost 3 years ago

  • Status changed from Need more info to Ready for development

#10 Updated by Martin Cording almost 3 years ago

Af en eller anden årsag er der blevet implementeret en logic som kræver at "katalog"-poster start med et 9-tal:
https://github.com/ding2/ding2/commit/12307f84107c793e175d8a3dbd927717961198ff#commitcomment-20201945
Det er i mange tilfælde ikke sandt.

Er der noget dokumentation på denne ændring? Ved godt det er 2 år gammelt.. :)

#11 Updated by Simon Holt almost 3 years ago

> Af en eller anden årsag er der blevet implementeret en logic som kræver at "katalog"-poster start med et 9-tal

Ja det har jo været konventionen, at lokale faust numre starter med 9.

#12 Updated by Christel Krabbenhøft almost 3 years ago

  • Assignee changed from Martin Cording to Laura Holm
  • Target version changed from Release 27 - Bugfixes (Inlead) to Release 27 - Bugfixes (2017 2. opgradering) (7.x-4.2.1)

#13 Updated by Martin Cording almost 3 years ago

  • Status changed from Ready for development to Needs code review
  • Assignee changed from Laura Holm to Gitte Barlach

#14 Updated by Simon Holt almost 3 years ago

Så i søger på faust uden rec.id og namespace og bruger pid fra det første materialer der returneres.

Er det virkelig så simpelt. Vil det virke i alle tilfælde?

Den gamle udgave af funktionen generede også PID'er for andre namespaces som f.eks. 870973-anmeld. Er det ikke nødvendigt længere med FBS og er der ikke andre steder i systemet hvor dette bliver brugt?

#15 Updated by Steen Larsen almost 3 years ago

Et id (faustnummer) er ikke unikt på tværs af alle kilder
Det kunne i princippet indgå i namespace:id for andre kilder og det findes faktisk også i de øvrige indekser som fritekst-søgningen søger i. Ikke som et faustnummer men som tilfældigvis de samme 8 cifre. Jeg skal gerne finde nogle eksempler på et senere tidspunkt.

Som en variation af min tidligere note så burde denne søgning (her vist for FBS) for ét faust (fra lånerstatus) netop give det ønskede:

rec.id=FAUST and facet.acsource=bibliotekskatalog and holdingsitem.agencyid=AGENCYID

Jeg vil tro at timeout for lange lånerstatus-lister også vil ramme her (ligesom huskelister), hvis hvert faustnummer skal søges frem og der skal genereres allobjects/facetter m.m som standard ting_do_search vistnok gør?

Mht til ding_provider_build_entity_id(id, agency) kaldes den vistnok kun ét sted fra, hvor agency er med og hvor id potentielt kan være noget andet end et faustnummer: i ting_object_load
Men er det et levn fra skiftet til brønd 2 til 3? Så det behøver vi måske ikke mere? Kan det være fra gamle urler der håndteres?

#16 Updated by Simon Holt almost 3 years ago

> Det kunne i princippet indgå i namespace:id for andre kilder og det findes faktisk også i de øvrige indekser som fritekst-søgningen søger i.

Tænkte det nok og det er vel også det, der er hele ideen med namespaces.

> rec.id=FAUST and facet.acsource=bibliotekskatalog and holdingsitem.agencyid=AGENCYID

Ser ud som om det er sådan noget vi skal ud i. Vil dog lige tilføje at holdingsitem.agencyid=AGENCYID, bliver tilføjet automatisk hvis man har valgt det under 'admin/config/ting/settings'. Går ud fra det er noget alle FBS biblioteker indstiller?

#17 Updated by Steen Larsen almost 3 years ago

Ja, "and holdingsitem.agencyid=AGENCYID" tilføjes automatisk i ting_do_search når man har konfigureret ddbcms korrekt til FBS.

#18 Updated by Martin Cording almost 3 years ago

I er så godt inde i det her drenge - hvad foreslår I vi gør? Tilføjer rec.id til søgningen?

#19 Updated by Steen Larsen almost 3 years ago

Jeg vil jo nok foreslå denne jvf ovenfor dog med et vist forbehold:

rec.id=FAUST and facet.acsource=bibliotekskatalog

Benyttes ting_do_search tilføjes den relevante brønd 3.5-del hvis det er FBS.

Det bør nok følges op af en vurdering om brønd 2-3 koden i ting_object_load kan udelades for ovenstående søgning vil ikke fremsøge disse pid'ere.
Den kode er for at håndtere meget gamle urler ala
/ting/collection/150040:13731 => /ting/collection/150040-verdyr:13731 (som fejler for verdens dyr har nummer 150033 i dag)
Vi kan alligevel ikke håndtere nyere urler ved skiftet til 3.5 så hvorfor håndtere de gamle?
Dvs hele if-sætningen i ting_object_load med preg_match droppes

En større udfordring er at en søgning på ét faust ad gangen betyder at vi (sandsynligvis) ikke kan vise lange lånerlister (lidt på samme måde som lang huskeliste ikke kan vises, jvf #1934)
Men hvordan det så lige kan optimeres har jeg ikke noget bud på bortset fra selve søgestrengen (se note 6 ovenfor)

#20 Updated by Gitte Barlach almost 3 years ago

  • Status changed from Needs code review to Need more info
  • Assignee changed from Gitte Barlach to Martin Cording

Er konklusionen at der lige skal laves lidt mere på denne før den er klar?

#21 Updated by Martin Cording almost 3 years ago

  • Status changed from Need more info to Needs code review
  • Assignee changed from Martin Cording to Gitte Barlach

Jeg har opdateret PR - tjek lige om det stemmer overens hvad I havde i tankerne

#22 Updated by Simon Holt almost 3 years ago

Ser fint ud. Opdagede dog lige at facet.acSource tilsyneladende er case sensitive i http://opensearch.addi.dk/4.0.1/. Så det er nok sikrest at bruge:

facet.acSource=bibliotekskatalog

#23 Updated by Martin Cording almost 3 years ago

Fixed

#24 Updated by Simon Holt almost 3 years ago

Lige en lille rettelse: den søger kun på faust. Skal være:

$obj = ting_do_search('rec.id=' . $id . ' AND facet.acSource=bibliotekskatalog');

Og skal det være stort eller lille AND og anførselstegn om $id og bibliotekskatalog? kan snart ikke finde rundt i det længere.

#25 Updated by Steen Larsen almost 3 years ago

Vi behøver vel ikke at kode dette til version 4.0.1?

Så til version 4.1 og højere er der ikke forskel på store/små bogstaver i indeks og operatorer.
Og anførelsestegn er kun nødvendige når der er visse specialtegn og mellemrum.

Så søgningen det skal ende med er netop

rec.id=FAUST and facet.acsource=bibliotekskatalog and holdingsitem.agencyid=AGENCYID
rec.id=FAUST and facet.acsource=bibliotekskatalog

for henholdsvis opensearch 3.5_4.x og opensearch 3.0_4.x

rec.id skal med ellers kan det fejle for visse numre (som tilfældigvis findes i andre felter)

#26 Updated by Simon Holt almost 3 years ago

Ah ok tak for svar. Men vigtigt vi lige får rec.id på så.

> Vi behøver vel ikke at kode dette til version 4.0.1?

Det skal vel stadig virke for biblioteker der ikke er overgået til FBS. Er der ikke mange af dem der bruger 4.0.1?

#27 Updated by Steen Larsen almost 3 years ago

4.0.1 skal udfases.
FBS skal bruge opensearch b3.5_4.3 og dem der ikke er FBS skal bruge b3.0_4.3 men det er først muligt når håndtering af namespace er løst.

#28 Updated by Simon Holt almost 3 years ago

Ja den skal udfases, men hvis vi nu bruger lille and og facet.acSource, vil det så ikke virke i alle versioner? Så kan vi få den her rettelse hurtigere ud.

#29 Updated by Steen Larsen almost 3 years ago

Tja, i 4.0.1 skal det så være "facet.acSource" hvorimod and/AND/aNd giver samme svar.

Med indførelses af denne løsning skal der vel skiftes til 4.3 med det samme.

#30 Updated by Simon Holt almost 3 years ago

Jamen vil facet.acSource ikke også virke i b3.5_4.3 og b3.0_4.3? Jeg ser ikke det store problem med at sikre den også virker i 4.0.1, når det er så simpelt.

Det er jo ikke sikkert at der bliver skiftet til 4.3 med det samme.

#31 Updated by Steen Larsen almost 3 years ago

jojo den del vil virke :-)

#32 Updated by Christel Krabbenhøft almost 3 years ago

  • Target version changed from Release 27 - Bugfixes (2017 2. opgradering) (7.x-4.2.1) to DDB CMS 2017 1. opgradering (7.x-4.0.2)

#33 Updated by Gitte Barlach almost 3 years ago

  • Assignee changed from Gitte Barlach to Jørgen Nielsen

#34 Updated by Rolf Madsen almost 3 years ago

  • Related to Bug #1271: FBS integration - Materiale titel vises ikke i oversigten Mellemværende added

#35 Updated by Jørgen Nielsen almost 3 years ago

  • Status changed from Needs code review to Reviewed - Needs info/rework
  • Assignee changed from Jørgen Nielsen to Martin Cording

Mangler opfølgning på kommentar #24

#36 Updated by Martin Cording almost 3 years ago

  • Status changed from Reviewed - Needs info/rework to Needs code review
  • Assignee changed from Martin Cording to Jørgen Nielsen

Fixed

#37 Updated by Jørgen Nielsen almost 3 years ago

  • Status changed from Needs code review to Reviewed
  • Assignee changed from Jørgen Nielsen to Gitte Barlach

reviewet og godkendt

#38 Updated by Simon Holt over 2 years ago

Jævnfør denne tråd fra facebook DDB CMS for praktikere: https://www.facebook.com/groups/ddbcms/permalink/513698865471774/

Specielt dette svar fra Nanna: https://www.facebook.com/groups/ddbcms/permalink/513698865471774/?comment_id=513702868804707&comment_tracking=%7B%22tn%22%3A%22R9%22%7D

Det er en lille ting, men bør vi så ikke bruge term.acSource her i stedet for facet.acSource? Eller er der en speciel grund til vi har brugt facet i dette tilfælde?

#39 Updated by Steen Larsen over 2 years ago

Term.acsource vs facet.acsource har vist ikke nogen nævneværdig betydning her. Hastighed? Forkerte resultater?
Hvis det var ereolen ville det give en forskel - term.acsource=ereolen giver resultater fra Ereolen og Ereolen Global hvorimod man med facet.acsource=ereolen kun får resultater fra Ereolen
facet.acsource=bibliotekskatalog giver poster fra kilderne "Bibliotekets katalog", "Bibliotekskatalogiserede online-materialer" og i "Folkebiblioteker og Nationalbibliografi" (hvis de er aktiveret i profilen selvfølgelig).
og det er det vi har brug for (online-materialerne er dog støj men det påvirker ikke resultatet).

Den største udfordring med nærværende løsning er, at den ikke er optimeret, så lånere med lang lånerstatus vil (ret) sandsynligt få en fejl 500 (pga timeout), da hver enkelt post skal søges frem.
Hvor grænsen på en lang lånerstatus ligger er ukendt og vi må håbe den ligger højt.

#40 Updated by Simon Holt over 2 years ago

Er med på det giver samme resultater. Det var mere denne her kommentar fra Nanna, som jeg linkede til i ovenstående:

> Facet-indekserne er særlige indekser lavet til at danne facetterne og som sådan ikke tænkt til at være søgeindekser.

Så tænkte det ville være mest rigtig med term.acsource. Der kan muligvis også være noget med performance?

#41 Updated by Steen Larsen over 2 years ago

Facetter bruger vi jo ofte når vi har lavet en søgning og derefter indskrænker/begrænser resultatet - det gør vi også her - vi søger på faustnummer og ønsker at begrænse til bibliotekskataloget, så jeg mener ikke vi skal gøre mere mht acsource..

#42 Updated by Simon Holt over 2 years ago

Jamen det er jo sådan man næsten altid bruger et søgeindeks; i en søgestreng med nogle andre ord/søgninger. Så kan ikke helt forstå dit argument.

Jeg prøver ikke at argumentere for, at det er decideret forkert at bruge facet-indekserne. Og i dette tilfælde giver det jo samme resultat.

Min pointe er, at hvis det er best-practice at anvende de almindelige søgeindekser, synes jeg også vi bør anvende det i koden.

Vi har vist ikke fået afklaret det med om der kan forskel i performance i de to indekser. Og hvis du allerede kan forudsige problemer med timeout, er det jo ikke helt urelevant.

#43 Updated by Steen Larsen over 2 years ago

Det lyder til at man ret hurtig rammer timeout og lånerstatus derfor ikke kan hentes ind.

Måske skal man alligevel prøve metoden beskrevet i note-6 med mindre andre har nogle bedre forslag?

Altså her med Aarhus som eksempel:

rec.id any "F1 F2 F3 F4" and facet.acsource=bibliotekskatalog and holdingsitem.agencyid=775100

hvor F1..F4 så er alle faustnumre fra en konkret lånerstatus.

Så kunne man f.eks. få disse retur:

775100-katalog:F3
870970-basis:F2
870970-basis:F1
870970-basis:F4

og på den måde har vi herved namespace til de 4 poster OG de dkabm-oplysninger der skal bruges til listen i lånerstatus.
Koden skal jo så matche posterne til de resp. faustnumre

Metoden vil virke på de fleste korrekte poster.

Der vil dog være nogle poster som er unit-matchet pga fejl og hvor metoden ikke virker, men det er faktisk poster som skal rettes i brønden.
I disse meget få tilfælde vil søgning på ét faust returnere et andet faust.

#44 Updated by Thomas Hansen over 2 years ago

Ja, det er sløvt i https://github.com/ding2/ding2/pull/485 fordi der laves en enkelt-post søgning i ding_provider_build_entity_id().

Det ville være smartere om providerne bare returnerede localId, og lookup'et så blev lavet senere, i bulk.

#45 Updated by Rolf Madsen over 2 years ago

  • Related to Bug #1948: Poster vises ikke korrekt i DDB CMS added

#46 Updated by Rolf Madsen over 2 years ago

  • Related to Bug #1849: getObject er tilsyneladende ligeglad med søgeprofil added

#47 Updated by Simon Holt over 2 years ago

Jeg har testet lidt på en installation hvor både timeout i Apache og max_execution_time i PHP er sat til 10 min.

Testen foregik med at jeg loggede ind med et "lånerbibliotek" i DDB CMS for herefter at prøve at tilgå /user/me/status og observer tiden i Chrome developer tools.

Status med 47 lån: 4,6 min
Status med 65 lån: 6.1 min
Status med 195 lån: Timeout efter 10 min.

Så der skal helt klart gøres noget.

Tilføjelse: Prøvede også lige en status med 10 lån og her tog det omkring 40 sekunder.

#48 Updated by Gitte Barlach over 2 years ago

  • Assignee changed from Gitte Barlach to Thomas Hansen

Hej Thomas

Har du mulighed for at se på denne?

#49 Updated by Simon Holt over 2 years ago

Kan godt lave et PR hvis det er? Har en ide til en løsning

#50 Updated by Steen Larsen over 2 years ago

Interessant - hvilken vej vil du gå?

#51 Updated by Simon Holt over 2 years ago

Opdater ding_provider_build_entity_id() så den også kan virke med et array af id'er og sørge for at fbs_loan_list() benytter sig af dette.

#52 Updated by Simon Holt over 2 years ago

Jeg tænker noget i retning af det her:

https://github.com/ding2/ding2/pull/608

ding_provider_build_entity_id() kan nu håndtere arrays med id'er. Dette anvendes ved låne, mellemværende og reserveringsstatus i PR. Har lige testet og det giver en stor performance forbedring.

Jeg har valgt den "nemme" metode og håndteret det i FBS-provider laget indtil videre. På den måde har jeg næsten kunnet undgå at ændre i eksisterende kode. Ulempen er at det skal laves seperat for ALMA også, men det kan dog nemt laves.

Det kan muligvis håndteres mere elegant i ding_debt, ding_reservation og ding_loan, men det så ud til at ville kræve en del ændringer i koden.

#53 Updated by Steen Larsen over 2 years ago

Jeg har nu prøvet at lave en rec.id any "..." på rigtigt mange faustnumre - er der mange (400-600) får jeg fejlen "Internal problem: Cannot decode Solr result" - det afhænger tilsyneladende også af profilen

Men mere vigtigt - er jeg under grænsen og får poster retur får jeg kun 50 collections - så der er en øvre grænse på stepValue

#54 Updated by Simon Holt over 2 years ago

> Jeg har nu prøvet at lave en rec.id any "..." på rigtigt mange faustnumre - er der mange (400-600) får jeg fejlen "Internal problem: Cannot decode Solr result" - det afhænger tilsyneladende også af profilen

Ok, den er vild nok :) Vi har flere lånerbiblioteker med over 500 lån, så vil lige prøve at teste med dem.

Skal vi mon ud i noget med at splitte id'erne op i bider på 50?

#55 Updated by Steen Larsen over 2 years ago

Ja, opsplitning i 50 ad gangen gør vi faktisk også i anden sammenhæng så det skal nok også gøres her.

Informationerne (titel, forfatter, mm.) skal efterfølgende benyttes til lånerlisten - dé oplysninger skal vi vel ikke hente igen? (jeg kan ikke helt gennemskue hvordan caching er her)

#56 Updated by Simon Holt over 2 years ago

Det ser ud til at den forsøger at cache alle objekter fra et søge-request i ting_execute_cache(). Om det virker og den rent faktisk rammer caching ved efterfølgende ting_get_object, skal nok lige undersøges.

Det ville være mest hensigtsmæssig, hvis det med hvad der blev vist blev håndteret i et højere lag og provider modulerne bare returnerede alt data fra bibliotekssystemet og slet ikke bekymrede sig om namespaces fra brønden. Men på nuværende tidspunkt er det lidt blandet sammen. Se f.eks. fbs_debt_list() der bruger objektet fra brønden til at finde ud af hvilken titel der skal vises.

På nuværende tidspunkt er det måske bedst ikke at pille for meget ved det og gå med den simple løsning, da jeg vil mene det er et rimelig akut problem.

#57 Updated by Gitte Barlach over 2 years ago

  • Assignee changed from Thomas Hansen to Simon Holt

Tak fordi du byder ind på denne her, Simon. Jeg har tilladt mig at assigne sagen til dig;-) Jeg tror også det er bedst at gå efter den simple løsning her og nu, men ser du behov for en re-faktorering senere, må du gerne oprette en sag på det.

#58 Updated by Simon Holt over 2 years ago

Hej Gitte. Det var så lidt. Jeg laver lige det med opsplitning som Steen har foreslået og så skulle den være klar.

#59 Updated by Kasper Garnæs over 2 years ago

  • Status changed from Reviewed to Development

Simon; Arbejder du stadig på denne? Ud fra seneste PR i tråden kan jeg ikke se at ændringen er godkendt.

#60 Updated by Simon Holt over 2 years ago

@Kasper. Arbejder stadig på denne og satser på at kunne færdiggøre den i morgen.

#61 Updated by Simon Holt over 2 years ago

Så har jeg opdateret PR så den henter 50 ad gangen og dermed sikre at vi oversat alle lokale id'er:

https://github.com/ding2/ding2/pull/608

Beklager det trak lidt ud. Jeg observerede at den nogle gange ikke fik oversat nogle af id'erne. Det viste sig at være vækrmatch i opensearch der drillede, så den for nogle poster "skjulte" nogle id'er for mig. Løsningen viste sig at være at benytte opensearch parameteren collectionType og sætte den til 'manifestation'. ting_do_search kiggede efter denne parameter, men det viste sig den ikke blev sendt med når requestet blev bygget i TingClientSearchRequest:getRequest(). Så her er et PR mod ting-client der fikser dette.

https://github.com/ding2/ting-client/pull/22

Den er vigtig at få ellers vil ding_provider_build_entity_id() sprænge over nogle id'er engang i mellem.

Mangler stadig at optimere performance for ALMA låne, mellemværende og reservationsliste. Men er det nødvendigt? Hvor kommer i praksis til at få brug for dette?

#62 Updated by Gitte Barlach over 2 years ago

  • Status changed from Development to Needs code review
  • Assignee changed from Simon Holt to Jørgen Nielsen

Hej Jørgen
Du må meget gerne code review´e denne

#63 Updated by Jørgen Nielsen over 2 years ago

  • Status changed from Needs code review to Reviewed
  • Assignee changed from Jørgen Nielsen to Gitte Barlach

reviewet og godkendt

#64 Updated by Rolf Madsen over 2 years ago

  • Related to Bug #2452: Manglende titler på lån i låneroversigten DDB CMS version 2.5.1 added

#65 Updated by Kasper Garnæs over 2 years ago

  • Status changed from Reviewed to Technical test

Merged.

#66 Updated by Rolf Madsen over 2 years ago

  • Assignee changed from Gitte Barlach to Simon Holt

@Simon, kan du give os et par hints til hvordan vi tester dette issue?

#67 Updated by Steen Larsen over 2 years ago

F.eks. kan du reservere/låne/få oprettet regninger m.m. på lokale poster, påhængsposter og basisposter så de kan ses i din lånerstatus.

De kan søges frem via
Lokal poster: rec.id=katalog not rec.id=870970
Påhængsposter: rec.id=katalog and rec.id=870970
Basisposter: rec.id=870970 not rec.id=katalog and facet.acsource=bibliotekskatalog

Vigtigt at slette cache mellem oprettelsen og den efterfølgende test.

#68 Updated by Simon Holt over 2 years ago

@Rolf har testet med materialer på poster af elle tre typer, som Steen nævner ovenfor.

Det ser ud til den viser alle posterne korrekt og finder det rigtige namespace i alle tilfælde (se vedhæftning).

Basispost: Arduino essentials, link: /ting/object/870970-basis%3A51719662
Påhængspost: Kvinden i graven, link: /ting/object/763000-katalog%3A52387078
Lokalpost: The map of your life, link: /ting/object/763000-katalog%3A53022561

Herefter har jeg testet med nogle af vores "biblioteks-lånere" (biblioteker der er opretter som låner ved os for at administrere fjernlån) med meget store lånelister på hhv. 22, 68 og 159 lån. Det er vigtigt at teste med så store lister, så vi kan verificere at det med opsplitning spiller.

Ved alle listerne så den ud til at finde det rigtige namespace for posterne uden fejl. I de få tilfælde hvor der ikke var titel, viste det sig at materialet var bortkommet og posten ikke længere søgbar med vores agencyid. Skal vi ud i noget med at søge uden holdingsitem.agencyid i disse tilfælde? Vi kan i alle tilfælde ikke lave en link til post, men måske kan vise noget information i det mindste.

Fandt nogle småfejl relateret til søgninger uden resultat, så laver lige et PR i morgen med nogle smårettelser til ding_provider_build_entity_id().

#69 Updated by Simon Holt over 2 years ago

Så er der nyt PR med rettelser til ding_provider_build_entity_id(), der finder pid for faust-numre:

https://github.com/ding2/ding2/pull/721

Der er primært tale om bedre håndtering af de tilfælde, hvor et faust nummer ikke kan fremsøges i brønden med det pågældende biblioteks agency id i holdingsitem filteret. Først to vigtige ting:

1. Hvis et bibliotek ikke har beholding på et faust nummer, kan det ikke fremsøges med deres agency id i holdingsitem.agencyid filteret.
2. Det kan derimod godt hentes via getObject også selvom man angiver agencyid i feltet og VIP-profil.

Med opdateringen til ding_provider_build_entity_id i denne sag kan lånelisten nu vise lokalposter og påhængsposter korrekt. Jeg observerede dog, at der pludselig var nogle titler den godt kunne vise korrekt før i lånestatus, men lige pludselig ikke kunne med den nye måde at finde namespace på. Dette skyldes bl.a. punkt 2 i ovenstående og at:

a. Den gamle ding_provider_build_entity_id() brugte basispost namespacet (870970-basis:) på næsten alle faust (fra bibliotekskatalog) med mindre de startede med 9 (gammel konvention for lokalposter).
b. Dette resulterede i at den altid returnede et pid med namespace.
c. Grunden til at den havde held med at fremsøge dette PID alligevel (trods holdingsitem.agencyid filter) var pga følgende typo: https://github.com/ding2/ding2/blob/be061603b051b4fc6bcb548cb6d2a5193a64ad4a/modules/ting/ting.client.inc#L154, som nu er blevet rettet. Rettelsen betyder at den nu laver en søgning for disse poster i stedet for at hente separat via ting_get_object(). Så derfor kunne den faktisk finde posten før rettelsen af ovenstående typo (jvf. punkt 2 i ovenstående).

For at undgå at skulle ændre i for meget lige nu (det er meget skrøbeligt det her), har jeg derfor valgt følgende fremgangsmåde i PR:

- Når ding_provider_build_entity_id() ikke kan finde namespace for et faust bruger den DBC's standard namespace (870970-basis) for basisposter.
- Så hvis providerne konstatere at entitien ikke kunne findes, kan de prøve at loade den direkte via ting_get_object med standard PID'et (dette gjorde ding_loan i forvejen).

Der er dog nogle problemer med dette:

- Hvis der f.eks. er tale om et fjernlån af en lokalpost på et andet bibliotek, vil det ikke virke.
- Der er ikke sikkert at entitien rent faktisk kan vises. Det afhænger af om den er blevet cachet ved det direkte kald til ting_get_object. Ligger den ikke i cachen vil den ikke kunne findes i brønden ved en materialevisning (da den nu bruger søgning her i stedet for ting_get_object) og visningen vil fejle. Providerne kan heller ikke skelne imellem hvilke materialer der kan være "farlige" at lave links til.

En mulig fremgangsmåde vil være at lave en ny build_entity funktion, der prøver at oversætte faust uden om holdingsitem.agency filteret. Måske kan vi i nogle tilfælde blive nødt til at falde tilbage på den ILL information vi kan få fra lånestatus fra FBS API (har ikke testet om vi kan fremsøge andre bibliotekers lokalpost namespace, når man ikke bruge holdingsitem.agencyid filter).

Det her gælder alle status listerne (låne, mellemværende og reservation). Tror det vil være en god ide at samle det hele i en ny sag og beslutte, hvordan vi vil håndtere materialer i statuslisterne, der ikke kan fremsøges i bibliotekets brønd. Derfor har jeg ikke prøvet at løse det i mit PR og har bare brugt DBC standard namespace i disse tilfælde, så vi kan få den her med hurtigst muligt.

#70 Updated by Lotte Tøstesen over 2 years ago

  • Status changed from Technical test to Needs code review
  • Assignee changed from Simon Holt to Gitte Barlach

Tak Simon, jeg sætter den til Code review og så må vi have resten med i et nyt issue.

#71 Updated by Gitte Barlach over 2 years ago

  • Assignee changed from Gitte Barlach to Kasper Garnæs

#72 Updated by Rolf Madsen over 2 years ago

Vi plejer at hente metadata fra biblioteksystemet når de f. eks. er fjernlån og ikke kan hentes fra brønden.

Er det ikke stadig fall back?

#73 Updated by Simon Holt over 2 years ago

Ikke på låne og mellemværende listen i hvert fald. Den har gjort det på lånelisten før, men er blevet fjernet ifb med denne sag #2404. (release 4.x branch)

På reservationslisten prøver den stadig at få providerne til at lave en pseudo-entity ud fra oplysninger fra bibliotekssystemet. Problemet er at FBS ikke implementerer dette og heller ikke har de sammme muligheder som ALMA (ser det umiddelbart ud til).

Så det er lidt noget rod det her og derfor vil jeg foreslå vi samler det i en sag og tager en beslutning. Tror faktisk søgninger uden holdingsitem.agencyid filtrering er en bedre vej i FBS og brønd 3.5.

Kan i øvrigt oplyse at punkt 2. fra mit tidligere indlæg også gælder i opensearch 4.5.

#74 Updated by Rolf Madsen over 2 years ago

Det lyder altså meget som det vi allerede har behandlet i #1270 hvor konklusionen var at titlen skulle hentes fra ilBibliographicRecord (http://platform.dandigbib.org/issues/1270#note-14) i FBS og at det var implementeret i fbs.loan.inc som beskrevet af Thomas i http://platform.dandigbib.org/issues/1270#note-19.

Eller er der noget jeg helt har misforstået?

#75 Updated by Simon Holt over 2 years ago

Ah kan godt se den bruger ILL information fra låneobjektet,hvis det er sat. Det jeg observerede var, at ALMA implementerer populate_pseudo_entity i alma.loan.inc og det gør FBS ikke.

Det er vel også fint nok bare at gøre det sådan. Men der var altså flere titler, der stadig ikke blev vist da jeg testede. Kan være de blev vist før pga af den typo, jeg har omtalt i ovenstående. Men nu er den blevet rettet,så det kan vi ikke falde tilbage på længere.

Den typo gjorde at alle basisposter fra bibliotekskataloget blev hentet ned separat med ting_get_object uden om agencyid filteret.

#76 Updated by Simon Holt over 2 years ago

Der kan være andre årsager end fjernlån til at et materiale kan fremsøges i bibliotekets brønd med deres agencyid, og vi dermed ikke kan vise information på låne, resevering og mellemværende status.

Mit forslag er at samle det i en sag. I mange af tilfældene kan vi muligvis fremsøge det alligevel uden om agencyid, så vi kan måske kigge på det i stedet.

#77 Updated by Kasper Garnæs over 2 years ago

  • Status changed from Needs code review to Technical test
  • Assignee changed from Kasper Garnæs to Gitte Barlach

Jeg har reviewed, godkendt og merged Simons PR.

#78 Updated by Rolf Madsen over 2 years ago

@Simon, kan jeg overtale dig til at oprette den sag du foreslår med en opsummering det der mangler?

På forhånd tak!!

#79 Updated by Rolf Madsen over 2 years ago

  • Assignee changed from Gitte Barlach to Simon Holt

#80 Updated by Simon Holt about 2 years ago

@Rolf Det står meget højt på min liste. Kigger på det så snart jeg får lidt tid!

#81 Updated by Christel Krabbenhøft about 2 years ago

  • Assignee changed from Simon Holt to Steen Larsen

#82 Updated by Christel Krabbenhøft about 2 years ago

  • Assignee changed from Steen Larsen to Simon Holt

#83 Updated by Simon Holt about 2 years ago

Jeg har testet på nogle ret store lånestatus lister på hhv. 49, 79 og 263 lån.

Alle faust blev oversat korrekt på alle tre lister (se vedhæftede filer).

Verificerede også at materialer på listerne blev vist korrekt (link titel, metatdata osv..) i det tilfælde hvor det var en påhængspost/lokalpost.

Det var heller ikke et eneste materiale, der manglede titel og oplysninger.

Så hermed testet og godkendt.

 

#84 Updated by Simon Holt about 2 years ago

  • Assignee changed from Simon Holt to Rolf Madsen

Kan ikke sætte til resolved, så assigner den bare lige til dig, Rolf

#85 Updated by Rolf Madsen about 2 years ago

  • Status changed from Technical test to Resolved (tag version)

Fremragende, og tak for det Simon!! :-)

Also available in: Atom PDF