From 27be739d5e4a41bfc6658efed7b087a0f57efc0b Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 19 May 2022 11:29:01 +0200 Subject: [PATCH 1/7] feat: improve migration speed --- src/Appwrite/Migration/Migration.php | 50 +++++++++------------------- 1 file changed, 15 insertions(+), 35 deletions(-) diff --git a/src/Appwrite/Migration/Migration.php b/src/Appwrite/Migration/Migration.php index b479a4226..844923993 100644 --- a/src/Appwrite/Migration/Migration.php +++ b/src/Appwrite/Migration/Migration.php @@ -124,21 +124,7 @@ abstract class Migration $old = $document->getArrayCopy(); $new = call_user_func($callback, $document); - foreach ($document as &$attr) { - if ($attr instanceof Document) { - $attr = call_user_func($callback, $attr); - } - - if (\is_array($attr)) { - foreach ($attr as &$child) { - if ($child instanceof Document) { - $child = call_user_func($callback, $child); - } - } - } - } - - if (!$this->check_diff_multi($new->getArrayCopy(), $old)) { + if (!$this->hasDifference($new->getArrayCopy(), $old)) { return; } @@ -170,32 +156,26 @@ abstract class Migration * * @param array $array1 * @param array $array2 - * @return array + * @return bool */ - public function check_diff_multi(array $array1, array $array2): array + public function hasDifference(array $array1, array $array2): bool { - $result = array(); - - foreach ($array1 as $key => $val) { - if (is_array($val) && isset($array2[$key])) { - $tmp = $this->check_diff_multi($val, $array2[$key]); - if ($tmp) { - $result[$key] = $tmp; + foreach ($array1 as $key => $value) { + if (is_array($value)) { + if (!isset($array2[$key]) || !is_array($array2[$key])) { + return true; + } else { + $new_diff = $this->hasDifference($value, $array2[$key]); + if ($new_diff) { + return true; + } } - } elseif (!isset($array2[$key])) { - $result[$key] = null; - } elseif ($val !== $array2[$key]) { - $result[$key] = $array2[$key]; - } - - if (isset($array2[$key])) { - unset($array2[$key]); + } elseif (!array_key_exists($key, $array2) || $array2[$key] !== $value) { + return true; } } - $result = array_merge($result, $array2); - - return $result; + return false; } /** From 4cbf3b1e12079fec4c4a723234ceb63e8dec0f28 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 19 May 2022 11:49:48 +0200 Subject: [PATCH 2/7] push debug logs --- src/Appwrite/Migration/Migration.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Appwrite/Migration/Migration.php b/src/Appwrite/Migration/Migration.php index 844923993..14e931c31 100644 --- a/src/Appwrite/Migration/Migration.php +++ b/src/Appwrite/Migration/Migration.php @@ -16,7 +16,7 @@ abstract class Migration /** * @var int */ - protected int $limit = 100; + protected int $limit = 250; /** * @var Document @@ -108,7 +108,9 @@ abstract class Migration Console::log('Migrating Collection ' . $collection['$id'] . ':'); do { + $start = microtime(true); $documents = $this->projectDB->find($collection['$id'], limit: $this->limit, cursor: $nextDocument); + var_dump("Fetching documents took " . microtime(true) - $start); $count = count($documents); $sum += $count; @@ -121,10 +123,17 @@ abstract class Migration return; } + $start = microtime(true); + $old = $document->getArrayCopy(); $new = call_user_func($callback, $document); + var_dump("migration took " . microtime(true) - $start); - if (!$this->hasDifference($new->getArrayCopy(), $old)) { + $start = microtime(true); + $diff = !$this->hasDifference($new->getArrayCopy(), $old); + var_dump("Diff took " . microtime(true) - $start); + + if (!$diff) { return; } From 2fcbaee0b65dd66d07deff5feb015954d8bca8ee Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 19 May 2022 11:53:06 +0200 Subject: [PATCH 3/7] Revert "push debug logs" This reverts commit 4cbf3b1e12079fec4c4a723234ceb63e8dec0f28. --- src/Appwrite/Migration/Migration.php | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/Appwrite/Migration/Migration.php b/src/Appwrite/Migration/Migration.php index 14e931c31..844923993 100644 --- a/src/Appwrite/Migration/Migration.php +++ b/src/Appwrite/Migration/Migration.php @@ -16,7 +16,7 @@ abstract class Migration /** * @var int */ - protected int $limit = 250; + protected int $limit = 100; /** * @var Document @@ -108,9 +108,7 @@ abstract class Migration Console::log('Migrating Collection ' . $collection['$id'] . ':'); do { - $start = microtime(true); $documents = $this->projectDB->find($collection['$id'], limit: $this->limit, cursor: $nextDocument); - var_dump("Fetching documents took " . microtime(true) - $start); $count = count($documents); $sum += $count; @@ -123,17 +121,10 @@ abstract class Migration return; } - $start = microtime(true); - $old = $document->getArrayCopy(); $new = call_user_func($callback, $document); - var_dump("migration took " . microtime(true) - $start); - $start = microtime(true); - $diff = !$this->hasDifference($new->getArrayCopy(), $old); - var_dump("Diff took " . microtime(true) - $start); - - if (!$diff) { + if (!$this->hasDifference($new->getArrayCopy(), $old)) { return; } From 706ecc6b409956ec573d714488c7afaefb30386b Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 19 May 2022 13:53:11 +0200 Subject: [PATCH 4/7] fix: migration method --- src/Appwrite/Migration/Migration.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Appwrite/Migration/Migration.php b/src/Appwrite/Migration/Migration.php index 844923993..0944b6e90 100644 --- a/src/Appwrite/Migration/Migration.php +++ b/src/Appwrite/Migration/Migration.php @@ -153,10 +153,10 @@ abstract class Migration /** * Checks 2 arrays for differences. - * - * @param array $array1 - * @param array $array2 - * @return bool + * + * @param array $array1 + * @param array $array2 + * @return bool */ public function hasDifference(array $array1, array $array2): bool { @@ -165,8 +165,7 @@ abstract class Migration if (!isset($array2[$key]) || !is_array($array2[$key])) { return true; } else { - $new_diff = $this->hasDifference($value, $array2[$key]); - if ($new_diff) { + if ($this->hasDifference($value, $array2[$key])) { return true; } } From 0a559a3562b4850540f45f65e3c3e66df24382b0 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 19 May 2022 14:17:18 +0200 Subject: [PATCH 5/7] tests: add more migration tests --- src/Appwrite/Migration/Migration.php | 6 +-- tests/unit/Migration/MigrationTest.php | 71 ++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/Appwrite/Migration/Migration.php b/src/Appwrite/Migration/Migration.php index 0944b6e90..99b3f9236 100644 --- a/src/Appwrite/Migration/Migration.php +++ b/src/Appwrite/Migration/Migration.php @@ -124,7 +124,7 @@ abstract class Migration $old = $document->getArrayCopy(); $new = call_user_func($callback, $document); - if (!$this->hasDifference($new->getArrayCopy(), $old)) { + if (!self::hasDifference($new->getArrayCopy(), $old)) { return; } @@ -158,14 +158,14 @@ abstract class Migration * @param array $array2 * @return bool */ - public function hasDifference(array $array1, array $array2): bool + public static function hasDifference(array $array1, array $array2): bool { foreach ($array1 as $key => $value) { if (is_array($value)) { if (!isset($array2[$key]) || !is_array($array2[$key])) { return true; } else { - if ($this->hasDifference($value, $array2[$key])) { + if (self::hasDifference($value, $array2[$key])) { return true; } } diff --git a/tests/unit/Migration/MigrationTest.php b/tests/unit/Migration/MigrationTest.php index 340e33772..fb7b47c0d 100644 --- a/tests/unit/Migration/MigrationTest.php +++ b/tests/unit/Migration/MigrationTest.php @@ -4,6 +4,7 @@ namespace Appwrite\Tests; use Appwrite\Migration\Migration; use PHPUnit\Framework\TestCase; +use ReflectionClass; use ReflectionMethod; use Utopia\Database\Document; @@ -36,12 +37,76 @@ abstract class MigrationTest extends TestCase */ public function testMigrationVersions() { - require_once __DIR__.'/../../../app/init.php'; + require_once __DIR__ . '/../../../app/init.php'; foreach (Migration::$versions as $class) { - $this->assertTrue(class_exists('Appwrite\\Migration\\Version\\'.$class)); + $this->assertTrue(class_exists('Appwrite\\Migration\\Version\\' . $class)); } // Test if current version exists - //$this->assertArrayHasKey(APP_VERSION_STABLE, Migration::$versions); + $this->assertArrayHasKey(APP_VERSION_STABLE, Migration::$versions); + } + + public function testHasDifference() + { + $this->assertFalse(Migration::hasDifference([], [])); + $this->assertFalse(Migration::hasDifference([ + 'a' => true, + 'b' => 'abc', + 'c' => 123, + 'd' => ['a', 'b', 'c'], + 'nested' => [ + 'a' => true, + 'b' => 'abc', + 'c' => 123, + 'd' => ['a', 'b', 'c'] + ] + ], [ + 'a' => true, + 'b' => 'abc', + 'c' => 123, + 'd' => ['a', 'b', 'c'], + 'nested' => [ + 'a' => true, + 'b' => 'abc', + 'c' => 123, + 'd' => ['a', 'b', 'c'] + ] + ])); + $this->assertTrue(Migration::hasDifference([ + 'a' => true + ], [ + 'b' => true + ])); + $this->assertTrue(Migration::hasDifference([ + 'a' => 'true' + ], [ + 'a' => true + ])); + $this->assertTrue(Migration::hasDifference([ + 'a' => true + ], [ + 'a' => false + ])); + $this->assertTrue(Migration::hasDifference([ + 'nested' => [ + 'a' => true + ] + ], [ + 'nested' => [] + ])); + $this->assertTrue(Migration::hasDifference([ + 'nested' => [ + 'a' => true, + 'b' => 'abc', + 'c' => 123, + 'd' => ['a', 'b', 'c'] + ] + ], [ + 'nested' => [ + 'a' => true, + 'c' => '123', + 'd' => ['a', 'b', 'c'] + ] + ])); } } From 33486ee2eb124a291bae82f89abe7daac97484fb Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 19 May 2022 14:18:55 +0200 Subject: [PATCH 6/7] tests: fix naming in migration --- tests/unit/Migration/MigrationTest.php | 34 +++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/unit/Migration/MigrationTest.php b/tests/unit/Migration/MigrationTest.php index fb7b47c0d..49aee87f5 100644 --- a/tests/unit/Migration/MigrationTest.php +++ b/tests/unit/Migration/MigrationTest.php @@ -50,22 +50,22 @@ abstract class MigrationTest extends TestCase { $this->assertFalse(Migration::hasDifference([], [])); $this->assertFalse(Migration::hasDifference([ - 'a' => true, - 'b' => 'abc', - 'c' => 123, - 'd' => ['a', 'b', 'c'], - 'nested' => [ + 'bool' => true, + 'string' => 'abc', + 'int' => 123, + 'array' => ['a', 'b', 'c'], + 'assoc' => [ 'a' => true, 'b' => 'abc', 'c' => 123, 'd' => ['a', 'b', 'c'] ] ], [ - 'a' => true, - 'b' => 'abc', - 'c' => 123, - 'd' => ['a', 'b', 'c'], - 'nested' => [ + 'bool' => true, + 'string' => 'abc', + 'int' => 123, + 'array' => ['a', 'b', 'c'], + 'assoc' => [ 'a' => true, 'b' => 'abc', 'c' => 123, @@ -95,17 +95,17 @@ abstract class MigrationTest extends TestCase 'nested' => [] ])); $this->assertTrue(Migration::hasDifference([ - 'nested' => [ - 'a' => true, - 'b' => 'abc', - 'c' => 123, - 'd' => ['a', 'b', 'c'] + 'assoc' => [ + 'bool' => true, + 'string' => 'abc', + 'int' => 123, + 'array' => ['a', 'b', 'c'] ] ], [ 'nested' => [ 'a' => true, - 'c' => '123', - 'd' => ['a', 'b', 'c'] + 'int' => '123', + 'array' => ['a', 'b', 'c'] ] ])); } From 3199baa14fac55d0ed1a5b14c4929a862bc03224 Mon Sep 17 00:00:00 2001 From: Torsten Dittmann Date: Thu, 19 May 2022 14:21:21 +0200 Subject: [PATCH 7/7] tests: add more variety to migration tests --- tests/unit/Migration/MigrationTest.php | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/unit/Migration/MigrationTest.php b/tests/unit/Migration/MigrationTest.php index 49aee87f5..0c8c34cbc 100644 --- a/tests/unit/Migration/MigrationTest.php +++ b/tests/unit/Migration/MigrationTest.php @@ -72,6 +72,30 @@ abstract class MigrationTest extends TestCase 'd' => ['a', 'b', 'c'] ] ])); + $this->assertFalse(Migration::hasDifference([ + 'bool' => true, + 'string' => 'abc', + 'int' => 123, + 'array' => ['a', 'b', 'c'], + 'assoc' => [ + 'a' => true, + 'b' => 'abc', + 'c' => 123, + 'd' => ['a', 'b', 'c'] + ] + ], [ + 'string' => 'abc', + 'assoc' => [ + 'a' => true, + 'b' => 'abc', + 'c' => 123, + 'd' => ['a', 'b', 'c'] + ], + 'int' => 123, + 'array' => ['a', 'b', 'c'], + 'bool' => true, + + ])); $this->assertTrue(Migration::hasDifference([ 'a' => true ], [