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.

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

extension API improvements #511

Merged
merged 1 commit into from
Aug 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static AnnotationBuilder of(Class<? extends Annotation> annotationType) {
* @param annotationType the annotation type, must not be {@code null}
* @return a new {@code AnnotationBuilder}
*/
static AnnotationBuilder of(ClassInfo<?> annotationType) {
static AnnotationBuilder of(ClassInfo annotationType) {
return AnnotationBuilderFactoryResolver.get().create(annotationType);
}

Expand Down Expand Up @@ -278,7 +278,7 @@ default AnnotationBuilder value(Class<? extends Enum<?>> enumType, String... enu
* @param enumValue name of the enum constant, must not be {@code null}
* @return this {@code AnnotationBuilder}
*/
default AnnotationBuilder value(ClassInfo<?> enumType, String enumValue) {
default AnnotationBuilder value(ClassInfo enumType, String enumValue) {
return member(AnnotationInfo.MEMBER_VALUE, enumType, enumValue);
}

Expand All @@ -289,7 +289,7 @@ default AnnotationBuilder value(ClassInfo<?> enumType, String enumValue) {
* @param enumValues names of the enum constants, must not be {@code null} or contain {@code null}
* @return this {@code AnnotationBuilder}
*/
default AnnotationBuilder value(ClassInfo<?> enumType, String... enumValues) {
default AnnotationBuilder value(ClassInfo enumType, String... enumValues) {
return member(AnnotationInfo.MEMBER_VALUE, enumType, enumValues);
}

Expand Down Expand Up @@ -319,7 +319,7 @@ default AnnotationBuilder value(Class<?>... values) {
* @param value the class value, must not be {@code null}
* @return this {@code AnnotationBuilder}
*/
default AnnotationBuilder value(ClassInfo<?> value) {
default AnnotationBuilder value(ClassInfo value) {
return member(AnnotationInfo.MEMBER_VALUE, value);
}

Expand All @@ -329,7 +329,7 @@ default AnnotationBuilder value(ClassInfo<?> value) {
* @param values the class array, must not be {@code null} or contain {@code null}
* @return this {@code AnnotationBuilder}
*/
default AnnotationBuilder value(ClassInfo<?>... values) {
default AnnotationBuilder value(ClassInfo... values) {
return member(AnnotationInfo.MEMBER_VALUE, values);
}

Expand Down Expand Up @@ -379,7 +379,7 @@ default AnnotationBuilder value(Type... values) {
* @param value the annotation value, must not be {@code null}
* @return this {@code AnnotationBuilder}
*/
default AnnotationBuilder value(AnnotationInfo<?> value) {
default AnnotationBuilder value(AnnotationInfo value) {
return member(AnnotationInfo.MEMBER_VALUE, value);
}

Expand All @@ -389,7 +389,7 @@ default AnnotationBuilder value(AnnotationInfo<?> value) {
* @param values the annotation array, must not be {@code null} or contain {@code null}
* @return this {@code AnnotationBuilder}
*/
default AnnotationBuilder value(AnnotationInfo<?>... values) {
default AnnotationBuilder value(AnnotationInfo... values) {
return member(AnnotationInfo.MEMBER_VALUE, values);
}

Expand Down Expand Up @@ -630,7 +630,7 @@ default AnnotationBuilder value(Annotation... values) {
* @param enumValue name of the enum constant, must not be {@code null}
* @return this {@code AnnotationBuilder}
*/
AnnotationBuilder member(String name, ClassInfo<?> enumType, String enumValue);
AnnotationBuilder member(String name, ClassInfo enumType, String enumValue);

/**
* Adds an enum array-valued annotation member with given {@code name}.
Expand All @@ -640,7 +640,7 @@ default AnnotationBuilder value(Annotation... values) {
* @param enumValues names of the enum constants, must not be {@code null} or contain {@code null}
* @return this {@code AnnotationBuilder}
*/
AnnotationBuilder member(String name, ClassInfo<?> enumType, String... enumValues);
AnnotationBuilder member(String name, ClassInfo enumType, String... enumValues);

/**
* Adds a class-valued annotation member with given {@code name}.
Expand All @@ -667,7 +667,7 @@ default AnnotationBuilder value(Annotation... values) {
* @param value the class value, must not be {@code null}
* @return this {@code AnnotationBuilder}
*/
AnnotationBuilder member(String name, ClassInfo<?> value);
AnnotationBuilder member(String name, ClassInfo value);

/**
* Adds a class array-valued annotation member with given {@code name}.
Expand All @@ -676,7 +676,7 @@ default AnnotationBuilder value(Annotation... values) {
* @param values the class array, must not be {@code null} or contain {@code null}
* @return this {@code AnnotationBuilder}
*/
AnnotationBuilder member(String name, ClassInfo<?>... values);
AnnotationBuilder member(String name, ClassInfo... values);

/**
* Adds a class-valued annotation member with given {@code name}.
Expand Down Expand Up @@ -724,7 +724,7 @@ default AnnotationBuilder value(Annotation... values) {
* @param value the annotation value, must not be {@code null}
* @return this {@code AnnotationBuilder}
*/
AnnotationBuilder member(String name, AnnotationInfo<?> value);
AnnotationBuilder member(String name, AnnotationInfo value);

/**
* Adds an annotation array-valued annotation member with given {@code name}.
Expand All @@ -733,7 +733,7 @@ default AnnotationBuilder value(Annotation... values) {
* @param values the annotation array, must not be {@code null} or contain {@code null}
* @return this {@code AnnotationBuilder}
*/
AnnotationBuilder member(String name, AnnotationInfo<?>... values);
AnnotationBuilder member(String name, AnnotationInfo... values);

/**
* Adds an annotation-valued annotation member with given {@code name}.
Expand All @@ -759,5 +759,5 @@ default AnnotationBuilder value(Annotation... values) {
*
* @return the built {@link AnnotationInfo}, never {@code null}
*/
AnnotationInfo<?> build();
AnnotationInfo build();
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,5 @@ public interface AnnotationBuilderFactory extends Prioritized {
* @param annotationType the annotation type
* @return a new {@link AnnotationBuilder}
*/
AnnotationBuilder create(ClassInfo<?> annotationType);
AnnotationBuilder create(ClassInfo annotationType);
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,52 +8,134 @@
import java.util.Collection;

/**
* @param <T> type of the inspected bean (that is, not the declaring class, but one of the types the bean has)
* Provides read-only information about a bean.
*/
public interface BeanInfo<T> {
// TODO remove the type parameter?

public interface BeanInfo {
/**
* Returns the {@link ScopeInfo scope} of this bean.
*
* @return the {@link ScopeInfo scope} of this bean, never {@code null}
*/
ScopeInfo scope();

/**
* Returns a collection of all {@link Type type}s of this bean.
*
* @return immutable collection of bean types, never {@code null}
*/
Collection<Type> types();

/**
* Returns a collection of this bean's qualifiers, represented as {@link AnnotationInfo}.
*
* @return immutable collection of qualifiers, never {@code null}
*/
// TODO method(s) for getting AnnotationInfo for given qualifier class?
Collection<AnnotationInfo> qualifiers();

/**
* Returns the class that declares the bean.
* In case of a bean defined by a class, that is the bean class directly.
* Returns the class that declares this bean.
* In case of a managed bean, also known as class-based bean, that is the bean class directly.
* In case of a producer method or field, that is the class that declares the producer method or field.
* TODO null for synthetic beans, or return Optional?
* Returns {@code null} if this bean is synthetic.
*
* @return {@link ClassInfo} for the class that declares the bean
* @return {@link ClassInfo} for the class that declares this bean, or {@code null} if this bean is synthetic
*/
ClassInfo<?> declaringClass();
ClassInfo declaringClass();

/**
* Returns whether this bean is a managed bean, sometimes also called class-based bean.
*
* @return whether this bean is a managed bean
*/
boolean isClassBean();

/**
* Returns whether this bean is defined by a producer method.
*
* @return whether this bean is defined by a producer method
*/
boolean isProducerMethod();

/**
* Returns whether this bean is defined by a producer field.
*
* @return whether this bean is defined by a producer field
*/
boolean isProducerField();

/**
* Returns whether this bean is synthetic. In other words, whether this bean
* doesn't correspond to a declaration in some source code and was created
* through other means (e.g. using an extension).
*
* @return whether this bean is synthetic
*/
boolean isSynthetic();

MethodInfo<?> producerMethod(); // TODO null if not producer method, or return Optional?
/**
* Returns the producer {@link MethodInfo method} that defines this bean.
* Returns {@code null} if this bean isn't defined by a producer method.
*
* @return producer method that defines this bean, or {@code null} if this bean isn't defined by a producer method
*/
MethodInfo producerMethod();

FieldInfo<?> producerField(); // TODO null if not producer field, or return Optional?
/**
* Returns the producer {@link FieldInfo field} that defines this bean.
* Returns {@code null} if this bean isn't defined by a producer field.
*
* @return producer field that defines this bean, or {@code null} if this bean isn't defined by a producer field
*/
FieldInfo producerField();

/**
* Returns whether this bean is an {@link jakarta.enterprise.inject.Alternative alternative}.
*
* @return whether this bean is an {@link jakarta.enterprise.inject.Alternative alternative}
*/
boolean isAlternative();

/**
* Returns the {@link jakarta.annotation.Priority priority} of this alternative bean.
* If this bean is not an alternative, the return value is undefined.
*
* @return the priority of this alternative bean
* @see #isAlternative()
*/
int priority();

// EL name (from @Named), IIUC
/**
* Returns the bean name of this bean. A bean name is usually defined
* using the {@link jakarta.inject.Named @Named} annotation.
* Returns {@code null} if the bean doesn't have a name.
*
* @return the bean name, or {@code null} if the bean doesn't have a name
*/
String getName();

DisposerInfo disposer(); // TODO null if not producer method/field, or return Optional?
/**
* Returns the {@link DisposerInfo disposer method} of this producer-based bean.
* Returns {@code null} if this bean is not a defined by a producer method or a producer field,
* or if this producer-based bean doesn't have a corresponding disposer method.
*
* @return the {@link DisposerInfo disposer}, or {@code null} if this bean doesn't have a disposer
*/
DisposerInfo disposer();

/**
* Returns a collection of this bean's {@link StereotypeInfo stereotype}s.
*
* @return immutable collection of stereotypes, never {@code null}
*/
Collection<StereotypeInfo> stereotypes();

// TODO interceptors?

/**
* Returns a collection of this bean's {@link InjectionPointInfo injection point}s.
*
* @return immutable collection of injection points, never {@code null}
*/
Collection<InjectionPointInfo> injectionPoints();
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,42 @@
import java.util.Collection;

/**
* @param <T> the configured class
* Allows adding annotations to and removing annotations from a class.
* Note that the class is not physically altered, the modifications
* are only seen by the CDI container.
*
* @see Enhancement
*/
public interface ClassConfig<T> extends ClassInfo<T>, AnnotationConfig {
// TODO remove the type parameter?
// TODO even if ClassInfo has equals/hashCode, ClassConfig probably shouldn't

// only constructors declared by this class, not inherited ones
// no [static] initializers
public interface ClassConfig extends DeclarationConfig<ClassConfig> {
/**
* Returns the {@link ClassInfo read-only information} about this transformed class.
*
* @return the {@link ClassInfo} corresponding to this transformed class, never {@code null}
*/
@Override
Collection<? extends MethodConfig<T>> constructors();
ClassInfo info();

// only methods declared by this class, not inherited ones
// no constructors nor [static] initializers
@Override
Collection<? extends MethodConfig<T>> methods();
/**
* Returns a collection of {@link MethodConfig} objects for each constructor of this class.
*
* @return immutable collection of {@link MethodConfig} objects, never {@code null}
*/
// TODO specify whether inherited constructors are also included
Collection<MethodConfig> constructors();
Copy link
Contributor

Choose a reason for hiding this comment

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

No these should just be the declared constructors, inherited ones can be obtained by traversing the super class?


// only fields declared by this class, not inherited ones
@Override
Collection<? extends FieldConfig<T>> fields();
/**
* Returns a collection of {@link MethodConfig} objects for each method of this class.
*
* @return immutable collection of {@link MethodConfig} objects, never {@code null}
*/
// TODO specify whether inherited methods are also included
Collection<MethodConfig> methods();
Copy link
Contributor

Choose a reason for hiding this comment

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

In this case it may make sense to have declaredMethods() vs methods()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this is something I'm a little ambivalent about. I think we already discussed something similar for annotations and kinda agreed that we'll always return all annotations, including inherited ones. We should probably do the same here.

I'm looking at what Weld does for Portable Extensions (because that seems quite under-documented on the Portable Extensions API side), and it works like this:

  • for constructors, returns only constructors declared on this class
  • for fields, returns all fields declared on this class and all its superclasses up to and excluding Object (which also includes private fields from superclasses)
  • for methods, returns all methods declared on this class and all its superclasses up to and excluding Object (which also includes private methods from superclasses), plus also all default methods from all interfaces directly and indirectly implemented by this class

I guess that kinda sorta makes sense. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense for these methods yes. Although they will be relatively expensive methods to call since materializing all that info could be costly.

Micronaut has an extensive querying API for methods/fields that lets you filter methods prior to materializing the actual equivalent of MethodConfig https://github.com/micronaut-projects/micronaut-core/blob/3.0.x/inject/src/main/java/io/micronaut/inject/ast/ElementQuery.java

I am not sure if something like this makes sense here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just thinking about it and I've got two little problems with methods from interfaces. We would include abstract methods from [super]classes, but not from [super]interfaces. Also, we would include all default methods, even if they potentially clash. It still kinda makes sense, but feels arbitrary. I'll think a bit more about it.

The situation for constructors and fields is rather clear, so I'll draft something and amend the PR.

Also, we already had a bit of querying API in AppArchive and AppDeployment. We might be able to bring some of that back for sure, but I don't think this is too big of an issue. Implementations should be able to materialize the collection lazily and memoize the result.


/**
* Returns a collection of {@link FieldConfig} objects for each field of this class.
*
* @return immutable collection of {@link FieldConfig} objects, never {@code null}
*/
// TODO specify whether inherited fields are also included
Collection<FieldConfig> fields();
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here with declaredFields

}
Loading