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

[Emmanuel Bernard]

I don't ahve much to say besides what has been said on this thread
already.
The "tolerated" feature works in less cases than I anticipated as
Guillaume demonstrated. The different with the new patch is that it
happens systematically (on criteria restrictions) and leads to a lot of
unitary entity loads. In other words, it wronger and slower :)

A proper implementation of this feature, would require a redesign of the
query SPIs I suppose.

Emmanuel

On Fri 2014-03-21 11:10, Guillaume Smet wrote:
> 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
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev