Hi Mike,
I spent some time on JMOCK (thanks for mentioning about it) and also to
create a testcase to protect the code change.
The method that I have to test is :
DataCacheStoreManager.loadAll(Collection<OpenJPAStateManager> sms, PCState
state, int load,
FetchConfiguration fetch, Object edata)
The piece of code that I have to protect with adding a test case is :
for (OpenJPAStateManager sm : smList) {
oidList.add((OpenJPAId) sm.getObjectId());
}
The loadAll() method does lot more than just the above and hence it is more
complicated to write a test case.
The reason why I say complicated is because the loadAll calls lots of other
methods on statemanager, JDBCStoreManager etc... meaning I may have to
create lots of mock objects to test the loadAll() method.
The best way I can think of to create a test case for my changes is to move
the above mentioned code into a method and test it.
The problem with it is I may have to make this method a public method, which
is not appropriate.
So I ran out of any easy ways to create a testcase to protect my code.
Iam willing to try any other suggestions that you may have about this.
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-tp4092236p4148690.html
Sent from the OpenJPA Developers mailing list archive at Nabble.com.
