Framework: assertExactJson false positive for JSON lists (ie non-assoc arrays)

[plumdog]

• Laravel Version: 5.7.11
• PHP Version: 7.1.3

Description:

The assertExactJson method sorts the JSON, both in the response and in the expected data before comparing. This is not the correct way of handling JSON data if the data contains a list. Eg:

{"foo": [1,2]}

is not the same JSON as

{"foo": [2,1]}

but assertExactJson behaves as though they are equivalent.

In other words: for JSON comparison, associative arrays should be compared ignoring order, non-associative arrays should be compared respecting order.

Steps To Reproduce:

    public function testJson1()
    {
        // I would expect this to pass
        $response = new TestResponse((new JsonResponse(['foo' => [1, 2]])));
        $response->assertExactJson(['foo' => [1, 2]]);
    }

    public function testJson2()
    {
        // I would expect this to fail
        $response = new TestResponse((new JsonResponse(['foo' => [2, 1]])));
        $response->assertExactJson(['foo' => [1, 2]]);
    }

[HongjiangHuang]

$response = new TestResponse((new JsonResponse(['people' => [['name' => 'albert1'], ['name' => 'albert2']]])));
$response->assertExactJson(['people' => [['name' => 'albert2'], ['name' => 'albert1']]]);

Do you think the above code should fail ?

[plumdog]

Yes, I think it should fail, as the two JSON lists under the people key are not equal.

[vlakoff]

For the history, see #17809 and https://github.com/laravel/framework/commit/0c1a306fa2b5b45a5c7f762d862ea99dd3e8b06d, where the method has been renamed (and its purpose somehow redefined).

I agree it would be more sensible to not sort the values.

[driesvints]

From the JSON spec:

An object is an unordered collection of zero or more name/value pairs, where a name is a string and a value is a string, number, boolean, null, object, or array.

Sorting shouldn't have an implication.

[plumdog]

@driesvints But also from the JSON spec:

An array is an ordered sequence of zero or more values.

So while sorting shouldn't have an implication for JSON objects, it should have an implication for JSON arrays.

So I stand by my example that {"foo": [1,2]} is not the same JSON as {"foo": [2,1]}.

[driesvints]

You're correct. This is indeed not intended and should be fixed.

[driesvints]

Since the assertExactJson method has been altered a lot already in the past and in every case it breaks at least something for someone we've decided that we won't alter it any further for the time being.

[plumdog]

@driesvints I would be just about OK with that, but for the fact that it has "exact" in its name. That makes me feel like this is not merely surprising behaviour, but that the method definitely doesn't do the thing its name says it does and so is broken behaviour.

Some possible solutions that come to mind:

1. just fix assertExactJson.
2. fix assertExactJson, add a assertJsonIgnoringOrder (or some other name) that preserves existing behaviour.
3. deprecate assertExactJson and create a new method called assertJsonEquals (or some other name) that does the correct thing - could add assertJsonIgnoringOrder if that has any long-term value.
4. keep assertExactJson as is and add assertRealExactJson that does the correct thing (...)

Personally, I think these are in order from best to worst. I understand that fixing assertExactJson will break some tests, but I reason that if someone wrote assertExactJson in their test, they deserve to know if the two bits of JSON they're comparing are in fact not the same.

[vlakoff]

I'd suggest:

• assertSameJson (fixed behavior)
• assertSameJsonIgnoringIndexes (current behavior)

The latter could be useful for cases like: userRights: ['admin', 'dictator', 'moderator']
where we are looking for "flags" regardless of their order.

[crissi]

I think this should really be consided to be opened again.