Fix Configuration initialisation

src/Api/ApiClient.php

@@ -13,8 +13,8 @@
 use Cloudinary\Api\Exception\ApiError;
 use Cloudinary\Api\Exception\GeneralError;
 use Cloudinary\ArrayUtils;
-use Cloudinary\Configuration\CloudConfig;
 use Cloudinary\Configuration\ApiConfig;
+use Cloudinary\Configuration\CloudConfig;
 use Cloudinary\Configuration\Configuration;
 use Cloudinary\FileUtils;
 use Cloudinary\Utils;
@@ -93,8 +93,8 @@ public function configuration($configuration)
 
         $tempConfiguration->cloud->assertNotEmpty(['cloudName', 'apiKey', 'apiSecret']);
 
-        $this->cloud = $tempConfiguration->cloud;
-        $this->api   = $tempConfiguration->api;
+        $this->cloud   = $tempConfiguration->cloud;
+        $this->api     = $tempConfiguration->api;
         $this->logging = $tempConfiguration->logging;
 
         return $this;

src/Asset/BaseAsset.php

@@ -12,8 +12,8 @@
 
 use Cloudinary\ArrayUtils;
 use Cloudinary\ClassUtils;
-use Cloudinary\Configuration\CloudConfig;
 use Cloudinary\Configuration\AssetConfigTrait;
+use Cloudinary\Configuration\CloudConfig;
 use Cloudinary\Configuration\Configuration;
 use Cloudinary\Configuration\LoggingConfig;
 use Cloudinary\Configuration\UrlConfig;
@@ -207,6 +207,7 @@ public static function fromParams($source, $params = [])
         $configuration->url->secure = false;
 
         $configuration->importJson($params);
+        $configuration->validate();
 
         return new static($asset, $configuration);
     }
