feat(*): remove obsolete methods from BaseService (#40)

Fixes: https://github.ibm.com/arf/planning-sdk-squad/issues/904

BREAKING CHANGE:
This PR removes support for the standalone "apikey" authentication path
in the BaseService class, including the removal of "setApiKey()".
Going forward, authentication should be configured on a service instance
using generated service stors or the BaseService.setAuthenticator() method.

Author: padamstx

src/main/java/com/ibm/cloud/sdk/core/security/icp4d/ICP4DAuthenticator.java

@@ -33,6 +33,7 @@
 /**
  * This class implements support for the ICP4D authentication mechanism.
  */
+@SuppressWarnings("unused")
 public class ICP4DAuthenticator implements Authenticator {
   private static final Logger LOG = Logger.getLogger(ICP4DAuthenticator.class.getName());
 

src/main/java/com/ibm/cloud/sdk/core/security/icp4d/ICP4DConfig.java

@@ -40,15 +40,15 @@ public String authenticationType() {
 
   @Override
   public void validate() {
-    if (StringUtils.isEmpty(url)) {
-      throw new IllegalArgumentException("You must provide a URL.");
-    }
-
-    // If the user specifies their own access token, then username/password are not required.
+    // If the user specifies their own access token, then no other properties are required.
     if (StringUtils.isNotEmpty(userManagedAccessToken)) {
       return;
     }
 
+    if (StringUtils.isEmpty(url)) {
+      throw new IllegalArgumentException("You must provide a URL.");
+    }
+
     if (StringUtils.isEmpty(username) || StringUtils.isEmpty(password)) {
       throw new IllegalArgumentException(
         "You must provide both username and password values.");

src/main/java/com/ibm/cloud/sdk/core/service/BaseService.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2017 IBM Corp. All Rights Reserved.
+ * Copyright 2017, 2019 IBM Corp. All Rights Reserved.
  *
  * 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
@@ -63,11 +63,9 @@ public abstract class BaseService {
   private static final String APIKEY_AS_USERNAME = "apikey";
   private static final String ICP_PREFIX = "icp-";
   private static final Logger LOG = Logger.getLogger(BaseService.class.getName());
-  private String apiKey;
   private String username;
   private String password;
   private String endPoint;
-  private String defaultEndPoint;
   private final String name;
   private Authenticator authenticator;
 
@@ -131,8 +129,6 @@ private void setCredentialFields(CredentialUtils.ServiceCredentials serviceCrede
 
     if ((serviceCredentials.getUsername() != null) && (serviceCredentials.getPassword() != null)) {
       setUsernameAndPassword(serviceCredentials.getUsername(), serviceCredentials.getPassword());
-    } else if (serviceCredentials.getOldApiKey() != null) {
-      setApiKey(serviceCredentials.getOldApiKey());
     }
 
     if (serviceCredentials.getIamApiKey() != null) {
@@ -240,18 +236,6 @@ protected final <T> ServiceCall<T> createServiceCall(final Request request, fina
     return new WatsonServiceCall<>(call, converter);
   }
 
-  /**
-   * Gets the API key.
-   *
-   *
-   * @return the API key
-   * @deprecated
-   */
-  @Deprecated
-  protected String getApiKey() {
-    return apiKey;
-  }
-
   /**
    * Gets the username.
    *
@@ -270,6 +254,7 @@ protected String getUsername() {
    *
    * @return the password
    */
+  @Deprecated
   protected String getPassword() {
     return password;
   }
@@ -305,28 +290,13 @@ public String getName() {
   }
 
   /**
-   * Sets the API key.
+   * Sets the authentication information on the specified request builder.
+   * This method will invoke the configured Authenticator instance to perform the
+   * authentication needed for that authentication type.   This could involve adding an
+   * Authorization header, or perhaps adding a specific query param to the request URL.
    *
-   * @param apiKey the new API key
-   * @deprecated Use setAuthenticator(AuthenticatorConfig) instead
-   */
-  @Deprecated
-  public void setApiKey(String apiKey) {
-    if (CredentialUtils.hasBadStartOrEndChar(apiKey)) {
-      throw new IllegalArgumentException("The API key shouldn't start or end with curly brackets or quotes. Please "
-          + "remove any surrounding {, }, or \" characters.");
-    }
-
-    if (this.endPoint.equals(this.defaultEndPoint)) {
-      this.endPoint = "https://gateway-a.watsonplatform.net/visual-recognition/api";
-    }
-    this.apiKey = apiKey;
-  }
-
-  /**
-   * Sets the authentication. Okhttp3 compliant.
-   *
-   * @param builder the new authentication
+   * @param builder the request builder that represents the outgoing requst on which
+   * the authentication information should be set.
    */
   protected void setAuthentication(final Builder builder) {
     if (this.skipAuthentication) {
@@ -353,15 +323,12 @@ public void setEndPoint(final String endPoint) {
 
     if ((endPoint != null) && !endPoint.isEmpty()) {
       String newEndPoint = endPoint.endsWith("/") ? endPoint.substring(0, endPoint.length() - 1) : endPoint;
-      if (this.endPoint == null) {
-        this.defaultEndPoint = newEndPoint;
-      }
       this.endPoint = newEndPoint;
     }
   }
 
   /**
-   * Sets the username and password.
+   * Sets the username and password on this instance.
    *
    * @param username the username
    * @param password the password
@@ -374,13 +341,19 @@ public void setUsernameAndPassword(final String username, final String password)
           + "quotes. Please remove any surrounding {, }, or \" characters.");
     }
 
-    // we'll perform the token exchange for users UNLESS they're on ICP
+    // These fields are set to provide compatibility with the getUsername() and getPassword() methods.
+    this.username = username;
+    this.password = password;
+
+    // If this method is being called to set the IAM api key, we'll configure the IAM authenticator.
+    // Note that we only do this if the password does NOT indicate ICP.
     if (username.equals(APIKEY_AS_USERNAME) && !password.startsWith(ICP_PREFIX)) {
       IamOptions iamOptions = new IamOptions.Builder()
           .apiKey(password)
           .build();
       setAuthenticator(iamOptions);
     } else {
+      // Otherwise, we'll just use the username and password to configure basic auth.
       BasicAuthConfig basicAuthConfig = new BasicAuthConfig.Builder()
           .username(username)
           .password(password)
@@ -418,15 +391,19 @@ public void setIamCredentials(IamOptions iamOptions) {
   }
 
   /**
-   * Initializes a new Authenticator instance based on the input AuthenticatorConfig instance and sets it as
-   * the current authenticator on the BaseService instance.
+   * Initializes a new Authenticator instance based on the input AuthenticatorConfig instance,
+   * and sets it as the current authenticator on this BaseService instance.
+   * The "skipAuthentication" flag is updated appropriately by this method.
    * @param authConfig the AuthenticatorConfig instance containing the authentication configuration
    */
-  protected void setAuthenticator(AuthenticatorConfig authConfig) {
+  public void setAuthenticator(AuthenticatorConfig authConfig) {
     try {
-      this.authenticator = AuthenticatorFactory.getAuthenticator(authConfig);
-      if (authenticator instanceof NoauthAuthenticator) {
-        setSkipAuthentication(true);
+      if (authConfig == null) {
+        this.authenticator = null;
+        this.skipAuthentication = false;
+      } else {
+        this.authenticator = AuthenticatorFactory.getAuthenticator(authConfig);
+        this.skipAuthentication = (Authenticator.AUTHTYPE_NOAUTH.equals(authConfig.authenticationType()));
       }
     } catch (Exception e) {
       throw new RuntimeException(e);
@@ -503,14 +480,30 @@ protected <T> T processServiceCall(final ResponseConverter<T> converter, final R
    * Sets the skip authentication.
    *
    * @param skipAuthentication the new skip authentication
+   *
+   * @deprecated  Use setAuthenticator(new NoauthConfig()) instead
    */
+  @Deprecated
   public void setSkipAuthentication(final boolean skipAuthentication) {
     this.skipAuthentication = skipAuthentication;
     if (this.skipAuthentication) {
       this.authenticator = new NoauthAuthenticator((NoauthConfig) null);
+    } else {
+      // If we're setting skipAuthentication to false, make sure we
+      // clear out the authenticator if it's currently set to "noauth".
+      if (Authenticator.AUTHTYPE_NOAUTH.equals(this.authenticator.authenticationType())) {
+        this.authenticator = null;
+      }
     }
   }
 
+  /**
+   * Returns true iff this instance has been configured to bypass authentication for outgoing requests.
+   * @return true to indicate authentication is being bypassed, false otherwise.
+   *
+   * @deprecated
+   */
+  @Deprecated
   public boolean isSkipAuthentication() {
     return this.skipAuthentication;
   }

src/main/java/com/ibm/cloud/sdk/core/service/security/IamOptions.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2018 IBM Corp. All Rights Reserved.
+ * Copyright 2018, 2019 IBM Corp. All Rights Reserved.
  *
  * 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

src/test/java/com/ibm/cloud/sdk/core/test/service/AuthenticationTest.java

@@ -16,6 +16,7 @@
 import org.junit.Test;
 
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 @SuppressWarnings("deprecation")
@@ -107,6 +108,51 @@ public void testSkipAuthentication() {
     assertTrue(service.isSkipAuthentication());
   }
 
+  @Test
+  public void testDefaultServiceCtor() {
+    TestService service = new TestService();
+    assertFalse(service.isSkipAuthentication());
+    assertNull(service.authenticator());
+  }
+
+  @Test
+  public void testSetAuthenticator() {
+    // Test use of setAuthenticator() on existing service instance.
+    TestService service = new TestService();
+    assertFalse(service.isSkipAuthentication());
+    assertNull(service.authenticator());
+
+    service.setAuthenticator(new NoauthConfig());
+    assertTrue(service.authenticator() instanceof NoauthAuthenticator);
+    assertTrue(service.isSkipAuthentication());
+
+    service.setAuthenticator(null);
+    assertNull(service.authenticator());
+    assertFalse(service.isSkipAuthentication());
+
+    BasicAuthConfig baConfig = new BasicAuthConfig.Builder()
+        .username("user")
+        .password("pw")
+        .build();
+    service.setAuthenticator(baConfig);
+    assertTrue(service.authenticator() instanceof BasicAuthenticator);
+    assertFalse(service.isSkipAuthentication());
+
+    IamOptions iamConfig = new IamOptions.Builder()
+        .apiKey("myapikey")
+        .build();
+    service.setAuthenticator(iamConfig);
+    assertTrue(service.authenticator() instanceof IamTokenManager);
+    assertFalse(service.isSkipAuthentication());
+
+    ICP4DConfig icp4dConfig = new ICP4DConfig.Builder()
+        .userManagedAccessToken("myuseraccesstoken")
+        .build();
+    service.setAuthenticator(icp4dConfig);
+    assertTrue(service.authenticator() instanceof ICP4DAuthenticator);
+    assertFalse(service.isSkipAuthentication());
+  }
+
   @Test
   public void multiAuthenticationWithMultiBindSameServiceOnVcapService() {