Automatic merge of gitlab-org/gitlab-ce master

app/controllers/import/bitbucket_server_controller.rb

@@ -40,7 +40,7 @@ class Import::BitbucketServerController < Import::BaseController
     else
       render json: { errors: 'This namespace has already been taken! Please choose another one.' }, status: :unprocessable_entity
     end
-  rescue BitbucketServer::Client::ServerError => e
+  rescue BitbucketServer::Connection::ConnectionError => e
     render json: { errors: "Unable to connect to server: #{e}" }, status: :unprocessable_entity
   end
 
@@ -62,7 +62,7 @@ class Import::BitbucketServerController < Import::BaseController
     already_added_projects_names = @already_added_projects.pluck(:import_source)
 
     @repos.reject! { |repo| already_added_projects_names.include?(repo.browse_url) }
-  rescue BitbucketServer::Connection::ConnectionError, BitbucketServer::Client::ServerError => e
+  rescue BitbucketServer::Connection::ConnectionError => e
     flash[:alert] = "Unable to connect to server: #{e}"
     clear_session_data
     redirect_to new_import_bitbucket_server_path

app/helpers/release_blog_post_helper.rb

+# frozen_string_literal: true
+
+module ReleaseBlogPostHelper
+  def blog_post_url
+    Gitlab::ReleaseBlogPost.instance.blog_post_url
+  end
+end

app/models/project.rb

@@ -2039,6 +2039,10 @@ class Project < ActiveRecord::Base
     pool_repository&.unlink_repository(repository) && update_column(:pool_repository_id, nil)
   end
 
+  def link_pool_repository
+    pool_repository&.link_repository(repository)
+  end
+
   private
 
   def merge_requests_allowing_collaboration(source_branch = nil)

app/views/layouts/header/_help_dropdown.html.haml

+- show_blog_link = current_user_menu?(:help) && blog_post_url.present?
 %ul
   - if current_user_menu?(:help)
     %li
       = link_to _("Help"), help_path
     %li.divider
+  - if show_blog_link
     %li
-      = link_to _("Submit feedback"), "https://about.gitlab.com/submit-feedback"
+      = link_to _("What's new?"), blog_post_url
+  %li
+    = link_to _("Submit feedback"), "https://about.gitlab.com/submit-feedback"
   - if current_user_menu?(:help) || current_user_menu?(:settings) || current_user_menu?(:profile)
     = render 'shared/user_dropdown_contributing_link'
   - if instance_review_permitted?

app/workers/git_garbage_collect_worker.rb

@@ -23,6 +23,7 @@ class GitGarbageCollectWorker
     end
 
     task = task.to_sym
+    project.link_pool_repository
     gitaly_call(task, project.repository.raw_repository)
 
     # Refresh the branch cache in case garbage collection caused a ref lookup to fail

app/workers/object_pool/join_worker.rb

@@ -5,14 +5,13 @@ module ObjectPool
     include ApplicationWorker
     include ObjectPoolQueue
 
-    def perform(pool_id, project_id)
-      pool = PoolRepository.find_by_id(pool_id)
-      return unless pool&.joinable?
-
+    # The use of pool id is deprecated. Keeping the argument allows old jobs to
+    # still be performed.
+    def perform(_pool_id, project_id)
       project = Project.find_by_id(project_id)
-      return unless project
+      return unless project&.pool_repository&.joinable?
 
-      pool.link_repository(project.repository)
+      project.link_pool_repository
 
       Projects::HousekeepingService.new(project).execute
     end

changelogs/unreleased/25341-add-what-s-new-menu-item-in-top-navigation.yml

+---
+title: Resolve Add What's new menu item in top navigation
+merge_request: 23186
+author:
+type: added

changelogs/unreleased/sh-fix-bitbucket-server-error-handling.yml

+---
+title: Fix Bitbucket Server importer error handling
+merge_request: 24343
+author:
+type: fixed

lib/bitbucket_server/client.rb

@@ -4,18 +4,6 @@ module BitbucketServer
   class Client
     attr_reader :connection
 
-    ServerError = Class.new(StandardError)
-
-    SERVER_ERRORS = [SocketError,
-                     OpenSSL::SSL::SSLError,
-                     Errno::ECONNRESET,
-                     Errno::ECONNREFUSED,
-                     Errno::EHOSTUNREACH,
-                     Net::OpenTimeout,
-                     Net::ReadTimeout,
-                     Gitlab::HTTP::BlockedUrlError,
-                     BitbucketServer::Connection::ConnectionError].freeze
-
     def initialize(options = {})
       @connection = Connection.new(options)
     end
