From 9658ed2716e377da03bf07b8e5b08ff2a7c12897 Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Fri, 7 Aug 2020 14:50:30 +0200 Subject: [PATCH 1/9] Support for bean producers in different package than beans that have package local methods. Signed-off-by: Tomas Langer --- .../cdi/HelidonProxyServices.java | 41 +++++++++++-- .../nativeimage/mp1/BeanProducer.java | 33 +++++++++++ .../integration/nativeimage/mp1/Mp1Main.java | 3 + .../integration/nativeimage/mp1/TestBean.java | 11 +++- .../nativeimage/mp1/other/BeanProcessor.java | 36 ++++++++++++ .../nativeimage/mp1/other/ProducedBean.java | 58 +++++++++++++++++++ .../nativeimage/mp1/other/package-info.java | 20 +++++++ .../mp-1/src/main/java/module-info.java | 2 + 8 files changed, 198 insertions(+), 6 deletions(-) create mode 100644 tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/BeanProducer.java create mode 100644 tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/BeanProcessor.java create mode 100644 tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/ProducedBean.java create mode 100644 tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/package-info.java diff --git a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java index 5ecb9e81404..76e234b5c82 100644 --- a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java +++ b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java @@ -23,6 +23,7 @@ import java.util.Collections; import java.util.IdentityHashMap; import java.util.Map; +import java.util.logging.Level; import java.util.logging.Logger; import io.helidon.common.NativeImageHelper; @@ -75,8 +76,17 @@ public Class defineClass(Class originalClass, String className, byte[] cla // classloader, preventing it from seeing these fields/methods return defineClassSamePackage(originalClass, className, classBytes, off, len); } else { - // use a custom classloader to define classes in a new package - return wrapCl(originalClass.getClassLoader()).doDefineClass(originalClass, className, classBytes, off, len); + // try to use same package approach (maybe in same module?) to support package local methods + try { + return defineClassSamePackage(originalClass, className, classBytes, off, len); + } catch (Exception e) { + LOGGER.log(Level.FINEST, + "Failed to create class " + className + " in same classloader. Will use a different one", + e); + // use a custom classloader to define classes in a new package + return wrapCl(originalClass.getClassLoader()) + .doDefineClass(originalClass, className, classBytes, off, len); + } } } @@ -91,8 +101,17 @@ public Class defineClass(Class originalClass, if (samePackage(originalClass, className)) { return defineClassSamePackage(originalClass, className, classBytes, off, len); } else { - return wrapCl(originalClass.getClassLoader()) - .doDefineClass(originalClass, className, classBytes, off, len, protectionDomain); + // try to use same package approach (maybe in same module?) to support package local methods + try { + return defineClassSamePackage(originalClass, className, classBytes, off, len); + } catch (Exception e) { + LOGGER.log(Level.FINEST, + "Failed to create class " + className + " in same classloader. Will use a different one", + e); + + return wrapCl(originalClass.getClassLoader()) + .doDefineClass(originalClass, className, classBytes, off, len, protectionDomain); + } } } @@ -117,8 +136,20 @@ private Class defineClassSamePackage(Class originalClass, String className // it also needs to open the package we are doing the lookup in myModule.addReads(classModule); } + + // I would like to create a private lookup in the same package as the proxied class, so let's + // try to load it + String proxiedClassName = className.substring(0, className.indexOf('$')); + Class lookupClass; + try { + lookupClass = originalClass.getClassLoader().loadClass(proxiedClassName); + } catch (Throwable e) { + LOGGER.log(Level.FINEST, "Cannot load proxied class: " + proxiedClassName, e); + lookupClass = originalClass; + } + // next line would fail if the module does not open its package, with a very meaningful error message - MethodHandles.Lookup lookup = MethodHandles.privateLookupIn(originalClass, MethodHandles.lookup()); + MethodHandles.Lookup lookup = MethodHandles.privateLookupIn(lookupClass, MethodHandles.lookup()); if (classBytes.length == len) { return lookup.defineClass(classBytes); } else { diff --git a/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/BeanProducer.java b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/BeanProducer.java new file mode 100644 index 00000000000..a5125d59723 --- /dev/null +++ b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/BeanProducer.java @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2020 Oracle and/or its affiliates. + * + * Licensed 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 io.helidon.tests.integration.nativeimage.mp1; + +import javax.enterprise.context.ApplicationScoped; +import javax.enterprise.inject.Produces; + +import io.helidon.tests.integration.nativeimage.mp1.other.ProducedBean; + +@ApplicationScoped +public class BeanProducer { + public static final String VALUE = "hi there"; + + @Produces + @ApplicationScoped + public ProducedBean produceBean() { + return new ProducedBean(VALUE); + } +} diff --git a/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/Mp1Main.java b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/Mp1Main.java index 5671c175fea..50bcc65a054 100644 --- a/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/Mp1Main.java +++ b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/Mp1Main.java @@ -177,6 +177,9 @@ private static void testBean(int port, String jwtToken) { // CDI - (tested indirectly by other tests) // Server - capability to start JAX-RS (tested indirectly by other tests) + // produce a bean with package local method + invoke(collector, "Produced bean", BeanProducer.VALUE, aBean::produced); + // Configuration invoke(collector, "Config injection", "Properties message", aBean::config); diff --git a/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/TestBean.java b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/TestBean.java index f9714b8710f..72dc7ddba6f 100644 --- a/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/TestBean.java +++ b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/TestBean.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2020 Oracle and/or its affiliates. */ package io.helidon.tests.integration.nativeimage.mp1; @@ -14,6 +14,8 @@ import javax.inject.Inject; import io.helidon.microprofile.server.ServerCdiExtension; +import io.helidon.tests.integration.nativeimage.mp1.other.BeanProcessor; +import io.helidon.tests.integration.nativeimage.mp1.other.ProducedBean; import org.eclipse.microprofile.config.inject.ConfigProperty; import org.eclipse.microprofile.faulttolerance.Asynchronous; @@ -40,6 +42,9 @@ public class TestBean { @Inject private BeanManager beanManager; + @Inject + private ProducedBean producedBean; + private final AtomicInteger retries = new AtomicInteger(); @Timed @@ -109,4 +114,8 @@ public String timeout() { public CompletionStage asynchronous() { return CompletableFuture.completedFuture("Async response"); } + + public String produced() { + return BeanProcessor.getProducedName(producedBean); + } } diff --git a/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/BeanProcessor.java b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/BeanProcessor.java new file mode 100644 index 00000000000..04438ca0700 --- /dev/null +++ b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/BeanProcessor.java @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2020 Oracle and/or its affiliates. + * + * Licensed 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 io.helidon.tests.integration.nativeimage.mp1.other; + +public final class BeanProcessor { + + public static String getProducedName(ProducedBean bean) { + Class sampleClass = ProducedBean.class; + Class proxyClass = bean.getClass(); + + Package samplePackage = sampleClass.getPackage(); + Package proxyPackage = proxyClass.getPackage(); + + System.out.println(samplePackage); + System.out.println(proxyPackage); + System.out.println("Equals: " + samplePackage.equals(proxyPackage)); + + String name = bean.getName(); + return name; + } + +} diff --git a/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/ProducedBean.java b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/ProducedBean.java new file mode 100644 index 00000000000..710c60ce822 --- /dev/null +++ b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/ProducedBean.java @@ -0,0 +1,58 @@ +/* + * Copyright (c) 2020 Oracle and/or its affiliates. + * + * Licensed 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 io.helidon.tests.integration.nativeimage.mp1.other; + +import java.util.Objects; + +/** + * A bean produced by a producer in a different package. + */ +public class ProducedBean { + + private final String name; + + /** + * Constructor to create a new instance outside of CDI. + * + * @param name name to use + */ + public ProducedBean(final String name) { + this.name = Objects.requireNonNull(name); + } + + // Add public to make it work + String getName() { + return name; + } + + @Override + public boolean equals(final Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + ProducedBean myClass = (ProducedBean) o; + return Objects.equals(name, myClass.name); + } + + @Override + public int hashCode() { + return Objects.hash(name); + } +} diff --git a/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/package-info.java b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/package-info.java new file mode 100644 index 00000000000..e2c09cfbd69 --- /dev/null +++ b/tests/integration/native-image/mp-1/src/main/java/io/helidon/tests/integration/nativeimage/mp1/other/package-info.java @@ -0,0 +1,20 @@ +/* + * Copyright (c) 2020 Oracle and/or its affiliates. + * + * Licensed 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. + */ +/** + * This package exists to make sure we can do proxies that have working + * package local + */ +package io.helidon.tests.integration.nativeimage.mp1.other; \ No newline at end of file diff --git a/tests/integration/native-image/mp-1/src/main/java/module-info.java b/tests/integration/native-image/mp-1/src/main/java/module-info.java index c554275ec6d..292c029f1ac 100644 --- a/tests/integration/native-image/mp-1/src/main/java/module-info.java +++ b/tests/integration/native-image/mp-1/src/main/java/module-info.java @@ -37,9 +37,11 @@ requires io.helidon.health.checks; exports io.helidon.tests.integration.nativeimage.mp1; + exports io.helidon.tests.integration.nativeimage.mp1.other; // opens is needed to inject private fields, create classes in the same package (proxy) opens io.helidon.tests.integration.nativeimage.mp1 to weld.core.impl, hk2.utils, io.helidon.microprofile.cdi; + opens io.helidon.tests.integration.nativeimage.mp1.other to weld.core.impl, io.helidon.microprofile.cdi; // we need to open the static resource on classpath directory to everybody, as otherwise // static content will not see it From 161d526a6d4133e765c838fc9e2cd7dc361921da Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Fri, 7 Aug 2020 17:04:23 +0200 Subject: [PATCH 2/9] Refactor to call each method just once. Signed-off-by: Tomas Langer --- .../cdi/HelidonProxyServices.java | 41 ++++++++++--------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java index 76e234b5c82..b23ba94b36c 100644 --- a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java +++ b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java @@ -70,20 +70,20 @@ public boolean supportsClassDefining() { public Class defineClass(Class originalClass, String className, byte[] classBytes, int off, int len) throws ClassFormatError { - if (samePackage(originalClass, className)) { - // when we need to define a class in the same package (to see package local fields and methods) - // we cannot use a classloader, as the new class would be in the same package, but in a different - // classloader, preventing it from seeing these fields/methods - return defineClassSamePackage(originalClass, className, classBytes, off, len); - } else { - // try to use same package approach (maybe in same module?) to support package local methods - try { - return defineClassSamePackage(originalClass, className, classBytes, off, len); - } catch (Exception e) { + // always try to define in private lookup, if fails (and not same package), try using a different classloader + try { + return defineClassPrivateLookup(originalClass, className, classBytes, off, len); + } catch (Exception e) { + if (samePackage(originalClass, className)) { + // when same package, we must use private lookup (as we may use package local) + throw e; + } else { LOGGER.log(Level.FINEST, "Failed to create class " + className + " in same classloader. Will use a different one", e); - // use a custom classloader to define classes in a new package + + // other cases (except for a few edge cases, such as producer in a different package and usage + // of bean in the same package) we can live with a different classloader to hold the proxy class return wrapCl(originalClass.getClassLoader()) .doDefineClass(originalClass, className, classBytes, off, len); } @@ -98,17 +98,20 @@ public Class defineClass(Class originalClass, int len, ProtectionDomain protectionDomain) throws ClassFormatError { - if (samePackage(originalClass, className)) { - return defineClassSamePackage(originalClass, className, classBytes, off, len); - } else { - // try to use same package approach (maybe in same module?) to support package local methods - try { - return defineClassSamePackage(originalClass, className, classBytes, off, len); - } catch (Exception e) { + // always try to define in private lookup, if fails (and not same package), try using a different classloader + try { + return defineClassPrivateLookup(originalClass, className, classBytes, off, len); + } catch (Exception e) { + if (samePackage(originalClass, className)) { + // when same package, we must use private lookup (as we may use package local) + throw e; + } else { LOGGER.log(Level.FINEST, "Failed to create class " + className + " in same classloader. Will use a different one", e); + // other cases (except for a few edge cases, such as producer in a different package and usage + // of bean in the same package) we can live with a different classloader to hold the proxy class return wrapCl(originalClass.getClassLoader()) .doDefineClass(originalClass, className, classBytes, off, len, protectionDomain); } @@ -120,7 +123,7 @@ public Class loadClass(Class originalClass, String classBinaryName) throws return wrapCl(originalClass.getClassLoader()).loadClass(classBinaryName); } - private Class defineClassSamePackage(Class originalClass, String className, byte[] classBytes, int off, int len) { + private Class defineClassPrivateLookup(Class originalClass, String className, byte[] classBytes, int off, int len) { if (NativeImageHelper.isRuntime()) { throw new IllegalStateException("Cannot define class in native image. Class name: " + className + ", original " + "class: " + originalClass From 117ad853feaef30e9ee12cfa888fe0380143b0de Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Mon, 10 Aug 2020 19:47:28 +0200 Subject: [PATCH 3/9] Review comments. Signed-off-by: Tomas Langer --- .../helidon/microprofile/cdi/HelidonProxyServices.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java index b23ba94b36c..300c0fcb50c 100644 --- a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java +++ b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java @@ -141,13 +141,15 @@ private Class defineClassPrivateLookup(Class originalClass, String classNa } // I would like to create a private lookup in the same package as the proxied class, so let's - // try to load it - String proxiedClassName = className.substring(0, className.indexOf('$')); + // try to load it - I load the enclosing class (or the proxied class if not inner) to have + // a lookup in the correct package/class + String lookupClassName = className.substring(0, className.indexOf('$')); Class lookupClass; try { - lookupClass = originalClass.getClassLoader().loadClass(proxiedClassName); + lookupClass = originalClass.getClassLoader().loadClass(lookupClassName); } catch (Throwable e) { - LOGGER.log(Level.FINEST, "Cannot load proxied class: " + proxiedClassName, e); + LOGGER.log(Level.FINEST, "Cannot load class to create private lookup: " + lookupClassName, e); + // fallback to the producer class, as we cannot load the enclosing class of the proxy lookupClass = originalClass; } From 34e9e501b2cff07d7c1ba3721b1a2597dae8cf12 Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Tue, 11 Aug 2020 20:44:06 +0200 Subject: [PATCH 4/9] Update to proxy services - name is generated with super interface first - proxy classes are defined using private lookup when in original package - proxy classes are defined using a class loader for weld specific packages Signed-off-by: Tomas Langer --- .../cdi/HelidonProxyServices.java | 149 +++++++++++------- .../jboss/weld/bean/proxy/ProxyFactory.java | 44 +++++- 2 files changed, 135 insertions(+), 58 deletions(-) diff --git a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java index 300c0fcb50c..3218759377e 100644 --- a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java +++ b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java @@ -33,6 +33,11 @@ class HelidonProxyServices implements ProxyServices { private static final Logger LOGGER = Logger.getLogger(HelidonProxyServices.class.getName()); + private static final String CLIENT_PROXY_SUFFIX = "$$Proxy$_$$_WeldClientProxy"; + private static final String PROXY_SUFFIX = "$$Proxy$_$$_Weld$Proxy$"; + private static final String WELDX_PACKAGE_PREFIX = "org.jboss.weldx"; + private static final String WELD_LANG_PACKAGE_PREFIX = "org.jboss.weld.lang"; + // a cache of all classloaders (this should be empty in most cases, as we use a single class loader in Helidon) private final Map classLoaders = Collections.synchronizedMap(new IdentityHashMap<>()); private final ClassLoader contextCl; @@ -70,23 +75,20 @@ public boolean supportsClassDefining() { public Class defineClass(Class originalClass, String className, byte[] classBytes, int off, int len) throws ClassFormatError { - // always try to define in private lookup, if fails (and not same package), try using a different classloader + if (weldInternalProxyClassName(className)) { + // this is special case - these classes are defined in a non-existent package + // and we need to use a classloader (public lookup will not allow this, and private lookup is not + // possible for an empty package) + return wrapCl(originalClass.getClassLoader()) + .doDefineClass(originalClass, className, classBytes, off, len); + } + // any other class should be defined using a private lookup try { return defineClassPrivateLookup(originalClass, className, classBytes, off, len); } catch (Exception e) { - if (samePackage(originalClass, className)) { - // when same package, we must use private lookup (as we may use package local) - throw e; - } else { - LOGGER.log(Level.FINEST, - "Failed to create class " + className + " in same classloader. Will use a different one", - e); - - // other cases (except for a few edge cases, such as producer in a different package and usage - // of bean in the same package) we can live with a different classloader to hold the proxy class - return wrapCl(originalClass.getClassLoader()) - .doDefineClass(originalClass, className, classBytes, off, len); - } + LOGGER.log(Level.FINEST, "Failed to create class " + className + " using private lookup", e); + + throw e; } } @@ -98,23 +100,20 @@ public Class defineClass(Class originalClass, int len, ProtectionDomain protectionDomain) throws ClassFormatError { - // always try to define in private lookup, if fails (and not same package), try using a different classloader + if (weldInternalProxyClassName(className)) { + // this is special case - these classes are defined in a non-existent package + // and we need to use a classloader (public lookup will not allow this, and private lookup is not + // possible for an empty package) + return wrapCl(originalClass.getClassLoader()) + .doDefineClass(originalClass, className, classBytes, off, len, protectionDomain); + } + // any other class should be defined using a private lookup try { return defineClassPrivateLookup(originalClass, className, classBytes, off, len); } catch (Exception e) { - if (samePackage(originalClass, className)) { - // when same package, we must use private lookup (as we may use package local) - throw e; - } else { - LOGGER.log(Level.FINEST, - "Failed to create class " + className + " in same classloader. Will use a different one", - e); - - // other cases (except for a few edge cases, such as producer in a different package and usage - // of bean in the same package) we can live with a different classloader to hold the proxy class - return wrapCl(originalClass.getClassLoader()) - .doDefineClass(originalClass, className, classBytes, off, len, protectionDomain); - } + LOGGER.log(Level.FINEST, "Failed to create class " + className + " using private lookup", e); + + throw e; } } @@ -123,6 +122,10 @@ public Class loadClass(Class originalClass, String classBinaryName) throws return wrapCl(originalClass.getClassLoader()).loadClass(classBinaryName); } + private boolean weldInternalProxyClassName(String className) { + return className.startsWith(WELDX_PACKAGE_PREFIX) || className.startsWith(WELD_LANG_PACKAGE_PREFIX); + } + private Class defineClassPrivateLookup(Class originalClass, String className, byte[] classBytes, int off, int len) { if (NativeImageHelper.isRuntime()) { throw new IllegalStateException("Cannot define class in native image. Class name: " + className + ", original " @@ -132,6 +135,8 @@ private Class defineClassPrivateLookup(Class originalClass, String classNa LOGGER.finest("Defining class " + className + " original class: " + originalClass.getName()); + MethodHandles.Lookup lookup; + try { Module classModule = originalClass.getModule(); if (!myModule.canRead(classModule)) { @@ -140,45 +145,81 @@ private Class defineClassPrivateLookup(Class originalClass, String classNa myModule.addReads(classModule); } + // lookup class name based on full class name (e.g. for inner classes) + String lookupClassName; + // lookup class name based on the first class in the name + String fallbackLookupClassName; + + if (className.contains("$")) { + // fallback is the package + first name in the compound proxy class name + fallbackLookupClassName = className.substring(0, className.indexOf('$')); + } else { + fallbackLookupClassName = className; + } + + // Let's try to get the actual class of the proxy + // the name ends with $$Proxy$_$$_WeldClientProxy or $$Proxy$_$$_Weld$Proxy$ + if (className.endsWith(CLIENT_PROXY_SUFFIX)) { + lookupClassName = className.substring(0, className.length() - CLIENT_PROXY_SUFFIX.length()); + } else if (className.endsWith(PROXY_SUFFIX)) { + lookupClassName = className.substring(0, className.length() - PROXY_SUFFIX.length()); + } else { + lookupClassName = fallbackLookupClassName; + } + // I would like to create a private lookup in the same package as the proxied class, so let's - // try to load it - I load the enclosing class (or the proxied class if not inner) to have - // a lookup in the correct package/class - String lookupClassName = className.substring(0, className.indexOf('$')); - Class lookupClass; - try { - lookupClass = originalClass.getClassLoader().loadClass(lookupClassName); - } catch (Throwable e) { - LOGGER.log(Level.FINEST, "Cannot load class to create private lookup: " + lookupClassName, e); - // fallback to the producer class, as we cannot load the enclosing class of the proxy + + // first if the producer class and the lookup class name is the same, just use the existing class + Class lookupClass = lookupClassName.equals(originalClass.getName()) ? originalClass : null; + + ClassLoader cl = originalClass.getClassLoader(); + + if (null == lookupClass) { + // try to load the full class name (may contain additional interfaces, so easily invalid) + lookupClass = tryLoading(cl, lookupClassName); + } + if (null == lookupClass && !lookupClassName.equals(fallbackLookupClassName)) { + // fallback to the simple class name until the first $ sign + lookupClass = tryLoading(cl, fallbackLookupClassName); + } + if (null == lookupClass) { + // and if that fails, just use the bean producer class lookupClass = originalClass; } // next line would fail if the module does not open its package, with a very meaningful error message - MethodHandles.Lookup lookup = MethodHandles.privateLookupIn(lookupClass, MethodHandles.lookup()); - if (classBytes.length == len) { - return lookup.defineClass(classBytes); - } else { - byte[] bytes = new byte[len]; - System.arraycopy(classBytes, off, bytes, 0, len); - return lookup.defineClass(bytes); - } + lookup = MethodHandles.privateLookupIn(lookupClass, MethodHandles.lookup()); } catch (IllegalAccessException e) { throw new RuntimeException("Failed to define class " + className, e); } + + return defineClass(lookup, className, classBytes, off, len); } - private boolean samePackage(Class originalClass, String className) { - String origPackage = originalClass.getPackageName(); - String newPackage = packageName(className); - return newPackage.equals(origPackage); + private Class tryLoading(ClassLoader cl, String className) { + try { + return cl.loadClass(className); + } catch (Exception e) { + LOGGER.log(Level.FINEST, "Attempt to load class " + className + " failed.", e); + } + return null; } - private String packageName(String className) { - int index = className.lastIndexOf('.'); - if (index > 0) { - return className.substring(0, index); + private Class defineClass(MethodHandles.Lookup lookup, String className, byte[] classBytes, int off, int len) { + try { + byte[] definitionBytes; + + if (classBytes.length == len) { + definitionBytes = classBytes; + } else { + definitionBytes = new byte[len]; + System.arraycopy(classBytes, off, definitionBytes, 0, len); + } + + return lookup.defineClass(definitionBytes); + } catch (IllegalAccessException e) { + throw new RuntimeException("Failed to define class " + className, e); } - return ""; } private ClassDefiningCl wrapCl(ClassLoader origCl) { diff --git a/microprofile/weld/weld-core-impl/src/main/java/org/jboss/weld/bean/proxy/ProxyFactory.java b/microprofile/weld/weld-core-impl/src/main/java/org/jboss/weld/bean/proxy/ProxyFactory.java index f5f5e667de8..827691c0663 100644 --- a/microprofile/weld/weld-core-impl/src/main/java/org/jboss/weld/bean/proxy/ProxyFactory.java +++ b/microprofile/weld/weld-core-impl/src/main/java/org/jboss/weld/bean/proxy/ProxyFactory.java @@ -79,13 +79,16 @@ /* * This class is copied from Weld sources. - * The only modified method is createCompoundProxyName. + * Modified methods: + * - getProxyName + * - createCompoundProxyName + * * Why? * In original Weld, the name is generated with bean identifier that is based on identity hashCode - and that is OK as * long as you run in a single JVM (which is the case with hotspot). * When running in native image, we go through the process of generating proxies at build time (in GraalVM when building the * native image) and then running them from the native image. - * As these are two separate instances of JVM, we get different identity has codes, and as a result different class names + * As these are two separate instances of JVM, we get different identity hash codes, and as a result different class names * at compile time and at runtime. The Helidon change ensures these names are equal and we can reuse the generated proxy * classes. * @@ -271,8 +274,14 @@ static String getProxyName(String contextId, Class proxiedBeanType, Set iface : typeInfo.getInterfaces()) { @@ -308,6 +317,33 @@ public void addInterfacesFromTypeClosure(Set typeClosure, Class< /* * Helidon modification + * + * This is used when there is no enclosing type and we may have multiple interfaces + * This method ensures the superinterface is the base of the name + */ + private static String createProxyName(TypeInfo typeInfo) { + Class superInterface = typeInfo.getSuperInterface(); + StringBuilder name = new StringBuilder(); + List interfaces = new ArrayList(); + for (Class type : typeInfo.getInterfaces()) { + if (!type.equals(superInterface)) { + interfaces.add(uniqueName(type)); + } + } + + Collections.sort(interfaces); + for (final String iface : interfaces) { + name.append(iface); + name.append('$'); + } + + return superInterface.getName() + '$' + name; + } + + /* + * Helidon modification + * + * Compound name is used when more than one interface needs to be proxied. */ private static String createCompoundProxyName(String contextId, Bean bean, TypeInfo typeInfo, StringBuilder name) { final List interfaces = new ArrayList(); From 74bfb48cd9c2f6008cd85f20181e6d02eaeec603 Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Tue, 11 Aug 2020 22:26:15 +0200 Subject: [PATCH 5/9] Update to proxy services - name is generated with super interface first - proxy classes are defined using private lookup when in original package - proxy classes are defined using a class loader for weld specific packages Signed-off-by: Tomas Langer --- .../io/helidon/microprofile/cdi/HelidonProxyServices.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java index 3218759377e..0e2eeeffd35 100644 --- a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java +++ b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java @@ -35,8 +35,8 @@ class HelidonProxyServices implements ProxyServices { private static final Logger LOGGER = Logger.getLogger(HelidonProxyServices.class.getName()); private static final String CLIENT_PROXY_SUFFIX = "$$Proxy$_$$_WeldClientProxy"; private static final String PROXY_SUFFIX = "$$Proxy$_$$_Weld$Proxy$"; - private static final String WELDX_PACKAGE_PREFIX = "org.jboss.weldx"; - private static final String WELD_LANG_PACKAGE_PREFIX = "org.jboss.weld.lang"; + private static final String WELD_JAVAX_PREFIX = "org.jboss.weldx."; + private static final String WELD_JAVA_PREFIX = "org.jboss.weld."; // a cache of all classloaders (this should be empty in most cases, as we use a single class loader in Helidon) private final Map classLoaders = Collections.synchronizedMap(new IdentityHashMap<>()); @@ -123,7 +123,7 @@ public Class loadClass(Class originalClass, String classBinaryName) throws } private boolean weldInternalProxyClassName(String className) { - return className.startsWith(WELDX_PACKAGE_PREFIX) || className.startsWith(WELD_LANG_PACKAGE_PREFIX); + return className.startsWith(WELD_JAVAX_PREFIX) || className.startsWith(WELD_JAVA_PREFIX); } private Class defineClassPrivateLookup(Class originalClass, String className, byte[] classBytes, int off, int len) { From 561734815864a068cb6a90562d102b8ebedbedf0 Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Tue, 11 Aug 2020 22:39:46 +0200 Subject: [PATCH 6/9] Adding opens statements for Helidon cdi module Signed-off-by: Tomas Langer --- .../health-checks/src/main/java/module-info.java | 2 +- .../access-log/src/main/java/module-info.java | 2 +- .../microprofile/cdi/HelidonProxyServices.java | 14 +++++++------- microprofile/config/src/main/java/module-info.java | 2 +- .../fault-tolerance/src/main/java/module-info.java | 2 +- microprofile/health/src/main/java/module-info.java | 2 +- .../jwt-auth/src/main/java/module-info.java | 2 +- .../metrics/src/main/java/module-info.java | 2 +- .../openapi/src/main/java/module-info.java | 2 +- .../security/src/main/java/module-info.java | 2 +- microprofile/server/src/main/java/module-info.java | 2 +- .../tracing/src/main/java/module-info.java | 2 +- .../websocket/src/main/java/module-info.java | 2 +- .../jersey/src/main/java/module-info.java | 2 +- security/security/src/main/java/module-info.java | 2 +- .../common/src/main/java/module-info.java | 2 +- .../mp-3/src/main/java/module-info.java | 2 +- 17 files changed, 23 insertions(+), 23 deletions(-) diff --git a/health/health-checks/src/main/java/module-info.java b/health/health-checks/src/main/java/module-info.java index 22bb21e3645..651e69c2008 100644 --- a/health/health-checks/src/main/java/module-info.java +++ b/health/health-checks/src/main/java/module-info.java @@ -33,5 +33,5 @@ exports io.helidon.health.checks; // required for CDI - opens io.helidon.health.checks to weld.core.impl; + opens io.helidon.health.checks to weld.core.impl, io.helidon.microprofile.cdi; } diff --git a/microprofile/access-log/src/main/java/module-info.java b/microprofile/access-log/src/main/java/module-info.java index 9d49455b837..b4673d9afd0 100644 --- a/microprofile/access-log/src/main/java/module-info.java +++ b/microprofile/access-log/src/main/java/module-info.java @@ -29,7 +29,7 @@ exports io.helidon.microprofile.accesslog; // this is needed for CDI extensions that use non-public observer methods - opens io.helidon.microprofile.accesslog to weld.core.impl; + opens io.helidon.microprofile.accesslog to weld.core.impl, io.helidon.microprofile.cdi; provides Extension with io.helidon.microprofile.accesslog.AccessLogCdiExtension; } diff --git a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java index 0e2eeeffd35..54491bd9ed9 100644 --- a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java +++ b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java @@ -138,13 +138,6 @@ private Class defineClassPrivateLookup(Class originalClass, String classNa MethodHandles.Lookup lookup; try { - Module classModule = originalClass.getModule(); - if (!myModule.canRead(classModule)) { - // we need to read the module to be able to create a private lookup in it - // it also needs to open the package we are doing the lookup in - myModule.addReads(classModule); - } - // lookup class name based on full class name (e.g. for inner classes) String lookupClassName; // lookup class name based on the first class in the name @@ -187,6 +180,13 @@ private Class defineClassPrivateLookup(Class originalClass, String classNa lookupClass = originalClass; } + Module lookupClassModule = lookupClass.getModule(); + if (!myModule.canRead(lookupClassModule)) { + // we need to read the module to be able to create a private lookup in it + // it also needs to open the package we are doing the lookup in + myModule.addReads(lookupClassModule); + } + // next line would fail if the module does not open its package, with a very meaningful error message lookup = MethodHandles.privateLookupIn(lookupClass, MethodHandles.lookup()); } catch (IllegalAccessException e) { diff --git a/microprofile/config/src/main/java/module-info.java b/microprofile/config/src/main/java/module-info.java index 15ce3099d00..5d59995beb5 100644 --- a/microprofile/config/src/main/java/module-info.java +++ b/microprofile/config/src/main/java/module-info.java @@ -31,7 +31,7 @@ exports io.helidon.microprofile.config; // this is needed for CDI extensions that use non-public observer methods - opens io.helidon.microprofile.config to weld.core.impl; + opens io.helidon.microprofile.config to weld.core.impl, io.helidon.microprofile.cdi; provides javax.enterprise.inject.spi.Extension with io.helidon.microprofile.config.ConfigCdiExtension; } diff --git a/microprofile/fault-tolerance/src/main/java/module-info.java b/microprofile/fault-tolerance/src/main/java/module-info.java index 5cd0370e38f..78facb8d3c4 100644 --- a/microprofile/fault-tolerance/src/main/java/module-info.java +++ b/microprofile/fault-tolerance/src/main/java/module-info.java @@ -42,7 +42,7 @@ exports io.helidon.microprofile.faulttolerance; // needed when running with modules - to make private methods accessible - opens io.helidon.microprofile.faulttolerance to weld.core.impl; + opens io.helidon.microprofile.faulttolerance to weld.core.impl, io.helidon.microprofile.cdi; provides javax.enterprise.inject.spi.Extension with io.helidon.microprofile.faulttolerance.FaultToleranceExtension; } diff --git a/microprofile/health/src/main/java/module-info.java b/microprofile/health/src/main/java/module-info.java index 323e2d571ab..30c7076bdb5 100644 --- a/microprofile/health/src/main/java/module-info.java +++ b/microprofile/health/src/main/java/module-info.java @@ -38,7 +38,7 @@ exports io.helidon.microprofile.health; // this is needed for CDI extensions that use non-public observer methods - opens io.helidon.microprofile.health to weld.core.impl; + opens io.helidon.microprofile.health to weld.core.impl, io.helidon.microprofile.cdi; uses io.helidon.microprofile.health.HealthCheckProvider; diff --git a/microprofile/jwt-auth/src/main/java/module-info.java b/microprofile/jwt-auth/src/main/java/module-info.java index 73732a840c4..4365a72b89c 100644 --- a/microprofile/jwt-auth/src/main/java/module-info.java +++ b/microprofile/jwt-auth/src/main/java/module-info.java @@ -41,7 +41,7 @@ exports io.helidon.microprofile.jwt.auth; // this is needed for CDI extensions that use non-public observer methods - opens io.helidon.microprofile.jwt.auth to weld.core.impl; + opens io.helidon.microprofile.jwt.auth to weld.core.impl, io.helidon.microprofile.cdi; provides io.helidon.security.providers.common.spi.AnnotationAnalyzer with io.helidon.microprofile.jwt.auth.JwtAuthAnnotationAnalyzer; provides io.helidon.security.spi.SecurityProviderService with io.helidon.microprofile.jwt.auth.JwtAuthProviderService; diff --git a/microprofile/metrics/src/main/java/module-info.java b/microprofile/metrics/src/main/java/module-info.java index 13295670106..545f50fe239 100644 --- a/microprofile/metrics/src/main/java/module-info.java +++ b/microprofile/metrics/src/main/java/module-info.java @@ -37,7 +37,7 @@ exports io.helidon.microprofile.metrics; // this is needed for CDI extensions that use non-public observer methods - opens io.helidon.microprofile.metrics to weld.core.impl; + opens io.helidon.microprofile.metrics to weld.core.impl, io.helidon.microprofile.cdi; provides javax.enterprise.inject.spi.Extension with io.helidon.microprofile.metrics.MetricsCdiExtension; } diff --git a/microprofile/openapi/src/main/java/module-info.java b/microprofile/openapi/src/main/java/module-info.java index aa07c3bcc31..c9181c6eb7a 100644 --- a/microprofile/openapi/src/main/java/module-info.java +++ b/microprofile/openapi/src/main/java/module-info.java @@ -33,7 +33,7 @@ exports io.helidon.microprofile.openapi; // this is needed for CDI extensions that use non-public observer methods - opens io.helidon.microprofile.openapi to weld.core.impl; + opens io.helidon.microprofile.openapi to weld.core.impl, io.helidon.microprofile.cdi; provides Extension with OpenApiCdiExtension; } diff --git a/microprofile/security/src/main/java/module-info.java b/microprofile/security/src/main/java/module-info.java index 878860ffbc7..1eafa54a60d 100644 --- a/microprofile/security/src/main/java/module-info.java +++ b/microprofile/security/src/main/java/module-info.java @@ -31,7 +31,7 @@ exports io.helidon.microprofile.security; // this is needed for CDI extensions that use non-public observer methods - opens io.helidon.microprofile.security to weld.core.impl; + opens io.helidon.microprofile.security to weld.core.impl, io.helidon.microprofile.cdi; provides javax.enterprise.inject.spi.Extension with io.helidon.microprofile.security.SecurityCdiExtension; } diff --git a/microprofile/server/src/main/java/module-info.java b/microprofile/server/src/main/java/module-info.java index 5158ce58fb9..c04977b3a68 100644 --- a/microprofile/server/src/main/java/module-info.java +++ b/microprofile/server/src/main/java/module-info.java @@ -46,5 +46,5 @@ io.helidon.microprofile.server.JaxRsCdiExtension; // needed when running with modules - to make private methods accessible - opens io.helidon.microprofile.server to weld.core.impl; + opens io.helidon.microprofile.server to weld.core.impl, io.helidon.microprofile.cdi; } diff --git a/microprofile/tracing/src/main/java/module-info.java b/microprofile/tracing/src/main/java/module-info.java index 45e4a048cc8..34fcccec7c9 100644 --- a/microprofile/tracing/src/main/java/module-info.java +++ b/microprofile/tracing/src/main/java/module-info.java @@ -47,7 +47,7 @@ exports io.helidon.microprofile.tracing; // this is needed for CDI extensions that use non-public observer methods - opens io.helidon.microprofile.tracing to weld.core.impl,hk2.utils; + opens io.helidon.microprofile.tracing to weld.core.impl,hk2.utils, io.helidon.microprofile.cdi; provides Extension with io.helidon.microprofile.tracing.TracingCdiExtension; provides org.glassfish.jersey.internal.spi.AutoDiscoverable with io.helidon.microprofile.tracing.MpTracingAutoDiscoverable; diff --git a/microprofile/websocket/src/main/java/module-info.java b/microprofile/websocket/src/main/java/module-info.java index 3ff03032627..3938e79f3bf 100644 --- a/microprofile/websocket/src/main/java/module-info.java +++ b/microprofile/websocket/src/main/java/module-info.java @@ -36,7 +36,7 @@ exports io.helidon.microprofile.tyrus; // this is needed for CDI extensions that use non-public observer methods - opens io.helidon.microprofile.tyrus to weld.core.impl; + opens io.helidon.microprofile.tyrus to weld.core.impl, io.helidon.microprofile.cdi; provides javax.enterprise.inject.spi.Extension with io.helidon.microprofile.tyrus.WebSocketCdiExtension; } diff --git a/security/integration/jersey/src/main/java/module-info.java b/security/integration/jersey/src/main/java/module-info.java index fcf00aecaf3..2b72a1c1e62 100644 --- a/security/integration/jersey/src/main/java/module-info.java +++ b/security/integration/jersey/src/main/java/module-info.java @@ -39,7 +39,7 @@ exports io.helidon.security.integration.jersey; // needed for jersey injection - opens io.helidon.security.integration.jersey to hk2.locator,hk2.utils,weld.core.impl; + opens io.helidon.security.integration.jersey to hk2.locator,hk2.utils,weld.core.impl, io.helidon.microprofile.cdi; uses io.helidon.security.providers.common.spi.AnnotationAnalyzer; } diff --git a/security/security/src/main/java/module-info.java b/security/security/src/main/java/module-info.java index 900c2054b0c..99df07d3f4b 100644 --- a/security/security/src/main/java/module-info.java +++ b/security/security/src/main/java/module-info.java @@ -43,7 +43,7 @@ exports io.helidon.security.internal to io.helidon.security.integration.jersey, io.helidon.security.integration.webserver, io.helidon.security.integration.grpc; // needed for CDI integration - opens io.helidon.security to weld.core.impl; + opens io.helidon.security to weld.core.impl, io.helidon.microprofile.cdi; uses io.helidon.security.spi.SecurityProviderService; } diff --git a/tests/apps/bookstore/common/src/main/java/module-info.java b/tests/apps/bookstore/common/src/main/java/module-info.java index 9a2a6761ea5..6873d919396 100644 --- a/tests/apps/bookstore/common/src/main/java/module-info.java +++ b/tests/apps/bookstore/common/src/main/java/module-info.java @@ -21,7 +21,7 @@ requires jakarta.enterprise.cdi.api; - opens io.helidon.tests.apps.bookstore.common to weld.core.impl; + opens io.helidon.tests.apps.bookstore.common to weld.core.impl, io.helidon.microprofile.cdi; exports io.helidon.tests.apps.bookstore.common; } diff --git a/tests/integration/native-image/mp-3/src/main/java/module-info.java b/tests/integration/native-image/mp-3/src/main/java/module-info.java index 6168ba6cb57..03f803a435f 100644 --- a/tests/integration/native-image/mp-3/src/main/java/module-info.java +++ b/tests/integration/native-image/mp-3/src/main/java/module-info.java @@ -4,5 +4,5 @@ exports io.helidon.tests.integration.nativeimage.mp3; - opens io.helidon.tests.integration.nativeimage.mp3 to weld.core.impl,hk2.utils; + opens io.helidon.tests.integration.nativeimage.mp3 to weld.core.impl,hk2.utils, io.helidon.microprofile.cdi; } \ No newline at end of file From 92f89db999e9ae8d627e3c5314a5450f8185e7f7 Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Wed, 12 Aug 2020 11:35:17 +0200 Subject: [PATCH 7/9] Simplify private lookup Signed-off-by: Tomas Langer --- .../cdi/HelidonProxyServices.java | 32 ++++--------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java index 54491bd9ed9..27f9bf22891 100644 --- a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java +++ b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java @@ -33,8 +33,6 @@ class HelidonProxyServices implements ProxyServices { private static final Logger LOGGER = Logger.getLogger(HelidonProxyServices.class.getName()); - private static final String CLIENT_PROXY_SUFFIX = "$$Proxy$_$$_WeldClientProxy"; - private static final String PROXY_SUFFIX = "$$Proxy$_$$_Weld$Proxy$"; private static final String WELD_JAVAX_PREFIX = "org.jboss.weldx."; private static final String WELD_JAVA_PREFIX = "org.jboss.weld."; @@ -138,43 +136,27 @@ private Class defineClassPrivateLookup(Class originalClass, String classNa MethodHandles.Lookup lookup; try { - // lookup class name based on full class name (e.g. for inner classes) + // lookup class name "guessed" from the class name of the proxy String lookupClassName; - // lookup class name based on the first class in the name - String fallbackLookupClassName; if (className.contains("$")) { - // fallback is the package + first name in the compound proxy class name - fallbackLookupClassName = className.substring(0, className.indexOf('$')); + // package + first name in the compound proxy class name + lookupClassName = className.substring(0, className.indexOf('$')); } else { - fallbackLookupClassName = className; + lookupClassName = className; } - // Let's try to get the actual class of the proxy - // the name ends with $$Proxy$_$$_WeldClientProxy or $$Proxy$_$$_Weld$Proxy$ - if (className.endsWith(CLIENT_PROXY_SUFFIX)) { - lookupClassName = className.substring(0, className.length() - CLIENT_PROXY_SUFFIX.length()); - } else if (className.endsWith(PROXY_SUFFIX)) { - lookupClassName = className.substring(0, className.length() - PROXY_SUFFIX.length()); - } else { - lookupClassName = fallbackLookupClassName; - } - - // I would like to create a private lookup in the same package as the proxied class, so let's - + // I would like to create a private lookup in the same package as the proxied class, so let's do it // first if the producer class and the lookup class name is the same, just use the existing class Class lookupClass = lookupClassName.equals(originalClass.getName()) ? originalClass : null; ClassLoader cl = originalClass.getClassLoader(); if (null == lookupClass) { - // try to load the full class name (may contain additional interfaces, so easily invalid) + // try to load the full class name lookupClass = tryLoading(cl, lookupClassName); } - if (null == lookupClass && !lookupClassName.equals(fallbackLookupClassName)) { - // fallback to the simple class name until the first $ sign - lookupClass = tryLoading(cl, fallbackLookupClassName); - } + if (null == lookupClass) { // and if that fails, just use the bean producer class lookupClass = originalClass; From 60eb5350ca857d4bb1833d95055ff3e89c21b9d5 Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Thu, 13 Aug 2020 11:08:31 +0200 Subject: [PATCH 8/9] Review comment. Signed-off-by: Tomas Langer --- .../cdi/HelidonProxyServices.java | 36 +++++++------------ 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java index 27f9bf22891..1f190ae7310 100644 --- a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java +++ b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java @@ -137,29 +137,18 @@ private Class defineClassPrivateLookup(Class originalClass, String classNa try { // lookup class name "guessed" from the class name of the proxy - String lookupClassName; + // proxy name must contain the $ - if it does not, we just use the originalClass as that is safe + int index = className.indexOf('$'); - if (className.contains("$")) { - // package + first name in the compound proxy class name - lookupClassName = className.substring(0, className.indexOf('$')); - } else { - lookupClassName = className; - } - - // I would like to create a private lookup in the same package as the proxied class, so let's do it - // first if the producer class and the lookup class name is the same, just use the existing class - Class lookupClass = lookupClassName.equals(originalClass.getName()) ? originalClass : null; - - ClassLoader cl = originalClass.getClassLoader(); - - if (null == lookupClass) { - // try to load the full class name - lookupClass = tryLoading(cl, lookupClassName); - } - - if (null == lookupClass) { - // and if that fails, just use the bean producer class + Class lookupClass; + if (index < 0) { + LOGGER.finest(() -> "Attempt to define a proxy class without a $ in its name. Class name: " + className + "," + + " original class name: " + originalClass.getName()); lookupClass = originalClass; + } else { + // I would like to create a private lookup in the same package as the proxied class, so let's do it + // use the "extracted" lookup class name, if that fails, use the original class + lookupClass = tryLoading(originalClass, className.substring(0, className.indexOf('$'))); } Module lookupClassModule = lookupClass.getModule(); @@ -178,13 +167,14 @@ private Class defineClassPrivateLookup(Class originalClass, String classNa return defineClass(lookup, className, classBytes, off, len); } - private Class tryLoading(ClassLoader cl, String className) { + private Class tryLoading(Class originalClass, String className) { try { + ClassLoader cl = originalClass.getClassLoader(); return cl.loadClass(className); } catch (Exception e) { LOGGER.log(Level.FINEST, "Attempt to load class " + className + " failed.", e); + return originalClass; } - return null; } private Class defineClass(MethodHandles.Lookup lookup, String className, byte[] classBytes, int off, int len) { From 0d6fba7f1a6a904d62607bab66d5f3a9cb95aa2b Mon Sep 17 00:00:00 2001 From: Tomas Langer Date: Thu, 13 Aug 2020 17:58:02 +0200 Subject: [PATCH 9/9] Review comment again. Signed-off-by: Tomas Langer --- .../io/helidon/microprofile/cdi/HelidonProxyServices.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java index 1f190ae7310..67031991e1c 100644 --- a/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java +++ b/microprofile/cdi/src/main/java/io/helidon/microprofile/cdi/HelidonProxyServices.java @@ -143,12 +143,12 @@ private Class defineClassPrivateLookup(Class originalClass, String classNa Class lookupClass; if (index < 0) { LOGGER.finest(() -> "Attempt to define a proxy class without a $ in its name. Class name: " + className + "," - + " original class name: " + originalClass.getName()); + + " original class name: " + originalClass.getName()); lookupClass = originalClass; } else { // I would like to create a private lookup in the same package as the proxied class, so let's do it // use the "extracted" lookup class name, if that fails, use the original class - lookupClass = tryLoading(originalClass, className.substring(0, className.indexOf('$'))); + lookupClass = tryLoading(originalClass, className.substring(0, index)); } Module lookupClassModule = lookupClass.getModule(); @@ -169,8 +169,7 @@ private Class defineClassPrivateLookup(Class originalClass, String classNa private Class tryLoading(Class originalClass, String className) { try { - ClassLoader cl = originalClass.getClassLoader(); - return cl.loadClass(className); + return originalClass.getClassLoader().loadClass(className); } catch (Exception e) { LOGGER.log(Level.FINEST, "Attempt to load class " + className + " failed.", e); return originalClass;