[jira] [Commented] (BEAM-6954) @Default not called if the options json has null value for a property

["Beam JIRA Bot (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/BEAM-6954?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17169357#comment-17169357 ] 

Beam JIRA Bot commented on BEAM-6954:
-------------------------------------

This issue is P2 but has been unassigned without any comment for 60 days so it has been labeled "stale-P2". If this issue is still affecting you, we care! Please comment and remove the label. Otherwise, in 14 days the issue will be moved to P3.

Please see https://beam.apache.org/contribute/jira-priorities/ for a detailed explanation of what these priorities mean.


> @Default not called if the options json has null value for a property
> ---------------------------------------------------------------------
>
>                 Key: BEAM-6954
>                 URL: https://issues.apache.org/jira/browse/BEAM-6954
>             Project: Beam
>          Issue Type: Bug
>          Components: sdk-java-core
>    Affects Versions: 2.11.0
>            Reporter: Balázs Németh
>            Priority: P2
>              Labels: stale-P2
>
> When a pipeline options get deserialized from a json with [https://github.com/apache/beam/blob/a85ea07b719385ec185e4fc5e4cdcc67b3598599/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ProxyInvocationHandler.java#L738-L760] it creates a map, where properties present in the json - even if with a null value - will be added to the map.
> So we can have String->NullNode mappings.
> When you create a ProxyInvocationHandler with this Map ( [https://github.com/apache/beam/blob/a85ea07b719385ec185e4fc5e4cdcc67b3598599/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ProxyInvocationHandler.java#L117-L125] ) this map will be the backing jsonOptions map.
> Later on when a getter is called on the options it will reach this code: [https://github.com/apache/beam/blob/a85ea07b719385ec185e4fc5e4cdcc67b3598599/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ProxyInvocationHandler.java#L156-L158] 
> Then the containsKey will return true, even for NullNodes. So we won't execute the getDefault() method hence not calculating the default value.
>  
> I'm not sure about the expected behaviour, but either:
>  - the containsKey check should be expanded with an !isNull check
>  OR
>  - when we serialize the json, it shouldn't serialize null values at [https://github.com/apache/beam/blob/a85ea07b719385ec185e4fc5e4cdcc67b3598599/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ProxyInvocationHandler.java#L653-L655]
>  
> Instinctively I would have expected the @Default.* annotations producing values every single time, when the value is null - so a property with a @Default.* annotation can't be null - but I was unable to find anything explicit regarding this in the documentation. So I'm not sure which of the suggested change has to be made.
> -------
> Okay, I have investigated further, and it seems the default value is indeed calculated before the json serialization by calling the mentioned method. The problem is that it returns a RuntimeValueProvider, which gets serialized as null ( [https://github.com/apache/beam/blob/a85ea07b719385ec185e4fc5e4cdcc67b3598599/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java#L275-L279] ), because the isAccessible returns false ( [https://github.com/apache/beam/blob/a85ea07b719385ec185e4fc5e4cdcc67b3598599/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java#L248-L250] + [https://github.com/apache/beam/blob/a85ea07b719385ec185e4fc5e4cdcc67b3598599/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java#L214-L220] )
> ... and then during deserialization it is found in the jsonOptions at ( [https://github.com/apache/beam/blob/a85ea07b719385ec185e4fc5e4cdcc67b3598599/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ProxyInvocationHandler.java#L156-L158] ), so it executes the getValueFromJson which uses an ObjectMapper to create a ValueProvider from a NullNode ( [https://github.com/apache/beam/blob/a85ea07b719385ec185e4fc5e4cdcc67b3598599/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ProxyInvocationHandler.java#L498] ) . 
> The problem here is that according to the JsonDeserializer documentation, the deserialize method isn't executed for null nodes. -> [https://static.javadoc.io/com.fasterxml.jackson.core/jackson-databind/2.9.6/com/fasterxml/jackson/databind/JsonDeserializer.html#deserialize(com.fasterxml.jackson.core.JsonParser,%20com.fasterxml.jackson.databind.DeserializationContext)]
> For this part of the issue see BEAM-6963 (that still doesn't solve this issue btw, but might be required for it)



--
This message was sent by Atlassian Jira
(v8.3.4#803005)