@@ -64,8 +52,6 @@ module BitbucketServer
     def get_collection(path, type, page_offset: 0, limit: nil)
       paginator = BitbucketServer::Paginator.new(connection, Addressable::URI.escape(path), type, page_offset: page_offset, limit: limit)
       BitbucketServer::Collection.new(paginator)
-    rescue *SERVER_ERRORS => e
-      raise ServerError, e
     end
   end
 end

lib/bitbucket_server/connection.rb

@@ -7,6 +7,17 @@ module BitbucketServer
     DEFAULT_API_VERSION = '1.0'
     SEPARATOR = '/'
 
+    NETWORK_ERRORS = [
+      SocketError,
+      OpenSSL::SSL::SSLError,
+      Errno::ECONNRESET,
+      Errno::ECONNREFUSED,
+      Errno::EHOSTUNREACH,
+      Net::OpenTimeout,
+      Net::ReadTimeout,
+      Gitlab::HTTP::BlockedUrlError
+    ].freeze
+
     attr_reader :api_version, :base_uri, :username, :token
 
     ConnectionError = Class.new(StandardError)
@@ -27,6 +38,8 @@ module BitbucketServer
       check_errors!(response)
 
       response.parsed_response
+    rescue *NETWORK_ERRORS => e
+      raise ConnectionError, e
     end
 
     def post(path, body)
@@ -38,6 +51,8 @@ module BitbucketServer
       check_errors!(response)
 
       response.parsed_response
+    rescue *NETWORK_ERRORS => e
+      raise ConnectionError, e
     end
 
     # We need to support two different APIs for deletion:
@@ -55,6 +70,8 @@ module BitbucketServer
       check_errors!(response)
 
       response.parsed_response
+    rescue *NETWORK_ERRORS => e
+      raise ConnectionError, e
     end
 
     private

lib/gitlab.rb

@@ -27,12 +27,52 @@ module Gitlab
     end
   end
 
