Store projects in ListGcpProjectsWorker

5 files changed, 45 insertions, 50 deletions

diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb
index 5409432e1ab..2d03c1f9f85 100644
--- a/app/controllers/projects/clusters/gcp_controller.rb
+++ b/app/controllers/projects/clusters/gcp_controller.rb
@@ -1,8 +1,8 @@
 class Projects::Clusters::GcpController < Projects::ApplicationController
   before_action :authorize_read_cluster!
-  before_action :authorize_google_api, except: [:login, :list_projects]
-  before_action :authorize_google_project_billing, only: [:new, :create]
   before_action :authorize_create_cluster!, only: [:new, :create]
+  before_action :authorize_google_api, except: [:login, :list_projects]
+  before_action :get_gcp_projects, only: [:new, :create]
   before_action :verify_billing, only: [:create]
 
   def login
@@ -36,21 +36,20 @@ class Projects::Clusters::GcpController < Projects::ApplicationController
   end
 
   def list_projects
-    projects = GoogleApi::CloudPlatform::Client.new(token_in_session, nil).projects_list
     respond_to do |format|
-      format.json { render status: :ok, json: { projects: projects } }
+      format.json { render status: :ok, json: { projects: gcp_projects } }
     end
   end
 
   private
 
   def verify_billing
-    case google_project_billing_status
+    case gcp_projects&.empty?
     when nil
       flash.now[:alert] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.')
-    when false
-      flash.now[:alert] = _('Please <a href=%{link_to_billing} target="_blank" rel="noopener noreferrer">enable billing for one of your projects to be able to create a Kubernetes cluster</a>, then try again.').html_safe % { link_to_billing: "https://console.cloud.google.com/freetrial?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral" }
     when true
+      flash.now[:alert] = _('Please <a href=%{link_to_billing} target="_blank" rel="noopener noreferrer">enable billing for one of your projects to be able to create a Kubernetes cluster</a>, then try again.').html_safe % { link_to_billing: "https://console.cloud.google.com/freetrial?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral" }
+    when false
       return
     end
 
@@ -82,13 +81,13 @@ class Projects::Clusters::GcpController < Projects::ApplicationController
     end
   end
 
-  def authorize_google_project_billing
+  def get_gcp_projects
     redis_token_key = ListGcpProjectsWorker.store_session_token(token_in_session)
     ListGcpProjectsWorker.perform_async(redis_token_key)
   end
 
-  def google_project_billing_status
-    ListGcpProjectsWorker.get_billing_state(token_in_session)
+  def gcp_projects
+    ListGcpProjectsWorker.get_projects(token_in_session)
   end
 
   def token_in_session
diff --git a/app/workers/list_gcp_projects_worker.rb b/app/workers/list_gcp_projects_worker.rb
index 12cb5f738e4..690980a4c4e 100644
--- a/app/workers/list_gcp_projects_worker.rb
+++ b/app/workers/list_gcp_projects_worker.rb
@@ -6,7 +6,7 @@ class ListGcpProjectsWorker
 
   LEASE_TIMEOUT = 3.seconds.to_i
   SESSION_KEY_TIMEOUT = 5.minutes
-  BILLING_TIMEOUT = 1.hour
+  PROJECT_TIMEOUT = 1.hour
   BILLING_CHANGED_LABELS = { state_transition: nil }.freeze
 
   def self.get_session_token(token_key)
@@ -23,10 +23,10 @@ class ListGcpProjectsWorker
     end
   end
 
-  def self.get_billing_state(token)
+  def self.read_projects(token)
     Gitlab::Redis::SharedState.with do |redis|
       value = redis.get(redis_shared_state_key_for(token))
-      ActiveRecord::Type::Boolean.new.type_cast_from_user(value)
+      JSON.parse(value)
     end
   end
 
@@ -37,9 +37,9 @@ class ListGcpProjectsWorker
     return unless token
     return unless try_obtain_lease_for(token)
 
-    billing_enabled_state = !ListGcpProjectsService.new.execute(token).empty?
-    update_billing_change_counter(self.class.get_billing_state(token), billing_enabled_state)
-    self.class.set_billing_state(token, billing_enabled_state)
+    billing_enabled_projects = ListGcpProjectsService.new.execute(token)
+    update_billing_change_counter(!self.class.read_projects(token).empty?, !billing_enabled_projects.empty?)
+    self.class.store_projects(token, billing_enabled_projects.to_json)
   end
 
   private
@@ -53,12 +53,12 @@ class ListGcpProjectsWorker
   end
 
   def self.redis_shared_state_key_for(token)
-    "gitlab:gcp:#{Digest::SHA1.hexdigest(token)}:billing_enabled"
+    "gitlab:gcp:#{Digest::SHA1.hexdigest(token)}:projects"
   end
 
-  def self.set_billing_state(token, value)
+  def self.store_projects(token, value)
     Gitlab::Redis::SharedState.with do |redis|
-      redis.set(redis_shared_state_key_for(token), value, ex: BILLING_TIMEOUT)
+      redis.set(redis_shared_state_key_for(token), value, ex: PROJECT_TIMEOUT)
     end
   end
 
diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb
index 3051f9cfde4..8b72c1a8cd9 100644
--- a/spec/controllers/projects/clusters/gcp_controller_spec.rb
+++ b/spec/controllers/projects/clusters/gcp_controller_spec.rb
@@ -77,7 +77,7 @@ describe Projects::Clusters::GcpController do
         end
 
         it 'has new object' do
-          expect(controller).to receive(:authorize_google_project_billing)
+          expect(controller).to receive(:get_gcp_projects)
 
           go
 
@@ -137,14 +137,12 @@ describe Projects::Clusters::GcpController do
       context 'when access token is valid' do
         before do
           stub_google_api_validate_token
-          allow_any_instance_of(described_class).to receive(:authorize_google_project_billing)
+          allow_any_instance_of(described_class).to receive(:get_gcp_projects)
         end
 
         context 'when google project billing is enabled' do
           before do
-            redis_double = double
-            allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double)
-            allow(redis_double).to receive(:get).with(ListGcpProjectsWorker.redis_shared_state_key_for('token')).and_return('true')
+            allow_any_instance_of(described_class).to receive(:gcp_projects).and_return([double])
           end
 
           it 'creates a new cluster' do
@@ -158,6 +156,10 @@ describe Projects::Clusters::GcpController do
         end
 
         context 'when google project billing is not enabled' do
+          before do
+            allow_any_instance_of(described_class).to receive(:gcp_projects).and_return([])
+          end
+
           it 'renders the cluster form with an error' do
             go
 
@@ -203,25 +205,19 @@ describe Projects::Clusters::GcpController do
   describe 'GET list_projects' do
     describe 'functionality' do
       let(:user) { create(:user) }
-      let(:api_response) { [project_id: 'test-project-1234'] }
+      let(:gcp_projects) { [project_id: 'test-project-1234'] }
 
       before do
         project.add_master(user)
         sign_in(user)
 
-        allow_any_instance_of(GoogleApi::CloudPlatform::Client).to receive(:projects_list).and_return(api_response)
-      end
-
-      it 'calls the Google Cloud Platform projects_list' do
-        expect_any_instance_of(GoogleApi::CloudPlatform::Client).to receive(:projects_list)
-
-        go
+        allow_any_instance_of(described_class).to receive(:gcp_projects).and_return(gcp_projects)
       end
 
       it 'renders the response as json' do
         go
 
-        expect(response.body).to eq({ projects: api_response }.to_json)
+        expect(response.body).to eq({ projects: gcp_projects }.to_json)
       end
     end
 
diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb
index dfe8e02dce0..3937fed490c 100644
--- a/spec/features/projects/clusters/gcp_spec.rb
+++ b/spec/features/projects/clusters/gcp_spec.rb
@@ -24,8 +24,8 @@ feature 'Gcp Cluster', :js do
 
     context 'when user has a GCP project with billing enabled' do
       before do
-        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing)
-        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return(true)
+        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:get_gcp_projects)
+        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:gcp_projects).and_return([double])
       end
 
       context 'when user does not have a cluster and visits cluster index page' do
@@ -133,8 +133,8 @@ feature 'Gcp Cluster', :js do
 
     context 'when user does not have a GCP project with billing enabled' do
       before do
-        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing)
-        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return(false)
+        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:get_gcp_projects)
+        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:gcp_projects).and_return([])
 
         visit project_clusters_path(project)
 
@@ -153,8 +153,8 @@ feature 'Gcp Cluster', :js do
 
     context 'when gcp billing status is not in redis' do
       before do
-        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing)
-        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return(nil)
+        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:get_gcp_projects)
+        allow_any_instance_of(Projects::Clusters::GcpController).to receive(:gcp_projects).and_return(nil)
 
         visit project_clusters_path(project)
 
diff --git a/spec/workers/list_gcp_projects_worker_spec.rb b/spec/workers/list_gcp_projects_worker_spec.rb
index ea94883b9a0..c415caf78f0 100644
--- a/spec/workers/list_gcp_projects_worker_spec.rb
+++ b/spec/workers/list_gcp_projects_worker_spec.rb
@@ -7,7 +7,7 @@ describe ListGcpProjectsWorker do
     subject { described_class.new.perform('token_key') }
 
     before do
-      allow(described_class).to receive(:get_billing_state)
+      allow(described_class).to receive(:read_projects).and_return([double])
       allow_any_instance_of(described_class).to receive(:update_billing_change_counter)
     end
 
@@ -27,9 +27,9 @@ describe ListGcpProjectsWorker do
           subject
         end
 
-        it 'stores billing status in redis' do
+        it 'stores projects in redis' do
           expect(ListGcpProjectsService).to receive_message_chain(:new, :execute).and_return([double])
-          expect(described_class).to receive(:set_billing_state).with(token, true)
+          expect(described_class).to receive(:store_projects).with(token, anything)
 
           subject
         end
@@ -67,15 +67,15 @@ describe ListGcpProjectsWorker do
     before do
       allow(described_class).to receive(:get_session_token).and_return('bogustoken')
       allow_any_instance_of(described_class).to receive(:try_obtain_lease_for).and_return('randomuuid')
-      allow(described_class).to receive(:set_billing_state)
+      allow(described_class).to receive(:store_projects)
     end
 
-    context 'when previous state was false' do
+    context 'when there are no projects in redis' do
       before do
-        expect(described_class).to receive(:get_billing_state).and_return(false)
+        expect(described_class).to receive(:read_projects).and_return([])
       end
 
-      context 'when the current state is false' do
+      context 'when the service does not return projects' do
         before do
           expect(ListGcpProjectsService).to receive_message_chain(:new, :execute).and_return([])
         end
@@ -87,7 +87,7 @@ describe ListGcpProjectsWorker do
         end
       end
 
-      context 'when the current state is true' do
+      context 'when the service returns projects' do
         before do
           expect(ListGcpProjectsService).to receive_message_chain(:new, :execute).and_return([double])
         end
@@ -100,13 +100,13 @@ describe ListGcpProjectsWorker do
       end
     end
 
-    context 'when previous state was true' do
+    context 'when there are projects in redis' do
       before do
-        expect(described_class).to receive(:get_billing_state).and_return(true)
+        expect(described_class).to receive(:read_projects).and_return([double])
         expect(ListGcpProjectsService).to receive_message_chain(:new, :execute).and_return([double])
       end
 
-      it 'increment the billing change counter' do
+      it 'increments the billing change counter' do
         expect_any_instance_of(described_class).to receive_message_chain(:billing_changed_counter, :increment)
 
         subject