Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix JSON, YAML use_*_discriminator for recursive struct types #13238

Merged
merged 4 commits into from
Mar 29, 2023
Merged

Fix JSON, YAML use_*_discriminator for recursive struct types #13238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0ecb203
Fix `use_json_discriminator`
HertzDevil Mar 28, 2023
25373c7
Fix `use_yaml_discriminator`
HertzDevil Mar 28, 2023
4c458da
fixup
HertzDevil Mar 28, 2023
656d2f8
do `value[:root]` as well
HertzDevil Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions spec/std/json/serializable_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,23 @@ end
class JSONVariableDiscriminatorEnum8 < JSONVariableDiscriminatorValueType
end

class JSONStrictDiscriminator
include JSON::Serializable
include JSON::Serializable::Strict

property type : String

use_json_discriminator "type", {foo: JSONStrictDiscriminatorFoo, bar: JSONStrictDiscriminatorBar}
end

class JSONStrictDiscriminatorFoo < JSONStrictDiscriminator
end

class JSONStrictDiscriminatorBar < JSONStrictDiscriminator
property x : JSONStrictDiscriminator
property y : JSONStrictDiscriminator
end

module JSONNamespace
struct FooRequest
include JSON::Serializable
Expand Down Expand Up @@ -1071,6 +1088,16 @@ describe "JSON mapping" do
object_enum = JSONVariableDiscriminatorValueType.from_json(%({"type": 18}))
object_enum.should be_a(JSONVariableDiscriminatorEnum8)
end

it "deserializes with discriminator, strict recursive type" do
foo = JSONStrictDiscriminator.from_json(%({"type": "foo"}))
foo = foo.should be_a(JSONStrictDiscriminatorFoo)

bar = JSONStrictDiscriminator.from_json(%({"type": "bar", "x": {"type": "foo"}, "y": {"type": "foo"}}))
bar = bar.should be_a(JSONStrictDiscriminatorBar)
bar.x.should be_a(JSONStrictDiscriminatorFoo)
bar.y.should be_a(JSONStrictDiscriminatorFoo)
end
end

describe "namespaced classes" do
Expand Down
27 changes: 27 additions & 0 deletions spec/std/yaml/serializable_spec.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,23 @@ end
class YAMLVariableDiscriminatorEnum8 < YAMLVariableDiscriminatorValueType
end

class YAMLStrictDiscriminator
include YAML::Serializable
include YAML::Serializable::Strict

property type : String

use_yaml_discriminator "type", {foo: YAMLStrictDiscriminatorFoo, bar: YAMLStrictDiscriminatorBar}
end

class YAMLStrictDiscriminatorFoo < YAMLStrictDiscriminator
end

class YAMLStrictDiscriminatorBar < YAMLStrictDiscriminator
property x : YAMLStrictDiscriminator
property y : YAMLStrictDiscriminator
end

describe "YAML::Serializable" do
it "works with record" do
YAMLAttrPoint.new(1, 2).to_yaml.should eq "---\nx: 1\ny: 2\n"
Expand Down Expand Up @@ -972,6 +989,16 @@ describe "YAML::Serializable" do
object_enum = YAMLVariableDiscriminatorValueType.from_yaml(%({"type": 18}))
object_enum.should be_a(YAMLVariableDiscriminatorEnum8)
end

it "deserializes with discriminator, strict recursive type" do
foo = YAMLStrictDiscriminator.from_yaml(%({"type": "foo"}))
foo = foo.should be_a(YAMLStrictDiscriminatorFoo)

bar = YAMLStrictDiscriminator.from_yaml(%({"type": "bar", "x": {"type": "foo"}, "y": {"type": "foo"}}))
bar = bar.should be_a(YAMLStrictDiscriminatorBar)
bar.x.should be_a(YAMLStrictDiscriminatorFoo)
bar.y.should be_a(YAMLStrictDiscriminatorFoo)
end
end

describe "namespaced classes" do
Expand Down
34 changes: 13 additions & 21 deletions src/json/serialization.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ module JSON
{% end %}

{% for name, value in properties %}
%var{name} = nil
%var{name} = {% if value[:has_default] || value[:nilable] %} nil {% else %} uninitialized ::Union({{value[:type]}}) {% end %}
%found{name} = false
{% end %}

Expand All @@ -216,26 +216,18 @@ module JSON
case key
{% for name, value in properties %}
when {{value[:key]}}
%found{name} = true
begin
%var{name} =
{% if value[:nilable] || value[:has_default] %} pull.read_null_or { {% end %}

{% if value[:root] %}
pull.on_key!({{value[:root]}}) do
{% end %}

{% if value[:converter] %}
{{value[:converter]}}.from_json(pull)
{% else %}
::Union({{value[:type]}}).new(pull)
{% end %}

{% if value[:root] %}
{% if value[:has_default] || value[:nilable] %} pull.read_null_or do {% else %} begin {% end %}
HertzDevil marked this conversation as resolved.
Show resolved Hide resolved
%var{name} =
{% if value[:root] %} pull.on_key!({{value[:root]}}) do {% else %} begin {% end %}
{% if value[:converter] %}
{{value[:converter]}}.from_json(pull)
{% else %}
::Union({{value[:type]}}).new(pull)
{% end %}
end
{% end %}

{% if value[:nilable] || value[:has_default] %} } {% end %}
end
%found{name} = true
rescue exc : ::JSON::ParseException
raise ::JSON::SerializableError.new(exc.message, self.class.to_s, {{value[:key]}}, *%key_location, exc)
end
Expand All @@ -248,7 +240,7 @@ module JSON

{% for name, value in properties %}
{% unless value[:nilable] || value[:has_default] %}
if %var{name}.nil? && !%found{name} && !::Union({{value[:type]}}).nilable?
if !%found{name}
raise ::JSON::SerializableError.new("Missing JSON attribute: {{value[:key].id}}", self.class.to_s, nil, *%location, nil)
end
{% end %}
Expand All @@ -264,7 +256,7 @@ module JSON
@{{name}} = %var{name}
end
{% else %}
@{{name}} = (%var{name}).as({{value[:type]}})
@{{name}} = %var{name}
{% end %}

{% if value[:presence] %}
Expand Down
31 changes: 15 additions & 16 deletions src/yaml/serialization.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ module YAML
{% end %}

{% for name, value in properties %}
%var{name} = nil
%var{name} = {% if value[:has_default] || value[:nilable] %} nil {% else %} uninitialized ::Union({{value[:type]}}) {% end %}
%found{name} = false
{% end %}

Expand All @@ -221,20 +221,19 @@ module YAML
case key
{% for name, value in properties %}
when {{value[:key]}}
%found{name} = true
begin
%var{name} =
{% if value[:nilable] || value[:has_default] %} YAML::Schema::Core.parse_null_or(value_node) { {% end %}

{% if value[:converter] %}
{{value[:converter]}}.from_yaml(ctx, value_node)
{% elsif value[:type].is_a?(Path) || value[:type].is_a?(Generic) %}
{{value[:type]}}.new(ctx, value_node)
{% else %}
::Union({{value[:type]}}).new(ctx, value_node)
{% end %}

{% if value[:nilable] || value[:has_default] %} } {% end %}
{% if value[:has_default] && !value[:nilable] %} YAML::Schema::Core.parse_null_or(value_node) do {% else %} begin {% end %}
%var{name} =
{% if value[:converter] %}
{{value[:converter]}}.from_yaml(ctx, value_node)
{% elsif value[:type].is_a?(Path) || value[:type].is_a?(Generic) %}
{{value[:type]}}.new(ctx, value_node)
{% else %}
::Union({{value[:type]}}).new(ctx, value_node)
{% end %}
end

%found{name} = true
end
{% end %}
else
Expand All @@ -253,7 +252,7 @@ module YAML

{% for name, value in properties %}
{% unless value[:nilable] || value[:has_default] %}
if %var{name}.nil? && !%found{name} && !::Union({{value[:type]}}).nilable?
if !%found{name}
node.raise "Missing YAML attribute: {{value[:key].id}}"
end
{% end %}
Expand All @@ -269,7 +268,7 @@ module YAML
@{{name}} = %var{name}
end
{% else %}
@{{name}} = (%var{name}).as({{value[:type]}})
@{{name}} = %var{name}
{% end %}

{% if value[:presence] %}
Expand Down