You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Maarten Bosteels <mb...@gmail.com> on 2007/09/30 00:13:55 UTC

MdcInjectionFilter enhancement

Hi all,

We should add a method for remving a property from the context:

    public static void removeProperty(IoSession session, String key) {
      if (key == null) {
        throw new NullPointerException("key should not be null");
      }
      Context context = getContext(session);
      context.remove(key);
    }

And maybe the option to immediately set the property in the MDC, since
in the current implementation, the property does not show up for log
statements generated during the handling of the current event.
Therefor I suggest adding this method:

    public static void setProperty (IoSession session, String key,
String value, boolean immediate) {
      if (key == null) {
        throw new NullPointerException("key should not be null");
      }
      if (value == null) {
        removeProperty(session, key);
      }
      Context context = getContext(session);
      context.put(key, value);
      if (immediate) {
        MDC.put(key, value);
      }
    }

And a default value of true:

    public static void setProperty (IoSession session, String key,
String value) {
      setProperty(session, key, value, true);
    }

I thought about checking the callDepth property instead of the extra
parameter, but the property isn't static. I think the extra parameter
is an acceptable solution.

WDYT ?

Maarten

Re: MdcInjectionFilter enhancement

Posted by Trustin Lee <tr...@gmail.com>.
On 9/30/07, Maarten Bosteels <mb...@gmail.com> wrote:
> On 9/30/07, Trustin Lee <tr...@gmail.com> wrote:
> > Hi Maarten,
> >
> > On 9/30/07, Maarten Bosteels <mb...@gmail.com> wrote:
> > > Hi all,
> > >
> > > We should add a method for remving a property from the context:
> > >
> > >     public static void removeProperty(IoSession session, String key) {
> > >       if (key == null) {
> > >         throw new NullPointerException("key should not be null");
> > >       }
> > >       Context context = getContext(session);
> > >       context.remove(key);
> > >     }
> >
> > Good idea.
> >
> > > And maybe the option to immediately set the property in the MDC, since
> > > in the current implementation, the property does not show up for log
> > > statements generated during the handling of the current event.
> > > Therefor I suggest adding this method:
> > >
> > >     public static void setProperty (IoSession session, String key,
> > > String value, boolean immediate) {
> > >       if (key == null) {
> > >         throw new NullPointerException("key should not be null");
> > >       }
> > >       if (value == null) {
> > >         removeProperty(session, key);
> > >       }
> > >       Context context = getContext(session);
> > >       context.put(key, value);
> > >       if (immediate) {
> > >         MDC.put(key, value);
> > >       }
> > >     }
> > >
> > > And a default value of true:
> > >
> > >     public static void setProperty (IoSession session, String key,
> > > String value) {
> > >       setProperty(session, key, value, true);
> > >     }
> >
> > Is there any case that a user need to call with false?
>
> I think it will be rare indeed. Suppose some user thread (not related
> to a mina event) is looping over a set of IoSessions and sets a
> property in the context of each session.
> Maybe it's too rare to take into account ?
> You think the extra parameter is cluttering the API ?

I can't think of a practical usage of the scenario you described.  We
could probably add it later when there's any user demand. :)

Cheers,
Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: MdcInjectionFilter enhancement

Posted by Maarten Bosteels <mb...@gmail.com>.
On 9/30/07, Trustin Lee <tr...@gmail.com> wrote:
> Hi Maarten,
>
> On 9/30/07, Maarten Bosteels <mb...@gmail.com> wrote:
> > Hi all,
> >
> > We should add a method for remving a property from the context:
> >
> >     public static void removeProperty(IoSession session, String key) {
> >       if (key == null) {
> >         throw new NullPointerException("key should not be null");
> >       }
> >       Context context = getContext(session);
> >       context.remove(key);
> >     }
>
> Good idea.
>
> > And maybe the option to immediately set the property in the MDC, since
> > in the current implementation, the property does not show up for log
> > statements generated during the handling of the current event.
> > Therefor I suggest adding this method:
> >
> >     public static void setProperty (IoSession session, String key,
> > String value, boolean immediate) {
> >       if (key == null) {
> >         throw new NullPointerException("key should not be null");
> >       }
> >       if (value == null) {
> >         removeProperty(session, key);
> >       }
> >       Context context = getContext(session);
> >       context.put(key, value);
> >       if (immediate) {
> >         MDC.put(key, value);
> >       }
> >     }
> >
> > And a default value of true:
> >
> >     public static void setProperty (IoSession session, String key,
> > String value) {
> >       setProperty(session, key, value, true);
> >     }
>
> Is there any case that a user need to call with false?

I think it will be rare indeed. Suppose some user thread (not related
to a mina event) is looping over a set of IoSessions and sets a
property in the context of each session.
Maybe it's too rare to take into account ?
You think the extra parameter is cluttering the API ?

>
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
> --
> PGP Key ID: 0x0255ECA6
>

