altinn-studio
altinn-studio copied to clipboard
Altinn
Better handling of values that cannot be deserialized into the underlying type.
What needs to be solved?
There is a need for a better layer of protection against parsing and deserialization issues for when a given value is invalid for the underlying type.
The most common issue is when a user provides a number that is too large for an integer or even decimal. but this can happen with any non string/text element.
How do you want it solved?
To be discussed.
Alternative solutions
- Frontend no longer send invalid data to backend for storage. This will in some cases require more/better restrictions in the JSON-schema being used to validate the input.
- Backend is made better at handling deserialization issues and are able to provide error message that any user can understand.
- Error messages or warnings when an XSD being imported is using types that could lead to errors like this.
- Warnings in the app editor when a field is bound to an element of a type with missing restrictions.
- All the above?
Solution X: We remove strongly typed models (C#). No more parsing or serialization. Solution Z: All/most properties are strings. No parsing if the value is treated as a string.
Additional context
Is this a different error from #3944 and #4068 (both closed)? I got the same error as discribed in #4068: "Could not convert string to decimal" in the APP: https://hmrhf.apps.tt02.altinn.no/hmrhf/fakturaklage/
Definition of Foedselsnummer
"Foedselsnummer" : { "maxLength" : 11, "type" : "integer" }
(https://altinn.studio/repos/hmrhf/fakturaklage/src/branch/master/App/models/fakturaklage-simple-model_2.schema.json)
All those issues and this issue are related with the same underlying limitation.
@tommyalmendingen One extra note about SSN (National Identity Number) is that they can often start with a zero. A starting zero would be lost when converting to an integer/decimal. I would recommend to always let number based fields with values that can start with zero to be strings. There might be that the agency system can handle it, but we don't. The zero will be lost and the value would be invalid with 10 digits instead of 11.
@SandGrainOne Thanks for you answer and additional info! That is really helpful.
I still get this problem of "Ukjent feil" when writing a very large number in an (non-negative) integer field. Is there a way to limit the number of digits that can be entered in a field? Or do I have to make a manual change in the JSON-file?
@xmrsa what kind of datatype do you use for your field in your model? And what type of datatype did you get on the generated c# model?
Originally it was defined as a nonNegativeInteger in SERES. The problem probably has to do with the "layering" of references in the datamodell. To avoid the "Ukjent feil" I introduced a maximum of 9 digits in SERES.
In the cs-file it looks like this now:
I have no idea why it says "decimal?", in the line
public decimal? ledigeStillinger { get; set; }
Since I put the maxvalue to 999 999 999 I avoid the big crash, but if I could use the XSD-primitive-type directly in SERES I probably would have avoided the problem altogether.
In the schema.json file-file it refers to "integer" but now with a maximum-value:
Without the maximum value here, it crashes when you write a number that is too big. I have updated the datamodel with a new XSD more than once (the XSD had the same name but with changes in some fields and datatypes), so something might have gone wrong there. The app I was working on is this one: https://altinn.studio/repos/xmrsa/ra0678-01
We match all XSD integer types to decimal because they can all be very large numbers. Decimal happens to be the numerical datatype with the most space for large numbers. It's a best effort choice we've made that is now up for debate because of issues like this one.
In this case we could have used the restriction (maximum) to find a better match, but even then it doesn't solve the primary issue here. Regardless of restrictions, a user can still type in a number that is too large, or even include some letters.
An important part of the challenge here is that we want to keep all user entered data even when invalid. If the user types in an invalid date, we still want to remember it untill the user corrects the mistake (a week later). This is currently conflicting with our wish to provide strongly typed models to the app developer.
In other words, we have some compromises to do.
The issue with BigInteger is that built-in xml and json serializers do not support it.
Yes, seems like you're correct. Also, seems like this will not be included in .NET 7. https://github.com/dotnet/runtime/issues/60780
@mirkoSekulic Should we consider creating a custom converter for BigInteger?
@altinnadmin Custom converters can be a solutions for us. However, that approach can have several problems. We need to create converters support for both json and xml, since we're storing data as xml. Xml support can be bit ugly. How to reference those converters? I'd that those converters should be stored in a separate nuget package since both Altinn app and Designer should reference it. That's not a big issue though. But it requires some further discussion since it's a braking change for us and we should have good design, naming for it etc. In addition to that, with my assumption that might be wrong, generated model classes are meant to be poco classes. In theory we're breaking that already with some annotations like Newtonsoft etc.,, but at least we don't have some "hard" references to other packages like potential one for converters.
Could the required converter code for both JSON and XML serialization be opt-in and part of app-lib-dotnet? //cc: @tjololo
Should be possible to add required converters to app-lib-dotnet. Note that, if I have understood the how correctly, this would introduce a(nother?) coupling between datamodeling in designer and the application. New features added to the converter could then possibly lead to non compiling code as the class generated by designer contains unknown Attributes for the application as it has an older/missing version of the converters.
It's failing runtime now for very large numbers (if I understand it correctly), and then failing compile-time sounds like an improvement :-P
Indeed! How are the fields that today result in these errors defined? Could a workaround be to define them as strings and write some custom data validation to ensure they can be converted to biginteger? But as a permanent solution custom converters should be possible. I think we should consider if they are published in a separate nuget as @mirkoSekulic suggests and version it independently from the rest of the app-lib-dotnet packages