From f05840451ea8083432c4279ad92fc296bafe36bd Mon Sep 17 00:00:00 2001 From: =?utf8?q?David=20Mudr=C3=A1k?= Date: Wed, 6 Mar 2019 20:24:06 +0100 Subject: [PATCH] MDL-61667 analytics: Add methods to read and validate model declarations Allow every component to declare prediction models it provides via the db/analytics.php file. --- analytics/classes/manager.php | 103 ++++++++++++++++++ .../db_analytics_php/invalid_enabled.php | 37 +++++++ .../db_analytics_php/invalid_indicators.php | 36 ++++++ .../db_analytics_php/invalid_target.php | 36 ++++++ .../invalid_time_splitting.php | 36 ++++++ .../invalid_time_splitting_fq.php | 36 ++++++ .../db_analytics_php/missing_indicators.php | 32 ++++++ .../db_analytics_php/missing_target.php | 35 ++++++ .../fixtures/db_analytics_php/no_teaching.php | 38 +++++++ analytics/tests/manager_test.php | 101 +++++++++++++++++ 10 files changed, 490 insertions(+) create mode 100644 analytics/tests/fixtures/db_analytics_php/invalid_enabled.php create mode 100644 analytics/tests/fixtures/db_analytics_php/invalid_indicators.php create mode 100644 analytics/tests/fixtures/db_analytics_php/invalid_target.php create mode 100644 analytics/tests/fixtures/db_analytics_php/invalid_time_splitting.php create mode 100644 analytics/tests/fixtures/db_analytics_php/invalid_time_splitting_fq.php create mode 100644 analytics/tests/fixtures/db_analytics_php/missing_indicators.php create mode 100644 analytics/tests/fixtures/db_analytics_php/missing_target.php create mode 100644 analytics/tests/fixtures/db_analytics_php/no_teaching.php diff --git a/analytics/classes/manager.php b/analytics/classes/manager.php index 8fcf6e58b1f..7b41ee069b0 100644 --- a/analytics/classes/manager.php +++ b/analytics/classes/manager.php @@ -40,6 +40,11 @@ class manager { */ const DEFAULT_MLBACKEND = '\mlbackend_php\processor'; + /** + * Name of the file where components declare their models. + */ + const ANALYTICS_FILENAME = 'db/analytics.php'; + /** * @var \core_analytics\predictor[] */ @@ -669,4 +674,102 @@ class manager { return $classes; } + + /** + * Return the list of models declared by the given component. + * + * @param string $componentname The name of the component to load models for. + * @throws \coding_exception Exception thrown in case of invalid syntax. + * @return array The $models description array. + */ + public static function load_default_models_for_component(string $componentname): array { + + $dir = \core_component::get_component_directory($componentname); + + if (!$dir) { + // This is either an invalid component, or a core subsystem without its own root directory. + return []; + } + + $file = $dir . '/' . self::ANALYTICS_FILENAME; + + if (!is_readable($file)) { + return []; + } + + $models = null; + include($file); + + if (!isset($models) || !is_array($models) || empty($models)) { + return []; + } + + foreach ($models as &$model) { + if (!isset($model['enabled'])) { + $model['enabled'] = false; + } else { + $model['enabled'] = clean_param($model['enabled'], PARAM_BOOL); + } + } + + static::validate_models_declaration($models); + + return $models; + } + + /** + * Validate the declaration of prediction models according the syntax expected in the component's db folder. + * + * The expected structure looks like this: + * + * [ + * [ + * 'target' => '\fully\qualified\name\of\the\target\class', + * 'indicators' => [ + * '\fully\qualified\name\of\the\first\indicator', + * '\fully\qualified\name\of\the\second\indicator', + * ], + * 'timesplitting' => '\optional\name\of\the\time_splitting\class', + * 'enabled' => true, + * ], + * ]; + * + * @param array $models List of declared models. + * @throws \coding_exception Exception thrown in case of invalid syntax. + */ + public static function validate_models_declaration(array $models) { + + foreach ($models as $model) { + if (!isset($model['target'])) { + throw new \coding_exception('Missing target declaration'); + } + + if (!static::is_valid($model['target'], '\core_analytics\local\target\base')) { + throw new \coding_exception('Invalid target classname', $model['target']); + } + + if (empty($model['indicators']) || !is_array($model['indicators'])) { + throw new \coding_exception('Missing indicators declaration'); + } + + foreach ($model['indicators'] as $indicator) { + if (!static::is_valid($indicator, '\core_analytics\local\indicator\base')) { + throw new \coding_exception('Invalid indicator classname', $indicator); + } + } + + if (isset($model['timesplitting'])) { + if (substr($model['timesplitting'], 0, 1) !== '\\') { + throw new \coding_exception('Expecting fully qualified time splitting classname', $model['timesplitting']); + } + if (!static::is_valid($model['timesplitting'], '\core_analytics\local\time_splitting\base')) { + throw new \coding_exception('Invalid time splitting classname', $model['timesplitting']); + } + } + + if (!empty($model['enabled']) && !isset($model['timesplitting'])) { + throw new \coding_exception('Cannot enable a model without time splitting method specified'); + } + } + } } diff --git a/analytics/tests/fixtures/db_analytics_php/invalid_enabled.php b/analytics/tests/fixtures/db_analytics_php/invalid_enabled.php new file mode 100644 index 00000000000..c19165cf10d --- /dev/null +++ b/analytics/tests/fixtures/db_analytics_php/invalid_enabled.php @@ -0,0 +1,37 @@ +. + +/** + * Example of an invalid db/analytics.php file content used for unit tests. + * + * @package core_analytics + * @category test + * @copyright 2019 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$models = [ + [ + 'target' => 'test_target_course_level_shortname', + 'indicators' => [ + '\core_course\analytics\indicator\no_teacher', + ], + // Cannot be enabled without timesplitting defined. + 'enabled' => true, + ], +]; diff --git a/analytics/tests/fixtures/db_analytics_php/invalid_indicators.php b/analytics/tests/fixtures/db_analytics_php/invalid_indicators.php new file mode 100644 index 00000000000..0aa0a39ea31 --- /dev/null +++ b/analytics/tests/fixtures/db_analytics_php/invalid_indicators.php @@ -0,0 +1,36 @@ +. + +/** + * Example of an invalid db/analytics.php file content used for unit tests. + * + * @package core_analytics + * @category test + * @copyright 2019 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$models = [ + [ + 'target' => 'test_target_course_level_shortname', + 'indicators' => [ + '\core_course\analytics\indicator\no_teacher', + '\non\existing\class\name', + ], + ], +]; diff --git a/analytics/tests/fixtures/db_analytics_php/invalid_target.php b/analytics/tests/fixtures/db_analytics_php/invalid_target.php new file mode 100644 index 00000000000..0ea91f33e27 --- /dev/null +++ b/analytics/tests/fixtures/db_analytics_php/invalid_target.php @@ -0,0 +1,36 @@ +. + +/** + * Example of an invalid db/analytics.php file content used for unit tests. + * + * @package core_analytics + * @category test + * @copyright 2019 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$models = [ + [ + 'target' => 'there_should_be_a_valid_fully_qualified_classname_here', + 'indicators' => [ + '\core_course\analytics\indicator\no_teacher', + '\core_course\analytics\indicator\no_student', + ], + ], +]; diff --git a/analytics/tests/fixtures/db_analytics_php/invalid_time_splitting.php b/analytics/tests/fixtures/db_analytics_php/invalid_time_splitting.php new file mode 100644 index 00000000000..c0567a2c1c2 --- /dev/null +++ b/analytics/tests/fixtures/db_analytics_php/invalid_time_splitting.php @@ -0,0 +1,36 @@ +. + +/** + * Example of an invalid db/analytics.php file content used for unit tests. + * + * @package core_analytics + * @category test + * @copyright 2019 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$models = [ + [ + 'target' => 'test_target_course_level_shortname', + 'indicators' => [ + '\core_course\analytics\indicator\no_teacher', + ], + 'timesplitting' => '\non\existing\class\name', + ], +]; diff --git a/analytics/tests/fixtures/db_analytics_php/invalid_time_splitting_fq.php b/analytics/tests/fixtures/db_analytics_php/invalid_time_splitting_fq.php new file mode 100644 index 00000000000..32157f988ad --- /dev/null +++ b/analytics/tests/fixtures/db_analytics_php/invalid_time_splitting_fq.php @@ -0,0 +1,36 @@ +. + +/** + * Example of an invalid db/analytics.php file content used for unit tests. + * + * @package core_analytics + * @category test + * @copyright 2019 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$models = [ + [ + 'target' => 'test_target_course_level_shortname', + 'indicators' => [ + '\core_course\analytics\indicator\no_teacher', + ], + 'timesplitting' => 'local_customplugin_non_fully_qualified_class_name', + ], +]; diff --git a/analytics/tests/fixtures/db_analytics_php/missing_indicators.php b/analytics/tests/fixtures/db_analytics_php/missing_indicators.php new file mode 100644 index 00000000000..80e86027448 --- /dev/null +++ b/analytics/tests/fixtures/db_analytics_php/missing_indicators.php @@ -0,0 +1,32 @@ +. + +/** + * Example of an invalid db/analytics.php file content used for unit tests. + * + * @package core_analytics + * @category test + * @copyright 2019 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$models = [ + [ + 'target' => 'test_target_course_level_shortname', + ], +]; diff --git a/analytics/tests/fixtures/db_analytics_php/missing_target.php b/analytics/tests/fixtures/db_analytics_php/missing_target.php new file mode 100644 index 00000000000..3a58e575db5 --- /dev/null +++ b/analytics/tests/fixtures/db_analytics_php/missing_target.php @@ -0,0 +1,35 @@ +. + +/** + * Example of an invalid db/analytics.php file content used for unit tests. + * + * @package core_analytics + * @category test + * @copyright 2019 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$models = [ + [ + 'indicators' => [ + '\core_course\analytics\indicator\no_teacher', + '\core_course\analytics\indicator\no_student', + ], + ], +]; diff --git a/analytics/tests/fixtures/db_analytics_php/no_teaching.php b/analytics/tests/fixtures/db_analytics_php/no_teaching.php new file mode 100644 index 00000000000..ac43c145ef8 --- /dev/null +++ b/analytics/tests/fixtures/db_analytics_php/no_teaching.php @@ -0,0 +1,38 @@ +. + +/** + * Example of a valid db/analytics.php file content used for unit tests. + * + * @package core_analytics + * @category test + * @copyright 2019 David Mudrák + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +$models = [ + [ + 'target' => '\core\analytics\target\no_teaching', + 'indicators' => [ + '\core_course\analytics\indicator\no_teacher', + '\core_course\analytics\indicator\no_student', + ], + 'timesplitting' => '\core\analytics\time_splitting\single_range', + 'enabled' => true, + ], +]; diff --git a/analytics/tests/manager_test.php b/analytics/tests/manager_test.php index f4c2a1c06bf..ce09c9e12c4 100644 --- a/analytics/tests/manager_test.php +++ b/analytics/tests/manager_test.php @@ -150,4 +150,105 @@ class analytics_manager_testcase extends advanced_testcase { get_log_manager(true); } + /** + * Tests for the {@link \core_analytics\manager::load_default_models_for_component()} implementation. + */ + public function test_load_default_models_for_component() { + $this->resetAfterTest(); + + // Attempting to load builtin models should always work without throwing exception. + \core_analytics\manager::load_default_models_for_component('core'); + + // Attempting to load from a core subsystem without its own subsystem directory. + $this->assertSame([], \core_analytics\manager::load_default_models_for_component('core_access')); + + // Attempting to load from a non-existing subsystem. + $this->assertSame([], \core_analytics\manager::load_default_models_for_component('core_nonexistingsubsystem')); + + // Attempting to load from a non-existing plugin of a known plugin type. + $this->assertSame([], \core_analytics\manager::load_default_models_for_component('mod_foobarbazquaz12240996776')); + + // Attempting to load from a non-existing plugin type. + $this->assertSame([], \core_analytics\manager::load_default_models_for_component('foo_bar2776327736558')); + } + + /** + * Tests for the successful execution of the {@link \core_analytics\manager::validate_models_declaration()}. + */ + public function test_validate_models_declaration() { + $this->resetAfterTest(); + + // This is expected to run without an exception. + $models = $this->load_models_from_fixture_file('no_teaching'); + \core_analytics\manager::validate_models_declaration($models); + } + + /** + * Tests for the exceptions thrown by {@link \core_analytics\manager::validate_models_declaration()}. + * + * @dataProvider validate_models_declaration_exceptions_provider + * @param array $models Models declaration. + * @param string $exception Expected coding exception message. + */ + public function test_validate_models_declaration_exceptions(array $models, string $exception) { + $this->resetAfterTest(); + + $this->expectException(\coding_exception::class); + $this->expectExceptionMessage($exception); + \core_analytics\manager::validate_models_declaration($models); + } + + /** + * Data provider for the {@link self::test_validate_models_declaration_exceptions()}. + * + * @return array of (string)testcase => [(array)models, (string)expected exception message] + */ + public function validate_models_declaration_exceptions_provider() { + return [ + 'missing_target' => [ + $this->load_models_from_fixture_file('missing_target'), + 'Missing target declaration', + ], + 'invalid_target' => [ + $this->load_models_from_fixture_file('invalid_target'), + 'Invalid target classname', + ], + 'missing_indicators' => [ + $this->load_models_from_fixture_file('missing_indicators'), + 'Missing indicators declaration', + ], + 'invalid_indicators' => [ + $this->load_models_from_fixture_file('invalid_indicators'), + 'Invalid indicator classname', + ], + 'invalid_time_splitting' => [ + $this->load_models_from_fixture_file('invalid_time_splitting'), + 'Invalid time splitting classname', + ], + 'invalid_time_splitting_fq' => [ + $this->load_models_from_fixture_file('invalid_time_splitting_fq'), + 'Expecting fully qualified time splitting classname', + ], + 'invalid_enabled' => [ + $this->load_models_from_fixture_file('invalid_enabled'), + 'Cannot enable a model without time splitting method specified', + ], + ]; + } + + /** + * Loads models as declared in the given fixture file. + * + * @param string $filename + * @return array + */ + protected function load_models_from_fixture_file(string $filename) { + global $CFG; + + $models = null; + + require($CFG->dirroot.'/analytics/tests/fixtures/db_analytics_php/'.$filename.'.php'); + + return $models; + } } -- 2.17.1