Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BFH-1: Improve verbal feedback questionnaire performance by reducing … #33

Merged
merged 4 commits into from
Nov 3, 2023
Merged

BFH-1: Improve verbal feedback questionnaire performance by reducing … #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bfb4e5b
BFH-1: Improve verbal feedback questionnaire performance by reducing …
gthomas2 Oct 9, 2023
c1b5603
Github Moodle Plugin CI Actions updated.
lucaboesch Oct 9, 2023
8473240
Merge pull request #1 from lucaboesch/guypluginci
gthomas2 Oct 10, 2023
f2bfecc
BFH-1: Additional performance improvements (on saving mod instance)
gthomas2 Oct 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 38 additions & 52 deletions .github/workflows/moodle-plugin-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ jobs:

services:
postgres:
image: postgres:12
image: postgres:13
env:
POSTGRES_USER: 'postgres'
POSTGRES_HOST_AUTH_METHOD: 'trust'
ports:
- 5432:5432
options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 3
mariadb:
image: mariadb:10.5
image: mariadb:10.6
env:
MYSQL_USER: 'root'
MYSQL_ALLOW_EMPTY_PASSWORD: "true"
Expand All @@ -28,6 +28,42 @@ jobs:
fail-fast: false
matrix:
include:
- php: 8.2
moodle-branch: MOODLE_403_STABLE
database: pgsql
- php: 8.2
moodle-branch: MOODLE_403_STABLE
database: mariadb
- php: 8.1
moodle-branch: MOODLE_403_STABLE
database: pgsql
- php: 8.1
moodle-branch: MOODLE_403_STABLE
database: mariadb
- php: 8.0
moodle-branch: MOODLE_403_STABLE
database: pgsql
- php: 8.0
moodle-branch: MOODLE_403_STABLE
database: mariadb
- php: 8.2
moodle-branch: MOODLE_402_STABLE
database: pgsql
- php: 8.2
moodle-branch: MOODLE_402_STABLE
database: mariadb
- php: 8.1
moodle-branch: MOODLE_402_STABLE
database: pgsql
- php: 8.1
moodle-branch: MOODLE_402_STABLE
database: mariadb
- php: 8.0
moodle-branch: MOODLE_402_STABLE
database: pgsql
- php: 8.0
moodle-branch: MOODLE_402_STABLE
database: mariadb
- php: 8.1
moodle-branch: MOODLE_401_STABLE
database: pgsql
Expand Down Expand Up @@ -58,56 +94,6 @@ jobs:
- php: 7.4
moodle-branch: MOODLE_400_STABLE
database: mariadb
- php: 7.4
moodle-branch: MOODLE_311_STABLE
database: pgsql
- php: 7.4
moodle-branch: MOODLE_311_STABLE
database: mariadb
- php: 7.4
moodle-branch: MOODLE_310_STABLE
database: pgsql
- php: 7.4
moodle-branch: MOODLE_310_STABLE
database: mariadb
- php: 7.4
moodle-branch: MOODLE_39_STABLE
database: pgsql
- php: 7.4
moodle-branch: MOODLE_39_STABLE
database: mariadb

- php: 7.3
moodle-branch: MOODLE_311_STABLE
database: pgsql
- php: 7.3
moodle-branch: MOODLE_311_STABLE
database: mariadb
- php: 7.3
moodle-branch: MOODLE_310_STABLE
database: pgsql
- php: 7.3
moodle-branch: MOODLE_310_STABLE
database: mariadb
- php: 7.3
moodle-branch: MOODLE_39_STABLE
database: pgsql
- php: 7.3
moodle-branch: MOODLE_39_STABLE
database: mariadb

- php: 7.2
moodle-branch: MOODLE_310_STABLE
database: pgsql
- php: 7.2
moodle-branch: MOODLE_310_STABLE
database: mariadb
- php: 7.2
moodle-branch: MOODLE_39_STABLE
database: pgsql
- php: 7.2
moodle-branch: MOODLE_39_STABLE
database: mariadb

steps:
- name: Check out repository code
Expand Down
155 changes: 122 additions & 33 deletions classes/repository/instance_repository.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@

defined('MOODLE_INTERNAL') || die();

raise_memory_limit(MEMORY_HUGE);

