From 4d985993d88b69b5a558a6a3e6a57e2bafd3dc75 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Mon, 4 Feb 2019 10:11:43 +0100 Subject: [PATCH 1/2] Get rid of indexes in ObserverGenerator and AnnotationLiteralProcessor - resolves #683 --- .../processor/AnnotationLiteralGenerator.java | 15 ++++++------ .../processor/AnnotationLiteralProcessor.java | 12 ++++------ .../arc/processor/ClientProxyGenerator.java | 13 +++++++---- .../arc/processor/ObserverGenerator.java | 23 +++++++++++++------ 4 files changed, 37 insertions(+), 26 deletions(-) diff --git a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/AnnotationLiteralGenerator.java b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/AnnotationLiteralGenerator.java index fe627adc79fd3..f9f8b3349ca1d 100644 --- a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/AnnotationLiteralGenerator.java +++ b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/AnnotationLiteralGenerator.java @@ -61,6 +61,8 @@ public class AnnotationLiteralGenerator extends AbstractGenerator { static final String ANNOTATION_LITERAL_SUFFIX = "_AnnotationLiteral"; + static final String SHARED_SUFFIX = "_Shared"; + private static final Logger LOGGER = Logger.getLogger(AnnotationLiteralGenerator.class); /** @@ -292,15 +294,14 @@ static String componentType(MethodInfo method) { return arrayType.component().name().toString(); } - static String generatedSharedName(DotName annotationName, AtomicInteger index) { - // com.foo.MyQualifier -> com.foo.MyQualifier1_AnnotationLiteral - return DotNames.packageName(annotationName) + "." + DotNames.simpleName(annotationName) + index.incrementAndGet() - + AnnotationLiteralGenerator.ANNOTATION_LITERAL_SUFFIX; + static String generatedSharedName(DotName annotationName) { + // com.foo.MyQualifier -> com.foo.MyQualifier1_Shared_AnnotationLiteral + return DotNames.packageName(annotationName) + "." + DotNames.simpleName(annotationName) + SHARED_SUFFIX + ANNOTATION_LITERAL_SUFFIX; } - static String generatedLocalName(String targetPackage, String simpleName, AtomicInteger index) { - // com.foo.MyQualifier -> com.bar.MyQualifier1_AnnotationLiteral - return targetPackage + "." + simpleName + index.incrementAndGet() + AnnotationLiteralGenerator.ANNOTATION_LITERAL_SUFFIX; + static String generatedLocalName(String targetPackage, String simpleName, String hash) { + // com.foo.MyQualifier -> com.bar.MyQualifier_somehashvalue_AnnotationLiteral + return targetPackage + "." + simpleName + hash + AnnotationLiteralGenerator.ANNOTATION_LITERAL_SUFFIX; } } diff --git a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/AnnotationLiteralProcessor.java b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/AnnotationLiteralProcessor.java index 5151186687c94..af9696365bfc2 100644 --- a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/AnnotationLiteralProcessor.java +++ b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/AnnotationLiteralProcessor.java @@ -20,7 +20,6 @@ import java.util.ListIterator; import java.util.Map; import java.util.Objects; -import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -42,14 +41,11 @@ */ class AnnotationLiteralProcessor { - private final AtomicInteger index; - private final ComputingCache cache; AnnotationLiteralProcessor(boolean shared, Predicate applicationClassPredicate) { - this.index = new AtomicInteger(1); this.cache = shared ? new ComputingCache<>(key -> { - return new Literal(AnnotationLiteralGenerator.generatedSharedName(key.annotationName, index), applicationClassPredicate.test(key.annotationName), + return new Literal(AnnotationLiteralGenerator.generatedSharedName(key.annotationName), applicationClassPredicate.test(key.annotationName), key.annotationClass.methods() .stream() .filter(m -> !m.name().equals(Methods.CLINIT) && !m.name().equals(Methods.INIT)) @@ -72,7 +68,7 @@ ComputingCache getCache() { * @param annotationClass * @param annotationInstance * @param targetPackage - * @return an annotation literal class name + * @return an annotation literal result handle */ ResultHandle process(BytecodeCreator bytecode, ClassOutput classOutput, ClassInfo annotationClass, AnnotationInstance annotationInstance, String targetPackage) { @@ -99,9 +95,9 @@ ResultHandle process(BytecodeCreator bytecode, ClassOutput classOutput, ClassInf return bytecode .newInstance(MethodDescriptor.ofConstructor(literal.className, literal.constructorParams.stream().map(m -> m.returnType().name().toString()).toArray()), constructorParams); - } else { - String literalClassName = AnnotationLiteralGenerator.generatedLocalName(targetPackage, DotNames.simpleName(annotationClass.name()), index); + String literalClassName = AnnotationLiteralGenerator.generatedLocalName(targetPackage, DotNames.simpleName(annotationClass.name()), + Hashes.sha1(annotationInstance.toString())); AnnotationLiteralGenerator.createAnnotationLiteral(classOutput, annotationClass, annotationInstance, literalClassName); return bytecode.newInstance(MethodDescriptor.ofConstructor(literalClassName)); } 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 fc714b889db1c..5e2c2e5b1e17d 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 @@ -41,6 +41,8 @@ import org.jboss.protean.arc.InjectableBean; import org.jboss.protean.arc.InjectableContext; import org.jboss.protean.arc.processor.ResourceOutput.Resource; +import org.jboss.protean.gizmo.AssignableResultHandle; +import org.jboss.protean.gizmo.BytecodeCreator; import org.jboss.protean.gizmo.ClassCreator; import org.jboss.protean.gizmo.DescriptorUtils; import org.jboss.protean.gizmo.FieldCreator; @@ -178,10 +180,13 @@ void implementDelegate(ClassCreator clientProxy, String providerTypeName, FieldD // getContext() ResultHandle context = creator.invokeInterfaceMethod(MethodDescriptor.ofMethod(ArcContainer.class, "getContext", InjectableContext.class, Class.class), container, scope); - // new CreationalContextImpl<>() - ResultHandle creationContext = creator.newInstance(MethodDescriptor.ofConstructor(CreationalContextImpl.class)); - ResultHandle result = creator.invokeInterfaceMethod(MethodDescriptors.CONTEXT_GET, context, bean, creationContext); - creator.returnValue(result); + AssignableResultHandle ret = creator.createVariable(Object.class); + creator.assign(ret, creator.invokeInterfaceMethod(MethodDescriptors.CONTEXT_GET_IF_PRESENT, context, bean)); + BytecodeCreator isNullBranch = creator.ifNull(ret).trueBranch(); + // Create a new contextual instance - new CreationalContextImpl<>() + ResultHandle creationContext = isNullBranch.newInstance(MethodDescriptor.ofConstructor(CreationalContextImpl.class)); + isNullBranch.assign(ret, isNullBranch.invokeInterfaceMethod(MethodDescriptors.CONTEXT_GET, context, bean, creationContext)); + creator.returnValue(ret); } void implementGetContextualInstance(ClassCreator clientProxy, String providerTypeName) { diff --git a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/ObserverGenerator.java b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/ObserverGenerator.java index b044b10fb2dfd..04b013b8f1887 100644 --- a/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/ObserverGenerator.java +++ b/independent-projects/arc/processor/src/main/java/org/jboss/protean/arc/processor/ObserverGenerator.java @@ -31,7 +31,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; import javax.enterprise.inject.spi.EventContext; @@ -67,8 +66,6 @@ public class ObserverGenerator extends AbstractGenerator { private static final Logger LOGGER = Logger.getLogger(ObserverGenerator.class); - private static final AtomicInteger OBSERVER_INDEX = new AtomicInteger(); - private final AnnotationLiteralProcessor annotationLiterals; private final Predicate applicationClassPredicate; @@ -97,13 +94,20 @@ Collection generate(ObserverInfo observer, ReflectionRegistration refl declaringClassBase = DotNames.simpleName(declaringClass.name()); } - String baseName = declaringClassBase + OBSERVER_SUFFIX + OBSERVER_INDEX.incrementAndGet(); + StringBuilder sigBuilder = new StringBuilder(); + sigBuilder.append(observer.getObserverMethod().name()) + .append("_") + .append(observer.getObserverMethod().returnType().name().toString()); + for(org.jboss.jandex.Type paramType : observer.getObserverMethod().parameters()) { + sigBuilder.append(paramType.name().toString()); + } + String baseName = declaringClassBase + OBSERVER_SUFFIX + "_" + observer.getObserverMethod().name() + "_" + Hashes.sha1(sigBuilder.toString()); String generatedName = DotNames.packageName(declaringClass.name()).replace('.', '/') + "/" + baseName; boolean isApplicationClass = applicationClassPredicate.test(observer.getObserverMethod().declaringClass().name()); ResourceClassOutput classOutput = new ResourceClassOutput(isApplicationClass, name -> name.equals(generatedName) ? SpecialType.OBSERVER : null); - // Foo_Observer1 implements ObserverMethod + // Foo_Observer_fooMethod_hash implements ObserverMethod ClassCreator observerCreator = ClassCreator.builder().classOutput(classOutput).className(generatedName).interfaces(InjectableObserverMethod.class) .build(); @@ -176,7 +180,10 @@ protected void implementNotify(ObserverInfo observer, ClassCreator observerCreat ResultHandle declaringProviderHandle = notify .readInstanceField(FieldDescriptor.of(observerCreator.getClassName(), "declaringProvider", InjectableBean.class.getName()), notify.getThis()); - ResultHandle ctxHandle = notify.newInstance(MethodDescriptor.ofConstructor(CreationalContextImpl.class)); + + // It is safe to skip CreationalContext.release() for normal scoped declaring provider and no injection points + boolean skipRelease = observer.getDeclaringBean().getScope().isNormal() && observer.getInjection().injectionPoints.isEmpty(); + ResultHandle ctxHandle = skipRelease ? notify.loadNull() : notify.newInstance(MethodDescriptor.ofConstructor(CreationalContextImpl.class)); ResultHandle declaringProviderInstanceHandle = notify.invokeInterfaceMethod(MethodDescriptors.INJECTABLE_REF_PROVIDER_GET, declaringProviderHandle, ctxHandle); @@ -222,7 +229,9 @@ protected void implementNotify(ObserverInfo observer, ClassCreator observerCreat } // Destroy @Dependent instances injected into method parameters of an observer method - notify.invokeInterfaceMethod(MethodDescriptors.CREATIONAL_CTX_RELEASE, ctxHandle); + if (!skipRelease) { + notify.invokeInterfaceMethod(MethodDescriptors.CREATIONAL_CTX_RELEASE, ctxHandle); + } // If the declaring bean is @Dependent we must destroy the instance afterwards if (ScopeInfo.DEPENDENT.equals(observer.getDeclaringBean().getScope())) { From 7175b85ecdcf5f763dd52f072707bc8714c940a9 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Tue, 5 Feb 2019 08:40:52 +0100 Subject: [PATCH 2/2] Scheduler invoker - also get rid of index suffix --- .../deployment/SchedulerProcessor.java | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/extensions/scheduler/deployment/src/main/java/org/jboss/shamrock/scheduler/deployment/SchedulerProcessor.java b/extensions/scheduler/deployment/src/main/java/org/jboss/shamrock/scheduler/deployment/SchedulerProcessor.java index 05bdea24001c1..71580918ab45f 100644 --- a/extensions/scheduler/deployment/src/main/java/org/jboss/shamrock/scheduler/deployment/SchedulerProcessor.java +++ b/extensions/scheduler/deployment/src/main/java/org/jboss/shamrock/scheduler/deployment/SchedulerProcessor.java @@ -17,15 +17,18 @@ import static org.jboss.shamrock.deployment.annotations.ExecutionTime.STATIC_INIT; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.text.ParseException; import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; +import java.util.Base64; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.AtomicInteger; import javax.inject.Singleton; @@ -52,15 +55,15 @@ import org.jboss.protean.gizmo.MethodCreator; import org.jboss.protean.gizmo.MethodDescriptor; import org.jboss.protean.gizmo.ResultHandle; -import org.jboss.shamrock.deployment.annotations.BuildProducer; -import org.jboss.shamrock.deployment.annotations.BuildStep; -import org.jboss.shamrock.deployment.annotations.Record; import org.jboss.shamrock.arc.deployment.AdditionalBeanBuildItem; import org.jboss.shamrock.arc.deployment.AnnotationsTransformerBuildItem; import org.jboss.shamrock.arc.deployment.BeanContainerBuildItem; import org.jboss.shamrock.arc.deployment.BeanDeploymentValidatorBuildItem; -import org.jboss.shamrock.arc.deployment.UnremovableBeanBuildItem.BeanClassAnnotationExclusion; import org.jboss.shamrock.arc.deployment.UnremovableBeanBuildItem; +import org.jboss.shamrock.arc.deployment.UnremovableBeanBuildItem.BeanClassAnnotationExclusion; +import org.jboss.shamrock.deployment.annotations.BuildProducer; +import org.jboss.shamrock.deployment.annotations.BuildStep; +import org.jboss.shamrock.deployment.annotations.Record; import org.jboss.shamrock.deployment.builditem.FeatureBuildItem; import org.jboss.shamrock.deployment.builditem.GeneratedClassBuildItem; import org.jboss.shamrock.deployment.builditem.substrate.ReflectiveClassBuildItem; @@ -92,8 +95,6 @@ public class SchedulerProcessor { static final String INVOKER_SUFFIX = "_ScheduledInvoker"; - private static final AtomicInteger INVOKER_INDEX = new AtomicInteger(); - @BuildStep List beans() { List beans = new ArrayList<>(); @@ -235,8 +236,13 @@ private String generateInvoker(BeanInfo bean, MethodInfo method, ProcessorClassO } else { baseName = DotNames.simpleName(bean.getImplClazz().name()); } + StringBuilder sigBuilder = new StringBuilder(); + sigBuilder.append(method.name()).append("_").append(method.returnType().name().toString()); + for (Type i : method.parameters()) { + sigBuilder.append(i.name().toString()); + } String targetPackage = DotNames.packageName(bean.getImplClazz().name()); - String generatedName = targetPackage.replace('.', '/') + "/" + baseName + INVOKER_SUFFIX + INVOKER_INDEX.incrementAndGet(); + String generatedName = targetPackage.replace('.', '/') + "/" + baseName + INVOKER_SUFFIX + "_" + method.name() + "_" + sha1(sigBuilder.toString()); ClassCreator invokerCreator = ClassCreator.builder().classOutput(processorClassOutput).className(generatedName).interfaces(ScheduledInvoker.class) .build(); @@ -266,6 +272,16 @@ private String generateInvoker(BeanInfo bean, MethodInfo method, ProcessorClassO invokerCreator.close(); return generatedName.replace('/', '.'); } + + static String sha1(String value) { + try { + MessageDigest md = MessageDigest.getInstance("SHA-1"); + return Base64.getUrlEncoder() + .encodeToString(md.digest(value.getBytes(StandardCharsets.UTF_8))); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException(e); + } + } private Object getValue(AnnotationValue annotationValue) { switch (annotationValue.kind()) {