Wednesday, April 30, 2014
On Tuesday, 29 April 2014 03:50:39 UTC-4, ngo...@googlemail.com wrote:
Hi,
On Monday, April 28, 2014 9:31:06 PM UTC+2, Matt Jones wrote:This doesn't make any sense to me - if I request a ReportsSubject object from the database directly (via `find`, for instance), what do I get if I ask for its subject? What would the reports_subjects table even *store*? A bare `subject_id` would be insufficient since without a class_name it's unclear what table that ID refers to.
A "report subject" (a row in the join table) doesn't carry any semantics without the report, which has further information about required details. A report also only ever has a single type of subject, so IMO it would not make sense to store a couple thousand repetitions of an STI name (although note that the subjects are not all part of a single STI hierarchy!) in a column that adds no information at all to the system.
And that's not even considering what should happen when this sort of code runs:Report.joins(:subjects).where(name: 'hey wait WHAT TABLE IS THIS EVEN QUERYING')
This is not functionally different from having the polymorphic subject type specified in the join table.
There are a bunch of ways out of this situation, but the right choice depends on what exactly you need to do with the models. Starting from this use case, I'm going to discuss some tradeoffs and possible approaches. The code is more for illustration than anything else; details may be missing, etc.
class Report < ActiveRecord::Base
has_many :reports_subjects
has_many :subjects, through: :reports_subjects, class_name: ->(report) { report.subjects_type }
def subjects_type
# divine required subject model class somehow
end
end
class ReportsSubject < ActiveRecord::Base
belongs_to :report
belongs_to :subject # DOESN'T EXIST, class_name is based on parent Report instance
end
Approaches:
* STI for subjects, with a validation to ensure all the subjects are the same type. I assume this is impractical in your use case. But if it works, it's ideal - eager-loading works, adding subjects with `@report.subjects << subject` works, joins work.
None of the approaches below will work with eager-loading.
* If the set of possible subject types is known to Report (likely, since it's deciding which one to use) a polymorphic association + some metaprogramming could work:
class Report < ActiveRecord::Base
POSSIBLE_TYPES = %w(Foo Bar Baz)
has_many :reports_subjects
POSSIBLE_TYPES.each do |type|
has_many "#{type.underscore}_subjects", through: :reports_subjects, source: :subject, source_type: type
end
def subjects(reload=false)
send("#{subjects_type.underscore}_subjects", reload)
end
def subjects_type
# divine required subject model class somehow
# return a *string* from POSSIBLE_TYPES
end
end
class ReportsSubject < ActiveRecord::Base
belongs_to :report
belongs_to :subject, polymorphic: true
end
This gives a "faux" association `subjects` that behaves mostly like a real association:
@report.subjects.to_a # works
@report.subjects << new_subject # works, raises if you try to push an object that isn't of subjects_type
@report.subjects.count # works
but not quite:
Report.joins(:subjects) # unknown association 'subjects'
This approach does require a `subject_type` column on reports_subjects. Worries about the performance impact of storing duplicate string values many times is (IMO) premature optimization, but your use case may be different.
* If a `subject_type` column is unfeasible for reasons, there's another way which drops more ActiveRecord machinery.
class Report < ActiveRecord::Base
has_many :reports_subjects
def subjects
subjects_type.joins(:reports_subject).where(reports_subject: { report_id: id })
end
def subjects_type
# divine required subject model class somehow
# prefer to return a real Class object here
end
end
class ReportsSubject < ActiveRecord::Base
belongs_to :report
def subject
report.subjects_type.find(subject_id)
end
end
class SomeSubjectThingy < ActiveRecord::Base
# this is needed in any class that Report can refer to; consider making it a concern
has_many :reports_subjects, foreign_key: :subject_id
end
You'll need to supply a method to actually build ReportsSubject objects for a given report, as << will not work on a report's `subjects` property. You'll also likely want to guard against inserting subjects of multiple types, as `ReportsSubject` no longer contains any information regarding what type its subject is.
Some even farther-afield approaches to think about:
* if a Report can only have one subject_type, are all Reports really instances of the same class? Perhaps Report is just a base class for more specific types (FooReport, etc) that have specific associations?
class FooReport < Report
has_many :foo_reports_subjects
has_many :subjects, through: :foo_reports_subjects
end
class FooReportsSubject < ActiveRecord::Base # or ReportsSubject, YMMV
self.table_name = 'reports_subjects'
belongs_to :report, class_name: 'FooReport'
belongs_to :subject, class_name: 'Foo'
end
Given how similar these classes are, it might make sense to metaprogram them. Note that `reports_subjects` does *not* need a `type` column here.
* if you need eager-loading but don't want to do STI subjects, maybe consider a "wrapper" object that lets you eager-load what you want. For instance, suppose that you'd like to get the name of each subject (for UI, etc) when loading a bunch of reports.
class Report < ActiveRecord::Base
POSSIBLE_TYPES = %w(Foo Bar Baz)
has_many :reports_subjects
has_many :subject_shims, through: :reports_subjects
def title
"Report on #{subject_shims.map(&:name).join(', ')}"
end
# deal with :subjects somehow
end
class ReportsSubject < ActiveRecord::Base
belongs_to :report
belongs_to :subject_shim
end
class SubjectShim < ActiveRecord::Base
# denormalize the `name` value from subject into subject_shims `name` column
has_many :reports_subjects
belongs_to :subject, polymorphic: true
end
Then you can still do this:
Report.includes(:subject_shims).each { |x| puts x.title }
without causing a SQL query per-Report. There are fewer `type` columns here (one per subject, instead of one per subject per report) but an extra database table.
---
Happy to help out if any of these look promising.
--Matt Jones
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Talk" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-talk+unsubscribe@googlegroups.com.
To post to this group, send email to rubyonrails-talk@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/rubyonrails-talk/71067b1b-42e9-4ae2-ac2d-ec8318807077%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Subscribe to:
Post Comments (Atom)
No comments:
Post a Comment