Skip to content

Commit a0a334b

Browse files
Speed up URL/tag generation via Configuration clone fast-path
Reduce per-asset/per-tag Configuration construction overhead and tighten a few hot paths exercised by URL building. - Add Configuration::__clone() that deep-clones every section so cloning the global singleton produces a fully independent working copy without the cost of a JSON import/export round-trip. - Use the clone fast-path in BaseAsset::fromParams, BaseTag::configuration (replaces a pre-existing TODO), and BaseTag::fromParamsDefaultConfig instead of new Configuration(...). - Memoize BaseConfigSection::exportableKeys() per (class, sensitivity) to eliminate repeated ReflectionClass::getConstants() calls on every config serialization. - Replace array_values(section)[0] with current(section) in the two jsonSerialize implementations. Add tests/Unit/Configuration/ConfigurationCloneTest.php to lock in clone isolation across all configuration sections and across the asset/tag fast-paths, so a future regression to a shallow copy fails loudly.
1 parent 2e3c4d8 commit a0a334b

5 files changed

Lines changed: 180 additions & 7 deletions

File tree

src/Asset/BaseAsset.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ public static function fromParams(string $source, array $params): static
199199
$params['public_id'] = $source;
200200

201201
$asset = AssetDescriptor::fromParams($source, $params);
202-
$configuration = new Configuration(Configuration::instance());
202+
$configuration = clone Configuration::instance();
203203

