From 93e71c712ddee02fb049a5cfe72ff0f57d689c42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Monlla=C3=B3?= Date: Tue, 17 Sep 2019 07:32:53 +0800 Subject: [PATCH] MDL-66091 report_insights: Unify contextwithinsights cache sets --- analytics/classes/manager.php | 39 +++++++++++++++++++++++++++++++++++ analytics/classes/model.php | 13 +----------- lib/deprecatedlib.php | 17 +++++++++++++++ report/insights/lib.php | 25 +++------------------- 4 files changed, 60 insertions(+), 34 deletions(-) diff --git a/analytics/classes/manager.php b/analytics/classes/manager.php index a5553d56197..699d22b7727 100644 --- a/analytics/classes/manager.php +++ b/analytics/classes/manager.php @@ -504,6 +504,45 @@ class manager { return $models; } + /** + * Returns the models that generated insights in the provided context. It can also be used to add new models to the context. + * + * Note that if you use this function with $newmodelid is the caller responsibility to ensure that the + * provided model id generated insights for the provided context. + * + * @throws \coding_exception + * @param \context $context + * @param int|null $newmodelid A new model to add to the list of models with insights in the provided context. + * @return int[] + */ + public static function cached_models_with_insights(\context $context, int $newmodelid = null) { + + $cache = \cache::make('core', 'contextwithinsights'); + $modelids = $cache->get($context->id); + if ($modelids === false) { + // The cache is empty, but we don't know if it is empty because there are no insights + // in this context or because cache/s have been purged, we need to be conservative and + // "pay" 1 db read to fill up the cache. + + $models = \core_analytics\manager::get_models_with_insights($context); + + if ($newmodelid && empty($models[$newmodelid])) { + throw new \coding_exception('The provided modelid ' . $newmodelid . ' did not generate any insights'); + } + + $modelids = array_keys($models); + $cache->set($context->id, $modelids); + + } else if ($newmodelid && !in_array($newmodelid, $modelids)) { + // We add the context we got as an argument to the cache. + + array_push($modelids, $newmodelid); + $cache->set($context->id, $modelids); + } + + return $modelids; + } + /** * Returns a prediction * diff --git a/analytics/classes/model.php b/analytics/classes/model.php index 83ccb923e4d..3678d97adba 100644 --- a/analytics/classes/model.php +++ b/analytics/classes/model.php @@ -966,19 +966,8 @@ class model { if ($this->get_target()->link_insights_report()) { // Update cache. - $cache = \cache::make('core', 'contextwithinsights'); foreach ($samplecontexts as $context) { - $modelids = $cache->get($context->id); - if (!$modelids) { - // The cache is empty, but we don't know if it is empty because there are no insights - // in this context or because cache/s have been purged, we need to be conservative and - // "pay" 1 db read to fill up the cache. - $models = \core_analytics\manager::get_models_with_insights($context); - $cache->set($context->id, array_keys($models)); - } else if (!in_array($this->get_id(), $modelids)) { - array_push($modelids, $this->get_id()); - $cache->set($context->id, $modelids); - } + \core_analytics\manager::cached_models_with_insights($context, $this->get_id()); } } } diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php index afc91d429f0..e8335e730ff 100644 --- a/lib/deprecatedlib.php +++ b/lib/deprecatedlib.php @@ -3449,3 +3449,20 @@ function get_courses_page($categoryid="all", $sort="c.sortorder ASC", $fields="c $rs->close(); return $visiblecourses; } + +/** + * Returns the models that generated insights in the provided context. + * + * @deprecated since Moodle 3.8 MDL-66091 - please do not use this function any more. + * @todo MDL-65799 This will be deleted in Moodle 4.2 + * @see \core_analytics\manager::cached_models_with_insights + * @param \context $context + * @return int[] + */ +function report_insights_context_insights(\context $context) { + + debugging('report_insights_context_insights is deprecated. Please use ' . + '\core_analytics\manager::cached_models_with_insights instead', DEBUG_DEVELOPER); + + return \core_analytics\manager::cached_models_with_insights($context); +} diff --git a/report/insights/lib.php b/report/insights/lib.php index 97c9ecb86e4..eb7b4f42139 100644 --- a/report/insights/lib.php +++ b/report/insights/lib.php @@ -36,7 +36,7 @@ function report_insights_extend_navigation_course($navigation, $course, $context if (has_capability('moodle/analytics:listinsights', $context)) { - $modelids = report_insights_context_insights($context); + $modelids = \core_analytics\manager::cached_models_with_insights($context); if (!empty($modelids)) { $url = new moodle_url('/report/insights/insights.php', array('contextid' => $context->id)); $node = navigation_node::create(get_string('insights', 'report_insights'), $url, navigation_node::TYPE_SETTING, @@ -61,7 +61,7 @@ function report_insights_myprofile_navigation(core_user\output\myprofile\tree $t $context = \context_user::instance($user->id); if (\core_analytics\manager::check_can_list_insights($context, true)) { - $modelids = report_insights_context_insights($context); + $modelids = \core_analytics\manager::cached_models_with_insights($context); if (!empty($modelids)) { $url = new moodle_url('/report/insights/insights.php', array('contextid' => $context->id)); $node = new core_user\output\myprofile\node('reports', 'insights', get_string('insights', 'report_insights'), @@ -82,7 +82,7 @@ function report_insights_extend_navigation_category_settings($navigation, $conte if (has_capability('moodle/analytics:listinsights', $context)) { - $modelids = report_insights_context_insights($context); + $modelids = \core_analytics\manager::cached_models_with_insights($context); if (!empty($modelids)) { $url = new moodle_url('/report/insights/insights.php', array('contextid' => $context->id)); @@ -99,22 +99,3 @@ function report_insights_extend_navigation_category_settings($navigation, $conte } } } - -/** - * Returns the models that generated insights in the provided context. - * - * @param \context $context - * @return int[] - */ -function report_insights_context_insights(\context $context) { - - $cache = \cache::make('core', 'contextwithinsights'); - $modelids = $cache->get($context->id); - if ($modelids === false) { - // They will be full unless a model has been cleared. - $models = \core_analytics\manager::get_models_with_insights($context); - $modelids = array_keys($models); - $cache->set($context->id, $modelids); - } - return $modelids; -}