Re: [BUGS]log can not be output when use DataSource

Dave,

This new patch from Chen addresses 5. & 6. in my reivew. Those aspects
removed. The patch seems to address properly the BUG identified in the
logLevel original posting & other minor fixes as identified by review.

danap.

Chen Huajun wrote:
> Hi
>
> According the review result,and i modified the patch.
> Deleted user & password from getUrl() and resumed getUrl() to private.
>
> I think the following should be considered in the future, even if it 's
> really needed.
>  > The changing is just for testing.
>  > It is also useful to compare two DataSource(for instance one is a
> copy of another )
>  > But if just for testing,now i have an another idea,
>  > implementing toString() method which just call getURL()
>
> Chen Huajun
> (2013/02/04 10:28), Chen Huajun wrote:
>> danap,
>>
>> > 5. getURL() - Changed from private to public, Why?, DriverManager
>> Contains
>> > no such public method nor Does the Java API define for interface
>> > DataSource,
>> > OK???
>>
>> The changing is just for testing.
>> It is also useful to compare two DataSource(for instance one is a copy
>> of another )
>> But if just for testing,now i have an another idea,
>> implementing toString() method which just call getURL().
>> What about that?
>>
>> > 6. user & password - Even if 5. approved public why would these be
>> open to
>> > a public interface for returning these values with URL. In a basic
>> > instinct I feel this is security risk.
>> > NOT OK
>>
>> oh,they are not needed just as you said.
>> Sorry,i failed to understood your reply.
>> > in getURL(). The properties user & password are included in
>> getConnection()
>> > and therefore do not not need to be in getURL().
>>
>> Chen Huajun
>> (2013/02/04 4:44), dmp wrote:
>>> Review for patch:
>>>
>>> org/postgresql/ds/common/BaseDataSource:
>>>
>>> 1. databaseName - Was null possibly so getURL(), getReference(), &
>>> writeBaseObject() checked,
>>> OK
>>>
>>> 2. binaryTransferEnable/Disable - Same as databaseName for
>>> writeBaseObject(),
>>> OK
>>>
>>> 3. logLevelSet - Needed to Address Bug this patch applies to directly,
>>> OK
>>>
>>> 4. receiveBufferSize & sendBufferSize - Addresses lack of getter
>>> Methods,
>>> OK
>>>
>>> 5. getURL() - Changed from private to public, Why?, DriverManager
>>> Contains
>>> no such public method nor Does the Java API define for interface
>>> DataSource,
>>> OK???
>>>
>>> 6. user & password - Even if 5. approved public why would these be
>>> open to
>>> a public interface for returning these values with URL. In a basic
>>> instinct I feel this is security risk.
>>> NOT OK
>>>
>>> 7. sslFactory - Proper check for null in getReference(),
>>> OK
>>>
>>> 8. applicationName - Proper check for null in getReference(),
>>> OK
>>>
>>> org/postgresql/test/jdbc2/optional/BaseDataSourceTest:
>>>
>>> 1. I'm not proficient with JUnit, but appears to add testing for
>>> getURL()
>>> in 5. above. If that is not approved then test suite addition is
>>> not needed.
>>> OK?
>>>
>>> danap
>>>
>>> Chen Huajun wrote:
>>>> danap,
>>>>
>>>> Please use the new patch.
>>>> I add a test case,and fixed a mistake in the old one.
>>>>
>>>> Chen Huajun
>>>> (2013/02/03 10:32), dmp wrote:
>>>>> Hello,
>>>>>
>>>>> I can review this tomorrow and get back by Monday, or sooner.
>>>>>
>>>>> danap.
>>>>>
>>>>> Chen Huajun wrote:
>>>>>> Hi
>>>>>>
>>>>>> I have made a new patch for BaseDataSource ,please check it.
>>>>>> Some modifications in getReference() is just for keeping
>>>>>> the same style to other String parameters .
>>>>>>
>>>>>> Chen Huajun