Udefinerte felter i response istedenfor tomme lister eller tomme verdier

[prange]

Description

Ved kall til /serviceowner/dialogs er inneholder returobjektet ikke items når ingen dialoger er funnet. Forventningen min er at det er en tom liste da typen PaginatedListOfSearchDialogSO ikke har spesifisert at verdien er optional. (se https://docs.altinn.studio/dialogporten/reference/entities/dialog/#search-1)

Reproduction

GET /serviceowner/dialogs?externalReference=xyz

Expected behavior

Alle felter er satt i objektet slik det er definert i datatypen, enten med tomme lister eller evt null.

[MagnusSandgren]

I dag purger vi alle felter som er null eller tomme fra responsen uavhengig av om de er nullable i responsmodellen eller ikke. Det ble gjort i sin tid for å lette trafikken, men kan definitivt skape forvirring og gjøre terskelen for å bruke DP høyere.

Foreslår å kun fjerne felter fra responsen dersom feltet på responsmodellen er nullable.

[elsand]

Enig, vi fjerner IgnoreEmptyCollection filteret.

[MagnusSandgren]

Jeg tenkte å modifisere IgnoreEmptyCollection filteret til å kun gjelde nullable retur typer, slik at vi har funksjonaliteten dersom vi velger å uttrykke at noen av listene skal være nullable i returmodellene. Men du vil heller fjerne hele filteret @elsand? 🙂

[elsand]

Hm, hvis vi gjør om en List<T> til å være nullable, gir det da ikke mening å initialisere den til null i stedet for [], slik at den da blir fjernet med Serializer.Options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull som vi allerede bruker? Eller mener du noe annet?

[MagnusSandgren]

Etter en diskusjon med @elsand og @oskogstad lander vi på å gjøre alle lister nullable i swagger output.

Rasjonale er oppsummert her:

Mitt forsøk på å strukturere tanker:

1. Null liste gir i utgangspunktet ikke mening: En null liste gir egentlig ikke mening i mitt hode - sett bor i fra i en veldig spesifikk kontekst. Det blir litt som en nullable boolean. Og en sånn en gir kun mening som input i et søk - altså jeg har tatt stilling til det (true/false), eller jeg vil ikke at det skal ha noe utfall i søket mitt (null). Samme for en liste - enten bryr jeg meg om det og sender inn en liste, eller null om jeg ikke vil at søket mitt skal ta hensyn til det. I denne konteksten vil [] og null kunne ha forskjellig betydning - men ikke alltid.
2. Null og tom output-liste må være ekvivalent: Finnes det scenarier hvor [] og null kan ha forskjellig betydning i en ren output kontekst? Jeg kommer ikke på noe i farten.
3. Spare trafikk: Dersom 2 holder vann og vi bruker Serializer.Options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull gir det mening å fjerne null - og dermed også tomme lister fra output.
4. Ikke lyv for sluttbruker: Dersom 3 holder vann må all output ICollection bli nullable. Dekorere alle lister som nullable i swagger?

[prange]

Jeg tror at dersom dere vil spare på bytes som sendes over kabelen er det andre ting en å ta vekk felter som gir mer effekt - f.eks. mulighet for å definere et felt for idempotency, for å redusere antall GETs man må gjøre for å forhindre dobbeltkall.

Punktlisten over resonnerer godt i mitt hode - jeg vil allikevel slå et slag for at null og [] skaper ulik forventning hos klienten: Dersom en liste er nullable er både null, [] og [e1,...] mulige svar som klienten må håndtere. En utvikler må dermed alltid dobbeltsjekke hva forskjellen mellom null og [] er, og skrive kode som håndterer de to casene som samme. - Ikke mye, men mange bekker små...

[oskogstad]

De skal være nullable i tt02 nå

[LeifHelstad]

Det er en forskjell, men det praktiske effekten er nok den samme. Med unntak av at empty indikerer at det på et tidspunkt er gjort et valg om at listen skal være tom. "null" betyr at den aldri er satt, men om det er for at man ønker "tom" eller om det er mer "don't care" kan ikke tolkes.

Test : Basert på Reproduction steget hadde jeg forventet å ikke få noen treff. altså et "items" på en eller annen måte var tom. Dette må granskes nærmere.

[LeifHelstad]

Typo fra min side i første testforsøk. Når jeg nå har fått skrevet externalReference rett så gir søket ingen treff.

Da blir den tomme "items" listen fjernet i sin helhet.

Dette i motseting til hvis det er et treff (eller flere)

For en mottaker av resultatet uten treff vil nå "items" være fjernet i sin helhet, uavhengig av om vi internt har ansett dem som [] eller null tidligere i kjeden. Det oppfatter jeg som formålet her.

[prange]

Som bruker av APIet ville jeg foretrukket

{
 "items":[],
 "hasNextPage":false,
 "continuationToken":null,
 "orderBy":"createdat_desc,id_desc"
}

Når det ikke er noen treff.
Det er det mest konsistente i mine øyne. Det at det er nullable i schema er en god forbedring, da jeg iallfall er gjort oppmerksom på at items kan være null, men jeg må fortsatt gjøre en vurdering på hva forskjellen mellom null og [] er, og i koden normalisere null og [] til [].