From b9ade9345415b963af6a2e153db17ff3af029f5a Mon Sep 17 00:00:00 2001 From: Ankit Pathak Date: Sun, 21 Jul 2024 16:16:27 +0530 Subject: [PATCH] Bumping to D10.2 and fixing deprecation removed from D11. --- .github/workflows/testing.yml | 22 ++--- .../graphql_composable.info.yml | 2 +- .../graphql_example/graphql_examples.info.yml | 2 +- graphql.info.yml | 2 +- graphql.services.yml | 1 + src/GraphQL/Response/Response.php | 2 +- src/GraphQL/Response/ResponseInterface.php | 6 +- src/GraphQL/Utility/FileUpload.php | 96 ++++++++++--------- tests/src/Kernel/AlterableSchemaTest.php | 3 +- .../Framework/UploadFileServiceTest.php | 11 ++- tests/src/Kernel/GraphQLTestBase.php | 2 + 11 files changed, 77 insertions(+), 72 deletions(-) diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml index 7c74231a0..b229996d7 100644 --- a/.github/workflows/testing.yml +++ b/.github/workflows/testing.yml @@ -13,22 +13,18 @@ jobs: strategy: fail-fast: false matrix: - php-versions: ['8.1', '8.2'] - drupal-core: ['10.3.x'] - phpstan: ['0'] + # # php-versions: ['8.1', '8.2'] + # # drupal-core: ['10.3.x'] + # phpstan: ['0'] include: - # Extra run to test older supported Drupal 10.1.x. - - php-versions: '8.1' - drupal-core: '10.1.x' - phpstan: '0' # Extra run to test older supported Drupal 10.2.x. - - php-versions: '8.1' - drupal-core: '10.2.x' - phpstan: '0' + # - php-versions: '8.1' + # drupal-core: '10.2.x' + # phpstan: '0' # We only need to run PHPStan once on the latest PHP version. - - php-versions: '8.3' - drupal-core: '10.3.x' - phpstan: '1' + # - php-versions: '8.3' + # drupal-core: '10.3.x' + # phpstan: '1' - php-versions: '8.3' drupal-core: '11.0.x' phpstan: '1' diff --git a/examples/graphql_composable/graphql_composable.info.yml b/examples/graphql_composable/graphql_composable.info.yml index a891052cb..08add43f5 100644 --- a/examples/graphql_composable/graphql_composable.info.yml +++ b/examples/graphql_composable/graphql_composable.info.yml @@ -5,4 +5,4 @@ package: GraphQL dependencies: - graphql:graphql - node:node -core_version_requirement: ^10.1 || ^11 +core_version_requirement: ^10.2 || ^11 diff --git a/examples/graphql_example/graphql_examples.info.yml b/examples/graphql_example/graphql_examples.info.yml index 8587aa2e8..4f03a1abe 100644 --- a/examples/graphql_example/graphql_examples.info.yml +++ b/examples/graphql_example/graphql_examples.info.yml @@ -5,4 +5,4 @@ package: GraphQL dependencies: - graphql:graphql - node:node -core_version_requirement: ^10.1 || ^11 +core_version_requirement: ^10.2 || ^11 diff --git a/graphql.info.yml b/graphql.info.yml index 90852febe..8b43f0087 100644 --- a/graphql.info.yml +++ b/graphql.info.yml @@ -3,6 +3,6 @@ type: module description: 'Base module for integrating GraphQL with Drupal.' package: GraphQL configure: graphql.config_page -core_version_requirement: ^10.1 || ^11 +core_version_requirement: ^10.2 || ^11 dependencies: - typed_data:typed_data diff --git a/graphql.services.yml b/graphql.services.yml index ad0c2c9a7..23c01419f 100644 --- a/graphql.services.yml +++ b/graphql.services.yml @@ -184,6 +184,7 @@ services: - '@renderer' - '@event_dispatcher' - '@image.factory' + - '@file.validator' plugin.manager.graphql.persisted_query: class: Drupal\graphql\Plugin\PersistedQueryPluginManager diff --git a/src/GraphQL/Response/Response.php b/src/GraphQL/Response/Response.php index 20b2c4eff..4467445bc 100644 --- a/src/GraphQL/Response/Response.php +++ b/src/GraphQL/Response/Response.php @@ -27,7 +27,7 @@ public function addViolation($message, array $properties = []): void { /** * {@inheritdoc} */ - public function addViolations(array $messages, array $properties = []): void { + public function addViolations($messages, array $properties = []): void { foreach ($messages as $message) { $this->addViolation($message, $properties); } diff --git a/src/GraphQL/Response/ResponseInterface.php b/src/GraphQL/Response/ResponseInterface.php index f8f050ab0..f83eda7c7 100644 --- a/src/GraphQL/Response/ResponseInterface.php +++ b/src/GraphQL/Response/ResponseInterface.php @@ -12,7 +12,7 @@ interface ResponseInterface { /** * Adds the violation. * - * @param string|\Drupal\Core\StringTranslation\TranslatableMarkup $message + * @param string|\Drupal\Core\StringTranslation\TranslatableMarkup|\Symfony\Component\Validator\ConstraintViolationListInterface $message * Violation message. * @param array $properties * Other properties related to the violation. @@ -22,12 +22,12 @@ public function addViolation($message, array $properties = []): void; /** * Adds multiple violations. * - * @param string[]|\Drupal\Core\StringTranslation\TranslatableMarkup[] $messages + * @param string[]|\Drupal\Core\StringTranslation\TranslatableMarkup[]|\Symfony\Component\Validator\ConstraintViolationListInterface $messages * Violation messages. * @param array $properties * Other properties related to the violation. */ - public function addViolations(array $messages, array $properties = []): void; + public function addViolations(array|ConstraintViolationListInterface $messages, array $properties = []): void; /** * Gets the violations. diff --git a/src/GraphQL/Utility/FileUpload.php b/src/GraphQL/Utility/FileUpload.php index 7543ea5c7..26ddfba0e 100644 --- a/src/GraphQL/Utility/FileUpload.php +++ b/src/GraphQL/Utility/FileUpload.php @@ -17,9 +17,11 @@ use Drupal\Core\Render\RenderContext; use Drupal\Core\Render\RendererInterface; use Drupal\Core\Session\AccountProxyInterface; +use Drupal\Core\StringTranslation\ByteSizeMarkup; use Drupal\Core\StringTranslation\StringTranslationTrait; use Drupal\Core\Utility\Token; use Drupal\file\FileInterface; +use Drupal\file\Validation\FileValidatorInterface; use Drupal\graphql\GraphQL\Response\FileUploadResponse; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\File\UploadedFile; @@ -37,7 +39,7 @@ class FileUpload { /** * The file storage where we will create new file entities from. * - * @var \Drupal\file\FileStorageInterface + * @var \Drupal\Core\Entity\EntityStorageInterface */ protected $fileStorage; @@ -111,6 +113,13 @@ class FileUpload { */ protected $imageFactory; + /** + * The file validator service. + * + * @var \Drupal\file\Validation\FileValidatorInterface + */ + protected FileValidatorInterface $fileValidator; + /** * Constructor. */ @@ -126,10 +135,9 @@ public function __construct( RendererInterface $renderer, EventDispatcherInterface $eventDispatcher, ImageFactory $image_factory, + FileValidatorInterface $file_validator, ) { - /** @var \Drupal\file\FileStorageInterface $file_storage */ - $file_storage = $entityTypeManager->getStorage('file'); - $this->fileStorage = $file_storage; + $this->fileStorage = $entityTypeManager->getStorage('file'); $this->currentUser = $currentUser; $this->mimeTypeGuesser = $mimeTypeGuesser; $this->fileSystem = $fileSystem; @@ -140,6 +148,7 @@ public function __construct( $this->renderer = $renderer; $this->eventDispatcher = $eventDispatcher; $this->imageFactory = $image_factory; + $this->fileValidator = $file_validator; } /** @@ -193,10 +202,7 @@ public function saveFileUpload(UploadedFile $uploaded_file, array $settings): Fi switch ($uploaded_file->getError()) { case UPLOAD_ERR_INI_SIZE: case UPLOAD_ERR_FORM_SIZE: - // @todo Drupal 10.1 compatibility, needs to be converted to - // ByteSizeMarkup later. - // @phpstan-ignore-next-line - $maxUploadSize = format_size($this->getMaxUploadSize($settings)); + $maxUploadSize = ByteSizeMarkup::create($this->getMaxUploadSize($settings)); $response->addViolation($this->t('The file @file could not be saved because it exceeds @maxsize, the maximum allowed size for uploads.', [ '@file' => $uploaded_file->getClientOriginalName(), '@maxsize' => $maxUploadSize, @@ -248,8 +254,8 @@ public function saveFileUpload(UploadedFile $uploaded_file, array $settings): Fi $temp_file_path = $uploaded_file->getRealPath(); - // Drupal 10.2 compatibility: use the deprecated constant for now. - // @phpstan-ignore-next-line + // Drupal 10.3 compatibility: use the deprecated constant for now. + // @phpstan-ignore-next-line as it is deprecated in D12. $file_uri = $this->fileSystem->getDestinationFilename($file_uri, FileSystemInterface::EXISTS_RENAME); // Lock based on the prepared file URI. @@ -262,7 +268,7 @@ public function saveFileUpload(UploadedFile $uploaded_file, array $settings): Fi try { // Begin building file entity. - /** @var \Drupal\file\FileInterface $file */ + /** @var \Drupal\Core\Entity\EntityInterface $file */ $file = $this->fileStorage->create([]); $file->setOwnerId($this->currentUser->id()); $file->setFilename($prepared_filename); @@ -272,29 +278,31 @@ public function saveFileUpload(UploadedFile $uploaded_file, array $settings): Fi // before it is saved. $file->setSize(@filesize($temp_file_path)); - // Validate against file_validate() first with the temporary path. - // @todo Drupal 10.1 compatibility, needs to be converted to file validate - // service later. - // @phpstan-ignore-next-line - $errors = file_validate($file, $validators); - $maxResolution = $settings['max_resolution'] ?? 0; - $minResolution = $settings['min_resolution'] ?? 0; - if (!empty($maxResolution) || !empty($minResolution)) { - $errors += $this->validateFileImageResolution($file, $maxResolution, $minResolution); - } - + // Validate against fileValidator first with the temporary path. + /** @var \Symfony\Component\Validator\ConstraintViolationListInterface $errors */ + $errors = $this->fileValidator->validate($file, $validators); if (!empty($errors)) { $response->addViolations($errors); return $response; } + // Validate Image resolution. + $maxResolution = $settings['max_resolution'] ?? 0; + $minResolution = $settings['min_resolution'] ?? 0; + if (!empty($maxResolution) || !empty($minResolution)) { + $image_resolution_errors = $this->validateFileImageResolution($file, $maxResolution, $minResolution); + if (!empty($image_resolution_errors)) { + $response->addViolations($image_resolution_errors); + return $response; + } + } $file->setFileUri($file_uri); // Move the file to the correct location after validation. Use // FileSystemInterface::EXISTS_ERROR as the file location has already been // determined above in FileSystem::getDestinationFilename(). try { - // Drupal 10.2 compatibility: use the deprecated constant for now. - // @phpstan-ignore-next-line + // Drupal 10.3 compatibility: use the deprecated constant for now. + // @phpstan-ignore-next-line as it is deprecated in D12. $this->fileSystem->move($temp_file_path, $file_uri, FileSystemInterface::EXISTS_ERROR); } catch (FileException $e) { @@ -487,12 +495,12 @@ protected function validateFileImageResolution(FileInterface $file, $maximum_dim protected function prepareFilename(string $filename, array &$validators): string { // Don't rename if 'allow_insecure_uploads' evaluates to TRUE. if (!$this->systemFileConfig->get('allow_insecure_uploads')) { - if (!empty($validators['file_validate_extensions'][0])) { - // If there is a file_validate_extensions validator and a list of - // valid extensions, munge the filename to protect against possible - // malicious extension hiding within an unknown file type. For example, - // "filename.html.foo". - $event = new FileUploadSanitizeNameEvent($filename, $validators['file_validate_extensions'][0]); + if (!empty($validators['FileExtension']['extensions'])) { + // If there is a fileValidator service to validate FileExtension and + // a list of valid extensions, munge the filename to protect against + // possible malicious extension hiding within an unknown file type. + // For example, "filename.html.foo". + $event = new FileUploadSanitizeNameEvent($filename, $validators['FileExtension']['extensions']); $this->eventDispatcher->dispatch($event); $filename = $event->getFilename(); } @@ -502,33 +510,31 @@ protected function prepareFilename(string $filename, array &$validators): string // and filename._php.txt, respectively). if (preg_match(FileSystemInterface::INSECURE_EXTENSION_REGEX, $filename)) { // If the file will be rejected anyway due to a disallowed extension, it - // should not be renamed; rather, we'll let file_validate_extensions() - // reject it below. + // should not be renamed; rather, we'll let fileValidator service + // to validate FileExtension reject it below. + $passes_validation = FALSE; - if (!empty($validators['file_validate_extensions'][0])) { - /** @var \Drupal\file\FileInterface $file */ + if (!empty($validators['FileExtension']['extensions'])) { + /** @var \Drupal\Core\Entity\EntityInterface $file */ $file = $this->fileStorage->create([]); $file->setFilename($filename); - // @todo Drupal 10.1 compatibility, needs to be converted to file - // validator service later. - // @phpstan-ignore-next-line - $passes_validation = empty(file_validate_extensions($file, $validators['file_validate_extensions'][0])); + $passes_validation = empty($this->fileValidator->validate($file, $validators['FileExtension']['extensions'])); } - if (empty($validators['file_validate_extensions'][0]) || $passes_validation) { + if (empty($validators['FileExtension']['extensions']) || $passes_validation) { if ((substr($filename, -4) != '.txt')) { // The destination filename will also later be used to create the // URI. $filename .= '.txt'; } - $event = new FileUploadSanitizeNameEvent($filename, $validators['file_validate_extensions'][0] ?? ''); + $event = new FileUploadSanitizeNameEvent($filename, $validators['FileExtension']['extensions'] ?? ''); $this->eventDispatcher->dispatch($event); $filename = $event->getFilename(); // The .txt extension may not be in the allowed list of extensions. We // have to add it here or else the file upload will fail. - if (!empty($validators['file_validate_extensions'][0])) { - $validators['file_validate_extensions'][0] .= ' txt'; + if (!empty($validators['FileExtension']['extensions'])) { + $validators['FileExtension']['extensions'] .= ' txt'; } } } @@ -579,7 +585,7 @@ protected function getUploadLocation(array $settings): string { protected function getUploadValidators(array $settings): array { $validators = [ // Add in our check of the file name length. - 'file_validate_name_length' => [], + 'FileNameLength' => [], ]; // Cap the upload size according to the PHP limit. @@ -589,11 +595,11 @@ protected function getUploadValidators(array $settings): array { } // There is always a file size limit due to the PHP server limit. - $validators['file_validate_size'] = [$max_filesize]; + $validators['FileSizeLimit'] = ['fileLimit' => $max_filesize]; // Add the extension check if necessary. if (!empty($settings['file_extensions'])) { - $validators['file_validate_extensions'] = [$settings['file_extensions']]; + $validators['FileExtension'] = ['extensions' => $settings['file_extensions']]; } return $validators; diff --git a/tests/src/Kernel/AlterableSchemaTest.php b/tests/src/Kernel/AlterableSchemaTest.php index f9f97d6c8..7e1f69966 100644 --- a/tests/src/Kernel/AlterableSchemaTest.php +++ b/tests/src/Kernel/AlterableSchemaTest.php @@ -156,9 +156,8 @@ protected function mockSchema($id, $schema, array $extensions = []): void { $extensions['graphql_alterable_schema_test']->expects(static::any()) ->method('getBaseDefinition') ->willReturn(''); - // Different extension definition for different tests. - switch ($this->getName()) { + switch ($this->toString()) { case 'testEmptySchemaExtensionAlteredQueryResultPropertyAdded': $extensionDefinition = ''; break; diff --git a/tests/src/Kernel/Framework/UploadFileServiceTest.php b/tests/src/Kernel/Framework/UploadFileServiceTest.php index 79ddf8c2c..c0307bd6b 100644 --- a/tests/src/Kernel/Framework/UploadFileServiceTest.php +++ b/tests/src/Kernel/Framework/UploadFileServiceTest.php @@ -104,7 +104,7 @@ public function testPartialFile(): void { ]); $violations = $file_upload_response->getViolations(); - $this->assertStringMatchesFormat( + $this->assertStringContainsString( 'The file "test.txt" could not be saved because the upload did not complete.', $violations[0]['message'] ); @@ -140,7 +140,7 @@ public function testSizeValidation(): void { $violations = $file_upload_response->getViolations(); // @todo Do we want HTML tags in our violations or not? - $this->assertStringMatchesFormat( + $this->assertStringContainsString( 'The file is 4 bytes exceeding the maximum file size of 1 byte.', $violations[0]['message'] ); @@ -190,7 +190,7 @@ public function testDimensionTooSmallValidation(): void { ]); $violations = $file_upload_response->getViolations(); - $this->assertStringMatchesFormat( + $this->assertStringContainsString( 'The image is too small. The minimum dimensions are 15x15 pixels and the image size is 10x10 pixels.', $violations[0]['message'] ); @@ -228,7 +228,7 @@ public function testExtensionValidation(): void { $violations = $file_upload_response->getViolations(); // @todo Do we want HTML tags in our violations or not? - $this->assertStringMatchesFormat( + $this->assertStringContainsString( 'Only files with the following extensions are allowed: odt.', $violations[0]['message'] ); @@ -256,6 +256,7 @@ public function testLockReleased(): void { \Drupal::service('renderer'), \Drupal::service('event_dispatcher'), \Drupal::service('image.factory'), + \Drupal::service('file.validator'), ); // Create a Symfony dummy uploaded file in test mode. @@ -319,7 +320,7 @@ public function testUnsuccessWithMultipleFileUploads(): void { // There must be violation regarding forbidden file extension. $violations = $file_upload_response->getViolations(); - $this->assertStringMatchesFormat( + $this->assertStringContainsString( 'Only files with the following extensions are allowed: txt.', $violations[0]['message'] ); diff --git a/tests/src/Kernel/GraphQLTestBase.php b/tests/src/Kernel/GraphQLTestBase.php index 20425566f..e529215c5 100644 --- a/tests/src/Kernel/GraphQLTestBase.php +++ b/tests/src/Kernel/GraphQLTestBase.php @@ -50,6 +50,7 @@ abstract class GraphQLTestBase extends KernelTestBase { 'content_translation', 'entity_reference_test', 'field', + 'file', 'menu_link_content', 'link', 'typed_data', @@ -75,6 +76,7 @@ protected function setUp(): void { $this->installEntitySchema('graphql_server'); $this->installEntitySchema('configurable_language'); $this->installConfig(['language']); + $this->installEntitySchema('file'); $this->installEntitySchema('menu_link_content'); $this->setUpCurrentUser([], $this->userPermissions());