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(a)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