204204
# set v1 defaults
205205
if (! $configuration->url->isExplicitlySet('secure')) {
@@ -377,7 +377,7 @@ public function jsonSerialize(bool $includeEmptyKeys = false, bool $includeEmpty
377377

378378
foreach ([$this->cloud, $this->urlConfig] as $confSection) {
379379
$section = $confSection->jsonSerialize(false, $includeEmptyKeys, $includeEmptySections);
380-
if (! $includeEmptySections && empty(array_values($section)[0])) {
380+
if (! $includeEmptySections && empty(current($section))) {
381381
continue;
382382
}
383383
$json += $section;

src/Configuration/BaseConfigSection.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ abstract class BaseConfigSection implements ConfigurableInterface
4343
*/
4444
protected array $explicitlySetKeys = [];
4545

46+
/**
47+
* @var array<string, array>
48+
*/
49+
private static array $exportableKeysCache = [];
50+
4651

4752
/**
4853
* BaseConfig constructor.
@@ -195,12 +200,17 @@ public function assertNotEmpty(array $keys): void
195200
*/
196201
protected static function exportableKeys(bool $includeSensitive = true): array
197202
{
203+
$cacheKey = static::class . ($includeSensitive ? ':1' : ':0');
204+
if (isset(self::$exportableKeysCache[$cacheKey])) {
205+
return self::$exportableKeysCache[$cacheKey];
206+
}
207+
198208
$blacklisted = [static::CONFIG_NAME];
199209
if (! $includeSensitive) {
200210
$blacklisted = array_merge($blacklisted, static::$sensitiveDataKeys);
201211
}
202212

203-
return array_filter(
213+
return self::$exportableKeysCache[$cacheKey] = array_filter(
204214
ClassUtils::getConstants(static::class, $blacklisted),
205215
static fn($key) => ! empty($key) && is_string($key)
206216
);

src/Configuration/Configuration.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,17 @@ public function __construct(Configuration|array|string|null $config = null, bool
118118
$this->init($config, $includeSensitive);
119119
}
120120

121+
public function __clone(): void
122+
{
123+
$this->cloud = clone $this->cloud;
124+
$this->api = clone $this->api;
125+
$this->url = clone $this->url;
126+
$this->tag = clone $this->tag;
127+
$this->responsiveBreakpoints = clone $this->responsiveBreakpoints;
128+
$this->authToken = clone $this->authToken;
129+
$this->logging = clone $this->logging;
130+
}
131+
121132
/**
122133
* Configuration initializer.
123134
*
@@ -328,7 +339,7 @@ public function jsonSerialize(
328339
foreach ($this->sections as $section) {
329340
$section = StringUtils::snakeCaseToCamelCase($section);
330341
$section = $this->$section->jsonSerialize($includeSensitive, $includeEmptyKeys);
331-
if (! $includeEmptySections && empty(array_values($section)[0])) {
342+
if (! $includeEmptySections && empty(current($section))) {
332343
continue;
333344
}
334345
$json += $section;

src/Tag/BaseTag.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,13 @@ public function __construct(Configuration|array|string|null $configuration = nul
8686
*/
8787
public function configuration(Configuration|array|string|null $configuration): Configuration
8888
{
89-
$tempConfiguration = new Configuration($configuration); // TODO: improve performance here
90-
$this->config = $tempConfiguration;
89+
if ($configuration instanceof Configuration) {
90+
$tempConfiguration = clone $configuration;
91+
} else {
92+
$tempConfiguration = new Configuration($configuration);
93+
}
94+
95+
$this->config = $tempConfiguration;
9196

9297
return $tempConfiguration;
9398
}
@@ -339,7 +344,7 @@ protected static function collectAttributesFromParams(array $params): array
339344
*/
340345
protected static function fromParamsDefaultConfig(): Configuration
341346
{
342-
$configuration = new Configuration(Configuration::instance());
347+
$configuration = clone Configuration::instance();
343348
# set v1 defaults
344349
$configuration->tag->quotesType = self::SINGLE_QUOTES;
345350
$configuration->tag->sortAttributes = true;
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
<?php
2+
/**
3+
* This file is part of the Cloudinary PHP package.
4+
*
5+
* (c) Cloudinary
6+
*
7+
* For the full copyright and license information, please view the LICENSE
8+
* file that was distributed with this source code.
9+
*/
10+
11+
namespace Cloudinary\Test\Unit\Configuration;
12+
13+
use Cloudinary\Asset\Image;
14+
use Cloudinary\Configuration\Configuration;
15+
use Cloudinary\Tag\ImageTag;
16+
use Cloudinary\Test\Unit\UnitTestCase;
17+
18+
/**
19+
* Verifies that cloning a Configuration (and the clone fast-paths in BaseAsset / BaseTag /
20+
* fromParams) produces a fully independent copy.
21+
*
22+
* If any of these tests fail, mutations on a per-asset / per-tag working copy can leak back
23+
* into the global Configuration::instance() singleton.
24+
*/
25+
class ConfigurationCloneTest extends UnitTestCase
26+
{
27+
public function testCloneCopiesEverySectionAsDistinctObject()
28+
{
29+
$source = Configuration::instance();
30+
$clone = clone $source;
31+
32+
self::assertNotSame($source, $clone);
33+
self::assertNotSame($source->cloud, $clone->cloud);
34+
self::assertNotSame($source->api, $clone->api);
35+
self::assertNotSame($source->url, $clone->url);
36+
self::assertNotSame($source->tag, $clone->tag);
37+
self::assertNotSame($source->responsiveBreakpoints, $clone->responsiveBreakpoints);
38+
self::assertNotSame($source->authToken, $clone->authToken);
39+
self::assertNotSame($source->logging, $clone->logging);
40+
}
41+
42+
public function testCloneMutationsDoNotLeakIntoSource()
43+
{
44+
$source = Configuration::instance();
45+
$clone = clone $source;
46+
47+
$originals = [
48+
'cloudName' => $source->cloud->cloudName,
49+
'secure' => $source->url->secure,
50+
'cname' => $source->url->cname,
51+
'sortAttributes' => $source->tag->sortAttributes,
52+
'authTokenKey' => $source->authToken->key,
53+
'loggingLevel' => $source->logging->level,
54+
'apiProxy' => $source->api->apiProxy,
55+
'breakpoints' => $source->responsiveBreakpoints->breakpoints,
56+
];
57+
58+
$clone->cloud->cloudName = 'mutated_cloud';
59+
$clone->url->secure = ! (bool) $originals['secure'];
60+
$clone->url->cname = 'clone.example.com';
61+
$clone->tag->sortAttributes = ! (bool) $originals['sortAttributes'];
62+
$clone->authToken->key = '00112233445566778899aabbccddeeff';
63+
$clone->logging->level = 'critical';
64+
$clone->api->apiProxy = 'http://proxy.example.com';
65+
$clone->responsiveBreakpoints->breakpoints = [100, 200, 300];
66+
67+
self::assertEquals($originals['cloudName'], $source->cloud->cloudName);
68+
self::assertEquals($originals['secure'], $source->url->secure);
69+
self::assertEquals($originals['cname'], $source->url->cname);
70+
self::assertEquals($originals['sortAttributes'], $source->tag->sortAttributes);
71+
self::assertEquals($originals['authTokenKey'], $source->authToken->key);
72+
self::assertEquals($originals['loggingLevel'], $source->logging->level);
73+
self::assertEquals($originals['apiProxy'], $source->api->apiProxy);
74+
self::assertEquals($originals['breakpoints'], $source->responsiveBreakpoints->breakpoints);
75+
}
76+
77+
public function testAssetConstructorClonesGlobalConfig()
78+
{
79+
$global = Configuration::instance();
80+
$image = new Image('sample.png');
81+
82+
self::assertNotSame($global->cloud, $image->cloud);
83+
self::assertNotSame($global->url, $image->urlConfig);
84+
self::assertNotSame($global->logging, $image->logging);
85+
86+
$originalCloudName = $global->cloud->cloudName;
87+
$originalSecure = $global->url->secure;
88+
$originalLevel = $global->logging->level;
89+
90+
$image->cloud->cloudName = 'mutated_cloud';
91+
$image->urlConfig->secure = ! (bool) $originalSecure;
92+
$image->logging->level = 'critical';
93+
94+
self::assertEquals($originalCloudName, $global->cloud->cloudName);
95+
self::assertEquals($originalSecure, $global->url->secure);
96+
self::assertEquals($originalLevel, $global->logging->level);
97+
}
98+
99+
public function testAssetFromParamsDoesNotMutateGlobalConfig()
100+
{
101+
$global = Configuration::instance();
102+
$originalCloudName = $global->cloud->cloudName;
103+
$originalSecure = $global->url->secure;
104+
105+
$image = Image::fromParams('sample.png', [
106+
'cloud_name' => 'from_params_cloud',
107+
'secure' => true,
108+
]);
109+
110+
self::assertEquals('from_params_cloud', $image->cloud->cloudName);
111+
self::assertEquals($originalCloudName, $global->cloud->cloudName);
112+
self::assertEquals($originalSecure, $global->url->secure);
113+
}
114+
115+
public function testTagConstructorClonesGivenConfiguration()
116+
{
117+
$source = new Configuration(Configuration::instance());
118+
$tag = new ImageTag('sample.jpg', $source);
119+
120+
self::assertNotSame($source, $tag->config);
121+
self::assertNotSame($source->cloud, $tag->config->cloud);
122+
self::assertNotSame($source->tag, $tag->config->tag);
123+
124+
$originalCloudName = $source->cloud->cloudName;
125+
$originalSortAttribute = $source->tag->sortAttributes;
126+
127+
$tag->config->cloud->cloudName = 'tag_mutated_cloud';
128+
$tag->config->tag->sortAttributes = ! (bool) $originalSortAttribute;
129+
130+
self::assertEquals($originalCloudName, $source->cloud->cloudName);
131+
self::assertEquals($originalSortAttribute, $source->tag->sortAttributes);
132+
}
133+
134+
public function testTagFromParamsDoesNotMutateGlobalConfig()
135+
{
136+
$global = Configuration::instance();
137+
$originalCloudName = $global->cloud->cloudName;
138+
$originalQuotes = $global->tag->quotesType;
139+
140+
ImageTag::fromParams('sample.jpg', [
141+
'cloud_name' => 'tag_from_params_cloud',
142+
]);
143+
144+
self::assertEquals($originalCloudName, $global->cloud->cloudName);
145+
self::assertEquals($originalQuotes, $global->tag->quotesType);
146+
}
147+
}

0 commit comments

Comments
 (0)