You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltaspike.apache.org by Mark Struberg <st...@yahoo.de.INVALID> on 2016/05/09 11:24:13 UTC

DeltaSpikeProxyFactory questions

a.)DeltaSpikeProxyFactory totally misses docs it sems. Or did I miss something?

b.) there are methods which are imo questionable, e.g.

public <T> Class<T> getProxyClass(BeanManager beanManager, Class<T> targetClass)
{
return getProxyClass(beanManager, targetClass, DummyInvocationHandler.class);
}


what do we need this for? Can I simply remove it?
The DummyInvocationHandler just returns null. Without calling the proxied instance. It's basically a no-op impl. Why do we need this?


c.) The ordering of the methods are mixed. Imo all public methods should be on top, protected and private at the bottom. Really hard to read atm.

Any thoughts?

LieGrue,
strub

Re: DeltaSpikeProxyFactory questions

Posted by Thomas Andraschko <an...@gmail.com>.
Mark, i think it should be enough that the proxy generator doesn't take the
implicit type of the invocationhandler to define the constructor+field, it
should be enough if it defines a InvocationHandler as interface.

2016-05-09 21:10 GMT+02:00 Mark Struberg <st...@yahoo.de.invalid>:

> Indeed, that could be a way forward. Almost forgot about it ^^
>
>
> LieGrue,
> strub
>
>
>
>
>
> > On Monday, 9 May 2016, 21:06, Romain Manni-Bucau <rm...@gmail.com>
> wrote:
> > > @Mark: why not using/enhancing [proxy2]? =>
> >
> http://svn.apache.org/repos/asf/commons/proper/proxy/trunk/asm/src/main/java/org/apache/commons/proxy2/asm/ASMProxyFactory.java
> >
> > this is designed to be reusable compared to ds
> >
> >
> > Romain Manni-Bucau
> > @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> > <http://rmannibucau.wordpress.com> | Github
> > <https://github.com/rmannibucau> |
> > LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber
> > <http://www.tomitribe.com> | JavaEE Factory
> > <https://javaeefactory-rmannibucau.rhcloud.com>
> >
> >
> > 2016-05-09 20:42 GMT+02:00 Mark Struberg <st...@yahoo.de.invalid>:
> >
> >>  Maybe I have something else in mind or the ds proxy is not yet there -
> or
> >>  maybe never will be...
> >>
> >>
> >>  I thought to use it kind like java.lang.reflect proxy.
> >>
> >>  So I do not ned any BeanManager in my case.
> >>
> >>
> >>  Let me explain a bit.
> >>
> >>  I'd like to have something like java.lang.reflect Proxy but with a
> >>  subclassing proxy and not a reflection proxy.
> >>
> >>
> >>  The main problem arises with serialisation.
> >>
> >>  The problem is that you can only have a single class with the same name
> >>  obviously ;)
> >>  When serialising it over, you have to either find an existing proxy
> class
> >>  or create the class on the fly while de-serialising on the other side.
> >>  Means the class name must follow a static rule or you are lost (as
> >>  javassist which ends up trashing the ClassLoader with new classes each
> >>  time).
> >>
> >>
> >>  An easy way to achieve this is to work like jlr.Proxy. It always only
> >>  creates the same proxy class bytecode for the same Set of interfaces.
> >>  But you can use this proxy class for different behaviour, because you
> can
> >>  simply use it with different InvocationHandlers.
> >>
> >>  For serialisation you only have to send the list of implemented
> interface
> >>  names + the InvocationHandler (which always needs to be Serializable of
> >>  course).
> >>
> >>  Having only a single proxy class per original class means that each
> method
> >>  invocation must get treated the same. And all of them need to be
> invoked
> >>  via the InvocationHandler.
> >>
> >>
> >>  Another way would be to create specialised proxy classes (like OWB) for
> >>  each InvocationHandler. BUT that would need distinct - but
> reproducible -
> >>  proxy class names.
> >>
> >>  In OWB we store the proxy class somewhere where you can find it again
> - in
> >>  the Bean ;)
> >>
> >>
> >>  Depending on those 2 flavours you get an API with
> >>
> >>
> >>  1.) a separate createProxyClass(Class, interface[]) and
> >>  newProxyInstance(InvocationHandler handler)
> >>
> >>  OR
> >>
> >>
> >>  2.) createProxyClass(Class, interfaces, NAME, InvocationHandler).
> >>
> >>  From looking at the current DS proxy code it looks a bit like a mixture
> >>  between both modes.
> >>
> >>
> >>  What is the overall design goal we aim for?
> >>
> >>  LieGrue,
> >>  strub
> >>
> >>
> >>  On Monday, 9 May 2016, 15:12, Thomas Andraschko <
> >>  andraschko.thomas@gmail.com> wrote:
> >>  >
> >>  >
> >>  >1) exactly, it was designed for that purpose. Therefor we always pass
> >>  InvocationHandler in the constructor.
> >>  >    Do you need different InvocationHandler for different methods?
> >>  >
> >>  >| -> We imo should split the creation of the proxy class from the
> >>  creation of the proxy instance.
> >>  >
> >>  >
> >>  >
> >>  >It's completely splitted currently. DeltaSpikeProxyFactory contains
> > only
> >>  the proxy class creation. proxy instance creation is always done
> outside.
> >>  >
> >>  >
> >>  >
> >>  >2) it's ok for me but please let me review before commiting
> > something.
> >>  >    For me it's only important that every test is working fine
> > (proxy
> >>  module, partial bean, data and jsf module).
> >>  >
> >>  >
> >>  >The beanManager is required to check for interceptor bindings. We must
> >>  proxy every method if interceptors are used. Otherwise we only proxy
> the
> >>  "delegate methods".
> >>  >
> >>  >
> >>  >| Maybe we could introduce a new API which just exposes a native java
> >>  subclass proxy to retain backward compat?
> >>  >
> >>  >What do you mean exactly?
> >>  >
> >>  >
> >>  >
> >>  >
> >>  >
> >>  >
> >>  >
> >>  >
> >>  >2016-05-09 14:55 GMT+02:00 Mark Struberg
> > <st...@yahoo.de.invalid>:
> >>  >
> >>  >Having a really hard time to grok it right now and think we can
> improve
> >>  quite a few things.
> >>  >>
> >>  >>1.) It atm only works with exactly a single InvocationHandler.
> > That's
> >>  currently a problem in my case
> >>  >>-> We imo should split the creation of the proxy class from the
> > creation
> >>  of the proxy instance.
> >>  >>
> >>  >>
> >>  >>2.) It's hard to get what is in the API and what is an impl
> > detail.
> >>  >>
> >>  >>-> We should create an interface and move the impl stuff to an
> > internal
> >>  class.
> >>  >>
> >>  >>Any objections?
> >>  >>
> >>  >>How freely can I change the API?
> >>  >>Is it only important that our internal tests and the JPA stuff is
> >>  working fine again, or do I need to care about native usages as well?
> >>  >>
> >>  >>
> >>  >>I also don't get why the current DeltaSpikeProxyFactory needs
> > the
> >>  BeanManager as parameter?
> >>  >>Maybe we could introduce a new API which just exposes a native java
> >>  subclass proxy to retain backward compat?
> >>  >>In any case we miss a lot of JavaDocs!
> >>  >>
> >>  >>
> >>  >>LieGrue,
> >>  >>strub
> >>  >>
> >>  >>
> >>  >>
> >>  >>
> >>  >>
> >>  >>
> >>  >>
> >>  >>
> >>  >>> On Monday, 9 May 2016, 13:47, Thomas Andraschko <
> >>  andraschko.thomas@gmail.com> wrote:
> >>  >>> > a) yup
> >>  >>> b) if it's unused, why not ;) Should be just an old case,
> > it was
> >>  refactored
> >>  >>> many times during developed because of some AppServers /
> > versions.
> >>  >>> c) yup, feel free to align it :)
> >>  >>>
> >>  >>>
> >>  >>> 2016-05-09 13:30 GMT+02:00 John D. Ament
> > <jo...@apache.org>:
> >>  >>>
> >>  >>>>  On Mon, May 9, 2016 at 7:24 AM Mark Struberg
> >>  >>> <st...@yahoo.de.invalid>
> >>  >>>>  wrote:
> >>  >>>>
> >>  >>>>  > a.)DeltaSpikeProxyFactory totally misses docs it
> > sems. Or did I
> >>  miss
> >>  >>>>  > something?
> >>  >>>>  >
> >>  >>>>
> >>  >>>>  The current docs are a step up from about two months ago
> > when the
> >>  only
> >>  >>>>  thing on the doc page was "TODO: Document the proxy
> > module" or
> >>  >>> something
> >>  >>>>  along those lines.
> >>  >>>>
> >>  >>>>  Yes, there's still stuff missing.
> >>  >>>>
> >>  >>>>
> >>  >>>>  >
> >>  >>>>  > b.) there are methods which are imo questionable,
> > e.g.
> >>  >>>>  >
> >>  >>>>  > public <T> Class<T>
> > getProxyClass(BeanManager beanManager,
> >>  >>> Class<T>
> >>  >>>>  > targetClass)
> >>  >>>>  > {
> >>  >>>>  > return getProxyClass(beanManager, targetClass,
> >>  >>>>  > DummyInvocationHandler.class);
> >>  >>>>  > }
> >>  >>>>  >
> >>  >>>>  >
> >>  >>>>  > what do we need this for? Can I simply remove it?
> >>  >>>>  > The DummyInvocationHandler just returns null.
> > Without calling the
> >>  >>> proxied
> >>  >>>>  > instance. It's basically a no-op impl. Why do we
> > need this?
> >>  >>>>  >
> >>  >>>>
> >>  >>>>  This smells like a strangle that wasn't completed.
> > I'd say if it
> >>  >>> can be
> >>  >>>>  delegated down, lets remove it.  Better is if its only
> > used in tests.
> >>  >>>>
> >>  >>>>
> >>  >>>>  >
> >>  >>>>  >
> >>  >>>>  > c.) The ordering of the methods are mixed. Imo all
> > public methods
> >>  >>> should
> >>  >>>>  > be on top, protected and private at the bottom.
> > Really hard to read
> >>  >>> atm.
> >>  >>>>  >
> >>  >>>>
> >>  >>>>  +1
> >>  >>>>
> >>  >>>>
> >>  >>>>  >
> >>  >>>>  > Any thoughts?
> >>  >>>>  >
> >>  >>>>  > LieGrue,
> >>  >>>>  > strub
> >>  >>>>  >
> >>  >>>>
> >>  >>>
> >>  >>
> >>  >
> >>  >
> >>  >
> >>
> >
>

Re: DeltaSpikeProxyFactory questions

Posted by Mark Struberg <st...@yahoo.de.INVALID>.
Indeed, that could be a way forward. Almost forgot about it ^^


LieGrue,
strub





> On Monday, 9 May 2016, 21:06, Romain Manni-Bucau <rm...@gmail.com> wrote:
> > @Mark: why not using/enhancing [proxy2]? =>
> http://svn.apache.org/repos/asf/commons/proper/proxy/trunk/asm/src/main/java/org/apache/commons/proxy2/asm/ASMProxyFactory.java
> 
> this is designed to be reusable compared to ds
> 
> 
> Romain Manni-Bucau
> @rmannibucau <https://twitter.com/rmannibucau> |  Blog
> <http://rmannibucau.wordpress.com> | Github 
> <https://github.com/rmannibucau> |
> LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber
> <http://www.tomitribe.com> | JavaEE Factory
> <https://javaeefactory-rmannibucau.rhcloud.com>
> 
> 
> 2016-05-09 20:42 GMT+02:00 Mark Struberg <st...@yahoo.de.invalid>:
> 
>>  Maybe I have something else in mind or the ds proxy is not yet there - or
>>  maybe never will be...
>> 
>> 
>>  I thought to use it kind like java.lang.reflect proxy.
>> 
>>  So I do not ned any BeanManager in my case.
>> 
>> 
>>  Let me explain a bit.
>> 
>>  I'd like to have something like java.lang.reflect Proxy but with a
>>  subclassing proxy and not a reflection proxy.
>> 
>> 
>>  The main problem arises with serialisation.
>> 
>>  The problem is that you can only have a single class with the same name
>>  obviously ;)
>>  When serialising it over, you have to either find an existing proxy class
>>  or create the class on the fly while de-serialising on the other side.
>>  Means the class name must follow a static rule or you are lost (as
>>  javassist which ends up trashing the ClassLoader with new classes each
>>  time).
>> 
>> 
>>  An easy way to achieve this is to work like jlr.Proxy. It always only
>>  creates the same proxy class bytecode for the same Set of interfaces.
>>  But you can use this proxy class for different behaviour, because you can
>>  simply use it with different InvocationHandlers.
>> 
>>  For serialisation you only have to send the list of implemented interface
>>  names + the InvocationHandler (which always needs to be Serializable of
>>  course).
>> 
>>  Having only a single proxy class per original class means that each method
>>  invocation must get treated the same. And all of them need to be invoked
>>  via the InvocationHandler.
>> 
>> 
>>  Another way would be to create specialised proxy classes (like OWB) for
>>  each InvocationHandler. BUT that would need distinct - but reproducible -
>>  proxy class names.
>> 
>>  In OWB we store the proxy class somewhere where you can find it again - in
>>  the Bean ;)
>> 
>> 
>>  Depending on those 2 flavours you get an API with
>> 
>> 
>>  1.) a separate createProxyClass(Class, interface[]) and
>>  newProxyInstance(InvocationHandler handler)
>> 
>>  OR
>> 
>> 
>>  2.) createProxyClass(Class, interfaces, NAME, InvocationHandler).
>> 
>>  From looking at the current DS proxy code it looks a bit like a mixture
>>  between both modes.
>> 
>> 
>>  What is the overall design goal we aim for?
>> 
>>  LieGrue,
>>  strub
>> 
>> 
>>  On Monday, 9 May 2016, 15:12, Thomas Andraschko <
>>  andraschko.thomas@gmail.com> wrote:
>>  >
>>  >
>>  >1) exactly, it was designed for that purpose. Therefor we always pass
>>  InvocationHandler in the constructor.
>>  >    Do you need different InvocationHandler for different methods?
>>  >
>>  >| -> We imo should split the creation of the proxy class from the
>>  creation of the proxy instance.
>>  >
>>  >
>>  >
>>  >It's completely splitted currently. DeltaSpikeProxyFactory contains 
> only
>>  the proxy class creation. proxy instance creation is always done outside.
>>  >
>>  >
>>  >
>>  >2) it's ok for me but please let me review before commiting 
> something.
>>  >    For me it's only important that every test is working fine 
> (proxy
>>  module, partial bean, data and jsf module).
>>  >
>>  >
>>  >The beanManager is required to check for interceptor bindings. We must
>>  proxy every method if interceptors are used. Otherwise we only proxy the
>>  "delegate methods".
>>  >
>>  >
>>  >| Maybe we could introduce a new API which just exposes a native java
>>  subclass proxy to retain backward compat?
>>  >
>>  >What do you mean exactly?
>>  >
>>  >
>>  >
>>  >
>>  >
>>  >
>>  >
>>  >
>>  >2016-05-09 14:55 GMT+02:00 Mark Struberg 
> <st...@yahoo.de.invalid>:
>>  >
>>  >Having a really hard time to grok it right now and think we can improve
>>  quite a few things.
>>  >>
>>  >>1.) It atm only works with exactly a single InvocationHandler. 
> That's
>>  currently a problem in my case
>>  >>-> We imo should split the creation of the proxy class from the 
> creation
>>  of the proxy instance.
>>  >>
>>  >>
>>  >>2.) It's hard to get what is in the API and what is an impl 
> detail.
>>  >>
>>  >>-> We should create an interface and move the impl stuff to an 
> internal
>>  class.
>>  >>
>>  >>Any objections?
>>  >>
>>  >>How freely can I change the API?
>>  >>Is it only important that our internal tests and the JPA stuff is
>>  working fine again, or do I need to care about native usages as well?
>>  >>
>>  >>
>>  >>I also don't get why the current DeltaSpikeProxyFactory needs 
> the
>>  BeanManager as parameter?
>>  >>Maybe we could introduce a new API which just exposes a native java
>>  subclass proxy to retain backward compat?
>>  >>In any case we miss a lot of JavaDocs!
>>  >>
>>  >>
>>  >>LieGrue,
>>  >>strub
>>  >>
>>  >>
>>  >>
>>  >>
>>  >>
>>  >>
>>  >>
>>  >>
>>  >>> On Monday, 9 May 2016, 13:47, Thomas Andraschko <
>>  andraschko.thomas@gmail.com> wrote:
>>  >>> > a) yup
>>  >>> b) if it's unused, why not ;) Should be just an old case, 
> it was
>>  refactored
>>  >>> many times during developed because of some AppServers / 
> versions.
>>  >>> c) yup, feel free to align it :)
>>  >>>
>>  >>>
>>  >>> 2016-05-09 13:30 GMT+02:00 John D. Ament 
> <jo...@apache.org>:
>>  >>>
>>  >>>>  On Mon, May 9, 2016 at 7:24 AM Mark Struberg
>>  >>> <st...@yahoo.de.invalid>
>>  >>>>  wrote:
>>  >>>>
>>  >>>>  > a.)DeltaSpikeProxyFactory totally misses docs it 
> sems. Or did I
>>  miss
>>  >>>>  > something?
>>  >>>>  >
>>  >>>>
>>  >>>>  The current docs are a step up from about two months ago 
> when the
>>  only
>>  >>>>  thing on the doc page was "TODO: Document the proxy 
> module" or
>>  >>> something
>>  >>>>  along those lines.
>>  >>>>
>>  >>>>  Yes, there's still stuff missing.
>>  >>>>
>>  >>>>
>>  >>>>  >
>>  >>>>  > b.) there are methods which are imo questionable, 
> e.g.
>>  >>>>  >
>>  >>>>  > public <T> Class<T> 
> getProxyClass(BeanManager beanManager,
>>  >>> Class<T>
>>  >>>>  > targetClass)
>>  >>>>  > {
>>  >>>>  > return getProxyClass(beanManager, targetClass,
>>  >>>>  > DummyInvocationHandler.class);
>>  >>>>  > }
>>  >>>>  >
>>  >>>>  >
>>  >>>>  > what do we need this for? Can I simply remove it?
>>  >>>>  > The DummyInvocationHandler just returns null. 
> Without calling the
>>  >>> proxied
>>  >>>>  > instance. It's basically a no-op impl. Why do we 
> need this?
>>  >>>>  >
>>  >>>>
>>  >>>>  This smells like a strangle that wasn't completed.  
> I'd say if it
>>  >>> can be
>>  >>>>  delegated down, lets remove it.  Better is if its only 
> used in tests.
>>  >>>>
>>  >>>>
>>  >>>>  >
>>  >>>>  >
>>  >>>>  > c.) The ordering of the methods are mixed. Imo all 
> public methods
>>  >>> should
>>  >>>>  > be on top, protected and private at the bottom. 
> Really hard to read
>>  >>> atm.
>>  >>>>  >
>>  >>>>
>>  >>>>  +1
>>  >>>>
>>  >>>>
>>  >>>>  >
>>  >>>>  > Any thoughts?
>>  >>>>  >
>>  >>>>  > LieGrue,
>>  >>>>  > strub
>>  >>>>  >
>>  >>>>
>>  >>>
>>  >>
>>  >
>>  >
>>  >
>> 
> 

