Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Implement HTTP API for Pusher broadcaster + statistics #95

Merged
merged 16 commits into from Jan 16, 2017

Conversation

barryvdh
Copy link
Contributor

@barryvdh barryvdh commented Jan 7, 2017

This pulls in Express to simplify HTTP server routing and json responses. It also adds additional info routes for information. See #92, routes and result based on Pusher (to keep it in line with other Echo services); https://pusher.com/docs/rest_api#channels

  • /status - Total numbers of connected clients
    {"user_count":123}

  • /channels or /channels?filter_by_prefix=private - List of channels + number of connected clients
    {"channels":{"private-App.Channel.14":{"user_count":12}}}

  • /channels/private-App.Channel.14 - Stats for just 1 channel
    {"user_count":12}

All routes are protected with the api key. The rooms that are created by default for a socket (eg. with the socket id as name) are filtered out.

Note that I am not really familiar with NodeJS programming. I kind a wanted to seperate this from the http-subscriber by adding a new class, but got in trouble with passing around this. In my head I would create a middleware for the ApiKey that can be used everywhere and create a new class for the statistics. I would also add a route to get a list of userIds (not socketIds if possible) for the Presence channels on /channels/:channelName/users, but that would depend on the database.

@barryvdh
Copy link
Contributor Author

barryvdh commented Jan 7, 2017

Alright, think I got the presence channel working now:

/channels/presence-chat.14/users
{"users":[{"id":1},{"id":2}]}

(Edit: removed the actual info, return just the IDs now, as there known in the app anyways)

@barryvdh
Copy link
Contributor Author

barryvdh commented Jan 8, 2017

We could also implement the broadcast similar to Pusher: https://pusher.com/docs/rest_api#events

And shall we remove the referrer check and use just the api key? And perhaps refactor it to just an array of apiKeys, with referrer/username?

And perhaps some statistics about how long the nodejs instance is running + memory usage, in the status request?

@jonnywilliamson
Copy link
Contributor

@barryvdh

This stuff is just brilliant. Thankyou.

@barryvdh
Copy link
Contributor Author

barryvdh commented Jan 8, 2017

So I guess this is more separated

  • created a global filter for the api tokens so we don't have to repeat that
  • new HttpApi class that register the api routes, separate from the subscriber

@barryvdh
Copy link
Contributor Author

barryvdh commented Jan 9, 2017

Just a thought, but with an HTTP api, it would be easier to implement namespaces: http://socket.io/docs/rooms-and-namespaces/
Eg. run 1 (forge?) server with a single socket instance, but multiple namespaces (chat, notifications) etc to connect to.

@tlaverdure
Copy link
Owner

@barryvdh

  • Yes on the broadcasting and removal of the referrer.
  • Time running seems simple, would just need save a timestamp when the server starts and use that in the api class to determine the time since. Memory should be accessible as well through process https://nodejs.org/api/process.html#process_process_memoryusage
  • Initially I thought of using namespaces but didn't really see the value. If you figure something out, let's create a separate issue/pr for that.

This is looking good, thank you!

@barryvdh
Copy link
Contributor Author

barryvdh commented Jan 9, 2017

Should we just follow the same route/parameters for broadcasting, or should we try to achieve 1:1 compatibility with Pusher? In that case, I think you could even use the Pusher php lib from Laravel and just change the pusher host/port in the options.
But that would need some different authentication, like this: https://github.com/pusher/pusher-http-node/blob/master/lib/token.js

@barryvdh
Copy link
Contributor Author

barryvdh commented Jan 9, 2017

I updated the API to follow Pusher, so now we can do this in our config/broadcasting.php

 'pusher' => [
            'driver' => 'pusher',
            'key' => env('PUSHER_KEY'),
            'secret' => env('PUSHER_SECRET'),
            'app_id' => env('PUSHER_APP_ID'),
            'options' => [
                'host' => 'localhost',
                'port' => 6001,
            ],
        ],

Only PUSHER_KEY is required, the secret and app_id can be empty. This will send HTTP requests to the host/post running laravel-echo-server instead of using Redis.

This also allows us to use the Pusher api the same as with actual Pusher:

use Illuminate\Support\Facades\Broadcast;
Route::get('pusher', function() {
    /** @var Pusher $pusher */
    $pusher = Broadcast::getPusher();

    dump($pusher->get_channels());
    dump($pusher->get_channels( array( 'filter_by_prefix' => 'private-' ) ));
    dump($pusher->get_channel_info('presence-chat.14'));
    dump($pusher->get( '/channels/presence-chat.14/users' ));
});

screen shot 2017-01-09 at 22 16 26

Right now I'm only showing the subscription_count for all channels (all socket connections) and not the user_count, which could be shown but is async callback, so not sure if that's smart.

Also still using the referrer key from the options, not just checking. Still need to change that to just 'apiKey' option or something (1 key seems enough?). I'm just checking the key (which is passed in the query params), not the actual signature. Not sure if that's needed, as we don't show the ApiKey publicly anyways..

Also extra status is on the ToDo.

@barryvdh
Copy link
Contributor Author

Alrighty then:

  • updated some responses to correct subscription vs users
  • added uptime/memory from the process functions to status, renamed route to align with rest ($pusher->get( '/status' ))
  • removed referrer, added apiKey option/cli command
  • updated readme

I think this is good to review now :)

@barryvdh
Copy link
Contributor Author

Or would it be better to actually use the app_id also? So generate an appId + appKey combo and use that to verify. Could make it easier to have multiple accounts.
In future you could think about adding namespaces that way.

@tlaverdure
Copy link
Owner

@barryvdh let's go with an app_id and key.

@barryvdh
Copy link
Contributor Author

Okay added that. Similar to the old referrers, you can add multiple ones and define the appId or generate it.

@barryvdh barryvdh changed the title Implement Express router, show channel statistics Implement HTTP API for Pusher broadcaster + statistics Jan 13, 2017
@tlaverdure tlaverdure self-requested a review January 16, 2017 12:27
@tlaverdure tlaverdure merged commit 1e7dffa into tlaverdure:master Jan 16, 2017
@barryvdh barryvdh deleted the feat-http branch January 16, 2017 12:31
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants