diff --git a/core/DataAccess/LogQueryBuilder.php b/core/DataAccess/LogQueryBuilder.php index d9b35408fe082b02277a31f8fcab56cb371c966d..5e36b4617cc86d0a34df72f80c8f03c9410a00be 100644 --- a/core/DataAccess/LogQueryBuilder.php +++ b/core/DataAccess/LogQueryBuilder.php @@ -15,20 +15,16 @@ use Piwik\Segment\SegmentExpression; class LogQueryBuilder { - public function __construct(SegmentExpression $segmentExpression) - { - $this->segmentExpression = $segmentExpression; - } - - public function getSelectQueryString($select, $from, $where, $bind, $groupBy, $orderBy, $limit) + public function getSelectQueryString(SegmentExpression $segmentExpression, $select, $from, $where, $bind, $groupBy, + $orderBy, $limit) { if (!is_array($from)) { $from = array($from); } - if (!$this->segmentExpression->isEmpty()) { - $this->segmentExpression->parseSubExpressionsIntoSqlExpressions($from); - $segmentSql = $this->segmentExpression->getSql(); + if (!$segmentExpression->isEmpty()) { + $segmentExpression->parseSubExpressionsIntoSqlExpressions($from); + $segmentSql = $segmentExpression->getSql(); $where = $this->getWhereMatchBoth($where, $segmentSql['where']); $bind = array_merge($bind, $segmentSql['bind']); } diff --git a/core/Segment.php b/core/Segment.php index 14e9230bfc0a6d28d94d1f8dbd1eff707cd484ea..2a64f705af09bb008be3020dba4c17c259dacbf6 100644 --- a/core/Segment.php +++ b/core/Segment.php @@ -9,6 +9,7 @@ namespace Piwik; use Exception; +use Piwik\Container\StaticContainer; use Piwik\DataAccess\LogQueryBuilder; use Piwik\Plugins\API\API; use Piwik\Segment\SegmentExpression; @@ -71,6 +72,11 @@ class Segment */ protected $idSites = null; + /** + * @var LogQueryBuilder + */ + private $segmentQueryBuilder; + /** * Truncate the Segments to 8k */ @@ -86,6 +92,8 @@ class Segment */ public function __construct($segmentCondition, $idSites) { + $this->segmentQueryBuilder = StaticContainer::get('Piwik\DataAccess\LogQueryBuilder'); + $segmentCondition = trim($segmentCondition); if (!SettingsPiwik::isSegmentationEnabled() && !empty($segmentCondition) @@ -253,8 +261,8 @@ class Segment $limit = (int) $offset . ', ' . (int) $limit; } - $segmentQuery = new LogQueryBuilder($segmentExpression); - return $segmentQuery->getSelectQueryString($select, $from, $where, $bind, $groupBy, $orderBy, $limit); + return $this->segmentQueryBuilder->getSelectQueryString($segmentExpression, $select, $from, $where, $bind, + $groupBy, $orderBy, $limit); } /** diff --git a/core/Segment/SegmentExpression.php b/core/Segment/SegmentExpression.php index c307509bf1117c3882666baed97dc6335bfd6646..a6aecc6c299a9172f345718ff8c503ba1e81621d 100644 --- a/core/Segment/SegmentExpression.php +++ b/core/Segment/SegmentExpression.php @@ -49,6 +49,11 @@ class SegmentExpression $this->tree = $this->parseTree(); } + public function getSegmentDefinition() + { + return $this->string; + } + public function isEmpty() { return count($this->tree) == 0; diff --git a/plugins/SegmentEditor/API.php b/plugins/SegmentEditor/API.php index c08f302e7387809a14b2f5b4c855a07950be4fbe..e69a86bb983cb6926771bc24be4d2906f785a9c2 100644 --- a/plugins/SegmentEditor/API.php +++ b/plugins/SegmentEditor/API.php @@ -9,6 +9,7 @@ namespace Piwik\Plugins\SegmentEditor; use Exception; +use Piwik\Cache\Transient as TransientCache; use Piwik\Common; use Piwik\Date; use Piwik\Db; @@ -23,6 +24,16 @@ use Piwik\Segment; */ class API extends \Piwik\Plugin\API { + /** + * @var Model + */ + private $model; + + public function __construct(Model $model) + { + $this->model = $model; + } + protected function checkSegmentValue($definition, $idSite) { // unsanitize so we don't record the HTML entitied segment @@ -201,7 +212,7 @@ class API extends \Piwik\Plugin\API private function getModel() { - return new Model(); + return $this->model; } /** diff --git a/plugins/SegmentEditor/SegmentQueryDecorator.php b/plugins/SegmentEditor/SegmentQueryDecorator.php new file mode 100644 index 0000000000000000000000000000000000000000..d501f6f9b16ee11076ff331d337f0fd3b6fe2e6c --- /dev/null +++ b/plugins/SegmentEditor/SegmentQueryDecorator.php @@ -0,0 +1,70 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ + +namespace Piwik\Plugins\SegmentEditor; + +use Piwik\DataAccess\LogQueryBuilder; +use Piwik\Plugins\SegmentEditor\Services\StoredSegmentService; +use Piwik\Segment\SegmentExpression; +use Piwik\SettingsServer; + +/** + * Decorates segment sub-queries in archiving queries w/ the idSegment of the segment, if + * a stored segment exists. + * + * This class is configured for use in SegmentEditor's DI config. + */ +class SegmentQueryDecorator extends LogQueryBuilder +{ + /** + * @var StoredSegmentService + */ + private $storedSegmentService; + + public function __construct(StoredSegmentService $storedSegmentService) + { + $this->storedSegmentService = $storedSegmentService; + } + + public function getSelectQueryString(SegmentExpression $segmentExpression, $select, $from, $where, $bind, $groupBy, + $orderBy, $limit) + { + $result = parent::getSelectQueryString($segmentExpression, $select, $from, $where, $bind, $groupBy, $orderBy, + $limit); + + $prefixParts = array(); + + if (SettingsServer::isArchivePhpTriggered()) { + $prefixParts[] = 'trigger = CronArchive'; + } + + $idSegments = $this->getSegmentIdOfExpression($segmentExpression); + if (!empty($idSegments)) { + $prefixParts[] = "idSegments = [" . implode(', ', $idSegments) . "]"; + } + + if (!empty($prefixParts)) { + $result['sql'] = "/* " . implode(', ', $prefixParts) . " */\n" . $result['sql']; + } + + return $result; + } + + private function getSegmentIdOfExpression(SegmentExpression $segmentExpression) + { + $allSegments = $this->storedSegmentService->getAllSegmentsAndIgnoreVisibility(); + + $idSegments = array(); + foreach ($allSegments as $segment) { + if ($segmentExpression->getSegmentDefinition() == $segment['definition']) { + $idSegments[] = $segment['idsegment']; + } + } + return $idSegments; + } +} diff --git a/plugins/SegmentEditor/Services/StoredSegmentService.php b/plugins/SegmentEditor/Services/StoredSegmentService.php new file mode 100644 index 0000000000000000000000000000000000000000..a03448bcdbacae83c69d239f1b9a1713330fb23e --- /dev/null +++ b/plugins/SegmentEditor/Services/StoredSegmentService.php @@ -0,0 +1,51 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ + +namespace Piwik\Plugins\SegmentEditor\Services; + +use Piwik\Plugins\SegmentEditor\Model; +use Piwik\Cache\Transient as TransientCache; + + +/** + * Service layer class for stored segments. + */ +class StoredSegmentService +{ + /** + * @var Model + */ + private $model; + + /** + * @var TransientCache + */ + private $transientCache; + + public function __construct(Model $model, TransientCache $transientCache) + { + $this->model = $model; + $this->transientCache = $transientCache; + } + + /** + * Returns all stored segments that haven't been deleted. + * + * @return array + */ + public function getAllSegmentsAndIgnoreVisibility() + { + $cacheKey = 'SegmentEditor.getAllSegmentsAndIgnoreVisibility'; + if (!$this->transientCache->contains($cacheKey)) { + $result = $this->model->getAllSegmentsAndIgnoreVisibility(); + + $this->transientCache->save($cacheKey, $result); + } + return $this->transientCache->fetch($cacheKey); + } +} diff --git a/plugins/SegmentEditor/config/config.php b/plugins/SegmentEditor/config/config.php new file mode 100644 index 0000000000000000000000000000000000000000..ada7b93ac9504bf12b69da90225d52262edbff85 --- /dev/null +++ b/plugins/SegmentEditor/config/config.php @@ -0,0 +1,7 @@ +<?php + +return array( + + 'Piwik\DataAccess\LogQueryBuilder' => DI\get('Piwik\Plugins\SegmentEditor\SegmentQueryDecorator'), + +); diff --git a/plugins/SegmentEditor/tests/Integration/SegmentQueryDecoratorTest.php b/plugins/SegmentEditor/tests/Integration/SegmentQueryDecoratorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..6d70acf2c51b81dc494aa9f74dd5c895caca8421 --- /dev/null +++ b/plugins/SegmentEditor/tests/Integration/SegmentQueryDecoratorTest.php @@ -0,0 +1,88 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ + +namespace Piwik\Plugins\SegmentEditor\tests\Integration; + +use Piwik\Plugins\SegmentEditor\API; +use Piwik\Plugins\SegmentEditor\SegmentQueryDecorator; +use Piwik\Segment; +use Piwik\Tests\Framework\Fixture; +use Piwik\Tests\Framework\TestCase\IntegrationTestCase; + +/** + * @group SegmentEditor + * @group SegmentEditor_Integration + */ +class SegmentQueryDecoratorTest extends IntegrationTestCase +{ + /** + * @var SegmentQueryDecorator + */ + private $segmentQueryDecorator; + + public function setUp() + { + parent::setUp(); + + Fixture::createWebsite('2011-01-01'); + Fixture::createWebsite('2011-01-01'); + Fixture::createWebsite('2011-01-01'); + + $this->segmentQueryDecorator = self::$fixture->piwikEnvironment->getContainer()->get( + 'Piwik\Plugins\SegmentEditor\SegmentQueryDecorator'); + + /** @var API $segmentEditorApi */ + $segmentEditorApi = self::$fixture->piwikEnvironment->getContainer()->get( + 'Piwik\Plugins\SegmentEditor\API'); + $segmentEditorApi->add('segment 1', 'visitCount<2', $idSite = false, $autoArchive = true); + $segmentEditorApi->add('segment 2', 'countryCode==fr', $idSite = false, $autoArchive = true); + $segmentEditorApi->add('segment 3', 'visitCount<2', 1, $autoArchive = true); + $segmentEditorApi->add('segment 4', 'visitCount<2', 2, $autoArchive = true); + + // test that segments w/ auto archive == false are included + $segmentEditorApi->add('segment 5', 'visitCount<2', 3, $autoArchive = false); + $segmentEditorApi->add('segment 6', 'countryCode!=fr', 3, $autoArchive = false); + } + + /** + * @dataProvider getTestDataForSegmentSqlTest + */ + public function test_SegmentSql_IsCorrectlyDecoratedWithIdSegment($segment, $triggerValue, $expectedPrefix) + { + if (!empty($triggerValue)) { + $_GET['trigger'] = $triggerValue; + } + + $segment = new Segment($segment, array()); + + $query = $segment->getSelectQuery('*', 'log_visit'); + $sql = $query['sql']; + + if (empty($expectedPrefix)) { + $this->assertStringStartsNotWith("/* idSegments", $sql); + } else { + $this->assertStringStartsWith($expectedPrefix, $sql); + } + } + + public function getTestDataForSegmentSqlTest() + { + return array( + array('countryCode==fr', null, '/* idSegments = [2] */'), + array('visitCount<2', null, '/* idSegments = [1, 3, 4, 5] */'), + array('', null, null), + array('countryCode!=fr', null, '/* idSegments = [6] */'), + + array('', 'archivephp', '/* trigger = CronArchive */'), + array('countryCode!=fr', 'archivephp', '/* trigger = CronArchive, idSegments = [6] */'), + + array('', 'garbage', null), + array('countryCode!=fr', 'garbage', '/* idSegments = [6] */'), + ); + } +} \ No newline at end of file diff --git a/plugins/SegmentEditor/tests/Unit/SegmentQueryDecoratorTest.php b/plugins/SegmentEditor/tests/Unit/SegmentQueryDecoratorTest.php new file mode 100644 index 0000000000000000000000000000000000000000..6ecad82f2fc4cd6e7c5ecbb930a1fa131cb37922 --- /dev/null +++ b/plugins/SegmentEditor/tests/Unit/SegmentQueryDecoratorTest.php @@ -0,0 +1,89 @@ +<?php +/** + * Piwik - free/libre analytics platform + * + * @link http://piwik.org + * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later + */ + +namespace Piwik\Plugins\SegmentEditor\tests\Unit; + +use Piwik\Plugins\SegmentEditor\SegmentQueryDecorator; +use Piwik\Plugins\SegmentEditor\Services\StoredSegmentService; +use Piwik\Segment\SegmentExpression; + +/** + * @group SegmentEditor + * @group SegmentEditor_Unit + */ +class SegmentQueryDecoratorTest extends \PHPUnit_Framework_TestCase +{ + public static $storedSegments = array( + array('definition' => 'countryCode==abc', 'idsegment' => 1), + array('definition' => 'region!=FL', 'idsegment' => 2), + array('definition' => 'browserCode==def;visitCount>2', 'idsegment' => 3), + array('definition' => 'region!=FL', 'idsegment' => 4), + ); + + /** + * @var SegmentQueryDecorator + */ + private $decorator; + + public function setUp() + { + parent::setUp(); + + /** @var StoredSegmentService $service */ + $service = $this->getMockSegmentEditorService(); + $this->decorator = new SegmentQueryDecorator($service); + } + + public function test_getSelectQueryString_DoesNotDecorateSql_WhenNoSegmentUsed() + { + $expression = new SegmentExpression(''); + $expression->parseSubExpressions(); + + $query = $this->decorator->getSelectQueryString($expression, '*', 'log_visit', '', array(), '', '', ''); + + $this->assertStringStartsNotWith('/* idSegments', $query['sql']); + } + + public function test_getSelectQueryString_DoesNotDecorateSql_WhenNoSegmentMatchesUsedSegment() + { + $expression = new SegmentExpression('referrerName==ooga'); + $expression->parseSubExpressions(); + + $query = $this->decorator->getSelectQueryString($expression, '*', 'log_visit', '', array(), '', '', ''); + + $this->assertStringStartsNotWith('/* idSegments', $query['sql']); + } + + public function test_getSelectQueryString_DecoratesSql_WhenOneSegmentMatchesUsedSegment() + { + $expression = new SegmentExpression('browserCode==def;visitCount>2'); + $expression->parseSubExpressions(); + + $query = $this->decorator->getSelectQueryString($expression, '*', 'log_visit', '', array(), '', '', ''); + + $this->assertStringStartsWith('/* idSegments = [3] */', $query['sql']); + } + + public function test_getSelectQueryString_DecoratesSql_WhenMultipleStoredSegmentsMatchUsedSegment() + { + $expression = new SegmentExpression('region!=FL'); + $expression->parseSubExpressions(); + + $query = $this->decorator->getSelectQueryString($expression, '*', 'log_visit', '', array(), '', '', ''); + + $this->assertStringStartsWith('/* idSegments = [2, 4] */', $query['sql']); + } + + private function getMockSegmentEditorService() + { + $mock = $this->getMock('Piwik\Plugins\SegmentEditor\Services\StoredSegmentService', + array('getAllSegmentsAndIgnoreVisibility'), array(), '', $callOriginalConstructor = false); + $mock->expects($this->any())->method('getAllSegmentsAndIgnoreVisibility')->willReturn(self::$storedSegments); + return $mock; + } +}