Automatic merge of gitlab-org/gitlab master

app/models/wiki.rb

@@ -3,6 +3,7 @@
 class Wiki
   extend ::Gitlab::Utils::Override
   include HasRepository
+  include CanHousekeepRepository
   include Gitlab::Utils::StrongMemoize
   include GlobalID::Identification
 

changelogs/unreleased/mc-backstage-create-artifact-expiry-backfill-migration.yml

+---
+title: Backfill artifact expiry date.
+merge_request: 47723
+author:
+type: other

db/post_migrate/20201208175117_schedule_backfilling_artifact_expiry_migration.rb

+# frozen_string_literal: true
+
+class ScheduleBackfillingArtifactExpiryMigration < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  SWITCH_DATE = Time.utc(2020, 6, 22).freeze
+  INDEX_NAME = 'expired_artifacts_temp_index'.freeze
+  INDEX_CONDITION = "expire_at IS NULL AND created_at < '#{SWITCH_DATE}'"
+
+  disable_ddl_transaction!
+
+  class JobArtifact < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'ci_job_artifacts'
+
+    scope :without_expiry_date, -> { where(expire_at: nil) }
+    scope :before_switch, -> { where('created_at < ?', SWITCH_DATE) }
+  end
+
+  def up
+    # Create temporary index for expired artifacts
+    # Needs to be removed in a later migration
+    add_concurrent_index(:ci_job_artifacts, %i(id created_at), where: INDEX_CONDITION, name: INDEX_NAME)
+
+    queue_background_migration_jobs_by_range_at_intervals(
+      JobArtifact.without_expiry_date.before_switch,
+      ::Gitlab::BackgroundMigration::BackfillArtifactExpiryDate,
+      2.minutes,
+      batch_size: 200_000
+    )
+  end
+
+  def down
+    remove_concurrent_index_by_name :ci_job_artifacts, INDEX_NAME
+  end
+end

db/schema_migrations/20201208175117

+68971e7f9a722e98d9e611f614b5465de83ff3d4dc8c7a8078ed1db8f21e6590
\ No newline at end of file

db/structure.sql

@@ -20807,6 +20807,8 @@ CREATE UNIQUE INDEX epic_user_mentions_on_epic_id_and_note_id_index ON epic_user
 
 CREATE UNIQUE INDEX epic_user_mentions_on_epic_id_index ON epic_user_mentions USING btree (epic_id) WHERE (note_id IS NULL);
 
+CREATE INDEX expired_artifacts_temp_index ON ci_job_artifacts USING btree (id, created_at) WHERE ((expire_at IS NULL) AND (created_at < '2020-06-22 00:00:00+00'::timestamp with time zone));
+
 CREATE INDEX finding_links_on_vulnerability_occurrence_id ON vulnerability_finding_links USING btree (vulnerability_occurrence_id);
 
 CREATE INDEX idx_audit_events_on_entity_id_desc_author_id_created_at ON audit_events_archived USING btree (entity_id, entity_type, id DESC, author_id, created_at);

ee/app/controllers/projects/integrations/jira/issues_controller.rb

@@ -34,8 +34,11 @@ module Projects
         private
 
         def issues_json
-          jira_issues = finder.execute
-          jira_issues = Kaminari.paginate_array(jira_issues, limit: finder.per_page, total_count: finder.total_count)
+          jira_issues = Kaminari.paginate_array(
+            finder.execute,
+            limit: finder.per_page,
+            total_count: finder.total_count
+          )
 
           ::Integrations::Jira::IssueSerializer.new
             .with_pagination(request, response)
@@ -43,11 +46,7 @@ module Projects
         end
 
         def finder
-          @finder ||= finder_type.new(project, finder_options)
-        end
-
-        def finder_type
-          ::Projects::Integrations::Jira::IssuesFinder
+          @finder ||= ::Projects::Integrations::Jira::IssuesFinder.new(project, finder_options)
         end
 
         def finder_options