Re: DeltaSpikeProxyFactory questions

Posted by Romain Manni-Bucau <rm...@gmail.com>.
@Mark: why not using/enhancing [proxy2]? =>
http://svn.apache.org/repos/asf/commons/proper/proxy/trunk/asm/src/main/java/org/apache/commons/proxy2/asm/ASMProxyFactory.java

this is designed to be reusable compared to ds


Romain Manni-Bucau
@rmannibucau <https://twitter.com/rmannibucau> |  Blog
<http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> |
LinkedIn <https://www.linkedin.com/in/rmannibucau> | Tomitriber
<http://www.tomitribe.com> | JavaEE Factory
<https://javaeefactory-rmannibucau.rhcloud.com>

2016-05-09 20:42 GMT+02:00 Mark Struberg <st...@yahoo.de.invalid>:

> Maybe I have something else in mind or the ds proxy is not yet there - or
> maybe never will be...
>
>
> I thought to use it kind like java.lang.reflect proxy.
>
> So I do not ned any BeanManager in my case.
>
>
> Let me explain a bit.
>
> I'd like to have something like java.lang.reflect Proxy but with a
> subclassing proxy and not a reflection proxy.
>
>
> The main problem arises with serialisation.
>
> The problem is that you can only have a single class with the same name
> obviously ;)
> When serialising it over, you have to either find an existing proxy class
> or create the class on the fly while de-serialising on the other side.
> Means the class name must follow a static rule or you are lost (as
> javassist which ends up trashing the ClassLoader with new classes each
> time).
>
>
> An easy way to achieve this is to work like jlr.Proxy. It always only
> creates the same proxy class bytecode for the same Set of interfaces.
> But you can use this proxy class for different behaviour, because you can
> simply use it with different InvocationHandlers.
>
> For serialisation you only have to send the list of implemented interface
> names + the InvocationHandler (which always needs to be Serializable of
> course).
>
> Having only a single proxy class per original class means that each method
> invocation must get treated the same. And all of them need to be invoked
> via the InvocationHandler.
>
>
> Another way would be to create specialised proxy classes (like OWB) for
> each InvocationHandler. BUT that would need distinct - but reproducible -
> proxy class names.
>
> In OWB we store the proxy class somewhere where you can find it again - in
> the Bean ;)
>
>
> Depending on those 2 flavours you get an API with
>
>
> 1.) a separate createProxyClass(Class, interface[]) and
> newProxyInstance(InvocationHandler handler)
>
> OR
>
>
> 2.) createProxyClass(Class, interfaces, NAME, InvocationHandler).
>
> From looking at the current DS proxy code it looks a bit like a mixture
> between both modes.
>
>
> What is the overall design goal we aim for?
>
> LieGrue,
> strub
>
>
> On Monday, 9 May 2016, 15:12, Thomas Andraschko <
> andraschko.thomas@gmail.com> wrote:
> >
> >
> >1) exactly, it was designed for that purpose. Therefor we always pass
> InvocationHandler in the constructor.
> >    Do you need different InvocationHandler for different methods?
> >
> >| -> We imo should split the creation of the proxy class from the
> creation of the proxy instance.
> >
> >
> >
> >It's completely splitted currently. DeltaSpikeProxyFactory contains only
> the proxy class creation. proxy instance creation is always done outside.
> >
> >
> >
> >2) it's ok for me but please let me review before commiting something.
> >    For me it's only important that every test is working fine (proxy
> module, partial bean, data and jsf module).
> >
> >
> >The beanManager is required to check for interceptor bindings. We must
> proxy every method if interceptors are used. Otherwise we only proxy the
> "delegate methods".
> >
> >
> >| Maybe we could introduce a new API which just exposes a native java
> subclass proxy to retain backward compat?
> >
> >What do you mean exactly?
> >
> >
> >
> >
> >
> >
> >
> >
> >2016-05-09 14:55 GMT+02:00 Mark Struberg <st...@yahoo.de.invalid>:
> >
> >Having a really hard time to grok it right now and think we can improve
> quite a few things.
> >>
> >>1.) It atm only works with exactly a single InvocationHandler. That's
> currently a problem in my case
> >>-> We imo should split the creation of the proxy class from the creation
> of the proxy instance.
> >>
> >>
> >>2.) It's hard to get what is in the API and what is an impl detail.
> >>
> >>-> We should create an interface and move the impl stuff to an internal
> class.
> >>
> >>Any objections?
> >>
> >>How freely can I change the API?
> >>Is it only important that our internal tests and the JPA stuff is
> working fine again, or do I need to care about native usages as well?
> >>
> >>
> >>I also don't get why the current DeltaSpikeProxyFactory needs the
> BeanManager as parameter?
> >>Maybe we could introduce a new API which just exposes a native java
> subclass proxy to retain backward compat?
> >>In any case we miss a lot of JavaDocs!
> >>
> >>
> >>LieGrue,
> >>strub
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>> On Monday, 9 May 2016, 13:47, Thomas Andraschko <
> andraschko.thomas@gmail.com> wrote:
> >>> > a) yup
> >>> b) if it's unused, why not ;) Should be just an old case, it was
> refactored
> >>> many times during developed because of some AppServers / versions.
> >>> c) yup, feel free to align it :)
> >>>
> >>>
> >>> 2016-05-09 13:30 GMT+02:00 John D. Ament <jo...@apache.org>:
> >>>
> >>>>  On Mon, May 9, 2016 at 7:24 AM Mark Struberg
> >>> <st...@yahoo.de.invalid>
> >>>>  wrote:
> >>>>
> >>>>  > a.)DeltaSpikeProxyFactory totally misses docs it sems. Or did I
> miss
> >>>>  > something?
> >>>>  >
> >>>>
> >>>>  The current docs are a step up from about two months ago when the
> only
> >>>>  thing on the doc page was "TODO: Document the proxy module" or
> >>> something
> >>>>  along those lines.
> >>>>
> >>>>  Yes, there's still stuff missing.
> >>>>
> >>>>
> >>>>  >
> >>>>  > b.) there are methods which are imo questionable, e.g.
> >>>>  >
> >>>>  > public <T> Class<T> getProxyClass(BeanManager beanManager,
> >>> Class<T>
> >>>>  > targetClass)
> >>>>  > {
> >>>>  > return getProxyClass(beanManager, targetClass,
> >>>>  > DummyInvocationHandler.class);
> >>>>  > }
> >>>>  >
> >>>>  >
> >>>>  > what do we need this for? Can I simply remove it?
> >>>>  > The DummyInvocationHandler just returns null. Without calling the
> >>> proxied
> >>>>  > instance. It's basically a no-op impl. Why do we need this?
> >>>>  >
> >>>>
> >>>>  This smells like a strangle that wasn't completed.  I'd say if it
> >>> can be
> >>>>  delegated down, lets remove it.  Better is if its only used in tests.
> >>>>
> >>>>
> >>>>  >
> >>>>  >
> >>>>  > c.) The ordering of the methods are mixed. Imo all public methods
> >>> should
> >>>>  > be on top, protected and private at the bottom. Really hard to read
> >>> atm.
> >>>>  >
> >>>>
> >>>>  +1
> >>>>
> >>>>
> >>>>  >
> >>>>  > Any thoughts?
> >>>>  >
> >>>>  > LieGrue,
> >>>>  > strub
> >>>>  >
> >>>>
> >>>
> >>
> >
> >
> >
>

Re: DeltaSpikeProxyFactory questions

Posted by Mark Struberg <st...@yahoo.de.INVALID>.
Maybe I have something else in mind or the ds proxy is not yet there - or maybe never will be...


I thought to use it kind like java.lang.reflect proxy. 

So I do not ned any BeanManager in my case. 


Let me explain a bit.

I'd like to have something like java.lang.reflect Proxy but with a subclassing proxy and not a reflection proxy. 


The main problem arises with serialisation.

The problem is that you can only have a single class with the same name obviously ;)
When serialising it over, you have to either find an existing proxy class or create the class on the fly while de-serialising on the other side. Means the class name must follow a static rule or you are lost (as javassist which ends up trashing the ClassLoader with new classes each time). 


An easy way to achieve this is to work like jlr.Proxy. It always only creates the same proxy class bytecode for the same Set of interfaces.
But you can use this proxy class for different behaviour, because you can simply use it with different InvocationHandlers.

For serialisation you only have to send the list of implemented interface names + the InvocationHandler (which always needs to be Serializable of course).

Having only a single proxy class per original class means that each method invocation must get treated the same. And all of them need to be invoked via the InvocationHandler. 


Another way would be to create specialised proxy classes (like OWB) for each InvocationHandler. BUT that would need distinct - but reproducible - proxy class names.

