From 53521a9e39b9d8af4165d7703c36dc905f1f8f67 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 21 Jul 2017 09:11:47 -0400 Subject: Refactor `substitute_binds` to perform substitution immediately I'm honestly not sure if replacing bind params with their concrete values is something that belongs in Arel at all, as it's something that will need to be coupled to the quoting mechanism of the caller, and could just be accomplished by using `Quoted` instead. Still, with the new structure we can provide a much simpler API around substitution. The expectation of the quoter responding to `quote` is a reasonably minimal API. I originally used `DelegateClass` here, with the one line override of `add_bind`, but realized that we have some funky code going on where the collector returns the next collector to use (in practice `self` is always returned, and I don't see why we'd ever want to do this). Removing that would likely be worthwhile, but would be a larger refactoring --- test/collectors/test_substitute_bind_collector.rb | 66 ++++++++--------------- 1 file changed, 21 insertions(+), 45 deletions(-) (limited to 'test/collectors') diff --git a/test/collectors/test_substitute_bind_collector.rb b/test/collectors/test_substitute_bind_collector.rb index adcaf98319..de3e3655da 100644 --- a/test/collectors/test_substitute_bind_collector.rb +++ b/test/collectors/test_substitute_bind_collector.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'helper' require 'arel/collectors/substitute_binds' +require 'arel/collectors/sql_string' module Arel module Collectors @@ -11,61 +12,36 @@ module Arel super end - def collect node - @visitor.accept(node, Collectors::SubstituteBinds.new) - end - - def compile node - collect(node).value - end - - def ast_with_binds bv + def ast_with_binds table = Table.new(:users) manager = Arel::SelectManager.new table - manager.where(table[:age].eq(bv)) - manager.where(table[:name].eq(bv)) + manager.where(table[:age].eq(Nodes::BindParam.new("hello"))) + manager.where(table[:name].eq(Nodes::BindParam.new("world"))) manager.ast end - def test_leaves_binds - node = Nodes::BindParam.new(nil) - list = compile node - assert_equal node, list.first - assert_equal node.class, list.first.class - end - - def test_adds_strings - bv = Nodes::BindParam.new(nil) - list = compile ast_with_binds bv - assert_operator list.length, :>, 0 - assert_equal bv, list.grep(Nodes::BindParam).first - assert_equal bv.class, list.grep(Nodes::BindParam).first.class - end - - def test_substitute_binds - bv = Nodes::BindParam.new(nil) - collector = collect ast_with_binds bv - - values = collector.value - - offsets = values.map.with_index { |v,i| - [v,i] - }.find_all { |(v,_)| Nodes::BindParam === v }.map(&:last) - - list = collector.substitute_binds ["hello", "world"] - assert_equal "hello", list[offsets[0]] - assert_equal "world", list[offsets[1]] - - assert_equal 'SELECT FROM "users" WHERE "users"."age" = hello AND "users"."name" = world', list.join + def compile(node, quoter) + collector = Collectors::SubstituteBinds.new(quoter, Collectors::SQLString.new) + @visitor.accept(node, collector).value end def test_compile - bv = Nodes::BindParam.new(nil) - collector = collect ast_with_binds bv - - sql = collector.compile ["hello", "world"] + quoter = Object.new + def quoter.quote(val) + val.to_s + end + sql = compile(ast_with_binds, quoter) assert_equal 'SELECT FROM "users" WHERE "users"."age" = hello AND "users"."name" = world', sql end + + def test_quoting_is_delegated_to_quoter + quoter = Object.new + def quoter.quote(val) + val.inspect + end + sql = compile(ast_with_binds, quoter) + assert_equal 'SELECT FROM "users" WHERE "users"."age" = "hello" AND "users"."name" = "world"', sql + end end end end -- cgit v1.2.3