@@ -56,7 +55,7 @@ module Projects
           # Used by view to highlight active option
           @sort = options[:sort]
 
-          params.permit(finder_type.valid_params).merge(options)
+          params.permit(::Projects::Integrations::Jira::IssuesFinder.valid_params).merge(options)
         end
 
         def default_state
@@ -74,7 +73,7 @@ module Projects
         protected
 
         def check_feature_enabled!
-          return render_404 unless project.jira_issues_integration_available? && project.external_issue_tracker
+          return render_404 unless project.jira_issues_integration_available? && project.jira_service.issues_enabled
         end
 
         # Return the informational message to the user

ee/spec/controllers/projects/integrations/jira/issues_controller_spec.rb

@@ -5,30 +5,32 @@ require 'spec_helper'
 RSpec.describe Projects::Integrations::Jira::IssuesController do
   include ProjectForksHelper
 
-  let(:project) { create(:project) }
-  let(:user)    { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user, developer_projects: [project]) }
+  let_it_be(:jira) { create(:jira_service, project: project, issues_enabled: true, project_key: 'TEST') }
 
   before do
     stub_licensed_features(jira_issues_integration: true)
+    sign_in(user)
   end
 
   describe 'GET #index' do
-    before do
-      sign_in(user)
-      project.add_developer(user)
-      create(:jira_service, project: project)
-    end
-
     context 'when jira_issues_integration licensed feature is not available' do
-      it 'returns 404 status' do
+      before do
         stub_licensed_features(jira_issues_integration: false)
+      end
 
+      it 'returns 404 status' do
         get :index, params: { namespace_id: project.namespace, project_id: project }
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
 
+    it_behaves_like 'unauthorized when external service denies access' do
+      subject { get :index, params: { namespace_id: project.namespace, project_id: project } }
+    end
+
     it 'renders the "index" template' do
       get :index, params: { namespace_id: project.namespace, project_id: project }
 
@@ -48,7 +50,7 @@ RSpec.describe Projects::Integrations::Jira::IssuesController do
       it 'redirects to the new issue tracker from the old one' do
         get :index, params: { namespace_id: project.namespace, project_id: project }
 
-        expect(response).to redirect_to(project_integrations_jira_issues_path(new_project))
+        expect(response).to redirect_to(Gitlab::Routing.url_helpers.project_integrations_jira_issues_path(new_project))
         expect(response).to have_gitlab_http_status(:moved_permanently)
       end
     end
@@ -170,16 +172,4 @@ RSpec.describe Projects::Integrations::Jira::IssuesController do
       end
     end
   end
-
-  context 'external authorization' do
-    before do
-      sign_in user
-      project.add_developer(user)
-      create(:jira_service, project: project)
-    end
-
-    it_behaves_like 'unauthorized when external service denies access' do
-      subject { get :index, params: { namespace_id: project.namespace, project_id: project } }
-    end
-  end
 end

ee/spec/models/group_wiki_spec.rb

@@ -119,4 +119,10 @@ RSpec.describe GroupWiki do
       expect(subject).not_to be_a(Elastic::WikiRepositoriesSearch)
     end
   end
+
+  it_behaves_like 'can housekeep repository' do
+    let_it_be(:resource) { create(:group_wiki) }
+
+    let(:resource_key) { 'group_wikis' }
+  end
 end

