diff --git a/.github/actions/run-tests/reset-from-container.sh b/.github/actions/run-tests/reset-from-container.sh index 059996341..fca3903ef 100755 --- a/.github/actions/run-tests/reset-from-container.sh +++ b/.github/actions/run-tests/reset-from-container.sh @@ -1,4 +1,4 @@ -#! /bin/sh -e +#! /bin/bash -e BACKUP="$1" @@ -13,8 +13,30 @@ case "$INPUT_DB" in mysql -u root -p"$MYSQL_ROOT_PASSWORD" -h mysql < "/dumps/$BACKUP/sql/dump.sql" ;; pgsql) + echo 'Dropping old data' PGPASSWORD="$POSTGRES_PASSWORD" \ - psql -d "$POSTGRES_DB" -h postgres -U "$POSTGRES_USER" < "/dumps/$BACKUP/sql/dump.sql" + psql -d "$POSTGRES_DB" -h postgres -U "$POSTGRES_USER" -v 'ON_ERROR_STOP=1' <<- EOF || exit 1 + DROP SCHEMA public CASCADE; + CREATE SCHEMA public; + GRANT ALL ON SCHEMA public TO $POSTGRES_USER; + GRANT ALL ON SCHEMA public TO public; + EOF + +# PGPASSWORD="$POSTGRES_PASSWORD" \ +# psql -d "$POSTGRES_DB" -h postgres -U "$POSTGRES_USER" <<- EOF +# \l +# \d +# SELECT * FROM oc_preferences WHERE appid='cookbook'; +# EOF + + echo 'Inserting dump data' + PGPASSWORD="$POSTGRES_PASSWORD" \ + psql -d "$POSTGRES_DB" -h postgres -U "$POSTGRES_USER" -f "/dumps/$BACKUP/sql/dump.sql" -v 'ON_ERROR_STOP=1' || exit 1 + +# PGPASSWORD="$POSTGRES_PASSWORD" \ +# psql -d "$POSTGRES_DB" -h postgres -U "$POSTGRES_USER" <<- EOF +# SELECT * FROM oc_preferences WHERE appid='cookbook'; +# EOF ;; sqlite) echo "Doing nothing as it was already restored during data restoration." diff --git a/.github/actions/run-tests/run-locally.sh b/.github/actions/run-tests/run-locally.sh index c28a083eb..3e046dbe1 100755 --- a/.github/actions/run-tests/run-locally.sh +++ b/.github/actions/run-tests/run-locally.sh @@ -274,10 +274,12 @@ drop_environment(){ fi ;; pgsql) - local tables=$(echo "SELECT tablename FROM pg_tables WHERE schemaname = 'public';" | call_postgres | head -n -1 ) - if [ -n "$tables" ]; then - echo "$tables" | sed 's@.*@DROP TABLE \0;@' | call_postgres - fi + call_postgres <<- EOF + DROP SCHEMA public CASCADE; + CREATE SCHEMA public; + GRANT ALL ON SCHEMA public TO $POSTGRES_USER; + GRANT ALL ON SCHEMA public TO public; + EOF ;; esac @@ -337,6 +339,12 @@ restore_env_dump() { ;; pgsql) echo "Restoring Postgres database from dump" + call_postgres <<- EOF + DROP SCHEMA public CASCADE; + CREATE SCHEMA public; + GRANT ALL ON SCHEMA public TO $POSTGRES_USER; + GRANT ALL ON SCHEMA public TO public; + EOF cat "volumes/dumps/$ENV_DUMP_PATH/sql/dump.sql" | call_postgres ;; sqlite) @@ -424,7 +432,7 @@ function call_mysql() { function call_postgres() { #docker-compose exec -T postgres psql -t nc_test tester - docker-compose exec -T postgres psql -U "$POSTGRES_USER" "$POSTGRES_DB" + docker-compose exec -T postgres psql -U "$POSTGRES_USER" "$POSTGRES_DB" -v 'ON_ERROR_STOP=1' --quiet "$@" } ##### Parameters as extracted from the CLI diff --git a/.github/actions/run-tests/run-occ.sh b/.github/actions/run-tests/run-occ.sh new file mode 100755 index 000000000..c6b53da16 --- /dev/null +++ b/.github/actions/run-tests/run-occ.sh @@ -0,0 +1,9 @@ +#! /bin/sh -e + +#set -x + +cd /nextcloud +php occ "$@" + +exit $? + diff --git a/CHANGELOG.md b/CHANGELOG.md index c48838770..5975b2829 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ ### Fixed - Fixed changes from #774 and minor extensions [#775](https://github.com/nextcloud/cookbook/pull/775) @christianlupus +- Clean tables from old, redundant, and non-unique data to allow migrations (see #762 #763) + [#776](https://github.com/nextcloud/cookbook/pull/776) @christianlupus ## 0.9.1 - 2021-07-05 diff --git a/lib/Migration/Version000000Date20210701093123.php b/lib/Migration/Version000000Date20210701093123.php index c432b91d1..382aee400 100644 --- a/lib/Migration/Version000000Date20210701093123.php +++ b/lib/Migration/Version000000Date20210701093123.php @@ -4,16 +4,84 @@ namespace OCA\Cookbook\Migration; -use Closure; +use OCP\IDBConnection; use OCP\DB\ISchemaWrapper; use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; +use Closure; +use OCP\DB\QueryBuilder\IQueryBuilder; /** * Auto-generated migration step: Please modify to your needs! */ class Version000000Date20210701093123 extends SimpleMigrationStep { + /** + * @var IDBConnection + */ + private $db; + + public function __construct(IDBConnection $db) { + $this->db = $db; + } + + public function preSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) { + $this->db->beginTransaction(); + try { + $qb = $this->db->getQueryBuilder(); + + // Fetch all rows that are non-unique + $qb->selectAlias('n.user_id', 'user') + ->selectAlias('n.recipe_id', 'recipe') + ->from('cookbook_names', 'n') + ->groupBy('n.user_id', 'n.recipe_id') + ->having('COUNT(*) > 1'); + //echo $qb->getSQL() . "\n"; + + $cursor = $qb->execute(); + $result = $cursor->fetchAll(); + + if (sizeof($result) > 0) { + // We have to fix the database + + // Drop all redundant rows + $qb = $this->db->getQueryBuilder(); + $qb->delete('cookbook_names') + ->where( + 'user_id = :user', + 'recipe_id = :recipe' + ); + + $qb2 = $this->db->getQueryBuilder(); + $qb2->update('preferences') + ->set('configvalue', $qb->expr()->literal('1', IQueryBuilder::PARAM_STR)) + ->where( + 'userid = :user', + 'appid = :app', + 'configkey = :property' + ); + $qb2->setParameter('app', 'cookbook'); + $qb2->setParameter('property', 'last_index_update'); + + foreach ($result as $r) { + $qb->setParameter('user', $r['user']); + $qb->setParameter('recipe', $r['recipe']); + $qb->execute(); + + $qb2->setParameter('user', $r['user']); + $qb2->execute(); + } + } + + // Finish the transaction + $this->db->commit(); + } catch (\Exception $e) { + // Abort the transaction + $this->db->rollBack(); + throw $e; + } + } + /** * @param IOutput $output * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` diff --git a/phpunit.integration.xml b/phpunit.integration.xml index 0841ab065..cd7878cfe 100755 --- a/phpunit.integration.xml +++ b/phpunit.integration.xml @@ -1,4 +1,11 @@ - + ./tests/Integration @@ -6,12 +13,7 @@ - lib/ - + lib diff --git a/phpunit.xml b/phpunit.xml index 37327e5de..ee08c702e 100755 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,4 +1,11 @@ - + ./tests/Unit @@ -7,11 +14,6 @@ lib - diff --git a/tests/Integration/Setup/Migrations/Version000000Date20210701093123Test.php b/tests/Integration/Setup/Migrations/Version000000Date20210701093123Test.php new file mode 100644 index 000000000..bccd92d34 --- /dev/null +++ b/tests/Integration/Setup/Migrations/Version000000Date20210701093123Test.php @@ -0,0 +1,205 @@ +container = $app->getContainer(); + + /** + * @var IDBConnection $db + */ + $this->db = $this->container->query(IDBConnection::class); + $this->assertIsObject($this->db); + /** + * @var SchemaWrapper $schema + */ + $schema = $this->container->query(SchemaWrapper::class); + $this->assertIsObject($schema); + + if (Util::getVersion()[0] >= 21) { + $connection = \OC::$server->query(Connection::class); + } else { + $connection = $this->db; + } + $this->migrationService = new MigrationService('cookbook', $connection); + + // undo all migrations of cookbook app + $qb = $this->db->getQueryBuilder(); + $numRows = $qb->delete('migrations') + ->where('app=:app') + ->setParameter('app', 'cookbook') + ->execute(); + $this->assertGreaterThan(0, $numRows); + + $schema->dropTable('cookbook_names'); + $this->assertFalse($schema->hasTable('cookbook_names')); + $schema->dropTable('cookbook_categories'); + $this->assertFalse($schema->hasTable('cookbook_categories')); + $schema->dropTable('cookbook_keywords'); + $this->assertFalse($schema->hasTable('cookbook_keywords')); + + $schema->performDropTableCalls(); + + // Reinstall app partially (just before the migration) + $this->migrationService->migrate('000000Date20210427082010'); + } + + protected function tearDown(): void { + unset($this->container); + unset($this->db); + unset($this->migrationService); + } + + /** + * @dataProvider dataProvider + * @runInSeparateProcess + * @covers \OCA\Cookbook\Migration\Version000000Date20210701093123 + */ + public function testRedundantEntriesInDB($data, $updatedUsers) { + // Add recipe dummy data from data provider + $qb = $this->db->getQueryBuilder(); + $qb->insert('cookbook_names') + ->values([ + 'recipe_id' => ':recipe', + 'user_id' => ':user', + 'name' => ':name', + ]); + $qb->setParameter('name', 'name of the recipe'); + foreach ($data as $d) { + $qb->setParameter('user', $d[0]); + $qb->setParameter('recipe', $d[1]); + + $this->assertEquals(1, $qb->execute()); + } + + // Initialize configuration values to track reindex timestamps + $current = time(); + + $qb = $this->db->getQueryBuilder(); + $qb->insert('preferences') + ->values([ + 'userid' => ':user', + 'appid' => ':appid', + 'configkey' => ':property', + 'configvalue' => ':value', + ]); + + $qb->setParameter('value', $current, IQueryBuilder::PARAM_STR); + $qb->setParameter('appid', 'cookbook'); + $qb->setParameter('property', 'last_index_update'); + + $users = array_unique(array_map(function ($x) { + return $x[0]; + }, $data)); + foreach ($users as $u) { + $qb->setParameter('user', $u); + $this->assertEquals(1, $qb->execute()); + } + + // Run the migration under test + $this->migrationService->migrate('000000Date20210701093123'); + + // Get the (updated) reindex timestamps + $qb = $this->db->getQueryBuilder(); + $qb->select('userid', 'configvalue') + ->from('preferences') + ->where( + 'appid = :appid', + 'configkey = :property' + ); + $qb->setParameter('appid', 'cookbook'); + $qb->setParameter('property', 'last_index_update'); + + $cursor = $qb->execute(); + $result = $cursor->fetchAll(); + + // Filter those entries from the configuration that were marked as to be reindexed + $result = array_filter($result, function ($x) use ($current) { + return $x['configvalue'] < $current; + }); + // Map the array to contain only the corresponding user names + $changedUsers = array_map(function ($x) { + return $x['userid']; + }, $result); + + // Sort the arrays to allow comparision of them + sort($changedUsers); + sort($updatedUsers); + + $this->assertEquals($updatedUsers, $changedUsers); + } + + public function dataProvider() { + return [ + 'caseA' => [ + [ + ['alice', 123], + ['alice', 124], + ['bob', 125] + ], + [], + ], + 'caseB' => [ + [ + ['alice', 123], + ['alice', 124], + ['bob', 124], + ['bob', 125], + ], + [], + ], + 'caseC' => [ + [ + ['alice', 123], + ['alice', 124], + ['bob', 124], + ['bob', 124], + ['bob', 125], + ], + ['bob'] + ], + 'caseD' => [ + [ + ['alice', 124], + ['alice', 124], + ['bob', 124], + ['bob', 124], + ['bob', 125], + ], + ['alice', 'bob'] + ], + ]; + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 3d022ac93..d7ea801f6 100755 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -5,12 +5,33 @@ // Fix for "Autoload path not allowed: .../cookbook/tests/testcase.php" \OC_App::loadApp('cookbook'); -function resetEnvironmentToBackup(string $name = 'default') { +function resetEnvironmentToBackup(string $name = 'default', bool $forceprint = false) { + $output = []; + $ret = -1; exec("./.github/actions/run-tests/reset-from-container.sh $name 2>&1", $output, $ret); - if ($ret !== 0) { + if ($ret !== 0 || $forceprint) { echo "\nStandard output:\n"; print_r($output); echo "Return value: $ret\n"; - throw new Exception("Could not reset environment"); + if ($ret !== 0) { + throw new Exception("Could not reset environment"); + } + } +} + +function runOCCCommand(array $args, bool $forceprint = false) { + $output = []; + $ret = -1; + $params = join(' ', array_map(function ($x) { + return escapeshellarg($x); + }, $args)); + exec("./.github/actions/run-tests/run-occ.sh $params 2>&1", $output, $ret); + if ($ret !== 0 || $forceprint) { + echo "\nStandard output:\n"; + print_r($output); + echo "Return value: $ret\n"; + if ($ret !== 0) { + throw new Exception("Could not run OCC command"); + } } }