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.

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

Avoided loading submissions when fetching all submissions #10

Merged
merged 5 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 13 additions & 7 deletions .github/workflows/pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down Expand Up @@ -54,7 +55,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": {
Expand Down
51 changes: 19 additions & 32 deletions src/Plugin/rest/resource/WebformAllFormSubmissions.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -39,18 +37,18 @@ class WebformAllFormSubmissions extends ResourceBase {
private $currentRequest;

/**
* The entity type manager object.
* The webform helper.
*
* @var \Drupal\Core\Entity\EntityTypeManager
* @var \Drupal\os2forms_rest_api\WebformHelper
*/
private $entityTypeManager;
private $webformHelper;

/**
* The webform helper.
* The database connection.
*
* @var \Drupal\os2forms_rest_api\WebformHelper
* @var \Drupal\Core\Database\Connection
*/
private $webformHelper;
private $database;

/**
* {@inheritdoc}
Expand All @@ -60,9 +58,9 @@ 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');

return $instance;
}
Expand Down Expand Up @@ -112,32 +110,20 @@ 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;

// 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';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a comment about why we use SQL and not the entity manager here. Perfomance...


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) {
Expand All @@ -152,9 +138,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 {
Expand All @@ -163,12 +150,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) {
Expand Down
2 changes: 1 addition & 1 deletion src/WebformHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading