Skip to content

Conversation

antoinesd
Copy link
Contributor

CDI-580 Allow interceptors and decorators to be applied to the return value of a producer method
Also works for custom beans create()

*
* @return self
*/
InterceptionProxyFactory<T> ignoreFinalMethods();
Copy link
Contributor

Choose a reason for hiding this comment

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

Using this will mean that you cannot call the final methods on the proxy (correct me if I am wrong). I think it is worth mentioning this at least in javadoc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

* @return a new {@link InterceptionProxyFactory} to create proxies for an instance of T
* @since 2.0
*/
<T> InterceptionProxyFactory<T> createInterceptionFactory(CreationalContext<T> ctx, Class<T> clazz);
Copy link
Contributor

Choose a reason for hiding this comment

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

If anyone can refresh my mind, I know we talked about built-in beans, but what was the reason behind a BM method? I would guess Extensions, is that right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To get it for custom bean create method


=== Apply interceptor and decorators programmatically

Contextual instances of bean having interceptors binding or defined decorators are provided by the container as a proxy.
Copy link
Contributor

Choose a reason for hiding this comment

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

The term proxy is undefined from the spec point of view. The spec defines client proxy but it's not related to interceptors and decorators. I believe the wording should follow section 7.2. Container invocations and interception and use terms like contextual reference and business method invocation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I'll try to rework with this vocabulary.

Copy link
Contributor Author

@antoinesd antoinesd Nov 4, 2016

Choose a reason for hiding this comment

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

On a second thought if we go in that direction (no mention of proxy) we should perhaps find another name for InterceptionProxyFactory interface, something like ContextualReferenceFactory or InterceptedInstanceFactory?
wdyt @mkouba ?


An `InterceptionProxyFactory` can be obtain be calling `BeanManager.createInterceptionFactory` as defined in <<bm_obtain_interception_proxy_factory>>

The container must also provide an instance of `InterceptionProxyFactory` for every producer methods injecting this interface.
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is supposed to be about the built in beans, it sounds rather confusing. I suggest this instead:
The container must also provide an InterceptionProxyFactory built-in bean. Therefore, another way to use this interface is to inject the built-in bean directly as a parameter of a producer method.
Or something similar which will specifically mention the built in bean.


An `InterceptionProxyFactory` can be obtain be calling `BeanManager.createInterceptionFactory` as defined in <<bm_obtain_interception_proxy_factory>>

The container must also provide an instance of `InterceptionProxyFactory` for every producer methods injecting this interface.
Copy link
Contributor

Choose a reason for hiding this comment

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

Must provide a built-in bean with bean type javax.enterprise.inject.spi.InterceptionProxyFactory, qualifier @Default and "injectable only in a producer method parameter" (needs rewording).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok.

An `InterceptionProxyFactory` can be obtain be calling `BeanManager.createInterceptionFactory` as defined in <<bm_obtain_interception_proxy_factory>>

The container must also provide an instance of `InterceptionProxyFactory` for every producer methods injecting this interface.
This allow creation of produced instances having interceptors and decorators applied to them.
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, this sounds somewhat clumsy, how about:
Either approach will allow to apply interceptors and decorators to beans created via producers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1

@antoinesd antoinesd force-pushed the CDI-580 branch 2 times, most recently from b16232c to 3435b22 Compare October 20, 2016 08:17
@antoinesd
Copy link
Contributor Author

antoinesd commented Nov 7, 2016

This new version remove the term proxy from Javadoc and doc to mention enhancement of instance by making method invocation business method invocation as defined in section 7.2.
This new interface definitely need a new name without proxy in it. My suggestions are:

  • InstanceEnhancer (short but not very clear)
  • BusinessMethodInvocationFactory (more exact from spec pov, but is it clear from user pov?)
  • InterceptionFactory (cleared from user pov and near our initial name)
  • InterceptionEnhancer

Other suggestion are welcome.
[EDIT] InterceptionFactory seems the favourite choice so far.

@antoinesd antoinesd force-pushed the CDI-580 branch 2 times, most recently from 80f2a26 to 87ed559 Compare November 8, 2016 14:26
… value of a producer method

Also works for custom beans create()
…d to the return value of a producer method

Adding clarification on ingnoreFinalMethods javadoc
Add reference to InterceptionProxyFactory in Decorators and Interceptors chapters
Add also reference to Producer interface and Javadoc
…d to the return value of a producer method

remove reference to proxy in javadoc and spec doc.
… value of a producer method

changed Interface name.
removed mention of decorators
add precsion regarding unproxyable bean types
@manovotn
Copy link
Contributor

I think we should specifically mention in the spec that this feature enables the interceptor with @AroundInvoke binding.
It might not be even possible to implement this for other bindings like @AroundConstruct.

@antoinesd
Copy link
Contributor Author

Good point @manovotn. Adding it with consistent rephrasing on ignoreFinals() with #328

… value of a producer method

chnage wording and add mention for @AroundInvoke
@antoinesd antoinesd merged commit b155833 into jakartaee:master Nov 18, 2016
* `configure()` returns an `AnnotatedTypeConfigurator` (as defined in <<annotated_type_configurator>>) initialized with the instance type to easily apply specific interceptor binding to apply when enhancing the instance.
The method always return the same `AnnotatedTypeConfigurator`
* `createInterceptedInstance()` returns an enhanced version of the instance for which each method invocations will be a business method invocation.
The method can be only called once, subsequent calls will throw an exception.
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I didn't have enough time to go through this PR. This sentence is too general.

@antoinesd antoinesd deleted the CDI-580 branch December 14, 2016 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants