consistent format for dataverseContacts JSON #5724

[pdurbin]

Connects to #5724.

[coveralls]

Coverage increased (+0.04%) to 18.026% when pulling 50c5c2d on 5724-dataverse-contacts into a377d33 on develop.

[djbrooke]

Closing this test-only PR as the tests are included in #5739.

[pdurbin]

Reopening as this is the "as usual" one we want (not a weird hotfix to master etc.)

[matthew-a-dunlap]

Thanks for fixing this! The lambda bug was super weird for sure!

[pdurbin]

Maybe @michbarsinai knows how to make it work but it was way easier to just take it out.

[michbarsinai]

@matthew-a-dunlap what lambda bug?

[pdurbin]

@michbarsinai please see #5724 (comment) for my take. In 50c5c2d I modified the public static JsonArrayBuilder json(List<DataverseContact> dataverseContacts) method, taking out

    return dataverseContacts.stream()
            .map(dc -> jsonObjectBuilder()
            .add("displayOrder", dc.getDisplayOrder())
            .add("contactEmail", dc.getContactEmail())
            ).collect(toJsonArray());

... and replacing it with this non-lamda function:

    JsonArrayBuilder jsonArrayOfContacts = Json.createArrayBuilder();
    for (DataverseContact dataverseContact : dataverseContacts) {
        NullSafeJsonBuilder contactJsonObject = NullSafeJsonBuilder.jsonObjectBuilder();
        contactJsonObject.add("displayOrder", dataverseContact.getDisplayOrder());
        contactJsonObject.add("contactEmail", dataverseContact.getContactEmail());
        jsonArrayOfContacts.add(contactJsonObject);
    }
    return jsonArrayOfContacts;

It's the weirdest thing. If you look at 50c0de8 (and earlier commit) you can see that the lambda version is emitting JSON for dataverse contacts from a unit test. But the later commit to remove the lambda was the quickest way to get the deployed app to emit JSON for dataverse contacts. I hope this makes sense. Again, in production the lambda would only ever emit an empty JSON array for dataset contacts. Weird.

[michbarsinai]

Weird indeed. We had issues with lambdas on classes weaved by our JPA implementation. These were fixed in later versions of EclipseLink, but I'm not sure whether we upgraded or not. This is probably not directly related, since JsonPrinter is not an entity class. But maybe some other byte code level mechanism is involved (e.g. dependency injection)?

…

On 11 Apr 2019, at 13:21, Philip Durbin ***@***.***> wrote: @michbarsinai <https://github.com/michbarsinai> please see #5724 (comment) <#5724 (comment)> for my take. In 50c5c2d <50c5c2d> I modified the public static JsonArrayBuilder json(List<DataverseContact> dataverseContacts) method, taking out return dataverseContacts.stream() .map(dc -> jsonObjectBuilder() .add("displayOrder", dc.getDisplayOrder()) .add("contactEmail", dc.getContactEmail()) ).collect(toJsonArray()); ... and replacing it with this non-lamda function: JsonArrayBuilder jsonArrayOfContacts = Json.createArrayBuilder(); for (DataverseContact dataverseContact : dataverseContacts) { NullSafeJsonBuilder contactJsonObject = NullSafeJsonBuilder.jsonObjectBuilder(); contactJsonObject.add("displayOrder", dataverseContact.getDisplayOrder()); contactJsonObject.add("contactEmail", dataverseContact.getContactEmail()); jsonArrayOfContacts.add(contactJsonObject); } return jsonArrayOfContacts; It's the weirdest thing. If you look at 50c0de8 <50c0de8> (and earlier commit) you can see that the lambda version is emitting JSON for dataverse contacts from a unit test. But the later commit to remove the lambda was the quickest way to get the deployed app to emit JSON for dataverse contacts. I hope this makes sense. Again, in production the lambda would only ever emit an empty JSON array for dataset contacts. Weird. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5737 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB2UJPnXqVZFCRm-i8hJoWVBjYUK_Zqiks5vfwywgaJpZM4cksZv>.

[pdurbin]

I have no idea but if we figure it out let's put it in a JavaOne^h^h^h CodeOne talk next year. 😄