use Exception;
use mod_verbalfeedback\model\instance;
use mod_verbalfeedback\model\instance_category;
use mod_verbalfeedback\model\instance_status;
use mod_verbalfeedback\repository\model\db_instance;
use mod_verbalfeedback\repository\model\db_instance_category;
Expand All @@ -52,50 +51,45 @@ class instance_repository {
*/
public static function get_by_id(int $id) : instance {
global $DB;

// Return cached $byid instance if available and if we are not running a php unit test.
static $byid = [];
if (isset($byid[$id]) && !PHPUNIT_TEST) {
return $byid[$id];
}

$dboinstance = $DB->get_record(tables::INSTANCE_TABLE, ["id" => $id]);
$instance = db_instance::to_instance($dboinstance);

$dbocategories = $DB->get_records(tables::INSTANCE_CATEGORY_TABLE, ["instanceid" => $id]);

$sortedstrings = [];
$dboheaders = $DB->get_records(tables::LOCALIZED_STRING_TABLE);
foreach ($dboheaders as $dboheader) {
$dbo_obj = new db_localized_string;
$dbo_obj->id = $dboheader->id;
$dbo_obj->languageid = $dboheader->languageid;
$dbo_obj->string = $dboheader->string;
$dbo_obj->type = $dboheader->type;
$dbo_obj->foreignkey = $dboheader->foreignkey;

$sortedstrings[$dbo_obj->type][$dbo_obj->foreignkey][$dbo_obj->languageid] = $dbo_obj;
}
$criteriabycatid = self::get_criteria_by_category_for_instance_id($id);
$subratingsbycritid = self::get_subratings_by_criterion_for_instance($id);

foreach ($dbocategories as $dbocategory) {
$category = db_instance_category::to_instance_category($dbocategory);

// Load category headers.
$dbolocalizedstrings = $sortedstrings[localized_string_type::INSTANCE_CATEGORY_HEADER][$category->get_id()] ?? [];
$dbolocalizedstrings = self::get_strings(localized_string_type::INSTANCE_CATEGORY_HEADER, $category->get_id());
foreach ($dbolocalizedstrings as $dbo) {
$header = db_localized_string::to_localized_string($dbo);
$category->add_header($header);
}

// Load category criteria.
$dbocriteria = $DB->get_records(tables::INSTANCE_CRITERION_TABLE, ["categoryid" => $category->get_id()]);
foreach ($dbocriteria as $dbocriterion) {
$criterion = db_instance_criterion::to_instance_criterion($dbocriterion);
$criteria = $criteriabycatid[$category->get_id()];
foreach ($criteria as $criterion) {

// Load criterion description.
$dbodescriptions = $sortedstrings[localized_string_type::INSTANCE_CRITERION][$criterion->get_id()] ?? [];
$dbodescriptions = self::get_strings(localized_string_type::INSTANCE_CRITERION, $criterion->get_id());
foreach ($dbodescriptions as $dbodescription) {
$description = db_localized_string::to_localized_string($dbodescription);
$criterion->add_description($description);
}

// Load criterion subratings.
$dbosubratings = $DB->get_records(tables::INSTANCE_SUBRATING_TABLE, ["criterionid" => $criterion->get_id()]);
foreach ($dbosubratings as $dbosubrating) {
$subrating = db_instance_subrating::to_subrating($dbosubrating);
$subratings = $subratingsbycritid[$criterion->get_id()];
foreach ($subratings as $subrating) {
foreach ([
// Load subrating titles (1 title per language).
localized_string_type::INSTANCE_SUBRATING_TITLE => 'titles',
Expand All @@ -110,48 +104,43 @@ public static function get_by_id(int $id) : instance {
// Load subrating very positive texts.
localized_string_type::INSTANCE_SUBRATING_VERY_POSITIVE => 'verypositives',
] as $type => $attribute) {
$dbotitles = $sortedstrings[$type][$subrating->get_id()] ?? [];
$dbotitles = self::get_strings($type, $subrating->get_id());
foreach ($dbotitles as $dbotitle) {
$item = db_localized_string::to_localized_string($dbotitle);
$subrating->{$attribute}[] = $item;
}
}

// Load subrating descriptions.
$dbosubratingdescriptions = $DB->get_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_SUBRATING_DESCRIPTION, "foreignkey" => $subrating->get_id()]);
$dbosubratingdescriptions = self::get_strings(localized_string_type::INSTANCE_SUBRATING_DESCRIPTION, $subrating->get_id());
foreach ($dbosubratingdescriptions as $dbosubratingdescription) {
$subratingdescription = db_localized_string::to_localized_string($dbosubratingdescription);
$subrating->add_description($subratingdescription);
}

// Load subrating very negative texts.
$dboverynegatives = $DB->get_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_SUBRATING_VERY_NEGATIVE, "foreignkey" => $subrating->get_id()]);
$dboverynegatives = self::get_strings(localized_string_type::INSTANCE_SUBRATING_VERY_NEGATIVE, $subrating->get_id());
foreach ($dboverynegatives as $dboverynegative) {
$verynegative = db_localized_string::to_localized_string($dboverynegative);
$subrating->add_verynegative($verynegative);
}

// Load subrating negative texts.
$dbonegatives = $DB->get_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_SUBRATING_NEGATIVE, "foreignkey" => $subrating->get_id()]);
$dbonegatives = self::get_strings(localized_string_type::INSTANCE_SUBRATING_NEGATIVE, $subrating->get_id());
foreach ($dbonegatives as $dbonegative) {
$negative = db_localized_string::to_localized_string($dbonegative);
$subrating->add_negative($negative);
}

// Load subrating positive texts.
$dbopositives = $DB->get_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_SUBRATING_POSITIVE, "foreignkey" => $subrating->get_id()]);
$dbopositives = self::get_strings(localized_string_type::INSTANCE_SUBRATING_POSITIVE, $subrating->get_id());
foreach ($dbopositives as $dbopositive) {
$positive = db_localized_string::to_localized_string($dbopositive);
$subrating->add_positive($positive);
}

