VNG-Realisatie / gemma-zaken

Samen ontwikkelen van API's voor Zaakgericht werken
https://vng-realisatie.github.io/gemma-zaken/
Other
41 stars 27 forks source link

Als developer/consumer wil ik uit kunnen gaan van valide waarden voor string-velden met een specificatie #2090

Open WilliamRoxit opened 2 years ago

WilliamRoxit commented 2 years ago

Er zijn verschillende varianten van string-properties:

  1. string($string)
  2. string($url)
  3. string($enum)
  4. string($date)
  5. string($duration)

a. Required icm Nullable Allereerst is het verwarrend dat sommige velden niet verplicht zijn, maar ook niet nullable. Het is niet duidelijk wat de usecase is voor deze configuratie. Mag de server zelf een default waarde bepalen? Logisch en eenduidig zou zijn: alle verplichte velden zijn niet-nullable, alle optionele velden zijn nullable.

b. Validatie van string($specificatie) velden Voor zowel nullable als niet-nullable velden is het validatiegedrag ook verschillend. Wat ik als developer/consumer verwacht: Voor een string($enum) veld wat nullable is, mag ik in mijn request OF null, OF een geldige enumeration waarde opgeven. Een random tekst of een lege string ("") zijn geen geldige enumeratie waarden en zou een HTTP 400 moeten opleveren. Voor een string($enum) veld wat niet nullable is, mag ik in mijn request alleen een geldige enumeration waarde opgeven. Een random tekst of een lege string ("") zijn geen geldige enumeratie waarden en zou een HTTP 400 moeten opleveren.

Ik heb wat testjes gedaan en het lijkt erop dat, in ieder geval in de Openzaak en daarmee waarschijnlijk ook de referentie implementatie, de formaat validaties op verschillende type velden anders gedaan worden. Zie daarvoor onderstaande tabel. Naar mijn mening zou een lege string nooit een valide waarde mogen zijn bij velden van type 2 t/m 5. Als een veld mogelijk leeg kan zijn, dan zou deze simpelweg als nullable gemarkeerd moeten worden. Ik verwacht dan in de response op een HTTP GET ook enkel valide waarden, of null terug en geen lege strings.

a. is in mijn ogen verwarrend in de standaard. b. lijkt mij een bug aangezien het gedrag inconsequent is voor verschillende velden.

image

michielverhoef commented 2 years ago

In ieder geval moet dit opgelost/voorkomen worden in de nieuwe versies van de API's.

Voor de attributen uit de bestaande versies moet dit ook opgelost worden maar voor de zekerheid eerst uitzoeken wat eventuele gevolgen (backwards compatibility etc.) zijn.

WilliamRoxit commented 1 year ago

@michielverhoef wat is de ETA van milestone 1.4? (ik zie dat dit issue is doorgeschoven). De 1.3 is al 5 maanden overdue; de 1.4 al twee maanden.

Dit issue is best wel een vervelende aangezien dit tot nu toe elke keer weer tot discussies leidt tussen consumers en producers. Het is geen issue als een consumer specifke op 1 producer gemaakt wordt, maar doordat er geen uitsluitsel is over hoe om te gaan met dit scenario is het vrijwel niet te doen om een consumer te bouwen die met meerdere implementaties van ZGW api's koppelt. Ook met het oog op de lifetime / support van een versie ben ik bang dat het nog minimaal een jaar gaat duren voordat een consumer er van 'het besluit van dit ticket' uit kan gaan. Aannemende dat alle ZGW leveranciers zich houden aan de api versie support lifecycle.

shazada commented 1 year ago

In ieder geval moet dit opgelost/voorkomen worden in de nieuwe versies van de API's.

Hoi @michielverhoef we hebben dit opnieuw bij de implementatie van de DRC 1.1 Bijv. image

inhoud | string or null (inhoud) Binaire inhoud, in base64 geëncodeerd. -- | --

Wij doen een validatie of een lege string van het type base64 is en in dit geval wordt dus een lege string verstuurd.

Wat doen we in zo'n situatie? M.a.w. dit wordt nu continu doorgeschoven en bij iedere implementaties komt dit weer naar boven. Graag hier met prio naar kijken.

michielverhoef commented 1 year ago

@shazada Helaas is de standaard in deze ambigu. In de OAS van de Documenten API staat dit over het veld inhoud (zie header https://documenten-api.vng.cloud/api/v1/schema/):

Het INFORMATIEOBJECT aanmaken in de API, waarbij de totale bestandsgrootte meegestuurd wordt en **de inhoud leeggelaten wordt**. De API antwoordt met een lijst van BESTANDSDEELen, elk met een volgnummer en bestandsgrootte. De API lockt tegelijkertijd het INFORMATIEOBJECT.

Een inhoud kan op twee manieren leeg gelaten worden:

Hoe vervelend ook, dat betekent in de praktijk dat de keuze bij de consumer ligt en de provider beide mogelijkheden zal moeten ondersteunen.

Overigens denk ik dat er nog andere dingen niet helemaal goed uitgewerkt zijn rondom het gebruik van bestandsdelen, zie #2257.

johannesbattjes commented 1 year ago

@michielverhoef dank voor je antwoord. Je citaat en de consequenties daarvan zijn mij duidelijk.

Wat minder duidelijk is: in de geciteerde tekst staat dit: Webservers moeten op deze endpoints een minimale request body size van 4.0 GB ondersteunen. .... Bestanden kunnen groter zijn dan de minimale die door providers ondersteund moet worden. De consumer moet dan:

Als je deze tekst letterlijk neemt lijkt het alsof bestandsdelen alleen gebruikt worden als het request groter is dan 4 GB. Het lijkt hier dat de minimaal door DRC ondersteunde berichtgrootte verward wordt met de grens waarbij bestandsdelen gebruikt worden. Volgens mij is het niet logisch die twee verschillende getallen aan één constante op te hangen. 4 GB is absurd groot voor het request en zal in de praktijk systemen omver trekken. Waarom de grens zo hoog leggen terwijl we hetzelfde bestand zonder enig probleem middels bestandsdelen kunnen overzetten? Bovendien is de berekenwijze zeer kostbaar. De consumer weet al hoe groot de bestandsinhoud zonder BASE64cencoding is. Maar om te kijken of het request groter is dan 4GB moet hij dus eerst het bericht maken incl BASE64 encoding en dan op een of andere mannier de grootte van het bericht berekenen. Mogelijk moet hij het bericht daarvoor tijdelijk opslaan. Dit alles is kostbaar om te maken en kostbaar in performance. Stel dat het request dan groter is dan 4 GB dan moet consumer het bericht een tweede keer maken.

Veel logischer zou zijn de volgende tekst: Webservers moeten op deze endpoints een minimale request body size van 5 mB ondersteunen. .... Requests kunnen groter zijn dan de minimale die door providers ondersteund moet worden. De consumer moet dan: .... Ook als het request kleiner is dan de minimale grootte mag de consumer inhoud leeg laten en zo bestandsdelen vragen.

michielverhoef commented 1 year ago

Zoals ik het lees is er geen verplichting om pas met bestandsdelen te gaan werken wanneer een base-64 encoded document groter dan 4GB is. De "moet" uit de zin "De consumer moet dan: " is volgens mij meer bedoeld als "in het geval een document groter is dan 4GB moet een consumer het volgende doen om een document toe te voegen.".

Overigens kan Ik me heel goed voorstellen dat een consumer altijd met bestandsdelen werkt, ongeacht de grootte van het totale document. Dat zou het voor de consumer wel eenvoudiger maken, nl altijd op dezelfde manier werken.

Maar dit is dus een keuze van de consumer, geen verplichting vanuit de standaard.