8000 bug #48209 [VarExporter] Use `array<property-name,Closure>` for parti… · symfony/symfony@c6b54c5 · GitHub
[go: up one dir, main page]
Skip to content

Commit c6b54c5

Browse files
nicolas-grekasnicolas-grekas
committed
bug #48209 [VarExporter] Use array<property-name,Closure> for partial initialization of lazy ghost objects (nicolas-grekas)
This PR was merged into the 6.2 branch. Discussion ---------- [VarExporter] Use `array<property-name,Closure>` for partial initialization of lazy ghost objects | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT Lazy ghost objects can be either fully-initialized or partially-initialized: either the lazy-properties are all initialized at once, or they're initialized one-by-one. In the one-by-one way, the initializer is still one closure for all properties that accepts the property to initialize as argument. While preparing my talk for SymfonyCon, I realized that it would be better to pass an array of closures instead, keyed by the properties to lazy-initialize. Commits ------- efd6966 [VarExporter] Use array<property-name,Closure> for partial initialization of lazy ghost objects
2 parents ea9ed6c + efd6966 commit c6b54c5

File tree

5 files changed

+99
-83
lines changed

5 files changed

+99
-83
lines changed

‎src/Symfony/Component/VarExporter/Internal/LazyObjectRegistry.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,18 +70,18 @@ public static function getClassResetters($class)
7070

