Skip to content

Commit

Permalink
Change column type from type to typeid in languages. Optimize string …
Browse files Browse the repository at this point in the history
…fetching for templates.
  • Loading branch information
srobotta committed Oct 22, 2024
1 parent 4b6bb46 commit ef0a12b
Show file tree
Hide file tree
Showing 7 changed files with 129 additions and 81 deletions.
25 changes: 14 additions & 11 deletions classes/repository/instance_repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,16 @@ private static function get_strings(string $type, int $subratingid, bool $throwo

if (!array_key_exists($cachekey, $sortedstrings) || PHPUNIT_TEST) {
$sortedstrings[$cachekey] = [];
$rs = $DB->get_recordset(tables::LOCALIZED_STRING_TABLE, ['type' => $type, 'foreignkey' => $subratingid]);
$rs = $DB->get_recordset(
tables::LOCALIZED_STRING_TABLE,
['typeid' => localized_string_type::str2id($type), 'foreignkey' => $subratingid]
);
foreach ($rs as $dboheader) {
$dbobj = new db_localized_string;
$dbobj->id = $dboheader->id;
$dbobj->languageid = $dboheader->languageid;
$dbobj->string = $dboheader->string;
$dbobj->type = $dboheader->type;
$dbobj->typeid = $dboheader->typeid;
$dbobj->foreignkey = $dboheader->foreignkey;

$sortedstrings[$cachekey][$dbobj->languageid] = $dbobj;
Expand Down Expand Up @@ -277,7 +280,7 @@ public function save(instance $instance) {
$id = $DB->insert_record(tables::INSTANCE_TABLE, $dboinstance);
$instance->set_id($id);
// Set the grade.
$DB->set_field('verbalfeedback', 'grade', $instance->grade, ['id' => $id]);
$DB->set_field(tables::INSTANCE_TABLE, 'grade', $instance->grade, ['id' => $id]);
} else {
$DB->update_record(tables::INSTANCE_TABLE, $dboinstance);
}
Expand Down Expand Up @@ -408,43 +411,43 @@ public function delete(int $instanceid) {
foreach ($dbocategories as $dbocategory) {
// Delete category headers.
$DB->delete_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_CATEGORY_HEADER, "foreignkey" => $dbocategory->id]);
["typeid" => localized_string_type::str2id(localized_string_type::INSTANCE_CATEGORY_HEADER), "foreignkey" => $dbocategory->id]);

// Delete category criteria.
$dbocriteria = $DB->get_records(tables::INSTANCE_CRITERION_TABLE, ["categoryid" => $dbocategory->id]);
foreach ($dbocriteria as $dbocriterion) {

// Delete criterion description.
$dbodescriptions = $DB->delete_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_CRITERION, "foreignkey" => $dbocriterion->id]);
["typeid" => localized_string_type::str2id(localized_string_type::INSTANCE_CRITERION), "foreignkey" => $dbocriterion->id]);

// Delete criterion subratings.
$dbosubratings = $DB->get_records(tables::INSTANCE_SUBRATING_TABLE, ["criterionid" => $dbocriterion->id]);
foreach ($dbosubratings as $dbosubrating) {

// Delete subrating titles.
$DB->delete_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_SUBRATING_TITLE, "foreignkey" => $dbosubrating->id]);
["typeid" => localized_string_type::str2id(localized_string_type::INSTANCE_SUBRATING_TITLE), "foreignkey" => $dbosubrating->id]);

// Delete subrating descriptions.
$DB->delete_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_SUBRATING_DESCRIPTION, "foreignkey" => $dbosubrating->id]);
["typeid" => localized_string_type::str2id(localized_string_type::INSTANCE_SUBRATING_DESCRIPTION), "foreignkey" => $dbosubrating->id]);

// Delete subrating very negative texts.
$DB->delete_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_SUBRATING_VERY_NEGATIVE, "foreignkey" => $dbosubrating->id]);
["typeid" => localized_string_type::str2id(localized_string_type::INSTANCE_SUBRATING_VERY_NEGATIVE), "foreignkey" => $dbosubrating->id]);

// Delete subrating negative texts.
$DB->delete_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_SUBRATING_NEGATIVE, "foreignkey" => $dbosubrating->id]);
["typeid" => localized_string_type::str2id(localized_string_type::INSTANCE_SUBRATING_NEGATIVE), "foreignkey" => $dbosubrating->id]);

// Delete subrating positive texts.
$DB->delete_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_SUBRATING_POSITIVE, "foreignkey" => $dbosubrating->id]);
["typeid" => localized_string_type::str2id(localized_string_type::INSTANCE_SUBRATING_POSITIVE), "foreignkey" => $dbosubrating->id]);

