VNG-Realisatie / Haal-Centraal-common

Project repository tbv de ontwikkeling van de over alle Haal Centraal API specificaties gedeelde specificaties
Other
2 stars 7 forks source link

Parameters voldoen niet aan DD1.5 #116

Closed melsk-r closed 2 months ago

melsk-r commented 3 years ago

In de common.yaml staan een aantal componenten voor parameters gedefinieerd. Een aantal daarvan voldoet qua 'name' property niet aan DD1.5 waarin wordt gesteld dat namen van endpoints, url's en parameters alleen kleine letters mogen bevatten. Het gaat om de componenten:

Overigens is DD1.5 denk ik niet helemaal correct aangezien ik vermoed dat endpoints, url's en parameters ook tekens zoals bijv. '-' mogen bevatten.

JohanBoer commented 3 years ago

Een keertje agenderen op een werkoverleg hoe hier mee om te gaan.

michielverhoef commented 3 years ago

Ik ben wel voorstander van alles altijd in lowercase houden. Dit omdat niet alle webservers hetzelfde omgaan met case. Apache is bijvoorbeeld case-sensitive, IIS is dat niet.

Een voorbeeld van github pages: https://vng-realisatie.github.io/StUF-Standaarden/ werkt wel. https://vng-realisatie.github.io/stuf-standaarden/ geeft een 404.

Wanneer niet vantevoren duidelijk is of en hoe een webserver omgaat met case-sesitivity is het beter het zekere voor het onzekere te nemen en vanuit de standaard altijd lower-case voor te schrijven. Dan is het gedrag, ongeacht de implementatie, altijd voorspelbaar.

fsamwel commented 3 years ago

@michielverhoef hoe is het feit dat een webserver case sensitive is een reden geen casing te gebruiken?

Je haalt je een heleboel problemen op de hals wanneer je helemaal geen hoofdletters in de url wilt hebben. Dan mag je namelijk ook geen hoofdletters in parameterwaarden hebben:

Wie verwacht dat gebruikers dat snappen?

ik kan op internet veel vinden over case sensitive vs case insensitive webservers, en de issues die dat kan geven op webpagina's. De issues die ik daar vind gaan allemaal over Uri's die helemaal identiek zijn behalve de casing, en dan naar een andere resource verwijzen (behalve bij case-insensitive servers waar het naar dezelfde resource verwijst).

Wat we m.i. dus moeten voorkomen is dat we parameters krijgen die volledig identiek zijn behalve casing. Dat geeft inderdaad onvoorspelbare resultaten (dus als voornaam=michiel iets anders doet dan voorNaam=michiel), maar dat is om meerdere andere redenen een slecht idee (begrijp een gebruiker het verschil tussen voornaam en voorNaam?).

michielverhoef commented 3 years ago

Wat we m.i. dus moeten voorkomen is dat we parameters krijgen die volledig identiek zijn behalve casing

Helemaal mee eens, Dus door altijd lowercase voor parameternamen te gebruiken in parameters voorkom je al veel onduidelijkheid en problemen.

fsamwel commented 3 years ago

Helemaal mee eens, Dus door altijd lowercase voor parameternamen te gebruiken in parameters voorkom je al veel onduidelijkheid en problemen.

Voorkom dat een fietser op je voorbumper botst door altijd achteruit te rijden. Het is een oplossing, of toch niet?

Je vervangt m.i. een risico door een groter risico

fsamwel commented 3 years ago
parameter naam request url resultaat op case sensitive server resultaat op case insensitive server
pandIdentificatie /adressen?pandIdentificatie=1234 levert adressen in pand 1234 levert adressen in pand 1234
pandIdentificatie /adressen?pandidentificatie=1234 levert foutmelding parameter bestaat niet levert adressen in pand 1234
pand_identificatie /adressen?pand_identificatie=1234 levert adressen in pand 1234 levert adressen in pand 1234
pand_identificatie /adressen?pand_Identificatie=1234 levert foutmelding parameter bestaat niet levert adressen in pand 1234

wat heb je precies gewonnen door het gebruik van kleine letters in je parameternamen?

MelvLee commented 3 years ago

Ik weet niet of ik het probleem goed begrijp, maar de url en query parameter namen lowercase maken betekent niet dat consumers dan geen casing meer kunnen/gaan toepassen. Als ze dat doen gaat het nog steeds fout op servers die case sensitive urls ondersteunen.

Wat volgens mij verloren gaat in deze discussie is de reden om name casing toe te passen, nl. het verhogen van leesbaarheid. De volgende path template is volgens mij heel moeilijk te lezen als je geen vorm van name casing toepast:

/kadastraalonroerendezaken/{kadastraalonroerendezaakidentificatie}/zakelijkgerechtigden/{zakelijkgerechtigdeidentificatie}

Als er behoefte is om name casing te standaardiseren, kies dan het liefst voor eentje die al bestaat. In jullie geval zou kebab casing volgens mij het beste passen.

michielverhoef commented 3 years ago

Je vervangt m.i. een risico door een groter risico

Wat is dan precies het grotere risico?

Ik weet niet of ik het probleem goed begrijp, maar de url en query parameter namen lowercase maken betekent niet dat consumers dan geen casing meer kunnen/gaan toepassen.

De standaard bepaalt hoe de url en paramters er uit zien. Dus wanneer een consumer iets anders doet (wel casing toepassen) volgt deze niet de standaard.

Je haalt je een heleboel problemen op de hals wanneer je helemaal geen hoofdletters in de url wilt hebben. Dan mag je namelijk ook geen hoofdletters in parameterwaarden hebben:

Het afvangen van de correcte waarden van parameters is echt wat anders dan de case van parameternamen. Dat zou logica moeten zijn die altijd toegepast wordt, ongeacht of je een REST/JSON api, StUF koppeling of bestandsuitwisseling gebruikt.

michielverhoef commented 3 years ago
parameter naam request url resultaat op case sensitive server resultaat op case insensitive server
pandIdentificatie /adressen?pandIdentificatie=1234 levert adressen in pand 1234 levert adressen in pand 1234
pandIdentificatie /adressen?pandidentificatie=1234 levert foutmelding parameter bestaat niet levert adressen in pand 1234
pand_identificatie /adressen?pand_identificatie=1234 levert adressen in pand 1234 levert adressen in pand 1234
pand_identificatie /adressen?pand_Identificatie=1234 levert foutmelding parameter bestaat niet levert adressen in pand 1234

wat heb je precies gewonnen door het gebruik van kleine letters in je parameternamen?

Mijns inziens klopt je voorbeeld niet. Het is een pleidooi vboor het gebruik van een case-insensitive webserver, geen pleidooi om case te gebruiken.

In je voorbeeld ga je er van uit dat parameternamen case hebben. Dus gaat het in regel 2 en regel 4 mis. Door altijd alle parameternamen in lowercase te maken en dit in de standaard te bepalen weet de consumer altijd de case van parameters. gebruik je wat anders volg je niet de standaard. Mag je doen maar dan moet je niet raar staan te kijken dat iets niet werkt. Dan doet die consumer iets verkeerd.

Waar het uiteindelijk op neer komt is dat implementaties de standaard moeten volgen en ondersteunen. Op het moment dat een parameter anders geschreven wordt dan in de standaard staat zeg je iets anders. Dat heeft niets te maken met case sensitive of niet.

Het grote voordeel van een regel dat alle parameters lower case zijn is dat duidelijk is hoe je de naam van die parameter schrijft. Dus niet meer per ongeluk InformatieObject waar het Informatieobject had moeten zijn etc. omdat je als ontwikkelaar gewend bent het woord object met een hoofdletter te schrijven.

(begrijp een gebruiker het verschil tussen voornaam en voorNaam?).

Hier zullen best problemen mee zijn, zeker bij ontwikkelaars die de Nederlandse taal niet machtig zijn. Maar om alle mogelijke verwarring voor te zijn is een keuze om alle parameternamen altijd lower case te schrijven helemaal niet zo'n slechte. Wajnt hoe leg ik een Roemeense of Indiase ontwikkelaar uit wat een los woord is in Nederland en wanneer je wel of geen hoofdletter in een parameternaam gebruikt? Dat is nog moeilijker dan zeggen: alle parameternamen zijn lower case.

