You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by Vadim Gritsenko <va...@verizon.net> on 2003/01/16 17:11:35 UTC
Re: cvs commit: xml-cocoon2/src/java/org/apache/cocoon/environment
AbstractEnvironment.java
cziegeler@apache.org wrote:
>cziegeler 2003/01/16 06:33:32
>
> Modified: src/java/org/apache/cocoon/environment/http
> HttpEnvironment.java
> src/java/org/apache/cocoon/environment
> AbstractEnvironment.java
> Log:
> Only debug when level is set
>
> Revision Changes Path
> 1.21 +27 -16 xml-cocoon2/src/java/org/apache/cocoon/environment/http/HttpEnvironment.java
>
> Index: HttpEnvironment.java
> ===================================================================
> RCS file: /home/cvs/xml-cocoon2/src/java/org/apache/cocoon/environment/http/HttpEnvironment.java,v
> retrieving revision 1.20
> retrieving revision 1.21
> diff -u -r1.20 -r1.21
> --- HttpEnvironment.java 18 Dec 2002 08:09:24 -0000 1.20
> +++ HttpEnvironment.java 16 Jan 2003 14:33:31 -0000 1.21
> @@ -154,38 +154,43 @@
> * Redirect the client to new URL with session mode
> */
> public void redirect(boolean sessionmode, String newURL) throws IOException {
> - if (request == null) {
> - getLogger().debug("redirect: something's broken, request = null");
> - return;
> - }
> -
> this.hasRedirected = true;
>
> // check if session mode shall be activated
> if (sessionmode) {
> // The session
> Session session = null;
> - getLogger().debug("redirect: entering session mode");
> + if (getLogger().isDebugEnabled()) {
> + getLogger().debug("redirect: entering session mode");
> + }
>
Hi Carsten,
I think you overdoing this a bit: if(){debug()} here is twice less
efficient as single debug() call, because debug()'s argument is the
constant from the class' constants pool, not a dynamic string.
IIRC, we already agreed some time ago that simple debug output should
not be wrapped into if().
Vadim
> String s = request.getRequestedSessionId();
> if (s != null) {
> - getLogger().debug("Old session ID found in request, id = " + s);
> - if ( request.isRequestedSessionIdValid() ) {
> - getLogger().debug("And this old session ID is valid");
> + if (getLogger().isDebugEnabled()) {
> + getLogger().debug("Old session ID found in request, id = " + s);
> + if ( request.isRequestedSessionIdValid() ) {
> + getLogger().debug("And this old session ID is valid");
> + }
> }
> }
> // get session from request, or create new session
> session = request.getSession(true);
> if (session == null) {
> - getLogger().debug("redirect session mode: unable to get session object!");
> + if (getLogger().isDebugEnabled()) {
> + getLogger().debug("redirect session mode: unable to get session object!");
> + }
> + }
> + if (getLogger().isDebugEnabled()) {
> + getLogger().debug ("redirect: session mode completed, id = " + session.getId() );
> }
> - getLogger().debug ("redirect: session mode completed, id = " + session.getId() );
> }
> // redirect
> String redirect = this.response.encodeRedirectURL(newURL);
>
> // FIXME (VG): WebSphere 4.0/4.0.1 bug
> if (!newURL.startsWith("/") && newURL.indexOf(':') == -1 && redirect.indexOf(':') != -1) {
> - getLogger().debug("Redirect: WebSpehere Bug Detected!");
> + if (getLogger().isDebugEnabled()) {
> + getLogger().debug("Redirect: WebSpehere Bug Detected!");
> + }
> String base = NetUtils.getPath(request.getRequestURI());
> if (base.startsWith("/")) {
> base = base.substring(1);
> @@ -193,7 +198,9 @@
> redirect = response.encodeRedirectURL(base + '/' + newURL);
> }
>
> - getLogger().debug("Sending redirect to '" + redirect + "'");
> + if (getLogger().isDebugEnabled()) {
> + getLogger().debug("Sending redirect to '" + redirect + "'");
> + }
> this.response.sendRedirect (redirect);
> }
>
> @@ -265,14 +272,18 @@
> try {
> if (!this.response.isCommitted()) {
> this.response.reset();
> - getLogger().debug("Response successfully reset");
> + if (getLogger().isDebugEnabled()) {
> + getLogger().debug("Response successfully reset");
> + }
> return true;
> }
> } catch (Exception e) {
> // Log the error, but don't transmit it
> getLogger().warn("Problem resetting response", e);
> }
> - getLogger().debug("Response wasn't reset");
> + if (getLogger().isDebugEnabled()) {
> + getLogger().debug("Response wasn't reset");
> + }
> return false;
> }
> return true;
>
>
>
> 1.33 +4 -2 xml-cocoon2/src/java/org/apache/cocoon/environment/AbstractEnvironment.java
>
> Index: AbstractEnvironment.java
> ===================================================================
> RCS file: /home/cvs/xml-cocoon2/src/java/org/apache/cocoon/environment/AbstractEnvironment.java,v
> retrieving revision 1.32
> retrieving revision 1.33
> diff -u -r1.32 -r1.33
> --- AbstractEnvironment.java 14 Jan 2003 09:54:19 -0000 1.32
> +++ AbstractEnvironment.java 16 Jan 2003 14:33:32 -0000 1.33
> @@ -308,7 +308,9 @@
> if (this.context.getProtocol().equals("zip")) {
> // if the resource is zipped into a war file (e.g. Weblogic temp deployment)
> // FIXME (VG): Is this still required? Better to unify both cases.
> - getLogger().debug("Base context is zip: " + this.context);
> + if (getLogger().isDebugEnabled()) {
> + getLogger().debug("Base context is zip: " + this.context);
> + }
> this.context = new URL(this.context.toString() + newContext);
> } else {
> String sContext;
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org
Re: cvs commit: xml-cocoon2/src/java/org/apache/cocoon/environment
AbstractEnvironment.java
Posted by Vadim Gritsenko <va...@verizon.net>.
Carsten Ziegeler wrote:
>Vadim Gritsenko wrote:
>
>
>
>>> - getLogger().debug("redirect: entering session mode");
>>> + if (getLogger().isDebugEnabled()) {
>>> + getLogger().debug("redirect: entering session mode");
>>> + }
>>>
>>>
>>>
>>Hi Carsten,
>>
>>I think you overdoing this a bit: if(){debug()} here is twice less
>>efficient as single debug() call, because debug()'s argument is the
>>constant from the class' constants pool, not a dynamic string.
>>
>>
>
>I don't want to discuss if isDebugEnabled() is not as efficient as
>debug() - but I really guess it is.
>The reason for the change above is simple, it's a) consistent and
>b) if you change the log message and add some dynamic information the
>extra check is there and cannot be forgotten.
>
>
Ok, fair enough.
Vadim
>This is really a problem, because we agreed a long time ago do to the
>extra checks. Take a look at the code, there are still places where
>the extra checks are not done as not all of us really care about this.
>
>
>
>>IIRC, we already agreed some time ago that simple debug output should
>>not be wrapped into if().
>>
>>
>>
>Hmm, I cannot remember anything like this, sorry. (weak mind perhaps?)
>
>Carsten
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org
RE: cvs commit: xml-cocoon2/src/java/org/apache/cocoon/environment AbstractEnvironment.java
Posted by Carsten Ziegeler <cz...@s-und-n.de>.
Vadim Gritsenko wrote:
> > - getLogger().debug("redirect: entering session mode");
> > + if (getLogger().isDebugEnabled()) {
> > + getLogger().debug("redirect: entering session mode");
> > + }
> >
>
> Hi Carsten,
>
> I think you overdoing this a bit: if(){debug()} here is twice less
> efficient as single debug() call, because debug()'s argument is the
> constant from the class' constants pool, not a dynamic string.
I don't want to discuss if isDebugEnabled() is not as efficient as
debug() - but I really guess it is.
The reason for the change above is simple, it's a) consistent and
b) if you change the log message and add some dynamic information the
extra check is there and cannot be forgotten.
This is really a problem, because we agreed a long time ago do to the
extra checks. Take a look at the code, there are still places where
the extra checks are not done as not all of us really care about this.
>
> IIRC, we already agreed some time ago that simple debug output should
> not be wrapped into if().
>
Hmm, I cannot remember anything like this, sorry. (weak mind perhaps?)
Carsten
---------------------------------------------------------------------
To unsubscribe, e-mail: cocoon-dev-unsubscribe@xml.apache.org
For additional commands, email: cocoon-dev-help@xml.apache.org