In OWB we store the proxy class somewhere where you can find it again - in the Bean ;)


Depending on those 2 flavours you get an API with 


1.) a separate createProxyClass(Class, interface[]) and newProxyInstance(InvocationHandler handler)

OR


2.) createProxyClass(Class, interfaces, NAME, InvocationHandler).

From looking at the current DS proxy code it looks a bit like a mixture between both modes. 


What is the overall design goal we aim for?

LieGrue,
strub


On Monday, 9 May 2016, 15:12, Thomas Andraschko <an...@gmail.com> wrote:
>
>
>1) exactly, it was designed for that purpose. Therefor we always pass InvocationHandler in the constructor.
>    Do you need different InvocationHandler for different methods?
>
>| -> We imo should split the creation of the proxy class from the creation of the proxy instance.
>
>
>
>It's completely splitted currently. DeltaSpikeProxyFactory contains only the proxy class creation. proxy instance creation is always done outside.
>
>
>
>2) it's ok for me but please let me review before commiting something.
>    For me it's only important that every test is working fine (proxy module, partial bean, data and jsf module).
>
>
>The beanManager is required to check for interceptor bindings. We must proxy every method if interceptors are used. Otherwise we only proxy the "delegate methods".
>
>
>| Maybe we could introduce a new API which just exposes a native java subclass proxy to retain backward compat?
>
>What do you mean exactly?
>
>
>
>
>
>
>
>
>2016-05-09 14:55 GMT+02:00 Mark Struberg <st...@yahoo.de.invalid>:
>
>Having a really hard time to grok it right now and think we can improve quite a few things.
>>
>>1.) It atm only works with exactly a single InvocationHandler. That's currently a problem in my case
>>-> We imo should split the creation of the proxy class from the creation of the proxy instance.
>>
>>
>>2.) It's hard to get what is in the API and what is an impl detail.
>>
>>-> We should create an interface and move the impl stuff to an internal class.
>>
>>Any objections?
>>
>>How freely can I change the API?
>>Is it only important that our internal tests and the JPA stuff is working fine again, or do I need to care about native usages as well?
>>
>>
>>I also don't get why the current DeltaSpikeProxyFactory needs the BeanManager as parameter?
>>Maybe we could introduce a new API which just exposes a native java subclass proxy to retain backward compat?
>>In any case we miss a lot of JavaDocs!
>>
>>
>>LieGrue,
>>strub
>>
>>
>>
>>
>>
>>
>>
>>
>>> On Monday, 9 May 2016, 13:47, Thomas Andraschko <an...@gmail.com> wrote:
>>> > a) yup
>>> b) if it's unused, why not ;) Should be just an old case, it was refactored
>>> many times during developed because of some AppServers / versions.
>>> c) yup, feel free to align it :)
>>>
>>>
>>> 2016-05-09 13:30 GMT+02:00 John D. Ament <jo...@apache.org>:
>>>
>>>>  On Mon, May 9, 2016 at 7:24 AM Mark Struberg
>>> <st...@yahoo.de.invalid>
>>>>  wrote:
>>>>
>>>>  > a.)DeltaSpikeProxyFactory totally misses docs it sems. Or did I miss
>>>>  > something?
>>>>  >
>>>>
>>>>  The current docs are a step up from about two months ago when the only
>>>>  thing on the doc page was "TODO: Document the proxy module" or
>>> something
>>>>  along those lines.
>>>>
>>>>  Yes, there's still stuff missing.
>>>>
>>>>
>>>>  >
>>>>  > b.) there are methods which are imo questionable, e.g.
>>>>  >
>>>>  > public <T> Class<T> getProxyClass(BeanManager beanManager,
>>> Class<T>
>>>>  > targetClass)
>>>>  > {
>>>>  > return getProxyClass(beanManager, targetClass,
>>>>  > DummyInvocationHandler.class);
>>>>  > }
>>>>  >
>>>>  >
>>>>  > what do we need this for? Can I simply remove it?
>>>>  > The DummyInvocationHandler just returns null. Without calling the
>>> proxied
>>>>  > instance. It's basically a no-op impl. Why do we need this?
>>>>  >
>>>>
>>>>  This smells like a strangle that wasn't completed.  I'd say if it
>>> can be
>>>>  delegated down, lets remove it.  Better is if its only used in tests.
>>>>
>>>>
>>>>  >
>>>>  >
>>>>  > c.) The ordering of the methods are mixed. Imo all public methods
>>> should
>>>>  > be on top, protected and private at the bottom. Really hard to read
>>> atm.
>>>>  >
>>>>
>>>>  +1
>>>>
>>>>
>>>>  >
>>>>  > Any thoughts?
>>>>  >
>>>>  > LieGrue,
>>>>  > strub
>>>>  >
>>>>
>>>
>>
>
>
>

Re: DeltaSpikeProxyFactory questions

Posted by Thomas Andraschko <an...@gmail.com>.
1) exactly, it was designed for that purpose. Therefor we always pass
InvocationHandler in the constructor.
    Do you need different InvocationHandler for different methods?

| -> We imo should split the creation of the proxy class from the creation
of the proxy instance.

It's completely splitted currently. DeltaSpikeProxyFactory contains only
the proxy class creation. proxy instance creation is always done outside.


2) it's ok for me but please let me review before commiting something.
    For me it's only important that every test is working fine (proxy
module, partial bean, data and jsf module).


The beanManager is required to check for interceptor bindings. We must
proxy every method if interceptors are used. Otherwise we only proxy the
"delegate methods".


| Maybe we could introduce a new API which just exposes a native java
subclass proxy to retain backward compat?

What do you mean exactly?




2016-05-09 14:55 GMT+02:00 Mark Struberg <st...@yahoo.de.invalid>:

> Having a really hard time to grok it right now and think we can improve
> quite a few things.
>
> 1.) It atm only works with exactly a single InvocationHandler. That's
> currently a problem in my case
> -> We imo should split the creation of the proxy class from the creation
> of the proxy instance.
>
>
> 2.) It's hard to get what is in the API and what is an impl detail.
>
> -> We should create an interface and move the impl stuff to an internal
> class.
>
> Any objections?
>
> How freely can I change the API?
> Is it only important that our internal tests and the JPA stuff is working
> fine again, or do I need to care about native usages as well?
>
>
> I also don't get why the current DeltaSpikeProxyFactory needs the
> BeanManager as parameter?
> Maybe we could introduce a new API which just exposes a native java
> subclass proxy to retain backward compat?
> In any case we miss a lot of JavaDocs!
>
>
> LieGrue,
> strub
>
>
>
>
>
>
>
> > On Monday, 9 May 2016, 13:47, Thomas Andraschko <
> andraschko.thomas@gmail.com> wrote:
> > > a) yup
> > b) if it's unused, why not ;) Should be just an old case, it was
> refactored
> > many times during developed because of some AppServers / versions.
> > c) yup, feel free to align it :)
> >
> >
> > 2016-05-09 13:30 GMT+02:00 John D. Ament <jo...@apache.org>:
> >
> >>  On Mon, May 9, 2016 at 7:24 AM Mark Struberg
> > <st...@yahoo.de.invalid>
> >>  wrote:
> >>
> >>  > a.)DeltaSpikeProxyFactory totally misses docs it sems. Or did I miss
> >>  > something?
> >>  >
> >>
> >>  The current docs are a step up from about two months ago when the only
> >>  thing on the doc page was "TODO: Document the proxy module" or
> > something
> >>  along those lines.
> >>
> >>  Yes, there's still stuff missing.
> >>
> >>
> >>  >
> >>  > b.) there are methods which are imo questionable, e.g.
> >>  >
> >>  > public <T> Class<T> getProxyClass(BeanManager beanManager,
> > Class<T>
> >>  > targetClass)
> >>  > {
> >>  > return getProxyClass(beanManager, targetClass,
> >>  > DummyInvocationHandler.class);
> >>  > }
> >>  >
> >>  >
> >>  > what do we need this for? Can I simply remove it?
> >>  > The DummyInvocationHandler just returns null. Without calling the
> > proxied
> >>  > instance. It's basically a no-op impl. Why do we need this?
> >>  >
> >>
> >>  This smells like a strangle that wasn't completed.  I'd say if it
> > can be
> >>  delegated down, lets remove it.  Better is if its only used in tests.
> >>
> >>
> >>  >
> >>  >
> >>  > c.) The ordering of the methods are mixed. Imo all public methods
> > should
> >>  > be on top, protected and private at the bottom. Really hard to read
> > atm.
> >>  >
> >>
> >>  +1
> >>
> >>
> >>  >
> >>  > Any thoughts?
> >>  >
> >>  > LieGrue,
> >>  > strub
> >>  >
> >>
> >
>

