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

Problem with :create command and custom types #423

Open
katafrakt opened this issue Nov 5, 2023 · 3 comments
Open

Problem with :create command and custom types #423

katafrakt opened this issue Nov 5, 2023 · 3 comments

Comments

@katafrakt
Copy link

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
Copy link
Author

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
Copy link
Author

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.

@katafrakt katafrakt reopened this Nov 6, 2023
@katafrakt
Copy link
Author

I did some more digging and found out that this problem is only present for SQlite and MySQL. Or, to be more precise, is not a problem in PostgreSQL. This lead me to discovery that it's because PostgreSQL has it's own implementation of insert for create command. The default insert implementation is broken and tries to apply read type twice - first when relation.where(...).to_a is called at the end of insert and after that in finalize hook. For PostgreSQL the insert does not use relation, so the type is onl applied during finalize.

I think that's enough for me to prepare a PR with a fix.

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

No branches or pull requests

1 participant