Remove "socket" key from message data

[kwhohasamullet]

Noticed that all messages being sent form Laravel broadcast events included an extra socket key in the message data. Depending on the structure of data I was sending from Laravel (and in turn receiving in the client) this extra attribute could cause the data structure to be different from what was expected and cause errors.

It appears that this socket value comes from the InteractsWithSockets trait which is included by default in all new events created by artisan so this should be handed as the default. Using the Pusher implementation as inspiration it appears they are handling this by pulling the socket key from the payload array before building up the message.

https://github.com/illuminate/broadcasting/blob/68939fc64fb3d553e066dbfcee7c0d835d7d24bc/Broadcasters/PusherBroadcaster.php#L151

This pull request implements the same logic that is used by the Pusher implementation. The Arr::pull function returns a default value of null which is the same as data_get function currently used.

[sacOO7]

Hi @kwhohasamullet, thanks for raising the PR. we will take a look 👍

[sacOO7]

Is it possible to add a unit test for this to check that socket key shouldn't exist in the response?

[kwhohasamullet]

Hi @sacOO7.

I do not have a huge amount of experience writing unit tests. Had a quick go at it below at am able to get the serialised string when it hits the post message but the de-serialisation does not seem straight forward (and I imagine is not really required in the php SDK). Strictly speaking it would be easier to test the buildAblyMessage method directly but it is a protected method and a quick google suggests the general view is not to test protected methods (and if you do requires using reflection to make it public for testing)

If there is an easier way to do this that I am not seeing and you can nudge me in the right direction I am happy to keep looking at this - else I imagine it would be a pretty simple test for someone with experience to write.

[sacOO7]

I do not have a huge amount of experience writing unit tests. Had a quick go at it below at am able to get the serialised string when it hits the post message but the de-serialisation does not seem straight forward (and I imagine is not really required in the php SDK). Strictly speaking it would be easier to test the buildAblyMessage method directly but it is a protected method and a quick google suggests the general view is not to test protected methods (and if you do requires using reflection to make it public for testing)

@qsdstefan would you be able to take a look at this? This does look simple ...

[qsdstefan]

@sacOO7 @kwhohasamullet I've decided to expose the protected method by creating a new class that inherits AblyBroadcaster and provides a single public method which we use in the test. This seems a bit cleaner than using reflection.

[sacOO7]

LGTM

[qsdstefan]

Looks good, let's wait for @owenpearson to perform the final review!

[owenpearson]

LGTM, thanks for the contribution @kwhohasamullet :)