schultek / dart_mappable

Improved json serialization and data classes with full support for generics, inheritance, customization and more.
https://pub.dev/packages/dart_mappable
MIT License
145 stars 22 forks source link

Deserialization is causing Stack Overflow (using custom mapper) #145

Closed Medformatik closed 8 months ago

Medformatik commented 10 months ago

I am this package for JSON serialization and deserialization. For some data I get from an API, the data model in the app is different to the structure of the JSON, so I have to use a custom mapper here. This is my code. The problem is that if I use this for deserialization, I get this error which is a Stack Overflow, so it seems that there is some kind of recursion happening. I have tried to debug this, but the data that I put in for deserialization looks as expected and I cannot find out what is causing the recursion. I am not sure whether there is an issue with my custom mapper or whether this is an issue of the package itself.

schultek commented 10 months ago

Looks like it already fails at TrimInputModel.fromEntity, when it tries to throw the argument error and stringify the provided entity.

  1. Can you reproduce the error if you just manually call .toString() on the entity?
  2. I'm trying to unterstand the structure. Whats 'SliderTrimInput' and how does it relate to 'SliderTrimInputModel'?
Medformatik commented 10 months ago

I will try whether I can reproduce the error with .toString(). To clarify what SliderTrimInput and SliderTrimInputModel are... I am using Clean Architecture. With this I have a presentation, domain and data layer. In the data layer data is stored in models (e.g. SliderTrimInputModel), but the repository, which connects to the domain layer, always returns entities (e.g. SliderTrimInput). Entities are often just simple classes to store the data, some of them have functions that are used by the business logic of the app. The models use these entities with extends and extend them with functions like those for JSON serialization/deserialization. This way I work exclusively with the objects in the business logic and am free of Maps or any parsing functions.

Medformatik commented 10 months ago

I was just now able to solve the problem. So I was wrong with this issue, sorry for that and thanks for the hint. The fromEntity function was indeed the problem. I didn't notice that in the long error with so many lines with MapperContainerBase.

Medformatik commented 10 months ago

Unfortunately I have to reopen this issue because I get a similar error again. This one is definitely triggered at a different point, except this time there is no indication at all where in my code this error is triggered. I have checked, nowhere do I use .toString() or similar on TrimInputModel, BoatModel or BoatClassModel. I have absolutely no idea what could be causing this recursion. This is the latest version of my code.

schultek commented 10 months ago

The code links don't work anymore.

Medformatik commented 10 months ago

Sorry, I didn't know that. I just replaced the links with non-expiring Pastebin links.

schultek commented 10 months ago

Thx. Do you have a code snippet I can run to reproduce the error? The linked code only contains the classes, not how you use them.

Medformatik commented 8 months ago

Sorry for the late reply. I have finally got back to work on the problem. I have simplified the code significantly and am now using a hook instead of a custom mapper as this solves another problem I was experiencing. This is my new code, but I still have a recursion issue which leads to this error. The reason for this is that the line return SliderTrimInputModel.fromMap(map); in the beforeDecode function of the TrimInputHook is called again and again. This seems to be the way dart_mappable works, but I have no idea how to deal with this problem in my case. Anyway, the code should be much more readable now. Do you have an idea how I can implement the functionality I want and avoid the recursion caused by inheritance.

Medformatik commented 8 months ago

These are some examples of what I am trying to parse here:

SubjectiveSliderTrimInputModel (subtype of SliderTrimInputModel)

{
    "input": {
        "subjective": true,
        "type": "slider"
    },
    "labels": {
        "max": "firm",
        "min": "loose",
        "title": "outhaulTension"
    }
}

ObjectiveSliderTrimInputModel (subtype of SliderTrimInputModel)

{
    "input": {
        "default": 0,
        "max": 40,
        "min": 0,
        "step": 2,
        "type": "slider"
    },
    "labels": {
        "max": "top",
        "min": "bottom",
        "title": "daggerboardHeight",
        "unit": "units.cm"
    }
}

SelectionsTrimInputModel

{
    "input": {
        "constant": true,
        "type": "selection"
    },
    "labels": {
        "selections": {
            "1": "ilca4",
            "2": "ilca6",
            "3": "ilca7"
        },
        "title": "sailSize"
    }
}

All of these are of course TrimInputModels. Maybe this helps with understanding what I am trying to achieve.

schultek commented 8 months ago

I see.

The reason for this is that the line return SliderTrimInputModel.fromMap(map); in the beforeDecode function of the TrimInputHook is called again and again. This seems to be the way dart_mappable works, but I have no idea how to deal with this problem in my case.

