doctrine / mongodb-odm

The Official PHP MongoDB ORM/ODM
https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/latest/
MIT License
1.09k stars 504 forks source link

using flush after update()->multiple(true) does NOT show newly updated values #880

Closed nicktacular closed 9 years ago

nicktacular commented 10 years ago

Summary

When using multiple() in conjunction with update() followed by an immediate flush(), the documents affected by the update are not properly updated and still reflect the old values.

Details

I've written a script to help debug this problem and to show the proof of this issue at play. You'll notice that I'm using doctrine/mongodb-odm through composer and bootstrapping from within ZF2.

The script basically does these things:

If you go to the section with Should see all the category=1 objects as category=3, however you won't due to this issue: you'll notice that this occurs immediately after the multiple update followed by a flush. You'd expect that the categories have all been updated to 3 since that's the query that was just previously run. However, Doctrine doesn't pick up on the new data even though going directly to the DB shows that those values were updated.

This doesn't seem like expected behavior, especially after a call to flush.

<?php

/** @var Composer\Autoload\ClassLoader $autoloader */
$autoloader = include 'bootstrap.php';

use Zend\ServiceManager\ServiceManager;
use Zend\Mvc\Service;
use My\Document\Test;

//bootstrap configs for zend
$config = include APPLICATION_DIR . '/config/application.config.php';
$sm = new ServiceManager(new Service\ServiceManagerConfig($config));
$sm->setService('ApplicationConfig', $config);
$sm->get('ModuleManager')->loadModules();
$zendConfig = $sm->get('config');

/** @var Doctrine\ODM\MongoDB\DocumentManager $mgr */
$mgr = $sm->get('doctrine.documentmanager.odm_default');
$mgr->getConfiguration()->setDefaultDB($zendConfig['doctrine']['connection']['odm_default']['dbname']);

//this is the document name we shall use
$docName = 'My\Document\Test';

//will create 3 new documents
$docs = [];
$docs[] = new Test('hello', 1);
$docs[] = new Test('world', 1);
$docs[] = new Test('yes', 1);
$docs[] = new Test('other', 2);

//save it
foreach ($docs as $doc) {
    $mgr->persist($doc);
}

//make sure it's in the db
$mgr->flush();

//make sure that worked
$query = $mgr->createQueryBuilder($docName);
$cursor = $query->find()->getQuery()->execute();
echo "Should see 3 objects below ('hello', 'world', 'yes', and 'other'):\n";
foreach ($cursor as $obj) {
    echo "{$obj->getId()}: status={$obj->status}, category={$obj->category}\n";
}

//now let's try to update all the objects with multiple based on category == 1
$query = $mgr->createQueryBuilder($docName);
$query->update()
    ->multiple(true)
    ->field('category')->equals(1)
    ->field('category')->set(3)
    ->getQuery()
    ->execute();

//just to show that flush has no impact here
$mgr->flush();

//lets see what's in the DB now
$query = $mgr->createQueryBuilder($docName);
$cursor = $query->find()->getQuery()->execute();
echo "Should see all the category=1 objects as category=3, however you won't due to this issue:\n";
foreach ($cursor as $obj) {
    echo "{$obj->getId()}: status={$obj->status}, category={$obj->category}\n";
}

//am I insane? let's go straight to MongoDB to verify
$dbname = $zendConfig['doctrine']['connection']['odm_default']['dbname'];
$mongo = new MongoClient('mongodb://localhost');
$mongo->connect();
$collection = $mongo->selectDB($dbname)->selectCollection('test');
$cursor = $collection->find();
echo "What's actually in the DB:\n";
foreach ($cursor as $obj) {
    $id = (string)$obj['_id'];
    $cat = $obj['category'];
    $status = $obj['status'];
    echo "$id: status={$status}, category={$cat}\n";
}

//cleanup
$query = $mgr->createQueryBuilder($docName);
$query->remove()
    ->getQuery()
    ->execute();

//report back some version info
echo "PHP: " . phpversion() . PHP_EOL;
echo "PHP Mongo: " . phpversion('mongo') . PHP_EOL;

