Re: [hibernate-dev] HSEARCH-679 status

Hi Tom, thank you, I'm playing with it right now. As I mentioned on JIRA, the main reason for which I didn't fix this yet is that my proposed solution is incomplete, I'm working on a real solution and a good test would help a lot. I'm having some problems with your code: 1) The test is always green: even if I don't fix anything, the test is fine. 2) We're having all tests in the package org.hibernate.search.TEST, I see you had to use org.hibernate.search.event to be able to assign protected fields. I've had a similar issue other times, so I'll propose to change this convention, but for the short time you have many possible workarounds: - by registering instead an extensions of FullTextIndexEventListener - by reassigning the field via reflection (you make it public) - by using Byteman you can avoid all the mocking and have the framework "blow up" if it ever tries to perform some collections work 3) "Copyright (c) 2011 Attensa, Inc. All rights reserved." this is the most critical. I don't think I can add a class having such a clause to the Hibernate codebase, I'll need you to provide a "clean" copy. I'm going to try fixing the implementation tonight, do you think you'll be able to fix the tests for no later than tomorrow afternoon (release date!) ? Cheers, Sanne 2011/4/4 Tom Waterhouse <tomwaterhouse(a)gmail.com&gt;: > Sanne, > > I've added a comment to HSEARCH-679 that includes the instructions necessary > to add the attached unit test to the hibernate-search code base. Let me > know if you need anything more to check in the fix for HSEARCH-679. > > Tom > > On Tue, Mar 22, 2011 at 12:17 PM, Tom Waterhouse < tomwaterhouse(a)gmail.com&gt; > wrote: >> >> Sanne, that testing strategy doesn't work unfortunately. The collection >> is loaded by virtue of creating test data, so it is intialized. I don't see >> how HSearch can be tested at the entity level. >> >> Perhaps a smaller unit test of FullIndexTextEventListener is the way to go >> instead. I'm not sure what the Hibernate development testing methodology >> is, but my suggestion would be to create a unit test of >> processCollectionEvent with mocks as necessary. >> >> Tom >> >> On Tue, Mar 22, 2011 at 11:34 AM, Sanne Grinovero <sanne(a)hibernate.org&gt; >> wrote: >>> >>> Hi Tom, >>> sorry, copy paste error: I meant >>> org.hibernate.collection.PersistentCollection, all collections >>> retrieved from the database implement PersistentCollection, which >>> exposes the wasInitialized() method. >>> So just cast the collection to PersistentCollection and you should be >>> able to check if it was fully initialized. >>> >>> BTW I fixed the pending issue on github, as soon as you have a >>> testcase we can proceed with a pull request. >>> >>> Sanne >>> >>> 2011/3/22 Tom Waterhouse <tomwaterhouse(a)gmail.com&gt;: >>> > Sanne, >>> > >>> > I don't see the method you've identified in PersistentClass. The >>> > closest I >>> > could find to identifying if a property has been initialized is >>> > Hibernate.isPropertyInitialized(), but the method always returns true >>> > (object isn't a HibernateProxy, >>> > FieldInterceptionHelper.isInstrumented() >>> > returns false). >>> > >>> > Any other ideas on how I can tell if a lazy property has been >>> > initialized? >>> > >>> > Tom >>> > >>> > On Mon, Mar 21, 2011 at 4:40 PM, Sanne Grinovero < sanne(a)hibernate.org&gt; >>> > wrote: >>> >> >>> >> 2011/3/21 Tom Waterhouse <tomwaterhouse(a)gmail.com&gt;: >>> >> > Sanne, the unit test doesn't make any assertions, so I don't know >>> >> > how >>> >> > useful >>> >> > it would be as a test. It allows a user to view the >>> >> > Hibernate-generated >>> >> > SQL, illustrating the entities loaded. >>> >> >>> >> Yes I saw that :) so I was asking if you could make a unit test out of >>> >> it to contribute to the project. >>> >> The wasInitialized() method I mentioned in the previous email is a >>> >> suggestion for a possible implementation. >>> >> >>> >> Sanne >>> >> >>> >> > >>> >> > On Mon, Mar 21, 2011 at 2:46 PM, Sanne Grinovero >>> >> > <sanne(a)hibernate.org&gt; >>> >> > wrote: >>> >> >> >>> >> >> Hi Tom, >>> >> >> thanks for the reminder, you might have noticed we're very busy >>> >> >> lately, so thank you for your help proposal. >>> >> >> >>> >> >> I've just committed a solution draft on my HSEARCH-679 branch: >>> >> >> >>> >> >> https://github.com/Sanne/hibernate-search/tree/HSEARCH-679 >>> >> >> >>> >> >> It's a draft but seems to work; I still have to get someone into a >>> >> >> code review, maybe you could start having a look and try it out? >>> >> >> I've run your tests, very useful thank you. Do you think you could >>> >> >> re-shape that in a patch for Search, so we can include that as >>> >> >> functional test? >>> >> >> I would need you to remove the dependencies from Spring, and to >>> >> >> think >>> >> >> on a way to have the test fail if it loads the undesired collection >>> >> >> without looking into the logs. >>> >> >> >>> >> >> I think you might try casting it to a >>> >> >> org.hibernate.mapping.PersistentClass >>> >> >> and then see if it ".wasInitialized()". >>> >> >> >>> >> >> Regards, >>> >> >> Sanne >>> >> >> >>> >> >> >>> >> >> >>> >> >> >>> >> >> >>> >> >> 2011/3/21 Tom Waterhouse <tomwaterhouse(a)gmail.com&gt;: >>> >> >> > I see that HSEARCH-679 is scheduled to be released in Hibernate >>> >> >> > Search >>> >> >> > 3.4.0. I haven't seen any progress on the issue though. If it >>> >> >> > will >>> >> >> > help >>> >> >> > moving the issue along I can definitely test proposed solutions, >>> >> >> > we >>> >> >> > can >>> >> >> > reproduce the issue consistently. Unfortunately we've reproduced >>> >> >> > the >>> >> >> > issue >>> >> >> > during product demonstrations to potential customers, hence the >>> >> >> > urgency >>> >> >> > on >>> >> >> > our end to see the issue resolved. >>> >> >> > >>> >> >> > Cheers, >>> >> >> > >>> >> >> > Tom >>> >> >> > _______________________________________________ >>> >> >> > hibernate-dev mailing list >>> >> >> > hibernate-dev(a)lists.jboss.org >>> >> >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>> >> >> > >>> >> > >>> >> > >>> > >>> > >> > >