Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am writing a small oauth2 library for Play! 2.1 scala. As I am still learning I got stuck trying to TDD and ended up writing the code first, then refactoring for testability. By testability I mean two things:

  1. make the code testable by me as the author
  2. make it easy for users (since I will be a user) to stub/mock it out.

The first working version of the code looked like (complete file on github):

object GoogleOAuth2 extends OAuth2(GoogleOAuth2Settings) {

    lazy val signIn = s"${settings.signInUrl}?client_id=${settings.clientId}&redirect_uri=${settings.redirectUri}"+
      "&response_type=code&approval_prompt=force&hd=xebia.fr"+    
      "&scope=https://www.googleapis.com/auth/userinfo.email+https://www.googleapis.com/auth/userinfo.profile"

    def requestAccessTockenWS(code: String): Future[Response] =    
      WS.url(settings.accessTokenUrl).post(
        Map("client_id" -> Seq(settings.clientId)
          , "client_secret" -> Seq(settings.clientSecret)
          , "code" -> Seq(code)
          , "redirect_uri" -> Seq(settings.redirectUri)
          , "grant_type" -> Seq("authorization_code")))
}

The requestAccessTockensWS method depends on the WS API in Play2! This API is exposed as a scala object whith methods which create builders. These builders allow you to compose and call HTTP requests. In a normal setting I might choose not to test requestAccessTockenWS since it is a very small method with almost no logic of its own. However it is a nice "simple" case of dependecy on a scala object for exploring design patterns which enable tests.

Here is the testcase I ended up writing (complete on github):

  "GoogleOAuth2" should {
    import play.api.libs.ws.WS
    import play.api.libs.ws.Response
    object TestSettings extends GoogleOAuth2Settings {
      override lazy val clientId = testConfig("oauth2.google.cliend_id")
      override lazy val clientSecret = testConfig("oauth2.google.cliend_secret")
      override lazy val signInUrl = testConfig("oauth2.google.signInUrl")
      override lazy val accessTokenUrl = testConfig("oauth2.google.accessTokenUrl")
      override lazy val userInfoUrl = testConfig("oauth2.google.userInfoUrl")
      override lazy val redirectUri = testConfig("oauth2.google.redirect_uri")
    }
    class MockedGoogleOauth2 extends GoogleOAuth2(TestSettings) {
      override def url = testUrl _

      lazy val requestHolder: WS.WSRequestHolder = mock[WS.WSRequestHolder]

      def testUrl(s: String): WS.WSRequestHolder = {
        requestHolder.url returns s
        requestHolder
      }
    }
    "requestAccessTokens" in {
      import concurrent._
      import ExecutionContext.Implicits.global
      import concurrent.duration._
      import play.api.http.{ Writeable, ContentTypeOf }
      import org.mockito.Matchers.{ eq => mEq }
      import org.mockito.Matchers
      val underTest = new MockedGoogleOauth2()
      val mockWs = underTest.requestHolder
      val fakeResponse = mock[Response]
      val expected = Map("client_id" -> Seq("client_id"), "client_secret" -> Seq("client_secret"), "code" -> Seq("code"), "redirect_uri" -> Seq("http://redirect"), "grant_type" -> Seq("authorization_code"))
      type Params = Map[String, Seq[String]]
      mockWs.post[Params](mEq(expected))(Matchers.any[Writeable[Params]], Matchers.any[ContentTypeOf[Params]]) returns Future.successful(fakeResponse)

      //when
      val willBeResponse = underTest.requestAccessTockenWS("code")

      there was one(mockWs).post(mEq(expected))(any, any)
      Await.result(willBeResponse, 5 milli) must beEqualTo(fakeResponse)

    }
  }

and the corresponding refactored code (complete on github):

abstract class GoogleOAuth2[U <: GoogleOAuth2Settings](override val settings: U) extends OAuth2(settings) {

    lazy val signIn = s"${settings.signInUrl}?client_id=${settings.clientId}&redirect_uri=${settings.redirectUri}"+
      "&response_type=code&approval_prompt=force&hd=xebia.fr"+
      "&scope=https://www.googleapis.com/auth/userinfo.email+https://www.googleapis.com/auth/userinfo.profile"

    def url = WS.url _

    def requestAccessTockenWS(code: String): Future[Response] = {
      val u = url(settings.accessTokenUrl)
      val params = Map("client_id" -> Seq(settings.clientId)
          , "client_secret" -> Seq(settings.clientSecret)
          , "code" -> Seq(code)
          , "redirect_uri" -> Seq(settings.redirectUri)
          , "grant_type" -> Seq("authorization_code"))
      u.post(params)
    }
  }

  object GoogleOAuth2 extends GoogleOAuth2(GoogleOAuth2Settings)

The code and the tests work, and the client can use the same technique to mock it out. How can I improve on this?

share|improve this question

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.