Add latest changes from gitlab-org/gitlab@master

Gemfile

@@ -386,6 +386,10 @@ group :development, :test do
   gem 'simple_po_parser', '~> 1.1.2', require: false
 
   gem 'timecop', '~> 0.8.0'
+
+  gem 'png_quantizator', '~> 0.2.1', require: false
+
+  gem 'parallel', '~> 1.17.0', require: false
 end
 
 # Gems required in omnibus-gitlab pipeline

Gemfile.lock

@@ -734,6 +734,7 @@ GEM
     peek (1.1.0)
       railties (>= 4.0.0)
     pg (1.1.4)
+    png_quantizator (0.2.1)
     po_to_json (1.0.1)
       json (>= 1.6.0)
     premailer (1.11.1)
@@ -1286,8 +1287,10 @@ DEPENDENCIES
   omniauth_crowd (~> 2.2.0)
   omniauth_openid_connect (~> 0.3.3)
   org-ruby (~> 0.9.12)
+  parallel (~> 1.17.0)
   peek (~> 1.1)
   pg (~> 1.1)
+  png_quantizator (~> 0.2.1)
   premailer-rails (~> 1.10.3)
   prometheus-client-mmap (~> 0.9.10)
   pry-byebug (~> 3.5.1)

app/assets/javascripts/performance_bar/components/detailed_metric.vue

@@ -59,7 +59,8 @@ export default {
   <div
     v-if="currentRequest.details && metricDetails"
     :id="`peek-view-${metric}`"
-    class="view qa-performance-bar-detailed-metric"
+    class="view"
+    data-qa-selector="detailed_metric_content"
   >
     <button
       :data-target="`#modal-peek-${metric}-details`"

app/assets/javascripts/performance_bar/components/performance_bar_app.vue

@@ -107,7 +107,11 @@ export default {
 </script>
 <template>
   <div id="js-peek" :class="env">
-    <div v-if="currentRequest" class="d-flex container-fluid container-limited qa-performance-bar">
+    <div
+      v-if="currentRequest"
+      class="d-flex container-fluid container-limited"
+      data-qa-selector="performance_bar"
+    >
       <div id="peek-view-host" class="view">
         <span
           v-if="hasHost"

app/assets/javascripts/performance_bar/components/request_selector.vue

@@ -45,13 +45,13 @@ export default {
 };
 </script>
 <template>
-  <div id="peek-request-selector">
+  <div id="peek-request-selector" data-qa-selector="request_dropdown">
     <select v-model="currentRequestId">
       <option
         v-for="request in requests"
         :key="request.id"
         :value="request.id"
-        class="qa-performance-bar-request"
+        data-qa-selector="request_dropdown_option"
       >
         {{ request.truncatedUrl }}
         <span v-if="request.hasWarnings">(!)</span>

app/controllers/projects/error_tracking/stack_traces_controller.rb

+# frozen_string_literal: true
+
+module Projects
+  module ErrorTracking
+    class StackTracesController < Projects::ApplicationController
+      respond_to :json
+
+      before_action :authorize_read_sentry_issue!
+
+      def index
+        result = fetch_latest_event_issue
+
+        if result[:status] == :success
+          result_with_syntax_highlight = Gitlab::ErrorTracking::StackTraceHighlightDecorator.decorate(result[:latest_event])
+
+          render json: { error: serialize_error_event(result_with_syntax_highlight) }
+        else
+          render json: { message: result[:message] }, status: result.fetch(:http_status, :bad_request)
+        end
+      end
+
+      private
+
+      def fetch_latest_event_issue
+        ::ErrorTracking::IssueLatestEventService
+          .new(project, current_user, issue_id: params[:issue_id])
+          .execute
+      end
+
+      def serialize_error_event(event)
+        ::ErrorTracking::ErrorEventSerializer
+          .new(project: project, user: current_user)
+          .represent(event)
+      end
+    end
+  end
+end

app/controllers/projects/error_tracking_controller.rb

@@ -2,7 +2,7 @@
 
 class Projects::ErrorTrackingController < Projects::ApplicationController
   before_action :authorize_read_sentry_issue!
-  before_action :set_issue_id, only: [:details, :stack_trace]
+  before_action :set_issue_id, only: :details
 
   POLLING_INTERVAL = 10_000
 
@@ -25,14 +25,6 @@ class Projects::ErrorTrackingController < Projects::ApplicationController
     end
   end
 
-  def stack_trace
-    respond_to do |format|
-      format.json do
-        render_issue_stack_trace_json
-      end
-    end
-  end
-
   private
 
   def render_index_json
@@ -63,19 +55,6 @@ class Projects::ErrorTrackingController < Projects::ApplicationController
     }
   end
 
-  def render_issue_stack_trace_json
-    service = ErrorTracking::IssueLatestEventService.new(project, current_user, issue_details_params)
-    result = service.execute
-
-    return if handle_errors(result)
-
-    result_with_syntax_highlight = Gitlab::ErrorTracking::StackTraceHighlightDecorator.decorate(result[:latest_event])
-
-    render json: {
-      error: serialize_error_event(result_with_syntax_highlight)
-    }
-  end
-
   def handle_errors(result)
     unless result[:status] == :success
       render json: { message: result[:message] },
@@ -110,10 +89,4 @@ class Projects::ErrorTrackingController < Projects::ApplicationController
       .new(project: project, user: current_user)
       .represent(error)
   end
-
-  def serialize_error_event(event)
-    ErrorTracking::ErrorEventSerializer
-      .new(project: project, user: current_user)
-      .represent(event)
-  end
 end

app/helpers/application_helper.rb

@@ -198,7 +198,7 @@ module ApplicationHelper
   end
 
   def external_storage_url_or_path(path, project = @project)
-    return path unless static_objects_external_storage_enabled?
+    return path if @snippet || !static_objects_external_storage_enabled?
 
     uri = URI(Gitlab::CurrentSettings.static_objects_external_storage_url)
     path = URI(path) # `path` could have query parameters, so we need to split query and path apart

changelogs/unreleased/ff-toggle-backend.yml

+---
+title: Add feature flag override toggle
+merge_request: 21598
+author:
+type: added

changelogs/unreleased/fix-skip-snippet-raw-buttons-for-external-caching.yml

+---
+title: Exclude snippets from external caching handling
+merge_request:
+author:
+type: fixed

config/routes/project.rb

@@ -269,7 +269,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
               to: 'error_tracking#details',
               as: 'details'
             get ':issue_id/stack_trace',
-              to: 'error_tracking#stack_trace',
+              to: 'error_tracking/stack_traces#index',
               as: 'stack_trace'
           end
         end

db/migrate/20191213184609_backfill_operations_feature_flags_active.rb

+# frozen_string_literal: true
+
+class BackfillOperationsFeatureFlagsActive < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class OperationsFeatureFlag < ActiveRecord::Base
+    self.table_name = 'operations_feature_flags'
+    self.inheritance_column = :_type_disabled
+  end
+
+  def up
+    OperationsFeatureFlag.where(active: false).update_all(active: true)
+  end
+
+  def down
+    # no-op
+  end
+end

doc/user/project/operations/feature_flags.md

@@ -51,7 +51,10 @@ with ability to edit or remove them.
 To make a feature flag active or inactive, click the pencil icon to edit it,
 and toggle the status for each [spec](#define-environment-specs).
 
-![Feature flags list](img/feature_flags_list.png)
+The toggles next to each feature flag on the list page function as global shutoff switches.
+If a toggle is off, that feature flag is disabled for every environment.
+
+![Feature flags list](img/feature_flags_list_v12_7.png)
 
 ## Define environment specs
 

lib/gitlab/database/migration_helpers.rb

@@ -158,7 +158,7 @@ module Gitlab
       # name - The name of the foreign key.
       #
       # rubocop:disable Gitlab/RailsLogger
-      def add_concurrent_foreign_key(source, target, column:, on_delete: :cascade, name: nil)
+      def add_concurrent_foreign_key(source, target, column:, on_delete: :cascade, name: nil, validate: true)
         # Transactions would result in ALTER TABLE locks being held for the
         # duration of the transaction, defeating the purpose of this method.
         if transaction_open?
@@ -197,10 +197,16 @@ module Gitlab
         # Validate the existing constraint. This can potentially take a very
         # long time to complete, but fortunately does not lock the source table
         # while running.
+        # Disable this check by passing `validate: false` to the method call
+        # The check will be enforced for new data (inserts) coming in,
+        # but validating existing data is delayed.
         #
         # Note this is a no-op in case the constraint is VALID already
-        disable_statement_timeout do
-          execute("ALTER TABLE #{source} VALIDATE CONSTRAINT #{options[:name]};")
+
+        if validate
+          disable_statement_timeout do
+            execute("ALTER TABLE #{source} VALIDATE CONSTRAINT #{options[:name]};")
+          end
         end
       end
       # rubocop:enable Gitlab/RailsLogger

lib/sentry/client.rb

@@ -71,9 +71,22 @@ module Sentry
     end
 
     def http_get(url, params = {})
-      response = handle_request_exceptions do
+      http_request do
         Gitlab::HTTP.get(url, **request_params.merge(params))
       end
+    end
+
+    def http_put(url, params = {})
+      http_request do
+        Gitlab::HTTP.put(url, **request_params.merge({ body: params }))
+      end
+    end
+
+    def http_request
+      response = handle_request_exceptions do
+        yield
+      end
+
       handle_response(response)
     end
 

lib/sentry/client/issue.rb

@@ -15,6 +15,10 @@ module Sentry
         http_get(issue_api_url(issue_id))[:body]
       end
 
+      def update_issue(issue_id:, params:)
+        http_put(issue_api_url(issue_id), params)[:body]
+      end
+
       def issue_api_url(issue_id)
         issue_url = URI(url)
         issue_url.path = "/api/0/issues/#{CGI.escape(issue_id.to_s)}/"

lib/tasks/pngquant.rake

+return if Rails.env.production?
+
+require 'png_quantizator'
+require 'parallel'
+
+# The amount of variance (in bytes) allowed in
+# file size when testing for compression size
+TOLERANCE = 10
+
+namespace :pngquant do
+  # Returns an array of all images eligible for compression
+  def doc_images
+    Dir.glob('doc/**/*.png', File::FNM_CASEFOLD)
+  end
+
+  # Runs pngquant on an image and optionally
+  # writes the result to disk
+  def compress_image(file, overwrite_original)
+    compressed_file = "#{file}.compressed"
+    FileUtils.copy(file, compressed_file)
+
+    pngquant_file = PngQuantizator::Image.new(compressed_file)
+
+    # Run the image repeatedly through pngquant until
+    # the change in file size is within TOLERANCE
+    loop do
+      before = File.size(compressed_file)
+      pngquant_file.quantize!
+      after = File.size(compressed_file)
+      break if before - after <= TOLERANCE
+    end
+
+    savings = File.size(file) - File.size(compressed_file)
+    is_uncompressed = savings > TOLERANCE
+
+    if is_uncompressed && overwrite_original
+      FileUtils.copy(compressed_file, file)
+    end
+
+    FileUtils.remove(compressed_file)
+
+    [is_uncompressed, savings]
+  end
+
+  # Ensures pngquant is available and prints an error if not
+  def check_executable
+    unless system('pngquant --version', out: File::NULL)
+      warn(
+        'Error: pngquant executable was not detected in the system.'.color(:red),
+        'Download pngquant at https://pngquant.org/ and place the executable in /usr/local/bin'.color(:green)
+      )
+      abort
+    end
+  end
+
+  desc 'GitLab | pngquant | Compress all documentation PNG images using pngquant'
+  task :compress do
+    check_executable
+
+    files = doc_images
+    puts "Compressing #{files.size} PNG files in doc/**"
+
+    Parallel.each(files) do |file|
+      was_uncompressed, savings = compress_image(file, true)
+
+      if was_uncompressed
+        puts "#{file} was reduced by #{savings} bytes"
+      end
+    end
+  end
+
+  desc 'GitLab | pngquant | Checks that all documentation PNG images have been compressed with pngquant'
+  task :lint do
+    check_executable
+
+    files = doc_images
+    puts "Checking #{files.size} PNG files in doc/**"
+
+    uncompressed_files = Parallel.map(files) do |file|
+      is_uncompressed, _ = compress_image(file, false)
+      if is_uncompressed
+        puts "Uncompressed file detected: ".color(:red) + file
+        file
+      end
+    end.compact
+
+    if uncompressed_files.empty?
+      puts "All documentation images are optimally compressed!".color(:green)
+    else
+      warn(
+        "The #{uncompressed_files.size} image(s) above have not been optimally compressed using pngquant.".color(:red),
+        'Please run "bin/rake pngquant:compress" and commit the result.'
+      )
+      abort
+    end
+  end
+end

qa/qa/page/layout/performance_bar.rb

@@ -9,11 +9,12 @@ module QA
         end
 
         view 'app/assets/javascripts/performance_bar/components/detailed_metric.vue' do
-          element :performance_bar_detailed_metric
+          element :detailed_metric_content
         end
 
         view 'app/assets/javascripts/performance_bar/components/request_selector.vue' do
-          element :performance_bar_request
+          element :request_dropdown_option
+          element :request_dropdown
         end
 
         def has_performance_bar?
@@ -21,13 +22,18 @@ module QA
         end
 
         def has_detailed_metrics?
-          all_elements(:performance_bar_detailed_metric).all? do |metric|
-            metric.has_text?(%r{\d+})
+          retry_until(sleep_interval: 1) do
+            all_elements(:detailed_metric_content).all? do |metric|
+              metric.has_text?(%r{\d+})
+            end
           end
         end
 
         def has_request_for?(path)
-          has_element?(:performance_bar_request, text: path)
+          click_element(:request_dropdown)
+          retry_until(sleep_interval: 1) do
+            has_element?(:request_dropdown_option, text: path)
+          end
         end
       end
     end

spec/controllers/projects/error_tracking/stack_traces_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::ErrorTracking::StackTracesController do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:user) { create(:user) }
+
+  before do
+    sign_in(user)
+    project.add_maintainer(user)
+  end
+
+  describe 'GET #index' do
+    let(:issue_id) { 1234 }
+    let(:issue_stack_trace_service) { spy(:issue_stack_trace_service) }
+
+    subject(:get_stack_trace) do
+      get :index, params: { namespace_id: project.namespace, project_id: project, issue_id: issue_id, format: :json }
+    end
+
+    before do
+      expect(ErrorTracking::IssueLatestEventService)
+        .to receive(:new).with(project, user, issue_id: issue_id.to_s)
+        .and_return(issue_stack_trace_service)
+      expect(issue_stack_trace_service).to receive(:execute).and_return(service_response)
+
+      get_stack_trace
+    end
+
+    context 'awaiting data' do
+      let(:service_response) { { status: :error, http_status: :no_content }}
+
+      it 'responds with no data' do
+        expect(response).to have_gitlab_http_status(:no_content)
+      end
+    end
+
+    context 'service result is successful' do
+      let(:service_response) { { status: :success, latest_event: error_event } }
+      let(:error_event) { build(:error_tracking_error_event) }
+
+      it 'responds with success' do
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+
+      it 'responds with error' do
+        expect(response).to match_response_schema('error_tracking/issue_stack_trace')
+      end
+
+      it 'highlights stack trace source code' do
+        expect(json_response['error']).to eq(
+          Gitlab::ErrorTracking::StackTraceHighlightDecorator.decorate(error_event).as_json
+        )
+      end
+    end
+
+    context 'service result is erroneous' do
+      let(:error_message) { 'error message' }
+
+      context 'without http_status' do
+        let(:service_response) { { status: :error, message: error_message } }
+
+        it 'responds with bad request' do
+          expect(response).to have_gitlab_http_status(:bad_request)
+        end
+
+        it 'responds with error message' do
+          expect(json_response['message']).to eq(error_message)
+        end
+      end
+
+      context 'with explicit http_status' do
+        let(:http_status) { :no_content }
+        let(:service_response) { { status: :error, message: error_message, http_status: http_status } }
+
+        it 'responds with custom http status' do
+          expect(response).to have_gitlab_http_status(http_status)
+        end
+
+        it 'responds with error message' do
+          expect(json_response['message']).to eq(error_message)
+        end
+      end
+    end
+  end
+end

spec/controllers/projects/error_tracking_controller_spec.rb

@@ -266,102 +266,6 @@ describe Projects::ErrorTrackingController do
     end
   end
 
-  describe 'GET #stack_trace' do
-    let_it_be(:issue_id) { 1234 }
-
-    let(:issue_stack_trace_service) { spy(:issue_stack_trace_service) }
-
-    let(:permitted_params) do
-      ActionController::Parameters.new(
-        { issue_id: issue_id.to_s }
-      ).permit!
-    end
-
-    subject(:get_stack_trace) do
-      get :stack_trace, params: issue_params(issue_id: issue_id, format: :json)
-    end
-
-    before do
-      expect(ErrorTracking::IssueLatestEventService)
-        .to receive(:new).with(project, user, permitted_params)
-        .and_return(issue_stack_trace_service)
-    end
-
-    describe 'format json' do
-      context 'awaiting data' do
-        before do
-          expect(issue_stack_trace_service).to receive(:execute)
-            .and_return(status: :error, http_status: :no_content)
-        end
-
-        it 'returns no data' do
-          get_stack_trace
-
-          expect(response).to have_gitlab_http_status(:no_content)
-        end
-      end
-
-      context 'service result is successful' do
-        before do
-          expect(issue_stack_trace_service).to receive(:execute)
-            .and_return(status: :success, latest_event: error_event)
-
-          get_stack_trace
-        end
-
-        let(:error_event) { build(:error_tracking_error_event) }
-
-        it 'returns an error' do
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(response).to match_response_schema('error_tracking/issue_stack_trace')
-        end
-
-        it 'highlights stack trace source code' do
-          expect(json_response['error']).to eq(
-            Gitlab::ErrorTracking::StackTraceHighlightDecorator.decorate(error_event).as_json
-          )
-        end
-      end
-
-      context 'service result is erroneous' do
-        let(:error_message) { 'error message' }
-
-        context 'without http_status' do
-          before do
-            expect(issue_stack_trace_service).to receive(:execute)
-              .and_return(status: :error, message: error_message)
-          end
-
-          it 'returns 400 with message' do
-            get_stack_trace
-
-            expect(response).to have_gitlab_http_status(:bad_request)
-            expect(json_response['message']).to eq(error_message)
-          end
-        end
-
-        context 'with explicit http_status' do
-          let(:http_status) { :no_content }
-
-          before do
-            expect(issue_stack_trace_service).to receive(:execute).and_return(
-              status: :error,
-              message: error_message,
-              http_status: http_status
-            )
-          end
-
-          it 'returns http_status with message' do
-            get_stack_trace
-
-            expect(response).to have_gitlab_http_status(http_status)
-            expect(json_response['message']).to eq(error_message)
-          end
-        end
-      end
-    end
-  end
-
   private
 
   def issue_params(opts = {})

spec/helpers/application_helper_spec.rb

@@ -206,6 +206,15 @@ describe ApplicationHelper do
       end
     end
 
+    context 'when @snippet is set' do
+      it 'returns the passed path' do
+        snippet = create(:snippet)
+        assign(:snippet, snippet)
+
+        expect(helper.external_storage_url_or_path('/foo/bar', project)).to eq('/foo/bar')
+      end
+    end
+
     context 'when external storage is enabled' do
       let(:user) { create(:user, static_object_token: 'hunter1') }
 

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -325,6 +325,25 @@ describe Gitlab::Database::MigrationHelpers do
           end
         end
       end
+
+      describe 'validate option' do
+        let(:args) { [:projects, :users] }
+        let(:options) { { column: :user_id, on_delete: nil } }
+
+        context 'when validate is supplied with a falsey value' do
+          it_behaves_like 'skips validation', validate: false
+          it_behaves_like 'skips validation', validate: nil
+        end
+
+        context 'when validate is supplied with a truthy value' do
+          it_behaves_like 'performs validation', validate: true
+          it_behaves_like 'performs validation', validate: :whatever
+        end
+
+        context 'when validate is not supplied' do
+          it_behaves_like 'performs validation', {}
+        end
+      end
     end
   end
 

spec/migrations/backfill_operations_feature_flags_active_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20191213184609_backfill_operations_feature_flags_active.rb')
+
+describe BackfillOperationsFeatureFlagsActive, :migration do
+  let(:namespaces)   { table(:namespaces) }
+  let(:projects)     { table(:projects) }
+  let(:flags)        { table(:operations_feature_flags) }
+
+  def setup
+    namespace = namespaces.create!(name: 'foo', path: 'foo')
+    project = projects.create!(namespace_id: namespace.id)
+
+    project
+  end
+
+  it 'executes successfully when there are no flags in the table' do
+    setup
+
+    disable_migrations_output { migrate! }
+
+    expect(flags.count).to eq(0)
+  end
+
+  it 'updates active to true' do
+    project = setup
+    flag = flags.create!(project_id: project.id, name: 'test_flag', active: false)
+
+    disable_migrations_output { migrate! }
+
+    expect(flag.reload.active).to eq(true)
+  end
+
+  it 'updates active to true for multiple flags' do
+    project = setup
+    flag_a = flags.create!(project_id: project.id, name: 'test_flag', active: false)
+    flag_b = flags.create!(project_id: project.id, name: 'other_flag', active: false)
+
+    disable_migrations_output { migrate! }
+
+    expect(flag_a.reload.active).to eq(true)
+    expect(flag_b.reload.active).to eq(true)
+  end
+
+  it 'leaves active true if it is already true' do
+    project = setup
+    flag = flags.create!(project_id: project.id, name: 'test_flag', active: true)
+
+    disable_migrations_output { migrate! }
+
+    expect(flag.reload.active).to eq(true)
+  end
+end

spec/support/shared_examples/migration_helpers_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'skips validation' do |validation_option|
+  it 'skips validation' do
+    expect(model).not_to receive(:disable_statement_timeout)
+    expect(model).to receive(:execute).with(/ADD CONSTRAINT/)
+    expect(model).not_to receive(:execute).with(/VALIDATE CONSTRAINT/)
+
+    model.add_concurrent_foreign_key(*args, **options.merge(validation_option))
+  end
+end
+
+shared_examples 'performs validation' do |validation_option|
+  it 'performs validation' do
+    expect(model).to receive(:disable_statement_timeout).and_call_original
+    expect(model).to receive(:execute).with(/statement_timeout/)
+    expect(model).to receive(:execute).ordered.with(/NOT VALID/)
+    expect(model).to receive(:execute).ordered.with(/VALIDATE CONSTRAINT/)
+    expect(model).to receive(:execute).with(/RESET ALL/)
+
+    model.add_concurrent_foreign_key(*args, **options.merge(validation_option))
+  end
+end