Refactor: Make all Oauth::User methods instance methods

lib/gitlab/oauth/user.rb

@@ -6,55 +6,51 @@
 module Gitlab
   module OAuth
     class User
-      class << self
-        attr_reader :auth_hash
-
-        def find(auth_hash)
-          self.auth_hash = auth_hash
-          find_by_uid_and_provider
-        end
-
-        def create(auth_hash)
-          user = new(auth_hash)
-          user.save_and_trigger_callbacks
-        end
-
-        def model
-          ::User
-        end
-
-        def auth_hash=(auth_hash)
-          @auth_hash = AuthHash.new(auth_hash)
-        end
-
-        protected
-        def find_by_uid_and_provider
-          model.where(provider: auth_hash.provider, extern_uid: auth_hash.uid).last
-        end
-      end
-
-      # Instance methods
-      attr_accessor :auth_hash, :user
+      attr_accessor :auth_hash, :gl_user
 
       def initialize(auth_hash)
         self.auth_hash = auth_hash
-        self.user = self.class.model.new(user_attributes)
-        user.skip_confirmation!
       end
 
       def auth_hash=(auth_hash)
         @auth_hash = AuthHash.new(auth_hash)
       end
 
-      def save_and_trigger_callbacks
-        user.save!
+      def persisted?
+        gl_user.persisted?
+      end
+
+      def new?
+        !gl_user.persisted?
+      end
+
+      def valid?
+        gl_user.valid?
+      end
+
+      def save
+        gl_user.save!
         log.info "(OAuth) Creating user #{auth_hash.email} from login with extern_uid => #{auth_hash.uid}"
-        user.block if needs_blocking?
+        gl_user.block if needs_blocking?
 
-        user
+        gl_user
       rescue ActiveRecord::RecordInvalid => e
         log.info "(OAuth) Email #{e.record.errors[:email]}. Username #{e.record.errors[:username]}"
-        return nil, e.record.errors
+        return self, e.record.errors
+      end
+
+      def gl_user
+        @user ||= find_by_uid_and_provider || build_new_user
+      end
+
+      def find_by_uid_and_provider
+        model.where(provider: auth_hash.provider, extern_uid: auth_hash.uid).last
+      end
+
+      def build_new_user
+        model.new(user_attributes).tap do |user|
+          user.skip_confirmation!
+        end
       end
 
       def user_attributes
@@ -80,6 +76,10 @@ module Gitlab
       def needs_blocking?
         Gitlab.config.omniauth['block_auto_created_users']
       end
+
+      def model
+        ::User
+      end
     end
   end
 end

spec/lib/gitlab/oauth/user_spec.rb

 require 'spec_helper'
 
 describe Gitlab::OAuth::User do
-  let(:gl_auth) { Gitlab::OAuth::User }
-  let(:info) do
-    double(
+  let(:oauth_user) { Gitlab::OAuth::User.new(auth_hash) }
+  let(:gl_user) { oauth_user.gl_user }
+  let(:uid) { 'my-uid' }
+  let(:provider) { 'my-provider' }
+  let(:auth_hash) { double(uid: uid, provider: provider, info: double(info_hash)) }
+  let(:info_hash) do
+    {
       nickname: 'john',
       name: 'John',
       email: 'john@mail.com'
-    )
+    }
   end
 
   before do
     Gitlab.config.stub(omniauth: {})
   end
 
-  describe :find do
+  describe :persisted? do
     let!(:existing_user) { create(:user, extern_uid: 'my-uid', provider: 'my-provider') }
 
     it "finds an existing user based on uid and provider (facebook)" do
       auth = double(info: double(name: 'John'), uid: 'my-uid', provider: 'my-provider')
-      assert gl_auth.find(auth)
+      expect( oauth_user.persisted? ).to be_true
     end
 
-    it "finds an existing user based on nested uid and provider" do
-      auth = double(info: info, uid: 'my-uid', provider: 'my-provider')
-      assert gl_auth.find(auth)
+    it "returns false if use is not found in database" do
+      auth_hash.stub(uid: 'non-existing')
+      expect( oauth_user.persisted? ).to be_false
     end
   end
 
-  describe :create do
-    it "should create user from LDAP" do
-      auth = double(info: info, uid: 'my-uid', provider: 'ldap')
-      user = gl_auth.create(auth)
+  describe :save do
+    context "LDAP" do
+      let(:provider) { 'ldap' }
+      it "creates a user from LDAP" do
+        oauth_user.save
 
-      user.should be_valid
-      user.extern_uid.should == auth.uid
-      user.provider.should == 'ldap'
+        expect(gl_user).to be_valid
+        expect(gl_user.extern_uid).to eql uid
+        expect(gl_user.provider).to eql 'ldap'
+      end
     end
 
-    it "should create user from Omniauth" do
-      auth = double(info: info, uid: 'my-uid', provider: 'twitter')
-      user = gl_auth.create(auth)
+    context "twitter" do
+      let(:provider) { 'twitter' }
 
-      user.should be_valid
-      user.extern_uid.should == auth.uid
-      user.provider.should == 'twitter'
-    end
-
-    it "should apply defaults to user" do
-      auth = double(info: info, uid: 'my-uid', provider: 'ldap')
-      user = gl_auth.create(auth)
-
-      user.should be_valid
-      user.projects_limit.should == Gitlab.config.gitlab.default_projects_limit
-      user.can_create_group.should == Gitlab.config.gitlab.default_can_create_group
-    end
-
-    it "Set a temp email address if not provided (like twitter does)" do
-      info = double(
-        uid: 'my-uid',
-        nickname: 'john',
-        name: 'John'
-      )
-      auth = double(info: info, uid: 'my-uid', provider: 'my-provider')
-
-      user = gl_auth.create(auth)
-      expect(user.email).to_not be_empty
-    end
-
-    it 'generates a username if non provided (google)' do
-      info = double(
-        uid: 'my-uid',
-        name: 'John',
-        email: 'john@example.com'
-      )
-      auth = double(info: info, uid: 'my-uid', provider: 'my-provider')
+      it "creates a user from Omniauth" do
+        oauth_user.save
 
-      user = gl_auth.create(auth)
-      expect(user.username).to eql 'john'
+        expect(gl_user).to be_valid
+        expect(gl_user.extern_uid).to eql uid
+        expect(gl_user.provider).to eql 'twitter'
+      end
     end
   end
 end