search

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

foutbericht instance niet definieren als uri #113

Open fsamwel opened 3 years ago

fsamwel commented 3 years ago

in Foutbericht wordt instance gedefinieerd als format: uri:

        instance:
          type: string
          format: uri
          description: Uri van de aanroep die de fout heeft veroorzaakt

hierdoor is een relatieve url niet toegestaan, althans niet volgens sommige tooling, zoals Postman.

In Href voor andere links hebben we daarom format: uri verwijderd. Ik denk dat we dat hier ook zouden moeten doen.

@MelvLee @JohanBoer Is dat een breaking change wanneer we dat zouden doorvoeren op een bestaande api?

JohanBoer commented 3 years ago

Dat zou zo maar eens breaking kunnen zijn, afhankelijk van hoe de controle van het foutbericht wordt geïmplementeerd. Als dat niet generiek is ingericht op basis van het format, dan kan het zijn dat bij de consumer een check op het format een fout geeft als we daar gewoon string van maken.

Uiteraard heeft in deze vraagstukken @MelvLee het laatste woord (want hij weet echt hoe het werkt.)

MelvLee commented 3 years ago

Het is een breaking change in .NET. Met de format:uri property wordt instance van het type Uri en zonder die property wordt het een string. En een Uri object kan je niet als een string behandelen

fsamwel commented 3 years ago

dan kunnen we dat dus niet zomaar in common doorvoeren, want dan zou er ergens ongemerkt een breaking change insluipen. Ik sluit het issue

melsk-r commented 3 years ago

@fsamwel Waarom wordt dit issue gesloten? Is het niet van belang om dit issue ergens op een goed moment in de toekomst op te lossen? Bijv. als we toch al een breaking change op de common moeten introduceren. Als dat zo is zou ik het issue open laten staan en er het label 2.0.0 aan koppelen.

MelvLee commented 3 years ago

@JohanBoer @melsk-r @fsamwel, er blijkt nog een uri format te bestaan, nl uri-reference, zie 7.3.5 Resource Identifiers. Hiermee geeft je aan dat een uri absoluut of relatief kan zijn. Postman herkent deze format en ziet in dit geval relatieve urls ook als valide. Genereren van .NET code voor een string property met deze format resulteert in een string type en niet in een Uri type. Dat betekent dus nog steeds compile errors als in code de property wordt gebruikt. De vraag is of het in dit geval een groot probleem is. Het gaat alleen om het Foutbericht en ik denk dat er in consumer code veel minder foutafhandeling logica wordt geïmplementeerd. Voordelen zijn veel groter, omdat je hiermee kan communiceren of je alleen absolute urls (format: uri) of absolute en relatieve urls (format: uri-reference) verwacht en tooling (zoals Postman) kan hierop checken. Ik denk dat het daarom een goed idee is om deze format toe te voegen aan de Href type. Voor code generatie is dit geen probleem omdat het gegenereerde type een string blijft.

melsk-r commented 3 years ago

Ik heropen het issue zodat het op de radar blijft wat ons dwingt hier een beslissing over te nemen. @Melvin, mag ik aannemen dat dit geen breaking change is, alles wat een uri is zal immers ook een uri-reference zijn?

MelvLee commented 3 years ago

Ik heropen het issue zodat het op de radar blijft wat ons dwingt hier een beslissing over te nemen. @melvin, mag ik aannemen dat dit geen breaking change is, alles wat een uri is zal immers ook een uri-reference zijn?

Het is een breaking change. Het gedrag is veranderd. Als uri-reference als format wordt gebruikt, kan de uri absoluut of relatief zijn. Dat betekent dat de consumer nu logica moet toevoegen om hiermee rekening te houden. De vraag is hoe groot de impact in dit geval is. Het betreft de instance property van het Foutbericht component. Ik verwacht dat veel consumers er niets mee zullen doen. Hooguit loggen.