Skip to content

Commit cedae51

Browse files
fatkodimafatkodima
committed
Add new checker for incorrectly configured associations
1 parent 10411d4 commit cedae51

File tree

7 files changed

+1315
-1
lines changed

7 files changed

+1315
-1
lines changed

‎.rubocop.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,14 @@ Style/EmptyElse:
4545
Style/NegatedIf:
4646
Enabled: false
4747

48+
# Breaks the linear flow.
49+
Style/Next:
50+
Enabled: false
51+
52+
# Breaks the linear flow.
53+
Style/GuardClause:
54+
Enabled: false
55+
4856
# We allow the old key-value syntax (key => value) in Rakefiles as that's how
4957
# we define task dependencies.
5058
Style/HashSyntax:

‎README.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ can detect:
1313
* incorrect presence validations on boolean columns - [`active_record_doctor:incorrect_boolean_presence_validation`](#detecting-incorrect-presence-validations-on-boolean-columns)
1414
* mismatches between model length validations and database validation constraints - [`active_record_doctor:incorrect_length_validation`](#detecting-incorrect-length-validation)
1515
* incorrect values of `dependent` on associations - [`active_record_doctor:incorrect_dependent_option`](#detecting-incorrect-dependent-option-on-associations)
16+
* incorrectly configured associations - [`active_record_doctor:incorrect_association`](#detecting-incorrectly-configured-associations)
1617
* primary keys having short integer types - [`active_record_doctor:short_primary_key_type`](#detecting-primary-keys-having-short-integer-types)
1718
* mismatched foreign key types - [`active_record_doctor:mismatched_foreign_key_type`](#detecting-mismatched-foreign-key-types)
1819
* tables without primary keys - [`active_record_doctor:table_without_primary_key`](#detecting-tables-without-primary-keys)
@@ -552,6 +553,31 @@ use `dependent: :destroy` or similar on Post.comments - the associated model has
552553

553554
Supported configuration options:
554555

556+
- `enabled` - set to `false` to disable the detector altogether
557+
- `ignore_models` - models whose associations should not be checked.
558+
- `ignore_associations` - associations, written as Model.association, that should not
559+
be checked.
560+
561+
### Detecting Incorrectly Configured Associations
562+
563+
There are many reasons for the association to be configured incorrectly - the associated model
564+
or table does not exist, the provided `:through` association does not exist etc.
565+
566+
To detect incorrectly configured associations run the following command:
567+
568+
```
569+
bundle exec rake active_record_doctor:incorrect_association
570+
```
571+
572+
The output of the command looks like this:
573+
574+
```
575+
association User.company is incorrect - associated 'Company' model does not exist
576+
association Post.comments is incorrect - associated 'comments' table does not exist
577+
```
578+
579+
Supported configuration options:
580+
555581
- `enabled` - set to `false` to disable the detector altogether
556582
- `ignore_models` - models whose associations should not be checked.
557583
- `ignore_associations` - associations, written as Model.association, that should not

‎lib/active_record_doctor.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
require "active_record_doctor/detectors/mismatched_foreign_key_type"
2323
require "active_record_doctor/detectors/table_without_primary_key"
2424
require "active_record_doctor/detectors/table_without_timestamps"
25+
require "active_record_doctor/detectors/incorrect_association"
2526
require "active_record_doctor/errors"
2627
require "active_record_doctor/help"
2728
require "active_record_doctor/runner"

‎lib/active_record_doctor/config/default.rb‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@
2929
ignore_models: [],
3030
ignore_associations: []
3131

32+
detector :incorrect_association,
33+
enabled: true,
34+
ignore_models: [],
35+
ignore_associations: []
36+
3237
detector :mismatched_foreign_key_type,
3338
enabled: true,
3439
ignore_tables: [],

‎lib/active_record_doctor/detectors/base.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,8 @@ def each_data_source(except: [])
290290
end
291291
end
292292

293-
def each_association(model, except: [], type: [:has_many, :has_one, :belongs_to], has_scope: nil, through: nil)
293+
def each_association(model, except: [], type: [:has_many, :has_one, :belongs_to, :has_and_belongs_to_many],
294+
has_scope: nil, through: nil)
294295
type = Array(type)
295296

296297
log("Iterating over associations on #{model.name}") do

‎lib/active_record_doctor/detectors/incorrect_association.rb‎

Lines changed: 303 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,303 @@
1+
# frozen_string_literal: true
2+
3+
require "active_record_doctor/detectors/base"
4+
5+
module ActiveRecordDoctor
6+
module Detectors
7+
class IncorrectAssociation < Base # :nodoc:
8+
@description = "detect incorrectly configured associations"
9+
@config = {
10+
ignore_models: {
11+
description: "models whose associations should not be checked",
12+
global: true
13+
},
14+
ignore_associations: {
15+
description: "associations, written as Model.association, that should not be checked"
16+
}
17+
}
18+
19+
private
20+
21+
def message(model:, association:, reason:)
22+
"association #{model.name}.#{association.name} is incorrect - #{reason}"
23+
end
24+
25+
def detect
26+
each_model(except: config(:ignore_models), existing_tables_only: true) do |model|
27+
each_association(model, except: config(:ignore_associations)) do |association|
28+
case association.macro
29+
when :belongs_to
30+
check_belongs_to(model, association)
31+
when :has_many
32+
check_has_many(model, association)
33+
when :has_one
34+
check_has_one(model, association)
35+
when :has_and_belongs_to_many
36+
check_has_and_belongs_to_many(model, association)
37+
end
38+
end
39+
end
40+
end
41+
42+
def check_belongs_to(model, association)
43+
column_names = model.column_names
44+
45+
if association.polymorphic?
46+
# Check :foreign_type option
47+
unless column_names.include?(association.foreign_type)
48+
problem!(
49+
model: model,
50+
association: association,
51+
reason: "there is no '#{model.table_name}.#{association.foreign_type}' column"
52+
)
53+
end
54+
else
55+
success = check_association_model(model, association)
56+
return unless success
57+
58+
association_model = association.klass
59+
60+
# Check :foreign_key option
61+
unless column_names.include?(association.foreign_key)
62+
problem!(
63+
model: model,
64+
association: association,
65+
reason: "there is no '#{model.table_name}.#{association.foreign_key}' column"
66+
)
67+
end
68+
69+
check_primary_key(model, association)
70+
association_columns = association_model.column_names
71+
72+
# Check :counter_cache option
73+
counter_cache_column = association.counter_cache_column
74+
if association.options[:counter_cache] && !association_columns.include?(counter_cache_column)
75+
problem!(
76+
model: model,
77+
association: association,
78+
reason: "there is no '#{association_model.table_name}.#{counter_cache_column}' counter cache column"
79+
)
80+
end
81+
82+
check_touch_option(model, association)
83+
84+
# Check :inverse_of option
85+
if incorrect_inverse_of?(association)
86+
problem!(
87+
model: model,
88+
association: association,
89+
reason: "there is no '#{association.class_name}.#{association.options[:inverse_of]}' association"
90+
)
91+
end
92+
end
93+
end
94+
95+
def check_has_many(model, association)
96+
if association.through_reflection?
97+
check_source_association(model, association)
98+
else
99+
success = check_association_model(model, association)
100+
return unless success
101+
102+
association_model = association.klass
103+
104+
column_names = model.column_names
105+
association_columns = association_model.column_names
106+
107+
# Check :foreign_key option
108+
# Don't do foreign_key check if inverse_of is incorrect, because Active Record
109+
# uses inverse_of for getting foreign key.
110+
if !incorrect_inverse_of?(association) && !association_columns.include?(association.foreign_key)
111+
problem!(
112+
model: model,
113+
association: association,
114+
reason: "there is no '#{association_model.table_name}.#{association.foreign_key}' column"
115+
)
116+
end
117+
118+
# Check :foreign_type option
119+
if association.type && !association_columns.include?(association.type)
120+
problem!(
121+
model: model,
122+
association: association,
123+
reason: "there is no '#{association_model.table_name}.#{association.type}' column"
124+
)
125+
end
126+
127+
check_primary_key(model, association)
128+
129+
# Check :counter_cache option
130+
if association.options[:counter_cache] && !column_names.include?(association.counter_cache_column)
131+
problem!(
132+
model: model,
133+
association: association,
134+
reason: "there is no '#{model.table_name}.#{association.counter_cache_column}' counter cache column"
135+
)
136+
end
137+
138+
# Check :inverse_of option
139+
if incorrect_inverse_of?(association)
140+
problem!(
141+
model: model,
142+
association: association,
143+
reason: "there is no '#{association.class_name}.#{association.options[:inverse_of]}' association"
144+
)
145+
end
146+
end
147+
end
148+
149+
def check_has_one(model, association)
150+
if association.through_reflection?
151+
check_source_association(model, association)
152+
else
153+
success = check_association_model(model, association)
154+
return unless success
155+
156+
association_model = association.klass
157+
158+
association_columns = association_model.column_names
159+
160+
# Check :foreign_key option
161+
if !incorrect_inverse_of?(association) && !association_columns.include?(association.foreign_key)
162+
problem!(
163+
model: model,
164+
association: association,
165+
reason: "there is no '#{association_model.table_name}.#{association.foreign_key}' column"
166+
)
167+
end
168+
169+
# Check :foreign_type option
170+
if association.type && !association_columns.include?(association.type)
171+
problem!(
172+
model: model,
173+
association: association,
174+
reason: "there is no '#{association_model.table_name}.#{association.type}' column"
175+
)
176+
end
177+
178+
check_primary_key(model, association)
179+
check_touch_option(model, association)
180+
181+
# Check :inverse_of option
182+
if incorrect_inverse_of?(association)
183+
problem!(
184+
model: model,
185+
association: association,
186+
reason: "there is no '#{association.class_name}.#{association.options[:inverse_of]}' association"
187+
)
188+
end
189+
end
190+
end
191+
192+
def check_has_and_belongs_to_many(model, association)
193+
success = check_association_model(model, association)
194+
return unless success
195+
196+
join_table = association.join_table
197+
198+
# Check that join table exists
199+
if connection.data_source_exists?(join_table)
200+
# Check :foreign_key and :association_foreign_key options
201+
[association.foreign_key, association.association_foreign_key].each do |foreign_key|
202+
unless column(join_table, foreign_key)
203+
problem!(
204+
model: model,
205+
association: association,
206+
reason: "there is no '#{join_table}.#{foreign_key}' column"
207+
)
208+
end
209+
end
210+
else
211+
problem!(
212+
model: model,
213+
association: association,
214+
reason: "associated '#{join_table}' table does not exist"
215+
)
216+
end
217+
end
218+
219+
def check_association_model(model, association)
220+
association_model = association.klass
221+
222+
if association_model.table_exists?
223+
true
224+
else
225+
problem!(
226+
model: model,
227+
association: association,
228+
reason: "associated '#{association_model.table_name}' table does not exist"
229+
)
230+
false
231+
end
232+
rescue NameError
233+
problem!(
234+
model: model,
235+
association: association,
236+
reason: "associated '#{association.class_name}' model does not exist"
237+
)
238+
239+
false
240+
end
241+
242+
def check_primary_key(model, association)
243+
# Can't use association.active_record_primary_key, because it raises
244+
# if model does not have a primary key.
245+
primary_key = association.options[:primary_key]&.to_s || model.primary_key&.to_s || "id"
246+
247+
unless model.column_names.include?(primary_key)
248+
problem!(
249+
model: model,
250+
association: association,
251+
reason: "there is no '#{model.table_name}.#{primary_key}' column"
252+
)
253+
end
254+
end
255+
256+
def check_touch_option(model, association)
257+
return unless (touch = association.options[:touch])
258+
259+
association_model = association.klass
260+
columns = association_model.column_names
261+
262+
unless columns.include?("updated_at") || columns.include?("updated_on")
263+
problem!(
264+
model: model,
265+
association: association,
266+
reason: "there is no '#{association_model.table_name}.updated_at' touch column"
267+
)
268+
end
269+
270+
if touch.is_a?(Symbol) && !columns.include?(touch.to_s)
271+
problem!(
272+
model: model,
273+
association: association,
274+
reason: "there is no '#{association_model.table_name}.#{touch}' touch column"
275+
)
276+
end
277+
end
278+
279+
def check_source_association(model, association)
280+
if association.through_reflection
281+
unless association.source_reflection
282+
target_model = association.through_reflection.klass.name
283+
problem!(
284+
model: model,
285+
association: association,
286+
reason: "there is no '#{association.name}' association on '#{target_model}' model"
287+
)
288+
end
289+
else
290+
problem!(
291+
model: model,
292+
association: association,
293+
reason: "there is no '#{model.name}.#{association.options[:through]}' association"
294+
)
295+
end
296+
end
297+
298+
def incorrect_inverse_of?(association)
299+
association.options[:inverse_of] && association.inverse_of.nil?
300+
end
301+
end
302+
end
303+
end

0 commit comments

Comments
 (0)