Skip to content

Commit 56c8826

Browse files
Sijastraight-shoota
Sija
and
straight-shoota
authored
Fixes regression in Object.from_json [fixup #15840] (#16142)
Co-authored-by: Johannes Müller <[email protected]>
1 parent 05555e6 commit 56c8826

File tree

3 files changed

+164
-99
lines changed

3 files changed

+164
-99
lines changed

‎spec/std/json/serializable_spec.cr‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,20 @@ require "big/json"
66
require "uuid"
77
require "uuid/json"
88

9+
enum JSONSerializableEnum
10+
Zero
11+
One
12+
Two
13+
OneHundred
14+
end
15+
16+
@[Flags]
17+
enum JSONSerializableFlagEnum
18+
One
19+
Two
20+
OneHundred
21+
end
22+
923
class JSONAttrValue(T)
1024
include JSON::Serializable
1125

@@ -515,6 +529,44 @@ module JsonDiscriminatorBug
515529
end
516530
end
517531

532+
record JSONAttrWithEnumValue, value : JSONSerializableEnum do
533+
include JSON::Serializable
534+
535+
@[JSON::Field(converter: Enum::ValueConverter(JSONSerializableEnum))]
536+
@value : JSONSerializableEnum
537+
end
538+
539+
record JSONAttrWithFlagEnumValue, value : JSONSerializableFlagEnum do
540+
include JSON::Serializable
541+
542+
@[JSON::Field(converter: Enum::ValueConverter(JSONSerializableFlagEnum))]
543+
@value : JSONSerializableFlagEnum
544+
end
545+
546+
abstract class SerializableFoo
547+
include JSON::Serializable
548+
549+
module Converter
550+
def self.from_json(pull : JSON::PullParser) : SerializableFoo
551+
SerializableFoo.find.from_json("{}")
552+
end
553+
end
554+
555+
def self.find : SerializableFoo.class
556+
SerializableBar.as(SerializableFoo.class)
557+
end
558+
end
559+
560+
class SerializableBar < SerializableFoo
561+
@[JSON::Field(converter: SerializableFoo::Converter)]
562+
getter foo : SerializableFoo = SerializableBaz.new
563+
end
564+
565+
class SerializableBaz < SerializableFoo
566+
def initialize
567+
end
568+
end
569+
518570
class JSONInitializeOpts
519571
include JSON::Serializable
520572

@@ -1182,6 +1234,112 @@ describe "JSON mapping" do
11821234
end
11831235
end
11841236

1237+
describe "Enum::ValueConverter.from_json" do
1238+
it "normal enum" do
1239+
JSONAttrWithEnumValue.from_json(%({"value": 0})).value.should eq(JSONSerializableEnum::Zero)
1240+
JSONAttrWithEnumValue.from_json(%({"value": 1})).value.should eq(JSONSerializableEnum::One)
1241+
JSONAttrWithEnumValue.from_json(%({"value": 2})).value.should eq(JSONSerializableEnum::Two)
1242+
JSONAttrWithEnumValue.from_json(%({"value": 3})).value.should eq(JSONSerializableEnum::OneHundred)
1243+
1244+
expect_raises(JSON::ParseException, %(Expected Int but was String)) do
1245+
JSONAttrWithEnumValue.from_json(%({"value": "3"}))
1246+
end
1247+
expect_raises(JSON::ParseException, %(Unknown enum JSONSerializableEnum value: 4)) do
1248+
JSONAttrWithEnumValue.from_json(%({"value": 4}))
1249+
end
1250+
expect_raises(JSON::ParseException, %(Unknown enum JSONSerializableEnum value: -1)) do
1251+
JSONAttrWithEnumValue.from_json(%({"value": -1}))
1252+
end
1253+
expect_raises(JSON::ParseException, %(Expected Int but was String)) do
1254+
JSONAttrWithEnumValue.from_json(%({"value": ""}))
1255+
end
1256+
1257+
expect_raises(JSON::ParseException, "Expected Int but was String") do
1258+
JSONAttrWithEnumValue.from_json(%({"value": "one"}))
1259+
end
1260+
1261+
expect_raises(JSON::ParseException, "Expected Int but was BeginObject") do
1262+
JSONAttrWithEnumValue.from_json(%({"value": {}}))
1263+
end
1264+
expect_raises(JSON::ParseException, "Expected Int but was BeginArray") do
1265+
JSONAttrWithEnumValue.from_json(%({"value": []}))
1266+
end
1267+
end
1268+
1269+
it "flag enum" do
1270+
JSONAttrWithFlagEnumValue.from_json(%({"value": 0})).value.should eq(JSONSerializableFlagEnum::None)
1271+
JSONAttrWithFlagEnumValue.from_json(%({"value": 1})).value.should eq(JSONSerializableFlagEnum::One)
1272+
JSONAttrWithFlagEnumValue.from_json(%({"value": 2})).value.should eq(JSONSerializableFlagEnum::Two)
1273+
JSONAttrWithFlagEnumValue.from_json(%({"value": 4})).value.should eq(JSONSerializableFlagEnum::OneHundred)
1274+
JSONAttrWithFlagEnumValue.from_json(%({"value": 5})).value.should eq(JSONSerializableFlagEnum::OneHundred | JSONSerializableFlagEnum::One)
1275+
JSONAttrWithFlagEnumValue.from_json(%({"value": 7})).value.should eq(JSONSerializableFlagEnum::All)
1276+
1277+
expect_raises(JSON::ParseException, %(Unknown enum JSONSerializableFlagEnum value: 8)) do
1278+
JSONAttrWithFlagEnumValue.from_json(%({"value": 8}))
1279+
end
1280+
expect_raises(JSON::ParseException, %(Unknown enum JSONSerializableFlagEnum value: -1)) do
1281+
JSONAttrWithFlagEnumValue.from_json(%({"value": -1}))
1282+
end
1283+
expect_raises(JSON::ParseException, %(Expected Int but was String)) do
1284+
JSONAttrWithFlagEnumValue.from_json(%({"value": ""}))
1285+
end
1286+
expect_raises(JSON::ParseException, "Expected Int but was String") do
1287+
JSONAttrWithFlagEnumValue.from_json(%({"value": "one"}))
1288+
end
1289+
expect_raises(JSON::ParseException, "Expected Int but was BeginObject") do
1290+
JSONAttrWithFlagEnumValue.from_json(%({"value": {}}))
1291+
end
1292+
expect_raises(JSON::ParseException, "Expected Int but was BeginArray") do
1293+
JSONAttrWithFlagEnumValue.from_json(%({"value": []}))
1294+
end
1295+
end
1296+
end
1297+
1298+
describe "Enum::ValueConverter.to_json" do
1299+
it "normal enum" do
1300+
klass = JSONAttrWithEnumValue
1301+
1302+
klass.new(JSONSerializableEnum::One).to_json.should eq %({"value":1})
1303+
klass.from_json(klass.new(JSONSerializableEnum::One).to_json).value
1304+
.should eq(JSONSerializableEnum::One)
1305+
1306+
klass.new(JSONSerializableEnum::OneHundred).to_json.should eq %({"value":3})
1307+
klass.from_json(klass.new(JSONSerializableEnum::OneHundred).to_json).value
1308+
.should eq(JSONSerializableEnum::OneHundred)
1309+
1310+
# undefined members can't be parsed back because the standard converter only accepts
1311+
# named members
1312+
klass.new(JSONSerializableEnum.new(42)).to_json.should eq %({"value":42})
1313+
end
1314+
1315+
it "flag enum" do
1316+
klass = JSONAttrWithFlagEnumValue
1317+
1318+
klass.new(JSONSerializableFlagEnum::One).to_json.should eq %({"value":1})
1319+
klass.from_json(klass.new(JSONSerializableFlagEnum::One).to_json).value
1320+
.should eq(JSONSerializableFlagEnum::One)
1321+
1322+
klass.new(JSONSerializableFlagEnum::OneHundred).to_json.should eq %({"value":4})
1323+
klass.from_json(klass.new(JSONSerializableFlagEnum::OneHundred).to_json).value
1324+
.should eq(JSONSerializableFlagEnum::OneHundred)
1325+
1326+
combined = JSONSerializableFlagEnum::OneHundred | JSONSerializableFlagEnum::One
1327+
1328+
klass.new(combined).to_json.should eq %({"value":5})
1329+
klass.from_json(klass.new(combined).to_json).value.should eq(combined)
1330+
1331+
klass.new(JSONSerializableFlagEnum::None).to_json.should eq %({"value":0})
1332+
klass.from_json(klass.new(JSONSerializableFlagEnum::None).to_json).value
1333+
.should eq(JSONSerializableFlagEnum::None)
1334+
1335+
klass.new(JSONSerializableFlagEnum::All).to_json.should eq %({"value":7})
1336+
klass.from_json(klass.new(JSONSerializableFlagEnum::All).to_json).value
1337+
.should eq(JSONSerializableFlagEnum::All)
1338+
1339+
klass.new(JSONSerializableFlagEnum.new(42)).to_json.should eq %({"value":42})
1340+
end
1341+
end
1342+
11851343
describe "namespaced classes" do
11861344
it "lets default values use the object's own namespace" do
11871345
request = JSONNamespace::FooRequest.from_json(%({"foo":{}}))
@@ -1201,4 +1359,8 @@ describe "JSON mapping" do
12011359
it "supports generic type variables in converters" do
12021360
JSONAttrWithGenericConverter(Time::EpochConverter).from_json(%({"value":1459859781})).value.should eq(Time.unix(1459859781))
12031361
end
1362+
1363+
it "fixes #16141" do
1364+
SerializableFoo.find.from_json("{}").should be_a(SerializableBar)
1365+
end
12041366
end

‎spec/std/json/serialization_spec.cr‎

Lines changed: 0 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -389,67 +389,6 @@ describe "JSON serialization" do
389389
end
390390
end
391391

392-
describe "Enum::ValueConverter.from_json" do
393-
it "normal enum" do
394-
Enum::ValueConverter(JSONSpecEnum).from_json("0").should eq(JSONSpecEnum::Zero)
395-
Enum::ValueConverter(JSONSpecEnum).from_json("1").should eq(JSONSpecEnum::One)
396-
Enum::ValueConverter(JSONSpecEnum).from_json("2").should eq(JSONSpecEnum::Two)
397-
Enum::ValueConverter(JSONSpecEnum).from_json("3").should eq(JSONSpecEnum::OneHundred)
398-
399-
expect_raises(JSON::ParseException, %(Expected Int but was String)) do
400-
Enum::ValueConverter(JSONSpecEnum).from_json(%("3"))
401-
end
402-
expect_raises(JSON::ParseException, %(Unknown enum JSONSpecEnum value: 4)) do
403-
Enum::ValueConverter(JSONSpecEnum).from_json("4")
404-
end
405-
expect_raises(JSON::ParseException, %(Unknown enum JSONSpecEnum value: -1)) do
406-
Enum::ValueConverter(JSONSpecEnum).from_json("-1")
407-
end
408-
expect_raises(JSON::ParseException, %(Expected Int but was String)) do
409-
Enum::ValueConverter(JSONSpecEnum).from_json(%(""))
410-
end
411-
412-
expect_raises(JSON::ParseException, "Expected Int but was String") do
413-
Enum::ValueConverter(JSONSpecEnum).from_json(%("one"))
414-
end
415-
416-
expect_raises(JSON::ParseException, "Expected Int but was BeginObject") do
417-
Enum::ValueConverter(JSONSpecEnum).from_json(%({}))
418-
end
419-
expect_raises(JSON::ParseException, "Expected Int but was BeginArray") do
420-
Enum::ValueConverter(JSONSpecEnum).from_json(%([]))
421-
end
422-
end
423-
424-
it "flag enum" do
425-
Enum::ValueConverter(JSONSpecFlagEnum).from_json("0").should eq(JSONSpecFlagEnum::None)
426-
Enum::ValueConverter(JSONSpecFlagEnum).from_json("1").should eq(JSONSpecFlagEnum::One)
427-
Enum::ValueConverter(JSONSpecFlagEnum).from_json("2").should eq(JSONSpecFlagEnum::Two)
428-
Enum::ValueConverter(JSONSpecFlagEnum).from_json("4").should eq(JSONSpecFlagEnum::OneHundred)
429-
Enum::ValueConverter(JSONSpecFlagEnum).from_json("5").should eq(JSONSpecFlagEnum::OneHundred | JSONSpecFlagEnum::One)
430-
Enum::ValueConverter(JSONSpecFlagEnum).from_json("7").should eq(JSONSpecFlagEnum::All)
431-
432-
expect_raises(JSON::ParseException, %(Unknown enum JSONSpecFlagEnum value: 8)) do
433-
Enum::ValueConverter(JSONSpecFlagEnum).from_json("8")
434-
end
435-
expect_raises(JSON::ParseException, %(Unknown enum JSONSpecFlagEnum value: -1)) do
436-
Enum::ValueConverter(JSONSpecFlagEnum).from_json("-1")
437-
end
438-
expect_raises(JSON::ParseException, %(Expected Int but was String)) do
439-
Enum::ValueConverter(JSONSpecFlagEnum).from_json(%(""))
440-
end
441-
expect_raises(JSON::ParseException, "Expected Int but was String") do
442-
Enum::ValueConverter(JSONSpecFlagEnum).from_json(%("one"))
443-
end
444-
expect_raises(JSON::ParseException, "Expected Int but was BeginObject") do
445-
Enum::ValueConverter(JSONSpecFlagEnum).from_json(%({}))
446-
end
447-
expect_raises(JSON::ParseException, "Expected Int but was BeginArray") do
448-
Enum::ValueConverter(JSONSpecFlagEnum).from_json(%([]))
449-
end
450-
end
451-
end
452-
453392
it "deserializes with root" do
454393
Int32.from_json(%({"foo": 1}), root: "foo").should eq(1)
455394
Array(Int32).from_json(%({"foo": [1, 2]}), root: "foo").should eq([1, 2])
@@ -703,42 +642,6 @@ describe "JSON serialization" do
703642
end
704643
end
705644

706-
describe "Enum::ValueConverter" do
707-
it "normal enum" do
708-
converter = Enum::ValueConverter(JSONSpecEnum)
709-
converter.to_json(JSONSpecEnum::One).should eq %(1)
710-
converter.from_json(converter.to_json(JSONSpecEnum::One)).should eq(JSONSpecEnum::One)
711-
712-
converter.to_json(JSONSpecEnum::OneHundred).should eq %(3)
713-
converter.from_json(converter.to_json(JSONSpecEnum::OneHundred)).should eq(JSONSpecEnum::OneHundred)
714-
715-
# undefined members can't be parsed back because the standard converter only accepts named
716-
# members
717-
converter.to_json(JSONSpecEnum.new(42)).should eq %(42)
718-
end
719-
720-
it "flag enum" do
721-
converter = Enum::ValueConverter(JSONSpecFlagEnum)
722-
converter.to_json(JSONSpecFlagEnum::One).should eq %(1)
723-
converter.from_json(converter.to_json(JSONSpecFlagEnum::One)).should eq(JSONSpecFlagEnum::One)
724-
725-
converter.to_json(JSONSpecFlagEnum::OneHundred).should eq %(4)
726-
converter.from_json(converter.to_json(JSONSpecFlagEnum::OneHundred)).should eq(JSONSpecFlagEnum::OneHundred)
727-
728-
combined = JSONSpecFlagEnum::OneHundred | JSONSpecFlagEnum::One
729-
converter.to_json(combined).should eq %(5)
730-
converter.from_json(converter.to_json(combined)).should eq(combined)
731-
732-
converter.to_json(JSONSpecFlagEnum::None).should eq %(0)
733-
converter.from_json(converter.to_json(JSONSpecFlagEnum::None)).should eq(JSONSpecFlagEnum::None)
734-
735-
converter.to_json(JSONSpecFlagEnum::All).should eq %(7)
736-
converter.from_json(converter.to_json(JSONSpecFlagEnum::All)).should eq(JSONSpecFlagEnum::All)
737-
738-
converter.to_json(JSONSpecFlagEnum.new(42)).should eq %(42)
739-
end
740-
end
741-
742645
it "does for BigInt" do
743646
big = BigInt.new("123456789123456789123456789123456789123456789")
744647
big.to_json.should eq("123456789123456789123456789123456789123456789")

‎src/json/from_json.cr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
# Int32.from_json("1") # => 1
99
# Array(Int32).from_json("[1, 2, 3]") # => [1, 2, 3]
1010
# ```
11-
def Object.from_json(string_or_io : String | IO) : Object
11+
def Object.from_json(string_or_io : String | IO)
1212
parser = JSON::PullParser.new(string_or_io)
1313
new parser
1414
end
@@ -21,7 +21,7 @@ end
2121
# ```
2222
# Int32.from_json(%({"main": 1}), root: "main") # => 1
2323
# ```
24-
def Object.from_json(string_or_io : String | IO, root : String) : Object
24+
def Object.from_json(string_or_io : String | IO, root : String)
2525
parser = JSON::PullParser.new(string_or_io)
2626
parser.on_key!(root) do
2727
new parser

0 commit comments

Comments
 (0)