From a29067d8d48e40097bc225e5708198b1a4dca726 Mon Sep 17 00:00:00 2001 From: Dan Healy <dan@beyondludus.com> Date: Fri, 12 Jul 2024 08:50:13 -0700 Subject: [PATCH] MONGOID-5789 database_field_name given nil or empty string should raise UnknownAttribute exception (#5836) * database_field_name given nil or empty string should raise UnknownAttribute exception * fix spec syntax * database_field_name return empty string instead of exception * `field` might be an empty string, not merely nil * fix specs to expect empty string instead of nil This is okay, because the database_field_name method is a private API. We can change the contract here without regard for who else might be using it. --------- Co-authored-by: Jamis Buck <jamis.buck@mongodb.com> --- lib/mongoid/fields.rb | 4 ++-- lib/mongoid/touchable.rb | 2 +- spec/mongoid/attributes_spec.rb | 16 ++++++++++++++++ spec/mongoid/fields_spec.rb | 6 +++--- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/mongoid/fields.rb b/lib/mongoid/fields.rb index ef12e18e2f..1b05623a9d 100644 --- a/lib/mongoid/fields.rb +++ b/lib/mongoid/fields.rb @@ -405,12 +405,12 @@ def traverse_association_tree(key, fields, associations, aliased_associations) # # @api private def database_field_name(name, relations, aliased_fields, aliased_associations) + return '' unless name.present? + if Mongoid.broken_alias_handling - return nil unless name normalized = name.to_s aliased_fields[normalized] || normalized else - return nil unless name.present? key = name.to_s segment, remaining = key.split('.', 2) diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index 6fd8aeeb7e..238658b987 100644 --- a/lib/mongoid/touchable.rb +++ b/lib/mongoid/touchable.rb @@ -25,7 +25,7 @@ def touch(field = nil) current = Time.configured.now field = database_field_name(field) write_attribute(:updated_at, current) if respond_to?("updated_at=") - write_attribute(field, current) if field + write_attribute(field, current) if field.present? # If the document being touched is embedded, touch its parents # all the way through the composition hierarchy to the root object, diff --git a/spec/mongoid/attributes_spec.rb b/spec/mongoid/attributes_spec.rb index fe8b32f15f..3c2e610693 100644 --- a/spec/mongoid/attributes_spec.rb +++ b/spec/mongoid/attributes_spec.rb @@ -288,6 +288,22 @@ end end + context "when given nil" do + + it "returns nil" do + expect(person[nil]).to be nil + end + + end + + context "when given an empty string" do + + it "returns nil" do + expect(person[""]).to be nil + end + + end + context "when the field was not explicitly defined" do context "when excluding with only and the field was not excluded" do diff --git a/spec/mongoid/fields_spec.rb b/spec/mongoid/fields_spec.rb index 4c7a0268e4..43d2113e9c 100644 --- a/spec/mongoid/fields_spec.rb +++ b/spec/mongoid/fields_spec.rb @@ -1845,12 +1845,12 @@ class DiscriminatorChild2 < DiscriminatorParent context 'given nil' do subject { Person.database_field_name(nil) } - it { is_expected.to eq nil } + it { is_expected.to eq '' } end context 'given an empty String' do subject { Person.database_field_name('') } - it { is_expected.to eq nil } + it { is_expected.to eq '' } end context 'given a String' do @@ -1869,7 +1869,7 @@ class DiscriminatorChild2 < DiscriminatorParent context 'given nil' do subject { Person.database_field_name(nil) } - it { is_expected.to eq nil } + it { is_expected.to eq '' } end context 'given an empty String' do