[sling-whiteboard] 03/03: Simplify child classes by passing a CtClass instance instead of a class name

[ro...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rombert pushed a commit to branch feature/jmx
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git

commit 4c72894e5f010ca2bdeda0cb3a3e83be28936b98
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Tue Jun 18 10:49:22 2019 +0200

    Simplify child classes by passing a CtClass instance instead of a class name
    
    This also leads to removing a test class since the code it was testing became a one-liner.
---
 .../uca/impl/HttpClient3TimeoutTransformer.java    |  6 +--
 .../uca/impl/HttpClient4TimeoutTransformer.java    |  2 -
 .../sling/uca/impl/JavaNetTimeoutTransformer.java  | 18 +-------
 .../uca/impl/MBeanAwareTimeoutTransformer.java     | 26 ++++++++---
 ...pdateFieldsInConstructorTimeoutTransformer.java |  5 +--
 .../uca/impl/JaveNetTimeoutTransformerTest.java    | 51 ----------------------
 6 files changed, 25 insertions(+), 83 deletions(-)

diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient3TimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient3TimeoutTransformer.java
index e4da3ec..aa0b807 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient3TimeoutTransformer.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient3TimeoutTransformer.java
@@ -18,7 +18,6 @@ package org.apache.sling.uca.impl;
 
 import java.util.Collections;
 
-import javassist.ClassPool;
 import javassist.CtClass;
 import javassist.CtMethod;
 import javassist.bytecode.Descriptor;
@@ -43,10 +42,7 @@ public class HttpClient3TimeoutTransformer extends MBeanAwareTimeoutTransformer
     }
 
     @Override
