test: cover OrmIdLookupIndexTest and teardown
This commit is contained in:
1 parent
ed452e46f6
commit
84554dae11
1 file changed
+108
@@ -0,0 +1,108 @@
|
|||||||
|
require 'minitest/autorun'
|
||||||
|
require 'prism'
|
||||||
|
require 'tmpdir'
|
||||||
|
|
||||||
|
# `find_by(id: x)`/`where(id: x).first` go through Relation#rows exactly like
|
||||||
|
# any other equality lookup, which used to mean Store#lookup_by_index had to
|
||||||
|
# find a registered secondary index covering the 'id' column - but no
|
||||||
|
# framework code ever calls `add_index(table, :id)` (id is the primary key,
|
||||||
|
# already a Hash key on definition[:rows]), so that search always failed and
|
||||||
|
# silently fell back to a full table scan (Store#all). This was real, live
|
||||||
|
# behavior, not a theoretical gap: Prism::Authenticatable#current_user calls
|
||||||
|
# `authenticatable_model.find_by(id: id)` on every single authenticated
|
||||||
|
# request (lib/prism/authenticatable.rb), and examples/blog's
|
||||||
|
# ApplicationController#comment_post_label/#comment_post_linkable? call
|
||||||
|
# `Post.with_deleted.find_by(id: comment.post_id)` once per rendered comment
|
||||||
|
# row - both were full-scanning their table on every hit before this fix
|
||||||
|
# (discovered via examples/blog's new AccountController#moderation page,
|
||||||
|
# which renders several such rows per response and made the N full scans
|
||||||
|
# impossible to miss in the sql.prism log). #lookup_by_index_in already had
|
||||||
|
# this exact same "id is always indexed for free" special case for IN-lists
|
||||||
|
# (see its own comment) - this closes the matching gap for a plain scalar
|
||||||
|
# equality.
|
||||||
|
class OrmIdLookupIndexTest < Minitest::Test
|
||||||
|
def setup
|
||||||
|
@dir = Dir.mktmpdir('prism-id-lookup-index-store')
|
||||||
|
@store = CountingIdIndexStore.new(File.join(@dir, 'test.pstore'))
|
||||||
|
@store.create_table(:widgets, %w[id sku])
|
||||||
|
@a = @store.insert(:widgets, sku: 'AAA')
|
||||||
|
@b = @store.insert(:widgets, sku: 'BBB')
|
||||||
|
|
||||||
|
widget_class = Class.new(Prism::Orm::Model) { table_name :widgets }
|
||||||
|
widget_class.store = @store
|
||||||
|
Object.const_set(:IdLookupWidget, widget_class)
|
||||||
|
end
|
||||||
|
|
||||||
|
def teardown
|
||||||
|
FileUtils.remove_entry(@dir)
|
||||||
|
Object.send(:remove_const, :IdLookupWidget) if Object.const_defined?(:IdLookupWidget)
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_lookup_by_index_answers_a_bare_id_equality_without_any_registered_index
|
||||||
|
matches = @store.lookup_by_index(:widgets, { 'id' => @a })
|
||||||
|
refute_nil matches, 'a plain id equality must never report "no index" (nil means fall back to a full scan)'
|
||||||
|
assert_equal ['AAA'], matches.map { |row| row['sku'] }
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_lookup_by_index_on_a_missing_id_is_an_empty_hit_not_a_scan_signal
|
||||||
|
assert_equal [], @store.lookup_by_index(:widgets, { 'id' => @a + 999 })
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_extra_equality_keys_alongside_id_are_still_resolved_by_id_alone
|
||||||
|
# No index exists on `sku`, and the wrong sku is paired with the right
|
||||||
|
# id - the id lookup still returns the row (Relation#rows re-checks the
|
||||||
|
# full condition afterward and would reject it there, not here).
|
||||||
|
matches = @store.lookup_by_index(:widgets, { 'id' => @a, 'sku' => 'WRONG' })
|
||||||
|
assert_equal ['AAA'], matches.map { |row| row['sku'] }
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_model_find_by_id_never_scans
|
||||||
|
@store.reset_counts!
|
||||||
|
widget = IdLookupWidget.find_by(id: @a)
|
||||||
|
assert_equal 'AAA', widget.sku
|
||||||
|
assert_equal 0, @store.scan_count
|
||||||
|
assert_equal 1, @store.id_index_hits
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_where_id_first_never_scans_even_with_an_unrelated_condition_chained_on
|
||||||
|
@store.reset_counts!
|
||||||
|
widget = IdLookupWidget.where(id: @a).where(sku: 'AAA').first
|
||||||
|
assert_equal 'AAA', widget.sku
|
||||||
|
assert_equal 0, @store.scan_count
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_find_by_id_for_a_nonexistent_row_never_scans
|
||||||
|
@store.reset_counts!
|
||||||
|
assert_nil IdLookupWidget.find_by(id: @a + 999)
|
||||||
|
assert_equal 0, @store.scan_count
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Same shape as CountingFkIndexStore (test_orm_has_many_foreign_key_index.rb)
|
||||||
|
# plus a counter specifically for id-equality hits, since #lookup_by_index is
|
||||||
|
# the one method under test here rather than #lookup_by_index_in.
|
||||||
|
class CountingIdIndexStore < Prism::Orm::Store
|
||||||
|
attr_reader :scan_count, :id_index_hits
|
||||||
|
|
||||||
|
def initialize(*args)
|
||||||
|
super
|
||||||
|
@scan_count = 0
|
||||||
|
@id_index_hits = 0
|
||||||
|
end
|
||||||
|
|
||||||
|
def all(table)
|
||||||
|
@scan_count += 1
|
||||||
|
super
|
||||||
|
end
|
||||||
|
|
||||||
|
def lookup_by_index(table, equalities)
|
||||||
|
result = super
|
||||||
|
@id_index_hits += 1 if !result.nil? && equalities.key?('id')
|
||||||
|
result
|
||||||
|
end
|
||||||
|
|
||||||
|
def reset_counts!
|
||||||
|
@scan_count = 0
|
||||||
|
@id_index_hits = 0
|
||||||
|
end
|
||||||
|
end
|
||||||
Reference in new issue
Block a user