@@ -267,7 +268,7 @@ public function importJson($json)
     public function configuration($configuration)
     {
         $tempConfiguration = new Configuration($configuration, true); // TODO: improve performance here
-        $this->cloud     = $tempConfiguration->cloud;
+        $this->cloud       = $tempConfiguration->cloud;
         $this->urlConfig   = $tempConfiguration->url;
         $this->logging     = $tempConfiguration->logging;
 

src/Cloudinary.php

@@ -55,6 +55,8 @@ class Cloudinary
     public function __construct($config = null)
     {
         $this->configuration = new Configuration($config);
+        $this->configuration->validate();
+
         $this->tagBuilder    = new TagBuilder($this->configuration);
     }
 
@@ -126,7 +128,10 @@ public function imageTag($publicId)
      */
     public function videoTag($publicId, $sources = null)
     {
-        return $this->createWithConfiguration($publicId, VideoTag::class, $sources);
+        $videoTag = ClassUtils::forceInstance($publicId, VideoTag::class, null, $sources, $this->configuration);
+        $videoTag->importConfiguration($this->configuration);
+
+        return $videoTag;
     }
 
     /**
@@ -172,7 +177,8 @@ public function searchApi()
      */
     protected function createWithConfiguration($publicId, $className, ...$args)
     {
-        $instance = ClassUtils::forceInstance($publicId, $className, null, ...$args);
+        $instance =  ClassUtils::forceInstance($publicId, $className, null, $this->configuration, ...$args);
+        // this covers the case when an instance of the asset is provided and the line above is a no op.
         $instance->importConfiguration($this->configuration);
 
         return $instance;

src/Configuration/ConfigUtils.php

@@ -34,7 +34,8 @@ class ConfigUtils
      */
     public static function isCloudinaryUrl($cloudinaryUrl)
     {
-        return Utils::tryParseUrl($cloudinaryUrl, [self::CLOUDINARY_URL_SCHEME]) ? true : false;
+        return Utils::tryParseUrl(self::normalizeCloudinaryUrl($cloudinaryUrl), [self::CLOUDINARY_URL_SCHEME]) ? true
+            : false;
     }
 
     /**
@@ -52,7 +53,7 @@ public static function parseCloudinaryUrl($cloudinaryUrl)
             );
         }
 
-        $uri = Utils::tryParseUrl($cloudinaryUrl, [self::CLOUDINARY_URL_SCHEME]);
+        $uri = Utils::tryParseUrl(self::normalizeCloudinaryUrl($cloudinaryUrl), [self::CLOUDINARY_URL_SCHEME]);
 
         if (! $uri) {
             throw new UnexpectedValueException(
@@ -88,6 +89,22 @@ public static function parseCloudinaryUrl($cloudinaryUrl)
         return $config;
     }
 
+    /**
+     * Tries to normalize the supplied cloudinary url string.
+     *
+     * @param string $cloudinaryUrl Cloudinary url candidate.
+     *
+     * @return string
+     */
+    public static function normalizeCloudinaryUrl($cloudinaryUrl)
+    {
+        if (! is_string($cloudinaryUrl)) {
+            return $cloudinaryUrl;
+        }
+
+        return StringUtils::truncatePrefix($cloudinaryUrl, Configuration::CLOUDINARY_URL_ENV_VAR . '=');
+    }
+
     /**
      * Builds the main part of the Cloudinary url (not including query parameters)
      *

src/Configuration/Configuration.php

@@ -36,15 +36,16 @@ class Configuration implements ConfigurableInterface
      *
      * @var array
      */
-    protected $sections = [
-        CloudConfig::CONFIG_NAME,
-        ApiConfig::CONFIG_NAME,
-        UrlConfig::CONFIG_NAME,
-        TagConfig::CONFIG_NAME,
-        ResponsiveBreakpointsConfig::CONFIG_NAME,
-        AuthTokenConfig::CONFIG_NAME,
-        LoggingConfig::CONFIG_NAME,
-    ];
+    protected $sections
+        = [
+            CloudConfig::CONFIG_NAME,
+            ApiConfig::CONFIG_NAME,
+            UrlConfig::CONFIG_NAME,
+            TagConfig::CONFIG_NAME,
+            ResponsiveBreakpointsConfig::CONFIG_NAME,
+            AuthTokenConfig::CONFIG_NAME,
+            LoggingConfig::CONFIG_NAME,
+        ];
 
     /**
      * @var bool Indicates whether to include sensitive keys during serialisation to string/json.
@@ -134,36 +135,51 @@ public function init($config = null, $includeSensitive = true)
 
         $this->initSections();
 
-        // When no configuration provided, fallback to the environment variable.
-        if ($config === null) {
-            $config = (string)getenv(self::CLOUDINARY_URL_ENV_VAR) ?: null;
-        }
-
-        if (ConfigUtils::isCloudinaryUrl($config)) {
-            $this->importCloudinaryUrl($config);
-        } elseif (is_array($config) || JsonUtils::isJsonString($config)) {
-            $this->importJson($config);
-        } elseif ($config instanceof self) {
-            $this->importConfig($config);
-        } else {
-            throw new ConfigurationException('Invalid configuration, please set up your environment');
-        }
+        $this->import($config);
     }
 
     /**
      * Initializes configuration sections.
      */
     protected function initSections()
     {
-        $this->cloud = new CloudConfig();
-        $this->api   = new ApiConfig();
+        $this->cloud                 = new CloudConfig();
+        $this->api                   = new ApiConfig();
         $this->url                   = new UrlConfig();
         $this->tag                   = new TagConfig();
         $this->responsiveBreakpoints = new ResponsiveBreakpointsConfig();
         $this->authToken             = new AuthTokenConfig();
         $this->logging               = new LoggingConfig();
     }
 
+    /**
+     * Imports configuration.
+     *
+     * @param Configuration|string|array|null $config Configuration source. Can be Cloudinary url, json, array, another
+     *                                                instance of the configuration.
+     */
+    public function import($config = null)
+    {
+        if ($config === null) {
+            if (! getenv(self::CLOUDINARY_URL_ENV_VAR)) {
+                // Nothing to import
+                return;
+            }
+            // When no configuration provided, fallback to the environment variable.
+            $config = (string)getenv(self::CLOUDINARY_URL_ENV_VAR);
+        }
+
+        if (ConfigUtils::isCloudinaryUrl($config)) {
+            $this->importCloudinaryUrl($config);
+        } elseif (is_array($config) || JsonUtils::isJsonString($config)) {
+            $this->importJson($config);
+        } elseif ($config instanceof self) {
+            $this->importConfig($config);
+        } else {
+            throw new ConfigurationException('Invalid configuration, please set up your environment');
+        }
+    }
+
     /**
      * Singleton instance for effective access to global configuration.
      *
@@ -271,6 +287,13 @@ public function importConfig($otherConfig)
         return $this;
     }
 
+    public function validate()
+    {
+        if (empty($this->cloud->cloudName)) {
+            throw new ConfigurationException('Invalid configuration, please set up your environment');
+        }
+    }
+
     /**
      * Serialises Configuration to Cloudinary url
      *
@@ -282,7 +305,7 @@ public function toString()
 
         $sections = [];
         foreach ($this->sections as $section) {
-            $section = StringUtils::snakeCaseToCamelCase($section);
+            $section     = StringUtils::snakeCaseToCamelCase($section);
             $sections [] = (string)($this->$section);
         }
 

tests/Unit/Asset/MediaFromParamsTest.php

@@ -17,6 +17,7 @@
 use Cloudinary\Asset\Media;
 use Cloudinary\Configuration\Configuration;
 use Cloudinary\Configuration\UrlConfig;
+use InvalidArgumentException;
 
 /**
  * Class MediaFromParamsTest
@@ -49,6 +50,34 @@ public function testCloudNameOptions()
         );
     }
 
+    /**
+     * Should allow overriding cloud_name in $options
+     */
+    public function testCloudNameNoConfig()
+    {
+        self::clearEnvironment();
+
+        $options = ['cloud_name' => 'test321', 'analytics' => false];
+
+        self::assertMediaFromParamsUrl(
+            self::IMAGE_NAME,
+            $options,
+            $options
+        );
+    }
+
+    /**
+     * @expectedException InvalidArgumentException
+     */
+    public function testNoConfig()
+    {
+        self::clearEnvironment();
+
+        self::assertMediaFromParamsUrl(
+            self::IMAGE_NAME
+        );
+    }
+
     /**
      * Should use default secure distribution if secure=true
      */

tests/Unit/Cloudinary/CloudinaryAssetTest.php

@@ -12,6 +12,7 @@
 
 use Cloudinary\Asset\DeliveryType;
 use Cloudinary\Cloudinary;
+use Cloudinary\Configuration\Configuration;
 use Cloudinary\Test\Unit\Asset\AssetTestCase;
 
 /**
@@ -28,7 +29,7 @@ public function setUp()
     {
         parent::setUp();
 
-        $this->c = new Cloudinary($this->cloudinaryUrl);
+        $this->c = new Cloudinary(Configuration::instance());
     }
 
     public function testSimpleCloudinaryImage()

tests/Unit/Cloudinary/CloudinaryTest.php

@@ -34,7 +34,8 @@ public function testCloudinaryUrlFromEnv()
      */
     public function testCloudinaryUrlNotSet()
     {
-        putenv(Configuration::CLOUDINARY_URL_ENV_VAR); // unset CLOUDINARY_URL
+        self::clearEnvironment();
+
         new Cloudinary(); // Boom!
     }
 
@@ -44,8 +45,8 @@ public function testCloudinaryFromOptions()
             [
                 'cloud' => [
                     'cloud_name' => self::CLOUD_NAME,
-                    'api_key'        => self::API_KEY,
-                    'api_secret'     => self::API_SECRET,
+                    'api_key'    => self::API_KEY,
+                    'api_secret' => self::API_SECRET,
                 ],
             ]
         );
@@ -63,4 +64,17 @@ public function testCloudinaryFromUrl()
         self::assertEquals(self::API_KEY, $c->configuration->cloud->apiKey);
         self::assertEquals(self::API_SECRET, $c->configuration->cloud->apiSecret);
     }
+
+    public function testCloudinaryFromConfiguration()
+    {
+        self::clearEnvironment();
+
+        $config = new Configuration($this->cloudinaryUrl);
+
+        $c = new Cloudinary($config);
+
+        self::assertEquals(self::CLOUD_NAME, $c->configuration->cloud->cloudName);
+        self::assertEquals(self::API_KEY, $c->configuration->cloud->apiKey);
+        self::assertEquals(self::API_SECRET, $c->configuration->cloud->apiSecret);
+    }
 }

tests/Unit/Configuration/ConfigurationTest.php

@@ -28,6 +28,30 @@ public function testConfigFromUrl()
         self::assertEquals(self::API_SECRET, $config->cloud->apiSecret);
     }
 
+    /**
+     * Should allow passing Cloudinary URL that starts with a 'CLOUDINARY_URL=' prefix, which is technically illegal,
+     * but we are permissive.
+     */
+    public function testConfigFromFullUrl()
+    {
+        $config = new Configuration(Configuration::CLOUDINARY_URL_ENV_VAR . '=' . $this->cloudinaryUrl);
+
+        self::assertEquals(self::CLOUD_NAME, $config->cloud->cloudName);
+        self::assertEquals(self::API_KEY, $config->cloud->apiKey);
+        self::assertEquals(self::API_SECRET, $config->cloud->apiSecret);
+    }
+
+    public function testConfigNoEnv()
+    {
+        self::clearEnvironment();
+
+        $config = new Configuration();
+
+        $config->cloud->cloudName(self::CLOUD_NAME);
+
+        self::assertEquals(self::CLOUD_NAME, $config->cloud->cloudName);
+    }
+
     public function testConfigToString()
     {
         $config = Configuration::fromCloudinaryUrl($this->cloudinaryUrl);

tests/Unit/UnitTestCase.php

@@ -58,4 +58,11 @@ public function tearDown()
 
         putenv(Configuration::CLOUDINARY_URL_ENV_VAR . '=' . $this->cldUrlEnvBackup);
     }
+
+    protected static function clearEnvironment()
+    {
+        putenv(Configuration::CLOUDINARY_URL_ENV_VAR); // unset CLOUDINARY_URL
+
+        Configuration::instance()->init();
+    }
 }