Cherry-picks for bug fixes and refactoring changes

src/e2e-test/java/io/cdap/plugin/servicenowsink/actions/ServiceNowSinkPropertiesPageActions.java

@@ -50,15 +50,15 @@ public class ServiceNowSinkPropertiesPageActions {
   private static Gson gson = new Gson();
 
   public static void getRecordFromServiceNowTable(String query, String tableName)
-    throws OAuthProblemException, OAuthSystemException {
+    throws OAuthProblemException, OAuthSystemException, IOException {
     config = new ServiceNowSourceConfig(
       "", "", "", "", "",
       System.getenv("SERVICE_NOW_CLIENT_ID"),
       System.getenv("SERVICE_NOW_CLIENT_SECRET"),
       System.getenv("SERVICE_NOW_REST_API_ENDPOINT"),
       System.getenv("SERVICE_NOW_USERNAME"),
       System.getenv("SERVICE_NOW_PASSWORD"),
-      "", "", "");
+      "", "", "", null);
 
     ServiceNowTableAPIClientImpl tableAPIClient = new ServiceNowTableAPIClientImpl(config.getConnection());
     responseFromServiceNowTable = tableAPIClient.getRecordFromServiceNowTable(tableName, query);

src/e2e-test/java/io/cdap/plugin/tests/hooks/TestSetupHooks.java

@@ -58,7 +58,7 @@ public static void initializeServiceNowSourceConfig() {
       System.getenv("SERVICE_NOW_REST_API_ENDPOINT"),
       System.getenv("SERVICE_NOW_USERNAME"),
       System.getenv("SERVICE_NOW_PASSWORD"),
-      "", "", "");
+      "", "", "", null);
   }
 
   @Before(order = 2, value = "@SN_PRODUCT_CATALOG_ITEM")

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,18 +265,26 @@ 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 {
+    return fetchTableSchema(tableName, getAccessToken());
+  }
+
+  /**
+   * Fetches the table schema from ServiceNow
+   *
+   * @param tableName ServiceNow table name for which schema is getting fetched
+   * @param accessToken Access Token to use
+   * @return schema for given ServiceNow table
+   */
+  public Schema fetchTableSchema(String tableName, String accessToken) throws IOException {
     ServiceNowTableAPIRequestBuilder requestBuilder = new ServiceNowTableAPIRequestBuilder(
       this.conf.getRestApiEndpoint(), tableName, true)
       .setExcludeReferenceLink(true);
 
     RestAPIResponse apiResponse;
-    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,20 +305,29 @@ 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 {
+    return getTableRecordCount(tableName, getAccessToken());
+  }
+
+  /**
+   * Get the total number of records in the table
+   *
+   * @param tableName ServiceNow table name for which record count is fetched.
+   * @param accessToken Access Token for the call
+   * @return the table record count
+   * @throws IOException
+   */
+  public int getTableRecordCount(String tableName, String accessToken) throws IOException {
     ServiceNowTableAPIRequestBuilder requestBuilder = new ServiceNowTableAPIRequestBuilder(
       this.conf.getRestApiEndpoint(), tableName, false)
       .setExcludeReferenceLink(true)
       .setDisplayValue(SourceValueType.SHOW_DISPLAY_VALUE)
       .setLimit(1);
     RestAPIResponse apiResponse = null;
-    String accessToken = getAccessToken();
     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 +352,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 +365,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

@@ -89,12 +89,26 @@ public void test(ConnectorContext connectorContext) throws ValidationException {
 
   @Override
   public BrowseDetail browse(ConnectorContext connectorContext, BrowseRequest browseRequest) throws IOException {
+    ServiceNowTableAPIClientImpl serviceNowTableAPIClient = new ServiceNowTableAPIClientImpl(config);
+    try {
+      String accessToken = serviceNowTableAPIClient.getAccessToken();
+      return browse(connectorContext, accessToken);
+    } catch (OAuthSystemException | OAuthProblemException e) {
+      throw new IOException(e);
+    }
+  }
+
+  /**
+   * Browse Details for the given AccessToken.
+   */
+  public BrowseDetail browse(ConnectorContext connectorContext,
+                             String accessToken) throws IOException {
     int count = 0;
     FailureCollector collector = connectorContext.getFailureCollector();
     config.validateCredentialsFields(collector);
     collector.getOrThrowException();
     BrowseDetail.Builder browseDetailBuilder = BrowseDetail.builder();
-    Table[] table = listTables().getResult();
+    Table[] table = listTables(accessToken).getResult();
     for (int i = 0; i < table.length; i++) {
       String name = table[i].getName();
       String label = table[i].getLabel();
@@ -108,33 +122,18 @@ public BrowseDetail browse(ConnectorContext connectorContext, BrowseRequest brow
     return browseDetailBuilder.setTotalCount(count).build();
   }
 
-
   /**
    * @return the list of tables.
    */
-  private TableList listTables() throws IOException {
+  private TableList listTables(String accessToken) throws IOException {
     ServiceNowTableAPIRequestBuilder requestBuilder = new ServiceNowTableAPIRequestBuilder(
       config.getRestApiEndpoint(), OBJECT_TABLE_LIST, false);
-    String accessToken = null;
-    ServiceNowTableAPIClientImpl serviceNowTableAPIClient = new ServiceNowTableAPIClientImpl(config);
-    try {
-      accessToken = serviceNowTableAPIClient.getAccessToken();
-    } catch (OAuthSystemException | OAuthProblemException e) {
-      throw new IOException(e);
-    }
     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);
-    }
+    ServiceNowTableAPIClientImpl serviceNowTableAPIClient = new ServiceNowTableAPIClientImpl(config);
+    RestAPIResponse apiResponse = serviceNowTableAPIClient.executeGet(requestBuilder.build());
+    return GSON.fromJson(apiResponse.getResponseBody(), TableList.class);
   }
 
   public ConnectorSpec generateSpec(ConnectorContext connectorContext, ConnectorSpecRequest connectorSpecRequest) {
@@ -156,16 +155,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 +169,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<>();