You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by mi...@apache.org on 2020/04/19 21:35:29 UTC
[maven-wagon] 01/02: First changes
This is an automated email from the ASF dual-hosted git repository.
michaelo pushed a commit to branch WAGON-580
in repository https://gitbox.apache.org/repos/asf/maven-wagon.git
commit 691f9929d583e95bec0798ca6273bfd5ddbcec87
Author: Michael Osipov <mi...@apache.org>
AuthorDate: Sun Apr 19 22:38:13 2020 +0200
First changes
---
.../apache/maven/wagon/http/HttpWagonTestCase.java | 12 +++---
.../providers/http/LightweightHttpWagonTest.java | 20 +++++-----
.../maven/wagon/shared/http/HttpMessageUtils.java | 43 +++++++++++++---------
.../wagon/providers/http/HttpWagonErrorTest.java | 10 ++---
.../apache/maven/wagon/tck/http/Assertions.java | 24 ++++++------
5 files changed, 58 insertions(+), 51 deletions(-)
diff --git a/wagon-provider-test/src/main/java/org/apache/maven/wagon/http/HttpWagonTestCase.java b/wagon-provider-test/src/main/java/org/apache/maven/wagon/http/HttpWagonTestCase.java
index b747768..f7b2341 100644
--- a/wagon-provider-test/src/main/java/org/apache/maven/wagon/http/HttpWagonTestCase.java
+++ b/wagon-provider-test/src/main/java/org/apache/maven/wagon/http/HttpWagonTestCase.java
@@ -2397,7 +2397,7 @@ public abstract class HttpWagonTestCase
assertTrue( "404 not found response should throw ResourceDoesNotExistException",
e instanceof ResourceDoesNotExistException );
reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? " Not Found" : ( " " + forReasonPhrase );
- assertEquals( assertMessageForBadMessage, "Resource missing at " + forUrl + " 404"
+ assertEquals( assertMessageForBadMessage, "resource missing at " + forUrl + ", status: 404"
+ reasonPhrase, e.getMessage() );
break;
@@ -2408,7 +2408,7 @@ public abstract class HttpWagonTestCase
+ "methods",
e instanceof AuthorizationException );
reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? " Unauthorized" : ( " " + forReasonPhrase );
- assertEquals( assertMessageForBadMessage, "Authentication failed for " + forUrl + " 401"
+ assertEquals( assertMessageForBadMessage, "authentication failed for " + forUrl + ", status: 401"
+ reasonPhrase, e.getMessage() );
break;
@@ -2417,15 +2417,15 @@ public abstract class HttpWagonTestCase
e instanceof AuthorizationException );
reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? " Proxy Authentication Required"
: ( " " + forReasonPhrase );
- assertEquals( assertMessageForBadMessage, "HTTP proxy server authentication failed for "
- + forUrl + " 407" + reasonPhrase, e.getMessage() );
+ assertEquals( assertMessageForBadMessage, "proxy authentication failed for "
+ + forUrl + ", status: 407" + reasonPhrase, e.getMessage() );
break;
case HttpServletResponse.SC_FORBIDDEN:
assertTrue( "403 Forbidden should throw AuthorizationException",
e instanceof AuthorizationException );
reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? " Forbidden" : ( " " + forReasonPhrase );
- assertEquals( assertMessageForBadMessage, "Authorization failed for " + forUrl + " 403"
+ assertEquals( assertMessageForBadMessage, "authorization failed for " + forUrl + ", status: 403"
+ reasonPhrase, e.getMessage() );
break;
@@ -2435,7 +2435,7 @@ public abstract class HttpWagonTestCase
assertTrue( "expected status code for transfer failures should be >= 400",
forStatusCode >= HttpServletResponse.SC_BAD_REQUEST );
reasonPhrase = forReasonPhrase == null ? "" : " " + forReasonPhrase;
- assertEquals( assertMessageForBadMessage, "Transfer failed for " + forUrl + " "
+ assertEquals( assertMessageForBadMessage, "transfer failed for " + forUrl + ", status: "
+ forStatusCode + reasonPhrase, e.getMessage() );
break;
}
diff --git a/wagon-providers/wagon-http-lightweight/src/test/java/org/apache/maven/wagon/providers/http/LightweightHttpWagonTest.java b/wagon-providers/wagon-http-lightweight/src/test/java/org/apache/maven/wagon/providers/http/LightweightHttpWagonTest.java
index 1db8454..2c07ed6 100644
--- a/wagon-providers/wagon-http-lightweight/src/test/java/org/apache/maven/wagon/providers/http/LightweightHttpWagonTest.java
+++ b/wagon-providers/wagon-http-lightweight/src/test/java/org/apache/maven/wagon/providers/http/LightweightHttpWagonTest.java
@@ -99,12 +99,12 @@ public class LightweightHttpWagonTest
e.getCause() instanceof FileNotFoundException );
// the status code and reason phrase cannot always be learned due to implementation limitations
// which means the message may not include them
- assertEquals( assertMessageForBadMessage, "Resource missing at " + forUrl, e.getMessage() );
+ assertEquals( assertMessageForBadMessage, "resource missing at " + forUrl, e.getMessage() );
}
else
{
- assertEquals( assertMessageForBadMessage, "Resource missing at " + forUrl
- + " " + forStatusCode + " " + forReasonPhrase, e.getMessage() );
+ assertEquals( assertMessageForBadMessage, "resource missing at " + forUrl
+ + ", status: " + forStatusCode + " " + forReasonPhrase, e.getMessage() );
}
break;
@@ -113,7 +113,7 @@ public class LightweightHttpWagonTest
assertTrue( "403 Forbidden throws AuthorizationException",
e instanceof AuthorizationException );
- assertEquals( assertMessageForBadMessage, "Authorization failed for " + forUrl + " 403"
+ assertEquals( assertMessageForBadMessage, "authorization failed for " + forUrl + ", status: 403"
+ ( StringUtils.isEmpty( forReasonPhrase ) ? " Forbidden" : ( " " + forReasonPhrase ) ),
e.getMessage() );
break;
@@ -122,7 +122,7 @@ public class LightweightHttpWagonTest
assertTrue( "401 Unauthorized throws AuthorizationException",
e instanceof AuthorizationException );
- assertEquals( assertMessageForBadMessage, "Authentication failed for " + forUrl + " 401"
+ assertEquals( assertMessageForBadMessage, "authentication failed for " + forUrl + ", status: 401"
+ ( StringUtils.isEmpty( forReasonPhrase ) ? " Unauthorized" :
( " " + forReasonPhrase ) ),
e.getMessage() );
@@ -144,18 +144,18 @@ public class LightweightHttpWagonTest
// the status code and reason phrase cannot always be learned due to implementation limitations
// so the message may not include them, but the implementation should use a consistent format
assertTrue( "message should always include url",
- e.getMessage().startsWith( "Transfer failed for " + forUrl ) );
+ e.getMessage().startsWith( "transfer failed for " + forUrl ) );
- if ( e.getMessage().length() > ( "Transfer failed for " + forUrl ).length() )
+ if ( e.getMessage().length() > ( "transfer failed for " + forUrl ).length() )
{
assertTrue( "message should include url and status code",
- e.getMessage().startsWith( "Transfer failed for " + forUrl + " " + forStatusCode ) );
+ e.getMessage().startsWith( "transfer failed for " + forUrl + ", status: " + forStatusCode ) );
}
- if ( e.getMessage().length() > ( "Transfer failed for " + forUrl + " " + forStatusCode ).length() )
+ if ( e.getMessage().length() > ( "transfer failed for " + forUrl + ", status: " + forStatusCode ).length() )
{
assertEquals( "message should include url and status code and reason phrase",
- "Transfer failed for " + forUrl + " " + forStatusCode + " " + forReasonPhrase,
+ "transfer failed for " + forUrl + ", status: " + forStatusCode + " " + forReasonPhrase,
e.getMessage() );
}
diff --git a/wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/HttpMessageUtils.java b/wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/HttpMessageUtils.java
index a58b653..3477863 100644
--- a/wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/HttpMessageUtils.java
+++ b/wagon-providers/wagon-http-shared/src/main/java/org/apache/maven/wagon/shared/http/HttpMessageUtils.java
@@ -19,6 +19,8 @@ package org.apache.maven.wagon.shared.http;
* under the License.
*/
+import java.util.Objects;
+
import org.apache.maven.wagon.ResourceDoesNotExistException;
import org.apache.maven.wagon.TransferFailedException;
import org.apache.maven.wagon.authorization.AuthorizationException;
@@ -35,16 +37,17 @@ import org.codehaus.plexus.util.StringUtils;
* Status-Code is intended for use by automata and the Reason-Phrase is intended for the human user. The client is not
* required to examine or display the Reason- Phrase.</cite></li>
* <li>it has been later largely deprecated in <a href="https://tools.ietf.org/html/rfc7230#section-3.1.2">the updated
- * HTTP/1.1 RFC-7230</a>: <cite>The reason-phrase element exists for the sole purpose of providing a textual description
- * associated with the numeric status code, mostly out of deference to earlier Internet application protocols that were
- * more frequently used with interactive text clients. A client SHOULD ignore the reason-phrase content.</cite></li>
- * <li>it has been removed from <a href="https://tools.ietf.org/html/rfc7540#section-8.1.2.4">HTTP/2 RFC 7540</a>:
+ * HTTP/1.1 in RFC 7230</a>: <cite>The reason-phrase element exists for the sole purpose of providing a textual
+ * description associated with the numeric status code, mostly out of deference to earlier Internet application
+ * protocols that were more frequently used with interactive text clients. A client SHOULD ignore the reason-phrase
+ * content.</cite></li>
+ * <li>it has been removed from <a href="https://tools.ietf.org/html/rfc7540#section-8.1.2.4">HTTP/2 in RFC 7540</a>:
* <cite>HTTP/2 does not define a way to carry the version or reason phrase that is included in an HTTP/1.1 status
* line.</cite>.</li>
* </ul>
* The use of Reason Phrase done here to improve the message to the end-user (particularly in case of failures) will
- * disappear while HTTP/2 is deployed: a new mechanism to provide such a message needs to be defined...
- *
+ * disappear while HTTP/2 is deployed: a new mechanism to provide such a message needs to be defined...
+ *
* @since 3.3.4
*/
public class HttpMessageUtils
@@ -63,16 +66,16 @@ public class HttpMessageUtils
public static final int UNKNOWN_STATUS_CODE = -1;
/**
- * Format a consistent HTTP transfer debug message combining url, status code, status line reason phrase and HTTP
+ * Format a consistent HTTP transfer debug message combining URL, status code, reason phrase and HTTP
* proxy server info.
* <p>
- * Url will always be included in the message. A status code other than {@link #UNKNOWN_STATUS_CODE} will be
+ * URL will always be included in the message. A status code other than {@link #UNKNOWN_STATUS_CODE} will be
* included. A reason phrase will only be included if non-empty and status code is not {@link #UNKNOWN_STATUS_CODE}.
* Proxy information will only be included if not null.
*
* @param url the required non-null URL associated with the message
* @param statusCode an HTTP response status code
- * @param reasonPhrase an HTTP status line reason phrase
+ * @param reasonPhrase an HTTP reason phrase
* @param proxyInfo proxy server used during the transfer, may be null if none used
* @return a formatted debug message combining the parameters of this method
* @throws NullPointerException if url is null
@@ -80,6 +83,7 @@ public class HttpMessageUtils
public static String formatTransferDebugMessage( String url, int statusCode, String reasonPhrase,
ProxyInfo proxyInfo )
{
+ Objects.requireNonNull( url, "url cannot be null" );
String msg = url;
if ( statusCode != UNKNOWN_STATUS_CODE )
{
@@ -91,7 +95,7 @@ public class HttpMessageUtils
}
if ( proxyInfo != null )
{
- msg += " -- " + proxyInfo.toString();
+ msg += " -- proxy: " + proxyInfo;
}
return msg;
}
@@ -125,7 +129,7 @@ public class HttpMessageUtils
public static String formatTransferFailedMessage( String url, int statusCode, String reasonPhrase,
ProxyInfo proxyInfo )
{
- return formatMessage( "Transfer failed for ", url, statusCode, reasonPhrase, proxyInfo );
+ return formatMessage( "transfer failed for ", url, statusCode, reasonPhrase, proxyInfo );
}
/**
@@ -140,26 +144,27 @@ public class HttpMessageUtils
* @param proxyInfo proxy server used during the transfer, may be null if none used
* @return a consistent message for a HTTP related {@link AuthorizationException}
*/
+ // TODO Split when WAGON-568 is implemented
public static String formatAuthorizationMessage( String url, int statusCode, String reasonPhrase,
ProxyInfo proxyInfo )
{
switch ( statusCode )
{
case SC_UNAUTHORIZED: // no credentials or auth was not valid
- return formatMessage( "Authentication failed for ", url, statusCode, reasonPhrase, null );
+ return formatMessage( "authentication failed for ", url, statusCode, reasonPhrase, null );
case SC_FORBIDDEN: // forbidden based on permissions usually
- return formatMessage( "Authorization failed for ", url, statusCode, reasonPhrase, null );
+ return formatMessage( "authorization failed for ", url, statusCode, reasonPhrase, null );
case SC_PROXY_AUTH_REQUIRED:
- return formatMessage( "HTTP proxy server authentication failed for ", url, statusCode,
+ return formatMessage( "proxy authentication failed for ", url, statusCode,
reasonPhrase, null );
default:
break;
}
- return formatMessage( "Authorization failed for ", url, statusCode, reasonPhrase, proxyInfo );
+ return formatMessage( "authorization failed for ", url, statusCode, reasonPhrase, proxyInfo );
}
/**
@@ -177,16 +182,18 @@ public class HttpMessageUtils
public static String formatResourceDoesNotExistMessage( String url, int statusCode, String reasonPhrase,
ProxyInfo proxyInfo )
{
- return formatMessage( "Resource missing at ", url, statusCode, reasonPhrase, proxyInfo );
+ return formatMessage( "resource missing at ", url, statusCode, reasonPhrase, proxyInfo );
}
private static String formatMessage( String message, String url, int statusCode, String reasonPhrase,
ProxyInfo proxyInfo )
{
+ Objects.requireNonNull( message, "message cannot be null" );
+ Objects.requireNonNull( url, "url cannot be null" );
String msg = message + url;
if ( statusCode != UNKNOWN_STATUS_CODE )
{
- msg += " " + statusCode;
+ msg += ", status: " + statusCode;
if ( StringUtils.isNotEmpty( reasonPhrase ) )
{
@@ -223,7 +230,7 @@ public class HttpMessageUtils
}
if ( proxyInfo != null )
{
- msg += " " + proxyInfo.toString();
+ msg += ", proxy: " + proxyInfo;
}
return msg;
}
diff --git a/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/HttpWagonErrorTest.java b/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/HttpWagonErrorTest.java
index 63dd944..17b4e5a 100644
--- a/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/HttpWagonErrorTest.java
+++ b/wagon-providers/wagon-http/src/test/java/org/apache/maven/wagon/providers/http/HttpWagonErrorTest.java
@@ -79,7 +79,7 @@ public class HttpWagonErrorTest
assertNotNull( thrown );
assertEquals( AuthorizationException.class, thrown.getClass() );
- assertEquals( "Authentication failed for http://localhost:" + serverPort + "/401 401 "
+ assertEquals( "authentication failed for http://localhost:" + serverPort + "/401, status: 401 "
+ ErrorWithMessageServlet.MESSAGE, thrown.getMessage() );
}
@@ -115,7 +115,7 @@ public class HttpWagonErrorTest
assertNotNull( thrown );
assertEquals( AuthorizationException.class, thrown.getClass() );
- assertEquals( "Authorization failed for http://localhost:" + serverPort + "/403 403 "
+ assertEquals( "authorization failed for http://localhost:" + serverPort + "/403, status: 403 "
+ ErrorWithMessageServlet.MESSAGE, thrown.getMessage() );
}
@@ -151,7 +151,7 @@ public class HttpWagonErrorTest
assertNotNull( thrown );
assertEquals( ResourceDoesNotExistException.class, thrown.getClass() );
- assertEquals( "Resource missing at http://localhost:" + serverPort + "/404 404 "
+ assertEquals( "resource missing at http://localhost:" + serverPort + "/404, status: 404 "
+ ErrorWithMessageServlet.MESSAGE, thrown.getMessage() );
}
@@ -187,7 +187,7 @@ public class HttpWagonErrorTest
assertNotNull( thrown );
assertEquals( AuthorizationException.class, thrown.getClass() );
- assertEquals( "HTTP proxy server authentication failed for http://localhost:" + serverPort + "/407 407 "
+ assertEquals( "proxy authentication failed for http://localhost:" + serverPort + "/407, status: 407 "
+ ErrorWithMessageServlet.MESSAGE, thrown.getMessage() );
}
@@ -223,7 +223,7 @@ public class HttpWagonErrorTest
assertNotNull( thrown );
assertEquals( TransferFailedException.class, thrown.getClass() );
- assertEquals( "Transfer failed for http://localhost:" + serverPort + "/500 500 "
+ assertEquals( "transfer failed for http://localhost:" + serverPort + "/500, status: 500 "
+ ErrorWithMessageServlet.MESSAGE, thrown.getMessage() );
}
}
diff --git a/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/Assertions.java b/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/Assertions.java
index eb7cfa8..ca9dc66 100644
--- a/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/Assertions.java
+++ b/wagon-tcks/wagon-tck-http/src/main/java/org/apache/maven/wagon/tck/http/Assertions.java
@@ -40,7 +40,7 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
/**
- *
+ *
*/
public final class Assertions
{
@@ -119,7 +119,7 @@ public final class Assertions
assertTrue( "404 not found response should throw ResourceDoesNotExistException",
e instanceof ResourceDoesNotExistException );
reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? " Not Found" : ( " " + forReasonPhrase );
- assertEquals( assertMessageForBadMessage, "Resource missing at " + forUrl + " 404"
+ assertEquals( assertMessageForBadMessage, "resource missing at " + forUrl + ", status: 404"
+ reasonPhrase, e.getMessage() );
break;
@@ -130,7 +130,7 @@ public final class Assertions
+ "methods",
e instanceof AuthorizationException );
reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? " Unauthorized" : ( " " + forReasonPhrase );
- assertEquals( assertMessageForBadMessage, "Authentication failed for " + forUrl + " 401"
+ assertEquals( assertMessageForBadMessage, "authentication failed for " + forUrl + ", status: 401"
+ reasonPhrase, e.getMessage() );
break;
@@ -139,15 +139,15 @@ public final class Assertions
e instanceof AuthorizationException );
reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? " Proxy Authentication Required"
: ( " " + forReasonPhrase );
- assertEquals( assertMessageForBadMessage, "HTTP proxy server authentication failed for "
- + forUrl + " 407" + reasonPhrase, e.getMessage() );
+ assertEquals( assertMessageForBadMessage, "proxy authentication failed for "
+ + forUrl + ", status: 407" + reasonPhrase, e.getMessage() );
break;
case HttpServletResponse.SC_FORBIDDEN:
assertTrue( "403 Forbidden should throw AuthorizationException",
e instanceof AuthorizationException );
reasonPhrase = StringUtils.isEmpty( forReasonPhrase ) ? " Forbidden" : ( " " + forReasonPhrase );
- assertEquals( assertMessageForBadMessage, "Authorization failed for " + forUrl + " 403"
+ assertEquals( assertMessageForBadMessage, "authorization failed for " + forUrl + ", status: 403"
+ reasonPhrase, e.getMessage() );
break;
@@ -158,29 +158,29 @@ public final class Assertions
// the status code and reason phrase cannot always be learned due to implementation limitations
// so the message may not include them, but the implementation should use a consistent format
assertTrue( "message should always include url tried: " + e.getMessage(),
- e.getMessage().startsWith( "Transfer failed for " + forUrl ) );
+ e.getMessage().startsWith( "transfer failed for " + forUrl ) );
String statusCodeStr = forStatusCode == NO_RESPONSE_STATUS_CODE ? ""
- : String.valueOf( forStatusCode );
+ : ", status: " + forStatusCode;
if ( forStatusCode != NO_RESPONSE_STATUS_CODE )
{
assertTrue( "if there was a response status line, the status code should be >= 400",
forStatusCode >= HttpServletResponse.SC_BAD_REQUEST );
- if ( e.getMessage().length() > ( "Transfer failed for " + forUrl ).length() )
+ if ( e.getMessage().length() > ( "transfer failed for " + forUrl ).length() )
{
assertTrue( "message should include url and status code: " + e.getMessage(),
- e.getMessage().startsWith( "Transfer failed for " + forUrl + statusCodeStr ) );
+ e.getMessage().startsWith( "transfer failed for " + forUrl + statusCodeStr ) );
}
reasonPhrase = forReasonPhrase == null ? "" : " " + forReasonPhrase;
- if ( reasonPhrase.length() > 0 && e.getMessage().length() > ( "Transfer failed for " + forUrl
+ if ( reasonPhrase.length() > 0 && e.getMessage().length() > ( "transfer failed for " + forUrl
+ statusCodeStr ).length() )
{
assertTrue( "message should include url and status code and reason phrase: "
- + e.getMessage(), e.getMessage().startsWith( "Transfer failed for "
+ + e.getMessage(), e.getMessage().startsWith( "transfer failed for "
+ forUrl + statusCodeStr
+ reasonPhrase ) );
}