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 <[email protected]>:
> 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 <
> [email protected]> 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 <[email protected]>:
> >
> >> On Mon, May 9, 2016 at 7:24 AM Mark Struberg
> > <[email protected]>
> >> 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
> >> >
> >>
> >
>
