DRUP-747 SDKConnector refactoring, mock API responses in tests and extra test for Authentication form

.travis.yml

@@ -27,31 +27,31 @@ env:
     - PHP_VERSION=7.1 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/tests"
     - PHP_VERSION=7.1 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/tests" DB_DRIVER=pgsql DB_IMAGE=wodby/postgres:9.6-1.3.1
     - PHP_VERSION=7.2 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/tests" PHP_IMAGE=wodby/drupal-php:7.2-dev-4.5.0
-    - PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/tests"
+    - PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev PHP_API_CLIENT_DEV=true DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/tests"
     # Apigee Edge API product RBAC module tests.
     - PHP_VERSION=7.1 DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apiproduct_rbac/tests"
     - PHP_VERSION=7.1 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apiproduct_rbac/tests"
     - PHP_VERSION=7.1 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apiproduct_rbac/tests" DB_DRIVER=pgsql DB_IMAGE=wodby/postgres:9.6-1.3.1
     - PHP_VERSION=7.2 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apiproduct_rbac/tests" PHP_IMAGE=wodby/drupal-php:7.2-dev-4.5.0
-    - PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apiproduct_rbac/tests"
+    - PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev PHP_API_CLIENT_DEV=true DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apiproduct_rbac/tests"
     # Apigee Edge Teams module tests.
     - PHP_VERSION=7.1 DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_teams/tests"
     - PHP_VERSION=7.1 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_teams/tests"
     - PHP_VERSION=7.1 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_teams/tests" DB_DRIVER=pgsql DB_IMAGE=wodby/postgres:9.6-1.3.1
     - PHP_VERSION=7.2 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_teams/tests" PHP_IMAGE=wodby/drupal-php:7.2-dev-4.5.0
-    - PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_teams/tests"
+    - PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev PHP_API_CLIENT_DEV=true DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_teams/tests"
     # Apigee Edge API Docs module tests.
     - PHP_VERSION=7.1 DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apidocs/tests"
     - PHP_VERSION=7.1 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apidocs/tests"
     - PHP_VERSION=7.1 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apidocs/tests" DB_DRIVER=pgsql DB_IMAGE=wodby/postgres:9.6-1.3.1
     - PHP_VERSION=7.2 DEPENDENCIES="" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apidocs/tests" PHP_IMAGE=wodby/drupal-php:7.2-dev-4.5.0
-    - PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apidocs/tests"
+    - PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev PHP_API_CLIENT_DEV=true DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apidocs/tests"
 matrix:
   allow_failures:
-    - env: PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/tests"
-    - env: PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apiproduct_rbac/tests"
-    - env: PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_teams/tests"
-    - env: PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apidocs/tests"
+    - env: PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev PHP_API_CLIENT_DEV=true DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/tests"
+    - env: PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev PHP_API_CLIENT_DEV=true DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apiproduct_rbac/tests"
+    - env: PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev PHP_API_CLIENT_DEV=true DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_teams/tests"
+    - env: PHP_VERSION=7.1 DRUPAL_CORE=8.7.x-dev PHP_API_CLIENT_DEV=true DEPENDENCIES="--prefer-lowest" TEST_ROOT="modules/contrib/apigee_edge/modules/apigee_edge_apidocs/tests"
   fast_finish: true
 
 # TODO Cache Docker images and PHP service build (common parts of the built, like Drupal core install, for a day).

.travis/docker-compose.override.yml

@@ -10,6 +10,7 @@ services:
       - log:/mnt/files/log
     environment:
       DRUPAL_CORE: ${DRUPAL_CORE:-}
+      PHP_API_CLIENT_DEV: ${PHP_API_CLIENT_DEV:-false}
       DEPENDENCIES: ${DEPENDENCIES:-}
       # We can not pass these environment variables with `docker-compose run -e` until
       # this has not been improved. https://github.com/wodby/php/pull/21#issuecomment-361200733

.travis/prepare-test-env.sh

@@ -40,6 +40,15 @@ if [[ -n "${DRUPAL_CORE}" ]]; then
   composer require drupal/core:${DRUPAL_CORE} webflo/drupal-core-require-dev:${DRUPAL_CORE} ${COMPOSER_GLOBAL_OPTIONS};
 fi
 
+# Allow to run tests with the latest dev version of the Apigee PHP API Client.
+if [[ "${PHP_API_CLIENT_DEV}" = true ]]; then
+  # Little trick that fakes the latest dev version as the latest tagged version
+  # in the 2.x branch.
+  composer require apigee/apigee-client-php:"2.x-dev as 2.1024.0" ${COMPOSER_GLOBAL_OPTIONS};
+  # For some reasons without this the client files does not get registered.
+  composer update none
+fi
+
 # Downgrade dependencies if needed.
 # (This fix is necessary since PR#130 had been merged because after that lowest
 # builds started to fail. Probably caused by a merge plugin issue because this

apigee_edge.services.yml

@@ -15,7 +15,7 @@ services:
 
   apigee_edge.sdk_connector:
     class: Drupal\apigee_edge\SDKConnector
-    arguments: ['@http_client_factory', '@key.repository', '@entity_type.manager', '@config.factory', '@module_handler', '@info_parser']
+    arguments: ['@http_client_factory', '@key.repository', '@config.factory', '@module_handler', '@info_parser', '@service_container']
 
   apigee_edge.key_entity_form_enhancer:
     class: Drupal\apigee_edge\KeyEntityFormEnhancer

modules/apigee_edge_debug/apigee_edge_debug.services.yml

@@ -8,7 +8,7 @@ services:
     decorates: apigee_edge.sdk_connector
     decoration_priority: -10
     public: false
-    arguments: ['@apigee_edge_debug.sdk_connector.inner', '@http_client_factory', '@key.repository', '@entity_type.manager',  '@config.factory', '@module_handler', '@info_parser']
+    arguments: ['@apigee_edge_debug.sdk_connector.inner']
     properties:
       # Without this property for decorated services, serialization will fail. @see: https://www.drupal.org/project/drupal/issues/2536370
       _serviceId: apigee_edge.sdk_connector

modules/apigee_edge_debug/src/SDKConnector.php

@@ -20,22 +20,20 @@
 
 namespace Drupal\apigee_edge_debug;
 
+use Apigee\Edge\ClientInterface;
 use Drupal\apigee_edge\SDKConnector as OriginalSDKConnector;
 use Drupal\apigee_edge\SDKConnectorInterface;
-use Drupal\Core\Config\ConfigFactoryInterface;
-use Drupal\Core\Entity\EntityTypeManagerInterface;
-use Drupal\Core\Extension\InfoParserInterface;
-use Drupal\Core\Extension\ModuleHandlerInterface;
-use Drupal\Core\Http\ClientFactory;
-use Drupal\key\KeyRepositoryInterface;
+use Drupal\Component\Utility\NestedArray;
+use Drupal\key\KeyInterface;
+use Http\Message\Authentication;
 
 /**
  * Service decorator for SDKConnector.
  */