7171
$resetters = [];
7272
foreach ($classProperties as $scope => $properties) {
73-
$resetters[] = \Closure::bind(static function ($instance, $skippedProperties = []) use ($properties) {
73+
$resetters[] = \Closure::bind(static function ($instance, $skippedProperties, $onlyProperties = null) use ($properties) {
7474
foreach ($properties as $name => $key) {
75-
if (!\array_key_exists($key, $skippedProperties)) {
75+
if (!\array_key_exists($key, $skippedProperties) && (null === $onlyProperties || \array_key_exists($key, $onlyProperties))) {
7676
unset($instance->$name);
7777
}
7878
}
7979
}, null, $scope);
8080
}
8181

82-
$resetters[] = static function ($instance, $skippedProperties = []) {
82+
$resetters[] = static function ($instance, $skippedProperties, $onlyProperties = null) {
8383
foreach ((array) $instance as $name => $value) {
84-
if ("\0" !== ($name[0] ?? '') && !\array_key_exists($name, $skippedProperties)) {
84+
if ("\0" !== ($name[0] ?? '') && !\array_key_exists($name, $skippedProperties) && (null === $onlyProperties || \array_key_exists($name, $onlyProperties))) {
8585
unset($instance->$name);
8686
}
8787
}

‎src/Symfony/Component/VarExporter/Internal/LazyObjectState.php

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,10 @@
2222
*/
2323
class LazyObjectState
2424
{
25-
public const STATUS_INITIALIZED_PARTIAL = 1;
26-
public const STATUS_UNINITIALIZED_FULL = 2;
25+
public const STATUS_UNINITIALIZED_FULL = 1;
26+
public const STATUS_UNINITIALIZED_PARTIAL = 2;
2727
public const STATUS_INITIALIZED_FULL = 3;
28+
public const STATUS_INITIALIZED_PARTIAL = 4;
2829

2930
/**
3031
* @var array<string, true>
@@ -36,37 +37,34 @@ class LazyObjectState
3637
*/
3738
public int $status = 0;
3839

39-
public function __construct(public \Closure $initializer, $skippedProperties = [])
40+
public function __construct(public readonly \Closure|array $initializer, $skippedProperties = [])
4041
{
4142
$this->skippedProperties = $skippedProperties;
43+
$this->status = \is_array($initializer) ? self::STATUS_UNINITIALIZED_PARTIAL : self::STATUS_UNINITIALIZED_FULL;
4244
}
4345

4446
public function initialize($instance, $propertyName, $propertyScope)
4547
{
46-
if (!$this->status) {
47-
$this->status = 4 <= (new \ReflectionFunction($this->initializer))->getNumberOfParameters() ? self::STATUS_INITIALIZED_PARTIAL : self::STATUS_UNINITIALIZED_FULL;
48-
49-
if (null === $propertyName) {
50-
return $this->status;
51-
}
52-
}
53-
5448
if (self::STATUS_INITIALIZED_FULL === $this->status) {
5549
return self::STATUS_INITIALIZED_FULL;
5650
}
5751

58-
if (self::STATUS_INITIALIZED_PARTIAL === $this->status) {
52+
if (\is_array($this->initializer)) {
5953
$class = $instance::class;
6054
$propertyScope ??= $class;
6155
$propertyScopes = Hydrator::$propertyScopes[$class];
6256
$propertyScopes[$k = "\0$propertyScope\0$propertyName"] ?? $propertyScopes[$k = "\0*\0$propertyName"] ?? $k = $propertyName;
6357

64-
$value = ($this->initializer)(...[$instance, $propertyName, $propertyScope, LazyObjectRegistry::$defaultProperties[$class][$k] ?? null]);
58+
if (!$initializer = $this->initializer[$k] ?? null) {
59+
return self::STATUS_UNINITIALIZED_PARTIAL;
60+
}
61+
62+
$value = $initializer(...[$instance, $propertyName, $propertyScope, LazyObjectRegistry::$defaultProperties[$class][$k] ?? null]);
6563

6664
$accessor = LazyObjectRegistry::$classAccessors[$propertyScope] ??= LazyObjectRegistry::getClassAccessors($propertyScope);
6765
$accessor['set']($instance, $propertyName, $value);
6866

69-
return self::STATUS_INITIALIZED_PARTIAL;
67+
return $this->status = self::STATUS_INITIALIZED_PARTIAL;
7068
}
7169

7270
$this->status = self::STATUS_INITIALIZED_FULL;
@@ -93,6 +91,7 @@ public function reset($instance): void
9391
$propertyScopes = Hydrator::$propertyScopes[$class] ??= Hydrator::getPropertyScopes($class);
9492
$skippedProperties = $this->skippedProperties;
9593
$properties = (array) $instance;
94+
$onlyProperties = \is_array($this->initializer) ? $this->initializer : null;
9695

9796
foreach ($propertyScopes as $key => [$scope, $name, $readonlyScope]) {
9897
$propertyScopes[$k = "\0$scope\0$name"] ?? $propertyScopes[$k = "\0*\0$name"] ?? $k = $name;
@@ -103,7 +102,9 @@ public function reset($instance): void
103102
}
104103

105104
foreach (LazyObjectRegistry::$classResetters[$class] as $reset) {
106-
$reset($instance, $skippedProperties);
105+
$reset($instance, $skippedProperties, $onlyProperties);
107106
}
107+
108+
$this->status = self::STATUS_INITIALIZED_FULL === $this->status ? self::STATUS_UNINITIALIZED_FULL : self::STATUS_UNINITIALIZED_PARTIAL;
108109
}
109110
}

‎src/Symfony/Component/VarExporter/LazyGhostTrait.php

Lines changed: 34 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -16,28 +16,26 @@
1616
use Symfony\Component\VarExporter\Internal\LazyObjectState;
1717

1818
/**
19-
* @property int $lazyObjectId This property must be declared in classes using this trait
19+
* @property int $lazyObjectId This property must be declared as private in classes using this trait
2020
*/
2121
trait LazyGhostTrait
2222
{
2323
/**
2424
* Creates a lazy-loading ghost instance.
2525
*
26-
* The initializer can take two forms. In both forms,
27-
* the instance to initialize is passed as first argument.
26+
* When the initializer is a closure, it should initialize all properties at
27+
* once and is given the instance to initialize as argument.
2828
*
29-
* When the initializer takes only one argument, it is expected to initialize all
30-
* properties at once.
29+
* When the initializer is an array of closures, it should be indexed by
30+
* properties and closures should accept 4 arguments: the instance to
31+
* initialize, the property to initialize, its write-scope, and its default
32+
* value. Each closure should return the value of the corresponding property.
3133
*
32-
* When 4 arguments are required, the initializer is expected to return the value
33-
* of each property one by one. The extra arguments are the name of the property
34-
* to initialize, the write-scope of that property, and its default value.
35-
*
36-
* @param \Closure(static):void|\Closure(static, string, ?string, mixed):mixed $initializer
37-
* @param array<string, true> $skippedProperties An array indexed by the properties to skip,
38-
* aka the ones that the initializer doesn't set
34+
* @param \Closure(static):void|array<string, \Closure(static, string, ?string, mixed):mixed> $initializer
35+
* @param array<string, true> $skippedProperties An array indexed by the properties to skip, aka the ones
36+
* that the initializer doesn't set when its a closure
3937
*/
40-
public static function createLazyGhost(\Closure $initializer, array $skippedProperties = [], self $instance = null): static
38+
public static function createLazyGhost(\Closure|array $initializer, array $skippedProperties = [], self $instance = null): static
4139
{
4240
if (self::class !== $class = $instance ? $instance::class : static::class) {
4341
$skippedProperties["\0".self::class."\0lazyObjectId"] = true;
@@ -49,9 +47,10 @@ public static function createLazyGhost(\Closure $initializer, array $skippedProp
4947
Registry::$defaultProperties[$class] ??= (array) $instance;
5048
$instance->lazyObjectId = $id = spl_object_id($instance);
5149
Registry::$states[$id] = new LazyObjectState($initializer, $skippedProperties);
50+
$onlyProperties = \is_array($initializer) ? $initializer : null;
5251

5352
foreach (Registry::$classResetters[$class] ??= Registry::getClassResetters($class) as $reset) {
54-
$reset($instance, $skippedProperties);
53+
$reset($instance, $skippedProperties, $onlyProperties);
5554
}
5655

5756
return $instance;
@@ -66,17 +65,15 @@ public function isLazyObjectInitialized(): bool
6665
return true;
6766
}
6867

69-
if (LazyObjectState::STATUS_INITIALIZED_PARTIAL !== $state->status) {
68+
if (!\is_array($state->initializer)) {
7069
return LazyObjectState::STATUS_INITIALIZED_FULL === $state->status;
7170
}
7271

7372
$class = $this::class;
7473
$properties = (array) $this;
7574
$propertyScopes = Hydrator::$propertyScopes[$class] ??= Hydrator::getPropertyScopes($class);
76-
foreach ($propertyScopes as $key => [$scope, $name]) {
77-
$propertyScopes[$k = "\0$scope\0$name"] ?? $propertyScopes[$k = "\0*\0$name"] ?? $k = $name;
78-
79-
if ($k === $key && !\array_key_exists($k, $properties)) {
75+
foreach ($state->initializer as $key => $initializer) {
76+
if (!\array_key_exists($key, $properties) && isset($propertyScopes[$key])) {
8077
return false;
8178
}
8279
}
@@ -93,7 +90,7 @@ public function initializeLazyObject(): static
9390
return $this;
9491
}
9592

96-
if (LazyObjectState::STATUS_INITIALIZED_PARTIAL !== ($state->status ?: $state->initialize($this, null, null))) {
93+
if (!\is_array($state->initializer)) {
9794
if (LazyObjectState::STATUS_UNINITIALIZED_FULL === $state->status) {
9895
$state->initialize($this, '', null);
9996
}
@@ -104,10 +101,8 @@ public function initializeLazyObject(): static
104101
$class = $this::class;
105102
$properties = (array) $this;
106103
$propertyScopes = Hydrator::$propertyScopes[$class] ??= Hydrator::getPropertyScopes($class);
107-
foreach ($propertyScopes as $key => [$scope, $name, $readonlyScope]) {
108-
$propertyScopes[$k = "\0$scope\0$name"] ?? $propertyScopes[$k = "\0".($scope = '*')."\0$name"] ?? $k = $name;
109-
110-
if ($k !== $key || \array_key_exists($k, $properties)) {
104+
foreach ($state->initializer as $key => $initializer) {
105+
if (\array_key_exists($key, $properties) || ![$scope, $name, $readonlyScope] = $propertyScopes[$key] ?? null) {
111106
continue;
112107
}
113108

@@ -127,14 +122,8 @@ public function resetLazyObject(): bool
127122
return false;
128123
}
129124

130-
if (!$state->status) {
131-
return $state->initialize($this, null, null) || true;
132-
}
133-
134-
$state->reset($this);
135-
136-
if (LazyObjectState::STATUS_INITIALIZED_FULL === $state->status) {
137-
$state->status = LazyObjectState::STATUS_UNINITIALIZED_FULL;
125+
if (LazyObjectState::STATUS_UNINITIALIZED_FULL !== $state->status) {
126+
$state->reset($this);
138127
}
139128

140129
return true;
10000
@@ -149,8 +138,9 @@ public function &__get($name): mixed
149138
$scope = Registry::getScope($propertyScopes, $class, $name);
150139
$state = Registry::$states[$this->lazyObjectId ?? ''] ?? null;
151140

152-
if ($state && (null === $scope || isset($propertyScopes["\0$scope\0$name"]))) {
153-
$state->initialize($this, $name, $readonlyScope ?? $scope);
141+
if ($state && (null === $scope || isset($propertyScopes["\0$scope\0$name"]))
142+
&& LazyObjectState::STATUS_UNINITIALIZED_PARTIAL !== $state->initialize($this, $name, $readonlyScope ?? $scope)
143+
) {
154144
goto get_in_scope;
155145
}
156146
}
@@ -192,10 +182,10 @@ public function __set($name, $value): void
192182

193183
if ([$class, , $readonlyScope] = $propertyScopes[$name] ?? null) {
194184
$scope = Registry::getScope($propertyScopes, $class, $name, $readonlyScope);
195-
196185
$state = Registry::$states[$this->lazyObjectId ?? ''] ?? null;
186+
197187
if ($state && ($readonlyScope === $scope || isset($propertyScopes["\0$scope\0$name"]))) {
198-
if (LazyObjectState::STATUS_UNINITIALIZED_FULL === ($state->status ?: $state->initialize($this, null, null))) {
188+
if (LazyObjectState::STATUS_UNINITIALIZED_FULL === $state->status) {
199189
$state->initialize($this, $name, $readonlyScope ?? $scope);
200190
}
201191
goto set_in_scope;
@@ -227,8 +217,9 @@ public function __isset($name): bool
227217
$scope = Registry::getScope($propertyScopes, $class, $name);
228218
$state = Registry::$states[$this->lazyObjectId ?? ''] ?? null;
229219

230-
if ($state && (null === $scope || isset($propertyScopes["\0$scope\0$name"]))) {
231-
$state->initialize($this, $name, $readonlyScope ?? $scope);
220+
if ($state && (null === $scope || isset($propertyScopes["\0$scope\0$name"]))
221+
&& LazyObjectState::STATUS_UNINITIALIZED_PARTIAL !== $state->initialize($this, $name, $readonlyScope ?? $scope)
222+
) {
232223
goto isset_in_scope;
233224
}
234225
}
@@ -257,7 +248,7 @@ public function __unset($name): void
257248
$state = Registry::$states[$this->lazyObjectId ?? ''] ?? null;
258249

259250
if ($state && ($readonlyScope === $scope || isset($propertyScopes["\0$scope\0$name"]))) {
260-
if (LazyObjectState::STATUS_UNINITIALIZED_FULL === ($state->status ?: $state->initialize($this, null, null))) {
251+
if (LazyObjectState::STATUS_UNINITIALIZED_FULL === $state->status) {
261252
$state->initialize($this, $name, $readonlyScope ?? $scope);
262253
}
263254
goto unset_in_scope;
@@ -328,7 +319,7 @@ public function __destruct()
328319
$state = Registry::$states[$this->lazyObjectId ?? ''] ?? null;
329320

330321
try {
331-
if ($state && !\in_array($state->status, [LazyObjectState::STATUS_INITIALIZED_FULL, LazyObjectState::STATUS_INITIALIZED_PARTIAL], true)) {
322+
if ($state && \in_array($state->status, [LazyObjectState::STATUS_UNINITIALIZED_FULL, LazyObjectState::STATUS_UNINITIALIZED_PARTIAL], true)) {
332323
return;
333324
}
334325

@@ -344,7 +335,9 @@ public function __destruct()
344335

345336
private function setLazyObjectAsInitialized(bool $initialized): void
346337
{
347-
if ($state = Registry::$states[$this->lazyObjectId ?? ''] ?? null) {
338+
$state = Registry::$states[$this->lazyObjectId ?? ''];
339+
340+
if ($state && !\is_array($state->initializer)) {
348341
$state->status = $initialized ? LazyObjectState::STATUS_INITIALIZED_FULL : LazyObjectState::STATUS_UNINITIALIZED_FULL;
349342
}
350343
}

‎src/Symfony/Component/VarExporter/LazyProxyTrait.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
use Symfony\Component\VarExporter\Internal\LazyObjectState;
1818

1919
/**
20-
* @property int $lazyObjectId This property must be declared in classes using this trait
21-
* @property parent $lazyObjectReal This property must be declared in classes using this trait;
20+
* @property int $lazyObjectId This property must be declared as private in classes using this trait
21+
* @property parent $lazyObjectReal This property must be declared as private in classes using this trait;
2222
* its type should match the type of the proxied object
2323
*/
2424
trait LazyProxyTrait

‎src/Symfony/Component/VarExporter/Tests/LazyGhostTraitTest.php

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -210,20 +210,39 @@ public function testFullInitialization()
210210
public function testPartialInitialization()
211211
{
212212
$counter = 0;
213-
$instance = ChildTestClass::createLazyGhost(function (ChildTestClass $instance, string $property, ?string $scope, mixed $default) use (&$counter) {
214-
++$counter;
215-
216-
return match ($property) {
217-
'public' => 4 === $default ? 123 : -1,
218-
'publicReadonly' => 234,
219-
'protected' => 5 === $default ? 345 : -1,
220-
'protectedReadonly' => 456,
221-
'private' => match ($scope) {
222-
TestClass::class => 3 === $default ? 567 : -1,
223-
ChildTestClass::class => 6 === $default ? 678 : -1,
224-
},
225-
};
226-
});
213+
$instance = ChildTestClass::createLazyGhost([
214+
'public' => static function (ChildTestClass $instance, string $property, ?string $scope, mixed $default) use (&$counter) {
215+
++$counter;
216+
217+
return 4 === $default ? 123 : -1;
218+
},
219+
'publicReadonly' => static function (ChildTestClass $instance, string $property, ?string $scope, mixed $default) use (&$counter) {
220+
++$counter;
221+
222+
return 234;
223+
},
224+
"\0*\0protected" => static function (ChildTestClass $instance, string $property, ?string $scope, mixed $default) use (&$counter) {
225+
++$counter;
226+
227+
return 5 === $default ? 345 : -1;
228+
},
229+
"\0*\0protectedReadonly" => static function (ChildTestClass $instance, string $property, ?string $scope, mixed $default) use (&$counter) {
230+
++$counter;
231+
232+
return 456;
233+
},
234+
"\0".TestClass::class."\0private" => static function (ChildTestClass $instance, string $property, ?string $scope, mixed $default) use (&$counter) {
235+
++$counter;
236+
237+
return 3 === $default ? 567 : -1;
238+
},
239+
"\0".ChildTestClass::class."\0private" => static function (ChildTestClass $instance, string $property, ?string $scope, mixed $default) use (&$counter) {
240+
++$counter;
241+
242+
return 6 === $default ? 678 : -1;
243+
},
244+
'dummyProperty' => fn () => 123,
245+
]);
227246

228247
$this->assertSame(["\0".TestClass::class."\0lazyObjectId"], array_keys((array) $instance));
229248
$this->assertFalse($instance->isLazyObjectInitialized());
@@ -246,9 +265,14 @@ public function testPartialInitialization()
246265

247266
public function testPartialInitializationWithReset()
248267
{
249-
$instance = ChildTestClass::createLazyGhost(function (ChildTestClass $instance, string $property, ?string $scope, mixed $default) {
268+
$initializer = static function (ChildTestClass $instance, string $property, ?string $scope, mixed $default) {
250269
return 234;
251-
});
270+
};
271+
$instance = ChildTestClass::createLazyGhost([
272+
'public' => $initializer,
273+
'publicReadonly' => $initializer,
274+
"\0*\0protected" => $initializer,
275+
]);
252276

253277
$r = new \ReflectionProperty($instance, 'public');
254278
$r->setValue($instance, 123);
@@ -262,9 +286,7 @@ public function testPartialInitializationWithReset()
262286
$this->assertSame(234, $instance->publicReadonly);
263287
$this->assertSame(234, $instance->public);
264288

265-
$instance = ChildTestClass::createLazyGhost(function (ChildTestClass $instance, string $property, ?string $scope, mixed $default) {
266-
return 234;
267-
});
289+
$instance = ChildTestClass::createLazyGhost(['public' => $initializer]);
268290

269291
$instance->resetLazyObject();
270292

@@ -277,9 +299,9 @@ public function testPartialInitializationWithReset()
277299

278300
public function testPartialInitializationWithNastyPassByRef()
279301
{
280-
$instance = ChildTestClass::createLazyGhost(function (ChildTestClass $instance, string &$property, ?string &$scope, mixed $default) {
302+
$instance = ChildTestClass::createLazyGhost(['public' => function (ChildTestClass $instance, string &$property, ?string &$scope, mixed $default) {
281303
return $property = $scope = 123;
282-
});
304+
}]);
283305

284306
$this->assertSame(123, $instance->public);
285307
}

0 commit comments

Comments
 (0)
0