Parameterize N1QL queries with positional parameters

lib/couchbase-orm/n1ql.rb

@@ -95,10 +95,10 @@ def convert_values(keys, values)
                 end
             end
 
-            def build_where(keys, values)
+            def build_where(keys, values, params: nil)
                 where = values == NO_VALUE ? '' : keys.zip(Array.wrap(values))
                             .reject { |key, value| key.nil? && value.nil? }
-                            .map { |key, value| build_match(key, value) }
+                            .map { |key, value| build_match(key, value, params: params) }
                             .join(" AND ")
                 "type=\"#{design_document}\" #{"AND " + where unless where.blank?}"
             end
@@ -119,12 +119,17 @@ def run_query(keys, values, query_fn, custom_order: nil, descending: false, limi
                     N1qlProxy.new(query_fn.call(bucket, values, Couchbase::Options::Query.new(**options)))
                 else
                     bucket_name = bucket.name
-                    where = build_where(keys, values)
+                    params = []
+                    where = build_where(keys, values, params: params)
                     order = custom_order || build_order(keys, descending)
                     limit = build_limit(limit)
                     n1ql_query = "select raw meta().id from `#{bucket_name}` where #{where} order by #{order} #{limit}"
-                    result = cluster.query(n1ql_query, Couchbase::Options::Query.new(**options))
-                    CouchbaseOrm.logger.debug  "N1QL query: #{n1ql_query} return #{result.rows.to_a.length} rows with scan_consistency : #{options[:scan_consistency]}"
+
+                    query_options = options.merge(positional_parameters: params)
+                    result = cluster.query(n1ql_query, Couchbase::Options::Query.new(**query_options))
+                    CouchbaseOrm.logger.debug {
+                        "N1QL query: #{n1ql_query} params: #{params.inspect} return #{result.rows.to_a.length} rows with scan_consistency: #{options[:scan_consistency]}"
+                    }
                     N1qlProxy.new(result)
                 end
             end

lib/couchbase-orm/relation.rb

@@ -21,31 +21,41 @@ def to_s
 
             def to_n1ql
                 bucket_name = @model.bucket.name
-                where = build_where
+                where = build_where_with_params(nil)
                 order = build_order
                 limit = build_limit
                 "select raw meta().id from `#{bucket_name}` where #{where} order by #{order} #{limit}"
             end
 
-            def execute(n1ql_query)
-                result = @model.cluster.query(n1ql_query, Couchbase::Options::Query.new(scan_consistency: CouchbaseOrm::N1ql.config[:scan_consistency]))
-                CouchbaseOrm.logger.debug { "Relation query: #{n1ql_query} return #{result.rows.to_a.length} rows with scan_consistency : #{CouchbaseOrm::N1ql.config[:scan_consistency]}" }
+            def to_n1ql_with_params
+                bucket_name = @model.bucket.name
+                params = []
+                where = build_where_with_params(params)
+                order = build_order
+                limit = build_limit
+                ["select raw meta().id from `#{bucket_name}` where #{where} order by #{order} #{limit}", params]
+            end
+
+            def execute(n1ql_query, params = [])
+                result = @model.cluster.query(n1ql_query, build_query_options(positional_parameters: params))
+                CouchbaseOrm.logger.debug { "Relation query: #{n1ql_query} params: #{params.inspect} return #{result.rows.to_a.length} rows" }
                 N1qlProxy.new(result)
             end
 
             def query
                 CouchbaseOrm::logger.debug("Query: #{self}")
-                n1ql_query = to_n1ql
-                execute(n1ql_query)
+                n1ql_query, params = to_n1ql_with_params
+                execute(n1ql_query, params)
             end
-            
+
             def update_all(**cond)
                 bucket_name = @model.bucket.name
-                where = build_where
+                params = []
+                where = build_where_with_params(params)
                 limit = build_limit
-                update = build_update(**cond)
+                update = build_update_with_params(params, **cond)
                 n1ql_query = "update `#{bucket_name}` set #{update} where #{where} #{limit}"
-                execute(n1ql_query)
+                execute(n1ql_query, params)
             end
 
             def ids
@@ -61,14 +71,16 @@ def strict_loading?
             end
 
             def first
-                result = @model.cluster.query(self.limit(1).to_n1ql, Couchbase::Options::Query.new(scan_consistency: CouchbaseOrm::N1ql.config[:scan_consistency]))
+                n1ql_query, params = self.limit(1).to_n1ql_with_params
+                result = @model.cluster.query(n1ql_query, build_query_options(positional_parameters: params))
                 return unless (first_id = result.rows.to_a.first)
 
                 @model.find(first_id, with_strict_loading: @strict_loading)
             end
 
             def last
-                result = @model.cluster.query(to_n1ql, Couchbase::Options::Query.new(scan_consistency: CouchbaseOrm::N1ql.config[:scan_consistency]))
+                n1ql_query, params = to_n1ql_with_params
+                result = @model.cluster.query(n1ql_query, build_query_options(positional_parameters: params))
                 last_id = result.rows.to_a.last
                 @model.find(last_id, with_strict_loading: @strict_loading) if last_id
             end
@@ -166,7 +178,7 @@ def merge_order(*lorder, **horder)
                     .merge(Array.wrap(lorder).map{ |o| [o, :asc] }.to_h)
                     .merge(horder)
             end
-     
+
             def merge_where(conds, _not = false)
                 @where + (_not ? conds.to_a.map{|k,v|[k,v,:not]} : conds.to_a)
             end
@@ -183,48 +195,54 @@ def build_order
                 end.join(", ")
                 order.empty? ? "meta().id" : order
             end
-            
-            def build_where
-                build_conds([[:type, @model.design_document]] + @where)
+
+            def build_where_with_params(params)
+                build_conds_with_params([[nil, "type = #{@model.quote(@model.design_document)}"]] + @where, params)
             end
 
-            def build_conds(conds)
+            def build_conds_with_params(conds, params)
                 conds.map do |key, value, opt|
                     if key
-                        opt == :not ? 
-                            @model.build_not_match(key, value) : 
-                            @model.build_match(key, value)
+                        opt == :not ?
+                            @model.build_not_match(key, value, params: params) :
+                            @model.build_match(key, value, params: params)
                     else
                         value
                     end
                 end.join(" AND ")
             end
 
-            def build_update(**cond)
+            def build_update_with_params(params, **cond)
                 cond.map do |key, value|
-                    for_clause=""
+                    for_clause = ""
                     if value.is_a?(Hash) && value[:_for]
                         path_clause = value.delete(:_for)
                         var_clause = path_clause.to_s.split(".").last.singularize
-                        
+
                         _when = value.delete(:_when)
-                        when_clause = _when ? build_conds(_when.to_a) : ""
-                        
-                        _set = value.delete(:_set)                       
+                        when_clause = _when ? build_conds_with_params(_when.to_a, params) : ""
+
+                        _set = value.delete(:_set)
                         value = _set if _set
 
                         for_clause = " for #{var_clause} in #{path_clause} when #{when_clause} end"
                     end
                     if value.is_a?(Hash)
                         value.map do |k, v|
-                            "#{key}.#{k} = #{@model.quote(v) || 'NULL'}"
+                            "#{key}.#{k} = #{v.nil? ? 'NULL' : @model.bind(v, params)}"
                         end.join(", ") + for_clause
                     else
-                        "#{key} = #{@model.quote(value)}#{for_clause}"
+                        "#{key} = #{value.nil? ? 'NULL' : @model.bind(value, params)}#{for_clause}"
                     end
                 end.join(", ")
             end
 
+            def build_query_options(positional_parameters: [])
+                opts = { scan_consistency: CouchbaseOrm::N1ql.config[:scan_consistency] }
+                opts[:positional_parameters] = positional_parameters unless positional_parameters.empty?
+                Couchbase::Options::Query.new(**opts)
+            end
+
             def method_missing(method, *args, &block)
                 if @model.respond_to?(method)
                     scoping {

lib/couchbase-orm/utilities/has_many.rb

@@ -96,7 +96,9 @@ def build_index_n1ql(klass, remote_class, remote_method, through_key, foreign_ke
                 klass.class_eval do
                   n1ql remote_method, emit_key: 'id', query_fn: proc { |bucket, values, options|
                     raise ArgumentError, "values[0] must not be blank" if values[0].blank?
-                    cluster.query("SELECT raw #{through_key} FROM `#{bucket.name}` where type = \"#{design_document}\" and #{foreign_key} = #{quote(values[0])}", options)
+                    n1ql_query = "SELECT raw #{through_key} FROM `#{bucket.name}` where type = \"#{design_document}\" and #{foreign_key} = $1"
+                    options.positional_parameters([values[0]])
+                    cluster.query(n1ql_query, options)
                   }
                 end
             else

lib/couchbase-orm/utilities/query_helper.rb

@@ -4,7 +4,34 @@ module QueryHelper
 
         module ClassMethods
 
-            def build_match(key, value)
+            def serialize_for_binding(value)
+                if value.is_a?(Array)
+                    value.map { |v| serialize_for_binding(v) }
+                elsif [DateTime, Time].any? { |clazz| value.is_a?(clazz) } || (value.respond_to?(:acts_like?) && value.acts_like?(:time))
+                    value.iso8601(@precision || 0)
+                elsif value.is_a?(Date)
+                    value.to_s
+                else
+                    value
+                end
+            end
+
+            def bind(value, params)
+                if value.nil?
+                    nil
+                else
+                    params << serialize_for_binding(value)
+                    "$#{params.length}"
+                end
+            end
+
+            # Renders a value either as a positional parameter (when +params+ is
+            # provided) or as an inline quoted literal (when it is nil).
+            def resolve_value(value, params)
+                params ? bind(value, params) : quote(value)
+            end
+
+            def build_match(key, value, params: nil)
                 use_is_null = self.properties_always_exists_in_document
                 key = "meta().id" if key.to_s == "id"
                 case
@@ -13,35 +40,35 @@ def build_match(key, value)
                 when value.nil? && !use_is_null
                     "#{key} IS NOT VALUED"
                 when value.is_a?(Hash) && attribute_types[key.to_s].is_a?(CouchbaseOrm::Types::Array)
-                    "any #{key.to_s.singularize} in #{key} satisfies (#{build_match_hash("#{key.to_s.singularize}", value)}) end"
+                    "any #{key.to_s.singularize} in #{key} satisfies (#{build_match_hash("#{key.to_s.singularize}", value, params: params)}) end"
                 when value.is_a?(Hash) && !attribute_types[key.to_s].is_a?(CouchbaseOrm::Types::Array)
-                    build_match_hash(key, value)
+                    build_match_hash(key, value, params: params)
                 when value.is_a?(Array) && value.include?(nil)
-                    "(#{build_match(key, nil)} OR #{build_match(key, value.compact)})"
+                    "(#{build_match(key, nil, params: params)} OR #{build_match(key, value.compact, params: params)})"
                 when value.is_a?(Array)
-                    "#{key} IN #{quote(value)}"
+                    "#{key} IN #{resolve_value(value, params)}"
                 when value.is_a?(Range)
-                    build_match_range(key, value)
+                    build_match_range(key, value, params: params)
                 else
-                    "#{key} = #{quote(value)}"
+                    "#{key} = #{resolve_value(value, params)}"
                 end
             end
 
-            def build_match_hash(key, value)
+            def build_match_hash(key, value, params: nil)
                 matches = []
                 value.each do |k, v|
                     case k
                     when :_gt
-                        matches << "#{key} > #{quote(v)}"
+                        matches << "#{key} > #{resolve_value(v, params)}"
                     when :_gte
-                        matches << "#{key} >= #{quote(v)}"
+                        matches << "#{key} >= #{resolve_value(v, params)}"
                     when :_lt
-                        matches << "#{key} < #{quote(v)}"
+                        matches << "#{key} < #{resolve_value(v, params)}"
                     when :_lte
-                        matches << "#{key} <= #{quote(v)}"
+                        matches << "#{key} <= #{resolve_value(v, params)}"
                     when :_ne
-                        matches << "#{key} != #{quote(v)}"
-                    
+                        matches << "#{key} != #{resolve_value(v, params)}"
+
                     # TODO v2
                     # when :_in
                     #     matches << "#{key} IN #{quote(v)}"
@@ -65,7 +92,7 @@ def build_match_hash(key, value)
                     #     matches << "#{key} MATCH #{quote(v)}"
                     # when :_nmatch
                     #     matches << "#{key} NOT MATCH #{quote(v)}"
-                    
+
                     # TODO v3
                     # when :_any
                     #     matches << "#{key} ANY #{quote(v)}"
@@ -80,26 +107,26 @@ def build_match_hash(key, value)
                     #when :_nwithin
                     #    matches << "#{key} NOT WITHIN #{quote(v)}"
                     else
-                        matches << build_match("#{key}.#{k}", v)
+                        matches << build_match("#{key}.#{k}", v, params: params)
                     end
                 end
-                
+
                 matches.join(" AND ")
             end
 
-            def build_match_range(key, value)
+            def build_match_range(key, value, params: nil)
                 matches = []
-                matches << "#{key} >= #{quote(value.begin)}"
+                matches << "#{key} >= #{resolve_value(value.begin, params)}"
                 if value.exclude_end?
-                    matches << "#{key} < #{quote(value.end)}"
+                    matches << "#{key} < #{resolve_value(value.end, params)}"
                 else
-                    matches << "#{key} <= #{quote(value.end)}"
+                    matches << "#{key} <= #{resolve_value(value.end, params)}"
                 end
                 matches.join(" AND ")
             end
 
 
-            def build_not_match(key, value)
+            def build_not_match(key, value, params: nil)
                 use_is_null = self.properties_always_exists_in_document
                 key = "meta().id" if key.to_s == "id"
                 case
@@ -108,11 +135,11 @@ def build_not_match(key, value)
                 when value.nil? && !use_is_null
                     "#{key} IS VALUED"
                 when value.is_a?(Array) && value.include?(nil)
-                    "(#{build_not_match(key, nil)} AND #{build_not_match(key, value.compact)})"
+                    "(#{build_not_match(key, nil, params: params)} AND #{build_not_match(key, value.compact, params: params)})"
                 when value.is_a?(Array)
-                    "#{key} NOT IN #{quote(value)}"
+                    "#{key} NOT IN #{resolve_value(value, params)}"
                 else
-                    "#{key} != #{quote(value)}"
+                    "#{key} != #{resolve_value(value, params)}"
                 end
             end
 

spec/n1ql_spec.rb

@@ -172,19 +172,28 @@ class N1QLTest < CouchbaseOrm::Base
     it "should log the default scan_consistency when n1ql query is executed" do
         allow(CouchbaseOrm.logger).to receive(:debug)
         N1QLTest.by_rating_reverse()
-        expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once).with("N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=\"n1_ql_test\"  order by name DESC  return 0 rows with scan_consistency : #{described_class::DEFAULT_SCAN_CONSISTENCY}")
+        expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once) do |&block|
+            msg = block ? block.call : nil
+            msg == "N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=\"n1_ql_test\"  order by name DESC  params: [] return 0 rows with scan_consistency: #{described_class::DEFAULT_SCAN_CONSISTENCY}"
+        end
     end
 
     it "should log the set scan_consistency when n1ql query is executed with a specific scan_consistency" do
         allow(CouchbaseOrm.logger).to receive(:debug)
         default_n1ql_config = CouchbaseOrm::N1ql.config
         CouchbaseOrm::N1ql.config({ scan_consistency: :not_bounded })
         N1QLTest.by_rating_reverse()
-        expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once).with("N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=\"n1_ql_test\"  order by name DESC  return 0 rows with scan_consistency : not_bounded")
+        expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once) do |&block|
+            msg = block ? block.call : nil
+            msg == "N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=\"n1_ql_test\"  order by name DESC  params: [] return 0 rows with scan_consistency: not_bounded"
+        end
 
         CouchbaseOrm::N1ql.config(default_n1ql_config)
         N1QLTest.by_rating_reverse()
-        expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once).with("N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=\"n1_ql_test\"  order by name DESC  return 0 rows with scan_consistency : #{described_class::DEFAULT_SCAN_CONSISTENCY}")
+        expect(CouchbaseOrm.logger).to have_received(:debug).at_least(:once) do |&block|
+            msg = block ? block.call : nil
+            msg == "N1QL query: select raw meta().id from `#{CouchbaseOrm::Connection.bucket.name}` where type=\"n1_ql_test\"  order by name DESC  params: [] return 0 rows with scan_consistency: #{described_class::DEFAULT_SCAN_CONSISTENCY}"
+        end
     end
 
     after(:all) do