[MongoMapper-0.8.6] Querying subclass object is not working

[Rodrigo]

Hi all.

First, thanx all for library.


I am working with mongo_mapper at Abril ( second largest Brazilian
media conglomerate) in the CMS project. Today... I was trying resolve
a problem that I cannot deal:

I have an entity named Article. To handle deleted Articles, I created
another one called DeletedArticle...

class DeletedArticle < Article
set_collection_name 'deleted_articles'
end

Unfortunately, DeletedArticle is not working properly. Using
DeletedArticle.all results in [] but DeletedArticle.collection.find()
returns a cursor to the collection in the Database properly filled.

I have no idea how to make DeletedArticle queries works. I tried to
investigate how PluckyQuery works to create the mongo queries but I
didn't find anything wrong.
Could you guys help me?

Thank you very much.

[Brian Hempel]

It has to do with single collection inheritance... http://mongomapper.com/documentation/plugins/single-collection-inheritance.html

When you subclass, MongoMapper assumes you're putting two types of objects in the same collection and automatically adds a check for the :_type key to all the queries. See https://github.com/jnunemaker/mongomapper/blob/master/lib/mongo_mapper/plugins/sci.rb

You can see the extra criteria added by inspecting the query that MongoMapper uses to find documents:

Article.query.to_hash
=> {:transformer=>#<Proc:...>}

DeletedArticle.query.to_hash
=> {:transformer=>#<Proc:...>, :_type=>{"$in"=>["DeletedArticle"]}}

The easiest fix right now is to make sure that all your deleted articles have their :_type set to "DeletedArticle", or define all the articles keys in a module that you include and then you don't have to use inheritance like so:

module Article
extend ActiveSupport::Concern

included do
key :title
key :author_id
timestamps!
end

# etc...see MongoMapper's source for many good examples
end

class ActiveArticle
include MongoMapper::Document
include Article
end

class DeletedArticle
include MongoMapper::Document
include Article
end

More broadly, and this is a question for the MongoMapper community, how should MongoMapper behave when an inherited model defines a new collection name? Should it shut off SCI at that point?

Brian

> --
> 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

[Brandon Keepers]

On Thursday, March 8, 2012 at 4:16 PM, Brian Hempel wrote:

More broadly, and this is a question for the MongoMapper community, how should MongoMapper behave when an inherited model defines a new collection name? Should it shut off SCI at that point?

I would expect that.

=b 

[John Nunemaker]

Yeah. Shut off makes sense to me. Though I don't think I would inherit. I'd separate shared stuff into a module and include it in both. Then the issue goes away. 

[Jamie Orchard-Hays]

+1 on both.

[Jon Kern]

curious: how much difference is there with a DeletedArticle vs a regular
Article to warrant the big hammer known as inheritance?

jon

blog: http://technicaldebt.com
twitter: http://twitter.com/JonKernPA

Rodrigo said the following on 3/8/12 3:22 PM:

[Rodrigo]

Hi jon,

There's no difference at all except by the collection. In fact, I just
wanna to consult and save DeletedArticle in a separate collection
using virtually the same model. Should I? My approach using
inheritance occurred me just because I haven't an easier way to
associate a query with a model and a collection. Brian suggests an
extract superclass refactoring but can I just redefine collection that
a query is associated? When I send a message "all" to a model object
how does mongo mapper create a plucky query associated with a specific
collection?

[]'s
Rodrigo

[Brian Hempel]

> When I send a message "all" to a model object
> how does mongo mapper create a plucky query associated with a specific
> collection?

See the following code, the "all" method is delegated to the "query" method, the "query" method returns a plucky query that has extra behavior added.

https://github.com/jnunemaker/mongomapper/blob/master/lib/mongo_mapper/plugins/querying/plucky_methods.rb#L9-11
https://github.com/jnunemaker/mongomapper/blob/master/lib/mongo_mapper/plugins/querying.rb#L63-70
https://github.com/jnunemaker/mongomapper/blob/master/lib/mongo_mapper/plugins/persistence.rb#L42-50
If using SCI: https://github.com/jnunemaker/mongomapper/blob/master/lib/mongo_mapper/plugins/sci.rb#L23-27
https://github.com/jnunemaker/plucky/blob/master/lib/plucky/query.rb#L73-75
https://github.com/jnunemaker/plucky/blob/master/lib/plucky/query.rb#L54-57
https://github.com/mongodb/mongo-ruby-driver/blob/master/lib/mongo/collection.rb#L207-260

Let me know if you have any questions on the above.

Brian

[Jamie Orchard-Hays]

Rodrigo, why have a separate collection? Why not just mark articles as deleted and keep them in the same collection? You probably have a great reason for having two collections, but I have to ask. :-)

Jamie

[John Nunemaker]

I recommend a separate collection for deletion. If you put them in the same collection and mark them, you have to also include the deleted_at or whatever flag in your query and your indexes. Always felt like it ended up messy to me.

For gauges, we have users.archived and gauges.archived. Anytime something gets deleted it just gets stuffed in there. We don't index that collection or every query it. It is only there so we can restore if someone does something on accident.

[Rodrigo]

Exactly: =)

"If you put them in the same collection and mark them, you have to
also include the deleted_at or whatever flag in your query and your
indexes."

Any option sound me bad to lookup registers not deleted (within the
same collection). E.g. :

1) find({status: {$ne: deleted}}) # querying with $ne is not a good
idea. mongo will parse every document to know if status != deleted
2) find({special_flag: not_deleted}) #if "special_flag should be
included in every queries and indexes" as John said.

On Mar 9, 1:49 pm, John Nunemaker <nunema...@gmail.com> wrote:
> I recommend a separate collection for deletion. If you put them in the same collection and mark them, you have to also include the deleted_at or whatever flag in your query and your indexes. Always felt like it ended up messy to me.
>
> For gauges, we have users.archived and gauges.archived. Anytime something gets deleted it just gets stuffed in there. We don't index that collection or every query it. It is only there so we can restore if someone does something on accident.
>
>
>
>
>
>
>
> On Friday, March 9, 2012 at 10:45 AM, Jamie Orchard-Hays wrote:
> > Rodrigo, why have a separate collection? Why not just mark articles as deleted and keep them in the same collection? You probably have a great reason for having two collections, but I have to ask. :-)
>
> > Jamie
>
> > On Mar 9, 2012, at 9:45 AM, Rodrigo wrote:
>
> > > Hi jon,
>
> > > There's no difference at all except by the collection. In fact, I just
> > > wanna to consult and save DeletedArticle in a separate collection
> > > using virtually the same model. Should I? My approach using
> > > inheritance occurred me just because I haven't an easier way to
> > > associate a query with a model and a collection. Brian suggests an
> > > extract superclass refactoring but can I just redefine collection that
> > > a query is associated? When I send a message "all" to a model object
> > > how does mongo mapper create a plucky query associated with a specific
> > > collection?
>
> > > []'s
> > > Rodrigo
>

[Rodrigo]

Brian, Thank you very much!

I reread your post and find another way.

The answer lives on sci.rb. In my code I revert
single_collection_inherited to false...

class DeletedArticle < Article
set_collection_name 'materias_deletadas'
instance_variable_set("@single_collection_inherited", false)
end

... and everything works perfectly =)
Thx

[Jon Kern]

well, *that* approach i like... slough off the deleted to another place for safe-keeping, and keep them out of the way for current usage.

the inheritance approach, not so much...

jon

blog: http://technicaldebt.com
twitter: http://twitter.com/JonKernPA

John Nunemaker said the following on 3/9/12 11:49 AM:

[John Nunemaker]

Yeah, I don't even have a class for the deleted data. If I were to, it would not inherit. I'd probably just share the needed stuff between using a module.

module FooCore

  extend ActveSupport::Concern

  included do

    key :title, String

    # ….

  end

  module ClassMethods

    def some_shared_class_method

    end

  end

  def some_shared_method

  end

end

[Rodrigo]

Hi jon,

Just commenting: "the inheritance approach, not so much..."

I understand that idea about avoid inheritance. However, I'm just
following Liskov substitution principle:
DeletedArticle is a Article (even in another collection) for all
(other) purposes in the system.

Best regards,
Rodrigo

On Mar 9, 3:16 pm, Jon Kern <jonker...@gmail.com> wrote:
> well, *that* approach i like... slough off the deleted to another place for safe-keeping, and keep them out of the way for current usage.

> the inheritance approach, not so much...jon blog:http://technicaldebt.comtwitter:http://twitter.com/JonKernPA

[Jon Kern]

well, i tend to fight against inheritance unless the domain object
really screams it loud. In general, i prefer to extend capability by
plugging in functionality (association).

In this case, it seems to me more for a technical reason that you sought
inheritance. After all, a "state" that could reflect DraftArticle,
UnderReviewArticle, PublishedArticle, DeletedArticle would not, I hope,
engender a new subclass?

And, in a single-inheritance language, you can really get into trouble
rather quickly doing inheritance.

(Of course, when it comes to more technical classes that are servicing
UIs, network IO, or other non-business domain classes, I relax my
constraints.)

Probably a fault of cutting my teeth on O-O in the (!)halcyon days of
C++ multiple inheritance being thoroughly abused by folks.

I have learned that inheritance can unnecessarily complicate a
developer's life when it isn't the best approach for the situation at
hand...

Of course, as the saying goes, YMMV.

jon

blog: http://technicaldebt.com
twitter: http://twitter.com/JonKernPA

Rodrigo said the following on 3/9/12 3:30 PM: