10000 merged branch bschussek/issue4535 (PR #4827) · symfony/symfony@c8c26d0 · GitHub
[go: up one dir, main page]
Skip to content

Commit c8c26d0

Browse files
fabpotfabpot
committed
merged branch bschussek/issue4535 (PR #4827)
Commits ------- e8bb834 [Form] Fixed data to be written back by PropertyPath if it cannot be handled by reference Discussion ---------- [Form] Fixed data to be written back by PropertyPath if it cannot be handled by reference Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: #4535 Todo: -
2 parents 9e32d90 + e8bb834 commit c8c26d0

File tree

2 files changed

+94
-34
lines changed

2 files changed

+94
-34
lines changed

‎src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ public function getAxes() {}
8181
class PropertyPathCollectionTest_CompositeCar
8282
{
8383
public function getStructure() {}
84+
85+
public function setStructure($structure) {}
8486
}
8587

8688
class PropertyPathCollectionTest_CarStructure
@@ -105,6 +107,15 @@ public function testGetValueReadsArrayAccess()
105107
$this->assertEquals('Bernhard', $path->getValue($object));
106108
}
107109

110+
public function testGetValueReadsNestedArrayAccess()
111+
{
112+
$object = $this->getCollection(array('person' => array('firstName' => 'Bernhard')));
113+
114+
$path = new PropertyPath('[person][firstName]');
115+
116+
$this->assertEquals('Bernhard', $path->getValue($object));
117+
}
118+
108119
/**
109120
* @expectedException Symfony\Component\Form\Exception\InvalidPropertyException
110121
*/
@@ -125,6 +136,16 @@ public function testSetValueUpdatesArrayAccess()
125136
$this->assertEquals('Bernhard', $object['firstName']);
126137
}
127138

139+
public function testSetValueUpdatesNestedArrayAccess()
140+
{
141+
$object = $this->getCollection(array());
142+
143+
$path = new PropertyPath('[person][firstName]');
144+
$path->setValue($object, 'Bernhard');
145+
146+
$this->assertEquals('Bernhard', $object['person']['firstName']);
147+
}
148+
128149
/**
129150
* @expectedException Symfony\Component\Form\Exception\InvalidPropertyException
130151
*/

‎src/Symfony/Component/Form/Util/PropertyPath.php

Lines changed: 73 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface
3131
*/
3232
const SINGULAR_SEPARATOR = '|';
3333

34+
const VALUE = 0;
35+
const IS_REF = 1;
36+
3437
/**
3538
* The elements of the property path
3639
* @var array
@@ -272,7 +275,9 @@ public function isIndex($index)
272275
*/
273276
public function getValue($objectOrArray)
274277
{
275-
return $this->readPropertyAt($objectOrArray, $this->length - 1);
278+
$propertyValues =& $this->readPropertiesUntil($objectOrArray, $this->length - 1);
279+
280+
return $propertyValues[count($propertyValues) - 1][self::VALUE];
276281
}
277282

278283
/**
@@ -306,48 +311,70 @@ public function getValue($objectOrArray)
306311
*/
307312
public function setValue(&$objectOrArray, $value)
308313
{
309-
$objectOrArray =& $this->readPropertyAt($objectOrArray, $this->length - 2);
314+
$propertyValues =& $this->readPropertiesUntil($objectOrArray, $this->length - 2);
315+
$overwrite = true;
310316

311-
if (!is_object($objectOrArray) && !is_array($objectOrArray)) {
312-
throw new UnexpectedTypeException($objectOrArray, 'object or array');
313-
}
317+
// Add the root object to the list
318+
array_unshift($propertyValues, array(
319+
self::VALUE => &$objectOrArray,
320+
self::IS_REF => true,
321+
));
314322

315-
$property = $this->elements[$this->length - 1];
316-
$singular = $this->singulars[$this->length - 1];
317-
$isIndex = $this->isIndex[$this->length - 1];
323+
for ($i = count($propertyValues) - 1; $i >= 0; --$i) {
324+
$objectOrArray =& $propertyValues[$i][self::VALUE];
325+
326+
if ($overwrite) {
327+
if (!is_object($objectOrArray) && !is_array($objectOrArray)) {
328+
throw new UnexpectedTypeException($objectOrArray, 'object or array');
329+
}
318330

319-
$this->writeProperty($objectOrArray, $property, $singular, $isIndex, $value);
331+
$property = $this->elements[$i];
332+
$singular = $this->singulars[$i];
333+
$isIndex = $this->isIndex[$i];
334+
335+
$this->writeProperty($objectOrArray, $property, $singular, $isIndex, $value);
336+
}
337+
338+
$value =& $objectOrArray;
339+
$overwrite = !$propertyValues[$i][self::IS_REF];
340+
}
320341
}
321342

