Fix embedded documents: recursion crash, .all/.count, honest spec exclusion
Live-run finding: the full suite against Couchbase was 423 examples with 17 failures — all in embedding_spec. The session report claimed embedded specs were 'excluded by design', but the exclusion filter targeted a :embedding tag no example carries, so the file ran (and failed) all along. Three fixes: - HasManyEmbedded::Property#build wired parent_object before installing the collection ivar on the parent; parent_object= reaches back into the parent's collection getter, which re-entered build -> SystemStackError on any access to embedded documents loaded from the database. Install the ivar first. - EmbeddedClass.all / .count referenced raw JS views (all_documents / all_documents_for_count) that the N1QL port silently drops, so .all returned [] and .count raised NoMethodError. Rebuilt them by enumerating the parent documents (same work as the old view scan, client-side; an UNNEST query can replace it later). - Only reverse lookups (N1QL on a foreign key inside an embedded array) remain unsupported: tagged :embedded_in_reverse_lookup and excluded in spec_helper with an explanatory comment, replacing the broken :embedding filter line. embedding_spec.rb: 16 examples, 0 failures against live Couchbase 8.0.1 CE. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
d2202bda03
commit
1beb8ead11
4 files changed
+37
-7
No files matched your search
@@ -94,8 +94,23 @@ module SimplyBased
|
|||||||
:include_docs => true
|
:include_docs => true
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# The CouchDB implementation used raw JS views (all_documents /
|
||||||
|
# all_documents_for_count) that emitted one row per embedded
|
||||||
|
# element. Those views are no-ops on Couchbase (see
|
||||||
|
# ClassMethods::Base#view), so enumerate through the parent
|
||||||
|
# documents instead: embedded records only exist inside their
|
||||||
|
# parent, and the parent getter wires parent_object/index.
|
||||||
|
# NOTE: this loads all parent documents — same order of work as
|
||||||
|
# the CouchDB view scan, but client-side. Fine for small/medium
|
||||||
|
# collections; revisit with an UNNEST query if it ever matters.
|
||||||
|
define_method :all do |*args|
|
||||||
|
parent_klass = Object.const_get(embedded_in_class_name)
|
||||||
|
embedded_property = self.name.property_name.pluralize
|
||||||
|
parent_klass.all.flat_map { |p| Array.wrap(p.send(embedded_property)) }
|
||||||
|
end
|
||||||
|
|
||||||
define_method :count do |options = {}|
|
define_method :count do |options = {}|
|
||||||
database.view(all_documents_for_count(options.merge(:reduce => true)))['rows'].try(:first).try('[]', 'value').to_i
|
all.size
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -145,17 +145,22 @@ module SimplyBased
|
|||||||
|
|
||||||
def build(object, json)
|
def build(object, json)
|
||||||
values = Array.wrap(json[name]).map do |v|
|
values = Array.wrap(json[name]).map do |v|
|
||||||
obj = if v.is_a?(Hash)
|
if v.is_a?(Hash)
|
||||||
options[:class_name].constantize.new.tap { |o| o._document = v }
|
options[:class_name].constantize.new.tap { |o| o._document = v }
|
||||||
else
|
else
|
||||||
v
|
v
|
||||||
end
|
end
|
||||||
obj.parent_object = object
|
|
||||||
obj
|
|
||||||
end
|
end
|
||||||
values.define_singleton_method(:parent_object_set?) { true }
|
values.define_singleton_method(:parent_object_set?) { true }
|
||||||
values.each_with_index { |v, i| v.index = i }
|
# Install the collection on the parent *before* wiring parent_object:
|
||||||
|
# parent_object= reaches back into the parent's collection getter (via
|
||||||
|
# the embedded_in "#{name}=" writer's include? check), and while the
|
||||||
|
# ivar is unset that getter re-enters build — infinite mutual
|
||||||
|
# recursion (SystemStackError).
|
||||||
object.instance_variable_set("@#{name}", values)
|
object.instance_variable_set("@#{name}", values)
|
||||||
|
values.each_with_index { |v, i| v.index = i }
|
||||||
|
values.each { |v| v.parent_object = object }
|
||||||
|
values
|
||||||
end
|
end
|
||||||
|
|
||||||
def initialize(owner_clazz, name, options = {})
|
def initialize(owner_clazz, name, options = {})
|
||||||
|
|||||||
@@ -1,5 +1,11 @@
|
|||||||
require 'spec_helper'
|
require 'spec_helper'
|
||||||
RSpec.configure { |c| c.filter_run_excluding(:embedding) } if defined?(RSpec)
|
# NOTE: this file used to carry `filter_run_excluding(:embedding)` — but no
|
||||||
|
# example was tagged :embedding, so nothing was ever excluded and the whole
|
||||||
|
# file ran (and failed) against Couchbase. The basic embedded flows are now
|
||||||
|
# fixed and expected green; only reverse lookups (finding embedded documents
|
||||||
|
# through an N1QL query on a foreign key inside the embedded array) remain
|
||||||
|
# unimplemented, and are tagged :embedded_in_reverse_lookup below (excluded in
|
||||||
|
# spec_helper).
|
||||||
|
|
||||||
describe 'Embedding' do
|
describe 'Embedding' do
|
||||||
context 'initialized comments' do
|
context 'initialized comments' do
|
||||||
@@ -128,7 +134,7 @@ describe 'Embedding' do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'belongs to strict_post' do
|
context 'belongs to strict_post', :embedded_in_reverse_lookup do
|
||||||
before do
|
before do
|
||||||
@user = User.create(name: 'embedding user')
|
@user = User.create(name: 'embedding user')
|
||||||
@strict_post = StrictPost.create(user: @user)
|
@strict_post = StrictPost.create(user: @user)
|
||||||
|
|||||||
@@ -14,6 +14,10 @@ RSpec.configure do |config|
|
|||||||
config.expect_with(:rspec) { |c| c.syntax = [:expect, :should] }
|
config.expect_with(:rspec) { |c| c.syntax = [:expect, :should] }
|
||||||
config.color = true
|
config.color = true
|
||||||
config.filter_run_excluding :couchdb_only # CouchDB-mechanic specs with no Couchbase equivalent
|
config.filter_run_excluding :couchdb_only # CouchDB-mechanic specs with no Couchbase equivalent
|
||||||
|
# Reverse lookups into embedded documents (querying a foreign key held
|
||||||
|
# *inside* an embedded array element) are not implemented on Couchbase —
|
||||||
|
# they would need an UNNEST-based query path. Documented gap.
|
||||||
|
config.filter_run_excluding :embedded_in_reverse_lookup
|
||||||
|
|
||||||
config.before(:suite) do
|
config.before(:suite) do
|
||||||
SimplyBased.database_url = COUCHBASE_URL
|
SimplyBased.database_url = COUCHBASE_URL
|
||||||
|
|||||||
Reference in new issue
Block a user