Merge branch '33685-refactor-npm-requests-specs' into 'master'

Refactor NPM requests specs for shared endpoints

See merge request gitlab-org/gitlab!53491

spec/requests/api/npm_instance_packages_spec.rb

@@ -6,7 +6,7 @@ RSpec.describe API::NpmInstancePackages do
   include_context 'npm api setup'
 
   describe 'GET /api/v4/packages/npm/*package_name' do
-    it_behaves_like 'handling get metadata requests' do
+    it_behaves_like 'handling get metadata requests', scope: :instance do
       let(:url) { api("/packages/npm/#{package_name}") }
     end
   end

spec/requests/api/npm_project_packages_spec.rb

@@ -6,25 +6,25 @@ RSpec.describe API::NpmProjectPackages do
   include_context 'npm api setup'
 
   describe 'GET /api/v4/projects/:id/packages/npm/*package_name' do
-    it_behaves_like 'handling get metadata requests' do
+    it_behaves_like 'handling get metadata requests', scope: :project do
       let(:url) { api("/projects/#{project.id}/packages/npm/#{package_name}") }
     end
   end
 
   describe 'GET /api/v4/projects/:id/packages/npm/-/package/*package_name/dist-tags' do
-    it_behaves_like 'handling get dist tags requests' do
+    it_behaves_like 'handling get dist tags requests', scope: :project do
       let(:url) { api("/projects/#{project.id}/packages/npm/-/package/#{package_name}/dist-tags") }
     end
   end
 
   describe 'PUT /api/v4/projects/:id/packages/npm/-/package/*package_name/dist-tags/:tag' do
-    it_behaves_like 'handling create dist tag requests' do
+    it_behaves_like 'handling create dist tag requests', scope: :project do
       let(:url) { api("/projects/#{project.id}/packages/npm/-/package/#{package_name}/dist-tags/#{tag_name}") }
     end
   end
 
   describe 'DELETE /api/v4/projects/:id/packages/npm/-/package/*package_name/dist-tags/:tag' do
-    it_behaves_like 'handling delete dist tag requests' do
+    it_behaves_like 'handling delete dist tag requests', scope: :project do
       let(:url) { api("/projects/#{project.id}/packages/npm/-/package/#{package_name}/dist-tags/#{tag_name}") }
     end
   end
@@ -32,10 +32,14 @@ RSpec.describe API::NpmProjectPackages do
   describe 'GET /api/v4/projects/:id/packages/npm/*package_name/-/*file_name' do
     let_it_be(:package_file) { package.package_files.first }
 
-    let(:params) { {} }
-    let(:url) { api("/projects/#{project.id}/packages/npm/#{package_file.package.name}/-/#{package_file.file_name}") }
+    let(:headers) { {} }
+    let(:url) { api("/projects/#{project.id}/packages/npm/#{package.name}/-/#{package_file.file_name}") }
 
-    subject { get(url, params: params) }
+    subject { get(url, headers: headers) }
+
+    before do
+      project.add_developer(user)
+    end
 
     shared_examples 'a package file that requires auth' do
       it 'denies download with no token' do
@@ -45,7 +49,7 @@ RSpec.describe API::NpmProjectPackages do
       end
 
       context 'with access token' do
-        let(:params) { { access_token: token.token } }
+        let(:headers) { build_token_auth_header(token.token) }
 
         it 'returns the file' do
           subject
@@ -56,7 +60,7 @@ RSpec.describe API::NpmProjectPackages do
       end
 
       context 'with job token' do
-        let(:params) { { job_token: job.token } }
+        let(:headers) { build_token_auth_header(job.token) }
 
         it 'returns the file' do
           subject
@@ -86,7 +90,7 @@ RSpec.describe API::NpmProjectPackages do
       it_behaves_like 'a package file that requires auth'
 
       context 'with guest' do
-        let(:params) { { access_token: token.token } }
+        let(:headers) { build_token_auth_header(token.token) }
 
         it 'denies download when not enough permissions' do
           project.add_guest(user)
@@ -108,7 +112,11 @@ RSpec.describe API::NpmProjectPackages do
   end
 
   describe 'PUT /api/v4/projects/:id/packages/npm/:package_name' do
-    RSpec.shared_examples 'handling invalid record with 400 error' do
+    before do
+      project.add_developer(user)
+    end
+
+    shared_examples 'handling invalid record with 400 error' do
       it 'handles an ActiveRecord::RecordInvalid exception with 400 error' do
         expect { upload_package_with_token(package_name, params) }
           .not_to change { project.packages.count }
@@ -261,7 +269,9 @@ RSpec.describe API::NpmProjectPackages do
     end
 
     def upload_package(package_name, params = {})
-      put api("/projects/#{project.id}/packages/npm/#{package_name.sub('/', '%2f')}"), params: params
+      token = params.delete(:access_token) || params.delete(:job_token)
+      headers = build_token_auth_header(token)
+      put api("/projects/#{project.id}/packages/npm/#{package_name.sub('/', '%2f')}"), params: params, headers: headers
     end
 
     def upload_package_with_token(package_name, params = {})

spec/support/shared_contexts/requests/api/npm_packages_shared_context.rb

@@ -4,10 +4,10 @@ RSpec.shared_context 'npm api setup' do
   include PackagesManagerApiSpecHelpers
   include HttpBasicAuthHelpers
 
-  let_it_be(:user) { create(:user) }
+  let_it_be(:user, reload: true) { create(:user) }
   let_it_be(:group) { create(:group) }
   let_it_be(:project, reload: true) { create(:project, :public, namespace: group) }
-  let_it_be(:package, reload: true) { create(:npm_package, project: project) }
+  let_it_be(:package, reload: true) { create(:npm_package, project: project, name: "@#{group.path}/scoped_package") }
   let_it_be(:token) { create(:oauth_access_token, scopes: 'api', resource_owner: user) }
   let_it_be(:personal_access_token) { create(:personal_access_token, user: user) }
   let_it_be(:job, reload: true) { create(:ci_build, user: user, status: :running) }
@@ -15,8 +15,15 @@ RSpec.shared_context 'npm api setup' do
   let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) }
 
   let(:package_name) { package.name }
+end
 
-  before do
-    project.add_developer(user)
+RSpec.shared_context 'set package name from package name type' do
+  let(:package_name) do
+    case package_name_type
+    when :scoped_naming_convention
+      "@#{group.path}/scoped-package"
+    when :non_existing
+      'non-existing-package'
+    end
   end
 end

spec/support/shared_examples/requests/api/packages_tags_shared_examples.rb → spec/support/shared_examples/requests/api/npm_packages_tags_shared_examples.rb

 # frozen_string_literal: true
 
-RSpec.shared_examples 'rejects package tags access' do |user_type, status|
-  context "for user type #{user_type}" do
-    before do
-      project.send("add_#{user_type}", user) unless user_type == :no_type
-    end
-
-    it_behaves_like 'returning response status', status
+RSpec.shared_examples 'rejects package tags access' do |status:|
+  before do
+    package.update!(name: package_name) unless package_name == 'non-existing-package'
   end
+
+  it_behaves_like 'returning response status', status
 end
 
-RSpec.shared_examples 'returns package tags' do |user_type|
+RSpec.shared_examples 'accept package tags request' do |status:|
   using RSpec::Parameterized::TableSyntax
 
   before do
     stub_application_setting(npm_package_requests_forwarding: false)
-    project.send("add_#{user_type}", user) unless user_type == :no_type
   end
 
-  it_behaves_like 'returning response status', :success
+  context 'with valid package name' do
+    before do
+      package.update!(name: package_name) unless package_name == 'non-existing-package'
+    end
 
-  it 'returns a valid json response' do
-    subject
+    it_behaves_like 'returning response status', status
 
-    expect(response.media_type).to eq('application/json')
-    expect(json_response).to be_a(Hash)
-  end
+    it 'returns a valid json response' do
+      subject
 
-  it 'returns two package tags' do
-    subject
+      expect(response.media_type).to eq('application/json')
+      expect(json_response).to be_a(Hash)
+    end
 
-    expect(json_response).to match_schema('public_api/v4/packages/npm_package_tags')
-    expect(json_response.length).to eq(3) # two tags + latest (auto added)
-    expect(json_response[package_tag1.name]).to eq(package.version)
-    expect(json_response[package_tag2.name]).to eq(package.version)
-    expect(json_response['latest']).to eq(package.version)
+    it 'returns two package tags' do
+      subject
+
+      expect(json_response).to match_schema('public_api/v4/packages/npm_package_tags')
+      expect(json_response.length).to eq(3) # two tags + latest (auto added)
+      expect(json_response[package_tag1.name]).to eq(package.version)
+      expect(json_response[package_tag2.name]).to eq(package.version)
+      expect(json_response['latest']).to eq(package.version)
+    end
   end
 
   context 'with invalid package name' do
@@ -49,47 +52,49 @@ RSpec.shared_examples 'returns package tags' do |user_type|
   end
 end
 
-RSpec.shared_examples 'create package tag' do |user_type|
+RSpec.shared_examples 'accept create package tag request' do |user_type|
   using RSpec::Parameterized::TableSyntax
 
-  before do
-    project.send("add_#{user_type}", user) unless user_type == :no_type
-  end
+  context 'with valid package name' do
+    before do
+      package.update!(name: package_name) unless package_name == 'non-existing-package'
+    end
 
-  it_behaves_like 'returning response status', :no_content
+    it_behaves_like 'returning response status', :no_content
 
-  it 'creates the package tag' do
-    expect { subject }.to change { Packages::Tag.count }.by(1)
+    it 'creates the package tag' do
+      expect { subject }.to change { Packages::Tag.count }.by(1)
 
-    last_tag = Packages::Tag.last
-    expect(last_tag.name).to eq(tag_name)
-    expect(last_tag.package).to eq(package)
-  end
+      last_tag = Packages::Tag.last
+      expect(last_tag.name).to eq(tag_name)
+      expect(last_tag.package).to eq(package)
+    end
 
-  it 'returns a valid response' do
-    subject
+    it 'returns a valid response' do
+      subject
 
-    expect(response.body).to be_empty
-  end
+      expect(response.body).to be_empty
+    end
 
-  context 'with already existing tag' do
-    let_it_be(:package2) { create(:npm_package, project: project, name: package.name, version: '5.5.55') }
-    let_it_be(:tag) { create(:packages_tag, package: package2, name: tag_name) }
+    context 'with already existing tag' do
+      let_it_be(:package2) { create(:npm_package, project: project, name: package.name, version: '5.5.55') }
+      let_it_be(:tag) { create(:packages_tag, package: package2, name: tag_name) }
 
-    it_behaves_like 'returning response status', :no_content
+      it_behaves_like 'returning response status', :no_content
 
-    it 'reuses existing tag' do
-      expect(package.tags).to be_empty
-      expect(package2.tags).to eq([tag])
-      expect { subject }.to not_change { Packages::Tag.count }
-      expect(package.reload.tags).to eq([tag])
-      expect(package2.reload.tags).to be_empty
-    end
+      it 'reuses existing tag' do
+        expect(package.tags).to be_empty
+        expect(package2.tags).to eq([tag])
+        expect { subject }.to not_change { Packages::Tag.count }
+        expect(package.reload.tags).to eq([tag])
+        expect(package2.reload.tags).to be_empty
+      end
 
-    it 'returns a valid response' do
-      subject
+      it 'returns a valid response' do
+        subject
 
-      expect(response.body).to be_empty
+        expect(response.body).to be_empty
+      end
     end
   end
 
@@ -129,14 +134,14 @@ RSpec.shared_examples 'create package tag' do |user_type|
   end
 end
 
-RSpec.shared_examples 'delete package tag' do |user_type|
+RSpec.shared_examples 'accept delete package tag request' do |user_type|
   using RSpec::Parameterized::TableSyntax
 
-  before do
-    project.send("add_#{user_type}", user) unless user_type == :no_type
-  end
+  context 'with valid package name' do
+    before do
+      package.update!(name: package_name) unless package_name == 'non-existing-package'
+    end
 
-  context "for #{user_type} user" do
     it_behaves_like 'returning response status', :no_content
 
     it 'returns a valid response' do
@@ -157,29 +162,29 @@ RSpec.shared_examples 'delete package tag' do |user_type|
 
       it_behaves_like 'returning response status', :not_found
     end
+  end
 
-    context 'with invalid package name' do
-      where(:package_name, :status) do
-        'unknown' | :not_found
-        ''        | :not_found
-        '%20'     | :bad_request
-      end
+  context 'with invalid package name' do
+    where(:package_name, :status) do
+      'unknown' | :not_found
+      ''        | :not_found
+      '%20'     | :bad_request
+    end
 
-      with_them do
-        it_behaves_like 'returning response status', params[:status]
-      end
+    with_them do
+      it_behaves_like 'returning response status', params[:status]
     end
+  end
 
-    context 'with invalid tag name' do
-      where(:tag_name, :status) do
-        'unknown' | :not_found
-        ''        | :not_found
-        '%20'     | :bad_request
-      end
+  context 'with invalid tag name' do
+    where(:tag_name, :status) do
+      'unknown' | :not_found
+      ''        | :not_found
+      '%20'     | :bad_request
+    end
 
-      with_them do
-        it_behaves_like 'returning response status', params[:status]
-      end
+    with_them do
+      it_behaves_like 'returning response status', params[:status]
     end
   end
 end