[infinispan-dev] new Infinispan Query API - ISPN-194

[Emmanuel Bernard]

On 4 avr. 2011, at 19:24, Sanne Grinovero wrote:

> thanks for the quick and comprehensive feedback.
> diving inline:
> 
> 2011/4/4 Emmanuel Bernard <emmanuel at hibernate.org>:
>> Nice job :)
>> 
>> - getBasicQuery methods are deprecated. That's from the previous design right? Was it a tech preview or was it named as full fledged API?
>> In the former case I'd get rid of them.
> 
> Yes that was a tech preview, I'll remove them, but maybe better keep
> them for a single release?
> There's a grails plugin which exposes these methods.

Discuss with the maintainer. There is probably a solution.

> 
>> - buildQueryBuilderForClass is that really needed?
>> 
>> qf.buildQueryBuilderForClass( Car.class ).get();
>> The alternative is
>> qf.getSearchFactory().buildQueryBuilder(Car.class).get();
> 
> Ah, you found a "security hole".
> If you do
> gf.getSearchFactory().buildQueryBuilder()
> I won't be able to intercept the type and reconfigure H.S. so it might
> fail if you're asking for a builder for a type which is yet unknown to
> the SearchFactory.
> I guess I should remove my method, and have the type discovery fixed later.

That's not going to be for Hibernate Search 3.4 but do you think it's a feature that should be part of Hibernate Search per se?

> 
>> - I'm not a big fan of the constructor approach to get QueryFactory for various reasons (including the fact that it forces a concrete type and no interfaces) but it seems to be an ISPN-wide design decision
>> That's definitely something that could be improved in a Seam Infinispan module.
> 
> I knew you where going to say that :) but yes this seems to be more
> Infinispan-style.
> I've considered something like a Search.getQueryFactory(Cache c) .. WDYT?
> This wouldn't return a "FullTextCache" but the manager, I see no
> reason to delegate a cache like we do with a o.h.Session.

For sure the reasons are weaker, but the idea of a FullTextSession is also to get all useful interaction methods from a single object.
I've never been extremely happy with the static helper method but it has the advantage of letting you return an interface. I guess the most important is to be consistent with the rest of ISPN's API.
See also the unwrap() approach I've just described on Hibernate-Dev.

> 
>> - I'd rename QueryFactory to something else as conceptually it's more an entry point to anything related to Hibernate Search, potentially indexing, stats etc: maybe SearchManager, SearchProvider, GridSearcher, TheGridReaper?
> right!
> which one? I like the first three.

I guess GridSearcher is fine but collect other opinions

> 
>> 
>> - is there an easy metaconfig to ask ISPN to store the HSearch indexes in ISPN :)
> No, but agree that would be awesome. would require changes though in
> all involved projects:
> 1) H.S. would be to be able to start on a DirectoryProvider instance we pass in
> 2) Infinispan Directory module (the submodule of H.S.) would need to
> be able to reconfigure an existing cachemanager configuration during
> the startup
> 3) Infinispan Core needs to be adapted for the additional configuration fields
> currently it's not that hard, just a single property:
> 
> <indexing enabled="true" indexLocalOnly="true">
>   <properties>
>      <property
> name="hibernate.search.default.directory_provider" value="infinispan" />
>      </properties>
> </indexing>
> 
> downside is that it will start a second cachemanager with a second
> JGroups channel, instead of reusing the same.

OK not for today, let's put that in the TODO truck