fix: use appropriate methods to convert the numbers in XML

[emanuelaepure10]

Conversion of the numbers using appropriate methods of each different type.

ING-3965

[stempler]

See comment.

Additionally it would be great to have unit tests accompany the implementation to (1) verify it and (2) prevent future regressions.

main changes were done as requested

[stempler]

Generally looks good to me, but I have some doubts if the added converters are sufficient.

As mentioned previously it would be great to have unit tests to accompany your implementation to (1) verify it and (2) prevent future regressions.

Related to the conversions there could be other types of values/bindings for the XML simple types, than what you implemented now.
When we load an XML schema we here use functionality from Geotools to determine the binding. As an example, for the integer simple type a BigInteger binding is used. So I think at least the respective binding types from that class need to be supported for the conversion.

[stempler]

Points from my previous review comment are still open - any feedback on those?
See #1171 (review)

[emanuelaepure10]

If you refer to the points about testing, yes there are yet no test done.
Would you have an example of something similar from where to get an idea on how to create these kind of test?

[stempler]

If you refer to the points about testing, yes there are yet no test done.

Yes, I meant that but also the other mentioned point on the conversions, which should at least cover the conversions between the bindings we use for the XML simple types and the XmlBeans classes. The example I mentioned was that for the integer XML simple type the binding BigInteger is used.
Though it might be possible that this is covered by a transformation chain with intermediate types? If that's the case then this could be verified with the unit tests. Those should ideally include at least one test for every number related simple type.

Would you have an example of something similar from where to get an idea on how to create these kind of test?

I would propose to look at StreamGmlWriter2Test for examples of tests writing XML/GML. Though in your case for the validation you would want to check the written XML instead of loading it as instance, since you want to verify how it is represented in XML as text.

[emanuelaepure10]

Yes, I meant that but also the other mentioned point on the conversions, which should at least cover the conversions between the bindings we use for the XML simple types and the XmlBeans classes. The example I mentioned was that for the integer XML simple type the binding BigInteger is used.

Why for an integer cannot be used Integer instead of BigInteger?

I would propose to look at StreamGmlWriter2Test for examples of tests writing XML/GML. Though in your case for the validation you would want to check the written XML instead of loading it as instance, since you want to verify how it is represented in XML as text.

So that means that StreamGmlWriter2Test doesn't help me.
There is anything around that would check the written XML?

[stempler]

@emanuelaepure10 It would be great if you would use fixup commits while working on the PR because that makes reviews much easier.

Why for an integer cannot be used Integer instead of BigInteger?

I would guess that the reason they did that in Geotools might have been that they wanted to be able to represent numbers that are outside the range supported by integer.

So that means that StreamGmlWriter2Test doesn't help me.

Why?

You can in a similar way

• load an XML schema
• create test data programmatically
• write the data

The main difference would be that you validate the written data in a different way.

There is anything around that would check the written XML?

An easy way to load and process XML is for instance the Groovy XmlSlurper class.

[emanuelaepure10]

It would be great if you would use fixup commits while working on the PR because that makes reviews much easier.

I'll do so. I knew that I should always squash them.

Yes, I meant that but also the other mentioned point on the conversions, which should at least cover the conversions between the bindings we use for the XML simple types and the XmlBeans classes.

I think that all of this has been already covered.

[stempler]

Do you plan to still add tests?

Related to the commit message I think this is a fix, not a feature.

[emanuelaepure10]

@stempler
I have extended an existing test in GmlInstanceCollectionTest, as it seemed the most appropriate place from my perspective. Please review the solution and let me know if it aligns with your expectations.

Thank you.

[we-release]

🎉 This PR is included in version 5.2.0 🎉

The release is available on:

• v5.2.0
• GitHub release

Your semantic-release bot 📦🚀