diff --git a/lib/mongoid/contexts/mongo.rb b/lib/mongoid/contexts/mongo.rb index fadde243d6..bf0e0da9f1 100644 --- a/lib/mongoid/contexts/mongo.rb +++ b/lib/mongoid/contexts/mongo.rb @@ -6,6 +6,14 @@ class Mongo delegate :klass, :options, :field_list, :selector, :to => :criteria + def add_to_set(field, value) + klass.collection.update( + selector, + { "$addToSet" => { field => value } }, + :multi => true + ) + end + # Aggregate the context. This will take the internally built selector and options # and pass them on to the Ruby driver's +group()+ method on the collection. The # collection itself will be retrieved from the class provided, and once the @@ -235,6 +243,14 @@ def min(field) grouped(:min, field.to_s, Javascript.min) end + def pull(field, value) + klass.collection.update( + selector, + { "$pull" => { field => value } }, + :multi => true + ) + end + # Return the first result for the +Context+ and skip it # for successive calls. # @@ -267,6 +283,8 @@ def sum(field) # attributes provided in the hash. Can be expanded to later for more # robust functionality. # + # @todo Fix safe mode options. + # # @example Update all matching documents. # context.update_all(:title => "Sir") # diff --git a/lib/mongoid/criteria.rb b/lib/mongoid/criteria.rb index d68aeca143..3713edb4c8 100644 --- a/lib/mongoid/criteria.rb +++ b/lib/mongoid/criteria.rb @@ -42,6 +42,7 @@ class Criteria :field_list delegate \ + :add_to_set, :aggregate, :avg, :blank?, @@ -59,6 +60,7 @@ class Criteria :max, :min, :one, + :pull, :shift, :sum, :update, diff --git a/lib/mongoid/relations.rb b/lib/mongoid/relations.rb index 2d9d8ccb97..f63cd003d1 100644 --- a/lib/mongoid/relations.rb +++ b/lib/mongoid/relations.rb @@ -22,6 +22,7 @@ require "mongoid/relations/referenced/many_to_many" require "mongoid/relations/referenced/one" require "mongoid/relations/reflections" +require "mongoid/relations/synchronization" require "mongoid/relations/metadata" require "mongoid/relations/macros" @@ -40,6 +41,7 @@ module Relations include Macros include Polymorphic include Reflections + include Synchronization included do attr_accessor :metadata diff --git a/lib/mongoid/relations/bindings/referenced/many_to_many.rb b/lib/mongoid/relations/bindings/referenced/many_to_many.rb index 5d52778250..0dba63ce67 100644 --- a/lib/mongoid/relations/bindings/referenced/many_to_many.rb +++ b/lib/mongoid/relations/bindings/referenced/many_to_many.rb @@ -21,6 +21,8 @@ def bind_one(doc) binding do inverse_keys = doc.do_or_do_not(metadata.inverse_foreign_key) inverse_keys.push(base.id) if inverse_keys + base.synced[metadata.foreign_key] = true + doc.synced[metadata.inverse_foreign_key] = true end end end @@ -37,6 +39,8 @@ def unbind_one(doc) base.send(metadata.foreign_key).delete_one(doc.id) inverse_keys = doc.do_or_do_not(metadata.inverse_foreign_key) inverse_keys.delete_one(base.id) if inverse_keys + base.synced[metadata.foreign_key] = true + doc.synced[metadata.inverse_foreign_key] = true end end end diff --git a/lib/mongoid/relations/macros.rb b/lib/mongoid/relations/macros.rb index 8f4f6c1aa8..bc1744bd99 100644 --- a/lib/mongoid/relations/macros.rb +++ b/lib/mongoid/relations/macros.rb @@ -193,6 +193,7 @@ def references_and_referenced_in_many(name, options = {}, &block) reference(meta, Array) autosave(meta) validates_relation(meta) + synced(meta) end end alias :has_and_belongs_to_many :references_and_referenced_in_many diff --git a/lib/mongoid/relations/metadata.rb b/lib/mongoid/relations/metadata.rb index 1fdaf9a5f7..0f10e064c2 100644 --- a/lib/mongoid/relations/metadata.rb +++ b/lib/mongoid/relations/metadata.rb @@ -234,6 +234,10 @@ def foreign_key @foreign_key ||= determine_foreign_key end + def foreign_key_check + @foreign_key_check ||= "#{foreign_key}_changed?" + end + # Returns the name of the method used to set the foreign key on a # document. # diff --git a/lib/mongoid/relations/referenced/many_to_many.rb b/lib/mongoid/relations/referenced/many_to_many.rb index 2d21158d4b..5e13342721 100644 --- a/lib/mongoid/relations/referenced/many_to_many.rb +++ b/lib/mongoid/relations/referenced/many_to_many.rb @@ -37,7 +37,10 @@ def <<(*args) base.send(metadata.foreign_key).push(doc.id) end end - base.push_all(metadata.foreign_key, ids) if persistable? + if persistable? + base.push_all(metadata.foreign_key, ids) + base.synced[metadata.foreign_key] = false + end end end end @@ -81,6 +84,7 @@ def create(attributes = nil, type = nil, &block) super.tap do |doc| base.send(metadata.foreign_key).delete_one(doc.id) base.push(metadata.foreign_key, doc.id) + base.synced[metadata.foreign_key] = false end end @@ -103,6 +107,7 @@ def create!(attributes = nil, type = nil, &block) super.tap do |doc| base.send(metadata.foreign_key).delete_one(doc.id) base.push(metadata.foreign_key, doc.id) + base.synced[metadata.foreign_key] = false end end @@ -122,6 +127,7 @@ def delete(document) super.tap do |doc| if doc && persistable? base.pull(metadata.foreign_key, doc.id) + base.synced[metadata.foreign_key] = false end end end @@ -146,6 +152,7 @@ def initialize(base, target, metadata) bind_one(doc) if persistable? base.push(metadata.foreign_key, doc.id) + base.synced[metadata.foreign_key] = false doc.save else base.send(metadata.foreign_key).push(doc.id) diff --git a/lib/mongoid/relations/synchronization.rb b/lib/mongoid/relations/synchronization.rb new file mode 100644 index 0000000000..80abe3b35f --- /dev/null +++ b/lib/mongoid/relations/synchronization.rb @@ -0,0 +1,113 @@ +# encoding: utf-8 +module Mongoid # :nodoc: + module Relations #:nodoc: + + # This module handles the behaviour for synchronizing foreign keys between + # both sides of a many to many relations. + module Synchronization + extend ActiveSupport::Concern + + # Is the document able to be synced on the inverse side? This is only if + # the key has changed and the relation bindings have not been run. + # + # @example Are the foreign keys syncable? + # document.syncable?(metadata) + # + # @param [ Metadata ] metadata The relation metadata. + # + # @return [ true, false ] If we can sync. + # + # @since 2.1.0 + def syncable?(metadata) + !synced?(metadata.foreign_key) && send(metadata.foreign_key_check) + end + + # Get the synced foreign keys. + # + # @example Get the synced foreign keys. + # document.synced + # + # @return [ Hash ] The synced foreign keys. + # + # @since 2.1.0 + def synced + @synced ||= {} + end + + # Has the document been synced for the foreign key? + # + # @todo Change the sync to be key based. + # + # @example Has the document been synced? + # document.synced? + # + # @param [ String ] foreign_key The foreign key. + # + # @return [ true, false ] If we can sync. + # + # @since 2.1.0 + def synced?(foreign_key) + !!synced[foreign_key] + end + + # Update the inverse keys for the relation. + # + # @example Update the inverse keys + # document.update_inverse_keys(metadata) + # + # @param [ Metadata ] meta The document metadata. + # + # @return [ Object ] The updated values. + # + # @since 2.1.0 + def update_inverse_keys(meta) + old, new = changes[meta.foreign_key] + meta.criteria(new - old).add_to_set(meta.inverse_foreign_key, id) + meta.criteria(old - new).pull(meta.inverse_foreign_key, id) + end + + module ClassMethods #:nodoc: + + # Set up the syncing of many to many foreign keys. + # + # @example Set up the syncing. + # Person.synced(metadata) + # + # @param [ Metadata ] metadata The relation metadata. + # + # @since 2.1.0 + def synced(metadata) + synced_save(metadata) + end + + private + + # Set up the sync of inverse keys that needs to happen on a save. + # + # If the foreign key field has changed and the document is not + # synced, $addToSet the new ids, $pull the ones no longer in the + # array from the inverse side. + # + # @example Set up the save syncing. + # Person.synced_save(metadata) + # + # @param [ Metadata ] metadata The relation metadata. + # + # @return [ Class ] The class getting set up. + # + # @since 2.1.0 + def synced_save(metadata) + tap do + set_callback( + :save, + :after, + :if => lambda { |doc| doc.syncable?(metadata) } + ) do |doc| + doc.update_inverse_keys(metadata) + end + end + end + end + end + end +end diff --git a/spec/functional/mongoid/relations/referenced/many_to_many_spec.rb b/spec/functional/mongoid/relations/referenced/many_to_many_spec.rb index b431770683..376fb10ac9 100644 --- a/spec/functional/mongoid/relations/referenced/many_to_many_spec.rb +++ b/spec/functional/mongoid/relations/referenced/many_to_many_spec.rb @@ -788,12 +788,11 @@ context "when the documents are part of the relation" do before do - Preference.create(:person_ids => person.id) + Preference.create(:person_ids => [ person.id ]) end - pending "returns the count from the db" do - # @todo: Durran this gets fixed with m-t-m key fix. - person.preferences.count.should == 1 + it "returns the count from the db" do + person.reload.preferences.count.should == 1 end end diff --git a/spec/functional/mongoid/relations/synchronization_spec.rb b/spec/functional/mongoid/relations/synchronization_spec.rb new file mode 100644 index 0000000000..27155fda06 --- /dev/null +++ b/spec/functional/mongoid/relations/synchronization_spec.rb @@ -0,0 +1,310 @@ +require "spec_helper" + +describe Mongoid::Relations::Synchronization do + + before do + [ Person, Preference ].each(&:delete_all) + end + + context "when first setting by the relation itself" do + + let!(:person) do + Person.create(:ssn => "342-12-2222") + end + + let!(:one) do + Preference.create(:name => "one") + end + + before do + person.preferences << one + end + + it "sets the inverse foreign key" do + one.person_ids.should eq([ person.id ]) + end + + it "resets the synced flag" do + person.synced["preference_ids"].should be_false + end + + context "when subsequently setting with keys" do + + let!(:two) do + Preference.create(:name => "two") + end + + before do + person.preference_ids << two.id + person.save + end + + it "sets the inverse foreign key" do + two.reload.person_ids.should eq([ person.id ]) + end + end + end + + context "when setting new ids" do + + let!(:person) do + Person.create(:ssn => "342-12-2222") + end + + let!(:one) do + Preference.create(:name => "one") + end + + let!(:two) do + Preference.create(:name => "two") + end + + before do + person.preference_ids = [ one.id, two.id ] + end + + it "sets the foreign_key" do + person.preference_ids.should eq([ one.id, two.id ]) + end + + it "does not set the first inverse key" do + one.reload.person_ids.should be_empty + end + + it "does not set the second inverse key" do + two.reload.person_ids.should be_empty + end + + context "when saving the base" do + + context "when validation passes" do + + before do + person.save + end + + it "persists the foreign_key" do + person.reload.preference_ids.should eq([ one.id, two.id ]) + end + + it "sets the first inverse key" do + one.reload.person_ids.should eq([ person.id ]) + end + + it "sets the second inverse key" do + two.reload.person_ids.should eq([ person.id ]) + end + end + end + end + + context "when replacing ids" do + + let!(:one) do + Preference.create(:name => "one") + end + + let!(:two) do + Preference.create(:name => "two") + end + + let!(:person) do + Person.create( + :ssn => "342-12-2222", + :preference_ids => [ one.id, two.id ] + ) + end + + let!(:three) do + Preference.create(:name => "three") + end + + before do + person.preference_ids = [ three.id ] + end + + it "sets the foreign_key" do + person.preference_ids.should eq([ three.id ]) + end + + it "does not remove the first inverse key" do + one.reload.person_ids.should eq([ person.id ]) + end + + it "does not remove the second inverse key" do + two.reload.person_ids.should eq([ person.id ]) + end + + it "does not set the third inverse key" do + three.reload.person_ids.should be_empty + end + + context "when saving the base" do + + context "when validation passes" do + + before do + person.save + end + + it "persists the foreign_key" do + person.reload.preference_ids.should eq([ three.id ]) + end + + it "removes the first inverse key" do + one.reload.person_ids.should be_empty + end + + it "removes the second inverse key" do + two.reload.person_ids.should be_empty + end + + it "sets the third inverse key" do + three.reload.person_ids.should eq([ person.id ]) + end + end + end + end + + context "when setting ids to empty" do + + let!(:one) do + Preference.create(:name => "one") + end + + let!(:two) do + Preference.create(:name => "two") + end + + let!(:person) do + Person.create( + :ssn => "342-12-2222", + :preference_ids => [ one.id, two.id ] + ) + end + + before do + person.preference_ids = [] + end + + it "sets the foreign_key" do + person.preference_ids.should be_empty + end + + it "does not remove the first inverse key" do + one.reload.person_ids.should eq([ person.id ]) + end + + it "does not remove the second inverse key" do + two.reload.person_ids.should eq([ person.id ]) + end + + context "when saving the base" do + + context "when validation passes" do + + before do + person.save + end + + it "persists the foreign_key" do + person.reload.preference_ids.should be_empty + end + + it "removes the first inverse key" do + one.reload.person_ids.should be_empty + end + + it "removes the second inverse key" do + two.reload.person_ids.should be_empty + end + end + end + end + + context "when setting ids to nil" do + + let!(:one) do + Preference.create(:name => "one") + end + + let!(:two) do + Preference.create(:name => "two") + end + + let!(:person) do + Person.create( + :ssn => "342-12-2222", + :preference_ids => [ one.id, two.id ] + ) + end + + before do + person.preference_ids = nil + end + + it "sets the foreign_key" do + person.preference_ids.should be_empty + end + + it "does not remove the first inverse key" do + one.reload.person_ids.should eq([ person.id ]) + end + + it "does not remove the second inverse key" do + two.reload.person_ids.should eq([ person.id ]) + end + + context "when saving the base" do + + context "when validation passes" do + + before do + person.save + end + + it "persists the foreign_key" do + person.reload.preference_ids.should be_empty + end + + it "removes the first inverse key" do + one.reload.person_ids.should be_empty + end + + it "removes the second inverse key" do + two.reload.person_ids.should be_empty + end + end + end + end + + context "when destroying the document" do + + let!(:one) do + Preference.create(:name => "one") + end + + let!(:two) do + Preference.create(:name => "two") + end + + let!(:person) do + Person.create( + :ssn => "342-12-2222", + :preference_ids => [ one.id, two.id ] + ) + end + + before do + person.destroy + end + + it "removes the first inverse key" do + one.reload.person_ids.should be_empty + end + + it "removes the second inverse key" do + two.reload.person_ids.should be_empty + end + end +end diff --git a/spec/unit/mongoid/relations/bindings/referenced/many_to_many_spec.rb b/spec/unit/mongoid/relations/bindings/referenced/many_to_many_spec.rb index 81a5f6ca93..e87844f79a 100644 --- a/spec/unit/mongoid/relations/bindings/referenced/many_to_many_spec.rb +++ b/spec/unit/mongoid/relations/bindings/referenced/many_to_many_spec.rb @@ -41,6 +41,14 @@ it "passes the binding options through to the inverse" do person.expects(:save).never end + + it "syncs the base" do + person.should be_synced("preference_ids") + end + + it "syncs the inverse" do + preference_two.should be_synced("person_ids") + end end context "when ensuring minimal saves" do @@ -88,6 +96,14 @@ it "removed the foreign keys" do preference.person_ids.should be_empty end + + it "syncs the base" do + person.should be_synced("preference_ids") + end + + it "syncs the inverse" do + preference.should be_synced("person_ids") + end end context "when preventing multiple db hits" do