Add latest changes from gitlab-org/gitlab@master

8 files changed, 208 insertions, 41 deletions

diff --git a/spec/finders/repositories/tree_finder_spec.rb b/spec/finders/repositories/tree_finder_spec.rb
index 0d70d5f92d3..42b4047c4e8 100644
--- a/spec/finders/repositories/tree_finder_spec.rb
+++ b/spec/finders/repositories/tree_finder_spec.rb
@@ -26,10 +26,10 @@ RSpec.describe Repositories::TreeFinder do
     end
 
     it "accepts a gitaly_pagination argument" do
-      expect(repository).to receive(:tree).with(anything, anything, recursive: nil, pagination_params: { limit: 20, page_token: nil }).and_call_original
+      expect(repository).to receive(:tree).with(anything, anything, recursive: nil, rescue_not_found: nil, pagination_params: { limit: 20, page_token: nil }).and_call_original
       expect(tree_finder.execute(gitaly_pagination: true)).to be_an(Array)
 
-      expect(repository).to receive(:tree).with(anything, anything, recursive: nil).and_call_original
+      expect(repository).to receive(:tree).with(anything, anything, recursive: nil, rescue_not_found: nil).and_call_original
       expect(tree_finder.execute(gitaly_pagination: false)).to be_an(Array)
     end
 
diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb
index 4a20e0b1156..84ab8376fe1 100644
--- a/spec/lib/gitlab/git/tree_spec.rb
+++ b/spec/lib/gitlab/git/tree_spec.rb
@@ -9,13 +9,14 @@ RSpec.describe Gitlab::Git::Tree do
   let(:repository) { project.repository.raw }
 
   shared_examples 'repo' do
-    subject(:tree) { Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, pagination_params) }
+    subject(:tree) { Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, rescue_not_found, pagination_params) }
 
     let(:sha) { SeedRepo::Commit::ID }
     let(:path) { nil }
     let(:recursive) { false }
     let(:pagination_params) { nil }
     let(:skip_flat_paths) { false }
+    let(:rescue_not_found) { true }
 
     let(:entries) { tree.first }
     let(:cursor) { tree.second }
@@ -30,8 +31,14 @@ RSpec.describe Gitlab::Git::Tree do
     context 'with an invalid ref' do
       let(:sha) { 'foobar-does-not-exist' }
 
-      it { expect(entries).to eq([]) }
-      it { expect(cursor).to be_nil }
+      context 'when handle_structured_gitaly_errors feature is disabled' do
+        before do
+          stub_feature_flags(handle_structured_gitaly_errors: false)
+        end
+
+        it { expect(entries).to eq([]) }
+        it { expect(cursor).to be_nil }
+      end
     end
 
     context 'when path is provided' do
@@ -162,11 +169,23 @@ RSpec.describe Gitlab::Git::Tree do
       end
 
       context 'and invalid reference is used' do
-        it 'returns no entries and nil cursor' do
+        before do
           allow(repository.gitaly_commit_client).to receive(:tree_entries).and_raise(Gitlab::Git::Index::IndexError)
+        end
+
+        context 'when rescue_not_found is set to false' do
+          let(:rescue_not_found) { false }
 
-          expect(entries.count).to eq(0)
-          expect(cursor).to be_nil
+          it 'raises an IndexError error' do
+            expect { entries }.to raise_error(Gitlab::Git::Index::IndexError)
+          end
+        end
+
+        context 'when rescue_not_found is set to true' do
+          it 'returns no entries and nil cursor' do
+            expect(entries.count).to eq(0)
+            expect(cursor).to be_nil
+          end
         end
       end
     end
@@ -196,7 +215,7 @@ RSpec.describe Gitlab::Git::Tree do
           let(:entries_count) { entries.count }
 
           it 'returns all entries without a cursor' do
-            result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, { limit: entries_count, page_token: nil })
+            result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, rescue_not_found, { limit: entries_count, page_token: nil })
 
             expect(cursor).to be_nil
             expect(result.entries.count).to eq(entries_count)
@@ -225,7 +244,7 @@ RSpec.describe Gitlab::Git::Tree do
           let(:entries_count) { entries.count }
 
           it 'returns all entries' do
-            result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, { limit: -1, page_token: nil })
+            result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, rescue_not_found, { limit: -1, page_token: nil })
 
             expect(result.count).to eq(entries_count)
             expect(cursor).to be_nil
@@ -236,7 +255,7 @@ RSpec.describe Gitlab::Git::Tree do
             let(:token) { entries.second.id }
 
             it 'returns all entries after token' do
-              result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, { limit: -1, page_token: token })
+              result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, rescue_not_found, { limit: -1, page_token: token })
 
               expect(result.count).to eq(entries.count - 2)
               expect(cursor).to be_nil
@@ -268,7 +287,7 @@ RSpec.describe Gitlab::Git::Tree do
           expected_entries = entries
 
           loop do
-            result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, { limit: 5, page_token: token })
+            result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, rescue_not_found, { limit: 5, page_token: token })
 
             collected_entries += result.entries
             token = cursor&.next_cursor
diff --git a/spec/policies/organizations/organization_policy_spec.rb b/spec/policies/organizations/organization_policy_spec.rb
index 9914ce455d3..e51362227c9 100644
--- a/spec/policies/organizations/organization_policy_spec.rb
+++ b/spec/policies/organizations/organization_policy_spec.rb
@@ -7,6 +7,12 @@ RSpec.describe Organizations::OrganizationPolicy, feature_category: :cell do
 
   subject(:policy) { described_class.new(current_user, organization) }
 
+  context 'when the user is anonymous' do
+    let_it_be(:current_user) { nil }
+
+    it { is_expected.to be_allowed(:read_organization) }
+  end
+
   context 'when the user is an admin' do
     let_it_be(:current_user) { create(:user, :admin) }
 
@@ -17,7 +23,7 @@ RSpec.describe Organizations::OrganizationPolicy, feature_category: :cell do
 
     context 'when admin mode is disabled' do
       it { is_expected.to be_disallowed(:admin_organization) }
-      it { is_expected.to be_disallowed(:read_organization) }
+      it { is_expected.to be_allowed(:read_organization) }
     end
   end
 
@@ -30,11 +36,4 @@ RSpec.describe Organizations::OrganizationPolicy, feature_category: :cell do
 
     it { is_expected.to be_allowed(:read_organization) }
   end
-
-  context 'when the user is not an organization user' do
-    let_it_be(:current_user) { create :user }
-
-    it { is_expected.to be_disallowed(:admin_organization) }
-    it { is_expected.to be_disallowed(:read_organization) }
-  end
 end
diff --git a/spec/presenters/packages/nuget/v2/metadata_index_presenter_spec.rb b/spec/presenters/packages/nuget/v2/metadata_index_presenter_spec.rb
new file mode 100644
index 00000000000..598db641b75
--- /dev/null
+++ b/spec/presenters/packages/nuget/v2/metadata_index_presenter_spec.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Packages::Nuget::V2::MetadataIndexPresenter, feature_category: :package_registry do
+  describe '#xml' do
+    let(:presenter) { described_class.new }
+
+    subject(:xml) { Nokogiri::XML(presenter.xml.to_xml) }
+
+    specify { expect(xml.root.name).to eq('Edmx') }
+
+    specify { expect(xml.at_xpath('//edmx:Edmx')).to be_present }
+
+    specify { expect(xml.at_xpath('//edmx:Edmx/edmx:DataServices')).to be_present }
+
+    specify do
+      expect(xml.css('*').map(&:name)).to include(
+        'Schema', 'EntityType', 'Key', 'PropertyRef', 'EntityContainer', 'EntitySet', 'FunctionImport', 'Parameter'
+      )
+    end
+
+    specify do
+      expect(xml.css('*').select { |el| el.name == 'Property' }.map { |el| el.attribute_nodes.first.value })
+        .to match_array(
+          %w[Id Version Authors Dependencies Description DownloadCount IconUrl Published ProjectUrl Tags Title
+            LicenseUrl]
+        )
+    end
+
+    specify { expect(xml.css('*').detect { |el| el.name == 'EntityContainer' }.attr('Name')).to eq('V2FeedContext') }
+
+    specify { expect(xml.css('*').detect { |el| el.name == 'FunctionImport' }.attr('Name')).to eq('FindPackagesById') }
+  end
+end
diff --git a/spec/requests/api/nuget_project_packages_spec.rb b/spec/requests/api/nuget_project_packages_spec.rb
index 2d3781da42b..da74409cd77 100644
--- a/spec/requests/api/nuget_project_packages_spec.rb
+++ b/spec/requests/api/nuget_project_packages_spec.rb
@@ -50,6 +50,44 @@ RSpec.describe API::NugetProjectPackages, feature_category: :package_registry do
     it_behaves_like 'accept get request on private project with access to package registry for everyone'
   end
 
+  describe 'GET /api/v4/projects/:id/packages/nuget/v2/$metadata' do
+    let(:url) { "/projects/#{target.id}/packages/nuget/v2/$metadata" }
+
+    subject(:api_request) { get api(url) }
+
+    it { is_expected.to have_request_urgency(:low) }
+
+    context 'with valid target' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:visibility_level, :user_role, :member, :expected_status) do
+        'PUBLIC'   | :developer  | true  | :success
+        'PUBLIC'   | :guest      | true  | :success
+        'PUBLIC'   | :developer  | false | :success
+        'PUBLIC'   | :guest      | false | :success
+        'PUBLIC'   | :anonymous  | false | :success
+        'PRIVATE'  | :developer  | true  | :success
+        'PRIVATE'  | :guest      | true  | :success
+        'PRIVATE'  | :developer  | false | :success
+        'PRIVATE'  | :guest      | false | :success
+        'PRIVATE'  | :anonymous  | false | :success
+        'INTERNAL' | :developer  | true  | :success
+        'INTERNAL' | :guest      | true  | :success
+        'INTERNAL' | :developer  | false | :success
+        'INTERNAL' | :guest      | false | :success
+        'INTERNAL' | :anonymous  | false | :success
+      end
+
+      with_them do
+        before do
+          update_visibility_to(Gitlab::VisibilityLevel.const_get(visibility_level, false))
+        end
+
+        it_behaves_like 'process nuget v2 $metadata service request', params[:user_role], params[:expected_status], params[:member]
+      end
+    end
+  end
+
   describe 'GET /api/v4/projects/:id/packages/nuget/metadata/*package_name/index' do
     let(:url) { "/projects/#{target.id}/packages/nuget/metadata/#{package_name}/index.json" }
 
diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb
index 8853eff0b3e..a94ed63bf47 100644
--- a/spec/requests/api/repositories_spec.rb
+++ b/spec/requests/api/repositories_spec.rb
@@ -37,6 +37,52 @@ RSpec.describe API::Repositories, feature_category: :source_code_management do
         end
       end
 
+      context 'when path does not exist' do
+        let(:path) { 'bogus' }
+
+        context 'when handle_structured_gitaly_errors feature is disabled' do
+          before do
+            stub_feature_flags(handle_structured_gitaly_errors: false)
+          end
+
+          it 'returns an empty array' do
+            get api("#{route}?path=#{path}", current_user)
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(response).to include_pagination_headers
+            expect(json_response).to be_an(Array)
+            expect(json_response).to be_an_empty
+          end
+        end
+
+        context 'when handle_structured_gitaly_errors feature is enabled' do
+          before do
+            stub_feature_flags(handle_structured_gitaly_errors: true)
+          end
+
+          it_behaves_like '404 response' do
+            let(:request) { get api("#{route}?path=#{path}", current_user) }
+            let(:message) { '404 invalid revision or path Not Found' }
+          end
+        end
+      end
+
+      context 'when path is empty directory ' do
+        context 'when handle_structured_gitaly_errors feature is disabled' do
+          before do
+            stub_feature_flags(handle_structured_gitaly_errors: false)
+          end
+
+          it 'returns an empty array' do
+            get api(route, current_user)
+
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(response).to include_pagination_headers
+            expect(json_response).to be_an(Array)
+          end
+        end
+      end
+
       context 'when repository is disabled' do
         include_context 'disabled repository'
 