-    protected byte[] doTransformClass(String className) throws Exception {
-        
-        ClassPool defaultPool = ClassPool.getDefault();
-        CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
+    protected byte[] doTransformClass(CtClass cc) throws Exception {
         
         CtMethod getSoTimeout =  cc.getDeclaredMethod("createParams");
         // javassist seems unable to resolve the constant values, so just inline them
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient4TimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient4TimeoutTransformer.java
index 6a3990a..f888936 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient4TimeoutTransformer.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/HttpClient4TimeoutTransformer.java
@@ -26,8 +26,6 @@ import javassist.bytecode.Descriptor;
  */
 public class HttpClient4TimeoutTransformer extends UpdateFieldsInConstructorTimeoutTransformer {
 
-    // org.apache.http.client.config.RequestConfig.Builder
-    
     private static final String REQUEST_CONFIG_BUILDER_CLASS_NAME = Descriptor.toJvmName("org.apache.http.client.config.RequestConfig$Builder");
     
     public HttpClient4TimeoutTransformer(long connectTimeoutMillis, long readTimeoutMillis, AgentInfo agentInfoMBean) {
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
index 4dfcf89..2e5654b 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/JavaNetTimeoutTransformer.java
@@ -20,10 +20,8 @@ import java.net.URLConnection;
 import java.util.HashSet;
 import java.util.Set;
 
-import javassist.ClassPool;
 import javassist.CtClass;
 import javassist.CtMethod;
-import javassist.NotFoundException;
 import javassist.bytecode.Descriptor;
 
 /**
@@ -55,25 +53,13 @@ class JavaNetTimeoutTransformer extends MBeanAwareTimeoutTransformer {
         this.readTimeoutMillis = readTimeout;
     }
 
-    protected byte[] doTransformClass(String className) throws Exception {
-        CtMethod connectMethod = findConnectMethod(className);
+    protected byte[] doTransformClass(CtClass cc) throws Exception {
+        CtMethod connectMethod = cc.getDeclaredMethod("connect");
         connectMethod.insertBefore("if ( getConnectTimeout() == 0 ) { setConnectTimeout(" + connectTimeoutMillis + "); }");
         connectMethod.insertBefore("if ( getReadTimeout() == 0 ) { setReadTimeout(" + readTimeoutMillis + "); }");
         byte[] classfileBuffer = connectMethod.getDeclaringClass().toBytecode();
         connectMethod.getDeclaringClass().detach();
         return classfileBuffer;
     }
-    
-    CtMethod findConnectMethod(String className) throws NotFoundException {
-        
-        ClassPool defaultPool = ClassPool.getDefault();
-        CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
-        if (cc == null) {
-            Log.get().log("No class found with name %s", className);
-            return null;
-        }
-        return cc.getDeclaredMethod("connect");
-
-    }
 
 }
\ No newline at end of file
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/MBeanAwareTimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/MBeanAwareTimeoutTransformer.java
index 8211457..bf7eb02 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/MBeanAwareTimeoutTransformer.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/MBeanAwareTimeoutTransformer.java
@@ -20,6 +20,10 @@ import java.lang.instrument.ClassFileTransformer;
 import java.security.ProtectionDomain;
 import java.util.Set;
 
+import javassist.ClassPool;
+import javassist.CtClass;
+import javassist.bytecode.Descriptor;
+
 /**
  * Support class for transformers that expose runtime information through JMX
  * 
@@ -42,10 +46,15 @@ public abstract class MBeanAwareTimeoutTransformer implements ClassFileTransform
         try {
             if (classesToTransform.contains(className)) {
                 Log.get().log("%s asked to transform %s", getClass().getSimpleName(), className);
-                classfileBuffer = doTransformClass(className);
-                Log.get().log("Transformation complete.");
-                
-                this.agentInfo.registerTransformedClass(className);
+                ClassPool defaultPool = ClassPool.getDefault();
+                CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
+                if ( cc == null ) {
+                    Log.get().log("Could not find a class for %s in the default class pool, skipping transformation", className);
+                } else {
+                    classfileBuffer = doTransformClass(cc);
+                    Log.get().log("Transformation of %s complete", className);
+                    this.agentInfo.registerTransformedClass(className);
+                }
             }
             return classfileBuffer;
         } catch (Exception e) {
@@ -54,6 +63,13 @@ public abstract class MBeanAwareTimeoutTransformer implements ClassFileTransform
         }
     }
 
-    protected abstract byte[] doTransformClass(String className) throws Exception;
+    /**
+     * Transform a class that is guaranteed to exist and in scope of this agent instance
+     * 
+     * @param cc the class
+     * @return the new class definition
+     * @throws Exception in case of any problems while transforming
+     */
+    protected abstract byte[] doTransformClass(CtClass cc) throws Exception;
 
 }
\ No newline at end of file
diff --git a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/UpdateFieldsInConstructorTimeoutTransformer.java b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/UpdateFieldsInConstructorTimeoutTransformer.java
index 77ed113..3757137 100644
--- a/url-connection-agent/src/main/java/org/apache/sling/uca/impl/UpdateFieldsInConstructorTimeoutTransformer.java
+++ b/url-connection-agent/src/main/java/org/apache/sling/uca/impl/UpdateFieldsInConstructorTimeoutTransformer.java
@@ -18,7 +18,6 @@ package org.apache.sling.uca.impl;
 
 import java.util.Collections;
 
-import javassist.ClassPool;
 import javassist.CtClass;
 import javassist.CtConstructor;
 import javassist.CtField;
@@ -46,9 +45,7 @@ public abstract class UpdateFieldsInConstructorTimeoutTransformer extends MBeanA
     }
     
     @Override
-    protected byte[] doTransformClass(String className) throws Exception {
-        ClassPool defaultPool = ClassPool.getDefault();
-        CtClass cc = defaultPool.get(Descriptor.toJavaName(className));
+    protected byte[] doTransformClass(CtClass cc) throws Exception {
         
         CtConstructor noArgCtor = cc.getConstructor(Descriptor.ofConstructor(new CtClass[0]));
         CtField connectTimeout = cc.getDeclaredField(connectTimeoutFieldName);
diff --git a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/JaveNetTimeoutTransformerTest.java b/url-connection-agent/src/test/java/org/apache/sling/uca/impl/JaveNetTimeoutTransformerTest.java
deleted file mode 100644
index 9ddffde..0000000
--- a/url-connection-agent/src/test/java/org/apache/sling/uca/impl/JaveNetTimeoutTransformerTest.java
+++ /dev/null
@@ -1,51 +0,0 @@
-/*
- * 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.sling.uca.impl;
-
-
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-
-import javassist.NotFoundException;
-
-public class JaveNetTimeoutTransformerTest {
-    
-    private JavaNetTimeoutTransformer transformer;
-
-    @BeforeEach
-    public void initFields() {
-        transformer = new JavaNetTimeoutTransformer(1, 1, new AgentInfo(1, 1));
-    }
-
-    @Test
-    public void findDeclaredConnectMethod() throws NotFoundException {
-        assertNotNull(transformer.findConnectMethod("sun/net/www/protocol/http/HttpURLConnection"));
-    }
-
-    @Test
-    public void findInheritedConnectMethod() throws NotFoundException {
-        // do NOT look for inherited methods, as we can only rewrite the precise classes the
-        // retransform was triggered for
-        assertThrows( NotFoundException.class,
-            () -> transformer.findConnectMethod("sun/net/www/protocol/https/DelegateHttpsURLConnection")
-        );
-    }
-    
-}