322343
/**
323344
* Reads the path from an object up to a given path index.
324345
*
325346
* @param object|array $objectOrArray The object or array to read from.
326-
* @param integer $index The integer up to which should be read.
347+
* @param integer $lastIndex The integer up to which should be read.
327348
*
328-
* @return mixed The value read at the end of the path.
349+
* @return array The values read in the path.
329350
*
330351
* @throws UnexpectedTypeException If a value within the path is neither object nor array.
331352
*/
332-
protected function &readPropertyAt(&$objectOrArray, $index)
353+
private function &readPropertiesUntil(&$objectOrArray, $lastIndex)
333354
{
334-
for ($i = 0; $i <= $index; ++$i) {
355+
$propertyValues = array();
356+
357+
for ($i = 0; $i <= $lastIndex; ++$i) {
335358
if (!is_object($objectOrArray) && !is_array($objectOrArray)) {
336359
throw new UnexpectedTypeException($objectOrArray, 'object or array');
337360
}
338361

362+
$property = $this->elements[$i];
363+
$isIndex = $this->isIndex[$i];
364+
$isArrayAccess = is_array($objectOrArray) || $objectOrArray instanceof \ArrayAccess;
365+
339366
// Create missing nested arrays on demand
340-
if (is_array($objectOrArray) && !array_key_exists($this->elements[$i], $objectOrArray)) {
341-
$objectOrArray[$this->elements[$i]] = $i + 1 < $this->length ? array() : null;
367+
if ($isIndex && $isArrayAccess && !isset($objectOrArray[$property])) {
368+
$objectOrArray[$property] = $i + 1 < $this->length ? array() : null;
342369
}
343370

344-
$property = $this->elements[$i];
345-
$isIndex = $this->isIndex[$i];
371+
$propertyValue =& $this->readProperty($objectOrArray, $property, $isIndex);
372+
$objectOrArray =& $propertyValue[self::VALUE];
346373

347-
$objectOrArray =& $this->readProperty($objectOrArray, $property, $isIndex);
374+
$propertyValues[] =& $propertyValue;
348375
}
349376

350-
return $objectOrArray;
377+
return $propertyValues;
351378
}
352379

353380
/**
@@ -363,11 +390,14 @@ protected function &readPropertyAt(&$objectOrArray, $index)
363390
* @throws PropertyAccessDeniedException If the property cannot be accessed due to
364391
* access restrictions (private or protected).
365392
*/
366-
protected function &readProperty(&$objectOrArray, $property, $isIndex)
393+
private function &readProperty(&$objectOrArray, $property, $isIndex)
367394
{
368-
// The local variable (instead of immediate returns) is necessary
369-
// because we want to return by reference!
370-
$result = null;
395+
// Use an array instead of an object since performance is
396+
// very crucial here
397+
$result = array(
398+
self::VALUE => null,
399+
self::IS_REF => false
400+
);
371401

372402
if ($isIndex) {
373403
if (!$objectOrArray instanceof \ArrayAccess && !is_array($objectOrArray)) {
@@ -376,9 +406,10 @@ protected function &readProperty(&$objectOrArray, $property, $isIndex)
376406

377407
if (isset($objectOrArray[$property])) {
378408
if (is_array($objectOrArray)) {
379-
$result =& $objectOrArray[$property];
409+
$result[self::VALUE] =& $objectOrArray[$property];
410+
$result[self::IS_REF] = true;
380411
} else {
381-
$result = $objectOrArray[$property];
412+
$result[self::VALUE] = $objectOrArray[$property];
382413
}
383414
}
384415
} elseif (is_object($objectOrArray)) {
@@ -393,38 +424,45 @@ protected function &readProperty(&$objectOrArray, $property, $isIndex)
393424
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $getter, $reflClass->name));
394425
}
395426

396-
$result = $objectOrArray->$getter();
427+
$result[self::VALUE] = $objectOrArray->$getter();
397428
} elseif ($reflClass->hasMethod($isser)) {
398429
if (!$reflClass->getMethod($isser)->isPublic()) {
399430
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $isser, $reflClass->name));
400431
}
401432

