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

Lavalink Major Version Header #111

Merged
merged 6 commits into from
Jun 5, 2018
Merged

Lavalink Major Version Header #111

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9088b86
add lavaclient to list of libs which support v3
luaugg Jun 3, 2018
4ce7ad7
Merge remote-tracking branch 'origin/v3' into v3
luaugg Jun 3, 2018
c906902
testing lavalink version header
luaugg Jun 4, 2018
77df2c0
change Lavalink-Version to Lavalink-Major-Version
luaugg Jun 4, 2018
ede5bdf
update implementation file to make ppl aware of the change
luaugg Jun 4, 2018
67b49c3
significant over breaking, removal of non breaking
luaugg Jun 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions IMPLEMENTATION.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ The Java client has support for JDA, but can also be adapted to work with other
## Breaking changes v2.0 -> v3.0
The response of `/loadtracks` has been completely changed.

## Non-breaking changes v2.0 -> v3.0
Copy link
Member

Choose a reason for hiding this comment

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

I don't see why we need an extra header. I would remove this and change "Breaking" to "Significant" in the other two

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, you want me to just change "Non-breaking" to "Significant" then?

Copy link
Contributor Author

@luaugg luaugg Jun 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

oh no sorry i gotcha, significant changes for v2 -> v3 and v1 -> v2 and then list em all without the need for an extra markdown header

Lavalink v3.0 now reports its version as a handshake response header.
`Lavalink-Major-Version` has a value of `3` for v3.0 and it's missing for any version below, allowing clients to change their behaviour for different nodes.

## Breaking changes v1.3 -> v2.0
With the release of v2.0 many unnecessary ops were removed:

Expand Down
2 changes: 2 additions & 0 deletions LavalinkServer/src/main/java/lavalink/server/config/SentryConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
/**
* Created by napster on 25.04.18.
*/

@Configuration
@SuppressWarnings("WeakerAccess")
public class SentryConfiguration {

private static final Logger log = LoggerFactory.getLogger(SentryConfiguration.class);
Expand Down
10 changes: 10 additions & 0 deletions LavalinkServer/src/main/java/lavalink/server/io/SocketServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@
import net.dv8tion.jda.Core;
import net.dv8tion.jda.manager.AudioManager;
import org.java_websocket.WebSocket;
import org.java_websocket.drafts.Draft;
import org.java_websocket.exceptions.InvalidDataException;
import org.java_websocket.handshake.ClientHandshake;
import org.java_websocket.handshake.ServerHandshakeBuilder;
import org.java_websocket.server.WebSocketServer;
import org.json.JSONObject;
import org.slf4j.Logger;
Expand Down Expand Up @@ -76,6 +79,13 @@ public void start() {
super.start();
}

@Override
public ServerHandshakeBuilder onWebsocketHandshakeReceivedAsServer(WebSocket conn, Draft draft, ClientHandshake request) throws InvalidDataException {
ServerHandshakeBuilder builder = super.onWebsocketHandshakeReceivedAsServer(conn, draft, request);
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure super's method is not empty?

Copy link
Contributor Author

@luaugg luaugg Jun 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

yes i'm sure, it returns an empty handshake impl but it seems java websocket initializes it elsewhere. it returns all the necessary websocket headers as defined in the RFC, its own server name as well as the custom header i specified. i also printed out all the headers received in my own client which returned exactly what i described while using the branch here so i'm very sure it works.

image

^ that was the image i posted in the jda server of the initial commit just before i changed Lavalink-Version to Lavalink-Major-Version as per your request, using code in my client that really does just print out the provided headers from the handshake

Copy link
Member

Choose a reason for hiding this comment

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

Ah fair enough, I didn't stop to consider the return value of it

builder.put("Lavalink-Major-Version", "3");
return builder;
}

@Override
public void onOpen(WebSocket webSocket, ClientHandshake clientHandshake) {
try {
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Being used in production by FredBoat, Dyno, Rythm, LewdBot, and more.
## Client libraries:
### Supports 3.x and older:
* [JDA client](https://github.com/Frederikam/Lavalink/tree/master/LavalinkClient) (JDA or generic, Java)
* [LavaClient](https://github.com/SamOphis/LavaClient/tree/lavalink-v3) (Java)
* [LavaClient](https://github.com/SamOphis/LavaClient) (Java)
* [Lavalink.py](https://github.com/Devoxin/Lavalink.py) (discord.py, Python)
* [SandySounds](https://github.com/MrJohnCoder/SandySounds) (JavaScript)

Expand Down