Permalink
Browse files

! can: "privatize" all classes/objects not meant to be part of public…

… API
  • Loading branch information...
sirthias committed Sep 19, 2013
1 parent 912b8b1 commit da29cdfb1b94abacf91e2c0ab7d1ee625783c48c
Showing with 60 additions and 62 deletions.
  1. +1 −2 spray-can-tests/src/test/scala/spray/can/server/HttpServerConnectionPipelineSpec.scala
  2. +1 −1 spray-can/src/main/scala/spray/can/client/ClientFrontend.scala
  3. +5 −5 spray-can/src/main/scala/spray/can/client/HttpClientConnection.scala
  4. +5 −5 spray-can/src/main/scala/spray/can/client/HttpHostConnectionSlot.scala
  5. +1 −1 spray-can/src/main/scala/spray/can/client/RequestRendering.scala
  6. +1 −1 spray-can/src/main/scala/spray/can/client/ResponseChunkAggregation.scala
  7. +1 −1 spray-can/src/main/scala/spray/can/client/ResponseParsing.scala
  8. +1 −1 spray-can/src/main/scala/spray/can/parsing/CharUtils.scala
  9. +1 −1 spray-can/src/main/scala/spray/can/parsing/HttpHeaderParser.scala
  10. +1 −1 spray-can/src/main/scala/spray/can/parsing/HttpRequestPartParser.scala
  11. +1 −1 spray-can/src/main/scala/spray/can/parsing/HttpResponsePartParser.scala
  12. +1 −2 spray-can/src/main/scala/spray/can/parsing/SSLSessionInfoSupport.scala
  13. +1 −1 spray-can/src/main/scala/spray/can/parsing/SpecializedHeaderValueParsers.scala
  14. +4 −4 spray-can/src/main/scala/spray/can/parsing/package.scala
  15. +1 −1 spray-can/src/main/scala/spray/can/rendering/RenderSupport.scala
  16. +2 −2 spray-can/src/main/scala/spray/can/rendering/RenderedMessagePart.scala
  17. +1 −1 spray-can/src/main/scala/spray/can/rendering/RequestRenderingComponent.scala
  18. +1 −1 spray-can/src/main/scala/spray/can/rendering/ResponseRenderingComponent.scala
  19. +2 −2 spray-can/src/main/scala/spray/can/rendering/package.scala
  20. +4 −4 spray-can/src/main/scala/spray/can/server/HttpListener.scala
  21. +7 −7 spray-can/src/main/scala/spray/can/server/HttpServerConnection.scala
  22. +8 −8 spray-can/src/main/scala/spray/can/server/OpenRequest.scala
  23. +1 −1 spray-can/src/main/scala/spray/can/server/PipeliningLimiter.scala
  24. +1 −1 spray-can/src/main/scala/spray/can/server/RemoteAddressHeaderSupport.scala
  25. +1 −1 spray-can/src/main/scala/spray/can/server/RequestChunkAggregation.scala
  26. +1 −1 spray-can/src/main/scala/spray/can/server/RequestParsing.scala
  27. +2 −2 spray-can/src/main/scala/spray/can/server/ResponseReceiverRef.scala
  28. +1 −1 spray-can/src/main/scala/spray/can/server/ResponseRendering.scala
  29. +1 −1 spray-can/src/main/scala/spray/can/server/ServerFrontend.scala
  30. +1 −1 spray-can/src/main/scala/spray/can/server/StatsSupport.scala
@@ -32,9 +32,8 @@ import spray.can.Http
import spray.http._
import spray.can.TestSupport._
import HttpHeaders._
-import MediaTypes._
-class HttpServerConnectionPipelineSpec extends Specification with RawSpecs2PipelineStageTest {
+private class HttpServerConnectionPipelineSpec extends Specification with RawSpecs2PipelineStageTest {
type Context = ServerFrontend.Context with SslTlsContext
"The HttpServer pipeline" should {
@@ -26,7 +26,7 @@ import spray.http._
import spray.io._
import spray.util.Timestamp
-object ClientFrontend {
+private object ClientFrontend {
def apply(initialRequestTimeout: Duration): PipelineStage = {
new PipelineStage {
@@ -24,10 +24,10 @@ import spray.can.parsing.SSLSessionInfoSupport
import spray.http.{ SetRequestTimeout, Confirmed, HttpRequestPart }
import spray.io._
-private[can] class HttpClientConnection(connectCommander: ActorRef,
- connect: Http.Connect,
- pipelineStage: RawPipelineStage[SslTlsContext],
- settings: ClientConnectionSettings) extends ConnectionHandler { actor
+private class HttpClientConnection(connectCommander: ActorRef,
+ connect: Http.Connect,
+ pipelineStage: RawPipelineStage[SslTlsContext],
+ settings: ClientConnectionSettings) extends ConnectionHandler { actor
import context.system
import connect._
@@ -76,7 +76,7 @@ private[can] class HttpClientConnection(connectCommander: ActorRef,
}
}
-private[can] object HttpClientConnection {
+private object HttpClientConnection {
def pipelineStage(settings: ClientConnectionSettings) = {
import settings._
@@ -27,11 +27,11 @@ import spray.io.ClientSSLEngineProvider
import spray.http._
import spray.util.SimpleStash
-private[client] class HttpHostConnectionSlot(host: String, port: Int,
- sslEncryption: Boolean,
- options: immutable.Traversable[Inet.SocketOption],
- idleTimeout: Duration,
- clientConnectionSettingsGroup: ActorRef)(implicit sslEngineProvider: ClientSSLEngineProvider)
+private class HttpHostConnectionSlot(host: String, port: Int,
+ sslEncryption: Boolean,
+ options: immutable.Traversable[Inet.SocketOption],
+ idleTimeout: Duration,
+ clientConnectionSettingsGroup: ActorRef)(implicit sslEngineProvider: ClientSSLEngineProvider)
extends Actor with SimpleStash with ActorLogging {
// we cannot sensibly recover from crashes
@@ -22,7 +22,7 @@ import spray.can.rendering._
import spray.io._
import spray.util._
-object RequestRendering {
+private[can] object RequestRendering {
def apply(settings: ClientConnectionSettings): PipelineStage =
new PipelineStage with RequestRenderingComponent {
@@ -20,7 +20,7 @@ import spray.http._
import spray.io._
import spray.can.Http
-object ResponseChunkAggregation {
+private object ResponseChunkAggregation {
def apply(limit: Int): PipelineStage =
new PipelineStage {
@@ -26,7 +26,7 @@ import spray.can.parsing._
import spray.http._
import spray.io._
-object ResponseParsing {
+private object ResponseParsing {
def apply(settings: ParserSettings): PipelineStage = {
val rootParser = new HttpResponsePartParser(settings)()
@@ -22,7 +22,7 @@ import scala.annotation.tailrec
import java.lang.{ StringBuilder JStringBuilder }
// TODO: replace with spray.http.parser.CharMask
-private[parsing] object CharUtils {
+private object CharUtils {
// compile time constants
private final val LOWER_ALPHA = 0x01
private final val UPPER_ALPHA = 0x02
@@ -379,7 +379,7 @@ private[parsing] final class HttpHeaderParser private (val settings: ParserSetti
def formatSizes: String = s"$nodeCount nodes, ${branchDataCount / 3} nodeData rows, $valueCount values"
}
-private[parsing] object HttpHeaderParser {
+private object HttpHeaderParser {
import SpecializedHeaderValueParsers._
object EmptyHeader extends HttpHeader {
@@ -25,7 +25,7 @@ import StatusCodes._
import HttpHeaders._
import CharUtils._
-class HttpRequestPartParser(_settings: ParserSettings)(_headerParser: HttpHeaderParser = HttpHeaderParser(_settings))
+private[can] class HttpRequestPartParser(_settings: ParserSettings)(_headerParser: HttpHeaderParser = HttpHeaderParser(_settings))
extends HttpMessagePartParser[HttpRequestPart](_settings, _headerParser) {
private[this] var method: HttpMethod = GET
@@ -23,7 +23,7 @@ import HttpHeaders._
import StatusCodes._
import CharUtils._
-class HttpResponsePartParser(_settings: ParserSettings)(_headerParser: HttpHeaderParser = HttpHeaderParser(_settings))
+private[can] class HttpResponsePartParser(_settings: ParserSettings)(_headerParser: HttpHeaderParser = HttpHeaderParser(_settings))
extends HttpMessagePartParser[HttpResponsePart](_settings, _headerParser) {
private[this] var isResponseToHeadRequest: Boolean = false
@@ -21,10 +21,9 @@ import spray.can.server.RequestParsing
import spray.io._
import spray.http.HttpMessageStart
import spray.http.HttpHeaders.`SSL-Session-Info`
-import spray.util.SSLSessionInfo
/** Pipeline stage that adds the `SSL-Session-Info` header to incoming HTTP messages. */
-object SSLSessionInfoSupport extends PipelineStage {
+private[can] object SSLSessionInfoSupport extends PipelineStage {
def apply(context: PipelineContext, commandPL: CPL, eventPL: EPL): Pipelines =
new Pipelines {
@@ -23,7 +23,7 @@ import HttpHeaders._
import CharUtils._
import ProtectedHeaderCreation.enable
-private[parsing] object SpecializedHeaderValueParsers {
+private object SpecializedHeaderValueParsers {
import HttpHeaderParser._
def specializedHeaderValueParsers = Seq(ContentLengthParser)
@@ -20,12 +20,12 @@ import akka.util.CompactByteString
import spray.util.SingletonException
import spray.http._
-trait Parser[Part <: HttpMessagePart] extends (CompactByteString Result[Part]) {
+private[can] trait Parser[Part <: HttpMessagePart] extends (CompactByteString Result[Part]) {
def parse: CompactByteString Result[Part]
}
-sealed trait Result[+T <: HttpMessagePart]
-object Result {
+private[can] sealed trait Result[+T <: HttpMessagePart]
+private[can] object Result {
case object NeedMoreData extends Result[Nothing]
case class Ok[T <: HttpMessagePart](part: T, remainingData: CompactByteString,
closeAfterResponseCompletion: Boolean) extends Result[T]
@@ -40,4 +40,4 @@ class ParsingException(val status: StatusCode, val info: ErrorInfo) extends Runt
this(StatusCodes.BadRequest, ErrorInfo(summary))
}
-object NotEnoughDataException extends SingletonException
+private object NotEnoughDataException extends SingletonException
@@ -19,7 +19,7 @@ package spray.can.rendering
import spray.util._
import spray.http._
-private[rendering] object RenderSupport {
+private object RenderSupport {
val DefaultStatusLine = "HTTP/1.1 200 OK\r\n".getAsciiBytes
val StatusLineStart = "HTTP/1.1 ".getAsciiBytes
val Chunked = "chunked".getAsciiBytes
@@ -20,11 +20,11 @@ import akka.io.Tcp
import spray.io.Command
import spray.http._
-case class RequestPartRenderingContext(
+private[can] case class RequestPartRenderingContext(
requestPart: HttpRequestPart,
ack: Tcp.Event = Tcp.NoAck) extends Command
-case class ResponsePartRenderingContext(
+private[can] case class ResponsePartRenderingContext(
responsePart: HttpResponsePart,
requestMethod: HttpMethod = HttpMethods.GET,
requestProtocol: HttpProtocol = HttpProtocols.`HTTP/1.1`,
@@ -24,7 +24,7 @@ import spray.http._
import HttpHeaders._
import RenderSupport._
-trait RequestRenderingComponent {
+private[can] trait RequestRenderingComponent {

This comment has been minimized.

Show comment
Hide comment
@alexbool

alexbool Mar 13, 2014

As far as I can understand, there is no way you can render a http request from external (non-spray) code if all you have is an instance of spray.http.HttpRequest. If that is the case, this makes existence of a standalone library spray-http mostly meaningless, because you can construct instances of HttpRequests and HttpResonses, but you cannot render them to wire, because all rendering is package-private. If my assumption is true, I would recommend to make rendering and parsing of whole HttpRequests and HttpResonses part of spray-http public API. Then it will be possible to use glorious HTTP model of spray in other libraries.

@alexbool

alexbool Mar 13, 2014

As far as I can understand, there is no way you can render a http request from external (non-spray) code if all you have is an instance of spray.http.HttpRequest. If that is the case, this makes existence of a standalone library spray-http mostly meaningless, because you can construct instances of HttpRequests and HttpResonses, but you cannot render them to wire, because all rendering is package-private. If my assumption is true, I would recommend to make rendering and parsing of whole HttpRequests and HttpResonses part of spray-http public API. Then it will be possible to use glorious HTTP model of spray in other libraries.

This comment has been minimized.

Show comment
Hide comment
@jrudolph

jrudolph Mar 13, 2014

Member

Agreed. The problem is that we don't have the capacity to maintain the increased API surface created by including the parsing/rendering parts. So, we'd rather have to withdraw the original claim (if we ever made it that way) that spray-http can be used easily for implementing third party HTTP software. It is certainly possible but at this time we cannot make any guarantees that any of this internal code stays the same.

@jrudolph

jrudolph Mar 13, 2014

Member

Agreed. The problem is that we don't have the capacity to maintain the increased API surface created by including the parsing/rendering parts. So, we'd rather have to withdraw the original claim (if we ever made it that way) that spray-http can be used easily for implementing third party HTTP software. It is certainly possible but at this time we cannot make any guarantees that any of this internal code stays the same.

This comment has been minimized.

Show comment
Hide comment
@alexbool

alexbool Mar 13, 2014

Well, I guess, rendering/parsing will stabilize and mature at some point. And as for now, there's nothing that prevents external developers from copy/pasting some code from spray-can :)

@alexbool

alexbool Mar 13, 2014

Well, I guess, rendering/parsing will stabilize and mature at some point. And as for now, there's nothing that prevents external developers from copy/pasting some code from spray-can :)

This comment has been minimized.

Show comment
Hide comment
@sirthias

sirthias Mar 13, 2014

Member

Yes. However, the spray-http module merely provides a model with only little supporting logic. It intentionally does not include the full rendering logic as this depends at least partially on the underlying transport.
Still, spray-http does come with a basic Rendering infrastructure that is fully extensible and allows for very efficient rendering of all parts of an HTTP message into whatever representation you'd like.

@sirthias

sirthias Mar 13, 2014

Member

Yes. However, the spray-http module merely provides a model with only little supporting logic. It intentionally does not include the full rendering logic as this depends at least partially on the underlying transport.
Still, spray-http does come with a basic Rendering infrastructure that is fully extensible and allows for very efficient rendering of all parts of an HTTP message into whatever representation you'd like.

def userAgent: Option[`User-Agent`]
def renderRequestPart(r: Rendering, part: HttpRequestPart, serverAddress: InetSocketAddress,
@@ -25,7 +25,7 @@ import HttpProtocols._
import HttpHeaders._
import RenderSupport._
-trait ResponseRenderingComponent {
+private[can] trait ResponseRenderingComponent {
def serverHeaderValue: String
def chunklessStreaming: Boolean
def transparentHeadRequests: Boolean
@@ -20,14 +20,14 @@ import spray.http.HttpData
import akka.io.Tcp
package object rendering {
- def toTcpWriteCommand(data: HttpData, ack: Tcp.Event): Tcp.WriteCommand =
+ private[can] def toTcpWriteCommand(data: HttpData, ack: Tcp.Event): Tcp.WriteCommand =
data match {
case HttpData.Empty Tcp.Write.empty
case HttpData.Compound(head, tail) toTcpWriteCommand(head, ack) +: toTcpWriteCommand(tail, ack)
case x: HttpData.SimpleNonEmpty toTcpWriteCommand(x, ack)
}
- def toTcpWriteCommand(data: HttpData.SimpleNonEmpty, ack: Tcp.Event): Tcp.SimpleWriteCommand =
+ private[can] def toTcpWriteCommand(data: HttpData.SimpleNonEmpty, ack: Tcp.Event): Tcp.SimpleWriteCommand =
data match {
case HttpData.Bytes(byteString) Tcp.Write(byteString, ack)
case HttpData.FileBytes(fileName, offset, len) Tcp.WriteFile(fileName, offset, len, ack)
@@ -28,10 +28,10 @@ private[can] class HttpListener(bindCommander: ActorRef,
import context.system
import bind._
- val connectionCounter = Iterator from 0
- val settings = bind.settings getOrElse ServerSettings(system)
- val statsHolder = if (settings.statsSupport) Some(new StatsHolder) else None
- val pipelineStage = HttpServerConnection.pipelineStage(settings, statsHolder)
+ private val connectionCounter = Iterator from 0
+ private val settings = bind.settings getOrElse ServerSettings(system)
+ private val statsHolder = if (settings.statsSupport) Some(new StatsHolder) else None
+ private val pipelineStage = HttpServerConnection.pipelineStage(settings, statsHolder)
context.watch(listener)
@@ -26,12 +26,12 @@ import spray.can.Http
import spray.can.parsing.SSLSessionInfoSupport
import spray.http.{ SetTimeoutTimeout, SetRequestTimeout }
-private[can] class HttpServerConnection(tcpConnection: ActorRef,
- userLevelListener: ActorRef,
- pipelineStage: RawPipelineStage[ServerFrontend.Context with SslTlsContext],
- remoteAddress: InetSocketAddress,
- localAddress: InetSocketAddress,
- settings: ServerSettings)(implicit val sslEngineProvider: ServerSSLEngineProvider)
+private class HttpServerConnection(tcpConnection: ActorRef,
+ userLevelListener: ActorRef,
+ pipelineStage: RawPipelineStage[ServerFrontend.Context with SslTlsContext],
+ remoteAddress: InetSocketAddress,
+ localAddress: InetSocketAddress,
+ settings: ServerSettings)(implicit val sslEngineProvider: ServerSSLEngineProvider)
extends ConnectionHandler { actor
userLevelListener ! Http.Connected(remoteAddress, localAddress)
@@ -82,7 +82,7 @@ private[can] class HttpServerConnection(tcpConnection: ActorRef,
}
}
-private[can] object HttpServerConnection {
+private object HttpServerConnection {
/**
* The HttpServerConnection pipeline setup:
@@ -28,7 +28,7 @@ import spray.can.server.ServerFrontend.Context
import akka.io.Tcp
import spray.util.Timestamp
-sealed trait OpenRequest {
+private sealed trait OpenRequest {
def context: ServerFrontend.Context
def isEmpty: Boolean
def request: HttpRequest
@@ -50,7 +50,7 @@ sealed trait OpenRequest {
def handleClosed(ev: Http.ConnectionClosed)
}
-trait OpenRequestComponent { component
+private trait OpenRequestComponent { component
def context: ServerFrontend.Context
def settings: ServerSettings
def downstreamCommandPL: Pipeline[Command]
@@ -237,10 +237,10 @@ trait OpenRequestComponent { component ⇒
}
-private[server] case class AckEventWithReceiver(ack: Any, receiver: ActorRef) extends Event
-private[server] case class PartAndSender(part: HttpResponsePart, sender: ActorRef)
+private case class AckEventWithReceiver(ack: Any, receiver: ActorRef) extends Event
+private case class PartAndSender(part: HttpResponsePart, sender: ActorRef)
-private[server] sealed trait RequestState
-private[server] case object WaitingForChunkedEnd extends RequestState
-private[server] case class WaitingForResponse(timestamp: Timestamp = Timestamp.now) extends RequestState
-private[server] case class WaitingForTimeoutResponse(timestamp: Timestamp = Timestamp.now) extends RequestState
+private sealed trait RequestState
+private case object WaitingForChunkedEnd extends RequestState
+private case class WaitingForResponse(timestamp: Timestamp = Timestamp.now) extends RequestState
+private case class WaitingForTimeoutResponse(timestamp: Timestamp = Timestamp.now) extends RequestState
@@ -38,7 +38,7 @@ import RequestParsing._
* i.e. during the `eventPL` call in handleEvent the complete request is handled and already sent
* out so that the openRequests counter is never increased.
*/
-object PipeliningLimiter {
+private object PipeliningLimiter {
def apply(pipeliningLimit: Int): PipelineStage =
new PipelineStage {
@@ -20,7 +20,7 @@ import spray.io._
import spray.http._
import HttpHeaders._
-object RemoteAddressHeaderSupport extends PipelineStage {
+private object RemoteAddressHeaderSupport extends PipelineStage {
def apply(context: PipelineContext, commandPL: CPL, eventPL: EPL): Pipelines =
new Pipelines {
val raHeader = `Remote-Address`(context.remoteAddress.getAddress)
@@ -22,7 +22,7 @@ import spray.io._
import spray.http._
import spray.can.Http
-object RequestChunkAggregation {
+private object RequestChunkAggregation {
def apply(limit: Int): PipelineStage =
new PipelineStage {
@@ -28,7 +28,7 @@ import spray.util._
import spray.io._
import HttpHeaders.`Raw-Request-URI`
-object RequestParsing {
+private[can] object RequestParsing {
val continue = ByteString("HTTP/1.1 100 Continue\r\n\r\n")
@@ -23,7 +23,7 @@ import spray.io.Command
import spray.http._
import spray.can.Http
-object ResponseReceiverRef {
+private object ResponseReceiverRef {
private val responseStateOffset = Unsafe.instance.objectFieldOffset(
classOf[ResponseReceiverRef].getDeclaredField("_responseStateDoNotCallMeDirectly"))
@@ -84,4 +84,4 @@ private class ResponseReceiverRef(openRequest: OpenRequest)
private def requestInfo = openRequest.request.method.toString + " request to '" + openRequest.request.uri + '\''
}
-private[server] case class Response(openRequest: OpenRequest, cmd: Command) extends Command
+private case class Response(openRequest: OpenRequest, cmd: Command) extends Command
@@ -21,7 +21,7 @@ import spray.http.HttpDataRendering
import spray.can.rendering._
import spray.io._
-object ResponseRendering {
+private object ResponseRendering {
def apply(settings: ServerSettings): PipelineStage =
new PipelineStage with ResponseRenderingComponent {
@@ -27,7 +27,7 @@ import spray.http._
import spray.io._
import spray.util.Timestamp
-object ServerFrontend {
+private object ServerFrontend {
trait Context extends PipelineContext {
// the application-level request handler
@@ -24,7 +24,7 @@ import spray.io._
import spray.can.Http
import spray.util.{ Timestamp, PaddedAtomicLong }
-object StatsSupport {
+private object StatsSupport {
class StatsHolder {
val startTimestamp = Timestamp.now

0 comments on commit da29cdf

Please sign in to comment.