Merge pull request #2686 from oddnoc/indexes-documentation

API: Support string descriptors for unique indexes

docs/en/reference/dataobject.md

@@ -250,6 +250,35 @@ The CMS default sections as well as custom interfaces like
 `[ModelAdmin](/reference/modeladmin)` or `[GridField](/reference/gridfield)`
 already enforce these permissions.
 
+## Indexes
+
+It is sometimes desirable to add indexes to your data model, whether to
+optimize queries or add a uniqueness constraint to a field. This is done
+through the `DataObject::$indexes` map, which maps index names to descriptor
+arrays that represent each index.
+
+The general pattern for the descriptor arrays is
+
+	:::php
+	array('type' => 'index|unique|fulltext', 'value' => '"FieldA","FieldB"')
+
+You can also express the descriptor as a string.
+
+	:::php
+	'unique ("Name")'
+
+
+Note that some databases support more keywords for 'type' than shown above.
+
+Example: a unique index on a field
+
+	:::php
+	private static $db = array('SEOName' => 'Varchar(255)',);
+	private static $indexes = array(
+		'Name_IDX' => array('type' => 'unique', 'value' => '"Name"'),
+		'OtherField_IDX' => 'unique ("OtherField")',
+	);
+
 ## API Documentation
 
 `[api:DataObject]`

docs/en/topics/datamodel.md

@@ -762,6 +762,11 @@ Example: Validate postcodes based on the selected country
 		}
 	}
 
+<div class="hint" markdown='1'>
+**Tip:** If you decide to add unique or other indexes to your model via
+`static $indexes`, see [DataObject](/reference/dataobject) for details.
+</div>
+
 ## Maps
 
 A map is an array where the array indexes contain data as well as the values.

model/Versioned.php

@@ -411,12 +411,7 @@ class Versioned extends DataExtension {
 					// Extra tables for _Live, etc.
 					// Change unique indexes to 'index'.  Versioned tables may run into unique indexing difficulties
 					// otherwise.
-					foreach($indexes as $key=>$index){
+					$indexes = $this->uniqueToIndex($indexes);
-						if(is_array($index) && $index['type']=='unique'){
-							$indexes[$key]['type']='index';
-						}
-					}
-					
 					if($stage != $this->defaultStage) {
 						DB::requireTable("{$table}_$stage", $fields, $indexes, false, $options);
 					}
@@ -454,12 +449,7 @@ class Versioned extends DataExtension {
 					);
 				
 					//Unique indexes will not work on versioned tables, so we'll convert them to standard indexes:
-					foreach($indexes as $key=>$index){
+					$indexes = $this->uniqueToIndex($indexes);
-						if(is_array($index) && strtolower($index['type'])=='unique'){
-							$indexes[$key]['type']='index';
-						}
-					}
-					
 					$versionIndexes = array_merge(
 						array(
 							'RecordID_Version' => array('type' => 'unique', 'value' => '"RecordID","Version"'),
@@ -531,6 +521,35 @@ class Versioned extends DataExtension {
 		}
 	}
 	
+	/**
+	 * Helper for augmentDatabase() to find unique indexes and convert them to non-unique
+	 * 
+	 * @param array $indexes The indexes to convert
+	 * @return array $indexes
+	 */
+	private function uniqueToIndex($indexes) {
+		$unique_regex = '/unique/i';
+		$results = array();
+		foreach ($indexes as $key => $index) {
+			$results[$key] = $index;
+
+			// support string descriptors
+			if (is_string($index)) {
+				if (preg_match($unique_regex, $index)) {
+					$results[$key] = preg_replace($unique_regex, 'index', $index);
+				}
+			}
+
+			// canonical, array-based descriptors
+			elseif (is_array($index)) {
+				if (strtolower($index['type']) == 'unique') {
+					$results[$key]['type'] = 'index';
+				}
+			}
+		}
+		return $results;
+	}
+
 	/**
 	 * Augment a write-record request.
 	 *

tests/model/VersionedTest.php

@@ -12,13 +12,70 @@ class VersionedTest extends SapphireTest {
 		'VersionedTest_DataObject',
 		'VersionedTest_Subclass',
 		'VersionedTest_RelatedWithoutVersion',
-		'VersionedTest_SingleStage'
+		'VersionedTest_SingleStage',
+		'VersionedTest_WithIndexes',
 	);
 	
 	protected $requiredExtensions = array(
-		"VersionedTest_DataObject" => array('Versioned')
+		"VersionedTest_DataObject" => array('Versioned'),
+		"VersionedTest_WithIndexes" => array('Versioned'),
 	);
 
+	public function testUniqueIndexes() {
+		$table_expectations = array(
+			'VersionedTest_WithIndexes' =>
+			array('value' => 1, 'message' => 'Unique indexes are unique in main table'),
+			'VersionedTest_WithIndexes_versions' =>
+			array('value' => 0, 'message' => 'Unique indexes are no longer unique in _versions table'),
+			'VersionedTest_WithIndexes_Live' =>
+			array('value' => 0, 'message' => 'Unique indexes are no longer unique in _Live table'),
+		);
+
+		// Check for presence of all unique indexes
+		$db = DB::getConn();
+		$db_class = get_class($db);
+		$tables = array_keys($table_expectations);
+		switch ($db_class) {
+			case 'MySQLDatabase':
+				$our_indexes = array('UniqA_idx', 'UniqS_idx');
+				foreach ($tables as $t) {
+					$indexes = array_keys($db->indexList($t));
+					sort($indexes);
+					$this->assertEquals(
+							array_values($our_indexes), array_values(array_intersect($indexes, $our_indexes)),
+							"$t has both indexes");
+				}
+				break;
+			case 'SQLite3Database':
+				$our_indexes = array('"UniqA"', '"UniqS"');
+				foreach ($tables as $t) {
+					$indexes = array_values($db->indexList($t));
+					sort($indexes);
+					$this->assertEquals(array_values($our_indexes),
+							array_values(array_intersect(array_values($indexes), $our_indexes)), "$t has both indexes");
+				}
+				break;
+			default:
+				$this->markTestSkipped("Test for DBMS $db_class not implemented; skipped.");
+				break;
+		}
+
+		// Check unique -> non-unique conversion
+		foreach ($table_expectations as $table_name => $expectation) {
+			$indexes = $db->indexList($table_name);
+
+			foreach ($indexes as $idx_name => $idx_value) {
+				if (in_array($idx_name, $our_indexes)) {
+					$match_value = preg_match('/unique/', $idx_value);
+					if (false === $match_value) {
+						user_error('preg_match failure');
+					}
+					$this->assertEquals($match_value, $expectation['value'], $expectation['message']);
+				}
+			}
+		}
+	}
+
 	public function testDeletingOrphanedVersions() {
 		$obj = new VersionedTest_Subclass();
 		$obj->ExtraField = 'Foo'; // ensure that child version table gets written
@@ -521,6 +578,22 @@ class VersionedTest_DataObject extends DataObject implements TestOnly {
 
 }
 
+class VersionedTest_WithIndexes extends DataObject implements TestOnly {
+
+	private static $db = array(
+		'UniqA' => 'Int',
+		'UniqS' => 'Int',
+	);
+	private static $extensions = array(
+		"Versioned('Stage', 'Live')"
+	);
+	private static $indexes = array(
+		'UniqS_idx' => 'unique ("UniqS")',
+		'UniqA_idx' => array('type' => 'unique', 'name' => 'UniqA_idx', 'value' => '"UniqA"',),
+	);
+
+}
+
 /**
  * @package framework
  * @subpackage tests