MongoMapper::Plugins::Associations::Proxy and nil

[Jaron]

I just started using MongoMapper (and mongodb in general) a few days
ago and so far am really enjoying it!

Today I ran into an issue with how the Proxy class behaves when the
association is supposed to be nil. Effectively what you end up with
is a Proxy class pretending to be nil in some aspects, but not in
every way.

Say you have the following classes:

class User
include MongoMapper::Document
end

class Comment
include MongoMapper::Document

belongs_to :user
end

This looks like what I would expect:
>> user = Comment.new.user
=> nil

Then the differences start to appear:
>> user.class
=> nil
(instead of NilClass)


>> user.do_some_method_that_doesnt_exist(1, 2)
=> nil
method_missing as it stands returns nil if load_target doesn't return
an actual object, so anything you throw at a nil proxy will return
nil, rather than raising a NoMethodError

The real issue I ran into was how it behaved in if statements without
an explicit .nil? or .blank? check:

>> if (user)
>> true
>> else
>> false
>> end
=> true

I toyed with the idea of modifying Proxy to behave more like nil, but
as far as I know, no custom object can truly mimic nil (such as in the
"if (user)" example), so ultimately for my own sanity I modified get
proxy from

def get_proxy(association)
unless proxy = self.instance_variable_get(association.ivar)
proxy = association.proxy_class.new(self, association)
self.instance_variable_set(association.ivar, proxy)
end
proxy
end

To include an extra check before it returns:

def get_proxy(association)
unless proxy = self.instance_variable_get(association.ivar)
proxy = association.proxy_class.new(self, association)
self.instance_variable_set(association.ivar, proxy)
end
if (!proxy.nil?)
proxy
end
end

This way it returns a true nil that behaves how I would expect. So
far that seems to behave how I would expect in my code, but I'm not
sure if that negatively affects something else in the code, or if
there was a reason that an actual nil wasn't returned in cases where
the association doesn't exist.

[Brandon Keepers]

I've had issues around this issue too. I'd be curious to see how Active Record handles this differently.  It may be the same, but I haven't had issues with Active Record and nil proxies.

=b

On Sunday, January 2, 2011 at 3:29 PM, Jaron wrote:

I just started using MongoMapper (and mongodb in general) a few days
ago and so far am really enjoying it!

Today I ran into an issue with how the Proxy class behaves when the
association is supposed to be nil. Effectively what you end up with
is a Proxy class pretending to be nil in some aspects, but not in
every way.

Say you have the following classes:

class User
include MongoMapper::Document
end

class Comment

include MoongoMapper::Document

--
You received this message because you are subscribed to the Google
Groups "MongoMapper" group.
For more options, visit this group at
http://groups.google.com/group/mongomapper?hl=en?hl=en

[Jaron]

I've had my first chance to actually spend some more time looking into
this, and there are definitely some other issues (mainly with the =
and ? methods on the association calling replace and present? on
nil). When I have time over the next day or two I'll fork the project
and see if I can patch in something that still gives a nil object when
you request the association, but doesn't break anything.

On Jan 2, 8:48 pm, Brandon Keepers <bran...@opensoul.org> wrote:
> I've had issues around this issue too. I'd be curious to see how Active Record handles this differently. It may be the same, but I haven't had issues with Active Record and nil proxies.
>
> =b
>
> On Sunday, January 2, 2011 at 3:29 PM, Jaron wrote:
> > I just started using MongoMapper (and mongodb in general) a few days
> > ago and so far am really enjoying it!
>
> > Today I ran into an issue with how the Proxy class behaves when the
> > association is supposed to be nil. Effectively what you end up with
> > is a Proxy class pretending to be nil in some aspects, but not in
> > every way.
>
> > Say you have the following classes:
>
> > class User
> >  include MongoMapper::Document
> > end
>
> > class Comment

> >  include MongoMapper::Document

[Jaron]

Okay, I looked into this a little. For singular associations like
has_one and belongs_to, ActiveRecord does return a true nil object if
the association isn't found.

Implementing this for MongoMapper ( I moved the nil check into the
accessor method, since that's the only case where we would actually
care whether or not we get a true nil object (I believe).

This breaks a few tests, since you can no longer call .build
or .create once it's actually nil. My guess is this is the reason
build and create for singular associations in ActiveRecord are
prefixed calls (.build_foo, .create_foo), rather than calls on the
object.

I'm inclined to change it so it works the same in MongoMapper, so I
would call comment.build_user() instead of comment.user.build().

For reference, the singular association methods that ActiveRecord uses
are created in activerecord/lib/active_record/associations.rb

association_accessor_methods defines the accessors, and does the nil
check in the last line, similar to the change I'm making
association_constructor_method defines the prefixed build and create
methods, which I would mimic for MongoMapper

I'm at the very least going to make this change in a forked copy for
my use, but would like for this to make it into master if John is fine
with moving in this direction.

[Brian Hempel]

Hey,

Thanks for doing all this work Jaron, I'm all in favor of returning true nil's.

However, I very much like the consistent .build syntax in MM. How about returning a nil class that has build and other appropriate methods defined on it?

a = nil

def a.build
"build logic here"
end

a.build # => "build logic here"

if a
"this isn't reached"
end

Ruby magic! As "hackish" as it seems, I think it's less likely to cause bugs than a proxy pretending to be nil, and it still retains the nice syntax you get from the proxy.

Brian Hempel

[John Nunemaker]

I am fine with that change. Personally I do not really even use the proxy build/create methods.

[Jaron]

Brian,

The problem with defining methods on nil is that there is only one
actual instance of nil, so defining a method for one defines it for
all of them.

a = nil

def a.build
"build logic here"
end

a.build # => "build logic here"

b = nil

b.build # => "build logic here"

I also really like the .build syntax, but in choosing between having
an actual nil versus having .build and .create, I'm inclined to go
with nil, and move to build_ and create_

[Nathan Stults]

Definitely. It seems to be a pretty consistent confusion for people coming to MM. At least it comes up chronically on the mailing list. FWIW, the build syntax isn't really all that consistent across the MM API. It isn't defined for in array proxies for reasons due to internal implementation details, for example, so it isn't sacrilege to make it more consistent with AR and people's natural expectations about the meaning of nil. The only problem is backwards compatibility, it would definitely be a breaking change, so all due communication would be important.

[John Nunemaker]

This kind of communication is exactly what the mailing list and UPGRADES file is for:

https://github.com/jnunemaker/mongomapper/blob/master/UPGRADES

I am not worried about small breaking changes like this. Seems very manageable if it improves expectations.

[Scott Tamosunas]

Hey Guys,

This improvement would be fantastic as I've been bitten by this a
bunch of times now. Jaron, have you made any headway? I'd be happy to
help if this is still a WIP.

Thanks,

Scott

On Jan 3, 3:10 pm, John Nunemaker <nunema...@gmail.com> wrote:
> This kind of communication is exactly what the mailing list and UPGRADES
> file is for:
>
> https://github.com/jnunemaker/mongomapper/blob/master/UPGRADES
>
> I am not worried about small breaking changes like this. Seems
> very manageable if it improves expectations.
>
> On Mon, Jan 3, 2011 at 2:47 PM, Nathan Stults

> <Nathan_Stu...@hsihealth.com>wrote:

[Jaron]

Hi Scott,

I submitted a pull request in January that was pulled into the master
branch on github, but I'm not sure of the release cycle of the gem
itself.