From 3d721ea7757278bdc366d93fd7545ee2b1a3a7dd Mon Sep 17 00:00:00 2001 From: Adrian Tello Date: Tue, 6 Oct 2015 13:35:13 +0200 Subject: [PATCH 1/2] Don't use this in a closure --- Commands/MigrateSite.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Commands/MigrateSite.php b/Commands/MigrateSite.php index 2287605..dc86c8c 100644 --- a/Commands/MigrateSite.php +++ b/Commands/MigrateSite.php @@ -59,12 +59,14 @@ protected function configure() protected function execute(InputInterface $input, OutputInterface $output) { + $self = $this; + // Set memory limit to off @ini_set('memory_limit', -1); - Piwik::doAsSuperUser(function() use ($input, $output){ + Piwik::doAsSuperUser(function() use ($input, $output, $self){ $settings = new MigratorSettings(); $settings->idSite = $input->getArgument('idSite'); - $settings->site = $this->getSite($settings->idSite); + $settings->site = $self->getSite($settings->idSite); $settings->dateFrom = $input->getOption('date-from') ? new \DateTime($input->getOption('date-from')) : null; $settings->dateTo = $input->getOption('date-to') ? new \DateTime($input->getOption('date-to')) : null; $settings->skipArchiveData = $input->getOption('skip-archive-data'); @@ -73,7 +75,7 @@ protected function execute(InputInterface $input, OutputInterface $output) $config = Db::getDatabaseConfig(); $startTime = microtime(true); - $this->createTargetDatabaseConfig($input, $output, $config); + $self->createTargetDatabaseConfig($input, $output, $config); $tmpConfig = $config; $sourceDb = Db::get(); @@ -103,7 +105,7 @@ protected function execute(InputInterface $input, OutputInterface $output) } - protected function getSite($idSite) + public function getSite($idSite) { if (!Site::getSite($idSite)) { throw new \InvalidArgumentException('idSite is not a valid, no such site found'); @@ -115,7 +117,7 @@ protected function getSite($idSite) ); } - private function createTargetDatabaseConfig(InputInterface $input, OutputInterface $output, &$config) + public function createTargetDatabaseConfig(InputInterface $input, OutputInterface $output, &$config) { $notNullValidator = function ($answer) { if (strlen(trim($answer)) == 0) { From 79b3384936a9e49dbdc011b83d5b7f4071823e76 Mon Sep 17 00:00:00 2001 From: Adrian Tello Date: Thu, 24 Sep 2015 12:53:29 +0200 Subject: [PATCH 2/2] BugFix: Invalid mapping in the new site, when action id is -1, 0 or null. --- Migrator/ActionMigrator.php | 7 +++++-- Migrator/ConversionItemMigrator.php | 4 ---- Migrator/ConversionMigrator.php | 8 +------- Test/ActionMigratorTest.php | 6 ++++++ Test/ConversionItemMigratorTest.php | 2 +- 5 files changed, 13 insertions(+), 14 deletions(-) diff --git a/Migrator/ActionMigrator.php b/Migrator/ActionMigrator.php index 01f2985..a87a72c 100644 --- a/Migrator/ActionMigrator.php +++ b/Migrator/ActionMigrator.php @@ -89,12 +89,15 @@ public function ensureActionIsMigrated($idAction) public function getNewId($idAction) { + if($idAction == null || $idAction < 1){ + return $idAction; + } + if ($this->ensureActionIsMigrated($idAction)) { return $this->idMap[$idAction]; } else { - return 0; + throw new \InvalidArgumentException('Id ' . $idAction . ' not found in ' . __CLASS__); } - } /** diff --git a/Migrator/ConversionItemMigrator.php b/Migrator/ConversionItemMigrator.php index b410e52..f6c0135 100644 --- a/Migrator/ConversionItemMigrator.php +++ b/Migrator/ConversionItemMigrator.php @@ -59,10 +59,6 @@ protected function translateRow(&$row) $row['idvisit'] = $this->visitMigrator->getNewId($row['idvisit']); foreach ($this->actionsToTranslate as $translationKey) { - if ($row[$translationKey] == 0) { - continue; - } - $row[$translationKey] = $this->actionMigrator->getNewId($row[$translationKey]); } } diff --git a/Migrator/ConversionMigrator.php b/Migrator/ConversionMigrator.php index 0c63ed7..ed2dfaf 100644 --- a/Migrator/ConversionMigrator.php +++ b/Migrator/ConversionMigrator.php @@ -59,13 +59,7 @@ protected function translateRow(&$row) $row['idlink_va'] = $this->linkVisitActionMigrator->getNewId($row['idlink_va']); } - if ($row['idaction_url']) { - $row['idaction_url'] = $this->actionMigrator->getNewId( - $row['idaction_url'] - ); - } else { - $row['idaction_url'] = 0; - } + $row['idaction_url'] = $this->actionMigrator->getNewId( $row['idaction_url']); } /** diff --git a/Test/ActionMigratorTest.php b/Test/ActionMigratorTest.php index e0c2e1c..d88c999 100644 --- a/Test/ActionMigratorTest.php +++ b/Test/ActionMigratorTest.php @@ -170,6 +170,12 @@ public function test_loadExistingActions() $this->assertEquals($this->dummyExistingActions, $this->actionMigrator->getExistingActions()); } + public function test_getNewIdSpecialActionsIds(){ + $this->assertNull($this->actionMigrator->getNewId(null)); + $this->assertEquals(0, $this->actionMigrator->getNewId(0)); + $this->assertEquals(-1, $this->actionMigrator->getNewId(-1)); + } + protected function setupEnsureActionIsMigratedMigrationTest($action) { $this->setupDbHelperGetAdapter($this->fromDbHelper); diff --git a/Test/ConversionItemMigratorTest.php b/Test/ConversionItemMigratorTest.php index 987b45a..98d5399 100644 --- a/Test/ConversionItemMigratorTest.php +++ b/Test/ConversionItemMigratorTest.php @@ -129,7 +129,7 @@ public function test_migrateConversionItems() $this->siteMigrator->expects($this->once())->method('getNewId')->with(1)->willReturn(2); $this->visitMigrator->expects($this->once())->method('getNewId')->with(3)->willReturn(4); - $this->actionMigrator->expects($this->exactly(6))->method('getNewId')->will( + $this->actionMigrator->expects($this->exactly(7))->method('getNewId')->will( $this->onConsecutiveCalls(2, 4, 6, 8, 12, 14, 16, 18, 20) );