Correct error code handler overlaying
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1691957 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ErrorPageRegistry.java b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ErrorPageRegistry.java
index 939d57c..a5048f2 100644
--- a/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ErrorPageRegistry.java
+++ b/http/base/src/main/java/org/apache/felix/http/base/internal/registry/ErrorPageRegistry.java
@@ -75,9 +75,14 @@
private final ServletHandler handler;
public final Map<Integer, ErrorRegistration> reasonMapping = new HashMap<Integer, ErrorRegistration>();
+ public final boolean usesClientErrorCodes;
+ public final boolean usesServerErrorCodes;
+
public ErrorRegistrationStatus(final ServletHandler handler)
{
this.handler = handler;
+ this.usesClientErrorCodes = hasErrorCode(handler, CLIENT_ERROR);
+ this.usesServerErrorCodes = hasErrorCode(handler, SERVER_ERROR);
}
public ServletHandler getHandler()
@@ -92,6 +97,18 @@
}
}
+ private static boolean hasErrorCode(final ServletHandler handler, final String key)
+ {
+ for(final String val : handler.getServletInfo().getErrorPage())
+ {
+ if ( key.equals(val) )
+ {
+ return true;
+ }
+ }
+ return false;
+ }
+
private static List<Long> hundredOf(final int start)
{
List<Long> result = new ArrayList<Long>();
@@ -301,7 +318,22 @@
if ( oldStatus != null )
{
removeReason(oldStatus, code, exception, -1);
- addReason(oldStatus, code, exception, DTOConstants.FAILURE_REASON_SHADOWED_BY_OTHER_SERVICE);
+ boolean addReason = true;
+ if ( exception == null )
+ {
+ if ( code >= 400 && code < 500 && oldStatus.usesClientErrorCodes && !status.usesClientErrorCodes )
+ {
+ addReason = false;
+ }
+ else if ( code >= 500 && code < 600 && oldStatus.usesServerErrorCodes && !status.usesServerErrorCodes )
+ {
+ addReason = false;
+ }
+ }
+ if ( addReason )
+ {
+ addReason(oldStatus, code, exception, DTOConstants.FAILURE_REASON_SHADOWED_BY_OTHER_SERVICE);
+ }
}
}
else
@@ -313,12 +345,37 @@
else
{
// failure
- addReason(status, code, exception, DTOConstants.FAILURE_REASON_SHADOWED_BY_OTHER_SERVICE);
+ boolean addReason = true;
+ if ( exception == null )
+ {
+ if ( code >= 400 && code < 500 && status.usesClientErrorCodes && !hasErrorCode(newList.get(0), CLIENT_ERROR) )
+ {
+ addReason = false;
+ }
+ else if ( code >= 500 && code < 600 && status.usesServerErrorCodes && !hasErrorCode(newList.get(0), SERVER_ERROR) )
+ {
+ addReason = false;
+ }
+ }
+ if ( addReason )
+ {
+ addReason(status, code, exception, DTOConstants.FAILURE_REASON_SHADOWED_BY_OTHER_SERVICE);
+ }
errorMapping.put(key, newList);
}
}
- private void addReason(final ErrorRegistrationStatus status, final long code, final String exception, final int reason)
+ /**
+ * Make an entry in the status object (which are used to create the DTOs)
+ * @param status The status object
+ * @param code Either the code
+ * @param exception or the exception
+ * @param reason The code for the failure reason or {@code -1} for success.
+ */
+ private void addReason(final ErrorRegistrationStatus status,
+ final long code,
+ final String exception,
+ final int reason)
{
ErrorRegistration reg = status.reasonMapping.get(reason);
if ( reg == null )
diff --git a/http/base/src/test/java/org/apache/felix/http/base/internal/registry/ErrorPageRegistryTest.java b/http/base/src/test/java/org/apache/felix/http/base/internal/registry/ErrorPageRegistryTest.java
index fc8c146..bb548da 100644
--- a/http/base/src/test/java/org/apache/felix/http/base/internal/registry/ErrorPageRegistryTest.java
+++ b/http/base/src/test/java/org/apache/felix/http/base/internal/registry/ErrorPageRegistryTest.java
@@ -246,9 +246,9 @@
// check DTO
reg.getRuntimeInfo(dto, holder.failedErrorPageDTOs);
- assertEquals(1, holder.failedErrorPageDTOs.size());
- assertEquals(2, dto.errorPageDTOs.length);
- assertEquals(98, dto.errorPageDTOs[1].errorCodes.length);
+ // a 4xx is only registered as failure DTO if overlayed by a 4xx!
+ // -> no failure in this case
+ assertEquals(0, holder.failedErrorPageDTOs.size());
final Set<Long> codes4 = new HashSet<Long>();
for(final long c : dto.errorPageDTOs[1].errorCodes)
{
@@ -266,16 +266,6 @@
codes.add(c);
}
assertEquals(2, codes.size());
-
- final FailedErrorPageDTO fep = holder.failedErrorPageDTOs.iterator().next();
- assertEquals(2, fep.errorCodes.length);
- codes.clear();
- for(final long c : fep.errorCodes)
- {
- assertTrue(c >= 403 && c < 405);
- codes.add(c);
- }
- assertEquals(2, codes.size());
}
private static ServletInfo createServletInfo(final long id, final int ranking, final String... codes) throws InvalidSyntaxException