Fix issue with proxies not implementing mixins

Closes #262

agent/core/src/main/java/org/glowroot/agent/weaving/ClassAnalyzer.java

@@ -174,7 +174,7 @@ boolean isWeavingRequired() {
         checkNotNull(methodAdvisors);
         checkNotNull(methodsThatOnlyNowFulfillAdvice);
         if (Modifier.isInterface(thinClass.access())) {
-            return false;
+            return !matchedMixinTypes.isEmpty();
         }
         return !methodAdvisors.isEmpty() || !methodsThatOnlyNowFulfillAdvice.isEmpty()
                 || !matchedShimTypes.isEmpty() || !matchedMixinTypes.isEmpty();

agent/core/src/main/java/org/glowroot/agent/weaving/WeavingClassVisitor.java

@@ -127,11 +127,13 @@ public WeavingClassVisitor(ClassWriter cw, @Nullable ClassLoader loader,
         // MethodNode.accept() cannot be called multiple times (at least not across multiple
         // threads) without throwing an occassional NPE
         mixinClassNodes = Lists.newArrayList();
-        for (MixinType mixinType : mixinTypes) {
-            ClassReader cr = new ClassReader(mixinType.implementationBytes());
-            ClassNode cn = new ClassNode();
-            cr.accept(cn, ClassReader.EXPAND_FRAMES);
-            mixinClassNodes.add(cn);
+        if (!analyzedClass.isInterface()) {
+            for (MixinType mixinType : mixinTypes) {
+                ClassReader cr = new ClassReader(mixinType.implementationBytes());
+                ClassNode cn = new ClassNode();
+                cr.accept(cn, ClassReader.EXPAND_FRAMES);
+                mixinClassNodes.add(cn);
+            }
         }
     }
 
@@ -150,6 +152,9 @@ public void visit(int version, int access, String internalName, @Nullable String
     @Override
     public @Nullable MethodVisitor visitMethod(int access, String name, String desc,
             @Nullable String signature, String /*@Nullable*/ [] exceptions) {
+        if (analyzedClass.isInterface()) {
+            return cw.visitMethod(access, name, desc, signature, exceptions);
+        }
         if (isMixinProxy(name, desc)) {
             return null;
         }
@@ -193,6 +198,10 @@ private boolean isMixinProxy(String name, String desc) {
     @Override
     public void visitEnd() {
         analyzedWorld.add(analyzedClass, loader);
+        if (analyzedClass.isInterface()) {
+            cw.visitEnd();
+            return;
+        }
         checkNotNull(type); // type is non null if there is something to weave
         for (ShimType shimType : shimTypes) {
             addShim(shimType);

agent/plugins/jdbc-plugin/pom.xml

@@ -21,6 +21,7 @@
     <postgresql.version>42.1.4.jre6</postgresql.version>
     <commons.dbcp.version>1.4</commons.dbcp.version>
     <tomcat.version>7.0.79</tomcat.version>
+    <glassfish.jdbc.version>4.1.2</glassfish.jdbc.version>
   </properties>
 
   <dependencies>
@@ -88,6 +89,12 @@
       <version>${tomcat.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.glassfish.main.jdbc.jdbc-ra.jdbc40</groupId>
+      <artifactId>jdbc40</artifactId>
+      <version>${glassfish.jdbc.version}</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

agent/plugins/jdbc-plugin/src/test/java/org/glowroot/agent/plugin/jdbc/Connections.java

@@ -15,13 +15,19 @@
  */
 package org.glowroot.agent.plugin.jdbc;
 
+import java.lang.reflect.UndeclaredThrowableException;
 import java.sql.Connection;
 import java.sql.DriverManager;
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.util.Properties;
 
+import javax.sql.DataSource;
+
+import com.sun.gjc.spi.DMManagedConnectionFactory;
 import org.apache.commons.dbcp.BasicDataSource;
+import org.glassfish.api.jdbc.SQLTraceListener;
+import org.glassfish.api.jdbc.SQLTraceRecord;
 import org.h2.jdbc.JdbcConnection;
 import org.hsqldb.jdbc.JDBCDriver;
 
@@ -39,7 +45,8 @@ public class Connections {
     }
 
     enum ConnectionType {
-        HSQLDB, H2, COMMONS_DBCP_WRAPPED, TOMCAT_JDBC_POOL_WRAPPED, POSTGRES, ORACLE, SQLSERVER
+        HSQLDB, H2, COMMONS_DBCP_WRAPPED, TOMCAT_JDBC_POOL_WRAPPED, GLASSFISH_JDBC_POOL_WRAPPED,
+        POSTGRES, ORACLE, SQLSERVER
     }
 
     static Connection createConnection() throws Exception {
@@ -52,6 +59,8 @@ static Connection createConnection() throws Exception {
                 return createCommonsDbcpWrappedConnection();
             case TOMCAT_JDBC_POOL_WRAPPED:
                 return createTomcatJdbcPoolWrappedConnection();
+            case GLASSFISH_JDBC_POOL_WRAPPED:
+                return createGlassfishJdbcPoolWrappedConnection();
             case POSTGRES:
                 return createPostgresConnection();
             case ORACLE:
@@ -70,7 +79,10 @@ static void closeConnection(Connection connection) throws SQLException {
         } finally {
             statement.close();
         }
-        connection.close();
+        if (connectionType != ConnectionType.GLASSFISH_JDBC_POOL_WRAPPED) {
+            // TODO figure out why glassfish connection throws NullPointerException here
+            connection.close();
+        }
     }
 
     static ConnectionType getConnectionType() {
@@ -114,6 +126,20 @@ private static Connection createTomcatJdbcPoolWrappedConnection() throws SQLExce
         return connection;
     }
 
+    private static Connection createGlassfishJdbcPoolWrappedConnection() throws SQLException {
+        // set up database
+        DMManagedConnectionFactory connectionFactory = new DMManagedConnectionFactory();
+        connectionFactory.setClassName("org.hsqldb.jdbc.JDBCDriver");
+        connectionFactory.setURL("jdbc:hsqldb:mem:test");
+        connectionFactory.setStatementWrapping("true");
+        connectionFactory.setSqlTraceListeners(
+                "org.glowroot.agent.plugin.jdbc.Connections$GlassfishSQLTraceListener");
+        DataSource ds = (DataSource) connectionFactory.createConnectionFactory();
+        Connection connection = ds.getConnection();
+        insertRecords(connection);
+        return connection;
+    }
+
     private static Connection createPostgresConnection() throws SQLException {
         // set up database
         Connection connection = DriverManager.getConnection("jdbc:postgresql://localhost/glowroot",
@@ -178,6 +204,7 @@ private static void insertRecords(Connection connection, String binaryTypeName,
                 // in case of previous failure mid-test
                 statement.execute("drop table employee");
             } catch (SQLException e) {
+            } catch (UndeclaredThrowableException e) {
             }
             statement.execute("create table employee (name varchar(100), misc " + binaryTypeName
                     + ", misc2 " + clobTypeName + ")");
@@ -188,4 +215,9 @@ private static void insertRecords(Connection connection, String binaryTypeName,
             statement.close();
         }
     }
+
+    public static class GlassfishSQLTraceListener implements SQLTraceListener {
+        @Override
+        public void sqlTrace(SQLTraceRecord record) {}
+    }
 }

build/multi-lib-tests/src/main/java/org/glowroot/testing/JDBC.java

@@ -25,7 +25,7 @@ public class JDBC {
 
     public static void main(String[] args) throws Exception {
         // TODO run tests against different versions hsqldb, h2, postgresql, commons dbcp (wrapped
-        // connection), tomcat (wrapped connection)
+        // connection), tomcat (wrapped connection), glassfish (wrapped connection)
         Util.log("jdbc plugin");
         Util.runTests(MODULE_PATH, JAVA6, JAVA7, JAVA8);
     }

build/travis-ci/build.sh

@@ -94,6 +94,18 @@ case "$1" in
                                 -Dglowroot.test.jdbcConnectionType=TOMCAT_JDBC_POOL_WRAPPED \
                                 --no-snapshot-updates \
                                 -B
+               if [[ "$GLOWROOT_HARNESS" == "javaagent" ]]
+               then
+                 # GLASSFISH_JDBC_POOL_WRAPPED tests only work with javaagent container because they
+                 # depend on weaving bootstrap classes (e.g. java.sql.Statement)
+                 mvn clean verify -pl :glowroot-agent-jdbc-plugin \
+                                  -DargLine="$surefire_jvm_args" \
+                                  $skip_shading_opt \
+                                  -Dglowroot.it.harness=javaagent \
+                                  -Dglowroot.test.jdbcConnectionType=GLASSFISH_JDBC_POOL_WRAPPED \
+                                  --no-snapshot-updates \
+                                  -B
+               fi
                ;;
 
       "test3") if [[ "$java_version" < "1.8" ]]