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@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev
