Merge branch 'pl-spec-policy-project-perf' into 'master'

Speed project policy specs for both FOSS and EE

See merge request gitlab-org/gitlab!42248

ee/spec/policies/project_policy_spec.rb

@@ -5,16 +5,9 @@ require 'spec_helper'
 RSpec.describe ProjectPolicy do
   include ExternalAuthorizationServiceHelpers
   include AdminModeHelper
+  include_context 'ProjectPolicy context'
 
-  let_it_be(:owner) { create(:user) }
-  let_it_be(:admin) { create(:admin) }
-  let_it_be(:maintainer) { create(:user) }
-  let_it_be(:developer) { create(:user) }
-  let_it_be(:reporter) { create(:user) }
-  let_it_be(:guest) { create(:user) }
-  let_it_be(:non_member) { create(:user) }
-  let_it_be(:project, refind: true) { create(:project, :public, namespace: owner.namespace) }
-  let_it_be(:private_project, refind: true) { create(:project, :private, namespace: owner.namespace) }
+  let(:project) { public_project }
 
   subject { described_class.new(current_user, project) }
 
@@ -22,21 +15,7 @@ RSpec.describe ProjectPolicy do
     stub_licensed_features(license_scanning: true)
   end
 
-  before_all do
-    project.add_maintainer(maintainer)
-    project.add_developer(developer)
-    project.add_reporter(reporter)
-    project.add_guest(guest)
-
-    private_project.add_maintainer(maintainer)
-    private_project.add_developer(developer)
-    private_project.add_reporter(reporter)
-    private_project.add_guest(guest)
-  end
-
   context 'basic permissions' do
-    include_context 'ProjectPolicy context'
-
     let(:additional_reporter_permissions) do
       %i[read_software_license_policy]
     end
@@ -178,7 +157,7 @@ RSpec.describe ProjectPolicy do
       end
 
       context 'when user is logged out' do
-        let(:current_user) { nil }
+        let(:current_user) { anonymous }
 
         it { is_expected.to be_allowed(:read_iteration) }
         it { is_expected.to be_disallowed(:create_iteration, :admin_iteration) }
@@ -194,7 +173,7 @@ RSpec.describe ProjectPolicy do
         end
 
         context 'when user is logged out' do
-          let(:current_user) { nil }
+          let(:current_user) { anonymous }
 
           it { is_expected.to be_disallowed(:read_iteration, :create_iteration, :admin_iteration) }
         end
@@ -207,8 +186,7 @@ RSpec.describe ProjectPolicy do
 
     context 'when the feature is disabled' do
       before do
-        project.issues_enabled = false
-        project.save!
+        project.update!(issues_enabled: false)
       end
 
       it 'disables boards permissions' do
@@ -512,7 +490,7 @@ RSpec.describe ProjectPolicy do
       end
 
       context 'with anonymous' do
-        let(:current_user) { nil }
+        let(:current_user) { anonymous }
 
         it { is_expected.to be_disallowed(permission) }
       end
@@ -595,7 +573,7 @@ RSpec.describe ProjectPolicy do
       end
 
       context 'with anonymous' do
-        let(:current_user) { nil }
+        let(:current_user) { anonymous }
 
         it { is_expected.to be_disallowed(:read_threat_monitoring) }
       end
@@ -735,7 +713,7 @@ RSpec.describe ProjectPolicy do
     end
 
     context 'with anonymous' do
-      let(:current_user) { nil }
+      let(:current_user) { anonymous }
 
       it { is_expected.to be_disallowed(:admin_software_license_policy) }
     end
@@ -763,7 +741,7 @@ RSpec.describe ProjectPolicy do
         let(:current_user) { create(:user) }
 
         context 'with public access to repository' do
-          let(:project) { create(:project, :public) }
+          let(:project) { public_project }
 
           it { is_expected.to be_allowed(:read_dependencies) }
         end
@@ -827,7 +805,7 @@ RSpec.describe ProjectPolicy do
         end
 
         context 'with anonymous' do
-          let(:current_user) { nil }
+          let(:current_user) { anonymous }
 
           it { is_expected.to be_disallowed(:read_dependencies) }
         end
@@ -887,7 +865,7 @@ RSpec.describe ProjectPolicy do
         end
 
         context 'with anonymous' do
-          let(:current_user) { nil }
+          let(:current_user) { anonymous }
 
           it { is_expected.to be_disallowed(:read_licenses) }
         end
@@ -906,7 +884,6 @@ RSpec.describe ProjectPolicy do
   end
 
   describe 'publish_status_page' do
-    let(:anonymous) { nil }
     let(:feature) { :status_page }
     let(:policy) { :publish_status_page }
 
@@ -1152,7 +1129,7 @@ RSpec.describe ProjectPolicy do
     end
 
     context 'with anonymous' do
-      let(:current_user) { nil }
+      let(:current_user) { anonymous }
 
       it { is_expected.to be_disallowed(:read_group_timelogs) }
     end

spec/policies/project_policy_spec.rb

@@ -6,21 +6,9 @@ RSpec.describe ProjectPolicy do
   include ExternalAuthorizationServiceHelpers
   include_context 'ProjectPolicy context'
 
-  let_it_be(:other_user) { create(:user) }
-  let_it_be(:guest) { create(:user) }
-  let_it_be(:reporter) { create(:user) }
-  let_it_be(:developer) { create(:user) }
-  let_it_be(:maintainer) { create(:user) }
-  let_it_be(:owner) { create(:user) }
-  let_it_be(:admin) { create(:admin) }
-  let(:project) { create(:project, :public, namespace: owner.namespace) }
+  let(:project) { public_project }
 
-  before do
-    project.add_guest(guest)
-    project.add_maintainer(maintainer)
-    project.add_developer(developer)
-    project.add_reporter(reporter)
-  end
+  subject { described_class.new(current_user, project) }
 
   def expect_allowed(*permissions)
     permissions.each { |p| is_expected.to be_allowed(p) }
@@ -31,7 +19,7 @@ RSpec.describe ProjectPolicy do
   end
 
   context 'with no project feature' do
-    subject { described_class.new(owner, project) }
+    let(:current_user) { owner }
 
     before do
       project.project_feature.destroy!
@@ -63,7 +51,7 @@ RSpec.describe ProjectPolicy do
   end
 
   context 'issues feature' do
-    subject { described_class.new(owner, project) }
+    let(:current_user) { owner }
 
     context 'when the feature is disabled' do
       before do
@@ -91,7 +79,7 @@ RSpec.describe ProjectPolicy do
   end
 
   context 'merge requests feature' do
-    subject { described_class.new(owner, project) }
+    let(:current_user) { owner }
 
     it 'disallows all permissions when the feature is disabled' do
       project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
@@ -105,9 +93,8 @@ RSpec.describe ProjectPolicy do
   end
 
   context 'for a guest in a private project' do
-    let(:project) { create(:project, :private) }
-
-    subject { described_class.new(guest, project) }
+    let(:current_user) { guest }
+    let(:project) { private_project }
 
     it 'disallows the guest from reading the merge request and merge request iid' do
       expect_disallowed(:read_merge_request)
@@ -116,12 +103,10 @@ RSpec.describe ProjectPolicy do
   end
 
   context 'pipeline feature' do
-    let(:project) { create(:project) }
+    let(:project) { private_project }
 
     describe 'for unconfirmed user' do
-      let(:unconfirmed_user) { create(:user, confirmed_at: nil) }
-
-      subject { described_class.new(unconfirmed_user, project) }
+      let(:current_user) { create(:user, confirmed_at: nil) }
 
       it 'disallows to modify pipelines' do
         expect_disallowed(:create_pipeline)
@@ -131,7 +116,7 @@ RSpec.describe ProjectPolicy do
     end
 
     describe 'for confirmed user' do
-      subject { described_class.new(developer, project) }
+      let(:current_user) { developer }
 
       it 'allows modify pipelines' do
         expect_allowed(:create_pipeline)
@@ -143,7 +128,7 @@ RSpec.describe ProjectPolicy do
 
   context 'builds feature' do
     context 'when builds are disabled' do
-      subject { described_class.new(owner, project) }
+      let(:current_user) { owner }
 
       before do
         project.project_feature.update!(builds_access_level: ProjectFeature::DISABLED)
@@ -163,7 +148,7 @@ RSpec.describe ProjectPolicy do
     end
 
     context 'when builds are disabled only for some users' do
-      subject { described_class.new(guest, project) }
+      let(:current_user) { guest }
 
       before do
         project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE)
@@ -194,7 +179,7 @@ RSpec.describe ProjectPolicy do
     end
 
     context 'when user is a project member' do
-      subject { described_class.new(owner, project) }
+      let(:current_user) { owner }
 
       context 'when it is disabled' do
         before do
@@ -212,8 +197,8 @@ RSpec.describe ProjectPolicy do
       end
     end
 
-    context 'when user is some other user' do
-      subject { described_class.new(other_user, project) }
+    context 'when user is non-member' do
+      let(:current_user) { non_member }
 
       context 'when access level is private' do
         before do
@@ -243,7 +228,7 @@ RSpec.describe ProjectPolicy do
 
   context 'when a public project has merge requests allowing access' do
     include ProjectForksHelper
-    let(:user) { create(:user) }
+    let(:current_user) { create(:user) }
     let(:target_project) { create(:project, :public) }
     let(:project) { fork_project(target_project) }
     let!(:merge_request) do
@@ -259,20 +244,18 @@ RSpec.describe ProjectPolicy do
       %w(create_build create_pipeline)
     end
 
-    subject { described_class.new(user, project) }
-
     it 'does not allow pushing code' do
       expect_disallowed(*maintainer_abilities)
     end
 
     it 'allows pushing if the user is a member with push access to the target project' do
-      target_project.add_developer(user)
+      target_project.add_developer(current_user)
 
       expect_allowed(*maintainer_abilities)
     end
 
     it 'disallows abilities to a maintainer if the merge request was closed' do
-      target_project.add_developer(user)
+      target_project.add_developer(current_user)
       merge_request.close!
 
       expect_disallowed(*maintainer_abilities)
@@ -280,12 +263,9 @@ RSpec.describe ProjectPolicy do
   end
 
   it_behaves_like 'clusterable policies' do
-    let(:clusterable) { create(:project, :repository) }
-    let(:cluster) do
-      create(:cluster,
-             :provided_by_gcp,
-             :project,
-             projects: [clusterable])
+    let_it_be(:clusterable) { create(:project, :repository) }
+    let_it_be(:cluster) do
+      create(:cluster, :provided_by_gcp, :project, projects: [clusterable])
     end
   end
 
@@ -356,16 +336,14 @@ RSpec.describe ProjectPolicy do
   end
 
   context 'forking a project' do
-    subject { described_class.new(current_user, project) }
-
     context 'anonymous user' do
-      let(:current_user) { nil }
+      let(:current_user) { anonymous }
 
       it { is_expected.to be_disallowed(:fork_project) }
     end
 
     context 'project member' do
-      let_it_be(:project) { create(:project, :private) }
+      let(:project) { private_project }
 
       context 'guest' do
         let(:current_user) { guest }
@@ -384,10 +362,8 @@ RSpec.describe ProjectPolicy do
   end
 
   describe 'update_max_artifacts_size' do
-    subject { described_class.new(current_user, project) }
-
     context 'when no user' do
-      let(:current_user) { nil }
+      let(:current_user) { anonymous }
 
       it { expect_disallowed(:update_max_artifacts_size) }
     end
@@ -416,12 +392,10 @@ RSpec.describe ProjectPolicy do
   context 'alert bot' do
     let(:current_user) { User.alert_bot }
 
-    subject { described_class.new(current_user, project) }
-
     it { is_expected.to be_allowed(:reporter_access) }
 
     context 'within a private project' do
-      let(:project) { create(:project, :private) }
+      let(:project) { private_project }
 
       it { is_expected.to be_allowed(:admin_issue) }
     end
@@ -430,8 +404,6 @@ RSpec.describe ProjectPolicy do
   context 'support bot' do
     let(:current_user) { User.support_bot }
 
-    subject { described_class.new(current_user, project) }
-
     context 'with service desk disabled' do
       it { expect_allowed(:guest_access) }
       it { expect_disallowed(:create_note, :read_project) }
@@ -455,8 +427,6 @@ RSpec.describe ProjectPolicy do
   end
 
   describe 'read_prometheus_alerts' do
-    subject { described_class.new(current_user, project) }
-
     context 'with admin' do
       let(:current_user) { admin }
 
@@ -500,17 +470,15 @@ RSpec.describe ProjectPolicy do
     end
 
     context 'with anonymous' do
-      let(:current_user) { nil }
+      let(:current_user) { anonymous }
 
       it { is_expected.to be_disallowed(:read_prometheus_alerts) }
     end
   end
 
   describe 'metrics_dashboard feature' do
-    subject { described_class.new(current_user, project) }
-
     context 'public project' do
-      let(:project) { create(:project, :public) }
+      let(:project) { public_project }
 
       context 'feature private' do
         context 'with reporter' do
@@ -530,7 +498,7 @@ RSpec.describe ProjectPolicy do
         end
 
         context 'with anonymous' do
-          let(:current_user) { nil }
+          let(:current_user) { anonymous }
 
           it { is_expected.to be_disallowed(:metrics_dashboard) }
         end
@@ -562,7 +530,7 @@ RSpec.describe ProjectPolicy do
         end
 
         context 'with anonymous' do
-          let(:current_user) { nil }
+          let(:current_user) { anonymous }
 
           it { is_expected.to be_allowed(:metrics_dashboard) }
           it { is_expected.to be_allowed(:read_prometheus) }
@@ -574,7 +542,7 @@ RSpec.describe ProjectPolicy do
     end
 
     context 'internal project' do
-      let(:project) { create(:project, :internal) }
+      let(:project) { internal_project }
 
       context 'feature private' do
         context 'with reporter' do
@@ -594,7 +562,7 @@ RSpec.describe ProjectPolicy do
         end
 
         context 'with anonymous' do
-          let(:current_user) { nil }
+          let(:current_user) { anonymous }
 
           it { is_expected.to be_disallowed(:metrics_dashboard)}
         end
@@ -626,7 +594,7 @@ RSpec.describe ProjectPolicy do
         end
 
         context 'with anonymous' do
-          let(:current_user) { nil }
+          let(:current_user) { anonymous }
 
           it { is_expected.to be_disallowed(:metrics_dashboard) }
         end
@@ -634,7 +602,7 @@ RSpec.describe ProjectPolicy do
     end
 
     context 'private project' do
-      let(:project) { create(:project, :private) }
+      let(:project) { private_project }
 
       context 'feature private' do
         context 'with reporter' do
@@ -654,7 +622,7 @@ RSpec.describe ProjectPolicy do
         end
 
         context 'with anonymous' do
-          let(:current_user) { nil }
+          let(:current_user) { anonymous }
 
           it { is_expected.to be_disallowed(:metrics_dashboard) }
         end
@@ -678,7 +646,7 @@ RSpec.describe ProjectPolicy do
         end
 
         context 'with anonymous' do
-          let(:current_user) { nil }
+          let(:current_user) { anonymous }
 
           it { is_expected.to be_disallowed(:metrics_dashboard) }
         end
@@ -703,7 +671,7 @@ RSpec.describe ProjectPolicy do
       end
 
       context 'with anonymous' do
-        let(:current_user) { nil }
+        let(:current_user) { anonymous }
 
         it { is_expected.to be_disallowed(:metrics_dashboard) }
       end
@@ -735,8 +703,6 @@ RSpec.describe ProjectPolicy do
   end
 
   describe 'create_web_ide_terminal' do
-    subject { described_class.new(current_user, project) }
-
     context 'with admin' do
       let(:current_user) { admin }
 
@@ -780,20 +746,20 @@ RSpec.describe ProjectPolicy do
     end
 
     context 'with non member' do
-      let(:current_user) { create(:user) }
+      let(:current_user) { non_member }
 
       it { is_expected.to be_disallowed(:create_web_ide_terminal) }
     end
 
     context 'with anonymous' do
-      let(:current_user) { nil }
+      let(:current_user) { anonymous }
 
       it { is_expected.to be_disallowed(:create_web_ide_terminal) }
     end
   end
 
   describe 'read_repository_graphs' do
-    subject { described_class.new(guest, project) }
+    let(:current_user) { guest }
 
     before do
       allow(subject).to receive(:allowed?).with(:read_repository_graphs).and_call_original
@@ -814,7 +780,7 @@ RSpec.describe ProjectPolicy do
   end
 
   describe 'design permissions' do
-    subject { described_class.new(guest, project) }
+    let(:current_user) { guest }
 
     let(:design_permissions) do
       %i[read_design_activity read_design]
@@ -836,7 +802,7 @@ RSpec.describe ProjectPolicy do
   end
 
   describe 'read_build_report_results' do
-    subject { described_class.new(guest, project) }
+    let(:current_user) { guest }
 
     before do
       allow(subject).to receive(:allowed?).with(:read_build_report_results).and_call_original
@@ -874,8 +840,6 @@ RSpec.describe ProjectPolicy do
   end
 
   describe 'read_package' do
-    subject { described_class.new(current_user, project) }
-
     context 'with admin' do
       let(:current_user) { admin }
 
@@ -926,13 +890,13 @@ RSpec.describe ProjectPolicy do
     end
 
     context 'with non member' do
-      let(:current_user) { create(:user) }
+      let(:current_user) { non_member }
 
       it { is_expected.to be_allowed(:read_package) }
     end
 
     context 'with anonymous' do
-      let(:current_user) { nil }
+      let(:current_user) { anonymous }
 
       it { is_expected.to be_allowed(:read_package) }
     end

spec/support/shared_contexts/policies/project_policy_shared_context.rb

 # frozen_string_literal: true
 
 RSpec.shared_context 'ProjectPolicy context' do
+  let_it_be(:anonymous) { nil }
   let_it_be(:guest) { create(:user) }
   let_it_be(:reporter) { create(:user) }
   let_it_be(:developer) { create(:user) }
   let_it_be(:maintainer) { create(:user) }
   let_it_be(:owner) { create(:user) }
   let_it_be(:admin) { create(:admin) }
-  let(:project) { create(:project, :public, namespace: owner.namespace) }
+  let_it_be(:non_member) { create(:user) }
+  let_it_be_with_refind(:private_project) { create(:project, :private, namespace: owner.namespace) }
+  let_it_be_with_refind(:internal_project) { create(:project, :internal, namespace: owner.namespace) }
+  let_it_be_with_refind(:public_project) { create(:project, :public, namespace: owner.namespace) }
 
   let(:base_guest_permissions) do
     %i[
@@ -86,10 +90,12 @@ RSpec.shared_context 'ProjectPolicy context' do
   let(:maintainer_permissions) { base_maintainer_permissions + additional_maintainer_permissions }
   let(:owner_permissions) { base_owner_permissions + additional_owner_permissions }
 
-  before do
+  before_all do
+    [private_project, internal_project, public_project].each do |project|
       project.add_guest(guest)
-    project.add_maintainer(maintainer)
-    project.add_developer(developer)
       project.add_reporter(reporter)
+      project.add_developer(developer)
+      project.add_maintainer(maintainer)
+    end
   end
 end

spec/support/shared_examples/policies/project_policy_shared_examples.rb

@@ -59,8 +59,7 @@ RSpec.shared_examples 'project policies as anonymous' do
       let(:project) { create(:project, :public, namespace: group) }
       let(:user_permissions) { [:create_merge_request_in, :create_project, :create_issue, :create_note, :upload_file, :award_emoji] }
       let(:anonymous_permissions) { guest_permissions - user_permissions }
-
-      subject { described_class.new(nil, project) }
+      let(:current_user) { anonymous }
 
       before do
         create(:group_member, :invited, group: group)
@@ -78,9 +77,8 @@ RSpec.shared_examples 'project policies as anonymous' do
   end
 
   context 'abilities for non-public projects' do
-    let(:project) { create(:project, namespace: owner.namespace) }
-
-    subject { described_class.new(nil, project) }
+    let(:project) { private_project }
+    let(:current_user) { anonymous }
 
     it { is_expected.to be_banned }
   end
@@ -109,10 +107,10 @@ RSpec.shared_examples 'deploy token does not get confused with user' do
 end
 
 RSpec.shared_examples 'project policies as guest' do
-  subject { described_class.new(guest, project) }
-
   context 'abilities for non-public projects' do
-    let(:project) { create(:project, namespace: owner.namespace) }
+    let(:project) { private_project }
+    let(:current_user) { guest }
+
     let(:reporter_public_build_permissions) do
       reporter_permissions - [:read_build, :read_pipeline]
     end
@@ -167,9 +165,8 @@ end
 
 RSpec.shared_examples 'project policies as reporter' do
   context 'abilities for non-public projects' do
-    let(:project) { create(:project, namespace: owner.namespace) }
-
-    subject { described_class.new(reporter, project) }
+    let(:project) { private_project }
+    let(:current_user) { reporter }
 
     it do
       expect_allowed(*guest_permissions)
@@ -192,9 +189,8 @@ end
 
 RSpec.shared_examples 'project policies as developer' do
   context 'abilities for non-public projects' do
-    let(:project) { create(:project, namespace: owner.namespace) }
-
-    subject { described_class.new(developer, project) }
+    let(:project) { private_project }
+    let(:current_user) { developer }
 
     it do
       expect_allowed(*guest_permissions)
@@ -217,9 +213,8 @@ end
 
 RSpec.shared_examples 'project policies as maintainer' do
   context 'abilities for non-public projects' do
-    let(:project) { create(:project, namespace: owner.namespace) }
-
-    subject { described_class.new(maintainer, project) }
+    let(:project) { private_project }
+    let(:current_user) { maintainer }
 
     it do
       expect_allowed(*guest_permissions)
@@ -242,9 +237,8 @@ end
 
 RSpec.shared_examples 'project policies as owner' do
   context 'abilities for non-public projects' do
-    let(:project) { create(:project, namespace: owner.namespace) }
-
-    subject { described_class.new(owner, project) }
+    let(:project) { private_project }
+    let(:current_user) { owner }
 
     it do
       expect_allowed(*guest_permissions)
@@ -267,9 +261,8 @@ end
 
 RSpec.shared_examples 'project policies as admin with admin mode' do
   context 'abilities for non-public projects', :enable_admin_mode do
-    let(:project) { create(:project, namespace: owner.namespace) }
-
-    subject { described_class.new(admin, project) }
+    let(:project) { private_project }
+    let(:current_user) { admin }
 
     it do
       expect_allowed(*guest_permissions)
@@ -316,9 +309,8 @@ end
 
 RSpec.shared_examples 'project policies as admin without admin mode' do
   context 'abilities for non-public projects' do
-    let(:project) { create(:project, namespace: owner.namespace) }
-
-    subject { described_class.new(admin, project) }
+    let(:project) { private_project }
+    let(:current_user) { admin }
 
     it { is_expected.to be_banned }