feature voor paginering

[fsamwel]

feature die de werking beschrijft van paginering met de parameters page en pageSize

[strijm]

Er wordt geen page parameter meegegeven door de client, maar hij staat hier wel in de _links. Dat is niet erg, want ik wilde voorstellen altijd de page en pageSize als param in de href op te nemen. Als een client een waarde heeft opgegeven, dan wordt die gebruikt en anders gebruikt de provider de default. Dat is voor de clients en providers makkelijker.

[fsamwel]

ik begrijp niet zo goed wat je bedoelt dat page in links wordt meegegeven. Deze zit onderdaad in de next link, maar zit niet in de self link.
In de next is die in dit voorbeeld nodig, om op de juiste pagina te komen.
In de scenario's waarin ook een first link is opgenomen, is ook page opgenomen. Die was daar niet nodig geweest. Maakt mij niet uit wat je hier doet. Ik vindt de page=1 dan wel duidelijk/expliciet, maar het is technisch overbodig.

[strijm]

@fsamwel OK, misschien had ik hem even concreter moeten maken. Bij deze alsnog.
Ik denk dat page en pageSize bij alle pagineer links (_links.first.href, _links.prev.href (indien aanwezig), _links,next.href (indien aanwezig) en evt. _links.last.href) ingevuld zouden moeten worden. Als de client een page en/of pageSize heeft ingevuld in een request, dan zet de provider deze in de bovenstaande links en anders vult de provider de default waarden in. Als dit altijd op dezelfde manier wordt gedaan, dan is dat voor zowel de client als provider leveranciers eenvoudiger te implementeren en testen.

[fsamwel]

@MelvLee hoe zie jij dit? Wil je bij pagineerlinks altijd de pageSize opgenomen zien, ook wanneer deze niet door de gebruiker was ingevuld en dus de defaultwaarde heeft?

[MelvLee]

Ik zou er persoonlijk niet voor kiezen om bij default waarden de page en pagesize toe te voegen aan de url. Ik vind het dan niet consistent met hoe er met andere query parameters worden omgegaan. Als je de default waarde invult voor deze parameters, dan zou je het ook moeten doen voor fields, expand, etc en dus krijgt je fields=""&expand="" etc.

[strijm]

Hoe weten clients in dit geval om welke pagina het hier gaat? _links.self is er wel maar zonder page en pageSize, _links.first is er niet, _links.prev is er niet en _links.next is er wel. Dat betekent dat een client dus niet uit de self link kan bepalen om welk pagina het hier gaat en dan uit het feit dat er geen _links.first of _links.prev en wel een _links.next moet halen dat het om de eerste pagina gaat? Wat een omslachtige manier van werken. Ik ben er echt voor om gewoon in de self page en pageSize op te nemen, dan is er één gegeven waar een client altijd aan kan zien om welke pagina het gaat. De andere gegevens kunnen dan gebruikt worden om of helemaal naar het begin of helemaal naar het einde van een result set te gaan (altijd) of één pagina terug of verder te gaan (indien aanwezig), supersimpel!
Ik vind het argument dat dit consistent moet zijn met het gedrag van fields en expand niet helemaal kloppen. Ik ben het er wel mee eens dat als deze parameters een default zouden hebben, deze ook gebruikt moeten worden, omdat dat eigenlijk impliciet is wat er door een client is opgegeven, maar fields en expand hebben nu geen default in de specificatie, dus is het gedrag per definitie anders dan van parameters die wel een default hebben.

Als een result set leeg is, kun je erover twisten of er wel of geen pagina is. Geen pagina omdat er geen resultaten zijn of is er één pagina zonder resultaten. M.i. de laatste.
En natuurlijk geven we geen prev en next links terug als een result set leeg is. Prev en next links kunnen immers niet ingevuld worden omdat er geen (andere) pagina's zijn met resultaten. Als er geen pagina is zonder resultaten, dan kunnen ook first en last links niet worden geretourneerd. Als er wel één pagina zonder resultaten is, dan zijn de eerste (en dus ook de laatste pagina) hetzelfde (net als self overigens) en bevatten geen resultaten. We kunnen dus wel altijd een first, last en self link invullen. Dat die soms hetzelfde zijn lijkt me geen probleem, als clients altijd een self link krijgen met page en pageSize weten ze altijd waar ze zijn en met de links kunnen ze bepalen welke mogelijkheden ze hebben qua navigatie.

[strijm]

Een next kan alleen worden opgegeven zolang het niet de laatste pagina is. Maar hoe weet de client of het de laatste pagina is? Ik neem aan als er geen next meer in het result zit?
Is het niet handiger om ook een last link te hebben en een count en limt parameter in de HalPaginationLink?

[fsamwel]

voor de gebruiker kan een last link inderdaad handig zijn. Dit hebben we initieel niet opgenomen, omdat dit zou betekenen dat je elke keer een query moet doen over je hele tabel, met minimaal een count om te berekenen hoeveel pagina's er zijn. Dat geeft (mogelijk aanzienlijke) extra belasting. Wanneer dat voor de BAG geen probleem is, dus geen performance problemen zal geven, nemen we die graag op. Kunnen we die opnemen?

[fsamwel]

@strijm kan je aangeven of ik inderdaad een last kan toevoegen? M.a.w. kunnen jullie dat leveren (i.v.m. de systeembelasting die dat kan leveren)?

[strijm]

Dit is qua performance geen probleem omdat we toch al de complete result set hebben bepaald.
De properties count en total en de headers zoals in één van mijn andere opmerkingen hieronder kunnen we dan ook gewoon vullen.

[strijm]

Deze opmerking gaat wellicht breder dan alleen deze feature beschrijving, maar wat ik mis in het pagination stuk (feature en yaml spec), is het gebruik van _links.last voor de laatste pagina en het gebruik van twee attributen in de HalPaginationLinks structuur nl. count en total. Waarbij count het totaal aantal pagina's is en total het totaal aantal resultaten. Ook het gebruik van de pagination headers X-Total-Count voor het totaal aantal resultaten en X-Pagination-Count voor het totaal aantal pagina's mis ik. De 'last' link en X-Total-Count en X-Pagination-Count staan wel benoemd in de NL API Strategie (hoofdstuk 12 Paging in de versie van 17 januari 2020), maar ik weet niet wat de reden is om deze hier weg te laten.
De provider heeft de query om de resultaten te bepalen al uitgevoerd en het totaal aantal resultaten is dus bekend (ook het totaal aantal pagina's is dan snel bepaald), om performance redenen hoeven deze gegevens dus niet te worden weggelaten.

[fsamwel]

zie eerdere opmerking. Wanneer het geen probleem is voor de BAG API nemen we die graag op. De last en count zijn volgens API strategie geen properties in de HAL pagination links, maar response headers: https://geonovum.github.io/KP-APIs/API-strategie-extensies/#paging.

[MelvLee]

Er is wat tekst dubbel

[strijm]

Ik ben het nog niet helemaal eens met de manier waarop er met de links wordt omgegaan en zou graag zien dat dit wordt veranderd (zie mijn argumentatie in de opmerkingen).

[strijm]

Dit is qua performance geen probleem omdat we toch al de complete result set hebben bepaald.
De properties count en total en de headers zoals in één van mijn andere opmerkingen hieronder kunnen we dan ook gewoon vullen.

[strijm]

_links.self ontbreekt hier nog.

[fsamwel]

opgelost via #46 in $47

[strijm]

_links.self ontbreekt hier nog.

[fsamwel]

opgelost via #46 in $47

[strijm]

_links.self ontbreekt hier nog.

[fsamwel]

opgelost via #46 in $47

[strijm]

Hoe weten clients in dit geval om welke pagina het hier gaat? _links.self is er wel maar zonder page en pageSize, _links.first is er niet, _links.prev is er niet en _links.next is er wel. Dat betekent dat een client dus niet uit de self link kan bepalen om welk pagina het hier gaat en dan uit het feit dat er geen _links.first of _links.prev en wel een _links.next moet halen dat het om de eerste pagina gaat? Wat een omslachtige manier van werken. Ik ben er echt voor om gewoon in de self page en pageSize op te nemen, dan is er één gegeven waar een client altijd aan kan zien om welke pagina het gaat. De andere gegevens kunnen dan gebruikt worden om of helemaal naar het begin of helemaal naar het einde van een result set te gaan (altijd) of één pagina terug of verder te gaan (indien aanwezig), supersimpel!
Ik vind het argument dat dit consistent moet zijn met het gedrag van fields en expand niet helemaal kloppen. Ik ben het er wel mee eens dat als deze parameters een default zouden hebben, deze ook gebruikt moeten worden, omdat dat eigenlijk impliciet is wat er door een client is opgegeven, maar fields en expand hebben nu geen default in de specificatie, dus is het gedrag per definitie anders dan van parameters die wel een default hebben.

Als een result set leeg is, kun je erover twisten of er wel of geen pagina is. Geen pagina omdat er geen resultaten zijn of is er één pagina zonder resultaten. M.i. de laatste.
En natuurlijk geven we geen prev en next links terug als een result set leeg is. Prev en next links kunnen immers niet ingevuld worden omdat er geen (andere) pagina's zijn met resultaten. Als er geen pagina is zonder resultaten, dan kunnen ook first en last links niet worden geretourneerd. Als er wel één pagina zonder resultaten is, dan zijn de eerste (en dus ook de laatste pagina) hetzelfde (net als self overigens) en bevatten geen resultaten. We kunnen dus wel altijd een first, last en self link invullen. Dat die soms hetzelfde zijn lijkt me geen probleem, als clients altijd een self link krijgen met page en pageSize weten ze altijd waar ze zijn en met de links kunnen ze bepalen welke mogelijkheden ze hebben qua navigatie.

[strijm]

... wordt een foutmelding getoond.
Ik neem aan een 404? Wat wordt de foutmelding: Pagina niet gevonden / Page not found? Of gewoon Not found?

[fsamwel]

Waarom een 404? In de feature staat dat een 400 fout moet worden gegeven. Er is immers een incorrecte waarde opgegeven voor een query parameter.

[MelvLee]

Ik zou de self link niet wijzigen omdat dit een heel ander doel heeft dan navigatie, maar ik begreep uit een eerdere reactie dat dit het gedrag is van een gebruikte framework. Mijn voorstel is daarom om dan de page en pageSize parameter verplicht te maken omdat deze door de framework toch in alle paging links worden toegevoegd.

Verder vind ik het prima om de first en prev links altijd toe te voegen.

[fsamwel]

Mijn voorstel is daarom om dan de page en pageSize parameter verplicht te maken omdat deze door de framework toch in alle paging links worden toegevoegd.

We hebben deze specificaties nu in de Haal-Centraal-common zitten. Dat betekent dat deze specificaties ook gelden voor andere API's die paginering gebruiken, zoals voor het Handelsregister.
Ik ben er niet voor om ook voor bijvoorbeeld het HR dingen gedrag te beschrijven (verplichten) omdat voor het BAG een framework is gebruikt die blijkbaar niet voldoende configureerbaar is om het eigenlijk gewenste gedrag te vertonen.

Dan moeten we de paginering (parameters) specifiek in de BAG specs opnemen en niet hergebruiken van de common.

Ook zou ik het gek vinden dat een request "/adressen?zoekresultaatidentificatie=adr-5c01945883b0f0390d7a52f8462b0686" een foutmelding zou geven, omdat geen page en pageSize is opgegeven. Bij deze vraag zal er immers maar één adres terugkomen.

Wat bedoel je met "Verder vind ik het prima om de first en prev links altijd toe te voegen."?
Dat je ook wanneer er maar één resultaat is (of zelfs geen resultaat) toch first en previous links worden geleverd? Waarom?

[fsamwel]

@strijm en @MelvLee aangezien deze pull request is gesloten en hier dus een ingewikkelde discussie over is, heb ik hiervoor een apart issue #48 gemaakt. Daarin kunnen we de discussie dan beter doorvoeren en de conclusies daaruit dan t.z.t. in een pull request doorvoeren.