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.

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

Deserialize fails when fetching class name from collection #2

Closed
AndiLavera opened this issue Jul 15, 2020 · 5 comments
Closed

Deserialize fails when fetching class name from collection #2

AndiLavera opened this issue Jul 15, 2020 · 5 comments

Comments

@AndiLavera
Copy link

AndiLavera commented Jul 15, 2020

This is a strange one. I played around quite a bit to recreate the bug every way i could.

Example code:

require "crystalizer/json"

class Base
  property name : String = "Base"

  macro subclasses
    {% classes = {} of String => Base %}
    {% for subclass in @type.subclasses %}
      {% classes[subclass.stringify] = subclass %}
    {% end %}
    {{ classes }}
  end
end

class Test < Base
  property myString : String = "Hello"
end

# Remove this class & the script works
class Test2 < Base
  property myString2 : String = "Hello"
end

test = Test.new
serialize_test = Crystalizer::JSON.serialize(test)

puts Base.subclasses                               # => {"Test" => Test, "Test2" => Test2}
klass = Base.subclasses["Test"]                    # => Test
# klass = {"Test" => Test, "Test2" => Test2}["Test"] # This also fails
# klass = [Test, Test2][0]                                         # Also fails
# klass = Test                                       # This works

puts klass.new.inspect # Proves the value of `klass` can be instantiated

puts Crystalizer::JSON.deserialize(serialize_test, to: klass).inspect

Error Message:

Unhandled exception: Unknown key in Base: myString (Crystalizer::Deserializer::Object::Exception)
  from lib/crystalizer/src/deserializer/object.cr:41:5 in 'deserialize'
  from lib/crystalizer/src/json/deserialize.cr:4:5 in 'deserialize'
  from test.cr:35:6 in '__crystal_main'
  from ../../../.crenv/versions/0.35.0/share/crystal/src/crystal/main.cr:105:5 in 'main_user_code'
  from ../../../.crenv/versions/0.35.0/share/crystal/src/crystal/main.cr:91:7 in 'main'
  from ../../../.crenv/versions/0.35.0/share/crystal/src/crystal/main.cr:114:3 in 'main'
  from __libc_start_main
  from _start
  from ???

Edit: Bonus Bug - Didn't want to create another bug report for the watchers. Will do so if you would like me to.

Deserialization fails when a propertys default value is IO::Memory.new. This bug occurs with getter, setter and property keywords. I can get around it with @[Crystalizer::Field(ignore: true)] as I don't need that property serialized. Just pointing it out.

@AndiLavera AndiLavera changed the title Deserialize fails when fetch class name from collection Deserialize fails when fetching class name from collection Jul 15, 2020
@j8r
Copy link
Owner

j8r commented Jul 15, 2020

That's expected, because Base.subclasses returns a Hash(String, Base) - Base.subclasses["Test"] will in fact return a Base.
Test is a Base indeed, but the language can't prove that Base.subclasses["Test"] will always return Type, a Hash being dynamic and mutable.

You definitely need a NamedTuple here, so each key can only have a type. This code works:

class Base
  macro subclasses
    {
    {% for subclass in @type.subclasses %}
      {{subclass}}: {{subclass.id}},
    {% end %}
    }
  end
  # Or also
  macro subclass(type)
    {{ @type.subclasses.find { type.id } }}
  end
end

@j8r
Copy link
Owner

j8r commented Jul 15, 2020

I reworded a bit error messages. Except that, the error is legitimate.

@AndiLavera
Copy link
Author

AndiLavera commented Jul 15, 2020

This is ultimately what I needed. Thank you! My apologies on opening an issue on my own mistake! And thanks for the awesome explaination :)

macro subclass(type)
    {{ @type.subclasses.find { type.id } }}
  end

@j8r
Copy link
Owner

j8r commented Jul 15, 2020

No problem @andrewc910 , glad to help!

@j8r j8r closed this as completed Jul 15, 2020
@j8r
Copy link
Owner

j8r commented Jul 15, 2020

I missed the second bonus bug for IO::Memory.new. That is also kind of expected.
That is a not evident point, because all objects are potentially serializable. However, in facts, some in the stdlib (including primitives) can't be, and some others we don't want to serialize them. All objects with pointers can't obviously be serialized.

Note a compile-time error will also happen when using JSON::Serializable.

I think it is fine for now, an option is to have a blacklist of unserializable stdlib objects that will be automatically ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants