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

Jump to bottom

Rename read/write to send/receive #1603

Merged
merged 6 commits into from Dec 19, 2017
Merged

Rename read/write to send/receive #1603

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5818177
Rename read/write to send/receive for better understanding. Closes #1596
rsoeldner Dec 18, 2017
ea15b13
Add deprecationName to Websocket read/write method
rsoeldner Dec 18, 2017
41b4db1
Add version to deprecatedName
rsoeldner Dec 18, 2017
a311b7c
Fix to correct deprecation version
rsoeldner Dec 18, 2017
eda3197
Clearify parameter description
rsoeldner Dec 19, 2017
cb667d4
Fix compilation on Scala 2.11
rsoeldner Dec 19, 2017
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
4 changes: 2 additions & 2 deletions blaze-core/src/main/scala/org/http4s/blazecore/websocket/Http4sWSStage.scala
View file
Open in desktop
Expand Up @@ -88,8 +88,8 @@ class Http4sWSStage[F[_]](ws: ws4s.Websocket[F])(implicit F: Effect[F], val ec:

val wsStream = for {
dead <- deadSignal
in = inputstream.to(ws.write).onFinalize(onStreamFinalize)
out = ws.read.onFinalize(onStreamFinalize).to(snk).drain
in = inputstream.to(ws.receive).onFinalize(onStreamFinalize)
out = ws.send.onFinalize(onStreamFinalize).to(snk).drain
merged <- in.mergeHaltR(out).interruptWhen(dead).onFinalize(sendClose).run
} yield merged

Expand Down
13 changes: 10 additions & 3 deletions core/src/main/scala/org/http4s/websocket/Websocket.scala
View file
Open in desktop
Expand Up @@ -4,6 +4,13 @@ import fs2._
import org.http4s.websocket.WebsocketBits.WebSocketFrame

private[http4s] final case class Websocket[F[_]](
read: Stream[F, WebSocketFrame],
write: Sink[F, WebSocketFrame]
)
@deprecatedName('read) send: Stream[F, WebSocketFrame],
Copy link
Member

Choose a reason for hiding this comment

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

Can the deprecation be versioned here?

@deprecatedName('write) receive: Sink[F, WebSocketFrame]
) {

@deprecated("Parameter has been renamed to `send`", "0.18.0-M7")
def read: Stream[F, WebSocketFrame] = send

@deprecated("Parameter has been renamed to `receive`", "0.18.0-M7")
def write: Sink[F, WebSocketFrame] = receive
}
15 changes: 7 additions & 8 deletions server/src/main/scala/org/http4s/server/websocket/package.scala
View file
Open in desktop
Expand Up @@ -17,9 +17,8 @@ package object websocket {
/**
* Build a response which will accept an HTTP websocket upgrade request and initiate a websocket connection using the
* supplied exchange to process and respond to websocket messages.
* @param read The read side of the Exchange represents the stream of messages that should be sent to the client
* @param write The write side of the Exchange is a sink to which the framework will push the websocket messages
* received from the client.
* @param send The send side of the Exchange represents the outgoing stream of messages that should be sent to the client
* @param receive The receive side of the Exchange is a sink to which the framework will push the incoming websocket messages
* Once both streams have terminated, the server will initiate a close of the websocket connection.
* As defined in the websocket specification, this means the server
* will send a CloseFrame to the client and wait for a CloseFrame in response before closing the
Expand All @@ -40,13 +39,13 @@ package object websocket {
* @param status The status code to return to a client making a non-websocket HTTP request to this route
*/
def WS[F[_]](
read: Stream[F, WebSocketFrame],
write: Sink[F, WebSocketFrame],
send: Stream[F, WebSocketFrame],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aeons I'm unsure how to handle the deprecation here

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 think we can in this case. It'll break for people using named arguments, but at least the types are unique. I think it'll be okay.

Copy link
Member

Choose a reason for hiding this comment

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

👍 to this as well. It should be a simple fix for users.

receive: Sink[F, WebSocketFrame],
status: F[Response[F]])(implicit F: Functor[F]): F[Response[F]] =
status.map(_.withAttribute(AttributeEntry(websocketKey[F], Websocket(read, write))))
status.map(_.withAttribute(AttributeEntry(websocketKey[F], Websocket(send, receive))))

def WS[F[_]](read: Stream[F, WebSocketFrame], write: Sink[F, WebSocketFrame])(
def WS[F[_]](send: Stream[F, WebSocketFrame], receive: Sink[F, WebSocketFrame])(
implicit F: Monad[F],
W: EntityEncoder[F, String]): F[Response[F]] =
WS(read, write, Response[F](Status.NotImplemented).withBody("This is a WebSocket route."))
WS(send, receive, Response[F](Status.NotImplemented).withBody("This is a WebSocket route."))
}