TypedRelation service call in Alchemy Language Java SDK not returning entities

[nastacio]

🐛

This is a proposed fix for
#367

It takes the "arguments/argument" indirection into account when deserializing the server payloads into TypedRelation Java objects. Without this fix, the "entities" are not deserialized correctly and TypedRelation.getEntities() always returns a null pointer.

I also changed the type of TypedRelation.entities from Entity to TypedEntity, which seemed the original intention of the design all along.

[germanattanasio]

I feel like this can be done with POJOs and without TypeAdapters. Is that something you have looked into @nastacio ?

[nastacio]

I have not used this Google library before (I know it is popular) , but I understand a plain-POJO
resolution would require the introduction of new classes to represent the “arguments” and “argument”
elements surrounding the entities. I felt this would only clutter the Java API.

The fundamental problem resides with the payload design itself, which is unnecessarily verbose
in the representation of the entities.

Denilson Nastacio
dnastacio@gmail.com

On Jul 5, 2016, at 5:13 PM, German Attanasio Ruiz notifications@github.com wrote:

I feel like this can be done with POJOs and without TypeAdapters. Is that something you have looked into @nastacio https://github.com/nastacio ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #370 (comment), or mute the thread https://github.com/notifications/unsubscribe/ADIHH-J1ASo22QeJfASiyehLuCjnS4D6ks5qSskWgaJpZM4JDwYt.

[max-vogler]

@nastacio: I totally agree with your opinion about the payload design - it is complex! That cluttering of the Watson Java SDK is a definitely problem we're facing.

Nevertheless, we used the approach with POJOs throughout the Java SDK and try to keep the amount of custom (de)serializers low. Although unintuitive at first, this is benefitial in this case because of multiple reasons:

1. Because the Java objects mirror the API responses very closely, we can employ a simple equals() check by comparing the JSON representation of Java objects. This requires a write() method in TypeAdapters, which is currently failing silently in your implementation.
2. Testing custom (de)serializers is tedious because they virtually consist of branches only. It requires a lot of unit tests to get a good coverage. When not using GSON's default (de)serializer, nasty edge cases have to be handled manually - null handling for example.
3. The maintainability, reusability, and extensibility of custom (de)serializers is simply bad. Most API changes implicate changes in the Java object anway. Requiring the custom (de)serializers to be changed as well results in duplicate work.

An example of a commit that solved a similar problem is 8a13f41

[nastacio]

@max-vogler: at some point one must decide what is the fundamental interface for developers: the server payload or the language binding. The design for the server payload goes from verbose (e.g. the "arguments/argument" wrappers) to whimsical ( the "part:first | second" elements inside the "argument" element) to inconsistent (the "argument" element only exists in XML, but not in JSON) , so I somehow assumed it would make sense to exorcise some of it in the language binding.

I definitely understand the choice of POJOs as a design philosophy that makes the the API mappings more predictable for people familiar with the server payload API, so I mean no criticism towards that approach.

That said, points #2 and #3 on maintainability/testing/etc were lost on me. The presumed savings in not writing the tedious test code for null pointers and multiple payload variants were the very cause of this bug. The GSON library glides over misalignments between payload and Java classes and will silently assign null pointers to attributes it cannot find in the payloads, so that you still need to test all of the mapping possibilities even if you did not rely on type adapters.

If the idea is to stick to a literal mapping (thus eliminating the inherent obscurity of type adapters) AND to cut down on testing the payload mapping and variations to the nth degree, then those two purposes may be best served by creating an XSD schema from complete payloads (couple of clicks on a decent IDE) and then using the xjc compiler to automatically generate the POJOs.

[RemyLespagnol]

Hi,

When this request will be merge to a new version of the API ?
I need this entities' information in my project.

Thanks.

[germanattanasio]

The API changed since you did this work @nastacio but I managed to update what you did to match the new API and wrote a pull request, see #403

[codecov-io]

Current coverage is 75.43% (diff: 62.72%)

Merging #370 into master will decrease coverage by 0.12%

@@             master       #370   diff @@
==========================================
  Files            66         66          
  Lines          2328       2410    +82   
  Methods           0          0          
  Messages          0          0          
  Branches        412        424    +12   
==========================================
+ Hits           1759       1818    +59   
- Misses          306        321    +15   
- Partials        263        271     +8   

Powered by Codecov. Last update 4ddb3a6...338bfa0