+  def self.version_info
+    Gitlab::VersionInfo.parse(Gitlab::VERSION)
+  end
+
   COM_URL = 'https://gitlab.com'.freeze
   APP_DIRS_PATTERN = %r{^/?(app|config|ee|lib|spec|\(\w*\))}
   SUBDOMAIN_REGEX = %r{\Ahttps://[a-z0-9]+\.gitlab\.com\z}
   VERSION = File.read(root.join("VERSION")).strip.freeze
   INSTALLATION_TYPE = File.read(root.join("INSTALLATION_TYPE")).strip.freeze
 
+  def self.pre_release?
+    VERSION.include?('pre')
+  end
+
+  def self.final_release?
+    !VERSION.include?('rc') && !pre_release?
+  end
+
+  def self.minor_release
+    "#{version_info.major}.#{version_info.minor}"
+  end
+
+  def self.prev_minor_release
+    "#{version_info.major}.#{version_info.minor - 1}"
+  end
+
+  def self.prev_major_release
+    "#{version_info.major.to_i - 1}"
+  end
+
+  def self.new_major_release?
+    version_info.minor.to_i.zero?
+  end
+
+  def self.previous_release
+    if version_info.minor_version?
+      if version_info.patch_version?
+        minor_release
+      else
+        prev_minor_release
+      end
+    else
+      prev_major_release
+    end
+  end
+
   def self.com?
     # Check `gl_subdomain?` as well to keep parity with gitlab.com
     Gitlab.config.gitlab.url == COM_URL || gl_subdomain?
@@ -49,12 +89,4 @@ module Gitlab
   def self.dev_env_or_com?
     Rails.env.development? || org? || com?
   end
-
-  def self.pre_release?
-    VERSION.include?('pre')
-  end
-
-  def self.version_info
-    Gitlab::VersionInfo.parse(Gitlab::VERSION)
-  end
 end

lib/gitlab/release_blog_post.rb

+# frozen_string_literal: true
+require 'singleton'
+
+module Gitlab
+  class ReleaseBlogPost
+    include Singleton
+
+    RELEASE_RSS_URL = 'https://about.gitlab.com/releases.xml'
+
+    def blog_post_url
+      @url ||= fetch_blog_post_url
+    end
+
+    private
+
+    def fetch_blog_post_url
+      installed_version = Gitlab.final_release? ? Gitlab.minor_release : Gitlab.previous_release
+      response = Gitlab::HTTP.get(RELEASE_RSS_URL, verify: false)
+
+      return unless response.code == 200
+
+      blog_entry = find_installed_blog_entry(response, installed_version)
+      blog_entry['id'] if blog_entry
+    end
+
+    def find_installed_blog_entry(response, installed_version)
+      response['feed']['entry'].find do |entry|
+        entry['release'] == installed_version || matches_previous_release_post(entry['release'], installed_version)
+      end
+    end
+
+    def should_match_previous_release_post?
+      Gitlab.new_major_release? && !Gitlab.final_release?
+    end
+
+    def matches_previous_release_post(rss_release_version, installed_version)
+      should_match_previous_release_post? && rss_release_version[/\d+/] == installed_version
+    end
+  end
+end

lib/gitlab/version_info.rb

@@ -20,6 +20,14 @@ module Gitlab
       @patch = patch
     end
 
+    def minor_version?
+      minor.to_i > 0
+    end
+
+    def patch_version?
+      patch.to_i > 0
+    end
+
     def <=>(other)
       return unless other.is_a? VersionInfo
       return unless valid? && other.valid?

spec/controllers/import/bitbucket_server_controller_spec.rb

@@ -78,7 +78,7 @@ describe Import::BitbucketServerController do
     end
 
     it "returns an error when the server can't be contacted" do
-      expect(client).to receive(:repo).with(project_key, repo_slug).and_raise(BitbucketServer::Client::ServerError)
+      expect(client).to receive(:repo).with(project_key, repo_slug).and_raise(::BitbucketServer::Connection::ConnectionError)
 
       post :create, params: { project: project_key, repository: repo_slug }, format: :json
 

spec/features/dashboard/help_spec.rb

@@ -5,13 +5,23 @@ RSpec.describe 'Dashboard Help' do
     sign_in(create(:user))
   end
 
-  it 'renders correctly markdown' do
-    visit help_page_path("administration/raketasks/maintenance")
+  context 'help dropdown' do
+    it 'shows the "What\'s new?" menu item' do
+      visit root_dashboard_path
 
-    expect(page).to have_content('Gather information about GitLab and the system it runs on')
+      expect(page.find('.header-help .dropdown-menu')).to have_text("What's new?")
+    end
+  end
+
+  context 'documentation' do
+    it 'renders correctly markdown' do
+      visit help_page_path("administration/raketasks/maintenance")
+
+      expect(page).to have_content('Gather information about GitLab and the system it runs on')
 
-    node = find('.documentation h2 a#user-content-check-gitlab-configuration')
-    expect(node[:href]).to eq '#check-gitlab-configuration'
-    expect(find(:xpath, "#{node.path}/..").text).to eq 'Check GitLab configuration'
+      node = find('.documentation h2 a#user-content-check-gitlab-configuration')
+      expect(node[:href]).to eq '#check-gitlab-configuration'
+      expect(find(:xpath, "#{node.path}/..").text).to eq 'Check GitLab configuration'
+    end
   end
 end

spec/lib/bitbucket_server/client_spec.rb

@@ -17,12 +17,6 @@ describe BitbucketServer::Client do
 
       subject.pull_requests(project, repo_slug)
     end
-
-    it 'throws an exception when connection fails' do
-      allow(BitbucketServer::Collection).to receive(:new).and_raise(OpenSSL::SSL::SSLError)
-
-      expect { subject.pull_requests(project, repo_slug) }.to raise_error(described_class::ServerError)
-    end
   end
 
   describe '#activities' do

spec/lib/bitbucket_server/connection_spec.rb

@@ -26,6 +26,12 @@ describe BitbucketServer::Connection do
 
       expect { subject.get(url) }.to raise_error(described_class::ConnectionError)
     end
+
+    it 'throws an exception upon a network error' do
+      WebMock.stub_request(:get, url).with(headers: { 'Accept' => 'application/json' }).to_raise(OpenSSL::SSL::SSLError)
+
+      expect { subject.get(url) }.to raise_error(described_class::ConnectionError)
+    end
   end
 
   describe '#post' do
@@ -42,6 +48,12 @@ describe BitbucketServer::Connection do
 
       expect { subject.post(url, payload) }.to raise_error(described_class::ConnectionError)
     end
+
+    it 'throws an exception upon a network error' do
+      WebMock.stub_request(:post, url).with(headers: { 'Accept' => 'application/json' }).to_raise(OpenSSL::SSL::SSLError)
+
+      expect { subject.post(url, payload) }.to raise_error(described_class::ConnectionError)
+    end
   end
 
   describe '#delete' do
@@ -63,6 +75,12 @@ describe BitbucketServer::Connection do
 
         expect { subject.delete(:branches, branch_path, payload) }.to raise_error(described_class::ConnectionError)
       end
+
+      it 'throws an exception upon a network error' do
+        WebMock.stub_request(:delete, branch_url).with(headers: headers).to_raise(OpenSSL::SSL::SSLError)
+
+        expect { subject.delete(:branches, branch_path, payload) }.to raise_error(described_class::ConnectionError)
+      end
     end
   end
 end

spec/lib/gitlab/release_blog_post_spec.rb

+require 'spec_helper'
+
+describe Gitlab::ReleaseBlogPost do
+  describe '.blog_post_url' do
+    let(:releases_xml) do
+      <<~EOS
+      <?xml version='1.0' encoding='utf-8' ?>
+      <feed xmlns='http://www.w3.org/2005/Atom'>
+      <entry>
+      <release>11.2</release>
+      <id>https://about.gitlab.com/2018/08/22/gitlab-11-2-released/</id>
+      </entry>
+      <entry>
+      <release>11.1</release>
+      <id>https://about.gitlab.com/2018/07/22/gitlab-11-1-released/</id>
+      </entry>
+      <entry>
+      <release>11.0</release>
+      <id>https://about.gitlab.com/2018/06/22/gitlab-11-0-released/</id>
+      </entry>
+      <entry>
+      <release>10.8</release>
+      <id>https://about.gitlab.com/2018/05/22/gitlab-10-8-released/</id>
+      </entry>
+      </feed>
+      EOS
+    end
+
+    subject { described_class.send(:new).blog_post_url }
+
+    before do
+      stub_request(:get, 'https://about.gitlab.com/releases.xml')
+        .to_return(status: 200, headers: { 'content-type' => ['text/xml'] }, body: releases_xml)
+    end
+
+    context 'matches GitLab version to blog post url' do
+      it 'returns the correct url for major pre release' do
+        stub_const('Gitlab::VERSION', '11.0.0-pre')
+
+        expect(subject).to eql('https://about.gitlab.com/2018/05/22/gitlab-10-8-released/')
+      end
+
+      it 'returns the correct url for major release candidate' do
+        stub_const('Gitlab::VERSION', '11.0.0-rc3')
+
+        expect(subject).to eql('https://about.gitlab.com/2018/05/22/gitlab-10-8-released/')
+      end
+
+      it 'returns the correct url for major release' do
+        stub_const('Gitlab::VERSION', '11.0.0')
+
+        expect(subject).to eql('https://about.gitlab.com/2018/06/22/gitlab-11-0-released/')
+      end
+
+      it 'returns the correct url for minor pre release' do
+        stub_const('Gitlab::VERSION', '11.2.0-pre')
+
+        expect(subject).to eql('https://about.gitlab.com/2018/07/22/gitlab-11-1-released/')
+      end
+
+      it 'returns the correct url for minor release candidate' do
+        stub_const('Gitlab::VERSION', '11.2.0-rc3')
+
+        expect(subject).to eql('https://about.gitlab.com/2018/07/22/gitlab-11-1-released/')
+      end
+
+      it 'returns the correct url for minor release' do
+        stub_const('Gitlab::VERSION', '11.2.0')
+
+        expect(subject).to eql('https://about.gitlab.com/2018/08/22/gitlab-11-2-released/')
+      end
+
+      it 'returns the correct url for patch pre release' do
+        stub_const('Gitlab::VERSION', '11.2.1-pre')
+        expect(subject).to eql('https://about.gitlab.com/2018/08/22/gitlab-11-2-released/')
+      end
+
+      it 'returns the correct url for patch release candidate' do
+        stub_const('Gitlab::VERSION', '11.2.1-rc3')
+
+        expect(subject).to eql('https://about.gitlab.com/2018/08/22/gitlab-11-2-released/')
+      end
+
+      it 'returns the correct url for patch release' do
+        stub_const('Gitlab::VERSION', '11.2.1')
+
+        expect(subject).to eql('https://about.gitlab.com/2018/08/22/gitlab-11-2-released/')
+      end
+
+      it 'returns nil when no blog post is matched' do
+        stub_const('Gitlab::VERSION', '9.0.0')
+
+        expect(subject).to be(nil)
+      end
+    end
+  end
+end

spec/lib/gitlab_spec.rb

@@ -8,6 +8,7 @@ describe Gitlab do
       expect(described_class.root).to eq(Pathname.new(File.expand_path('../..', __dir__)))
     end
   end
+
   describe '.revision' do
     let(:cmd) { %W[#{described_class.config.git.bin_path} log --pretty=format:%h -n 1] }
 
@@ -69,6 +70,82 @@ describe Gitlab do
     end
   end
 
+  describe '.final_release?' do
+    subject { described_class.final_release? }
+
+    context 'returns the corrent boolean value' do
+      it 'is false for a pre release' do
+        stub_const('Gitlab::VERSION', '11.0.0-pre')
+
+        expect(subject).to be false
+      end
+
+      it 'is false for a release candidate' do
+        stub_const('Gitlab::VERSION', '11.0.0-rc2')
+
+        expect(subject).to be false
+      end
+
+      it 'is true for a final release' do
+        stub_const('Gitlab::VERSION', '11.0.2')
+
+        expect(subject).to be true
+      end
+    end
+  end
+
+  describe '.minor_release' do
+    subject { described_class.minor_release }
+
+    it 'returns the minor release of the full GitLab version' do
+      stub_const('Gitlab::VERSION', '11.0.1-rc3')
+
+      expect(subject).to eql '11.0'
+    end
+  end
+
+  describe '.previous_release' do
+    subject { described_class.previous_release }
+
+    context 'it should return the previous release' do
+      it 'returns the previous major version when GitLab major version is not final' do
+        stub_const('Gitlab::VERSION', '11.0.1-pre')
+
+        expect(subject).to eql '10'
+      end
+
+      it 'returns the current minor version when the GitLab patch version is RC and > 0' do
+        stub_const('Gitlab::VERSION', '11.2.1-rc3')
+
+        expect(subject).to eql '11.2'
+      end
+
+      it 'returns the previous minor version when the GitLab patch version is RC and 0' do
+        stub_const('Gitlab::VERSION', '11.2.0-rc3')
+
+        expect(subject).to eql '11.1'
+      end
+    end
+  end
+
+  describe '.new_major_release?' do
+    subject { described_class.new_major_release? }
+
+    context 'returns the corrent boolean value' do
+      it 'is true when the minor version is 0 and the patch is a pre release' do
+        stub_const('Gitlab::VERSION', '11.0.1-pre')
+
+        expect(subject).to be true
+      end
+
+      it 'is false when the minor version is above 0' do
+        stub_const('Gitlab::VERSION', '11.2.1-rc3')
+
+        expect(subject).to be false
+      end
+    end
+  end
+
   describe '.com?' do
     it 'is true when on GitLab.com' do
       stub_config_setting(url: 'https://gitlab.com')

spec/spec_helper.rb

@@ -129,6 +129,11 @@ RSpec.configure do |config|
       .and_return(false)
   end
 
+  config.before(:suite) do
+    # Set latest release blog post URL for "What's new?" link
+    Gitlab::ReleaseBlogPost.instance.instance_variable_set(:@url, 'https://about.gitlab.com')
+  end
+
   config.before(:example, :request_store) do
     RequestStore.begin!
   end

spec/workers/git_garbage_collect_worker_spec.rb

@@ -71,6 +71,17 @@ describe GitGarbageCollectWorker do
 
           subject.perform(project.id)
         end
+
+        context 'when the repository has joined a pool' do
+          let!(:pool) { create(:pool_repository, :ready) }
+          let(:project) { pool.source_project }
+
+          it 'ensures the repositories are linked' do
+            expect_any_instance_of(PoolRepository).to receive(:link_repository).once
+
+            subject.perform(project.id)
+          end
+        end
       end
 
       context 'when no lease can be obtained' do