-class SDKConnector extends OriginalSDKConnector implements SDKConnectorInterface {
+final class SDKConnector implements SDKConnectorInterface {
 
   /**
-   * Customer http request header.
+   * Custom HTTP request header.
    *
    * This tells the ApiClientProfiler to profile requests made by the underlying
    * HTTP client.
@@ -49,40 +47,60 @@ class SDKConnector extends OriginalSDKConnector implements SDKConnectorInterface
   /**
    * The inner SDK connector service.
    *
-   * @var \Drupal\apigee_edge\SDKConnector
+   * @var \Drupal\apigee_edge\SDKConnectorInterface
    */
   private $innerService;
 
+  /**
+   * The API client initialized from the saved credentials and default config.
+   *
+   * @var null|\Apigee\Edge\ClientInterface
+   *
+   * @see getClient()
+   */
+  private $defaultClient;
+
   /**
    * Constructs a new SDKConnector.
    *
    * @param \Drupal\apigee_edge\SDKConnectorInterface $inner_service
    *   The decorated SDK connector service.
-   * @param \Drupal\Core\Http\ClientFactory $client_factory
-   *   Http client.
-   * @param \Drupal\key\KeyRepositoryInterface $key_repository
-   *   The key repository.
-   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
-   *   Entity type manager service.
-   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
-   *   The factory for configuration objects.
-   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
-   *   Module handler service.
-   * @param \Drupal\Core\Extension\InfoParserInterface $info_parser
-   *   Info file parser service.
    */
-  public function __construct(SDKConnectorInterface $inner_service, ClientFactory $client_factory, KeyRepositoryInterface $key_repository, EntityTypeManagerInterface $entity_type_manager, ConfigFactoryInterface $config_factory, ModuleHandlerInterface $module_handler, InfoParserInterface $info_parser) {
+  public function __construct(SDKConnectorInterface $inner_service) {
     $this->innerService = $inner_service;
-    parent::__construct($client_factory, $key_repository, $entity_type_manager, $config_factory, $module_handler, $info_parser);
   }
 
   /**
    * {@inheritdoc}
    */
-  protected function httpClientConfiguration(): array {
-    $config = $this->innerService->httpClientConfiguration();
-    $config['headers'][static::HEADER] = static::HEADER;
-    return $config;
+  public function getOrganization(): string {
+    return $this->innerService->getOrganization();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getClient(?Authentication $authentication = NULL, ?string $endpoint = NULL, array $options = []): ClientInterface {
+    $extra_options[OriginalSDKConnector::CLIENT_FACTORY_OPTIONS]['headers'][static::HEADER] = static::HEADER;
+
+    // If method got called without default parameters, initialize and/or
+    // return the default API client from the internal cache.
+    if (!isset($authentication, $endpoint) && empty($options)) {
+      if ($this->defaultClient === NULL) {
+        $this->defaultClient = $this->innerService->getClient($authentication, $endpoint, $extra_options);
+      }
+
+      return $this->defaultClient;
+    }
+
+    return $this->innerService->getClient($authentication, $endpoint, NestedArray::mergeDeep($options, $extra_options));
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function testConnection(KeyInterface $key = NULL): void {
+    $this->innerService->testConnection($key);
   }
 
 }

src/KeyEntityFormEnhancer.php

@@ -21,8 +21,10 @@
 namespace Drupal\apigee_edge;
 
 use Apigee\Edge\Exception\ApiRequestException;
+use Apigee\Edge\Exception\ApiResponseException;
 use Apigee\Edge\Exception\OauthAuthenticationException;
 use Apigee\Edge\HttpClient\Plugin\Authentication\Oauth;
+use Drupal\apigee_edge\Exception\AuthenticationKeyException;
 use Drupal\apigee_edge\Exception\KeyProviderRequirementsException;
 use Drupal\apigee_edge\Plugin\EdgeKeyTypeInterface;
 use Drupal\apigee_edge\Plugin\KeyProviderRequirementsInterface;
@@ -324,7 +326,7 @@ public function validateForm(array &$form, FormStateInterface $form_state): void
       $this->connector->testConnection($test_key);
       $this->messenger()->addStatus($this->t('Connection successful.'));
     }
-    catch (\Exception $exception) {
+    catch (AuthenticationKeyException $exception) {
       watchdog_exception('apigee_edge', $exception);
 
       $form_state->setError($form, $this->t('@suggestion Error message: %response', [
@@ -423,15 +425,16 @@ private function keyIsWritable(KeyInterface $key): bool {
   /**
    * Creates a suggestion text to be displayed in the connection failed message.
    *
-   * @param \Exception $exception
-   *   The thrown exception during form validation.
+   * @param \Drupal\apigee_edge\Exception\AuthenticationKeyException $exception
+   *   The thrown exception during testing connection.
    * @param \Drupal\key\KeyInterface $key
    *   The used key during form validation.
    *
    * @return \Drupal\Component\Render\MarkupInterface
    *   The suggestion text to be displayed.
    */
-  private function createSuggestion(\Exception $exception, KeyInterface $key): MarkupInterface {
+  private function createSuggestion(AuthenticationKeyException $exception, KeyInterface $key): MarkupInterface {
+    $prev_exception = $exception->getPrevious() ?? $exception;
     $fail_text = $this->t('Failed to connect to Apigee Edge.');
     // General error message.
     $suggestion = $this->t('@fail_text', [
@@ -441,14 +444,14 @@ private function createSuggestion(\Exception $exception, KeyInterface $key): Mar
     $key_type = $key->getKeyType();
 
     // Failed to connect to the Oauth authorization server.
-    if ($exception instanceof OauthAuthenticationException) {
+    if ($prev_exception instanceof OauthAuthenticationException) {
       $fail_text = $this->t('Failed to connect to the OAuth authorization server.');
       // General error message.
       $suggestion = $this->t('@fail_text Check the debug information below for more details.', [
         '@fail_text' => $fail_text,
       ]);
       // Invalid credentials.
-      if ($exception->getCode() === 401) {
+      if ($prev_exception->getCode() === 401) {
         // Invalid credentials using defined client_id/client_secret.
         if ($key_type->getClientId($key) !== Oauth::DEFAULT_CLIENT_ID || $key_type->getClientSecret($key) !== Oauth::DEFAULT_CLIENT_SECRET) {
           $suggestion = $this->t('@fail_text The given username (%username) or password or client ID (%client_id) or client secret is incorrect.', [
@@ -466,10 +469,10 @@ private function createSuggestion(\Exception $exception, KeyInterface $key): Mar
         }
       }
       // Failed request.
-      elseif ($exception->getCode() === 0) {
-        if ($exception->getPrevious() instanceof ApiRequestException && $exception->getPrevious()->getPrevious() instanceof NetworkException && $exception->getPrevious()->getPrevious()->getPrevious() instanceof ConnectException) {
+      elseif ($prev_exception->getCode() === 0) {
+        if ($prev_exception->getPrevious() instanceof ApiRequestException && $prev_exception->getPrevious()->getPrevious() instanceof NetworkException && $prev_exception->getPrevious()->getPrevious()->getPrevious() instanceof ConnectException) {
           /** @var \GuzzleHttp\Exception\ConnectException $curl_exception */
-          $curl_exception = $exception->getPrevious()->getPrevious()->getPrevious();
+          $curl_exception = $prev_exception->getPrevious()->getPrevious()->getPrevious();
           // Resolving timed out.
           if ($curl_exception->getHandlerContext()['errno'] === CURLE_OPERATION_TIMEDOUT) {
             $suggestion = $this->t('@fail_text The connection timeout threshold (%connect_timeout) or the request timeout (%timeout) is too low or something is wrong with the connection.', [
@@ -496,24 +499,24 @@ private function createSuggestion(\Exception $exception, KeyInterface $key): Mar
       // regression bug in the Apigee Edge for Public Cloud 19.03.01 release. If
       // valid organization name and username provided with an invalid password
       // the MGMT server returns HTTP 500 with an error instead of HTTP 401.
-      if ($exception->getCode() === 401 || ($exception->getCode() === 500 && $exception->getEdgeErrorCode() === 'usersandroles.SsoInternalServerError')) {
+      if ($prev_exception->getCode() === 401 || ($prev_exception->getCode() === 500 && $prev_exception instanceof ApiResponseException && $prev_exception->getEdgeErrorCode() === 'usersandroles.SsoInternalServerError')) {
         $suggestion = $this->t('@fail_text The given username (%username) or password is incorrect.', [
           '@fail_text' => $fail_text,
           '%username' => $key_type->getUsername($key),
         ]);
       }
       // Invalid organization name.
-      elseif ($exception->getCode() === 403) {
+      elseif ($prev_exception->getCode() === 403) {
         $suggestion = $this->t('@fail_text The given organization name (%organization) is incorrect.', [
           '@fail_text' => $fail_text,
           '%organization' => $key_type->getOrganization($key),
         ]);
       }
       // Failed request.
-      elseif ($exception->getCode() === 0) {
-        if ($exception->getPrevious() instanceof NetworkException && $exception->getPrevious()->getPrevious() instanceof ConnectException) {
+      elseif ($prev_exception->getCode() === 0) {
+        if ($prev_exception->getPrevious() instanceof NetworkException && $prev_exception->getPrevious()->getPrevious() instanceof ConnectException) {
           /** @var \GuzzleHttp\Exception\ConnectException $curl_exception */
-          $curl_exception = $exception->getPrevious()->getPrevious();
+          $curl_exception = $prev_exception->getPrevious()->getPrevious();
           // Resolving timed out.
           if ($curl_exception->getHandlerContext()['errno'] === CURLE_OPERATION_TIMEDOUT) {
             $suggestion = $this->t('@fail_text The connection timeout threshold (%connect_timeout) or the request timeout (%timeout) is too low or something is wrong with the connection.', [
@@ -539,15 +542,16 @@ private function createSuggestion(\Exception $exception, KeyInterface $key): Mar
   /**
    * Creates debug text if there was an error during form validation.
    *
-   * @param \Exception $exception
-   *   The thrown exception during form validation.
+   * @param \Drupal\apigee_edge\Exception\AuthenticationKeyException $exception
+   *   The thrown exception during testing connection.
    * @param \Drupal\key\KeyInterface $key
    *   The used key during form validation.
    *
    * @return string
    *   The debug text to be displayed.
    */
-  private function createDebugText(\Exception $exception, KeyInterface $key): string {
+  private function createDebugText(AuthenticationKeyException $exception, KeyInterface $key): string {
+    $prev_exception = $exception->getPrevious() ?? $exception;
     $key_type = $key->getKeyType();
 
     $credentials = !($key_type instanceof EdgeKeyTypeInterface) ? [] : [
@@ -578,7 +582,7 @@ private function createDebugText(\Exception $exception, KeyInterface $key): stri
       '$1***mfa-token***$3',
       '$1***password***$3',
       '$1***credentials***',
-    ], (string) $exception);
+    ], (string) $prev_exception);
 
     // Filter out any private values from config.
     $client_config = array_filter($this->configFactory->get('apigee_edge.client')->get(), static function ($key) {

src/OauthAuthentication.php

@@ -35,7 +35,7 @@ class OauthAuthentication extends Oauth {
   protected function authClient(): ClientInterface {
     /** @var \Drupal\apigee_edge\SDKConnectorInterface $sdk_connector */
     $sdk_connector = \Drupal::service('apigee_edge.sdk_connector');
-    return $sdk_connector->buildClient(new BasicAuth($this->clientId, $this->clientSecret), $this->auth_server);
+    return $sdk_connector->getClient(new BasicAuth($this->clientId, $this->clientSecret), $this->auth_server);
   }
 
 }