From 9651bfd08e434df7aa9ae1f7d5cf89124fa05b5d Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 17 Dec 2024 15:26:27 +0100 Subject: [PATCH 1/5] Avoided loading submissions when fetching all submissions --- CHANGELOG.md | 7 +++- .../resource/WebformAllFormSubmissions.php | 40 ++++++++----------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69dab2e..f3b5636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ about writing changes to this log. ## [Unreleased] +## [2.2.0] + +- Avoided loading submissions through `webform_submission` storage. + ## [2.1.0] - Drupal 10 compatibility. @@ -40,7 +44,8 @@ about writing changes to this log. - Release 1.0.0 -[Unreleased]: https://github.com/OS2Forms/os2forms_rest_api/compare/2.1.0...HEAD +[Unreleased]: https://github.com/OS2Forms/os2forms_rest_api/compare/2.2.0...HEAD +[2.2.0]: https://github.com/OS2Forms/os2forms_rest_api/compare/2.1.0...2.2.0 [2.1.0]: https://github.com/OS2Forms/os2forms_rest_api/compare/2.0.3...2.1.0 [2.0.3]: https://github.com/OS2Forms/os2forms_rest_api/compare/2.0.2...2.0.3 [2.0.2]: https://github.com/OS2Forms/os2forms_rest_api/compare/2.0.1...2.0.2 diff --git a/src/Plugin/rest/resource/WebformAllFormSubmissions.php b/src/Plugin/rest/resource/WebformAllFormSubmissions.php index e1c8d39..9dd41b6 100644 --- a/src/Plugin/rest/resource/WebformAllFormSubmissions.php +++ b/src/Plugin/rest/resource/WebformAllFormSubmissions.php @@ -52,6 +52,13 @@ class WebformAllFormSubmissions extends ResourceBase { */ private $webformHelper; + /** + * The database connection. + * + * @var \Drupal\Core\Database\Connection + */ + private $database; + /** * {@inheritdoc} * @@ -63,6 +70,7 @@ public static function create(ContainerInterface $container, array $configuratio $instance->entityTypeManager = $container->get('entity_type.manager'); $instance->currentRequest = $container->get('request_stack')->getCurrentRequest(); $instance->webformHelper = $container->get(WebformHelper::class); + $instance->database = $container->get('database'); return $instance; } @@ -112,32 +120,17 @@ public function get(string $webform_id): ModifiedResourceResponse { $result = ['webform_id' => $webform_id]; - try { - $submissionEntityStorage = $this->entityTypeManager->getStorage('webform_submission'); - } - catch (InvalidPluginDefinitionException | PluginNotFoundException $e) { - $errors = [ - 'error' => [ - 'message' => $this->t('Could not load webform submission storage'), - ], - ]; - - return new ModifiedResourceResponse($errors, Response::HTTP_INTERNAL_SERVER_ERROR); - } - - // Query for webform submissions with this webform_id. - $submissionQuery = $submissionEntityStorage->getQuery() - ->condition('webform_id', $webform_id); - $requestQuery = $this->currentRequest->query; + $query = 'SELECT sid, uuid FROM webform_submission WHERE webform_id = :webform_id'; + foreach (self::ALLOWED_DATETIME_QUERY_PARAMS as $param => $operator) { $value = $requestQuery->get($param); if (!empty($value)) { try { $dateTime = new \DateTimeImmutable($value); - $submissionQuery->condition('created', $dateTime->getTimestamp(), $operator); + $query .= sprintf(' AND created %s %s', $operator, $dateTime->getTimestamp()); $result[$param] = $value; } catch (\Exception $e) { @@ -152,9 +145,10 @@ public function get(string $webform_id): ModifiedResourceResponse { } } - // Complete query. - $submissionQuery->accessCheck(FALSE); - $sids = $submissionQuery->execute(); + $submissions = $this->database->query( + $query, + [':webform_id' => $webform_id] + )->fetchAllKeyed(); // Generate submission URLs. try { @@ -163,12 +157,12 @@ public function get(string $webform_id): ModifiedResourceResponse { 'rest.webform_rest_submission.GET', [ 'webform_id' => $webform_id, - 'uuid' => $submission->uuid(), + 'uuid' => $submission, ] ) ->setAbsolute() ->toString(TRUE)->getGeneratedUrl(), - $submissionEntityStorage->loadMultiple($sids ?: []) + $submissions ?: [] ); } catch (\Exception $e) { From ddf1c6f5394eb6108ebf2395b71fd788f6350c21 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 17 Dec 2024 15:33:56 +0100 Subject: [PATCH 2/5] Cleanup actions and drupal lenient --- .github/workflows/pr.yaml | 20 +++++++++++++------- composer.json | 3 ++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index f563fbb..2ded5ab 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -23,7 +23,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - php-versions: [ '8.1' ] + php-versions: [ '8.3' ] dependency-version: [ prefer-lowest, prefer-stable ] steps: - uses: actions/checkout@master @@ -55,7 +55,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - php-versions: [ '8.1' ] + php-versions: [ '8.3' ] dependency-version: [ prefer-lowest, prefer-stable ] steps: - uses: actions/checkout@master @@ -88,7 +88,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - php-versions: [ '8.1' ] + php-versions: [ '8.3' ] dependency-version: [ prefer-lowest, prefer-stable ] steps: - uses: actions/checkout@master @@ -113,18 +113,24 @@ jobs: run: | # We need a Drupal project to run drupal-check (cf. https://github.com/mglaman/drupal-check#usage) # Install Drupal - composer --no-interaction create-project drupal/recommended-project:^9 --stability=dev drupal + composer --no-interaction create-project drupal/recommended-project:^10 --stability=dev drupal # Copy our module source code into the Drupal module folder. mkdir -p drupal/web/modules/contrib/os2forms_rest_api cp -r os2forms_rest_api.* composer.json src drupal/web/modules/contrib/os2forms_rest_api + + # Allow all plugins + composer --working-dir=drupal config --no-plugins allow-plugins true + # Add our module as a composer repository. composer --no-interaction --working-dir=drupal config repositories.os2forms/os2forms_rest_api path web/modules/contrib/os2forms_rest_api + # Restore Drupal composer repository. composer --no-interaction --working-dir=drupal config repositories.drupal composer https://packages.drupal.org/8 - composer --no-interaction --working-dir=drupal config --no-plugins allow-plugins.cweagans/composer-patches true - composer --no-interaction --working-dir=drupal config --no-plugins allow-plugins.zaporylie/composer-drupal-optimizations true - composer --no-interaction --working-dir=drupal config --no-plugins allow-plugins.simplesamlphp/composer-module-installer true + # Make Drupal 10 compatible + composer --working-dir=drupal --no-interaction require psr/http-message:^1.0 + composer --working-dir=drupal --no-interaction require 'mglaman/composer-drupal-lenient' + composer --working-dir=drupal config --no-plugins --merge --json extra.drupal-lenient.allowed-list '["drupal/coc_forms_auto_export", "drupal/webform_node_element"]' # @see https://getcomposer.org/doc/03-cli.md#modifying-extra-values composer --no-interaction --working-dir=drupal config --no-plugins --json extra.enable-patching true diff --git a/composer.json b/composer.json index c2b849d..57f7de9 100644 --- a/composer.json +++ b/composer.json @@ -54,7 +54,8 @@ "dealerdirect/phpcodesniffer-composer-installer": true, "cweagans/composer-patches": true, "zaporylie/composer-drupal-optimizations": true, - "simplesamlphp/composer-module-installer": true + "simplesamlphp/composer-module-installer": true, + "mglaman/composer-drupal-lenient": true } }, "extra": { From 5e806f217f7f359b76bbcfd32c4f04f3b5724435 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Tue, 17 Dec 2024 15:41:54 +0100 Subject: [PATCH 3/5] Cleanup --- composer.json | 3 ++- src/Plugin/rest/resource/WebformAllFormSubmissions.php | 2 -- src/WebformHelper.php | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 57f7de9..54a6c51 100644 --- a/composer.json +++ b/composer.json @@ -26,7 +26,8 @@ "require-dev": { "dealerdirect/phpcodesniffer-composer-installer": "^0.7.2", "drupal/coder": "^8.3", - "mglaman/drupal-check": "^1.4" + "mglaman/drupal-check": "^1.4", + "mglaman/phpstan-drupal": "~1.2.0" }, "scripts": { "code-analysis/drupal-check": [ diff --git a/src/Plugin/rest/resource/WebformAllFormSubmissions.php b/src/Plugin/rest/resource/WebformAllFormSubmissions.php index 9dd41b6..3a76f78 100644 --- a/src/Plugin/rest/resource/WebformAllFormSubmissions.php +++ b/src/Plugin/rest/resource/WebformAllFormSubmissions.php @@ -2,8 +2,6 @@ namespace Drupal\os2forms_rest_api\Plugin\rest\resource; -use Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException; -use Drupal\Component\Plugin\Exception\PluginNotFoundException; use Drupal\Core\Url; use Drupal\os2forms_rest_api\WebformHelper; use Drupal\rest\ModifiedResourceResponse; diff --git a/src/WebformHelper.php b/src/WebformHelper.php index d118160..b99305f 100644 --- a/src/WebformHelper.php +++ b/src/WebformHelper.php @@ -179,7 +179,7 @@ public function webformThirdPartySettingsFormAlter(array &$form, FormStateInterf * @return \Drupal\webform\WebformInterface|null * The webform if found. */ - public function getWebform(string $webformId, string $submissionUuid = NULL): ?WebformInterface { + public function getWebform(string $webformId, ?string $submissionUuid = NULL): ?WebformInterface { if (NULL !== $submissionUuid) { $storage = $this->entityTypeManager->getStorage('webform_submission'); $submissionIds = $storage From 461174edee721043e4dbb2a7fc11a08e7fed9dae Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Wed, 18 Dec 2024 08:22:29 +0100 Subject: [PATCH 4/5] Removed unused code --- src/Plugin/rest/resource/WebformAllFormSubmissions.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Plugin/rest/resource/WebformAllFormSubmissions.php b/src/Plugin/rest/resource/WebformAllFormSubmissions.php index 3a76f78..19a545b 100644 --- a/src/Plugin/rest/resource/WebformAllFormSubmissions.php +++ b/src/Plugin/rest/resource/WebformAllFormSubmissions.php @@ -36,13 +36,6 @@ class WebformAllFormSubmissions extends ResourceBase { */ private $currentRequest; - /** - * The entity type manager object. - * - * @var \Drupal\Core\Entity\EntityTypeManager - */ - private $entityTypeManager; - /** * The webform helper. * @@ -65,7 +58,6 @@ class WebformAllFormSubmissions extends ResourceBase { public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { $instance = parent::create($container, $configuration, $plugin_id, $plugin_definition); - $instance->entityTypeManager = $container->get('entity_type.manager'); $instance->currentRequest = $container->get('request_stack')->getCurrentRequest(); $instance->webformHelper = $container->get(WebformHelper::class); $instance->database = $container->get('database'); From 7a5505c2eac321254e96d188bb0e2071c13976d1 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Wed, 18 Dec 2024 09:39:01 +0100 Subject: [PATCH 5/5] Added comment --- src/Plugin/rest/resource/WebformAllFormSubmissions.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Plugin/rest/resource/WebformAllFormSubmissions.php b/src/Plugin/rest/resource/WebformAllFormSubmissions.php index 19a545b..31eb341 100644 --- a/src/Plugin/rest/resource/WebformAllFormSubmissions.php +++ b/src/Plugin/rest/resource/WebformAllFormSubmissions.php @@ -112,6 +112,9 @@ public function get(string $webform_id): ModifiedResourceResponse { $requestQuery = $this->currentRequest->query; + // We use raw SQL to fetch submission sids and uuids. + // This is to avoid degraded performance by having to load every + // single submission through the webform submissions storage. $query = 'SELECT sid, uuid FROM webform_submission WHERE webform_id = :webform_id'; foreach (self::ALLOWED_DATETIME_QUERY_PARAMS as $param => $operator) {