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 <lbaca@luminanetworks.com>
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
Signed-off-by: Jamo Luhrsen <jluhrsen@gmail.com>

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 44890df1a98e6efa2ac38420f8ae8d161c91802f..ba8d73f7b0315fd6633e1dfc39a70e01f1aeada9 100644 (file)
--- 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 <a href="https://tools.ietf.org/html/draft-bierman-netconf-restconf-02">RESTCONF</a>.
  */
 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, "enabled");
+            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 e5be1fa141a168bc7d7f77fb119a60e802ec09c4..9f7902fe13c23d57bcb7f13f4595700e2b027d4c 100644 (file)
--- 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
@@ -270,21 +270,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-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 696ae89ee59c7cc83a2b7d2d878a15ecf6faef06..b0fce39c4853728dc5d666f7a6c514814aef3d57 100644 (file)
--- 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 d5f3568b6b1a0baf16307a83c48c52ad4d8175c1..e15feccc39d7d32c4520481a8817774c7465a299 100644 (file)
--- 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 58e6a44e8a6e23102994c6af8de7fef6acbab5e4..7d1bd5c65ada5b63dfbfa25b0c4b4aac30936ad8 100644 (file)
--- 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
@@ -400,8 +400,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());
         }
     }
 
@@ -421,8 +419,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());
         }
     }