Boolquery issue

[kwisatz]

Hey, in Bool.php you strip QueryInterface Objects of their object and only leave their params array when only one object is passed to the must(), should() or should_not() methods. See https://github.com/polyfractal/sherlock/blob/master/src/Sherlock/components/queries/Bool.php#L71

Sorry, I don't know how to specify just a single commit but you should only need to apply the latest. Maybe you'll simply want to apply this manually.

Also, regarding the ServiceProvider for Silex, I did this to have sort of a singleton pattern and to be able to configure it centrally in a silex project. Maybe you feel this isn't a good idea to add random adapters to your source base, just let me know and I'll keep it inside the contrib folders of my silex projects.

[polyfractal]

Went ahead and just applied this manually since it was easier. Also added some docblocks and tests. Looks like Bool Filter suffered the same problem (among a few other bugs), so those have been fixed as well. Thanks for finding this!

Not sure I understand your question about Silex...I don't see any Silex code in this PR?

[kwisatz]

Well oops, I forgot to add it ;)

[kwisatz]

Added it in https://github.com/kwisatz/sherlock/commit/80988f2dcf12e3106fa410297efd5e5b56ed540a

[polyfractal]

Hmm...lemme think about this and look around at other projects with similar requirements. I definitely want to support adapters/bundles for popular frameworks (Silex, Symfony and Laravel at the forefront), but it may be wise to keep those in separate repos. I imagine the bundles will be considerably larger than the service provider for silex.

Hmmm....

[Mopster]

In my opinion each framework should have its own integration of Sherlock and have its own bundle/module/.... I did the same for our Symfony2 CMS and made it as generic as possible so other Symfony devs could maybe try it.

[polyfractal]

Yeah, that's what I'm leaning towards. If the module/bundle/adapter is provided in the Sherlock source, it officially "endorses" one particular method of integrating. While one dev may like a singleton usage of Sherlock in Silex, another may want something different...then do we include both versions in the source? Etc etc.

I think we should stick to keeping them in separate repos/projects.

[kwisatz]

Well ideally they would be included with the respective frameworks. But I'm not sure how fast (or even if at all) they'll accept any of these adapters/providers/bundles, etc...

[Mopster]

They don't need to be integrated in the respective framework. For Symfony you can just plug in loose bundles and choose the one which works best for you.

[polyfractal]

BTW, I found a bug in the new bugfix for the Bool stuff. The new Bool code failed on "array syntax":

$must = array($must1, $must2, $must3);
Sherlock::queryBuilder()->Bool->must($must)

Which made me remember why I had the arg checking code in there to begin with. I replaced it with code that works on all three construction methods:

//single
Sherlock::queryBuilder()->Bool->must($must1)

//inline
Sherlock::queryBuilder()->Bool->must($must1, $must2, $must3, ....)

//array
$must = array($must1, $must2, $must3);
Sherlock::queryBuilder()->Bool->must($must)

The fix is here: https://github.com/polyfractal/sherlock/commit/2fe5a6e8ebb3ac60089525d76e875f6cad4652f9

The key is checking count == 1 AND checking if the first parameter is an array.

Yay :)

(Of course, you could still argue that having three construction methods means I'm simply not choosing a proper method...which is probably bad. Oh well, a fight for another day)

[Mopster]

Funny, I encountered this bug 1-2 days ago, even had a possible fix for it. But removing the array and using multiple parameters solved it. Didn't give it much further thought :-)

[polyfractal]

Yet another motivation to put serious work into unit tests...this should have been caught before it was even released. =)