lib/gitlab/background_migration/backfill_artifact_expiry_date.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # Backfill expire_at for a range of Ci::JobArtifact
+    class BackfillArtifactExpiryDate
+      include Gitlab::Utils::StrongMemoize
+
+      BATCH_SIZE = 1_000
+      DEFAULT_EXPIRATION_SWITCH_DATE = Date.new(2020, 6, 22).freeze
+      OLD_ARTIFACT_AGE = 15.months
+      OLD_ARTIFACT_EXPIRY_OFFSET = 3.months
+      RECENT_ARTIFACT_EXPIRY_OFFSET = 1.year
+
+      # Ci::JobArtifact model
+      class Ci::JobArtifact < ActiveRecord::Base
+        include ::EachBatch
+
+        self.table_name = 'ci_job_artifacts'
+
+        scope :between, -> (start_id, end_id) { where(id: start_id..end_id) }
+        scope :before_default_expiration_switch, -> { where('created_at < ?', DEFAULT_EXPIRATION_SWITCH_DATE) }
+        scope :without_expiry_date, -> { where(expire_at: nil) }
+        scope :old, -> { where(self.arel_table[:created_at].lt(OLD_ARTIFACT_AGE.ago)) }
+        scope :recent, -> { where(self.arel_table[:created_at].gt(OLD_ARTIFACT_AGE.ago)) }
+      end
+
+      def perform(start_id, end_id)
+        Ci::JobArtifact.between(start_id, end_id)
+          .without_expiry_date.before_default_expiration_switch
+          .each_batch(of: BATCH_SIZE) do |batch|
+          batch.old.update_all(expire_at: old_artifact_expiry_date)
+          batch.recent.update_all(expire_at: recent_artifact_expiry_date)
+        end
+      end
+
+      private
+
+      def offset_date
+        strong_memoize(:offset_date) do
+          current_date = Time.current
+          target_date = Time.zone.local(current_date.year, current_date.month, 22, 0, 0, 0)
+
+          current_date.day < 22 ? target_date : target_date.next_month
+        end
+      end
+
+      def old_artifact_expiry_date
+        offset_date + OLD_ARTIFACT_EXPIRY_OFFSET
+      end
+
+      def recent_artifact_expiry_date
+        offset_date + RECENT_ARTIFACT_EXPIRY_OFFSET
+      end
+    end
+  end
+end

rubocop/cop/lint/last_keyword_argument.rb

@@ -52,11 +52,14 @@ module RuboCop
 
         def known_match?(file_path, line_number, method_name)
           file_path_from_root = file_path.sub(File.expand_path('../../..', __dir__), '')
+          file_and_line = "#{file_path_from_root}:#{line_number}"
 
           method_name = 'initialize' if method_name == 'new'
 
-          self.class.keyword_warnings.any? do |warning|
-            warning.include?("#{file_path_from_root}:#{line_number}") && warning.include?("called method `#{method_name}'")
+          return unless self.class.keyword_warnings[method_name]
+
+          self.class.keyword_warnings[method_name].any? do |warning|
+            warning.include?(file_and_line)
           end
         end
 
@@ -69,7 +72,16 @@ module RuboCop
             hash.merge!(YAML.safe_load(File.read(file)))
           end
 
-          hash.values.flatten.select { |str| str.include?(KEYWORD_DEPRECATION_STR) }.uniq
+          hash.values.flatten.each_with_object({}) do |str, results|
+            next unless str.include?(KEYWORD_DEPRECATION_STR)
+
+            match_data = str.match(/called method `([^\s]+)'/)
+            next unless match_data
+
+            key = match_data[1]
+            results[key] ||= []
+            results[key] << str
+          end
         end
       end
     end

spec/frontend/artifacts_settings/components/keep_latest_artifact_checkbox_spec.js

 import { GlFormCheckbox, GlLink } from '@gitlab/ui';
-import { shallowMount } from '@vue/test-utils';
+import { shallowMount, createLocalVue } from '@vue/test-utils';
+import VueApollo from 'vue-apollo';
+import createMockApollo from 'jest/helpers/mock_apollo_helper';
 import KeepLatestArtifactCheckbox from '~/artifacts_settings/keep_latest_artifact_checkbox.vue';
+import GetKeepLatestArtifactProjectSetting from '~/artifacts_settings/graphql/queries/get_keep_latest_artifact_project_setting.query.graphql';
 import UpdateKeepLatestArtifactProjectSetting from '~/artifacts_settings/graphql/mutations/update_keep_latest_artifact_project_setting.mutation.graphql';
 
+const localVue = createLocalVue();
+localVue.use(VueApollo);
+
+const keepLatestArtifactMock = {
+  data: {
+    project: {
+      ciCdSettings: { keepLatestArtifact: true },
+    },
+  },
+};
+
+const keepLatestArtifactMockResponse = {
+  data: { ciCdSettingsUpdate: { errors: [], __typename: 'CiCdSettingsUpdatePayload' } },
+};
+
 describe('Keep latest artifact checkbox', () => {
   let wrapper;
+  let apolloProvider;
+  let requestHandlers;
 
-  const mutate = jest.fn().mockResolvedValue();
   const fullPath = 'gitlab-org/gitlab';
   const helpPagePath = '/help/ci/pipelines/job_artifacts';
 
   const findCheckbox = () => wrapper.find(GlFormCheckbox);
   const findHelpLink = () => wrapper.find(GlLink);
 
-  const createComponent = () => {
+  const createComponent = (handlers) => {
+    requestHandlers = {
+      keepLatestArtifactQueryHandler: jest.fn().mockResolvedValue(keepLatestArtifactMock),
+      keepLatestArtifactMutationHandler: jest
+        .fn()
+        .mockResolvedValue(keepLatestArtifactMockResponse),
+      ...handlers,
+    };
+
+    apolloProvider = createMockApollo([
+      [GetKeepLatestArtifactProjectSetting, requestHandlers.keepLatestArtifactQueryHandler],
+      [UpdateKeepLatestArtifactProjectSetting, requestHandlers.keepLatestArtifactMutationHandler],
+    ]);
+
     wrapper = shallowMount(KeepLatestArtifactCheckbox, {
       provide: {
         fullPath,
         helpPagePath,
       },
-      mocks: {
-        $apollo: {
-          mutate,
-        },
-      },
+      localVue,
+      apolloProvider,
     });
   };
 
@@ -34,6 +63,7 @@ describe('Keep latest artifact checkbox', () => {
   afterEach(() => {
     wrapper.destroy();
     wrapper = null;
+    apolloProvider = null;
   });
 
   it('displays the checkbox and the help link', () => {
@@ -42,21 +72,17 @@ describe('Keep latest artifact checkbox', () => {
   });
 
   it('sets correct setting value in checkbox with query result', async () => {
-    await wrapper.setData({ keepLatestArtifact: true });
+    await wrapper.vm.$nextTick();
+
     expect(wrapper.element).toMatchSnapshot();
   });
 
   it('calls mutation on artifact setting change with correct payload', () => {
     findCheckbox().vm.$emit('change', false);
 
-    const expected = {
-      mutation: UpdateKeepLatestArtifactProjectSetting,
-      variables: {
-        fullPath,
-        keepLatestArtifact: false,
-      },
-    };
-
-    expect(mutate).toHaveBeenCalledWith(expected);
+    expect(requestHandlers.keepLatestArtifactMutationHandler).toHaveBeenCalledWith({
+      fullPath,
+      keepLatestArtifact: false,
+    });
   });
 });

spec/lib/gitlab/background_migration/backfill_artifact_expiry_date_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::BackfillArtifactExpiryDate, :migration, schema: 20201111152859 do
+  subject(:perform) { migration.perform(1, 99) }
+
+  let(:migration) { described_class.new }
+  let(:artifact_outside_id_range) { create_artifact!(id: 100, created_at: 1.year.ago, expire_at: nil) }
+  let(:artifact_outside_date_range) { create_artifact!(id: 40, created_at: Time.current, expire_at: nil) }
+  let(:old_artifact) { create_artifact!(id: 10, created_at: 16.months.ago, expire_at: nil) }
+  let(:recent_artifact) { create_artifact!(id: 20, created_at: 1.year.ago, expire_at: nil) }
+  let(:artifact_with_expiry) { create_artifact!(id: 30, created_at: 1.year.ago, expire_at: Time.current + 1.day) }
+
+  before do
+    table(:namespaces).create!(id: 1, name: 'the-namespace', path: 'the-path')
+    table(:projects).create!(id: 1, name: 'the-project', namespace_id: 1)
+    table(:ci_builds).create!(id: 1, allow_failure: false)
+  end
+
+  context 'when current date is before the 22nd' do
+    before do
+      travel_to(Time.zone.local(2020, 1, 1, 0, 0, 0))
+    end
+
+    it 'backfills the expiry date for old artifacts' do
+      expect(old_artifact.reload.expire_at).to eq(nil)
+
+      perform
+
+      expect(old_artifact.reload.expire_at).to be_within(1.minute).of(Time.zone.local(2020, 4, 22, 0, 0, 0))
+    end
+
+    it 'backfills the expiry date for recent artifacts' do
+      expect(recent_artifact.reload.expire_at).to eq(nil)
+
+      perform
+
+      expect(recent_artifact.reload.expire_at).to be_within(1.minute).of(Time.zone.local(2021, 1, 22, 0, 0, 0))
+    end
+  end
+
+  context 'when current date is after the 22nd' do
+    before do
+      travel_to(Time.zone.local(2020, 1, 23, 0, 0, 0))
+    end
+
+    it 'backfills the expiry date for old artifacts' do
+      expect(old_artifact.reload.expire_at).to eq(nil)
+
+      perform
+
+      expect(old_artifact.reload.expire_at).to be_within(1.minute).of(Time.zone.local(2020, 5, 22, 0, 0, 0))
+    end
+
+    it 'backfills the expiry date for recent artifacts' do
+      expect(recent_artifact.reload.expire_at).to eq(nil)
+
+      perform
+
+      expect(recent_artifact.reload.expire_at).to be_within(1.minute).of(Time.zone.local(2021, 2, 22, 0, 0, 0))
+    end
+  end
+
+  it 'does not touch artifacts with expiry date' do
+    expect { perform }.not_to change { artifact_with_expiry.reload.expire_at }
+  end
+
+  it 'does not touch artifacts outside id range' do
+    expect { perform }.not_to change { artifact_outside_id_range.reload.expire_at }
+  end
+
+  it 'does not touch artifacts outside date range' do
+    expect { perform }.not_to change { artifact_outside_date_range.reload.expire_at }
+  end
+
+  private
+
+  def create_artifact!(**args)
+    table(:ci_job_artifacts).create!(**args, project_id: 1, job_id: 1, file_type: 1)
+  end
+end

spec/lib/gitlab/config/entry/composable_hash_spec.rb

@@ -92,7 +92,7 @@ RSpec.describe Gitlab::Config::Entry::ComposableHash, :aggregate_failures do
     end
 
     let(:entry) do
-      parent_entry = composable_hash_parent_class.new(secrets: config)
+      parent_entry = composable_hash_parent_class.new({ secrets: config })
       parent_entry.compose!
 
       parent_entry[:secrets]

spec/lib/gitlab/git/diff_spec.rb

@@ -58,7 +58,7 @@ EOT
 
       context 'using a diff that is too large' do
         it 'prunes the diff' do
-          diff = described_class.new(diff: 'a' * 204800)
+          diff = described_class.new({ diff: 'a' * 204800 })
 
           expect(diff.diff).to be_empty
           expect(diff).to be_too_large

spec/models/project_wiki_spec.rb

@@ -42,4 +42,10 @@ RSpec.describe ProjectWiki do
       end
     end
   end
+
+  it_behaves_like 'can housekeep repository' do
+    let_it_be(:resource) { create(:project_wiki) }
+
+    let(:resource_key) { 'project_wikis' }
+  end
 end