rom-rb / rom

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

Associations not registered when using classes #449

Closed simonc closed 7 years ago

simonc commented 7 years ago

Hello,

I'm having an issue with the associations when defining my relations with classes.

Working example

rom = ROM.container(:sql, 'postgres://localhost/my-awesome-db') do |conf|
  conf.relation(:houses) do
    schema(infer: true) do
      associations do
        belongs_to :destination
      end
    end
  end

  conf.relation(:destinations) do
    schema(infer: true) do
      associations do
        has_many :houses
      end
    end
  end
end

class DestinationRepo < ROM::Repository[:destinations]
  relations :houses
end

class HouseRepo < ROM::Repository[:houses]
  relations :destinations
end

destination_repo = DestinationRepo.new(rom)
destination_repo.aggregate(:houses).first
# => #<ROM::Struct::Destination id=1 … houses=[
# =>   #<ROM::Struct::House id=1 … destination_id=1>,
# =>   #<ROM::Struct::House id=2 … destination_id=1>
# => ]>

Broken example

rom = ROM.container(:sql, 'postgres://localhost/my-awesome-db')

class Houses < ROM::Relation[:sql]
  schema(infer: true)
    associations do
      belongs_to :destination
    end
  end
end

class Destinations < ROM::Relation[:sql]
  schema(infer: true) do
    associations do
      has_many :houses
    end
  end
end

class DestinationRepo < ROM::Repository[:destinations]
  relations :houses
end

class HouseRepo < ROM::Repository[:houses]
  relations :destinations
end

destination_repo = DestinationRepo.new(rom)
destination_repo.aggregate(:houses).to_a
# ~> /Users/simonc/.gem/ruby/2.4.0/gems/rom-3.3.1/lib/rom/registry.rb:35:in `block in fetch': :houses doesn't exist in ROM::AssociationSet registry (ROM::Registry::ElementNotFoundError)
# ~>    from /Users/simonc/.gem/ruby/2.4.0/gems/rom-3.3.1/lib/rom/registry.rb:32:in `fetch'
# ~>    from /Users/simonc/.gem/ruby/2.4.0/gems/rom-3.3.1/lib/rom/registry.rb:32:in `fetch'
# ~>    from /Users/simonc/.gem/ruby/2.4.0/gems/rom-3.3.1/lib/rom/association_set.rb:39:in `[]'
# ~>    from /Users/simonc/.gem/ruby/2.4.0/gems/rom-repository-1.4.0/lib/rom/repository/relation_proxy/combine.rb:267:in `combine_opts_for_assoc'
# ~>    from /Users/simonc/.gem/ruby/2.4.0/gems/rom-repository-1.4.0/lib/rom/repository/relation_proxy/combine.rb:95:in `block in combine'
# ~>    from /Users/simonc/.gem/ruby/2.4.0/gems/rom-repository-1.4.0/lib/rom/repository/relation_proxy/combine.rb:62:in `each'
# ~>    from /Users/simonc/.gem/ruby/2.4.0/gems/rom-repository-1.4.0/lib/rom/repository/relation_proxy/combine.rb:62:in `combine'
# ~>    from /Users/simonc/.gem/ruby/2.4.0/gems/rom-repository-1.4.0/lib/rom/repository/root.rb:101:in `aggregate'
# ~>    from -:67:in `<main>'

Additional details

My Gemfile contains the following:

source "https://rubygems.org"

gem "pg"
gem "rom"
gem "rom-repository"
gem "rom-sql"

I'm running with the following versions

ROM::VERSION # => "3.3.1"
ROM::Repository::VERSION # => "1.4.0"
ROM::SQL::VERSION # => "1.3.3"

Expected behavior

I would expect the second version to work the same way than the first one. Both versions are written in a single file. I didn't find anything indicating that I need to register something manually for the second version.

Is it a ROM bug or a misunderstanding of how things are supposed to work?

Thank you very much ❤️

solnic commented 7 years ago

Unfortunately you're yet another "victim" of relation inference. This is a feature that was removed from rom 4.0.0 (still in beta), because it bites people like in your case. What's happening here is that you defined classes after finalizing rom setup, which means rom doesn't know about your relation classes but in 3.x rom infers relations from the database schema automatically but it can't infer associations. Anyway, relation inference is gone in rom 4.0, so this won't happen again.

Please check out explicit setup docs to learn how to register your classes manually or use auto-registration feature.

simonc commented 7 years ago

@solnic Ok I see, I've been looking for this indication but sadly I skipped the explicit setup part thinking the info wouldn't appear in the "setup" part.

I think the Relations doc should include this line in at least one example. As of today it assumes that all the previous pages have been read and it rarely is the case. Devs have a tendency to muddle through things in my XP so it would make sense to hint here and there about registration, even if it means repeating the info. (DRY does not apply to docs 🙂)

Maybe you already addressed this issue in the 4.0.0. Otherwise I'm willing to open a PR in this direction.

Cheers ❤️

solnic commented 7 years ago

@simonc thanks for the feedback re docs, I totally agree with you. Before 4.0 final is released, we'll revamp the docs completely, it's one of the biggest tasks in this release, actually :)

simonc commented 7 years ago

@solnic awesome 😊