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

Fuzzy-search op adres restfull maken #74

Open
JohanBoer opened this issue Mar 30, 2021 · 7 comments
Open

Fuzzy-search op adres restfull maken #74

JohanBoer opened this issue Mar 30, 2021 · 7 comments
Assignees

Comments

@JohanBoer
Copy link
Collaborator

De huidige endpoint voor fuzzy search op adres in de BAG-bevragen API is uitgevoerd in een tweetraps-raket. Er wordt op basis van de fuzzy-searche een API-string opgeleverd en met die string kan de consumer het daadwerkelijke adres ophalen.

Bekijk in hoeverre de specificaties aangeapst moeten worden om in plaats van een array adresstrings een collectie van adressen op te halen zodat dit een restful endpoint wordt.

@JohanBoer JohanBoer self-assigned this Mar 30, 2021
@JohanBoer
Copy link
Collaborator Author

  • Parameter "zoek" wordt toegevoegd aan endpoint "/adressen"
  • Parameter "zoekresultaatIdentificatie" wordt deprecated bij endpoint "/adressen"
  • Endpoint "/adressen/zoek" wordt deprecated
  • Parameter-component "zoek" wordt required: false (Heeft non-breaking impact op deprecated endpoint "/adressen/zoek")
  • Schema-component "ZoekResultaat" wordt deprecated
  • Schema-component "ZoekResultaatHal" wordt deprecated
  • Schema-component "ZoekResultaat_links" wordt deprecated
  • Schema-component "ZoekResultaatHalCollectie" wordt deprecated

@fsamwel
Copy link
Collaborator

fsamwel commented Apr 1, 2021

Parameter-component "zoek" wordt required: false (Heeft non-breaking impact op deprecated endpoint "/adressen/zoek")

Dit betekent dat "zoek" ook niet meer required is bij /adressen/zoek? Waarom is dat nodig? Je hebt er nu "last" van (hoewel niet breaking) dat je de parameter in een component stopt en die hergebruikt, terwijl je voorheen de parameter maar één keer gebruikte en deze nu op verschillende plekken verschillend gebruikt.
In #364 was er namelijk ook de behoefte om op zoek een minLength en (vooral) een maxLength (van 255) te definiëren. Dat is toen niet gedaan omdat het breaking zou zijn. De API kan nu fouten op invoer geven terwijl die invoer wel voldoet aan de interface specificaties, dus optimaal is dat niet. Het verplaatsen van de zoekparameter naar /adressen geeft de kans dat alsnog goed te doen.

Dus mijn voorstel is om de ouder parameter, of de nieuwe parameter (of beide) te definiëren bij het endpoint, dan wel voor de nieuwe parameter ook een nieuwe parametercomponent te maken.

@MelvLee
Copy link
Collaborator

MelvLee commented Apr 1, 2021

Als een nieuwe parameter wordt gemaakt, dan stel ik voor om het dan ook een goede naam te geven. zoekTerm of iets in die geest

@fsamwel
Copy link
Collaborator

fsamwel commented Apr 1, 2021

De API design rules extensions noemt deze parameter "zoek" (niet normatief): "APIs support full-text searching using the query parameter zoek."

In BRK noemen we deze "q" (zoeken kadasterpersonen).

Is het dan handig hier een derde parameternaam (zoekTerm) voor te verzinnen?

Ik ben het trouwens wel met Melvin eens dat "zoekTerm" duidelijker is dan "zoek" of "q".

@MelvLee
Copy link
Collaborator

MelvLee commented Apr 1, 2021

q is de naam die door andere zoek engines wordt gebruikt. Zie bijv. https://www.elastic.co/guide/en/elasticsearch/reference/current/search-search.html. Mijn suggestie was toen om dan dezelfde benaming te gebruiken. Dus wat mij betreft mag q ook. Maar aangezien we nu als best practice hanteren om duidelijke namen te kiezen voor properties en parameters, is denk ik zoekTerm een betere benaming. Als we hiervoor kiezen, kunnen we "q" in kadasterpersonen deprecaten.

@JohanBoer
Copy link
Collaborator Author

Dit betekent dat "zoek" ook niet meer required is bij /adressen/zoek? Waarom is dat nodig? Je hebt er nu "last" van (hoewel niet breaking) dat je de parameter in een component stopt en die hergebruikt, terwijl je voorheen de parameter maar één keer gebruikte en deze nu op verschillende plekken verschillend gebruikt.
In #364 was er namelijk ook de behoefte om op zoek een minLength en (vooral) een maxLength (van 255) te definiëren. Dat is toen niet gedaan omdat het breaking zou zijn. De API kan nu fouten op invoer geven terwijl die invoer wel voldoet aan de interface specificaties, dus optimaal is dat niet. Het verplaatsen van de zoekparameter naar /adressen geeft de kans dat alsnog goed te doen.

Dus mijn voorstel is om de ouder parameter, of de nieuwe parameter (of beide) te definiëren bij het endpoint, dan wel voor de nieuwe parameter ook een nieuwe parametercomponent te maken

Als ik het goed heb zal een developer die een nieuwe consumer-implementatie doet niet kiezen voor de deprecated zoek-functie. Dat betekent dat alle bestaande implementaties gewoon de zoek-parameter zullen vullen voordat er een request gedaan wordt.
In feite wordt de component bij de eerstvolgende versie niet meer hergebruikt omdat dan de deprecated onderdelen verwijderd worden en deze parameter dus maar op 1 plek wordt gebruikt.

Ik heb de bestaande zoek-parameter hergebruikt om zo dicht mogelijk bij de huidige implementaties te blijven. Natuurlijk kunnen we nu minLength en maxLength toevoegen, maar aangezien #364 gesloten is had ik dat niet op mijn netvlies. \

Ten aanzien van het hernoemen van de "zoek"-parameter heb ik nergens een aanleiding voorbij zien komen om die naam nu aan te passen. De discussie over "q" of "zoek" hebben we in het ontwikkeltraject ook met het BAG-team ook gevoerd en daar is "zoek" uitgekomen.
Als we roepen dat we ons conformeren aan de landelijke API_strategie dan zie ik niet in waarom we van "zoek" nu "zoekTerm" moeten maken. Ik snap dat "zoek" niet normatief is, maar waarom zou je afwijken als dat geen toegevoegde waarde heeft.

Dat gezegd hebbende. Ik ben geen productowner. De wijzigingen zijn nu zo doorgevoerd om zo dicht mogelijk bij de huidige implementatie te blijven. Het toevoegen van maxLength en minLength likt mij een goed idee. Een andere naam voor de zoek-parameter vind ik geen goed idee.

@CathyDingemanse Kan/wil jij uitsluitsel geven of hebben we het hier maandag over ?

@JohanBoer
Copy link
Collaborator Author

We gaan q gebruiken en daar voegen we maxLength en minLength toegevoegd.

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

No branches or pull requests

3 participants