Skip to content

Patch too many queries sluggable #485

Merged
merged 5 commits into from Dec 10, 2012

4 participants

@jaugustin
Propel member

This PR replace #447

I squashed @Divi commits, fix CS.
I added test to prove that MakeUniqSlug was doing n query where n is the number of duplicate slug.

@jaugustin
Propel member

Do not merge now, @Divi found a bug, we need to patch it before merge

@willdurand
Propel member

Alright ;)

Divi and others added some commits Aug 14, 2012
@Divi Divi Too many queries for the makeSlugUnique method
There is a problem with the method makeSlugUnique, indeed, when you choose just one column to sluggable and if this column can be exists many time in the database, the makeSlugUnique will be launched as many times that you have a line with the same column value.

For exemple, if you have a column "label", and a categoy with label value : "Home", now try to insert new row 100 times with the label value "Home", the makeUniqueSlug with be launched n+1 times at each row.

So, I changed the method : try one time without ending separator, and if the line already exists, I try one more time but with the ending sepator and the count + 1 times of the line exists. So two queries max per line.

Miss PHP Doc'

Fix LIKE condition

Some fixes when update object & without pattern

Fix method name

Oops... fix count query

Fix object without pattern

Sorry, can't launch test myself atm.
100925e
@jaugustin jaugustin fix CS 8005052
@jaugustin jaugustin add tests to prove that sluggable queries for uniq slug don't exeed 2…
… queries
ac8c698
@jaugustin jaugustin rework patch, fix eol windows 18672bb
@jaugustin jaugustin fix slug pattern, add tests 83b7036
@jaugustin
Propel member

Wait for travis to run test but it should be good now :)

@jaugustin
Propel member

@willdurand you can merge this PR ;) all green

@willdurand willdurand merged commit ce4c833 into propelorm:master Dec 10, 2012

1 check passed

Details default The Travis build passed
@willdurand
Propel member

Thank ou guys.

@jaugustin jaugustin added a commit to jaugustin/Propel that referenced this pull request Dec 11, 2012
@jaugustin jaugustin this fix issue introduced by PR #485 f864886
@havvg havvg added a commit to Ormigo/Propel that referenced this pull request Dec 19, 2012
@havvg havvg Merge branch 'master' into ormigo
* master:
  add 'add_cleanup' option to SluggableBehavior
  Fixed typo
  this fix issue introduced by PR #485
  fix slug pattern, add tests
  rework patch, fix eol windows
  add tests to prove that sluggable queries for uniq slug don't exeed 2 queries
  fix CS
  Too many queries for the makeSlugUnique method
  fix call to support php < 5.4
  fix problems when relation object has a composite PK
0f7cb67
@havvg havvg commented on the diff Dec 19, 2012
generator/lib/behavior/sluggable/SluggableBehavior.php
$script .= "
-} elseif (!\$this->{$this->getColumnGetter()}()) {
+} elseif (";
+ $count = preg_match_all('/{([a-zA-Z]+)}/', $pattern, $matches, PREG_PATTERN_ORDER);
+
+ foreach ($matches[1] as $key => $match) {
+
+ $column = $this->getTable()->getColumn($this->underscore(ucfirst($match)));
@havvg
Propel member
havvg added a note Dec 19, 2012

This breaks conjunction with DelegateBehavior, where a column of the pattern is provided by the target class ("parent") and not by this class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.