From fd293598282e772041fdd7e1166e9551935b4540 Mon Sep 17 00:00:00 2001 From: Jan Britz Date: Thu, 15 Aug 2024 13:08:27 +0200 Subject: [PATCH] refactor: apply requested changes --- classes/external/load_packages.php | 12 +-- classes/external/remove_packages.php | 11 +-- classes/package/package.php | 93 +++++++++++------------ classes/package/package_info.php | 8 +- classes/package/package_versions_info.php | 29 ++++--- 5 files changed, 69 insertions(+), 84 deletions(-) diff --git a/classes/external/load_packages.php b/classes/external/load_packages.php index f7757b94..9da07f27 100644 --- a/classes/external/load_packages.php +++ b/classes/external/load_packages.php @@ -72,17 +72,11 @@ public static function execute(): array { // Remove old packages. if (empty($incomingpackageids)) { // There are no incoming packages -> remove every package. - $packages = package::get_records(); - foreach ($packages as $package) { - $package->delete(); - } + package::delete_all(); } else { - [$sql, $params] = $DB->get_in_or_equal($incomingpackageids, SQL_PARAMS_NAMED, 'packageid', false); + [$sql, $params] = $DB->get_in_or_equal($incomingpackageids, SQL_PARAMS_NAMED, prefix: 'packageid', equal: false); $removedpackageids = $DB->get_fieldset_select('qtype_questionpy_package', 'id', "id $sql", $params); - foreach ($removedpackageids as $id) { - [$package] = package::get_records(['id' => $id]); - $package->delete(); - } + package::delete_by_id(...$removedpackageids); } $transaction->allow_commit(); diff --git a/classes/external/remove_packages.php b/classes/external/remove_packages.php index c46a8544..c0fe9826 100644 --- a/classes/external/remove_packages.php +++ b/classes/external/remove_packages.php @@ -26,7 +26,7 @@ use external_single_structure; use external_value; use moodle_exception; -use qtype_questionpy\package\package_version; +use qtype_questionpy\package\package; /** * This service removes every package from the database. @@ -55,14 +55,7 @@ public static function execute_parameters(): external_function_parameters { public static function execute(): array { global $DB; - $transaction = $DB->start_delegated_transaction(); - - $versions = package_version::get_many(); - foreach ($versions as $version) { - $version->delete(); - } - - $transaction->allow_commit(); + package::delete_all(); return [ 'packages' => $DB->count_records('qtype_questionpy_package'), diff --git a/classes/package/package.php b/classes/package/package.php index 1687c830..a22866bc 100644 --- a/classes/package/package.php +++ b/classes/package/package.php @@ -75,17 +75,6 @@ public function __construct(int $id, string $shortname, string $namespace, array ); } - /** - * Retrieves each version of the package. - * - * @return array - * @throws moodle_exception - */ - public function get_version_array(): array { - global $DB; - return $DB->get_records('qtype_questionpy_pkgversion', ['packageid' => $this->id]); - } - /** * Returns the package of the given package version id. * @@ -101,22 +90,57 @@ public static function get_by_version(int $pkgversionid): package { } /** - * Get packages from the db matching given conditions. Note: only conditions stored in the package version table - * are applicable. + * Deletes a package and every version from the database with the given id. * - * @param array|null $conditions - * @return package[] + * @param int ...$ids * @throws moodle_exception */ - public static function get_records(?array $conditions = null): array { + public static function delete_by_id(int ...$ids): void { global $DB; - $packages = []; - $records = $DB->get_records('qtype_questionpy_package', $conditions); - foreach ($records as $record) { - $package = self::get_package_data($record->id); - $packages[] = array_converter::from_array(self::class, (array) $package); + + if (empty($ids)) { + return; + } + + $transaction = $DB->start_delegated_transaction(); + foreach ($ids as $id) { + $DB->delete_records('qtype_questionpy_pkgversion', ['packageid' => $id]); + $DB->delete_records('qtype_questionpy_language', ['packageid' => $id]); + $DB->delete_records('qtype_questionpy_pkgtag', ['packageid' => $id]); + $DB->delete_records('qtype_questionpy_package', ['id' => $id]); + last_used_service::remove_by_package($id); } - return $packages; + + $DB->execute(" + DELETE + FROM {qtype_questionpy_tag} + WHERE id NOT IN ( + SELECT tagid + FROM {qtype_questionpy_pkgtag} + ) + "); + $transaction->allow_commit(); + } + + /** + * Deletes every package and every version from the database. + * + * @throws moodle_exception + */ + public static function delete_all(): void { + global $DB; + $ids = $DB->get_fieldset('qtype_questionpy_package', 'id'); + self::delete_by_id(...$ids); + } + + /** + * Deletes the package and every version from the database. + * + * @return void + * @throws moodle_exception + */ + public function delete(): void { + self::delete_by_id($this->id); } /** @@ -178,31 +202,6 @@ private static function get_tag_data(int $packageid): array { ", ['packageid' => $packageid]); } - /** - * Deletes the package and every version from the database. - * - * @return void - * @throws moodle_exception - */ - public function delete(): void { - global $DB; - $transaction = $DB->start_delegated_transaction(); - $DB->delete_records('qtype_questionpy_pkgversion', ['packageid' => $this->id]); - $DB->delete_records('qtype_questionpy_language', ['packageid' => $this->id]); - $DB->delete_records('qtype_questionpy_pkgtag', ['packageid' => $this->id]); - $DB->execute(" - DELETE - FROM {qtype_questionpy_tag} - WHERE id NOT IN ( - SELECT tagid - FROM {qtype_questionpy_pkgtag} - ) - "); - $DB->delete_records('qtype_questionpy_package', ['id' => $this->id]); - last_used_service::remove_by_package($this->id); - $transaction->allow_commit(); - } - /** * Provides the differences between two packages, i.e. an array with all the parameters which are different in the * two objects. diff --git a/classes/package/package_info.php b/classes/package/package_info.php index 03adbe64..08cc42b7 100644 --- a/classes/package/package_info.php +++ b/classes/package/package_info.php @@ -23,7 +23,7 @@ use qtype_questionpy\array_converter\converter_config; /** - * Represents an overview of available QuestionPy packages on the application server. + * Represents an available QuestionPy package on the application server. * * @package qtype_questionpy * @copyright 2024 Jan Britz, TU Berlin, innoCampus - www.questionpy.org @@ -31,12 +31,12 @@ */ class package_info extends package_base { /** - * Checks whether this package exists or not. + * Gets the row id if this package is already stored in the DB, or false otherwise. * - * @return bool|int either false or the DB id + * @return false|int either false or the DB id * @throws moodle_exception */ - public function exists(): bool|int { + public function get_id(): false|int { global $DB; return $DB->get_field('qtype_questionpy_package', 'id', [ 'shortname' => $this->shortname, diff --git a/classes/package/package_versions_info.php b/classes/package/package_versions_info.php index efa140f5..fb261e4c 100644 --- a/classes/package/package_versions_info.php +++ b/classes/package/package_versions_info.php @@ -55,10 +55,13 @@ public function upsert(?int $timestamp = null): array { $timestamp ??= time(); - $packageid = $this->manifest->exists(); + $packageid = $this->manifest->get_id(); if ($packageid === false) { $packageid = $this->manifest->insert($timestamp); - $versionids = $this->insert($packageid, $timestamp); + $versionids = []; + foreach ($this->versions as $index => $version) { + $versionids[] = $this->insert($packageid, $version, order: $index, timestamp: $timestamp); + } } else { // Get the latest package version stored in the DB and check if we need to update the package info. $latestexisting = $DB->get_field('qtype_questionpy_pkgversion', 'hash', ['packageid' => $packageid, @@ -73,23 +76,21 @@ public function upsert(?int $timestamp = null): array { } /** - * Inserts the package versions of a new package. + * Inserts a package version in the database. * * @param int $packageid + * @param package_version_specific_info $version + * @param int $order * @param int $timestamp - * @return array + * @return int * @throws moodle_exception */ - private function insert(int $packageid, int $timestamp): array { + private function insert(int $packageid, package_version_specific_info $version, int $order, int $timestamp): int { global $DB; - $versionids = []; - foreach ($this->versions as $index => $version) { - $versionids[] = $DB->insert_record('qtype_questionpy_pkgversion', ['packageid' => $packageid, - 'version' => $version->version, 'hash' => $version->hash, 'versionorder' => $index, 'timemodified' => $timestamp, - 'timecreated' => $timestamp]); - } - return $versionids; + return $DB->insert_record('qtype_questionpy_pkgversion', ['packageid' => $packageid, + 'version' => $version->version, 'hash' => $version->hash, 'versionorder' => $order, 'timemodified' => $timestamp, + 'timecreated' => $timestamp], bulk: true); } /** @@ -126,9 +127,7 @@ private function update(int $packageid, int $timestamp): array { $new = array_flip(array_diff($incoming, $existing)); foreach ($this->versions as $index => $version) { if (isset($new[$version->hash])) { - $versionids[] = $DB->insert_record('qtype_questionpy_pkgversion', ['packageid' => $packageid, - 'version' => $version->version, 'hash' => $version->hash, 'versionorder' => $index, - 'timemodified' => $timestamp, 'timecreated' => $timestamp]); + $versionids[] = $this->insert($packageid, $version, order: $index, timestamp: $timestamp); } else { // The get_records(...) returns an array indexed by the first field which we set to be the hash of the version. $versionid = $existingrecords[$version->hash]->id;