Treat NON_NULL(DESC(null)) of any kind the same as NULL. See msolomon…

…/argo#18

apps/argo/src/value/argo_block_value.erl

@@ -23,6 +23,7 @@
 %% API
 -export([
     new/2,
+    is_null/1,
     key/1,
     to_block_wire_type/1
 ]).
@@ -43,6 +44,10 @@
 new(BlockWireType = #argo_block_wire_type{}, Value = #argo_scalar_value{}) ->
     #argo_block_value{wire_type = BlockWireType, value = Value}.
 
+-spec is_null(BlockValue) -> boolean() when BlockValue :: t().
+is_null(#argo_block_value{value = ScalarValue}) ->
+    argo_scalar_value:is_null(ScalarValue).
+
 -compile({inline, [key/1]}).
 -spec key(BlockValue) -> Key when BlockValue :: t(), Key :: argo_types:name().
 key(#argo_block_value{wire_type = #argo_block_wire_type{key = Key}}) ->

apps/argo/src/value/argo_nullable_value.erl

@@ -56,14 +56,13 @@ null(NullableWireType = #argo_nullable_wire_type{}) ->
 
 -spec non_null(NullableWireType, Value) -> NullableValue when
     NullableWireType :: argo_nullable_wire_type:t(), Value :: argo_value:t(), NullableValue :: t().
-non_null(
-    NullableWireType = #argo_nullable_wire_type{'of' = #argo_wire_type{inner = #argo_desc_wire_type{}}}, #argo_value{
-        inner = #argo_desc_value{inner = null}
-    }
-) ->
-    null(NullableWireType);
 non_null(NullableWireType = #argo_nullable_wire_type{}, Value = #argo_value{}) ->
-    #argo_nullable_value{wire_type = NullableWireType, inner = {non_null, Value}}.
+    case argo_value:is_null(Value) of
+        false ->
+            #argo_nullable_value{wire_type = NullableWireType, inner = {non_null, Value}};
+        true ->
+            null(NullableWireType)
+    end.
 
 -spec field_errors(NullableWireType, FieldErrors) -> NullableValue when
     NullableWireType :: argo_nullable_wire_type:t(), FieldErrors :: [argo_error_value:t()], NullableValue :: t().

apps/argo/src/value/argo_scalar_value.erl

@@ -41,6 +41,7 @@
     is_fixed_length/2,
     is_float64/1,
     is_labeled/1,
+    is_null/1,
     is_string/1,
     is_varint/1,
     supports_deduplication/1,
@@ -122,6 +123,12 @@ is_labeled(#argo_scalar_value{inner = {boolean, _}}) -> true;
 is_labeled(#argo_scalar_value{inner = {bytes, _}}) -> true;
 is_labeled(#argo_scalar_value{}) -> false.
 
+-spec is_null(ScalarValue) -> boolean() when ScalarValue :: t().
+is_null(#argo_scalar_value{inner = {desc, DescValue}}) ->
+    argo_desc_value:is_null(DescValue);
+is_null(#argo_scalar_value{}) ->
+    false.
+
 -spec is_string(ScalarValue) -> boolean() when ScalarValue :: t().
 is_string(#argo_scalar_value{inner = {T, _}}) -> T =:= string.
 

apps/argo/src/value/argo_value.erl

@@ -61,6 +61,7 @@
     is_error/1,
     is_extensions/1,
     is_labeled/1,
+    is_null/1,
     is_nullable/1,
     is_path/1,
     is_record/1,
@@ -260,6 +261,18 @@ is_labeled(#argo_value{inner = #argo_array_value{}}) ->
 is_labeled(#argo_value{}) ->
     false.
 
+-spec is_null(Value) -> boolean() when Value :: t().
+is_null(#argo_value{inner = BlockValue = #argo_block_value{}}) ->
+    argo_block_value:is_null(BlockValue);
+is_null(#argo_value{inner = DescValue = #argo_desc_value{}}) ->
+    argo_desc_value:is_null(DescValue);
+is_null(#argo_value{inner = NullableValue = #argo_nullable_value{}}) ->
+    argo_nullable_value:is_null(NullableValue);
+is_null(#argo_value{inner = ScalarValue = #argo_scalar_value{}}) ->
+    argo_scalar_value:is_null(ScalarValue);
+is_null(#argo_value{}) ->
+    false.
+
 -spec is_nullable(Value) -> boolean() when Value :: t().
 is_nullable(#argo_value{inner = #argo_nullable_value{}}) -> true;
 is_nullable(#argo_value{}) -> false.

apps/argo_test/test/argo_value_SUITE.erl

@@ -34,6 +34,8 @@
 
 %% Test Cases
 -export([
+    issue_18_non_null_desc_null/0,
+    issue_18_non_null_desc_null/1,
     prop_roundtrip_encoder_and_decoder/0,
     prop_roundtrip_encoder_and_decoder/1,
     prop_roundtrip_json_encoder_and_json_decoder/0,
@@ -54,6 +56,7 @@ all() ->
 groups() ->
     [
         {value, [parallel], [
+            issue_18_non_null_desc_null,
             prop_roundtrip_encoder_and_decoder,
             prop_roundtrip_json_encoder_and_json_decoder,
             prop_to_wire_type
@@ -82,6 +85,60 @@ end_per_testcase(_TestCase, _Config) ->
 %%% Test Cases
 %%%=============================================================================
 
+issue_18_non_null_desc_null() ->
+    [
+        {doc, "Edge case where NON_NULL(DESC(NULL)) occurs. See https://github.com/msolomon/argo/issues/18"},
+        {timetrap, {seconds, 60}}
+    ].
+
+issue_18_non_null_desc_null(_Config) ->
+    InlineHeader = argo_header:new(#{inline_everything => true}),
+    InlineSelfDescribingHeader = argo_header:new(#{inline_everything => true, self_describing => true}),
+    SelfDescribingHeader = argo_header:new(#{self_describing => true}),
+
+    ScalarWireType = argo_scalar_wire_type:desc(),
+    BlockWireType = argo_block_wire_type:new(ScalarWireType, <<"JSON">>, false),
+    NullableWireType = argo_nullable_wire_type:new(argo_wire_type:block(BlockWireType)),
+    WireType = argo_wire_type:nullable(NullableWireType),
+
+    DescValueNull = argo_desc_value:null(),
+    ScalarValueNull = argo_scalar_value:desc(DescValueNull),
+    BlockValueNull = argo_block_value:new(BlockWireType, ScalarValueNull),
+    NullableValueNull = argo_nullable_value:non_null(NullableWireType, argo_value:block(BlockValueNull)),
+    ValueNull = argo_value:nullable(NullableValueNull),
+
+    ValueNullDefaultEncoded = argo_value:to_writer(ValueNull),
+    ?assertEqual({<<>>, ValueNull}, argo_value:from_reader(WireType, ValueNullDefaultEncoded)),
+
+    ValueNullInlineEncoded = argo_value:to_writer(ValueNull, InlineHeader),
+    ?assertEqual({<<>>, ValueNull}, argo_value:from_reader(WireType, ValueNullInlineEncoded)),
+
+    ValueNullInlineSelfDescribingEncoded = argo_value:to_writer(ValueNull, InlineSelfDescribingHeader),
+    ?assertEqual({<<>>, ValueNull}, argo_value:from_reader(WireType, ValueNullInlineSelfDescribingEncoded)),
+
+    ValueNullSelfDescribingEncoded = argo_value:to_writer(ValueNull, SelfDescribingHeader),
+    ?assertEqual({<<>>, ValueNull}, argo_value:from_reader(WireType, ValueNullSelfDescribingEncoded)),
+
+    DescValueNonNull = argo_desc_value:string(<<>>),
+    ScalarValueNonNull = argo_scalar_value:desc(DescValueNonNull),
+    BlockValueNonNull = argo_block_value:new(BlockWireType, ScalarValueNonNull),
+    NullableValueNonNull = argo_nullable_value:non_null(NullableWireType, argo_value:block(BlockValueNonNull)),
+    ValueNonNull = argo_value:nullable(NullableValueNonNull),
+
+    ValueNonNullDefaultEncoded = argo_value:to_writer(ValueNonNull),
+    ?assertEqual({<<>>, ValueNonNull}, argo_value:from_reader(WireType, ValueNonNullDefaultEncoded)),
+
+    ValueNonNullInlineEncoded = argo_value:to_writer(ValueNonNull, InlineHeader),
+    ?assertEqual({<<>>, ValueNonNull}, argo_value:from_reader(WireType, ValueNonNullInlineEncoded)),
+
+    ValueNonNullInlineSelfDescribingEncoded = argo_value:to_writer(ValueNonNull, InlineSelfDescribingHeader),
+    ?assertEqual({<<>>, ValueNonNull}, argo_value:from_reader(WireType, ValueNonNullInlineSelfDescribingEncoded)),
+
+    ValueNonNullSelfDescribingEncoded = argo_value:to_writer(ValueNonNull, SelfDescribingHeader),
+    ?assertEqual({<<>>, ValueNonNull}, argo_value:from_reader(WireType, ValueNonNullSelfDescribingEncoded)),
+
+    ok.
+
 prop_roundtrip_encoder_and_decoder() ->
     [
         {doc, "Property-based test for `argo_value' encoder and decoder."},