Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

switch httpclient tests to use undertow to try working around NPE in Vert.x #37

Merged
merged 4 commits into from May 19, 2015
Merged

switch httpclient tests to use undertow to try working around NPE in Vert.x #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
01863a5
switch to using undertow-based http server fixture to try to work aro…
jdcasey May 19, 2015
4edb251
Merge branch 'master' of github.com:Commonjava/galley
jdcasey May 19, 2015
428cafc
Make deployment conditional on whether it's a PR build.
jdcasey May 19, 2015
cf28436
Merge branch 'master' of http://github.com/commonjava/commonjava-trav…
jdcasey May 19, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .travis.yml
View file
Open in desktop
Expand Up @@ -9,4 +9,5 @@ cache:
- '$HOME/.m2/repository'

install: /bin/true
script: mvn deploy -s ./travis-settings.xml -V -Prun-its -q -B -e
script:
- '[ "${TRAVIS_PULL_REQUEST}" = "false" ] && mvn -s ./travis-settings.xml -Prun-its -q -B -e install || mvn deploy -s ./travis-settings.xml -V -Prun-its -q -B -e'
20 changes: 20 additions & 0 deletions pom.xml
View file
Open in desktop
Expand Up @@ -45,6 +45,7 @@

<properties>
<projectOwner>Red Hat, Inc.</projectOwner>
<undertowVersion>1.1.2.Final</undertowVersion>
<atlasVersion>0.14.0</atlasVersion>
<partylineVersion>1.4-SNAPSHOT</partylineVersion>
</properties>
Expand Down Expand Up @@ -178,6 +179,25 @@
<artifactId>cal10n-api</artifactId>
<version>0.7.4</version>
</dependency>

<dependency>
<groupId>io.undertow</groupId>
<artifactId>undertow-core</artifactId>
<version>${undertowVersion}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.undertow</groupId>
<artifactId>undertow-servlet</artifactId>
<version>${undertowVersion}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jboss.spec.javax.servlet</groupId>
<artifactId>jboss-servlet-api_3.0_spec</artifactId>
<version>1.0.1.Final</version>
<scope>test</scope>
</dependency>
</dependencies>
</dependencyManagement>

Expand Down
18 changes: 15 additions & 3 deletions transports/httpclient/pom.xml
View file
Open in desktop
Expand Up @@ -37,12 +37,24 @@
<artifactId>jsoup</artifactId>
</dependency>
<dependency>
<groupId>io.vertx</groupId>
<artifactId>vertx-core</artifactId>
<groupId>org.slf4j</groupId>
<artifactId>jcl-over-slf4j</artifactId>
</dependency>
<dependency>
<groupId>io.undertow</groupId>
<artifactId>undertow-core</artifactId>
</dependency>
<dependency>
<groupId>io.undertow</groupId>
<artifactId>undertow-servlet</artifactId>
</dependency>
<dependency>
<groupId>org.jboss.spec.javax.servlet</groupId>
<artifactId>jboss-servlet-api_3.0_spec</artifactId>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>jcl-over-slf4j</artifactId>
<artifactId>log4j-over-slf4j</artifactId>
</dependency>
</dependencies>

