rom-rb / rom-factory

Data generator with support for persistence backends
MIT License
83 stars 42 forks source link

Attributes on struct (but not relation) raises error when defining a Factory #90

Open cllns opened 2 months ago

cllns commented 2 months ago

Describe the bug

I have a custom struct for Article that holds the number of favorites attached to it (as favorites_count). This is computed via a join with the favorites relation.

When trying to create a Factory for Article that has a favorites_count specified, I get an error on ROM::Factory::DSL saying the favorites_count is not defined.

To Reproduce

require "rom"
require "rom-factory"

Types = Dry.Types()
module Structs
  class Article < ROM::Struct
    # Including `title` and `body` is optional, we are able to declare struct-only attributes
    # If you comment these two lines out, we get the same result and same error
    attribute :title, Types::String
    attribute :body, Types::String
    attribute :favorites_count, Types::Integer
  end
end

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

  conf.default.create_table(:articles) do
    primary_key :id
    column :title, String, null: false
    column :body, String, null: false
  end

  conf.default.create_table(:favorites) do
    primary_key %i[user_id article_id]

    foreign_key :user_id, :users
    foreign_key :article_id, :articles
  end

  conf.relation(:users) do
    schema(infer: true)
  end

  conf.relation(:articles) do
    schema(infer: true) do
      associations do
        has_many :favorites
      end
    end

    def with_favorites_count
      left_join(favorites, {id: :article_id})
        .group(:id)
        .select_append { integer.count(:user_id).as(:favorites_count) }
    end
  end

  conf.relation(:favorites) do
    schema(infer: true) do
      associations do
        belongs_to :user
        belongs_to :article
      end
    end
  end
end

class ArticleRepo < ROM::Repository[:articles]
  struct_namespace Structs
  auto_struct true

  def with_favorites_count
    articles.with_favorites_count
  end
end

user1_id = rom.relations[:users].insert(email: "user1@example.com")
user2_id = rom.relations[:users].insert(email: "user2@example.com")
article_id = rom.relations[:articles].insert(title: "Title", body: "Body")

rom.relations[:favorites].insert(user_id: user1_id, article_id: article_id)
rom.relations[:favorites].insert(user_id: user2_id, article_id: article_id)

p ArticleRepo.new(rom).with_favorites_count.to_a
#=> [#<Article title="Title" body="Body" favorites_count=2>]

MyFactory = ROM::Factory.configure do |config|
  config.rom = rom
end

MyFactory.define(:article, struct_namespace: Structs) do |f|
  f.title "Title"
  f.favorites_count(123)
end

# Raises following error:
# ~/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/rom-factory-0.12.0/lib/rom/factory/dsl.rb:172:in `method_missing': undefined method `favorites_count' for an instance of ROM::Factory::DSL (NoMethodError)
#   from example.rb:85:in `block in <main>'
#   from /Users/sean/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/rom-factory-0.12.0/lib/rom/factory/dsl.rb:48:in `initialize'
#   from /Users/sean/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/rom-factory-0.12.0/lib/rom/factory/factories.rb:143:in `new'
#   from /Users/sean/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/rom-factory-0.12.0/lib/rom/factory/factories.rb:143:in `define'
#   from example.rb:83:in `<main>'

Expected behavior

I would expect that I could define the factory as above. Then be able to use it with MyFactory[:article] and MyFactory.structs[:article] where the Struct::Article would have a favorites_count of 123.

I think I would expect rom-factory to infer the attributes from the relation, but then also look at the associated struct (in the struct_namespace) to see if there are any other attributes there and allow those to be declared in the factory as well.

This may not be the proper way of doing this, so I'm open to feedback on the approach, and workarounds.

My environment

cllns commented 1 month ago

This just got me again.

My workaround for this not working was adding a default value in the struct with attribute :favorites_count, Types::Integer.default(0) instead of in the factory definition.

It can properly create a struct, and it even fills in this favorites_count = 0.

Then, I use a Repo to find that record I just created, and I get it back, with favorites_count also = 0 in this case.

But then I get article_repo.find_by_id(123) == factory_article as false, even though article_repo.find_by_id(123).to_h == factory_article.to_h is true. This is very surprising because they both appear to have the same class but they don't (see rom-rb/rom#610), when looking at their object_id's, one can see they're different. I looked into ROM::StructCompiler and indeed, the first time it's compiled (from the factory), it's missing the favorites_count, so it's a different Struct class than the one that does have favorites_count.

solnic commented 1 month ago

Factories are defined based on relation schemas, not struct schemas. If we wanted to support projected attributes we'd need a way of telling the factory that they exist. This can be done by defining your projected schema using view DSL in relations. They are not supported by factory but we could make them work.

cllns commented 1 month ago

What if the definitions are still based on the relation, but the actual Factory object creation would allow values that exist on the Struct that's associated with the factory?

e.g. Factory[:article, favorites_count: 123]

Right now that raises this error: Unknown attributes: favorites_count (ROM::Factory::UnknownFactoryAttributes)

I'm not sure if that's simpler to implement.

Or, alternately, what if there were a syntax in defining the Factories that would let users define arbitrary attributes? After all, users can select_append arbitrary tuples that get mapped to the struct, they're already "open" structs.

e.g.

MyFactory.define(:article, struct_namespace: Structs) do |f|
  f.title "Title"

  f.open do |o|
    o.favorites_count(123) # which exists on the schema for the associated struct
    o.promoted(false) # which doesn't need to exist on the schema for the associated struct
  end
end
solnic commented 1 month ago

We could have a DSL for defining factories for custom structs too, like MyFactory.define(Article) do ...