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.
vsive...@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.sive...@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
