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. > > >
