From 3544d523d785a29c67c6ff8e5af16525458cf751 Mon Sep 17 00:00:00 2001
From: Piotr Sarnacki <drogus@gmail.com>
Date: Wed, 21 May 2014 14:24:44 +0200
Subject: [PATCH] Refactor auth code

* don't use __container__
* register it as a factory on container and inject into routes and
  controller
* avoid afterSignIn errors by checking on hooksTarget, ApplicationRoute
  should register itself as a hooksTarget
* keep user info on auth
---
 assets/scripts/app/app.coffee                 | 20 ------
 assets/scripts/app/auth.coffee                | 63 ++++++++++++-------
 assets/scripts/app/controllers.coffee         | 12 ----
 .../app/controllers/current_user.coffee       |  6 +-
 assets/scripts/app/routes.coffee              | 29 +++++----
 assets/scripts/app/templates/auth/signin.hbs  |  4 +-
 assets/scripts/app/templates/layouts/top.hbs  | 10 +--
 assets/scripts/app/views/signin.coffee        |  4 --
 assets/scripts/app/views/top.coffee           |  4 +-
 assets/scripts/spec/spec_helper.coffee        |  8 ++-
 10 files changed, 76 insertions(+), 84 deletions(-)

diff --git a/assets/scripts/app/app.coffee b/assets/scripts/app/app.coffee
index 20002ac0..c4608eb1 100644
--- a/assets/scripts/app/app.coffee
+++ b/assets/scripts/app/app.coffee
@@ -1,8 +1,6 @@
 unless window.TravisApplication
   window.TravisApplication = Em.Application.extend(Ember.Evented,
     LOG_TRANSITIONS: true,
-    authState: Ember.computed.alias('auth.state')
-    signedIn: (-> @get('authState') == 'signed-in' ).property('authState')
 
     mappings: (->
       broadcasts:   Travis.Broadcast
@@ -40,8 +38,6 @@ unless window.TravisApplication
       @tailing = new Travis.Tailing($(window), '#tail', '#log')
       @toTop   = new Travis.ToTop($(window), '.to-top', '#log-container')
 
-      @set('auth', Travis.Auth.create(app: this, endpoint: Travis.config.api_endpoint))
-
     reset: ->
       @_super.apply(this, arguments)
       @get('modelClasses').forEach (klass) ->
@@ -54,22 +50,6 @@ unless window.TravisApplication
     flash: (options) ->
       Travis.lookup('controller:flash').loadFlashes([options])
 
-    storeAfterSignInPath: (path) ->
-      @get('auth').storeAfterSignInPath(path)
-
-    autoSignIn: (path) ->
-      @get('auth').autoSignIn()
-
-    signIn: ->
-      @get('auth').signIn()
-
-    signOut: ->
-      @get('auth').signOut()
-
-    signingIn: (->
-      Travis.get('authState') == 'signing-in'
-    ).property('authState')
-
     receive: (event, data) ->
       [name, type] = event.split(':')
 
diff --git a/assets/scripts/app/auth.coffee b/assets/scripts/app/auth.coffee
index 92ecbb25..364bc34b 100644
--- a/assets/scripts/app/auth.coffee
+++ b/assets/scripts/app/auth.coffee
@@ -1,32 +1,32 @@
-@Travis.Auth = Ember.Object.extend
+Auth = Ember.Object.extend
   state:        "signed-out"
   receivingEnd: "#{location.protocol}//#{location.host}"
 
   init: ->
     window.addEventListener('message', (e) => @receiveMessage(e))
 
+  endpoint: (->
+    @container.lookup('application:main').config.api_endpoint
+  ).property(),
+
   signOut: ->
     Travis.storage.removeItem('travis.user')
     Travis.storage.removeItem('travis.token')
     Travis.sessionStorage.clear()
     @set('state', 'signed-out')
     @set('user', undefined)
-    if user = Travis.__container__.lookup('controller:currentUser').get('content')
+    if user = @get('currentUser')
       user.unload()
-    Travis.__container__.lookup('controller:currentUser').set('content', null)
-    if controller = Travis.__container__.lookup('controller:currentUser')
-      try
-        controller.send('afterSignOut')
-      catch e
-        throw e unless e.message =~ /There are no active handlers/
-
+    @set('currentUser', null)
+    if hooksTarget = @get('hooksTarget')
+      hooksTarget.afterSignOut()
 
   signIn: (data) ->
     if data
       @autoSignIn(data)
     else
       @set('state', 'signing-in')
-      url = "#{@endpoint}/auth/post_message?origin=#{@receivingEnd}"
+      url = "#{@get('endpoint')}/auth/post_message?origin=#{@receivingEnd}"
       $('<iframe id="auth-frame" />').hide().appendTo('body').attr('src', url)
 
   autoSignIn: (data) ->
@@ -60,33 +60,36 @@
     @storeData(data, Travis.sessionStorage)
     @storeData(data, Travis.storage) unless @userDataFrom(Travis.storage)
     user = @loadUser(data.user)
-    # TODO: we should not use __container__ directly, how to do it better?
-    #        A good answer seems to do auth in context of controller.
-    Travis.__container__.lookup('controller:currentUser').set('content', user)
+    @set('currentUser', user)
 
     @set('state', 'signed-in')
     Travis.trigger('user:signed_in', data.user)
-    if controller = Travis.__container__.lookup('controller:currentUser')
-      Ember.run.next =>
-        try
-          controller.send('afterSignIn')
-        catch e
-          throw e unless e =~ /There are no active handlers/ || e =~ /Can't trigger action "afterSignIn/
-        @refreshUserData(data.user)
+    # TODO: I would like to get rid of this dependency in the future
+    if hooksTarget = @get('hooksTarget')
+      hooksTarget.afterSignIn()
 
   refreshUserData: (user) ->
     Travis.ajax.get "/users/#{user.id}", (data) =>
       Travis.loadOrMerge(Travis.User, data.user)
       # if user is still signed in, update saved data
-      if @signedIn()
+      if @get('signedIn')
         data.user.token = user.token
         @storeData(data, Travis.sessionStorage)
         @storeData(data, Travis.storage)
     , (data, status, xhr) =>
       @signOut() if xhr.status == 401
 
-  signedIn: ->
+  signedIn: (->
     @get('state') == 'signed-in'
+  ).property('state')
+
+  signedOut: (->
+    @get('state') == 'signed-out'
+  ).property('state')
+
+  signingIn: (->
+    @get('state') == 'signing-in'
+  ).property('state')
 
   storeData: (data, storage) ->
     storage.setItem('travis.token', data.token) if data.token
@@ -101,10 +104,22 @@
   receiveMessage: (event) ->
     if event.origin == @expectedOrigin()
       if event.data == 'redirect'
-        window.location = "#{@endpoint}/auth/handshake?redirect_uri=#{location}"
+        window.location = "#{@get('endpoint')}/auth/handshake?redirect_uri=#{location}"
       else if event.data.user?
         event.data.user.token = event.data.travis_token if event.data.travis_token
         @setData(event.data)
 
   expectedOrigin: ->
-    if @endpoint[0] == '/' then @receivingEnd else @endpoint
+    endpoint = @get('endpoint')
+    if endpoint[0] == '/' then @receivingEnd else endpoint
+
+Ember.onLoad 'Ember.Application', (Application) ->
+  Application.initializer
+    name: "auth",
+
+    initialize: (container, application) ->
+      application.register 'auth:main', Auth
+
+      application.inject('route', 'auth', 'auth:main')
+      application.inject('controller', 'auth', 'auth:main')
+      application.inject('application', 'auth', 'auth:main')
diff --git a/assets/scripts/app/controllers.coffee b/assets/scripts/app/controllers.coffee
index 14b33440..f5d9b3f4 100644
--- a/assets/scripts/app/controllers.coffee
+++ b/assets/scripts/app/controllers.coffee
@@ -13,18 +13,6 @@ Travis.TopController = Em.Controller.extend
     "#{location.protocol}//www.gravatar.com/avatar/#{@get('user.gravatarId')}?s=48&d=mm"
   ).property('user.gravatarId')
 
-  signedIn: (->
-    Travis.get('authState') == 'signed-in'
-  ).property('Travis.authState')
-
-  signedOut: (->
-    Travis.get('authState') == 'signed-out'
-  ).property('Travis.authState')
-
-  signingIn: (->
-    Travis.get('authState') == 'signing-in'
-  ).property('Travis.authState')
-
 Travis.ApplicationController = Em.Controller.extend
   templateName: 'layouts/home'
 
diff --git a/assets/scripts/app/controllers/current_user.coffee b/assets/scripts/app/controllers/current_user.coffee
index 7a29ca81..affccf9c 100644
--- a/assets/scripts/app/controllers/current_user.coffee
+++ b/assets/scripts/app/controllers/current_user.coffee
@@ -8,8 +8,12 @@ Travis.CurrentUserController = Em.ObjectController.extend
   sync: ->
     @get('content').sync()
 
+  content: (->
+    @get('auth.currentUser')
+  ).property('auth.currentUser')
+
   syncingDidChange: (->
     if (user = @get('content')) && user.get('isSyncing') && !user.get('syncedAt')
       Ember.run.scheduleOnce 'routerTransitions', this, ->
         @container.lookup('router:main').send('renderFirstSync')
-  ).observes('isSyncing', 'content')
\ No newline at end of file
+  ).observes('isSyncing', 'content')
diff --git a/assets/scripts/app/routes.coffee b/assets/scripts/app/routes.coffee
index 6ed126ba..4afe264a 100644
--- a/assets/scripts/app/routes.coffee
+++ b/assets/scripts/app/routes.coffee
@@ -12,10 +12,10 @@ Travis.Route = Ember.Route.extend
     @transitionTo 'first_sync'
 
   beforeModel: (transition) ->
