From 9935c58d59bf8f449ea04861a0c75301b7deaf5e Mon Sep 17 00:00:00 2001 From: Lukas Baca Date: Mon, 4 May 2020 11:47:23 +0200 Subject: [PATCH] Report HTTP status 409 on DATA_MISSING error Change the HTTP status reported on DATA_MISSING conditions to 409, to match RESTCONF specification. Previous behavior of using 404 can be selected globally via a system property. JIRA: NETCONF-682 Change-Id: Ibb10b7f9b8d49cd85edaf4fee8986a14cc9f3506 Signed-off-by: Lukas Baca Signed-off-by: Robert Varga --- .../restconf/common/errors/RestconfError.java | 27 ++++++++++++++++++- .../restconf/impl/test/BrokerFacadeTest.java | 5 ++-- ...RestconfDocumentedExceptionMapperTest.java | 4 +-- .../restconf/impl/test/RestconfErrorTest.java | 2 +- .../utils/DeleteDataTransactionUtilTest.java | 3 +-- .../utils/parser/ParserIdentifierTest.java | 2 -- ...angInstanceIdentifierDeserializerTest.java | 4 --- 7 files changed, 32 insertions(+), 15 deletions(-) diff --git a/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/errors/RestconfError.java b/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/errors/RestconfError.java index 44890df1a9..d88652b914 100644 --- a/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/errors/RestconfError.java +++ b/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/errors/RestconfError.java @@ -13,6 +13,8 @@ import java.io.Serializable; import java.util.Locale; import org.opendaylight.yangtools.yang.common.RpcError; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Encapsulates a restconf error as defined in the ietf restconf draft. @@ -25,6 +27,7 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; * See also RESTCONF. */ public class RestconfError implements Serializable { + private static final Logger LOG = LoggerFactory.getLogger(RestconfError.class); private static final long serialVersionUID = 1L; public enum ErrorType { @@ -74,7 +77,7 @@ public class RestconfError implements Serializable { RESOURCE_DENIED("resource-denied", 409 /* Conflict */), ROLLBACK_FAILED("rollback-failed", 500 /* INTERNAL_SERVER_ERROR */), DATA_EXISTS("data-exists", 409 /* Conflict */), - DATA_MISSING("data-missing", 404 /* Resource Not Found */), + DATA_MISSING("data-missing", dataMissingHttpStatus()), OPERATION_NOT_SUPPORTED("operation-not-supported", 501 /* Not Implemented */), OPERATION_FAILED("operation-failed", 500 /* INTERNAL_SERVER_ERROR */), PARTIAL_OPERATION("partial-operation", 500 /* INTERNAL_SERVER_ERROR */), @@ -104,6 +107,28 @@ public class RestconfError implements Serializable { public int getStatusCode() { return statusCode; } + + private static int dataMissingHttpStatus() { + // Control over the HTTP status reported on "data-missing" conditions. This defaults to disabled, + // HTTP status 409 as specified by RFC8040 (and all previous drafts). See the discussion in: + // https://www.rfc-editor.org/errata/eid5565 + // https://mailarchive.ietf.org/arch/msg/netconf/hkVDdHK4xA74NgvXzWP0zObMiyY/ + final String propName = "org.opendaylight.restconf.eid5565"; + final String propValue = System.getProperty(propName, "disabled"); + switch (propValue) { + case "enabled": + // RFC7231 interpretation: 404 Not Found + LOG.info("RESTCONF data-missing condition is reported as HTTP status 404 (Errata 5565)"); + return 404; + case "disabled": + break; + default: + LOG.warn("Unhandled {} value \"{}\", assuming disabled", propName, propValue); + } + + // RFC8040 specification: 409 Conflict + return 409; + } } private final ErrorType errorType; diff --git a/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java b/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java index cc3e8ff062..9d612e2870 100644 --- a/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java +++ b/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java @@ -271,21 +271,20 @@ public class BrokerFacadeTest { } /** - * Negative test of delete operation when data to delete does not exist. Error 404 should be returned. + * Negative test of delete operation when data to delete does not exist. Error DATA_MISSING should be returned. */ @Test public void testCommitConfigurationDataDeleteNoData() throws Exception { // assume that data to delete does not exist prepareDataForDelete(false); - // try to delete and expect 404 error + // try to delete and expect DATA_MISSING error try { this.brokerFacade.commitConfigurationDataDelete(this.instanceID); fail("Delete operation should fail due to missing data"); } catch (final RestconfDocumentedException e) { assertEquals(ErrorType.PROTOCOL, e.getErrors().get(0).getErrorType()); assertEquals(ErrorTag.DATA_MISSING, e.getErrors().get(0).getErrorTag()); - assertEquals(404, e.getErrors().get(0).getErrorTag().getStatusCode()); } } diff --git a/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfDocumentedExceptionMapperTest.java b/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfDocumentedExceptionMapperTest.java index 563c14ceb6..89f0dfb9cc 100644 --- a/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfDocumentedExceptionMapperTest.java +++ b/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfDocumentedExceptionMapperTest.java @@ -333,7 +333,7 @@ public class RestconfDocumentedExceptionMapperTest extends JerseyTest { public void testToJsonResponseWithDataMissingErrorTag() throws Exception { testJsonResponse(new RestconfDocumentedException("mock error", ErrorType.PROTOCOL, ErrorTag.DATA_MISSING), - Status.NOT_FOUND, ErrorType.PROTOCOL, ErrorTag.DATA_MISSING, "mock error", null, null); + Status.CONFLICT, ErrorType.PROTOCOL, ErrorTag.DATA_MISSING, "mock error", null, null); } @Test @@ -556,7 +556,7 @@ public class RestconfDocumentedExceptionMapperTest extends JerseyTest { public void testToXMLResponseWithDataMissingErrorTag() throws Exception { testXMLResponse(new RestconfDocumentedException("mock error", ErrorType.PROTOCOL, ErrorTag.DATA_MISSING), - Status.NOT_FOUND, ErrorType.PROTOCOL, ErrorTag.DATA_MISSING, "mock error", null, null); + Status.CONFLICT, ErrorType.PROTOCOL, ErrorTag.DATA_MISSING, "mock error", null, null); } @Test diff --git a/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java b/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java index a5d7bbaa54..469c6e0762 100644 --- a/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java +++ b/restconf/restconf-nb-bierman02/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java @@ -91,7 +91,7 @@ public class RestconfErrorTest { lookUpMap.put("resource-denied", 409); lookUpMap.put("rollback-failed", 500); lookUpMap.put("data-exists", 409); - lookUpMap.put("data-missing", 404); + lookUpMap.put("data-missing", 409); lookUpMap.put("operation-not-supported", 501); lookUpMap.put("operation-failed", 500); lookUpMap.put("partial-operation", 500); diff --git a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtilTest.java b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtilTest.java index 696ae89ee5..b0fce39c48 100644 --- a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtilTest.java +++ b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtilTest.java @@ -73,7 +73,7 @@ public class DeleteDataTransactionUtilTest { } /** - * Negative test for DELETE operation when data to delete does not exist. Error 404 is expected. + * Negative test for DELETE operation when data to delete does not exist. Error DATA_MISSING is expected. */ @Test public void deleteDataNegativeTest() throws Exception { @@ -89,7 +89,6 @@ public class DeleteDataTransactionUtilTest { } catch (final RestconfDocumentedException e) { assertEquals(ErrorType.PROTOCOL, e.getErrors().get(0).getErrorType()); assertEquals(ErrorTag.DATA_MISSING, e.getErrors().get(0).getErrorTag()); - assertEquals(404, e.getErrors().get(0).getErrorTag().getStatusCode()); } } } diff --git a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/ParserIdentifierTest.java b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/ParserIdentifierTest.java index ca871654ea..0383d53d59 100644 --- a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/ParserIdentifierTest.java +++ b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/ParserIdentifierTest.java @@ -250,8 +250,6 @@ public class ParserIdentifierTest { RestconfError.ErrorType.PROTOCOL, e.getErrors().get(0).getErrorType()); assertEquals("Not expected error tag", ErrorTag.DATA_MISSING, e.getErrors().get(0).getErrorTag()); - assertEquals("Not expected error status code", - 404, e.getErrors().get(0).getErrorTag().getStatusCode()); } } diff --git a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializerTest.java b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializerTest.java index 7c5d9ead44..1b92a5688e 100644 --- a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializerTest.java +++ b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/utils/parser/YangInstanceIdentifierDeserializerTest.java @@ -401,8 +401,6 @@ public class YangInstanceIdentifierDeserializerTest { RestconfError.ErrorType.PROTOCOL, e.getErrors().get(0).getErrorType()); assertEquals("Not expected error tag", RestconfError.ErrorTag.DATA_MISSING, e.getErrors().get(0).getErrorTag()); - assertEquals("Not expected error status code", - 404, e.getErrors().get(0).getErrorTag().getStatusCode()); } } @@ -422,8 +420,6 @@ public class YangInstanceIdentifierDeserializerTest { RestconfError.ErrorType.PROTOCOL, e.getErrors().get(0).getErrorType()); assertEquals("Not expected error tag", RestconfError.ErrorTag.DATA_MISSING, e.getErrors().get(0).getErrorTag()); - assertEquals("Not expected error status code", - 404, e.getErrors().get(0).getErrorTag().getStatusCode()); } } -- 2.36.6