From f674f2ec2ed4656575faac00ba2e4e07b3043ff9 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 29 Aug 2023 06:53:59 +0900 Subject: [PATCH 01/28] MONGOID-5653 - Move Hash#__nested__ monkey patch method to new module Mongoid::Attributes::Embedded.traverse (#5692) * Move Hash#__nested__ monkey patch method to new module Mongoid::Attributes::Embedded.traverse * Fix whitespace * Fix rubocop whitespace warning * Update embedded.rb * minor changes to test names --------- Co-authored-by: Jamis Buck --- lib/mongoid/attributes.rb | 3 +- lib/mongoid/attributes/embedded.rb | 34 +++++++ 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 | 118 +++++++++++++++++++++++ spec/mongoid/extensions/hash_spec.rb | 62 ------------ 8 files changed, 158 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..7269f909d1 --- /dev/null +++ b/lib/mongoid/attributes/embedded.rb @@ -0,0 +1,34 @@ +# 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 attributes.respond_to?(:each) && key.match?(/\A\d+\z/) + 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..f06269d272 --- /dev/null +++ b/spec/mongoid/attributes/embedded_spec.rb @@ -0,0 +1,118 @@ +# 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 'returns nil' 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 value 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 2840061e72af5ffba555fa82d6a5a90b038ad4f4 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 29 Aug 2023 07:18:43 +0900 Subject: [PATCH 02/28] Upgrade libmongocrypt-helper gem to latest (#5691) --- gemfiles/standard.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gemfiles/standard.rb b/gemfiles/standard.rb index 1428a0e7ee..59cdf8d0ff 100644 --- a/gemfiles/standard.rb +++ b/gemfiles/standard.rb @@ -50,6 +50,6 @@ def standard_dependencies end if ENV['FLE'] == 'helper' - gem 'libmongocrypt-helper', '~> 1.7.0' + gem 'libmongocrypt-helper', '~> 1.8.0' end end From cce678547a603658c4802c6c1b3f4479a4e76f23 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Tue, 29 Aug 2023 11:30:17 -0600 Subject: [PATCH 03/28] MONGOID-5647 Allow #count to be used with #for_js (#5693) * MONGOID-5647 Allow #count to be used with #for_js * look for $where in nested hashes * fix typo in method name --- lib/mongoid/contextual/mongo.rb | 25 ++++++++++++++++++++++++- spec/mongoid/contextual/mongo_spec.rb | 10 ++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/mongoid/contextual/mongo.rb b/lib/mongoid/contextual/mongo.rb index 14b85124a8..8797b7efd1 100644 --- a/lib/mongoid/contextual/mongo.rb +++ b/lib/mongoid/contextual/mongo.rb @@ -74,7 +74,12 @@ class Mongo # @return [ Integer ] The number of matches. def count(options = {}, &block) return super(&block) if block_given? - view.count_documents(options) + + if valid_for_count_documents? + view.count_documents(options) + else + view.count(options) + end end # Get the estimated number of documents matching the query. @@ -1038,6 +1043,24 @@ def process_raw_docs(raw_docs, limit) limit ? docs : docs.first end + # Queries whether the current context is valid for use with + # the #count_documents? predicate. A context is valid if it + # does not include a `$where` operator. + # + # @return [ true | false ] whether or not the current context + # excludes a `$where` operator. + def valid_for_count_documents?(hash = view.filter) + # Note that `view.filter` is a BSON::Document, and all keys in a + # BSON::Document are strings; we don't need to worry about symbol + # representations of `$where`. + hash.keys.each do |key| + return false if key == '$where' + return false if hash[key].is_a?(Hash) && !valid_for_count_documents?(hash[key]) + end + + true + end + def raise_document_not_found_error raise Errors::DocumentNotFound.new(klass, nil, nil) end diff --git a/spec/mongoid/contextual/mongo_spec.rb b/spec/mongoid/contextual/mongo_spec.rb index cf4c534ab1..fd55839939 100644 --- a/spec/mongoid/contextual/mongo_spec.rb +++ b/spec/mongoid/contextual/mongo_spec.rb @@ -159,6 +159,16 @@ end end end + + context 'when for_js is present' do + let(:context) do + Band.for_js('this.name == "Depeche Mode"') + end + + it 'counts the expected records' do + expect(context.count).to eq(1) + end + end end describe "#estimated_count" do From 30f46d47950e9d1facad302bd5cc5ae2284ecc2c Mon Sep 17 00:00:00 2001 From: Alex Bevilacqua Date: Fri, 1 Sep 2023 07:20:49 -0400 Subject: [PATCH 04/28] MONGOID-5649: added network compression docs (#5697) --- docs/reference/configuration.txt | 49 ++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/docs/reference/configuration.txt b/docs/reference/configuration.txt index 507511c557..ffb4e8e36e 100644 --- a/docs/reference/configuration.txt +++ b/docs/reference/configuration.txt @@ -738,6 +738,55 @@ For more information about TLS context hooks, including best practices for assigning and removing them, see `the Ruby driver documentation `_. +Network Compression +=================== + +Mongoid supports compression of messages to and from MongoDB servers. This functionality is provided by +the Ruby driver, which implements the three algorithms that are supported by MongoDB servers: + +- `Snappy `_: ``snappy`` compression + can be used when connecting to MongoDB servers starting with the 3.4 release, + and requires the `snappy `_ library to be + installed. +- `Zlib `_: ``zlib`` compression can be used when + connecting to MongoDB servers starting with the 3.6 release. +- `Zstandard `_: ``zstd`` compression can be + used when connecting to MongoDB servers starting with the 4.2 release, and + requires the `zstd-ruby `_ library to + be installed. + +To use wire protocol compression, at least one compressor must be explicitly requested +using either the `compressors URI option `_, +or directly within the ``mongoid.yml``: + +.. code-block:: yaml + + development: + # Configure available database clients. (required) + clients: + # Define the default client. (required) + default: + # ... + options: + # These options are Ruby driver options, documented in + # https://mongodb.com/docs/ruby-driver/current/reference/create-client/ + # ... + # Compressors to use. (default is to not use compression) + # Valid values are zstd, zlib or snappy - or any combination of the three + compressors: ["zstd", "snappy"] + +If no compressors are explicitly requested, the driver will not use compression, +even if the required dependencies for one or more compressors are present on the +system. + +The driver chooses the first compressor of the ones requested that is also supported +by the server. The ``zstd`` compressor is recommended as it produces the highest +compression at the same CPU consumption compared to the other compressors. + +For maximum server compatibility all three compressors can be specified, e.g. +as ``compressors: ["zstd", "snappy", "zlib"]``. + + Client-Side Encryption ====================== From 60eb57d31e29710f8aeb378ff22ae14103c72c80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mustafa=20Ate=C5=9F=20Uzun?= Date: Fri, 8 Sep 2023 18:32:27 +0300 Subject: [PATCH 05/28] fix: Rakefile desc spelling (#5719) --- Rakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index c7e0093008..61323eae4e 100644 --- a/Rakefile +++ b/Rakefile @@ -89,7 +89,7 @@ desc "Generate all documentation" task :docs => 'docs:yard' namespace :docs do - desc "Generate yard documention" + desc "Generate yard documentation" task :yard do out = File.join('yard-docs', Mongoid::VERSION) FileUtils.rm_rf(out) From c9e55f713149e44ee26e9c42b86c369e941840e5 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Tue, 12 Sep 2023 08:02:49 -0600 Subject: [PATCH 06/28] convenience tasks for evergreen (#5720) --- Rakefile | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/Rakefile b/Rakefile index 61323eae4e..0a02a96754 100644 --- a/Rakefile +++ b/Rakefile @@ -41,6 +41,32 @@ RSpec::Core::RakeTask.new('spec:progress') do |spec| spec.pattern = "spec/**/*_spec.rb" end +desc 'Build and validate the evergreen config' +task eg: %w[ eg:build eg:validate ] + +# 'eg' == 'evergreen', but evergreen is too many letters for convenience +namespace :eg do + desc 'Builds the .evergreen/config.yml file from the templates' + task :build do + ruby '.evergreen/update-evergreen-configs' + end + + desc 'Validates the .evergreen/config.yml file' + task :validate do + system 'evergreen validate --project mongoid .evergreen/config.yml' + end + + desc 'Updates the evergreen executable to the latest available version' + task :update do + system 'evergreen get-update --install' + end + + desc 'Runs the current branch as an evergreen patch' + task :patch do + system 'evergreen patch --uncommitted --project mongoid --browse --auto-description --yes' + end +end + CLASSIFIERS = [ [%r,^mongoid/attribute,, :attributes], [%r,^mongoid/association/[or],, :associations_referenced], From f40bb73c23c0aea6459009b9b0c24f7a878603fe Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Thu, 14 Sep 2023 10:20:24 +0200 Subject: [PATCH 07/28] RUBY-5651 Deprecate for_js API (#5721) --- docs/release-notes/mongoid-9.0.txt | 6 ++++++ lib/mongoid/contextual/mongo.rb | 5 +++++ lib/mongoid/criteria.rb | 4 ++++ lib/mongoid/deprecation.rb | 9 ++++++--- spec/mongoid/criteria_spec.rb | 6 ++++++ 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/docs/release-notes/mongoid-9.0.txt b/docs/release-notes/mongoid-9.0.txt index 0a5ab664bd..b47977010a 100644 --- a/docs/release-notes/mongoid-9.0.txt +++ b/docs/release-notes/mongoid-9.0.txt @@ -18,6 +18,12 @@ please consult GitHub releases for detailed release notes and JIRA for the complete list of issues fixed in each release, including bug fixes. +``for_js`` method is deprecated +------------------------------- + +The ``for_js`` method is deprecated and will be removed in Mongoid 10.0. + + Deprecated options removed -------------------------- diff --git a/lib/mongoid/contextual/mongo.rb b/lib/mongoid/contextual/mongo.rb index 8797b7efd1..916d8a2a72 100644 --- a/lib/mongoid/contextual/mongo.rb +++ b/lib/mongoid/contextual/mongo.rb @@ -78,6 +78,8 @@ def count(options = {}, &block) if valid_for_count_documents? view.count_documents(options) else + # TODO: Remove this when we remove the deprecated for_js API. + # https://jira.mongodb.org/browse/MONGOID-5681 view.count(options) end end @@ -1049,6 +1051,9 @@ def process_raw_docs(raw_docs, limit) # # @return [ true | false ] whether or not the current context # excludes a `$where` operator. + # + # TODO: Remove this method when we remove the deprecated for_js API. + # https://jira.mongodb.org/browse/MONGOID-5681 def valid_for_count_documents?(hash = view.filter) # Note that `view.filter` is a BSON::Document, and all keys in a # BSON::Document are strings; we don't need to worry about symbol diff --git a/lib/mongoid/criteria.rb b/lib/mongoid/criteria.rb index e90a7c751f..ad365b5a8f 100644 --- a/lib/mongoid/criteria.rb +++ b/lib/mongoid/criteria.rb @@ -41,6 +41,8 @@ class Criteria include Clients::Sessions include Options + Mongoid.deprecate(self, :for_js) + # Static array used to check with method missing - we only need to ever # instantiate once. CHECK = [] @@ -439,6 +441,8 @@ def without_options # @param [ Hash ] scope The scope for the code. # # @return [ Criteria ] The criteria. + # + # @deprecated def for_js(javascript, scope = {}) code = if scope.empty? # CodeWithScope is not supported for $where as of MongoDB 4.4 diff --git a/lib/mongoid/deprecation.rb b/lib/mongoid/deprecation.rb index c97c909f75..4fb299418f 100644 --- a/lib/mongoid/deprecation.rb +++ b/lib/mongoid/deprecation.rb @@ -6,10 +6,13 @@ module Mongoid # Utility class for logging deprecation warnings. class Deprecation < ::ActiveSupport::Deprecation - @gem_name = 'Mongoid' + def initialize + # Per change policy, deprecations will be removed in the next major version. + deprecation_horizon = "#{Mongoid::VERSION.split('.').first.to_i + 1}.0".freeze + gem_name = 'Mongoid' + super(deprecation_horizon, gem_name) + end - # Per change policy, deprecations will be removed in the next major version. - @deprecation_horizon = "#{Mongoid::VERSION.split('.').first.to_i + 1}.0".freeze # Overrides default ActiveSupport::Deprecation behavior # to use Mongoid's logger. diff --git a/spec/mongoid/criteria_spec.rb b/spec/mongoid/criteria_spec.rb index a18da5a356..6996341e2c 100644 --- a/spec/mongoid/criteria_spec.rb +++ b/spec/mongoid/criteria_spec.rb @@ -2977,6 +2977,12 @@ def self.ages; self; end Band.create!(name: "Depeche Mode") end + it 'is deprecated' do + expect(Mongoid.logger).to receive(:warn).with(/for_js is deprecated/).and_call_original + + Band.for_js("this.name == 'Depeche Mode'") + end + context "when the code has no scope" do let(:criteria) do From 8f74b689c2295616d4042725655f45917e062bb7 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Thu, 14 Sep 2023 08:05:34 -0600 Subject: [PATCH 08/28] MONGOID-5656 Rails specs ought to enable FLE (#5718) * make sure and test the rails specs with fle enabled * only run FLE with Rails 7 (due to OS constraints with cmake/libmongocrypt-helper) * see about making GH actions build and use libmongocrypt-helper * make sure command-line parsing doesn't pick up arguments that weren't intended for it * a bit more information for deciphering what's wrong under GH actions * *grumble* use a real exception class * run `rake ci` instead, to better isolate the specs * disambiguate test names * use logger to report error in collection creation also, use log-levels to minimize log spam in tests, instead of stubbing the logger --- .evergreen/config.yml | 3 +- .evergreen/config/variants.yml.erb | 5 ++- .github/workflows/test.yml | 41 ++++++------------- lib/mongoid/tasks/database.rb | 3 ++ lib/mongoid/tasks/encryption.rake | 51 ++++++++++++++++-------- spec/mongoid/tasks/database_rake_spec.rb | 4 +- 6 files changed, 56 insertions(+), 51 deletions(-) diff --git a/.evergreen/config.yml b/.evergreen/config.yml index 1eda42aa3a..3aff112ac7 100644 --- a/.evergreen/config.yml +++ b/.evergreen/config.yml @@ -680,7 +680,8 @@ buildvariants: mongodb-version: "6.0" topology: "standalone" rails: ['7.0'] - os: debian11 + os: ubuntu-22.04 + fle: helper display_name: "${rails}, ${driver}, ${mongodb-version}" tasks: - name: "test" diff --git a/.evergreen/config/variants.yml.erb b/.evergreen/config/variants.yml.erb index 43098d9f1f..b0d633844e 100644 --- a/.evergreen/config/variants.yml.erb +++ b/.evergreen/config/variants.yml.erb @@ -116,8 +116,9 @@ buildvariants: mongodb-version: "6.0" topology: "standalone" rails: ['7.0'] - os: debian11 - display_name: "${rails}, ${driver}, ${mongodb-version}" + os: ubuntu-22.04 + fle: helper + display_name: "${rails}, ${driver}, ${mongodb-version} (FLE ${fle})" tasks: - name: "test" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 70285f0cab..dc8961f5cf 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,8 +7,8 @@ name: Run Mongoid Tests - pull_request jobs: build: - name: "${{matrix.ruby}} driver-${{matrix.driver}} mongodb-${{matrix.mongodb}} - ${{matrix.topology}}" + name: "${{matrix.ruby}} drv:${{matrix.driver}} db:${{matrix.mongodb}} + rails:${{matrix.rails}} fle:${{matrix.fle}} ${{matrix.topology}}" env: CI: true TESTOPTS: "-v" @@ -24,8 +24,6 @@ jobs: os: ubuntu-20.04 task: test driver: current - rails: - i18n: gemfile: Gemfile experimental: false - mongodb: '6.0' @@ -34,8 +32,6 @@ jobs: os: ubuntu-20.04 task: test driver: current - rails: - i18n: gemfile: Gemfile experimental: false - mongodb: '6.0' @@ -44,8 +40,6 @@ jobs: os: ubuntu-20.04 task: test driver: current - rails: - i18n: gemfile: Gemfile experimental: false - mongodb: '6.0' @@ -54,8 +48,6 @@ jobs: os: ubuntu-20.04 task: test driver: master - rails: - i18n: gemfile: gemfiles/driver_master.gemfile experimental: true - mongodb: '6.0' @@ -64,8 +56,6 @@ jobs: os: ubuntu-20.04 task: test driver: stable - rails: - i18n: gemfile: gemfiles/driver_stable.gemfile experimental: false - mongodb: '6.0' @@ -75,7 +65,7 @@ jobs: task: test driver: current rails: '7.0' - i18n: + fle: helper gemfile: gemfiles/rails-7.0.gemfile experimental: false - mongodb: '6.0' @@ -85,7 +75,7 @@ jobs: task: test driver: current rails: '6.1' - i18n: + fle: helper gemfile: gemfiles/rails-6.1.gemfile experimental: false - mongodb: '6.0' @@ -95,7 +85,7 @@ jobs: task: test driver: current rails: '7.0' - i18n: + fle: helper gemfile: gemfiles/rails-7.0.gemfile experimental: false - mongodb: '6.0' @@ -105,7 +95,7 @@ jobs: task: test driver: current rails: '6.1' - i18n: + fle: helper gemfile: gemfiles/rails-6.1.gemfile experimental: false - mongodb: '6.0' @@ -115,7 +105,7 @@ jobs: task: test driver: current rails: '6.0' - i18n: + fle: helper gemfile: gemfiles/rails-6.0.gemfile experimental: false - mongodb: '6.0' @@ -125,7 +115,7 @@ jobs: task: test driver: current rails: '5.2' - i18n: + fle: helper gemfile: gemfiles/rails-5.2.gemfile experimental: false - mongodb: '6.0' @@ -135,7 +125,7 @@ jobs: task: test driver: current rails: '6.0' - i18n: + fle: helper gemfile: gemfiles/rails-6.0.gemfile experimental: false - mongodb: '5.0' @@ -144,8 +134,6 @@ jobs: os: ubuntu-20.04 task: test driver: current - rails: - i18n: gemfile: Gemfile experimental: false - mongodb: '4.4' @@ -154,8 +142,6 @@ jobs: os: ubuntu-20.04 task: test driver: current - rails: - i18n: gemfile: Gemfile experimental: false - mongodb: '4.0' @@ -164,8 +150,6 @@ jobs: os: ubuntu-20.04 task: test driver: current - rails: - i18n: gemfile: Gemfile experimental: false - mongodb: '3.6' @@ -174,8 +158,6 @@ jobs: os: ubuntu-20.04 task: test driver: current - rails: - i18n: gemfile: Gemfile experimental: false @@ -193,6 +175,7 @@ jobs: - name: load ruby uses: ruby/setup-ruby@v1 env: + FLE: "${{matrix.fle}}" BUNDLE_GEMFILE: "${{matrix.gemfile}}" with: ruby-version: "${{matrix.ruby}}" @@ -200,11 +183,13 @@ jobs: - name: bundle run: bundle install --jobs 4 --retry 3 env: + FLE: "${{matrix.fle}}" BUNDLE_GEMFILE: "${{matrix.gemfile}}" - name: test timeout-minutes: 60 continue-on-error: "${{matrix.experimental}}" - run: bundle exec rake spec + run: bundle exec rake ci env: BUNDLE_GEMFILE: "${{matrix.gemfile}}" + FLE: "${{matrix.fle}}" MONGODB_URI: "${{ steps.start-mongodb.outputs.cluster-uri }}" diff --git a/lib/mongoid/tasks/database.rb b/lib/mongoid/tasks/database.rb index ab39f86e17..a779b7cfb5 100644 --- a/lib/mongoid/tasks/database.rb +++ b/lib/mongoid/tasks/database.rb @@ -26,6 +26,9 @@ def create_collections(models = ::Mongoid.models, force: false) else logger.info("MONGOID: collection options ignored on: #{model}, please define in the root model.") end + rescue Exception + logger.error "error while creating collection for #{model}" + raise end end diff --git a/lib/mongoid/tasks/encryption.rake b/lib/mongoid/tasks/encryption.rake index 2f9cf8cdfd..84871201ed 100644 --- a/lib/mongoid/tasks/encryption.rake +++ b/lib/mongoid/tasks/encryption.rake @@ -2,27 +2,45 @@ require 'optparse' -# rubocop:disable Metrics/BlockLength +def parse_data_key_options(argv = ARGV) + # The only way to use OptionParser to parse custom options in rake is + # to pass an empty argument ("--") before specifying them, e.g.: + # + # rake db:mongoid:encryption:create_data_key -- --client default + # + # Otherwise, rake complains about an unknown option. Thus, we can tell + # if the argument list is valid for us to parse by detecting this empty + # argument. + # + # (This works around an issue in the tests, where the specs are loading + # the tasks directly to test them, but the option parser is then picking + # up rspec command-line arguments and raising an exception). + return {} unless argv.include?('--') + + {}.tap do |options| + parser = OptionParser.new do |opts| + opts.on('-c', '--client CLIENT', 'Name of the client to use') do |v| + options[:client_name] = v + end + opts.on('-p', '--provider PROVIDER', 'KMS provider to use') do |v| + options[:kms_provider_name] = v + end + opts.on('-n', '--key-alt-name KEY_ALT_NAME', 'Alternate name for the key') do |v| + options[:key_alt_name] = v + end + end + # rubocop:disable Lint/EmptyBlock + parser.parse!(parser.order!(argv) {}) + # rubocop:enable Lint/EmptyBlock + end +end + namespace :db do namespace :mongoid do namespace :encryption do desc 'Create encryption key' task create_data_key: [ :environment ] do - options = {} - parser = OptionParser.new do |opts| - opts.on('-c', '--client CLIENT', 'Name of the client to use') do |v| - options[:client_name] = v - end - opts.on('-p', '--provider PROVIDER', 'KMS provider to use') do |v| - options[:kms_provider_name] = v - end - opts.on('-n', '--key-alt-name KEY_ALT_NAME', 'Alternate name for the key') do |v| - options[:key_alt_name] = v - end - end - # rubocop:disable Lint/EmptyBlock - parser.parse!(parser.order!(ARGV) {}) - # rubocop:enable Lint/EmptyBlock + options = parse_data_key_options result = Mongoid::Tasks::Encryption.create_data_key( client_name: options[:client_name], kms_provider_name: options[:kms_provider_name], @@ -39,4 +57,3 @@ namespace :db do end end end -# rubocop:enable Metrics/BlockLength diff --git a/spec/mongoid/tasks/database_rake_spec.rb b/spec/mongoid/tasks/database_rake_spec.rb index e920e99bea..9340b9d484 100644 --- a/spec/mongoid/tasks/database_rake_spec.rb +++ b/spec/mongoid/tasks/database_rake_spec.rb @@ -11,9 +11,7 @@ let(:task_file) { "mongoid/tasks/database" } let(:logger) do - double("logger").tap do |log| - allow(log).to receive(:info) - end + Logger.new(STDOUT, level: :error, formatter: ->(_sev, _dt, _prog, msg) { msg }) end before do From 34f7a919fbf6882f6f35b02403d939144afe1add Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Wed, 27 Sep 2023 15:24:42 -0600 Subject: [PATCH 09/28] MONGOID-5601 Atlas Search Index Management (#5723) * let's see if this'll start up an atlas cluster... * need to add a variant so the spec will actually run * appease rubocop * run on a supported os * need .mod/drivers-evergreen-tools * this got put in the wrong directory * get specs to pass * rubocop * finish specs for atlas index management * docs for the search index stuff * rubocop * return the correct value * add missing documentation --- .evergreen/config.yml | 169 ++++++++++++++++------- .evergreen/config/axes.yml.erb | 5 +- .evergreen/config/commands.yml.erb | 142 +++++++++++++------ .evergreen/config/variants.yml.erb | 10 ++ .evergreen/run-tests-atlas-full.sh | 24 ++++ .gitmodules | 3 + .mod/drivers-evergreen-tools | 1 + docs/reference/indexes.txt | 61 ++++++++ docs/release-notes/mongoid-9.0.txt | 47 +++++++ lib/mongoid/composable.rb | 2 + lib/mongoid/railties/database.rake | 5 + lib/mongoid/search_indexable.rb | 167 ++++++++++++++++++++++ lib/mongoid/tasks/database.rake | 11 ++ lib/mongoid/tasks/database.rb | 56 ++++++++ lib/mongoid/utils.rb | 11 ++ spec/lite_spec_helper.rb | 45 ++++-- spec/mongoid/search_indexable_spec.rb | 147 ++++++++++++++++++++ spec/mongoid/tasks/database_rake_spec.rb | 70 ++++++++++ spec/mongoid/tasks/database_spec.rb | 60 +++++++- spec/spec_helper.rb | 39 +++--- spec/support/spec_config.rb | 8 +- 21 files changed, 965 insertions(+), 118 deletions(-) create mode 100755 .evergreen/run-tests-atlas-full.sh create mode 160000 .mod/drivers-evergreen-tools create mode 100644 lib/mongoid/search_indexable.rb create mode 100644 spec/mongoid/search_indexable_spec.rb diff --git a/.evergreen/config.yml b/.evergreen/config.yml index 3aff112ac7..70e4ae92e6 100644 --- a/.evergreen/config.yml +++ b/.evergreen/config.yml @@ -44,50 +44,56 @@ functions: params: working_dir: "src" script: | - # Get the current unique version of this checkout - if [ "${is_patch}" = "true" ]; then - CURRENT_VERSION=$(git describe)-patch-${version_id} - else - CURRENT_VERSION=latest - fi - - export DRIVERS_TOOLS="$(pwd)/../drivers-tools" - - export MONGO_ORCHESTRATION_HOME="$DRIVERS_TOOLS/.evergreen/orchestration" - export MONGODB_BINARIES="$DRIVERS_TOOLS/mongodb/bin" - export UPLOAD_BUCKET="${project}" - export PROJECT_DIRECTORY="$(pwd)" - - cat < expansion.yml - CURRENT_VERSION: "$CURRENT_VERSION" - DRIVERS_TOOLS: "$DRIVERS_TOOLS" - MONGO_ORCHESTRATION_HOME: "$MONGO_ORCHESTRATION_HOME" - MONGODB_BINARIES: "$MONGODB_BINARIES" - UPLOAD_BUCKET: "$UPLOAD_BUCKET" - PROJECT_DIRECTORY: "$PROJECT_DIRECTORY" - PREPARE_SHELL: | - set -o errexit - set -o xtrace - export DRIVERS_TOOLS="$DRIVERS_TOOLS" - export MONGO_ORCHESTRATION_HOME="$MONGO_ORCHESTRATION_HOME" - export MONGODB_BINARIES="$MONGODB_BINARIES" - export UPLOAD_BUCKET="$UPLOAD_BUCKET" - export PROJECT_DIRECTORY="$PROJECT_DIRECTORY" - - export TMPDIR="$MONGO_ORCHESTRATION_HOME/db" - export PATH="$MONGODB_BINARIES:$PATH" - export PROJECT="${project}" - - export MONGODB_VERSION=${VERSION} - export TOPOLOGY=${TOPOLOGY} - export SINGLE_MONGOS=${SINGLE_MONGOS} - export AUTH=${AUTH} - export SSL=${SSL} - export APP_TESTS=${APP_TESTS} - export DOCKER_DISTRO=${DOCKER_DISTRO} - EOT - # See what we've done - cat expansion.yml + # Get the current unique version of this checkout + if [ "${is_patch}" = "true" ]; then + CURRENT_VERSION=$(git describe)-patch-${version_id} + else + CURRENT_VERSION=latest + fi + + export DRIVERS_TOOLS="$(pwd)/.mod/drivers-evergreen-tools" + + export MONGO_ORCHESTRATION_HOME="$DRIVERS_TOOLS/.evergreen/orchestration" + export MONGODB_BINARIES="$DRIVERS_TOOLS/mongodb/bin" + export UPLOAD_BUCKET="${project}" + export PROJECT_DIRECTORY="$(pwd)" + + cat < expansion.yml + CURRENT_VERSION: "$CURRENT_VERSION" + DRIVERS_TOOLS: "$DRIVERS_TOOLS" + MONGO_ORCHESTRATION_HOME: "$MONGO_ORCHESTRATION_HOME" + MONGODB_BINARIES: "$MONGODB_BINARIES" + UPLOAD_BUCKET: "$UPLOAD_BUCKET" + PROJECT_DIRECTORY: "$PROJECT_DIRECTORY" + PREPARE_SHELL: | + set -o errexit + set -o xtrace + export DRIVERS_TOOLS="$DRIVERS_TOOLS" + export MONGO_ORCHESTRATION_HOME="$MONGO_ORCHESTRATION_HOME" + export MONGODB_BINARIES="$MONGODB_BINARIES" + export UPLOAD_BUCKET="$UPLOAD_BUCKET" + export PROJECT_DIRECTORY="$PROJECT_DIRECTORY" + + export TMPDIR="$MONGO_ORCHESTRATION_HOME/db" + export PATH="$MONGODB_BINARIES:$PATH" + export PROJECT="${project}" + + export MONGODB_VERSION="${VERSION}" + export TOPOLOGY="${TOPOLOGY}" + export SINGLE_MONGOS="${SINGLE_MONGOS}" + export AUTH="${AUTH}" + export SSL="${SSL}" + export APP_TESTS="${APP_TESTS}" + export DOCKER_DISTRO="${DOCKER_DISTRO}" + export RVM_RUBY="${RVM_RUBY}" + export RAILS="${RAILS}" + export DRIVER="${DRIVER}" + export I18N="${I18N}" + export TEST_I18N_FALLBACKS="${TEST_I18N_FALLBACKS}" + export FLE="${FLE}" + EOT + # See what we've done + cat expansion.yml # Load the expansion file to make an evergreen variable with the current unique version - command: expansions.update @@ -266,7 +272,7 @@ functions: ${PREPARE_SHELL} env \ MONGODB_URI="${MONGODB_URI}" \ - TOPOLOGY=${TOPOLOGY} \ + TOPOLOGY="${TOPOLOGY}" \ RVM_RUBY="${RVM_RUBY}" \ RAILS="${RAILS}" \ DRIVER="${DRIVER}" \ @@ -307,10 +313,66 @@ post: #- func: "upload test results" - func: "upload test results to s3" +task_groups: + - name: testatlas_task_group + setup_group_can_fail_task: true + setup_group_timeout_secs: 1800 # 30 minutes + setup_group: + - func: fetch source + - func: create expansions + - command: shell.exec + params: + shell: "bash" + working_dir: "src" + script: | + ${PREPARE_SHELL} + + DRIVERS_ATLAS_PUBLIC_API_KEY="${DRIVERS_ATLAS_PUBLIC_API_KEY}" \ + DRIVERS_ATLAS_PRIVATE_API_KEY="${DRIVERS_ATLAS_PRIVATE_API_KEY}" \ + DRIVERS_ATLAS_GROUP_ID="${DRIVERS_ATLAS_GROUP_ID}" \ + DRIVERS_ATLAS_LAMBDA_USER="${DRIVERS_ATLAS_LAMBDA_USER}" \ + DRIVERS_ATLAS_LAMBDA_PASSWORD="${DRIVERS_ATLAS_LAMBDA_PASSWORD}" \ + LAMBDA_STACK_NAME="dbx-ruby-lambda" \ + MONGODB_VERSION="7.0" \ + task_id="${task_id}" \ + execution="${execution}" \ + $DRIVERS_TOOLS/.evergreen/atlas/setup-atlas-cluster.sh + - command: expansions.update + params: + file: src/atlas-expansion.yml + teardown_group: + - command: shell.exec + params: + shell: "bash" + working_dir: "src" + script: | + ${PREPARE_SHELL} + + DRIVERS_ATLAS_PUBLIC_API_KEY="${DRIVERS_ATLAS_PUBLIC_API_KEY}" \ + DRIVERS_ATLAS_PRIVATE_API_KEY="${DRIVERS_ATLAS_PRIVATE_API_KEY}" \ + DRIVERS_ATLAS_GROUP_ID="${DRIVERS_ATLAS_GROUP_ID}" \ + LAMBDA_STACK_NAME="dbx-ruby-lambda" \ + task_id="${task_id}" \ + execution="${execution}" \ + $DRIVERS_TOOLS/.evergreen/atlas/teardown-atlas-cluster.sh + tasks: + - test-full-atlas-task + tasks: - name: "test" commands: - func: "run tests" + - name: "test-full-atlas-task" + commands: + - command: shell.exec + type: test + params: + working_dir: "src" + shell: "bash" + script: | + ${PREPARE_SHELL} + MONGODB_URI="${MONGODB_URI}" \ + .evergreen/run-tests-atlas-full.sh axes: - id: "mongodb-version" display_name: MongoDB Version @@ -432,13 +494,16 @@ axes: - id: "os" display_name: OS values: + - id: actual-ubuntu-22.04 + display_name: "Ubuntu 22.04" + run_on: ubuntu2204-small - id: ubuntu-18.04 display_name: "Ubuntu 18.04" run_on: ubuntu2004-small variables: DOCKER_DISTRO: ubuntu1804 - id: ubuntu-22.04 - display_name: "Ubuntu 20.04" + display_name: "Ubuntu 22.04" run_on: ubuntu2004-small variables: DOCKER_DISTRO: ubuntu2204 @@ -682,7 +747,7 @@ buildvariants: rails: ['7.0'] os: ubuntu-22.04 fle: helper - display_name: "${rails}, ${driver}, ${mongodb-version}" + display_name: "${rails}, ${driver}, ${mongodb-version} (FLE ${fle})" tasks: - name: "test" @@ -809,3 +874,13 @@ buildvariants: display_name: "FLE: ${rails}, ${driver}, ${mongodb-version}" tasks: - name: "test" + +- matrix_name: atlas-full + matrix_spec: + ruby: ruby-3.2 + os: actual-ubuntu-22.04 + auth: auth + ssl: ssl + display_name: "Atlas (Full)" + tasks: + - name: testatlas_task_group diff --git a/.evergreen/config/axes.yml.erb b/.evergreen/config/axes.yml.erb index c5e8b8f878..2c1e5a44c3 100644 --- a/.evergreen/config/axes.yml.erb +++ b/.evergreen/config/axes.yml.erb @@ -119,13 +119,16 @@ axes: - id: "os" display_name: OS values: + - id: actual-ubuntu-22.04 + display_name: "Ubuntu 22.04" + run_on: ubuntu2204-small - id: ubuntu-18.04 display_name: "Ubuntu 18.04" run_on: ubuntu2004-small variables: DOCKER_DISTRO: ubuntu1804 - id: ubuntu-22.04 - display_name: "Ubuntu 20.04" + display_name: "Ubuntu 22.04" run_on: ubuntu2004-small variables: DOCKER_DISTRO: ubuntu2204 diff --git a/.evergreen/config/commands.yml.erb b/.evergreen/config/commands.yml.erb index 6d16b19908..396ab4531b 100644 --- a/.evergreen/config/commands.yml.erb +++ b/.evergreen/config/commands.yml.erb @@ -18,50 +18,56 @@ functions: params: working_dir: "src" script: | - # Get the current unique version of this checkout - if [ "${is_patch}" = "true" ]; then - CURRENT_VERSION=$(git describe)-patch-${version_id} - else - CURRENT_VERSION=latest - fi + # Get the current unique version of this checkout + if [ "${is_patch}" = "true" ]; then + CURRENT_VERSION=$(git describe)-patch-${version_id} + else + CURRENT_VERSION=latest + fi - export DRIVERS_TOOLS="$(pwd)/../drivers-tools" + export DRIVERS_TOOLS="$(pwd)/.mod/drivers-evergreen-tools" - export MONGO_ORCHESTRATION_HOME="$DRIVERS_TOOLS/.evergreen/orchestration" - export MONGODB_BINARIES="$DRIVERS_TOOLS/mongodb/bin" - export UPLOAD_BUCKET="${project}" - export PROJECT_DIRECTORY="$(pwd)" + export MONGO_ORCHESTRATION_HOME="$DRIVERS_TOOLS/.evergreen/orchestration" + export MONGODB_BINARIES="$DRIVERS_TOOLS/mongodb/bin" + export UPLOAD_BUCKET="${project}" + export PROJECT_DIRECTORY="$(pwd)" - cat < expansion.yml - CURRENT_VERSION: "$CURRENT_VERSION" - DRIVERS_TOOLS: "$DRIVERS_TOOLS" - MONGO_ORCHESTRATION_HOME: "$MONGO_ORCHESTRATION_HOME" - MONGODB_BINARIES: "$MONGODB_BINARIES" - UPLOAD_BUCKET: "$UPLOAD_BUCKET" - PROJECT_DIRECTORY: "$PROJECT_DIRECTORY" - PREPARE_SHELL: | - set -o errexit - set -o xtrace - export DRIVERS_TOOLS="$DRIVERS_TOOLS" - export MONGO_ORCHESTRATION_HOME="$MONGO_ORCHESTRATION_HOME" - export MONGODB_BINARIES="$MONGODB_BINARIES" - export UPLOAD_BUCKET="$UPLOAD_BUCKET" - export PROJECT_DIRECTORY="$PROJECT_DIRECTORY" + cat < expansion.yml + CURRENT_VERSION: "$CURRENT_VERSION" + DRIVERS_TOOLS: "$DRIVERS_TOOLS" + MONGO_ORCHESTRATION_HOME: "$MONGO_ORCHESTRATION_HOME" + MONGODB_BINARIES: "$MONGODB_BINARIES" + UPLOAD_BUCKET: "$UPLOAD_BUCKET" + PROJECT_DIRECTORY: "$PROJECT_DIRECTORY" + PREPARE_SHELL: | + set -o errexit + set -o xtrace + export DRIVERS_TOOLS="$DRIVERS_TOOLS" + export MONGO_ORCHESTRATION_HOME="$MONGO_ORCHESTRATION_HOME" + export MONGODB_BINARIES="$MONGODB_BINARIES" + export UPLOAD_BUCKET="$UPLOAD_BUCKET" + export PROJECT_DIRECTORY="$PROJECT_DIRECTORY" - export TMPDIR="$MONGO_ORCHESTRATION_HOME/db" - export PATH="$MONGODB_BINARIES:$PATH" - export PROJECT="${project}" + export TMPDIR="$MONGO_ORCHESTRATION_HOME/db" + export PATH="$MONGODB_BINARIES:$PATH" + export PROJECT="${project}" - export MONGODB_VERSION=${VERSION} - export TOPOLOGY=${TOPOLOGY} - export SINGLE_MONGOS=${SINGLE_MONGOS} - export AUTH=${AUTH} - export SSL=${SSL} - export APP_TESTS=${APP_TESTS} - export DOCKER_DISTRO=${DOCKER_DISTRO} - EOT - # See what we've done - cat expansion.yml + export MONGODB_VERSION="${VERSION}" + export TOPOLOGY="${TOPOLOGY}" + export SINGLE_MONGOS="${SINGLE_MONGOS}" + export AUTH="${AUTH}" + export SSL="${SSL}" + export APP_TESTS="${APP_TESTS}" + export DOCKER_DISTRO="${DOCKER_DISTRO}" + export RVM_RUBY="${RVM_RUBY}" + export RAILS="${RAILS}" + export DRIVER="${DRIVER}" + export I18N="${I18N}" + export TEST_I18N_FALLBACKS="${TEST_I18N_FALLBACKS}" + export FLE="${FLE}" + EOT + # See what we've done + cat expansion.yml # Load the expansion file to make an evergreen variable with the current unique version - command: expansions.update @@ -240,7 +246,7 @@ functions: ${PREPARE_SHELL} env \ MONGODB_URI="${MONGODB_URI}" \ - TOPOLOGY=${TOPOLOGY} \ + TOPOLOGY="${TOPOLOGY}" \ RVM_RUBY="${RVM_RUBY}" \ RAILS="${RAILS}" \ DRIVER="${DRIVER}" \ @@ -281,7 +287,63 @@ post: #- func: "upload test results" - func: "upload test results to s3" +task_groups: + - name: testatlas_task_group + setup_group_can_fail_task: true + setup_group_timeout_secs: 1800 # 30 minutes + setup_group: + - func: fetch source + - func: create expansions + - command: shell.exec + params: + shell: "bash" + working_dir: "src" + script: | + ${PREPARE_SHELL} + + DRIVERS_ATLAS_PUBLIC_API_KEY="${DRIVERS_ATLAS_PUBLIC_API_KEY}" \ + DRIVERS_ATLAS_PRIVATE_API_KEY="${DRIVERS_ATLAS_PRIVATE_API_KEY}" \ + DRIVERS_ATLAS_GROUP_ID="${DRIVERS_ATLAS_GROUP_ID}" \ + DRIVERS_ATLAS_LAMBDA_USER="${DRIVERS_ATLAS_LAMBDA_USER}" \ + DRIVERS_ATLAS_LAMBDA_PASSWORD="${DRIVERS_ATLAS_LAMBDA_PASSWORD}" \ + LAMBDA_STACK_NAME="dbx-ruby-lambda" \ + MONGODB_VERSION="7.0" \ + task_id="${task_id}" \ + execution="${execution}" \ + $DRIVERS_TOOLS/.evergreen/atlas/setup-atlas-cluster.sh + - command: expansions.update + params: + file: src/atlas-expansion.yml + teardown_group: + - command: shell.exec + params: + shell: "bash" + working_dir: "src" + script: | + ${PREPARE_SHELL} + + DRIVERS_ATLAS_PUBLIC_API_KEY="${DRIVERS_ATLAS_PUBLIC_API_KEY}" \ + DRIVERS_ATLAS_PRIVATE_API_KEY="${DRIVERS_ATLAS_PRIVATE_API_KEY}" \ + DRIVERS_ATLAS_GROUP_ID="${DRIVERS_ATLAS_GROUP_ID}" \ + LAMBDA_STACK_NAME="dbx-ruby-lambda" \ + task_id="${task_id}" \ + execution="${execution}" \ + $DRIVERS_TOOLS/.evergreen/atlas/teardown-atlas-cluster.sh + tasks: + - test-full-atlas-task + tasks: - name: "test" commands: - func: "run tests" + - name: "test-full-atlas-task" + commands: + - command: shell.exec + type: test + params: + working_dir: "src" + shell: "bash" + script: | + ${PREPARE_SHELL} + MONGODB_URI="${MONGODB_URI}" \ + .evergreen/run-tests-atlas-full.sh diff --git a/.evergreen/config/variants.yml.erb b/.evergreen/config/variants.yml.erb index b0d633844e..9348e6b05b 100644 --- a/.evergreen/config/variants.yml.erb +++ b/.evergreen/config/variants.yml.erb @@ -245,3 +245,13 @@ buildvariants: display_name: "FLE: ${rails}, ${driver}, ${mongodb-version}" tasks: - name: "test" + +- matrix_name: atlas-full + matrix_spec: + ruby: ruby-3.2 + os: actual-ubuntu-22.04 + auth: auth + ssl: ssl + display_name: "Atlas (Full)" + tasks: + - name: testatlas_task_group diff --git a/.evergreen/run-tests-atlas-full.sh b/.evergreen/run-tests-atlas-full.sh new file mode 100755 index 0000000000..f3114b8168 --- /dev/null +++ b/.evergreen/run-tests-atlas-full.sh @@ -0,0 +1,24 @@ +#!/bin/bash + +set -ex + +. `dirname "$0"`/../spec/shared/shlib/distro.sh +. `dirname "$0"`/../spec/shared/shlib/set_env.sh +. `dirname "$0"`/functions.sh + +set_env_vars +set_env_python +set_env_ruby + +export BUNDLE_GEMFILE=gemfiles/driver_master.gemfile +bundle install + +ATLAS_URI=$MONGODB_URI \ + EXAMPLE_TIMEOUT=600 \ + bundle exec rspec -fd spec/mongoid/search_indexable_spec.rb + +test_status=$? + +kill_jruby + +exit ${test_status} diff --git a/.gitmodules b/.gitmodules index 805feb77d5..05f15e6b98 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,3 +4,6 @@ [submodule "spec/shared"] path = spec/shared url = https://github.com/mongodb-labs/mongo-ruby-spec-shared +[submodule ".mod/drivers-evergreen-tools"] + path = .mod/drivers-evergreen-tools + url = https://github.com/mongodb-labs/drivers-evergreen-tools diff --git a/.mod/drivers-evergreen-tools b/.mod/drivers-evergreen-tools new file mode 160000 index 0000000000..1f018c7a24 --- /dev/null +++ b/.mod/drivers-evergreen-tools @@ -0,0 +1 @@ +Subproject commit 1f018c7a248c4fcda6cb7a77043fd673755e0986 diff --git a/docs/reference/indexes.txt b/docs/reference/indexes.txt index 6602dd0ff6..7a5d11a475 100644 --- a/docs/reference/indexes.txt +++ b/docs/reference/indexes.txt @@ -131,6 +131,28 @@ The ``background`` option has `no effect as of MongoDB 4.2 `_. +Specifying Search Indexes on MongoDB Atlas +========================================== + +If your application is connected to MongoDB Atlas, you can declare and manage +search indexes on your models. (This feature is only available on MongoDB +Atlas.) + +To declare a search index, use the ``search_index`` macro in your model: + +.. code-block:: ruby + + class Message + include Mongoid::Document + + search_index { ... } + search_index :named_index, { ... } + end + +Search indexes may be given an explicit name; this is necessary if you have +more than one search index on a model. + + Index Management Rake Tasks =========================== @@ -162,6 +184,45 @@ in Rails console: # Remove indexes for Model Model.remove_indexes +Managing Search Indexes on MongoDB Atlas +---------------------------------------- + +If you have defined search indexes on your model, there are rake tasks available +for creating and removing those search indexes: + +.. code-block:: bash + + $ rake db:mongoid:create_search_indexes + $ rake db:mongoid:remove_search_indexes + +By default, creating search indexes will wait for the indexes to be created, +which can take quite some time. If you want to simply let the database create +the indexes in the background, you can set the ``WAIT_FOR_SEARCH_INDEXES`` +environment variable to 0, like this: + +.. code-block:: bash + + $ rake WAIT_FOR_SEARCH_INDEXES=0 db:mongoid:create_search_indexes + +Note that the task for removing search indexes will remove all search indexes +from all models, and should be used with caution. + +You can also add and remove search indexes for a single model by invoking the +following in a Rails console: + +.. code-block:: ruby + + # Create all defined search indexes on the model; this will return + # immediately and the indexes will be created in the background. + Model.create_search_indexes + + # Remove all search indexes from the model + Model.remove_search_indexes + + # Enumerate all search indexes on the model + Model.search_indexes.each { |index| ... } + + Telling Mongoid Where to Look For Models ---------------------------------------- diff --git a/docs/release-notes/mongoid-9.0.txt b/docs/release-notes/mongoid-9.0.txt index b47977010a..1c6642e60d 100644 --- a/docs/release-notes/mongoid-9.0.txt +++ b/docs/release-notes/mongoid-9.0.txt @@ -338,6 +338,53 @@ Mongoid to allow literal BSON::Decimal128 fields: BSON 5 and later. BSON 4 and earlier ignore the setting entirely. +Search Index Management with MongoDB Atlas +------------------------------------------ + +When connected to MongoDB Atlas, Mongoid now supports creating and removing +search indexes. You may do so programmatically, via the Mongoid::SearchIndexable +API: + +.. code-block:: ruby + + class SearchablePerson + include Mongoid::Document + + search_index { ... } # define the search index here + end + + # create the declared search indexes; this returns immediately, but the + # search indexes may take several minutes before they are available. + SearchablePerson.create_search_indexes + + # query the available search indexes + SearchablePerson.search_indexes.each do |index| + # ... + end + + # remove all search indexes from the model's collection + SearchablePerson.remove_search_indexes + +If you are not connected to MongoDB Atlas, the search index definitions are +ignored. Trying to create, enumerate, or remove search indexes will result in +an error. + +There are also rake tasks available, for convenience: + +.. code-block:: bash + + # create search indexes for all models; waits for indexes to be created + # and shows progress on the terminal. + $ rake mongoid:db:create_search_indexes + + # as above, but returns immediately and lets the indexes be created in the + # background + $ rake WAIT_FOR_SEARCH_INDEXES=0 mongoid:db:create_search_indexes + + # removes search indexes from all models + $ rake mongoid:db:remove_search_indexes + + Bug Fixes and Improvements -------------------------- diff --git a/lib/mongoid/composable.rb b/lib/mongoid/composable.rb index 4afb69c24e..d935e88ca5 100644 --- a/lib/mongoid/composable.rb +++ b/lib/mongoid/composable.rb @@ -12,6 +12,7 @@ require "mongoid/matchable" require "mongoid/persistable" require "mongoid/reloadable" +require 'mongoid/search_indexable' require "mongoid/selectable" require "mongoid/scopable" require "mongoid/serializable" @@ -50,6 +51,7 @@ module Composable include Association include Reloadable include Scopable + include SearchIndexable include Selectable include Serializable include Shardable diff --git a/lib/mongoid/railties/database.rake b/lib/mongoid/railties/database.rake index d839602c29..5847b265ae 100644 --- a/lib/mongoid/railties/database.rake +++ b/lib/mongoid/railties/database.rake @@ -71,6 +71,11 @@ namespace :db do task :create_indexes => "mongoid:create_indexes" end + unless Rake::Task.task_defined?("db:create_search_indexes") + desc "Create search indexes specified in Mongoid models" + task :create_search_indexes => "mongoid:create_search_indexes" + end + unless Rake::Task.task_defined?("db:remove_indexes") desc "Remove indexes specified in Mongoid models" task :remove_indexes => "mongoid:remove_indexes" diff --git a/lib/mongoid/search_indexable.rb b/lib/mongoid/search_indexable.rb new file mode 100644 index 0000000000..1342539223 --- /dev/null +++ b/lib/mongoid/search_indexable.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +module Mongoid + # Encapsulates behavior around managing search indexes. This feature + # is only supported when connected to an Atlas cluster. + module SearchIndexable + extend ActiveSupport::Concern + + # Represents the status of the indexes returned by a search_indexes + # call. + # + # @api private + class Status + # @return [ Array ] the raw index documents + attr_reader :indexes + + # Create a new Status object. + # + # @param [ Array ] indexes the raw index documents + def initialize(indexes) + @indexes = indexes + end + + # Returns the subset of indexes that have status == 'READY' + # + # @return [ Array ] index documents for "ready" indices + def ready + indexes.select { |i| i['status'] == 'READY' } + end + + # Returns the subset of indexes that have status == 'PENDING' + # + # @return [ Array ] index documents for "pending" indices + def pending + indexes.select { |i| i['status'] == 'PENDING' } + end + + # Returns the subset of indexes that are marked 'queryable' + # + # @return [ Array ] index documents for 'queryable' indices + def queryable + indexes.select { |i| i['queryable'] } + end + + # Returns true if all the given indexes are 'ready' and 'queryable'. + # + # @return [ true | false ] ready status of all indexes + def ready? + indexes.all? { |i| i['status'] == 'READY' && i['queryable'] } + end + end + + included do + cattr_accessor :search_index_specs + self.search_index_specs = [] + end + + # Implementations for the feature's class-level methods. + module ClassMethods + # Request the creation of all registered search indices. Note + # that the search indexes are created asynchronously, and may take + # several minutes to be fully available. + # + # @return [ Array ] The names of the search indexes. + def create_search_indexes + return if search_index_specs.empty? + + collection.search_indexes.create_many(search_index_specs) + end + + # Waits for the named search indexes to be created. + # + # @param [ Array ] names the list of index names to wait for + # @param [ Integer ] interval the number of seconds to wait before + # polling again (only used when a progress callback is given). + # + # @yield [ SearchIndexable::Status ] the status object + def wait_for_search_indexes(names, interval: 5) + loop do + status = Status.new(get_indexes(names)) + yield status if block_given? + break if status.ready? + + sleep interval + end + end + + # A convenience method for querying the search indexes available on the + # current model's collection. + # + # @param [ Hash ] options the options to pass through to the search + # index query. + # + # @option options [ String ] :id The id of the specific index to query (optional) + # @option options [ String ] :name The name of the specific index to query (optional) + # @option options [ Hash ] :aggregate The options hash to pass to the + # aggregate command (optional) + def search_indexes(options = {}) + collection.search_indexes(options) + end + + # Removes the search index specified by the given name or id. Either + # name OR id must be given, but not both. + # + # @param [ String | nil ] name the name of the index to remove + # @param [ String | nil ] id the id of the index to remove + def remove_search_index(name: nil, id: nil) + logger.info( + "MONGOID: Removing search index '#{name || id}' " \ + "on collection '#{collection.name}'." + ) + + collection.search_indexes.drop_one(name: name, id: id) + end + + # Request the removal of all registered search indexes. Note + # that the search indexes are removed asynchronously, and may take + # several minutes to be fully deleted. + # + # @note It would be nice if this could remove ONLY the search indexes + # that have been declared on the model, but because the model may not + # name the index, we can't guarantee that we'll know the name or id of + # the corresponding indexes. It is not unreasonable to assume, though, + # that the intention is for the model to declare, one-to-one, all + # desired search indexes, so removing all search indexes ought to suffice. + # If a specific index or set of indexes needs to be removed instead, + # consider using search_indexes.each with remove_search_index. + def remove_search_indexes + search_indexes.each do |spec| + remove_search_index id: spec['id'] + end + end + + # Adds an index definition for the provided single or compound keys. + # + # @example Create a basic index. + # class Person + # include Mongoid::Document + # field :name, type: String + # search_index({ ... }) + # search_index :name_of_index, { ... } + # end + # + # @param [ Symbol | String | Hash ] name_or_defn Either the name of the index to + # define, or the index definition. + # @param [ Hash ] defn The search index definition. + def search_index(name_or_defn, defn = nil) + name = name_or_defn + name, defn = nil, name if name.is_a?(Hash) + + spec = { definition: defn }.tap { |s| s[:name] = name.to_s if name } + search_index_specs.push(spec) + end + + private + + # Retrieves the index records for the indexes with the given names. + # + # @param [ Array ] names the index names to query + # + # @return [ Array ] the raw index documents + def get_indexes(names) + collection.search_indexes.select { |i| names.include?(i['name']) } + end + end + end +end diff --git a/lib/mongoid/tasks/database.rake b/lib/mongoid/tasks/database.rake index 36a01de776..76786cfab1 100644 --- a/lib/mongoid/tasks/database.rake +++ b/lib/mongoid/tasks/database.rake @@ -17,6 +17,12 @@ namespace :db do ::Mongoid::Tasks::Database.create_indexes end + desc "Create search indexes specified in Mongoid models" + task :create_search_indexes => [:environment, :load_models] do + wait = Mongoid::Utils.truthy_string?(ENV['WAIT_FOR_SEARCH_INDEXES'] || '1') + ::Mongoid::Tasks::Database.create_search_indexes(wait: wait) + end + desc "Remove indexes that exist in the database but are not specified in Mongoid models" task :remove_undefined_indexes => [:environment, :load_models] do ::Mongoid::Tasks::Database.remove_undefined_indexes @@ -27,6 +33,11 @@ namespace :db do ::Mongoid::Tasks::Database.remove_indexes end + desc "Remove search indexes specified in Mongoid models" + task :remove_search_indexes => [:environment, :load_models] do + ::Mongoid::Tasks::Database.remove_search_indexes + end + desc "Shard collections with shard keys specified in Mongoid models" task :shard_collections => [:environment, :load_models] do ::Mongoid::Tasks::Database.shard_collections diff --git a/lib/mongoid/tasks/database.rb b/lib/mongoid/tasks/database.rb index a779b7cfb5..aa3edbcc79 100644 --- a/lib/mongoid/tasks/database.rb +++ b/lib/mongoid/tasks/database.rb @@ -56,6 +56,26 @@ def create_indexes(models = ::Mongoid.models) end.compact end + # Submit requests for the search indexes to be created. This will happen + # asynchronously. If "wait" is true, the method will block while it + # waits for the indexes to be created. + # + # @param [ Array ] models the models to build search + # indexes for. + # @param [ true | false ] wait whether to wait for the indexes to be + # built. + def create_search_indexes(models = ::Mongoid.models, wait: true) + searchable = models.select { |m| m.search_index_specs.any? } + + # queue up the search index creation requests + index_names_by_model = searchable.each_with_object({}) do |model, obj| + logger.info("MONGOID: Creating search indexes on #{model}...") + obj[model] = model.create_search_indexes + end + + wait_for_search_indexes(index_names_by_model) if wait + end + # Return the list of indexes by model that exist in the database but aren't # specified on the models. # @@ -128,6 +148,17 @@ def remove_indexes(models = ::Mongoid.models) end.compact end + # Remove all search indexes from the given models. + # + # @params [ Array ] models the models to remove + # search indexes from. + def remove_search_indexes(models = ::Mongoid.models) + models.each do |model| + next if model.embedded? + model.remove_search_indexes + end + end + # Shard collections for models that declare shard keys. # # Returns the model classes that have had their collections sharded, @@ -216,6 +247,31 @@ def shard_collections(models = ::Mongoid.models) def logger Mongoid.logger end + + # Waits for the search indexes to be built on the given models. + # + # @param [ Hash> ] models a mapping of + # index names for each model + def wait_for_search_indexes(models) + logger.info('MONGOID: Waiting for search indexes to be created') + logger.info('MONGOID: Press ctrl-c to skip the wait and let the indexes be created in the background') + + models.each do |model, names| + model.wait_for_search_indexes(names) do |status| + if status.ready? + puts + logger.info("MONGOID: Search indexes on #{model} are READY") + else + print '.' + $stdout.flush + end + end + end + rescue Interrupt + # ignore ctrl-C here; we assume it is meant only to skip + # the wait, and that subsequent tasks ought to continue. + logger.info('MONGOID: Skipping the wait for search indexes; they will be created in the background') + end end end end diff --git a/lib/mongoid/utils.rb b/lib/mongoid/utils.rb index b3ac761410..20f8dc2395 100644 --- a/lib/mongoid/utils.rb +++ b/lib/mongoid/utils.rb @@ -37,5 +37,16 @@ def placeholder?(value) def monotonic_time Process.clock_gettime(Process::CLOCK_MONOTONIC) end + + # Returns true if the string is any of the following values: "1", + # "yes", "true", "on". Anything else is assumed to be false. Case is + # ignored, as are leading or trailing spaces. + # + # @param [ String ] string the string value to consider + # + # @return [ true | false ] + def truthy_string?(string) + %w[ 1 yes true on ].include?(string.strip.downcase) + end end end diff --git a/spec/lite_spec_helper.rb b/spec/lite_spec_helper.rb index 19041be61f..d3868211e0 100644 --- a/spec/lite_spec_helper.rb +++ b/spec/lite_spec_helper.rb @@ -51,6 +51,28 @@ TimeoutInterrupt = Timeout end +STANDARD_TIMEOUTS = { + app: 500, # App tests under JRuby take a REALLY long time (over 5 minutes per test). + default: 30, +}.freeze + +def timeout_type + if ENV['EXAMPLE_TIMEOUT'].to_i > 0 + :custom + elsif SpecConfig.instance.app_tests? + :app + else + :default + end +end + +def example_timeout_seconds + STANDARD_TIMEOUTS.fetch( + timeout_type, + (ENV['EXAMPLE_TIMEOUT'] || STANDARD_TIMEOUTS[:default]).to_i + ) +end + RSpec.configure do |config| config.expect_with(:rspec) do |c| c.syntax = [:should, :expect] @@ -61,22 +83,25 @@ end if SpecConfig.instance.ci? && !%w(1 true yes).include?(ENV['INTERACTIVE']&.downcase) - timeout = if SpecConfig.instance.app_tests? - # App tests under JRuby take a REALLY long time (over 5 minutes per test). - 500 - else - # Allow a max of 30 seconds per test. - # Tests should take under 10 seconds ideally but it seems - # we have some that run for more than 10 seconds in CI. - 30 - end config.around(:each) do |example| - TimeoutInterrupt.timeout(timeout) do + TimeoutInterrupt.timeout(example_timeout_seconds) do example.run end end end + def local_env(env = nil, &block) + around do |example| + env ||= block.call + saved_env = ENV.to_h + ENV.update(env) + + example.run + ensure + ENV.replace(saved_env) if saved_env + end + end + config.extend(Mrss::LiteConstraints) end diff --git a/spec/mongoid/search_indexable_spec.rb b/spec/mongoid/search_indexable_spec.rb new file mode 100644 index 0000000000..22115623bc --- /dev/null +++ b/spec/mongoid/search_indexable_spec.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true + +require 'spec_helper' + +class SearchIndexHelper + attr_reader :model + + def initialize(model) + @model = model + model.collection.drop + model.collection.create + end + + def collection + model.collection + end + + # Wait for all of the indexes with the given names to be ready; then return + # the list of index definitions corresponding to those names. + def wait_for(*names, &condition) + names.flatten! + + timeboxed_wait do + result = collection.search_indexes + return filter_results(result, names) if names.all? { |name| ready?(result, name, &condition) } + end + end + + # Wait until all of the indexes with the given names are absent from the + # search index list. + def wait_for_absense_of(*names) + names.flatten.each do |name| + timeboxed_wait do + break if collection.search_indexes(name: name).empty? + end + end + end + + private + + def timeboxed_wait(step: 5, max: 300) + start = Mongo::Utils.monotonic_time + + loop do + yield + + sleep step + raise Timeout::Error, 'wait took too long' if Mongo::Utils.monotonic_time - start > max + end + end + + # Returns true if the list of search indexes includes one with the given name, + # which is ready to be queried. + def ready?(list, name, &condition) + condition ||= ->(index) { index['queryable'] } + list.any? { |index| index['name'] == name && condition[index] } + end + + def filter_results(result, names) + result.select { |index| names.include?(index['name']) } + end +end + +# rubocop:disable RSpec/MultipleMemoizedHelpers +describe Mongoid::SearchIndexable do + before do + skip "#{described_class} requires at Atlas environment (set ATLAS_URI)" if ENV['ATLAS_URI'].nil? + end + + let(:model) do + Class.new do + include Mongoid::Document + store_in collection: BSON::ObjectId.new.to_s + + search_index mappings: { dynamic: false } + search_index :with_dynamic_mappings, mappings: { dynamic: true } + end + end + + let(:helper) { SearchIndexHelper.new(model) } + + describe '.search_index_specs' do + context 'when no search indexes have been defined' do + it 'has no search index specs' do + expect(Person.search_index_specs).to be_empty + end + end + + context 'when search indexes have been defined' do + it 'has search index specs' do + expect(model.search_index_specs).to be == [ + { definition: { mappings: { dynamic: false } } }, + { name: 'with_dynamic_mappings', definition: { mappings: { dynamic: true } } } + ] + end + end + end + + context 'when needing to first create search indexes' do + let(:requested_definitions) { model.search_index_specs.map { |spec| spec[:definition].with_indifferent_access } } + let(:index_names) { model.create_search_indexes } + let(:actual_indexes) { helper.wait_for(*index_names) } + let(:actual_definitions) { actual_indexes.map { |i| i['latestDefinition'] } } + + describe '.create_search_indexes' do + it 'creates the indexes' do + expect(actual_definitions).to be == requested_definitions + end + end + + describe '.search_indexes' do + before { actual_indexes } # wait for the indices to be created + + let(:queried_definitions) { model.search_indexes.map { |i| i['latestDefinition'] } } + + it 'queries the available search indexes' do + expect(queried_definitions).to be == requested_definitions + end + end + + describe '.remove_search_index' do + let(:target_index) { actual_indexes.first } + + before do + model.remove_search_index id: target_index['id'] + helper.wait_for_absense_of target_index['name'] + end + + it 'removes the requested index' do + expect(model.search_indexes(id: target_index['id'])).to be_empty + end + end + + describe '.remove_search_indexes' do + before do + actual_indexes # wait for the indexes to be created + model.remove_search_indexes + helper.wait_for_absense_of(actual_indexes.map { |i| i['name'] }) + end + + it 'removes the indexes' do + expect(model.search_indexes).to be_empty + end + end + end +end +# rubocop:enable RSpec/MultipleMemoizedHelpers diff --git a/spec/mongoid/tasks/database_rake_spec.rb b/spec/mongoid/tasks/database_rake_spec.rb index 9340b9d484..c61adba2c5 100644 --- a/spec/mongoid/tasks/database_rake_spec.rb +++ b/spec/mongoid/tasks/database_rake_spec.rb @@ -31,6 +31,34 @@ end end + shared_examples_for 'create_search_indexes' do + [ nil, *%w( 1 true yes on ) ].each do |truthy| + context "when WAIT_FOR_SEARCH_INDEXES is #{truthy.inspect}" do + local_env 'WAIT_FOR_SEARCH_INDEXES' => truthy + + it 'receives create_search_indexes with wait: true' do + expect(Mongoid::Tasks::Database) + .to receive(:create_search_indexes) + .with(wait: true) + task.invoke + end + end + end + + %w( 0 false no off bogus ).each do |falsey| + context "when WAIT_FOR_SEARCH_INDEXES is #{falsey.inspect}" do + local_env 'WAIT_FOR_SEARCH_INDEXES' => falsey + + it 'receives create_search_indexes with wait: false' do + expect(Mongoid::Tasks::Database) + .to receive(:create_search_indexes) + .with(wait: false) + task.invoke + end + end + end + end + shared_examples_for "create_collections" do it "receives create_collections" do @@ -203,6 +231,26 @@ end end +describe 'db:mongoid:create_search_indexes' do + include_context 'rake task' + + it_behaves_like 'create_search_indexes' + + it 'calls load_models' do + expect(task.prerequisites).to include('load_models') + end + + it 'calls environment' do + expect(task.prerequisites).to include('environment') + end + + context 'when using rails task' do + include_context 'rails rake task' + + it_behaves_like 'create_search_indexes' + end +end + describe "db:mongoid:create_collections" do include_context "rake task" @@ -287,6 +335,28 @@ end end +describe 'db:mongoid:remove_search_indexes' do + include_context 'rake task' + + it 'receives remove_search_indexes' do + expect(Mongoid::Tasks::Database).to receive(:remove_search_indexes) + task.invoke + end + + it 'calls environment' do + expect(task.prerequisites).to include('environment') + end + + context 'when using rails task' do + include_context 'rails rake task' + + it 'receives remove_search_indexes' do + expect(Mongoid::Tasks::Database).to receive(:remove_search_indexes) + task.invoke + end + end +end + describe "db:mongoid:drop" do include_context "rake task" diff --git a/spec/mongoid/tasks/database_spec.rb b/spec/mongoid/tasks/database_spec.rb index d5ef01960b..91a0ecc9a7 100644 --- a/spec/mongoid/tasks/database_spec.rb +++ b/spec/mongoid/tasks/database_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" -describe "Mongoid::Tasks::Database" do +describe Mongoid::Tasks::Database do before(:all) do module DatabaseSpec @@ -213,6 +213,64 @@ class Note end end + describe '.create_search_indexes' do + let(:searchable_model) do + Class.new do + include Mongoid::Document + store_in collection: BSON::ObjectId.new.to_s + + search_index mappings: { dynamic: true } + end + end + + let(:index_names) { %w[ name1 name2 ] } + let(:searchable_model_spy) do + class_spy(searchable_model, + create_search_indexes: index_names, + search_index_specs: [ { mappings: { dynamic: true } } ]) + end + + context 'when wait is true' do + before do + allow(described_class).to receive(:wait_for_search_indexes) + described_class.create_search_indexes([ searchable_model_spy ], wait: true) + end + + it 'invokes both create_search_indexes and wait_for_search_indexes' do + expect(searchable_model_spy).to have_received(:create_search_indexes) + expect(described_class).to have_received(:wait_for_search_indexes).with(searchable_model_spy => index_names) + end + end + + context 'when wait is false' do + before do + allow(described_class).to receive(:wait_for_search_indexes) + described_class.create_search_indexes([ searchable_model_spy ], wait: false) + end + + it 'invokes only create_search_indexes' do + expect(searchable_model_spy).to have_received(:create_search_indexes) + expect(described_class).not_to have_received(:wait_for_search_indexes) + end + end + end + + describe '.remove_search_indexes' do + before do + models.each do |model| + allow(model).to receive(:remove_search_indexes) unless model.embedded? + end + + described_class.remove_search_indexes(models) + end + + it 'calls remove_search_indexes on all non-embedded models' do + models.each do |model| + expect(model).to have_received(:remove_search_indexes) unless model.embedded? + end + end + end + describe ".undefined_indexes" do before(:each) do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ba0f61ac4f..9da2629fb2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -51,10 +51,13 @@ def database_id_alt require 'support/constraints' require 'support/crypt' +use_ssl = %w[ ssl 1 true ].include?(ENV['SSL']) +ssl_options = { ssl: use_ssl }.freeze + # Give MongoDB servers time to start up in CI environments if SpecConfig.instance.ci? starting = true - client = Mongo::Client.new(SpecConfig.instance.addresses) + client = Mongo::Client.new(SpecConfig.instance.addresses, ssl_options) while starting begin client.command(ping: 1) @@ -71,15 +74,15 @@ def database_id_alt default: { database: database_id, hosts: SpecConfig.instance.addresses, - options: { + options: ssl_options.merge( server_selection_timeout: 3.42, wait_queue_timeout: 1, max_pool_size: 5, heartbeat_frequency: 180, - user: MONGOID_ROOT_USER.name, - password: MONGOID_ROOT_USER.password, - auth_source: Mongo::Database::ADMIN, - } + user: SpecConfig.instance.uri.client_options[:user] || MONGOID_ROOT_USER.name, + password: SpecConfig.instance.uri.client_options[:password] || MONGOID_ROOT_USER.password, + auth_source: Mongo::Database::ADMIN + ) } }, options: { @@ -121,17 +124,19 @@ class Query require "i18n/backend/fallbacks" end -# The user must be created before any of the tests are loaded, until -# https://jira.mongodb.org/browse/MONGOID-4827 is implemented. -client = Mongo::Client.new(SpecConfig.instance.addresses, server_selection_timeout: 3.03) -begin - # Create the root user administrator as the first user to be added to the - # database. This user will need to be authenticated in order to add any - # more users to any other databases. - client.database.users.create(MONGOID_ROOT_USER) -rescue Mongo::Error::OperationFailure => e -ensure - client.close +unless SpecConfig.instance.atlas? + # The user must be created before any of the tests are loaded, until + # https://jira.mongodb.org/browse/MONGOID-4827 is implemented. + client = Mongo::Client.new(SpecConfig.instance.addresses, server_selection_timeout: 3.03) + begin + # Create the root user administrator as the first user to be added to the + # database. This user will need to be authenticated in order to add any + # more users to any other databases. + client.database.users.create(MONGOID_ROOT_USER) + rescue Mongo::Error::OperationFailure => e + ensure + client.close + end end RSpec.configure do |config| diff --git a/spec/support/spec_config.rb b/spec/support/spec_config.rb index 12ebcfc66f..c4000f0d0c 100644 --- a/spec/support/spec_config.rb +++ b/spec/support/spec_config.rb @@ -17,8 +17,8 @@ def initialize STDERR.puts "Please consider providing the correct uri via MONGODB_URI environment variable." @uri_str = DEFAULT_MONGODB_URI end - - @uri = Mongo::URI.new(@uri_str) + + @uri = Mongo::URI.get(@uri_str) end attr_reader :uri_str @@ -56,6 +56,10 @@ def ci? !!ENV['CI'] end + def atlas? + !!ENV['ATLAS_URI'] + end + def rails_version v = ENV['RAILS'] if v == '' From 8c79f57e40d312ccef91d19e5f358c04a53e7b7c Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Fri, 29 Sep 2023 16:12:11 +0200 Subject: [PATCH 10/28] MONGOID-5686 Fix array operations in update_all (#5726) --- lib/mongoid/extensions/hash.rb | 20 +++++++++++- spec/mongoid/contextual/mongo_spec.rb | 47 +++++++++++++++++++++++---- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/lib/mongoid/extensions/hash.rb b/lib/mongoid/extensions/hash.rb index 00104c38ab..f1d1a27e22 100644 --- a/lib/mongoid/extensions/hash.rb +++ b/lib/mongoid/extensions/hash.rb @@ -46,7 +46,7 @@ def __consolidate__(klass) real_key = klass.database_field_name(key2) value.delete(key2) if real_key != key2 - value[real_key] = (key == "$rename") ? value2.to_s : mongoize_for(key, klass, real_key, value2) + value[real_key] = value_for(key, klass, real_key, value2) end consolidated[key] ||= {} consolidated[key].update(value) @@ -166,6 +166,24 @@ def to_criteria private + # Get the value for the provided operator, klass, key and value. + # + # This is necessary for special cases like $rename, $addToSet and $push. + # + # @param [ String ] operator The operator. + # @param [ Class ] klass The model class. + # @param [ String | Symbol ] key The field key. + # @param [ Object ] value The original value. + # + # @return [ Object ] Value prepared for the provided operator. + def value_for(operator, klass, key, value) + case operator + when "$rename" then value.to_s + when "$addToSet", "$push" then value.mongoize + else mongoize_for(operator, klass, operator, value) + end + end + # Mongoize for the klass, key and value. # # @api private diff --git a/spec/mongoid/contextual/mongo_spec.rb b/spec/mongoid/contextual/mongo_spec.rb index fd55839939..1e38025e3e 100644 --- a/spec/mongoid/contextual/mongo_spec.rb +++ b/spec/mongoid/contextual/mongo_spec.rb @@ -3608,16 +3608,51 @@ context "when the attributes are in the correct type" do - before do - context.update_all("$set" => { name: "Smiths" }) + context "when operation is $set" do + + before do + context.update_all("$set" => { name: "Smiths" }) + end + + it "updates the first matching document" do + expect(depeche_mode.reload.name).to eq("Smiths") + end + + it "updates the last matching document" do + expect(new_order.reload.name).to eq("Smiths") + end end - it "updates the first matching document" do - expect(depeche_mode.reload.name).to eq("Smiths") + context "when operation is $push" do + + before do + depeche_mode.update_attribute(:genres, ["electronic"]) + new_order.update_attribute(:genres, ["electronic"]) + context.update_all("$push" => { genres: "pop" }) + end + + it "updates the first matching document" do + expect(depeche_mode.reload.genres).to eq(["electronic", "pop"]) + end + + it "updates the last matching document" do + expect(new_order.reload.genres).to eq(["electronic", "pop"]) + end end - it "updates the last matching document" do - expect(new_order.reload.name).to eq("Smiths") + context "when operation is $addToSet" do + + before do + context.update_all("$addToSet" => { genres: "electronic" }) + end + + it "updates the first matching document" do + expect(depeche_mode.reload.genres).to eq(["electronic"]) + end + + it "updates the last matching document" do + expect(new_order.reload.genres).to eq(["electronic"]) + end end end From 355ecd751246a03bb9dc1c35e2775040d181f85d Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Mon, 9 Oct 2023 12:36:40 +0200 Subject: [PATCH 11/28] MONGOID-5689 Add Rails 7.1 support (#5728) * MONGOID-5689 Add Rails 7.1 support * Fix 7.1 incompatibilities * Update docs --- .evergreen/config.yml | 8 ++++++-- .evergreen/config/axes.yml.erb | 4 ++++ .evergreen/config/variants.yml.erb | 4 ++-- docs/reference/compatibility.txt | 13 +++++++++++++ lib/mongoid/deprecable.rb | 3 ++- lib/mongoid/deprecation.rb | 6 +++--- lib/mongoid/interceptable.rb | 8 +++++++- mongoid.gemspec | 2 +- 8 files changed, 38 insertions(+), 10 deletions(-) diff --git a/.evergreen/config.yml b/.evergreen/config.yml index 70e4ae92e6..a505b0755f 100644 --- a/.evergreen/config.yml +++ b/.evergreen/config.yml @@ -589,6 +589,10 @@ axes: display_name: "Rails 7.0" variables: RAILS: "7.0" + - id: "7.1" + display_name: "Rails 7.1" + variables: + RAILS: "7.1" - id: "i18n" display_name: I18n version @@ -744,7 +748,7 @@ buildvariants: driver: ["current"] mongodb-version: "6.0" topology: "standalone" - rails: ['7.0'] + rails: ['7.0', '7.1'] os: ubuntu-22.04 fle: helper display_name: "${rails}, ${driver}, ${mongodb-version} (FLE ${fle})" @@ -830,7 +834,7 @@ buildvariants: mongodb-version: '6.0' topology: standalone app-tests: yes - rails: ['6.0', '6.1', '7.0'] + rails: ['6.0', '6.1', '7.0', '7.1'] os: rhel80 display_name: "app tests ${driver}, ${ruby}, ${rails}" tasks: diff --git a/.evergreen/config/axes.yml.erb b/.evergreen/config/axes.yml.erb index 2c1e5a44c3..e7a5a2aba1 100644 --- a/.evergreen/config/axes.yml.erb +++ b/.evergreen/config/axes.yml.erb @@ -214,6 +214,10 @@ axes: display_name: "Rails 7.0" variables: RAILS: "7.0" + - id: "7.1" + display_name: "Rails 7.1" + variables: + RAILS: "7.1" - id: "i18n" display_name: I18n version diff --git a/.evergreen/config/variants.yml.erb b/.evergreen/config/variants.yml.erb index 9348e6b05b..78cfa7a66e 100644 --- a/.evergreen/config/variants.yml.erb +++ b/.evergreen/config/variants.yml.erb @@ -115,7 +115,7 @@ buildvariants: driver: ["current"] mongodb-version: "6.0" topology: "standalone" - rails: ['7.0'] + rails: ['7.0', '7.1'] os: ubuntu-22.04 fle: helper display_name: "${rails}, ${driver}, ${mongodb-version} (FLE ${fle})" @@ -201,7 +201,7 @@ buildvariants: mongodb-version: '6.0' topology: standalone app-tests: yes - rails: ['6.0', '6.1', '7.0'] + rails: ['6.0', '6.1', '7.0', '7.1'] os: rhel80 display_name: "app tests ${driver}, ${ruby}, ${rails}" tasks: diff --git a/docs/reference/compatibility.txt b/docs/reference/compatibility.txt index 85b4bbf4b2..4c7751a67a 100644 --- a/docs/reference/compatibility.txt +++ b/docs/reference/compatibility.txt @@ -399,6 +399,7 @@ are supported by Mongoid. :class: compatibility-large no-padding * - Mongoid + - Rails 7.1 - Rails 7.0 - Rails 6.1 - Rails 6.0 @@ -406,6 +407,7 @@ are supported by Mongoid. - Rails 5.1 * - 8.1 + - |checkmark| [#rails-7.1]_ - |checkmark| - |checkmark| - |checkmark| @@ -413,6 +415,7 @@ are supported by Mongoid. - * - 8.0 + - |checkmark| [#rails-7.1]_ - |checkmark| - |checkmark| - |checkmark| @@ -420,6 +423,7 @@ are supported by Mongoid. - * - 7.5 + - - |checkmark| - |checkmark| - |checkmark| @@ -427,6 +431,7 @@ are supported by Mongoid. - D * - 7.4 + - - |checkmark| - |checkmark| - |checkmark| @@ -434,6 +439,7 @@ are supported by Mongoid. - |checkmark| [#rails-5-ruby-3.0]_ * - 7.3 + - - |checkmark| [#rails-7-Mongoid-7.3]_ - |checkmark| - |checkmark| @@ -441,6 +447,7 @@ are supported by Mongoid. - |checkmark| [#rails-5-ruby-3.0]_ * - 7.2 + - - - |checkmark| [#rails-6.1]_ - |checkmark| @@ -448,6 +455,7 @@ are supported by Mongoid. - |checkmark| [#rails-5-ruby-3.0]_ * - 7.1 + - - - |checkmark| [#rails-6.1]_ - |checkmark| @@ -455,6 +463,7 @@ are supported by Mongoid. - |checkmark| * - 7.0 + - - - |checkmark| [#rails-6.1]_ - |checkmark| [#rails-6]_ @@ -465,6 +474,7 @@ are supported by Mongoid. - - - + - - |checkmark| - |checkmark| @@ -477,4 +487,7 @@ are supported by Mongoid. .. [#rails-7-Mongoid-7.3] Rails 7.x requires Mongoid 7.3.4 or later. +.. [#rails-7.1] Rails 7.1 requires Mongoid 8.0.7 or 8.1.3 in the respective + 8.0 and 8.1 stable branches. + .. include:: /includes/unicode-checkmark.rst diff --git a/lib/mongoid/deprecable.rb b/lib/mongoid/deprecable.rb index 327bd72b70..36ae936d0e 100644 --- a/lib/mongoid/deprecable.rb +++ b/lib/mongoid/deprecable.rb @@ -28,7 +28,8 @@ module Deprecable # @param [ [ Symbol | Hash ]... ] *method_descriptors # The methods to deprecate, with optional replacement instructions. def deprecate(target_module, *method_descriptors) - Mongoid::Deprecation.deprecate_methods(target_module, *method_descriptors) + @_deprecator ||= Mongoid::Deprecation.new + @_deprecator.deprecate_methods(target_module, *method_descriptors) end end end diff --git a/lib/mongoid/deprecation.rb b/lib/mongoid/deprecation.rb index 4fb299418f..faab29a256 100644 --- a/lib/mongoid/deprecation.rb +++ b/lib/mongoid/deprecation.rb @@ -19,10 +19,10 @@ def initialize # # @return [ Array ] The deprecation behavior. def behavior - @behavior ||= Array(->(message, callstack, _deprecation_horizon, _gem_name) { + @behavior ||= Array(->(*args) { logger = Mongoid.logger - logger.warn(message) - logger.debug(callstack.join("\n ")) if debug + logger.warn(args[0]) + logger.debug(args[1].join("\n ")) if debug }) end end diff --git a/lib/mongoid/interceptable.rb b/lib/mongoid/interceptable.rb index 155236eae7..e2148637ae 100644 --- a/lib/mongoid/interceptable.rb +++ b/lib/mongoid/interceptable.rb @@ -314,7 +314,13 @@ def run_targeted_callbacks(place, kind) end self.class.send :define_method, name do env = ActiveSupport::Callbacks::Filters::Environment.new(self, false, nil) - sequence = chain.compile + sequence = if chain.method(:compile).arity == 0 + # ActiveSupport < 7.1 + chain.compile + else + # ActiveSupport >= 7.1 + chain.compile(nil) + end sequence.invoke_before(env) env.value = !env.halted sequence.invoke_after(env) diff --git a/mongoid.gemspec b/mongoid.gemspec index 4444859fb2..0a12d74ecb 100644 --- a/mongoid.gemspec +++ b/mongoid.gemspec @@ -38,7 +38,7 @@ Gem::Specification.new do |s| # Ruby 3.0 requires ActiveModel 6.0 or higher. # activemodel 7.0.0 cannot be used due to Class#descendants issue # See: https://github.com/rails/rails/pull/43951 - s.add_dependency("activemodel", ['>=5.1', '<7.1', '!= 7.0.0']) + s.add_dependency("activemodel", ['>=5.1', '<7.2', '!= 7.0.0']) s.add_dependency("mongo", ['>=2.18.0', '<3.0.0']) s.add_dependency("concurrent-ruby", ['>= 1.0.5', '< 2.0']) From 512e1659f7803dd208efe5bb2fc80a50003712e3 Mon Sep 17 00:00:00 2001 From: Dmitry Rybakov Date: Tue, 17 Oct 2023 09:56:53 +0200 Subject: [PATCH 12/28] MONGOID-5658 Fix callbacks for embedded (#5727) * 5658 * 5658 * 5658 * Document the config option * Add warning * Add docstrings * Update for ActiveSupport 7.1 * Add test case * Add release notes * Update specs --- docs/release-notes/mongoid-9.0.txt | 17 + lib/mongoid/config.rb | 10 + lib/mongoid/interceptable.rb | 113 ++++++- spec/integration/callbacks_spec.rb | 20 ++ spec/mongoid/interceptable_spec.rb | 517 ++++++++++++++++++++--------- 5 files changed, 516 insertions(+), 161 deletions(-) diff --git a/docs/release-notes/mongoid-9.0.txt b/docs/release-notes/mongoid-9.0.txt index 1c6642e60d..f6974ac5d2 100644 --- a/docs/release-notes/mongoid-9.0.txt +++ b/docs/release-notes/mongoid-9.0.txt @@ -18,6 +18,23 @@ please consult GitHub releases for detailed release notes and JIRA for the complete list of issues fixed in each release, including bug fixes. +``around_*`` callbacks for embedded documents are now ignored +------------------------------------------------------------- + +Mongoid 8.x and older allows user to define ``around_*`` callbacks for embedded +documents. Starting from 9.0 these callbacks are ignored and will not be executed. +A warning will be printed to the console if such callbacks are defined. + +If you want to restore the old behavior, you can set +``Mongoid.around_embedded_document_callbacks`` to true in your application. + +.. note:: + Enabling ``around_*`` callbacks for embedded documents is not recommended + as it may cause ``SystemStackError`` exceptions when a document has many + embedded documents. See `MONGOID-5658 `_ + for more details. + + ``for_js`` method is deprecated ------------------------------- diff --git a/lib/mongoid/config.rb b/lib/mongoid/config.rb index efc78689e5..52eb3a7e9d 100644 --- a/lib/mongoid/config.rb +++ b/lib/mongoid/config.rb @@ -137,6 +137,16 @@ module Config # See https://jira.mongodb.org/browse/MONGOID-5542 option :prevent_multiple_calls_of_embedded_callbacks, default: true + # When this flag is false, callbacks for embedded documents will not be + # called. This is the default in 9.0. + # + # Setting this flag to true restores the pre-9.0 behavior, where callbacks + # for embedded documents are called. This may lead to stack overflow errors + # if there are more than cicrca 1000 embedded documents in the root + # document's dependencies graph. + # See https://jira.mongodb.org/browse/MONGOID-5658 for more details. + option :around_callbacks_for_embeds, default: false + # Returns the Config singleton, for use in the configure DSL. # # @return [ self ] The Config singleton. diff --git a/lib/mongoid/interceptable.rb b/lib/mongoid/interceptable.rb index e2148637ae..9b5f205f88 100644 --- a/lib/mongoid/interceptable.rb +++ b/lib/mongoid/interceptable.rb @@ -149,6 +149,28 @@ def run_callbacks(kind, with_children: true, skip_if: nil, &block) # # @api private def _mongoid_run_child_callbacks(kind, children: nil, &block) + if Mongoid::Config.around_callbacks_for_embeds + _mongoid_run_child_callbacks_with_around(kind, children: children, &block) + else + _mongoid_run_child_callbacks_without_around(kind, children: children, &block) + end + end + + # Execute the callbacks of given kind for embedded documents including + # around callbacks. + # + # @note This method is prone to stack overflow errors if the document + # has a large number of embedded documents. It is recommended to avoid + # using around callbacks for embedded documents until a proper solution + # is implemented. + # + # @param [ Symbol ] kind The type of callback to execute. + # @param [ Array ] children Children to execute callbacks on. If + # nil, callbacks will be executed on all cascadable children of + # the document. + # + # @api private + def _mongoid_run_child_callbacks_with_around(kind, children: nil, &block) child, *tail = (children || cascadable_children(kind)) with_children = !Mongoid::Config.prevent_multiple_calls_of_embedded_callbacks if child.nil? @@ -157,11 +179,73 @@ def _mongoid_run_child_callbacks(kind, children: nil, &block) child.run_callbacks(child_callback_type(kind, child), with_children: with_children, &block) else child.run_callbacks(child_callback_type(kind, child), with_children: with_children) do - _mongoid_run_child_callbacks(kind, children: tail, &block) + _mongoid_run_child_callbacks_with_around(kind, children: tail, &block) end end end + # Execute the callbacks of given kind for embedded documents without + # around callbacks. + # + # @param [ Symbol ] kind The type of callback to execute. + # @param [ Array ] children Children to execute callbacks on. If + # nil, callbacks will be executed on all cascadable children of + # the document. + # + # @api private + def _mongoid_run_child_callbacks_without_around(kind, children: nil, &block) + children = (children || cascadable_children(kind)) + callback_list = _mongoid_run_child_before_callbacks(kind, children: children) + return false if callback_list == false + value = block&.call + callback_list.each do |_next_sequence, env| + env.value &&= value + end + return false if _mongoid_run_child_after_callbacks(callback_list: callback_list) == false + + value + end + + # Execute the before callbacks of given kind for embedded documents. + # + # @param [ Symbol ] kind The type of callback to execute. + # @param [ Array ] children Children to execute callbacks on. + # @param [ Array ] callback_list List of + # pairs of callback sequence and environment. This list will be later used + # to execute after callbacks in reverse order. + # + # @api private + def _mongoid_run_child_before_callbacks(kind, children: [], callback_list: []) + children.each do |child| + chain = child.__callbacks[child_callback_type(kind, child)] + env = ActiveSupport::Callbacks::Filters::Environment.new(child, false, nil) + next_sequence = compile_callbacks(chain) + unless next_sequence.final? + Mongoid.logger.warn("Around callbacks are disabled for embedded documents. Skipping around callbacks for #{child.class.name}.") + Mongoid.logger.warn("To enable around callbacks for embedded documents, set Mongoid::Config.around_callbacks_for_embeds to true.") + end + next_sequence.invoke_before(env) + return false if env.halted + env.value = !env.halted + callback_list << [next_sequence, env] + if (grandchildren = child.send(:cascadable_children, kind)) + _mongoid_run_child_before_callbacks(kind, children: grandchildren, callback_list: callback_list) + end + end + callback_list + end + + # Execute the after callbacks. + # + # @param [ Array ] callback_list List of + # pairs of callback sequence and environment. + def _mongoid_run_child_after_callbacks(callback_list: []) + callback_list.reverse_each do |next_sequence, env| + next_sequence.invoke_after(env) + return false if env.halted + end + end + # Returns the stored callbacks to be executed later. # # @return [ Array ] Method symbols of the stored pending callbacks. @@ -314,13 +398,7 @@ def run_targeted_callbacks(place, kind) end self.class.send :define_method, name do env = ActiveSupport::Callbacks::Filters::Environment.new(self, false, nil) - sequence = if chain.method(:compile).arity == 0 - # ActiveSupport < 7.1 - chain.compile - else - # ActiveSupport >= 7.1 - chain.compile(nil) - end + sequence = compile_callbacks(chain) sequence.invoke_before(env) env.value = !env.halted sequence.invoke_after(env) @@ -330,5 +408,24 @@ def run_targeted_callbacks(place, kind) end send(name) end + + # Compile the callback chain. + # + # This method hides the differences between ActiveSupport implementations + # before and after 7.1. + # + # @param [ ActiveSupport::Callbacks::CallbackChain ] chain The callback chain. + # @param [ Symbol | nil ] type The type of callback chain to compile. + # + # @return [ ActiveSupport::Callbacks::CallbackSequence ] The compiled callback sequence. + def compile_callbacks(chain, type = nil) + if chain.method(:compile).arity == 0 + # ActiveSupport < 7.1 + chain.compile + else + # ActiveSupport >= 7.1 + chain.compile(type) + end + end end end diff --git a/spec/integration/callbacks_spec.rb b/spec/integration/callbacks_spec.rb index 206d408682..c580258d4b 100644 --- a/spec/integration/callbacks_spec.rb +++ b/spec/integration/callbacks_spec.rb @@ -558,6 +558,7 @@ def will_save_change_to_attribute_values_before context 'nested embedded documents' do config_override :prevent_multiple_calls_of_embedded_callbacks, true + config_override :around_callbacks_for_embeds, true let(:logger) { Array.new } @@ -582,4 +583,23 @@ def will_save_change_to_attribute_values_before expect(logger).to eq(%i[embedded_twice embedded_once root]) end end + + context 'cascade callbacks' do + ruby_version_gte '3.0' + + let(:book) do + Book.new + end + + before do + 1500.times do + book.pages.build + end + end + + # https://jira.mongodb.org/browse/MONGOID-5658 + it 'does not raise SystemStackError' do + expect { book.save! }.not_to raise_error(SystemStackError) + end + end end diff --git a/spec/mongoid/interceptable_spec.rb b/spec/mongoid/interceptable_spec.rb index 3e4ba18eba..4549ca675d 100644 --- a/spec/mongoid/interceptable_spec.rb +++ b/spec/mongoid/interceptable_spec.rb @@ -583,6 +583,7 @@ class TestClass context "when saving the root" do context 'with prevent_multiple_calls_of_embedded_callbacks enabled' do config_override :prevent_multiple_calls_of_embedded_callbacks, true + config_override :around_callbacks_for_embeds, true it "executes the callbacks only once for each document" do expect(note).to receive(:update_saved).once @@ -592,6 +593,7 @@ class TestClass context 'with prevent_multiple_calls_of_embedded_callbacks disabled' do config_override :prevent_multiple_calls_of_embedded_callbacks, false + config_override :around_callbacks_for_embeds, true it "executes the callbacks once for each ember" do expect(note).to receive(:update_saved).twice @@ -1784,40 +1786,80 @@ class TestClass end end - let(:expected) do - [ - [InterceptableSpec::CbCascadedChild, :before_validation], - [InterceptableSpec::CbCascadedChild, :after_validation], - [InterceptableSpec::CbParent, :before_validation], - [InterceptableSpec::CbCascadedChild, :before_validation], - [InterceptableSpec::CbCascadedChild, :after_validation], + context 'with around callbacks' do + config_override :around_callbacks_for_embeds, true - [InterceptableSpec::CbParent, :after_validation], - [InterceptableSpec::CbParent, :before_save], - [InterceptableSpec::CbParent, :around_save_open], - [InterceptableSpec::CbParent, :before_create], - [InterceptableSpec::CbParent, :around_create_open], + let(:expected) do + [ + [InterceptableSpec::CbCascadedChild, :before_validation], + [InterceptableSpec::CbCascadedChild, :after_validation], + [InterceptableSpec::CbParent, :before_validation], + [InterceptableSpec::CbCascadedChild, :before_validation], + [InterceptableSpec::CbCascadedChild, :after_validation], + + [InterceptableSpec::CbParent, :after_validation], + [InterceptableSpec::CbParent, :before_save], + [InterceptableSpec::CbParent, :around_save_open], + [InterceptableSpec::CbParent, :before_create], + [InterceptableSpec::CbParent, :around_create_open], + + [InterceptableSpec::CbCascadedChild, :before_save], + [InterceptableSpec::CbCascadedChild, :around_save_open], + [InterceptableSpec::CbCascadedChild, :before_create], + [InterceptableSpec::CbCascadedChild, :around_create_open], + + [InterceptableSpec::CbCascadedChild, :around_create_close], + [InterceptableSpec::CbCascadedChild, :after_create], + [InterceptableSpec::CbCascadedChild, :around_save_close], + [InterceptableSpec::CbCascadedChild, :after_save], + + [InterceptableSpec::CbParent, :around_create_close], + [InterceptableSpec::CbParent, :after_create], + [InterceptableSpec::CbParent, :around_save_close], + [InterceptableSpec::CbParent, :after_save] + ] + end - [InterceptableSpec::CbCascadedChild, :before_save], - [InterceptableSpec::CbCascadedChild, :around_save_open], - [InterceptableSpec::CbCascadedChild, :before_create], - [InterceptableSpec::CbCascadedChild, :around_create_open], + it 'calls callbacks in the right order' do + parent.save! + expect(registry.calls).to eq expected + end + end - [InterceptableSpec::CbCascadedChild, :around_create_close], - [InterceptableSpec::CbCascadedChild, :after_create], - [InterceptableSpec::CbCascadedChild, :around_save_close], - [InterceptableSpec::CbCascadedChild, :after_save], + context 'without around callbacks' do + config_override :around_callbacks_for_embeds, false - [InterceptableSpec::CbParent, :around_create_close], - [InterceptableSpec::CbParent, :after_create], - [InterceptableSpec::CbParent, :around_save_close], - [InterceptableSpec::CbParent, :after_save] - ] - end + let(:expected) do + [ + [InterceptableSpec::CbCascadedChild, :before_validation], + [InterceptableSpec::CbCascadedChild, :after_validation], + [InterceptableSpec::CbParent, :before_validation], + [InterceptableSpec::CbCascadedChild, :before_validation], + [InterceptableSpec::CbCascadedChild, :after_validation], + + [InterceptableSpec::CbParent, :after_validation], + [InterceptableSpec::CbParent, :before_save], + [InterceptableSpec::CbParent, :around_save_open], + [InterceptableSpec::CbParent, :before_create], + [InterceptableSpec::CbParent, :around_create_open], + + [InterceptableSpec::CbCascadedChild, :before_save], + [InterceptableSpec::CbCascadedChild, :before_create], + + [InterceptableSpec::CbCascadedChild, :after_create], + [InterceptableSpec::CbCascadedChild, :after_save], + + [InterceptableSpec::CbParent, :around_create_close], + [InterceptableSpec::CbParent, :after_create], + [InterceptableSpec::CbParent, :around_save_close], + [InterceptableSpec::CbParent, :after_save] + ] + end - it 'calls callbacks in the right order' do - parent.save! - expect(registry.calls).to eq expected + it 'calls callbacks in the right order' do + parent.save! + expect(registry.calls).to eq expected + end end end @@ -1880,89 +1922,180 @@ class TestClass end context "create" do - let(:expected) do - [ - [InterceptableSpec::CbEmbedsOneChild, :before_validation], - [InterceptableSpec::CbEmbedsOneChild, :after_validation], - [InterceptableSpec::CbEmbedsOneParent, :before_validation], - [InterceptableSpec::CbEmbedsOneChild, :before_validation], - [InterceptableSpec::CbEmbedsOneChild, :after_validation], - [InterceptableSpec::CbEmbedsOneParent, :after_validation], - - [InterceptableSpec::CbEmbedsOneParent, :before_save], - [InterceptableSpec::CbEmbedsOneParent, :around_save_open], - [InterceptableSpec::CbEmbedsOneParent, :before_create], - [InterceptableSpec::CbEmbedsOneParent, :around_create_open], - - [InterceptableSpec::CbEmbedsOneChild, :before_save], - [InterceptableSpec::CbEmbedsOneChild, :around_save_open], - [InterceptableSpec::CbEmbedsOneChild, :before_create], - [InterceptableSpec::CbEmbedsOneChild, :around_create_open], - - [InterceptableSpec::CbEmbedsOneParent, :insert_into_database], - - [InterceptableSpec::CbEmbedsOneChild, :around_create_close], - [InterceptableSpec::CbEmbedsOneChild, :after_create], - [InterceptableSpec::CbEmbedsOneChild, :around_save_close], - [InterceptableSpec::CbEmbedsOneChild, :after_save], - - [InterceptableSpec::CbEmbedsOneParent, :around_create_close], - [InterceptableSpec::CbEmbedsOneParent, :after_create], - [InterceptableSpec::CbEmbedsOneParent, :around_save_close], - [InterceptableSpec::CbEmbedsOneParent, :after_save] - ] + context "with around callbacks" do + config_override :around_callbacks_for_embeds, true + + let(:expected) do + [ + [InterceptableSpec::CbEmbedsOneChild, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :after_validation], + [InterceptableSpec::CbEmbedsOneParent, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :after_validation], + [InterceptableSpec::CbEmbedsOneParent, :after_validation], + + [InterceptableSpec::CbEmbedsOneParent, :before_save], + [InterceptableSpec::CbEmbedsOneParent, :around_save_open], + [InterceptableSpec::CbEmbedsOneParent, :before_create], + [InterceptableSpec::CbEmbedsOneParent, :around_create_open], + + [InterceptableSpec::CbEmbedsOneChild, :before_save], + [InterceptableSpec::CbEmbedsOneChild, :around_save_open], + [InterceptableSpec::CbEmbedsOneChild, :before_create], + [InterceptableSpec::CbEmbedsOneChild, :around_create_open], + + [InterceptableSpec::CbEmbedsOneParent, :insert_into_database], + + [InterceptableSpec::CbEmbedsOneChild, :around_create_close], + [InterceptableSpec::CbEmbedsOneChild, :after_create], + [InterceptableSpec::CbEmbedsOneChild, :around_save_close], + [InterceptableSpec::CbEmbedsOneChild, :after_save], + + [InterceptableSpec::CbEmbedsOneParent, :around_create_close], + [InterceptableSpec::CbEmbedsOneParent, :after_create], + [InterceptableSpec::CbEmbedsOneParent, :around_save_close], + [InterceptableSpec::CbEmbedsOneParent, :after_save] + ] + end + + it 'calls callbacks in the right order' do + parent.save! + expect(registry.calls).to eq expected + end end - it 'calls callbacks in the right order' do - parent.save! - expect(registry.calls).to eq expected + context "without around callbacks" do + config_override :around_callbacks_for_embeds, false + + let(:expected) do + [ + [InterceptableSpec::CbEmbedsOneChild, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :after_validation], + [InterceptableSpec::CbEmbedsOneParent, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :after_validation], + [InterceptableSpec::CbEmbedsOneParent, :after_validation], + + [InterceptableSpec::CbEmbedsOneParent, :before_save], + [InterceptableSpec::CbEmbedsOneParent, :around_save_open], + [InterceptableSpec::CbEmbedsOneParent, :before_create], + [InterceptableSpec::CbEmbedsOneParent, :around_create_open], + + [InterceptableSpec::CbEmbedsOneChild, :before_save], + [InterceptableSpec::CbEmbedsOneChild, :before_create], + + [InterceptableSpec::CbEmbedsOneParent, :insert_into_database], + + [InterceptableSpec::CbEmbedsOneChild, :after_create], + [InterceptableSpec::CbEmbedsOneChild, :after_save], + + [InterceptableSpec::CbEmbedsOneParent, :around_create_close], + [InterceptableSpec::CbEmbedsOneParent, :after_create], + [InterceptableSpec::CbEmbedsOneParent, :around_save_close], + [InterceptableSpec::CbEmbedsOneParent, :after_save] + ] + end + + it 'calls callbacks in the right order' do + parent.save! + expect(registry.calls).to eq expected + end end end context "update" do - let(:expected) do - [ - [InterceptableSpec::CbEmbedsOneChild, :before_validation], - [InterceptableSpec::CbEmbedsOneChild, :after_validation], - [InterceptableSpec::CbEmbedsOneParent, :before_validation], - [InterceptableSpec::CbEmbedsOneChild, :before_validation], - [InterceptableSpec::CbEmbedsOneChild, :after_validation], - [InterceptableSpec::CbEmbedsOneParent, :after_validation], - - [InterceptableSpec::CbEmbedsOneParent, :before_save], - [InterceptableSpec::CbEmbedsOneParent, :around_save_open], - [InterceptableSpec::CbEmbedsOneParent, :before_update], - [InterceptableSpec::CbEmbedsOneParent, :around_update_open], - - [InterceptableSpec::CbEmbedsOneChild, :before_save], - [InterceptableSpec::CbEmbedsOneChild, :around_save_open], - [InterceptableSpec::CbEmbedsOneChild, :before_update], - [InterceptableSpec::CbEmbedsOneChild, :around_update_open], - - [InterceptableSpec::CbEmbedsOneChild, :around_update_close], - [InterceptableSpec::CbEmbedsOneChild, :after_update], - [InterceptableSpec::CbEmbedsOneChild, :around_save_close], - [InterceptableSpec::CbEmbedsOneChild, :after_save], - - [InterceptableSpec::CbEmbedsOneParent, :around_update_close], - [InterceptableSpec::CbEmbedsOneParent, :after_update], - [InterceptableSpec::CbEmbedsOneParent, :around_save_close], - [InterceptableSpec::CbEmbedsOneParent, :after_save] - ] + context "with around callbacks" do + config_override :around_callbacks_for_embeds, true + + let(:expected) do + [ + [InterceptableSpec::CbEmbedsOneChild, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :after_validation], + [InterceptableSpec::CbEmbedsOneParent, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :after_validation], + [InterceptableSpec::CbEmbedsOneParent, :after_validation], + + [InterceptableSpec::CbEmbedsOneParent, :before_save], + [InterceptableSpec::CbEmbedsOneParent, :around_save_open], + [InterceptableSpec::CbEmbedsOneParent, :before_update], + [InterceptableSpec::CbEmbedsOneParent, :around_update_open], + + [InterceptableSpec::CbEmbedsOneChild, :before_save], + [InterceptableSpec::CbEmbedsOneChild, :around_save_open], + [InterceptableSpec::CbEmbedsOneChild, :before_update], + [InterceptableSpec::CbEmbedsOneChild, :around_update_open], + + [InterceptableSpec::CbEmbedsOneChild, :around_update_close], + [InterceptableSpec::CbEmbedsOneChild, :after_update], + [InterceptableSpec::CbEmbedsOneChild, :around_save_close], + [InterceptableSpec::CbEmbedsOneChild, :after_save], + + [InterceptableSpec::CbEmbedsOneParent, :around_update_close], + [InterceptableSpec::CbEmbedsOneParent, :after_update], + [InterceptableSpec::CbEmbedsOneParent, :around_save_close], + [InterceptableSpec::CbEmbedsOneParent, :after_save] + ] + end + + it 'calls callbacks in the right order' do + parent.callback_registry = nil + parent.child.callback_registry = nil + parent.save! + + parent.callback_registry = registry + parent.child.callback_registry = registry + parent.name = "name" + parent.child.age = 10 + + parent.save! + expect(registry.calls).to eq expected + end end - it 'calls callbacks in the right order' do - parent.callback_registry = nil - parent.child.callback_registry = nil - parent.save! + context "without around callbacks" do + config_override :around_callbacks_for_embeds, false - parent.callback_registry = registry - parent.child.callback_registry = registry - parent.name = "name" - parent.child.age = 10 + let(:expected) do + [ + [InterceptableSpec::CbEmbedsOneChild, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :after_validation], + [InterceptableSpec::CbEmbedsOneParent, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :before_validation], + [InterceptableSpec::CbEmbedsOneChild, :after_validation], + [InterceptableSpec::CbEmbedsOneParent, :after_validation], - parent.save! - expect(registry.calls).to eq expected + [InterceptableSpec::CbEmbedsOneParent, :before_save], + [InterceptableSpec::CbEmbedsOneParent, :around_save_open], + [InterceptableSpec::CbEmbedsOneParent, :before_update], + [InterceptableSpec::CbEmbedsOneParent, :around_update_open], + + [InterceptableSpec::CbEmbedsOneChild, :before_save], + [InterceptableSpec::CbEmbedsOneChild, :before_update], + + [InterceptableSpec::CbEmbedsOneChild, :after_update], + [InterceptableSpec::CbEmbedsOneChild, :after_save], + + [InterceptableSpec::CbEmbedsOneParent, :around_update_close], + [InterceptableSpec::CbEmbedsOneParent, :after_update], + [InterceptableSpec::CbEmbedsOneParent, :around_save_close], + [InterceptableSpec::CbEmbedsOneParent, :after_save] + ] + end + + it 'calls callbacks in the right order' do + parent.callback_registry = nil + parent.child.callback_registry = nil + parent.save! + + parent.callback_registry = registry + parent.child.callback_registry = registry + parent.name = "name" + parent.child.age = 10 + + parent.save! + expect(registry.calls).to eq expected + end end end end @@ -2042,59 +2175,114 @@ class TestClass end end - let(:expected) do - [ - [InterceptableSpec::CbEmbedsManyChild, :before_validation], - [InterceptableSpec::CbEmbedsManyChild, :after_validation], - [InterceptableSpec::CbEmbedsManyChild, :before_validation], - [InterceptableSpec::CbEmbedsManyChild, :after_validation], - [InterceptableSpec::CbEmbedsManyParent, :before_validation], - [InterceptableSpec::CbEmbedsManyChild, :before_validation], - [InterceptableSpec::CbEmbedsManyChild, :after_validation], - [InterceptableSpec::CbEmbedsManyChild, :before_validation], - [InterceptableSpec::CbEmbedsManyChild, :after_validation], - [InterceptableSpec::CbEmbedsManyParent, :after_validation], - - [InterceptableSpec::CbEmbedsManyParent, :before_save], - [InterceptableSpec::CbEmbedsManyParent, :around_save_open], - [InterceptableSpec::CbEmbedsManyParent, :before_create], - [InterceptableSpec::CbEmbedsManyParent, :around_create_open], - - [InterceptableSpec::CbEmbedsManyChild, :before_save], - [InterceptableSpec::CbEmbedsManyChild, :around_save_open], - [InterceptableSpec::CbEmbedsManyChild, :before_save], - - [InterceptableSpec::CbEmbedsManyChild, :around_save_open], - [InterceptableSpec::CbEmbedsManyChild, :before_create], - [InterceptableSpec::CbEmbedsManyChild, :around_create_open], - - [InterceptableSpec::CbEmbedsManyChild, :before_create], - [InterceptableSpec::CbEmbedsManyChild, :around_create_open], - - [InterceptableSpec::CbEmbedsManyParent, :insert_into_database], - - [InterceptableSpec::CbEmbedsManyChild, :around_create_close], - [InterceptableSpec::CbEmbedsManyChild, :after_create], - - [InterceptableSpec::CbEmbedsManyChild, :around_create_close], - [InterceptableSpec::CbEmbedsManyChild, :after_create], - - [InterceptableSpec::CbEmbedsManyChild, :around_save_close], - [InterceptableSpec::CbEmbedsManyChild, :after_save], - - [InterceptableSpec::CbEmbedsManyChild, :around_save_close], - [InterceptableSpec::CbEmbedsManyChild, :after_save], - - [InterceptableSpec::CbEmbedsManyParent, :around_create_close], - [InterceptableSpec::CbEmbedsManyParent, :after_create], - [InterceptableSpec::CbEmbedsManyParent, :around_save_close], - [InterceptableSpec::CbEmbedsManyParent, :after_save] - ] + context "with around callbacks" do + config_override :around_callbacks_for_embeds, true + + let(:expected) do + [ + [InterceptableSpec::CbEmbedsManyChild, :before_validation], + [InterceptableSpec::CbEmbedsManyChild, :after_validation], + [InterceptableSpec::CbEmbedsManyChild, :before_validation], + [InterceptableSpec::CbEmbedsManyChild, :after_validation], + [InterceptableSpec::CbEmbedsManyParent, :before_validation], + [InterceptableSpec::CbEmbedsManyChild, :before_validation], + [InterceptableSpec::CbEmbedsManyChild, :after_validation], + [InterceptableSpec::CbEmbedsManyChild, :before_validation], + [InterceptableSpec::CbEmbedsManyChild, :after_validation], + [InterceptableSpec::CbEmbedsManyParent, :after_validation], + + [InterceptableSpec::CbEmbedsManyParent, :before_save], + [InterceptableSpec::CbEmbedsManyParent, :around_save_open], + [InterceptableSpec::CbEmbedsManyParent, :before_create], + [InterceptableSpec::CbEmbedsManyParent, :around_create_open], + + [InterceptableSpec::CbEmbedsManyChild, :before_save], + [InterceptableSpec::CbEmbedsManyChild, :around_save_open], + [InterceptableSpec::CbEmbedsManyChild, :before_save], + + [InterceptableSpec::CbEmbedsManyChild, :around_save_open], + [InterceptableSpec::CbEmbedsManyChild, :before_create], + [InterceptableSpec::CbEmbedsManyChild, :around_create_open], + + [InterceptableSpec::CbEmbedsManyChild, :before_create], + [InterceptableSpec::CbEmbedsManyChild, :around_create_open], + + [InterceptableSpec::CbEmbedsManyParent, :insert_into_database], + + [InterceptableSpec::CbEmbedsManyChild, :around_create_close], + [InterceptableSpec::CbEmbedsManyChild, :after_create], + + [InterceptableSpec::CbEmbedsManyChild, :around_create_close], + [InterceptableSpec::CbEmbedsManyChild, :after_create], + + [InterceptableSpec::CbEmbedsManyChild, :around_save_close], + [InterceptableSpec::CbEmbedsManyChild, :after_save], + + [InterceptableSpec::CbEmbedsManyChild, :around_save_close], + [InterceptableSpec::CbEmbedsManyChild, :after_save], + + [InterceptableSpec::CbEmbedsManyParent, :around_create_close], + [InterceptableSpec::CbEmbedsManyParent, :after_create], + [InterceptableSpec::CbEmbedsManyParent, :around_save_close], + [InterceptableSpec::CbEmbedsManyParent, :after_save] + ] + end + + it 'calls callbacks in the right order' do + parent.save! + expect(registry.calls).to eq expected + end end - it 'calls callbacks in the right order' do - parent.save! - expect(registry.calls).to eq expected + context "without around callbacks" do + config_override :around_callbacks_for_embeds, false + + let(:expected) do + [ + [InterceptableSpec::CbEmbedsManyChild, :before_validation], + [InterceptableSpec::CbEmbedsManyChild, :after_validation], + [InterceptableSpec::CbEmbedsManyChild, :before_validation], + [InterceptableSpec::CbEmbedsManyChild, :after_validation], + [InterceptableSpec::CbEmbedsManyParent, :before_validation], + [InterceptableSpec::CbEmbedsManyChild, :before_validation], + [InterceptableSpec::CbEmbedsManyChild, :after_validation], + [InterceptableSpec::CbEmbedsManyChild, :before_validation], + [InterceptableSpec::CbEmbedsManyChild, :after_validation], + [InterceptableSpec::CbEmbedsManyParent, :after_validation], + + [InterceptableSpec::CbEmbedsManyParent, :before_save], + [InterceptableSpec::CbEmbedsManyParent, :around_save_open], + [InterceptableSpec::CbEmbedsManyParent, :before_create], + [InterceptableSpec::CbEmbedsManyParent, :around_create_open], + + [InterceptableSpec::CbEmbedsManyChild, :before_save], + [InterceptableSpec::CbEmbedsManyChild, :before_save], + + [InterceptableSpec::CbEmbedsManyChild, :before_create], + + [InterceptableSpec::CbEmbedsManyChild, :before_create], + + [InterceptableSpec::CbEmbedsManyParent, :insert_into_database], + + [InterceptableSpec::CbEmbedsManyChild, :after_create], + + [InterceptableSpec::CbEmbedsManyChild, :after_create], + + [InterceptableSpec::CbEmbedsManyChild, :after_save], + + [InterceptableSpec::CbEmbedsManyChild, :after_save], + + [InterceptableSpec::CbEmbedsManyParent, :around_create_close], + [InterceptableSpec::CbEmbedsManyParent, :after_create], + [InterceptableSpec::CbEmbedsManyParent, :around_save_close], + [InterceptableSpec::CbEmbedsManyParent, :after_save] + ] + end + + it 'calls callbacks in the right order' do + parent.save! + expect(registry.calls).to eq expected + end end end end @@ -2383,4 +2571,27 @@ class TestClass end.to_not raise_error(Mongoid::Errors::AttributeNotLoaded) end end + + context "when around callbacks for embedded are disabled" do + config_override :around_callbacks_for_embeds, false + + context "when around callback is defined" do + let(:registry) { InterceptableSpec::CallbackRegistry.new } + + let(:parent) do + InterceptableSpec::CbEmbedsOneParent.new(registry).tap do |parent| + parent.child = InterceptableSpec::CbEmbedsOneChild.new(registry) + end + end + + before do + expect(Mongoid.logger).to receive(:warn).with(/Around callbacks are disabled for embedded documents/).twice.and_call_original + expect(Mongoid.logger).to receive(:warn).with(/To enable around callbacks for embedded documents/).twice.and_call_original + end + + it "logs a warning" do + parent.save! + end + end + end end From 78fb959cb40905e8f7aaa0305ea7eaf0468daeed Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Thu, 2 Nov 2023 09:59:42 -0600 Subject: [PATCH 13/28] MONGOID-5608 allow using `#exists?` with args on relations (#5736) * MONGOID-5608 allow using `#exists?` with args on relations * simplify * test the nil/false and _id matching paths, too --- .../association/embedded/embeds_many/proxy.rb | 21 ++++++++-- .../association/referenced/has_many/proxy.rb | 13 +++++- .../embedded/embeds_many/proxy_spec.rb | 35 ++++++++++++++++ .../referenced/has_many/proxy_spec.rb | 42 +++++++++++++++++++ 4 files changed, 106 insertions(+), 5 deletions(-) diff --git a/lib/mongoid/association/embedded/embeds_many/proxy.rb b/lib/mongoid/association/embedded/embeds_many/proxy.rb index 5c3d7b8397..f7e5d810a1 100644 --- a/lib/mongoid/association/embedded/embeds_many/proxy.rb +++ b/lib/mongoid/association/embedded/embeds_many/proxy.rb @@ -292,9 +292,24 @@ def destroy_all(conditions = {}) # @example Are there persisted documents? # person.posts.exists? # - # @return [ true | false ] True is persisted documents exist, false if not. - def exists? - _target.any?(&:persisted?) + # @param [ :none | nil | false | Hash | Object ] id_or_conditions + # When :none (the default), returns true if any persisted + # documents exist in the association. When nil or false, this + # will always return false. When a Hash is given, this queries + # the documents in the association for those that match the given + # conditions, and returns true if any match which have been + # persisted. Any other argument is interpreted as an id, and + # queries for the existence of persisted documents in the + # association with a matching _id. + # + # @return [ true | false ] True if persisted documents exist, false if not. + def exists?(id_or_conditions = :none) + case id_or_conditions + when :none then _target.any?(&:persisted?) + when nil, false then false + when Hash then where(id_or_conditions).any?(&:persisted?) + else where(_id: id_or_conditions).any?(&:persisted?) + end end # Finds a document in this association through several different diff --git a/lib/mongoid/association/referenced/has_many/proxy.rb b/lib/mongoid/association/referenced/has_many/proxy.rb index 6a652d4246..ef0bd2bd08 100644 --- a/lib/mongoid/association/referenced/has_many/proxy.rb +++ b/lib/mongoid/association/referenced/has_many/proxy.rb @@ -217,9 +217,18 @@ def each(&block) # @example Are there persisted documents? # person.posts.exists? # + # @param [ :none | nil | false | Hash | Object ] id_or_conditions + # When :none (the default), returns true if any persisted + # documents exist in the association. When nil or false, this + # will always return false. When a Hash is given, this queries + # the documents in the association for those that match the given + # conditions, and returns true if any match. Any other argument is + # interpreted as an id, and queries for the existence of documents + # in the association with a matching _id. + # # @return [ true | false ] True is persisted documents exist, false if not. - def exists? - criteria.exists? + def exists?(id_or_conditions = :none) + criteria.exists?(id_or_conditions) end # Find the matching document on the association, either based on id or diff --git a/spec/mongoid/association/embedded/embeds_many/proxy_spec.rb b/spec/mongoid/association/embedded/embeds_many/proxy_spec.rb index ae501c84c6..7ef43ae68c 100644 --- a/spec/mongoid/association/embedded/embeds_many/proxy_spec.rb +++ b/spec/mongoid/association/embedded/embeds_many/proxy_spec.rb @@ -2311,9 +2311,37 @@ class TrackingIdValidationHistory person.addresses.create!(street: "Bond St") end + let(:address) { person.addresses.first } + it "returns true" do expect(person.addresses.exists?).to be true end + + context 'when given specifying conditions' do + context 'when the record exists in the association' do + it 'returns true by condition' do + expect(person.addresses.exists?(street: 'Bond St')).to be true + end + + it 'returns true by id' do + expect(person.addresses.exists?(address._id)).to be true + end + + it 'returns false when given false' do + expect(person.addresses.exists?(false)).to be false + end + + it 'returns false when given nil' do + expect(person.addresses.exists?(nil)).to be false + end + end + + context 'when the record does not exist in the association' do + it 'returns false' do + expect(person.addresses.exists?(street: 'Garfield Ave')).to be false + end + end + end end context "when no documents exist in the database" do @@ -2325,6 +2353,13 @@ class TrackingIdValidationHistory it "returns false" do expect(person.addresses.exists?).to be false end + + context 'when given specifying conditions' do + it 'returns false' do + expect(person.addresses.exists?(street: 'Hyde Park Dr')).to be false + expect(person.addresses.exists?(street: 'Garfield Ave')).to be false + end + end end end diff --git a/spec/mongoid/association/referenced/has_many/proxy_spec.rb b/spec/mongoid/association/referenced/has_many/proxy_spec.rb index 8e4e3629ef..3f1f345a05 100644 --- a/spec/mongoid/association/referenced/has_many/proxy_spec.rb +++ b/spec/mongoid/association/referenced/has_many/proxy_spec.rb @@ -1924,6 +1924,42 @@ def with_transaction_via(model, &block) expect_query(1) { expect(person.posts.exists?).to be true } end end + + context 'when invoked with specifying conditions' do + let(:other_person) { Person.create! } + let(:post) { person.posts.first } + + before do + person.posts.create title: 'bumfuzzle' + other_person.posts.create title: 'bumbershoot' + end + + context 'when the conditions match an associated record' do + it 'detects its existence by condition' do + expect(person.posts.exists?(title: 'bumfuzzle')).to be true + expect(other_person.posts.exists?(title: 'bumbershoot')).to be true + end + + it 'detects its existence by id' do + expect(person.posts.exists?(post._id)).to be true + end + + it 'returns false when given false' do + expect(person.posts.exists?(false)).to be false + end + + it 'returns false when given nil' do + expect(person.posts.exists?(nil)).to be false + end + end + + context 'when the conditions match an unassociated record' do + it 'does not detect its existence' do + expect(person.posts.exists?(title: 'bumbershoot')).to be false + expect(other_person.posts.exists?(title: 'bumfuzzle')).to be false + end + end + end end context 'when documents exist in application but not in database' do @@ -1972,6 +2008,12 @@ def with_transaction_via(model, &block) expect_query(1) { expect(person.posts.exists?).to be false } end end + + context 'when invoked with specifying conditions' do + it 'returns false' do + expect(person.posts.exists?(title: 'hullaballoo')).to be false + end + end end end From ccb519ce82a52b9a58ffb867de4c20ba3545c812 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 7 Nov 2023 07:13:29 +0900 Subject: [PATCH 14/28] MONGOID-5663 [Monkey Patch Removal] Remove Object#__sortable__ (#5708) * Remove Object#__sortable__ monkey patch, which is only used in Contextual::Memory. Its been moved to private method with a minor amount of refactoring. Existing tests already cover all the functionality. * deprecate __sortable__ instead of removing it --------- Co-authored-by: Jamis Buck --- lib/mongoid/contextual/memory.rb | 37 +++++++++++++++------ lib/mongoid/extensions/false_class.rb | 6 ++-- lib/mongoid/extensions/object.rb | 2 ++ lib/mongoid/extensions/true_class.rb | 6 ++-- spec/mongoid/extensions/false_class_spec.rb | 7 ---- spec/mongoid/extensions/object_spec.rb | 7 ---- spec/mongoid/extensions/true_class_spec.rb | 7 ---- 7 files changed, 35 insertions(+), 37 deletions(-) diff --git a/lib/mongoid/contextual/memory.rb b/lib/mongoid/contextual/memory.rb index 30ceb30f04..6f9d2e8083 100644 --- a/lib/mongoid/contextual/memory.rb +++ b/lib/mongoid/contextual/memory.rb @@ -627,7 +627,8 @@ def apply_sorting end end - # Compare two values, checking for nil. + # Compare two values, handling the cases when + # either value is nil. # # @api private # @@ -635,15 +636,15 @@ def apply_sorting # context.compare(a, b) # # @param [ Object ] a The first object. - # @param [ Object ] b The first object. + # @param [ Object ] b The second object. # # @return [ Integer ] The comparison value. def compare(a, b) - case - when a.nil? then b.nil? ? 0 : 1 - when b.nil? then -1 - else a <=> b - end + return 0 if a.nil? && b.nil? + return 1 if a.nil? + return -1 if b.nil? + + compare_operand(a) <=> compare_operand(b) end # Sort the documents in place. @@ -655,9 +656,8 @@ def compare(a, b) def in_place_sort(values) documents.sort! do |a, b| values.map do |field, direction| - a_value, b_value = a[field], b[field] - direction * compare(a_value.__sortable__, b_value.__sortable__) - end.find { |value| !value.zero? } || 0 + direction * compare(a[field], b[field]) + end.detect { |value| !value.zero? } || 0 end end @@ -683,6 +683,23 @@ def _session @criteria.send(:_session) end + # Get the operand value to be used in comparison. + # Adds capability to sort boolean values. + # + # @example Get the comparison operand. + # compare_operand(true) #=> 1 + # + # @param [ Object ] value The value to be used in comparison. + # + # @return [ Integer | Object ] The comparison operand. + def compare_operand(value) + case value + when TrueClass then 1 + when FalseClass then 0 + else value + end + end + # Retrieve the value for the current document at the given field path. # # For example, if I have the following models: diff --git a/lib/mongoid/extensions/false_class.rb b/lib/mongoid/extensions/false_class.rb index 4b3cfad259..d50e6a6b9d 100644 --- a/lib/mongoid/extensions/false_class.rb +++ b/lib/mongoid/extensions/false_class.rb @@ -3,19 +3,19 @@ module Mongoid module Extensions - # Adds type-casting behavior to FalseClass. module FalseClass - # Get the value of the object as a mongo friendly sort value. # # @example Get the object as sort criteria. # object.__sortable__ # # @return [ Integer ] 0. + # @deprecated def __sortable__ 0 end + Mongoid.deprecate(self, :__sortable__) # Is the passed value a boolean? # @@ -35,4 +35,4 @@ def is_a?(other) end end -::FalseClass.__send__(:include, Mongoid::Extensions::FalseClass) +FalseClass.include Mongoid::Extensions::FalseClass diff --git a/lib/mongoid/extensions/object.rb b/lib/mongoid/extensions/object.rb index ec0cd89f91..69e06680d2 100644 --- a/lib/mongoid/extensions/object.rb +++ b/lib/mongoid/extensions/object.rb @@ -60,9 +60,11 @@ def __setter__ # object.__sortable__ # # @return [ Object ] self. + # @deprecated def __sortable__ self end + Mongoid.deprecate(self, :__sortable__) # Conversion of an object to an $inc-able value. # diff --git a/lib/mongoid/extensions/true_class.rb b/lib/mongoid/extensions/true_class.rb index 147659f0bd..779586c743 100644 --- a/lib/mongoid/extensions/true_class.rb +++ b/lib/mongoid/extensions/true_class.rb @@ -3,19 +3,19 @@ module Mongoid module Extensions - # Adds type-casting behavior to TrueClass module TrueClass - # Get the value of the object as a mongo friendly sort value. # # @example Get the object as sort criteria. # object.__sortable__ # # @return [ Integer ] 1. + # @deprecated def __sortable__ 1 end + Mongoid.deprecate(self, :__sortable__) # Is the passed value a boolean? # @@ -35,4 +35,4 @@ def is_a?(other) end end -::TrueClass.__send__(:include, Mongoid::Extensions::TrueClass) +TrueClass.include Mongoid::Extensions::TrueClass diff --git a/spec/mongoid/extensions/false_class_spec.rb b/spec/mongoid/extensions/false_class_spec.rb index b2dd763e3e..6450898548 100644 --- a/spec/mongoid/extensions/false_class_spec.rb +++ b/spec/mongoid/extensions/false_class_spec.rb @@ -5,13 +5,6 @@ describe Mongoid::Extensions::FalseClass do - describe "#__sortable__" do - - it "returns 0" do - expect(false.__sortable__).to eq(0) - end - end - describe "#is_a?" do context "when provided a Boolean" do diff --git a/spec/mongoid/extensions/object_spec.rb b/spec/mongoid/extensions/object_spec.rb index 80d313a3af..0e95c16caf 100644 --- a/spec/mongoid/extensions/object_spec.rb +++ b/spec/mongoid/extensions/object_spec.rb @@ -128,13 +128,6 @@ end end - describe "#__sortable__" do - - it "returns self" do - expect(object.__sortable__).to eq(object) - end - end - describe ".demongoize" do let(:object) do diff --git a/spec/mongoid/extensions/true_class_spec.rb b/spec/mongoid/extensions/true_class_spec.rb index 221aaf5532..7aa561412e 100644 --- a/spec/mongoid/extensions/true_class_spec.rb +++ b/spec/mongoid/extensions/true_class_spec.rb @@ -5,13 +5,6 @@ describe Mongoid::Extensions::TrueClass do - describe "#__sortable__" do - - it "returns 1" do - expect(true.__sortable__).to eq(1) - end - end - describe "#is_a?" do context "when provided a Boolean" do From 66bd54dadc9d35013f89d883216e47a220a55d82 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 7 Nov 2023 07:22:16 +0900 Subject: [PATCH 15/28] MONGOID-5664 [Monkey Patch Removal] Remove Object#__setter__ monkey-patch (#5707) * Remove Object#__setter__ monkey-patch. This is a trivial method so I've just inlined it. (It was already inlined in several other places.) * deprecate __setter__ instead of removing it --------- Co-authored-by: Jamis Buck --- lib/mongoid/association/relatable.rb | 4 ++-- lib/mongoid/extensions/nil_class.rb | 6 +++--- lib/mongoid/extensions/object.rb | 2 ++ 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/mongoid/association/relatable.rb b/lib/mongoid/association/relatable.rb index fb7ca7ebe0..fc485c4d94 100644 --- a/lib/mongoid/association/relatable.rb +++ b/lib/mongoid/association/relatable.rb @@ -81,7 +81,7 @@ def get_callbacks(callback_type) # # @return [ String ] The type setter method. def type_setter - @type_setter ||= type.__setter__ + @type_setter ||= "#{type}=" if type end # Whether trying to bind an object using this association should raise @@ -233,7 +233,7 @@ def path(document) # # @return [ String ] The name of the setter. def inverse_type_setter - @inverse_type_setter ||= inverse_type.__setter__ + @inverse_type_setter ||= "#{inverse_type}=" if inverse_type end # Get the name of the method to check if the foreign key has changed. diff --git a/lib/mongoid/extensions/nil_class.rb b/lib/mongoid/extensions/nil_class.rb index f8ca4e8809..7cfc52a951 100644 --- a/lib/mongoid/extensions/nil_class.rb +++ b/lib/mongoid/extensions/nil_class.rb @@ -3,19 +3,19 @@ module Mongoid module Extensions - # Adds type-casting behavior to NilClass. module NilClass - # Try to form a setter from this object. # # @example Try to form a setter. # object.__setter__ # # @return [ nil ] Always nil. + # @deprecated def __setter__ self end + Mongoid.deprecate(self, :__setter__) # Get the name of a nil collection. # @@ -30,4 +30,4 @@ def collectionize end end -::NilClass.__send__(:include, Mongoid::Extensions::NilClass) +NilClass.include Mongoid::Extensions::NilClass diff --git a/lib/mongoid/extensions/object.rb b/lib/mongoid/extensions/object.rb index 69e06680d2..c3c469e9d3 100644 --- a/lib/mongoid/extensions/object.rb +++ b/lib/mongoid/extensions/object.rb @@ -50,9 +50,11 @@ def __mongoize_time__ # object.__setter__ # # @return [ String ] The object as a string plus =. + # @deprecated def __setter__ "#{self}=" end + Mongoid.deprecate(self, :__setter__) # Get the value of the object as a mongo friendly sort value. # From 20c656345d84e1d02da4614acf7279f9a4a7a6a1 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 7 Nov 2023 07:54:13 +0900 Subject: [PATCH 16/28] MONGOID-5669 [Monkey Patch Removal] Remove Array#multi_arged? (#5702) * Move Array#multi_arged? to Criteria::Findable.multiple_args? * Update findable_spec.rb * Clarify code docs * Add pending for Set spec * deprecate __multi_arged?__ instead of removing it --------- Co-authored-by: Jamis Buck --- lib/mongoid/criteria/findable.rb | 21 ++++++-- lib/mongoid/extensions/array.rb | 3 +- lib/mongoid/extensions/object.rb | 2 + spec/mongoid/extensions/array_spec.rb | 72 --------------------------- 4 files changed, 22 insertions(+), 76 deletions(-) diff --git a/lib/mongoid/criteria/findable.rb b/lib/mongoid/criteria/findable.rb index 09d557b44c..2f116dc353 100644 --- a/lib/mongoid/criteria/findable.rb +++ b/lib/mongoid/criteria/findable.rb @@ -14,7 +14,8 @@ module Findable # criteria.execute_or_raise(id) # # @param [ Object ] ids The arguments passed. - # @param [ true | false ] multi Whether there arguments were a list. + # @param [ true | false ] multi Whether there arguments were a list, + # and therefore the return value should be an array. # # @raise [ Errors::DocumentNotFound ] If nothing returned. # @@ -42,7 +43,7 @@ def execute_or_raise(ids, multi) def find(*args) ids = args.__find_args__ raise_invalid if ids.any?(&:nil?) - for_ids(ids).execute_or_raise(ids, args.multi_arged?) + for_ids(ids).execute_or_raise(ids, multi_args?(args)) end # Adds a criterion to the +Criteria+ that specifies an id that must be matched. @@ -108,7 +109,7 @@ def from_database(ids) from_database_selector(ids).entries end - private def from_database_selector(ids) + def from_database_selector(ids) if ids.size > 1 any_in(_id: ids) else @@ -133,6 +134,20 @@ def mongoize_ids(ids) end end + # Indicates whether the given arguments array is a list of values. + # Used by the +find+ method to determine whether to return an array + # or single value. + # + # @example Are these arguments a list of values? + # multi_args?([ 1, 2, 3 ]) #=> true + # + # @param [ Array ] args The arguments. + # + # @return [ true | false ] Whether the arguments are a list. + def multi_args?(args) + args.size > 1 || !args.first.is_a?(Hash) && args.first.resizable? + end + # Convenience method of raising an invalid options error. # # @example Raise the error. diff --git a/lib/mongoid/extensions/array.rb b/lib/mongoid/extensions/array.rb index 812ac620da..7cf774c754 100644 --- a/lib/mongoid/extensions/array.rb +++ b/lib/mongoid/extensions/array.rb @@ -3,7 +3,6 @@ module Mongoid module Extensions - # Adds type-casting behavior to Array class. module Array @@ -80,9 +79,11 @@ def blank_criteria? # [ 1, 2, 3 ].multi_arged? # # @return [ true | false ] If the array is multi args. + # @deprecated def multi_arged? !first.is_a?(Hash) && first.resizable? || size > 1 end + Mongoid.deprecate(self, :multi_arged?) # Turn the object from the ruby type we deal with to a Mongo friendly # type. diff --git a/lib/mongoid/extensions/object.rb b/lib/mongoid/extensions/object.rb index c3c469e9d3..98e480c912 100644 --- a/lib/mongoid/extensions/object.rb +++ b/lib/mongoid/extensions/object.rb @@ -140,9 +140,11 @@ def mongoize # object.multi_arged? # # @return [ false ] false. + # @deprecated def multi_arged? false end + Mongoid.deprecate(self, :multi_arged?) # Is the object a number? # diff --git a/spec/mongoid/extensions/array_spec.rb b/spec/mongoid/extensions/array_spec.rb index 18ab1ee2f2..49c402986e 100644 --- a/spec/mongoid/extensions/array_spec.rb +++ b/spec/mongoid/extensions/array_spec.rb @@ -533,78 +533,6 @@ end end - describe "#multi_arged?" do - - context "when there are multiple elements" do - - let(:array) do - [ 1, 2, 3 ] - end - - it "returns true" do - expect(array).to be_multi_arged - end - end - - context "when there is one element" do - - context "when the element is a non enumerable" do - - let(:array) do - [ 1 ] - end - - it "returns false" do - expect(array).to_not be_multi_arged - end - end - - context "when the element is resizable Hash instance" do - - let(:array) do - [{'key' => 'value'}] - end - - it "returns false" do - expect(array).to_not be_multi_arged - end - end - - context "when the element is array of resizable Hash instances" do - - let(:array) do - [[{'key1' => 'value2'},{'key1' => 'value2'}]] - end - - it "returns true" do - expect(array).to be_multi_arged - end - end - - context "when the element is an array" do - - let(:array) do - [[ 1 ]] - end - - it "returns true" do - expect(array).to be_multi_arged - end - end - - context "when the element is a range" do - - let(:array) do - [ 1..2 ] - end - - it "returns true" do - expect(array).to be_multi_arged - end - end - end - end - describe ".resizable?" do it "returns true" do From 67c8035298550892716c427e82c7ee732090da9c Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Tue, 7 Nov 2023 08:41:13 +0900 Subject: [PATCH 17/28] MONGOID-5665 [Monkey Patch Removal] Remove Object#__find_args__ (#5706) * Remove Object#__find_args__. This primarily used in Criteria::Findable where it is now a private method. 2 points to note: - __find_args__ was mistakenly called in Atomic#unset method. Although technically __find_args__ does a similar thing as what #unset requires for its argument preparation, this is pure coincidence and its a kludge to use find_args here. So I've inlined the code and made a new private method. - __find_args__ for Range previously called to_a. This is actually a bug / DOS risk, because ranges of strings tend to explode when you use to_a. As an example, ('64e0'..'64ff').to_a.size => 9320, and it gets much worse for 24-char BSON ids! Interestingly however, MongoDB itself can handle ranges of ids gracefully, so I am now just passing them into the DB as-is and it magically works--I've added tests. * Fix specs * Add test to cover #find for nested arrays, which was pre-existing behavior. * deprecate __find_args__ instead of removing it * Set#resizable? has to be true for this to work --------- Co-authored-by: Jamis Buck --- lib/mongoid/contextual/atomic.rb | 34 +++-- lib/mongoid/criteria/findable.rb | 23 ++- lib/mongoid/extensions/array.rb | 2 + lib/mongoid/extensions/object.rb | 2 + lib/mongoid/extensions/range.rb | 10 +- lib/mongoid/extensions/set.rb | 12 +- spec/mongoid/criteria/findable_spec.rb | 190 +++++++++++++++++++++++++ spec/mongoid/extensions/object_spec.rb | 7 - spec/mongoid/extensions/range_spec.rb | 11 -- 9 files changed, 257 insertions(+), 34 deletions(-) diff --git a/lib/mongoid/contextual/atomic.rb b/lib/mongoid/contextual/atomic.rb index 1587c4aca7..8ab4911504 100644 --- a/lib/mongoid/contextual/atomic.rb +++ b/lib/mongoid/contextual/atomic.rb @@ -169,17 +169,14 @@ def set(sets) # @example Unset the field on the matches. # context.unset(:name) # - # @param [ [ String | Symbol | Array | Hash ]... ] *args - # The name(s) of the field(s) to unset. - # If a Hash is specified, its keys will be used irrespective of what - # each key's value is, even if the value is nil or false. + # @param [ [ String | Symbol | Array | Hash ]... ] *unsets + # The name(s) of the field(s) to unset. If a Hash is specified, + # its keys will be used irrespective of value, even if the value + # is nil or false. # # @return [ nil ] Nil. - def unset(*args) - fields = args.map { |a| a.is_a?(Hash) ? a.keys : a } - .__find_args__ - .map { |f| [database_field_name(f), true] } - view.update_many("$unset" => Hash[fields]) + def unset(*unsets) + view.update_many('$unset' => collect_unset_operations(unsets)) end # Performs an atomic $min update operation on the given field or fields. @@ -247,6 +244,25 @@ def collect_each_operations(ops) operations[database_field_name(field)] = { "$each" => Array.wrap(value).mongoize } end end + + # Builds the selector an atomic $unset operation from arguments. + # + # @example Prepare selector from array. + # context.collect_unset_operations([:name, :age]) + # #=> { "name" => true, "age" => true } + # + # @example Prepare selector from hash. + # context.collect_unset_operations({ name: 1 }, { age: 1 }) + # #=> { "name" => true, "age" => true } + # + # @param [ String | Symbol | Array | Hash ] ops + # The name(s) of the field(s) to unset. + # + # @return [ Hash ] The selector for the atomic $unset operation. + def collect_unset_operations(ops) + ops.map { |op| op.is_a?(Hash) ? op.keys : op }.flatten + .map { |field| [database_field_name(field), true] }.to_h + end end end end diff --git a/lib/mongoid/criteria/findable.rb b/lib/mongoid/criteria/findable.rb index 2f116dc353..527821a631 100644 --- a/lib/mongoid/criteria/findable.rb +++ b/lib/mongoid/criteria/findable.rb @@ -41,7 +41,7 @@ def execute_or_raise(ids, multi) # # @return [ Document | Array ] The matching document(s). def find(*args) - ids = args.__find_args__ + ids = prepare_ids_for_find(args) raise_invalid if ids.any?(&:nil?) for_ids(ids).execute_or_raise(ids, multi_args?(args)) end @@ -134,6 +134,27 @@ def mongoize_ids(ids) end end + # Convert args to the +#find+ method into a flat array of ids. + # + # @example Get the ids. + # prepare_ids_for_find([ 1, [ 2, 3 ] ]) + # + # @param [ Array ] args The arguments. + # + # @return [ Array ] The array of ids. + def prepare_ids_for_find(args) + args.flat_map do |arg| + case arg + when Array, Set + prepare_ids_for_find(arg) + when Range + arg.begin&.numeric? && arg.end&.numeric? ? arg.to_a : arg + else + arg + end + end.uniq(&:to_s) + end + # Indicates whether the given arguments array is a list of values. # Used by the +find+ method to determine whether to return an array # or single value. diff --git a/lib/mongoid/extensions/array.rb b/lib/mongoid/extensions/array.rb index 7cf774c754..072a1d42b4 100644 --- a/lib/mongoid/extensions/array.rb +++ b/lib/mongoid/extensions/array.rb @@ -23,9 +23,11 @@ def __evolve_object_id__ # [ 1, 2, 3 ].__find_args__ # # @return [ Array ] The array of args. + # @deprecated def __find_args__ flat_map{ |a| a.__find_args__ }.uniq{ |a| a.to_s } end + Mongoid.deprecate(self, :__find_args__) # Mongoize the array into an array of object ids. # diff --git a/lib/mongoid/extensions/object.rb b/lib/mongoid/extensions/object.rb index 98e480c912..c14661bbcb 100644 --- a/lib/mongoid/extensions/object.rb +++ b/lib/mongoid/extensions/object.rb @@ -24,9 +24,11 @@ def __evolve_object_id__ # object.__find_args__ # # @return [ Object ] self. + # @deprecated def __find_args__ self end + Mongoid.deprecate(self, :__find_args__) # Mongoize a plain object into a time. # diff --git a/lib/mongoid/extensions/range.rb b/lib/mongoid/extensions/range.rb index a804dc2da6..e5dc8e89c3 100644 --- a/lib/mongoid/extensions/range.rb +++ b/lib/mongoid/extensions/range.rb @@ -3,9 +3,11 @@ module Mongoid module Extensions - # Adds type-casting behavior to Range class. module Range + def self.included(base) + base.extend(ClassMethods) + end # Get the range as arguments for a find. # @@ -13,9 +15,11 @@ module Range # range.__find_args__ # # @return [ Array ] The range as an array. + # @deprecated def __find_args__ to_a end + Mongoid.deprecate(self, :__find_args__) # Turn the object from the ruby type we deal with to a Mongo friendly # type. @@ -39,7 +43,6 @@ def resizable? end module ClassMethods - # Convert the object from its mongo friendly ruby type to this type. # # @example Demongoize the object. @@ -107,5 +110,4 @@ def __mongoize_range__(object) end end -::Range.__send__(:include, Mongoid::Extensions::Range) -::Range.extend(Mongoid::Extensions::Range::ClassMethods) +Range.include(Mongoid::Extensions::Range) diff --git a/lib/mongoid/extensions/set.rb b/lib/mongoid/extensions/set.rb index 21a887a5f6..574723f845 100644 --- a/lib/mongoid/extensions/set.rb +++ b/lib/mongoid/extensions/set.rb @@ -6,7 +6,6 @@ module Extensions # Adds type-casting behavior to Set class. module Set - # Turn the object from the ruby type we deal with to a Mongo friendly # type. # @@ -18,8 +17,17 @@ def mongoize ::Set.mongoize(self) end - module ClassMethods + # Returns whether the object's size can be changed. + # + # @example Is the object resizable? + # object.resizable? + # + # @return [ true ] true. + def resizable? + true + end + module ClassMethods # Convert the object from its mongo friendly ruby type to this type. # # @example Demongoize the object. diff --git a/spec/mongoid/criteria/findable_spec.rb b/spec/mongoid/criteria/findable_spec.rb index 9715fe1ae3..1468fc5f80 100644 --- a/spec/mongoid/criteria/findable_spec.rb +++ b/spec/mongoid/criteria/findable_spec.rb @@ -260,6 +260,196 @@ end end + context "when providing nested arrays of ids" do + + let!(:band_two) do + Band.create!(name: "Tool") + end + + context "when all ids match" do + + let(:found) do + Band.find([ [ band.id ], [ [ band_two.id ] ] ]) + end + + it "contains the first match" do + expect(found).to include(band) + end + + it "contains the second match" do + expect(found).to include(band_two) + end + + context "when ids are duplicates" do + + let(:found) do + Band.find([ [ band.id ], [ [ band.id ] ] ]) + end + + it "contains only the first match" do + expect(found).to eq([band]) + end + end + end + + context "when any id does not match" do + + context "when raising a not found error" do + config_override :raise_not_found_error, true + + let(:found) do + Band.find([ [ band.id ], [ [ BSON::ObjectId.new ] ] ]) + end + + it "raises an error" do + expect { + found + }.to raise_error(Mongoid::Errors::DocumentNotFound, /Document\(s\) not found for class Band with id\(s\)/) + end + end + + context "when raising no error" do + config_override :raise_not_found_error, false + + let(:found) do + Band.find([ [ band.id ], [ [ BSON::ObjectId.new ] ] ]) + end + + it "returns only the matching documents" do + expect(found).to eq([ band ]) + end + end + end + end + + context "when providing a Set of ids" do + + let!(:band_two) do + Band.create!(name: "Tool") + end + + context "when all ids match" do + let(:found) do + Band.find(Set[ band.id, band_two.id ]) + end + + it "contains the first match" do + expect(found).to include(band) + end + + it "contains the second match" do + expect(found).to include(band_two) + end + end + + context "when any id does not match" do + + context "when raising a not found error" do + config_override :raise_not_found_error, true + + let(:found) do + Band.find(Set[ band.id, BSON::ObjectId.new ]) + end + + it "raises an error" do + expect { + found + }.to raise_error(Mongoid::Errors::DocumentNotFound, /Document\(s\) not found for class Band with id\(s\)/) + end + end + + context "when raising no error" do + config_override :raise_not_found_error, false + + let(:found) do + Band.find(Set[ band.id, BSON::ObjectId.new ]) + end + + it "returns only the matching documents" do + expect(found).to eq([ band ]) + end + end + end + end + + context "when providing a Range of ids" do + + let!(:band_two) do + Band.create!(name: "Tool") + end + + context "when all ids match" do + let(:found) do + Band.find(band.id.to_s..band_two.id.to_s) + end + + it "contains the first match" do + expect(found).to include(band) + end + + it "contains the second match" do + expect(found).to include(band_two) + end + + + context "when any id does not match" do + + context "when raising a not found error" do + config_override :raise_not_found_error, true + + let(:found) do + Band.find(band_two.id.to_s..BSON::ObjectId.new) + end + + it "does not raise error and returns only the matching documents" do + expect(found).to eq([ band_two ]) + end + end + + context "when raising no error" do + config_override :raise_not_found_error, false + + let(:found) do + Band.find(band_two.id.to_s..BSON::ObjectId.new) + end + + it "returns only the matching documents" do + expect(found).to eq([ band_two ]) + end + end + end + end + + context "when all ids do not match" do + + context "when raising a not found error" do + config_override :raise_not_found_error, true + + let(:found) do + Band.find(BSON::ObjectId.new..BSON::ObjectId.new) + end + + it "raises an error" do + expect { + found + }.to raise_error(Mongoid::Errors::DocumentNotFound, /Document\(s\) not found for class Band with id\(s\)/) + end + end + + context "when raising no error" do + config_override :raise_not_found_error, false + + let(:found) do + Band.find(BSON::ObjectId.new..BSON::ObjectId.new) + end + + it "returns only the matching documents" do + expect(found).to eq([]) + end + end + end + end + context "when providing a single id as extended json" do context "when the id matches" do diff --git a/spec/mongoid/extensions/object_spec.rb b/spec/mongoid/extensions/object_spec.rb index 0e95c16caf..4b2e294ded 100644 --- a/spec/mongoid/extensions/object_spec.rb +++ b/spec/mongoid/extensions/object_spec.rb @@ -16,13 +16,6 @@ end end - describe "#__find_args__" do - - it "returns self" do - expect(object.__find_args__).to eq(object) - end - end - describe "#__mongoize_object_id__" do it "returns self" do diff --git a/spec/mongoid/extensions/range_spec.rb b/spec/mongoid/extensions/range_spec.rb index 572f076fb2..58ea85c9ff 100644 --- a/spec/mongoid/extensions/range_spec.rb +++ b/spec/mongoid/extensions/range_spec.rb @@ -5,17 +5,6 @@ describe Mongoid::Extensions::Range do - describe "#__find_args__" do - - let(:range) do - 1..3 - end - - it "returns the range as an array" do - expect(range.__find_args__).to eq([ 1, 2, 3 ]) - end - end - describe ".demongoize" do subject { Range.demongoize(hash) } From 5dff6d639b7e710f753c40299e1c3cf5484a4d85 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Tue, 7 Nov 2023 10:06:08 -0700 Subject: [PATCH 18/28] MONGOID-5654: refactor and deprecate Hash#__consolidate__ (#5740) * refactor and deprecate Hash#__consolidate__ * fix linter complaints * another minor refactoring for optimization * remove pre-refactoring docs * fix failing specs --- lib/mongoid/atomic_update_preparer.rb | 88 +++++++++++++++++++++ lib/mongoid/contextual/mongo.rb | 5 +- lib/mongoid/extensions/hash.rb | 69 ++-------------- spec/mongoid/atomic_update_preparer_spec.rb | 83 +++++++++++++++++++ spec/mongoid/extensions/hash_spec.rb | 57 ------------- 5 files changed, 181 insertions(+), 121 deletions(-) create mode 100644 lib/mongoid/atomic_update_preparer.rb create mode 100644 spec/mongoid/atomic_update_preparer_spec.rb diff --git a/lib/mongoid/atomic_update_preparer.rb b/lib/mongoid/atomic_update_preparer.rb new file mode 100644 index 0000000000..c4655eefd1 --- /dev/null +++ b/lib/mongoid/atomic_update_preparer.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +module Mongoid + # A singleton class to assist with preparing attributes for atomic + # updates. + # + # Once the deprecated Hash#__consolidate__ method is removed entirely, + # these methods may be moved into Mongoid::Contextual::Mongo as private + # methods. + # + # @api private + class AtomicUpdatePreparer + class << self + # Convert the key/values in the attributes into a hash of atomic updates. + # Non-operator keys are assumed to use $set operation. + # + # @param [ Class ] klass The model class. + # @param [ Hash ] attributes The attributes to convert. + # + # @return [ Hash ] The prepared atomic updates. + def prepare(attributes, klass) + attributes.each_pair.with_object({}) do |(key, value), atomic_updates| + key = klass.database_field_name(key.to_s) + + if key.to_s.start_with?('$') + (atomic_updates[key] ||= {}).update(prepare_operation(klass, key, value)) + else + (atomic_updates['$set'] ||= {})[key] = mongoize_for(key, klass, key, value) + end + end + end + + private + + # Treats the key as if it were a MongoDB operator and prepares + # the value accordingly. + # + # @param [ Class ] klass the model class + # @param [ String | Symbol ] key the operator + # @param [ Hash ] value the operand + # + # @return [ Hash ] the prepared value. + def prepare_operation(klass, key, value) + value.each_with_object({}) do |(key2, value2), hash| + key2 = klass.database_field_name(key2) + hash[key2] = value_for(key, klass, value2) + end + end + + # Get the value for the provided operator, klass, key and value. + # + # This is necessary for special cases like $rename, $addToSet and $push. + # + # @param [ String ] operator The operator. + # @param [ Class ] klass The model class. + # @param [ Object ] value The original value. + # + # @return [ Object ] Value prepared for the provided operator. + def value_for(operator, klass, value) + case operator + when '$rename' then value.to_s + when '$addToSet', '$push' then value.mongoize + else mongoize_for(operator, klass, operator, value) + end + end + + # Mongoize for the klass, key and value. + # + # @param [ String ] operator The operator. + # @param [ Class ] klass The model class. + # @param [ String | Symbol ] key The field key. + # @param [ Object ] value The value to mongoize. + # + # @return [ Object ] The mongoized value. + def mongoize_for(operator, klass, key, value) + field = klass.fields[key.to_s] + return value unless field + + mongoized = field.mongoize(value) + if Mongoid::Persistable::LIST_OPERATIONS.include?(operator) && field.resizable? && !value.is_a?(Array) + return mongoized.first + end + + mongoized + end + end + end +end diff --git a/lib/mongoid/contextual/mongo.rb b/lib/mongoid/contextual/mongo.rb index 916d8a2a72..3fceba9dfc 100644 --- a/lib/mongoid/contextual/mongo.rb +++ b/lib/mongoid/contextual/mongo.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true # rubocop:todo all +require 'mongoid/atomic_update_preparer' require "mongoid/contextual/mongo/documents_loader" require "mongoid/contextual/atomic" require "mongoid/contextual/aggregable/mongo" @@ -817,8 +818,8 @@ def load_async # @return [ true | false ] If the update succeeded. def update_documents(attributes, method = :update_one, opts = {}) return false unless attributes - attributes = Hash[attributes.map { |k, v| [klass.database_field_name(k.to_s), v] }] - view.send(method, attributes.__consolidate__(klass), opts) + + view.send(method, AtomicUpdatePreparer.prepare(attributes, klass), opts) end # Apply the field limitations. diff --git a/lib/mongoid/extensions/hash.rb b/lib/mongoid/extensions/hash.rb index f1d1a27e22..1cf19d539d 100644 --- a/lib/mongoid/extensions/hash.rb +++ b/lib/mongoid/extensions/hash.rb @@ -32,31 +32,20 @@ def __mongoize_object_id__ end # Consolidate the key/values in the hash under an atomic $set. + # DEPRECATED. This was never intended to be a public API and + # the functionality will no longer be exposed once this method + # is eventually removed. # # @example Consolidate the hash. # { name: "Placebo" }.__consolidate__ # # @return [ Hash ] A new consolidated hash. + # + # @deprecated def __consolidate__(klass) - consolidated = {} - each_pair do |key, value| - if key =~ /\$/ - value.keys.each do |key2| - value2 = value[key2] - real_key = klass.database_field_name(key2) - - value.delete(key2) if real_key != key2 - value[real_key] = value_for(key, klass, real_key, value2) - end - consolidated[key] ||= {} - consolidated[key].update(value) - else - consolidated["$set"] ||= {} - consolidated["$set"].update(key => mongoize_for(key, klass, key, value)) - end - end - consolidated + Mongoid::AtomicUpdatePreparer.prepare(self, klass) end + Mongoid.deprecate(self, :__consolidate__) # Checks whether conditions given in this hash are known to be # unsatisfiable, i.e., querying with this hash will always return no @@ -166,50 +155,6 @@ def to_criteria private - # Get the value for the provided operator, klass, key and value. - # - # This is necessary for special cases like $rename, $addToSet and $push. - # - # @param [ String ] operator The operator. - # @param [ Class ] klass The model class. - # @param [ String | Symbol ] key The field key. - # @param [ Object ] value The original value. - # - # @return [ Object ] Value prepared for the provided operator. - def value_for(operator, klass, key, value) - case operator - when "$rename" then value.to_s - when "$addToSet", "$push" then value.mongoize - else mongoize_for(operator, klass, operator, value) - end - end - - # Mongoize for the klass, key and value. - # - # @api private - # - # @example Mongoize for the klass, field and value. - # {}.mongoize_for("$push", Band, "name", "test") - # - # @param [ String ] operator The operator. - # @param [ Class ] klass The model class. - # @param [ String | Symbol ] key The field key. - # @param [ Object ] value The value to mongoize. - # - # @return [ Object ] The mongoized value. - def mongoize_for(operator, klass, key, value) - field = klass.fields[key.to_s] - if field - val = field.mongoize(value) - if Mongoid::Persistable::LIST_OPERATIONS.include?(operator) && field.resizable? - val = val.first if !value.is_a?(Array) - end - val - else - value - end - end - module ClassMethods # Turn the object from the ruby type we deal with to a Mongo friendly diff --git a/spec/mongoid/atomic_update_preparer_spec.rb b/spec/mongoid/atomic_update_preparer_spec.rb new file mode 100644 index 0000000000..6c39ac3aa6 --- /dev/null +++ b/spec/mongoid/atomic_update_preparer_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Mongoid::AtomicUpdatePreparer do + describe '#prepare' do + let(:prepared) { described_class.prepare(hash, Band) } + + context 'when the hash already contains $set' do + context 'when the $set is first' do + let(:hash) do + { '$set' => { name: 'Tool' }, likes: 10, '$inc' => { plays: 1 } } + end + + it 'moves the non hash values under the provided key' do + expect(prepared).to eq( + '$set' => { 'name' => 'Tool', 'likes' => 10 }, + '$inc' => { 'plays' => 1 } + ) + end + end + + context 'when the $set is not first' do + let(:hash) do + { likes: 10, '$inc' => { plays: 1 }, '$set' => { name: 'Tool' } } + end + + it 'moves the non hash values under the provided key' do + expect(prepared).to eq( + '$set' => { 'likes' => 10, 'name' => 'Tool' }, + '$inc' => { 'plays' => 1 } + ) + end + end + end + + context 'when the hash does not contain $set' do + let(:hash) do + { likes: 10, '$inc' => { plays: 1 }, name: 'Tool' } + end + + it 'moves the non hash values under the provided key' do + expect(prepared).to eq( + '$set' => { 'likes' => 10, 'name' => 'Tool' }, + '$inc' => { 'plays' => 1 } + ) + end + end + + context 'when the hash contains $rename' do + let(:hash) { { likes: 10, '$rename' => { old: 'new' } } } + + it 'preserves the $rename operator' do + expect(prepared).to eq( + '$set' => { 'likes' => 10 }, + '$rename' => { 'old' => 'new' } + ) + end + end + + context 'when the hash contains $addToSet' do + let(:hash) { { likes: 10, '$addToSet' => { list: 'new' } } } + + it 'preserves the $addToSet operator' do + expect(prepared).to eq( + '$set' => { 'likes' => 10 }, + '$addToSet' => { 'list' => 'new' } + ) + end + end + + context 'when the hash contains $push' do + let(:hash) { { likes: 10, '$push' => { list: 14 } } } + + it 'preserves the $push operator' do + expect(prepared).to eq( + '$set' => { 'likes' => 10 }, + '$push' => { 'list' => 14 } + ) + end + end + end +end diff --git a/spec/mongoid/extensions/hash_spec.rb b/spec/mongoid/extensions/hash_spec.rb index cc4135a742..bcc1a58d6f 100644 --- a/spec/mongoid/extensions/hash_spec.rb +++ b/spec/mongoid/extensions/hash_spec.rb @@ -163,63 +163,6 @@ end end - describe "#__consolidate__" do - - context "when the hash already contains the key" do - - context "when the $set is first" do - - let(:hash) do - { "$set" => { name: "Tool" }, likes: 10, "$inc" => { plays: 1 }} - end - - let(:consolidated) do - hash.__consolidate__(Band) - end - - it "moves the non hash values under the provided key" do - expect(consolidated).to eq({ - "$set" => { 'name' => "Tool", likes: 10 }, "$inc" => { 'plays' => 1 } - }) - end - end - - context "when the $set is not first" do - - let(:hash) do - { likes: 10, "$inc" => { plays: 1 }, "$set" => { name: "Tool" }} - end - - let(:consolidated) do - hash.__consolidate__(Band) - end - - it "moves the non hash values under the provided key" do - expect(consolidated).to eq({ - "$set" => { likes: 10, 'name' => "Tool" }, "$inc" => { 'plays' => 1 } - }) - end - end - end - - context "when the hash does not contain the key" do - - let(:hash) do - { likes: 10, "$inc" => { plays: 1 }, name: "Tool"} - end - - let(:consolidated) do - hash.__consolidate__(Band) - end - - it "moves the non hash values under the provided key" do - expect(consolidated).to eq({ - "$set" => { likes: 10, name: "Tool" }, "$inc" => { 'plays' => 1 } - }) - end - end - end - describe ".demongoize" do let(:hash) do From f99e917c7e1fbba0f66f66836b89577e33ee40d0 Mon Sep 17 00:00:00 2001 From: Jamis Buck Date: Tue, 7 Nov 2023 11:05:33 -0700 Subject: [PATCH 19/28] MONGOID-5662 Deprecate Object#__to_inc__ and BigDecimal#__to_inc__ (#5741) --- lib/mongoid/extensions/big_decimal.rb | 15 +++++++++++---- lib/mongoid/extensions/object.rb | 13 +++++++------ lib/mongoid/persistable/incrementable.rb | 2 +- lib/mongoid/persistable/multipliable.rb | 2 +- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lib/mongoid/extensions/big_decimal.rb b/lib/mongoid/extensions/big_decimal.rb index 72641890ca..cce430f2bc 100644 --- a/lib/mongoid/extensions/big_decimal.rb +++ b/lib/mongoid/extensions/big_decimal.rb @@ -3,9 +3,16 @@ module Mongoid module Extensions - # Adds type-casting behavior to BigDecimal class. module BigDecimal + # Behavior to be invoked when the module is included. + # + # @param [ Module ] base the class or module doing the including + # + # @api private + def self.included(base) + base.extend(ClassMethods) + end # Convert the big decimal to an $inc-able value. # @@ -13,9 +20,11 @@ module BigDecimal # bd.__to_inc__ # # @return [ Float ] The big decimal as a float. + # @deprecated def __to_inc__ to_f end + Mongoid.deprecate(self, :__to_inc__) # Turn the object from the ruby type we deal with to a Mongo friendly # type. @@ -39,7 +48,6 @@ def numeric? end module ClassMethods - # Convert the object from its mongo friendly ruby type to this type. # # @param [ Object ] object The object to demongoize. @@ -89,5 +97,4 @@ def mongoize(object) end end -::BigDecimal.__send__(:include, Mongoid::Extensions::BigDecimal) -::BigDecimal.extend(Mongoid::Extensions::BigDecimal::ClassMethods) +BigDecimal.include Mongoid::Extensions::BigDecimal diff --git a/lib/mongoid/extensions/object.rb b/lib/mongoid/extensions/object.rb index c14661bbcb..089269bc7e 100644 --- a/lib/mongoid/extensions/object.rb +++ b/lib/mongoid/extensions/object.rb @@ -3,9 +3,11 @@ module Mongoid module Extensions - # Adds type-casting behavior to Object class. module Object + def self.included(base) + base.extend(ClassMethods) + end # Evolve a plain object into an object id. # @@ -76,9 +78,11 @@ def __sortable__ # 1.__to_inc__ # # @return [ Object ] The object. + # @deprecated def __to_inc__ self end + Mongoid.deprecate(self, :__to_inc__) # Checks whether conditions given in this object are known to be # unsatisfiable, i.e., querying with this object will always return no @@ -93,6 +97,7 @@ def __to_inc__ def blank_criteria? false end + Mongoid.deprecate(self, :blank_criteria?) # Do or do not, there is no try. -- Yoda. # @@ -210,7 +215,6 @@ def you_must(name, *args) end module ClassMethods - # Convert the provided object to a foreign key, given the metadata key # contstraint. # @@ -255,7 +259,4 @@ def mongoize(object) end end -::Object.__send__(:include, Mongoid::Extensions::Object) -::Object.extend(Mongoid::Extensions::Object::ClassMethods) - -::Mongoid.deprecate(Object, :blank_criteria) +Object.include Mongoid::Extensions::Object diff --git a/lib/mongoid/persistable/incrementable.rb b/lib/mongoid/persistable/incrementable.rb index 566b3c113a..1fd5b9a458 100644 --- a/lib/mongoid/persistable/incrementable.rb +++ b/lib/mongoid/persistable/incrementable.rb @@ -21,7 +21,7 @@ module Incrementable def inc(increments) prepare_atomic_operation do |ops| process_atomic_operations(increments) do |field, value| - increment = value.__to_inc__ + increment = value.is_a?(BigDecimal) ? value.to_f : value current = attributes[field] new_value = (current || 0) + increment process_attribute field, new_value if executing_atomically? diff --git a/lib/mongoid/persistable/multipliable.rb b/lib/mongoid/persistable/multipliable.rb index 4569d01da2..787d940590 100644 --- a/lib/mongoid/persistable/multipliable.rb +++ b/lib/mongoid/persistable/multipliable.rb @@ -21,7 +21,7 @@ module Multipliable def mul(factors) prepare_atomic_operation do |ops| process_atomic_operations(factors) do |field, value| - factor = value.__to_inc__ + factor = value.is_a?(BigDecimal) ? value.to_f : value current = attributes[field] new_value = (current || 0) * factor process_attribute field, new_value if executing_atomically? From c30458910b970a0b89c42f61ab032ac76b9a69e3 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Wed, 8 Nov 2023 03:07:55 +0900 Subject: [PATCH 20/28] MONGOID-5667 [Monkey Patch Removal] Remove String/Integer#unconvertable_to_bson? (#5704) * Remove String#unconvertable_to_bson? and Integer#unconvertable_to_bson? * Remove one more usage * MONGOID-5667 deprecate instead of remove --------- Co-authored-by: Jamis Buck --- lib/mongoid/extensions/integer.rb | 2 ++ lib/mongoid/extensions/string.rb | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/lib/mongoid/extensions/integer.rb b/lib/mongoid/extensions/integer.rb index 3076256272..d8fbf4a649 100644 --- a/lib/mongoid/extensions/integer.rb +++ b/lib/mongoid/extensions/integer.rb @@ -33,9 +33,11 @@ def numeric? # object.unconvertable_to_bson? # # @return [ true ] If the object is unconvertable. + # @deprecated def unconvertable_to_bson? true end + Mongoid.deprecate(self, :unconvertable_to_bson?) module ClassMethods diff --git a/lib/mongoid/extensions/string.rb b/lib/mongoid/extensions/string.rb index 38858afe5b..5e31d84ef8 100644 --- a/lib/mongoid/extensions/string.rb +++ b/lib/mongoid/extensions/string.rb @@ -8,7 +8,9 @@ module Extensions module String # @attribute [rw] unconvertable_to_bson If the document is unconvertable. + # @deprecated attr_accessor :unconvertable_to_bson + Mongoid.deprecate(self, :unconvertable_to_bson, :unconvertable_to_bson=) # Evolve the string into an object id if possible. # @@ -126,15 +128,18 @@ def before_type_cast? ends_with?("_before_type_cast") end + # Is the object not to be converted to bson on criteria creation? # # @example Is the object unconvertable? # object.unconvertable_to_bson? # # @return [ true | false ] If the object is unconvertable. + # @deprecated def unconvertable_to_bson? @unconvertable_to_bson ||= false end + Mongoid.deprecate(self, :unconvertable_to_bson?) private From fc98ff0f31f3f6c5a62ae9268f81ec0db2f656ff Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Wed, 8 Nov 2023 03:09:01 +0900 Subject: [PATCH 21/28] MONGOID-5668 [Monkey Patch Removal] Remove String/Symbol#mongoid_id? (#5703) * Remove String#mongoid_id? and Symbol#mongoid_id? * deprecate instead of remove --------- Co-authored-by: Jamis Buck --- lib/mongoid/extensions/string.rb | 2 ++ lib/mongoid/extensions/symbol.rb | 2 ++ spec/mongoid/extensions/string_spec.rb | 31 -------------------------- spec/mongoid/extensions/symbol_spec.rb | 31 -------------------------- 4 files changed, 4 insertions(+), 62 deletions(-) diff --git a/lib/mongoid/extensions/string.rb b/lib/mongoid/extensions/string.rb index 5e31d84ef8..df210f945e 100644 --- a/lib/mongoid/extensions/string.rb +++ b/lib/mongoid/extensions/string.rb @@ -71,9 +71,11 @@ def collectionize # "_id".mongoid_id? # # @return [ true | false ] If the string is id or _id. + # @deprecated def mongoid_id? self =~ /\A(|_)id\z/ end + Mongoid.deprecate(self, :mongoid_id?) # Is the string a number? The literals "NaN", "Infinity", and "-Infinity" # are counted as numbers. diff --git a/lib/mongoid/extensions/symbol.rb b/lib/mongoid/extensions/symbol.rb index 65c9b519a5..c37536e965 100644 --- a/lib/mongoid/extensions/symbol.rb +++ b/lib/mongoid/extensions/symbol.rb @@ -13,9 +13,11 @@ module Symbol # :_id.mongoid_id? # # @return [ true | false ] If the symbol is :id or :_id. + # @deprecated def mongoid_id? to_s.mongoid_id? end + Mongoid.deprecate(self, :mongoid_id?) module ClassMethods diff --git a/spec/mongoid/extensions/string_spec.rb b/spec/mongoid/extensions/string_spec.rb index adc9cd49bd..e7d27c7aa3 100644 --- a/spec/mongoid/extensions/string_spec.rb +++ b/spec/mongoid/extensions/string_spec.rb @@ -172,37 +172,6 @@ class Patient end end - describe "#mongoid_id?" do - - context "when the string is id" do - - it "returns true" do - expect("id").to be_mongoid_id - end - end - - context "when the string is _id" do - - it "returns true" do - expect("_id").to be_mongoid_id - end - end - - context "when the string contains id" do - - it "returns false" do - expect("identity").to_not be_mongoid_id - end - end - - context "when the string contains _id" do - - it "returns false" do - expect("something_id").to_not be_mongoid_id - end - end - end - [ :mongoize, :demongoize ].each do |method| describe ".#{method}" do diff --git a/spec/mongoid/extensions/symbol_spec.rb b/spec/mongoid/extensions/symbol_spec.rb index 2bd4d7b6a5..6ec6bd0e75 100644 --- a/spec/mongoid/extensions/symbol_spec.rb +++ b/spec/mongoid/extensions/symbol_spec.rb @@ -5,37 +5,6 @@ describe Mongoid::Extensions::Symbol do - describe "#mongoid_id?" do - - context "when the string is id" do - - it "returns true" do - expect(:id).to be_mongoid_id - end - end - - context "when the string is _id" do - - it "returns true" do - expect(:_id).to be_mongoid_id - end - end - - context "when the string contains id" do - - it "returns false" do - expect(:identity).to_not be_mongoid_id - end - end - - context "when the string contains _id" do - - it "returns false" do - expect(:something_id).to_not be_mongoid_id - end - end - end - [ :mongoize, :demongoize ].each do |method| describe ".mongoize" do From 73781415b5dc5d2c2c28221c0898bf7c517b445d Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Wed, 8 Nov 2023 07:22:34 +0900 Subject: [PATCH 22/28] MONGOID-5670 [Monkey Patch Removal] Remove Hash#delete_id and Hash#extract_id (#5701) * Move Hash#delete_id and Hash#extract_id to Nested::NestedBuildable * Fix failing spec * deprecate, don't remove --------- Co-authored-by: Jamis Buck --- lib/mongoid/association/nested/many.rb | 5 ++-- .../association/nested/nested_buildable.rb | 27 +++++++++++++++++++ lib/mongoid/association/nested/one.rb | 2 +- lib/mongoid/criteria.rb | 4 +-- lib/mongoid/extensions/hash.rb | 4 +++ 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/lib/mongoid/association/nested/many.rb b/lib/mongoid/association/nested/many.rb index 636bc8014c..a078a5c647 100644 --- a/lib/mongoid/association/nested/many.rb +++ b/lib/mongoid/association/nested/many.rb @@ -101,7 +101,8 @@ def over_limit?(attributes) # @param [ Hash ] attrs The single document attributes to process. def process_attributes(parent, attrs) return if reject?(parent, attrs) - if id = attrs.extract_id + + if (id = extract_id(attrs)) update_nested_relation(parent, id, attrs) else existing.push(Factory.build(@class_name, attrs)) unless destroyable?(attrs) @@ -153,7 +154,7 @@ def destroy_document(relation, doc) # @param [ Document ] doc The document to update. # @param [ Hash ] attrs The attributes. def update_document(doc, attrs) - attrs.delete_id + delete_id(attrs) if association.embedded? doc.assign_attributes(attrs) else diff --git a/lib/mongoid/association/nested/nested_buildable.rb b/lib/mongoid/association/nested/nested_buildable.rb index 55254450bd..81d6b292b6 100644 --- a/lib/mongoid/association/nested/nested_buildable.rb +++ b/lib/mongoid/association/nested/nested_buildable.rb @@ -65,6 +65,33 @@ def update_only? def convert_id(klass, id) klass.using_object_ids? ? BSON::ObjectId.mongoize(id) : id end + + private + + # Get the id attribute from the given hash, whether it's + # prefixed with an underscore or is a symbol. + # + # @example Get the id. + # extract_id({ _id: 1 }) + # + # @param [ Hash ] hash The hash from which to extract. + # + # @return [ Object ] The value of the id. + def extract_id(hash) + hash['_id'] || hash[:_id] || hash['id'] || hash[:id] + end + + # Deletes the id key from the given hash. + # + # @example Delete an id value. + # delete_id({ "_id" => 1 }) + # + # @param [ Hash ] hash The hash from which to delete. + # + # @return [ Object ] The deleted value, or nil. + def delete_id(hash) + hash.delete('_id') || hash.delete(:_id) || hash.delete('id') || hash.delete(:id) + end end end end diff --git a/lib/mongoid/association/nested/one.rb b/lib/mongoid/association/nested/one.rb index 126e6f1de6..5fbc9397da 100644 --- a/lib/mongoid/association/nested/one.rb +++ b/lib/mongoid/association/nested/one.rb @@ -29,7 +29,7 @@ def build(parent) return if reject?(parent, attributes) @existing = parent.send(association.name) if update? - attributes.delete_id + delete_id(attributes) existing.assign_attributes(attributes) elsif replace? parent.send(association.setter, Factory.build(@class_name, attributes)) diff --git a/lib/mongoid/criteria.rb b/lib/mongoid/criteria.rb index ad365b5a8f..a7013e5714 100644 --- a/lib/mongoid/criteria.rb +++ b/lib/mongoid/criteria.rb @@ -162,7 +162,7 @@ def embedded? # # @return [ Object ] The id. def extract_id - selector.extract_id + selector['_id'] || selector[:_id] || selector['id'] || selector[:id] end # Adds a criterion to the +Criteria+ that specifies additional options @@ -225,7 +225,7 @@ def initialize(klass) # may be desired. # # @example Merge the criteria with another criteria. - # criteri.merge(other_criteria) + # criteria.merge(other_criteria) # # @example Merge the criteria with a hash. The hash must contain a klass # key and the key/value pairs correspond to method names/args. diff --git a/lib/mongoid/extensions/hash.rb b/lib/mongoid/extensions/hash.rb index 1cf19d539d..0bb04eaa2f 100644 --- a/lib/mongoid/extensions/hash.rb +++ b/lib/mongoid/extensions/hash.rb @@ -101,9 +101,11 @@ def _mongoid_unsatisfiable_criteria? # {}.delete_id # # @return [ Object ] The deleted value, or nil. + # @deprecated def delete_id delete("_id") || delete(:_id) || delete("id") || delete(:id) end + Mongoid.deprecate(self, :delete_id) # Get the id attribute from this hash, whether it's prefixed with an # underscore or is a symbol. @@ -112,9 +114,11 @@ def delete_id # { :_id => 1 }.extract_id # # @return [ Object ] The value of the id. + # @deprecated def extract_id self["_id"] || self[:_id] || self["id"] || self[:id] end + Mongoid.deprecate(self, :extract_id) # Turn the object from the ruby type we deal with to a Mongo friendly # type. From fb1d2805ccb0f6f2c44d9d570ff5ed484606c378 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Wed, 8 Nov 2023 07:58:45 +0900 Subject: [PATCH 23/28] MONGOID-5671 [Monkey Patch Removal] Remove Object#blank_criteria? and Hash#__mongoid_unsatisfiable_criteria? (#5700) * - Remove ``Object#blank_criteria?`` method entirely (was previously deprecated and not used in code.) - Remove ``Hash#_mongoid_unsatisfiable_criteria?`` method is removed (was previously marked @api private) and move it to a private method of Referenced::HasMany::Enumerable. * Update enumerable.rb * removing the specs for unsatisifiable_criteria this tests an implementation detail and not a behavior, which is fragile. I'm not even sure this is an implementation detail we want, and testing it specifically pours metaphorical concrete around it, making it that much harder to remove later. --------- Co-authored-by: Jamis Buck --- docs/release-notes/mongoid-9.0.txt | 1 + .../referenced/has_many/enumerable.rb | 38 ++++++- lib/mongoid/extensions/array.rb | 22 ---- lib/mongoid/extensions/hash.rb | 50 --------- lib/mongoid/extensions/object.rb | 15 --- spec/mongoid/extensions/array_spec.rb | 28 ----- spec/mongoid/extensions/hash_spec.rb | 103 ------------------ spec/mongoid/extensions/object_spec.rb | 7 -- 8 files changed, 38 insertions(+), 226 deletions(-) diff --git a/docs/release-notes/mongoid-9.0.txt b/docs/release-notes/mongoid-9.0.txt index f6974ac5d2..620f2a69a4 100644 --- a/docs/release-notes/mongoid-9.0.txt +++ b/docs/release-notes/mongoid-9.0.txt @@ -74,6 +74,7 @@ Deprecated functionality removed The method ``Mongoid::QueryCache#clear_cache`` should be replaced with ``Mongo::QueryCache#clear``. All other methods and submodules are identically named. Refer to the `driver query cache documentation `_ for more details. +- ``Object#blank_criteria?`` method is removed (was previously deprecated.) ``touch`` method now clears changed state diff --git a/lib/mongoid/association/referenced/has_many/enumerable.rb b/lib/mongoid/association/referenced/has_many/enumerable.rb index 131a095680..b388e404be 100644 --- a/lib/mongoid/association/referenced/has_many/enumerable.rb +++ b/lib/mongoid/association/referenced/has_many/enumerable.rb @@ -478,12 +478,48 @@ def set_base(document) end def unloaded_documents - if _unloaded.selector._mongoid_unsatisfiable_criteria? + if unsatisfiable_criteria?(_unloaded.selector) [] else _unloaded end end + + # Checks whether conditions in the given hash are known to be + # unsatisfiable, i.e. querying with this hash will always return no + # documents. + # + # This method only handles condition shapes that Mongoid itself uses when + # it builds association queries. Return value true indicates the condition + # always produces an empty document set. Note however that return value false + # is not a guarantee that the condition won't produce an empty document set. + # + # @example Unsatisfiable conditions + # unsatisfiable_criteria?({'_id' => {'$in' => []}}) + # # => true + # + # @example Conditions which may be satisfiable + # unsatisfiable_criteria?({'_id' => '123'}) + # # => false + # + # @example Conditions which are unsatisfiable that this method does not handle + # unsatisfiable_criteria?({'foo' => {'$in' => []}}) + # # => false + # + # @param [ Hash ] selector The conditions to check. + # + # @return [ true | false ] Whether hash contains known unsatisfiable + # conditions. + def unsatisfiable_criteria?(selector) + unsatisfiable_criteria = { '_id' => { '$in' => [] } } + return true if selector == unsatisfiable_criteria + return false unless selector.length == 1 && selector.keys == %w[$and] + + value = selector.values.first + value.is_a?(Array) && value.any? do |sub_value| + sub_value.is_a?(Hash) && unsatisfiable_criteria?(sub_value) + end + end end end end diff --git a/lib/mongoid/extensions/array.rb b/lib/mongoid/extensions/array.rb index 072a1d42b4..5188ca557a 100644 --- a/lib/mongoid/extensions/array.rb +++ b/lib/mongoid/extensions/array.rb @@ -55,26 +55,6 @@ def __mongoize_time__ ::Time.configured.local(*self) end - # Checks whether conditions given in this array are known to be - # unsatisfiable, i.e., querying with this array will always return no - # documents. - # - # This method used to assume that the array is the list of criteria - # to be used with an $and operator. This assumption is no longer made; - # therefore, since the interpretation of conditions in the array differs - # between $and, $or and $nor operators, this method now always returns - # false. - # - # This method is deprecated. Mongoid now uses - # +_mongoid_unsatisfiable_criteria?+ internally; this method is retained - # for backwards compatibility only. It always returns false. - # - # @return [ false ] Always false. - # @deprecated - def blank_criteria? - false - end - # Is the array a set of multiple arguments in a method? # # @example Is this multi args? @@ -175,5 +155,3 @@ def resizable? ::Array.__send__(:include, Mongoid::Extensions::Array) ::Array.extend(Mongoid::Extensions::Array::ClassMethods) - -::Mongoid.deprecate(Array, :blank_criteria) diff --git a/lib/mongoid/extensions/hash.rb b/lib/mongoid/extensions/hash.rb index 0bb04eaa2f..67490aa5f0 100644 --- a/lib/mongoid/extensions/hash.rb +++ b/lib/mongoid/extensions/hash.rb @@ -47,54 +47,6 @@ def __consolidate__(klass) end Mongoid.deprecate(self, :__consolidate__) - # Checks whether conditions given in this hash are known to be - # unsatisfiable, i.e., querying with this hash will always return no - # documents. - # - # This method only handles condition shapes that Mongoid itself uses when - # it builds association queries. It does not guarantee that a false - # return value means the condition can produce a non-empty document set - - # only that if the return value is true, the condition always produces - # an empty document set. - # - # @example Unsatisfiable conditions - # {'_id' => {'$in' => []}}._mongoid_unsatisfiable_criteria? - # # => true - # - # @example Conditions which could be satisfiable - # {'_id' => '123'}._mongoid_unsatisfiable_criteria? - # # => false - # - # @example Conditions which are unsatisfiable that this method does not handle - # {'foo' => {'$in' => []}}._mongoid_unsatisfiable_criteria? - # # => false - # - # @return [ true | false ] Whether hash contains known unsatisfiable - # conditions. - # @api private - def _mongoid_unsatisfiable_criteria? - unsatisfiable_criteria = { "_id" => { "$in" => [] }} - return true if self == unsatisfiable_criteria - return false unless length == 1 && keys == %w($and) - value = values.first - value.is_a?(Array) && value.any? do |sub_v| - sub_v.is_a?(Hash) && sub_v._mongoid_unsatisfiable_criteria? - end - end - - # Checks whether conditions given in this hash are known to be - # unsatisfiable, i.e., querying with this hash will always return no - # documents. - # - # This method is deprecated. Mongoid now uses - # +_mongoid_unsatisfiable_criteria?+ internally; this method is retained - # for backwards compatibility only. - # - # @return [ true | false ] Whether hash contains known unsatisfiable - # conditions. - # @deprecated - alias :blank_criteria? :_mongoid_unsatisfiable_criteria? - # Deletes an id value from the hash. # # @example Delete an id value. @@ -196,5 +148,3 @@ def resizable? ::Hash.__send__(:include, Mongoid::Extensions::Hash) ::Hash.extend(Mongoid::Extensions::Hash::ClassMethods) - -::Mongoid.deprecate(Hash, :blank_criteria) diff --git a/lib/mongoid/extensions/object.rb b/lib/mongoid/extensions/object.rb index 089269bc7e..52fa4b4b37 100644 --- a/lib/mongoid/extensions/object.rb +++ b/lib/mongoid/extensions/object.rb @@ -84,21 +84,6 @@ def __to_inc__ end Mongoid.deprecate(self, :__to_inc__) - # Checks whether conditions given in this object are known to be - # unsatisfiable, i.e., querying with this object will always return no - # documents. - # - # This method is deprecated. Mongoid now uses - # +_mongoid_unsatisfiable_criteria?+ internally; this method is retained - # for backwards compatibility only. It always returns false. - # - # @return [ false ] Always false. - # @deprecated - def blank_criteria? - false - end - Mongoid.deprecate(self, :blank_criteria?) - # Do or do not, there is no try. -- Yoda. # # @example Do or do not. diff --git a/spec/mongoid/extensions/array_spec.rb b/spec/mongoid/extensions/array_spec.rb index 49c402986e..e9fa5b2fde 100644 --- a/spec/mongoid/extensions/array_spec.rb +++ b/spec/mongoid/extensions/array_spec.rb @@ -378,34 +378,6 @@ end end - describe "#blank_criteria?" do - - context "when the array has an empty _id criteria" do - - context "when only the id criteria is in the array" do - - let(:array) do - [{ "_id" => { "$in" => [] }}] - end - - it "is false" do - expect(array.blank_criteria?).to be false - end - end - - context "when the id criteria is in the array with others" do - - let(:array) do - [{ "_id" => "test" }, { "_id" => { "$in" => [] }}] - end - - it "is false" do - expect(array.blank_criteria?).to be false - end - end - end - end - describe "#delete_one" do context "when the object doesn't exist" do diff --git a/spec/mongoid/extensions/hash_spec.rb b/spec/mongoid/extensions/hash_spec.rb index bcc1a58d6f..5d797a14da 100644 --- a/spec/mongoid/extensions/hash_spec.rb +++ b/spec/mongoid/extensions/hash_spec.rb @@ -294,107 +294,4 @@ expect(Hash).to be_resizable end end - - shared_examples_for 'unsatisfiable criteria method' do - - context "when the hash has only an empty _id criteria" do - - let(:hash) do - { "_id" => { "$in" => [] }} - end - - it "is true" do - expect(hash.send(meth)).to be true - end - end - - context "when the hash has an empty _id criteria and another criteria" do - - let(:hash) do - { "_id" => { "$in" => [] }, 'foo' => 'bar'} - end - - it "is false" do - expect(hash.send(meth)).to be false - end - end - - context "when the hash has an empty _id criteria via $and" do - - let(:hash) do - {'$and' => [{ "_id" => { "$in" => [] }}]} - end - - it "is true" do - expect(hash.send(meth)).to be true - end - end - - context "when the hash has an empty _id criteria via $and and another criteria at top level" do - - let(:hash) do - {'$and' => [{ "_id" => { "$in" => [] }}], 'foo' => 'bar'} - end - - it "is false" do - expect(hash.send(meth)).to be false - end - end - - context "when the hash has an empty _id criteria via $and and another criteria in $and" do - - let(:hash) do - {'$and' => [{ "_id" => { "$in" => [] }}, {'foo' => 'bar'}]} - end - - it "is true" do - expect(hash.send(meth)).to be true - end - end - - context "when the hash has an empty _id criteria via $and and another criteria in $and value" do - - let(:hash) do - {'$and' => [{ "_id" => { "$in" => [] }, 'foo' => 'bar'}]} - end - - it "is false" do - expect(hash.send(meth)).to be false - end - end - - context "when the hash has an empty _id criteria via $or" do - - let(:hash) do - {'$or' => [{ "_id" => { "$in" => [] }}]} - end - - it "is false" do - expect(hash.send(meth)).to be false - end - end - - context "when the hash has an empty _id criteria via $nor" do - - let(:hash) do - {'$nor' => [{ "_id" => { "$in" => [] }}]} - end - - it "is false" do - expect(hash.send(meth)).to be false - end - end - end - - describe "#blank_criteria?" do - let(:meth) { :blank_criteria? } - - it_behaves_like 'unsatisfiable criteria method' - end - - describe "#_mongoid_unsatisfiable_criteria?" do - let(:meth) { :_mongoid_unsatisfiable_criteria? } - - it_behaves_like 'unsatisfiable criteria method' - end end diff --git a/spec/mongoid/extensions/object_spec.rb b/spec/mongoid/extensions/object_spec.rb index 4b2e294ded..73c01fb680 100644 --- a/spec/mongoid/extensions/object_spec.rb +++ b/spec/mongoid/extensions/object_spec.rb @@ -265,11 +265,4 @@ expect(object.numeric?).to eq(false) end end - - describe "#blank_criteria?" do - - it "is false" do - expect(object.blank_criteria?).to be false - end - end end From f12e44235b165239f9840eedd58b90e52e660d18 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Thu, 9 Nov 2023 01:03:44 +0900 Subject: [PATCH 24/28] MONGOID-5673 [Monkey Patch Removal] Remove Object#do_or_do_not and Object#you_must (#5713) * Remove do_or_do_not and you_must * Remove more do_or_do_not * reduce indent * Remove test * deprecate instead of remove --------- Co-authored-by: Jamis Buck --- lib/mongoid/association/bindable.rb | 32 ++++++++--- .../embedded/embedded_in/binding.rb | 8 +-- .../embedded/embeds_many/binding.rb | 4 +- .../embedded/embeds_one/binding.rb | 4 +- .../has_and_belongs_to_many/binding.rb | 6 +- lib/mongoid/cacheable.rb | 2 +- lib/mongoid/extensions/object.rb | 5 ++ lib/mongoid/validatable.rb | 2 +- spec/mongoid/extensions/object_spec.rb | 57 ------------------- 9 files changed, 43 insertions(+), 77 deletions(-) diff --git a/lib/mongoid/association/bindable.rb b/lib/mongoid/association/bindable.rb index 5a38148299..e1ec116a53 100644 --- a/lib/mongoid/association/bindable.rb +++ b/lib/mongoid/association/bindable.rb @@ -120,7 +120,7 @@ def remove_associated_in_to(doc, inverse) # @param [ Object ] id The id of the bound document. def bind_foreign_key(keyed, id) unless keyed.frozen? - keyed.you_must(_association.foreign_key_setter, id) + try_method(keyed, _association.foreign_key_setter, id) end end @@ -135,8 +135,8 @@ def bind_foreign_key(keyed, id) # @param [ Document ] typed The document that stores the type field. # @param [ String ] name The name of the model. def bind_polymorphic_type(typed, name) - if _association.type - typed.you_must(_association.type_setter, name) + if _association.type && !typed.frozen? + try_method(typed, _association.type_setter, name) end end @@ -151,8 +151,8 @@ def bind_polymorphic_type(typed, name) # @param [ Document ] typed The document that stores the type field. # @param [ String ] name The name of the model. def bind_polymorphic_inverse_type(typed, name) - if _association.inverse_type - typed.you_must(_association.inverse_type_setter, name) + if _association.inverse_type && !typed.frozen? + try_method(typed, _association.inverse_type_setter, name) end end @@ -167,8 +167,8 @@ def bind_polymorphic_inverse_type(typed, name) # @param [ Document ] doc The base document. # @param [ Document ] inverse The inverse document. def bind_inverse(doc, inverse) - if doc.respond_to?(_association.inverse_setter) - doc.you_must(_association.inverse_setter, inverse) + if doc.respond_to?(_association.inverse_setter) && !doc.frozen? + try_method(doc, _association.inverse_setter, inverse) end end @@ -223,6 +223,24 @@ def unbind_from_relational_parent(doc) bind_polymorphic_type(doc, nil) bind_inverse(doc, nil) end + + # Convenience method to perform +#try+ but return + # nil if the method argument is nil. + # + # @example Call method if it exists. + # object.try_method(:use, "The Force") + # + # @example Return nil if method argument is nil. + # object.try_method(nil, "The Force") #=> nil + # + # @param [ String | Symbol ] method_name The method name. + # @param [ Object... ] *args The arguments. + # + # @return [ Object | nil ] The result of the try or nil if the + # method does not exist. + def try_method(object, method_name, *args) + object.try(method_name, *args) if method_name + end end end end diff --git a/lib/mongoid/association/embedded/embedded_in/binding.rb b/lib/mongoid/association/embedded/embedded_in/binding.rb index a799bf2e6d..ea55921ab7 100644 --- a/lib/mongoid/association/embedded/embedded_in/binding.rb +++ b/lib/mongoid/association/embedded/embedded_in/binding.rb @@ -25,10 +25,10 @@ def bind_one _base._association = _association.inverse_association(_target) unless _base._association _base.parentize(_target) if _base.embedded_many? - _target.do_or_do_not(_association.inverse(_target)).push(_base) + _target.send(_association.inverse(_target)).push(_base) else remove_associated(_target) - _target.do_or_do_not(_association.inverse_setter(_target), _base) + try_method(_target, _association.inverse_setter(_target), _base) end end end @@ -42,9 +42,9 @@ def bind_one def unbind_one binding do if _base.embedded_many? - _target.do_or_do_not(_association.inverse(_target)).delete(_base) + _target.send(_association.inverse(_target)).delete(_base) else - _target.do_or_do_not(_association.inverse_setter(_target), nil) + try_method(_target, _association.inverse_setter(_target), nil) end end end diff --git a/lib/mongoid/association/embedded/embeds_many/binding.rb b/lib/mongoid/association/embedded/embeds_many/binding.rb index c178486a88..e2d7f41a77 100644 --- a/lib/mongoid/association/embedded/embeds_many/binding.rb +++ b/lib/mongoid/association/embedded/embeds_many/binding.rb @@ -21,7 +21,7 @@ def bind_one(doc) doc.parentize(_base) binding do remove_associated(doc) - doc.do_or_do_not(_association.inverse_setter(_target), _base) + try_method(doc, _association.inverse_setter(_target), _base) end end @@ -33,7 +33,7 @@ def bind_one(doc) # @param [ Document ] doc The single document to unbind. def unbind_one(doc) binding do - doc.do_or_do_not(_association.inverse_setter(_target), nil) + try_method(doc, _association.inverse_setter(_target), nil) end end end diff --git a/lib/mongoid/association/embedded/embeds_one/binding.rb b/lib/mongoid/association/embedded/embeds_one/binding.rb index 9b719721aa..6c6c356fe3 100644 --- a/lib/mongoid/association/embedded/embeds_one/binding.rb +++ b/lib/mongoid/association/embedded/embeds_one/binding.rb @@ -22,7 +22,7 @@ class Binding def bind_one _target.parentize(_base) binding do - _target.do_or_do_not(_association.inverse_setter(_target), _base) + try_method(_target, _association.inverse_setter(_target), _base) end end @@ -34,7 +34,7 @@ def bind_one # person.name = nil def unbind_one binding do - _target.do_or_do_not(_association.inverse_setter(_target), nil) + try_method(_target, _association.inverse_setter(_target), nil) end end end diff --git a/lib/mongoid/association/referenced/has_and_belongs_to_many/binding.rb b/lib/mongoid/association/referenced/has_and_belongs_to_many/binding.rb index 5b56a8670f..c67f421549 100644 --- a/lib/mongoid/association/referenced/has_and_belongs_to_many/binding.rb +++ b/lib/mongoid/association/referenced/has_and_belongs_to_many/binding.rb @@ -19,11 +19,11 @@ class Binding # @param [ Document ] doc The single document to bind. def bind_one(doc) binding do - inverse_keys = doc.you_must(_association.inverse_foreign_key) + inverse_keys = try_method(doc, _association.inverse_foreign_key) unless doc.frozen? if inverse_keys record_id = inverse_record_id(doc) unless inverse_keys.include?(record_id) - doc.you_must(_association.inverse_foreign_key_setter, inverse_keys.push(record_id)) + try_method(doc, _association.inverse_foreign_key_setter, inverse_keys.push(record_id)) end doc.reset_relation_criteria(_association.inverse) end @@ -39,7 +39,7 @@ def bind_one(doc) def unbind_one(doc) binding do _base.send(_association.foreign_key).delete_one(record_id(doc)) - inverse_keys = doc.you_must(_association.inverse_foreign_key) + inverse_keys = try_method(doc, _association.inverse_foreign_key) unless doc.frozen? if inverse_keys inverse_keys.delete_one(inverse_record_id(doc)) doc.reset_relation_criteria(_association.inverse) diff --git a/lib/mongoid/cacheable.rb b/lib/mongoid/cacheable.rb index 71bba22a22..dae0e47317 100644 --- a/lib/mongoid/cacheable.rb +++ b/lib/mongoid/cacheable.rb @@ -27,7 +27,7 @@ module Cacheable # @return [ String ] the string with or without updated_at def cache_key return "#{model_key}/new" if new_record? - return "#{model_key}/#{_id}-#{updated_at.utc.to_formatted_s(cache_timestamp_format)}" if do_or_do_not(:updated_at) + return "#{model_key}/#{_id}-#{updated_at.utc.to_formatted_s(cache_timestamp_format)}" if try(:updated_at) "#{model_key}/#{_id}" end end diff --git a/lib/mongoid/extensions/object.rb b/lib/mongoid/extensions/object.rb index 52fa4b4b37..495f7b2c40 100644 --- a/lib/mongoid/extensions/object.rb +++ b/lib/mongoid/extensions/object.rb @@ -84,6 +84,7 @@ def __to_inc__ end Mongoid.deprecate(self, :__to_inc__) + # Do or do not, there is no try. -- Yoda. # # @example Do or do not. @@ -94,9 +95,11 @@ def __to_inc__ # # @return [ Object | nil ] The result of the method call or nil if the # method does not exist. + # @deprecated def do_or_do_not(name, *args) send(name, *args) if name && respond_to?(name) end + Mongoid.deprecate(self, :do_or_do_not) # Get the value for an instance variable or false if it doesn't exist. # @@ -195,9 +198,11 @@ def substitutable # # @return [ Object | nil ] The result of the method call or nil if the # method does not exist. Nil if the object is frozen. + # @deprecated def you_must(name, *args) frozen? ? nil : do_or_do_not(name, *args) end + Mongoid.deprecate(self, :you_must) module ClassMethods # Convert the provided object to a foreign key, given the metadata key diff --git a/lib/mongoid/validatable.rb b/lib/mongoid/validatable.rb index 34183e3480..e43fe432f2 100644 --- a/lib/mongoid/validatable.rb +++ b/lib/mongoid/validatable.rb @@ -68,7 +68,7 @@ def read_attribute_for_validation(attr) begin_validate relation = without_autobuild { send(attr) } exit_validate - relation.do_or_do_not(:in_memory) || relation + relation.try(:in_memory) || relation elsif fields[attribute].try(:localized?) attributes[attribute] else diff --git a/spec/mongoid/extensions/object_spec.rb b/spec/mongoid/extensions/object_spec.rb index 73c01fb680..2a71c1931d 100644 --- a/spec/mongoid/extensions/object_spec.rb +++ b/spec/mongoid/extensions/object_spec.rb @@ -132,45 +132,6 @@ end end - describe "#do_or_do_not" do - - context "when the object is nil" do - - let(:result) do - nil.do_or_do_not(:not_a_method, "The force is strong with you") - end - - it "returns nil" do - expect(result).to be_nil - end - end - - context "when the object is not nil" do - - context "when the object responds to the method" do - - let(:result) do - [ "Yoda", "Luke" ].do_or_do_not(:join, ",") - end - - it "returns the result of the method" do - expect(result).to eq("Yoda,Luke") - end - end - - context "when the object does not respond to the method" do - - let(:result) do - "Yoda".do_or_do_not(:use, "The Force", 1000) - end - - it "returns the result of the method" do - expect(result).to be_nil - end - end - end - end - describe ".mongoize" do let(:object) do @@ -200,24 +161,6 @@ end end - describe "#you_must" do - - context "when the object is frozen" do - - let(:person) do - Person.new.tap { |peep| peep.freeze } - end - - let(:result) do - person.you_must(:aliases=, []) - end - - it "returns nil" do - expect(result).to be_nil - end - end - end - describe "#remove_ivar" do context "when the instance variable is defined" do From 3e2dd93bbc6a3fb6391e5557fc21b6d75bb2c7a6 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Thu, 9 Nov 2023 02:01:05 +0900 Subject: [PATCH 25/28] MONGOID-5674 [Monkey Patch Removal] Remove Object#regexp? (#5714) * Remove Object#regexp? * deprecate rather than remove --------- Co-authored-by: Jamis Buck --- .../criteria/queryable/extensions/object.rb | 2 + .../criteria/queryable/extensions/regexp.rb | 4 + .../criteria/queryable/extensions/string.rb | 15 ++- .../queryable/extensions/regexp_raw_spec.rb | 11 --- .../queryable/extensions/regexp_spec.rb | 11 --- .../queryable/extensions/string_spec.rb | 93 +++++++++---------- 6 files changed, 65 insertions(+), 71 deletions(-) diff --git a/lib/mongoid/criteria/queryable/extensions/object.rb b/lib/mongoid/criteria/queryable/extensions/object.rb index aaf8890063..32605781d4 100644 --- a/lib/mongoid/criteria/queryable/extensions/object.rb +++ b/lib/mongoid/criteria/queryable/extensions/object.rb @@ -128,9 +128,11 @@ def __expand_complex__ # obj.regexp? # # @return [ false ] Always false. + # @deprecated def regexp? false end + Mongoid.deprecate(self, :regexp?) module ClassMethods diff --git a/lib/mongoid/criteria/queryable/extensions/regexp.rb b/lib/mongoid/criteria/queryable/extensions/regexp.rb index 31cdafb95c..514c564b26 100644 --- a/lib/mongoid/criteria/queryable/extensions/regexp.rb +++ b/lib/mongoid/criteria/queryable/extensions/regexp.rb @@ -15,7 +15,9 @@ module Regexp # /\A[123]/.regexp? # # @return [ true ] Always true. + # @deprecated def regexp?; true; end + Mongoid.deprecate(self, :regexp?) module ClassMethods @@ -43,7 +45,9 @@ module Raw_ # bson_raw_regexp.regexp? # # @return [ true ] Always true. + # @deprecated def regexp?; true; end + Mongoid.deprecate(self, :regexp?) module ClassMethods diff --git a/lib/mongoid/criteria/queryable/extensions/string.rb b/lib/mongoid/criteria/queryable/extensions/string.rb index 4cc5133349..1bbd83d0a5 100644 --- a/lib/mongoid/criteria/queryable/extensions/string.rb +++ b/lib/mongoid/criteria/queryable/extensions/string.rb @@ -82,7 +82,7 @@ module ClassMethods # @return [ Hash ] The selection. def __expr_part__(key, value, negating = false) if negating - { key => { "$#{value.regexp? ? "not" : "ne"}" => value }} + { key => { "$#{__regexp?(value) ? "not" : "ne"}" => value }} else { key => value } end @@ -99,9 +99,20 @@ def __expr_part__(key, value, negating = false) # @return [ String ] The value as a string. def evolve(object) __evolve__(object) do |obj| - obj.regexp? ? obj : obj.to_s + __regexp?(obj) ? obj : obj.to_s end end + + private + + # Returns whether the object is Regexp-like. + # + # @param [ Object ] object The object to evaluate. + # + # @return [ Boolean ] Whether the object is Regexp-like. + def __regexp?(object) + object.is_a?(Regexp) || object.is_a?(BSON::Regexp::Raw) + end end end end diff --git a/spec/mongoid/criteria/queryable/extensions/regexp_raw_spec.rb b/spec/mongoid/criteria/queryable/extensions/regexp_raw_spec.rb index 11be3f6705..1f8807bf12 100644 --- a/spec/mongoid/criteria/queryable/extensions/regexp_raw_spec.rb +++ b/spec/mongoid/criteria/queryable/extensions/regexp_raw_spec.rb @@ -78,15 +78,4 @@ end end end - - describe "#regexp?" do - - let(:regexp) do - BSON::Regexp::Raw.new('^[123]') - end - - it "returns true" do - expect(regexp).to be_regexp - end - end end diff --git a/spec/mongoid/criteria/queryable/extensions/regexp_spec.rb b/spec/mongoid/criteria/queryable/extensions/regexp_spec.rb index 644d77beb8..9a0639f0bc 100644 --- a/spec/mongoid/criteria/queryable/extensions/regexp_spec.rb +++ b/spec/mongoid/criteria/queryable/extensions/regexp_spec.rb @@ -79,15 +79,4 @@ end end end - - describe "#regexp?" do - - let(:regexp) do - /\A[123]/ - end - - it "returns true" do - expect(regexp).to be_regexp - end - end end diff --git a/spec/mongoid/criteria/queryable/extensions/string_spec.rb b/spec/mongoid/criteria/queryable/extensions/string_spec.rb index 2eb5a722a2..c99c3a4209 100644 --- a/spec/mongoid/criteria/queryable/extensions/string_spec.rb +++ b/spec/mongoid/criteria/queryable/extensions/string_spec.rb @@ -181,84 +181,83 @@ end end - describe "#__expr_part__" do + describe '#__expr_part__' do + subject(:specified) { 'field'.__expr_part__(value) } + let(:value) { 10 } - let(:specified) do - "field".__expr_part__(10) + it 'returns the expression with the value' do + expect(specified).to eq({ 'field' => 10 }) end - it "returns the string with the value" do - expect(specified).to eq({ "field" => 10 }) - end - - context "with a regexp" do + context 'with a Regexp' do + let(:value) { /test/ } - let(:specified) do - "field".__expr_part__(/test/) + it 'returns the expression with the value' do + expect(specified).to eq({ 'field' => /test/ }) end + end - it "returns the symbol with the value" do - expect(specified).to eq({ "field" => /test/ }) - end + context 'with a BSON::Regexp::Raw' do + let(:value) { BSON::Regexp::Raw.new('^[123]') } + it 'returns the expression with the value' do + expect(specified).to eq({ 'field' => BSON::Regexp::Raw.new('^[123]') }) + end end - context "when negated" do + context 'when negated' do + subject(:specified) { 'field'.__expr_part__(value, true) } - context "with a regexp" do + context 'with a Regexp' do + let(:value) { /test/ } - let(:specified) do - "field".__expr_part__(/test/, true) + it 'returns the expression with the value negated' do + expect(specified).to eq({ 'field' => { '$not' => /test/ } }) end - - it "returns the string with the value negated" do - expect(specified).to eq({ "field" => { "$not" => /test/ } }) - end - end - context "with anything else" do + context 'with a BSON::Regexp::Raw' do + let(:value) { BSON::Regexp::Raw.new('^[123]') } - let(:specified) do - "field".__expr_part__('test', true) + it 'returns the expression with the value' do + expect(specified).to eq({ 'field' => { '$not' => BSON::Regexp::Raw.new('^[123]') } }) end + end - it "returns the string with the value negated" do - expect(specified).to eq({ "field" => { "$ne" => "test" }}) + context 'with anything else' do + let(:value) { 'test' } + + it 'returns the expression with the value negated' do + expect(specified).to eq({ 'field' => { '$ne' => 'test' }}) end end end end - describe ".evolve" do + describe '.evolve' do + subject(:evolved) { described_class.evolve(object) } - context "when provided a regex" do + context 'when provided a Regexp' do + let(:object) { /\A[123]/.freeze } - let(:regex) do - /\A[123]/.freeze - end - - let(:evolved) do - described_class.evolve(regex) - end - - it "returns the regex" do - expect(evolved).to eq(regex) + it 'returns the regexp' do + expect(evolved).to eq(/\A[123]/) end end - context "when provided an object" do + context 'when provided a BSON::Regexp::Raw' do + let(:object) { BSON::Regexp::Raw.new('^[123]') } - let(:object) do - 1234 + it 'returns the BSON::Regexp::Raw' do + expect(evolved).to eq(BSON::Regexp::Raw.new('^[123]')) end + end - let(:evolved) do - described_class.evolve(object) - end + context 'when provided an object' do + let(:object) { 1234 } - it "returns the object as a string" do - expect(evolved).to eq("1234") + it 'returns the object as a string' do + expect(evolved).to eq('1234') end end end From f4b29cecda47dad38b0e963eafe5b4dd22431ed6 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Thu, 9 Nov 2023 03:00:32 +0900 Subject: [PATCH 26/28] MONGOID-5676 [Monkey Patch Removal] Remove Time#configured (#5716) * Removes Time#configured monkey patch. * Set UTC timezone by default * More spec cleanup * Re-add error check * add release notes for removal of `Time.configured` --------- Co-authored-by: Jamis Buck --- docs/release-notes/mongoid-9.0.txt | 40 +++++++++++++++++++ .../criteria/queryable/extensions/date.rb | 2 +- lib/mongoid/extensions/array.rb | 2 +- lib/mongoid/extensions/date.rb | 2 +- lib/mongoid/extensions/float.rb | 2 +- lib/mongoid/extensions/integer.rb | 2 +- lib/mongoid/extensions/string.rb | 18 ++++----- lib/mongoid/extensions/time.rb | 13 ------ lib/mongoid/timestamps/created.rb | 6 +-- lib/mongoid/timestamps/updated.rb | 2 +- lib/mongoid/touchable.rb | 2 +- spec/integration/criteria/raw_value_spec.rb | 1 - spec/mongoid/findable_spec.rb | 2 +- spec/spec_helper.rb | 3 +- spec/support/macros.rb | 7 +--- spec/support/shared/time.rb | 8 +--- 16 files changed, 64 insertions(+), 48 deletions(-) diff --git a/docs/release-notes/mongoid-9.0.txt b/docs/release-notes/mongoid-9.0.txt index 620f2a69a4..85e496eb81 100644 --- a/docs/release-notes/mongoid-9.0.txt +++ b/docs/release-notes/mongoid-9.0.txt @@ -403,6 +403,46 @@ There are also rake tasks available, for convenience: $ rake mongoid:db:remove_search_indexes +``Time.configured`` has been removed +------------------------------------ + +``Time.configured`` returned either the time object wrapping the configured +time zone, or the standard Ruby ``Time`` class. This allowed you to query +a time value even if no time zone had been configured. + +Mongoid now requires that you set a time zone if you intend to do +anything with time values (including using timestamps in your documents). +Any uses of ``Time.configured`` must be replaced with ``Time.zone``. + +... code-block:: ruby + + # before: + puts Time.configured.now + + # after: + puts Time.zone.now + + # or, better for finding the current Time specifically: + puts Time.current + +If you do not set a time zone, you will see errors in your code related +to ``nil`` values. If you are using Rails, the default time zone is already +set to UTC. If you are not using Rails, you may set a time zone at the start +of your program like this: + +... code-block:: ruby + + Time.zone = 'UTC' + +This will set the time zone to UTC. You can see all available time zone names +by running the following command: + +... code-block:: bash + +$ ruby -ractive_support/values/time_zone \ + -e 'puts ActiveSupport::TimeZone::MAPPING.keys' + + Bug Fixes and Improvements -------------------------- diff --git a/lib/mongoid/criteria/queryable/extensions/date.rb b/lib/mongoid/criteria/queryable/extensions/date.rb index 55b95bfa43..d4bb4837d7 100644 --- a/lib/mongoid/criteria/queryable/extensions/date.rb +++ b/lib/mongoid/criteria/queryable/extensions/date.rb @@ -26,7 +26,7 @@ def __evolve_date__ # # @return [ Time | ActiveSupport::TimeWithZone ] The date as a local time. def __evolve_time__ - ::Time.configured.local(year, month, day) + ::Time.zone.local(year, month, day) end module ClassMethods diff --git a/lib/mongoid/extensions/array.rb b/lib/mongoid/extensions/array.rb index 5188ca557a..6fba2f4746 100644 --- a/lib/mongoid/extensions/array.rb +++ b/lib/mongoid/extensions/array.rb @@ -52,7 +52,7 @@ def __mongoize_object_id__ # configured default time zone corresponding to date/time components # in this array. def __mongoize_time__ - ::Time.configured.local(*self) + ::Time.zone.local(*self) end # Is the array a set of multiple arguments in a method? diff --git a/lib/mongoid/extensions/date.rb b/lib/mongoid/extensions/date.rb index 4a0bfa605a..d28e181d16 100644 --- a/lib/mongoid/extensions/date.rb +++ b/lib/mongoid/extensions/date.rb @@ -17,7 +17,7 @@ module Date # configured default time zone corresponding to local midnight of # this date. def __mongoize_time__ - ::Time.configured.local(year, month, day) + ::Time.zone.local(year, month, day) end # Turn the object from the ruby type we deal with to a Mongo friendly diff --git a/lib/mongoid/extensions/float.rb b/lib/mongoid/extensions/float.rb index bedd71b4d5..52f45a0f4d 100644 --- a/lib/mongoid/extensions/float.rb +++ b/lib/mongoid/extensions/float.rb @@ -14,7 +14,7 @@ module Float # # @return [ Time | ActiveSupport::TimeWithZone ] The time. def __mongoize_time__ - ::Time.configured.at(self) + ::Time.zone.at(self) end # Is the float a number? diff --git a/lib/mongoid/extensions/integer.rb b/lib/mongoid/extensions/integer.rb index d8fbf4a649..ec35ee97bd 100644 --- a/lib/mongoid/extensions/integer.rb +++ b/lib/mongoid/extensions/integer.rb @@ -14,7 +14,7 @@ module Integer # # @return [ Time | ActiveSupport::TimeWithZone ] The time. def __mongoize_time__ - ::Time.configured.at(self) + ::Time.zone.at(self) end # Is the integer a number? diff --git a/lib/mongoid/extensions/string.rb b/lib/mongoid/extensions/string.rb index df210f945e..b541b3adc9 100644 --- a/lib/mongoid/extensions/string.rb +++ b/lib/mongoid/extensions/string.rb @@ -40,19 +40,17 @@ def __mongoize_object_id__ # "2012-01-01".__mongoize_time__ # # => 2012-01-01 00:00:00 -0500 # + # @raise [ ArgumentError ] The string is not a valid time string. + # # @return [ Time | ActiveSupport::TimeWithZone ] Local time in the # configured default time zone corresponding to this string. def __mongoize_time__ - # This extra parse from Time is because ActiveSupport::TimeZone - # either returns nil or Time.now if the string is empty or invalid, - # which is a regression from pre-3.0 and also does not agree with - # the core Time API. - parsed = ::Time.parse(self) - if ::Time == ::Time.configured - parsed - else - ::Time.configured.parse(self) - end + # This extra Time.parse is required to raise an error if the string + # is not a valid time string. ActiveSupport::TimeZone does not + # perform this check. + ::Time.parse(self) + + ::Time.zone.parse(self) end # Convert the string to a collection friendly name. diff --git a/lib/mongoid/extensions/time.rb b/lib/mongoid/extensions/time.rb index 3f3e897cc7..a2689d664a 100644 --- a/lib/mongoid/extensions/time.rb +++ b/lib/mongoid/extensions/time.rb @@ -30,19 +30,6 @@ def mongoize module ClassMethods - # Get the configured time to use when converting - either the time zone - # or the time. - # - # @example Get the configured time. - # ::Time.configured - # - # @return [ Time ] The configured time. - # - # @deprecated - def configured - ::Time.zone || ::Time - end - # Convert the object from its mongo friendly ruby type to this type. # # @example Demongoize the object. diff --git a/lib/mongoid/timestamps/created.rb b/lib/mongoid/timestamps/created.rb index 42dc003f7e..28564148dc 100644 --- a/lib/mongoid/timestamps/created.rb +++ b/lib/mongoid/timestamps/created.rb @@ -24,9 +24,9 @@ module Created # person.set_created_at def set_created_at if !timeless? && !created_at - time = Time.configured.now - self.updated_at = time if is_a?(Updated) && !updated_at_changed? - self.created_at = time + now = Time.current + self.updated_at = now if is_a?(Updated) && !updated_at_changed? + self.created_at = now end clear_timeless_option end diff --git a/lib/mongoid/timestamps/updated.rb b/lib/mongoid/timestamps/updated.rb index 0e1951303a..7ad78dc6d1 100644 --- a/lib/mongoid/timestamps/updated.rb +++ b/lib/mongoid/timestamps/updated.rb @@ -25,7 +25,7 @@ module Updated # person.set_updated_at def set_updated_at if able_to_set_updated_at? - self.updated_at = Time.configured.now unless updated_at_changed? + self.updated_at = Time.current unless updated_at_changed? end clear_timeless_option diff --git a/lib/mongoid/touchable.rb b/lib/mongoid/touchable.rb index a7aeb7a87f..ded0a8f884 100644 --- a/lib/mongoid/touchable.rb +++ b/lib/mongoid/touchable.rb @@ -52,7 +52,7 @@ def touch(field = nil) return false if _root.new_record? begin - touches = _gather_touch_updates(Time.configured.now, field) + touches = _gather_touch_updates(Time.current, field) _root.send(:persist_atomic_operations, '$set' => touches) if touches.present? _run_touch_callbacks_from_root ensure diff --git a/spec/integration/criteria/raw_value_spec.rb b/spec/integration/criteria/raw_value_spec.rb index f72e055145..1b13975147 100644 --- a/spec/integration/criteria/raw_value_spec.rb +++ b/spec/integration/criteria/raw_value_spec.rb @@ -4,7 +4,6 @@ require 'spec_helper' describe 'Queries with Mongoid::RawValue criteria' do - before { Time.zone = 'UTC'} let(:now_utc) { Time.utc(2020, 1, 1, 16, 0, 0, 0) } let(:today) { Date.new(2020, 1, 1) } diff --git a/spec/mongoid/findable_spec.rb b/spec/mongoid/findable_spec.rb index 6189f3f3ad..bbe576902b 100644 --- a/spec/mongoid/findable_spec.rb +++ b/spec/mongoid/findable_spec.rb @@ -904,7 +904,7 @@ time_zone_override "Asia/Kolkata" let!(:time) do - Time.zone.now.tap do |t| + Time.current.tap do |t| User.create!(last_login: t, name: 'Tom') end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9da2629fb2..4a3ba31cce 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -117,8 +117,9 @@ class Query inflect.singular("address_components", "address_component") end -I18n.config.enforce_available_locales = false +Time.zone = 'UTC' +I18n.config.enforce_available_locales = false if %w(yes true 1).include?((ENV['TEST_I18N_FALLBACKS'] || '').downcase) require "i18n/backend/fallbacks" diff --git a/spec/support/macros.rb b/spec/support/macros.rb index 294fca7eb9..8751cdf085 100644 --- a/spec/support/macros.rb +++ b/spec/support/macros.rb @@ -103,12 +103,9 @@ def persistence_context_override(component, value) end end - def time_zone_override(tz) + def time_zone_override(time_zone) around do |example| - old_tz = Time.zone - Time.zone = tz - example.run - Time.zone = old_tz + Time.use_zone(time_zone) { example.run } end end diff --git a/spec/support/shared/time.rb b/spec/support/shared/time.rb index 3c98efe170..0144470216 100644 --- a/spec/support/shared/time.rb +++ b/spec/support/shared/time.rb @@ -2,13 +2,7 @@ # rubocop:todo all shared_context 'setting ActiveSupport time zone' do - before do - Time.zone = "Tokyo" - end - - after do - Time.zone = nil - end + time_zone_override 'Tokyo' end shared_examples_for 'mongoizes to AS::TimeWithZone' do From 0aaf8d510a84f7a94f754af875dd0ec987f3cb7f Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Thu, 9 Nov 2023 06:25:55 +0900 Subject: [PATCH 27/28] MONGOID-5677 [Monkey Patch Removal] Remove Hash#to_criteria and Criteria#to_criteria. Add Criteria.from_hash (#5717) * Remove Hash#to_criteria and Criteria#to_criteria. Add Criteria.from_hash * deprecate rathr than remove also, some code-style standardization --------- Co-authored-by: Jamis Buck --- lib/mongoid/criteria.rb | 37 +++++++-- lib/mongoid/extensions/hash.rb | 8 +- spec/mongoid/criteria_spec.rb | 137 ++++++++++++++++++++++----------- 3 files changed, 123 insertions(+), 59 deletions(-) diff --git a/lib/mongoid/criteria.rb b/lib/mongoid/criteria.rb index a7013e5714..ca024cace9 100644 --- a/lib/mongoid/criteria.rb +++ b/lib/mongoid/criteria.rb @@ -41,7 +41,25 @@ class Criteria include Clients::Sessions include Options - Mongoid.deprecate(self, :for_js) + class << self + # Convert the given hash to a criteria. Will iterate over each keys in the + # hash which must correspond to method on a criteria object. The hash + # must also include a "klass" key. + # + # @example Convert the hash to a criteria. + # Criteria.from_hash({ klass: Band, where: { name: "Depeche Mode" }) + # + # @param [ Hash ] hash The hash to convert. + # + # @return [ Criteria ] The criteria. + def from_hash(hash) + criteria = Criteria.new(hash.delete(:klass) || hash.delete('klass')) + hash.each_pair do |method, args| + criteria = criteria.__send__(method, args) + end + criteria + end + end # Static array used to check with method missing - we only need to ever # instantiate once. @@ -250,16 +268,16 @@ def merge(other) # @example Merge another criteria into this criteria. # criteria.merge(Person.where(name: "bob")) # - # @param [ Criteria ] other The criteria to merge in. + # @param [ Criteria | Hash ] other The criteria to merge in. # # @return [ Criteria ] The merged criteria. def merge!(other) - criteria = other.to_criteria - selector.merge!(criteria.selector) - options.merge!(criteria.options) - self.documents = criteria.documents.dup unless criteria.documents.empty? - self.scoping_options = criteria.scoping_options - self.inclusions = (inclusions + criteria.inclusions).uniq + other = self.class.from_hash(other) if other.is_a?(Hash) + selector.merge!(other.selector) + options.merge!(other.options) + self.documents = other.documents.dup unless other.documents.empty? + self.scoping_options = other.scoping_options + self.inclusions = (inclusions + other.inclusions).uniq self end @@ -352,9 +370,11 @@ def respond_to?(name, include_private = false) # criteria.to_criteria # # @return [ Criteria ] self. + # @deprecated def to_criteria self end + Mongoid.deprecate(self, :to_criteria) # Convert the criteria to a proc. # @@ -452,6 +472,7 @@ def for_js(javascript, scope = {}) end js_query(code) end + Mongoid.deprecate(self, :for_js) private diff --git a/lib/mongoid/extensions/hash.rb b/lib/mongoid/extensions/hash.rb index 67490aa5f0..19aaef11af 100644 --- a/lib/mongoid/extensions/hash.rb +++ b/lib/mongoid/extensions/hash.rb @@ -101,13 +101,11 @@ def resizable? # { klass: Band, where: { name: "Depeche Mode" }.to_criteria # # @return [ Criteria ] The criteria. + # @deprecated def to_criteria - criteria = Criteria.new(delete(:klass) || delete("klass")) - each_pair do |method, args| - criteria = criteria.__send__(method, args) - end - criteria + Criteria.from_hash(self) end + Mongoid.deprecate(self, :to_criteria) private diff --git a/spec/mongoid/criteria_spec.rb b/spec/mongoid/criteria_spec.rb index 6996341e2c..3808166cc1 100644 --- a/spec/mongoid/criteria_spec.rb +++ b/spec/mongoid/criteria_spec.rb @@ -1461,52 +1461,68 @@ end end - describe "#merge!" do + describe '#merge!' do + let(:band) { Band.new } + let(:criteria) { Band.scoped.where(name: 'Depeche Mode').asc(:name) } + let(:association) { Band.relations['records'] } + subject(:merged) { criteria.merge!(other) } - let(:band) do - Band.new - end + context 'when merging a Criteria' do + let(:other) do + { klass: Band, includes: [:records] } + end - let(:criteria) do - Band.scoped.where(name: "Depeche Mode").asc(:name) - end + it 'merges the selector' do + expect(merged.selector).to eq({ 'name' => 'Depeche Mode' }) + end - let(:mergeable) do - Band.includes(:records).tap do |crit| - crit.documents = [ band ] + it 'merges the options' do + expect(merged.options).to eq({ sort: { 'name' => 1 }}) end - end - let(:association) do - Band.relations["records"] - end + it 'merges the scoping options' do + expect(merged.scoping_options).to eq([ nil, nil ]) + end - let(:merged) do - criteria.merge!(mergeable) - end + it 'merges the inclusions' do + expect(merged.inclusions).to eq([ association ]) + end - it "merges the selector" do - expect(merged.selector).to eq({ "name" => "Depeche Mode" }) + it 'returns the same criteria' do + expect(merged).to equal(criteria) + end end - it "merges the options" do - expect(merged.options).to eq({ sort: { "name" => 1 }}) - end + context 'when merging a Hash' do + let(:other) do + Band.includes(:records).tap do |crit| + crit.documents = [ band ] + end + end - it "merges the documents" do - expect(merged.documents).to eq([ band ]) - end + it 'merges the selector' do + expect(merged.selector).to eq({ 'name' => 'Depeche Mode' }) + end - it "merges the scoping options" do - expect(merged.scoping_options).to eq([ nil, nil ]) - end + it 'merges the options' do + expect(merged.options).to eq({ sort: { 'name' => 1 }}) + end - it "merges the inclusions" do - expect(merged.inclusions).to eq([ association ]) - end + it 'merges the documents' do + expect(merged.documents).to eq([ band ]) + end + + it 'merges the scoping options' do + expect(merged.scoping_options).to eq([ nil, nil ]) + end - it "returns the same criteria" do - expect(merged).to equal(criteria) + it 'merges the inclusions' do + expect(merged.inclusions).to eq([ association ]) + end + + it 'returns the same criteria' do + expect(merged).to equal(criteria) + end end end @@ -2308,17 +2324,6 @@ def self.ages; self; end end end - describe "#to_criteria" do - - let(:criteria) do - Band.all - end - - it "returns self" do - expect(criteria.to_criteria).to eq(criteria) - end - end - describe "#to_proc" do let(:criteria) do @@ -3031,11 +3036,11 @@ def self.ages; self; end context "when the method exists on the criteria" do before do - expect(criteria).to receive(:to_criteria).and_call_original + expect(criteria).to receive(:only).and_call_original end it "calls the method on the criteria" do - expect(criteria.to_criteria).to eq(criteria) + expect(criteria.only).to eq(criteria) end end @@ -3242,4 +3247,44 @@ def self.ages; self; end end end end + + describe '.from_hash' do + subject(:criteria) { described_class.from_hash(hash) } + + context 'when klass is specified' do + let(:hash) do + { klass: Band, where: { name: 'Songs Ohia' } } + end + + it 'returns a criteria' do + expect(criteria).to be_a(Mongoid::Criteria) + end + + it 'sets the klass' do + expect(criteria.klass).to eq(Band) + end + + it 'sets the selector' do + expect(criteria.selector).to eq({ 'name' => 'Songs Ohia' }) + end + end + + context 'when klass is missing' do + let(:hash) do + { where: { name: 'Songs Ohia' } } + end + + it 'returns a criteria' do + expect(criteria).to be_a(Mongoid::Criteria) + end + + it 'has klass nil' do + expect(criteria.klass).to be_nil + end + + it 'sets the selector' do + expect(criteria.selector).to eq({ 'name' => 'Songs Ohia' }) + end + end + end end From ca3c6a500d2932c2851e762a9cbf1fc80d25a912 Mon Sep 17 00:00:00 2001 From: Johnny Shields <27655+johnnyshields@users.noreply.github.com> Date: Thu, 9 Nov 2023 06:53:54 +0900 Subject: [PATCH 28/28] MONGOID-5675 [Monkey Patch Removal] Remove Object#__mongoize_fk__ (#5715) * __mongoize_fk step 1: move code to right class * __mongoize_fk step 1: #mongoize_foreign_key should use class member variables * __mongoize_fk step 3: consolidate all specs to public #mongoize method * Update foreign_key.rb * deprecate, rather than remove * Set support requires a bit of baby-sitting... --------- Co-authored-by: Jamis Buck --- lib/mongoid/extensions/array.rb | 2 + lib/mongoid/extensions/object.rb | 2 + lib/mongoid/fields/foreign_key.rb | 26 +- spec/mongoid/extensions/array_spec.rb | 150 --------- spec/mongoid/extensions/object_spec.rb | 91 ------ spec/mongoid/fields/foreign_key_spec.rb | 400 +++++++++++++++++------- 6 files changed, 309 insertions(+), 362 deletions(-) diff --git a/lib/mongoid/extensions/array.rb b/lib/mongoid/extensions/array.rb index 6fba2f4746..0cf2a92d14 100644 --- a/lib/mongoid/extensions/array.rb +++ b/lib/mongoid/extensions/array.rb @@ -114,6 +114,7 @@ module ClassMethods # @param [ Object ] object The object to convert. # # @return [ Array ] The array of ids. + # @deprecated def __mongoize_fk__(association, object) if object.resizable? object.blank? ? object : association.convert_to_foreign_key(object) @@ -121,6 +122,7 @@ def __mongoize_fk__(association, object) object.blank? ? [] : association.convert_to_foreign_key(Array(object)) end end + Mongoid.deprecate(self, :__mongoize_fk__) # Turn the object from the ruby type we deal with to a Mongo friendly # type. diff --git a/lib/mongoid/extensions/object.rb b/lib/mongoid/extensions/object.rb index 495f7b2c40..879602f39e 100644 --- a/lib/mongoid/extensions/object.rb +++ b/lib/mongoid/extensions/object.rb @@ -215,10 +215,12 @@ module ClassMethods # @param [ Object ] object The object to convert. # # @return [ Object ] The converted object. + # @deprecated def __mongoize_fk__(association, object) return nil if !object || object == "" association.convert_to_foreign_key(object) end + Mongoid.deprecate(self, :__mongoize_fk__) # Convert the object from its mongo friendly ruby type to this type. # diff --git a/lib/mongoid/fields/foreign_key.rb b/lib/mongoid/fields/foreign_key.rb index 19cb802ab9..815d5cdc99 100644 --- a/lib/mongoid/fields/foreign_key.rb +++ b/lib/mongoid/fields/foreign_key.rb @@ -14,7 +14,7 @@ class ForeignKey < Standard # @example Add the atomic changes. # field.add_atomic_changes(doc, "key", {}, [], []) # - # @todo: Durran: Refactor, big time. + # @todo: Refactor, big time. # # @param [ Document ] document The document to add to. # @param [ String ] name The name of the field. @@ -95,7 +95,7 @@ def lazy? # @return [ Object ] The mongoized object. def mongoize(object) if type.resizable? || object_id_field? - type.__mongoize_fk__(association, object) + mongoize_foreign_key(object) else related_id_field.mongoize(object) end @@ -124,6 +124,28 @@ def resizable? private + # Convert the provided object to a Mongo-friendly foreign key. + # + # @example Convert the object to a foreign key. + # mongoize_foreign_key(object) + # + # @param [ Object ] object The object to convert. + # + # @return [ Object ] The converted object. + def mongoize_foreign_key(object) + if type == Array || type == Set + object = object.to_a if type == Set || object.is_a?(Set) + + if object.resizable? + object.blank? ? object : association.convert_to_foreign_key(object) + else + object.blank? ? [] : association.convert_to_foreign_key(Array(object)) + end + elsif !(object.nil? || object == '') + association.convert_to_foreign_key(object) + end + end + # Evaluate the default proc. In some cases we need to instance exec, # in others we don't. # diff --git a/spec/mongoid/extensions/array_spec.rb b/spec/mongoid/extensions/array_spec.rb index e9fa5b2fde..6c007320fb 100644 --- a/spec/mongoid/extensions/array_spec.rb +++ b/spec/mongoid/extensions/array_spec.rb @@ -203,156 +203,6 @@ end end - describe ".__mongoize_fk__" do - - context "when the related model uses object ids" do - - let(:association) do - Person.relations["preferences"] - end - - context "when provided an object id" do - - let(:object_id) do - BSON::ObjectId.new - end - - let(:fk) do - Array.__mongoize_fk__(association, object_id) - end - - it "returns the object id as an array" do - expect(fk).to eq([ object_id ]) - end - end - - context "when provided a object ids" do - - let(:object_id) do - BSON::ObjectId.new - end - - let(:fk) do - Array.__mongoize_fk__(association, [ object_id ]) - end - - it "returns the object ids" do - expect(fk).to eq([ object_id ]) - end - end - - context "when provided a string" do - - context "when the string is a legal object id" do - - let(:object_id) do - BSON::ObjectId.new - end - - let(:fk) do - Array.__mongoize_fk__(association, object_id.to_s) - end - - it "returns the object id in an array" do - expect(fk).to eq([ object_id ]) - end - end - - context "when the string is not a legal object id" do - - let(:string) do - "blah" - end - - let(:fk) do - Array.__mongoize_fk__(association, string) - end - - it "returns the string in an array" do - expect(fk).to eq([ string ]) - end - end - - context "when the string is blank" do - - let(:fk) do - Array.__mongoize_fk__(association, "") - end - - it "returns an empty array" do - expect(fk).to be_empty - end - end - end - - context "when provided nil" do - - let(:fk) do - Array.__mongoize_fk__(association, nil) - end - - it "returns an empty array" do - expect(fk).to be_empty - end - end - - context "when provided an array of strings" do - - context "when the strings are legal object ids" do - - let(:object_id) do - BSON::ObjectId.new - end - - let(:fk) do - Array.__mongoize_fk__(association, [ object_id.to_s ]) - end - - it "returns the object id in an array" do - expect(fk).to eq([ object_id ]) - end - end - - context "when the strings are not legal object ids" do - - let(:string) do - "blah" - end - - let(:fk) do - Array.__mongoize_fk__(association, [ string ]) - end - - it "returns the string in an array" do - expect(fk).to eq([ string ]) - end - end - - context "when the strings are blank" do - - let(:fk) do - Array.__mongoize_fk__(association, [ "", "" ]) - end - - it "returns an empty array" do - expect(fk).to be_empty - end - end - end - - context "when provided nils" do - - let(:fk) do - Array.__mongoize_fk__(association, [ nil, nil, nil ]) - end - - it "returns an empty array" do - expect(fk).to be_empty - end - end - end - end - describe "#__mongoize_time__" do let(:array) do diff --git a/spec/mongoid/extensions/object_spec.rb b/spec/mongoid/extensions/object_spec.rb index 2a71c1931d..9d5aae2d2c 100644 --- a/spec/mongoid/extensions/object_spec.rb +++ b/spec/mongoid/extensions/object_spec.rb @@ -23,97 +23,6 @@ end end - describe ".__mongoize_fk__" do - - context "when the related model uses object ids" do - - let(:association) do - Game.relations["person"] - end - - context "when provided an object id" do - - let(:object_id) do - BSON::ObjectId.new - end - - let(:fk) do - Object.__mongoize_fk__(association, object_id) - end - - it "returns the object id" do - expect(fk).to eq(object_id) - end - end - - context "when provided a string" do - - context "when the string is a legal object id" do - - let(:object_id) do - BSON::ObjectId.new - end - - let(:fk) do - Object.__mongoize_fk__(association, object_id.to_s) - end - - it "returns the object id" do - expect(fk).to eq(object_id) - end - end - - context "when the string is not a legal object id" do - - let(:string) do - "blah" - end - - let(:fk) do - Object.__mongoize_fk__(association, string) - end - - it "returns the string" do - expect(fk).to eq(string) - end - end - - context "when the string is blank" do - - let(:fk) do - Object.__mongoize_fk__(association, "") - end - - it "returns nil" do - expect(fk).to be_nil - end - end - end - - context "when provided nil" do - - let(:fk) do - Object.__mongoize_fk__(association, nil) - end - - it "returns nil" do - expect(fk).to be_nil - end - end - - context "when provided an empty array" do - - let(:fk) do - Object.__mongoize_fk__(association, []) - end - - it "returns an empty array" do - expect(fk).to eq([]) - end - end - end - end - describe "#__mongoize_time__" do it "returns self" do diff --git a/spec/mongoid/fields/foreign_key_spec.rb b/spec/mongoid/fields/foreign_key_spec.rb index 3aafef65a5..fedfef3eca 100644 --- a/spec/mongoid/fields/foreign_key_spec.rb +++ b/spec/mongoid/fields/foreign_key_spec.rb @@ -497,181 +497,343 @@ end end - describe "#mongoize" do + describe '#mongoize' do + let(:field) do + described_class.new( + :vals, + type: type, + default: [], + identity: true, + association: association, + overwrite: true + ) + end + let(:association) { Game.relations['person'] } + subject(:mongoized) { field.mongoize(object) } + + context 'type is Array' do + let(:type) { Array } + + context 'when the object is a BSON::ObjectId' do + let(:object) { BSON::ObjectId.new } + + it 'returns the object id as an array' do + expect(mongoized).to eq([object]) + end + end + + context 'when the object is an Array of BSON::ObjectId' do + let(:object) { [BSON::ObjectId.new] } + + it 'returns the object ids' do + expect(mongoized).to eq(object) + end + end + + context 'when the object is a String which is a legal object id' do + let(:object) { BSON::ObjectId.new.to_s } + + it 'returns the object id in an array' do + expect(mongoized).to eq([BSON::ObjectId.from_string(object)]) + end + end + + context 'when the object is a String which is not a legal object id' do + let(:object) { 'blah' } + + it 'returns the object id in an array' do + expect(mongoized).to eq(%w[blah]) + end + end + + context 'when the object is a blank String' do + let(:object) { '' } + + it 'returns an empty array' do + expect(mongoized).to eq([]) + end + end + + context 'when the object is nil' do + let(:object) { nil } + + it 'returns an empty array' do + expect(mongoized).to eq([]) + end + end + + context 'when the object is Array of Strings which are legal object ids' do + let(:object) { [BSON::ObjectId.new.to_s] } + + it 'returns the object id in an array' do + expect(mongoized).to eq([BSON::ObjectId.from_string(object.first)]) + end + end + + context 'when the object is Array of Strings which are not legal object ids' do + let(:object) { %w[blah] } + + it 'returns the Array' do + expect(mongoized).to eq(%w[blah]) + end + end + + context 'when the object is Array of Strings which are blank' do + let(:object) { ['', ''] } + + it 'returns an empty Array' do + expect(mongoized).to eq([]) + end + end + + context 'when the object is Array of nils' do + let(:object) { [nil, nil, nil] } + + it 'returns an empty Array' do + expect(mongoized).to eq([]) + end + end + + context 'when the object is an empty Array' do + let(:object) { [] } + + it 'returns an empty Array' do + expect(mongoized).to eq([]) + end - context "when the type is array" do + it 'returns the same instance' do + expect(mongoized).to equal(object) + end + end - context "when the array is object ids" do + context 'when the object is a Set' do + let(:object) { Set['blah'] } - let(:association) do - Game.relations["person"] + it 'returns the object id in an array' do + expect(mongoized).to eq(%w[blah]) end + end - let(:field) do - described_class.new( - :vals, - type: Array, - default: [], - identity: true, - association: association, + context 'when foreign key is a String' do + before do + Person.field(:_id, type: String, overwrite: true) + end + + after do + Person.field( + :_id, + type: BSON::ObjectId, + pre_processed: true, + default: ->{ BSON::ObjectId.new }, overwrite: true ) end - context "when provided nil" do + context 'when the object is a String' do + let(:object) { %w[1] } - it "returns an empty array" do - expect(field.mongoize(nil)).to be_empty + it 'returns String' do + expect(mongoized).to eq(object) end end - context "when provided an empty array" do + context 'when the object is a BSON::ObjectId' do + let(:object) { [BSON::ObjectId.new] } - let(:array) do - [] + it 'converts to String' do + expect(mongoized).to eq([object.first.to_s]) end + end - it "returns an empty array" do - expect(field.mongoize(array)).to eq(array) - end + context 'when the object is an Integer' do + let(:object) { [1] } - it "returns the same instance" do - expect(field.mongoize(array)).to equal(array) + it 'converts to String' do + expect(mongoized).to eq(%w[1]) end end + end - context "when using object ids" do + context 'when foreign key is an Integer' do + before do + Person.field(:_id, type: Integer, overwrite: true) + end - let(:object_id) do - BSON::ObjectId.new - end + after do + Person.field( + :_id, + type: BSON::ObjectId, + pre_processed: true, + default: ->{ BSON::ObjectId.new }, + overwrite: true + ) + end - it "performs conversion on the ids if strings" do - expect(field.mongoize([object_id.to_s])).to eq([object_id]) + context 'when the object is a String' do + let(:object) { %w[1] } + + it 'converts to Integer' do + expect(mongoized).to eq([1]) end end - context "when not using object ids" do + context 'when the object is an Integer' do + let(:object) { [1] } - let(:object_id) do - BSON::ObjectId.new + it 'returns Integer' do + expect(mongoized).to eq([1]) end + end + end + end - before do - Person.field( - :_id, - type: String, - pre_processed: true, - default: ->{ BSON::ObjectId.new.to_s }, - overwrite: true - ) - end + context 'type is Set' do + let(:type) { Set } - after do - Person.field( - :_id, - type: BSON::ObjectId, - pre_processed: true, - default: ->{ BSON::ObjectId.new }, - overwrite: true - ) - end + context 'when the object is an Array of BSON::ObjectId' do + let(:object) { [BSON::ObjectId.new] } - it "does not convert" do - expect(field.mongoize([object_id.to_s])).to eq([object_id.to_s]) - end + it 'returns the object ids' do + expect(mongoized).to eq(object) + end + end + + context 'when the object is a Set of BSON::ObjectId' do + let(:object) { Set[BSON::ObjectId.new] } + + it 'returns the object id in an array' do + expect(mongoized).to eq([object.first]) end end end - context "when the type is object" do + context 'type is Object' do + let(:type) { Object } - context "when the array is object ids" do + context 'when the object is a BSON::ObjectId' do + let(:object) { BSON::ObjectId.new } - let(:association) do - Game.relations['person'] + it 'returns the object id' do + expect(mongoized).to eq(object) end + end - let(:field) do - described_class.new( - :vals, - type: Object, - default: nil, - identity: true, - association: association, - overwrite: true - ) + context 'when the object is a String which is a legal object id' do + let(:object) { BSON::ObjectId.new.to_s } + + it 'returns the object id' do + expect(mongoized).to eq(BSON::ObjectId.from_string(object)) end + end - context "when using object ids" do + context 'when the object is a String which is not a legal object id' do + let(:object) { 'blah' } - let(:object_id) do - BSON::ObjectId.new - end + it 'returns the string' do + expect(mongoized).to eq('blah') + end + end - it "performs conversion on the ids if strings" do - expect(field.mongoize(object_id.to_s)).to eq(object_id) - end + context 'when the String is blank' do + let(:object) { '' } + + it 'returns nil' do + expect(mongoized).to be_nil + end + end + + context 'when the object is nil' do + let(:object) { '' } + + it 'returns nil' do + expect(mongoized).to be_nil end + end - context "when not using object ids" do + context 'when object is an empty Array' do + let(:object) { [] } - context "when using strings" do + it 'returns an empty array' do + expect(mongoized).to eq([]) + end + end - context "when provided a string" do + context 'when the object is a Set' do + let(:object) { Set['blah'] } - let(:object_id) do - BSON::ObjectId.new - end + it 'returns the set' do + expect(mongoized).to eq(Set['blah']) + end + end - before do - Person.field( - :_id, - type: String, - pre_processed: true, - default: ->{ BSON::ObjectId.new.to_s }, - overwrite: true - ) - end + context 'when foreign key is a String' do + before do + Person.field(:_id, type: String, overwrite: true) + end - after do - Person.field( - :_id, - type: BSON::ObjectId, - pre_processed: true, - default: ->{ BSON::ObjectId.new }, - overwrite: true - ) - end + after do + Person.field( + :_id, + type: BSON::ObjectId, + pre_processed: true, + default: ->{ BSON::ObjectId.new }, + overwrite: true + ) + end - it "does not convert" do - expect(field.mongoize(object_id.to_s)).to eq(object_id.to_s) - end - end + context 'when the object is a String' do + let(:object) { '1' } + + it 'returns String' do + expect(mongoized).to eq(object) + end + end + + context 'when the object is a BSON::ObjectId' do + let(:object) { BSON::ObjectId.new } + + it 'converts to String' do + expect(mongoized).to eq(object.to_s) end + end - context "when using integers" do + context 'when the object is an Integer' do + let(:object) { 1 } - context "when provided a string" do + it 'converts to String' do + expect(mongoized).to eq('1') + end + end + end - before do - Person.field(:_id, type: Integer, overwrite: true) - end + context 'when foreign key is an Integer' do + before do + Person.field(:_id, type: Integer, overwrite: true) + end - after do - Person.field( - :_id, - type: BSON::ObjectId, - pre_processed: true, - default: ->{ BSON::ObjectId.new }, - overwrite: true - ) - end + after do + Person.field( + :_id, + type: BSON::ObjectId, + pre_processed: true, + default: ->{ BSON::ObjectId.new }, + overwrite: true + ) + end - it "converts the string to an integer" do - expect(field.mongoize("1")).to eq(1) - end - end + context 'when the object is a String' do + let(:object) { '1' } + + it 'converts to Integer' do + expect(mongoized).to eq(1) + end + end + + context 'when the object is an Integer' do + let(:object) { 1 } + + it 'returns Integer' do + expect(mongoized).to eq(object) end end end