POJOs being generated with initialized collections by default

[hbelmiro]

POJOs are being generated with initialized collections by default. This causes properties not set in the workflow to be unmarshaled to empty collections.

For instance, given the following JSON:

{
  "id": "helloworld",
  "annotations": ["hello", "world"]
}

getAnnotations() should return a collection with two objects. This is working as expected.

Given the followin JSON:

{
  "id": "helloworld",
  "annotations": []
}

getAnnotations() should return an empty collection. This is also working as expected.

Given the followin JSON:

{
  "id": "helloworld"
}

getAnnotations() should return null, but currently it's returning an empty collection. Here's the problem.

This is a problem because there are properties that aren't required, but when present, they should not be empty.
Translating this to Java:

Value	Valid
null	Yes
empty	No
not empty	Yes

To fix this behavior, we just need to set initializeCollections to false in pom.xml.

But, this change can break compatibility, since methods that today never return null might return null after the change, which may cause NullPointerExceptions to users.

So, what do you guys think? Can I submit a PR to fix this issue?

[tsurdilo]

i think this was fixed via 8a61e3c

[hbelmiro]

It wasn't.
We can confirm that in this assertion. It should be failing, since produceEvents is not defined in the workflow.

assertNotNull(defaultDefinition.getTransition().getProduceEvents());

      "defaultCondition": {
        "transition": {
          "nextState": "RejectApplication",
          "compensate": true
        }
      }

[hbelmiro]

@tsurdilo as it was supposed to be fixed, I opened the PR #206

[tsurdilo]

| getAnnotations() should return null, but currently it's returning an empty collection

ok is it bad to check for null + empty in this case? I am kinda not following the issue sorry :(

[hbelmiro]

ok is it bad to check for null + empty in this case? I am kinda not following the issue sorry :(

It is, because there are cases that null is a valid value, but empty isn't. Annotations is one of them. For these cases, a valid workflow will result in an invalid instance of Workflow.

[tsurdilo]

hmmm can u explain ? null is never a valid value for json yaml. if its empty or not in markup should be same thing??

[hbelmiro]

Sorry. By null I meant absent.
For instance, as annotations is not required, the following is a valid workflow:

{
  "id": "absent"
}

The following is also valid:

{
  "id": "ab",
  "annotations": [ "a", "b" ]
}

But, the following is invalid, since annotations minSize = 1:

{
  "id": "empty",
  "annotations": [ ]
}

So, a valid workflow like the above with id = "absent" will be parsed to an invalid instance of Workflow (it is an invalid instance because getAnnotations.size() == 0)

[tsurdilo]

thanks! yeah in tests we rly need to add validation (json schema) of generated outputs. this is rly important