Expand Down
12 changes: 2 additions & 10 deletions ...ient/src/main/java/org/commonjava/maven/galley/transport/htcli/internal/HttpDownload.java
View file
Open in desktop
Expand Up @@ -35,6 +35,7 @@
import org.commonjava.maven.galley.model.TransferOperation;
import org.commonjava.maven.galley.spi.transport.DownloadJob;
import org.commonjava.maven.galley.transport.htcli.Http;
import org.commonjava.maven.galley.transport.htcli.internal.util.TransferResponseUtils;
import org.commonjava.maven.galley.transport.htcli.model.HttpLocation;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -144,16 +145,7 @@ private HttpResponse executeGet( final HttpGet request, final String url )
logger.debug( "GET {} : {}", line, url );
if ( sc != HttpStatus.SC_OK )
{
EntityUtils.consume( response.getEntity() );

if ( sc == HttpStatus.SC_NOT_FOUND )
{
return null;
}
else
{
throw new TransferException( "HTTP request failed: {}", line );
}
return TransferResponseUtils.handleUnsuccessfulResponse( request, response, url );
}
else
{
Expand Down
15 changes: 4 additions & 11 deletions ...ent/src/main/java/org/commonjava/maven/galley/transport/htcli/internal/HttpExistence.java
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@
import org.commonjava.maven.galley.TransferException;
import org.commonjava.maven.galley.spi.transport.ExistenceJob;
import org.commonjava.maven.galley.transport.htcli.Http;
import org.commonjava.maven.galley.transport.htcli.internal.util.TransferResponseUtils;
import org.commonjava.maven.galley.transport.htcli.model.HttpLocation;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -82,8 +83,6 @@ public TransferException getError()
private boolean execute( final HttpHead request, final String url )
throws TransferException
{
boolean result = false;

try
{
final HttpResponse response = http.getClient()
Expand All @@ -93,14 +92,8 @@ private boolean execute( final HttpHead request, final String url )
logger.debug( "HEAD {} : {}", line, url );
if ( sc != HttpStatus.SC_OK )
{
if ( sc != HttpStatus.SC_NOT_FOUND )
{
throw new TransferException( "HTTP request failed: {}", line );
}
}
else
{
result = true;
TransferResponseUtils.handleUnsuccessfulResponse( request, response, url );
return false;
}
}
catch ( final ClientProtocolException e )
Expand All @@ -112,7 +105,7 @@ private boolean execute( final HttpHead request, final String url )
throw new TransferException( "Repository remote request failed for: {}. Reason: {}", e, url, e.getMessage() );
}

return result;
return true;
}

