Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Logitech#765 - Provide support for npm cometd client #767

Merged
merged 1 commit into from
Feb 20, 2022
Merged

Logitech#765 - Provide support for npm cometd client #767

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions Slim/Web/Cometd.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,19 @@ sub handler {
my $delayedResponse; # false if we want to call sendResponse at the end of this method

for my $obj ( @{$objs} ) {

# Issue #765 - For comet meta messages handshake, connect, reconnect, disconnect, subscribe, unsubscribe populate 'id' field on response to ensure compatibility with the readily available npm comet library.
my $msgid;
if ( !$msgid ) {
# obtain 'id' from outer object if present, otherwise default to - Here we use different variable name '$msgid' as '$id' is defined for use within slim/subscribe channel.
if ( $obj->{id} ) {
$msgid = $obj->{id};
} else {
#default to empty string if not available.
$msgid = '';
}
}
Comment on lines +160 to +169
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$msgid will always be falsy right after definition. This block could be simplified to the following, couldn't it?

my $msgid = $obj->{id} || '';

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, I'll do this on my end. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes Michael. that is perfect and what I'd have done if I knew how.

I'm a VisualBasic, C/C++, Java, NodeJS, Fortran, C#, .NET guy.
I dabbled in Perl and Python early on and to get the right syntax, I Google.

While working on this I considered the following:
In the /slim/request section we could move the my $id variable to the top and use it instead of my $msgid. That way there's no confusion and $id means $id.

Please double-check my work for all the error conditions to make sure I did not screw things up.


if ( ref $obj ne 'HASH' ) {
sendResponse(
@{$conn},
Expand Down Expand Up @@ -218,6 +231,7 @@ sub handler {
if ( !$manager->is_valid_clid( $clid ) ) {
# Invalid clientId, send advice to re-handshake
push @{$events}, {
id => $msgid,
channel => $obj->{channel},
clientId => undef,
successful => JSON::XS::false,
Expand All @@ -241,6 +255,7 @@ sub handler {
};

push @{$events}, {
id => $msgid,
channel => '/meta/handshake',
version => PROTOCOL_VERSION,
supportedConnectionTypes => [ 'long-polling', 'streaming' ],
Expand All @@ -257,6 +272,7 @@ sub handler {
# We want the /meta/(re)connect response to always be the first event
# sent in the response, so it's stored in the special first_event slot
$conn->[HTTP_CLIENT]->first_event( {
id => $msgid,
channel => $obj->{channel},
clientId => $clid,
successful => JSON::XS::true,
Expand Down Expand Up @@ -318,6 +334,7 @@ sub handler {

# disconnect them
push @{$events}, {
id => $msgid,
channel => '/meta/disconnect',
clientId => $clid,
successful => JSON::XS::true,
Expand All @@ -344,6 +361,7 @@ sub handler {

for my $sub ( @{$subscriptions} ) {
push @{$events}, {
id => $msgid,
channel => '/meta/subscribe',
clientId => $clid,
successful => JSON::XS::true,
Expand All @@ -364,6 +382,7 @@ sub handler {

for my $sub ( @{$subscriptions} ) {
push @{$events}, {
id => $msgid,
channel => '/meta/unsubscribe',
clientId => $clid,
subscription => $sub,
Expand Down