8000 [Form] PropertyPath readValue fails for custom array objects · Issue #4535 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

[Form] PropertyPath readValue fails for custom array objects #4535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
uwej711 opened this issue Jun 9, 2012 · 7 comments
Closed

[Form] PropertyPath readValue fails for custom array objects #4535

uwej711 opened this issue Jun 9, 2012 · 7 comments
Labels
Bug Form
Milestone
2.1

Comments

@uwej711
Copy link
Contributor
uwej711 commented Jun 9, 2012

the following test fails with current master (add it to PropertyPathTest.php)

public function testCustomArrayCollection()
{
    $collection = new \Symfony\Component\Form\Tests\Fixtures\CustomArrayObject(array('foo', 'bar'));
    $array = array('foo', 'bar');

    $path = new PropertyPath('[1]');
    $bar = $path->getValue($array);
    $this->assertEquals('bar', $bar);

    $bar = $path->getValue($collection);
    $this->assertEquals('bar', $bar);
}

There was 1 error:

  1. Symfony\Component\Form\Tests\Util\PropertyPathTest::testCustomArrayCollection
    Indirect modification of overloaded element of Symfony\Component\Form\Tests\Fixtures\CustomArrayObject has no effect

There seems to be a issue that getOffset does not return a reference in PropertyPath.php:375
@uwej711
Copy link
Contributor Author
uwej711 commented Jun 10, 2012

Looks like this was introduced with c2a243f

asm89 added a commit to asm89/symfony that referenced this issue Jun 18, 2012
@asm89
[Form] PropertyPath: Fix reading from custom array object. Fixes symf…
f7dd770 
…ony#4535
@asm89 asm89 mentioned this issue Jun 18, 2012
Closed
@uwej711
Copy link
Contributor Author
uwej711 commented Jun 19, 2012

See #4612

@uwej711
Copy link
Contributor Author
uwej711 commented Jun 21, 2012

I'm not totally sure but it seems that this breaks all forms with collections from Doctrine (that was the case where this issue occurred). I consider this as a blocker ...

@uwej711 uwej711 mentioned this issue Jun 22, 2012
Closed
@webmozart webmozart mentioned this issue Jul 9, 2012
Merged
@webmozart
Copy link
Contributor

Can you please verify whether this is fixed by #4806?

fabpot added a commit that referenced this issue Jul 9, 2012
@fabpot
merged branch bschussek/issue4450 (PR #4806)
35e8d08 
Commits
-------

1345360 [Form] Fixed PropertyPath handling of offsetGet() that returns a constant value
6e1462e [Form] Fixed PropertyPath handling of __get() method that returns a constant

Discussion
----------

[Form] Fixed "Indirect modification.." exceptions in PropertyPath

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #4450, #4535?, #4612
Todo: -
@uwej711
Copy link
Contributor Author
uwej711 commented Jul 9, 2012

This test still fails since you modify a copy of the array in the collection:

public function testSetCustomArrayCollection()
{
    $collection = new \Symfony\Component\Form\Tests\Fixtures\CustomArrayObject(array('foo', array('bar')));
    $path = new PropertyPath('[1][0]');
    $path->setValue($collection, 'baz');
    $this->assertEquals('baz', $collection[1][0]);
}

It worked with my solution though ...

@uwej711
Copy link
Contributor Author
uwej711 commented Jul 9, 2012

I guess you can't fix that while keeping working with the reference - offsetGet of the collection returns a copy of the array ... you need to set it to the modified array - no way of changing it directly ...

@webmozart webmozart mentioned this issue Jul 10, 2012
Merged
@webmozart
Copy link
Contributor

This test is now fixed. Thanks for the feedback!

fabpot added a commit that referenced this issue Jul 10, 2012
@fabpot
merged branch bschussek/issue4535 (PR #4827)
c8c26d0 
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: -
@fabpot fabpot closed this as completed Jul 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Form
Projects
None yet
Milestone
2.1
Development

No branches or pull requests
3 participants
@fabpot @webmozart @uwej711
0