Skip to content

Commit 0b1eb53

Browse files
klausiklausi
authored
tests(files): Add tests for file upload service and refactor responses (drupal-graphql#1112)
1 parent 86d503f commit 0b1eb53

File tree

6 files changed

+327
-19
lines changed

6 files changed

+327
-19
lines changed

‎.github/workflows/testing.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ jobs:
6464
webonyx/graphql-php:^14.3 \
6565
drupal/typed_data:^1.0 \
6666
drupal/redirect:^1.6 \
67-
phpstan/phpstan:^0.12.50 \
67+
phpstan/phpstan:^0.12.56 \
6868
mglaman/phpstan-drupal:^0.12.3 \
6969
phpstan/phpstan-deprecation-rules:^0.12.2 \
7070
jangregor/phpstan-prophecy:^0.8 \

‎phpstan.neon

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ parameters:
2929
- "#deprecated class Symfony\\\\Component\\\\EventDispatcher\\\\Event#"
3030
- "#deprecated class Symfony\\\\Component\\\\HttpKernel\\\\Event\\\\GetResponseEvent#"
3131
- "#^Method Symfony\\\\Contracts\\\\EventDispatcher\\\\EventDispatcherInterface\\:\\:dispatch\\(\\) invoked with 2 parameters, 1 required\\.$#"
32+
- "#deprecated interface Symfony\\\\Component\\\\HttpFoundation\\\\File\\\\MimeType\\\\MimeTypeGuesserInterface#"
3233
# Drupal allows object property access to custom fields, so we cannot fix
3334
# that.
3435
- "#^Property Drupal\\\\.+ \\(Drupal\\\\Core\\\\Field\\\\FieldItemListInterface\\) does not accept .+\\.$#"

‎src/GraphQL/Response/FileUploadResponse.php

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
namespace Drupal\graphql\GraphQL\Response;
6+
7+
use Drupal\file\FileInterface;
8+
9+
/**
10+
* A response that either has a file entity or some violations.
11+
*/
12+
class FileUploadResponse extends Response {
13+
14+
/**
15+
* The file entity in case of successful file upload.
16+
*
17+
* @var \Drupal\file\FileInterface|null
18+
*/
19+
protected $fileEntity;
20+
21+
/**
22+
* Sets file entity.
23+
*
24+
* @param \Drupal\file\FileInterface $fileEntity
25+
* File entity.
26+
*/
27+
public function setFileEntity(FileInterface $fileEntity): void {
28+
$this->fileEntity = $fileEntity;
29+
}
30+
31+
/**
32+
* Get the file entity if there is one.
33+
*/
34+
public function getFileEntity(): ?FileInterface {
35+
return $this->fileEntity;
36+
}
37+
38+
}

‎src/GraphQL/Utility/FileUpload.php

+42-18
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010
use Drupal\Core\Logger\LoggerChannelInterface;
1111
use Drupal\Core\Session\AccountProxyInterface;
1212
use Drupal\Core\StringTranslation\StringTranslationTrait;
13-
use Drupal\graphql\Wrappers\FileUploadResponse;
13+
use Drupal\graphql\GraphQL\Response\FileUploadResponse;
1414
use Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesserInterface;
1515
use Symfony\Component\HttpFoundation\File\UploadedFile;
1616

1717
/**
1818
* Service to manage file uploads within GraphQL mutations.
19+
*
20+
* This service handles file validations like max upload size.
1921
*/
2022
class FileUpload {
2123

@@ -110,7 +112,10 @@ protected function getUploadValidators(array $settings) {
110112
$validators['file_validate_size'] = [$this->getMaxUploadSize($settings)];
111113

112114
// Add the extension check if necessary.
113-
if (!empty($settings['file_extensions'])) {
115+
if (empty($settings['file_extensions'])) {
116+
$validators['file_validate_extensions'] = ['txt'];
117+
}
118+
else {
114119
$validators['file_validate_extensions'] = [$settings['file_extensions']];
115120
}
116121

@@ -134,13 +139,14 @@ protected function getUploadValidators(array $settings) {
134139
* - file_extensions: List of valid file extensions (eg [xml, pdf])
135140
* - max_filesize: Maximum allowed size of uploaded file.
136141
*
137-
* @return \Drupal\graphql\Wrappers\FileUploadResponse
142+
* @return \Drupal\graphql\GraphQL\Response\FileUploadResponse
138143
* The file upload response containing file entity or list of violations.
139144
*
140145
* @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException
141146
* @throws \Drupal\Component\Plugin\Exception\PluginNotFoundException
147+
* @throws \RuntimeException
142148
*/
143-
public function createTemporaryFileUpload(UploadedFile $file, array $settings) {
149+
public function createTemporaryFileUpload(UploadedFile $file, array $settings): FileUploadResponse {
144150
$response = new FileUploadResponse();
145151

146152
// Check for file upload errors and return FALSE for this file if a lower
@@ -150,33 +156,43 @@ public function createTemporaryFileUpload(UploadedFile $file, array $settings) {
150156
case UPLOAD_ERR_INI_SIZE:
151157
case UPLOAD_ERR_FORM_SIZE:
152158
$maxUploadSize = format_size($this->getMaxUploadSize($settings));
153-
$response->setViolation($this->t('The file @file could not be saved because it exceeds @maxsize, the maximum allowed size for uploads.', ['@file' => $file->getClientOriginalName(), '@maxsize' => $maxUploadSize]));
154-
$this->logger->error('The file @file could not be saved because it exceeds @maxsize, the maximum allowed size for uploads.', ['@file' => $file->getFilename(), '@maxsize' => $maxUploadSize]);
159+
$response->addViolation($this->t('The file @file could not be saved because it exceeds @maxsize, the maximum allowed size for uploads.', [
160+
'@file' => $file->getClientOriginalName(),
161+
'@maxsize' => $maxUploadSize,
162+
]));
155163
return $response;
156164

157165
case UPLOAD_ERR_PARTIAL:
158166
case UPLOAD_ERR_NO_FILE:
159-
$response->setViolation($this->t('The file "@file" could not be saved because the upload did not complete.', ['@file' => $file->getClientOriginalName()]));
160-
$this->logger->error('The file "@file" could not be saved because the upload did not complete.', ['@file' => $file->getFilename()]);
167+
$response->addViolation($this->t('The file "@file" could not be saved because the upload did not complete.', [
168+
'@file' => $file->getClientOriginalName(),
169+
]));
161170
return $response;
162171

163172
case UPLOAD_ERR_OK:
164173
// Final check that this is a valid upload, if it isn't, use the
165174
// default error handler.
166-
if (is_uploaded_file($file->getRealPath())) {
175+
if ($file->isValid()) {
167176
break;
168177
}
169178

170179
default:
171-
$response->setViolation($this->t('Unknown error while uploading the file "@file".', ['@file' => $file->getClientOriginalName()]));
172-
$this->logger->error('Error while uploading the file "@file" with an error code "@code".', ['@file' => $file->getFilename(), '@code' => $file->getError()]);
180+
$response->addViolation($this->t('Unknown error while uploading the file "@file".', ['@file' => $file->getClientOriginalName()]));
181+
$this->logger->error('Error while uploading the file "@file" with an error code "@code".', [
182+
'@file' => $file->getFilename(),
183+
'@code' => $file->getError(),
184+
]);
173185
return $response;
174186
}
175187

188+
if (empty($settings['uri_scheme']) || empty($settings['file_directory'])) {
189+
throw new \RuntimeException('uri_scheme or file_directory missing in settings');
190+
}
191+
176192
// Make sure the destination directory exists.
177193
$uploadDir = $settings['uri_scheme'] . '://' . trim($settings['file_directory'], '/');
178194
if (!$this->fileSystem->prepareDirectory($uploadDir, FileSystem::CREATE_DIRECTORY)) {
179-
$response->setViolation($this->t('Unknown error while uploading the file "@file".', ['@file' => $file->getClientOriginalName()]));
195+
$response->addViolation($this->t('Unknown error while uploading the file "@file".', ['@file' => $file->getClientOriginalName()]));
180196
$this->logger->error('Could not create directory "@upload_directory".', ["@upload_directory" => $uploadDir]);
181197
return $response;
182198
}
@@ -198,29 +214,37 @@ public function createTemporaryFileUpload(UploadedFile $file, array $settings) {
198214
$fileEntity = $storage->create($values);
199215

200216
// Validate the entity values.
201-
if (($violations = $fileEntity->validate()) && $violations->count()) {
202-
$response->setViolations($violations);
217+
$violations = $fileEntity->validate();
218+
if ($violations->count()) {
219+
foreach ($violations as $violation) {
220+
$response->addViolation($violation->getMessage());
221+
}
203222
return $response;
204223
}
205224

206225
// Validate the file name length.
207226
if ($violations = file_validate($fileEntity, $this->getUploadValidators($settings))) {
208-
$response->setViolations($violations);
227+
$response->addViolations($violations);
209228
return $response;
210229
}
211230

212231
// Move uploaded files from PHP's upload_tmp_dir to Drupal's temporary
213232
// directory. This overcomes open_basedir restrictions for future file
214233
// operations.
215234
if (!$this->fileSystem->moveUploadedFile($file->getRealPath(), $fileEntity->getFileUri())) {
216-
$response->setViolation($this->t('Unknown error while uploading the file "@file".', ['@file' => $file->getClientOriginalName()]));
217-
$this->logger->error('Unable to move file from "@file" to "@destination".', ['@file' => $file->getRealPath(), '@destination' => $fileEntity->getFileUri()]);
235+
$response->addViolation($this->t('Unknown error while uploading the file "@file".', [
236+
'@file' => $file->getClientOriginalName(),
237+
]));
238+
$this->logger->error('Unable to move file from "@file" to "@destination".', [
239+
'@file' => $file->getRealPath(),
240+
'@destination' => $fileEntity->getFileUri(),
241+
]);
218242
return $response;
219243
}
220244

221245
// Adjust permissions.
222246
if (!$this->fileSystem->chmod($fileEntity->getFileUri())) {
223-
$response->setViolation($this->t('Unknown error while uploading the file "@file".', ['@file' => $file->getClientOriginalName()]));
247+
$response->addViolation($this->t('Unknown error while uploading the file "@file".', ['@file' => $file->getClientOriginalName()]));
224248
$this->logger->error('Unable to set file permission for file "@file".', ['@file' => $fileEntity->getFileUri()]);
225249
return $response;
226250
}

‎tests/src/Kernel/Framework/MockFileSystem.php

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
3+
namespace Drupal\Tests\graphql\Kernel\Framework;
4+
5+
use Drupal\Core\File\FileSystem;
6+
7+
/**
8+
* Helper class to mock the moveUploadedFile() method during testing.
9+
*
10+
* @internal
11+
*/
12+
class MockFileSystem extends FileSystem {
13+
14+
/**
15+
* The file system service used to proxy to.
16+
*
17+
* @var \Drupal\Core\File\FileSystem
18+
*/
19+
private $fileSystem;
20+
21+
/**
22+
* Constructs this mock with the file system service used to proxy.
23+
*/
24+
public function __construct(FileSystem $file_system) {
25+
$this->fileSystem = $file_system;
26+
}
27+
28+
/**
29+
* {@inheritdoc}
30+
*/
31+
public function prepareDirectory(&$directory, $options = self::MODIFY_PERMISSIONS) {
32+
return $this->fileSystem->prepareDirectory($directory, $options);
33+
}
34+
35+
/**
36+
* {@inheritdoc}
37+
*/
38+
public function moveUploadedFile($filename, $uri) {
39+
// We can use the normal move() functionality instead during testing.
40+
$this->fileSystem->move($filename, $uri);
41+
return TRUE;
42+
}
43+
44+
/**
45+
* {@inheritdoc}
46+
*/
47+
public function chmod($uri, $mode = NULL) {
48+
return $this->fileSystem->chmod($uri, $mode);
49+
}
50+
51+
}

0 commit comments

Comments
 (0)