// Delete subrating very positive texts.
$DB->delete_records(tables::LOCALIZED_STRING_TABLE,
["type" => localized_string_type::INSTANCE_SUBRATING_VERY_POSITIVE, "foreignkey" => $dbosubrating->id]);
["typeid" => localized_string_type::str2id(localized_string_type::INSTANCE_SUBRATING_VERY_POSITIVE), "foreignkey" => $dbosubrating->id]);
}

// Delete subratings.
Expand Down
6 changes: 3 additions & 3 deletions classes/repository/model/db_localized_string.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ class db_localized_string {
*/
public $id;
/**
* @var string The type
* @var int The type
*/
public $type;
public $typeid;
/**
* @var int The foreign key
*/
Expand Down Expand Up @@ -66,7 +66,7 @@ public static function from_localized_string(localized_string $localizedstring,
}
$dbo = new db_localized_string();
$dbo->id = $localizedstring->get_id();
$dbo->type = $type;
$dbo->typeid = localized_string_type::str2id($type);
$dbo->foreignkey = $foreignkey;
$dbo->languageid = $localizedstring->get_language_id();
$dbo->string = $localizedstring->get_string();
Expand Down
92 changes: 56 additions & 36 deletions classes/repository/model/localized_string_type.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,48 +62,68 @@ class localized_string_type {
public const TEMPLATE_SUBRATING_VERY_POSITIVE = 'template_subrating_verypositive';

/**
* Return whether a type exists
* Return all existing types in a order, so that the numeric value + 1 can
* be used as a string.
*
* @return string[]
*/
public static function getStringTypes(): array {
return [
self::INSTANCE_CRITERION,
self::INSTANCE_CATEGORY_HEADER,
self::INSTANCE_SUBRATING_TITLE,
self::INSTANCE_SUBRATING_DESCRIPTION,
self::INSTANCE_SUBRATING_VERY_NEGATIVE,
self::INSTANCE_SUBRATING_NEGATIVE,
self::INSTANCE_SUBRATING_POSITIVE,
self::INSTANCE_SUBRATING_VERY_POSITIVE,
self::TEMPLATE_CRITERION,
self::TEMPLATE_CATEGORY_HEADER,
self::TEMPLATE_SUBRATING_TITLE,
self::TEMPLATE_SUBRATING_DESCRIPTION,
self::TEMPLATE_SUBRATING_VERY_NEGATIVE,
self::TEMPLATE_SUBRATING_NEGATIVE,
self::TEMPLATE_SUBRATING_POSITIVE,
self::TEMPLATE_SUBRATING_VERY_POSITIVE,
];
}
/**
* Return whether a type string exists.
*
* @param string $type A string type
* @return bool If a string type exists
*/
public static function exists(string $type) {
switch($type) {
case self::INSTANCE_CRITERION:
return true;
case self::INSTANCE_CATEGORY_HEADER:
return true;
case self::INSTANCE_SUBRATING_TITLE:
return true;
case self::INSTANCE_SUBRATING_DESCRIPTION:
return true;
case self::INSTANCE_SUBRATING_VERY_NEGATIVE:
return true;
case self::INSTANCE_SUBRATING_NEGATIVE:
return true;
case self::INSTANCE_SUBRATING_POSITIVE:
return true;
case self::INSTANCE_SUBRATING_VERY_POSITIVE:
return true;
return in_array($type, self::getStringTypes());
}

case self::TEMPLATE_CRITERION:
return true;
case self::TEMPLATE_CATEGORY_HEADER:
return true;
case self::TEMPLATE_SUBRATING_TITLE:
return true;
case self::TEMPLATE_SUBRATING_DESCRIPTION:
return true;
case self::TEMPLATE_SUBRATING_VERY_NEGATIVE:
return true;
case self::TEMPLATE_SUBRATING_NEGATIVE:
return true;
case self::TEMPLATE_SUBRATING_POSITIVE:
return true;
case self::TEMPLATE_SUBRATING_VERY_POSITIVE:
return true;
/**
* Convert string constant to id.
*
* @param string $type
* @return int
* @throws \InvalidArgumentException
*/
public static function str2id(string $type):int {
$key = array_search($type, self::getStringTypes());
if ($key === false) {
throw new \InvalidArgumentException("Invalid str: $type");
}
return $key + 1;
}

default:
return false;
/**
* Convert id to string contant.
*
* @param int $id
* @return string
* @throws \InvalidArgumentException
*/
public static function id2str(int $id):string {
$constants = self::getStringTypes();
if ($id < 1 || $id > count($constants)) {
throw new \InvalidArgumentException("Invalid id: $id");
}
return $constants[$id - 1];
}
}
13 changes: 9 additions & 4 deletions classes/repository/template_category_repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,10 @@ public function delete_by_id(int $id): bool {
global $DB;
try {
$transaction = $DB->start_delegated_transaction();
$DB->delete_records(tables::LOCALIZED_STRING_TABLE, ['foreignkey' => $id,
'type' => localized_string_type::TEMPLATE_CATEGORY_HEADER, ]);
$DB->delete_records(tables::LOCALIZED_STRING_TABLE, [
'foreignkey' => $id,
'typeid' => localized_string_type::str2id(localized_string_type::TEMPLATE_CATEGORY_HEADER),
]);
$DB->delete_records(tables::PARAMETRIZED_TEMPLATE_CRITERION_TABLE, ['categoryid' => $id]);
$DB->delete_records(tables::TEMPLATE_CATEGORY_TABLE, ['id' => $id]);

Expand All @@ -163,7 +165,7 @@ private function get_headers($foreignkey): array {
global $DB;

$dboheaders = $DB->get_records(tables::LOCALIZED_STRING_TABLE,
['foreignkey' => $foreignkey, 'type' => localized_string_type::TEMPLATE_CATEGORY_HEADER]);
['foreignkey' => $foreignkey, 'typeid' => localized_string_type::str2id(localized_string_type::TEMPLATE_CATEGORY_HEADER)]);
$headers = [];
foreach ($dboheaders as $dboheader) {
$headers[] = db_localized_string::to_localized_string($dboheader);
Expand All @@ -182,7 +184,10 @@ private function get_headers_by_category_ids(): array {
global $DB;

$headers = [];
$rs = $DB->get_recordset(tables::LOCALIZED_STRING_TABLE, ['type' => localized_string_type::TEMPLATE_CATEGORY_HEADER]);
$rs = $DB->get_recordset(
tables::LOCALIZED_STRING_TABLE,
['typeid' => localized_string_type::str2id(localized_string_type::TEMPLATE_CATEGORY_HEADER)]
);
foreach ($rs as $row) {
if (!\array_key_exists($row->foreignkey, $headers)) {
$headers[$row->foreignkey] = [];
Expand Down
48 changes: 22 additions & 26 deletions classes/repository/template_criterion_repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -243,23 +243,23 @@ public function delete(int $id) {
$dbosubratings = $DB->get_records(tables::TEMPLATE_SUBRATINGS_TABLE, ['criterionid' => $id]);
foreach ($dbosubratings as $dbosubrating) {
$DB->delete_records(tables::LOCALIZED_STRING_TABLE, ['foreignkey' => $dbosubrating->id,
'type' => localized_string_type::TEMPLATE_SUBRATING_TITLE, ], );
'typeid' => localized_string_type::str2id(localized_string_type::TEMPLATE_SUBRATING_TITLE), ], );
$DB->delete_records(tables::LOCALIZED_STRING_TABLE, ['foreignkey' => $dbosubrating->id,
'type' => localized_string_type::TEMPLATE_SUBRATING_DESCRIPTION, ], );
'typeid' => localized_string_type::str2id(localized_string_type::TEMPLATE_SUBRATING_DESCRIPTION), ], );
$DB->delete_records(tables::LOCALIZED_STRING_TABLE, ['foreignkey' => $dbosubrating->id,
'type' => localized_string_type::TEMPLATE_SUBRATING_VERY_NEGATIVE, ], );
'typeid' => localized_string_type::str2id(localized_string_type::TEMPLATE_SUBRATING_VERY_NEGATIVE), ], );
$DB->delete_records(tables::LOCALIZED_STRING_TABLE, ['foreignkey' => $dbosubrating->id,
'type' => localized_string_type::TEMPLATE_SUBRATING_NEGATIVE, ], );
'typeid' => localized_string_type::str2id(localized_string_type::TEMPLATE_SUBRATING_NEGATIVE), ], );
$DB->delete_records(tables::LOCALIZED_STRING_TABLE, ['foreignkey' => $dbosubrating->id,
'type' => localized_string_type::TEMPLATE_SUBRATING_POSITIVE, ], );
'typeid' => localized_string_type::str2id(localized_string_type::TEMPLATE_SUBRATING_POSITIVE), ], );
$DB->delete_records(tables::LOCALIZED_STRING_TABLE, ['foreignkey' => $dbosubrating->id,
'type' => localized_string_type::TEMPLATE_SUBRATING_VERY_POSITIVE, ], );
'typeid' => localized_string_type::str2id(localized_string_type::TEMPLATE_SUBRATING_VERY_POSITIVE), ], );
}
$DB->delete_records(tables::TEMPLATE_SUBRATINGS_TABLE, ['criterionid' => $id]);

