Fix Slugged.

docs/Upgrade.md

@@ -22,3 +22,4 @@
 ## Behavior
 - `run`/`before` config options for callback decisions have been unified to `on` and complete callback/event name, e.g. `'on' => 'beforeValidate'`.
 - model names are now table names, and plural.
+- Slugged option "slugField" is now "field"

src/Model/Behavior/SluggedBehavior.php

@@ -176,12 +176,36 @@ class SluggedBehavior extends Behavior {
 	 * @return void
 	 */
 	public function slug(Entity $entity) {
-		foreach ((array)$this->_config['label'] as $k => $v) {
-			break;
+		$overwrite = $this->_config['overwrite'];
+		if (!$overwrite && $entity->get($this->_config['overwriteField'])) {
+			$overwrite = true;
 		}
-		$value = $entity->get($v);
+		if ($overwrite || $entity->isNew()) {
+			$slug = array();
+			foreach ((array)$this->_config['label'] as $v) {
+				$v = $this->generateSlug($entity->get($v), $entity);
+				if ($v) {
+					$slug[] = $v;
+				}
+			}
+			$slug = implode($slug, $this->_config['separator']);
+			$entity->set($this->_config['field'], $slug);
+		}
+	}
 
-		$entity->set($this->_config['field'], $this->generateSlug($entity, $value));
+	/**
+	 * SluggedBehavior::_needsUpdating()
+	 *
+	 * @param Entity $entity
+	 * @return bool
+	 */
+	protected function _needsUpdating($entity) {
+		foreach ((array)$this->_config['label'] as $label) {
+			if ($entity->dirty($label)) {
+				return true;
+			}
+		}
+		return false;
 	}
 
 	/**
@@ -192,14 +216,12 @@ class SluggedBehavior extends Behavior {
 	 *
 	 * @return void
 	 */
-	public function _generateSlug(Model $Model) {
-		if ($this->_config['length'] && !$Model->hasField($this->_config['field'])) {
-			return;
-		}
-		if (!$overwrite && !empty($Model->data[$this->_table->alias()][$overwriteField])) {
+	public function _slug(Entity $entity) {
+		$overwrite = $this->config['overwrite'];
+		if (!$overwrite && !$entity->get($overwriteField)) {
 			$overwrite = true;
 		}
-		if ($overwrite || !$Model->id) {
+		if ($overwrite || $entity->isNew()) {
 			if ($label) {
 				$somethingToDo = false;
 				foreach ($label as $field) {
@@ -249,14 +271,12 @@ class SluggedBehavior extends Behavior {
 	 * If unique is set to true, check for a unique slug and if unavailable suffix the slug with -1, -2, -3 etc.
 	 * until a unique slug is found
 	 *
-	 * @param Model $Model
-	 * @param mixed $string
-	 * @param boolean $tidy
+	 * @param string $string
+	 * @param Entity $entity
 	 * @return string a slug
 	 */
-	public function generateSlug(Entity $entity, $value) {
+	public function generateSlug($value, Entity $entity = null) {
 		$separator = $this->_config['separator'];
-		$case = $this->_config['case'];
 
 		$string = str_replace(array("\r\n", "\r", "\n"), ' ', $value);
 		if ($replace = $this->_config['replace']) {
@@ -279,13 +299,11 @@ class SluggedBehavior extends Behavior {
 				$slug = 'x' . $slug;
 			}
 		}
-		if ($this->_config['length'] & (strlen($slug) > $this->_config['length'])) {
+		if ($this->_config['length'] && (mb_strlen($slug) > $this->_config['length'])) {
 			$slug = mb_substr($slug, 0, $this->_config['length']);
-			while ($slug && strlen($slug) > $this->_config['length']) {
-				$slug = mb_substr($slug, 0, mb_strlen($slug) - 1);
-			}
 		}
-		if ($case) {
+		if ($this->_config['case']) {
+			$case = $this->_config['case'];
 			if ($case === 'up') {
 				$slug = mb_strtoupper($slug);
 			} else {
@@ -307,6 +325,9 @@ class SluggedBehavior extends Behavior {
 			}
 		}
 		if ($this->_config['unique']) {
+			if (!$entity) {
+				throw new \Exception('Needs an Entity to work on');
+			}
 			$field = $this->_table->alias() . '.' . $this->_config['field'];
 			$conditions = array($field => $slug);
 			$conditions = array_merge($conditions, $this->_config['scope']);
@@ -319,8 +340,8 @@ class SluggedBehavior extends Behavior {
 			while ($this->_table->exists($conditions)) {
 				$i++;
 				$suffix	= $separator . $i;
-				if ($this->_config['length'] && (strlen($slug . $suffix) > $this->_config['length'])) {
-					$slug = substr($slug, 0, $this->_config['length'] - strlen($suffix));
+				if ($this->_config['length'] && (mb_strlen($slug . $suffix) > $this->_config['length'])) {
+					$slug = mb_substr($slug, 0, $this->_config['length'] - mb_strlen($suffix));
 				}
 				$conditions[$field] = $slug . $suffix;
 			}
@@ -405,23 +426,27 @@ class SluggedBehavior extends Behavior {
 	 * Handle both slug and label fields using the translate behavior, and being edited
 	 * in multiple locales at once
 	 *
+	 * //FIXME
 	 * @param Model $Model
 	 * @return void
 	 */
 	protected function _multiSlug(Entity $entity) {
 		extract($this->_config);
-		$data = $Model->data;
 		$field = current($label);
-		foreach ($Model->data[$this->_table->alias()][$field] as $locale => $_) {
+		$fields = (array)$entity->get($field);
+
+		$locale = array();
+		foreach ($fields as $locale => $_) {
 			foreach ($label as $field) {
-				if (is_array($data[$this->_table->alias()][$field])) {
-					$Model->data[$this->_table->alias()][$field] = $Model->slug($data[$this->_table->alias()][$field][$locale]);
+				$res = $entity->get($field);
+				if (is_array($entity->get($field))) {
+					$res = $this->generateSlug($field[$locale], $entity);
 				}
 			}
-			$this->beforeValidate($Model);
-			$data[$this->_table->alias()][$slugField][$locale] = $Model->data[$this->_table->alias()][$field];
+			//$this->beforeValidate($entity);
+			$locale[$locale] = $res;
 		}
-		$Model->data = $data;
+		$entity->set($slugField, $locale);
 	}
 
 	/**

tests/TestCase/Model/Behavior/SluggedBehaviorTest.php

@@ -36,8 +36,9 @@ class SluggedBehaviorTest extends TestCase {
 
 		$options = ['alias' => 'Articles'];
 		$this->articles = TableRegistry::get('SluggedArticles', $options);
-
 		Configure::delete('Slugged');
+
+		$this->articles->addBehavior('Tools.Slugged');
 	}
 
 /**
@@ -58,8 +59,6 @@ class SluggedBehaviorTest extends TestCase {
  * @return void
  */
 	public function testAdd() {
-		$this->articles->addBehavior('Tools.Slugged');
-
 		$entity = $this->_getEntity();
 		$result = $this->articles->save($entity);
 
@@ -72,7 +71,7 @@ class SluggedBehaviorTest extends TestCase {
  * @return void
  */
 	public function testAddUnique() {
-		$this->articles->addBehavior('Tools.Slugged', ['unique' => true]);
+		$this->articles->behaviors()->Slugged->config(['unique' => true]);
 
 		$entity = $this->_getEntity();
 		$result = $this->articles->save($entity);
@@ -90,7 +89,6 @@ class SluggedBehaviorTest extends TestCase {
  * @return void
  */
 	public function testCustomFinder() {
-		$this->articles->addBehavior('Tools.Slugged');
 		$article = $this->articles->find()->find('slugged', ['slug' => 'foo'])->first();
 		$this->assertEquals('Foo', $article->get('title'));
 	}
@@ -101,17 +99,16 @@ class SluggedBehaviorTest extends TestCase {
  * @return void
  */
 	public function testLengthRestrictionManual() {
-		$this->articles->addBehavior('Tools.Slugged', ['length' => 155]);
+		$this->articles->behaviors()->Slugged->config(['length' => 155]);
 		$entity = $this->_getEntity(str_repeat('foo bar ', 31));
 
 		$result = $this->articles->save($entity);
 		$this->assertEquals(155, strlen($result->get('slug')));
 
-		// For non ascii chars it might be longer, though...
 		$this->articles->behaviors()->Slugged->config(['length' => 10, 'mode' => 'ascii']);
 		$entity = $this->_getEntity('ä ö ü ä ö ü');
 		$result = $this->articles->save($entity);
-		$this->assertEquals('ae-oe-ue-ae-oe-ue', $result->get('slug'));
+		$this->assertEquals('ae-oe-ue-a', $result->get('slug'));
 	}
 
 /**
@@ -120,7 +117,6 @@ class SluggedBehaviorTest extends TestCase {
  * @return void
  */
 	public function testLengthRestrictionAutoDetect() {
-		$this->articles->addBehavior('Tools.Slugged');
 		$entity = $this->_getEntity(str_repeat('foo bar ', 31));
 
 		$result = $this->articles->save($entity);
@@ -133,7 +129,7 @@ class SluggedBehaviorTest extends TestCase {
  * @return void
  */
 	public function testLengthRestrictionNoLimit() {
-		$this->articles->addBehavior('Tools.Slugged', ['length' => 0, 'label' => 'long_title', 'field' => 'long_slug']);
+		$this->articles->behaviors()->Slugged->config(['length' => 0, 'label' => 'long_title', 'field' => 'long_slug']);
 		$entity = $this->_getEntity(str_repeat('foo bar ', 100), 'long_title');
 
 		$result = $this->articles->save($entity);
@@ -146,6 +142,8 @@ class SluggedBehaviorTest extends TestCase {
 	 * @return void
 	 */
 	public function testResetSlugs() {
+		$this->articles->removeBehavior('Slugged');
+
 		$article = $this->articles->newEntity(array('title' => 'Andy Dawson', 'slug' => 'foo'));
 		$this->articles->save($article);
 		$article = $this->articles->newEntity(array('title' => 'Andy Dawsom', 'slug' => 'bar'));
@@ -188,7 +186,7 @@ class SluggedBehaviorTest extends TestCase {
 	public function testDuplicateWithLengthRestriction() {
 		return;
 
-		$this->articles->addBehavior('Tools.Slugged', ['length' => 10, 'unique' => true]);
+		$this->articles->behaviors()->Slugged->config(['length' => 10, 'unique' => true]);
 
 		$article = $this->articles->newEntity(array('title' => 'Andy Dawson'));
 		$this->articles->save($article);
@@ -218,7 +216,7 @@ class SluggedBehaviorTest extends TestCase {
 			'fields' => array('title', 'slug'),
 			'order' => 'title'
 		))->combine('title', 'slug')->toArray();
-		$expects = array(
+		$expected = array(
 			'Andy Dawson' => 'Andy-Dawso',
 			'Andy Dawsom' => 'Andy-Daw-1',
 			'Andy Dawsoo' => 'Andy-Daw-2',
@@ -231,7 +229,243 @@ class SluggedBehaviorTest extends TestCase {
 			'Andy Dawso9' => 'Andy-Daw-9',
 			'Andy Dawso0' => 'Andy-Da-10'
 		);
-		$this->assertEquals($expects, $result);
+		$this->assertEquals($expected, $result);
+	}
+
+	/**
+	 * TestTruncateMultibyte method
+	 *
+	 * @return void
+	 */
+	/**
+	 * TestTruncateMultibyte method
+	 *
+	 * Ensure that the first test doesn't cut a multibyte character The test string is:
+	 * 	17 chars
+	 * 	51 bytes UTF-8 encoded
+	 *
+	 * @return void
+	 */
+	public function testTruncateMultibyte() {
+		$this->articles->behaviors()->Slugged->config(array('length' => 16));
+
+		$result = $this->articles->generateSlug('モデルのデータベースとデータソース');
+		$expected = 'モデルのデータベースとデータソー';
+		$this->assertEquals($expected, $result);
+	}
+
+	/**
+	 * Test Url method
+	 *
+	 * @return void
+	 */
+	public function testUrlMode() {
+		$this->articles->behaviors()->Slugged->config(array('mode' => 'url', 'replace' => false));
+
+		$string = 'standard string';
+		$expected = 'standard-string';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a \' in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a " in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a / in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a ? in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a < in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a > in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a . in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a $ in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a / in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a : in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a ; in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a ? in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a @ in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a = in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a + in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a & in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a % in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a \ in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+
+		$string = 'something with a # in it';
+		$expected = 'something-with-a-in-it';
+		$result = $this->articles->generateSlug($string);
+		$this->assertEquals($expected, $result);
+	}
+
+
+	/**
+	 * Test slug with ascii
+	 *
+	 * @return void
+	 */
+	public function testSlugGenerationModeAscii() {
+		$this->articles->removeBehavior('Slugged');
+		$this->articles->addBehavior('Tools.Slugged', array(
+			'mode' => 'ascii'));
+
+		$article = $this->articles->newEntity(array('title' => 'Some Article 25271'));
+		$result = $this->articles->save($article);
+		$this->assertTrue((bool)$result);
+
+		$this->assertEquals('Some-Article-25271', $result['slug']);
+	}
+
+	/**
+	 * Test slug generation/update on beforeSave
+	 *
+	 * @return void
+	 */
+	public function testSlugGenerationBeforeSave() {
+		$this->articles->removeBehavior('Slugged');
+		$this->articles->addBehavior('Tools.Slugged', array(
+			'on' => 'beforeSave', 'overwrite' => true));
+
+		$article = $this->articles->newEntity(array('title' => 'Some Article 25271'));
+		$result = $this->articles->save($article);
+
+		//$result['id'] = $result['id'];
+		$this->assertEquals('Some-Article-25271', $result['slug']);
+	}
+
+	/**
+	 * Test slug generation with i18n replacement pieces
+	 *
+	 * @return void
+	 */
+	public function testSlugGenerationI18nReplacementPieces() {
+		$this->articles->removeBehavior('Slugged');
+		$this->articles->addBehavior('Tools.Slugged', array(
+			'overwrite' => true));
+
+		$article = $this->articles->newEntity(array('title' => 'Some & More'));
+		$result = $this->articles->save($article);
+		$this->assertEquals('Some-' . __d('tools', 'and') . '-More', $result['slug']);
+	}
+
+	/**
+	 * Test dynamic slug overwrite
+	 *
+	 * @return void
+	 */
+	public function testSlugDynamicOverwrite() {
+		$this->articles->removeBehavior('Slugged');
+		$this->articles->addBehavior('Tools.Slugged', array(
+			'overwrite' => false, 'overwriteField' => 'overwrite_my_slug'));
+
+		$article = $this->articles->newEntity(array('title' => 'Some Cool String', 'overwrite_my_slug' => false));
+		$result = $this->articles->save($article);
+		$this->assertEquals('Some-Cool-String', $result['slug']);
+
+		$this->articles->patchEntity($article, ['title' => 'Some Cool Other String']);
+		$result = $this->articles->save($article);
+		$this->assertEquals('Some-Cool-String', $result['slug']);
+
+		$this->articles->patchEntity($article, ['title' => 'Some Cool Other String', 'overwrite_my_slug' => true]);
+		$result = $this->articles->save($article);
+		$this->assertEquals('Some-Cool-Other-String', $result['slug']);
+	}
+
+	/**
+	 * Test slug generation/update based on scope
+	 *
+	 * @return void
+	 */
+	public function testSlugGenerationWithScope() {
+		$this->articles->removeBehavior('Slugged');
+		$this->articles->addBehavior('Tools.Slugged', array('unique' => true));
+
+		$data = array('title' => 'Some Article 12345', 'section' => 0);
+
+		$article = $this->articles->newEntity($data);
+		$result = $this->articles->save($article);
+		$this->assertTrue((bool)$result);
+		$this->assertEquals('Some-Article-12345', $result['slug']);
+
+		$article = $this->articles->newEntity($data);
+		$result = $this->articles->save($article);
+		$this->assertTrue((bool)$result);
+		$this->assertEquals('Some-Article-12345-1', $result['slug']);
+
+		$this->articles->removeBehavior('Slugged');
+		$this->articles->addBehavior('Tools.Slugged', array('unique' => true, 'scope' => array('section' => 1)));
+
+		$data = array('title' => 'Some Article 12345', 'section' => 1);
+
+		$article = $this->articles->newEntity($data);
+		$result = $this->articles->save($article);
+		$this->assertTrue((bool)$result);
+		$this->assertEquals('Some-Article-12345', $result['slug']);
 	}
 
 /**