Better deserialization of empty elements / empty strings

[Rossi1337]

Hi,
We lately ran into an issue with a an XML file that contains a mandatory element but that one contains an "empty string" like this.

 <?xml version="1.0"?>
  <test>
      <notnull></notnull>
  </test>

The element is there but it is empty. This cannot be parsed and raises an exception. The problem is that the getter code that is generated converts empty string to "null" in dart but at the same time puts a ! on it.

The model to process this XML looks like this:

import 'package:xml/xml.dart';
import 'package:xml_annotation/xml_annotation.dart' as annotation;

part 'xml_null_test.g.dart';


@annotation.XmlRootElement(name: 'test')
@annotation.XmlSerializable()
class XMLNullTest {
  @annotation.XmlElement(name: 'notnull')
  String notnull;

  XMLNullTest({required this.notnull});

  factory XMLNullTest.fromXmlElement(XmlElement element) =>
      _$XMLNullTestFromXmlElement(element);
...

As you can see the field notnull is of type String and is mandatory. But when we parse the XML above it fails with an exception.
the reason for this is the way how the getter is generated. It is generated like this:

final notnull = element.getElement('notnull')!.getText()!;

The method getText() returns null when the string is "empty" in the XML.
See: xml_text_getter_generator.dart
and getText extension method

Null vs "empty string" is not clearly specified in XML and there are many problems related to this. What you did is not "wrong" as it is not clear if an empty element should be interpreted as null or as "empty string". But the problem is that there is no way I could workaround this issue cleanly (only by making my field String? in my code but this is also not nice).

I would propose to make this somehow configurable if empty elements should be treated as null or as "empty strings".
Then it should generate this code:
final notnull = element.getElement('notnull')!.getText() ?? '';

If you do not want to change this as this might be a compatibility issue with existing code, then we could allow to select this via a flag on the @annotation.XmlSerializable() for the whole XML via a new flag.

---

Here a test case to reproduce this with the simple model class from above. It throws an exception when parsing the XML:

 test('Null XML', () {
    const test = '''
    <?xml version="1.0"?>
    <test>
      <notnull></notnull>
    </test>''';

    final document = XmlDocument.parse(test);
    final model = XMLNullTest.fromXmlElement(document.rootElement);
    expect(model, isNotNull);
  });

Testing started at 09:40 ...

package:amos_mobile_etlb/src/test/xml_null_test.g.dart 29:59 _$XMLNullTestFromXmlElement
package:amos_mobile_etlb/src/test/xml_null_test.dart 15:7 new XMLNullTest.fromXmlElement
test\xml_null_test.dart 15:31 main.

Null check operator used on a null value

[tnc1997]

Hi @Rossi1337, thank you for raising this issue.

Adding the XmlElement annotation onto a String field is a shortcut for:

<book>
  <title></title>
</book>

@annotation.XmlRootElement(name: 'book')
@annotation.XmlSerializable()
class Book {
  @annotation.XmlElement(name: 'title')
  Title? title;
  ...
}

@annotation.XmlRootElement(name: 'title')
@annotation.XmlSerializable()
class Title {
  @annotation.XmlText()
  String? text;

  ...
}

This can be seen in more detail in the XmlTextXmlElementGetterGenerator.

In the example above there are no XmlText children belonging to the title XmlElement and so the field text is null.

A potential solution could be to make the field notnull nullable and then do something like this.

[Rossi1337]

Hi
Yes I see that I can work around this by marking all my String values in my model class as String? but this basically forces null checks and null handling into my code everywhere and there and if by accident someone does define a String we will run again into this issue. For now this is not a blocker for us but it remains an area to run again and again into bugs if people do not take care.

For the serialization we have an way to influence this via "includeIfNull"
For deserialization we have no option.

I see that is is now a little dilemma which way to go. Lets take an example where we serialize a database field into XML and send it out.

Book title is on the database for some reason an empty string "" and would produce this

<title></title>

DB = "" -> XML = "" -> Deserialized in Flutter = null
So what you send is no longer what you receive.
Would we now interpret empty elements as empty strings this problem is solved

DB = "" -> XML = "" -> Deserialized in Flutter = ""

All good for this example right? But what if the DB value is now null? Now we get this

DB = null -> XML = "" -> Deserialized in Flutter = ""
So again what you send is no longer what you receive. Null is converted to empty string on the other side.

There is no good way to fix both scenarios. You can now decide if empty string handling should be consistent or null handling should be consistent. (actually some workaround could be to do something like this <title nil="true" /> but this is just a convention used sometimes and also not by all)

Currently we have something in the middle. Null handling is consistent but empty string handling is "broken".

[tnc1997]

Currently we have something in the middle.

Apologies if I am misunderstanding, however it appears that the proposal would make empty string handling consistent, whilst simultaneously causing null handling to become inconsistent?

As mentioned previously, the intended behaviour of the XmlText annotation is to get all XmlText nodes, or null if there are no XmlText nodes. In the example <title></title> there are no XmlText nodes and so the field would be null.

[tnc1997]

Hi @Rossi1337, could I get you to test the branch feat/49, as I think that an XmlConverter could resolve this issue?

[Rossi1337]

Hi,
I tested this now by implementing my own XmlConverter. With that one my test works and empty string elements are now returned as empty strings and no longer raise an exception.

This is my simple converter:

import 'package:xml/xml.dart';
import 'package:xml_annotation/xml_annotation.dart' as an;

class StringEmptyConverter extends an.XmlConverter<String> {
  const StringEmptyConverter();

  @override
  void buildXmlChildren(
    String instance,
    XmlBuilder builder, {
    Map<String, String> namespaces = const {},
  }) {
    // Do nothing
  }

  @override
  String fromXmlElement(XmlElement element) {
    return element.getText() ?? '';
  }

  @override
  List<XmlAttribute> toXmlAttributes(
    String instance, {
    Map<String, String?> namespaces = const {},
  }) {
    return List.empty();
  }

  @override
  List<XmlNode> toXmlChildren(
    String instance, {
    Map<String, String?> namespaces = const {},
  }) {
    return [XmlText(instance)];
  }
}

and on my model I registered it. It also works when registered on the field level:

@annotation.XmlRootElement(name: 'test')
@annotation.XmlSerializable()
@StringEmptyConverter()
class XMLNullTest {
  @annotation.XmlElement(name: 'notnull')
  String notnull;

  XMLNullTest({required this.notnull});
...

the generated code looks good:

XMLNullTest _$XMLNullTestFromXmlElement(XmlElement element) {
  final notnull = element.getElement('notnull')!;
  return XMLNullTest(
      notnull: const StringEmptyConverter().fromXmlElement(notnull));
}

With this in place empty elements will now return empty strings and the test succeeds.

this is OK but not optimal because I would basically now always register this converter on all model classes I ever write. I still think this should be the default behavior of the library.

[tnc1997]

With that one my test works and empty string elements are now returned as empty strings and no longer raise an exception.

Thank you very much for testing that feature branch for me.

I am planning to further optimise the use of custom XmlConverters by implementing this behaviour in the near future.

I still think this should be the default behavior of the library.

This would be a breaking change, as it would change the default behaviour, which I would like to avoid.

[tnc1997]

I am planning to further optimise the use of custom XmlConverters by implementing this behaviour in the near future.

Hi @Rossi1337, just to let you know that this functionality is previewable in the latest changes to the feature branch.