Store: fail loudly on untranslatable views; quoting hardening; true batched bulk KV

- database.view now raises SimplyBased::UnsupportedView when a declared view
  carries CouchDB-era options the N1QL translation silently ignored before:
  :conditions (a JS fragment) and :emit_value, plus CouchDB list functions.
  Silently dropping :conditions was the worst of these — a
  'view :completed, key: :name, conditions: ...' query returned *unfiltered*
  results with no hint anything was wrong. Covered by unsupported_view_spec.
- All column/property names interpolated into N1QL are now routed through
  quote_ident, which rejects backticks/backslashes. These names come from the
  model DSL rather than user input, but a smuggled backtick must never become
  query text. Class names in index predicates go through a proper string
  literal quoting helper for the same reason.
- ensure_index: failures are still swallowed (an index build in progress must
  not take down queries) but now emit a warning naming the index, because the
  silent version left every subsequent query on a primary-index scan with no
  trace. The ensured-indexes bookkeeping is mutex-guarded for multithreaded
  servers (Puma).
- bulk_load / bulk_destroy now actually batch (collection.get_multi /
  remove_multi — one KV round trip). The README already advertised
  'bulk (batched KV)' but the code looped one get/remove per id.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

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

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 ──────────────────────────────────────────────────

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