402-
$result = $objectOrArray->$isser();
433+
$result[self::VALUE] = $objectOrArray->$isser();
403434
} elseif ($reflClass->hasMethod($hasser)) {
404435
if (!$reflClass->getMethod($hasser)->isPublic()) {
405436
throw new PropertyAccessDeniedException(sprintf('Method "%s()" is not public in class "%s"', $hasser, $reflClass->name));
406437
}
407438

408-
$result = $objectOrArray->$hasser();
439+
$result[self::VALUE] = $objectOrArray->$hasser();
409440
} elseif ($reflClass->hasMethod('__get')) {
410441
// needed to support magic method __get
411-
$result = $objectOrArray->$property;
442+
$result[self::VALUE] = $objectOrArray->$property;
412443
} elseif ($reflClass->hasProperty($property)) {
413444
if (!$reflClass->getProperty($property)->isPublic()) {
414445
throw new PropertyAccessDeniedException(sprintf('Property "%s" is not public in class "%s". Maybe you should create the method "%s()" or "%s()"?', $property, $reflClass->name, $getter, $isser));
415446
}
416447

417-
$result =& $objectOrArray->$property;
448+
$result[self::VALUE] =& $objectOrArray->$property;
449+
$result[self::IS_REF] = true;
418450
} elseif (property_exists($objectOrArray, $property)) {
419451
// needed to support \stdClass instances
420-
$result =& $objectOrArray->$property;
452+
$result[self::VALUE] =& $objectOrArray->$property;
453+
$result[self::IS_REF] = true;
421454
} else {
422455
throw new InvalidPropertyException(sprintf('Neither property "%s" nor method "%s()" nor method "%s()" exists in class "%s"', $property, $getter, $isser, $reflClass->name));
423456
}
424457
} else {
425458
throw new InvalidPropertyException(sprintf('Cannot read property "%s" from an array. Maybe you should write the property path as "[%s]" instead?', $property, $property));
426459
}
427460

461+
// Objects are always passed around by reference
462+
if (is_object($result[self::VALUE])) {
463+
$result[self::IS_REF] = true;
464+
}
465+
428466
return $result;
429467
}
430468

@@ -441,7 +479,7 @@ protected function &readProperty(&$objectOrArray, $property, $isIndex)
441479
* @throws PropertyAccessDeniedException If the property cannot be accessed due to
442480
* access restrictions (private or protected).
443481
*/
444-
protected function writeProperty(&$objectOrArray, $property, $singular, $isIndex, $value)
482+
private function writeProperty(&$objectOrArray, $property, $singular, $isIndex, $value)
445483
{
446484
$adderRemoverError = null;
447485

@@ -466,7 +504,8 @@ protected function writeProperty(&$objectOrArray, $property, $singular, $isIndex
466504
// At this point the add and remove methods have been found
467505
$itemsToAdd = is_object($value) ? clone $value : $value;
468506
$itemToRemove = array();
469-
$previousValue = $this->readProperty($objectOrArray, $property, $isIndex);
507+
$propertyValue = $this->readProperty($objectOrArray, $property, $isIndex);
508+
$previousValue = $propertyValue[self::VALUE];
470509

471510
if (is_array($previousValue) || $previousValue instanceof Traversable) {
472511
foreach ($previousValue as $previousItem) {
@@ -538,7 +577,7 @@ protected function writeProperty(&$objectOrArray, $property, $singular, $isIndex
538577
*
539578
* @return string The camelized version of the string.
540579
*/
541-
protected function camelize($string)
580+
private function camelize($string)
542581
{
543582
return preg_replace_callback('/(^|_|\.)+(.)/', function ($match) { return ('.' === $match[1] ? '_' : '').strtoupper($match[2]); }, $string);
544583
}

0 commit comments

Comments
 (0)
0