Bug #59691
closedrepository->add must not update existing object
0%
Description
The behaviour of the repository->add() method was changed by changes https://review.typo3.org/19992/ and https://review.typo3.org/20134
The repository->add method must not update/handle an existing object that already has in identity. For Updating existing object the repository->update() method must be used.
->add() should (again) throw a proper Exception for an invalid usage of using add() for an existing object.
Updated by Gerrit Code Review over 10 years ago
- Status changed from New to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/30909
Updated by Gerrit Code Review over 10 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/30909
Updated by Mathias Brodala over 10 years ago
But this would basically require a clunky construct like this if all you care about is persisting an object, no matter if it is new or not:
if ($foo->_isNew()) { $this->fooRepository->add($foo); } else { $this->fooRepository->update($foo); }
Quite ugly if you ask me.
Also note that not even the Generic persistence manager in TYPO3 Flow cares about this. (I know that the Doctrine persistence manager does since #34527.)
Updated by Peter Niederlag over 10 years ago
I know it's not looking nice and I know I found it to be stupid some years ago myself. But ...
- first of all this was an unintentional side effect of some patches!
- it really sucks if you have a create/new style application, which is just intended to create fresh/new objects. Now you/developer must take care of securing your application so it can't be abused to modify arbitrary objects of the same type
I prefer ugly code over my data being open for unintentional manipulatation.
Until this get's cleaned I suggest everyone in create/new actions (that don't have any access check) to use something like:
if ((int)$myobject->getUid() > 1) throw new Exception("Error"); # somebody trìed to goof in an existing object
Updated by Helmut Hummel over 10 years ago
Hi!
Mathias Brodala wrote:
Also note that not even the Generic persistence manager in TYPO3 Flow cares about this.
Which is a bug
(I know that the Doctrine persistence manager does since #34527.)
.. which was only fixed in the Doctrine persistence manager
I agree with Peter, that the initial behavior (throwing an exception) should be restored.
Updated by Gerrit Code Review almost 10 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/30909
Updated by Gerrit Code Review over 9 years ago
Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/30909
Updated by Gerrit Code Review over 9 years ago
Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/30909
Updated by Gerrit Code Review over 9 years ago
Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/30909
Updated by Gerrit Code Review about 9 years ago
Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/30909
Updated by Gerrit Code Review about 9 years ago
Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/30909
Updated by Mathias Schreiber about 9 years ago
- Target version deleted (
next-patchlevel)
Updated by Anja Leichsenring almost 8 years ago
- Status changed from Under Review to New
can someone please verify this issue is still valid? I am under the impression it was solved meanwhile.
Updated by Benni Mack over 4 years ago
- Status changed from New to Closed
will close this one now, I haven't gotten feedback for three years in the issue and the review are not worked on. If somebody wants to pick this up, we can re-open the issue.