// Load subrating very positive texts.
$dboverypositives = $DB->get_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_SUBRATING_VERY_POSITIVE, "foreignkey" => $subrating->get_id()]);
$dboverypositives = self::get_strings(localized_string_type::INSTANCE_SUBRATING_VERY_POSITIVE, $subrating->get_id());
foreach ($dboverypositives as $dboverypositive) {
$verypositive = db_localized_string::to_localized_string($dboverypositive);
$subrating->add_verypositive($verypositive);
Expand All @@ -162,9 +151,109 @@ public static function get_by_id(int $id) : instance {
}
$instance->add_category($category);
}

// Add to cache.
$byid[$id] = $instance;
return $instance;
}

private static function get_strings(string $type, int $subratingid, bool $throwonerror = false): array {
global $DB;

static $sortedstrings = null;

if ($sortedstrings === null || PHPUNIT_TEST) {
$sortedstrings = [];
$rs = $DB->get_recordset(tables::LOCALIZED_STRING_TABLE);
foreach ($rs as $dboheader) {
$dbo_obj = new db_localized_string;
$dbo_obj->id = $dboheader->id;
$dbo_obj->languageid = $dboheader->languageid;
$dbo_obj->string = $dboheader->string;
$dbo_obj->type = $dboheader->type;
$dbo_obj->foreignkey = $dboheader->foreignkey;

$sortedstrings[$dbo_obj->type][$dbo_obj->foreignkey][$dbo_obj->languageid] = $dbo_obj;
}
$rs->close();
}

if (!isset($sortedstrings[$type])) {
if ($throwonerror) {
throw new \coding_exception("Invalid type .$type");
}
return [];
}
if (!isset($sortedstrings[$type][$subratingid])) {
if ($throwonerror) {
throw new \coding_exception("Invalid subratingid $subratingid for type $type");
}
return [];
}

return $sortedstrings[$type][$subratingid];
}

/**
* Gets all criteria hashed by category id for a specific instance.
* @param int $id - instance id
* @return array
* @throws \dml_exception
*/
private static function get_criteria_by_category_for_instance_id(int $id): array {
global $DB;

$crittab = tables::INSTANCE_CRITERION_TABLE;
$cattab = tables::INSTANCE_CATEGORY_TABLE;

$sql = "SELECT crit.*
FROM {{$crittab}} crit
JOIN {{$cattab}} cat
ON crit.categoryid = cat.id
WHERE cat.instanceid = ?";

$bycat = [];
$rs = $DB->get_recordset_sql($sql, [$id]);
foreach ($rs as $dbocriterion) {
if (!isset($bycat[$dbocriterion->categoryid])) {
$bycat[$dbocriterion->categoryid] = [];
}
$bycat[$dbocriterion->categoryid][$dbocriterion->id] = db_instance_criterion::to_instance_criterion($dbocriterion);
}
$rs->close();
return $bycat;
}

/**
* Get subratings hashed by criterion id for an instance.
* @param int $id - instance id
* @return array
* @throws \dml_exception
*/
private static function get_subratings_by_criterion_for_instance(int $id): array {
global $DB;
$crittab = tables::INSTANCE_CRITERION_TABLE;
$cattab = tables::INSTANCE_CATEGORY_TABLE;
$srattab = tables::INSTANCE_SUBRATING_TABLE;
$sql = "SELECT srat.*
FROM {{$srattab}} srat
JOIN {{$crittab}} crit
ON srat.criterionid = crit.id
JOIN {{$cattab}} mvic
ON crit.categoryid = mvic.id
WHERE mvic.instanceid = ?";
$rs = $DB->get_recordset_sql($sql, [$id]);
$bycrit = [];
foreach ($rs as $dbosubrating) {
if (!isset($bycrit[$dbosubrating->criterionid])) {
$bycrit[$dbosubrating->criterionid] = [];
}
$bycrit[$dbosubrating->criterionid][$dbosubrating->id] = db_instance_subrating::to_subrating($dbosubrating);
}
$rs->close();
return $bycrit;
}

/**
* Save the given instance
* @param instance $instance The instance object.
Expand Down
Loading
Loading