allowed to pass filter objects to the configurator #434

Merged
merged 1 commit into from Nov 3, 2012

Conversation

Projects
None yet
6 participants
Contributor

bamarni commented Aug 31, 2012

If DDC-2004 gets approved.

This pull request passes (merged f961ebc1 into bc2476f).

This pull request passes (merged 7cd75611 into bc2476f).

@stof stof commented on an outdated diff Sep 3, 2012

tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php
+
+ public function testConfigureFilterObject()
+ {
+ $config = new \Doctrine\ORM\Configuration();
+
+ $validFilter = $this->getMockBuilder('\Doctrine\ORM\Query\Filter\SQLFilter')
+ ->disableOriginalConstructor()
+ ->getMock();
+ $config->addFilter("geolocation", $validFilter);
+ $this->assertInstanceOf("\Doctrine\ORM\Query\Filter\SQLFilter", $config->getFilter("geolocation"));
+
+ $invalidFilter = $this->getMock('\StdClass');
+ try {
+ $config->addFilter("geolocation", $invalidFilter);
+ $this->fail('An \InvalidArgumentException should have be thrown when adding an invalid filter object.');
+ } catch (\InvalidArgumentException $expected) {}
@stof

stof Sep 3, 2012

Member

you should be using PHPUnit's @expectedException instead

This pull request passes (merged a9b4deb into bc2476f).

@guilhermeblanco guilhermeblanco added a commit that referenced this pull request Nov 3, 2012

@guilhermeblanco guilhermeblanco Merge pull request #434 from bamarni/filter-objects
allowed to pass filter objects to the configurator
7d7287a

@guilhermeblanco guilhermeblanco merged commit 7d7287a into doctrine:master Nov 3, 2012

1 check passed

default The Travis build passed
Details

bamarni referenced this pull request in doctrine/DoctrineBundle Nov 10, 2012

Closed

Support services as filters #134

Member

asm89 commented Jan 26, 2013

ping @guilhermeblanco @bamarnini

Can you think with me? This PR should probably be reverted. I see people holding their own state in the SQL filters, but this can lead to all kinds of bugs. Check the base filter class, it has final constructor for a purpose. The SQL filters are taken into consideration when doing query cache calculations and more. If you change the state of the filter during a request in any way not supported by the base class, you will see unexpected behavior.

Contributor

bamarni commented Jan 26, 2013

@asm89 : Looking more deeply at the FilterCollection class, I can see it has been tought to have a state flag that will become dirty as soon as a new filter is added or a parameter is added / changed to a filter, so apparenly having custom objects can lead to issue if the state isn't updated properly when needed, I have no objection reverting this.

Member

asm89 commented Feb 9, 2013

I reverted the commit here: 9bf501d.

jmikola referenced this pull request in doctrine/mongodb-odm Feb 12, 2013

Closed

Added object filters for MongoDB ODM like ORM does. #488

@schmittjoh schmittjoh pushed a commit to schmittjoh/doctrine2 that referenced this pull request Nov 1, 2013

@asm89 asm89 Revert "allowed to pass filter objects to the configurator"
This reverts commit a9b4deb. See the
discussion on the original PR:

doctrine#434

Conflicts:
	lib/Doctrine/ORM/Configuration.php
9bf501d
Contributor

TomasVotruba commented Jul 7, 2015

I'd like to look deeply on this, since I want to use service/DI approach for filters.

Where exactly is this state flag? Do you have some failing test, or methods that should be tested first? Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment