On Mon, 2006-02-20 at 21:47 -0800, Craig McClanahan wrote:
> No, I was not aware of that change ... but does it actually work?
> Declaring something Serializable is not by itself sufficient if there
> are transient variables inside the implementation.  (On a separate
> thread on commons-dev, I recommended that there be unit tests to
> validate this behavior:  use a Log instance, serialize it, deserialize
> it, and use it some more). 

I believe Serializable is correctly implemented for all standard
adapters except possibly AvalonLog [will follow that up on commons-dev].
The deserialized objects correctly locate a fresh "underlying" log
object to forward calls to.

Yes there need to be unit tests for this; I'll look for them and add
some if not already present. 

> 
> 
>         The 1.0.4 release occurred around June 2004 so perhaps there
>         are a few
>         containers out there capable of running MyFaces but which are
>         still on 
>         JCL 1.0.3. In addition there might be a few custom Log
>         adapters around
>         which *might* not implement Serializable.
>         
>         However in view of the complicated/ugly workaround for
>         non-serializable
>         Log implementations, it seems ok to me for MyFaces to just
>         declare a 
>         dependency on JCL 1.0.4. If people don't comply then there is
>         a very
>         obvious "NotSerializableException: logAdapterClassName"
>         message
>         generated. The fix is then to upgrade to 1.0.4 (which is
>         binary
>         compatible with earlier releases) or a later release. 
>         
>         Comments/Opinions?
> 
> +1 on declaring a dependency on Commons Logging 1.0.4 ... anything
> prior to that is just asking for trouble.  I use 1.0.4 exclusively for
> eveything that has a C-L dependency ... works fine, lasts a long
> time :-). 

So the conclusion seems to be that if/when using commons-logging in
MyFaces, use:
  private Log foo = LogFactory.getLog("foo");
everywhere except in short-lived and frequently-created objects. In
those cases, either call LogFactory.getLog as needed or have some
longer-lived object store the Log object.

For StateHolder classes, things "just work"; the logger isn't saved in
saveState; it gets recreated in the new component and that works.

For Serializable classes, the Log object handles its own serialization
correctly. No need for "transient", no need for any other special hacks.

MyFaces does need to declare a dependency on commons-logging 1.0.4 (not
earlier versions) in order for things to work this simply. Release 1.0.4
was in June 2004.

Do NOT use "static Log" anywhere.

Is this all ok?

Cheers,

Simon

Reply via email to