Review request for 7192302 Remove JDBCRowSetImpl dependency on java.beans

[Lance Andersen - Oracle]

On Sep 6, 2012, at 9:31 AM, Alan Bateman wrote:

> On 06/09/2012 14:09, Lance Andersen - Oracle wrote:
>> :
>>> 
>>> The latest webrev looks okay except that in one of the constructors you have removed a call to ensure that the connection is established, I'm not sure about the significance of that.
>> 
>> This is not needed here and given I have already tested with this removed, I figured I would OK to keep this as part of the change
> Okay, I'll have to trust you on this one but I am a little bit concerned that it could cause a NPE, say someone creates a JdbcRowSet and invokes a method such as comment, rollback or getAutoCommit without doing an explicit connect.

connect() is typically called to get a connection and it will validate that there is a non-null Connection.  I have unit tests which leverage that constructor and run clean with that change.

I agree that commit(), rollback, etc  should call checkState() and that is another fix I need to do but the potential for the NPE is there depending on which constructor was used and if you did something silly like making a rollback without doing any work.

I can revert that change if you like but I have not seen any failures under what I would term normal use.
> 
> 
>> 
>> I left those in as a reminder to go back as part of the rest of the Rave clean-up.  I would prefer to leave them for now and when I  make another pass for Rave, I will get rid of them
> Okay.
> 
>>> 
>>> One method that looks like it could be removed too is setConcurrency but I agree that keeping focused on just removing the beans dependency is right for now.
>> 
>> I had thought about that but I have to think about this one a bit more as the getConcurrency() is leveraging the value returned from the active ResultSet which is why I did not remove this at this time.
> 
> Okay although it looks to be just an optimization introduced as part of adding listener events

I am not sure I agree with the code either, just want to spend a bit more time looking at the code flow and see what tests if any we have here
>  I'm fine with leaving it as it is.

thank you
> 
> -Alan


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com