Re: MdcInjectionFilter enhancement

Posted by Maarten Bosteels <mb...@gmail.com>.
On 9/30/07, Trustin Lee <tr...@gmail.com> wrote:
> One more question:  what is the purpose of calling MDC.remove("name")?

oops, it's a left-over from my debugging :-p
I will remove it.

>
> On 9/30/07, Trustin Lee <tr...@gmail.com> wrote:
> > Hi Maarten,
> >
> > On 9/30/07, Maarten Bosteels <mb...@gmail.com> wrote:
> > > Hi all,
> > >
> > > We should add a method for remving a property from the context:
> > >
> > >     public static void removeProperty(IoSession session, String key) {
> > >       if (key == null) {
> > >         throw new NullPointerException("key should not be null");
> > >       }
> > >       Context context = getContext(session);
> > >       context.remove(key);
> > >     }
> >
> > Good idea.
> >
> > > And maybe the option to immediately set the property in the MDC, since
> > > in the current implementation, the property does not show up for log
> > > statements generated during the handling of the current event.
> > > Therefor I suggest adding this method:
> > >
> > >     public static void setProperty (IoSession session, String key,
> > > String value, boolean immediate) {
> > >       if (key == null) {
> > >         throw new NullPointerException("key should not be null");
> > >       }
> > >       if (value == null) {
> > >         removeProperty(session, key);
> > >       }
> > >       Context context = getContext(session);
> > >       context.put(key, value);
> > >       if (immediate) {
> > >         MDC.put(key, value);
> > >       }
> > >     }
> > >
> > > And a default value of true:
> > >
> > >     public static void setProperty (IoSession session, String key,
> > > String value) {
> > >       setProperty(session, key, value, true);
> > >     }
> >
> > Is there any case that a user need to call with false?
> >
> > Trustin
> > --
> > what we call human nature is actually human habit
> > --
> > http://gleamynode.net/
> > --
> > PGP Key ID: 0x0255ECA6
> >
>
>
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
> --
> PGP Key ID: 0x0255ECA6
>

Re: MdcInjectionFilter enhancement

Posted by Trustin Lee <tr...@gmail.com>.
One more question:  what is the purpose of calling MDC.remove("name")?

On 9/30/07, Trustin Lee <tr...@gmail.com> wrote:
> Hi Maarten,
>
> On 9/30/07, Maarten Bosteels <mb...@gmail.com> wrote:
> > Hi all,
> >
> > We should add a method for remving a property from the context:
> >
> >     public static void removeProperty(IoSession session, String key) {
> >       if (key == null) {
> >         throw new NullPointerException("key should not be null");
> >       }
> >       Context context = getContext(session);
> >       context.remove(key);
> >     }
>
> Good idea.
>
> > And maybe the option to immediately set the property in the MDC, since
> > in the current implementation, the property does not show up for log
> > statements generated during the handling of the current event.
> > Therefor I suggest adding this method:
> >
> >     public static void setProperty (IoSession session, String key,
> > String value, boolean immediate) {
> >       if (key == null) {
> >         throw new NullPointerException("key should not be null");
> >       }
> >       if (value == null) {
> >         removeProperty(session, key);
> >       }
> >       Context context = getContext(session);
> >       context.put(key, value);
> >       if (immediate) {
> >         MDC.put(key, value);
> >       }
> >     }
> >
> > And a default value of true:
> >
> >     public static void setProperty (IoSession session, String key,
> > String value) {
> >       setProperty(session, key, value, true);
> >     }
>
> Is there any case that a user need to call with false?
>
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
> --
> PGP Key ID: 0x0255ECA6
>


-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: MdcInjectionFilter enhancement

Posted by Trustin Lee <tr...@gmail.com>.
Hi Maarten,

On 9/30/07, Maarten Bosteels <mb...@gmail.com> wrote:
> Hi all,
>
> We should add a method for remving a property from the context:
>
>     public static void removeProperty(IoSession session, String key) {
>       if (key == null) {
>         throw new NullPointerException("key should not be null");
>       }
>       Context context = getContext(session);
>       context.remove(key);
>     }

Good idea.

> And maybe the option to immediately set the property in the MDC, since
> in the current implementation, the property does not show up for log
> statements generated during the handling of the current event.
> Therefor I suggest adding this method:
>
>     public static void setProperty (IoSession session, String key,
> String value, boolean immediate) {
>       if (key == null) {
>         throw new NullPointerException("key should not be null");
>       }
>       if (value == null) {
>         removeProperty(session, key);
>       }
>       Context context = getContext(session);
>       context.put(key, value);
>       if (immediate) {
>         MDC.put(key, value);
>       }
>     }
>
> And a default value of true:
>
>     public static void setProperty (IoSession session, String key,
> String value) {
>       setProperty(session, key, value, true);
>     }

Is there any case that a user need to call with false?

Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6