Merge branch '322128-token-order-attempt-2' into 'master'

Order cluster token by last used

See merge request gitlab-org/gitlab!59716

app/models/clusters/agent.rb

@@ -8,6 +8,7 @@ module Clusters
     belongs_to :project, class_name: '::Project' # Otherwise, it will load ::Clusters::Project
 
     has_many :agent_tokens, class_name: 'Clusters::AgentToken'
+    has_many :last_used_agent_tokens, -> { order_last_used_at_desc }, class_name: 'Clusters::AgentToken', inverse_of: :agent
 
     scope :ordered_by_name, -> { order(:name) }
     scope :with_name, -> (name) { where(name: name) }

app/models/clusters/agent_token.rb

@@ -6,7 +6,7 @@ module Clusters
     include TokenAuthenticatable
 
     add_authentication_token_field :token, encrypted: :required, token_generator: -> { Devise.friendly_token(50) }
-    cached_attr_reader :last_contacted_at
+    cached_attr_reader :last_used_at
 
     self.table_name = 'cluster_agent_tokens'
 
@@ -21,6 +21,8 @@ module Clusters
     validates :description, length: { maximum: 1024 }
     validates :name, presence: true, length: { maximum: 255 }
 
+    scope :order_last_used_at_desc, -> { order(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) }
+
     def track_usage
       track_values = { last_used_at: Time.current.utc }
 

changelogs/unreleased/322128-token-order.yml

+---
+title: Add ability to order cluster token by last used
+merge_request: 59716
+author:
+type: changed

db/migrate/20210420012909_index_cluster_agent_tokens_on_last_used_at.rb

+# frozen_string_literal: true
+
+class IndexClusterAgentTokensOnLastUsedAt < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  OLD_INDEX = 'index_cluster_agent_tokens_on_agent_id'
+  NEW_INDEX = 'index_cluster_agent_tokens_on_agent_id_and_last_used_at'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :cluster_agent_tokens, 'agent_id, last_used_at DESC NULLS LAST', name: NEW_INDEX
+    remove_concurrent_index_by_name :cluster_agent_tokens, OLD_INDEX
+  end
+
+  def down
+    add_concurrent_index :cluster_agent_tokens, :agent_id, name: OLD_INDEX
+    remove_concurrent_index_by_name :cluster_agent_tokens, NEW_INDEX
+  end
+end

db/schema_migrations/20210420012909

+c9e8c49bf272ef49d906431bdc11a24abe967a9d7e95976d70c48b21b48a062b
\ No newline at end of file

db/structure.sql

@@ -22324,7 +22324,7 @@ CREATE INDEX index_ci_variables_on_key ON ci_variables USING btree (key);
 
 CREATE UNIQUE INDEX index_ci_variables_on_project_id_and_key_and_environment_scope ON ci_variables USING btree (project_id, key, environment_scope);
 
-CREATE INDEX index_cluster_agent_tokens_on_agent_id ON cluster_agent_tokens USING btree (agent_id);
+CREATE INDEX index_cluster_agent_tokens_on_agent_id_and_last_used_at ON cluster_agent_tokens USING btree (agent_id, last_used_at DESC NULLS LAST);
 
 CREATE INDEX index_cluster_agent_tokens_on_created_by_user_id ON cluster_agent_tokens USING btree (created_by_user_id);
 

ee/app/graphql/resolvers/clusters/agent_tokens_resolver.rb

@@ -12,7 +12,7 @@ module Resolvers
       def resolve(**args)
         return ::Clusters::AgentToken.none unless can_read_agent_tokens?
 
-        agent.agent_tokens
+        agent.last_used_agent_tokens
       end
 
       private

ee/app/graphql/resolvers/clusters/agents_resolver.rb

@@ -28,7 +28,7 @@ module Resolvers
       private
 
       def preloads
-        { tokens: :agent_tokens }
+        { tokens: :last_used_agent_tokens }
       end
     end
   end

ee/spec/graphql/resolvers/clusters/agent_tokens_resolver_spec.rb

@@ -14,8 +14,8 @@ RSpec.describe Resolvers::Clusters::AgentTokensResolver do
     let(:feature_available) { true }
     let(:ctx) { Hash(current_user: user) }
 