private void cleanup( final HttpHead request )
Expand Down
11 changes: 3 additions & 8 deletions ...lient/src/main/java/org/commonjava/maven/galley/transport/htcli/internal/HttpListing.java
View file
Open in desktop
Expand Up @@ -37,6 +37,7 @@
import org.commonjava.maven.galley.model.TransferOperation;
import org.commonjava.maven.galley.spi.transport.ListingJob;
import org.commonjava.maven.galley.transport.htcli.Http;
import org.commonjava.maven.galley.transport.htcli.internal.util.TransferResponseUtils;
import org.commonjava.maven.galley.transport.htcli.model.HttpLocation;
import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
Expand Down Expand Up @@ -166,14 +167,8 @@ private InputStream executeGet( final HttpGet request, final String url )
logger.debug( "GET {} : {}", line, url );
if ( sc != HttpStatus.SC_OK )
{
if ( sc == HttpStatus.SC_NOT_FOUND )
{
result = null;
}
else
{
throw new TransferException( "HTTP request failed: %s", line );
}
TransferResponseUtils.handleUnsuccessfulResponse( request, response, url );
result = null;
}
else
{
Expand Down
3 changes: 2 additions & 1 deletion ...lient/src/main/java/org/commonjava/maven/galley/transport/htcli/internal/HttpPublish.java
View file
Open in desktop
Expand Up @@ -28,6 +28,7 @@
import org.commonjava.maven.galley.TransferException;
import org.commonjava.maven.galley.spi.transport.PublishJob;
import org.commonjava.maven.galley.transport.htcli.Http;
import org.commonjava.maven.galley.transport.htcli.internal.util.TransferResponseUtils;
import org.commonjava.maven.galley.transport.htcli.model.HttpLocation;
import org.commonjava.maven.galley.util.ContentTypeUtils;
import org.slf4j.Logger;
Expand Down Expand Up @@ -114,7 +115,7 @@ private HttpResponse executePut( final HttpPut request, final String url )
if ( sc != HttpStatus.SC_OK && sc != HttpStatus.SC_CREATED )
{
logger.warn( "{} : {}", line, url );
throw new TransferException( "HTTP request failed: {}", line );
return TransferResponseUtils.handleUnsuccessfulResponse( request, response, url, false );
}
else
{
Expand Down
90 changes: 90 additions & 0 deletions ...java/org/commonjava/maven/galley/transport/htcli/internal/util/TransferResponseUtils.java
View file
Open in desktop
@@ -0,0 +1,90 @@
package org.commonjava.maven.galley.transport.htcli.internal.util;

import static org.apache.commons.io.IOUtils.copy;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;

import org.apache.commons.io.IOUtils;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.StatusLine;
import org.apache.http.client.methods.AbstractExecutionAwareRequest;
import org.apache.http.util.EntityUtils;
import org.commonjava.maven.galley.TransferException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public final class TransferResponseUtils
{

private TransferResponseUtils()
{
}

public static HttpResponse handleUnsuccessfulResponse( final AbstractExecutionAwareRequest request,
final HttpResponse response, final String url )
throws TransferException
{
return handleUnsuccessfulResponse( request, response, url, true );
}

public static HttpResponse handleUnsuccessfulResponse( final AbstractExecutionAwareRequest request,
final HttpResponse response, final String url,
final boolean graceful404 )
throws TransferException
{
final Logger logger = LoggerFactory.getLogger( TransferResponseUtils.class );

final StatusLine line = response.getStatusLine();
InputStream in = null;
HttpEntity entity = null;
try
{
entity = response.getEntity();
final int sc = line.getStatusCode();
if ( graceful404 && sc == HttpStatus.SC_NOT_FOUND )
{
return null;
}
else
{
ByteArrayOutputStream out = null;
if ( entity != null )
{
in = entity.getContent();
out = new ByteArrayOutputStream();
copy( in, out );
}

throw new TransferException( "HTTP request failed: %s%s", line, ( out == null ? "" : "\n\n"
+ new String( out.toByteArray() ) ) );
}
}
catch ( final IOException e )
{
request.abort();
throw new TransferException(
"Error reading body of unsuccessful request.\nStatus: %s.\nURL: %s.\nReason: %s",
e, line, url, e.getMessage() );
}
finally
{
IOUtils.closeQuietly( in );
if ( entity != null )
{
try
{
EntityUtils.consume( entity );
}
catch ( final IOException e )
{
logger.debug( "Failed to consume entity: " + e.getMessage(), e );
}
}
}
}

}
21 changes: 10 additions & 11 deletions .../src/test/java/org/commonjava/maven/galley/transport/htcli/internal/HttpDownloadTest.java
View file
Open in desktop
Expand Up @@ -20,8 +20,6 @@
import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertThat;

import java.util.Map;

import org.commonjava.maven.galley.TransferException;
import org.commonjava.maven.galley.model.ConcreteResource;
import org.commonjava.maven.galley.model.Transfer;
Expand All @@ -42,6 +40,9 @@ public void simpleRetrieveOfAvailableUrl()
{
final String fname = "simple-retrieval.html";

fixture.getServer()
.expect( fixture.formatUrl( fname ), 200, fname );

final String baseUri = fixture.getBaseUri();
final SimpleHttpLocation location = new SimpleHttpLocation( "test", baseUri, true, true, true, true, null );
final Transfer transfer = fixture.getTransfer( new ConcreteResource( location, fname ) );
Expand All @@ -60,10 +61,9 @@ public void simpleRetrieveOfAvailableUrl()
assertThat( result.exists(), equalTo( true ) );
assertThat( transfer.exists(), equalTo( true ) );

final Map<String, Integer> accessesByPath = fixture.getAccessesByPath();
final String path = fixture.getUrlPath( url );

assertThat( accessesByPath.get( path ), equalTo( 1 ) );
assertThat( fixture.getAccessesFor( path ), equalTo( 1 ) );
}

@Test
Expand All @@ -89,10 +89,9 @@ public void simpleRetrieveOfMissingUrl()
assertThat( result.exists(), equalTo( false ) );
assertThat( transfer.exists(), equalTo( false ) );

final Map<String, Integer> accessesByPath = fixture.getAccessesByPath();
final String path = fixture.getUrlPath( url );

assertThat( accessesByPath.get( path ), equalTo( 1 ) );
assertThat( fixture.getAccessesFor( path ), equalTo( 1 ) );
}

@Test
Expand All @@ -107,7 +106,8 @@ public void simpleRetrieveOfUrlWithError()
final String url = fixture.formatUrl( fname );

final String error = "Test Error.";
fixture.registerException( fixture.getUrlPath( url ), error );
final String path = fixture.getUrlPath( url );
fixture.registerException( path, error );

assertThat( transfer.exists(), equalTo( false ) );

Expand All @@ -116,16 +116,15 @@ public void simpleRetrieveOfUrlWithError()

final TransferException err = dl.getError();
assertThat( err, notNullValue() );

assertThat( err.getMessage()
.endsWith( error ), equalTo( true ) );
.contains( error ), equalTo( true ) );

assertThat( result, nullValue() );
assertThat( transfer.exists(), equalTo( false ) );

final Map<String, Integer> accessesByPath = fixture.getAccessesByPath();
final String path = fixture.getUrlPath( url );

assertThat( accessesByPath.get( path ), equalTo( 1 ) );
assertThat( fixture.getAccessesFor( path ), equalTo( 1 ) );
}

}