diff --git a/src/Api/ApiClient.php b/src/Api/ApiClient.php index 9ba24d8f..5edbbaf4 100644 --- a/src/Api/ApiClient.php +++ b/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; diff --git a/src/Asset/BaseAsset.php b/src/Asset/BaseAsset.php index e2fbcb77..d2ef05cd 100644 --- a/src/Asset/BaseAsset.php +++ b/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; diff --git a/src/Cloudinary.php b/src/Cloudinary.php index a827ce1c..23f0e5b2 100644 --- a/src/Cloudinary.php +++ b/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; diff --git a/src/Configuration/ConfigUtils.php b/src/Configuration/ConfigUtils.php index 36d356b4..4b1e9a67 100644 --- a/src/Configuration/ConfigUtils.php +++ b/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) * diff --git a/src/Configuration/Configuration.php b/src/Configuration/Configuration.php index 0dbb14bb..c0e75592 100644 --- a/src/Configuration/Configuration.php +++ b/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,20 +135,7 @@ 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); } /** @@ -155,8 +143,8 @@ public function init($config = null, $includeSensitive = true) */ 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(); @@ -164,6 +152,34 @@ protected function initSections() $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); } diff --git a/tests/Unit/Asset/MediaFromParamsTest.php b/tests/Unit/Asset/MediaFromParamsTest.php index 465837ed..d666c479 100644 --- a/tests/Unit/Asset/MediaFromParamsTest.php +++ b/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 */ diff --git a/tests/Unit/Cloudinary/CloudinaryAssetTest.php b/tests/Unit/Cloudinary/CloudinaryAssetTest.php index face2952..a2eafc64 100644 --- a/tests/Unit/Cloudinary/CloudinaryAssetTest.php +++ b/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() diff --git a/tests/Unit/Cloudinary/CloudinaryTest.php b/tests/Unit/Cloudinary/CloudinaryTest.php index 5afc51e0..380c668a 100644 --- a/tests/Unit/Cloudinary/CloudinaryTest.php +++ b/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); + } } diff --git a/tests/Unit/Configuration/ConfigurationTest.php b/tests/Unit/Configuration/ConfigurationTest.php index c5ef6799..78435f4d 100644 --- a/tests/Unit/Configuration/ConfigurationTest.php +++ b/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); diff --git a/tests/Unit/UnitTestCase.php b/tests/Unit/UnitTestCase.php index cc1c3190..818849a7 100644 --- a/tests/Unit/UnitTestCase.php +++ b/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(); + } }