You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@isis.apache.org by Martin Grigorov <mg...@apache.org> on 2015/03/04 20:20:30 UTC

ISIS-1062 Guice injector should create serializable proxies for the injected beans

Hi Dan,

I still don't understand how the tiny change of a boolean in in Java class
[1] adds ASM in the classpath. It is either in the classpath and everything
works or it is not there and usage at runtime fails with
ClassNotFoundException.
It seems ASM is in the classpath already because everything was working
fine so far.
With changing the boolean flag to true wicket-ioc just started using it,
but didn't introduce it!

I see two possible solutions:

1) use serializable proxies for Guice beans and make sure only ASM 4.x is
in the classpath since it is required by DataNucleus.
AFAIK ASM tries to keep its APIs stable between major releases so there is
no problem if wicket-ioc uses ASM 4.x. Wicket 7.x uses ASM 5.x for better
Java 8 support and again there were no changes in wicket-ioc classes for
this update.

2) make all Guice beans in Isis serializable
This will solve the issue that led to ISIS-1062 but if an application adds
custom Guice beans then they have to make sure they are serializable

IMO 1) is better.

Martin

1.
https://git-wip-us.apache.org/repos/asf?p=isis.git;a=commitdiff;h=f800860;hp=55e569c9bbadddd3de1b5e6e703181400424658d
(this is the revert)

Re: ISIS-1062 Guice injector should create serializable proxies for the injected beans

Posted by Martin Grigorov <mg...@apache.org>.
OK.

I've
made org.apache.isis.viewer.wicket.ui.pages.EmailVerificationUrlService
serializable.
Now
only org.apache.isis.viewer.wicket.ui.app.registry.ComponentFactoryRegistry
and org.apache.isis.viewer.wicket.ui.app.registry.ComponentFactoryRegistrar
are not serializable. They are usually used in MyApplication class so there
is no real need to serialize them.
An application may inject them in a Wicket component though and this will
break the storing of the page.

Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov

On Thu, Mar 5, 2015 at 10:45 AM, Dan Haywood <da...@haywood-associates.co.uk>
wrote:

> OK; apols if I was being unclear in my comments on that ticket.
>
> What I found was that running the app gave me a ClassDefNotFound for cglib
>   (This is the isis-app-todoapp in isisaddons).
>
> It's possible (I didn't check) that there is an explicit exclusion of cglib
> somewhere.
> If so, I would have done that deliberately because I spent some time
> eradicating all our various uses of cglib 2.x (=asm 3) in order to be ready
> to move up to DN 4.x with its dependency on asm 4.
> For example, I reimplemented the JMock ClassImposteriser using javassist
> rather than the provided cglib one, and have standardized on our javassist
> version of our own wrapper factory.
> I also noticed that wicket-guice had this dependency, but figured (at the
> time) that we didn't need the feature that used cglib, so that's probably
> where the explicit exclusion came from.
> So... you can see why I'm not keen to bring in the cglib dependency again
> if possible.
>
> On the other hand...
> ... if wicket-guice uses asm 4.x, or is otherwise compatible with DN, then
> I guess we can consider it once more.
>
> On 4 March 2015 at 19:20, Martin Grigorov <mg...@apache.org> wrote:
>
> > Hi Dan,
> >
> > I still don't understand how the tiny change of a boolean in in Java
> class
> > [1] adds ASM in the classpath. It is either in the classpath and
> everything
> > works or it is not there and usage at runtime fails with
> > ClassNotFoundException.
> > It seems ASM is in the classpath already because everything was working
> > fine so far.
> > With changing the boolean flag to true wicket-ioc just started using it,
> > but didn't introduce it!
> >
> > I see two possible solutions:
> >
> > 1) use serializable proxies for Guice beans and make sure only ASM 4.x is
> > in the classpath since it is required by DataNucleus.
> > AFAIK ASM tries to keep its APIs stable between major releases so there
> is
> > no problem if wicket-ioc uses ASM 4.x. Wicket 7.x uses ASM 5.x for better
> > Java 8 support and again there were no changes in wicket-ioc classes for
> > this update.
> >
> > 2) make all Guice beans in Isis serializable
> > This will solve the issue that led to ISIS-1062 but if an application
> adds
> > custom Guice beans then they have to make sure they are serializable
> >
> > IMO 1) is better.
> >
> > Martin
> >
> > 1.
> >
> >
> https://git-wip-us.apache.org/repos/asf?p=isis.git;a=commitdiff;h=f800860;hp=55e569c9bbadddd3de1b5e6e703181400424658d
> > (this is the revert)
> >
>

Re: ISIS-1062 Guice injector should create serializable proxies for the injected beans

Posted by Dan Haywood <da...@haywood-associates.co.uk>.
OK; apols if I was being unclear in my comments on that ticket.

What I found was that running the app gave me a ClassDefNotFound for cglib
  (This is the isis-app-todoapp in isisaddons).

It's possible (I didn't check) that there is an explicit exclusion of cglib
somewhere.
If so, I would have done that deliberately because I spent some time
eradicating all our various uses of cglib 2.x (=asm 3) in order to be ready
to move up to DN 4.x with its dependency on asm 4.
For example, I reimplemented the JMock ClassImposteriser using javassist
rather than the provided cglib one, and have standardized on our javassist
version of our own wrapper factory.
I also noticed that wicket-guice had this dependency, but figured (at the
time) that we didn't need the feature that used cglib, so that's probably
where the explicit exclusion came from.
So... you can see why I'm not keen to bring in the cglib dependency again
if possible.

On the other hand...
... if wicket-guice uses asm 4.x, or is otherwise compatible with DN, then
I guess we can consider it once more.

On 4 March 2015 at 19:20, Martin Grigorov <mg...@apache.org> wrote:

> Hi Dan,
>
> I still don't understand how the tiny change of a boolean in in Java class
> [1] adds ASM in the classpath. It is either in the classpath and everything
> works or it is not there and usage at runtime fails with
> ClassNotFoundException.
> It seems ASM is in the classpath already because everything was working
> fine so far.
> With changing the boolean flag to true wicket-ioc just started using it,
> but didn't introduce it!
>
> I see two possible solutions:
>
> 1) use serializable proxies for Guice beans and make sure only ASM 4.x is
> in the classpath since it is required by DataNucleus.
> AFAIK ASM tries to keep its APIs stable between major releases so there is
> no problem if wicket-ioc uses ASM 4.x. Wicket 7.x uses ASM 5.x for better
> Java 8 support and again there were no changes in wicket-ioc classes for
> this update.
>
> 2) make all Guice beans in Isis serializable
> This will solve the issue that led to ISIS-1062 but if an application adds
> custom Guice beans then they have to make sure they are serializable
>
> IMO 1) is better.
>
> Martin
>
> 1.
>
> https://git-wip-us.apache.org/repos/asf?p=isis.git;a=commitdiff;h=f800860;hp=55e569c9bbadddd3de1b5e6e703181400424658d
> (this is the revert)
>