Minor refactoring.

commerce/src/main/scala/com/ticketmaster/api/commerce/HttpCommerceApi.scala

@@ -12,7 +12,7 @@ import scala.concurrent.{ExecutionContext, Future}
 trait HttpCommerceApi extends CommerceApi with HttpHandler {
   val ROOT_URL = "https://app.ticketmaster.com/commerce/v2"
 
-  val USER_AGENT = "Ticketmaster Commerce Scala"
+  val userAgent = "Ticketmaster Commerce Scala"
 
   override def getEventOffers(getEventOffersRequest: GetEventOffersRequest)(implicit ec: ExecutionContext): Future[Response[EventOffers]] = {
     val req = HttpRequest(root = ROOT_URL) / "events" / getEventOffersRequest.id / "offers.json"

core/src/main/scala/com/ticketmaster/api/http/HttpHandler.scala

@@ -9,13 +9,14 @@ import com.ticketmaster.api.Api._
 import com.ticketmaster.api.http.protocol.{HttpRequest, HttpResponse}
 
 import scala.concurrent.ExecutionContext
-import scalaz._
+import scala.util.{Failure, Success, Try}
+import scalaz.{-\/, \/, \/-}
 
 
 trait HttpHandler {
   val apiKey: String
 
-  val USER_AGENT: String
+  val userAgent: String
 
   val okRange = 200 to 299
 
@@ -26,34 +27,65 @@ trait HttpHandler {
   def handleRequest[B, R](req: HttpRequest, handler: ResponseHandler[B, R])(implicit ec: ExecutionContext, decode: DecodeJson[B]) = {
     http(req
       .addQueryParameter("apikey", apiKey)
-      .addHeader("User-Agent", s"${USER_AGENT}/${build.Info.version}")
+      .addHeader("User-Agent", s"${userAgent}/${build.Info.version}")
     ).map(res => handleResponse(res, handler))
   }
 
   private def handleResponse[B, R](response: HttpResponse, handler: ResponseHandler[B, R])(implicit decode: DecodeJson[B]): R = {
-    def errors = decodeEither[Errors](response.body.get).map(_.toString).getOrElse(s"Failed to decode body: ${response.body.get}")
+    def eitherBody = response.body.fold[String \/ String](-\/("No response body"))(b => \/-(b))
+
+    def errorMsg(str: String) = decodeBody[Errors](str)//.map(_.toString).getOrElse(s"Failed to decode body: ${response.body.get}")
 
     response.status match {
       case status if okRange contains status => {
         val parts = for {
-          decoded <- decodeEither[B](response.body.get)
-          rateLimits <- extractRateLimitInfo(response)
-        } yield (decoded, rateLimits)
+          body <- eitherBody
+          decodedBody <- decodeBody[B](body)
+          rateLimits <- extractRateLimits(response)
+        } yield(decodedBody, rateLimits)
+
+        val (body, rateLimits) = parts.valueOr(msg => throw new ApiException(msg))
+        handler(body, rateLimits)
+      }
+      case 404 => {
+        val parts = for {
+          body <- eitherBody
+          decoded <- errorMsg(body)
+        } yield(decoded.toString)
+
+        throw new ResourceNotFoundException(parts.valueOr(identity))
+      }
+      case _ => {
+        val parts = for {
+          body <- eitherBody
+          decoded <- errorMsg(body)
+        } yield(decoded.toString)
 
-        parts.map(right => handler(right._1, right._2)) | (throw new ApiException("Failed to read response"))
+        throw new ApiException(parts.valueOr(identity))
       }
-      case 404 => throw new ResourceNotFoundException(errors)
-      case _ => throw new ApiException(errors)
     }
   }
 
-  private def extractRateLimitInfo(response: HttpResponse) = {
-    \/-(RateLimits(
-      response.headers("Rate-Limit").toInt,
-      response.headers("Rate-Limit-Available").toInt,
-      response.headers("Rate-Limit-Over").toInt,
-      ZonedDateTime.ofInstant(Instant.ofEpochMilli(response.headers("Rate-Limit-Reset").toLong), ZoneId.of("UTC"))))
-  }
+  private def decodeBody[T](json: String)(implicit decode: DecodeJson[T]): \/[String, T] = Parse.decodeEither[T](json)
+
+  private def extractRateLimits(response: HttpResponse) = {
+    def extract[T](t: => T) = {
+      Try(t) match {
+        case Success(i) => \/-(i)
+        case Failure(e) => -\/("Missing rate limit")
+      }
+    }
 
-  private def decodeEither[T](json: String)(implicit decode: DecodeJson[T]): \/[String, T] = Parse.decodeEither[T](json)
+    for {
+      rateLimit <- extract(response.headers("Rate-Limit").toInt)
+      rateLimitAvailable <- extract(response.headers("Rate-Limit-Available").toInt)
+      rateLimitOver <- extract(response.headers("Rate-Limit-Over").toInt)
+      rateLimitReset <- extract(response.headers("Rate-Limit-Reset").toLong)
+    } yield {
+      RateLimits(rateLimit,
+        rateLimitAvailable,
+        rateLimitOver,
+        ZonedDateTime.ofInstant(Instant.ofEpochMilli(rateLimitReset), ZoneId.of("UTC")))
+    }
+  }
 }

core/src/main/scala/com/ticketmaster/api/test/BaseSpec.scala

@@ -1,11 +1,14 @@
 package com.ticketmaster.api.test
 
 import org.scalamock.scalatest.MockFactory
-import org.scalatest.{FlatSpec, Matchers, Suite}
 import org.scalatest.concurrent.ScalaFutures
+import org.scalatest.{FlatSpec, Matchers, Suite}
 
+import scala.concurrent.duration._
 import scala.language.implicitConversions
 
 trait BaseSpec extends FlatSpec with Matchers with ScalaFutures with MockFactory {
   this: Suite =>
+
+  override implicit val patienceConfig = PatienceConfig(2 seconds, 200 millis)
 }

core/src/test/scala/com/ticketmaster/api/http/HttpHandlerTest.scala

@@ -0,0 +1,154 @@
+package com.ticketmaster.api.http
+
+import java.time.ZonedDateTime
+
+import argonaut.Shapeless._
+import com.ticketmaster.api.Api._
+import com.ticketmaster.api.http.protocol.{HttpRequest, HttpResponse}
+import com.ticketmaster.api.test.BaseSpec
+import org.scalamock.scalatest.MockFactory
+import org.scalatest.Suite
+
+import scala.concurrent.ExecutionContext.Implicits.global
+import scala.concurrent.{ExecutionContext, Future}
+
+
+trait TestableHttpHandler extends MockFactory {
+  this: Suite =>
+
+  def newHandler(expectedRequest: HttpRequest, response: HttpResponse) = {
+    new HttpHandler {
+      override val apiKey: String = "12345"
+      override val userAgent: String = "Ticketmaster Test Scala"
+
+      override def http: Http = {
+        val mockHttp = mock[Http]
+        (mockHttp.apply(_: HttpRequest)(_: ExecutionContext))
+          .expects(expectedRequest, *)
+          .returning(Future.successful(response))
+        mockHttp
+      }
+    }
+  }
+}
+
+case class Body(name: String)
+
+case class SomeResponse(body: Body, rateLimits: RateLimits)
+
+class HttpHandlerTest extends BaseSpec with TestableHttpHandler {
+
+  val responseHeaders = Map("Rate-Limit" -> "5000",
+    "Rate-Limit-Available" -> "5000",
+    "Rate-Limit-Over" -> "0",
+    "Rate-Limit-Reset" -> "1453180594367")
+
+  behavior of "http handler"
+
+  it should "handle valid request" in {
+    val expectedHttpRequest = HttpRequest("https://app.ticketmaster.com/", queryParams = Map("apikey" -> "12345"))
+      .addHeader(s"User-Agent", s"Ticketmaster Test Scala/${build.Info.version}")
+    val httpResponse = HttpResponse(200, body = Some("""{"name" : "max"}"""), headers = responseHeaders)
+
+    val handler = newHandler(expectedHttpRequest, httpResponse)
+
+    val actualHttpRequest = HttpRequest("https://app.ticketmaster.com/")
+    val responseHandler = (body: Body, rateLimits: RateLimits) => SomeResponse(body, rateLimits)
+    val pendingResponse = handler.handleRequest[Body, SomeResponse](actualHttpRequest, responseHandler)
+
+    whenReady(pendingResponse) { r =>
+      r.body should be(Body("max"))
+      r.rateLimits should be(RateLimits(5000, 5000, 0, ZonedDateTime.parse("2016-01-19T05:16:34.367Z[UTC]")))
+    }
+  }
+
+  it should "handle missing rate limits response headers" in {
+    val expectedHttpRequest = HttpRequest("https://app.ticketmaster.com/", queryParams = Map("apikey" -> "12345"))
+      .addHeader(s"User-Agent", s"Ticketmaster Test Scala/${build.Info.version}")
+    val httpResponse = HttpResponse(200, body = Some("""{"name" : "max"}"""))
+
+    val handler = newHandler(expectedHttpRequest, httpResponse)
+
+    val actualHttpRequest = HttpRequest("https://app.ticketmaster.com/")
+    val responseHandler = (body: Body, rateLimits: RateLimits) => SomeResponse(body, rateLimits)
+
+    val pendingResponse = handler.handleRequest[Body, SomeResponse](actualHttpRequest, responseHandler)
+
+    whenReady(pendingResponse.failed) { t =>
+      t shouldBe a[ApiException]
+      t.getMessage should be("Missing rate limit")
+    }
+  }
+
+  it should "handle missing json response body" in {
+    val expectedHttpRequest = HttpRequest("https://app.ticketmaster.com/", queryParams = Map("apikey" -> "12345"))
+      .addHeader(s"User-Agent", s"Ticketmaster Test Scala/${build.Info.version}")
+    val httpResponse = HttpResponse(200)
+
+    val handler = newHandler(expectedHttpRequest, httpResponse)
+
+    val actualHttpRequest = HttpRequest("https://app.ticketmaster.com/")
+    val responseHandler = (body: Body, rateLimits: RateLimits) => SomeResponse(body, rateLimits)
+
+    val pendingResponse = handler.handleRequest[Body, SomeResponse](actualHttpRequest, responseHandler)
+
+    whenReady(pendingResponse.failed) { t =>
+      t shouldBe a[ApiException]
+      t.getMessage should be("No response body")
+    }
+  }
+
+  it should "handle invalid json response body" in {
+    val expectedHttpRequest = HttpRequest("https://app.ticketmaster.com/", queryParams = Map("apikey" -> "12345"))
+      .addHeader(s"User-Agent", s"Ticketmaster Test Scala/${build.Info.version}")
+    val httpResponse = HttpResponse(200, body = Some("""{"person" : "max"}"""))
+
+    val handler = newHandler(expectedHttpRequest, httpResponse)
+
+    val actualHttpRequest = HttpRequest("https://app.ticketmaster.com/")
+    val responseHandler = (body: Body, rateLimits: RateLimits) => SomeResponse(body, rateLimits)
+
+    val pendingResponse = handler.handleRequest[Body, SomeResponse](actualHttpRequest, responseHandler)
+
+    whenReady(pendingResponse.failed) { t =>
+      t shouldBe a[ApiException]
+      t.getMessage should be("Attempt to decode value on failed cursor.: [*.--\\(name)]")
+    }
+  }
+
+  it should "handle 404 response" in {
+    val expectedHttpRequest = HttpRequest("https://app.ticketmaster.com/", queryParams = Map("apikey" -> "12345"))
+      .addHeader(s"User-Agent", s"Ticketmaster Test Scala/${build.Info.version}")
+    val httpResponse = HttpResponse(404, body = Some(HttpHandlerTest.error404))
+
+    val handler = newHandler(expectedHttpRequest, httpResponse)
+
+    val actualHttpRequest = HttpRequest("https://app.ticketmaster.com/")
+    val responseHandler = (body: Body, rateLimits: RateLimits) => SomeResponse(body, rateLimits)
+
+    val pendingResponse = handler.handleRequest[Body, SomeResponse](actualHttpRequest, responseHandler)
+
+    whenReady(pendingResponse.failed) { t =>
+      t shouldBe a[ResourceNotFoundException]
+      t.getMessage should be("Errors(Vector(Error(ABC123,Resource not found with provided criteria,404)))")
+    }
+  }
+}
+
+object HttpHandlerTest {
+  val error404 =
+    """
+      |{
+      |	"errors": [{
+      |		"code": "ABC123",
+      |		"detail": "Resource not found with provided criteria",
+      |		"status": "404",
+      |		"_links": {
+      |			"about": {
+      |				"href": "/errors.html#ABC123"
+      |			}
+      |		}
+      |	}]
+      |}
+    """.stripMargin
+}

discovery/src/main/scala/com/ticketmaster/api/discovery/HttpDiscoveryApi.scala

@@ -12,7 +12,7 @@ import scala.concurrent.{ExecutionContext, Future}
 trait HttpDiscoveryApi extends DiscoveryApi with HttpHandler {
   val ROOT_URL = "https://app.ticketmaster.com/discovery/v2"
 
-  val USER_AGENT = "Ticketmaster Discovery Scala"
+  val userAgent = "Ticketmaster Discovery Scala"
 
   override def searchEvents(searchEventsRequest: SearchEventsRequest)(implicit ec: ExecutionContext): Future[PageResponse[Events]] = {
     val filters = Map[String, Filter[_]]()

discovery/src/test/scala/com/ticketmaster/api/discovery/AttractionsSpec.scala

@@ -1,19 +1,15 @@
 package com.ticketmaster.api.discovery
 
-import java.time.ZonedDateTime
-
 import com.ticketmaster.api.Api._
 import com.ticketmaster.api.discovery.domain._
 import com.ticketmaster.api.http.protocol.{HttpRequest, HttpResponse}
 import com.ticketmaster.api.test.BaseSpec
 
 import scala.concurrent.ExecutionContext.Implicits.global
 import scala.concurrent.Future
-import scala.concurrent.duration._
 
-class AttractionsSpec extends BaseSpec with TestableDiscoveryApi {
 
-  override implicit val patienceConfig = PatienceConfig(2 seconds, 200 millis)
+class AttractionsSpec extends BaseSpec with TestableDiscoveryApi {
 
   val testApiKey = "12345"
 
@@ -36,7 +32,6 @@ class AttractionsSpec extends BaseSpec with TestableDiscoveryApi {
       r.pageResult._embedded.attractions.head.name should be("Coachella Valley Music and Arts Festival")
       r.pageResult.page should be(Page(20, 1, 1, 0))
       r.pageResult._links.self should be(Link("/discovery/v2/attractions.json{?page,size,sort}", Some(true)))
-      r.rateLimits should be(RateLimits(5000, 5000, 0, ZonedDateTime.parse("2016-01-19T05:16:34.367Z[UTC]")))
     }
   }
 

discovery/src/test/scala/com/ticketmaster/api/discovery/ClassificationsSpec.scala

@@ -1,19 +1,15 @@
 package com.ticketmaster.api.discovery
 
-import java.time.ZonedDateTime
-
 import com.ticketmaster.api.Api._
 import com.ticketmaster.api.discovery.domain._
 import com.ticketmaster.api.http.protocol.{HttpRequest, HttpResponse}
 import com.ticketmaster.api.test.BaseSpec
 
 import scala.concurrent.ExecutionContext.Implicits.global
 import scala.concurrent.Future
-import scala.concurrent.duration._
 
-class ClassificationsSpec extends BaseSpec with TestableDiscoveryApi {
 
-  override implicit val patienceConfig = PatienceConfig(2 seconds, 200 millis)
+class ClassificationsSpec extends BaseSpec with TestableDiscoveryApi {
 
   val testApiKey = "12345"
 
@@ -36,7 +32,6 @@ class ClassificationsSpec extends BaseSpec with TestableDiscoveryApi {
       r.pageResult._embedded.classifications.head.segment.name should be("Arts & Theatre")
       r.pageResult.page should be(Page(20, 6, 1, 0))
       r.pageResult._links.self should be(Link("/discovery/v2/classifications.json{?page,size,sort}", Some(true)))
-      r.rateLimits should be(RateLimits(5000, 5000, 0, ZonedDateTime.parse("2016-01-19T05:16:34.367Z[UTC]")))
     }
   }
 

discovery/src/test/scala/com/ticketmaster/api/discovery/EventsSpec.scala

@@ -9,11 +9,9 @@ import com.ticketmaster.api.test.BaseSpec
 
 import scala.concurrent.ExecutionContext.Implicits.global
 import scala.concurrent.Future
-import scala.concurrent.duration._
 
 
 class EventsSpec extends BaseSpec with TestableDiscoveryApi {
-  override implicit val patienceConfig = PatienceConfig(2 seconds, 200 millis)
 
   val testApiKey = "12345"
 
@@ -36,7 +34,6 @@ class EventsSpec extends BaseSpec with TestableDiscoveryApi {
       r.pageResult._embedded.events.head.name should be("The Fearless Freakcast Presents: Live From Coachella Music Festival")
       r.pageResult.page should be(Page(20, 1, 1, 0))
       r.pageResult._links.self should be(Link("/discovery/v2/events.json{?page,size,sort}", Some(true)))
-      r.rateLimits should be(RateLimits(5000, 5000, 0, ZonedDateTime.parse("2016-01-19T05:16:34.367Z[UTC]")))
     }
   }
 
@@ -53,7 +50,6 @@ class EventsSpec extends BaseSpec with TestableDiscoveryApi {
       r.pageResult._embedded.events.head.name should be("The Fearless Freakcast Presents: Live From Coachella Music Festival")
       r.pageResult.page should be(Page(20, 1, 1, 0))
       r.pageResult._links.self should be(Link("/discovery/v2/events.json{?page,size,sort}", Some(true)))
-      r.rateLimits should be(RateLimits(5000, 5000, 0, ZonedDateTime.parse("2016-01-19T05:16:34.367Z[UTC]")))
     }
   }