Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fields filteren die voorkomen als attribuut en/of als _link onnodig complex #44

Closed
strijm opened this issue Jun 10, 2020 · 3 comments
Closed

Comments

@strijm
Copy link
Collaborator

strijm commented Jun 10, 2020

In de fields.feature beschrijving staat dat er gefilterd moet kunnen worden met de fields parameter bv. op:

  1. attribuut pand en/of
  2. _links.pand
    Hiermee kunnen functioneel gezien één van beide of beide parameters worden opgevraagd, dit is conform NL API Strategie Designrules Extension hoofdstuk 7 en 17.15 en compleet en eenduidig.
    De functionaliteit zoals beschreven bij 1 en 2 is functioneel correct, compleet en eenduidig.

Daarnaast is er een constructie beschreven waarbij met bv. fields=pand het volgende opgevraagd kan worden:

  1. het pand attribuut van de resource als dit attribuut bestaat of
  2. _links.pand als er geen pand attribuut in de resource bestaat
    In dit geval werkt pand als een soort alias voor alles in een resource.
    Functioneel gezien voegen 3 en 4 helemaal niets toe aan de fields functionaliteit die bij 1 en 2 zijn beschreven, omdat met 1 en 2 alle parameters al opgevraagd kunnen worden, die is compleet en duidelijk.
    De constructie waarbij met een soort alias wordt gezocht op attributen of _links komt m.i. niet overeen met wat er in de NL API strategie Designrules Extensions hoofdstuk 7 en 17.15 staat:

17.15 API-30: Use query parameters corresponding to the queryable fields
Use unique query parameters that correspond to the fields that can be queried.

Ik maak hieruit op dat de attributen die bij fields worden gebruikt de naam van een attribuut van de resource moet zijn (zonder vermelding van de resource, in dit geval pand) of van een attribuut van een field of subresource (met vermelding van het attribuut/de resource geschieden door '.', in dit geval _links.pand).
Het gebruik van aliassen is hiermee strijdig omdat die het attribuut van een (sub)resource niet uniek identificeert.
Deze constructie maakt de implementatie aan de provider kant tevens onnodig complex en foutgevoelig.
Mijn voorstel en advies is dan ook om de functionaliteit zoals beschreven bij 3 en 4 te laten vervallen uit de fields.feature beschrijving.

@fsamwel
Copy link
Collaborator

fsamwel commented Jun 25, 2020

Deze paragraaf uit de NL API strategie heeft als status "This extension is in development and may be modified at any time." en is sinds het schrijven van de feature hierover (die oorspronkelijk geschreven is voor de haal centraal BRP API en daarna nog uitgebreid voor de haal centraal BRK API).

Het principe uit de API strategie ondersteunen we ook, want fields=_links.woonplaats moet ook werken.

Daar bovenop is in de genoemde eerdere projecten besproken dat het vriendelijk is voor gebruikers om ook te ondersteunen links te vragen zonder de toevoeging _links. Dit is immers geen onderdeel van de contentstructuur, maar alleen een wrapper voor json-HAL. Dit is denk ik niet in strijd met de API strategie, maar een gebruikersvriendelijke toevoeging die volgen van de API strategie werkwijze niet hindert.
Zie ook VNG-Realisatie/Haal-Centraal-BRK-bevragen#418 over dit onderwerp. Voor BRK is besloten dit principe door te trekken naar elke property in een groep. Dus bijvoorbeeld fields=tenaamstelling.aantekeningen.begrenzing zou ook mogelijk moeten zijn via de veel kortere fields=begrenzing.

Open vraag in deze werkwijze is de situatie dat er een link (of property in een groep) is die met dezelfde naam ook als gegeven in de resource voorkomt. De aanduiding is dan niet uniek identificerend voor een property. Dit is mogelijk in de BAG resource adressen met property woonplaats en _links.woonplaats. Er is nog niet besloten/vastgelegd hoe hiermee moet worden omgegaan. Mijn voorstel zou in dat geval zijn om beide te leveren.

Deze constructie is puur beschreven vanuit wat wij denken dat het meest vriendelijk en duidelijk is voor gebruikers. Hierbij is dus weinig/niet rekening gehouden met de complexiteit voor de provider. @strijm geef jij hiermee aan dat dit niet goed te implementeren is aan providerkant?

@strijm
Copy link
Collaborator Author

strijm commented Jun 25, 2020

Ik weet niet welke paragraaf je bedoelt met 'Deze paragraaf...', maar paragraaf 17.15 is een API principe, waar de rest van de API Strategie op is gebaseerd. Ik blijf erbij dat dit leidend moet zijn en vind echt dat de gewenste constructie daar strijdig mee is. Als een client bv. fields=woonplaats opgeeft, dan kan dat met de feature beschrijving 2 dingen betekenen:

  1. het attribuut woonplaats (de identificatie van een woonplaats) of
  2. de relatie met de woonplaats (_links.woonplaats)

Dan is fields=woonplaats dus geen unieke identificatie van het attribuut of de relatie in de resource (de velden die gefilterd kunnen worden met fields param).
Ik geef hiermee aan, dat ik denk dat het functioneel niets toevoegt en dit dermate complex is, dat het veel tijd gaat kosten om dit goed geïmplementeerd te krijgen. Ik vraag me af of dat de investering waard is, maar die afweging kan alleen HC zelf maken. Mijn advies is dan ook om dit niet of in ieder geval nu nog niet te (laten) implementeren. Het is volgens mij mogelijk dit evt. in een latere release te implementeren zonder een breaking change te veroorzaken. Dat biedt de ruimte om een afweging te maken of deze gewenste wijziging de investering waard is.

@strijm
Copy link
Collaborator Author

strijm commented Jul 1, 2020

Nadere discussie noodzakelijk na livegang.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants