rodrigolive / mongoose Goto Github PK

MongoDB 0.42, Mongoose 0.08. Perl 5.12.3 on Mac.

Here's the output:

> perl -Ilib t/basic.t
ok 1 - created, id defined
ok 2 - delete ok
ok 3 - xref, id defined
Can't call method "name" on an undefined value at t/basic.t line 52.
# Tests were run but no plan was declared and done_testing() was not seen.

The failed line is https://github.com/rodrigolive/mongoose/blob/master/t/basic.t#L37

I briefly looked into this failure in this test case, it turns out $homer->_id is the same as $id, but in the mongodb, no documents with this ID exists. There is another "Homer Simpson" document with different _id value, and the spouse of Marge document is undefined instead of a reference to Homer.

my $articles = Authorship->find_one({ author=>$author->_id }); as stated in the Cookbook doesn't work. After some tracing it appears that Mongoose simply passes the params to MonoDB::Collection which doesn't do anything special (didn't follow it that far down).

Suggested fix:
Analyze params for refs == "MongoDB::OID" and convert the key to 'collection.$id' and value to MongoDB::OID->{value} .. yada yada

When you are retrieving objects from MongoDB, attributes are not checked, so you can get an object without the required attributes.

Here is a test that proves it:

use strict;
use warnings;

{
    package Doppelganger;
    use Moose;

    with 'Mongoose::Document';

    # Required attributes
    has 'something' => ( is => 'rw', isa => 'Str', required => 1 );
    has 'everything' => ( is => 'rw', isa => 'Str', required => 1 );
    has 'nothing' => ( is => 'rw', isa => 'Num', required => 1 );
}

package main;

use Moose;

use MongoDB;

use Mongoose;
Mongoose->db(  '_mongoose_test' ); 

my $client = MongoDB::MongoClient->new( host => 'localhost', port => 27017 );
my $database = $client->get_database( '_mongoose_test' );
my $collection = $database->get_collection( 'doppelganger' );
my $id = $collection->insert(
    {
        name => 'Peter Griffin',
        shirt_color => 'white',
        kids => [ 'Meg', 'Chris', 'Stewie' ]
    }
);

my $d = Doppelganger->find_one( $id ); # It should croak

use Data::Dumper;
print Dumper( $d );
print Dumper( $d->something() );
print $d->{name}, "\n";

Here I would expect to fail but instead it prints:

$VAR1 = bless( {
                 '_id' => bless( {
                                 'value' => '55d2178a03c30108210e7b21'
                               }, 'MongoDB::OID' ),
                 'name' => 'Peter Griffin',
                 'shirt_color' => 'white',
                 'kids' => [
                           'Meg',
                           'Chris',
                           'Stewie'
                         ]
               }, 'Doppelganger' );
$VAR1 = undef;
Peter Griffin

So now I have a Doppelganger object with some random attributes and without the ones that are required.

MongoDB::MongoClient should be used instead.

It's possible this change is a good time to re-think how per-class databases/connections should be handled

Boolean values are not saved properly in MongoDB.
When you read Boolean values from database they are correctly resolved as boolean, but when you save them, they are saved as Numbers

use strict;
use warnings;

{
    package Thingummy;
    use Moose;

    use boolean qw( true false );

    with 'Mongoose::Document';

    has 'is_sweet' => ( is => 'rw', isa => 'Bool', default => sub { true } );
    has 'has_round_edges' => ( is => 'rw', isa => 'Bool', default => sub { true } );
    has 'it_floats' => ( is => 'rw', isa => 'Bool', default => sub { false } );
}

package main;

use Moose;

use boolean qw( true false );

use MongoDB;

use Mongoose;
Mongoose->db(  '_mongoose_test' ); 

my $client = MongoDB::MongoClient->new( host => 'localhost', port => 27017 );
my $database = $client->get_database( '_mongoose_test' );
my $collection = $database->get_collection( 'thingummy' );
my $id = $collection->insert(
    {
        is_sweet => false,
        has_round_edges => true,
        it_floats => true,
    }
);

# The object in MongoDB is: 
# {
#     "_id" : ObjectId("55d21bb203c301082e64ece1"),
#     "has_round_edges" : true,
#     "it_floats" : true,
#     "is_sweet" : false
# }


my $t = Thingummy->find_one( $id ); 
$t->save();

# Now we messed up everything in our MongoDB:
# {
#     "_id" : ObjectId("55d21c2f03c301083f34e6f1"),
#     "has_round_edges" : NumberLong(1),
#     "it_floats" : NumberLong(1),
#     "is_sweet" : NumberLong(0)
# }

I'm running the tests with MongoDB 0.42 and getting this failure:
t/basic.t ...................... 1/? type (Person) unhandled at /opt/local/lib/perl5/site_perl/5.8.9/darwin-2level/MongoDB/Collection.pm line 301.
It looks like save() isn't doing "the right thing" with the circular reference in this test case:
Marge->spouse(Homer);
Homer->spouse(Marge);

When shortening, the shortened name is a glob alias, which causes class methods on the alias to get a class name that doesn't correspond to a metaclass registered with Class::MOP.

I think the fix is to do what aliased.pm does, which is to import a constant subroutine that returns the original package name, e.g. something like this:

*{ join q{::} => $callpack, $alias } = sub () { $package };

As per subject. Grep shows no declaration of a dependency for DateTime: https://grep.metacpan.org/search?q=DateTime&qd=Mongoose

Sample fail report for a missing DateTime: http://www.cpantesters.org/cpan/report/99081272

Probably a misconfiguration of dzil, but the generated Document.pm has an invalid version declaration in $DB:: package: https://metacpan.org/source/RODRIGO/Mongoose-0.25/lib/Mongoose/Document.pm#L34

and that confuses modules like Module::Metadata that tries to evaluate VERSION by scanning the code.

Because save does an upsert, there is no atomic way to tell if saving a document would overwrite an existing document. This means that if a PK is based on user-provided data (e.g. email address, driver's license), a new document can easily overwrite an existing one.

You can't really find() on the old PK and then only save if nothing is found, because you have a race between the find() and the save().

Instead, I think for the case where a new document with a PK is saved, an insert() should be used. If there is a uniqueness constraint on the PK index (which there really should be for any field being used as PK), the insert will fail, allowing the conflict to be detected.

I tried a trivial change to do so and the only thing that fails is the t/pk.t test that expects overwriting. I'm not sure why you have that expectation, so I'm opening an issue for discussion rather than just submitting a pull request with the change.

Hi,

I have a fairly simple model with just a dozen fields in it, inlcuding one array field and no relations. Now fetching a query from the database which returns 533 objects, it takes about 13 seconds. Now that seems really slow ... I then tried fetching without the ->all() then it takes about 2 seconds, but itterating over the items and pulling out the data I need takes about 11 more seconds.

I let it run with NYTProf and it seems most time is spent in validation.

I can provide you with the nytprof outputfile if you need.

Is this a known issue (I could not find anything about this).

Also when I fetch the data directly using the Mongo perl driver I can get thru the query in about 2 seconds.

The idea is to upgrade Mongoose to work on the upcoming MongoDBv1 driver (https://metacpan.org/release/MONGODB/MongoDB-v0.999.999.5-TRIAL) but keeping support for old versions of it by detecting version and loading the corresponding engine (driver upgrading will not be forced by mongoose until new engine "get stable").

the pod indicates that you need to specify the raw trait if you want to bypass the Mongoose serialization of DateTime and instead use the more storage-efficient MongoDB mechanism. from looking at my data and then reading the changelog, i believe this hasn't been true since version .08

There is a new version of MongoDB Driver that makes almost all tests to fail as it has moved to BSON::* on some internal objects and moved away from DateTime to BSON::Time.

There are lots of other warnings and deprecations to be re-worked like count().

When the belongs_to side of a has_one relationship is deleted the key remains on the parent (has_one side). This causes the method used to access the relationship to return 2 different results:

package Item;
has_one 'queue'

package Queue;
belongs_to 'item'

package main;
#...
my $item = Item->find_one($id);
$item->queue->delete;

$item = Item->find_one($id);
say "Find one:";
dd $item->queue;

my @items = Item->find->all;
say "Find all:";
for(@items) {
    dd $_->queue;
}

Outputs

Find one:
undef

Find all:
{}

Is there any reason why you store the key on the has_one side?

On a similar note, given the above relationship, when an Item is deleted the associated Queue remains.

I'd like to submit a patch for all of these things but I see you have multiple branches so I'm not sure what direction you're going. E.g, What's the status of Mongoose::Engine::Base::delete_cascade, etc..?

Just a thought that occurred to me, the find_last method could be the equivalent of the find_one with the addition that it does a desc sort on the special _id key. What do you think?

Hi @rodrigolive, and all Mongoose users,

Here's thought: It would be nice to make 'save' work nicely with Moose's before/after/around method modifiers.

Currently it is working, data gets saved, but there are unexpected outcomes. For one example, I've been using Mongoose for $client, and some of their data need to be exported to another DB on save. Since Mongoose::Document is a Moose class, I find it handy to say:

after 'save' => sub {
    export(....)
};

However, when the document has relations, save may be invoked multiple times during the collapsing process, so does the export subroutine, hence the problem.

I would like to propose to make 'save' routine friendly to Moose's method modifier because there can be many useful scenarios, the expected result, is to only call the modifier when user explicitly calls 'save'.

I've soved this problem by simply making 'save' a wrapper around '_save', which does the real work. I've made a commit with a small test that demonstrate the effect:

Commit: https://github.com/gugod/mongoose/commit/a4db6d7ea27ea3a1d83e9728eb74b6e15267e836

Without the patch, line 48 of after-save.t https://github.com/gugod/mongoose/blob/a4db6d7ea27ea3a1d83e9728eb74b6e15267e836/t/after-save.t#L48 would shows you that $cat_count is 11 instead of 1, which seems to me a bit too much, consider those invocations actually saves to db, but that's an issue the commit does not solve.

Does anyone also like the idea ?

Doc->new->find_one should not fail when searching on a required key IMHO

package Bar; 
use Moose; 
with 'Mongoose::Document'; 
has 'stuff' => (is => 'rw', isa => 'Str', required => 1 ); 
1; 

package Foo; 
use Moose; 
with 'Mongoose::Document'; 
has 'other_stuff' => (is => 'rw', isa => 'Str' ); 
has 'bars' => (is => 'rw', isa => 'Bar' ); 
1; 

This is saved as:

{ "bar" : { '$ref' : '...', '$id': '4e9ac....'  ....

The problem with this is I can't do:

db.foo_bar.find( { "bar" : { "$ref" : { "$oid" : "4e9ac ...

That is JSON error as '$' is an operation in the query syntax for MongoDB for example '$ne' ...

This is a big problem because it prevents find from working and we have to do a foreach and do a search in Perl which is slower. Can we not use '_' instead there?

I have many 'ro' accessors in the class, and on expansion (sub expand) an error is thrown because they are part of the @Later array and just hope the method exists to write.

If I check that the attribute has a writer (line 202, Mongoose/Engine/Base.pm) it does the right thing and keeps the value in $doc.

This sounds like a reasonable solution, and if so I can send you a patch but wanted to document here and get your take before just sending you patches.

Thanks,
-Jay

MongoDB doesn't allow '.' in keys within a hash. This needs to be detected and either get escaped somehow or else a sensible error get thrown.

The problem with escaping is that it makes searches more difficult, but it may be the least worst option.

I've got a Mongoose object with an attribute that's defined to be an Int. If set the attribute with a quoted string (either single or double) and save the object, what actually makes it into MongoDB is a string (which is problematic for querying further down the line) rather than Int as intended.

Here's a sample Mongoose object:

package MongooseTest;
use strict;
use warnings;
use Mongoose::Class;
with 'Mongoose::Document';
has_one 'sampleInt' => ( isa => 'Int' );
1;

and test that shows the problematic behavior:

use strict;
use warnings;
use Test::More;    
use MongooseTestDB;

my $testdb = MongooseTestDB->new()->init;

use MongooseTest;
MongooseTest->collection->drop; # clean slate for testing

my $testString = '1234';
my $c = MongooseTest->new( sampleInt => $testString );
$c->save;

# try to match as a string
my $r = MongooseTest->collection->find( {sampleInt=> '1234'} );
is($r->count,1,q|found by string.|);

# try to match as an int.
$r = MongooseTest->collection->find( {sampleInt=> 1234} );
is($r->count,1,q|found by int.|);

done_testing;

I understand that Mongoose rides on top of the MongoDB module, which says its up to you to cast data to proper types yourself if you care about how they end up in MongoDB. But with the meta layer provided by Moose, it seems that Mongoose can/should take care of the cast automatically.

I really like specifying everything about my collections in the Moose object definitions and mongoose seems to cover most of what I'm looking for (and thank you :). It looks like if I want non primary unique indexes or compound indexes I have to create those separately. Mongoose could maybe take care of that for you through traits. Do you have any plans to add such functionality?

If you use MooseX::Storage and Mongoose in the same app, you'll get a compile time

"Subroutine register_implementation redefined at" ...

And one or the other DoNotSerialize traits will get clobbered depending on which order your use statements appear. In addition, the DoNotSerialize trait is automatically applied to the MongoDB::OID class, and I actually want it to be serialized when I freeze() objects for xmission to the browser.

My current workaround is to rename the trait to DoNotMongoSerialize. Is that something you'd be willing to pull?

I seem to have to do some weird stuff to get Mongoose::Join to work.

That violates Class::MOP encapsulation.

Better, I think, would be to add a "class_config" hash-of-hash attribute to the Mongoose singleton and store it there.