Remove RestconfDocumentedException.status 34/108534/1
authorRobert Varga <robert.varga@pantheon.tech>
Fri, 20 Oct 2023 10:44:40 +0000 (12:44 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Fri, 20 Oct 2023 12:10:51 +0000 (14:10 +0200)
The actual status is derived from the errors reported, which now have to
have at least one element. RestconfDocumentedExceptionMapper is taught
to just infer the status.

JIRA: NETCONF-1188
Change-Id: I90b367a8fa1bf995e3d47d8b9f581a29f76eb2f2
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/errors/RestconfDocumentedException.java
restconf/restconf-nb/src/main/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/errors/RestconfDocumentedExceptionMapper.java
restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/databind/AbstractResourceBodyTest.java
restconf/restconf-nb/src/test/java/org/opendaylight/restconf/nb/rfc8040/jersey/providers/errors/RestconfDocumentedExceptionMapperTest.java

index 686d486e1adbdd88d80d288ca103a5235f5e348a..c04d9d98e8b5ef4a3df9c7d53351622f1c98ff21 100644 (file)
@@ -12,7 +12,6 @@ import static java.util.Objects.requireNonNull;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
-import javax.ws.rs.core.Response.Status;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.restconf.common.ErrorTags;
@@ -37,7 +36,6 @@ public class RestconfDocumentedException extends RuntimeException {
     private static final long serialVersionUID = 3L;
 
     private final List<RestconfError> errors;
-    private final Status status;
 
     // FIXME: this field should be non-null
     private final transient @Nullable EffectiveModelContext modelContext;
@@ -129,13 +127,12 @@ public class RestconfDocumentedException extends RuntimeException {
         // FIXME: We override getMessage so supplied message is lost for any public access
         // this was lost also in original code.
         super(cause);
-        if (!errors.isEmpty()) {
-            this.errors = List.copyOf(errors);
-        } else {
+        if (errors.isEmpty()) {
             this.errors = List.of(new RestconfError(ErrorType.APPLICATION, ErrorTag.OPERATION_FAILED, message));
+        } else {
+            this.errors = List.copyOf(errors);
         }
 
-        status = null;
         modelContext = null;
     }
 
@@ -147,21 +144,8 @@ public class RestconfDocumentedException extends RuntimeException {
         this(message, cause, convertToRestconfErrors(rpcErrors));
     }
 
-    /**
-     * Constructs an instance with an HTTP status and no error information.
-     *
-     * @param status
-     *            the HTTP status.
-     */
-    public RestconfDocumentedException(final Status status) {
-        errors = List.of();
-        modelContext = null;
-        this.status = requireNonNull(status, "Status can't be null");
-    }
-
     public RestconfDocumentedException(final Throwable cause, final RestconfError error) {
         super(cause);
-        status = ErrorTags.statusOf(error.getErrorTag());
         errors = List.of(error);
         modelContext = null;
     }
@@ -169,14 +153,15 @@ public class RestconfDocumentedException extends RuntimeException {
     public RestconfDocumentedException(final Throwable cause, final RestconfError error,
             final EffectiveModelContext modelContext) {
         super(cause);
-        status = ErrorTags.statusOf(error.getErrorTag());
         errors = List.of(error);
         this.modelContext = requireNonNull(modelContext);
     }
 
     public RestconfDocumentedException(final Throwable cause, final List<RestconfError> errors) {
         super(cause);
-        status = ErrorTags.statusOf(errors.get(0).getErrorTag());
+        if (errors.isEmpty()) {
+            throw new IllegalArgumentException("At least one error is required");
+        }
         this.errors = List.copyOf(errors);
         modelContext = null;
     }
@@ -291,8 +276,4 @@ public class RestconfDocumentedException extends RuntimeException {
     public List<RestconfError> getErrors() {
         return errors;
     }
-
-    public Status getStatus() {
-        return status;
-    }
 }
index fde05cfe31a647f299539a71580ac154e18ce404..ff3f94278cf0a93da1a9fe640055ba570112ed30 100644 (file)
@@ -212,29 +212,24 @@ public final class RestconfDocumentedExceptionMapper implements ExceptionMapper<
      * @return Derived status code.
      */
     private static Status getResponseStatusCode(final RestconfDocumentedException exception) {
-        final Status status = exception.getStatus();
-        if (status != null) {
-            // status code that is specified directly as field in exception has the precedence over error entries
-            return status;
-        }
-
-        final List<RestconfError> errors = exception.getErrors();
+        final var errors = exception.getErrors();
         if (errors.isEmpty()) {
             // if the module, that thrown exception, doesn't specify status code, it is treated as internal
             // server error
             return DEFAULT_STATUS_CODE;
         }
 
-        final Set<Status> allStatusCodesOfErrorEntries = errors.stream()
+        final var allStatusCodesOfErrorEntries = errors.stream()
                 .map(restconfError -> ErrorTags.statusOf(restconfError.getErrorTag()))
                 // we would like to preserve iteration order in collected entries - hence usage of LinkedHashSet
                 .collect(Collectors.toCollection(LinkedHashSet::new));
         // choosing of the first status code from appended errors, if there are different status codes in error
         // entries, we should create WARN message
         if (allStatusCodesOfErrorEntries.size() > 1) {
-            LOG.warn("An unexpected error occurred during translation of exception {} to response: "
-                    + "Different status codes have been found in appended error entries: {}. The first error "
-                    + "entry status code is chosen for response.", exception, allStatusCodesOfErrorEntries);
+            LOG.warn("""
+                An unexpected error occurred during translation of exception {} to response: Different status codes
+                have been found in appended error entries: {}. The first error entry status code is chosen for
+                response.""", exception, allStatusCodesOfErrorEntries);
         }
         return allStatusCodesOfErrorEntries.iterator().next();
     }
index 524da1c6f9d0e0cc0d868c14501621273f54d390..4813295a0c7c246e258ebc70b4eec7715ffa809d 100644 (file)
@@ -18,7 +18,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.util.Optional;
 import java.util.function.Function;
-import javax.ws.rs.core.Response.Status;
 import org.eclipse.jdt.annotation.NonNull;
 import org.junit.BeforeClass;
 import org.junit.function.ThrowingRunnable;
@@ -95,8 +94,6 @@ abstract class AbstractResourceBodyTest extends AbstractBodyTest {
 
     static final void assertRangeViolation(final ThrowingRunnable runnable) {
         final var ex = assertThrows(RestconfDocumentedException.class, runnable);
-        assertEquals(Status.BAD_REQUEST, ex.getStatus());
-
         final var errors = ex.getErrors();
         assertEquals(1, errors.size());
 
index 78f64857ea94f4a7f3a5826fad8247a7d93c5fb9..7218acfe8907d9f079d1674dae1553c88af7b04f 100644 (file)
@@ -43,9 +43,6 @@ import org.skyscreamer.jsonassert.JSONAssert;
 
 @RunWith(Parameterized.class)
 public class RestconfDocumentedExceptionMapperTest {
-
-    private static final String EMPTY_XML = "<errors xmlns=\"urn:ietf:params:xml:ns:yang:ietf-restconf\"></errors>";
-    private static final String EMPTY_JSON = "{}";
     private static final QNameModule MONITORING_MODULE_INFO = QNameModule.create(
         XMLNamespace.of("instance:identifier:patch:module"), Revision.of("2015-11-21"));
 
@@ -83,78 +80,6 @@ public class RestconfDocumentedExceptionMapperTest {
                     .build())));
 
         return Arrays.asList(new Object[][] {
-            {
-                "Mapping of the exception without any errors and XML output derived from content type",
-                new RestconfDocumentedException(Status.BAD_REQUEST),
-                mockHttpHeaders(MediaType.APPLICATION_XML_TYPE, List.of()),
-                Response.status(Status.BAD_REQUEST)
-                        .type(MediaTypes.APPLICATION_YANG_DATA_XML_TYPE)
-                        .entity(EMPTY_XML)
-                        .build()
-            },
-            {
-                "Mapping of the exception without any errors and JSON output derived from unsupported content type",
-                new RestconfDocumentedException(Status.INTERNAL_SERVER_ERROR),
-                mockHttpHeaders(MediaType.APPLICATION_FORM_URLENCODED_TYPE, List.of()),
-                Response.status(Status.INTERNAL_SERVER_ERROR)
-                        .type(MediaTypes.APPLICATION_YANG_DATA_JSON_TYPE)
-                        .entity(EMPTY_JSON)
-                        .build()
-            },
-            {
-                "Mapping of the exception without any errors and JSON output derived from missing content type "
-                        + "and accepted media types",
-                new RestconfDocumentedException(Status.NOT_IMPLEMENTED),
-                mockHttpHeaders(null, List.of()),
-                Response.status(Status.NOT_IMPLEMENTED)
-                        .type(MediaTypes.APPLICATION_YANG_DATA_JSON_TYPE)
-                        .entity(EMPTY_JSON)
-                        .build()
-            },
-            {
-                "Mapping of the exception without any errors and JSON output derived from expected types - both JSON"
-                        + "and XML types are accepted, but server should prefer JSON format",
-                new RestconfDocumentedException(Status.INTERNAL_SERVER_ERROR),
-                mockHttpHeaders(MediaType.APPLICATION_JSON_TYPE, List.of(
-                        MediaType.APPLICATION_FORM_URLENCODED_TYPE, MediaType.APPLICATION_XML_TYPE,
-                        MediaType.APPLICATION_JSON_TYPE, MediaType.APPLICATION_OCTET_STREAM_TYPE)),
-                Response.status(Status.INTERNAL_SERVER_ERROR)
-                        .type(MediaTypes.APPLICATION_YANG_DATA_JSON_TYPE)
-                        .entity(EMPTY_JSON)
-                        .build()
-            },
-            {
-                "Mapping of the exception without any errors and JSON output derived from expected types - there"
-                        + "is only a wildcard type that should be mapped to default type",
-                new RestconfDocumentedException(Status.NOT_FOUND),
-                mockHttpHeaders(null, List.of(MediaType.WILDCARD_TYPE)),
-                Response.status(Status.NOT_FOUND)
-                        .type(MediaTypes.APPLICATION_YANG_DATA_JSON_TYPE)
-                        .entity(EMPTY_JSON)
-                        .build()
-            },
-            {
-                "Mapping of the exception without any errors and XML output derived from expected types - "
-                        + "we should choose the most specific and supported type",
-                new RestconfDocumentedException(Status.NOT_FOUND),
-                mockHttpHeaders(null, List.of(MediaType.valueOf("*/yang-data+json"),
-                        MediaType.valueOf("application/yang-data+xml"), MediaType.WILDCARD_TYPE)),
-                Response.status(Status.NOT_FOUND)
-                        .type(MediaTypes.APPLICATION_YANG_DATA_XML_TYPE)
-                        .entity(EMPTY_XML)
-                        .build()
-            },
-            {
-                "Mapping of the exception without any errors and XML output derived from expected types - "
-                        + "we should choose the most specific and supported type",
-                new RestconfDocumentedException(Status.NOT_FOUND),
-                mockHttpHeaders(null, List.of(MediaType.valueOf("*/unsupported"),
-                        MediaType.valueOf("application/*"), MediaType.WILDCARD_TYPE)),
-                Response.status(Status.NOT_FOUND)
-                        .type(MediaTypes.APPLICATION_YANG_DATA_JSON_TYPE)
-                        .entity(EMPTY_JSON)
-                        .build()
-            },
             {
                 "Mapping of the exception with one error entry but null status code. This status code should"
                         + "be derived from single error entry; JSON output",
@@ -162,17 +87,18 @@ public class RestconfDocumentedExceptionMapperTest {
                 mockHttpHeaders(MediaType.APPLICATION_JSON_TYPE, List.of(MediaTypes.APPLICATION_YANG_PATCH_JSON_TYPE)),
                 Response.status(Status.INTERNAL_SERVER_ERROR)
                         .type(MediaTypes.APPLICATION_YANG_DATA_JSON_TYPE)
-                        .entity("{\n"
-                                + "  \"errors\": {\n"
-                                + "    \"error\": [\n"
-                                + "      {\n"
-                                + "        \"error-tag\": \"operation-failed\",\n"
-                                + "        \"error-message\": \"Sample error message\",\n"
-                                + "        \"error-type\": \"application\"\n"
-                                + "      }\n"
-                                + "    ]\n"
-                                + "  }\n"
-                                + "}")
+                        .entity("""
+                            {
+                              "errors": {
+                                "error": [
+                                  {
+                                    "error-tag": "operation-failed",
+                                    "error-message": "Sample error message",
+                                    "error-type": "application"
+                                  }
+                                ]
+                              }
+                            }""")
                         .build()
             },
             {
@@ -184,18 +110,19 @@ public class RestconfDocumentedExceptionMapperTest {
                 mockHttpHeaders(MediaType.APPLICATION_JSON_TYPE, List.of(MediaTypes.APPLICATION_YANG_PATCH_XML_TYPE)),
                 Response.status(Status.BAD_REQUEST)
                         .type(MediaTypes.APPLICATION_YANG_DATA_XML_TYPE)
-                        .entity("<errors xmlns=\"urn:ietf:params:xml:ns:yang:ietf-restconf\">\n"
-                                + "<error>\n"
-                                + "<error-message>message 1</error-message>\n"
-                                + "<error-tag>bad-attribute</error-tag>\n"
-                                + "<error-type>application</error-type>\n"
-                                + "</error>\n"
-                                + "<error>\n"
-                                + "<error-message>message 2</error-message>\n"
-                                + "<error-tag>operation-failed</error-tag>\n"
-                                + "<error-type>application</error-type>\n"
-                                + "</error>\n"
-                                + "</errors>")
+                        .entity("""
+                            <errors xmlns="urn:ietf:params:xml:ns:yang:ietf-restconf">
+                              <error>
+                                <error-message>message 1</error-message>
+                                <error-tag>bad-attribute</error-tag>
+                                <error-type>application</error-type>
+                              </error>
+                              <error>
+                                <error-message>message 2</error-message>
+                                <error-tag>operation-failed</error-tag>
+                                <error-type>application</error-type>
+                              </error>
+                            </errors>""")
                         .build()
             },
             {
@@ -205,34 +132,35 @@ public class RestconfDocumentedExceptionMapperTest {
                         MediaType.APPLICATION_JSON_TYPE)),
                 Response.status(Status.BAD_REQUEST)
                         .type(MediaTypes.APPLICATION_YANG_DATA_JSON_TYPE)
-                        .entity("{\n"
-                                + "  \"errors\": {\n"
-                                + "    \"error\": [\n"
-                                + "      {\n"
-                                + "        \"error-tag\": \"bad-attribute\",\n"
-                                + "        \"error-app-tag\": \"app tag #1\",\n"
-                                + "        \"error-message\": \"message 1\",\n"
-                                + "        \"error-type\": \"application\"\n"
-                                + "      },\n"
-                                + "      {\n"
-                                + "        \"error-tag\": \"operation-failed\",\n"
-                                + "        \"error-app-tag\": \"app tag #2\",\n"
-                                + "        \"error-info\": \"my info\",\n"
-                                + "        \"error-message\": \"message 2\",\n"
-                                + "        \"error-type\": \"application\"\n"
-                                + "      },\n"
-                                + "      {\n"
-                                + "        \"error-tag\": \"data-missing\",\n"
-                                + "        \"error-app-tag\": \" app tag #3\",\n"
-                                + "        \"error-info\": \"my error info\",\n"
-                                + "        \"error-message\": \"message 3\",\n"
-                                + "        \"error-path\": \"/instance-identifier-patch-module:patch-cont/"
-                                + "my-list1[name='sample']/my-leaf12\",\n"
-                                + "        \"error-type\": \"rpc\"\n"
-                                + "      }\n"
-                                + "    ]\n"
-                                + "  }\n"
-                                + "}")
+                        .entity("""
+                            {
+                              "errors": {
+                                "error": [
+                                  {
+                                    "error-tag": "bad-attribute",
+                                    "error-app-tag": "app tag #1",
+                                    "error-message": "message 1",
+                                    "error-type": "application"
+                                  },
+                                  {
+                                    "error-tag": "operation-failed",
+                                    "error-app-tag": "app tag #2",
+                                    "error-info": "my info",
+                                    "error-message": "message 2",
+                                    "error-type": "application"
+                                  },
+                                  {
+                                    "error-tag": "data-missing",
+                                    "error-app-tag": " app tag #3",
+                                    "error-info": "my error info",
+                                    "error-message": "message 3",
+                                    "error-path": "/instance-identifier-patch-module:patch-cont/\
+                            my-list1[name='sample']/my-leaf12",
+                                    "error-type": "rpc"
+                                  }
+                                ]
+                              }
+                            }""")
                         .build()
             },
             {
@@ -242,29 +170,30 @@ public class RestconfDocumentedExceptionMapperTest {
                         List.of(MediaTypes.APPLICATION_YANG_DATA_XML_TYPE)),
                 Response.status(Status.BAD_REQUEST)
                         .type(MediaTypes.APPLICATION_YANG_DATA_XML_TYPE)
-                        .entity("<errors xmlns=\"urn:ietf:params:xml:ns:yang:ietf-restconf\">\n"
-                                + "<error>\n"
-                                + "<error-type>application</error-type>\n"
-                                + "<error-message>message 1</error-message>\n"
-                                + "<error-tag>bad-attribute</error-tag>\n"
-                                + "<error-app-tag>app tag #1</error-app-tag>\n"
-                                + "</error>\n"
-                                + "<error>\n"
-                                + "<error-type>application</error-type>\n"
-                                + "<error-message>message 2</error-message>\n"
-                                + "<error-tag>operation-failed</error-tag>\n"
-                                + "<error-app-tag>app tag #2</error-app-tag>\n"
-                                + "<error-info>my info</error-info></error>\n"
-                                + "<error>\n"
-                                + "<error-type>rpc</error-type>\n"
-                                + "<error-path xmlns:a=\"instance:identifier:patch:module\">/a:patch-cont/"
-                                + "a:my-list1[a:name='sample']/a:my-leaf12</error-path>\n"
-                                + "<error-message>message 3</error-message>\n"
-                                + "<error-tag>data-missing</error-tag>\n"
-                                + "<error-app-tag> app tag #3</error-app-tag>\n"
-                                + "<error-info>my error info</error-info>\n"
-                                + "</error>\n"
-                                + "</errors>")
+                        .entity("""
+                            <errors xmlns="urn:ietf:params:xml:ns:yang:ietf-restconf">
+                              <error>
+                                <error-type>application</error-type>
+                                <error-message>message 1</error-message>
+                                <error-tag>bad-attribute</error-tag>
+                                <error-app-tag>app tag #1</error-app-tag>
+                              </error>
+                              <error>
+                                <error-type>application</error-type>
+                                <error-message>message 2</error-message>
+                                <error-tag>operation-failed</error-tag>
+                                <error-app-tag>app tag #2</error-app-tag>
+                                <error-info>my info</error-info></error>
+                              <error>
+                                <error-type>rpc</error-type>
+                                <error-path xmlns:a="instance:identifier:patch:module">/a:patch-cont/\
+                            a:my-list1[a:name='sample']/a:my-leaf12</error-path>
+                                <error-message>message 3</error-message>
+                                <error-tag>data-missing</error-tag>
+                                <error-app-tag> app tag #3</error-app-tag>
+                                <error-info>my error info</error-info>
+                              </error>
+                            </errors>""")
                         .build()
             }
         });