Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support jakarta.annotation version 3.0 in E4 Injector #1566

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Support jakarta.annotation version 3.0 in E4 Injector #1566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
995deaf
Support multiple versions of annotation classes in E4 Injector
HannesWell Sep 23, 2024
306174f
Version bump(s) for 4.34 stream
eclipse-platform-bot Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion runtime/bundles/org.eclipse.e4.core.di/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-SymbolicName: org.eclipse.e4.core.di
Bundle-Version: 1.9.500.qualifier
Bundle-Version: 1.9.600.qualifier
Bundle-Name: %pluginName
Bundle-Vendor: %providerName
Bundle-Localization: plugin
Expand Down
196 changes: 105 additions & 91 deletions .../bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/AnnotationLookup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2023, 2023 Hannes Wellmann and others.
* Copyright (c) 2023, 2024 Hannes Wellmann and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -10,18 +10,24 @@
*
* Contributors:
* Hannes Wellmann - initial API and implementation
* Hannes Wellmann - support multiple versions of one annotation class
*******************************************************************************/

package org.eclipse.e4.core.internal.di;

import java.lang.annotation.Annotation;
import java.lang.invoke.CallSite;
import java.lang.invoke.LambdaMetafactory;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import java.util.function.Supplier;
import org.eclipse.e4.core.di.IInjector;
Expand All @@ -42,129 +48,135 @@ public class AnnotationLookup {
private AnnotationLookup() {
}

public static record AnnotationProxy(List<Class<? extends Annotation>> classes) {
public static record AnnotationProxy(List<String> classes) {

public AnnotationProxy {
classes = List.copyOf(classes);
}

public boolean isPresent(AnnotatedElement element) {
for (Class<? extends Annotation> annotationClass : classes) {
if (element.isAnnotationPresent(annotationClass)) {
for (Annotation annotation : element.getAnnotations()) {
if (classes.contains(annotation.annotationType().getName())) {
return true;
}
}
return false;
}
}

static final AnnotationProxy INJECT = createProxyForClasses(jakarta.inject.Inject.class,
() -> javax.inject.Inject.class);
static final AnnotationProxy SINGLETON = createProxyForClasses(jakarta.inject.Singleton.class,
() -> javax.inject.Singleton.class);
static final AnnotationProxy QUALIFIER = createProxyForClasses(jakarta.inject.Qualifier.class,
() -> javax.inject.Qualifier.class);

static final AnnotationProxy PRE_DESTROY = createProxyForClasses(jakarta.annotation.PreDestroy.class,
() -> javax.annotation.PreDestroy.class);
public static final AnnotationProxy POST_CONSTRUCT = createProxyForClasses(jakarta.annotation.PostConstruct.class,
() -> javax.annotation.PostConstruct.class);

static final AnnotationProxy OPTIONAL = createProxyForClasses(org.eclipse.e4.core.di.annotations.Optional.class,
null);

private static AnnotationProxy createProxyForClasses(Class<? extends Annotation> jakartaAnnotationClass,
Supplier<Class<? extends Annotation>> javaxAnnotationClass) {
List<Class<?>> classes = getAvailableClasses(jakartaAnnotationClass, javaxAnnotationClass);
@SuppressWarnings({ "rawtypes", "unchecked" })
List<Class<? extends Annotation>> annotationClasses = (List) classes;
return new AnnotationProxy(annotationClasses);
static final AnnotationProxy INJECT = createProxyForClasses("jakarta.inject.Inject", //$NON-NLS-1$
"javax.inject.Inject"); //$NON-NLS-1$
static final AnnotationProxy SINGLETON = createProxyForClasses("jakarta.inject.Singleton", //$NON-NLS-1$
"javax.inject.Singleton"); //$NON-NLS-1$
static final AnnotationProxy QUALIFIER = createProxyForClasses("jakarta.inject.Qualifier", //$NON-NLS-1$
"javax.inject.Qualifier"); //$NON-NLS-1$

static final AnnotationProxy PRE_DESTROY = createProxyForClasses("jakarta.annotation.PreDestroy", //$NON-NLS-1$
"javax.annotation.PreDestroy"); //$NON-NLS-1$
public static final AnnotationProxy POST_CONSTRUCT = createProxyForClasses("jakarta.annotation.PostConstruct", //$NON-NLS-1$
"javax.annotation.PostConstruct"); //$NON-NLS-1$

static final AnnotationProxy OPTIONAL = createProxyForClasses("org.eclipse.e4.core.di.annotations.Optional", null); //$NON-NLS-1$

private static AnnotationProxy createProxyForClasses(String jakartaAnnotationClass,
String javaxAnnotationClass) {
return new AnnotationProxy(getAvailableClasses(jakartaAnnotationClass, javaxAnnotationClass));
}

private static final List<Class<?>> PROVIDER_TYPES = getAvailableClasses(jakarta.inject.Provider.class,
() -> javax.inject.Provider.class);
private static final Set<String> PROVIDER_TYPES = Set
.copyOf(getAvailableClasses("jakarta.inject.Provider", "javax.inject.Provider")); //$NON-NLS-1$//$NON-NLS-2$

static boolean isProvider(Type type) {
for (Class<?> clazz : PROVIDER_TYPES) {
if (clazz.equals(type)) {
return true;
}
}
return false;
return PROVIDER_TYPES.contains(type.getTypeName());
}

@FunctionalInterface
private interface ProviderFactory {
Object create(IObjectDescriptor descriptor, IInjector injector, PrimaryObjectSupplier provider);
}
private static final Map<Class<?>, MethodHandle> PROVIDER_FACTORYS = new ConcurrentHashMap<>();

private static final ProviderFactory PROVIDER_FACTORY;
static {
ProviderFactory factory;
try {
/**
* This subclass solely exists for the purpose to not require the presence of
* the javax.inject.Provider interface in the runtime when the base-class is
* loaded. This can be deleted when support for javax is removed form the
* E4-injector.
*/
class JavaxCompatibilityProviderImpl<T> extends ProviderImpl<T> implements javax.inject.Provider<T> {
public JavaxCompatibilityProviderImpl(IObjectDescriptor descriptor, IInjector injector,
PrimaryObjectSupplier provider) {
super(descriptor, injector, provider);
}
public static Object getProvider(IObjectDescriptor descriptor, IInjector injector, PrimaryObjectSupplier provider) {

Supplier<Object> genericProvider = () -> ((InjectorImpl) injector).makeFromProvider(descriptor, provider);

Class<?> providerClass;
if ((descriptor.getDesiredType() instanceof ParameterizedType parameterizedType
&& parameterizedType.getRawType() instanceof Class<?> clazz)) {
providerClass = clazz;
} else {
throw new IllegalStateException(); // The caller must ensure the providerClass can be extracted
}
// At runtime dynamically create a method-reference that implements the specific
// providerClass 'foo.bar.Provider':
// (foo.bar.Provider) genericProvider::get
MethodHandle factory = PROVIDER_FACTORYS.computeIfAbsent(providerClass, providerType -> {
try {
MethodHandles.Lookup lookup = MethodHandles.lookup();
MethodType suppliedType = MethodType.methodType(Object.class);
CallSite callSite = LambdaMetafactory.metafactory(lookup, "get", //$NON-NLS-1$
MethodType.methodType(providerClass, Supplier.class), suppliedType.erase(), //
lookup.findVirtual(Supplier.class, "get", MethodType.methodType(Object.class)), //$NON-NLS-1$
suppliedType);
return callSite.getTarget();
} catch (Exception e) {
throw new IllegalStateException(e);
}
factory = JavaxCompatibilityProviderImpl::new;
// Attempt to load the class early in order to enforce an early class-loading
// and to be able to handle the NoClassDefFoundError below in case
// javax-Provider is not available in the runtime:
factory.create(null, null, null);
} catch (NoClassDefFoundError e) {
factory = ProviderImpl::new;
});
try {
Object providerImpl = factory.bindTo(genericProvider).invoke();
return providerClass.cast(providerImpl);
} catch (Throwable e) {
throw new IllegalStateException(e);
}
PROVIDER_FACTORY = factory;
}

public static Object getProvider(IObjectDescriptor descriptor, IInjector injector, PrimaryObjectSupplier provider) {
return PROVIDER_FACTORY.create(descriptor, injector, provider);
}

public static String getQualifierValue(IObjectDescriptor descriptor) {
var annotations = NAMED_ANNOTATION2VALUE_GETTER.entrySet();
for (Entry<Class<? extends Annotation>, Function<Annotation, String>> entry : annotations) {
Class<? extends Annotation> annotationClass = entry.getKey();
if (descriptor.hasQualifier(annotationClass)) {
Annotation namedAnnotation = descriptor.getQualifier(annotationClass);
return entry.getValue().apply(namedAnnotation);
Annotation[] qualifiers = descriptor.getQualifiers();
if (qualifiers != null) {
for (Annotation namedAnnotation : qualifiers) {
Class<? extends Annotation> annotationType = namedAnnotation.annotationType();
if (NAMED_ANNOTATION_CLASSES.contains(annotationType.getName())) {
return namedAnnotationValueGetter(annotationType).apply(namedAnnotation);
}
}
}
return null;
}

private static final Map<Class<? extends Annotation>, Function<Annotation, String>> NAMED_ANNOTATION2VALUE_GETTER;

static {
Map<Class<? extends Annotation>, Function<Annotation, String>> annotation2valueGetter = new HashMap<>();
annotation2valueGetter.put(jakarta.inject.Named.class, a -> ((jakarta.inject.Named) a).value());
loadJavaxClass(
() -> annotation2valueGetter.put(javax.inject.Named.class, a -> ((javax.inject.Named) a).value()));
NAMED_ANNOTATION2VALUE_GETTER = Map.copyOf(annotation2valueGetter);
private static Function<Annotation, String> namedAnnotationValueGetter(
Class<? extends Annotation> annotationType) {
return NAMED_ANNOTATION2VALUE_GETTER2.computeIfAbsent(annotationType, type -> {
try {
// At runtime dynamically create the method-reference: 'foo.bar.Named::value'
// where 'foo.bar.Named' is the passed specific annotationType. Invoking the
// returned Function built from the method reference is much faster than using
// reflection.
MethodHandles.Lookup lookup = MethodHandles.lookup();
MethodType functionApplySignature = MethodType.methodType(String.class, type);
CallSite site = LambdaMetafactory.metafactory(lookup, "apply", //$NON-NLS-1$
MethodType.methodType(Function.class), functionApplySignature.erase(),
lookup.findVirtual(type, "value", MethodType.methodType(String.class)), //$NON-NLS-1$
functionApplySignature);
return (Function<Annotation, String>) site.getTarget().invokeExact();
} catch (Throwable e) {
throw new IllegalStateException(e);
}
});
}

private static List<Class<?>> getAvailableClasses(Class<?> jakartaClass, Supplier<? extends Class<?>> javaxClass) {
List<Class<?>> classes = new ArrayList<>();
classes.add(jakartaClass);
if (javaxClass != null) {
loadJavaxClass(() -> classes.add(javaxClass.get()));
}
return classes;
private static final Map<Class<? extends Annotation>, Function<Annotation, String>> NAMED_ANNOTATION2VALUE_GETTER2 = new ConcurrentHashMap<>();
private static final Set<String> NAMED_ANNOTATION_CLASSES = Set.of("jakarta.inject.Named", "javax.inject.Named"); //$NON-NLS-1$//$NON-NLS-2$
// TODO: warn about the javax-class?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove TODO


private static List<String> getAvailableClasses(String jakartaClass, String javaxClass) {
return javaxClass != null && canLoadJavaxClass(javaxClass) //
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it needed to check if javax is loadable?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This methods checks if the javax-classes are even available in the application in any version and if it's therefore worth to check for them. And, maybe even more important, it emits the warning about javax annotations being 'deprecated'.

But in the past it was asked to improve the implementation because the message is to coarse. So this can definitively improved. OTOH, if support for javax annotation does not require their classes in the build-system we can probably support it much easier and longer and it could be consider to un-deprecate their support.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it can be supported by reflection for very little ongoing maintenance overhead, that seems like a good thing for the consumers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most work would be testing it. Currently we have the test-suite duplicated once for javax and once for jakarta.
But if we just use the class-names permanently to support e.g. multiple versions of a class eventually, the injector would even be simpler without all the logic for the deprecation warnings etc.

The main question is probably for how long we want to explicitly test javax annotations.
At the same time it might not even be necessary to explicitly test with all supported class names, as long as we make sure that the injector just cares about class-names and can handle multiple Class-objects with the same name.

? List.of(jakartaClass, javaxClass)
: List.of(jakartaClass);
}

private static boolean javaxWarningPrinted = false;

private static void loadJavaxClass(Runnable run) {
private static boolean canLoadJavaxClass(String className) {
try {
if (!getSystemPropertyFlag("eclipse.e4.inject.javax.disabled", false)) { //$NON-NLS-1$
run.run();
Class.forName(className); // fails if absent
if (!javaxWarningPrinted) {
if (getSystemPropertyFlag("eclipse.e4.inject.javax.warning", true)) { //$NON-NLS-1$
@SuppressWarnings("nls")
Expand All @@ -179,10 +191,12 @@ private static void loadJavaxClass(Runnable run) {
}
javaxWarningPrinted = true;
}
return true;
}
} catch (NoClassDefFoundError e) {
} catch (NoClassDefFoundError | ClassNotFoundException e) {
// Ignore exception: javax-annotation seems to be unavailable in the runtime
}
return false;
}

private static boolean getSystemPropertyFlag(String key, boolean defaultValue) {
Expand Down
51 changes: 7 additions & 44 deletions runtime/bundles/org.eclipse.e4.core.di/src/org/eclipse/e4/core/internal/di/InjectorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.lang.reflect.Modifier;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.security.CodeSource;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand All @@ -37,7 +36,6 @@
import java.util.Map;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.eclipse.e4.core.di.IBinding;
import org.eclipse.e4.core.di.IInjector;
Expand All @@ -50,8 +48,6 @@
import org.eclipse.e4.core.di.suppliers.PrimaryObjectSupplier;
import org.eclipse.e4.core.internal.di.AnnotationLookup.AnnotationProxy;
import org.eclipse.e4.core.internal.di.osgi.LogHelper;
import org.osgi.framework.Bundle;
import org.osgi.framework.FrameworkUtil;

/**
* Reflection-based dependency injector.
Expand Down Expand Up @@ -330,7 +326,7 @@ public Object makeFromProvider(IObjectDescriptor descriptor, PrimaryObjectSuppli
Binding binding = findBinding(descriptor);
Class<?> implementationClass;
if (binding == null)
implementationClass = getProviderType(descriptor.getDesiredType());
implementationClass = getProvidedType(descriptor.getDesiredType());
else
implementationClass = binding.getImplementationClass();
if (objectSupplier != null) {
Expand Down Expand Up @@ -486,7 +482,7 @@ private Object[] resolveArgs(Requestor<?> requestor, PrimaryObjectSupplier objec

// 1) check if we have a Provider<T>
for (int i = 0; i < actualArgs.length; i++) {
Class<?> providerClass = getProviderType(descriptors[i].getDesiredType());
Class<?> providerClass = getProvidedType(descriptors[i].getDesiredType());
if (providerClass == null) {
continue;
}
Expand Down Expand Up @@ -837,14 +833,14 @@ private Class<?> getDesiredClass(Type desiredType) {
/**
* Returns null if not a provider
*/
private Class<?> getProviderType(Type type) {
if (!(type instanceof ParameterizedType))
private Class<?> getProvidedType(Type type) {
if (!(type instanceof ParameterizedType parameterizedType))
return null;
Type rawType = ((ParameterizedType) type).getRawType();
Type rawType = parameterizedType.getRawType();
if (!AnnotationLookup.isProvider(rawType)) {
return null;
}
Type[] actualTypes = ((ParameterizedType) type).getActualTypeArguments();
Type[] actualTypes = parameterizedType.getActualTypeArguments();
if (actualTypes.length != 1)
return null;
if (!(actualTypes[0] instanceof Class<?>))
Expand Down Expand Up @@ -883,7 +879,7 @@ public IBinding addBinding(IBinding binding) {
}

private Binding findBinding(IObjectDescriptor descriptor) {
Class<?> desiredClass = getProviderType(descriptor.getDesiredType());
Class<?> desiredClass = getProvidedType(descriptor.getDesiredType());
if (desiredClass == null)
desiredClass = getDesiredClass(descriptor.getDesiredType());
synchronized (bindings) {
Expand Down Expand Up @@ -942,23 +938,6 @@ private void processAnnotated(AnnotationProxy annotation, Object userObject, Cla
Method[] methods = getDeclaredMethods(objectClass);
for (Method method : methods) {
if (!isAnnotationPresent(method, annotation)) {
if (shouldDebug) {
for (Annotation a : method.getAnnotations()) {
if (annotation.classes().stream().map(Class::getName)
.anyMatch(a.annotationType().getName()::equals)) {
StringBuilder tmp = new StringBuilder();
tmp.append("Possbible annotation mismatch: method \""); //$NON-NLS-1$
tmp.append(method.toString());
tmp.append("\" annotated with \""); //$NON-NLS-1$
tmp.append(describeClass(a.annotationType()));
tmp.append("\" but was looking for \""); //$NON-NLS-1$
tmp.append(annotation.classes().stream().map(InjectorImpl::describeClass)
.collect(Collectors.joining(System.lineSeparator() + " or "))); //$NON-NLS-1$
tmp.append("\""); //$NON-NLS-1$
LogHelper.logWarning(tmp.toString(), null);
}
}
}
continue;
}
if (isOverridden(method, classHierarchy))
Expand All @@ -978,22 +957,6 @@ private void processAnnotated(AnnotationProxy annotation, Object userObject, Cla
}
}

/** Provide a human-meaningful description of the provided class */
private static String describeClass(Class<?> cl) {
Bundle b = FrameworkUtil.getBundle(cl);
if (b != null) {
return b.getSymbolicName() + ":" + b.getVersion() + ":" + cl.getName(); //$NON-NLS-1$ //$NON-NLS-2$
}
CodeSource clazzCS = cl.getProtectionDomain().getCodeSource();
if (clazzCS != null) {
return clazzCS.getLocation() + ">" + cl.getName(); //$NON-NLS-1$
}
if (cl.getClassLoader() == null) {
return cl.getName() + " [via bootstrap classloader]"; //$NON-NLS-1$
}
return cl.getName();
}

@Override
public void setDefaultSupplier(PrimaryObjectSupplier objectSupplier) {
defaultSupplier = objectSupplier;
Expand Down
Loading
Loading