From aa30e8d84cbbb7d73acd135ba69f90c36b23c657 Mon Sep 17 00:00:00 2001 From: johnnyshields <27655+johnnyshields@users.noreply.github.com> Date: Sun, 27 Aug 2023 19:51:54 +0900 Subject: [PATCH 1/5] Move Hash#__nested__ monkey patch method to new module Mongoid::Attributes::Embedded.traverse --- lib/mongoid/attributes.rb | 3 +- lib/mongoid/attributes/embedded.rb | 35 +++++++ lib/mongoid/attributes/nested.rb | 2 +- lib/mongoid/extensions/hash.rb | 22 ----- lib/mongoid/reloadable.rb | 22 +---- lib/mongoid/utils.rb | 14 --- spec/mongoid/attributes/embedded_spec.rb | 119 +++++++++++++++++++++++ spec/mongoid/extensions/hash_spec.rb | 62 ------------ 8 files changed, 160 insertions(+), 119 deletions(-) create mode 100644 lib/mongoid/attributes/embedded.rb create mode 100644 spec/mongoid/attributes/embedded_spec.rb diff --git a/lib/mongoid/attributes.rb b/lib/mongoid/attributes.rb index 888d2c8fdf..063cfd3caa 100644 --- a/lib/mongoid/attributes.rb +++ b/lib/mongoid/attributes.rb @@ -3,6 +3,7 @@ require "active_model/attribute_methods" require "mongoid/attributes/dynamic" +require "mongoid/attributes/embedded" require "mongoid/attributes/nested" require "mongoid/attributes/processing" require "mongoid/attributes/projector" @@ -299,7 +300,7 @@ def read_raw_attribute(name) if fields.key?(normalized) attributes[normalized] else - attributes.__nested__(normalized) + Embedded.traverse(attributes, normalized) end else attributes[normalized] diff --git a/lib/mongoid/attributes/embedded.rb b/lib/mongoid/attributes/embedded.rb new file mode 100644 index 0000000000..45727eda43 --- /dev/null +++ b/lib/mongoid/attributes/embedded.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Mongoid + module Attributes + + # Utility module for working with embedded attributes. + module Embedded + extend self + + # Fetch an embedded value or subset of attributes via dot notation. + # + # @example Fetch an embedded value via dot notation. + # Embedded.traverse({ 'name' => { 'en' => 'test' } }, 'name.en') + # #=> 'test' + # + # @param [ Hash ] attributes The document attributes. + # @param [ String ] path The dot notation string. + # + # @return [ Object | nil ] The attributes at the given path, + # or nil if the path doesn't exist. + def traverse(attributes, path) + path.split('.').each do |key| + break if attributes.nil? + + attributes = if attributes.try(:key?, key) + attributes[key] + elsif key.match?(/\A\d+\z/) && attributes.respond_to?(:each) + attributes[key.to_i] + end + end + attributes + end + end + end +end diff --git a/lib/mongoid/attributes/nested.rb b/lib/mongoid/attributes/nested.rb index 75eca7e540..e57d07460a 100644 --- a/lib/mongoid/attributes/nested.rb +++ b/lib/mongoid/attributes/nested.rb @@ -4,7 +4,7 @@ module Mongoid module Attributes - # Defines behavior around that lovel Rails feature nested attributes. + # Defines behavior for the Rails nested attributes feature. module Nested extend ActiveSupport::Concern diff --git a/lib/mongoid/extensions/hash.rb b/lib/mongoid/extensions/hash.rb index 02f49097bb..00104c38ab 100644 --- a/lib/mongoid/extensions/hash.rb +++ b/lib/mongoid/extensions/hash.rb @@ -127,28 +127,6 @@ def extract_id self["_id"] || self[:_id] || self["id"] || self[:id] end - # Fetch a nested value via dot syntax. - # - # @example Fetch a nested value via dot syntax. - # { "name" => { "en" => "test" }}.__nested__("name.en") - # - # @param [ String ] string the dot syntax string. - # - # @return [ Object ] The matching value. - def __nested__(string) - keys = string.split(".") - value = self - keys.each do |key| - return nil if value.nil? - value_for_key = value[key] - if value_for_key.nil? && key.to_i.to_s == key - value_for_key = value[key.to_i] - end - value = value_for_key - end - value - end - # Turn the object from the ruby type we deal with to a Mongo friendly # type. # diff --git a/lib/mongoid/reloadable.rb b/lib/mongoid/reloadable.rb index 7a4d27a29b..e90b72168b 100644 --- a/lib/mongoid/reloadable.rb +++ b/lib/mongoid/reloadable.rb @@ -91,26 +91,10 @@ def reload_root_document # # @return [ Hash ] The reloaded attributes. def reload_embedded_document - extract_embedded_attributes( - collection(_root).find(_root.atomic_selector, session: _session).read(mode: :primary).first + Mongoid::Attributes::Embedded.traverse( + collection(_root).find(_root.atomic_selector, session: _session).read(mode: :primary).first, + atomic_position ) end - - # Extract only the desired embedded document from the attributes. - # - # @example Extract the embedded document. - # document.extract_embedded_attributes(attributes) - # - # @param [ Hash ] attributes The document in the db. - # - # @return [ Hash | nil ] The document's extracted attributes or nil if the - # document doesn't exist. - def extract_embedded_attributes(attributes) - # rubocop:disable Lint/UnmodifiedReduceAccumulator - atomic_position.split('.').inject(attributes) do |attrs, part| - attrs[Utils.maybe_integer(part)] - end - # rubocop:enable Lint/UnmodifiedReduceAccumulator - end end end diff --git a/lib/mongoid/utils.rb b/lib/mongoid/utils.rb index 3c43e4f277..b3ac761410 100644 --- a/lib/mongoid/utils.rb +++ b/lib/mongoid/utils.rb @@ -22,20 +22,6 @@ def placeholder?(value) value == PLACEHOLDER end - # If value can be coerced to an integer, return it as an integer. - # Otherwise, return the value itself. - # - # @param [ String ] value the string to possibly coerce. - # - # @return [ String | Integer ] the result of the coercion. - def maybe_integer(value) - if value.match?(/^\d/) - value.to_i - else - value - end - end - # This function should be used if you need to measure time. # @example Calculate elapsed time. # starting = Utils.monotonic_time diff --git a/spec/mongoid/attributes/embedded_spec.rb b/spec/mongoid/attributes/embedded_spec.rb new file mode 100644 index 0000000000..ddf3b87de6 --- /dev/null +++ b/spec/mongoid/attributes/embedded_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Mongoid::Attributes::Embedded do + + describe '.traverse' do + subject(:embedded) { described_class.traverse(attributes, path) } + + let(:path) { '100.name' } + + context 'when the attribute key is a string' do + let(:attributes) { { '100' => { 'name' => 'hundred' } } } + + it 'retrieves an embedded value under the provided key' do + expect(embedded).to eq 'hundred' + end + + context 'when the value is false' do + let(:attributes) { { '100' => { 'name' => false } } } + + it 'retrieves the embedded value under the provided key' do + expect(embedded).to be false + end + end + + context 'when the value does not exist' do + let(:attributes) { { '100' => { 0 => 'Please do not return this value!' } } } + + it 'retrieves the nil embedded value under the provided key' do + expect(embedded).to be_nil + end + end + end + + context 'when the attribute key is an integer' do + let(:attributes) { { 100 => { 'name' => 'hundred' } } } + + it 'retrieves an embedded value under the provided key' do + expect(embedded).to eq 'hundred' + end + end + + context 'when the attribute key is nil' do + let(:attributes) { { 100 => { 'name' => nil } } } + + it 'returns nil' do + expect(embedded).to be_nil + end + end + + context 'when both string and integer keys are present' do + let(:attributes) { { '100' => { 'name' => 'Fred' }, 100 => { 'name' => 'Daphne' } } } + + it 'returns the string key value' do + expect(embedded).to eq 'Fred' + end + + context 'when the string key value is nil' do + let(:attributes) { { '100' => nil, 100 => { 'name' => 'Daphne' } } } + + it 'returns nil' do + expect(embedded).to be_nil + end + end + end + + context 'when attributes is an array' do + let(:attributes) do + [ { 'name' => 'Fred' }, { 'name' => 'Daphne' }, { 'name' => 'Velma' }, { 'name' => 'Shaggy' } ] + end + let(:path) { '2.name' } + + it 'retrieves the nth value' do + expect(embedded).to eq 'Velma' + end + + context 'when the member does not exist' do + let(:attributes) { [ { 'name' => 'Fred' }, { 'name' => 'Daphne' } ] } + + it 'returns nil' do + expect(embedded).to be_nil + end + end + end + + context 'when the path includes a scalar value' do + let(:attributes) { { '100' => 'name' } } + + it 'returns nil' do + expect(embedded).to be_nil + end + end + + context 'when the parent key is not present' do + let(:attributes) { { '101' => { 'name' => 'hundred and one' } } } + + it 'returns nil' do + expect(embedded).to be_nil + end + end + + context 'when the attributes are deeply nested' do + let(:attributes) { { '100' => { 'name' => { 300 => %w[a b c] } } } } + + it 'retrieves the embedded subset of attributes' do + expect(embedded).to eq(300 => %w[a b c]) + end + + context 'when the path is deeply nested' do + let(:path) { '100.name.300.1' } + + it 'retrieves the embedded value' do + expect(embedded).to eq 'b' + end + end + end + end +end diff --git a/spec/mongoid/extensions/hash_spec.rb b/spec/mongoid/extensions/hash_spec.rb index c110b233f6..cc4135a742 100644 --- a/spec/mongoid/extensions/hash_spec.rb +++ b/spec/mongoid/extensions/hash_spec.rb @@ -220,68 +220,6 @@ end end - context "when the hash key is a string" do - - let(:hash) do - { "100" => { "name" => "hundred" } } - end - - let(:nested) do - hash.__nested__("100.name") - end - - it "should retrieve a nested value under the provided key" do - expect(nested).to eq "hundred" - end - - context 'and the value is falsey' do - let(:hash) do - { "100" => { "name" => false } } - end - it "should retrieve the falsey nested value under the provided key" do - expect(nested).to eq false - end - end - - context 'and the value is nil' do - let(:hash) do - { "100" => { 0 => "Please don't return this value!" } } - end - it "should retrieve the nil nested value under the provided key" do - expect(nested).to eq nil - end - end - end - - context "when the hash key is an integer" do - let(:hash) do - { 100 => { "name" => "hundred" } } - end - - let(:nested) do - hash.__nested__("100.name") - end - - it "should retrieve a nested value under the provided key" do - expect(nested).to eq("hundred") - end - end - - context "when the parent key is not present" do - - let(:hash) do - { "101" => { "name" => "hundred and one" } } - end - - let(:nested) do - hash.__nested__("100.name") - end - - it "should return nil" do - expect(nested).to eq(nil) - end - end - describe ".demongoize" do let(:hash) do From a962b25d732c78061d6bc50458cccbd7c9aa23e0 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Sun, 27 Aug 2023 19:58:14 +0900 Subject: [PATCH 2/5] Fix whitespace --- lib/mongoid/attributes/embedded.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/mongoid/attributes/embedded.rb b/lib/mongoid/attributes/embedded.rb index 45727eda43..a958d09f09 100644 --- a/lib/mongoid/attributes/embedded.rb +++ b/lib/mongoid/attributes/embedded.rb @@ -2,7 +2,6 @@ module Mongoid module Attributes - # Utility module for working with embedded attributes. module Embedded extend self From aa5f3ec8aea50b53851c32921e3981bf4ee3cedc Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Sun, 27 Aug 2023 20:00:23 +0900 Subject: [PATCH 3/5] Fix rubocop whitespace warning --- spec/mongoid/attributes/embedded_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/mongoid/attributes/embedded_spec.rb b/spec/mongoid/attributes/embedded_spec.rb index ddf3b87de6..4f9b65768c 100644 --- a/spec/mongoid/attributes/embedded_spec.rb +++ b/spec/mongoid/attributes/embedded_spec.rb @@ -3,7 +3,6 @@ require 'spec_helper' describe Mongoid::Attributes::Embedded do - describe '.traverse' do subject(:embedded) { described_class.traverse(attributes, path) } From 636d429d571c826deabe6f98c225e67edc8c8066 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Sun, 27 Aug 2023 20:31:01 +0900 Subject: [PATCH 4/5] Update embedded.rb --- lib/mongoid/attributes/embedded.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mongoid/attributes/embedded.rb b/lib/mongoid/attributes/embedded.rb index a958d09f09..7269f909d1 100644 --- a/lib/mongoid/attributes/embedded.rb +++ b/lib/mongoid/attributes/embedded.rb @@ -23,7 +23,7 @@ def traverse(attributes, path) attributes = if attributes.try(:key?, key) attributes[key] - elsif key.match?(/\A\d+\z/) && attributes.respond_to?(:each) + elsif attributes.respond_to?(:each) && key.match?(/\A\d+\z/) attributes[key.to_i] end end From f068dd09e154c4f4e7a0f4637a80185f4c3f9b1f Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Mon, 28 Aug 2023 15:53:06 -0600 Subject: [PATCH 5/5] minor changes to test names --- spec/mongoid/attributes/embedded_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/mongoid/attributes/embedded_spec.rb b/spec/mongoid/attributes/embedded_spec.rb index 4f9b65768c..f06269d272 100644 --- a/spec/mongoid/attributes/embedded_spec.rb +++ b/spec/mongoid/attributes/embedded_spec.rb @@ -26,7 +26,7 @@ context 'when the value does not exist' do let(:attributes) { { '100' => { 0 => 'Please do not return this value!' } } } - it 'retrieves the nil embedded value under the provided key' do + it 'returns nil' do expect(embedded).to be_nil end end @@ -40,7 +40,7 @@ end end - context 'when the attribute key is nil' do + context 'when the attribute value is nil' do let(:attributes) { { 100 => { 'name' => nil } } } it 'returns nil' do