Merge d049929 into 7933165

justafish · Jan 13, 2016 · 0a03d76 · 0a03d76
2 parents 7933165 + d049929
commit 0a03d76
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 31 deletions.
diff --git a/amazons3.admin.inc b/amazons3.admin.inc
@@ -28,6 +28,16 @@ function amazons3_admin() {
     '#element_validate' => array('amazons3_form_bucket_validate'),
   );
 
+  $client = \Aws\S3\S3Client::factory();
+
+  $form['amazons3_region'] = array(
+    '#type' => 'select',
+    '#title' => t('Default region'),
+    '#default_value' => variable_get('amazons3_region', ''),
+    '#required' => TRUE,
+    '#options' => array_combine(array_keys($client->getRegions()), array_keys($client->getRegions())),
+  );
+
   $form['amazons3_cache'] = array(
     '#type' => 'checkbox',
     '#title' => t('Enable metadata caching'),

diff --git a/amazons3.module b/amazons3.module
@@ -566,6 +566,7 @@ function _amazons3_field_configuration(array &$form, $type) {
     if ($type == $types) {
       $settings = &$form['field']['settings'];
       $bucket_setting = isset($form['#field']['settings']['amazons3_bucket']) ? $form['#field']['settings']['amazons3_bucket'] : '';
+      $region_setting = isset($form['#field']['settings']['amazons3_region']) ? $form['#field']['settings']['amazons3_region'] : '';
 
       $settings['uri_scheme']['#weight'] = 50;
 
@@ -588,6 +589,28 @@ function _amazons3_field_configuration(array &$form, $type) {
         '#element_validate' => array('amazons3_form_bucket_validate'),
         '#weight' => 51,
       );
+
+      $client = \Aws\S3\S3Client::factory();
+
+      $settings['amazons3_region'] = array(
+        '#type' => 'select',
+        '#title' => t('Amazon S3 region'),
+        '#description' => t(
+          'Use <em>default region</em> to use the site-wide default region <a href="@config">currently set to %region</a>.',
+          array(
+            '@config' => url('admin/config/media/amazons3'),
+            '%region' => variable_get('amazons3_region', ''),
+          )
+        ),
+        '#states' => array(
+          'visible' => array(
+            ':input[name="field[settings][uri_scheme]"]' => array('value' => 's3'),
+          ),
+        ),
+        '#default_value' => $region_setting,
+        '#options' => array(0 => t('- default region -')) + array_combine(array_keys($client->getRegions()), array_keys($client->getRegions())),
+        '#weight' => 51,
+      );
     }
   }
 }
@@ -621,6 +644,13 @@ function amazons3_form_bucket_validate(array &$element, array &$form_state, arra
     $config['credentials'] = new Credentials($form_state['values']['amazons3_key'], $form_state['values']['amazons3_secret']);
   }
 
+  if (isset($form_state['values']['amazons3_region'])) {
+    $config['region'] = $form_state['values']['amazons3_region'];
+  }
+  elseif (isset($form_state['values']['field']['settings']['amazons3_region']) && $form_state['values']['field']['settings']['amazons3_region']) {
+    $config['region'] = $form_state['values']['field']['settings']['amazons3_region'];
+  }
+
   if (!empty($hostname)) {
     $config['endpoint'] = $hostname;
   }

diff --git a/src/S3Client.php b/src/S3Client.php
@@ -92,17 +92,16 @@ public static function factory($config = array(), $bucket = NULL) {
 
     $config['curl.options'] += $curl_defaults;
 
-    $client = AwsS3Client::factory($config);
-
     // Set the default client location to the associated bucket.
-    if (!isset($config['region']) && $bucket) {
-      $region = static::getBucketLocation($bucket, $client);
-      if (!empty($region) && $client->getRegion() != $region) {
+    if (!isset($config['region'])) {
+      $region = static::variable_get('amazons3_region');
+      if (!empty($region)) {
         $config['region'] = $region;
-        $client = AwsS3Client::factory($config);
       }
     }
 
+    $client = AwsS3Client::factory($config);
+
     static::setCommandFactory($client);
 
     return $client;

diff --git a/src/StreamWrapperConfiguration.php b/src/StreamWrapperConfiguration.php
@@ -75,7 +75,7 @@ protected static function defaults() {
     $defaults = array(
       'hostname' => NULL,
       'bucket' => NULL,
-      'region' => 'us-east-1',
+      'region' => NULL,
       'torrentPaths' => new MatchablePaths(),
       'presignedPaths' => new MatchablePaths(),
       'saveAsPaths' => new MatchablePaths(),
@@ -96,6 +96,7 @@ protected static function defaults() {
   protected static function required() {
     $required = array(
       'bucket',
+      'region',
     );
     return $required;
   }
@@ -369,7 +370,10 @@ public function setReducedRedundancyPaths(MatchablePaths $reducedRedundancyPaths
    *   A StreamWrapperConfiguration object.
    */
   public static function fromDrupalVariables() {
-    $config = self::fromConfig(array('bucket' => static::variable_get('amazons3_bucket', NULL)));
+    $config = self::fromConfig(array(
+      'bucket' => static::variable_get('amazons3_bucket', NULL),
+      'region' => static::variable_get('amazons3_region', NULL)
+    ));
     $defaults = $config->defaults();
 
     $config->setHostname(static::variable_get('amazons3_hostname', $defaults['hostname']));

diff --git a/tests/S3ClientTest.php b/tests/S3ClientTest.php
@@ -22,14 +22,15 @@ public function testFactory() {
       'amazons3_key' => 'key',
       'amazons3_secret' => 'secret',
       'amazons3_hostname' => 'hostname',
+      'amazons3_region' => 'region',
     ]);
     DrupalS3Client::resetCalled();
     $client = DrupalS3Client::factory(array(), 'fake-bucket');
     $this->assertInstanceOf('Aws\S3\S3Client', $client);
     $this->assertEquals('key', $client->getCredentials()->getAccessKeyId());
     $this->assertEquals('secret', $client->getCredentials()->getSecretKey());
     $this->assertEquals('hostname', $client->getBaseUrl());
-    $this->assertTrue(DrupalS3Client::isGetBucketLocationCalled());
+    $this->assertEquals('region', $client->getRegion());
 
     DrupalS3Client::setVariableData(array());
   }

diff --git a/tests/StreamWrapperConfigurationTest.php b/tests/StreamWrapperConfigurationTest.php
@@ -17,7 +17,7 @@ class StreamWrapperConfigurationTest extends \PHPUnit_Framework_TestCase {
    * @covers Drupal\amazons3\StreamWrapperConfiguration::required
    */
   public function testFromConfig() {
-    $settings = array('bucket' => 'bucket');
+    $settings = array('bucket' => 'bucket', 'region' => 'region');
     $config = StreamWrapperConfiguration::fromConfig($settings);
     $this->assertInstanceOf('Drupal\amazons3\StreamWrapperConfiguration', $config);
   }
@@ -43,7 +43,7 @@ public function testMissingBucket() {
    * @covers Drupal\amazons3\StreamWrapperConfiguration
    */
   public function testSetters() {
-    $config = StreamWrapperConfiguration::fromConfig(array('bucket' => 'bucket'));
+    $config = StreamWrapperConfiguration::fromConfig(array('bucket' => 'bucket', 'region' => 'region'));
 
     $config->setBucket('different-bucket');
     $this->assertEquals('different-bucket', $config->getBucket());
@@ -94,7 +94,7 @@ public function testSetters() {
    * @expectedException \LogicException
    */
   public function testCacheLifetimeException() {
-    $config = StreamWrapperConfiguration::fromConfig(array('bucket' => 'bucket'));
+    $config = StreamWrapperConfiguration::fromConfig(array('bucket' => 'bucket', 'region' => 'region'));
     $config->setCacheLifetime(0);
   }
 
@@ -103,7 +103,7 @@ public function testCacheLifetimeException() {
    * @expectedException \InvalidArgumentException
    */
   public function testCloudFrontCredentials() {
-    $config = StreamWrapperConfiguration::fromConfig(array('bucket' => 'bucket'));
+    $config = StreamWrapperConfiguration::fromConfig(array('bucket' => 'bucket', 'region' => 'region'));
     $config->setCloudFrontCredentials('/does-not-exist', 'asdf');
   }
 
@@ -112,15 +112,15 @@ public function testCloudFrontCredentials() {
    * @expectedException \LogicException
    */
   public function testCloudFrontNotSetUp() {
-    $config = StreamWrapperConfiguration::fromConfig(array('bucket' => 'bucket'));
+    $config = StreamWrapperConfiguration::fromConfig(array('bucket' => 'bucket', 'region' => 'region'));
     $config->getCloudFront();
   }
 
   /**
    * @covers Drupal\amazons3\StreamWrapperConfiguration::fromConfig
    */
   public function testDefaultHostname() {
-    $config = StreamWrapperConfiguration::fromConfig(array('bucket' => 'bucket'));
+    $config = StreamWrapperConfiguration::fromConfig(array('bucket' => 'bucket', 'region' => 'region'));
     $this->assertEquals('bucket.s3.amazonaws.com', $config->getDomain());
   }
 
@@ -131,6 +131,7 @@ public function testDefaultHostname() {
   public function testFromDrupalVariables() {
     StreamWrapperConfiguration::setVariableData([
       'amazons3_bucket' => 'default.example.com',
+      'amazons3_region' => 'region',
       'amazons3_hostname' => 'api.example.com',
       'amazons3_cname' => TRUE,
       'amazons3_domain' => 'static.example.com',
@@ -158,6 +159,7 @@ public function testFromDrupalVariables() {
 
     StreamWrapperConfiguration::setVariableData([
       'amazons3_bucket' => 'default.example.com',
+      'amazons3_region' => 'region',
       'amazons3_cname' => TRUE,
       'amazons3_cache' => FALSE,
     ]);
@@ -168,6 +170,7 @@ public function testFromDrupalVariables() {
     // When the bucket has a dot, check that the bucket is not in the domain.
     StreamWrapperConfiguration::setVariableData([
       'amazons3_bucket' => 'default.example.com',
+      'amazons3_region' => 'region',
     ]);
     $config = StreamWrapperConfiguration::fromDrupalVariables();
     $this->assertEquals('s3.amazonaws.com', $config->getDomain());
@@ -176,6 +179,7 @@ public function testFromDrupalVariables() {
     // subdomain.
     StreamWrapperConfiguration::setVariableData([
       'amazons3_bucket' => 'bucket',
+      'amazons3_region' => 'region',
     ]);
     $config = StreamWrapperConfiguration::fromDrupalVariables();
     $this->assertEquals('bucket.s3.amazonaws.com', $config->getDomain());

diff --git a/tests/StreamWrapperTest.php b/tests/StreamWrapperTest.php
@@ -36,6 +36,7 @@ public function setUp() {
 
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
       'expiration' => 0,
     ]);
@@ -63,6 +64,7 @@ public function testSetDefaultConfig() {
 
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
     ]);
     StreamWrapper::setDefaultConfig($config);
@@ -81,6 +83,7 @@ public function testSetDefaultConfig() {
   public function testConstructDefaultConfig() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'defaultconfig.example.com',
+      'region' => 'region',
       'caching' => FALSE,
     ]);
     StreamWrapper::setDefaultConfig($config);
@@ -98,6 +101,7 @@ public function testCreateClient() {
     StreamWrapper::setClient(null);
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
       'region' => 'us-east-1',
     ]);
@@ -113,6 +117,7 @@ public function testCreateClient() {
   public function testCreateCache() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => TRUE,
       'expiration' => 0,
     ]);
@@ -341,6 +346,7 @@ public function testSetS3ClientClass() {
   public function testBasename() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
       'expiration' => 0,
     ]);
@@ -370,6 +376,7 @@ public function testBasenameUriNotSet() {
   public function testSaveAs() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
       'expiration' => 0,
       'saveAsPaths' => new MatchablePaths(BasicPath::factory(array('force-download/.*'))),
@@ -385,6 +392,7 @@ public function testSaveAs() {
   public function testSaveAsExcluded() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
       'expiration' => 0,
       'saveAsPaths' => new MatchablePaths(BasicPath::factory(array('force-download/*'))),
@@ -404,6 +412,7 @@ public function testSaveAsExcluded() {
   public function testSaveAsAll() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
       'expiration' => 0,
       'saveAsPaths' => new MatchablePaths(BasicPath::factory(array('*'))),
@@ -423,6 +432,7 @@ public function testSaveAsAll() {
   public function testAttachmentSpace() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
       'expiration' => 0,
       'saveAsPaths' => new MatchablePaths(BasicPath::factory(array('force-download/.*'))),
@@ -443,6 +453,7 @@ public function testAttachmentSpace() {
   public function testTorrentPath() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
       'expiration' => 0,
       'torrentPaths' => new MatchablePaths(BasicPath::factory(array('torrents/.*'))),
@@ -461,6 +472,7 @@ public function testTorrentPath() {
   public function testPresignedPath() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
       'expiration' => 0,
       'presignedPaths' => new MatchablePaths(PresignedPath::factory(array('presigned/.*' => 30))),
@@ -487,6 +499,7 @@ public function testPresignedPath() {
   public function testCustomDomain() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'domain' => 'static.example.com',
       'caching' => FALSE,
       'expiration' => 0,
@@ -504,6 +517,7 @@ public function testCustomDomain() {
   public function testNoCustomDomain() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
     ]);
     $wrapper = new StreamWrapper($config);
@@ -519,6 +533,7 @@ public function testNoCustomDomain() {
   public function testReducedRedundancyStorage() {
     $config = StreamWrapperConfiguration::fromConfig([
       'bucket' => 'bucket.example.com',
+      'region' => 'region',
       'caching' => FALSE,
       'reducedRedundancyPaths' => new MatchablePaths(BasicPath::factory(array('*'))),
     ]);

diff --git a/tests/Stub/S3Client.php b/tests/Stub/S3Client.php
@@ -17,11 +17,9 @@ class S3Client extends \Drupal\amazons3\S3Client {
   use Bootstrap;
 
   protected static $factoryCalled = FALSE;
-  protected static $getBucketLocationCalled = FALSE;
 
   public static function resetCalled() {
     static::$factoryCalled = FALSE;
-    static::$getBucketLocationCalled = FALSE;
   }
 
   /**
@@ -31,13 +29,6 @@ public static function isFactoryCalled() {
     return static::$factoryCalled;
   }
 
-  /**
-   * @return boolean
-   */
-  public static function isGetBucketLocationCalled() {
-    return self::$getBucketLocationCalled;
-  }
-
   /**
    * Override setCommandFactory to use our stub.
    *
@@ -60,11 +51,4 @@ public static function factory($config = array(), $bucket = NULL) {
     return parent::factory($config, $bucket);
   }
 
-  /**
-   * {@inheritdoc}
-   */
-  public static function getBucketLocation($bucket, AwsS3Client $client) {
-    static::$getBucketLocationCalled = TRUE;
-    return 'fake-region';
-  }
 }