[BC break] Refactored the SDKConnector

* Made sure the SDKConnector is open for extension but closed for modification. * As a consequence, the SDKConnector became final. * Merged the buildClient() method with the getClient() method and removed from the interface. * Added missing return types and concretized type of thrown exceptions. * Removed dead-code and unused properties.
apigee · May 8, 2019 · 23dba1c · 23dba1c
1 parent 6cfe266
commit 23dba1c
Show file tree

Hide file tree

Showing 9 changed files with 221 additions and 193 deletions.
diff --git a/apigee_edge.services.yml b/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']
 
   apigee_edge.key_entity_form_enhancer:
     class: Drupal\apigee_edge\KeyEntityFormEnhancer

diff --git a/modules/apigee_edge_debug/apigee_edge_debug.services.yml b/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

diff --git a/modules/apigee_edge_debug/src/SDKConnector.php b/modules/apigee_edge_debug/src/SDKConnector.php
@@ -20,22 +20,19 @@
 
 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\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 +46,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, array_merge($options, $extra_options));
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function testConnection(KeyInterface $key = NULL): void {
+    $this->innerService->testConnection($key);
   }
 
 }
diff --git a/src/KeyEntityFormEnhancer.php b/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;
@@ -329,7 +331,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', [
@@ -428,15 +430,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', [
@@ -446,14 +449,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.', [
@@ -471,10 +474,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.', [
@@ -501,24 +504,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.', [
@@ -544,15 +547,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) ? [] : [
@@ -583,7 +587,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) {

diff --git a/src/OauthAuthentication.php b/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);
   }
 
 }