[DC-659] Sluggable behavior does not check uniqueness on insert if a slug is manually set, causing SQL error/crash Created: 01/May/10  Updated: 05/Oct/10

Status: Open
Project: Doctrine 1
Component/s: Behaviors
Affects Version/s: 1.2.2
Fix Version/s: None

Type: Improvement Priority: Critical
Reporter: Christian Seaman Assignee: Jonathan H. Wage
Resolution: Unresolved Votes: 0
Labels: None
Environment:

symfony-1.3.4 and doctrine-1.2.2



 Description   

The Sluggable behavior has the following code:

Sluggable.php
    /**
     * Set the slug value automatically when a record is inserted
     *
     * @param Doctrine_Event $event
     * @return void
     */
    public function preInsert(Doctrine_Event $event)
    {
        $record = $event->getInvoker();
        $name = $record->getTable()->getFieldName($this->_options['name']);

        if ( ! $record->$name) {
            $record->$name = $this->buildSlugFromFields($record);
        }
    }

However, this can lead to problems...

If the user incorrectly assigns a duplicate slug to the record then there is no uniqueness checking in doctrine and you get an uncaught SQL error looking something like this:

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'my-slug-en_GB' for key 'foo_i18n_sluggable_idx'

If this kind of "don't do a preInsert check if I manunally set the slug" behavior is a FEATURE then it would be best to have an option to allow it to be disabled. If it is a BUG then I would suggest that the preInsert method should be changed to:

Sluggable.php
    /**
     * Set the slug value automatically when a record is inserted
     *
     * @param Doctrine_Event $event
     * @return void
     */
    public function preInsert(Doctrine_Event $event)
    {
        $record = $event->getInvoker();
        $name = $record->getTable()->getFieldName($this->_options['name']);

        if ( ! $record->$name) {
            $record->$name = $this->buildSlugFromFields($record);
        } else { // Still check for slug uniqueness when you insert
            $record->$name = $this->buildSlugFromSlugField($record);
        }
    }

C



 Comments   
Comment by Jonathan H. Wage [ 08/Jun/10 ]

Can you provide your changes as a patch/diff with a test case?

Comment by Christian Seaman [ 05/Oct/10 ]

Hi Jonathan,

I'm not so hot at making patches or test cases, but it should be fairly easy if you know what you're doing...

Just try to create and save two records with the same hard-coded slug and the second one will fail with an ugly MySQL crash.

If you add the lines

        } else { // Still check for slug uniqueness when you insert
            $record->$name = $this->buildSlugFromSlugField($record);

to the Sluggable::preInsert() method then this problem is averted and the test cases will pass.

I have been running with this modification in our production version of Doctrine since I first reported this in May and it all seems to work well.

If you really need me to figure out how to make a patch and test case please re-comment on this ticket and I'll see what I can do when I have some free time.

C

Generated at Fri Aug 22 15:52:31 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.