Null-värden skickas felaktigt ut där data saknas

[Andreas Henriksson]

Vi har upptäckt att EduCloud skickar ut "null" som värde på properties som inte är nullable, exempelvis datumfält. Enligt OpenApi kan inte null skickas som värde för typer som inte är nullable, även om dessa inte är required. Dessa properties skall istället utelämnas från svaret.

Detta leder till att data inte går att läsa med kod som genereras från OpenAPI-generatorer och på så sätt inte går att konsumera.

Som exempel skickas detta ut för placeringar från /persons ändpunkten.

"_embedded": {
	"duties": [],
	"placements": [{
		"id": "045f48b9-fb61-4f48-97c7-bef7a40974fd",
		"placedAt": {
			"id": "7765ced4-e3d3-466a-a29b-30046ac04913",				
            "displayName": "XXX förskola"
		},
		"child": {
			"id": "0312c443-38b1-4d69-907c-6b3a5229bcf0",
			"displayName": "Barn Namnsson"
		},
		"group": {
			"id": "8e13ee2c-6920-4384-88a1-0059834486cc",
			"displayName": "Avdelningsnamn"
		},
		"owners": [{
			"id": "3685f7c3-5865-4744-9abf-6d44f08bffa8",
			"displayName": "Mamma Namnsson"
		}, {
			"id": "3c88a60a-6784-4519-b5fc-e689d8a179e8",
			"displayName": "Pappa Namnsson"
		}],
		"schoolType": "FS",
		"startDate": "2022-04-04",
		"endDate": null,
		"reason": "Omsorg",
		"maxWeeklyScheduleHours": 100,
		"meta": {
			"created": "2022-03-30T11:01:35.664Z",
			"modified": "2022-03-30T11:01:35.664Z"
		}
	}],
	"responsibleFor": []
}

 

[Jonas Erlandsson]

Hej. På vilket sätt går de sönder, menar du?

När man skapar upp nya DTO-objekt så om du inte har värdet alls så kommer attributen alltid att bli null, så om ni sätter värdena och skippar de som är null så borde det gå spara ändå, eller?

Du har en poäng i att om värdena inte är null i specifikationen så ska det inte vara det i implementationen.
Dock har jag svårt att se att det skulle vara ett rejält problem att komma runt. Kan du utveckla vidare hur och varför det går sönder för er?

[Andreas Henriksson]

Hej,

Det beror helt på det underliggande språket hos klienten. Du bygger ditt antagande på ett språk som enbart har nullable-types och inga value-types.

Enligt OpenAPI specifikationen så får inte ett attribut som inte är nullable ha värdet nullable. För semantiskt sett är det inte giltigt.

Ta språket C# som ett konkret exempel.

Typen DateTime, int osv är value-types, dvs de måste ha ett värde så de kan inte vara null. Ett OpenAPI Property av typen Date som inte är nullable mappas då mot DateTime i C#. Semantiskt sett kan inte DateTime ha värdet null och att då försöka binda null från JSON datat till attributet genererar runtime fel. Således kraschar en klient i C# som genererats från OpenAPI specifikationen när den försöker läsa data som inte är förenligt med standarden.
 

[Jonas Erlandsson]

@Tonny - Om jag minns rätt så bygger ni också i C#?
Hur har ni hanterat det här? Vore intressant att höra hur andra integratörer gör.

[Tonny]

Det stämmer att vi bygger i C#.  Vi använder oss inte av någon OpenAPI kodgenerator utan har skapat upp modellerna för hand. 

Så hos oss är exempelvis Placement.EndDate en nullable DateTime.  Samma för ett antal andra värden som inte alltid är med.

[Jonas Erlandsson]

@Andreas Henriksson Vad använder ni för generator?

Det verkar nämligen vara en bug (https://github.com/OpenAPITools/openapi-generator/issues/4816) i OpenAPITools client-generator för C#. Det är till synes en drös som har det här problemet.

Jag tänker också att även om fältet inte är required (således optional) så måste det kunna generera en klient även om man inte har värdet.
Vad skulle hända om vi skulle ommita värdet helt, vilket är helt i linje med OASv3 för optional field. I dagsläget skulle det också krascha klienten antar jag?

Lite vidare diskussion i https://github.com/airbytehq/airbyte/pull/9277#discussion_r781302590 också.

 

[Andreas Henriksson]

Nej det är inte en bugg i generatorn, de flesta såna här issues är relaterade till OpenAPI 2.0 där nullable inte var del av standarden. I OpenAPI 3.0 så är det en explicit del av standarden och nullable typer skall därför bara genereras när attributet är markerat som nullable.

Man kan sedan tycka vad man vill om non-required attribut som är non-nullable, men semantiskt sett så är de inte nullable enligt API specen och skall inte genereras som nullable.

Om ni utelämnar attributet i enlighet med specen så är det upp till klienten att hantera det. I C# garanteras det att attributen får typen default värde när ett objekt skapas. Ovanpå det kan eventuell semantik i JSON avkodaren läggas till. För en OpenAPI tool C# client så kommer alla attribut som inte är med i JSON datat att få värdet default(type). För ett datum blir detta default(DateTime) vilket motsvarar 1/1/0001 12:00:00 AM. Sen blir det upp till klienten att identifiera och hantera detta.

 

 

[Andreas Henriksson]

Kan tilläggas att vi läser ifrån två andra SS12000:2020 implementationer som följer detta och vår genererade klient har inga problem med att läsa från dessa implementationer.

[Jonas Erlandsson]

Det jag menar är att andra har haft samma problem med genereringen av klienter för OASv3 (som det explicit står i issuen) och där man menar på att klienten borde ta höjd för det.

Om ni får tillbaks ett default-värde som är 0001-01-01 så kommer ni alltså sätta ett datum som inte är korrekt? I mina öron låter det värre och som att det borde vara nullable i specen eftersom t ex grupper kan ha ett öppet / odefinierat slutdatum, men det är givetvis en fråga för SIS.

Notera att jag inte säger att du har fel. Jag håller med om att de attributen borde omittas i APIet om de är null.
Samtidigt tycker jag nog allt att klienten borde vara så pass motståndskraftigt att det kan hantera det. Det är såklart högst subjektivt.

[Jonas Erlandsson]

Vi håller internt på att kolla på hur vi ska hantera det här scenariot vill jag också säga.

Den mest troliga lösningen är att omitta attributet helt, även om det egentligen inte är en bra lösning. Det vore som sagt bättre om datum-attribut för vissa entiteter vore nullable. Problemet blir då att vi nu hanterar det, sen ändras specen och då får vi ändra tillbaks. 
Att ha slut-datum öppna / nullable är ju rent verksamhetsmässigt inte fel, men som sagt, det är ju en fråga för SIS-gruppen. 

[Andreas Henriksson]

Det låter bra.

Under tiden vore det intressant att veta om denne generella princip även gäller andra attribut än endDate, eller är det bara endDate som skickas ut som null hos er nu för icke nullable attribut?

[Jonas Erlandsson]

Jag skulle säga att det gäller alla attribut som inte innehåller någon data.
Beroende på hur vi löser det så kommer vi isåfall se över det också.
Och även där tycker jag att specen borde ta höjd för att optional-värden måste vara nullable.

Utifrån mitt perspektiv ser jag det som mer fel att ta bort attributet helt och hållet än att göra det nullable då det känns mer logiskt att ge tillbaks ett attribut-set än att returnera en godtycklig struktur tillbaks, och iom att svaret pagineras så borde det inte innebära en brutalt stor skillnad i varesig responstid eller svarsstorlek.

[Jonas Erlandsson]

@Andreas Henriksson Nu finns det en planerad release för det här.

Den kommer gå live den 18e Maj och Micke har redan lagt ut release notes för det med en länk till en artikel:

 

[Joakim Ganse]

Förstår att ni ändrar från "endDate": null,

Menar ni då att "enddate" helt saknas i dokumentet, så tolkar jag det.

Det innebär att det blir svårare att hantera en förändring från tex "endDate": 2022-06-10, till att sakna endDate.

Jag vet inte hur det står i standarden men hade föredragit ett tomt värde så som "endDate":,

 

klart att det beror på implentation men det innebär att alla värden som saknas måste kollas och om det fanns ett värde tidigera så behöver de tas bort.

 

[Andreas Henriksson]

13 minutes ago, Joakim Ganse said:

Jag vet inte hur det står i standarden men hade föredragit ett tomt värde så som "endDate":,

Det är inte giltig JSON så det är ingen möjlighet. Semantiskt sett så är det ingen som helst skillnad om värdet är tomt eller om attributet helt saknas från objektet i JSON-strängen.

[Joakim Ganse]

5 minutes ago, Andreas Henriksson said:

Det är inte giltig JSON så det är ingen möjlighet. Semantiskt sett så är det ingen som helst skillnad om värdet är tomt eller om attributet helt saknas från objektet i JSON-strängen.

Ok, Tack.

[Andreas Henriksson]

@Jonas Erlandsson behöver kunderna aktivera detta på något sätt? Jag springer fortfarande in i problem kring detta, nu exempelvis att 'null' skickas ut i _embedded.placements[].maxWeeklyScheduleHours på personobjekt från /person ändpunkten.

[Jonas Erlandsson]

Det ska vara en del av standard. Jag har rapporterat det.

När du säger att 'null' skickas ut, menar du att värdet faktiskt är 'null' (dvs strängen) eller är det ett null-värde?

Kan du se om det även gäller /placements? Gör det inte det så skulle du kunna gå den vägen tills det är åtgärdat på /persons
Jag kan inte återskapa det med SE00100 nämligen (innebär såklart inte att det inte är sant).

 

[Andreas Henriksson]

Se exemplet nedan från /person ändpunkten

{..."maxWeeklyScheduleHours":null }

maxWeeklyScheduleHours är av typen int och inte nullable, men ändå så kommer null ut som värde vilket nu skulle vara åtgärdat enligt er.

Edited June 21, 2022 by Jonas Erlandsson
Redigerar bort IDn mm som inte bör bara publika och behåller det viktiga.

[Jonas Erlandsson]

Då måste det ha slunkit igenom. Vi är bara människor, men det tittas på as we speak.
Om du prenumererar på Release Notes-sidan så får du notifikation om när det kommer ut i release.

Tack för att du uppmärksammade det.

[Andreas Henriksson]

Tack för tipset med release notes.

Ville mest kontrollera så att kunden inte behövde aktivera någon flagga, men då vet jag att det adresseras senare 👍