Re: DeltaSpikeProxyFactory questions

Posted by Mark Struberg <st...@yahoo.de.INVALID>.
Having a really hard time to grok it right now and think we can improve quite a few things.

1.) It atm only works with exactly a single InvocationHandler. That's currently a problem in my case
-> We imo should split the creation of the proxy class from the creation of the proxy instance.


2.) It's hard to get what is in the API and what is an impl detail. 

-> We should create an interface and move the impl stuff to an internal class.

Any objections?

How freely can I change the API?
Is it only important that our internal tests and the JPA stuff is working fine again, or do I need to care about native usages as well?


I also don't get why the current DeltaSpikeProxyFactory needs the BeanManager as parameter?
Maybe we could introduce a new API which just exposes a native java subclass proxy to retain backward compat?
In any case we miss a lot of JavaDocs!


LieGrue,
strub







> On Monday, 9 May 2016, 13:47, Thomas Andraschko <an...@gmail.com> wrote:
> > a) yup
> b) if it's unused, why not ;) Should be just an old case, it was refactored
> many times during developed because of some AppServers / versions.
> c) yup, feel free to align it :)
> 
> 
> 2016-05-09 13:30 GMT+02:00 John D. Ament <jo...@apache.org>:
> 
>>  On Mon, May 9, 2016 at 7:24 AM Mark Struberg 
> <st...@yahoo.de.invalid>
>>  wrote:
>> 
>>  > a.)DeltaSpikeProxyFactory totally misses docs it sems. Or did I miss
>>  > something?
>>  >
>> 
>>  The current docs are a step up from about two months ago when the only
>>  thing on the doc page was "TODO: Document the proxy module" or 
> something
>>  along those lines.
>> 
>>  Yes, there's still stuff missing.
>> 
>> 
>>  >
>>  > b.) there are methods which are imo questionable, e.g.
>>  >
>>  > public <T> Class<T> getProxyClass(BeanManager beanManager, 
> Class<T>
>>  > targetClass)
>>  > {
>>  > return getProxyClass(beanManager, targetClass,
>>  > DummyInvocationHandler.class);
>>  > }
>>  >
>>  >
>>  > what do we need this for? Can I simply remove it?
>>  > The DummyInvocationHandler just returns null. Without calling the 
> proxied
>>  > instance. It's basically a no-op impl. Why do we need this?
>>  >
>> 
>>  This smells like a strangle that wasn't completed.  I'd say if it 
> can be
>>  delegated down, lets remove it.  Better is if its only used in tests.
>> 
>> 
>>  >
>>  >
>>  > c.) The ordering of the methods are mixed. Imo all public methods 
> should
>>  > be on top, protected and private at the bottom. Really hard to read 
> atm.
>>  >
>> 
>>  +1
>> 
>> 
>>  >
>>  > Any thoughts?
>>  >
>>  > LieGrue,
>>  > strub
>>  >
>> 
> 

Re: DeltaSpikeProxyFactory questions

Posted by Thomas Andraschko <an...@gmail.com>.
a) yup
b) if it's unused, why not ;) Should be just an old case, it was refactored
many times during developed because of some AppServers / versions.
c) yup, feel free to align it :)

2016-05-09 13:30 GMT+02:00 John D. Ament <jo...@apache.org>:

> On Mon, May 9, 2016 at 7:24 AM Mark Struberg <st...@yahoo.de.invalid>
> wrote:
>
> > a.)DeltaSpikeProxyFactory totally misses docs it sems. Or did I miss
> > something?
> >
>
> The current docs are a step up from about two months ago when the only
> thing on the doc page was "TODO: Document the proxy module" or something
> along those lines.
>
> Yes, there's still stuff missing.
>
>
> >
> > b.) there are methods which are imo questionable, e.g.
> >
> > public <T> Class<T> getProxyClass(BeanManager beanManager, Class<T>
> > targetClass)
> > {
> > return getProxyClass(beanManager, targetClass,
> > DummyInvocationHandler.class);
> > }
> >
> >
> > what do we need this for? Can I simply remove it?
> > The DummyInvocationHandler just returns null. Without calling the proxied
> > instance. It's basically a no-op impl. Why do we need this?
> >
>
> This smells like a strangle that wasn't completed.  I'd say if it can be
> delegated down, lets remove it.  Better is if its only used in tests.
>
>
> >
> >
> > c.) The ordering of the methods are mixed. Imo all public methods should
> > be on top, protected and private at the bottom. Really hard to read atm.
> >
>
> +1
>
>
> >
> > Any thoughts?
> >
> > LieGrue,
> > strub
> >
>

Re: DeltaSpikeProxyFactory questions

Posted by "John D. Ament" <jo...@apache.org>.
On Mon, May 9, 2016 at 7:24 AM Mark Struberg <st...@yahoo.de.invalid>
wrote:

> a.)DeltaSpikeProxyFactory totally misses docs it sems. Or did I miss
> something?
>

The current docs are a step up from about two months ago when the only
thing on the doc page was "TODO: Document the proxy module" or something
along those lines.

Yes, there's still stuff missing.


>
> b.) there are methods which are imo questionable, e.g.
>
> public <T> Class<T> getProxyClass(BeanManager beanManager, Class<T>
> targetClass)
> {
> return getProxyClass(beanManager, targetClass,
> DummyInvocationHandler.class);
> }
>
>
> what do we need this for? Can I simply remove it?
> The DummyInvocationHandler just returns null. Without calling the proxied
> instance. It's basically a no-op impl. Why do we need this?
>

This smells like a strangle that wasn't completed.  I'd say if it can be
delegated down, lets remove it.  Better is if its only used in tests.


>
>
> c.) The ordering of the methods are mixed. Imo all public methods should
> be on top, protected and private at the bottom. Really hard to read atm.
>

+1


>
> Any thoughts?
>
> LieGrue,
> strub
>