You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "fmeheust (via GitHub)" <gi...@apache.org> on 2023/11/06 14:58:40 UTC

[PR] Added support for JDBC 4.3 beginRequest and endRequest methods. [tomcat]

fmeheust opened a new pull request, #677:
URL: https://github.com/apache/tomcat/pull/677

   JDBC 4.3 added the methods beginRequest and endRequest to java.sql.Connection. These methods hint the driver of the beginning and completion of a request (independent unit of work). Connection pooling manager should call these methods on the underlying connection prior to returning a connection to the caller, and when the applications returns the connection back to the connection pool respectively.
   
   This pull request adds support for beginRequest and endRequest methods to the Tomcat JDBC Connection Pool.


-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PR] Added support for JDBC 4.3 beginRequest and endRequest methods. [tomcat]

Posted by "rmaucher (via GitHub)" <gi...@apache.org>.
rmaucher closed pull request #677: Added support for JDBC 4.3 beginRequest and endRequest methods.
URL: https://github.com/apache/tomcat/pull/677


-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PR] Added support for JDBC 4.3 beginRequest and endRequest methods. [tomcat]

Posted by "fmeheust (via GitHub)" <gi...@apache.org>.
fmeheust commented on code in PR #677:
URL: https://github.com/apache/tomcat/pull/677#discussion_r1384567373


##########
modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectionBoundariesTest.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.jdbc.test;
+
+import org.apache.tomcat.jdbc.test.driver.Driver;
+
+import java.sql.Connection;
+
+import org.junit.Test;
+import org.junit.Assert;
+
+public class ConnectionBoundariesTest extends DefaultTestCase {
+
+
+    @Override
+    public org.apache.tomcat.jdbc.pool.DataSource createDefaultDataSource() {
+        // TODO Auto-generated method stub
+        org.apache.tomcat.jdbc.pool.DataSource ds = super.createDefaultDataSource();
+        ds.getPoolProperties().setDriverClassName(Driver.class.getName());
+        ds.getPoolProperties().setUrl(Driver.url);
+        ds.getPoolProperties().setInitialSize(0);
+        ds.getPoolProperties().setMaxIdle(10);
+        ds.getPoolProperties().setMinIdle(10);
+        ds.getPoolProperties().setMaxActive(10);
+        return ds;
+    }
+    @Test
+    public void connectionWithRequestBoundariesTest() throws Exception {
+        datasource.getPoolProperties().getDbProperties().setProperty("requestBoundaries", "true");

Review Comment:
   I had created another Connection class that would be returned if that property was set, but finally I just added the methods to the existing Connection class, which makes that line useless. I have removed it



-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PR] Added support for JDBC 4.3 beginRequest and endRequest methods. [tomcat]

Posted by "rmaucher (via GitHub)" <gi...@apache.org>.
rmaucher commented on PR #677:
URL: https://github.com/apache/tomcat/pull/677#issuecomment-6493634962

   Merged with changes. Thanks.


-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PR] Added support for JDBC 4.3 beginRequest and endRequest methods. [tomcat]

Posted by "fmeheust (via GitHub)" <gi...@apache.org>.
fmeheust commented on code in PR #677:
URL: https://github.com/apache/tomcat/pull/677#discussion_r1384572235


##########
modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java:
##########
@@ -812,6 +829,28 @@ protected PooledConnection createConnection(long now, PooledConnection notUsed,
         }//catch
     }
 
+    /**
+     * If request boundaries are not initialised, checks if beginRequest and
+     * endRequest methods are implemented on connection object.
+     *
+     * @param connection The connection
+     * @return Returns true if connection is not null and connection  implements
+     * JDBC 4.3 beginRequest and endRequest methods
+     */
+    private boolean hasRequestBoundaryMethods(Connection connection) {
+        if (!requestBoundaryMethodsInitialised && connection != null) {
+            try {
+                beginRequest = connection.getClass().getMethod("beginRequest");
+                endRequest = connection.getClass().getMethod("endRequest");
+            } catch (NoSuchMethodException ex) {
+                // begin and end request not implemented, ignore exception
+            } finally {
+                requestBoundaryMethodsInitialised = true;

Review Comment:
   I have changed the code and moved the code to a static block. The getMethod call is now made on the java.sql.Connection interface and not on the instance if the underlying connection, with this solution request boundaries are only called if the version of java is 9 or later. I have changed the tests accordingly.



-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PR] Added support for JDBC 4.3 beginRequest and endRequest methods. [tomcat]

Posted by "aooohan (via GitHub)" <gi...@apache.org>.
aooohan commented on code in PR #677:
URL: https://github.com/apache/tomcat/pull/677#discussion_r1384334368


##########
modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/ConnectionBoundariesTest.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.jdbc.test;
+
+import org.apache.tomcat.jdbc.test.driver.Driver;
+
+import java.sql.Connection;
+
+import org.junit.Test;
+import org.junit.Assert;
+
+public class ConnectionBoundariesTest extends DefaultTestCase {
+
+
+    @Override
+    public org.apache.tomcat.jdbc.pool.DataSource createDefaultDataSource() {
+        // TODO Auto-generated method stub
+        org.apache.tomcat.jdbc.pool.DataSource ds = super.createDefaultDataSource();
+        ds.getPoolProperties().setDriverClassName(Driver.class.getName());
+        ds.getPoolProperties().setUrl(Driver.url);
+        ds.getPoolProperties().setInitialSize(0);
+        ds.getPoolProperties().setMaxIdle(10);
+        ds.getPoolProperties().setMinIdle(10);
+        ds.getPoolProperties().setMaxActive(10);
+        return ds;
+    }
+    @Test
+    public void connectionWithRequestBoundariesTest() throws Exception {
+        datasource.getPoolProperties().getDbProperties().setProperty("requestBoundaries", "true");

Review Comment:
   What does this parameter do, and where is it used?



##########
modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java:
##########
@@ -812,6 +829,28 @@ protected PooledConnection createConnection(long now, PooledConnection notUsed,
         }//catch
     }
 
+    /**
+     * If request boundaries are not initialised, checks if beginRequest and
+     * endRequest methods are implemented on connection object.
+     *
+     * @param connection The connection
+     * @return Returns true if connection is not null and connection  implements
+     * JDBC 4.3 beginRequest and endRequest methods
+     */
+    private boolean hasRequestBoundaryMethods(Connection connection) {
+        if (!requestBoundaryMethodsInitialised && connection != null) {
+            try {
+                beginRequest = connection.getClass().getMethod("beginRequest");
+                endRequest = connection.getClass().getMethod("endRequest");
+            } catch (NoSuchMethodException ex) {
+                // begin and end request not implemented, ignore exception
+            } finally {
+                requestBoundaryMethodsInitialised = true;

Review Comment:
   Maybe this block move to static block and assign to some static variables is better?



-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [PR] Added support for JDBC 4.3 beginRequest and endRequest methods. [tomcat]

Posted by "fmeheust (via GitHub)" <gi...@apache.org>.
fmeheust commented on PR #677:
URL: https://github.com/apache/tomcat/pull/677#issuecomment-2718315476

   Hi @aooohan  and @ahai-code this contribution has been approved for a while, is it possible to have an idea on when it will be comitted? Thank you!


-- 
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: dev-unsubscribe@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org