Make device display name check optional for federation/v1/user/keys/query

[anoadragon453]

Some homeservers (Synapse) have started omitting device's display name over federation by default. The spec doesn't mandate that this field be available, so Sytest shouldn't either.

This PR modifies the check so that it is optional - only if the field is present do we check the value.

[DMRobertson]

FWIW I have no idea if this is idomatic perl (from #1307).

IIRC I manually inlined assert_eq to get the a == b or croak "wrong" form, and then shoved an extra defined check at the start.

[richvdh]

I'd say in this situation it would be more idiomatic to use || between the conditions:

         (!defined $device_display_name || $device_display_name eq "test display name") or
            croak "Unexpected device_display_name: $device_display_name";

Note also the use of eq instead of ==: the latter is for numeric comparison:

$ perl -e '"a" == "b" and print "true\n";'
true

A simpler approach here would be to do something like:

         my $device_display_name = $alice_device_keys->{"unsigned"}->{"device_display_name"} // "";
         assert_eq "test display name", $device_display_name, "device display name";

(// is like an || operator, but whereas || returns its right-hand operand if the left-hand one is falsey, // only returns its right-hand operand if the left-hand operand is undef. See https://perldoc.perl.org/perlop#Logical-Defined-Or)

[richvdh]

Note also the use of eq instead of ==: the latter is for numeric comparison:

(and yes, this is a massive footgun)

[DMRobertson]

Ahh, yes, assert_eq does indeed use eq.

sytest/lib/SyTest/Assertions.pm

Lines 61 to 67 in 32129d9

	sub assert_eq
	{
	my ( $got, $want, $name ) = @_;
	defined $got && defined $want && $got eq $want or
	croak "Got ${\ pp $got }, expected ${\ pp $want } for $name";
	}

Presumably this is bogus with == then. Can you fix that up here Andrew---and the test from #1307?

[anoadragon453]

Sure, done in ad8ad25.

[anoadragon453]

I actually wonder if my original code would be easier to read for the average programmer?

         if (exists( $alice_device_keys->{"unsigned"}->{"device_display_name"} )) {
            assert_eq(
               $alice_device_keys->{"unsigned"}->{"device_display_name"},
               "test display name",
            );
         }

but either way I'm happy as long as it works.

[DMRobertson]

== -> eq (aka fix my mistakes pls)

[DMRobertson]

== -> eq (aka fix my mistakes pls)

(Or better: use Rich's formulation)

(Or better better: invent a time machine and prevent perl from being created)