Skip to content

Commit

Permalink
Merge branch 'master' into pr-5061
Browse files Browse the repository at this point in the history
* master:
  MONGOID-5173 Specs should use bang (!) methods (without describe/context change) (mongodb#5109)
  Fix doc syntax
  RUBY-2783 Require bson 4-stable for Mongoid as bson master is 5.0 and not compatible with driver (mongodb#5113)
  MONGOID-5208 fix error on reloading nil embedded document (mongodb#5116)
  MONGOID-5206 fix bug where embedded document is not re-embedded (mongodb#5115)
  MONGOID-5207 Add Ruby 3.1 to GH Actions (mongodb#5114)
  MONGOID-5207 Use YAML.safe_load in specs (mongodb#5112)
  MONGOID-5199 Reloadable#reload_embedded_document doesn't respect session (mongodb#5105)
  Fix MONGOID-5198 Calling children_changed? on a deep cyclical data structure will cause semi-infinite looping (mongodb#5104)
  • Loading branch information
p committed Jan 3, 2022
2 parents 9b195c1 + 32eb8b7 commit 1069e27
Show file tree
Hide file tree
Showing 19 changed files with 353 additions and 55 deletions.
22 changes: 21 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,26 @@ jobs:
fail-fast: false
matrix:
include:
- mongodb: '5.0'
ruby: ruby-3.1
topology: server
os: ubuntu-20.04
task: test
driver: current
rails:
i18n:
gemfile: Gemfile
experimental: false
- mongodb: '5.0'
ruby: ruby-3.1
topology: replica_set
os: ubuntu-20.04
task: test
driver: current
rails:
i18n:
gemfile: Gemfile
experimental: false
- mongodb: '5.0'
ruby: ruby-3.0
topology: server
Expand Down Expand Up @@ -109,7 +129,7 @@ jobs:
gemfile: Gemfile
experimental: false
- mongodb: '5.0'
ruby: ruby-3.0
ruby: ruby-3.1
topology: replica_set
os: ubuntu-20.04
task: test
Expand Down
1 change: 1 addition & 0 deletions docs/reference/associations.txt
Original file line number Diff line number Diff line change
Expand Up @@ -985,6 +985,7 @@ documents which satisfy the scope condition.. The scope may be either:
associated model.

.. code-block:: ruby

class Trainer
has_many :pets, scope: -> { where(species: 'dog') }
has_many :toys, scope: :rubber
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/driver_master.gemfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
source "https://rubygems.org"

gem 'bson', git: "https://github.com/mongodb/bson-ruby"
gem 'bson', git: "https://github.com/mongodb/bson-ruby", branch: '4-stable'
gem 'mongo', git: "https://github.com/mongodb/mongo-ruby-driver"

gem 'actionpack'
Expand Down
2 changes: 1 addition & 1 deletion lib/mongoid/association/embedded/embeds_many/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def build(attributes = {}, type = nil)
doc.apply_post_processed_defaults
yield(doc) if block_given?
doc.run_callbacks(:build) { doc }
_base._reset_memoized_children!
_base._reset_memoized_descendants!
doc
end

Expand Down
25 changes: 24 additions & 1 deletion lib/mongoid/association/embedded/embeds_one/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def initialize(base, target, association)
characterize_one(_target)
bind_one
characterize_one(_target)
_base._reset_memoized_children!
_base._reset_memoized_descendants!
_target.save if persistable?
end
end
Expand All @@ -53,6 +53,29 @@ def substitute(replacement)
# The associated object will be replaced by the below update if non-nil, so only
# run the callbacks and state-changing code by passing persist: false in that case.
_target.destroy(persist: !replacement) if persistable?

# A little explanation on why this is needed... Say we have three assignments:
#
# canvas.palette = palette
# canvas.palette = nil
# canvas.palette = palette
# Where canvas embeds_one palette.
#
# Previously, what was happening was, on the first assignment,
# palette was considered a "new record" (new_record?=true) and
# thus palette was being inserted into the database. However,
# on the third assignment, we're trying to reassign the palette,
# palette is no longer considered a new record, because it had
# been inserted previously. This is not exactly accurate,
# because the second assignment ultimately removed the palette
# from the database, so it needs to be reinserted. Since the
# palette's new_record is false, Mongoid ends up "updating" the
# document, which doesn't reinsert it into the database.
#
# The change I introduce here, respecifies palette as a "new
# record" when it gets removed from the database, so if it is
# reassigned, it will be reinserted into the database.
_target.new_record = true
end
unbind_one
return nil unless replacement
Expand Down
2 changes: 1 addition & 1 deletion lib/mongoid/atomic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def atomic_updates(_use_indexes = false)
process_flagged_destroys
mods = Modifiers.new
generate_atomic_updates(mods, self)
_children.each do |child|
_descendants.each do |child|
child.process_flagged_destroys
generate_atomic_updates(mods, child)
end
Expand Down
5 changes: 2 additions & 3 deletions lib/mongoid/changeable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ def changed?

# Have any children (embedded documents) of this document changed?
#
# @example Have any children changed?
# model.children_changed?
# @note This intentionally only considers children and not descendants.
#
# @return [ true, false ] If any children have changed.
def children_changed?
Expand Down Expand Up @@ -81,7 +80,7 @@ def move_changes
# @example Handle post persistence.
# document.post_persist
def post_persist
reset_persisted_children
reset_persisted_descendants
move_changes
end

Expand Down
2 changes: 1 addition & 1 deletion lib/mongoid/persistable/creatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def insert_as_root
# @return [ true ] true.
def post_process_insert
self.new_record = false
flag_children_persisted
flag_descendants_persisted
true
end

Expand Down
7 changes: 4 additions & 3 deletions lib/mongoid/reloadable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def reload
end

reloaded = _reload
if Mongoid.raise_not_found_error && reloaded.empty?
if Mongoid.raise_not_found_error && (reloaded.nil? || reloaded.empty?)
raise Errors::DocumentNotFound.new(self.class, _id, _id)
end
@attributes = reloaded
Expand Down Expand Up @@ -67,7 +67,7 @@ def reload_root_document
# @return [ Hash ] The reloaded attributes.
def reload_embedded_document
extract_embedded_attributes({}.merge(
collection(_root).find(_root.atomic_selector).read(mode: :primary).first
collection(_root).find(_root.atomic_selector, session: _session).read(mode: :primary).first
))
end

Expand All @@ -78,7 +78,8 @@ def reload_embedded_document
#
# @param [ Hash ] attributes The document in the db.
#
# @return [ Hash ] The document's extracted attributes.
# @return [ Hash | nil ] The document's extracted attributes or nil if the
# document doesn't exist.
def extract_embedded_attributes(attributes)
atomic_position.split(".").inject(attributes) do |attrs, part|
attrs = attrs[part =~ /\d/ ? part.to_i : part]
Expand Down
102 changes: 65 additions & 37 deletions lib/mongoid/traversable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,49 +115,82 @@ def self.get_discriminator_mapping(value)
end
end

# Get all child +Documents+ to this +Document+, going n levels deep if
# necessary. This is used when calling update persistence operations from
# Get all child +Documents+ to this +Document+
#
# @return [ Array<Document> ] All child documents in the hierarchy.
#
# @api private
def _children
@__children ||= collect_children
end

# Get all descendant +Documents+ of this +Document+ recursively.
# This is used when calling update persistence operations from
# the root document, where changes in the entire tree need to be
# determined. Note that persistence from the embedded documents will
# always be preferred, since they are optimized calls... This operation
# can get expensive in domains with large hierarchies.
#
# @example Get all the document's children.
# person._children
# @return [ Array<Document> ] All descendant documents in the hierarchy.
#
# @return [ Array<Document> ] All child documents in the hierarchy.
def _children
@__children ||= collect_children
# @api private
def _descendants
@__descendants ||= collect_descendants
end

# Collect all the children of this document.
#
# @example Collect all the children.
# document.collect_children
#
# @return [ Array<Document> ] The children.
#
# @api private
def collect_children
children = []
embedded_relations.each_pair do |name, association|
without_autobuild do
child = send(name)
Array.wrap(child).each do |doc|
children.push(doc)
children.concat(doc._children)
end if child
if child
children += Array.wrap(child)
end
end
end
children
end

# Marks all children as being persisted.
# Collect all the descendants of this document.
#
# @example Flag all the children.
# document.flag_children_persisted
# @return [ Array<Document> ] The descendants.
#
# @api private
def collect_descendants
children = []
to_expand = []
expanded = {}
embedded_relations.each_pair do |name, association|
without_autobuild do
child = send(name)
if child
to_expand += Array.wrap(child)
end
end
end
until to_expand.empty?
expanding = to_expand
to_expand = []
expanding.each do |child|
next if expanded[child]
expanded[child] = true
children << child
to_expand += child._children
end
end
children
end

# Marks all descendants as being persisted.
#
# @return [ Array<Document> ] The flagged children.
def flag_children_persisted
_children.each do |child|
# @return [ Array<Document> ] The flagged descendants.
def flag_descendants_persisted
_descendants.each do |child|
child.new_record = false
end
end
Expand Down Expand Up @@ -204,33 +237,28 @@ def remove_child(child)
end
end

# After children are persisted we can call this to move all their changes
# and flag them as persisted in one call.
# After descendants are persisted we can call this to move all their
# changes and flag them as persisted in one call.
#
# @example Reset the children.
# document.reset_persisted_children
#
# @return [ Array<Document> ] The children.
def reset_persisted_children
_children.each do |child|
# @return [ Array<Document> ] The descendants.
def reset_persisted_descendants
_descendants.each do |child|
child.move_changes
child.new_record = false
end
_reset_memoized_children!
_reset_memoized_descendants!
end

# Resets the memoized children on the object. Called internally when an
# Resets the memoized descendants on the object. Called internally when an
# embedded array changes size.
#
# @api semiprivate
#
# @example Reset the memoized children.
# document._reset_memoized_children!
#
# @return [ nil ] nil.
def _reset_memoized_children!
_parent._reset_memoized_children! if _parent
#
# @api private
def _reset_memoized_descendants!
_parent._reset_memoized_descendants! if _parent
@__children = nil
@__descendants = nil
end

# Return the root document in the object graph. If the current document
Expand Down
29 changes: 29 additions & 0 deletions spec/integration/associations/embedded_dirty_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# frozen_string_literal: true

require 'spec_helper'
require_relative '../../mongoid/association/embedded/embeds_many_models'
require_relative '../../mongoid/association/embedded/embeds_one_models'

describe 'embedded associations' do
describe 'dirty tracking' do
context 'when association is cyclic' do
before do
# create deeply nested record
a = EmmOuter.create(level: 0)
level = 1
iter = a.inners.create(level: level)
loop do
iter.friends.create(level: (level += 1))
iter = iter.friends[0]
break if level == 40
end
end

let(:subject) { EmmOuter.first }

it 'performs dirty tracking efficiently' do
subject.changed?.should be false
end
end
end
end
27 changes: 26 additions & 1 deletion spec/integration/matcher_operator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,32 @@ def mop_error?(spec, kind)
describe 'Matcher operators' do
Dir[File.join(File.dirname(__FILE__), 'matcher_operator_data', '*.yml')].sort.each do |path|
context File.basename(path) do
specs = YAML.load(File.read(path))
permitted_classes = [ BigDecimal,
Date,
Time,
Range,
Regexp,
Symbol,
BSON::Binary,
BSON::Code,
BSON::CodeWithScope,
BSON::DbPointer,
BSON::Decimal128,
BSON::Int32,
BSON::Int64,
BSON::MaxKey,
BSON::MinKey,
BSON::ObjectId,
BSON::Regexp::Raw,
BSON::Symbol::Raw,
BSON::Timestamp,
BSON::Undefined ]

specs = if RUBY_VERSION.start_with?("2.5")
YAML.safe_load(File.read(path), permitted_classes, [], true)
else
YAML.safe_load(File.read(path), permitted_classes: permitted_classes, aliases: true)
end

specs.each do |spec|
context spec['name'] do
Expand Down
13 changes: 13 additions & 0 deletions spec/mongoid/association/accessors_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,19 @@
end
end
end

context "when the association is set to nil first" do

let!(:name) do
person.build_name
end

it "returns true" do
person.name = nil
person.name = name
expect(person).to have_name
end
end
end

context "when the association is an embeds many" do
Expand Down
Loading

0 comments on commit 1069e27

Please sign in to comment.