From 2f9535cb73753c55ab503bd3bfb2c6d77b6acef6 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Wed, 30 Jan 2019 22:10:16 +0100 Subject: [PATCH 1/2] RestClientProcessor - replace generated bean classes with BeanRegistrar - resolves #690 --- .../restclient/RestClientProcessor.java | 71 ++++++++++--------- .../arc/processor/BeanConfigurator.java | 6 ++ 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/extensions/rest-client/deployment/src/main/java/org/jboss/shamrock/restclient/RestClientProcessor.java b/extensions/rest-client/deployment/src/main/java/org/jboss/shamrock/restclient/RestClientProcessor.java index 75b8daea8c525..c643942f4032a 100644 --- a/extensions/rest-client/deployment/src/main/java/org/jboss/shamrock/restclient/RestClientProcessor.java +++ b/extensions/rest-client/deployment/src/main/java/org/jboss/shamrock/restclient/RestClientProcessor.java @@ -19,11 +19,7 @@ import java.lang.reflect.Modifier; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.atomic.AtomicReference; -import javax.enterprise.context.ApplicationScoped; -import javax.enterprise.context.Dependent; -import javax.enterprise.inject.Produces; import javax.inject.Inject; import javax.ws.rs.Path; import javax.ws.rs.client.ClientRequestFilter; @@ -37,9 +33,11 @@ import org.jboss.jandex.AnnotationTarget; import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; -import org.jboss.protean.gizmo.ClassCreator; -import org.jboss.protean.gizmo.ClassOutput; -import org.jboss.protean.gizmo.MethodCreator; +import org.jboss.jandex.Type; +import org.jboss.jandex.Type.Kind; +import org.jboss.protean.arc.processor.BeanConfigurator; +import org.jboss.protean.arc.processor.BeanRegistrar; +import org.jboss.protean.arc.processor.ScopeInfo; import org.jboss.protean.gizmo.MethodDescriptor; import org.jboss.protean.gizmo.ResultHandle; import org.jboss.resteasy.client.jaxrs.internal.proxy.ResteasyClientProxy; @@ -47,7 +45,7 @@ import org.jboss.shamrock.deployment.annotations.BuildProducer; import org.jboss.shamrock.deployment.annotations.BuildStep; import org.jboss.shamrock.arc.deployment.AdditionalBeanBuildItem; -import org.jboss.shamrock.arc.deployment.GeneratedBeanBuildItem; +import org.jboss.shamrock.arc.deployment.BeanRegistrarBuildItem; import org.jboss.shamrock.deployment.builditem.CombinedIndexBuildItem; import org.jboss.shamrock.deployment.builditem.FeatureBuildItem; import org.jboss.shamrock.deployment.builditem.GeneratedClassBuildItem; @@ -59,6 +57,8 @@ import org.jboss.shamrock.restclient.runtime.RestClientProxy; class RestClientProcessor { + + private static final DotName REST_CLIENT = DotName.createSimple(RestClient.class.getName()); private static final DotName[] CLIENT_ANNOTATIONS = { DotName.createSimple("javax.ws.rs.GET"), @@ -88,11 +88,17 @@ class RestClientProcessor { @Inject BuildProducer resources; + @Inject + BuildProducer beanRegistrars; + + @Inject + BuildProducer feature; + @Inject CombinedIndexBuildItem combinedIndexBuildItem; @BuildStep - public void build(BuildProducer generatedBeans, BuildProducer feature) throws Exception { + public void build() throws Exception { feature.produce(new FeatureBuildItem(FeatureBuildItem.MP_REST_CLIENT)); reflectiveClass.produce(new ReflectiveClassBuildItem(false, false, DefaultResponseExceptionMapper.class.getName(), @@ -133,33 +139,28 @@ public void build(BuildProducer generatedBeans, BuildPro proxyDefinition.produce(new SubstrateProxyDefinitionBuildItem(iName, ResteasyClientProxy.class.getName())); proxyDefinition.produce(new SubstrateProxyDefinitionBuildItem(iName, RestClientProxy.class.getName())); reflectiveClass.produce(new ReflectiveClassBuildItem(true, false, iName)); - - //now generate CDI beans - //TODO: do we need to check if CDI is enabled? Are we just assuming it always is? - String className = iName + "$$RestClientProxy"; - AtomicReference bytes = new AtomicReference<>(); - try (ClassCreator creator = new ClassCreator(new ClassOutput() { - @Override - public void write(String name, byte[] data) { - bytes.set(data); - generatedClass.produce(new GeneratedClassBuildItem(true, name, data)); + } + + BeanRegistrar beanRegistrar = new BeanRegistrar() { + + @Override + public void register(RegistrationContext registrationContext) { + for (Map.Entry entry : interfaces.entrySet()) { + BeanConfigurator configurator = registrationContext.configure(RestClientBase.class); + configurator.types(Type.create(entry.getValue().name(), Kind.CLASS)); + configurator.scope(ScopeInfo.APPLICATION); + configurator.addQualifier(REST_CLIENT); + configurator.creator(m -> { + // return new RestClientBase(proxyType).create(); + ResultHandle interfaceHandle = m.loadClass(entry.getValue().name().toString()); + ResultHandle baseHandle = m.newInstance(MethodDescriptor.ofConstructor(RestClientBase.class, Class.class), interfaceHandle); + ResultHandle ret = m.invokeVirtualMethod(MethodDescriptor.ofMethod(RestClientBase.class, "create", Object.class), baseHandle); + m.returnValue(ret); + }); + configurator.done(); } - }, className, null, RestClientBase.class.getName())) { - - creator.addAnnotation(Dependent.class); - MethodCreator producer = creator.getMethodCreator("producerMethod", iName); - producer.addAnnotation(Produces.class); - producer.addAnnotation(RestClient.class); - producer.addAnnotation(ApplicationScoped.class); - - ResultHandle ret = producer.invokeVirtualMethod(MethodDescriptor.ofMethod(RestClientBase.class, "create", Object.class), producer.getThis()); - producer.returnValue(ret); - - MethodCreator ctor = creator.getMethodCreator(MethodDescriptor.ofConstructor(className)); - ctor.invokeSpecialMethod(MethodDescriptor.ofConstructor(RestClientBase.class, Class.class), ctor.getThis(), ctor.loadClass(iName)); - ctor.returnValue(null); } - generatedBeans.produce(new GeneratedBeanBuildItem(className, bytes.get())); - } + }; + beanRegistrars.produce(new BeanRegistrarBuildItem(beanRegistrar)); } } diff --git a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanConfigurator.java b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanConfigurator.java index 733cace0b5fc2..8cf6566173743 100644 --- a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanConfigurator.java +++ b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanConfigurator.java @@ -26,6 +26,7 @@ import javax.enterprise.context.spi.CreationalContext; import org.jboss.jandex.AnnotationInstance; +import org.jboss.jandex.AnnotationValue; import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; import org.jboss.jandex.Type; @@ -132,6 +133,11 @@ public BeanConfigurator types(Type... types) { Collections.addAll(this.types, types); return this; } + + public BeanConfigurator addQualifier(DotName annotationName) { + this.qualifiers.add(AnnotationInstance.create(annotationName, null, new AnnotationValue[] {})); + return this; + } public BeanConfigurator qualifiers(AnnotationInstance... qualifiers) { Collections.addAll(this.qualifiers, qualifiers); From 151464cc25371c7597f5121855b66d9d9e593be7 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Thu, 31 Jan 2019 09:39:16 +0100 Subject: [PATCH 2/2] Only rest client interfaces annotated with RegisterRestClient registered as beans - this behavior follows the spec - also fixed some bugs in ArC --- .../restclient/RestClientProcessor.java | 60 ++++++++----------- .../arc/processor/BeanConfigurator.java | 16 ++--- .../protean/arc/processor/BeanDeployment.java | 4 +- .../protean/arc/processor/BeanGenerator.java | 4 +- .../protean/arc/processor/BeanRegistrar.java | 8 ++- .../arc/processor/ClientProxyGenerator.java | 2 + .../protean/arc/InjectableInterceptor.java | 33 +--------- .../shamrock/example/rest/ClientResource.java | 1 - .../shamrock/example/rest/RestInterface.java | 3 + 9 files changed, 54 insertions(+), 77 deletions(-) diff --git a/extensions/rest-client/deployment/src/main/java/org/jboss/shamrock/restclient/RestClientProcessor.java b/extensions/rest-client/deployment/src/main/java/org/jboss/shamrock/restclient/RestClientProcessor.java index c643942f4032a..ebc9f9c0f4687 100644 --- a/extensions/rest-client/deployment/src/main/java/org/jboss/shamrock/restclient/RestClientProcessor.java +++ b/extensions/rest-client/deployment/src/main/java/org/jboss/shamrock/restclient/RestClientProcessor.java @@ -21,7 +21,6 @@ import java.util.Map; import javax.inject.Inject; -import javax.ws.rs.Path; import javax.ws.rs.client.ClientRequestFilter; import javax.ws.rs.client.ClientResponseFilter; @@ -33,8 +32,6 @@ import org.jboss.jandex.AnnotationTarget; import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; -import org.jboss.jandex.Type; -import org.jboss.jandex.Type.Kind; import org.jboss.protean.arc.processor.BeanConfigurator; import org.jboss.protean.arc.processor.BeanRegistrar; import org.jboss.protean.arc.processor.ScopeInfo; @@ -60,18 +57,7 @@ class RestClientProcessor { private static final DotName REST_CLIENT = DotName.createSimple(RestClient.class.getName()); - private static final DotName[] CLIENT_ANNOTATIONS = { - DotName.createSimple("javax.ws.rs.GET"), - DotName.createSimple("javax.ws.rs.HEAD"), - DotName.createSimple("javax.ws.rs.DELETE"), - DotName.createSimple("javax.ws.rs.OPTIONS"), - DotName.createSimple("javax.ws.rs.PATCH"), - DotName.createSimple("javax.ws.rs.POST"), - DotName.createSimple("javax.ws.rs.PUT"), - DotName.createSimple("javax.ws.rs.PUT"), - DotName.createSimple(RegisterRestClient.class.getName()), - DotName.createSimple(Path.class.getName()) - }; + private static final DotName REGISTER_REST_CLIENT = DotName.createSimple(RegisterRestClient.class.getName()); @Inject BuildProducer generatedClass; @@ -115,23 +101,27 @@ public void build() throws Exception { reflectiveClass.produce(new ReflectiveClassBuildItem(false, false, "com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl")); reflectiveClass.produce(new ReflectiveClassBuildItem(true, true, "org.jboss.resteasy.plugins.providers.jsonb.JsonBindingProvider", "org.jboss.resteasy.plugins.providers.jsonb.AbstractJsonBindingProvider")); proxyDefinition.produce(new SubstrateProxyDefinitionBuildItem(ResteasyConfiguration.class.getName())); + + // According to the spec only rest client interfaces annotated with RegisterRestClient are registered as beans Map interfaces = new HashMap<>(); - for (DotName type : CLIENT_ANNOTATIONS) { - for (AnnotationInstance annotation : combinedIndexBuildItem.getIndex().getAnnotations(type)) { - AnnotationTarget target = annotation.target(); - ClassInfo theInfo; - if (target.kind() == AnnotationTarget.Kind.CLASS) { - theInfo = target.asClass(); - } else if (target.kind() == AnnotationTarget.Kind.METHOD) { - theInfo = target.asMethod().declaringClass(); - } else { - continue; - } - if (!Modifier.isInterface(theInfo.flags())) { - continue; - } - interfaces.put(theInfo.name(), theInfo); + for (AnnotationInstance annotation : combinedIndexBuildItem.getIndex().getAnnotations(REGISTER_REST_CLIENT)) { + AnnotationTarget target = annotation.target(); + ClassInfo theInfo; + if (target.kind() == AnnotationTarget.Kind.CLASS) { + theInfo = target.asClass(); + } else if (target.kind() == AnnotationTarget.Kind.METHOD) { + theInfo = target.asMethod().declaringClass(); + } else { + continue; } + if (!Modifier.isInterface(theInfo.flags())) { + continue; + } + interfaces.put(theInfo.name(), theInfo); + } + + if (interfaces.isEmpty()) { + return; } for (Map.Entry entry : interfaces.entrySet()) { @@ -146,13 +136,15 @@ public void build() throws Exception { @Override public void register(RegistrationContext registrationContext) { for (Map.Entry entry : interfaces.entrySet()) { - BeanConfigurator configurator = registrationContext.configure(RestClientBase.class); - configurator.types(Type.create(entry.getValue().name(), Kind.CLASS)); - configurator.scope(ScopeInfo.APPLICATION); + BeanConfigurator configurator = registrationContext.configure(entry.getKey()); + // The spec is not clear whether we should add superinterfaces too - let's keep aligned with SmallRye for now + configurator.addType(entry.getKey()); + // We use @Singleton here as we do not need another proxy + configurator.scope(ScopeInfo.SINGLETON); configurator.addQualifier(REST_CLIENT); configurator.creator(m -> { // return new RestClientBase(proxyType).create(); - ResultHandle interfaceHandle = m.loadClass(entry.getValue().name().toString()); + ResultHandle interfaceHandle = m.loadClass(entry.getKey().toString()); ResultHandle baseHandle = m.newInstance(MethodDescriptor.ofConstructor(RestClientBase.class, Class.class), interfaceHandle); ResultHandle ret = m.invokeVirtualMethod(MethodDescriptor.ofMethod(RestClientBase.class, "create", Object.class), baseHandle); m.returnValue(ret); diff --git a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanConfigurator.java b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanConfigurator.java index 8cf6566173743..d1a35b0c9760a 100644 --- a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanConfigurator.java +++ b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanConfigurator.java @@ -20,6 +20,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.Consumer; @@ -71,18 +72,14 @@ public final class BeanConfigurator { /** * - * @param implClass + * @param implClassName * @param beanDeployment * @param beanConsumer */ - BeanConfigurator(Class implClass, BeanDeployment beanDeployment, Consumer beanConsumer) { + BeanConfigurator(DotName implClassName, BeanDeployment beanDeployment, Consumer beanConsumer) { + this.implClass = beanDeployment.getIndex().getClassByName(Objects.requireNonNull(implClassName)); this.beanDeployment = beanDeployment; this.beanConsumer = beanConsumer; - this.implClass = beanDeployment.getIndex().getClassByName(DotName.createSimple(implClass.getName())); - if (this.implClass == null) { - // TODO we have a problem - throw new IllegalArgumentException(); - } this.types = new HashSet<>(); this.qualifiers = new HashSet<>(); this.scope = ScopeInfo.DEPENDENT; @@ -134,6 +131,11 @@ public BeanConfigurator types(Type... types) { return this; } + public BeanConfigurator addType(DotName className) { + this.types.add(Type.create(className, Kind.CLASS)); + return this; + } + public BeanConfigurator addQualifier(DotName annotationName) { this.qualifiers.add(AnnotationInstance.create(annotationName, null, new AnnotationValue[] {})); return this; diff --git a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanDeployment.java b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanDeployment.java index 964c0ce148cc7..b29e0184cbd9f 100644 --- a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanDeployment.java +++ b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanDeployment.java @@ -127,8 +127,8 @@ public class BeanDeployment { RegistrationContext registrationContext = new RegistrationContext() { @Override - public BeanConfigurator configure(Class beanClass) { - return new BeanConfigurator(beanClass, BeanDeployment.this, beans::add); + public BeanConfigurator configure(DotName beanClassName) { + return new BeanConfigurator(beanClassName, BeanDeployment.this, beans::add); } @Override diff --git a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanGenerator.java b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanGenerator.java index 41a8d547ae443..70589b640d524 100644 --- a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanGenerator.java +++ b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanGenerator.java @@ -139,11 +139,13 @@ Collection generateSyntheticBean(BeanInfo bean, ReflectionRegistration } else { baseName = DotNames.simpleName(bean.getImplClazz().name()); } + baseName += SYNTHETIC_SUFFIX; + Type providerType = bean.getProviderType(); ClassInfo providerClass = bean.getDeployment().getIndex().getClassByName(providerType.name()); String providerTypeName = providerClass.name().toString(); String targetPackage = getPackageName(bean); - String generatedName = targetPackage.replace('.', '/') + "/" + baseName + SYNTHETIC_SUFFIX + BEAN_SUFFIX; + String generatedName = targetPackage.replace('.', '/') + "/" + baseName + BEAN_SUFFIX; boolean isApplicationClass = applicationClassPredicate.test(bean.getImplClazz().name()); ResourceClassOutput classOutput = new ResourceClassOutput(isApplicationClass, name -> name.equals(generatedName) ? SpecialType.BEAN : null); diff --git a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanRegistrar.java b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanRegistrar.java index da673733a99c6..d1ccb03fcb329 100644 --- a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanRegistrar.java +++ b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/BeanRegistrar.java @@ -16,6 +16,7 @@ package org.jboss.protean.arc.processor; +import org.jboss.jandex.DotName; import org.jboss.protean.arc.InjectableBean; /** @@ -31,7 +32,6 @@ public interface BeanRegistrar extends BuildExtension { */ void register(RegistrationContext registrationContext); - interface RegistrationContext extends BuildContext { /** @@ -39,7 +39,11 @@ interface RegistrationContext extends BuildContext { * @param beanClass * @return a new synthetic bean builder */ - BeanConfigurator configure(Class beanClass); + BeanConfigurator configure(DotName beanClassName); + + default BeanConfigurator configure(Class beanClass) { + return configure(DotName.createSimple(beanClass.getName())); + } // TODO add synthetic observer? diff --git a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/ClientProxyGenerator.java b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/ClientProxyGenerator.java index 722201d59ddf1..a97c696d45c40 100644 --- a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/ClientProxyGenerator.java +++ b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/ClientProxyGenerator.java @@ -199,6 +199,8 @@ Collection getDelegatingMethods(BeanInfo bean) { resolved = Types.buildResolvedMap(producerField.type().asParameterizedType().arguments(), fieldClass.typeParameters(), Collections.emptyMap()); } Methods.addDelegatingMethods(bean.getDeployment().getIndex(), fieldClass, resolved, methods); + } else if (bean.isSynthetic()) { + Methods.addDelegatingMethods(bean.getDeployment().getIndex(), bean.getImplClazz(), Collections.emptyMap(), methods); } return methods.values(); } diff --git a/independent-projects/arc/runtime/src/main/java/org/jboss/protean/arc/InjectableInterceptor.java b/independent-projects/arc/runtime/src/main/java/org/jboss/protean/arc/InjectableInterceptor.java index c8b3f8e3ed610..56eaca4067b09 100644 --- a/independent-projects/arc/runtime/src/main/java/org/jboss/protean/arc/InjectableInterceptor.java +++ b/independent-projects/arc/runtime/src/main/java/org/jboss/protean/arc/InjectableInterceptor.java @@ -16,43 +16,16 @@ package org.jboss.protean.arc; -import java.lang.annotation.Annotation; -import java.util.Set; - -import javax.enterprise.inject.spi.InterceptionType; -import javax.interceptor.InvocationContext; +import javax.enterprise.inject.spi.Interceptor; /** - * Represents an injectable interceptor bean. It is an alternative to {@link javax.enterprise.inject.spi.Interceptor}. + * Represents an interceptor bean. * * @author Martin Kouba * * @param */ -public interface InjectableInterceptor extends InjectableBean { - - /** - * - * @return the interceptor bindings - */ - Set getInterceptorBindings(); - - /** - * - * @param type - * @return {@code true} if this interceptor intercepts the given kind of interception, {@code false} othewise - */ - boolean intercepts(InterceptionType type); - - /** - * - * @param type - * @param instance - * @param ctx - * @return the invocation return value - * @throws Exception - */ - Object intercept(InterceptionType type, T instance, InvocationContext ctx) throws Exception; +public interface InjectableInterceptor extends InjectableBean, Interceptor { /** * diff --git a/integration-tests/main/src/main/java/org/jboss/shamrock/example/rest/ClientResource.java b/integration-tests/main/src/main/java/org/jboss/shamrock/example/rest/ClientResource.java index ab5c0f9fea835..d06007f87b222 100644 --- a/integration-tests/main/src/main/java/org/jboss/shamrock/example/rest/ClientResource.java +++ b/integration-tests/main/src/main/java/org/jboss/shamrock/example/rest/ClientResource.java @@ -35,7 +35,6 @@ public class ClientResource { @GET @Path("/manual") public String manual() throws Exception { - RestInterface iface = RestClientBuilder.newBuilder() .baseUrl(new URL("http", "localhost", 8080, "/")) .build(RestInterface.class); diff --git a/integration-tests/main/src/main/java/org/jboss/shamrock/example/rest/RestInterface.java b/integration-tests/main/src/main/java/org/jboss/shamrock/example/rest/RestInterface.java index 478e4dee81ed5..2917e8cf3b8d8 100644 --- a/integration-tests/main/src/main/java/org/jboss/shamrock/example/rest/RestInterface.java +++ b/integration-tests/main/src/main/java/org/jboss/shamrock/example/rest/RestInterface.java @@ -19,6 +19,9 @@ import javax.ws.rs.GET; import javax.ws.rs.Path; +import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; + +@RegisterRestClient @Path("/test") public interface RestInterface {