Validate Http Response

src/main/java/io/cdap/plugin/servicenow/ServiceNowBaseConfig.java

@@ -34,6 +34,7 @@
 import org.apache.oltu.oauth2.common.exception.OAuthProblemException;
 import org.apache.oltu.oauth2.common.exception.OAuthSystemException;
 
+import java.io.IOException;
 import javax.annotation.Nullable;
 
 /**
@@ -137,24 +138,15 @@ public void validateTable(String tableName, SourceValueType valueType, FailureCo
       requestBuilder.setResponseHeaders(ServiceNowConstants.HEADER_NAME_TOTAL_COUNT);
 
       apiResponse = serviceNowTableAPIClient.executeGet(requestBuilder.build());
-      if (!apiResponse.isSuccess()) {
-        if (apiResponse.getHttpStatus() == HttpStatus.SC_BAD_REQUEST) {
-          collector.addFailure("Bad Request. Table: " + tableName + " is invalid.", "")
-            .withConfigProperty(tableField);
-        }
-      } else if (serviceNowTableAPIClient.parseResponseToResultListOfMap(apiResponse.getResponseBody()).isEmpty()) {
+      if (serviceNowTableAPIClient.parseResponseToResultListOfMap(apiResponse.getResponseBody()).isEmpty()) {
         // Removed config property as in case of MultiSource, only first table error was populating.
         collector.addFailure("Table: " + tableName + " is empty.", "");
       }
-    } catch (OAuthSystemException | OAuthProblemException e) {
-      collector.addFailure("Unable to connect to ServiceNow Instance.",
-                           "Ensure properties like Client ID, Client Secret, API Endpoint, User Name, Password " +
-                             "are correct.")
-        .withConfigProperty(ServiceNowConstants.PROPERTY_CLIENT_ID)
-        .withConfigProperty(ServiceNowConstants.PROPERTY_CLIENT_SECRET)
-        .withConfigProperty(ServiceNowConstants.PROPERTY_API_ENDPOINT)
-        .withConfigProperty(ServiceNowConstants.PROPERTY_USER)
-        .withConfigProperty(ServiceNowConstants.PROPERTY_PASSWORD);
+    } catch (Exception e) {
+      collector.addFailure(String.format("ServiceNow API returned an unexpected result or the specified table may " +
+                              "not exist. Cause: %s", e.getMessage()),
+                           "Ensure specified table exists in the datasource. ")
+        .withConfigProperty(ServiceNowConstants.PROPERTY_TABLE_NAME);
     }
   }
 

src/main/java/io/cdap/plugin/servicenow/apiclient/NonRetryableException.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright © 2022 Cask Data, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package io.cdap.plugin.servicenow.apiclient;
+
+/** Custom Exception Class for handling retrying API calls */
+public class NonRetryableException extends RuntimeException {
+
+  private static final long serialVersionUID = 1L;
+
+  public NonRetryableException() {
+    super();
+  }
+
+  public NonRetryableException(String message) {
+    super(message);
+  }
+
+  public NonRetryableException(String message, Throwable throwable) {
+    super(message, throwable);
+  }
+}

src/main/java/io/cdap/plugin/servicenow/apiclient/RetryableException.java

@@ -16,15 +16,20 @@
 
 package io.cdap.plugin.servicenow.apiclient;
 
-/**
- * Custom Exception Class for handling retrying API calls
- */
+/** Custom Exception Class for handling retrying API calls */
 public class RetryableException extends RuntimeException {
 
   private static final long serialVersionUID = 1L;
 
   public RetryableException() {
-         super();
+    super();
+  }
+
+  public RetryableException(String message) {
+    super(message);
+  }
+
+  public RetryableException(String message, Throwable throwable) {
+    super(message, throwable);
   }
-
 }

src/main/java/io/cdap/plugin/servicenow/apiclient/ServiceNowTableAPIClientImpl.java

@@ -51,7 +51,6 @@
 import java.io.UnsupportedEncodingException;
 import java.lang.reflect.Type;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Callable;
@@ -111,7 +110,7 @@ public String getAccessTokenRetryableMode() throws ExecutionException, RetryExce
    * @return The list of Map; each Map representing a table row
    */
   public List<Map<String, String>> fetchTableRecords(String tableName, SourceValueType valueType, String startDate,
-                                                     String endDate, int offset, int limit) {
+                                                     String endDate, int offset, int limit) throws IOException {
     ServiceNowTableAPIRequestBuilder requestBuilder = new ServiceNowTableAPIRequestBuilder(
       this.conf.getRestApiEndpoint(), tableName, false)
       .setExcludeReferenceLink(true)
@@ -124,25 +123,15 @@ public List<Map<String, String>> fetchTableRecords(String tableName, SourceValue
 
     applyDateRangeToRequest(requestBuilder, startDate, endDate);
 
-    RestAPIResponse apiResponse = null;
-
     try {
       String accessToken = getAccessToken();
       requestBuilder.setAuthHeader(accessToken);
-      apiResponse = executeGet(requestBuilder.build());
-      if (!apiResponse.isSuccess()) {
-        if (apiResponse.isRetryable()) {
-          throw new RetryableException();
-        }
-        return Collections.emptyList();
-      }
-
+      RestAPIResponse apiResponse = executeGet(requestBuilder.build());
       return parseResponseToResultListOfMap(apiResponse.getResponseBody());
     } catch (OAuthSystemException e) {
-      throw new RetryableException();
+      throw new RetryableException("Authentication error occurred", e);
     } catch (OAuthProblemException e) {
-      LOG.error("Error in fetchTableRecords", e);
-      return Collections.emptyList();
+      throw new IOException("Problem occurred while authenticating", e);
     }
   }
 
@@ -255,10 +244,10 @@ public Schema fetchTableSchema(String tableName, FailureCollector collector) {
     Schema schema = null;
     try {
       schema = fetchTableSchema(tableName);
-    } catch (OAuthProblemException | OAuthSystemException | RuntimeException e) {
-      LOG.error("Error in connection - {}", e.getMessage());
+    } catch (Exception e) {
+      LOG.error("Failed to fetch schema on table {}", tableName, e);
       collector.addFailure(String.format("Connection failed. Unable to fetch schema for table: %s. Cause: %s",
-                                         tableName, e.getStackTrace()), null);
+                                         tableName, e.getMessage()), null);
     }
     return schema;
   }
@@ -276,7 +265,8 @@ public SchemaResponse parseSchemaResponse(String responseBody) {
    * @throws OAuthProblemException
    * @throws OAuthSystemException
    */
-  public Schema fetchTableSchema(String tableName) throws OAuthProblemException, OAuthSystemException {
+  public Schema fetchTableSchema(String tableName)
+      throws OAuthProblemException, OAuthSystemException, IOException {
     ServiceNowTableAPIRequestBuilder requestBuilder = new ServiceNowTableAPIRequestBuilder(
       this.conf.getRestApiEndpoint(), tableName, true)
       .setExcludeReferenceLink(true);
@@ -285,9 +275,6 @@ public Schema fetchTableSchema(String tableName) throws OAuthProblemException, O
     String accessToken = getAccessToken();
     requestBuilder.setAuthHeader(accessToken);
     apiResponse = executeGet(requestBuilder.build());
-    if (!apiResponse.isSuccess()) {
-      throw new RuntimeException("Error - " + getErrorMessage(apiResponse.getResponseBody()));
-    }
     SchemaResponse response = parseSchemaResponse(apiResponse.getResponseBody());
     List<ServiceNowColumn> columns = new ArrayList<>();
 
@@ -308,7 +295,8 @@ public Schema fetchTableSchema(String tableName) throws OAuthProblemException, O
    * @throws OAuthProblemException
    * @throws OAuthSystemException
    */
-  public int getTableRecordCount(String tableName) throws OAuthProblemException, OAuthSystemException {
+  public int getTableRecordCount(String tableName)
+      throws OAuthProblemException, OAuthSystemException, IOException {
     ServiceNowTableAPIRequestBuilder requestBuilder = new ServiceNowTableAPIRequestBuilder(
       this.conf.getRestApiEndpoint(), tableName, false)
       .setExcludeReferenceLink(true)
@@ -319,9 +307,6 @@ public int getTableRecordCount(String tableName) throws OAuthProblemException, O
     requestBuilder.setResponseHeaders(ServiceNowConstants.HEADER_NAME_TOTAL_COUNT);
     requestBuilder.setAuthHeader(accessToken);
     apiResponse = executeGet(requestBuilder.build());
-    if (!apiResponse.isSuccess()) {
-      throw new RuntimeException("Error : " + apiResponse);
-    }
     return getRecordCountFromHeader(apiResponse);
   }
 
@@ -346,10 +331,6 @@ public String createRecord(String tableName, HttpEntity entity) throws IOExcepti
       apiResponse = executePost(requestBuilder.build());
 
       systemID = String.valueOf(getSystemId(apiResponse));
-
-      if (!apiResponse.isSuccess()) {
-        LOG.error("Error - {}", getErrorMessage(apiResponse.getResponseBody()));
-      }
     } catch (OAuthSystemException | OAuthProblemException | UnsupportedEncodingException e) {
       throw new IOException("Error in creating a new record", e);
     }
@@ -363,14 +344,14 @@ private String getSystemId(RestAPIResponse restAPIResponse) {
   }
 
   /**
-   * This function is being used in end-to-end (e2e) tests to fetch a record
-   * Return a record from ServiceNow application.
+   * This function is being used in end-to-end (e2e) tests to fetch a record Return a record from
+   * ServiceNow application.
    *
    * @param tableName The ServiceNow table name
-   * @param query     The query
+   * @param query The query
    */
   public Map<String, String> getRecordFromServiceNowTable(String tableName, String query)
-    throws OAuthProblemException, OAuthSystemException {
+      throws OAuthProblemException, OAuthSystemException, IOException {
 
     ServiceNowTableAPIRequestBuilder requestBuilder = new ServiceNowTableAPIRequestBuilder(
       this.conf.getRestApiEndpoint(), tableName, false)

src/main/java/io/cdap/plugin/servicenow/connector/ServiceNowConnector.java

@@ -125,16 +125,8 @@ private TableList listTables() throws IOException {
     requestBuilder.setAuthHeader(accessToken);
     requestBuilder.setAcceptHeader(MediaType.APPLICATION_JSON);
     requestBuilder.setContentTypeHeader(MediaType.APPLICATION_JSON);
-    RestAPIResponse apiResponse = null;
-    apiResponse = serviceNowTableAPIClient.executeGet(requestBuilder.build());
-    if (!apiResponse.isSuccess()) {
-      LOG.error("Error - {}", getErrorMessage(apiResponse.getResponseBody()));
-      throw new IOException(getErrorMessage(apiResponse.getResponseBody()));
-    } else {
-      String response = null;
-      response = apiResponse.getResponseBody();
-      return GSON.fromJson(response, TableList.class);
-    }
+    RestAPIResponse apiResponse = serviceNowTableAPIClient.executeGet(requestBuilder.build());
+    return GSON.fromJson(apiResponse.getResponseBody(), TableList.class);
   }
 
   public ConnectorSpec generateSpec(ConnectorContext connectorContext, ConnectorSpecRequest connectorSpecRequest) {
@@ -156,16 +148,6 @@ public ConnectorSpec generateSpec(ConnectorContext connectorContext, ConnectorSp
       .addRelatedPlugin(new PluginSpec(ServiceNowConstants.PLUGIN_NAME, BatchSink.PLUGIN_TYPE, properties)).build();
   }
 
-
-  private String getErrorMessage(String responseBody) {
-    try {
-      JsonObject jo = GSON.fromJson(responseBody, JsonObject.class);
-      return jo.getAsJsonObject(ServiceNowConstants.ERROR).get(ServiceNowConstants.MESSAGE).getAsString();
-    } catch (Exception e) {
-      return e.getMessage();
-    }
-  }
-
   @Override
   public List<StructuredRecord> sample(ConnectorContext connectorContext, SampleRequest sampleRequest)
     throws IOException {
@@ -180,19 +162,18 @@ public List<StructuredRecord> sample(ConnectorContext connectorContext, SampleRe
     }
   }
 
-  private List<StructuredRecord> getTableData(String tableName, int limit) throws OAuthProblemException,
-    OAuthSystemException {
+  private List<StructuredRecord> getTableData(String tableName, int limit)
+      throws OAuthProblemException, OAuthSystemException, IOException {
     ServiceNowTableAPIRequestBuilder requestBuilder = new ServiceNowTableAPIRequestBuilder(
       config.getRestApiEndpoint(), tableName, false)
       .setExcludeReferenceLink(true)
       .setDisplayValue(SourceValueType.SHOW_DISPLAY_VALUE)
       .setLimit(limit);
-    RestAPIResponse apiResponse = null;
     ServiceNowTableAPIClientImpl serviceNowTableAPIClient = new ServiceNowTableAPIClientImpl(config);
     String accessToken = serviceNowTableAPIClient.getAccessToken();
     requestBuilder.setAuthHeader(accessToken);
     requestBuilder.setResponseHeaders(ServiceNowConstants.HEADER_NAME_TOTAL_COUNT);
-    apiResponse = serviceNowTableAPIClient.executeGet(requestBuilder.build());
+    RestAPIResponse apiResponse = serviceNowTableAPIClient.executeGet(requestBuilder.build());
     List<Map<String, String>> result = serviceNowTableAPIClient.parseResponseToResultListOfMap
       (apiResponse.getResponseBody());
     List<StructuredRecord> recordList = new ArrayList<>();

src/main/java/io/cdap/plugin/servicenow/restapi/RestAPIClient.java

@@ -16,6 +16,11 @@
 
 package io.cdap.plugin.servicenow.restapi;
 
+import com.jcraft.jsch.IO;
+import io.cdap.plugin.servicenow.apiclient.NonRetryableException;
+import io.cdap.plugin.servicenow.apiclient.RetryableException;
+import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
@@ -33,32 +38,33 @@
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
 
 /**
  * An abstract class to call Rest API.
  */
 public abstract class RestAPIClient {
   private static final Logger LOG = LoggerFactory.getLogger(RestAPIClient.class);
+
   /**
    * Executes the Rest API request and returns the response.
    *
    * @param request the Rest API request
    * @return an instance of RestAPIResponse object.
    */
-  public RestAPIResponse executeGet(RestAPIRequest request) {
+  public RestAPIResponse executeGet(RestAPIRequest request) throws IOException {
     HttpGet httpGet = new HttpGet(request.getUrl());
     request.getHeaders().entrySet().forEach(e -> httpGet.addHeader(e.getKey(), e.getValue()));
-    RestAPIResponse apiResponse = null;
 
     try (CloseableHttpClient httpClient = HttpClientBuilder.create().build()) {
       try (CloseableHttpResponse httpResponse = httpClient.execute(httpGet)) {
-        apiResponse = RestAPIResponse.parse(httpResponse, request.getResponseHeaders());
+        return RestAPIResponse.parse(httpResponse, request.getResponseHeaders());
       }
-    } catch (Exception e) {
-      apiResponse = RestAPIResponse.defaultErrorResponse(e.getMessage());
     }
-
-    return apiResponse;
   }
 
   /**
@@ -71,23 +77,15 @@ public RestAPIResponse executePost(RestAPIRequest request) throws IOException {
     HttpPost httpPost = new HttpPost(request.getUrl());
     request.getHeaders().entrySet().forEach(e -> httpPost.addHeader(e.getKey(), e.getValue()));
     httpPost.setEntity(request.getEntity());
-    RestAPIResponse apiResponse;
 
+    // We're retrying all transport exceptions while executing the HTTP POST method and the generic transport
+    // exceptions in HttpClient are represented by the standard java.io.IOException class
+    // https://hc.apache.org/httpclient-legacy/exception-handling.html
     try (CloseableHttpClient httpClient = HttpClientBuilder.create().build()) {
       try (CloseableHttpResponse httpResponse = httpClient.execute(httpPost)) {
-        apiResponse = RestAPIResponse.parse(httpResponse, request.getResponseHeaders());
+        return RestAPIResponse.parse(httpResponse, request.getResponseHeaders());
       }
-    } catch (IOException e) {
-      // We're retrying all transport exceptions while executing the HTTP POST method and the generic transport
-      // exceptions in HttpClient are represented by the standard java.io.IOException class
-      // https://hc.apache.org/httpclient-legacy/exception-handling.html
-      throw e;
-    } catch (Exception e) {
-      LOG.error("Exception while executing post request", e);
-      apiResponse = RestAPIResponse.defaultErrorResponse(e.getMessage());
     }
-
-    return apiResponse;
   }
   /**
    * Generates access token and returns the same.