You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by mw...@apache.org on 2005/02/22 06:24:54 UTC

cvs commit: logging-log4j/src/java/org/apache/log4j/joran/action Action.java RepositoryPropertyAction.java

mwomack     2005/02/21 21:24:54

  Modified:    src/java/org/apache/log4j/joran/action Action.java
                        RepositoryPropertyAction.java
  Log:
  Moved getLoggerRepository to base Action class so it can be used by all subclasses.  Centralizes logic for locating the LoggerRepository being acted upon.
  
  Revision  Changes    Path
  1.3       +20 -0     logging-log4j/src/java/org/apache/log4j/joran/action/Action.java
  
  Index: Action.java
  ===================================================================
  RCS file: /home/cvs/logging-log4j/src/java/org/apache/log4j/joran/action/Action.java,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- Action.java	13 Jan 2005 16:12:26 -0000	1.2
  +++ Action.java	22 Feb 2005 05:24:54 -0000	1.3
  @@ -20,6 +20,8 @@
   import org.apache.log4j.joran.spi.ExecutionContext;
   import org.apache.log4j.joran.spi.Interpreter;
   import org.apache.log4j.spi.ComponentBase;
  +import org.apache.log4j.spi.LoggerRepository;
  +import org.apache.log4j.spi.ErrorItem;
   
   import org.xml.sax.Attributes;
   import org.xml.sax.Locator;
  @@ -79,4 +81,22 @@
       }
       return -1;
     }
  +  
  +  /**
  +   * Helper method to return the LoggerRepository of the  execution context.
  +   *
  +   * @param ec The ExecutionContext that contains the reference to the
  +   *   LoggerRepository
  +   * @return The LoggerRepository
  +   */
  +  protected LoggerRepository getLoggerRepository(ExecutionContext ec) {
  +    Object o = ec.getObject(0);
  +    if(o instanceof LoggerRepository) {
  +      return (LoggerRepository) o;
  +    } else {
  +      String errMsg = "There is no LoggerRepository at the top of the object stack.";
  +      ec.addError(new ErrorItem(errMsg));
  +      throw new IllegalStateException(errMsg);
  +    }
  +  }
   }
  
  
  
  1.5       +0 -11     logging-log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java
  
  Index: RepositoryPropertyAction.java
  ===================================================================
  RCS file: /home/cvs/logging-log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java,v
  retrieving revision 1.4
  retrieving revision 1.5
  diff -u -r1.4 -r1.5
  --- RepositoryPropertyAction.java	12 Jan 2005 15:04:18 -0000	1.4
  +++ RepositoryPropertyAction.java	22 Feb 2005 05:24:54 -0000	1.5
  @@ -28,17 +28,6 @@
    * Window>Preferences>Java>Code Generation>Code and Comments
    */
   public class RepositoryPropertyAction extends PropertyAction {
  -
  -  LoggerRepository getLoggerRepository(ExecutionContext ec) {
  -    Object o = ec.getObjectStack().get(0);
  -    if(o instanceof LoggerRepository) {
  -      return (LoggerRepository) o;
  -    } else {
  -      String errMsg = "There is no LoggerRepository at the top of the object stack.";
  -      ec.addError(new ErrorItem(errMsg));
  -      throw new IllegalStateException(errMsg);
  -    }
  -  }
     
     public void setProperties(ExecutionContext ec, Properties props) {
       LoggerRepository repository = getLoggerRepository(ec);
  
  
  

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


RE: cvs commit: logging-log4j/src/java/org/apache/log4j/joran/action Action.java RepositoryPropertyAction.java

Posted by Ceki Gülcü <ce...@qos.ch>.

Placing objects shared by multiple actions at the bottom of the execution 
context stack is the usual pattern for managing shared objects in Joran. 
Since LR happens to be also set through SimpleRuleStore, we get two 
redundant ways of accessing the LR.  Both are correct and existing actions 
can chose either method.

However, adding a getLoggerRepository method to Action makes a somewhat 
confusing situation a little worse. :-)


At 08:02 PM 2/24/2005, Mark Womack wrote:
>Hey Ceki,
>
>I'll have to look at the specific code you are referring to tonight, I don't
>have the code in front of me.  But it sounds like I should replace all the
>code that accesses the bottom of the stack with code to access whatever
>SimpleRuleStore sets up.  And clean up what I have already added.  I'll look
>at it tonight and let you know if I have any questions.
>
>I'm still looking at the reset configuration issues.  I am trying to set up
>a generic way to keep track of the "temp" appenders so that they can be
>removed and reattached.  Some of the issues I was seeing may have been
>related to the recent appender changes.  So, I will be looking at it again
>tonight.
>
>-Mark
>
> > -----Original Message-----
> > From: Ceki Gülcü [mailto:ceki@qos.ch]
> > Sent: Thursday, February 24, 2005 10:53 AM
> > To: Log4J Developers List; 'Log4J Developers List'
> > Subject: RE: cvs commit: logging-
> > log4j/src/java/org/apache/log4j/joran/action Action.java
> > RepositoryPropertyAction.java
> >
> >
> > Hi Mark,
> >
> > What you have done makes perfect sense. However, the addRule method in
> > SimpleRuleStore already sets the LoggerRepository for all Action instances
> > that JoranConfigurator refers to. There is no point in adding a
> > getLoggerRepository in Action if it is already in ComponentBase.
> >
> > In other words, the LoggerRepository entry at the bottom of the stack is
> > redundant.
> >
> > Does that make sense?
> >
> > At 10:32 PM 2/23/2005, Mark Womack wrote:
> > >Ceki,
> > >
> > >This version of the getLoggerRepository method returns the bottom element
> > of
> > >the Joran execution stack (element 0) which is set up by the
> > >JoranConfigurator at the start of configuration.  Many of the existing
> > >actions refer to element 0 to grab the LoggerRepository being configured,
> > >but I did not replace all the uses with this change.  I was waiting for
> > >someone to comment before I did that. :-)
> > >
> > >Are you saying that Actions should not be using element 0 as the
> > >LoggerRepository to act upon?  I just added/moved it as a common helper
> > >method so that the code would be in one place and could be easily updated
> > if
> > >it needed to change in the future.  From a design point of view, it seems
> > >better if Actions use an explicit method to get the "logger repository
> > being
> > >configured" than having know to grab it from element 0 of the current
> > >execution context.  A different method name might be better.
> > >
> > >Let me know if this does not make sense.  I'll be happy to make whatever
> > >changes are needed.  If we decide to keep the helper method, I'll update
> > the
> > >other Actions to use it instead of referencing element 0 of the ec.
> > >
> > >-Mark
> > >
> > > > -----Original Message-----
> > > > From: Ceki Gülcü [mailto:ceki@qos.ch]
> > > > Sent: Wednesday, February 23, 2005 10:41 AM
> > > > To: Log4J Developers List; logging-log4j-cvs@apache.org
> > > > Cc: mwomack@apache.org
> > > > Subject: Re: cvs commit: logging-
> > > > log4j/src/java/org/apache/log4j/joran/action Action.java
> > > > RepositoryPropertyAction.java
> > > >
> > > >
> > > > Mark,
> > > >
> > > > The getLoggerRepository(ExecutionContext ec) method should not be part
> > of
> > > > the Action class because ComponentBase already knows about its LR. The
> > > > method getLoggerRepository should be removed from Action and should
> > not
> > > > have been part of RepositoryPropertyAction.
> > > >
> > > > At 06:24 AM 2/22/2005, mwomack@apache.org wrote:
> > > > >mwomack     2005/02/21 21:24:54
> > > > >
> > > > >   Modified:    src/java/org/apache/log4j/joran/action Action.java
> > > > >                         RepositoryPropertyAction.java
> > > > >   Log:
> > > > >   Moved getLoggerRepository to base Action class so it can be used
> > by
> > > > all
> > > > > subclasses.  Centralizes logic for locating the LoggerRepository
> > being
> > > > > acted upon.
> > > > >
> > > > >   Revision  Changes    Path
> > > > >   1.3       +20
> > > > > -0     logging-
> > log4j/src/java/org/apache/log4j/joran/action/Action.java
> > > > >
> > > > >   Index: Action.java
> > > > >
> > ===================================================================
> > > > >   RCS file:
> > > > > /home/cvs/logging-
> > > > log4j/src/java/org/apache/log4j/joran/action/Action.java,v
> > > > >   retrieving revision 1.2
> > > > >   retrieving revision 1.3
> > > > >   diff -u -r1.2 -r1.3
> > > > >   --- Action.java       13 Jan 2005 16:12:26 -0000      1.2
> > > > >   +++ Action.java       22 Feb 2005 05:24:54 -0000      1.3
> > > > >   @@ -20,6 +20,8 @@
> > > > >    import org.apache.log4j.joran.spi.ExecutionContext;
> > > > >    import org.apache.log4j.joran.spi.Interpreter;
> > > > >    import org.apache.log4j.spi.ComponentBase;
> > > > >   +import org.apache.log4j.spi.LoggerRepository;
> > > > >   +import org.apache.log4j.spi.ErrorItem;
> > > > >
> > > > >    import org.xml.sax.Attributes;
> > > > >    import org.xml.sax.Locator;
> > > > >   @@ -79,4 +81,22 @@
> > > > >        }
> > > > >        return -1;
> > > > >      }
> > > > >   +
> > > > >   +  /**
> > > > >   +   * Helper method to return the LoggerRepository of the
> > execution
> > > > > context.
> > > > >   +   *
> > > > >   +   * @param ec The ExecutionContext that contains the reference
> > to
> > > > the
> > > > >   +   *   LoggerRepository
> > > > >   +   * @return The LoggerRepository
> > > > >   +   */
> > > > >   +  protected LoggerRepository getLoggerRepository(ExecutionContext
> > ec)
> > > > {
> > > > >   +    Object o = ec.getObject(0);
> > > > >   +    if(o instanceof LoggerRepository) {
> > > > >   +      return (LoggerRepository) o;
> > > > >   +    } else {
> > > > >   +      String errMsg = "There is no LoggerRepository at the top of
> > the
> > > > > object stack.";
> > > > >   +      ec.addError(new ErrorItem(errMsg));
> > > > >   +      throw new IllegalStateException(errMsg);
> > > > >   +    }
> > > > >   +  }
> > > > >    }
> > > > >
> > > > >
> > > > >
> > > > >   1.5       +0
> > > > > -11
> > > > > logging-
> > > >
> > log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java
> > > > >
> > > > >   Index:  .java
> > > > >
> > ===================================================================
> > > > >   RCS file:
> > > > > /home/cvs/logging-
> > > >
> > log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java
> > > > ,v
> > > > >   retrieving revision 1.4
> > > > >   retrieving revision 1.5
> > > > >   diff -u -r1.4 -r1.5
> > > > >   --- RepositoryPropertyAction.java     12 Jan 2005 15:04:18 -0000
> > > > 1.4
> > > > >   +++ RepositoryPropertyAction.java     22 Feb 2005 05:24:54 -0000
> > > > 1.5
> > > > >   @@ -28,17 +28,6 @@
> > > > >     * Window>Preferences>Java>Code Generation>Code and Comments
> > > > >     */
> > > > >    public class RepositoryPropertyAction extends PropertyAction {
> > > > >   -
> > > > >   -  LoggerRepository getLoggerRepository(ExecutionContext ec) {
> > > > >   -    Object o = ec.getObjectStack().get(0);
> > > > >   -    if(o instanceof LoggerRepository) {
> > > > >   -      return (LoggerRepository) o;
> > > > >   -    } else {
> > > > >   -      String errMsg = "There is no LoggerRepository at the top of
> > the
> > > > > object stack.";
> > > > >   -      ec.addError(new ErrorItem(errMsg));
> > > > >   -      throw new IllegalStateException(errMsg);
> > > > >   -    }
> > > > >   -  }
> > > > >
> > > > >      public void setProperties(ExecutionContext ec, Properties
> > props) {
> > > > >        LoggerRepository repository = getLoggerRepository(ec);
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >---------------------------------------------------------------------
> > > > >To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> > > > >For additional commands, e-mail: log4j-dev-help@logging.apache.org
> > > >
> > > > --
> > > > Ceki Gülcü
> > > >
> > > >    The complete log4j manual: http://www.qos.ch/log4j/
> > > >
> > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> > > > For additional commands, e-mail: log4j-dev-help@logging.apache.org
> > >
> > >
> > >---------------------------------------------------------------------
> > >To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> > >For additional commands, e-mail: log4j-dev-help@logging.apache.org
> >
> > --
> > Ceki Gülcü
> >
> >    The complete log4j manual: http://www.qos.ch/log4j/
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> > For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>For additional commands, e-mail: log4j-dev-help@logging.apache.org

-- 
Ceki Gülcü

   The complete log4j manual: http://www.qos.ch/log4j/



---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


RE: cvs commit: logging-log4j/src/java/org/apache/log4j/joran/action Action.java RepositoryPropertyAction.java

Posted by Mark Womack <wo...@adobe.com>.
Hey Ceki,

I'll have to look at the specific code you are referring to tonight, I don't
have the code in front of me.  But it sounds like I should replace all the
code that accesses the bottom of the stack with code to access whatever
SimpleRuleStore sets up.  And clean up what I have already added.  I'll look
at it tonight and let you know if I have any questions.

I'm still looking at the reset configuration issues.  I am trying to set up
a generic way to keep track of the "temp" appenders so that they can be
removed and reattached.  Some of the issues I was seeing may have been
related to the recent appender changes.  So, I will be looking at it again
tonight.

-Mark

> -----Original Message-----
> From: Ceki Gülcü [mailto:ceki@qos.ch]
> Sent: Thursday, February 24, 2005 10:53 AM
> To: Log4J Developers List; 'Log4J Developers List'
> Subject: RE: cvs commit: logging-
> log4j/src/java/org/apache/log4j/joran/action Action.java
> RepositoryPropertyAction.java
> 
> 
> Hi Mark,
> 
> What you have done makes perfect sense. However, the addRule method in
> SimpleRuleStore already sets the LoggerRepository for all Action instances
> that JoranConfigurator refers to. There is no point in adding a
> getLoggerRepository in Action if it is already in ComponentBase.
> 
> In other words, the LoggerRepository entry at the bottom of the stack is
> redundant.
> 
> Does that make sense?
> 
> At 10:32 PM 2/23/2005, Mark Womack wrote:
> >Ceki,
> >
> >This version of the getLoggerRepository method returns the bottom element
> of
> >the Joran execution stack (element 0) which is set up by the
> >JoranConfigurator at the start of configuration.  Many of the existing
> >actions refer to element 0 to grab the LoggerRepository being configured,
> >but I did not replace all the uses with this change.  I was waiting for
> >someone to comment before I did that. :-)
> >
> >Are you saying that Actions should not be using element 0 as the
> >LoggerRepository to act upon?  I just added/moved it as a common helper
> >method so that the code would be in one place and could be easily updated
> if
> >it needed to change in the future.  From a design point of view, it seems
> >better if Actions use an explicit method to get the "logger repository
> being
> >configured" than having know to grab it from element 0 of the current
> >execution context.  A different method name might be better.
> >
> >Let me know if this does not make sense.  I'll be happy to make whatever
> >changes are needed.  If we decide to keep the helper method, I'll update
> the
> >other Actions to use it instead of referencing element 0 of the ec.
> >
> >-Mark
> >
> > > -----Original Message-----
> > > From: Ceki Gülcü [mailto:ceki@qos.ch]
> > > Sent: Wednesday, February 23, 2005 10:41 AM
> > > To: Log4J Developers List; logging-log4j-cvs@apache.org
> > > Cc: mwomack@apache.org
> > > Subject: Re: cvs commit: logging-
> > > log4j/src/java/org/apache/log4j/joran/action Action.java
> > > RepositoryPropertyAction.java
> > >
> > >
> > > Mark,
> > >
> > > The getLoggerRepository(ExecutionContext ec) method should not be part
> of
> > > the Action class because ComponentBase already knows about its LR. The
> > > method getLoggerRepository should be removed from Action and should
> not
> > > have been part of RepositoryPropertyAction.
> > >
> > > At 06:24 AM 2/22/2005, mwomack@apache.org wrote:
> > > >mwomack     2005/02/21 21:24:54
> > > >
> > > >   Modified:    src/java/org/apache/log4j/joran/action Action.java
> > > >                         RepositoryPropertyAction.java
> > > >   Log:
> > > >   Moved getLoggerRepository to base Action class so it can be used
> by
> > > all
> > > > subclasses.  Centralizes logic for locating the LoggerRepository
> being
> > > > acted upon.
> > > >
> > > >   Revision  Changes    Path
> > > >   1.3       +20
> > > > -0     logging-
> log4j/src/java/org/apache/log4j/joran/action/Action.java
> > > >
> > > >   Index: Action.java
> > > >
> ===================================================================
> > > >   RCS file:
> > > > /home/cvs/logging-
> > > log4j/src/java/org/apache/log4j/joran/action/Action.java,v
> > > >   retrieving revision 1.2
> > > >   retrieving revision 1.3
> > > >   diff -u -r1.2 -r1.3
> > > >   --- Action.java       13 Jan 2005 16:12:26 -0000      1.2
> > > >   +++ Action.java       22 Feb 2005 05:24:54 -0000      1.3
> > > >   @@ -20,6 +20,8 @@
> > > >    import org.apache.log4j.joran.spi.ExecutionContext;
> > > >    import org.apache.log4j.joran.spi.Interpreter;
> > > >    import org.apache.log4j.spi.ComponentBase;
> > > >   +import org.apache.log4j.spi.LoggerRepository;
> > > >   +import org.apache.log4j.spi.ErrorItem;
> > > >
> > > >    import org.xml.sax.Attributes;
> > > >    import org.xml.sax.Locator;
> > > >   @@ -79,4 +81,22 @@
> > > >        }
> > > >        return -1;
> > > >      }
> > > >   +
> > > >   +  /**
> > > >   +   * Helper method to return the LoggerRepository of the
> execution
> > > > context.
> > > >   +   *
> > > >   +   * @param ec The ExecutionContext that contains the reference
> to
> > > the
> > > >   +   *   LoggerRepository
> > > >   +   * @return The LoggerRepository
> > > >   +   */
> > > >   +  protected LoggerRepository getLoggerRepository(ExecutionContext
> ec)
> > > {
> > > >   +    Object o = ec.getObject(0);
> > > >   +    if(o instanceof LoggerRepository) {
> > > >   +      return (LoggerRepository) o;
> > > >   +    } else {
> > > >   +      String errMsg = "There is no LoggerRepository at the top of
> the
> > > > object stack.";
> > > >   +      ec.addError(new ErrorItem(errMsg));
> > > >   +      throw new IllegalStateException(errMsg);
> > > >   +    }
> > > >   +  }
> > > >    }
> > > >
> > > >
> > > >
> > > >   1.5       +0
> > > > -11
> > > > logging-
> > >
> log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java
> > > >
> > > >   Index:  .java
> > > >
> ===================================================================
> > > >   RCS file:
> > > > /home/cvs/logging-
> > >
> log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java
> > > ,v
> > > >   retrieving revision 1.4
> > > >   retrieving revision 1.5
> > > >   diff -u -r1.4 -r1.5
> > > >   --- RepositoryPropertyAction.java     12 Jan 2005 15:04:18 -0000
> > > 1.4
> > > >   +++ RepositoryPropertyAction.java     22 Feb 2005 05:24:54 -0000
> > > 1.5
> > > >   @@ -28,17 +28,6 @@
> > > >     * Window>Preferences>Java>Code Generation>Code and Comments
> > > >     */
> > > >    public class RepositoryPropertyAction extends PropertyAction {
> > > >   -
> > > >   -  LoggerRepository getLoggerRepository(ExecutionContext ec) {
> > > >   -    Object o = ec.getObjectStack().get(0);
> > > >   -    if(o instanceof LoggerRepository) {
> > > >   -      return (LoggerRepository) o;
> > > >   -    } else {
> > > >   -      String errMsg = "There is no LoggerRepository at the top of
> the
> > > > object stack.";
> > > >   -      ec.addError(new ErrorItem(errMsg));
> > > >   -      throw new IllegalStateException(errMsg);
> > > >   -    }
> > > >   -  }
> > > >
> > > >      public void setProperties(ExecutionContext ec, Properties
> props) {
> > > >        LoggerRepository repository = getLoggerRepository(ec);
> > > >
> > > >
> > > >
> > > >
> > > >---------------------------------------------------------------------
> > > >To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> > > >For additional commands, e-mail: log4j-dev-help@logging.apache.org
> > >
> > > --
> > > Ceki Gülcü
> > >
> > >    The complete log4j manual: http://www.qos.ch/log4j/
> > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> > > For additional commands, e-mail: log4j-dev-help@logging.apache.org
> >
> >
> >---------------------------------------------------------------------
> >To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> >For additional commands, e-mail: log4j-dev-help@logging.apache.org
> 
> --
> Ceki Gülcü
> 
>    The complete log4j manual: http://www.qos.ch/log4j/
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


RE: cvs commit: logging-log4j/src/java/org/apache/log4j/joran/action Action.java RepositoryPropertyAction.java

Posted by Ceki Gülcü <ce...@qos.ch>.
Hi Mark,

What you have done makes perfect sense. However, the addRule method in 
SimpleRuleStore already sets the LoggerRepository for all Action instances 
that JoranConfigurator refers to. There is no point in adding a 
getLoggerRepository in Action if it is already in ComponentBase.

In other words, the LoggerRepository entry at the bottom of the stack is 
redundant.

Does that make sense?

At 10:32 PM 2/23/2005, Mark Womack wrote:
>Ceki,
>
>This version of the getLoggerRepository method returns the bottom element of
>the Joran execution stack (element 0) which is set up by the
>JoranConfigurator at the start of configuration.  Many of the existing
>actions refer to element 0 to grab the LoggerRepository being configured,
>but I did not replace all the uses with this change.  I was waiting for
>someone to comment before I did that. :-)
>
>Are you saying that Actions should not be using element 0 as the
>LoggerRepository to act upon?  I just added/moved it as a common helper
>method so that the code would be in one place and could be easily updated if
>it needed to change in the future.  From a design point of view, it seems
>better if Actions use an explicit method to get the "logger repository being
>configured" than having know to grab it from element 0 of the current
>execution context.  A different method name might be better.
>
>Let me know if this does not make sense.  I'll be happy to make whatever
>changes are needed.  If we decide to keep the helper method, I'll update the
>other Actions to use it instead of referencing element 0 of the ec.
>
>-Mark
>
> > -----Original Message-----
> > From: Ceki Gülcü [mailto:ceki@qos.ch]
> > Sent: Wednesday, February 23, 2005 10:41 AM
> > To: Log4J Developers List; logging-log4j-cvs@apache.org
> > Cc: mwomack@apache.org
> > Subject: Re: cvs commit: logging-
> > log4j/src/java/org/apache/log4j/joran/action Action.java
> > RepositoryPropertyAction.java
> >
> >
> > Mark,
> >
> > The getLoggerRepository(ExecutionContext ec) method should not be part of
> > the Action class because ComponentBase already knows about its LR. The
> > method getLoggerRepository should be removed from Action and should not
> > have been part of RepositoryPropertyAction.
> >
> > At 06:24 AM 2/22/2005, mwomack@apache.org wrote:
> > >mwomack     2005/02/21 21:24:54
> > >
> > >   Modified:    src/java/org/apache/log4j/joran/action Action.java
> > >                         RepositoryPropertyAction.java
> > >   Log:
> > >   Moved getLoggerRepository to base Action class so it can be used by
> > all
> > > subclasses.  Centralizes logic for locating the LoggerRepository being
> > > acted upon.
> > >
> > >   Revision  Changes    Path
> > >   1.3       +20
> > > -0     logging-log4j/src/java/org/apache/log4j/joran/action/Action.java
> > >
> > >   Index: Action.java
> > >   ===================================================================
> > >   RCS file:
> > > /home/cvs/logging-
> > log4j/src/java/org/apache/log4j/joran/action/Action.java,v
> > >   retrieving revision 1.2
> > >   retrieving revision 1.3
> > >   diff -u -r1.2 -r1.3
> > >   --- Action.java       13 Jan 2005 16:12:26 -0000      1.2
> > >   +++ Action.java       22 Feb 2005 05:24:54 -0000      1.3
> > >   @@ -20,6 +20,8 @@
> > >    import org.apache.log4j.joran.spi.ExecutionContext;
> > >    import org.apache.log4j.joran.spi.Interpreter;
> > >    import org.apache.log4j.spi.ComponentBase;
> > >   +import org.apache.log4j.spi.LoggerRepository;
> > >   +import org.apache.log4j.spi.ErrorItem;
> > >
> > >    import org.xml.sax.Attributes;
> > >    import org.xml.sax.Locator;
> > >   @@ -79,4 +81,22 @@
> > >        }
> > >        return -1;
> > >      }
> > >   +
> > >   +  /**
> > >   +   * Helper method to return the LoggerRepository of the  execution
> > > context.
> > >   +   *
> > >   +   * @param ec The ExecutionContext that contains the reference to
> > the
> > >   +   *   LoggerRepository
> > >   +   * @return The LoggerRepository
> > >   +   */
> > >   +  protected LoggerRepository getLoggerRepository(ExecutionContext ec)
> > {
> > >   +    Object o = ec.getObject(0);
> > >   +    if(o instanceof LoggerRepository) {
> > >   +      return (LoggerRepository) o;
> > >   +    } else {
> > >   +      String errMsg = "There is no LoggerRepository at the top of the
> > > object stack.";
> > >   +      ec.addError(new ErrorItem(errMsg));
> > >   +      throw new IllegalStateException(errMsg);
> > >   +    }
> > >   +  }
> > >    }
> > >
> > >
> > >
> > >   1.5       +0
> > > -11
> > > logging-
> > log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java
> > >
> > >   Index:  .java
> > >   ===================================================================
> > >   RCS file:
> > > /home/cvs/logging-
> > log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java
> > ,v
> > >   retrieving revision 1.4
> > >   retrieving revision 1.5
> > >   diff -u -r1.4 -r1.5
> > >   --- RepositoryPropertyAction.java     12 Jan 2005 15:04:18 -0000
> > 1.4
> > >   +++ RepositoryPropertyAction.java     22 Feb 2005 05:24:54 -0000
> > 1.5
> > >   @@ -28,17 +28,6 @@
> > >     * Window>Preferences>Java>Code Generation>Code and Comments
> > >     */
> > >    public class RepositoryPropertyAction extends PropertyAction {
> > >   -
> > >   -  LoggerRepository getLoggerRepository(ExecutionContext ec) {
> > >   -    Object o = ec.getObjectStack().get(0);
> > >   -    if(o instanceof LoggerRepository) {
> > >   -      return (LoggerRepository) o;
> > >   -    } else {
> > >   -      String errMsg = "There is no LoggerRepository at the top of the
> > > object stack.";
> > >   -      ec.addError(new ErrorItem(errMsg));
> > >   -      throw new IllegalStateException(errMsg);
> > >   -    }
> > >   -  }
> > >
> > >      public void setProperties(ExecutionContext ec, Properties props) {
> > >        LoggerRepository repository = getLoggerRepository(ec);
> > >
> > >
> > >
> > >
> > >---------------------------------------------------------------------
> > >To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> > >For additional commands, e-mail: log4j-dev-help@logging.apache.org
> >
> > --
> > Ceki Gülcü
> >
> >    The complete log4j manual: http://www.qos.ch/log4j/
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> > For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>For additional commands, e-mail: log4j-dev-help@logging.apache.org

-- 
Ceki Gülcü

   The complete log4j manual: http://www.qos.ch/log4j/



---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


RE: cvs commit: logging-log4j/src/java/org/apache/log4j/joran/action Action.java RepositoryPropertyAction.java

Posted by Mark Womack <wo...@adobe.com>.
Ceki,

This version of the getLoggerRepository method returns the bottom element of
the Joran execution stack (element 0) which is set up by the
JoranConfigurator at the start of configuration.  Many of the existing
actions refer to element 0 to grab the LoggerRepository being configured,
but I did not replace all the uses with this change.  I was waiting for
someone to comment before I did that. :-)

Are you saying that Actions should not be using element 0 as the
LoggerRepository to act upon?  I just added/moved it as a common helper
method so that the code would be in one place and could be easily updated if
it needed to change in the future.  From a design point of view, it seems
better if Actions use an explicit method to get the "logger repository being
configured" than having know to grab it from element 0 of the current
execution context.  A different method name might be better.

Let me know if this does not make sense.  I'll be happy to make whatever
changes are needed.  If we decide to keep the helper method, I'll update the
other Actions to use it instead of referencing element 0 of the ec.

-Mark

> -----Original Message-----
> From: Ceki Gülcü [mailto:ceki@qos.ch]
> Sent: Wednesday, February 23, 2005 10:41 AM
> To: Log4J Developers List; logging-log4j-cvs@apache.org
> Cc: mwomack@apache.org
> Subject: Re: cvs commit: logging-
> log4j/src/java/org/apache/log4j/joran/action Action.java
> RepositoryPropertyAction.java
> 
> 
> Mark,
> 
> The getLoggerRepository(ExecutionContext ec) method should not be part of
> the Action class because ComponentBase already knows about its LR. The
> method getLoggerRepository should be removed from Action and should not
> have been part of RepositoryPropertyAction.
> 
> At 06:24 AM 2/22/2005, mwomack@apache.org wrote:
> >mwomack     2005/02/21 21:24:54
> >
> >   Modified:    src/java/org/apache/log4j/joran/action Action.java
> >                         RepositoryPropertyAction.java
> >   Log:
> >   Moved getLoggerRepository to base Action class so it can be used by
> all
> > subclasses.  Centralizes logic for locating the LoggerRepository being
> > acted upon.
> >
> >   Revision  Changes    Path
> >   1.3       +20
> > -0     logging-log4j/src/java/org/apache/log4j/joran/action/Action.java
> >
> >   Index: Action.java
> >   ===================================================================
> >   RCS file:
> > /home/cvs/logging-
> log4j/src/java/org/apache/log4j/joran/action/Action.java,v
> >   retrieving revision 1.2
> >   retrieving revision 1.3
> >   diff -u -r1.2 -r1.3
> >   --- Action.java       13 Jan 2005 16:12:26 -0000      1.2
> >   +++ Action.java       22 Feb 2005 05:24:54 -0000      1.3
> >   @@ -20,6 +20,8 @@
> >    import org.apache.log4j.joran.spi.ExecutionContext;
> >    import org.apache.log4j.joran.spi.Interpreter;
> >    import org.apache.log4j.spi.ComponentBase;
> >   +import org.apache.log4j.spi.LoggerRepository;
> >   +import org.apache.log4j.spi.ErrorItem;
> >
> >    import org.xml.sax.Attributes;
> >    import org.xml.sax.Locator;
> >   @@ -79,4 +81,22 @@
> >        }
> >        return -1;
> >      }
> >   +
> >   +  /**
> >   +   * Helper method to return the LoggerRepository of the  execution
> > context.
> >   +   *
> >   +   * @param ec The ExecutionContext that contains the reference to
> the
> >   +   *   LoggerRepository
> >   +   * @return The LoggerRepository
> >   +   */
> >   +  protected LoggerRepository getLoggerRepository(ExecutionContext ec)
> {
> >   +    Object o = ec.getObject(0);
> >   +    if(o instanceof LoggerRepository) {
> >   +      return (LoggerRepository) o;
> >   +    } else {
> >   +      String errMsg = "There is no LoggerRepository at the top of the
> > object stack.";
> >   +      ec.addError(new ErrorItem(errMsg));
> >   +      throw new IllegalStateException(errMsg);
> >   +    }
> >   +  }
> >    }
> >
> >
> >
> >   1.5       +0
> > -11
> > logging-
> log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java
> >
> >   Index:  .java
> >   ===================================================================
> >   RCS file:
> > /home/cvs/logging-
> log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java
> ,v
> >   retrieving revision 1.4
> >   retrieving revision 1.5
> >   diff -u -r1.4 -r1.5
> >   --- RepositoryPropertyAction.java     12 Jan 2005 15:04:18 -0000
> 1.4
> >   +++ RepositoryPropertyAction.java     22 Feb 2005 05:24:54 -0000
> 1.5
> >   @@ -28,17 +28,6 @@
> >     * Window>Preferences>Java>Code Generation>Code and Comments
> >     */
> >    public class RepositoryPropertyAction extends PropertyAction {
> >   -
> >   -  LoggerRepository getLoggerRepository(ExecutionContext ec) {
> >   -    Object o = ec.getObjectStack().get(0);
> >   -    if(o instanceof LoggerRepository) {
> >   -      return (LoggerRepository) o;
> >   -    } else {
> >   -      String errMsg = "There is no LoggerRepository at the top of the
> > object stack.";
> >   -      ec.addError(new ErrorItem(errMsg));
> >   -      throw new IllegalStateException(errMsg);
> >   -    }
> >   -  }
> >
> >      public void setProperties(ExecutionContext ec, Properties props) {
> >        LoggerRepository repository = getLoggerRepository(ec);
> >
> >
> >
> >
> >---------------------------------------------------------------------
> >To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> >For additional commands, e-mail: log4j-dev-help@logging.apache.org
> 
> --
> Ceki Gülcü
> 
>    The complete log4j manual: http://www.qos.ch/log4j/
> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Re: cvs commit: logging-log4j/src/java/org/apache/log4j/joran/action Action.java RepositoryPropertyAction.java

Posted by Ceki Gülcü <ce...@qos.ch>.
Mark,

The getLoggerRepository(ExecutionContext ec) method should not be part of 
the Action class because ComponentBase already knows about its LR. The 
method getLoggerRepository should be removed from Action and should not 
have been part of RepositoryPropertyAction.

At 06:24 AM 2/22/2005, mwomack@apache.org wrote:
>mwomack     2005/02/21 21:24:54
>
>   Modified:    src/java/org/apache/log4j/joran/action Action.java
>                         RepositoryPropertyAction.java
>   Log:
>   Moved getLoggerRepository to base Action class so it can be used by all 
> subclasses.  Centralizes logic for locating the LoggerRepository being 
> acted upon.
>
>   Revision  Changes    Path
>   1.3       +20 
> -0     logging-log4j/src/java/org/apache/log4j/joran/action/Action.java
>
>   Index: Action.java
>   ===================================================================
>   RCS file: 
> /home/cvs/logging-log4j/src/java/org/apache/log4j/joran/action/Action.java,v
>   retrieving revision 1.2
>   retrieving revision 1.3
>   diff -u -r1.2 -r1.3
>   --- Action.java       13 Jan 2005 16:12:26 -0000      1.2
>   +++ Action.java       22 Feb 2005 05:24:54 -0000      1.3
>   @@ -20,6 +20,8 @@
>    import org.apache.log4j.joran.spi.ExecutionContext;
>    import org.apache.log4j.joran.spi.Interpreter;
>    import org.apache.log4j.spi.ComponentBase;
>   +import org.apache.log4j.spi.LoggerRepository;
>   +import org.apache.log4j.spi.ErrorItem;
>
>    import org.xml.sax.Attributes;
>    import org.xml.sax.Locator;
>   @@ -79,4 +81,22 @@
>        }
>        return -1;
>      }
>   +
>   +  /**
>   +   * Helper method to return the LoggerRepository of the  execution 
> context.
>   +   *
>   +   * @param ec The ExecutionContext that contains the reference to the
>   +   *   LoggerRepository
>   +   * @return The LoggerRepository
>   +   */
>   +  protected LoggerRepository getLoggerRepository(ExecutionContext ec) {
>   +    Object o = ec.getObject(0);
>   +    if(o instanceof LoggerRepository) {
>   +      return (LoggerRepository) o;
>   +    } else {
>   +      String errMsg = "There is no LoggerRepository at the top of the 
> object stack.";
>   +      ec.addError(new ErrorItem(errMsg));
>   +      throw new IllegalStateException(errMsg);
>   +    }
>   +  }
>    }
>
>
>
>   1.5       +0 
> -11 
> logging-log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java
>
>   Index:  .java
>   ===================================================================
>   RCS file: 
> /home/cvs/logging-log4j/src/java/org/apache/log4j/joran/action/RepositoryPropertyAction.java,v
>   retrieving revision 1.4
>   retrieving revision 1.5
>   diff -u -r1.4 -r1.5
>   --- RepositoryPropertyAction.java     12 Jan 2005 15:04:18 -0000      1.4
>   +++ RepositoryPropertyAction.java     22 Feb 2005 05:24:54 -0000      1.5
>   @@ -28,17 +28,6 @@
>     * Window>Preferences>Java>Code Generation>Code and Comments
>     */
>    public class RepositoryPropertyAction extends PropertyAction {
>   -
>   -  LoggerRepository getLoggerRepository(ExecutionContext ec) {
>   -    Object o = ec.getObjectStack().get(0);
>   -    if(o instanceof LoggerRepository) {
>   -      return (LoggerRepository) o;
>   -    } else {
>   -      String errMsg = "There is no LoggerRepository at the top of the 
> object stack.";
>   -      ec.addError(new ErrorItem(errMsg));
>   -      throw new IllegalStateException(errMsg);
>   -    }
>   -  }
>
>      public void setProperties(ExecutionContext ec, Properties props) {
>        LoggerRepository repository = getLoggerRepository(ec);
>
>
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>For additional commands, e-mail: log4j-dev-help@logging.apache.org

-- 
Ceki Gülcü

   The complete log4j manual: http://www.qos.ch/log4j/



---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org