Skip to content

Add WebSocket event stream for REST clients - #2

Open
peerchemist wants to merge 26 commits into
masterfrom
ws
Open

Add WebSocket event stream for REST clients#2
peerchemist wants to merge 26 commits into
masterfrom
ws

Conversation

@peerchemist

Copy link
Copy Markdown
Member

Adds a WebSocket event stream for browser REST clients. Event messages are sent as JSON text frames with explicit type names and base64-encoded event bytes:

{
  "type": "dkg_commitment",
  "data": "<base64 event bytes>"
}

@MatthewLM MatthewLM left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only skimmed through it. I can't thoroughly review all the new REST stuff.

Comment thread lib/src/server/state/state.dart Outdated

ServerState() {
ServerState({
Logger? logger,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doesn't need to be nullable?

Comment thread lib/src/server/state/state.dart Outdated
void sendEventToAll(Event e, {List<SessionID> exclude = const []}) {
for (final session in clientSessions.values) {
if (!exclude.contains(session.sessionID)) session.sendEvent(e);
final sessions = clientSessions.values.toList();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

toList() appears unnecessary. List is created for recipients on next line. Probably best to do:

final recipients = clientSessions
        .values
        .where((session) => !exclude.contains(session.sessionID))
        .toList();

Comment thread lib/src/server/api_handler.dart Outdated
logger: logger ?? createNoosphereRoastServerLogger(),
);

ServerApiHandler._({

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Don't see why a new _ constructor is required? Can't Logger be set similar to state?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants