kennknowles opened a new issue, #19506:
URL: https://github.com/apache/beam/issues/19506

   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](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](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](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](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](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#L248-L250)
 **** [https://github.com/apache/beam/blob/a85ea07b7193
 
85ec185e4fc5e4cdcc67b3598599/sdks/java/core/src/main/java/org/apache/beam/sdk/options/ValueProvider.java#L214-L220](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](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](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)](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)
   
   Imported from Jira 
[BEAM-6954](https://issues.apache.org/jira/browse/BEAM-6954). Original Jira may 
contain additional context.
   Reported by: bnemeth.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to