Skip to content

Commit

Permalink
Use require instead of require_dependency
Browse files Browse the repository at this point in the history
refs #45, where ActiveStorage has a circular dependency which fails to
load in one ordering (but not in the other).

As far as I can tell, `require_dependency` is mostly useful for
reloading, and since the entire use case in
`lib/consistency_fail/models.rb` is about *preloading*, this doesn't
seem necessary. Maybe this was necessary back when I was using
`ObjectSpace` as a terrible hack? Anyway, `require` it is.

This also sorts dependencies in any given `LOAD_PATH` directory before
loading them in sequence, just so whatever issues happen along these
lines in the future are more easily repeatable.
  • Loading branch information
trptcolin committed Oct 9, 2018
1 parent fc976ba commit 2e7db29
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 21 deletions.
4 changes: 4 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@ source "https://rubygems.org"
gemspec

gem 'pry'

group :test do
gem 'activesupport', '~> 5.0'
end
5 changes: 3 additions & 2 deletions lib/consistency_fail/models.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ def dirs

def preload_all
self.dirs.each do |d|
Dir.glob(File.join(d, "**", "*.rb")).each do |model_filename|
Kernel.require_dependency model_filename
ruby_files = Dir.glob(File.join(d, "**", "*.rb")).sort
ruby_files.each do |model_filename|
Kernel.require model_filename
end
end
end
Expand Down
27 changes: 15 additions & 12 deletions spec/index_spec.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
require_relative 'support/models/correct_address'
require_relative 'support/models/wrong_address'

describe ConsistencyFail::Index do

let(:index) do
ConsistencyFail::Index.new(
CorrectAddress,
CorrectAddress.table_name,
CorrectAddress,
CorrectAddress.table_name,
["city", "state"]
)
end

describe "value objectiness" do
it "holds onto model, table name, and columns" do
expect(index.model).to eq(CorrectAddress)
Expand All @@ -24,12 +27,12 @@
end
end

describe "equality test" do
describe "equality test" do
it "passes when everything matches" do
expect(index).to eq(
ConsistencyFail::Index.new(
"CorrectAddress".constantize,
"correct_addresses",
"CorrectAddress".constantize,
"correct_addresses",
["city", "state"]
)
)
Expand All @@ -38,8 +41,8 @@
it "fails when tables are different" do
expect(index).not_to eq(
ConsistencyFail::Index.new(
CorrectAttachment,
CorrectAttachment.table_name,
CorrectAttachment,
CorrectAttachment.table_name,
["attachable_id", "attachable_type"]
)
)
Expand All @@ -48,12 +51,12 @@
it "fails when columns are different" do
expect(index).not_to eq(
ConsistencyFail::Index.new(
CorrectAddress,
CorrectAddress.table_name,
CorrectAddress,
CorrectAddress.table_name,
["correct_user_id"]
)
)
end
end

end
16 changes: 10 additions & 6 deletions spec/models_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
describe ConsistencyFail::Models do

def models(load_path)
ConsistencyFail::Models.new(load_path)
end
Expand All @@ -19,7 +19,7 @@ def models(load_path)
expect(models.dirs).to eq([Pathname.new("app/models")])
end

it "preloads models by calling require_dependency" do
it "preloads models" do
models = models(["foo/bar/baz", "app/models", "some/other/models"])
allow(Dir).to receive(:glob).
with(File.join("app/models", "**", "*.rb")).
Expand All @@ -28,9 +28,9 @@ def models(load_path)
with(File.join("some/other/models", "**", "*.rb")).
and_return(["some/other/models/foo.rb"])

expect(Kernel).to receive(:require_dependency).with("app/models/user.rb")
expect(Kernel).to receive(:require_dependency).with("app/models/address.rb")
expect(Kernel).to receive(:require_dependency).with("some/other/models/foo.rb")
expect(Kernel).to receive(:require).with("app/models/user.rb")
expect(Kernel).to receive(:require).with("app/models/address.rb")
expect(Kernel).to receive(:require).with("some/other/models/foo.rb")

models.preload_all
end
Expand All @@ -44,5 +44,9 @@ def models(load_path)

expect(models([]).all).to eq([model_a, model_c, model_b])
end


it "preloads models successfully" do
models = models([File.join(File.dirname(__FILE__), "support/models")])
expect {models.preload_all}.not_to raise_error
end
end
5 changes: 4 additions & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

Bundler.require(:default, :test)

Dir['./spec/support/**/*.rb'].sort.each { |file| require file }
require_relative 'support/active_record'
require_relative 'support/schema'
require 'active_support/dependencies'
ActiveSupport::Dependencies.autoload_paths << './spec/support/models'

RSpec.configure do |config|
config.around do |example|
Expand Down
4 changes: 4 additions & 0 deletions spec/support/models/blob.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class Blob < ActiveRecord::Base
require_dependency 'blob/edible'
include Edible
end
2 changes: 2 additions & 0 deletions spec/support/models/blob/edible.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
module Blob::Edible
end
4 changes: 4 additions & 0 deletions spec/support/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,8 @@
execute 'CREATE VIEW new_correct_people AS '\
'SELECT * FROM correct_people '\
'WHERE created_at = updated_at'

create_table :blob do |t|
t.timestamps
end
end

0 comments on commit 2e7db29

Please sign in to comment.