melsk-r commented 3 years ago

Ik heb nog steeds moeite de discussie te volgen en de argumenten van iedereen op waarde te schatten.

Je vervangt m.i. een risico door een groter risico

Wat is dan precies het grotere risico?

Daar ben ik ook benieuwd naar.

In je voorbeeld ga je er van uit dat parameternamen case hebben. Dus gaat het in regel 2 en regel 4 mis. Door altijd alle parameternamen in lowercase te maken en dit in de standaard te bepalen weet de consumer altijd de case van parameters. gebruik je wat anders volg je niet de standaard. Mag je doen maar dan moet je niet raar staan te kijken dat iets niet werkt. Dan doet die consumer iets verkeerd.

Op case sensitive servers heb je sowieso altijd een kans dat het fout gaat of je de parameters nu lower-, upper-, kebab- of een andere case vorm gebruikt. Volgens mij moeten developers op het casing aspect van parameters vooral niet zelf gaan nadenken maar gewoon kijken hoe het in de OAS3 specs staat gedefinieerd. Juist als ze dat niet doen gaat n.m.m. je opmerking op:

Mag je doen maar dan moet je niet raar staan te kijken dat iets niet werkt. Dan doet die consumer iets verkeerd.

En dat beaam je eigenlijk zelf:

Waar het uiteindelijk op neer komt is dat implementaties de standaard moeten volgen en ondersteunen. Op het moment dat een parameter anders geschreven wordt dan in de standaard staat zeg je iets anders. Dat heeft niets te maken met case sensitive of niet.

Ze moeten dus gewoon volgen wat er in de OAS3 specs staat.

Het grote voordeel van een regel dat alle parameters lower case zijn is dat duidelijk is hoe je de naam van die parameter schrijft. Dus niet meer per ongeluk InformatieObject waar het Informatieobject had moeten zijn etc. omdat je als ontwikkelaar gewend bent het woord object met een hoofdletter te schrijven.

Dit krijg je dus juist alleen als een developer op dit aspect zelf na gaat denken.

(begrijp een gebruiker het verschil tussen voornaam en voorNaam?).

Hier zullen best problemen mee zijn, zeker bij ontwikkelaars die de Nederlandse taal niet machtig zijn. Maar om alle mogelijke verwarring voor te zijn is een keuze om alle parameternamen altijd lower case te schrijven helemaal niet zo'n slechte. Wajnt hoe leg ik een Roemeense of Indiase ontwikkelaar uit wat een los woord is in Nederland en wanneer je wel of geen hoofdletter in een parameternaam gebruikt? Dat is nog moeilijker dan zeggen: alle parameternamen zijn lower case.

Voor een buitenlandse developer geldt het devies om op dit aspect niet na te gaan denken nog eens dubbel zo hard. Op dit punt gewoon doen wat er in de specs staat dan kan het niet fout gaan.

Ik heb dus zelf het gevoel dat het wel of niet gebruiken van upper-, lower, kebap, of andere soort case op technisch vlak niet zo heel veel uitmaakt en dat voor het toepassen van casing andere argumenten belangrijker zijn, bijv. dat:

Maar zoals bij aanvang van deze post al gezegd, ik vind het nog steeds lastig om alle argumenten op waarde te schatten. Wellicht begrijp ik e.e.a. gewoon nog niet.

michielverhoef commented 3 years ago

Ik heb dus zelf het gevoel dat het wel of niet gebruiken van upper-, lower, kebap, of andere soort case op technisch vlak niet zo heel veel uitmaakt en dat voor het toepassen van casing andere argumenten belangrijker zijn, bijv. dat:

Helemaal mee eens. De belangrijkste reden dat ik pleit voor alles lowercase is dan ook dat dit tamelijk idiot-proof is.

Het mogelijke probleem met case-sensitive/case-insensitive servers zie ik voornamelijk wanneer een ontwikkelaar gewend is case-insensitive te kunnen werken (alles werkt toch wel) en er blijkt ineens een verschil te zitten tussen de implementatie en de standaard. Dit zal eerder bij leveranciers van consumers optreden die een eigen testomgeving hebben die niet case-sensitive is.

CathyDingemanse commented 3 years ago

Ik weet niet of ik het probleem goed begrijp, maar de url en query parameter namen lowercase maken betekent niet dat consumers dan geen casing meer kunnen/gaan toepassen. Als ze dat doen gaat het nog steeds fout op servers die case sensitive urls ondersteunen.

Wat volgens mij verloren gaat in deze discussie is de reden om name casing toe te passen, nl. het verhogen van leesbaarheid. De volgende path template is volgens mij heel moeilijk te lezen als je geen vorm van name casing toepast:

/kadastraalonroerendezaken/{kadastraalonroerendezaakidentificatie}/zakelijkgerechtigden/{zakelijkgerechtigdeidentificatie}

Als er behoefte is om name casing te standaardiseren, kies dan het liefst voor eentje die al bestaat. In jullie geval zou kebab casing volgens mij het beste passen.

Heren, kunnen jullie a.u.b. reageren op de opmerkingen van jullie klant/customer zero en mijn technische rechterhand inzake de vraagarticulatie?

michielverhoef commented 3 years ago

Ik had al eens gereageerd op een deel van de vraag:

Ik weet niet of ik het probleem goed begrijp, maar de url en query parameter namen lowercase maken betekent niet dat consumers dan geen casing meer kunnen/gaan toepassen.

De standaard bepaalt hoe de url en paramters er uit zien. Dus wanneer een consumer iets anders doet (wel casing toepassen) volgt deze niet de standaard.

Mijn pleidooi om parameters lower case te houden is ook gebaseerd op ervaringen dat niet iedereen op dezelfde manier case toepast. Binnen modellen als RSGB en RGBZ worden hoofdletters gebruikt wanneer het losse woorden betreft. Nederlands en bijvoorbeeld Engels verschillen hierin. Niet-Nederlandstalige ontwikkelaars (of soms ook Nederlandstalige ontwikkelaars die in het Engels denken en werken) zullen niet altijd precies weten of een parameternaam uit één of twee worden bestaat.

Toegegeven, dan moet je maar in de standaard kijken. Maar het helpt ook wanneer iets eenvoudig en voorspelbaar is. Door alles lowercase te maken ben je dat probleem kwijt.

Wat volgens mij verloren gaat in deze discussie is de reden om name casing toe te passen, nl. het verhogen van leesbaarheid. De volgende path template is volgens mij heel moeilijk te lezen als je geen vorm van name casing toepast:

/kadastraalonroerendezaken/{kadastraalonroerendezaakidentificatie}/zakelijkgerechtigden/{zakelijkgerechtigdeidentificatie}

Computers zullen hier geen moeite mee hebben denk ik? Dit is een API voor machine to machine communicatie, geen userinterface voor een menselijke gebruiker. Of zijn deze template bedoeld voor menselijke gebruikers?

MelvLee commented 3 years ago

Ik had al eens gereageerd op een deel van de vraag:

Ik weet niet of ik het probleem goed begrijp, maar de url en query parameter namen lowercase maken betekent niet dat consumers dan geen casing meer kunnen/gaan toepassen.

De standaard bepaalt hoe de url en paramters er uit zien. Dus wanneer een consumer iets anders doet (wel casing toepassen) volgt deze niet de standaard.

Mijn pleidooi om parameters lower case te houden is ook gebaseerd op ervaringen dat niet iedereen op dezelfde manier case toepast. Binnen modellen als RSGB en RGBZ worden hoofdletters gebruikt wanneer het losse woorden betreft. Nederlands en bijvoorbeeld Engels verschillen hierin. Niet-Nederlandstalige ontwikkelaars (of soms ook Nederlandstalige ontwikkelaars die in het Engels denken en werken) zullen niet altijd precies weten of een parameternaam uit één of twee worden bestaat.

Toegegeven, dan moet je maar in de standaard kijken. Maar het helpt ook wanneer iets eenvoudig en voorspelbaar is. Door alles lowercase te maken ben je dat probleem kwijt.

OK. Kan je dan aangeven waarom kebab-case geen goede keuze is? Zoals aangegeven zit dit het dichts bij wat jullie willen.

Wat volgens mij verloren gaat in deze discussie is de reden om name casing toe te passen, nl. het verhogen van leesbaarheid. De volgende path template is volgens mij heel moeilijk te lezen als je geen vorm van name casing toepast: /kadastraalonroerendezaken/{kadastraalonroerendezaakidentificatie}/zakelijkgerechtigden/{zakelijkgerechtigdeidentificatie}

Computers zullen hier geen moeite mee hebben denk ik? Dit is een API voor machine to machine communicatie, geen userinterface voor een menselijke gebruiker. Of zijn deze template bedoeld voor menselijke gebruikers?

Klopt. Maar de code die wordt geschreven gebeurt door mensen en wordt ook door mensen gelezen. Dat is de reden waarom developers casing gebruiken. Niet om een case-sensitive server case-insensitive te maken.

michielverhoef commented 3 years ago

Klopt. Maar de code die wordt geschreven gebeurt door mensen en wordt ook door mensen gelezen. Dat is de reden waarom developers casing gebruiken. Niet om een case-sensitive server case-insensitive te maken.

Mensen maken fouten en zoals ik al eerder aangaf, de caseregels zijn niet voor iedereen automatisch gelijk. Wanneer je je aan de standaard houdt is er geen enkel probleem.

Er gaan pas dingen mis wanneer verkeerd casegebruik niet gemerkt wordt omdat de omgeving niet-casesenistive is en bij het koppelen aan een server gekoppeld wordt die wel case-sensitive is.

Kebabcase is al een stuk overzichtelijker maar ook dan is het van belang te weten waar de spaties zitten zodat je weet waar de hyphens komen. Het grote voordeel is dat "informatie-object" een totaal andere string is als "informatieobject". Fouten ongeacht wel/niet casesensitive servers etc. zullen dan meteen naar voren komen.

Toegegeven, het valt of staat met het voldoen aan de standaard. In mijn ogen beperk je de kans op fouten rondom case door parameters altijd in lowercase te houden. Fouten die door verschillend gedrag van tooling komen zijn een stuk lastiger op te sporen dan echte typefouten. Dus zou gezien is kebabcase misschien wel de beste oplossing.

MelvLee commented 3 years ago

Ik kan het niet volgen. Is camel case, pascal case en kebab case geen standaard? Dat wordt internationaal gebruikt.

Als ik het begrijp wil je de url lowercase maken om fouten te minimaliseren. Moet je dan ook niet de schema componenten en hun properties ook allemaal lowercase maken? Daar kunnen ook fouten worden gemaakt.

Is het dan niet goedkoper en makkelijker om een API designer, die de Nederlandse taal wel machtig is, de API te laten designen, zodat de namen van de paths, components en properties conformeren aan de Nederlandse taal en de internationale standaarden? Want dan kunnen de developers gewoon code genereren die voldoen aan de specificatie. Dan is de kans op casing fouten naar mijn mening 0.

En als je de developers wil helpen die geen code genereren, dan kan je ook een API gateway voor de case-sensitive server plaatsen die de request corrigeert zodat het voldoet aan de spec.

melsk-r commented 3 years ago

Het Haal Centraal team is van mening dat de aangedragen argumenten geen reden zijn om Design Decision 1.5 toch te handhaven. DD1.5 zal dus uit de Design Decisions worden verwijderd.

michielverhoef commented 3 years ago

Het Haal Centraal team is van mening dat de aangedragen argumenten geen reden zijn om Design Decision 1.5 toch te handhaven. DD1.5 zal dus uit de Design Decisions worden verwijderd.

Welke Design Decisions worden hier bedoeld? Die van Haal Centraal of die voor alle API's van VNG Realisatie?

melsk-r commented 3 years ago

Welke Design Decisions worden hier bedoeld? Die van Haal Centraal of die voor alle API's van VNG Realisatie?

Die van Haal Centraal, bij VNG Realisatie gaat het om een Design Rule.