diff --git a/spec/requests/organizations/organizations_controller_spec.rb b/spec/requests/organizations/organizations_controller_spec.rb
index 1dc3c3eaaa3..788d740504a 100644
--- a/spec/requests/organizations/organizations_controller_spec.rb
+++ b/spec/requests/organizations/organizations_controller_spec.rb
@@ -26,38 +26,40 @@ RSpec.describe Organizations::OrganizationsController, feature_category: :cell d
   end
 
   shared_examples 'basic organization controller action' do
-    before do
-      sign_in(user)
+    context 'when the user is not logged in' do
+      it_behaves_like 'successful response'
+      it_behaves_like 'action disabled by `ui_for_organizations` feature flag'
     end
 
-    context 'when the user does not have authorization' do
-      let_it_be(:user) { create(:user) }
+    context 'when the user is logged in' do
+      before do
+        sign_in(user)
+      end
 
-      it 'renders 404' do
-        gitlab_request
+      context 'with no association to an organization' do
+        let_it_be(:user) { create(:user) }
 
-        expect(response).to have_gitlab_http_status(:not_found)
+        it_behaves_like 'successful response'
+        it_behaves_like 'action disabled by `ui_for_organizations` feature flag'
       end
 
-      it_behaves_like 'action disabled by `ui_for_organizations` feature flag'
-    end
+      context 'as as admin', :enable_admin_mode do
+        let_it_be(:user) { create(:admin) }
 
-    context 'when the user is an admin', :enable_admin_mode do
-      let_it_be(:user) { create(:admin) }
+        it_behaves_like 'successful response'
+        it_behaves_like 'action disabled by `ui_for_organizations` feature flag'
+      end
 
-      it_behaves_like 'successful response'
-      it_behaves_like 'action disabled by `ui_for_organizations` feature flag'
-    end
+      context 'as an organization user' do
+        let_it_be(:user) { create :user }
 
-    context 'when the user is an organization user' do
-      let_it_be(:user) { create :user }
+        before do
+          create :organization_user, organization: organization, user: user
+        end
 
-      before do
-        create :organization_user, organization: organization, user: user
+        it_behaves_like 'successful response'
+        it_behaves_like 'action disabled by `ui_for_organizations` feature flag'
       end
-
-      it_behaves_like 'successful response'
-      it_behaves_like 'action disabled by `ui_for_organizations` feature flag'
     end
   end
 
diff --git a/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb
index 5854958a06e..2e66bae26ba 100644
--- a/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb
+++ b/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb
@@ -51,6 +51,34 @@ RSpec.shared_examples 'process nuget service index request' do |user_type, statu
   end
 end
 
+RSpec.shared_examples 'process nuget v2 $metadata service request' do |user_type, status, add_member = true|
+  context "for user type #{user_type}" do
+    before do
+      target.send("add_#{user_type}", user) if add_member && user_type != :anonymous
+    end
+
+    it_behaves_like 'returning response status', status
+
+    it 'returns a valid xml response' do
+      api_request
+
+      doc = Nokogiri::XML(body)
+
+      expect(response.media_type).to eq('application/xml')
+      expect(doc.at_xpath('//edmx:Edmx')).to be_present
+      expect(doc.at_xpath('//edmx:Edmx/edmx:DataServices')).to be_present
+      expect(doc.css('*').map(&:name)).to include(
+        'Schema', 'EntityType', 'Key', 'PropertyRef', 'EntityContainer', 'EntitySet', 'FunctionImport', 'Parameter'
+      )
+      expect(doc.css('*').select { |el| el.name == 'Property' }.map { |el| el.attribute_nodes.first.value })
+        .to match_array(%w[Id Version Authors Dependencies Description DownloadCount IconUrl Published ProjectUrl
+          Tags Title LicenseUrl]
+                       )
+      expect(doc.css('*').detect { |el| el.name == 'FunctionImport' }.attr('Name')).to eq('FindPackagesById')
+    end
+  end
+end
+
 RSpec.shared_examples 'returning nuget metadata json response with json schema' do |json_schema|
   it 'returns a valid json response' do
     subject