Skip to content

Commit

Permalink
Merge pull request #282 from alphadevx/bug/82-error-while-attempting-…
Browse files Browse the repository at this point in the history 
…to-create-SQLite-indexes

#82 - added the optional indexName param to the ActiveRecord::createF…
  • Loading branch information
@alphadevx
alphadevx committed May 1, 2016
2 parents 815a4eb + c927abe commit 0cff81b
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 23 deletions.
7 changes: 4 additions & 3 deletions Alpha/Model/ActiveRecord.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1757,14 +1757,15 @@ public function getIndexes()
* @param string $relatedClass The name of the related class in the format "NameObject".
* @param string $relatedClassAttribute The name of the field to relate to on the related class.
* @param bool $allowNullValues For foreign key indexes that don't allow null values, set this to false (default is true).
* @param string $indexName The optional name for the index, will calculate if not provided.
*
* @since 1.0
*
* @throws Alpha\Exception\FailedIndexCreateException
*/
public function createForeignIndex($attributeName, $relatedClass, $relatedClassAttribute)
public function createForeignIndex($attributeName, $relatedClass, $relatedClassAttribute, $indexName = null)
{
self::$logger->debug('>>createForeignIndex(attributeName=['.$attributeName.'], relatedClass=['.$relatedClass.'], relatedClassAttribute=['.$relatedClassAttribute.']');
self::$logger->debug('>>createForeignIndex(attributeName=['.$attributeName.'], relatedClass=['.$relatedClass.'], relatedClassAttribute=['.$relatedClassAttribute.'], indexName=['.$indexName.']');

$config = ConfigProvider::getInstance();

Expand All @@ -1783,7 +1784,7 @@ public function createForeignIndex($attributeName, $relatedClass, $relatedClassA
}

$provider = ActiveRecordProviderFactory::getInstance($config->get('db.provider.name'), $this);
$provider->createForeignIndex($attributeName, $relatedClass, $relatedClassAttribute);
$provider->createForeignIndex($attributeName, $relatedClass, $relatedClassAttribute, $indexName);

if (method_exists($this, 'after_createForeignIndex_callback')) {
$this->after_createForeignIndex_callback();
Expand Down
3 changes: 2 additions & 1 deletion Alpha/Model/ActiveRecordProviderInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -427,12 +427,13 @@ public function getIndexes();
* @param string $relatedClass The fully-qualified name of the related class.
* @param string $relatedClassAttribute The name of the field to relate to on the related class.
* @param bool $allowNullValues For foreign key indexes that don't allow null values, set this to false (default is true).
* @param string $indexName The optional name for the index, will calculate if not provided.
*
* @since 1.1
*
* @throws Alpha\Exception\FailedIndexCreateException
*/
public function createForeignIndex($attributeName, $relatedClass, $relatedClassAttribute);
public function createForeignIndex($attributeName, $relatedClass, $relatedClassAttribute, $indexName = null);

/**
* Creates a unique index in the database on the given attribute(s).
Expand Down
24 changes: 17 additions & 7 deletions Alpha/Model/ActiveRecordProviderMySQL.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2112,9 +2112,9 @@ private function checkIndexes()
*
* @see Alpha\Model\ActiveRecordProviderInterface::createForeignIndex()
*/
public function createForeignIndex($attributeName, $relatedClass, $relatedClassAttribute)
public function createForeignIndex($attributeName, $relatedClass, $relatedClassAttribute, $indexName = null)
{
self::$logger->debug('>>createForeignIndex(attributeName=['.$attributeName.'], relatedClass=['.$relatedClass.'], relatedClassAttribute=['.$relatedClassAttribute.']');
self::$logger->debug('>>createForeignIndex(attributeName=['.$attributeName.'], relatedClass=['.$relatedClass.'], relatedClassAttribute=['.$relatedClassAttribute.'], indexName=['.$indexName.']');

$relatedBO = new $relatedClass();
$tableName = $relatedBO->getTableName();
Expand All @@ -2125,10 +2125,16 @@ public function createForeignIndex($attributeName, $relatedClass, $relatedClassA
$sqlQuery = '';

if ($attributeName == 'leftID') {
$sqlQuery = 'ALTER TABLE '.$this->BO->getTableName().' ADD INDEX '.$this->BO->getTableName().'_leftID_fk_idx (leftID);';
if ($indexName == null) {
$indexName = $this->BO->getTableName().'_leftID_fk_idx';
}
$sqlQuery = 'ALTER TABLE '.$this->BO->getTableName().' ADD INDEX '.$indexName.' (leftID);';
}
if ($attributeName == 'rightID') {
$sqlQuery = 'ALTER TABLE '.$this->BO->getTableName().' ADD INDEX '.$this->BO->getTableName().'_rightID_fk_idx (rightID);';
if ($indexName == null) {
$indexName = $this->BO->getTableName().'_rightID_fk_idx';
}
$sqlQuery = 'ALTER TABLE '.$this->BO->getTableName().' ADD INDEX '.$indexName.' (rightID);';
}

if (!empty($sqlQuery)) {
Expand All @@ -2141,16 +2147,20 @@ public function createForeignIndex($attributeName, $relatedClass, $relatedClassA
}
}

$sqlQuery = 'ALTER TABLE '.$this->BO->getTableName().' ADD FOREIGN KEY '.$this->BO->getTableName().'_'.$attributeName.'_fk_idx ('.$attributeName.') REFERENCES '.$tableName.' ('.$relatedClassAttribute.') ON DELETE SET NULL;';
if ($indexName == null) {
$indexName = $this->BO->getTableName().'_'.$attributeName.'_fk_idx';
}

$sqlQuery = 'ALTER TABLE '.$this->BO->getTableName().' ADD FOREIGN KEY '.$indexName.' ('.$attributeName.') REFERENCES '.$tableName.' ('.$relatedClassAttribute.') ON DELETE SET NULL;';

$this->BO->setLastQuery($sqlQuery);
$result = self::getConnection()->query($sqlQuery);
}

if ($result) {
self::$logger->debug('Successfully created the foreign key index ['.$this->BO->getTableName().'_'.$attributeName.'_fk_idx]');
self::$logger->debug('Successfully created the foreign key index ['.$indexName.']');
} else {
throw new FailedIndexCreateException('Failed to create the index ['.$this->BO->getTableName().'_'.$attributeName.'_fk_idx] on ['.$this->BO->getTableName().'], error is ['.self::getConnection()->error.'], query ['.$this->BO->getLastQuery().']');
throw new FailedIndexCreateException('Failed to create the index ['.$indexName.'] on ['.$this->BO->getTableName().'], error is ['.self::getConnection()->error.'], query ['.$this->BO->getLastQuery().']');
}

self::$logger->debug('<<createForeignIndex');
Expand Down
56 changes: 45 additions & 11 deletions Alpha/Model/ActiveRecordProviderSQLite.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,15 @@ class ActiveRecordProviderSQLite implements ActiveRecordProviderInterface
*/
private $BO;

/**
* An array of new foreign keys that need to be created.
*
* @var array
*
* @since 2.0.1
*/
private $foreignKeys = array();

/**
* The constructor.
*
Expand Down Expand Up @@ -1356,6 +1365,12 @@ public function makeTable()
}
}

if (count($this->foreignKeys) > 0) {
foreach ($this->foreignKeys as $field => $related) {
$sqlQuery .= ', FOREIGN KEY ('.$field.') REFERENCES '.$related[0].'('.$related[1].')';
}
}

$sqlQuery .= ');';

$this->BO->setLastQuery($sqlQuery);
Expand Down Expand Up @@ -1989,6 +2004,21 @@ public function getIndexes()
}
}

// in SQLite foreign keys are not stored in sqlite_master, so we have to run a different query and append the results
$sqlQuery = 'PRAGMA foreign_key_list('.$this->BO->getTableName().')';

$this->BO->setLastQuery($sqlQuery);

if (!$result = self::getConnection()->query($sqlQuery)) {
self::$logger->warn('Error during pragma table foreign key lookup ['.self::getLastDatabaseError().']');
} else {
while ($row = $result->fetchArray(SQLITE3_ASSOC)) {
// SQLite does not name FK indexes, so we will return a fake name based the same convention used in MySQL
$fakeIndexName = $this->BO->getTableName().'_'.$row['from'].'_fk_idx';
array_push($indexNames, $fakeIndexName);
}
}

self::$logger->debug('<<getIndexes');

return $indexNames;
Expand Down Expand Up @@ -2054,42 +2084,46 @@ private function checkIndexes()
*
* @see Alpha\Model\ActiveRecordProviderInterface::createForeignIndex()
*/
public function createForeignIndex($attributeName, $relatedClass, $relatedClassAttribute)
public function createForeignIndex($attributeName, $relatedClass, $relatedClassAttribute, $indexName = null)
{
self::$logger->info('>>createForeignIndex(attributeName=['.$attributeName.'], relatedClass=['.$relatedClass.'], relatedClassAttribute=['.$relatedClassAttribute.']');
self::$logger->info('>>createForeignIndex(attributeName=['.$attributeName.'], relatedClass=['.$relatedClass.'], relatedClassAttribute=['.$relatedClassAttribute.'], indexName=['.$indexName.']');

/*
* High-level approach
*
* 1. Rename the source table to [tablename]_temp
* 2. Creata a new [tablename] table, with the new FK in place.
* 2. Create a new [tablename] table, with the new FK in place.
* 3. Copy all of the data from [tablename]_temp to [tablename].
* 4. Drop [tablename]_temp.
*/
try {
ActiveRecord::begin($this->BO);

// rename the table to [tablename]_temp
$query = 'ALTER TABLE '.$this->BO->getTableName().' RENAME TO '.$this->BO->getTableName().'_temp;';
// rename the table to [tablename]_temp
$query = 'ALTER TABLE '.$this->BO->getTableName().' RENAME TO '.$this->BO->getTableName().'_temp;';
$this->BO->setLastQuery($query);
self::getConnection()->query($query);

self::$logger->info('Renamed the table ['.$this->BO->getTableName().'] to ['.$this->BO->getTableName().'_temp]');

// now create the new table with the FK in place
$this->BO->makeTable();
// now create the new table with the FK in place
$record = new $relatedClass();
$tableName = $record->getTableName();
$this->foreignKeys[$attributeName] = array($tableName, $relatedClassAttribute);

$this->makeTable();

self::$logger->info('Made a new copy of the table ['.$this->BO->getTableName().']');

// copy all of the old data to the new table
$query = 'INSERT INTO '.$this->BO->getTableName().' SELECT * FROM '.$this->BO->getTableName().'_temp;';
// copy all of the old data to the new table
$query = 'INSERT INTO '.$this->BO->getTableName().' SELECT * FROM '.$this->BO->getTableName().'_temp;';
$this->BO->setLastQuery($query);
self::getConnection()->query($query);

self::$logger->info('Copied all of the data from ['.$this->BO->getTableName().'] to ['.$this->BO->getTableName().'_temp]');

// finally, drop the _temp table and commit the changes
$this->BO->dropTable($this->BO->getTableName().'_temp');
// finally, drop the _temp table and commit the changes
$this->BO->dropTable($this->BO->getTableName().'_temp');

self::$logger->info('Dropped the table ['.$this->BO->getTableName().'_temp]');

Expand Down
24 changes: 23 additions & 1 deletion test/Alpha/Test/Model/ActiveRecordTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Alpha\Model\BadRequest;
use Alpha\Model\Article;
use Alpha\Model\ArticleComment;
use Alpha\Model\ArticleVote;
use Alpha\Model\BlacklistedIP;
use Alpha\Model\Tag;
use Alpha\Model\Type\RelationLookup;
Expand Down Expand Up @@ -110,6 +111,8 @@ protected function setUp()

$article = new Article();
$article->rebuildTable();
$comment = new ArticleComment();
$comment->rebuildTable();
$tag = new Tag();
$tag->rebuildTable();
}
Expand All @@ -133,6 +136,25 @@ private function createPersonObject($name)
return $person;
}

/**
* Testing the createForeignIndex method
*
* @since 2.0.1
*
* @dataProvider getActiveRecordProviders
*/
public function testCreateForeignIndex($provider)
{
$config = ConfigProvider::getInstance();
$config->set('db.provider.name', $provider);

$article = new Article();

$article->createForeignIndex('author', 'Alpha\Model\Person', 'displayName');

$this->assertTrue(in_array('Article_author_fk_idx', $article->getIndexes()), 'Testing the createForeignIndex method');
}

/**
* Test that the constructor sets the correct values of the "house keeping" attributes.
*
Expand Down Expand Up @@ -1248,4 +1270,4 @@ public function testAddProperty()

$this->assertEquals('test value', $record->get('anotherNewStringField'), 'Testing that the new column was added to the _history table');
}
}
}

0 comments on commit 0cff81b

Please sign in to comment.