[hibernate-dev] [Search] The case against searching with Criteria + restrictions

[Guillaume Smet]

Hi,

= Context =

So, my patch here [1] broke a test which checks that Criteria +
restrictions mostly work - even if it's documented as not supported
and not working.

"Mostly" as in "you can't get the result size but you might get the
results". See [2] for explanations.

I spent some time yesterday contemplating this issue and, while I'm
sorry for breaking this test, I still think we should apply my patch,
remove this test and make this case not supported for good.

= Why it mostly works =

In the original ObjectLoaderHelper implementation, we use
session.load: it doesn't force the proxy to be initialized. If a proxy
for an entity isn't initialized, it's filtered out from the results.

It's the job of the various implementations of ObjectsInitializer to
initialize the objects in the session so that they are later included
in the results.

In the case of Criteria + restrictions, the restrictions are applied
in the ObjectsInitializer so the entity which doesn't satisfy the
restrictions are not initialized in the ObjectsInitializer... and thus
not included in the results.

= Why my patch is breaking this behaviour consistently =

In my patch, I use Session.get which forces the initialization of the
proxy and I removed the filter removing the uninitialized proxies: it
became unnecessary as I was sure all proxies are now initialized.

This patch has been designed to solve HSEARCH-1448 and to simplify the
ObjectLoaderHelper code which was quite complicated.

Situation after my patch: all the results satisfying the full text
search are returned. The restrictions of the criteria are not taken
into account. In fact, it works as documented.

= Relying on the session state to filter out entities is wrong =

So the fact is that we basically rely on the session state to filter
out the results we don't want.

I had to check that my gut feeling was right so I checked out current
master, opened ResultSizeOnCriteriaTest and just added the following
lines before the search call:
//load in the session the object which shouldn't be returned just for fun
session.get( Tractor.class, 2 );

-> the object is returned and the test is broken too. This is expected
behaviour as this object has been initialized in the session and is
now considered as a valid candidate for the results.

= Conclusion =

I don't think we can have a really working criteria + restrictions
thing without refactoring a lot of things in search: the Initializer +
Loader concept can't work reliably in this case.

Therefore I think we should simply remove this test and clearly make
it fail as it can be a potential security flaw if we return entities
the user shouldn't see just because they were initialized in the
session for another purpose.

We might revisit it later but I really think it's a lot of work to get it right.

Thoughts?

References

[1] https://github.com/hibernate/hibernate-search/pull/581
[2] https://hibernate.atlassian.net/browse/HSEARCH-753