maven-doxia-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dennis Lundberg <denn...@apache.org>
Subject Re: svn commit: r799837 - in /maven/doxia/doxia-tools/trunk/doxia-linkcheck/src/main/java/org/apache/maven/doxia/linkcheck/validation: LinkValidatorManager.java OnlineHTTPLinkValidator.java
Date Sun, 02 Aug 2009 07:18:20 GMT
Vincent,

Is there a special reason for removing the level.checking for logging
statements, like the one below? You removed a bunch of these.

if ( LOG.isDebugEnabled() )
{
    LOG.debug( "Will use method : [" + bean.getMethod() + "]" );
}

These are logically not necessary, but are put in there for performance
reasons. Only if the logger is in debug mode (in this case) will the 3
String objects it uses actually be created by the JVM. Otherwise no
objects will be created, resulting in slightly enhanced
performance/memory usage. At least if used in heavy loops.



vsiveton@apache.org wrote:
> Author: vsiveton
> Date: Sat Aug  1 11:17:23 2009
> New Revision: 799837
> 
> URL: http://svn.apache.org/viewvc?rev=799837&view=rev
> Log:
> o added timeout and httpClientsParameters
> o improved log call
> 
> Modified:
>     maven/doxia/doxia-tools/trunk/doxia-linkcheck/src/main/java/org/apache/maven/doxia/linkcheck/validation/LinkValidatorManager.java
>     maven/doxia/doxia-tools/trunk/doxia-linkcheck/src/main/java/org/apache/maven/doxia/linkcheck/validation/OnlineHTTPLinkValidator.java
> 
> Modified: maven/doxia/doxia-tools/trunk/doxia-linkcheck/src/main/java/org/apache/maven/doxia/linkcheck/validation/LinkValidatorManager.java
> URL: http://svn.apache.org/viewvc/maven/doxia/doxia-tools/trunk/doxia-linkcheck/src/main/java/org/apache/maven/doxia/linkcheck/validation/LinkValidatorManager.java?rev=799837&r1=799836&r2=799837&view=diff
> ==============================================================================
> --- maven/doxia/doxia-tools/trunk/doxia-linkcheck/src/main/java/org/apache/maven/doxia/linkcheck/validation/LinkValidatorManager.java
(original)
> +++ maven/doxia/doxia-tools/trunk/doxia-linkcheck/src/main/java/org/apache/maven/doxia/linkcheck/validation/LinkValidatorManager.java
Sat Aug  1 11:17:23 2009
> @@ -34,6 +34,7 @@
>  import java.io.ObjectInputStream;
>  import java.io.ObjectOutputStream;
>  import java.io.Serializable;
> +import java.net.URI;
>  import java.util.HashMap;
>  import java.util.Iterator;
>  import java.util.LinkedList;
> @@ -404,6 +405,17 @@
>              return link.indexOf( pattern ) != -1;
>          }
>  
> +        URI uri = URI.create( link );
> +
> +        if ( uri.getScheme() != null && !pattern.startsWith( uri.getScheme()
) )
> +        {
> +            return true;
> +        }
> +
> +        if ( pattern.matches( "\\*+/?.*" ) && !link.startsWith( "/" ) &&
!link.startsWith( "./" ) )
> +        {
> +            link = "./" + link;
> +        }
>          String diff = StringUtils.difference( link, pattern );
>          if ( diff.startsWith( "/" ) )
>          {
> 
> Modified: maven/doxia/doxia-tools/trunk/doxia-linkcheck/src/main/java/org/apache/maven/doxia/linkcheck/validation/OnlineHTTPLinkValidator.java
> URL: http://svn.apache.org/viewvc/maven/doxia/doxia-tools/trunk/doxia-linkcheck/src/main/java/org/apache/maven/doxia/linkcheck/validation/OnlineHTTPLinkValidator.java?rev=799837&r1=799836&r2=799837&view=diff
> ==============================================================================
> --- maven/doxia/doxia-tools/trunk/doxia-linkcheck/src/main/java/org/apache/maven/doxia/linkcheck/validation/OnlineHTTPLinkValidator.java
(original)
> +++ maven/doxia/doxia-tools/trunk/doxia-linkcheck/src/main/java/org/apache/maven/doxia/linkcheck/validation/OnlineHTTPLinkValidator.java
Sat Aug  1 11:17:23 2009
> @@ -22,6 +22,8 @@
>  import java.io.IOException;
>  
>  import java.net.URL;
> +import java.util.Iterator;
> +import java.util.Map;
>  
>  import org.apache.commons.httpclient.Credentials;
>  import org.apache.commons.httpclient.Header;
> @@ -38,6 +40,8 @@
>  import org.apache.commons.httpclient.auth.AuthScope;
>  import org.apache.commons.httpclient.methods.GetMethod;
>  import org.apache.commons.httpclient.methods.HeadMethod;
> +import org.apache.commons.httpclient.params.HttpClientParams;
> +import org.apache.commons.httpclient.params.HttpMethodParams;
>  
>  import org.apache.commons.logging.Log;
>  import org.apache.commons.logging.LogFactory;
> @@ -45,7 +49,6 @@
>  import org.apache.maven.doxia.linkcheck.model.LinkcheckFileResult;
>  import org.codehaus.plexus.util.StringUtils;
>  
> -
>  /**
>   * Checks links which are normal URLs
>   *
> @@ -54,7 +57,8 @@
>   * @author <a href="mailto:vincent.siveton@gmail.com">Vincent Siveton</a>
>   * @version $Id$
>   */
> -public final class OnlineHTTPLinkValidator extends HTTPLinkValidator
> +public final class OnlineHTTPLinkValidator
> +    extends HTTPLinkValidator
>  {
>      /** Log for debug output. */
>      private static final Log LOG = LogFactory.getLog( OnlineHTTPLinkValidator.class
);
> @@ -97,10 +101,7 @@
>              bean = new HttpBean();
>          }
>  
> -        if ( LOG.isDebugEnabled() )
> -        {
> -            LOG.debug( "Will use method : [" + bean.getMethod() + "]" );
> -        }
> +        LOG.debug( "Will use method : [" + bean.getMethod() + "]" );
>  
>          this.http = bean;
>  
> @@ -135,42 +136,50 @@
>              initHttpClient();
>          }
>  
> -        String link = lvi.getLink();
> +        if ( this.http.getHttpClientParameters() != null )
> +        {
> +            for ( Iterator it = this.http.getHttpClientParameters().entrySet().iterator();
it.hasNext(); )
> +            {
> +                Map.Entry entry = (Map.Entry) it.next();
> +
> +                if ( entry.getValue() != null )
> +                {
> +                    System.setProperty( entry.getKey().toString(), entry.getValue().toString()
);
> +                }
> +            }
> +        }
>  
> +        // Some web servers don't allow the default user-agent sent by httpClient
> +        System.setProperty( HttpMethodParams.USER_AGENT, "Mozilla/4.0 (compatible; MSIE
6.0; Windows NT 5.0)" );
> +
> +        String link = lvi.getLink();
>          try
>          {
>              if ( link.startsWith( "/" ) )
>              {
>                  if ( getBaseURL() == null )
>                  {
> -                    if ( LOG.isWarnEnabled() )
> -                    {
> +                    LOG.warn( "Cannot check link [" + link + "] in page [" + lvi.getSource()
> +                        + "], as no base URL has been set!" );
>  
> -                        LOG.warn( "Cannot check link [" + link + "] in page [" + lvi.getSource()
> -                            + "], as no base URL has been set!" );
> -                    }
>                      return new LinkValidationResult( LinkcheckFileResult.WARNING_LEVEL,
false,
> -                            "No base URL specified" );
> +                                                     "No base URL specified" );
>                  }
>  
>                  link = getBaseURL() + link;
>              }
>  
>              HttpMethod hm = null;
> -
>              try
>              {
>                  hm = checkLink( link, 0 );
>              }
>              catch ( Throwable t )
>              {
> -                if ( LOG.isDebugEnabled() )
> -                {
> -                    LOG.debug( "Received: [" + t + "] for [" + link + "] in page ["
+ lvi.getSource() + "]", t );
> -                }
> +                LOG.debug( "Received: [" + t + "] for [" + link + "] in page [" + lvi.getSource()
+ "]", t );
>  
> -                return new LinkValidationResult( LinkcheckFileResult.ERROR_LEVEL, false,
t.getClass().getName() + " : "
> -                                + t.getMessage() );
> +                return new LinkValidationResult( LinkcheckFileResult.ERROR_LEVEL, false,
t.getClass().getName()
> +                    + " : " + t.getMessage() );
>              }
>  
>              if ( hm == null )
> @@ -182,116 +191,114 @@
>              if ( hm.getStatusCode() == HttpStatus.SC_OK )
>              {
>                  return new HTTPLinkValidationResult( LinkcheckFileResult.VALID_LEVEL,
true, hm.getStatusCode(),
> -                                                 hm.getStatusText() );
> -        }
> +                                                     hm.getStatusText() );
> +            }
>  
> -        // If there's a redirection ... add a warning
> -        if ( hm.getStatusCode() == HttpStatus.SC_MOVED_PERMANENTLY
> -                        || hm.getStatusCode() == HttpStatus.SC_MOVED_TEMPORARILY
> -                        || hm.getStatusCode() == HttpStatus.SC_TEMPORARY_REDIRECT )
> -        {
> -            if ( LOG.isWarnEnabled() )
> +            String msg =
> +                "Received: [" + hm.getStatusCode() + "] for [" + link + "] in page ["
+ lvi.getSource() + "]";
> +            // If there's a redirection ... add a warning
> +            if ( hm.getStatusCode() == HttpStatus.SC_MOVED_PERMANENTLY
> +                || hm.getStatusCode() == HttpStatus.SC_MOVED_TEMPORARILY
> +                || hm.getStatusCode() == HttpStatus.SC_TEMPORARY_REDIRECT )
>              {
> -                LOG.warn( "Received: [" + hm.getStatusCode() + "] for [" + link + "]
in page [" + lvi.getSource()
> -                        + "]" );
> -                }
> +                LOG.warn( msg );
>  
>                  return new HTTPLinkValidationResult( LinkcheckFileResult.WARNING_LEVEL,
true, hm.getStatusCode(),
>                                                       hm.getStatusText() );
>              }
>  
> -            if ( LOG.isDebugEnabled() )
> -            {
> -                LOG.debug( "Received: [" + hm.getStatusCode() + "] for [" + link + "]
in page [" + lvi.getSource()
> -                    + "]" );
> -            }
> +            LOG.debug( msg );
>  
>              return new HTTPLinkValidationResult( LinkcheckFileResult.ERROR_LEVEL, false,
hm.getStatusCode(),
>                                                   hm.getStatusText() );
> -
>          }
>          catch ( Throwable t )
>          {
> +            String msg = "Received: [" + t + "] for [" + link + "] in page [" + lvi.getSource()
+ "]";
>              if ( LOG.isDebugEnabled() )
>              {
> -                LOG.debug( "Received: [" + t + "] for [" + link + "] in page [" + lvi.getSource()
+ "]", t );
> +                LOG.debug( msg, t );
>              }
>              else
>              {
> -                LOG.error( "Received: [" + t + "] for [" + link + "] in page [" + lvi.getSource()
+ "]" );
> +                LOG.error( msg );
>              }
>  
>              return new LinkValidationResult( LinkcheckFileResult.ERROR_LEVEL, false,
t.getMessage() );
>          }
> +        finally
> +        {
> +            System.getProperties().remove( HttpMethodParams.USER_AGENT );
> +
> +            if ( this.http.getHttpClientParameters() != null )
> +            {
> +                for ( Iterator it = this.http.getHttpClientParameters().entrySet().iterator();
it.hasNext(); )
> +                {
> +                    Map.Entry entry = (Map.Entry) it.next();
> +
> +                    if ( entry.getValue() != null )
> +                    {
> +                        System.getProperties().remove( entry.getKey().toString() );
> +                    }
> +                }
> +            }
> +        }
>      }
>  
>      /** Initialize the HttpClient. */
>      private void initHttpClient()
>      {
> -        if ( LOG.isDebugEnabled() )
> -        {
> -            LOG.debug( "A new HttpClient instance is needed ..." );
> -        }
> -
> -        // Some web servers don't allow the default user-agent sent by httpClient
> -        System.setProperty( "httpclient.useragent", "Mozilla/4.0 (compatible; MSIE 6.0;
Windows NT 5.0)" );
> +        LOG.debug( "A new HttpClient instance is needed ..." );
>  
>          this.cl = new HttpClient( new MultiThreadedHttpConnectionManager() );
>  
> +        // Default params
> +        if ( this.http.getTimeout() != 0 )
> +        {
> +            this.cl.getHttpConnectionManager().getParams().setConnectionTimeout( this.http.getTimeout()
);
> +            this.cl.getHttpConnectionManager().getParams().setSoTimeout( this.http.getTimeout()
);
> +        }
> +        this.cl.getParams().setBooleanParameter( HttpClientParams.ALLOW_CIRCULAR_REDIRECTS,
true );
> +
>          HostConfiguration hc = new HostConfiguration();
>  
>          HttpState state = new HttpState();
> -
>          if ( StringUtils.isNotEmpty( this.http.getProxyHost() ) )
>          {
>              hc.setProxy( this.http.getProxyHost(), this.http.getProxyPort() );
>  
> -            if ( LOG.isDebugEnabled() )
> -            {
> -                LOG.debug( "Proxy Host:" + this.http.getProxyHost() );
> -                LOG.debug( "Proxy Port:" + this.http.getProxyPort() );
> -            }
> +            LOG.debug( "Proxy Host:" + this.http.getProxyHost() );
> +            LOG.debug( "Proxy Port:" + this.http.getProxyPort() );
>  
>              if ( StringUtils.isNotEmpty( this.http.getProxyUser() ) && this.http.getProxyPassword()
!= null )
>              {
> -                if ( LOG.isDebugEnabled() )
> -                {
> -                    LOG.debug( "Proxy User:" + this.http.getProxyUser() );
> -                }
> +                LOG.debug( "Proxy User:" + this.http.getProxyUser() );
>  
>                  Credentials credentials;
> -
>                  if ( StringUtils.isNotEmpty( this.http.getProxyNtlmHost() ) )
>                  {
> -                    credentials = new NTCredentials( this.http.getProxyUser(), this.http.getProxyPassword(),
this.http
> -                        .getProxyNtlmHost(), this.http.getProxyNtlmDomain() );
> +                    credentials =
> +                        new NTCredentials( this.http.getProxyUser(), this.http.getProxyPassword(),
> +                                           this.http.getProxyNtlmHost(), this.http.getProxyNtlmDomain()
);
>                  }
>                  else
>                  {
> -                    credentials = new UsernamePasswordCredentials( this.http.getProxyUser(),
this.http
> -                        .getProxyPassword() );
> +                    credentials =
> +                        new UsernamePasswordCredentials( this.http.getProxyUser(), this.http.getProxyPassword()
);
>                  }
>  
>                  state.setProxyCredentials( AuthScope.ANY, credentials );
>              }
> -
>          }
>          else
>          {
> -            if ( LOG.isDebugEnabled() )
> -            {
> -                LOG.debug( "Not using a proxy" );
> -            }
> +            LOG.debug( "Not using a proxy" );
>          }
>  
>          this.cl.setHostConfiguration( hc );
> -
>          this.cl.setState( state );
>  
> -        if ( LOG.isDebugEnabled() )
> -        {
> -            LOG.debug( "New HttpClient instance created." );
> -        }
> +        LOG.debug( "New HttpClient instance created." );
>      }
>  
>      /**
> @@ -305,13 +312,29 @@
>      private HttpMethod checkLink( String link, int nbRedirect )
>          throws IOException
>      {
> -        if ( nbRedirect > MAX_NB_REDIRECT )
> +        int max = MAX_NB_REDIRECT;
> +        if ( this.http.getHttpClientParameters() != null
> +            && this.http.getHttpClientParameters().get( HttpClientParams.MAX_REDIRECTS
) != null )
>          {
> -            throw new HttpException( "Maximum number of redirections (" + MAX_NB_REDIRECT
+ ") exceeded" );
> +            try
> +            {
> +                max =
> +                    Integer.valueOf(
> +                                     this.http.getHttpClientParameters().get( HttpClientParams.MAX_REDIRECTS
)
> +                                              .toString() ).intValue();
> +            }
> +            catch ( NumberFormatException e )
> +            {
> +                LOG.warn( "HttpClient parameter '" + HttpClientParams.MAX_REDIRECTS
> +                    + "' is not a number. Ignoring" );
> +            }
> +        }
> +        if ( nbRedirect > max )
> +        {
> +            throw new HttpException( "Maximum number of redirections (" + max + ") exceeded"
);
>          }
>  
>          HttpMethod hm;
> -
>          if ( HEAD_METHOD.equalsIgnoreCase( this.http.getMethod() ) )
>          {
>              hm = new HeadMethod( link );
> @@ -322,17 +345,15 @@
>          }
>          else
>          {
> -            if ( LOG.isErrorEnabled() )
> -            {
> -                LOG.error( "Unsupported method: " + this.http.getMethod() + ", using
'get'." );
> -            }
> +            LOG.error( "Unsupported method: " + this.http.getMethod() + ", using 'get'."
);
>              hm = new GetMethod( link );
>          }
>  
> +        // Default
> +        hm.setFollowRedirects( this.http.isFollowRedirects() );
> +
>          try
>          {
> -            hm.setFollowRedirects( this.http.isFollowRedirects() );
> -
>              URL url = new URL( link );
>  
>              cl.getHostConfiguration().setHost( url.getHost(), url.getPort(), url.getProtocol()
);
> @@ -340,28 +361,23 @@
>              cl.executeMethod( hm );
>  
>              StatusLine sl = hm.getStatusLine();
> -
>              if ( sl == null )
>              {
> -                if ( LOG.isErrorEnabled() )
> -                {
> -                    LOG.error( "Unknown error validating link : " + link );
> -                }
> +                LOG.error( "Unknown error validating link : " + link );
> +
>                  return null;
>              }
>  
>              if ( hm.getStatusCode() == HttpStatus.SC_MOVED_PERMANENTLY
> -                            || hm.getStatusCode() == HttpStatus.SC_MOVED_TEMPORARILY
> -                            || hm.getStatusCode() == HttpStatus.SC_TEMPORARY_REDIRECT
)
> +                || hm.getStatusCode() == HttpStatus.SC_MOVED_TEMPORARILY
> +                || hm.getStatusCode() == HttpStatus.SC_TEMPORARY_REDIRECT )
>              {
>                  Header locationHeader = hm.getResponseHeader( "location" );
>  
>                  if ( locationHeader == null )
>                  {
> -                    if ( LOG.isErrorEnabled() )
> -                    {
> -                        LOG.error( "Site sent redirect, but did not set Location header"
);
> -                    }
> +                    LOG.error( "Site sent redirect, but did not set Location header"
);
> +
>                      return hm;
>                  }
>  
> @@ -376,7 +392,7 @@
>  
>                          newLink =
>                              oldUrl.getProtocol() + "://" + oldUrl.getHost()
> -                                            + ( oldUrl.getPort() > 0 ? ":" + oldUrl.getPort()
: "" ) + newLink;
> +                                + ( oldUrl.getPort() > 0 ? ":" + oldUrl.getPort()
: "" ) + newLink;
>                      }
>                      else
>                      {
> @@ -386,10 +402,7 @@
>  
>                  HttpMethod oldHm = hm;
>  
> -                if ( LOG.isDebugEnabled() )
> -                {
> -                    LOG.debug( "[" + link + "] is redirected to [" + newLink + "]" );
> -                }
> +                LOG.debug( "[" + link + "] is redirected to [" + newLink + "]" );
>  
>                  oldHm.releaseConnection();
>  
> 
> 
> 


-- 
Dennis Lundberg

Mime
View raw message