Skip to content

Commit

Permalink
Add instance_specific_default plugin for setting default association …
Browse files Browse the repository at this point in the history
…:instance_specific value, or warning/raising for cases where it is not specified

This makes it easier to find cases where the :instance_specific
option can be set to false for better performance.
  • Loading branch information
jeremyevans committed Jul 16, 2020
1 parent 232fd3b commit 9727d92
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
=== master

* Add instance_specific_default plugin for setting default association :instance_specific value, or warning/raising for cases where it is not specified (jeremyevans)

* Make Model.finalize_associations not affect instance-specific associations (jeremyevans)

* Make Model.plugin issue deprecation warning if loading plugin with arguments and block if plugin does not accept arguments/block (jeremyevans)
Expand Down
8 changes: 7 additions & 1 deletion lib/sequel/model/associations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1798,7 +1798,7 @@ def associate(type, name, opts = OPTS, &block)
# It's possible the association is instance specific, in that it depends on
# values other than the foreign key value. This needs to be checked for
# in certain places to disable optimizations.
opts[:instance_specific] = true
opts[:instance_specific] = _association_instance_specific_default(name)
end
opts = assoc_class.new.merge!(opts)

Expand Down Expand Up @@ -1902,6 +1902,12 @@ def one_to_one(name, opts=OPTS, &block)
Plugins.def_dataset_methods(self, [:eager, :eager_graph, :eager_graph_with_options, :association_join, :association_full_join, :association_inner_join, :association_left_join, :association_right_join])

private

# The default value for the instance_specific option, if the association
# could be instance specific and the :instance_specific option is not specified.
def _association_instance_specific_default(_)
true
end

# The module to use for the association's methods. Defaults to
# the overridable_methods_module.
Expand Down
94 changes: 94 additions & 0 deletions lib/sequel/plugins/instance_specific_default.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# frozen-string-literal: true

module Sequel
module Plugins
# The instance_specific_default plugin exists to make it easier to use a
# global :instance_specific association option, or to warn or raise when Sequel
# has to guess which value to use :instance_specific option (Sequel defaults to
# guessing true as that is the conservative setting). It is helpful to
# use this plugin, particularly with the :warn or :raise settings, to determine
# which associations should have :instance_specific set. Setting the
# :instance_specific to false for associations that are not instance specific
# can improve performance.
#
# Associations are instance-specific if their block or dataset option calls
# an instance method. For example, with the following two associations:
#
# Album.one_to_one :first_track, class: :Track do |ds|
# ds.where(number: 1)
# end
#
# Album.one_to_one :last_track, class: :Track do |ds|
# ds.where(number: num_tracks)
# end
#
# +first_track+ is not instance specific, but +last_track+ is, because the
# +num_tracks+ call in the block is calling <tt>Album#num_tracks</tt>. This
# plugin allows you to find these cases, and set the :instance_specific option
# appropriately for them:
#
# Album.one_to_one :first_track, class: :Track, instance_specific: false do |ds|
# ds.where(number: 1)
# end
#
# Album.one_to_one :last_track, class: :Track, instance_specific: true do |ds|
# ds.where(number: num_tracks)
# end
#
# Possible arguments to provide when loading the plugin:
#
# true :: Set the :instance_specific option to true
# false :: Set the :instance_specific option to false
# :default :: Call super to set the :instance_specific option
# :warn :: Emit a warning before calling super to set the :instance_specific option
# :raise :: Raise a Sequel::Error if an :instance_specific option is not provided and
# an association could be instance-specific.
#
# Note that this plugin only affects associations which could be instance
# specific, such as those with blocks or with the :dataset option used, where
# the :instance_specific option was not specified when the association was created.
#
# Usage:
#
# # Set how to handle associations that could be instance specific
# # but did not specify an :instance_specific option, for all subclasses
# # (set before creating subclasses).
# Sequel::Model.plugin :instance_specific_default, :warn
#
# # Set how to handle associations that could be instance specific
# # but did not specify an :instance_specific option, for the Album class
# Album.plugin :instance_specific_default, :warn
module InstanceSpecificDefault
# Set how to handle associations that could be instance specific but did
# not specify an :instance_specific value.
def self.configure(model, default)
model.instance_variable_set(:@instance_specific_default, default)
end

module ClassMethods
Plugins.inherited_instance_variables(self, :@instance_specific_default=>nil)

private

# Return the appropriate :instance_specific value, or warn or raise if
# configured.
def _association_instance_specific_default(name)
case @instance_specific_default
when true, false
return @instance_specific_default
when :default
# nothing
when :warn
warn("possibly instance-specific association without :instance_specific option (class: #{self}, association: #{name})", :uplevel => 3)
when :raise
raise Sequel::Error, "possibly instance-specific association without :instance_specific option (class: #{self}, association: #{name})"
else
raise Sequel::Error, "invalid value passed to instance_specific_default plugin: #{@instance_specific_default.inspect}"
end

super
end
end
end
end
end
56 changes: 56 additions & 0 deletions spec/extensions/instance_specific_default_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
require_relative "spec_helper"

describe "instance_specific_default plugin" do
before do
@db = Sequel.mock
@c = Class.new(Sequel::Model(@db[:test]))
def @c.name; 'C' end
@c.columns :id, :name
@db.sqls
end

it "should support setting a true value" do
@c.plugin :instance_specific_default, true
@c.many_to_one :c, :class=>@c do |ds| ds end
@c.association_reflection(:c)[:instance_specific].must_equal true
end

it "should support setting a false value" do
@c.plugin :instance_specific_default, false
@c.many_to_one :c, :class=>@c do |ds| ds end
@c.association_reflection(:c)[:instance_specific].must_equal false
end

it "should support setting a :default value" do
@c.plugin :instance_specific_default, :default
@c.many_to_one :c, :class=>@c do |ds| ds end
@c.association_reflection(:c)[:instance_specific].must_equal true
end

it "should support setting a :warn value" do
warn_args = nil
@c.define_singleton_method(:warn){|*args| warn_args = args}
@c.plugin :instance_specific_default, :warn
@c.many_to_one :c, :class=>@c do |ds| ds end
@c.association_reflection(:c)[:instance_specific].must_equal true
warn_args[0].must_match(/possibly instance-specific association without :instance_specific option/)
warn_args[1].must_equal(:uplevel=>3)
end

it "should support setting a :raise value" do
@c.plugin :instance_specific_default, :raise
proc{@c.many_to_one :c, :class=>@c do |ds| ds end}.must_raise Sequel::Error
end

it "should raise in invalid option is given" do
@c.plugin :instance_specific_default, Object.new
proc{@c.many_to_one :c, :class=>@c do |ds| ds end}.must_raise Sequel::Error
end

it "should work correctly in subclasses" do
@c.plugin :instance_specific_default, false
c = Class.new(@c)
c.many_to_one :c, :class=>@c do |ds| ds end
c.association_reflection(:c)[:instance_specific].must_equal false
end
end

0 comments on commit 9727d92

Please sign in to comment.