Re: [PR] Fix minor issues [commons-dbcp]
garydgregory commented on PR #312: URL: https://github.com/apache/commons-dbcp/pull/312#issuecomment-1971744646 @rmaucher All issues have been addressed in git master. TY! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix minor issues [commons-dbcp]
garydgregory closed pull request #312: Fix minor issues URL: https://github.com/apache/commons-dbcp/pull/312 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix minor issues [commons-dbcp]
garydgregory commented on code in PR #312:
URL: https://github.com/apache/commons-dbcp/pull/312#discussion_r1508027928
##
src/main/java/org/apache/commons/dbcp2/managed/DataSourceXAConnectionFactory.java:
##
@@ -167,8 +167,8 @@ public Connection createConnection() throws SQLException {
}
// get the real connection and XAResource from the connection
-final Connection connection = xaConnection.getConnection();
final XAResource xaResource = xaConnection.getXAResource();
+final Connection connection = xaConnection.getConnection();
Review Comment:
Hm, the git master code has this extra call ATM. I'll close this PR then.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Fix minor issues [commons-dbcp]
garydgregory commented on code in PR #312:
URL: https://github.com/apache/commons-dbcp/pull/312#discussion_r1507882853
##
src/main/java/org/apache/commons/dbcp2/managed/DataSourceXAConnectionFactory.java:
##
@@ -167,8 +167,8 @@ public Connection createConnection() throws SQLException {
}
// get the real connection and XAResource from the connection
-final Connection connection = xaConnection.getConnection();
final XAResource xaResource = xaConnection.getXAResource();
+final Connection connection = xaConnection.getConnection();
Review Comment:
@rmaucher Does the order matter? Can we comment that order here...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Fix minor issues [commons-dbcp]
garydgregory commented on code in PR #312:
URL: https://github.com/apache/commons-dbcp/pull/312#discussion_r1507877935
##
src/main/java/org/apache/commons/dbcp2/cpdsadapter/PooledConnectionImpl.java:
##
@@ -333,7 +334,10 @@ protected PStmtKey createKey(final String sql, final
String[] columnNames) {
@Override
public void destroyObject(final PStmtKey key, final
PooledObject pooledObject)
throws SQLException {
-pooledObject.getObject().getInnermostDelegate().close();
+Statement s = pooledObject.getObject().getInnermostDelegate();
+if (s != null) {
Review Comment:
commit e627d55fe8a33b3ca482aac1da1b761c6146
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Fix minor issues [commons-dbcp]
garydgregory commented on code in PR #312:
URL: https://github.com/apache/commons-dbcp/pull/312#discussion_r1507877548
##
src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java:
##
@@ -422,7 +422,7 @@ public Object getObjectInstance(final Object obj, final
Name name, final Context
private void validatePropertyNames(final Reference ref, final Name name,
final List warnMessages,
final List infoMessages) {
final String nameString = name != null ? "Name = " + name.toString() +
" " : "";
-if (NUPROP_WARNTEXT != null && !NUPROP_WARNTEXT.isEmpty()) {
+if (!NUPROP_WARNTEXT.isEmpty()) {
Review Comment:
commit e627d55fe8a33b3ca482aac1da1b761c6146
##
src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java:
##
@@ -422,7 +422,7 @@ public Object getObjectInstance(final Object obj, final
Name name, final Context
private void validatePropertyNames(final Reference ref, final Name name,
final List warnMessages,
final List infoMessages) {
final String nameString = name != null ? "Name = " + name.toString() +
" " : "";
-if (NUPROP_WARNTEXT != null && !NUPROP_WARNTEXT.isEmpty()) {
+if (!NUPROP_WARNTEXT.isEmpty()) {
Review Comment:
commit e627d55fe8a33b3ca482aac1da1b761c6146
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Fix minor issues [commons-dbcp]
garydgregory commented on code in PR #312:
URL: https://github.com/apache/commons-dbcp/pull/312#discussion_r1368934365
##
src/main/java/org/apache/commons/dbcp2/BasicDataSourceFactory.java:
##
@@ -422,7 +422,7 @@ public Object getObjectInstance(final Object obj, final
Name name, final Context
private void validatePropertyNames(final Reference ref, final Name name,
final List warnMessages,
final List infoMessages) {
final String nameString = name != null ? "Name = " + name.toString() +
" " : "";
-if (NUPROP_WARNTEXT != null && !NUPROP_WARNTEXT.isEmpty()) {
+if (!NUPROP_WARNTEXT.isEmpty()) {
Review Comment:
OK, but the `isEmpty()` is also superfluous. Now in git master.
##
src/main/java/org/apache/commons/dbcp2/managed/DataSourceXAConnectionFactory.java:
##
@@ -167,8 +167,8 @@ public Connection createConnection() throws SQLException {
}
// get the real connection and XAResource from the connection
-final Connection connection = xaConnection.getConnection();
final XAResource xaResource = xaConnection.getXAResource();
+final Connection connection = xaConnection.getConnection();
Review Comment:
If the order matters, please document it in a comment. Does it?
##
src/main/java/org/apache/commons/dbcp2/PoolingConnection.java:
##
@@ -294,7 +295,10 @@ protected PStmtKey createKey(final String sql, final
String[] columnNames) {
@Override
public void destroyObject(final PStmtKey key, final
PooledObject pooledObject)
throws SQLException {
-pooledObject.getObject().getInnermostDelegate().close();
+Statement s = pooledObject.getObject().getInnermostDelegate();
+if (s != null) {
+s.close();
Review Comment:
Hello @rmaucher
This needs a unit test.
##
src/main/java/org/apache/commons/dbcp2/cpdsadapter/PooledConnectionImpl.java:
##
@@ -333,7 +334,10 @@ protected PStmtKey createKey(final String sql, final
String[] columnNames) {
@Override
public void destroyObject(final PStmtKey key, final
PooledObject pooledObject)
throws SQLException {
-pooledObject.getObject().getInnermostDelegate().close();
+Statement s = pooledObject.getObject().getInnermostDelegate();
+if (s != null) {
Review Comment:
This needs a unit test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
