From e1b2a0b4ef92ae8225f7d3f219bbf43769f41927 Mon Sep 17 00:00:00 2001 From: Ankit Agarwal Date: Tue, 18 Mar 2014 16:04:09 +0800 Subject: [PATCH] MDL-42892 logging: Refactor logstore_legacy to group all legacy related changes together --- .../log/store/legacy/classes/log/store.php | 61 ++++---- .../log/store/legacy/tests/fixtures/store.php | 18 +-- .../log/store/legacy/tests/store_test.php | 132 ++++++++++-------- 3 files changed, 107 insertions(+), 104 deletions(-) diff --git a/admin/tool/log/store/legacy/classes/log/store.php b/admin/tool/log/store/legacy/classes/log/store.php index 058fda13d7d..6f6df21c8e3 100644 --- a/admin/tool/log/store/legacy/classes/log/store.php +++ b/admin/tool/log/store/legacy/classes/log/store.php @@ -35,7 +35,7 @@ class store implements \tool_log\log\store, \core\log\sql_select_reader { } /** @var array list of db fields which needs to be replaced for legacy log query */ - protected $standardtolegacyfields = array( + protected static $standardtolegacyfields = array( 'timecreated' => 'time', 'courseid' => 'course', 'contextinstanceid' => 'cmid', @@ -44,41 +44,50 @@ class store implements \tool_log\log\store, \core\log\sql_select_reader { /** @var string Regex to replace the crud params */ const CRUD_REGEX = "/(crud).*?(<>|=|!=).*?'(.*?)'/s"; + /** - * This method contains hacks required for Moodle core to make legacy store compatible with other sql_select_reader based + * This method contains mapping required for Moodle core to make legacy store compatible with other sql_select_reader based * queries. * - * @param string $select Select statment + * @param string $selectwhere Select statment * @param array $params params for the sql + * @param string $sort sort fields * - * @return array returns an array containing the sql predicate and an array of params. + * @return array returns an array containing the sql predicate, an array of params and sorting parameter. */ - protected static function replace_sql_hacks($select, array $params) { - if ($select == "userid = :userid AND courseid = :courseid AND eventname = :eventname AND timecreated > :since") { + protected static function replace_sql_legacy($selectwhere, array $params, $sort = '') { + // Following mapping is done to make can_delete_course() compatible with legacy store. + if ($selectwhere == "userid = :userid AND courseid = :courseid AND eventname = :eventname AND timecreated > :since" and + empty($sort)) { $replace = "module = 'course' AND action = 'new' AND userid = :userid AND url = :url AND time > :since"; $params += array('url' => "view.php?id={$params['courseid']}"); - return array($replace, $params); + return array($replace, $params, $sort); } - return array($select, $params); - } - - public function get_events_select($selectwhere, array $params, $sort, $limitfrom, $limitnum) { - global $DB; - - // Replace the query with hardcoded hacks required for core. - list($selectwhere, $params) = self::replace_sql_hacks($selectwhere, $params); - // Replace db field names to make it compatible with legacy log. - foreach ($this->standardtolegacyfields as $from => $to) { + foreach (self::$standardtolegacyfields as $from => $to) { $selectwhere = str_replace($from, $to, $selectwhere); - $sort = str_replace($from, $to, $sort); + if (!empty($sort)) { + $sort = str_replace($from, $to, $sort); + } if (isset($params[$from])) { $params[$to] = $params[$from]; unset($params[$from]); } } - $selectwhere = preg_replace_callback(self::CRUD_REGEX, 'self::replace_crud', $selectwhere); + + // Replace crud fields. + $selectwhere = preg_replace_callback("/(crud).*?(<>|=|!=).*?'(.*?)'/s", 'self::replace_crud', $selectwhere); + + return array($selectwhere, $params, $sort); + } + + public function get_events_select($selectwhere, array $params, $sort, $limitfrom, $limitnum) { + global $DB; + + // Replace the query with hardcoded mappings required for core. + list($selectwhere, $params, $sort) = self::replace_sql_legacy($selectwhere, $params, $sort); + $events = array(); $records = array(); @@ -98,18 +107,8 @@ class store implements \tool_log\log\store, \core\log\sql_select_reader { public function get_events_select_count($selectwhere, array $params) { global $DB; - // Replace the query with hardcoded hacks required for core. - list($selectwhere, $params) = self::replace_sql_hacks($selectwhere, $params); - - // Replace db field names to make it compatible with legacy log. - foreach ($this->standardtolegacyfields as $from => $to) { - $selectwhere = str_replace($from, $to, $selectwhere); - if (isset($params[$from])) { - $params[$to] = $params[$from]; - unset($params[$from]); - } - } - $selectwhere = preg_replace_callback(self::CRUD_REGEX, 'self::replace_crud', $selectwhere); + // Replace the query with hardcoded mappings required for core. + list($selectwhere, $params) = self::replace_sql_legacy($selectwhere, $params); try { return $DB->count_records_select('log', $selectwhere, $params); diff --git a/admin/tool/log/store/legacy/tests/fixtures/store.php b/admin/tool/log/store/legacy/tests/fixtures/store.php index 73b8fdd13b7..f8fe163ca01 100644 --- a/admin/tool/log/store/legacy/tests/fixtures/store.php +++ b/admin/tool/log/store/legacy/tests/fixtures/store.php @@ -28,26 +28,16 @@ defined('MOODLE_INTERNAL') || die(); class unittest_logstore_legacy extends \logstore_legacy\log\store { - /** - * Wrapper to make protected method accessible during testing. - * - * @param array $match matched string for the passed pattern - * - * @return string The sql string to use instead of original - */ - public static function replace_crud($match) { - return parent::replace_crud($match); - } - /** * Wrapper to make protected method accessible during testing. * * @param string $select sql predicate. * @param array $params sql params. + * @param string $sort sort options. * - * @return array returns array of sql predicate and params. + * @return array returns array of sql predicate, params and sorting criteria. */ - public static function replace_sql_hack($select, array $params) { - return parent::replace_sql_hacks($select, $params); + public static function replace_sql_legacy($select, array $params, $sort = '') { + return parent::replace_sql_legacy($select, $params, $sort); } } diff --git a/admin/tool/log/store/legacy/tests/store_test.php b/admin/tool/log/store/legacy/tests/store_test.php index 3c59ad214e4..c99f017ed61 100644 --- a/admin/tool/log/store/legacy/tests/store_test.php +++ b/admin/tool/log/store/legacy/tests/store_test.php @@ -163,71 +163,85 @@ class logstore_legacy_store_testcase extends advanced_testcase { } /** - * Test replace_crud + * Test replace_sql_legacy() */ - public function test_replace_crud() { - - $crudregex = logstore_legacy\test\unittest_logstore_legacy::CRUD_REGEX; - - $selectwhere = "edulevel = 0"; - $updatewhere = preg_replace_callback($crudregex, - 'logstore_legacy\test\unittest_logstore_legacy::replace_crud', $selectwhere); - $this->assertEquals($selectwhere, $updatewhere); - - $selectwhere = "edulevel = 0 and crud = 'u'"; - $updatewhere = preg_replace_callback($crudregex, - 'logstore_legacy\test\unittest_logstore_legacy::replace_crud', $selectwhere); - $this->assertEquals("edulevel = 0 and action LIKE '%update%'", $updatewhere); - - $selectwhere = "edulevel = 0 and crud != 'u'"; - $updatewhere = preg_replace_callback($crudregex, - 'logstore_legacy\test\unittest_logstore_legacy::replace_crud', $selectwhere); - $this->assertEquals("edulevel = 0 and action NOT LIKE '%update%'", $updatewhere); - - $selectwhere = "edulevel = 0 and crud <> 'u'"; - $updatewhere = preg_replace_callback($crudregex, - 'logstore_legacy\test\unittest_logstore_legacy::replace_crud', $selectwhere); - $this->assertEquals("edulevel = 0 and action NOT LIKE '%update%'", $updatewhere); - - $selectwhere = "edulevel = 0 and crud = 'r'"; - $updatewhere = preg_replace_callback($crudregex, - 'logstore_legacy\test\unittest_logstore_legacy::replace_crud', $selectwhere); - $this->assertEquals("edulevel = 0 and action LIKE '%view%' OR action LIKE '%report%'", $updatewhere); - - $selectwhere = "edulevel = 0 and crud != 'r'"; - $updatewhere = preg_replace_callback($crudregex, - 'logstore_legacy\test\unittest_logstore_legacy::replace_crud', $selectwhere); - $this->assertEquals("edulevel = 0 and action NOT LIKE '%view%' AND action NOT LIKE '%report%'", $updatewhere); - - $selectwhere = "edulevel = 0 and crud <> 'r'"; - $updatewhere = preg_replace_callback($crudregex, - 'logstore_legacy\test\unittest_logstore_legacy::replace_crud', $selectwhere); - $this->assertEquals("edulevel = 0 and action NOT LIKE '%view%' AND action NOT LIKE '%report%'", $updatewhere); - - // The slq is incorrect, since quotes must not be present. Make sure this is not parsed. - $selectwhere = "edulevel = 0 and 'crud' != 'u'"; - $updatewhere = preg_replace_callback($crudregex, - 'logstore_legacy\test\unittest_logstore_legacy::replace_crud', $selectwhere); - $this->assertNotEquals("edulevel = 0 and action NOT LIKE '%update%'", $updatewhere); - - $selectwhere = "edulevel = 0 and crud = 'u' OR crud != 'r' or crud <> 'd'"; - $updatewhere = preg_replace_callback($crudregex, - 'logstore_legacy\test\unittest_logstore_legacy::replace_crud', $selectwhere); - $this->assertEquals("edulevel = 0 and action LIKE '%update%' OR action NOT LIKE '%view%' AND action NOT LIKE '%report%' or action NOT LIKE '%delete%'", $updatewhere); - - } - - /** - * Test replace_sql_hacks() - */ - public function test_replace_sql_hacks() { - $select = "userid = :userid AND courseid = :courseid AND eventname = :eventname AND timecreated > :since"; + public function test_replace_sql_legacy() { + $selectwhere = "userid = :userid AND courseid = :courseid AND eventname = :eventname AND timecreated > :since"; $params = array('userid' => 2, 'since' => 3, 'courseid' => 4, 'eventname' => '\core\event\course_created'); $expectedselect = "module = 'course' AND action = 'new' AND userid = :userid AND url = :url AND time > :since"; $expectedparams = $params + array('url' => "view.php?id=4"); - list($replaceselect, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_hack($select, $params); + list($replaceselect, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); $this->assertEquals($replaceselect, $expectedselect); $this->assertEquals($replaceparams, $expectedparams); + + // Test CRUD related changes. + $selectwhere = "edulevel = 0"; + list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); + $this->assertEquals($selectwhere, $updatewhere); + + $selectwhere = "edulevel = 0 and crud = 'u'"; + list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); + $this->assertEquals("edulevel = 0 and action LIKE '%update%'", $updatewhere); + + $selectwhere = "edulevel = 0 and crud != 'u'"; + list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); + $this->assertEquals("edulevel = 0 and action NOT LIKE '%update%'", $updatewhere); + + $selectwhere = "edulevel = 0 and crud <> 'u'"; + list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); + $this->assertEquals("edulevel = 0 and action NOT LIKE '%update%'", $updatewhere); + + $selectwhere = "edulevel = 0 and crud = 'r'"; + list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); + $this->assertEquals("edulevel = 0 and action LIKE '%view%' OR action LIKE '%report%'", $updatewhere); + + $selectwhere = "edulevel = 0 and crud != 'r'"; + list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); + $this->assertEquals("edulevel = 0 and action NOT LIKE '%view%' AND action NOT LIKE '%report%'", $updatewhere); + + $selectwhere = "edulevel = 0 and crud <> 'r'"; + list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); + $this->assertEquals("edulevel = 0 and action NOT LIKE '%view%' AND action NOT LIKE '%report%'", $updatewhere); + + // The slq is incorrect, since quotes must not be present. Make sure this is not parsed. + $selectwhere = "edulevel = 0 and 'crud' != 'u'"; + list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); + $this->assertNotEquals("edulevel = 0 and action NOT LIKE '%update%'", $updatewhere); + + $selectwhere = "edulevel = 0 and crud = 'u' OR crud != 'r' or crud <> 'd'"; + list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); + $this->assertEquals("edulevel = 0 and action LIKE '%update%' OR action NOT LIKE '%view%' AND action NOT LIKE '%report%' or action NOT LIKE '%delete%'", $updatewhere); + + // Test legacy field names are mapped. + $selectwhere = "timecreated = :timecreated and courseid = :courseid and contextinstanceid = :contextinstanceid and origin = :origin"; + $params = array('timecreated' => 2, 'courseid' => 3, 'contextinstanceid' => 4, 'origin' => 5 ); + $expectedparams = array('time' => 2, 'course' => 3, 'cmid' => 4, 'ip' => 5); + list($updatewhere, $replaceparams) = \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params); + $this->assertEquals("time = :time and course = :course and cmid = :cmid and ip = :ip", $updatewhere); + $this->assertSame($expectedparams, $replaceparams); + + // Test sorting parameters. + $selectwhere = "courseid = 3"; + $params = array(); + $sort = 'timecreated DESC'; + list($updatewhere, $replaceparams, $sort) = + \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params, $sort); + $this->assertSame('time DESC', $sort); + + $sort = 'courseid DESC'; + list($updatewhere, $replaceparams, $sort) = + \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params, $sort); + $this->assertSame('course DESC', $sort); + + $sort = 'contextinstanceid DESC'; + list($updatewhere, $replaceparams, $sort) = + \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params, $sort); + $this->assertSame('cmid DESC', $sort); + + $sort = 'origin DESC'; + list($updatewhere, $replaceparams, $sort) = + \logstore_legacy\test\unittest_logstore_legacy::replace_sql_legacy($selectwhere, $params, $sort); + $this->assertSame('ip DESC', $sort); } }