From 9051e5766b7d54294124952f8850cd62b3b1456c Mon Sep 17 00:00:00 2001 From: Julien BURNET-FAUCHE Date: Tue, 28 Oct 2025 17:26:46 +0100 Subject: [PATCH 1/6] Add raise_on_unknown_attributes class attribute and attribute_writer_missing - Add class_attribute :raise_on_unknown_attributes with default: true - Implement attribute_writer_missing to handle unknown attributes - When set to false, logs warning instead of raising exception --- lib/couchbase-orm/base.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/couchbase-orm/base.rb b/lib/couchbase-orm/base.rb index 9a3bdb7..8556157 100644 --- a/lib/couchbase-orm/base.rb +++ b/lib/couchbase-orm/base.rb @@ -53,6 +53,10 @@ class Document class MismatchTypeError < RuntimeError; end + # Configuration option to control whether unknown attributes should raise an error + # Set to false to silently ignore unknown attributes during mass assignment + class_attribute :raise_on_unknown_attributes, default: true + def initialize(model = nil, ignore_doc_type: false, **attributes) CouchbaseOrm.logger.debug { "Initialize model #{model} with #{attributes.to_s.truncate(200)}" } @__metadata__ = Metadata.new @@ -100,6 +104,17 @@ def []=(key, value) send(:"#{key}=", value) end + # Handle assignment to unknown attributes based on raise_on_unknown_attributes configuration + # If raise_on_unknown_attributes is false, unknown attributes are silently ignored + # If raise_on_unknown_attributes is true (default), ActiveModel::UnknownAttributeError is raised + def attribute_writer_missing(name, value) + if self.class.raise_on_unknown_attributes + super + else + CouchbaseOrm.logger.warn "Ignoring unknown attribute '#{name}' for #{self.class.name}" + end + end + protected def serialized_attributes From 7a43338f73c1ca8abf8eadc7bd3530a36339ed9a Mon Sep 17 00:00:00 2001 From: Julien BURNET-FAUCHE Date: Tue, 28 Oct 2025 17:26:57 +0100 Subject: [PATCH 2/6] Filter unknown attributes in assign_attributes when raise_on_unknown_attributes is false - Modify assign_attributes to check raise_on_unknown_attributes setting - Filter out unknown attributes before calling super when disabled - Log warning for ignored attributes --- lib/couchbase-orm/persistence.rb | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/couchbase-orm/persistence.rb b/lib/couchbase-orm/persistence.rb index ea85e87..4f55f57 100644 --- a/lib/couchbase-orm/persistence.rb +++ b/lib/couchbase-orm/persistence.rb @@ -155,7 +155,18 @@ def update_attribute(name, value) def assign_attributes(hash) hash = hash.with_indifferent_access if hash.is_a?(Hash) - super(hash.except("type")) + + # Filter unknown attributes if raise_on_unknown_attributes is false + if !self.class.raise_on_unknown_attributes + known_attrs = hash.slice(*self.class.attribute_names).except("type") + unknown_attrs = hash.keys - self.class.attribute_names - ["type"] + if unknown_attrs.any? + CouchbaseOrm.logger.warn "Ignoring unknown attribute(s) for #{self.class.name}: #{unknown_attrs.join(', ')}" + end + super(known_attrs) + else + super(hash.except("type")) + end end # Updates the attributes of the model from the passed-in hash and saves the From da85db984f6f08fd508aa1417c78d0ce92e2a68a Mon Sep 17 00:00:00 2001 From: Julien BURNET-FAUCHE Date: Tue, 28 Oct 2025 17:27:08 +0100 Subject: [PATCH 3/6] Add comprehensive tests for raise_on_unknown_attributes feature - Add BaseTestWithUnknownAttributesAllowed test class - Test behavior when raise_on_unknown_attributes is false - Test default behavior (true) raises UnknownAttributeError - Cover new, assign_attributes, and attribute storage scenarios --- spec/base_spec.rb | 65 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/spec/base_spec.rb b/spec/base_spec.rb index 037da37..c42d2f2 100644 --- a/spec/base_spec.rb +++ b/spec/base_spec.rb @@ -22,6 +22,12 @@ class BaseTestWithIgnoredProperties < CouchbaseOrm::Base attribute :job, :string end +class BaseTestWithUnknownAttributesAllowed < CouchbaseOrm::Base + self.raise_on_unknown_attributes = false + attribute :name, :string + attribute :job, :string +end + class BaseTestWithPropertiesAlwaysExistsInDocument < CouchbaseOrm::Base self.properties_always_exists_in_document = true attribute :name, :string @@ -349,6 +355,65 @@ class InvalidNested < CouchbaseOrm::NestedDocument end end + describe 'handling unknown attributes' do + context 'when raise_on_unknown_attributes is set to false' do + it 'returns false when queried' do + expect(BaseTestWithUnknownAttributesAllowed.raise_on_unknown_attributes).to be(false) + end + + it 'silently ignores unknown attributes in new' do + model = BaseTestWithUnknownAttributesAllowed.new(name: 'test', job: 'dev', unknown_attr: 'value') + expect(model.name).to eq('test') + expect(model.job).to eq('dev') + expect(model.respond_to?(:unknown_attr)).to be(false) + end + + it 'silently ignores unknown attributes in assign_attributes' do + model = BaseTestWithUnknownAttributesAllowed.new(name: 'test') + expect { + model.assign_attributes(name: 'updated', job: 'engineer', foo: 'bar', baz: 'qux') + }.not_to raise_error + expect(model.name).to eq('updated') + expect(model.job).to eq('engineer') + expect(model.respond_to?(:foo)).to be(false) + expect(model.respond_to?(:baz)).to be(false) + end + + it 'only stores known attributes' do + model = BaseTestWithUnknownAttributesAllowed.new( + name: 'Alice', + job: 'Developer', + unknown_field_1: 'value1', + unknown_field_2: 'value2' + ) + # Only known attributes should be stored + expect(model.name).to eq('Alice') + expect(model.job).to eq('Developer') + expect(model.respond_to?(:unknown_field_1)).to be(false) + expect(model.respond_to?(:unknown_field_2)).to be(false) + end + end + + context 'default behavior (raise_on_unknown_attributes = true)' do + it 'returns true by default' do + expect(BaseTest.raise_on_unknown_attributes).to be(true) + end + + it 'raises ActiveModel::UnknownAttributeError on unknown attributes in new' do + expect { + BaseTest.new(name: 'bob', job: 'dev', foo: 'bar') + }.to raise_error(ActiveModel::UnknownAttributeError) + end + + it 'raises ActiveModel::UnknownAttributeError on unknown attributes in assign_attributes' do + model = BaseTest.new(name: 'bob') + expect { + model.assign_attributes(job: 'dev', foo: 'bar') + }.to raise_error(ActiveModel::UnknownAttributeError) + end + end + end + describe '.properties_always_exists_in_document' do it 'Uses NOT VALUED when properties_always_exists_in_document = false' do where_clause = BaseTest.where(name: nil) From 551d89c6d4e94b5523f21fa6a31cea723426a469 Mon Sep 17 00:00:00 2001 From: Julien BURNET-FAUCHE Date: Tue, 28 Oct 2025 18:13:01 +0100 Subject: [PATCH 4/6] Refactor: Move assign_attributes to Document class for consistency - Move assign_attributes override from Persistence to Document class - Ensures consistent behavior for both Document and NestedDocument - Use Set for efficient unknown attribute lookup (performance optimization) - Add require 'set' to base.rb - Remove duplicate implementation from persistence.rb This addresses code review feedback about consistency and performance. --- lib/couchbase-orm/base.rb | 24 ++++++++++++++++++++++++ lib/couchbase-orm/persistence.rb | 17 ++--------------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/lib/couchbase-orm/base.rb b/lib/couchbase-orm/base.rb index 8556157..6cb386f 100644 --- a/lib/couchbase-orm/base.rb +++ b/lib/couchbase-orm/base.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true, encoding: ASCII-8BIT +require 'set' require 'active_model' require 'active_support/hash_with_indifferent_access' require 'couchbase' @@ -115,6 +116,29 @@ def attribute_writer_missing(name, value) end end + # Override assign_attributes to filter unknown attributes when raise_on_unknown_attributes is false + # This ensures consistent behavior across Document and NestedDocument + def assign_attributes(hash) + hash = hash.with_indifferent_access if hash.is_a?(Hash) + + if self.class.raise_on_unknown_attributes + super(hash.except("type")) + else + # Filter unknown attributes + known_names = self.class.attribute_names + known_attrs = hash.slice(*known_names) + + # Use a Set for efficient lookup of unknown keys + known_names_set = known_names.to_set + unknown_keys = hash.keys.reject { |k| known_names_set.include?(k) || k == "type" } + + if unknown_keys.any? + CouchbaseOrm.logger.warn "Ignoring unknown attribute(s) for #{self.class.name}: #{unknown_keys.join(', ')}" + end + super(known_attrs) + end + end + protected def serialized_attributes diff --git a/lib/couchbase-orm/persistence.rb b/lib/couchbase-orm/persistence.rb index 4f55f57..4844489 100644 --- a/lib/couchbase-orm/persistence.rb +++ b/lib/couchbase-orm/persistence.rb @@ -153,21 +153,8 @@ def update_attribute(name, value) changed? ? save(validate: false) : true end - def assign_attributes(hash) - hash = hash.with_indifferent_access if hash.is_a?(Hash) - - # Filter unknown attributes if raise_on_unknown_attributes is false - if !self.class.raise_on_unknown_attributes - known_attrs = hash.slice(*self.class.attribute_names).except("type") - unknown_attrs = hash.keys - self.class.attribute_names - ["type"] - if unknown_attrs.any? - CouchbaseOrm.logger.warn "Ignoring unknown attribute(s) for #{self.class.name}: #{unknown_attrs.join(', ')}" - end - super(known_attrs) - else - super(hash.except("type")) - end - end + # Note: assign_attributes is now handled in Document class (base.rb) + # to ensure consistent behavior across Document and NestedDocument # Updates the attributes of the model from the passed-in hash and saves the # record. If the object is invalid, the saving will fail and false will be returned. From d763a15d880140bcaf8768a2791b6750428b7ee7 Mon Sep 17 00:00:00 2001 From: Julien BURNET-FAUCHE Date: Tue, 28 Oct 2025 18:22:42 +0100 Subject: [PATCH 5/6] Add comprehensive tests for raise_on_unknown_attributes with NestedDocument - Add test classes: NestedDocWithUnknownAttributesAllowed and NestedDocWithDefaultBehavior - Add parent document test classes to test nested behavior - Test NestedDocument with raise_on_unknown_attributes = false - Test NestedDocument with default behavior (true) - Test nested documents within parent documents - Test assign_attributes on NestedDocument - All 14 tests pass (7 for base documents + 7 for nested documents) --- spec/base_spec.rb | 72 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/spec/base_spec.rb b/spec/base_spec.rb index c42d2f2..bcfa0fb 100644 --- a/spec/base_spec.rb +++ b/spec/base_spec.rb @@ -28,6 +28,27 @@ class BaseTestWithUnknownAttributesAllowed < CouchbaseOrm::Base attribute :job, :string end +class NestedDocWithUnknownAttributesAllowed < CouchbaseOrm::NestedDocument + self.raise_on_unknown_attributes = false + attribute :title, :string + attribute :value, :integer +end + +class NestedDocWithDefaultBehavior < CouchbaseOrm::NestedDocument + attribute :title, :string + attribute :value, :integer +end + +class ParentDocWithNestedUnknownAllowed < CouchbaseOrm::Base + attribute :name, :string + attribute :nested_item, :nested, type: NestedDocWithUnknownAttributesAllowed +end + +class ParentDocWithNestedDefault < CouchbaseOrm::Base + attribute :name, :string + attribute :nested_item, :nested, type: NestedDocWithDefaultBehavior +end + class BaseTestWithPropertiesAlwaysExistsInDocument < CouchbaseOrm::Base self.properties_always_exists_in_document = true attribute :name, :string @@ -412,6 +433,57 @@ class InvalidNested < CouchbaseOrm::NestedDocument }.to raise_error(ActiveModel::UnknownAttributeError) end end + + context 'for NestedDocument classes' do + it 'returns false when queried on NestedDocument with raise_on_unknown_attributes = false' do + expect(NestedDocWithUnknownAttributesAllowed.raise_on_unknown_attributes).to be(false) + end + + it 'returns true by default on NestedDocument' do + expect(NestedDocWithDefaultBehavior.raise_on_unknown_attributes).to be(true) + end + + it 'silently ignores unknown attributes in NestedDocument when disabled' do + nested = NestedDocWithUnknownAttributesAllowed.new(title: 'Test', value: 42, unknown: 'ignored') + expect(nested.title).to eq('Test') + expect(nested.value).to eq(42) + expect(nested.respond_to?(:unknown)).to be(false) + end + + it 'raises error for unknown attributes in NestedDocument when enabled' do + expect { + NestedDocWithDefaultBehavior.new(title: 'Test', value: 42, unknown: 'error') + }.to raise_error(ActiveModel::UnknownAttributeError) + end + + it 'silently ignores unknown attributes in nested documents within parent' do + parent = ParentDocWithNestedUnknownAllowed.new(name: 'Parent') + nested = NestedDocWithUnknownAttributesAllowed.new(title: 'Nested', value: 100, extra: 'ignored') + parent.nested_item = nested + + expect(parent.nested_item.title).to eq('Nested') + expect(parent.nested_item.value).to eq(100) + expect(parent.nested_item.respond_to?(:extra)).to be(false) + end + + it 'raises error for unknown attributes in nested documents with default behavior' do + parent = ParentDocWithNestedDefault.new(name: 'Parent') + expect { + NestedDocWithDefaultBehavior.new(title: 'Nested', value: 100, extra: 'error') + }.to raise_error(ActiveModel::UnknownAttributeError) + end + + it 'works with assign_attributes on NestedDocument' do + nested = NestedDocWithUnknownAttributesAllowed.new(title: 'Initial') + expect { + nested.assign_attributes(title: 'Updated', value: 50, unknown_field: 'ignored') + }.not_to raise_error + + expect(nested.title).to eq('Updated') + expect(nested.value).to eq(50) + expect(nested.respond_to?(:unknown_field)).to be(false) + end + end end describe '.properties_always_exists_in_document' do From 3222a85a6254f688684d238aedd5617f26b98569 Mon Sep 17 00:00:00 2001 From: Julien BURNET-FAUCHE Date: Tue, 28 Oct 2025 18:29:37 +0100 Subject: [PATCH 6/6] Performance optimization: Cache attribute_names Set - Add attribute_names_set class method with memoization - Use cached Set in assign_attributes instead of converting on each call - Eliminates O(m) Set conversion overhead, where m = number of attributes - Maintains O(1) lookup performance for unknown attribute detection - All 14 tests pass --- lib/couchbase-orm/base.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/couchbase-orm/base.rb b/lib/couchbase-orm/base.rb index 6cb386f..9950b98 100644 --- a/lib/couchbase-orm/base.rb +++ b/lib/couchbase-orm/base.rb @@ -58,6 +58,12 @@ class MismatchTypeError < RuntimeError; end # Set to false to silently ignore unknown attributes during mass assignment class_attribute :raise_on_unknown_attributes, default: true + # Returns a cached Set of attribute names for efficient lookup + # This avoids repeated array-to-set conversions in assign_attributes + def self.attribute_names_set + @attribute_names_set ||= attribute_names.to_set + end + def initialize(model = nil, ignore_doc_type: false, **attributes) CouchbaseOrm.logger.debug { "Initialize model #{model} with #{attributes.to_s.truncate(200)}" } @__metadata__ = Metadata.new @@ -124,13 +130,12 @@ def assign_attributes(hash) if self.class.raise_on_unknown_attributes super(hash.except("type")) else - # Filter unknown attributes + # Filter unknown attributes using cached Set for O(1) lookups known_names = self.class.attribute_names known_attrs = hash.slice(*known_names) - # Use a Set for efficient lookup of unknown keys - known_names_set = known_names.to_set - unknown_keys = hash.keys.reject { |k| known_names_set.include?(k) || k == "type" } + # Use cached Set for efficient O(1) lookup of unknown keys + unknown_keys = hash.keys.reject { |k| self.class.attribute_names_set.include?(k) || k == "type" } if unknown_keys.any? CouchbaseOrm.logger.warn "Ignoring unknown attribute(s) for #{self.class.name}: #{unknown_keys.join(', ')}"