-    let!(:matching_token1) { create(:cluster_agent_token, agent: agent) }
-    let!(:mathcing_token2) { create(:cluster_agent_token, agent: agent) }
+    let!(:matching_token1) { create(:cluster_agent_token, agent: agent, last_used_at: 5.days.ago) }
+    let!(:matching_token2) { create(:cluster_agent_token, agent: agent, last_used_at: 2.days.ago) }
     let!(:other_token) { create(:cluster_agent_token) }
 
     subject { resolve(described_class, obj: agent, ctx: ctx) }
@@ -24,8 +24,8 @@ RSpec.describe Resolvers::Clusters::AgentTokensResolver do
       stub_licensed_features(cluster_agents: feature_available)
     end
 
-    it 'returns tokens associated with the agent' do
-      expect(subject).to contain_exactly(matching_token1, mathcing_token2)
+    it 'returns tokens associated with the agent, ordered by last_used_at' do
+      expect(subject).to eq([matching_token2, matching_token1])
     end
 
     context 'feature is not available' do

ee/spec/requests/api/graphql/project/cluster_agents_spec.rb

@@ -50,22 +50,22 @@ RSpec.describe 'Project.cluster_agents' do
   end
 
   context 'selecting tokens' do
-    let(:cluster_agents_fields) { [:id, query_nodes(:tokens, of: 'ClusterAgentToken')] }
+    let_it_be(:token_1) { create(:cluster_agent_token, agent: agents.first) }
+    let_it_be(:token_2) { create(:cluster_agent_token, agent: agents.second) }
+    let_it_be(:token_3) { create(:cluster_agent_token, agent: agents.second, last_used_at: 2.days.ago) }
 
-    before do
-      create(:cluster_agent_token, agent: agents.first)
-      create(:cluster_agent_token, agent: agents.second)
-    end
+    let(:cluster_agents_fields) { [:id, query_nodes(:tokens, of: 'ClusterAgentToken')] }
 
-    it 'can select tokens' do
+    it 'can select tokens in last_used_at order' do
       post_graphql(query, current_user: current_user)
 
       tokens = graphql_data_at(:project, :cluster_agents, :nodes, :tokens, :nodes)
 
-      expect(tokens).to contain_exactly(
-        a_hash_including('id' => be_present),
-        a_hash_including('id' => be_present)
-      )
+      expect(tokens).to match([
+        a_hash_including('id' => global_id_of(token_1)),
+        a_hash_including('id' => global_id_of(token_3)),
+        a_hash_including('id' => global_id_of(token_2))
+      ])
     end
 
     it 'does not suffer from N+1 performance issues' do

spec/models/clusters/agent_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe Clusters::Agent do
   it { is_expected.to belong_to(:created_by_user).class_name('User').optional }
   it { is_expected.to belong_to(:project).class_name('::Project') }
   it { is_expected.to have_many(:agent_tokens).class_name('Clusters::AgentToken') }
+  it { is_expected.to have_many(:last_used_agent_tokens).class_name('Clusters::AgentToken') }
 
   it { is_expected.to validate_presence_of(:name) }
   it { is_expected.to validate_length_of(:name).is_at_most(63) }

spec/models/clusters/agent_token_spec.rb

@@ -9,6 +9,19 @@ RSpec.describe Clusters::AgentToken do
   it { is_expected.to validate_length_of(:name).is_at_most(255) }
   it { is_expected.to validate_presence_of(:name) }
 
+  describe 'scopes' do
+    describe '.order_last_used_at_desc' do
+      let_it_be(:token_1) { create(:cluster_agent_token, last_used_at: 7.days.ago) }
+      let_it_be(:token_2) { create(:cluster_agent_token, last_used_at: nil) }
+      let_it_be(:token_3) { create(:cluster_agent_token, last_used_at: 2.days.ago) }
+
+      it 'sorts by last_used_at descending, with null values at last' do
+        expect(described_class.order_last_used_at_desc)
+          .to eq([token_3, token_1, token_2])
+      end
+    end
+  end
+
   describe '#token' do
     it 'is generated on save' do
       agent_token = build(:cluster_agent_token, token_encrypted: nil)