-    Travis.autoSignIn() unless @signedIn()
+    @auth.autoSignIn() unless @signedIn()
 
     if !@signedIn() && @get('needsAuth')
-      Travis.auth.set('afterSignInTransition', transition)
+      @auth.set('afterSignInTransition', transition)
       Ember.RSVP.reject("needs-auth")
     else
       @_super.apply(this, arguments)
@@ -24,7 +24,7 @@ Travis.Route = Ember.Route.extend
     @controllerFor('currentUser').get('content')
 
   redirect: ->
-    Travis.autoSignIn() unless @signedIn()
+    @auth.autoSignIn() unless @signedIn()
 
     if @get('needsAuth')
       @authorize(@router.location.getURL())
@@ -33,10 +33,23 @@ Travis.Route = Ember.Route.extend
 
   authorize: (path) ->
     if !@signedIn()
-      Travis.storeAfterSignInPath(path)
+      @auth.storeAfterSignInPath(path)
       @transitionTo('auth')
 
 Travis.ApplicationRoute = Travis.Route.extend
+  init: ->
+    @_super.apply this, arguments
+
+    @auth.set('hooksTarget', this)
+
+  afterSignIn: ->
+    if transition = @auth.get('afterSignInTransition')
+      @auth.set('afterSignInTransition', null)
+      transition.retry()
+
+  afterSignOut: ->
+    @transitionTo('index.current')
+
   actions:
     redirectToGettingStarted: ->
       # do nothing, we handle it only in index path
