search

rom-rb / rom

Data mapping and persistence toolkit for Ruby
https://rom-rb.org
MIT License
2.08k stars 161 forks source link

Chaining the same combine method causes a crash while mapping #547

Closed timriley closed 5 years ago

timriley commented 5 years ago

Describe the bug

Running a query built by calling the same combine method called twice causes an error from within tranproc's combine (while trying to map the output).

To Reproduce

Fire up Postgres, createdb rom_bug_repro, and then run this script.

To see it work properly, comment out one of the .combine(:credit_card_purchase_charge) lines at the end, and the query completes.

It's not normal to have 2 identical combines chained like this, but in a large app, where they may be spread around different relation methods, it's indeed possible (this issue came from a real app of mine).

require "bundler/setup"
require "rom"
require "rom/sql"

# Configure database URL

config = ROM::Configuration.new(:sql, "postgres://localhost/rom_bug_repro")
config.plugin(:sql, relations: :auto_restrictions)

rom = ROM.container(config)

# Migrate database

rom.gateways[:default].tap do |gateway|
  migration = gateway.migration do
    change do
      drop_table? :credit_card_purchase_charges
      drop_table? :charges

      create_table :charges do
        primary_key :id
      end

      create_table :credit_card_purchase_charges do
        column :charge_id, :integer
        primary_key [:charge_id]
        index :charge_id
        column :card_name, :text
      end
    end
  end

  migration.apply gateway.connection, :up

  # Populate data

  gateway.connection.tap do |connection|
    connection.execute("INSERT INTO charges (id) VALUES (1)")
    connection.execute("INSERT INTO credit_card_purchase_charges (charge_id, card_name) VALUES (1, 'Jane Card')")
  end
end

# Define relations

module Relations
  class Charges < ROM::Relation[:sql]
    schema :charges, infer: true do
      associations do
        has_one :credit_card_purchase_charge
      end
    end
  end

  class CreditCardPurchaseCharges < ROM::Relation[:sql]
    schema :credit_card_purchase_charges, infer: true do
      associations do
        belongs_to :charge
      end
    end
  end
end

# Register relations and finalize rom container

config.register_relation Relations::Charges
config.register_relation Relations::CreditCardPurchaseCharges

rom = ROM.container(config)

class ChargeRepo < ROM::Repository::Root[:charges]
  def for_order
    charges
      .combine(:credit_card_purchase_charge)
      .combine(:credit_card_purchase_charge)
      .to_a
  end
end

# Instantiate repository and inspect output

repo = ChargeRepo.new(rom)
p repo.for_order

Output according to script above:

Traceback (most recent call last):
        19: from rom_bug.rb:82:in `<main>'
        18: from rom_bug.rb:75:in `for_order'
        17: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rom-7245b8cb6dad/core/lib/rom/relation/materializable.rb:15:in `to_a'
        16: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rom-7245b8cb6dad/core/lib/rom/relation/combined.rb:76:in `call'
        15: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rom-7245b8cb6dad/core/lib/rom/mapper.rb:97:in `call'
        14: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rom-7245b8cb6dad/core/lib/rom/mapper.rb:97:in `reduce'
        13: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rom-7245b8cb6dad/core/lib/rom/mapper.rb:97:in `each'
        12: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/bundler/gems/rom-7245b8cb6dad/core/lib/rom/mapper.rb:97:in `block in call'
        11: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/composite.rb:30:in `call'
        10: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/function.rb:47:in `call'
         9: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/function.rb:47:in `call'
         8: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/array.rb:123:in `combine'
         7: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/array/combine.rb:11:in `combine'
         6: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/array/combine.rb:33:in `group_nodes'
         5: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/array/combine.rb:33:in `map'
         4: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/array/combine.rb:33:in `each'
         3: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/array/combine.rb:33:in `each_with_index'
         2: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/array/combine.rb:33:in `each'
         1: from /Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/array/combine.rb:35:in `block in group_nodes'
/Users/tim/.asdf/installs/ruby/2.5.1/lib/ruby/gems/2.5.0/gems/transproc-1.0.2/lib/transproc/array/combine.rb:40:in `group_candidates': undefined method `[]' for nil:NilClass (NoMethodError)

Output when commenting out one of the combines:

[#<ROM::Struct::Charge id=1 credit_card_purchase_charge=#<ROM::Struct::CreditCardPurchaseCharge charge_id=1 card_name="Jane Card">>]

Expected behavior

The second combine is ignored or gracefully handled in some way, and the actual results are returned.

Your environment

n/a

solnic commented 5 years ago

This seems to work in #548 

[ruby-2.6.3][node-v8.12.0] git(547-fix-dupped-combine) 
➜  rom be ruby tmp/repros/547.rb 
[#<ROM::Struct::Charge id=1 credit_card_purchase_charge=#<ROM::Struct::CreditCardPurchaseCharge charge_id=1 card_name="Jane Card">>]