You are right. Reason is that hooks are inherited. Meaning if you extend a base class and that class has a hook, that hook is also executed when you decode the subclass. So for you SliderTrimInputModel extends TrimInputModel and therefore also inherits TrimInputHook, which calls itself recursively.

Do you have an idea how I can implement the functionality I want and avoid the recursion caused by inheritance.

Yes :) As I understand it you are basically doing normal polymorphic discrimination in your decoding hooks (read https://pub.dev/documentation/dart_mappable/latest/topics/Polymorphism-topic.html if you are unfamiliar). Luckily for you I added a feature some time ago where you can write custom discriminator logic to decide which subclass to decode. Its the last section from the link.

So roughly you can do this (I didn't test this but to give an idea)

@MappableClass(discriminatorValue: SliderTrimInputModel.checkType)
class SliderTrimInputModel extends TrimInputModel with SliderTrimInputModelMappable {

  /// checks if [value] should be decoded to [SliderTrimInputModel]
  static bool checkType(value) {
    final map = validateValueAndGetMap(value);
    final inputType = validateAndGetInputType(map, 'type', 'input');

    return inputType == InputType.slider;
  }

  ...
}
Medformatik commented 8 months ago

Thank you for the quick reply, that sounds like a solution. I have adopted this for Slider, SubjectiveSlider, ObjectiveSlider and Selections (...Model), as you can see here. I have regenerated everything and set breakpoints in the checkType functions, but none of these checkType are executed when decoding, so I still get the same error. Do I still have to specify a discriminatorKey or something?

schultek commented 8 months ago

You need to remove the beforeDecode methods in the hoos then, as this is replaced by the discriminator handling of the package.

Medformatik commented 8 months ago

Unfortunately, this won't work because I also need to retrieve other values from submaps in the beforeDecode.

final constant = validateAndGetFieldFromSubmap<bool>(map, 'input', 'constant', 'input');
final labelTitle = validateAndGetFieldFromSubmap<String>(map, 'labels', 'title', 'labels');
final labelMin = validateAndGetFieldFromSubmap<String>(map, 'labels', 'min', 'labels');
final labelMax = validateAndGetFieldFromSubmap<String>(map, 'labels', 'max', 'labels');

But I think I should use custom mappers instead of hooks anyway. Currently I'm already returning an instance of the object in the beforeDecode and not a modified map, so I'm actually already doing the decoding. This is what a custom mapper is intended for, but I had the problem that it was not used at all (breakpoints were never triggered), although I specified it in MappableClass wherever it might be needed and even registered the custom mapper globally.

MapperContainer.globals.use(const TrimInputModelCustomMapper());
MapperContainer.globals.use(const SliderTrimInputModelCustomMapper());
MapperContainer.globals.use(const SubjectiveSliderTrimInputModelCustomMapper());
MapperContainer.globals.use(const ObjectiveSliderTrimInputModelCustomMapper());
MapperContainer.globals.use(const SelectionsTrimInputModelCustomMapper());

This is my code with custom mappers instead of hooks and the checkType you mentioned above. When it decodes something, it first runs TrimInputModelCustomMapper.decode, then SubjectiveSliderTrimInputModel.checkType (somehow skipping the SliderTrimInputModelCustomMapper.decode which checks for subjective/objective) and then never reaches SubjectiveSliderTrimInputModelCustomMapper.decode but instead throws the error: _ChainedMapperException (MapperException: Failed to decode (BoatClassModel).trimTemplate(Map<String, TrimInputModel>).value(TrimInputModel)(SliderTrimInputModel).type: Parameter type is missing.) indicating that the custom mapper is not used so the decoder is looking for the type parameter (which is inside the "input" submap) but cannot find it as it is not in the main map.

Do you have any idea why this might happen? I have tried so many things, but somehow it always fails with Parameter type is missing. while the type field is definitely in the data as expected.

schultek commented 8 months ago

You are now mixing concepts together that are separate.

Either annotate a class (which will generate a mapper) or define a custom mapper for that class. Not both.

You can still use hooks and the custom discriminator functions together, just remove the parts that did the discrimination in the hook before.

includeCustomMapper is only for fields of the annotated class.

As you noticed, if you return an instance of a class in the beforeDecode hook of a class, the rest of the decoding is skipped. A better approach is often to return a modified map that is in the right format for the package to then do the rest of decoding.

schultek commented 8 months ago

For your setup I would do the following:

Medformatik commented 8 months ago

Finally, it works. I'm really happy now, I was starting to lose my nerves with the problem, but besides that I liked dart_mappable so much. Great that the problem is now solved. Thank you very much for your help and the development of dart_mappable!