@@ -55,14 +68,6 @@ Travis.ApplicationRoute = Travis.Route.extend
     renderFirstSync: ->
       @renderFirstSync()
 
-    afterSignIn: ->
-      if transition = Travis.auth.get('afterSignInTransition')
-        Travis.auth.set('afterSignInTransition', null)
-        transition.retry()
-
-    afterSignOut: ->
-      @transitionTo('index.current')
-
 Travis.Router.map ->
   @resource 'index', path: '/', ->
     @resource 'getting_started'
diff --git a/assets/scripts/app/templates/auth/signin.hbs b/assets/scripts/app/templates/auth/signin.hbs
index 1a83cc2e..6dcef908 100644
--- a/assets/scripts/app/templates/auth/signin.hbs
+++ b/assets/scripts/app/templates/auth/signin.hbs
@@ -6,8 +6,8 @@
 <h2>In order to view your repositories, please sign in.</h2>
 
 <p>
-  <a href="#" {{action "signIn" target="Travis"}}>
-    {{#if Travis.signingIn}}
+  <a href="#" {{action "signIn" target="auth"}}>
+    {{#if auth.signingIn}}
       Signing in...
     {{else}}
       Sign in with GitHub
diff --git a/assets/scripts/app/templates/layouts/top.hbs b/assets/scripts/app/templates/layouts/top.hbs
index 69fd6a62..8090f0eb 100644
--- a/assets/scripts/app/templates/layouts/top.hbs
+++ b/assets/scripts/app/templates/layouts/top.hbs
@@ -31,13 +31,13 @@
   </li>
   <li {{bind-attr class="view.classProfile"}}>
     <p class="handle">
-      {{#if signedOut}}
-        <a class="signed-out" href="#" {{action "signIn" target="Travis"}}>Sign in with GitHub</a>
+      {{#if auth.signedOut}}
+        <a class="signed-out" href="#" {{action "signIn" target="auth"}}>Sign in with GitHub</a>
       {{/if}}
-      {{#if signedIn}}
+      {{#if auth.signedIn}}
         {{#link-to "profile" class="signed-in"}}<img {{bind-attr src="gravatarUrl"}}/>{{userName}}{{/link-to}}
       {{/if}}
-      {{#if signingIn}}
+      {{#if auth.signingIn}}
         <span class="signing-in">Signing In</span>
       {{/if}}
     </p>
@@ -46,7 +46,7 @@
         {{#link-to "profile.index" class="signed-in"}}Accounts{{/link-to}}
       </li>
       <li>
-        <a href="/" {{action "signOut" target="Travis"}}>Sign Out</a>
+        <a href="/" {{action "signOut" target="auth"}}>Sign Out</a>
       </li>
     </ul>
   </li>
diff --git a/assets/scripts/app/views/signin.coffee b/assets/scripts/app/views/signin.coffee
index 39e75ef6..1dc06001 100644
--- a/assets/scripts/app/views/signin.coffee
+++ b/assets/scripts/app/views/signin.coffee
@@ -1,7 +1,3 @@
 @Travis.reopen
   SigninView: Travis.View.extend
     templateName: 'auth/signin'
-
-    signingIn: (->
-      Travis.get('authState') == 'signing-in'
-    ).property('Travis.authState')
diff --git a/assets/scripts/app/views/top.coffee b/assets/scripts/app/views/top.coffee
index fe5d7aea..a3bc90aa 100644
--- a/assets/scripts/app/views/top.coffee
+++ b/assets/scripts/app/views/top.coffee
@@ -16,9 +16,9 @@
     classProfile: (->
       classes = ['profile menu']
       classes.push('active') if @get('tab') == 'profile'
-      classes.push(Travis.get('authState') || 'signed-out')
+      classes.push(@get('controller.auth.state') || 'signed-out')
       classes.join(' ')
-    ).property('tab', 'Travis.authState')
+    ).property('tab', 'controller.auth.state')
 
     showProfile: ->
       $('#top .profile ul').show()
diff --git a/assets/scripts/spec/spec_helper.coffee b/assets/scripts/spec/spec_helper.coffee
index 82db9171..320eeee2 100644
--- a/assets/scripts/spec/spec_helper.coffee
+++ b/assets/scripts/spec/spec_helper.coffee
@@ -7,7 +7,11 @@ Travis.injectTestHelpers()
 oldSetup = Travis.setup
 Travis.ready = ->
   oldSetup.apply(this, arguments)
-  Travis.auth.signOut()
+  # TODO: in the future I would like to create a TestAuth class which
+  #       would have similar interface to Travis.Auth, but with the
+  #       auth logic stubbed out. It will give use ability to do things
+  #       like testAuth.shouldSucceedOnSignIn()
+  Travis.lookup('auth:main').signOut()
 
 window.exists = (selector) ->
   return !!find(selector).length
@@ -61,7 +65,7 @@ window.signInUser = (data) ->
 
   # for now let's just use harcoded data to log in the user,
   # we may extend it in the future to pass specific user data
-  Travis.auth.signIn
+  Travis.lookup('auth:main').signIn
     user: userData
     token: 'abcdef'