Fixing issue in event subscription error handling (#3)

Set custom status and error response to use in event subscription error handling

open-banking-accelerator/components/event-notifications/com.wso2.openbanking.accelerator.event.notifications.service/src/main/java/com/wso2/openbanking/accelerator/event/notifications/service/constants/EventNotificationConstants.java

@@ -53,6 +53,7 @@ public class EventNotificationConstants {
     public static final String REQUEST = "REQUEST";
 
     //Error Constants
+    public static final String INVALID_REQUEST = "invalid_request";
     public static final String EVENT_NOTIFICATION_CREATION_ERROR = "Error occurred while saving event " +
             "notifications in the  database";
     public static final String MISSING_REQ_PAYLOAD = "No request payload found";
@@ -112,4 +113,6 @@ public class EventNotificationConstants {
             "subscription in the database. ";
     public static final String EVENT_SUBSCRIPTION_NOT_FOUND = "Event subscription not found.";
     public static final String EVENT_SUBSCRIPTIONS_NOT_FOUND = "Event subscriptions not found for the given client id.";
+    public static final String ERROR_HANDLING_EVENT_SUBSCRIPTION = "Error occurred while handling the event " +
+            "subscription  request";
 }

open-banking-accelerator/internal-apis/internal-webapps/com.wso2.openbanking.accelerator.event.notifications.endpoint/src/main/java/com/wso2/openbanking/accelerator/event/notifications/endpoint/dto/EventNotificationErrorDTO.java → open-banking-accelerator/components/event-notifications/com.wso2.openbanking.accelerator.event.notifications.service/src/main/java/com/wso2/openbanking/accelerator/event/notifications/service/dto/EventNotificationErrorDTO.java

@@ -16,7 +16,7 @@
  * under the License.
  */
 
-package com.wso2.openbanking.accelerator.event.notifications.endpoint.dto;
+package com.wso2.openbanking.accelerator.event.notifications.service.dto;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 
@@ -29,7 +29,7 @@ public class EventNotificationErrorDTO {
     private String error;
 
     @JsonProperty("error_description")
-    public String getError_description() {
+    public String getErrorDescription() {
 
         return errorDescription;
     }

open-banking-accelerator/components/event-notifications/com.wso2.openbanking.accelerator.event.notifications.service/src/main/java/com/wso2/openbanking/accelerator/event/notifications/service/response/EventSubscriptionResponse.java

@@ -23,15 +23,15 @@
  */
 public class EventSubscriptionResponse {
 
-    private String status;
+    private int status;
     private Object responseBody;
-    private String errorResponse;
+    private Object errorResponse;
 
-    public String getStatus() {
+    public int getStatus() {
         return status;
     }
 
-    public void setStatus(String status) {
+    public void setStatus(int status) {
         this.status = status;
     }
 
@@ -43,11 +43,11 @@ public void setResponseBody(Object responseBody) {
         this.responseBody = responseBody;
     }
 
-    public String getErrorResponse() {
+    public Object getErrorResponse() {
         return errorResponse;
     }
 
-    public void setErrorResponse(String errorResponse) {
+    public void setErrorResponse(Object errorResponse) {
         this.errorResponse = errorResponse;
     }
 

open-banking-accelerator/components/event-notifications/com.wso2.openbanking.accelerator.event.notifications.service/src/main/java/com/wso2/openbanking/accelerator/event/notifications/service/util/EventNotificationServiceUtil.java

@@ -24,6 +24,7 @@
 import com.wso2.openbanking.accelerator.common.util.OpenBankingUtils;
 import com.wso2.openbanking.accelerator.consent.mgt.service.impl.ConsentCoreServiceImpl;
 import com.wso2.openbanking.accelerator.event.notifications.service.constants.EventNotificationConstants;
+import com.wso2.openbanking.accelerator.event.notifications.service.dto.EventNotificationErrorDTO;
 import com.wso2.openbanking.accelerator.event.notifications.service.exceptions.OBEventNotificationException;
 import com.wso2.openbanking.accelerator.event.notifications.service.handler.DefaultEventCreationServiceHandler;
 import com.wso2.openbanking.accelerator.event.notifications.service.realtime.service.RealtimeEventNotificationRequestGenerator;
@@ -77,6 +78,8 @@ public static RealtimeEventNotificationRequestGenerator getRealtimeEventNotifica
 
     /**
      * Method to modify event notification payload with custom eventValues.
+     *
+     * @param jsonNode Json Node to convert
      * @return String eventNotificationPayload
      */
     public static String getCustomNotificationPayload(JsonNode jsonNode) {
@@ -87,8 +90,8 @@ public static String getCustomNotificationPayload(JsonNode jsonNode) {
 
     /**
      * Method to get event JSON from eventInformation payload string.
-     * @param eventInformation
-     * @return
+     * @param eventInformation String event Information
+     * @return JSONObject converted event json
      * @throws ParseException
      */
     public static JSONObject getEventJSONFromString(String eventInformation) throws ParseException {
@@ -99,7 +102,7 @@ public static JSONObject getEventJSONFromString(String eventInformation) throws
 
     /**
      * Validate if the client ID is existing.
-     * @param clientId
+     * @param clientId  client ID of the TPP
      * @throws OBEventNotificationException
      */
     @Generated(message = "Excluded since this needs OAuth2Util service provider")
@@ -151,4 +154,18 @@ public static String getCallbackURL(String clientID) {
     public static DefaultEventCreationServiceHandler getDefaultEventCreationServiceHandler() {
         return new DefaultEventCreationServiceHandler();
     }
+
+    /**
+     * Method to map Event subscription Service error to API response.
+     *
+     * @param error             Error code
+     * @param errorDescription  Error description
+     * @return EventNotificationErrorDTO
+     */
+    public static EventNotificationErrorDTO getErrorDTO(String error, String errorDescription) {
+        EventNotificationErrorDTO eventNotificationErrorDTO = new EventNotificationErrorDTO();
+        eventNotificationErrorDTO.setError(error);
+        eventNotificationErrorDTO.setErrorDescription(errorDescription);
+        return eventNotificationErrorDTO;
+    }
 }

open-banking-accelerator/components/event-notifications/com.wso2.openbanking.accelerator.event.notifications.service/src/test/java/com/wso2/openbanking/accelerator/event/notifications/service/handler/DefaultEventSubscriptionServiceHandlerTests.java

@@ -26,6 +26,7 @@
 import com.wso2.openbanking.accelerator.event.notifications.service.util.EventNotificationServiceUtil;
 import com.wso2.openbanking.accelerator.event.notifications.service.utils.EventNotificationTestUtils;
 import net.minidev.json.JSONObject;
+import org.eclipse.jetty.http.HttpStatus;
 import org.mockito.Mockito;
 import org.powermock.core.classloader.annotations.PowerMockIgnore;
 import org.powermock.core.classloader.annotations.PrepareForTest;
@@ -73,7 +74,7 @@ public void testCreateEventSubscription() throws Exception {
         EventSubscriptionResponse eventSubscriptionCreationResponse = defaultEventSubscriptionServiceHandler
                 .createEventSubscription(EventNotificationTestUtils.getSampleEventSubscriptionDTO());
 
-        Assert.assertEquals(eventSubscriptionCreationResponse.getStatus(), EventNotificationConstants.CREATED);
+        Assert.assertEquals(eventSubscriptionCreationResponse.getStatus(), HttpStatus.CREATED_201);
     }
 
     @Test
@@ -92,7 +93,7 @@ public void testCreateEventSubscriptionServiceError() throws Exception {
                 .createEventSubscription(EventNotificationTestUtils.getSampleEventSubscriptionDTO());
 
         Assert.assertEquals(eventSubscriptionCreationResponse.getStatus(),
-                EventNotificationConstants.INTERNAL_SERVER_ERROR);
+                HttpStatus.INTERNAL_SERVER_ERROR_500);
     }
 
     @Test
@@ -110,7 +111,7 @@ public void testGetEventSubscription() throws Exception {
                 .getEventSubscription(EventNotificationTestConstants.SAMPLE_CLIENT_ID,
                         EventNotificationTestConstants.SAMPLE_SUBSCRIPTION_ID_1);
 
-        Assert.assertEquals(EventNotificationConstants.OK, eventSubscriptionRetrieveResponse.getStatus());
+        Assert.assertEquals(eventSubscriptionRetrieveResponse.getStatus(), HttpStatus.OK_200);
     }
 
     @Test
@@ -129,8 +130,7 @@ public void testGetEventSubscriptionServiceError() throws Exception {
                 .getEventSubscription(EventNotificationTestConstants.SAMPLE_CLIENT_ID,
                         EventNotificationTestConstants.SAMPLE_SUBSCRIPTION_ID_1);
 
-        Assert.assertEquals(EventNotificationConstants.INTERNAL_SERVER_ERROR,
-                eventSubscriptionRetrieveResponse.getStatus());
+        Assert.assertEquals(eventSubscriptionRetrieveResponse.getStatus(), HttpStatus.INTERNAL_SERVER_ERROR_500);
     }
 
     @Test
@@ -147,7 +147,7 @@ public void testGetAllEventSubscriptions() throws Exception {
         EventSubscriptionResponse eventSubscriptionRetrieveResponse = defaultEventSubscriptionServiceHandler
                 .getAllEventSubscriptions(EventNotificationTestConstants.SAMPLE_CLIENT_ID);
 
-        Assert.assertEquals(EventNotificationConstants.OK, eventSubscriptionRetrieveResponse.getStatus());
+        Assert.assertEquals(eventSubscriptionRetrieveResponse.getStatus(), HttpStatus.OK_200);
     }
 
     @Test
@@ -165,8 +165,7 @@ public void testGetAllEventSubscriptionsServiceError() throws Exception {
         EventSubscriptionResponse eventSubscriptionRetrieveResponse = defaultEventSubscriptionServiceHandler
                 .getAllEventSubscriptions(EventNotificationTestConstants.SAMPLE_CLIENT_ID);
 
-        Assert.assertEquals(EventNotificationConstants.INTERNAL_SERVER_ERROR,
-                eventSubscriptionRetrieveResponse.getStatus());
+        Assert.assertEquals(eventSubscriptionRetrieveResponse.getStatus(), HttpStatus.INTERNAL_SERVER_ERROR_500);
     }
 
     @Test
@@ -184,7 +183,7 @@ public void testGetEventSubscriptionsByEventType() throws Exception {
                 .getEventSubscriptionsByEventType(EventNotificationTestConstants.SAMPLE_CLIENT_ID,
                         EventNotificationTestConstants.SAMPLE_NOTIFICATION_EVENT_TYPE_1);
 
-        Assert.assertEquals(EventNotificationConstants.OK, eventSubscriptionRetrieveResponse.getStatus());
+        Assert.assertEquals(eventSubscriptionRetrieveResponse.getStatus(), HttpStatus.OK_200);
     }
 
     @Test
@@ -203,8 +202,7 @@ public void testGetEventSubscriptionsByEventTypeServiceError() throws Exception
                 .getEventSubscriptionsByEventType(EventNotificationTestConstants.SAMPLE_CLIENT_ID,
                         EventNotificationTestConstants.SAMPLE_NOTIFICATION_EVENT_TYPE_1);
 
-        Assert.assertEquals(EventNotificationConstants.INTERNAL_SERVER_ERROR,
-                eventSubscriptionRetrieveResponse.getStatus());
+        Assert.assertEquals(eventSubscriptionRetrieveResponse.getStatus(), HttpStatus.INTERNAL_SERVER_ERROR_500);
     }
 
     @Test
@@ -223,7 +221,7 @@ public void testUpdateEventSubscription() throws Exception {
         EventSubscriptionResponse eventSubscriptionUpdateResponse = defaultEventSubscriptionServiceHandler
                 .updateEventSubscription(EventNotificationTestUtils.getSampleEventSubscriptionUpdateDTO());
 
-        Assert.assertEquals(EventNotificationConstants.OK, eventSubscriptionUpdateResponse.getStatus());
+        Assert.assertEquals(eventSubscriptionUpdateResponse.getStatus(), HttpStatus.OK_200);
     }
 
     @Test
@@ -243,8 +241,7 @@ public void testUpdateEventSubscriptionServiceError() throws Exception {
         EventSubscriptionResponse eventSubscriptionUpdateResponse = defaultEventSubscriptionServiceHandler
                 .updateEventSubscription(EventNotificationTestUtils.getSampleEventSubscriptionUpdateDTO());
 
-        Assert.assertEquals(EventNotificationConstants.INTERNAL_SERVER_ERROR,
-                eventSubscriptionUpdateResponse.getStatus());
+        Assert.assertEquals(eventSubscriptionUpdateResponse.getStatus(), HttpStatus.INTERNAL_SERVER_ERROR_500);
     }
 
     @Test
@@ -262,7 +259,7 @@ public void testDeleteEventSubscription() throws Exception {
                 .deleteEventSubscription(EventNotificationTestConstants.SAMPLE_CLIENT_ID,
                         EventNotificationTestConstants.SAMPLE_SUBSCRIPTION_ID_1);
 
-        Assert.assertEquals(EventNotificationConstants.NO_CONTENT, eventSubscriptionDeletionResponse.getStatus());
+        Assert.assertEquals(eventSubscriptionDeletionResponse.getStatus(), HttpStatus.NO_CONTENT_204);
     }
 
     @Test
@@ -281,8 +278,7 @@ public void testDeleteEventSubscriptionServiceError() throws Exception {
                 .deleteEventSubscription(EventNotificationTestConstants.SAMPLE_CLIENT_ID,
                         EventNotificationTestConstants.SAMPLE_SUBSCRIPTION_ID_1);
 
-        Assert.assertEquals(EventNotificationConstants.INTERNAL_SERVER_ERROR,
-                eventSubscriptionDeletionResponse.getStatus());
+        Assert.assertEquals(eventSubscriptionDeletionResponse.getStatus(), HttpStatus.INTERNAL_SERVER_ERROR_500);
     }
 
     @Test

open-banking-accelerator/internal-apis/internal-webapps/com.wso2.openbanking.accelerator.event.notifications.endpoint/src/main/java/com/wso2/openbanking/accelerator/event/notifications/endpoint/util/EventNotificationUtils.java

@@ -22,8 +22,8 @@
 import com.wso2.openbanking.accelerator.common.constant.OpenBankingConstants;
 import com.wso2.openbanking.accelerator.common.util.OpenBankingUtils;
 import com.wso2.openbanking.accelerator.event.notifications.endpoint.constants.EventNotificationEndPointConstants;
-import com.wso2.openbanking.accelerator.event.notifications.endpoint.dto.EventNotificationErrorDTO;
 import com.wso2.openbanking.accelerator.event.notifications.service.constants.EventNotificationConstants;
+import com.wso2.openbanking.accelerator.event.notifications.service.dto.EventNotificationErrorDTO;
 import com.wso2.openbanking.accelerator.event.notifications.service.handler.EventCreationServiceHandler;
 import com.wso2.openbanking.accelerator.event.notifications.service.handler.EventPollingServiceHandler;
 import com.wso2.openbanking.accelerator.event.notifications.service.response.EventCreationResponse;

open-banking-accelerator/internal-apis/internal-webapps/com.wso2.openbanking.accelerator.event.notifications.endpoint/src/main/java/com/wso2/openbanking/accelerator/event/notifications/endpoint/util/EventSubscriptionUtils.java

@@ -21,14 +21,15 @@
 import com.wso2.openbanking.accelerator.common.config.OpenBankingConfigParser;
 import com.wso2.openbanking.accelerator.common.constant.OpenBankingConstants;
 import com.wso2.openbanking.accelerator.common.util.OpenBankingUtils;
-import com.wso2.openbanking.accelerator.event.notifications.endpoint.constants.EventNotificationEndPointConstants;
 import com.wso2.openbanking.accelerator.event.notifications.service.constants.EventNotificationConstants;
 import com.wso2.openbanking.accelerator.event.notifications.service.handler.EventSubscriptionServiceHandler;
 import com.wso2.openbanking.accelerator.event.notifications.service.response.EventSubscriptionResponse;
+import com.wso2.openbanking.accelerator.event.notifications.service.util.EventNotificationServiceUtil;
 import net.minidev.json.JSONObject;
 import net.minidev.json.parser.JSONParser;
 import net.minidev.json.parser.ParseException;
 import org.apache.commons.io.IOUtils;
+import org.eclipse.jetty.http.HttpStatus;
 
 import java.io.IOException;
 
@@ -74,34 +75,23 @@ public static JSONObject getJSONObjectPayload(HttpServletRequest request) throws
      * @return Response
      */
     public static Response mapEventSubscriptionServiceResponse(EventSubscriptionResponse eventSubscriptionResponse) {
-        String status = eventSubscriptionResponse.getStatus();
+        int status = eventSubscriptionResponse.getStatus();
         if (eventSubscriptionResponse.getErrorResponse() == null) {
-            switch (status) {
-                case EventNotificationConstants.CREATED:
-                    return Response.status(Response.Status.CREATED)
-                            .entity(eventSubscriptionResponse.getResponseBody())
-                            .build();
-                case EventNotificationConstants.OK:
-                    return Response.status(Response.Status.OK)
-                            .entity(eventSubscriptionResponse.getResponseBody())
-                            .build();
-                case EventNotificationConstants.NO_CONTENT:
-                    return Response.status(Response.Status.NO_CONTENT)
-                            .entity(eventSubscriptionResponse.getResponseBody())
-                            .build();
+            if (eventSubscriptionResponse.getResponseBody() != null) {
+                return Response.status(status)
+                        .entity(eventSubscriptionResponse.getResponseBody())
+                        .build();
+            } else {
+                return Response.status(HttpStatus.INTERNAL_SERVER_ERROR_500)
+                        .entity(EventNotificationServiceUtil.getErrorDTO(EventNotificationConstants.INVALID_REQUEST,
+                                EventNotificationConstants.ERROR_HANDLING_EVENT_SUBSCRIPTION))
+                        .build();
             }
-        } else if (status.equals(EventNotificationConstants.BAD_REQUEST)) {
-            return Response.status(Response.Status.BAD_REQUEST)
-                    .entity(EventNotificationUtils.getErrorDTO(
-                            EventNotificationEndPointConstants.INVALID_REQUEST,
-                            eventSubscriptionResponse.getErrorResponse()))
+        } else  {
+            return Response.status(status)
+                    .entity(eventSubscriptionResponse.getErrorResponse())
                     .build();
         }
-        return Response.status(Response.Status.INTERNAL_SERVER_ERROR)
-                .entity(EventNotificationUtils.getErrorDTO(
-                        "Internal Server Error",
-                        eventSubscriptionResponse.getErrorResponse()))
-                .build();
 
     }
 }