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.

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

[Edit form] User is persisted even with invalid data #2822

Closed
lobodol opened this Issue Jul 23, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@lobodol

lobodol commented Jul 23, 2018

Hello FOS community,

I face something strange with the UserBundle. Here are the details:

Configuration:

  • Symfony: 2.8
  • FOSUserBundle: master
  • php: 7.1
  • OS: Linux Mint
  • Apache: 2.0
  • MySQL: 5.7

I created a new SF2.8 project and installed FOSUserBundle following the official documentation.

Moreover, I added an event subscriber doing an EntityManager::flush() on each Kernel Request (it creates & persists a new entity, not related to the User entity).

Test procedure:

  1. Create a user with CLI
php app/console fos:user:create
  1. Log in and go to the profile edit form.

  2. Type a username only composed of spaces and submit.

The validation fails and an error message is displayed "Please enter a username." as expected.
But the modification is persisted in database :

SELECT * FROM user
+----+----------+--------------------+------------+-----------------+---------+------+--------------------------------------------------------------+---------------------+--------------------+-----------------------+--------+
| id | username | username_canonical | email      | email_canonical | enabled | salt | password                                                     | last_login          | confirmation_token | password_requested_at | roles  |
+----+----------+--------------------+------------+-----------------+---------+------+--------------------------------------------------------------+---------------------+--------------------+-----------------------+--------+
|  1 |          |                    | foo@foo.fr | foo@foo.fr      |       1 | NULL | $2y$13$y6gpr4jHQtNYi8ehRASFUedaZIPv7I1u7Z4AXZu7.j5Dhl2itOYQi | 2018-07-23 10:59:33 | NULL               | NULL                  | a:0:{} |
+----+----------+--------------------+------------+-----------------+---------+------+--------------------------------------------------------------+---------------------+--------------------+-----------------------+--------+

If I disable my event subscriber responsible of the EntityManager::flush(), the modification is not persisted anymore.

Do I misunderstand something ?
Is it a bug in the bundle ?
Why the User entity is persisted whereas it should not ?

Regards,

@lobodol

This comment has been minimized.

Show comment
Hide comment
@lobodol

lobodol Sep 4, 2018

Any news about this issue ?

lobodol commented Sep 4, 2018

Any news about this issue ?

@Devtronic

This comment has been minimized.

Show comment
Hide comment
@Devtronic

Devtronic Sep 4, 2018

Please post the content of your edit method.

Devtronic commented Sep 4, 2018

Please post the content of your edit method.

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Sep 4, 2018

Member

Well, if you have an event listener forcing a flush, you are creating the bug. As soon as an object is known by Doctrine, changes in it will be saved on flush. And during edition, the object being edited is already known by Doctrine, as it was loaded from the DB.

If you want to always do flushes automatically in a listener and avoid getting issues for invalid objects, you would need to use the explicit change tracking feature of Doctrine (which would consider changes only for objects on which persist was called again).
FOSUserBundle is compatible with this feature (we always call persist in our own code wanting to save changes, even on edition), but you have to enable it in your User entity if you want to use it.

I'm closing this issue, because the bug is not in FOSUserBundle, but in the way you use Doctrine in your project.

Member

stof commented Sep 4, 2018

Well, if you have an event listener forcing a flush, you are creating the bug. As soon as an object is known by Doctrine, changes in it will be saved on flush. And during edition, the object being edited is already known by Doctrine, as it was loaded from the DB.

If you want to always do flushes automatically in a listener and avoid getting issues for invalid objects, you would need to use the explicit change tracking feature of Doctrine (which would consider changes only for objects on which persist was called again).
FOSUserBundle is compatible with this feature (we always call persist in our own code wanting to save changes, even on edition), but you have to enable it in your User entity if you want to use it.

I'm closing this issue, because the bug is not in FOSUserBundle, but in the way you use Doctrine in your project.

@stof stof closed this Sep 4, 2018

@lobodol

This comment has been minimized.

Show comment
Hide comment
@lobodol

lobodol Sep 4, 2018

Thank you for your reply.

I believed FOSUserBundle did not call persist until the User entity is not valid thus, I though I could call flush without problem.

Indeed, it's my way of use of Doctrine that creates the bug.
Here is a link about Doctrine's change tracking policies for those who are interested.

Regards,

lobodol commented Sep 4, 2018

Thank you for your reply.

I believed FOSUserBundle did not call persist until the User entity is not valid thus, I though I could call flush without problem.

Indeed, it's my way of use of Doctrine that creates the bug.
Here is a link about Doctrine's change tracking policies for those who are interested.

Regards,

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Sep 4, 2018

Member

@lobodol we don't call persist if the object is not valid. But during edition, calling it is not needed for Doctrine when using implicit change tracking.

Member

stof commented Sep 4, 2018

@lobodol we don't call persist if the object is not valid. But during edition, calling it is not needed for Doctrine when using implicit change tracking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment