Media library does not enforce order which can lead to different hashes.

I think we can easily test this with a kernel test:

• Create two MediaLibraryState objects, with the same allowed media type IDs and opener parameters. These arrays should have the same values, but in a different order.
• Assert that both state objects produce the same value from their getHash() method.

Tagging for tests and marking as "needs work". Once we have that, we can easily land this. Very nice find!

Here's test coverage. I also moved the sorting into getHash(), as I think that's more appropriate.

We don't really need to be massaging the values in ::create() and and it was redundant in the following code, since it was being passed to create() just after.

+    ksort($allowed_media_type_ids);
+    ksort($opener_parameters);
+
     $state = MediaLibraryState::create('media_library.opener.field_widget', $allowed_media_type_ids, $selected_type_id, $remaining, $opener_parameters);

3) ksort($allowed_media_type_ids) fails when they are passed without keys, such as ['image', 'file'], so changed that up a bit:

+    $allowed_media_type_ids = array_values($this->getAllowedTypeIds());
+    sort($allowed_media_type_ids);

Oops, I left some test code in there while trying to figure out #4.3. Removing it.

Retriggering test of #5, It looks like a random failure.

Looking excellent! Can't wait to RTBC this bugfix. I'm nominating it for backport to 8.7.x because it is a non-disruptive bug fix, in an experimental module, no less.

1. +++ b/core/modules/media_library/src/MediaLibraryState.php
   @@ -179,13 +179,19 @@ protected function validateRequiredParameters($opener_id, array $allowed_media_t
   +    // optional opener-specific parameters.  Sort the allowed types and
   

   Nit: There is an extra space before "Sort".

2. +++ b/core/modules/media_library/src/MediaLibraryState.php
   @@ -179,13 +179,19 @@ protected function validateRequiredParameters($opener_id, array $allowed_media_t
   +    $allowed_media_type_ids = array_values($this->getAllowedTypeIds());
   +    sort($allowed_media_type_ids);
   

   Good call on using array_values()! 👍

3. +++ b/core/modules/media_library/tests/src/Kernel/MediaLibraryStateTest.php
   @@ -364,4 +364,22 @@ public function testOpenerParameters() {
   +    $state = MediaLibraryState::create('test', ['file', 'image'], 'image', 2);
   +    $state2 = MediaLibraryState::create('test', ['image', 'file'], 'image', 2);
   +    $this->assertSame($state2->getHash(), $state->getHash());
   +    $state3 = MediaLibraryState::create('test', ['file'], 'file', -1, [
   +      'foo' => 'baz',
   +      'baz' => 'foo',
   +    ]);
   +    $state4 = MediaLibraryState::create('test', ['file'], 'file', -1, [
   +      'baz' => 'foo',
   +      'foo' => 'baz',
   +    ]);
   +    $this->assertSame($state4->getHash(), $state3->getHash());
   

   This is a little more verbose than it needs to be. We can combine $state and $state3, and $state2 and $state4, and still get the same amount of test coverage.

1. Removed extra space.
3. After discussing this with @phenaproxima on Slack, I broke it up into two methods, one to test allowed media types and one to test open parameters. He agreed with me that keeping the two separate sets of tests is OK, as it tests the allowed view modes and the opener params in isolation.

Looks good to me. RTBC when Christmas-ey.

Committed 5a2c58c and pushed to 8.8.x. Thanks!

Thanks!

Linking to other tickets to increase the likelihood someone finds this issue.