Skip to content

doSave() method needs exception handling for having correct value of property alreadyInSave #419

Closed
wants to merge 1 commit into from

3 participants

@grray
grray commented Jul 17, 2012

Typical generated doSave method looks like this:

    protected function doSave(PropelPDO $con)
    {
        $affectedRows = 0; // initialize var to track total num of affected rows
        if (!$this->alreadyInSave) {
            $this->alreadyInSave = true;

            if ($this->isNew() || $this->isModified()) {
                // persist changes
                if ($this->isNew()) {
                    $this->doInsert($con);
                } else {
                    $this->doUpdate($con);
                }
                $affectedRows += 1;
                $this->resetModified();
            }

            $this->alreadyInSave = false;

        }

        return $affectedRows;
    } // doSave()

When exception occurs in doInsert or doUpdate, alreadyInSave property will not be set to false, and later $obj->save() calls silently will not save object to DB. This patch fixing it by handling exception like this:

    protected function doSave(PropelPDO $con)
    {
        $affectedRows = 0; // initialize var to track total num of affected rows
        if (!$this->alreadyInSave) {
            $this->alreadyInSave = true;

            try {

                if ($this->isNew() || $this->isModified()) {
                    // persist changes
                    if ($this->isNew()) {
                        $this->doInsert($con);
                    } else {
                        $this->doUpdate($con);
                    }
                    $affectedRows += 1;
                    $this->resetModified();
                }

                $this->alreadyInSave = false;

            } catch (Exception $e) {
                $this->alreadyInSave = false;
                throw $e;
            }

        }

        return $affectedRows;
    } // doSave()
@travisbot

This pull request fails (merged 09cb31c into 55fef69).

@grray
grray commented Jul 17, 2012

Seems like failed test is about another bug, which was compensated by fixed bug)

@willdurand
Propel member

@grray heya, thanks for this fix. It seems good but could you add a unit test to prove the fix? Especially the "and later $obj->save() calls silently will not save object to DB" part. thanks!

@willdurand willdurand closed this Dec 28, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.