Problem with :create command and custom types

katafrakt commented 8 months ago

Describe the bug

When using custom write type and read type to modify the data before inserting / after fetching data from the database, the create command passes original unmodified data to read type, sometimes causing exceptions.

To Reproduce

require 'bundler/inline'

gemfile do
  source 'https://rubygems.org'
  gem "json"
  gem "rom-sql"
  gem "sqlite3"
end

require 'rom'
require 'rom-repository'

rom = ROM.container(:sql, 'sqlite::memory') do |conf|
  conf.default.create_table(:test) do
    primary_key :id
    column :value, String, null: false
  end

  class TestRelation < ROM::Relation[:sql]
    WriteType = Dry.Types.Constructor(String) { |value| JSON.dump({content: value}) }
    ReadType = Dry.Types.Constructor(String) { |value| JSON.parse(value)["content"] }

    schema(:test, infer: true) do
      attribute :value, WriteType, read: ReadType
    end
  end

  conf.register_relation(TestRelation)
end

class TestRepo < ROM::Repository[:test]
  commands :create
end

record = TestRepo.new(rom).create(value: "test value")

This causes:

/home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/3.2.0/json/common.rb:216:in `parse': unexpected token at 'test value' (JSON::ParserError)
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/3.2.0/json/common.rb:216:in `parse'
    from rom_custom_type_problem.rb:21:in `block in <class:TestRelation>'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/dry-types-1.7.1/lib/dry/types/constructor/function.rb:17:in `call'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/dry-types-1.7.1/lib/dry/types/constructor.rb:81:in `call_unsafe'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/dry-types-1.7.1/lib/dry/types/schema/key.rb:46:in `call_unsafe'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/dry-types-1.7.1/lib/dry/types/schema.rb:330:in `block in resolve_unsafe'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/dry-types-1.7.1/lib/dry/types/schema.rb:324:in `each'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/dry-types-1.7.1/lib/dry/types/schema.rb:324:in `resolve_unsafe'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/dry-types-1.7.1/lib/dry/types/schema.rb:60:in `call_unsafe'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/dry-types-1.7.1/lib/dry/types/constructor.rb:81:in `call_unsafe'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/dry-types-1.7.1/lib/dry/types/type.rb:47:in `call'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rom-sql-3.6.1/lib/rom/sql/commands/create.rb:41:in `block in finalize'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rom-sql-3.6.1/lib/rom/sql/commands/create.rb:41:in `map'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rom-sql-3.6.1/lib/rom/sql/commands/create.rb:41:in `finalize'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rom-core-5.3.0/lib/rom/command.rb:479:in `block in apply_hooks'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rom-core-5.3.0/lib/rom/command.rb:474:in `each'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rom-core-5.3.0/lib/rom/command.rb:474:in `reduce'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rom-core-5.3.0/lib/rom/command.rb:474:in `apply_hooks'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rom-core-5.3.0/lib/rom/command.rb:289:in `call'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rom-sql-3.6.1/lib/rom/sql/commands/error_wrapper.rb:18:in `call'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rom-core-5.3.0/lib/rom/commands/composite.rb:19:in `call'
    from /home/katafrakt/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/rom-repository-5.3.0/lib/rom/repository/class_interface.rb:130:in `block in define_command_method'
    from rom_custom_type_problem.rb:35:in `<main>'

The data is actually written correctly to the database, but it fails on trying to return the value. Stacktrace points to finalize hook of rom-sql:

        def finalize(tuples, *)
          tuples.map { |t| relation.output_schema[t] }
        end

This indeed tries to feed original data, unmodified by write type, to a read type.

Expected behavior

Creating a record using custom write and read type returns correct result.

My environment

Affects my production application: NO
Ruby version: 3.2.2
OS: Linux

katafrakt commented 8 months ago

Oh, I just realized that it works when I specify WriteType as write

# not: attribute :value, WriteType, read: ReadType
attribute :value, write: WriteType, read: ReadType

Not sure why I missed this before. It makes sense.

katafrakt commented 8 months ago

Actually, that was premature optimism. With write: is does not encode the value before putting it in the db, so naturally in this case it does not crash, but it also does not do what's desired - there's just a plain string "test value" in the database.

rom-rb / rom-sql