Starting to modernize Gradle

[scribe]

This is a draft because I still don't know what all the dependencies need to be modernized, and I need to figure out why upgrading causes 3 tests to fail.

[lesserwhirls]

It looks like the failures are due to the mocks returning empty values rather than null values?

[scribe]

It looks like the failures are due to the mocks returning empty values rather than null values?

I think so? But I worry swapping something else has changed the real behavior

[lesserwhirls]

Looks like the upgrade from jackson 2.9.0 to 2.13.3 triggered the new failures. We can go up to 2.11.x, but the failure begins in 2.12.x due to this change: https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.12#xml-module.

So the question is, do we want to keep the behavior of mapping empty xml elements to null?

[scribe]

I'm heavily tempted to say we go with the newest behavior, but I also don't feel like that's my call to make? Empty elements are easier to deal with than nulls most of the time?

[lesserwhirls]

Yeah, I'm +1 for empty objects over nulls. @RachelTucker, what do you think?

[scribe]

@blitt How much should we try to preserve the current behavior of the SDK vs modernizing it?

[RachelTucker]

Would an empty Boolean be false and an empty Integer be 0 with the new behavior? The reason why some values are nullable to begin with is because a value not being returned in a response payload has different meaning than a default value being returned. One example of the difference in meaning is written up in GOSDK-36.

[lesserwhirls]

Would an empty Boolean be false and an empty Integer be 0 with the new behavior? The reason why some values are nullable to begin with is because a value not being returned in a response payload has different meaning than a default value being returned. One example of the difference in meaning is written up in GOSDK-36.

In this case, the issue is howjackson-dataformat-xml handles <tag/> vs <tag></tag> prior to coercion, so the question would be if BPs XML treats those as having a different meaning or not. Prior to version 2.12.0, <tag/> would be treated as null, but <tag></tag> would be treated as "". Starting with 2.12.0, the behavior is that both are treated as "" (unless xsi:nil is specified (with proper namespace) on the element, and possibly that the xsd indicates that it is nillable, I think). A little more info can be found here.

If I tell the XmlMapper to treat empty as null, the three failing tests in this PR pass. I think the thing to do is to keep the behavior as-is, but maybe However, as long as BP does not make a distinction between the two different empty element forms, I would suggest we consider changing it in 5.5.x.

[RachelTucker]

Edit: got that backwords

It looks like BP currently returns <tag/> when things are null, not <tag></tag>.

Example:
<Data><Activated>true</Activated><AllowNewJobRequests>true</AllowNewJobRequests><AutoActivateTimeoutInMins>44</AutoActivateTimeoutInMins><AutoInspect>MINIMAL</AutoInspect><CacheAvailableRetryAfterInSeconds>60</CacheAvailableRetryAfterInSeconds><DefaultVerifyDataAfterImport/><DefaultVerifyDataPriorToImport>true</DefaultVerifyDataPriorToImport><Id>d9a3a38b-1dfe-4a63-8d79-fc8efd5a8da6</Id><InstanceId>d9a3a38b-1dfe-4a63-8d79-fc8efd5a8da6</InstanceId><IomCacheLimitationPercent>0.5</IomCacheLimitationPercent><IomEnabled>true</IomEnabled><LastHeartbeat>2022-08-17T22:54:35.514Z</LastHeartbeat><MaxAggregatedBlobsPerChunk>20000</MaxAggregatedBlobsPerChunk><PartiallyVerifyLastPercentOfTapes/><PoolSafetyEnabled>true</PoolSafetyEnabled><UnavailableMediaPolicy>DISALLOW</UnavailableMediaPolicy><UnavailablePoolMaxJobRetryInMins>20</UnavailablePoolMaxJobRetryInMins><UnavailableTapePartitionMaxJobRetryInMins>20</UnavailableTapePartitionMaxJobRetryInMins><VerifyCheckpointBeforeRead>true</VerifyCheckpointBeforeRead></Data>

[lesserwhirls]

Ok, so by BP convention the empty element in the form of <tag/> means nil value, as opposed to something like:

<?xml version="1.0" encoding="UTF-8"?>
<root xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
   ...
   <ele xsi:nil="true"/> 
   ...
</root>

We'll make sure to keep the prior behavior and not look to change it in the future.