Permalink
Browse files

Proper separation of mongoization/evolution w/ fks:

- This is split up now, so that empty strings and nils aren't filtered
  out of criteria params on these fields.
- Fixes #2089.
  • Loading branch information...
1 parent 2187289 commit ba41bcf53b4f1909c2bc95900e3976521df64197 @durran durran committed Jun 13, 2012
View
@@ -529,6 +529,13 @@ For instructions on upgrading to newer versions, visit
### Major Changes (Backwards Incompatible)
+* Polymorphic relations can not have ids other than object ids. This is
+ because Mongoid cannot properly figure out in an optimized way what the
+ various classes on the other side of the relation store their ids as, as
+ they could potentially all be different.
+
+ This was not allowed before, but nowhere was it explicitly stated.
+
* \#2039 Validating presence of a relation now checks both the relation and
the foreign key value.
@@ -771,6 +778,9 @@ For instructions on upgrading to newer versions, visit
### Resolved Issues
+* \#2089 Allow proper separation of mongoization and evolving with respect to
+ foreign keys.
+
* \#2085 Allow demongoization of floats and ints to big decimals.
* \#2084 Don't cascade if metadata does not exist.
View
@@ -46,6 +46,7 @@
require "mongoid/copyable"
require "mongoid/criteria"
require "mongoid/dirty"
+require "mongoid/evolvable"
require "mongoid/factory"
require "mongoid/fields"
require "mongoid/finders"
@@ -21,6 +21,7 @@ module Components
include Mongoid::Atomic
include Mongoid::Dirty
include Mongoid::Attributes
+ include Mongoid::Evolvable
include Mongoid::Fields
include Mongoid::Hierarchy
include Mongoid::Indexes
View
@@ -0,0 +1,19 @@
+# encoding: utf-8
+module Mongoid
+
+ # Contains behaviour specific to evolving for origin queries.
+ module Evolvable
+
+ # Evolve the document into an object id.
+ #
+ # @example Evolve the document.
+ # document.__evolve_object_id__
+ #
+ # @return [ Object ] The document's id.
+ #
+ # @since 3.0.0
+ def __evolve_object_id__
+ id
+ end
+ end
+end
@@ -12,7 +12,20 @@ module Array
#
# @since 3.0.0
def __evolve_object_id__
- map!(&:__evolve_object_id__).compact!
+ map!(&:__evolve_object_id__)
+ self
+ end
+
+ # Mongoize the array into an array of object ids.
+ #
+ # @example Evolve the array to object ids.
+ # [ id ].__mongoize_object_id__
+ #
+ # @return [ Array<Moped::BSON::ObjectId> ] The converted array.
+ #
+ # @since 3.0.0
+ def __mongoize_object_id__
+ map!(&:__mongoize_object_id__).compact!
self
end
@@ -15,6 +15,18 @@ def __evolve_object_id__
update_values(&:__evolve_object_id__)
end
+ # Mongoizes each value in the hash to an object id if it is convertable.
+ #
+ # @example Convert the hash values.
+ # { field: id }.__mongoize_object_id__
+ #
+ # @return [ Hash ] The converted hash.
+ #
+ # @since 3.0.0
+ def __mongoize_object_id__
+ update_values(&:__mongoize_object_id__)
+ end
+
# Get the id attribute from this hash, whether it's prefixed with an
# underscore or is a symbol.
#
@@ -14,6 +14,7 @@ module Object
def __evolve_object_id__
self
end
+ alias :__mongoize_object_id__ :__evolve_object_id__
# Mongoize a plain object into a time.
#
@@ -14,6 +14,7 @@ module ObjectId
def __evolve_object_id__
self
end
+ alias :__mongoize_object_id__ :__evolve_object_id__
module ClassMethods
@@ -42,7 +43,7 @@ def evolve(object)
#
# @since 3.0.0
def mongoize(object)
- evolve(object)
+ object.__mongoize_object_id__
end
end
end
@@ -11,13 +11,23 @@ module String
# @example Evolve the string.
# "test".__evolve_object_id__
#
- # @return [ String, Moped::BSON::ObjectId, nil ] The evolved string.
+ # @return [ String, Moped::BSON::ObjectId ] The evolved string.
#
# @since 3.0.0
def __evolve_object_id__
- unless blank?
- BSON::ObjectId.legal?(self) ? BSON::ObjectId.from_string(self) : self
- end
+ convert_to_object_id
+ end
+
+ # Mongoize the string into an object id if possible.
+ #
+ # @example Evolve the string.
+ # "test".__mongoize_object_id__
+ #
+ # @return [ String, Moped::BSON::ObjectId, nil ] The mongoized string.
+ #
+ # @since 3.0.0
+ def __mongoize_object_id__
+ convert_to_object_id unless blank?
end
# Mongoize the string for storage.
@@ -116,6 +126,22 @@ def unconvertable_to_bson?
@unconvertable_to_bson ||= false
end
+ private
+
+ # If the string is a legal object id, convert it.
+ #
+ # @api private
+ #
+ # @example Convert to the object id.
+ # string.convert_to_object_id
+ #
+ # @return [ String, BSON::ObjectId ] The string or the id.
+ #
+ # @since 3.0.0
+ def convert_to_object_id
+ BSON::ObjectId.legal?(self) ? BSON::ObjectId.from_string(self) : self
+ end
+
module ClassMethods
# Convert the object from it's mongo friendly ruby type to this type.
@@ -62,9 +62,11 @@ def foreign_key?
#
# @since 3.0.0
def evolve(object)
- return object.id if object.is_a?(Document)
- evolved = mongoize(object)
- type.resizable? ? evolved.first : evolved
+ if object_id_field?
+ object.__evolve_object_id__
+ else
+ related_id_field.evolve(object)
+ end
end
# Mongoize the object into the Mongo friendly value.
@@ -81,7 +83,7 @@ def mongoize(object)
if type.resizable? || object_id_field?
type.__mongoize_fk__(constraint, object)
else
- metadata.klass.fields["_id"].mongoize(object)
+ related_id_field.mongoize(object)
end
end
@@ -115,6 +117,10 @@ def evaluate_default_proc(doc)
serialize_default(default_val[])
end
+ def related_id_field
+ @related_id_field ||= metadata.klass.fields["_id"]
+ end
+
# This is used when default values need to be serialized. Most of the
# time just return the object.
#
@@ -20,14 +20,14 @@
describe "#path" do
it "returns an empty string" do
- root.path.should eq("")
+ root.path.should be_empty
end
end
describe "#position" do
it "returns an empty string" do
- root.position.should eq("")
+ root.position.should be_empty
end
end
@@ -1711,20 +1711,85 @@
describe "\##{method}" do
- let!(:match) do
- Band.create(genres: [ "electro", "dub" ])
- end
+ context "when querying on a normal field" do
- let!(:non_match) do
- Band.create(genres: [ "house" ])
- end
+ let!(:match) do
+ Band.create(genres: [ "electro", "dub" ])
+ end
- let(:criteria) do
- Band.send(method, genres: [ "dub" ])
+ let!(:non_match) do
+ Band.create(genres: [ "house" ])
+ end
+
+ let(:criteria) do
+ Band.send(method, genres: [ "dub" ])
+ end
+
+ it "returns the matching documents" do
+ criteria.should eq([ match ])
+ end
end
- it "returns the matching documents" do
- criteria.should eq([ match ])
+ context "when querying on a foreign key" do
+
+ let(:id) do
+ BSON::ObjectId.new
+ end
+
+ let!(:match_one) do
+ Person.create(preference_ids: [ id ])
+ end
+
+ context "when providing valid ids" do
+
+ let(:criteria) do
+ Person.send(method, preference_ids: [ id ])
+ end
+
+ it "returns the matching documents" do
+ criteria.should eq([ match_one ])
+ end
+ end
+
+ context "when providing empty strings" do
+
+ let(:criteria) do
+ Person.send(method, preference_ids: [ id, "" ])
+ end
+
+ it "returns the matching documents" do
+ criteria.should eq([ match_one ])
+ end
+ end
+
+ context "when providing nils" do
+
+ context "when the relation is a many to many" do
+
+ let(:criteria) do
+ Person.send(method, preference_ids: [ id, nil ])
+ end
+
+ it "returns the matching documents" do
+ criteria.should eq([ match_one ])
+ end
+ end
+
+ context "when the relation is a one to one" do
+
+ let!(:game) do
+ Game.create
+ end
+
+ let(:criteria) do
+ Game.send(method, person_id: [ nil ])
+ end
+
+ it "returns the matching documents" do
+ criteria.should eq([ game ])
+ end
+ end
+ end
end
end
end
Oops, something went wrong.

0 comments on commit ba41bcf

Please sign in to comment.