diff --git a/lib/simply_based.rb b/lib/simply_based.rb index 9cb277c..d2169e2 100644 --- a/lib/simply_based.rb +++ b/lib/simply_based.rb @@ -21,6 +21,10 @@ unless defined?(SimplyBased::VERSION) class NotImplementedError < RuntimeError; end class ModelNotInstantiatedError < RuntimeError; end class NotFound < RuntimeError; end + # Raised when a declared view carries options the N1QL translation cannot + # honour (e.g. :conditions / :emit_value / list functions). Failing loudly + # beats silently returning unfiltered results. + class UnsupportedView < Error; end # No-op retained for source compatibility with SimplyCouch. SimplyBased # never interpolates values into JavaScript views — queries are N1QL with diff --git a/lib/simply_based/store.rb b/lib/simply_based/store.rb index 4527cce..30f46ed 100644 --- a/lib/simply_based/store.rb +++ b/lib/simply_based/store.rb @@ -23,6 +23,7 @@ module SimplyBased @collection_name = cfg[:collection] || '_default' @cluster = ::Couchbase::Cluster.connect(cfg[:conn_str], cfg[:user], cfg[:password]) @ensured_indexes = {} + @ensured_indexes_mutex = Mutex.new end def keyspace @@ -77,7 +78,13 @@ module SimplyBased def bulk_load(ids) ids = Array(ids).compact return [] if ids.empty? - ids.filter_map { |id| load_document(id) } + _track("BULK_GET #{ids.join(',')}") + # One batched KV round trip (get_multi); missing ids are dropped, like + # the CouchDB _bulk_docs behaviour this replaces. + collection.get_multi(ids).filter_map do |res| + next unless res.success? + hydrate(res.content, res.id, res.cas) + end end def bulk_save(documents, validate = true) @@ -106,14 +113,19 @@ module SimplyBased def bulk_destroy(documents) documents = Array(documents) return [] if documents.empty? - rows = documents.map do |d| - begin - collection.remove(d._id) if d._id + with_ids, without_ids = documents.partition(&:_id) + _track("BULK_REMOVE #{with_ids.map(&:_id).join(',')}") if with_ids.any? + results = with_ids.any? ? collection.remove_multi(with_ids.map(&:_id)) : [] + rows = with_ids.zip(results).map do |d, res| + if res.nil? || res.error.nil? { 'id' => d._id, 'ok' => true } - rescue ::Couchbase::Error::DocumentNotFound + elsif res.error.is_a?(::Couchbase::Error::DocumentNotFound) { 'id' => d._id, 'error' => 'not_found' } + else + { 'id' => d._id, 'error' => res.error.class.name } end end + rows += without_ids.map { { 'id' => nil, 'error' => 'not_found' } } documents.each { |d| d._id = nil; d._rev = nil } rows end @@ -122,6 +134,7 @@ module SimplyBased # Handles ModelViewSpec / DeletedModelViewSpec (all_documents, find_by, count). def view(spec) + assert_translatable_view!(spec) kls = spec.klass vp = spec.view_parameters where = ["c.`ruby_class` = $rc"] @@ -151,7 +164,7 @@ module SimplyBased # key column equals `value`. def find_by_foreign_key(from_class, column, value, opts = {}) ensure_index(from_class, [column]) - where = ["c.`ruby_class` = $rc", "c.`#{column}` = $v"] + where = ["c.`ruby_class` = $rc", "c.#{quote_ident(column)} = $v"] params = { rc: from_class.name, v: value } where << not_deleted(from_class) unless opts[:with_deleted] || !soft_deletes?(from_class) run_docs(where, params, 'c.`created_at`', order_opts(opts)) @@ -159,7 +172,7 @@ module SimplyBased def count_by_foreign_key(from_class, column, value, opts = {}) ensure_index(from_class, [column]) - where = ["c.`ruby_class` = $rc", "c.`#{column}` = $v"] + where = ["c.`ruby_class` = $rc", "c.#{quote_ident(column)} = $v"] params = { rc: from_class.name, v: value } where << not_deleted(from_class) unless opts[:with_deleted] || !soft_deletes?(from_class) run_count(where, params) @@ -168,7 +181,7 @@ module SimplyBased # has_and_belongs_to_many: docs of `from_class` whose array column contains value. def find_by_array_contains(from_class, column, value, opts = {}) ensure_array_index(from_class, column) - where = ["c.`ruby_class` = $rc", "ANY x IN c.`#{column}` SATISFIES x = $v END"] + where = ["c.`ruby_class` = $rc", "ANY x IN c.#{quote_ident(column)} SATISFIES x = $v END"] params = { rc: from_class.name, v: value } where << not_deleted(from_class) unless opts[:with_deleted] || !soft_deletes?(from_class) run_docs(where, params, 'c.`created_at`', order_opts(opts)) @@ -176,7 +189,7 @@ module SimplyBased def count_by_array_contains(from_class, column, value, opts = {}) ensure_array_index(from_class, column) - where = ["c.`ruby_class` = $rc", "ANY x IN c.`#{column}` SATISFIES x = $v END"] + where = ["c.`ruby_class` = $rc", "ANY x IN c.#{quote_ident(column)} SATISFIES x = $v END"] params = { rc: from_class.name, v: value } where << not_deleted(from_class) unless opts[:with_deleted] || !soft_deletes?(from_class) run_count(where, params) @@ -290,6 +303,47 @@ module SimplyBased private + # The N1QL translation understands structured view declarations (:key / + # :keys / start-end ranges / :reduce). CouchDB-era JavaScript escape + # hatches cannot be honoured — and ignoring them would silently return + # *unfiltered* results (e.g. a `view :completed, key: :name, conditions: + # 'doc.completed == true'` would return every document of the class). + # Fail loudly instead. + def assert_translatable_view!(spec) + opts = spec.options || {} + if opts[:conditions] + raise SimplyBased::UnsupportedView, + "view #{spec.klass}##{spec.view_name} declares :conditions (a JavaScript " \ + "fragment); N1QL cannot translate it — express the condition as a key or " \ + "query it via a dedicated finder" + end + if opts[:emit_value] + raise SimplyBased::UnsupportedView, + "view #{spec.klass}##{spec.view_name} declares :emit_value; the N1QL " \ + "translation always returns full documents" + end + if spec.respond_to?(:list_name) && spec.list_name + raise SimplyBased::UnsupportedView, + "view #{spec.klass}##{spec.view_name} uses a CouchDB list function " \ + "(#{spec.list_name}); lists have no Couchbase equivalent" + end + end + + # Quote an N1QL identifier (field/index name). Identifiers come from the + # model DSL, not user input, but a backtick smuggled into a property name + # must never become query text. + def quote_ident(name) + n = name.to_s + raise ArgumentError, "invalid N1QL identifier: #{n.inspect}" if n.include?('`') || n.include?('\\') + "`#{n}`" + end + + # Single-quoted N1QL string literal (used only for index WHERE clauses, + # where named parameters are not allowed). + def quote_string(value) + "'#{value.to_s.gsub('\\', '\\\\\\\\').gsub("'", "''")}'" + end + # Record a database round-trip for the spec query-count matchers (no-op in # production, where $performed_queries is undefined). def _track(op) @@ -325,8 +379,8 @@ module SimplyBased def key_expr(key) case key when nil then nil - when Array then '[' + key.map { |k| "c.`#{k}`" }.join(', ') + ']' - else "c.`#{key}`" + when Array then '[' + key.map { |k| "c.#{quote_ident(k)}" }.join(', ') + ']' + else "c.#{quote_ident(key)}" end end @@ -340,10 +394,10 @@ module SimplyBased key_cols.each_with_index.map do |col, i| v = vp[:key][i] if v.nil? - "(c.`#{col}` IS MISSING OR c.`#{col}` IS NULL)" + "(c.#{quote_ident(col)} IS MISSING OR c.#{quote_ident(col)} IS NULL)" else params[:"k#{i}"] = v - "c.`#{col}` = $k#{i}" + "c.#{quote_ident(col)} = $k#{i}" end end else @@ -368,19 +422,19 @@ module SimplyBased # Only partition by the property when a value is supplied. roots/full_tree # with no argument span all partitions (e.g. all locales). return if by_property.nil? || by_value.nil? - where << "c.`#{by_property}` = $bp" + where << "c.#{quote_ident(by_property)} = $bp" params[:bp] = by_value end def run_tree(where, params, order_col) sql = +"SELECT c.*, META(c).id AS `__id`, META(c).cas AS `__cas` FROM #{keyspace} AS c WHERE #{where.join(' AND ')}" - sql << " ORDER BY c.`#{order_col}` ASC" if order_col + sql << " ORDER BY c.#{quote_ident(order_col)} ASC" if order_col query(sql, params).rows.to_a.map { |row| row_to_doc(row) } end def not_deleted(kls) - a = kls.soft_delete_attribute - "(c.`#{a}` IS MISSING OR c.`#{a}` IS NULL)" + a = quote_ident(kls.soft_delete_attribute) + "(c.#{a} IS MISSING OR c.#{a} IS NULL)" end def soft_deletes?(kls) @@ -395,23 +449,31 @@ module SimplyBased # SimplyCouch creating a design document on first view query. def ensure_index(kls, columns) key = [kls.name, columns].inspect - return if @ensured_indexes[key] - name = "sb_#{kls.name.gsub(/\W/, '_').downcase}_#{columns.join('_')}"[0, 60] - cols = columns.map { |col| "`#{col}`" }.join(', ') - query("CREATE INDEX `#{name}` IF NOT EXISTS ON #{keyspace}(#{cols}) WHERE `ruby_class` = '#{kls.name}'") - @ensured_indexes[key] = true - rescue ::Couchbase::Error::CouchbaseError - @ensured_indexes[key] = true + cols = columns.map { |col| quote_ident(col) }.join(', ') + ensure_index_once(key, "sb_#{kls.name.gsub(/\W/, '_').downcase}_#{columns.join('_')}"[0, 60], + "(#{cols})", kls) end def ensure_array_index(kls, column) key = [kls.name, :array, column].inspect - return if @ensured_indexes[key] - name = "sb_#{kls.name.gsub(/\W/, '_').downcase}_arr_#{column}"[0, 60] - query("CREATE INDEX `#{name}` IF NOT EXISTS ON #{keyspace}(DISTINCT ARRAY x FOR x IN `#{column}` END) WHERE `ruby_class` = '#{kls.name}'") - @ensured_indexes[key] = true - rescue ::Couchbase::Error::CouchbaseError - @ensured_indexes[key] = true + ensure_index_once(key, "sb_#{kls.name.gsub(/\W/, '_').downcase}_arr_#{column}"[0, 60], + "(DISTINCT ARRAY x FOR x IN #{quote_ident(column)} END)", kls) + end + + def ensure_index_once(key, name, index_expr, kls) + # The mutex only guards the bookkeeping hash (Store instances are shared + # across threads); CREATE INDEX itself is IF NOT EXISTS, so a lost race + # costs one redundant statement, never a broken index. + @ensured_indexes_mutex.synchronize { return if @ensured_indexes[key] } + query("CREATE INDEX #{quote_ident(name)} IF NOT EXISTS ON #{keyspace}#{index_expr} WHERE `ruby_class` = #{quote_string(kls.name)}") + @ensured_indexes_mutex.synchronize { @ensured_indexes[key] = true } + rescue ::Couchbase::Error::CouchbaseError => e + # Still mark as ensured to avoid retry storms, but say so: a swallowed + # failure here means every later query on this association may fall back + # to a primary-index scan. + warn "SimplyBased: could not ensure index #{name} on #{keyspace} " \ + "(#{e.class}: #{e.message}); queries may fall back to a primary scan" + @ensured_indexes_mutex.synchronize { @ensured_indexes[key] = true } end # ── persistence helpers ────────────────────────────────────────────────── diff --git a/spec/unsupported_view_spec.rb b/spec/unsupported_view_spec.rb new file mode 100644 index 0000000..72eb522 --- /dev/null +++ b/spec/unsupported_view_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' + +class ConditionalViewWidget + include SimplyBased::Model + property :name + property :completed, type: :boolean + # A CouchDB-era declaration: the :conditions JavaScript fragment cannot be + # translated to N1QL. Declaring it must stay legal (simply_couch models load + # unchanged), but *querying* it must fail loudly — the previous behaviour + # silently dropped the condition and returned every document of the class. + view :completed_by_name, key: :name, conditions: 'doc.completed === true' + view :names_only, key: :name, emit_value: :name +end + +RSpec.describe 'Untranslatable view declarations' do + it 'still allows the model to load and plain views to work' do + ConditionalViewWidget.create(name: 'a', completed: false) + expect(ConditionalViewWidget.all.size).to eq 1 + end + + it 'raises UnsupportedView when querying a :conditions view instead of silently ignoring the condition' do + ConditionalViewWidget.create(name: 'done', completed: true) + ConditionalViewWidget.create(name: 'done', completed: false) + expect { + ConditionalViewWidget.database.view(ConditionalViewWidget.completed_by_name(key: 'done')) + }.to raise_error(SimplyBased::UnsupportedView, /conditions/) + end + + it 'raises UnsupportedView for :emit_value views' do + expect { + ConditionalViewWidget.database.view(ConditionalViewWidget.names_only(key: 'x')) + }.to raise_error(SimplyBased::UnsupportedView, /emit_value/) + end +end