// Delete localized strings.
$DB->delete_records(tables::LOCALIZED_STRING_TABLE, ['foreignkey' => $id,
'type' => localized_string_type::TEMPLATE_CRITERION, ], );
'typeid' => localized_string_type::str2id(localized_string_type::TEMPLATE_CRITERION), ], );

// Delete criterion.
$DB->delete_records(tables::TEMPLATE_CRITERION_TABLE, ['id' => $id]);
Expand All @@ -286,36 +286,33 @@ public static function dbo_to_template_criterion($dbo) {
/**
* Get all localized strings for a type, hashed by foreignkey, cached in memory for speed.
*
* // phpcs:ignore moodle.Commenting.ValidTags.Invalid
* @TODO - evaluate this for memory usage.
* @param string $type
* @param int $foreignkey
* @return array<localized_string[]>
* @throws \dml_exception
*/
private function get_all_localized_strings_for_type(string $type): array {
private function get_all_localized_strings_for_type(string $type, int $foreignkey): array {
global $DB;

static $strings = [];

if (isset($strings[$type]) && !PHPUNIT_TEST) {
$typeid = localized_string_type::str2id($type);
$cachekey = $typeid . '~~' . $foreignkey;

if (array_key_exists($cachekey, $strings) && !PHPUNIT_TEST) {
// We already have this cached, so return it.
return $strings[$type];
return $strings[$cachekey];
}

$strings[$type] = [];
$strings[$cachekey] = [];

$rs = $DB->get_recordset(tables::LOCALIZED_STRING_TABLE, ['type' => $type]);
$rs = $DB->get_recordset(tables::LOCALIZED_STRING_TABLE, ['typeid' => $typeid, 'foreignkey' => $foreignkey]);
foreach ($rs as $row) {
$type = $row->type;
$key = $row->foreignkey;
if (!isset($strings[$type][$key])) {
$strings[$type][$key] = [];
}
$strings[$type][$key][$row->id] = db_localized_string::to_localized_string($row);
$strings[$cachekey][$row->id] = db_localized_string::to_localized_string($row);
}
$rs->close();

return $strings[$type];
return $strings[$cachekey];
}

/**
Expand All @@ -331,13 +328,12 @@ private function get_localized_strings(int $foreignkey, string $type): array {
throw new \Exception("Unknown localized string type.");
}

$allstrings = $this->get_all_localized_strings_for_type($type);
$localizedstrings = $allstrings[$foreignkey] ?? null;
if (!$localizedstrings) {
throw new coding_exception("Couldn't find localized strings by type '$type' and foreignkey '$foreignkey'");
$allstrings = $this->get_all_localized_strings_for_type($type, $foreignkey);
if (empty($allstrings)) {
throw new \coding_exception("Couldn't find localized strings by type '$type' and foreignkey '$foreignkey'");
}

return $localizedstrings;
return $allstrings;
}

/**
Expand Down
24 changes: 24 additions & 0 deletions db/upgrade.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,5 +82,29 @@ function xmldb_verbalfeedback_upgrade($oldversion) {
upgrade_main_savepoint(true, 2021100103);
}

if ($oldversion < 2024101700) {
require_once($CFG->dirroot . '/mod/verbalfeedback/classes/repository/model/localized_string_type.php');
$table = new xmldb_table('verbalfeedback_local_string');
$field = new xmldb_field('typeid', XMLDB_TYPE_INTEGER, '3', XMLDB_UNSIGNED, XMLDB_NOTNULL, null, 0);

if (!$dbman->field_exists($table, $field)) {
$dbman->add_field($table, $field);
}

// Convert all strings to ids.
$offset = 0;
while ($records = $DB->get_records('verbalfeedback_local_string', null, 'id', 'id, type, typeid', $offset, 100)) {
foreach ($records as $record) {
$record->typeid = \mod_verbalfeedback\repository\model\localized_string_type::str2id($record->type);
$DB->update_record('verbalfeedback_local_string', $record);
}
$offset += 100;
}
$dbman->drop_field($table, new xmldb_field('type'));
$table->add_index('subitemtype', XMLDB_INDEX_NOTUNIQUE, ['foreignkey', 'typeid']);

upgrade_mod_savepoint(true, 2024101700, 'verbalfeedback');
}

return true;
}
2 changes: 1 addition & 1 deletion version.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
defined('MOODLE_INTERNAL') || die();

$plugin->component = 'mod_verbalfeedback';
$plugin->version = 2024071300;
$plugin->version = 2024101700;
$plugin->requires = 2022112800;
$plugin->maturity = MATURITY_STABLE;
$plugin->cron = 0;
Expand Down

0 comments on commit ef0a12b

Please sign in to comment.