Hi Ravi, Sorry I missed this thread until now. I've been snowed under at work. Do you have a patch file that you'd like me to look at?
-mike On Tue, Dec 15, 2009 at 4:18 PM, Ravi P Palacherla < [email protected]> wrote: > > Hi Mike, > > I moved the code that needs to protected into a seperate method and made > the > method's modifier as protected. This way I was able to create a test case > by subclassing DataCacheStoreManager and mocking OpenJPAStateManager. > > Regards, > Ravi. > > > Michael Dick wrote: > > > > You have to protect your code from someone like me :-). > > > > Maybe adding a testcase that verifies that verifies that the list accepts > > Objects (not OpenJPAIDs) would work. I'm thinking something very hacky > > (maybe even jMock) .That way even if I ignored / don't notice your > > comments > > I'd get another warning. Not sure how doable this is. I'm just thinking > > out > > loud. > > > > Best Regards, > > > > -mike > > > > On Wed, Dec 2, 2009 at 10:47 AM, Ravi Palacherla > > <[email protected] > >> wrote: > > > >> Thanks for your comments Mike. > >> >> It would really help me out later if we have comments indicating why > >> this list isn't genericized > >> Sorry, I did not added any comments to my patch. > >> Will do so. > >> > >> Regards, > >> Ravi. > >> > >> -----Original Message----- > >> From: Michael Dick [mailto:[email protected]] > >> Sent: Wednesday, December 02, 2009 8:48 AM > >> To: [email protected] > >> Subject: Re: Need process help for a code change that does not have an > >> openJPA testcase. > >> > >> I was thinking of adding a new interface that OpenJPAId would implement > >> and > >> then the Kodo wrapper (which appears to only exist in my mind) could > also > >> implement that. Since there is no Kodo equivalent of OpenJPAId or > >> ObjectId > >> it would be a major change for Kodo and I'm not terribly inclined to do > >> that > >> (at least not now). > >> > >> My real motivation was so that we can take advantage of Generics in our > >> code, but it's not worth the churn for Kodo just to beautify the code. > >> > >> It would really help me out later if we have comments indicating why > this > >> list isn't genericized [sic?] so that someone (me) doesn't go back and > >> make > >> it specific to OpenJPAID.. If this is already in your patch in the JIRA > I > >> appologize. I haven't caught up on the JIRA and commit emails today. > >> > >> -mike > >> > >> > >> On Tue, Dec 1, 2009 at 7:46 PM, Ravi P Palacherla < > >> [email protected]> wrote: > >> > >> > > >> > Mike, > >> > > >> > Also regarding > >> > > >> > "In trunk it might be better to define an interface that kodo can > >> extend > >> > rather than just removing the cast." > >> > > >> > In openJPA; OpenJPAId is an abstract class and ObjectId extends > >> OpenJPAId. > >> > The casting expects the object returned to be castable to OpenJPAId. > >> > So I think providing an interface may not be possible here. > >> > Please correct me if I am wrong. > >> > > >> > I opened a JIRA for this issue ( with removing cast) OPENJPA-1407. > >> > > >> > Regards, > >> > Ravi. > >> > > >> > > >> > > >> > Michael Dick wrote: > >> > > > >> > > Just a couple of thoughts.. > >> > > > >> > > Presumably Kodo uses a product derivation that causes a different > >> > ObjectId > >> > > . > >> > > If that's the case we probably could introduce a test derivation > that > >> > does > >> > > something similar and verifies the fix. > >> > > > >> > > That said, removing the cast shouldn't be impossible to do. In trunk > >> it > >> > > might be better to define an interface that kodo can extend rather > >> than > >> > > just > >> > > removing the cast. In the service releases I wouldn't want to make > >> that > >> > > kind > >> > > of change though. > >> > > > >> > > -mike > >> > > > >> > > On Tue, Dec 1, 2009 at 9:06 AM, Donald Woods <[email protected]> > >> wrote: > >> > > > >> > >> Yep, go ahead and open a JIRA and include the details from your > >> email, > >> > >> along with which OpenJPA releases this needs to be addressed in. > >> > >> > >> > >> > >> > >> -Donald > >> > >> > >> > >> > >> > >> > >> > >> Ravi P Palacherla wrote: > >> > >> > >> > >>> Hi All, > >> > >>> > >> > >>> As part of OPENJPA-263 the following code is added to > >> > >>> org/apache/openjpa/datacache/DataCacheStoreManager.java > >> > >>> > >> > >>> oidList.add((OpenJPAId) sm.getObjectId()); > >> > >>>>> > >> > >>>> > >> > >>> I think there is no need to perform casting in the above source. > >> > >>> Due to the above casting there is no issue in openJPA testcases > but > >> the > >> > >>> issue appears on products like Kodo which uses openJPA source > >> > >>> internally. > >> > >>> > >> > >>> Can I ask for removal of this casting via JIRA without an openJPA > >> test > >> > >>> case > >> > >>> ? > >> > >>> I think it is not possible to provide an openJPA testcase because > >> > >>> sm.getObjectId() will always return OpenJPAId in case of openJPA. > >> > >>> > >> > >>> Regards, > >> > >>> Ravi. > >> > >>> > >> > >>> To be more precise; the following exception stack trace is seen in > >> kodo > >> > >>> : > >> > >>> > >> > >>> [java] Caused by: java.lang.ClassCastException: > >> > >>> com.sample.TestTableId > >> > >>> [java] at > >> > >>> > >> > >>> > >> > > >> > org.apache.openjpa.datacache.DataCacheStoreManager.loadAll(DataCacheStoreManager.java:461) > >> > >>> [java] at > >> > >>> > >> > >>> > >> > > >> > org.apache.openjpa.kernel.DelegatingStoreManager.loadAll(DelegatingStoreManager.java:121) > >> > >>> [java] at > >> > >>> org.apache.openjpa.kernel.BrokerImpl.findAll(BrokerImpl.java:984) > >> > >>> [java] at > >> > >>> org.apache.openjpa.kernel.BrokerImpl.findAll(BrokerImpl.java:1027) > >> > >>> [java] at > >> > >>> org.apache.openjpa.kernel.BrokerImpl.findAll(BrokerImpl.java:913) > >> > >>> [java] at > >> > >>> > >> > >>> > >> > > >> > org.apache.openjpa.kernel.AbstractPCData.toRelationFields(AbstractPCData.java:217) > >> > >>> [java] at > >> > >>> > >> > >>> > >> > > >> > org.apache.openjpa.kernel.AbstractPCData.toNestedFields(AbstractPCData.java:184) > >> > >>> [java] at > >> > >>> > >> > > >> org.apache.openjpa.kernel.AbstractPCData.toField(AbstractPCData.java:78) > >> > >>> [java] at > >> > >>> > org.apache.openjpa.kernel.PCDataImpl.loadField(PCDataImpl.java:197) > >> > >>> [java] at > >> > >>> org.apache.openjpa.kernel.PCDataImpl.load(PCDataImpl.java:147) > >> > >>> [java] at > >> > >>> > >> > >>> > >> > > >> > org.apache.openjpa.datacache.DataCacheStoreManager.initialize(DataCacheStoreManager.java:343) > >> > >>> [java] at > >> > >>> > >> > >>> > >> > > >> > org.apache.openjpa.kernel.DelegatingStoreManager.initialize(DelegatingStoreManager.java:111) > >> > >>> [java] at > >> > >>> > >> > >>> > >> > > >> > org.apache.openjpa.kernel.ROPStoreManager.initialize(ROPStoreManager.java:57) > >> > >>> [java] at > >> > >>> > >> org.apache.openjpa.kernel.BrokerImpl.initialize(BrokerImpl.java:894) > >> > >>> [java] at > >> kodo.kernel.KodoBroker.initialize(KodoBroker.java:65) > >> > >>> > >> > >>> Reason behind the above exception is because when application > >> identity > >> > >>> class > >> > >>> is used; > >> > >>> In openJPA, ObjectId is generated by enhancer generated method > >> > >>> pcnewObjectIdInstance and it returns wrapped identity class > >> > >>> like as follows. > >> > >>> public Object pcNewObjectIdInstance() > >> > >>> { > >> > >>> return new ObjectId(ApplicationIdentityIdClass.class, new > >> > >>> AppId()); > >> > >>> } > >> > >>> So, the change was not affected to OpenJPA code. However, in > >> OpenJPA > >> > >>> derived products like Kodo, pcNewObjectIdInstance > >> > >>> returns raw application identity class instance. > >> > >>> Due to this "oidList.add((OpenJPAId) sm.getObjectId())" throws > >> > >>> classcast > >> > >>> exception. > >> > >>> > >> > >> > >> > > > >> > > > >> > > >> > -- > >> > View this message in context: > >> > > >> > http://n2.nabble.com/Need-process-help-for-a-code-change-that-does-not-have-an-openJPA-testcase-tp4092236p4097490.html > >> > Sent from the OpenJPA Developers mailing list archive at Nabble.com. > >> > > >> > > > > > > -- > View this message in context: > http://n2.nabble.com/Need-process-help-for-a-code-change-that-does-not-have-an-openJPA-testcase-tp4092236p4172559.html > Sent from the OpenJPA Developers mailing list archive at Nabble.com. >
