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.

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

Added setAttachment and getAttachment to WebSocket interface #627

Merged
merged 2 commits into from
Dec 4, 2017

Conversation

promatik
Copy link
Contributor

Description

This allows a WebSocket connection object to have any kind of data associated.
You can now store information of each connection/user and access it when onMessage or onClose is fired.

Usage example

public void onOpen(WebSocket conn, ClientHandshake handshake) {
    User user = new User(conn);
    conn.setAttachment(user);
}

public void onMessage(WebSocket conn, String message) {
    User user = conn.getAttachment();
    // ...
}

Related Issue

#485 Create key/value map in WebSocket

Motivation and Context

I need to store information about each connection, id, name, etc.
I started by open an issue, then @marci4 replied there was an open discussion about my problem here: #485. After reading all it was said about a possible implementation, I did some performance tests between implementing an HashMap or creating a getter and a setter to an attachment of type <T>, and this one was the most efficient by far, with tens and billions of elements.

How Has This Been Tested?

I'm using this feature now, and it's working great. I have a User.java that I use as the connection attachment, with many variables and some logic, everything is working as expected.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

This allows a WebSocket connection object to have any kind of data associated.
@marci4
Copy link
Collaborator

marci4 commented Nov 30, 2017

Hello @promatik,

thank you very much for your pull request.

I really like your approach, but would like to wait on some feedback from the rest of the community, if this is fine for you (waiting for one week or so).

Greetings
marci4

Copy link
Collaborator

@marci4 marci4 left a comment

Choose a reason for hiding this comment

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

Do you have any tests which you can include for this feature?
And could you please add the JavaDoc tag @since 1.3.7? Thank you

@promatik
Copy link
Contributor Author

promatik commented Dec 4, 2017

Hi @marci4, just added the javadoc since tag.
I'm not used to do tests, I don't know what kind of tests could I do for this feature.

Regards!

@marci4
Copy link
Collaborator

marci4 commented Dec 4, 2017

Hello @promatik,
thank you very much for your changes.

That is fine, I will add a simple JUnit test for this feature :)

Greetings
marci4

@marci4 marci4 merged commit 0529363 into TooTallNate:master Dec 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants