[Symfony6] The Drupal\Tests\media_library\Kernel\MediaLibraryStateTest fails for Symfony 5.4

Isn't the default NULL? If so can we just use ?: Instead of calling ::all?

+++ b/core/modules/media_library/src/MediaLibraryState.php
@@ -101,10 +101,10 @@ public static function fromRequest(Request $request) {
-      $query->get('media_library_opener_parameters', [])
+      $query->all()['media_library_opener_parameters'] ?? []

Sorry, I was asking is there a reason we can't write this as

$query->get('media_library_opener_parameters') ?: [];

And avoid calling ::all

But I get it now, its not about the 'null' case its about the 'value exists' case

+++ b/core/modules/media_library/src/MediaLibraryState.php
@@ -101,10 +101,10 @@ public static function fromRequest(Request $request) {
+      $query->all()['media_library_allowed_types'] ?? [],
...
+      $query->all()['media_library_opener_parameters'] ?? []

should we store the return from the first call to :all and avoid calling it twice?

In fact, could we re-write this using one call to :all and array_intersect_key and remove the ::get calls too?

Looks good to me

+++ b/core/modules/media_library/src/MediaLibraryState.php
@@ -96,15 +96,16 @@ public static function create($opener_id, array $allowed_media_type_ids, $select
-      $query->get('media_library_opener_id'),
...
-      $query->get('media_library_remaining'),
-      $query->get('media_library_opener_parameters', [])
+      $params['media_library_opener_id'] ?? NULL,
+      $params['media_library_allowed_types'] ?? [],
+      $params['media_library_selected_type'] ?? NULL,
+      $params['media_library_remaining'] ?? NULL,
+      $params['media_library_opener_parameters'] ?? []

The reason we use all() instead of all('key') or get('key') is when what we are retrieving really could either be a scalar or an array, and subsequent code is set up to handle both cases. Is it really the case that we don't know this for each of these values? Surely e.g. the media_library_opener_id can never be an array?

This was solved a cleaner way back in https://git.drupalcode.org/project/drupal/-/merge_requests/462/diffs#dif... - this was then reverted for other reasons but I think we can reintroduce the same fix for MediaLibraryState at least.

Alternative approach that uses all() and get() as Symfony intends, this provides extra safety against a malicious user sending an array instead of a string or vice versa.

The InputBag replacement in the test is done at runtime in bootstrap.php, this will be removed when we no longer need to support both versions of Symfony.

1. +++ b/core/modules/media_library/src/MediaLibraryState.php
   @@ -268,7 +268,29 @@ public function getAvailableSlots() {
   +  /**
   +   * Returns the parameters.
   +   *
   +   * @param string|null $key
   +   *   The name of the parameter to return or null to get them all.
   +   *
   +   * @return array
   +   *   An array of parameters.
   +   */
   +  public function all(string $key = NULL): array {
   

   We should add an @todo to remove this. I agree with @longwave - I think using all(PARAM_NAME) when we expect an array and get(PARAM_NAME) when we expect a scalar is how this is intended to be used.

   I would be great to have an InputBag clean up issue that we can link to.

2. +++ b/core/modules/media_library/tests/src/Kernel/MediaLibraryStateTest.php
   @@ -286,7 +287,12 @@ public function testFromRequest(array $query_overrides, $exception_expected) {
   +    // @todo Remove this when Symfony 4 is no longer supported.
   

   This could link to the inputbag clean up issue.

Inputbag clean-up issue should probably be this one #3162981: [Symfony 6] Use Symfony\Component\HttpFoundation\InputBag where appropriate instead of ParameterBag.

Adding #3254250: [Symfony 6] Retrieving a non-string value from "Symfony\Component\HttpFoundation\InputBag::get()" is deprecated to related issues, was previously duplicating the fix here.

Tried to address #14.1 and .2.

Committed/pushed to 10.0.x, also cherry-picked to 9.4.x to keep the tests in sync.