From 1c9941c21c2bfd71f9c19a7d8f016b66cf16d70a Mon Sep 17 00:00:00 2001 From: Andrew Bennett Date: Mon, 29 Apr 2024 08:38:35 -0500 Subject: [PATCH] Treat NON_NULL(DESC(null)) of any kind the same as NULL. See https://github.com/msolomon/argo/issues/18 --- apps/argo/src/value/argo_block_value.erl | 5 ++ apps/argo/src/value/argo_nullable_value.erl | 13 +++-- apps/argo/src/value/argo_scalar_value.erl | 7 +++ apps/argo/src/value/argo_value.erl | 13 +++++ apps/argo_test/test/argo_value_SUITE.erl | 57 +++++++++++++++++++++ 5 files changed, 88 insertions(+), 7 deletions(-) diff --git a/apps/argo/src/value/argo_block_value.erl b/apps/argo/src/value/argo_block_value.erl index 4e32ea1..f9afa7d 100644 --- a/apps/argo/src/value/argo_block_value.erl +++ b/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}}) -> diff --git a/apps/argo/src/value/argo_nullable_value.erl b/apps/argo/src/value/argo_nullable_value.erl index 3667a0f..f12b8f0 100644 --- a/apps/argo/src/value/argo_nullable_value.erl +++ b/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(). diff --git a/apps/argo/src/value/argo_scalar_value.erl b/apps/argo/src/value/argo_scalar_value.erl index 4283c05..64cedfd 100644 --- a/apps/argo/src/value/argo_scalar_value.erl +++ b/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. diff --git a/apps/argo/src/value/argo_value.erl b/apps/argo/src/value/argo_value.erl index 2fd4508..7d5fe31 100644 --- a/apps/argo/src/value/argo_value.erl +++ b/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. diff --git a/apps/argo_test/test/argo_value_SUITE.erl b/apps/argo_test/test/argo_value_SUITE.erl index a98a3d8..87ca7b5 100644 --- a/apps/argo_test/test/argo_value_SUITE.erl +++ b/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."},