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

Topic/status.register 2015 #2032

Merged
merged 11 commits into from
Aug 30, 2018
Merged
190 changes: 106 additions & 84 deletions core/src/main/scala/org/http4s/Status.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.http4s

import cats._
import java.util.concurrent.atomic.AtomicReferenceArray
import org.http4s.Status.ResponseClass
import org.http4s.util.Renderable

Expand Down Expand Up @@ -43,6 +42,8 @@ sealed abstract case class Status private (code: Int)(
}

object Status {
import Registry._

def apply(code: Int, reason: String = "", isEntityAllowed: Boolean = true): Status =
new Status(code)(reason, isEntityAllowed) {}

Expand Down Expand Up @@ -76,102 +77,123 @@ object Status {
val ServerError = Status.Informational
}

private def mkStatus(code: Int, reason: String = ""): ParseResult[Status] =
if (code >= 100 && code <= 599)
ParseResult.success(Status(code, reason, isEntityAllowed = true))
else ParseResult.fail("Invalid status", s"Code $code must be between 100 and 599, inclusive")

private def lookup(code: Int) =
if (code < 100 || code > 599) None else Option(registry.get(code))
val MinCode = 100
Copy link
Contributor

Choose a reason for hiding this comment

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

these could potentially be private I think? I Don't know how useful they are to expose (except to maybe give a coworker a trivia quiz), or private[http4s]

val MaxCode = 599

def fromInt(code: Int): ParseResult[Status] = lookup(code).getOrElse(mkStatus(code))
def fromInt(code: Int): ParseResult[Status] = lookup(code) match {
case None => parseAsStatus(code)
case Some(status) => Right(status)
}

def fromIntAndReason(code: Int, reason: String): ParseResult[Status] =
lookup(code).filter(_.right.get.reason == reason).getOrElse(mkStatus(code, reason))
lookup(code, reason) match {
case None => parseAsStatus(code, reason)
case Some(status) => Right(status)
}

// scalastyle:off magic.number
private val registry =
new AtomicReferenceArray[Right[Nothing, Status]](600)
// scalastyle:on magic.number
def registered: Iterable[Status] = all
Copy link
Member

Choose a reason for hiding this comment

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

Iterable is a not very useful thing. Maybe we should go ahead and commit to a List here? At minimum, an immutable.Seq, so the user knows it's, like, immutable.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm 👍 on List instead of immutable.Seq

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense.


private def parseAsStatus(code: Int, reason: String = ""): ParseResult[Status] =
if (isStandard(code)) ParseResult.success(Status(code, reason))
Copy link
Member

Choose a reason for hiding this comment

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

Isn't isInRange what's important here? We should be able to create, say, 473 I AM A PINK FLAMINGO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm confused about this. I just commented on this in the issue thread, because I thought @jmcardon was saying otherwise.

Copy link
Contributor

@jmcardon jmcardon Aug 26, 2018

Choose a reason for hiding this comment

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

In his defense, I did say that IMO, creating custom statuses should be disallowed.

A better question yet is why anyone would like to create a custom status code that's nonstandard and supported by no one else.

else
ParseResult.fail("Invalid status", s"$code is not a valid response code.")

private object Registry {
private val registry = Array.fill[Option[Status]](MaxCode + 1)(None)
Copy link
Member

Choose a reason for hiding this comment

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

Could we microoptimize this by filling it with Either[SomeDummyValue, Status]? Then we could return the same reference without unwrapping an Option and recreating a Right.


def registered: Iterable[Status] =
for {
code <- 100 to 599
status <- Option(registry.get(code)).map(_.right.get)
} yield status
def lookup(code: Int): Option[Status] = if (isInRange(code)) registry(code) else None

def lookup(code: Int, reason: String): Option[Status] =
for { status <- lookup(code) if status.reason == reason } yield status

def isStandard(code: Int): Boolean = isInRange(code) && registry(code).isDefined

def register(status: Status): Status = {
registry(status.code) = Some(status)
status
}

def all: Iterable[Status] =
for {
code <- MinCode to MaxCode
status <- lookup(code)
Copy link
Member

Choose a reason for hiding this comment

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

Wait, what? These monads don't line up. Is there some implicit optionToIterable silliness happening here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... now that you mention it, I don't know what's happening either. I'll try to simplify it.

} yield status

private def isInRange(code: Int) =
code >= MinCode && code <= MaxCode

def register(status: Status): status.type = {
registry.set(status.code, Right(status))
status
}

/**
* Status code list taken from http://www.iana.org/assignments/http-status-codes/http-status-codes.xml
*/
// scalastyle:off magic.number
val Continue = register(Status(100, "Continue", isEntityAllowed = false))
val SwitchingProtocols = register(Status(101, "Switching Protocols", isEntityAllowed = false))
val Processing = register(Status(102, "Processing", isEntityAllowed = false))

val Ok = register(Status(200, "OK"))
val Created = register(Status(201, "Created"))
val Accepted = register(Status(202, "Accepted"))
val NonAuthoritativeInformation = register(Status(203, "Non-Authoritative Information"))
val NoContent = register(Status(204, "No Content", isEntityAllowed = false))
val ResetContent = register(Status(205, "Reset Content", isEntityAllowed = false))
val PartialContent = register(Status(206, "Partial Content"))
val MultiStatus = register(Status(207, "Multi-Status"))
val AlreadyReported = register(Status(208, "Already Reported"))
val IMUsed = register(Status(226, "IM Used"))

val MultipleChoices = register(Status(300, "Multiple Choices"))
val MovedPermanently = register(Status(301, "Moved Permanently"))
val Found = register(Status(302, "Found"))
val SeeOther = register(Status(303, "See Other"))
val NotModified = register(Status(304, "Not Modified", isEntityAllowed = false))
val UseProxy = register(Status(305, "Use Proxy"))
val TemporaryRedirect = register(Status(307, "Temporary Redirect"))
val PermanentRedirect = register(Status(308, "Permanent Redirect"))

val BadRequest = register(Status(400, "Bad Request"))
val Unauthorized = register(Status(401, "Unauthorized"))
val PaymentRequired = register(Status(402, "Payment Required"))
val Forbidden = register(Status(403, "Forbidden"))
val NotFound = register(Status(404, "Not Found"))
val MethodNotAllowed = register(Status(405, "Method Not Allowed"))
val NotAcceptable = register(Status(406, "Not Acceptable"))
val ProxyAuthenticationRequired = register(Status(407, "Proxy Authentication Required"))
val RequestTimeout = register(Status(408, "Request Timeout"))
val Conflict = register(Status(409, "Conflict"))
val Gone = register(Status(410, "Gone"))
val LengthRequired = register(Status(411, "Length Required"))
val PreconditionFailed = register(Status(412, "Precondition Failed"))
val PayloadTooLarge = register(Status(413, "Payload Too Large"))
val UriTooLong = register(Status(414, "URI Too Long"))
val UnsupportedMediaType = register(Status(415, "Unsupported Media Type"))
val RangeNotSatisfiable = register(Status(416, "Range Not Satisfiable"))
val ExpectationFailed = register(Status(417, "Expectation Failed"))
val UnprocessableEntity = register(Status(422, "Unprocessable Entity"))
val Locked = register(Status(423, "Locked"))
val FailedDependency = register(Status(424, "Failed Dependency"))
val UpgradeRequired = register(Status(426, "Upgrade Required"))
val PreconditionRequired = register(Status(428, "Precondition Required"))
val TooManyRequests = register(Status(429, "Too Many Requests"))
val RequestHeaderFieldsTooLarge = register(Status(431, "Request Header Fields Too Large"))
val UnavailableForLegalReasons = register(Status(451, "Unavailable For Legal Reasons"))

val InternalServerError = register(Status(500, "Internal Server Error"))
val NotImplemented = register(Status(501, "Not Implemented"))
val BadGateway = register(Status(502, "Bad Gateway"))
val ServiceUnavailable = register(Status(503, "Service Unavailable"))
val GatewayTimeout = register(Status(504, "Gateway Timeout"))
val HttpVersionNotSupported = register(Status(505, "HTTP Version not supported"))
val VariantAlsoNegotiates = register(Status(506, "Variant Also Negotiates"))
val InsufficientStorage = register(Status(507, "Insufficient Storage"))
val LoopDetected = register(Status(508, "Loop Detected"))
val NotExtended = register(Status(510, "Not Extended"))
val NetworkAuthenticationRequired = register(Status(511, "Network Authentication Required"))
val Continue: Status = register(Status(100, "Continue", isEntityAllowed = false))
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't hurt the singleton type tricks we use in the DSL? I'm trying to think why these might not have been ascribed before. If it didn't break things, 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears to be alright, so I guess we're lucky.

val SwitchingProtocols: Status = register(
Status(101, "Switching Protocols", isEntityAllowed = false))
val Processing: Status = register(Status(102, "Processing", isEntityAllowed = false))

val Ok: Status = register(Status(200, "OK"))
val Created: Status = register(Status(201, "Created"))
val Accepted: Status = register(Status(202, "Accepted"))
val NonAuthoritativeInformation: Status = register(Status(203, "Non-Authoritative Information"))
val NoContent: Status = register(Status(204, "No Content", isEntityAllowed = false))
val ResetContent: Status = register(Status(205, "Reset Content", isEntityAllowed = false))
val PartialContent: Status = register(Status(206, "Partial Content"))
val MultiStatus: Status = register(Status(207, "Multi-Status"))
val AlreadyReported: Status = register(Status(208, "Already Reported"))
val IMUsed: Status = register(Status(226, "IM Used"))

val MultipleChoices: Status = register(Status(300, "Multiple Choices"))
val MovedPermanently: Status = register(Status(301, "Moved Permanently"))
val Found: Status = register(Status(302, "Found"))
val SeeOther: Status = register(Status(303, "See Other"))
val NotModified: Status = register(Status(304, "Not Modified", isEntityAllowed = false))
val UseProxy: Status = register(Status(305, "Use Proxy"))
val TemporaryRedirect: Status = register(Status(307, "Temporary Redirect"))
val PermanentRedirect: Status = register(Status(308, "Permanent Redirect"))

val BadRequest: Status = register(Status(400, "Bad Request"))
val Unauthorized: Status = register(Status(401, "Unauthorized"))
val PaymentRequired: Status = register(Status(402, "Payment Required"))
val Forbidden: Status = register(Status(403, "Forbidden"))
val NotFound: Status = register(Status(404, "Not Found"))
val MethodNotAllowed: Status = register(Status(405, "Method Not Allowed"))
val NotAcceptable: Status = register(Status(406, "Not Acceptable"))
val ProxyAuthenticationRequired: Status = register(Status(407, "Proxy Authentication Required"))
val RequestTimeout: Status = register(Status(408, "Request Timeout"))
val Conflict: Status = register(Status(409, "Conflict"))
val Gone: Status = register(Status(410, "Gone"))
val LengthRequired: Status = register(Status(411, "Length Required"))
val PreconditionFailed: Status = register(Status(412, "Precondition Failed"))
val PayloadTooLarge: Status = register(Status(413, "Payload Too Large"))
val UriTooLong: Status = register(Status(414, "URI Too Long"))
val UnsupportedMediaType: Status = register(Status(415, "Unsupported Media Type"))
val RangeNotSatisfiable: Status = register(Status(416, "Range Not Satisfiable"))
val ExpectationFailed: Status = register(Status(417, "Expectation Failed"))
val UnprocessableEntity: Status = register(Status(422, "Unprocessable Entity"))
val Locked: Status = register(Status(423, "Locked"))
val FailedDependency: Status = register(Status(424, "Failed Dependency"))
val UpgradeRequired: Status = register(Status(426, "Upgrade Required"))
val PreconditionRequired: Status = register(Status(428, "Precondition Required"))
val TooManyRequests: Status = register(Status(429, "Too Many Requests"))
val RequestHeaderFieldsTooLarge: Status = register(Status(431, "Request Header Fields Too Large"))
val UnavailableForLegalReasons: Status = register(Status(451, "Unavailable For Legal Reasons"))

val InternalServerError: Status = register(Status(500, "Internal Server Error"))
val NotImplemented: Status = register(Status(501, "Not Implemented"))
val BadGateway: Status = register(Status(502, "Bad Gateway"))
val ServiceUnavailable: Status = register(Status(503, "Service Unavailable"))
val GatewayTimeout: Status = register(Status(504, "Gateway Timeout"))
val HttpVersionNotSupported: Status = register(Status(505, "HTTP Version not supported"))
val VariantAlsoNegotiates: Status = register(Status(506, "Variant Also Negotiates"))
val InsufficientStorage: Status = register(Status(507, "Insufficient Storage"))
val LoopDetected: Status = register(Status(508, "Loop Detected"))
val NotExtended: Status = register(Status(510, "Not Extended"))
val NetworkAuthenticationRequired: Status = register(
Status(511, "Network Authentication Required"))
// scalastyle:on magic.number

}

trait StatusInstances {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ trait ArbitraryInstances {
Cogen[Int].contramap(_.##)

val genValidStatusCode =
choose(100, 599)
oneOf(Status.registered.map(_.code).toSeq)

val genStandardStatus =
oneOf(Status.registered.toSeq)
Expand Down
98 changes: 75 additions & 23 deletions tests/src/test/scala/org/http4s/StatusSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,42 +7,94 @@ import org.scalacheck.Gen
import org.scalacheck.Prop.forAll

class StatusSpec extends Http4sSpec {
"code is valid iff between 100 and 599" in {
forAll(Gen.choose(Int.MinValue, 99)) { i =>
fromInt(i).isLeft

checkAll("Status", OrderTests[Status].order)

"Statuses" should {
"not be equal if their codes are not" in {
prop { (s1: Status, s2: Status) =>
(s1.code != s2.code) ==> (s1 != s2)
}
}

"be equal if their codes are" in {
forAll(genValidStatusCode) { i =>
val s1: Status = getStatus(i, "a reason")
val s2: Status = getStatus(i, "another reason")
s1 must_== s2
}
}
foreach(100 to 599) { i =>
fromInt(i).isRight

"be ordered by their codes" in {
prop { (s1: Status, s2: Status) =>
(s1.code < s2.code) ==> (s1 < s2)
}
}
forAll(Gen.choose(600, Int.MaxValue)) { i =>
fromInt(i).isLeft

"have the appropriate response classes" in {
forAll(genValidStatusCode) { code =>
val expected = code / 100 match {
case 1 => Informational
case 2 => Successful
case 3 => Redirection
case 4 => ClientError
case 5 => ServerError
}
fromInt(code) must beRight.like { case s => s.responseClass must_== expected }
}
}

}

def responseClassTest(range: Range, responseClass: ResponseClass) =
foreach(range) { i =>
fromInt(i) must beRight.like {
case s => s.responseClass must_== responseClass
"Finding a status by code" should {

"fail if the code is not in the range of valid codes" in {
forAll(Gen.choose(Int.MinValue, 99)) { i =>
fromInt(i).isLeft
}
}

"codes from 100 to 199 are informational" in responseClassTest(100 to 199, Informational)
"codes from 200 to 299 are successful" in responseClassTest(200 to 299, Successful)
"codes from 300 to 399 are redirectional" in responseClassTest(300 to 399, Redirection)
"codes from 400 to 499 are client errors" in responseClassTest(400 to 499, ClientError)
"codes from 500 to 599 are server errors" in responseClassTest(500 to 599, ServerError)
"fail if the code is not in the range of valid codes" in {
forAll(Gen.choose(599, Int.MaxValue)) { i =>
fromInt(i).isLeft
}
}

checkAll("Status", OrderTests[Status].order)
"fail if the code is in the valid range, but not a standard code" in {
fromInt(371) must beLeft
fromInt(482) must beLeft
}

"status is not equal if code is not equal" in {
prop { (s1: Status, s2: Status) =>
(s1.code != s2.code) ==> (s1 != s2)
"yield a status with the standard reason for a standard code" in {
getStatus(NotFound.code).reason must_== "Not Found"
}
}

"status is equal if reason is equal" in {
prop { (s1: Status, reason: String) =>
(s1.reason != reason) ==> (s1 == s1.withReason(reason))
"Finding a status by code and reason" should {
"fail if the code is not in the valid range" in {
fromIntAndReason(17, "a reason") must beLeft
}

"fail if the code is in the valid range, but not a standard code" in {
fromIntAndReason(371, "some reason") must beLeft
fromIntAndReason(152, "another reason") must beLeft
}

"succeed for a standard code and nonstandard reason, without replacing the default reason" in {
getStatus(NotFound.code, "My dog ate my homework").reason must_== "My dog ate my homework"

getStatus(NotFound.code).reason must_== "Not Found"
}

"succeed for a standard code and reason" in {
getStatus(NotFound.code, "Not Found").reason must_== "Not Found"
}
}

private def getStatus(code: Int) =
fromInt(code).right.get

private def getStatus(code: Int, reason: String) =
fromIntAndReason(code, reason).right.get

}