//get the version from composer
$dir = dirname(dirname(__DIR__));
exec("cd $dir && composer show -i", $output);
echo implode("\n", $output) . "\n";

So what is the output of this gem?

$php doctrineProofOfConcept.php 
Should see 3 objects below ('hello', 'world', 'yes', and 'other'):
5377618e5a86025f130041a7: status=hello, category=1
5377618e5a86025f130041a8: status=world, category=1
5377618e5a86025f130041a9: status=yes, category=1
5377618e5a86025f130041aa: status=other, category=2
Should see all the category=1 objects as category=3, however you won't due to this issue:
5377618e5a86025f130041a7: status=hello, category=1
5377618e5a86025f130041a8: status=world, category=1
5377618e5a86025f130041a9: status=yes, category=1
5377618e5a86025f130041aa: status=other, category=2
What's actually in the DB:
5377618e5a86025f130041a7: status=hello, category=3
5377618e5a86025f130041a8: status=world, category=3
5377618e5a86025f130041a9: status=yes, category=3
5377618e5a86025f130041aa: status=other, category=2
PHP: 5.5.10
PHP Mongo: 1.4.5
doctrine/annotations               v1.1.2             Docblock Annotations Parser
doctrine/cache                     v1.3.0             Caching library offering an object-oriented API for many cache backends
doctrine/collections               v1.2               Collections Abstraction library
doctrine/common                    v2.4.1             Common Library for Doctrine projects
doctrine/data-fixtures             v1.0.0             Data Fixtures for all Doctrine Object Managers
doctrine/doctrine-module           0.8.0              Zend Framework 2 Module that provides Doctrine basic functionality required for ORM and ODM ...
doctrine/doctrine-mongo-odm-module 0.1.0              Zend Framework 2 Module that provides Doctrine MongoDB ODM functionality
doctrine/inflector                 v1.0               Common String Manipulations with regard to casing and singular/plural rules.
doctrine/lexer                     v1.0               Base library for a lexer that can be used in Top-Down, Recursive Descent Parsers.
doctrine/mongodb                   1.1.5              Doctrine MongoDB Abstraction Layer
doctrine/mongodb-odm               dev-master e9ad2be Doctrine MongoDB Object Document Mapper
guzzle/guzzle                      v3.8.1             Guzzle is a PHP HTTP client library and framework for building RESTful web service clients
mockery/mockery                    dev-master 2739e30 Mockery is a simple yet flexible PHP mock object framework for use in unit testing with PHPU...
monolog/monolog                    1.6.0              Sends your logs to files, sockets, inboxes, databases and various web services
phlask/phlask                      dev-master 1e050ef PHP parallel task management.
phpunit/php-code-coverage          1.2.17             Library that provides collection, processing, and rendering functionality for PHP code cover...
phpunit/php-file-iterator          1.3.4              FilterIterator implementation that filters files based on a list of suffixes.
phpunit/php-text-template          1.2.0              Simple template engine.
phpunit/php-timer                  1.0.5              Utility class for timing
phpunit/php-token-stream           1.2.2              Wrapper around PHP's tokenizer extension.
phpunit/phpunit                    3.7.34             The PHP Unit Testing framework.
phpunit/phpunit-mock-objects       1.2.3              Mock Object library for PHPUnit
psr/log                            1.0.0              Common interface for logging libraries
symfony/console                    v2.4.0             Symfony Console Component
symfony/event-dispatcher           v2.4.2             Symfony EventDispatcher Component
symfony/yaml                       v2.4.2             Symfony Yaml Component
zendframework/zendframework        2.2.5              Zend Framework 2

And finally, for reference, here's my model:

<?php
namespace My\Document;

use \Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

/**
 * @ODM\Document(collection="test")
 */
class Test
{
    /** @ODM\Id */
    protected $id;

    /**
     * @ODM\String
     */
    public $status;

    /**
     * @ODM\Int
     */
    public $category;

    public function __construct($status, $category)
    {
        $this->status = $status;
        $this->category = $category;
    }
}
malarzm commented 9 years ago

I dig a bit and the second query you run returns "stale" results because documents from result set are already managed by UoW and since ->refresh() wasn't added to QueryBuilder it did not hydrated new results into document but just returned already managed ones.

malarzm commented 9 years ago

As a solution in ODM itself Query could set refresh hint by itself if it fetches all fields but I'm not sure if it's good idea in all cases

OddEssay commented 9 years ago

I just spent a couple of hours coming to the same conclusion as this post, so this is definitely not expected behaviour.

If this is seen as correct, even if non-intuitive, should a note be added to the documentation where ->multi(true) is discussed to describe why this happens?

alcaeus commented 9 years ago

@malarzm Setting the refresh hint by default is not a good idea, at least I don't think it is. Only findAndUpdate() supports the returnNew() functionality, which is used to return the new object instead of the old object (which is the default behavior IIRC). So setting the refresh hint by default would result in an extra trip to the database along with (possibly unnecessary) document hydration. I think it's better to let the developer decide when to refresh objects. I do think however that this should be properly documented in the database: Updating documents using the query builder does not refresh objects managed by the document manager.

malarzm commented 9 years ago

@alcaeus thanks for giving a con for this, won't consider it anymore :)

nicktacular commented 9 years ago

How would you set the refresh hint?

alcaeus commented 9 years ago

@nicktacular By calling refresh() in the query builder: $query = $mgr->createQueryBuilder($docName); $query->refresh(true)->findAndUpdate()->field('something')->set('somethingElse');

Note that it only works when the query is a) returning data and b) hydration is enabled (i.e. no hydrate(false)). An update() query does not return data, only findAndUpdate does (and only one).

malarzm commented 9 years ago

Here's test case I've pulled together back then:

<?php

namespace Doctrine\ODM\MongoDB\Tests\Functional\Ticket;

use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;

class GH880Test extends \Doctrine\ODM\MongoDB\Tests\BaseTest
{
    public function test880()
    {
        $docs = array();
        $docs[] = new GH880Document('hello', 1);
        $docs[] = new GH880Document('world', 1);
        foreach ($docs as $doc) {
            $this->dm->persist($doc);
        }
        $this->dm->flush();
        $query = $this->dm->createQueryBuilder(__NAMESPACE__ . '\GH880Document');
        $cursor = $query->find()->getQuery()->execute();
        foreach ($cursor as $c) {
            $this->assertEquals(1, $c->category);
        }
        $query = $this->dm->createQueryBuilder(__NAMESPACE__ . '\GH880Document');
        $query->update()
            ->multiple(true)
            ->field('category')->equals(1)
            ->field('category')->set(3)
            ->getQuery()
            ->execute();
        $query = $this->dm->createQueryBuilder(__NAMESPACE__ . '\GH880Document');
        // here ->refresh() was needed for the test to pass
        $cursor = $query->find()->refresh()->getQuery()->execute();
        foreach ($cursor as $c) {
            $this->assertEquals(3, $c->category);
        }
    }
}

/** @ODM\Document */
class GH880Document
{
    /** @ODM\Id */
    public $id;

    /** @ODM\String */
    public $status;

    /** @ODM\Int */
    public $category;

    public function __construct($status = "", $category = 0)
    {
        $this->status = $status;
        $this->category = $category;
    }
}
malarzm commented 9 years ago

Retriaging this for documentation as I couldn't find any mention of refresh() in Query Builder API and using that was fixing reported issue.

nicktacular commented 9 years ago

Ok, so the key is the refresh() call in $query->find()->refresh()->getQuery()->execute() to ensure that the data in that cursor is rehydrated from DB?

Thanks for your help.

malarzm commented 9 years ago

@nicktacular yes, calling refresh() ensures objects will be data fetched from database will be hydrated into objects

jmikola commented 9 years ago

Note to self: let's add @malarzm's test case as a documentation example.

jmikola commented 9 years ago

This has been document in #1203. Additionally, I added @malarzm's test case.