Merge branch 'ce-to-ee-2018-09-25' into 'master'

CE upstream - 2018-09-25 15:25 UTC

Closes gitlab-ce#51695 and #7728

See merge request gitlab-org/gitlab-ee!7481

.gitlab-ci.yml

@@ -909,6 +909,9 @@ karma:
   dependencies:
     - compile-assets
     - setup-test-env
+  variables:
+    # we override the max_old_space_size to prevent OOM errors
+    NODE_OPTIONS: --max_old_space_size=3584
   script:
     - export BABEL_ENV=coverage CHROME_LOG_FILE=chrome_debug.log
     - date

app/assets/javascripts/build_variables.js

-import $ from 'jquery';
-
-export default function handleRevealVariables() {
-  $('.js-reveal-variables')
-    .off('click')
-    .on('click', function click() {
-      $('.js-build-variables').toggle();
-      $(this).hide();
-    });
-}

app/assets/javascripts/ide/components/commit_sidebar/editor_header.vue

@@ -52,6 +52,7 @@ export default {
     </strong>
     <changed-file-icon
       :file="activeFile"
+      class="ml-0"
     />
     <div class="ml-auto">
       <button

app/assets/javascripts/ide/components/commit_sidebar/list_item.vue

@@ -120,10 +120,6 @@ export default {
             :css-classes="iconClass"
           />
         </div>
-        <component
-          :is="actionComponent"
-          :path="file.path"
-        />
       </div>
     </div>
   </div>

app/assets/javascripts/jobs/components/artifacts_block.vue

 <script>
-  import TimeagoTooltiop from '~/vue_shared/components/time_ago_tooltip.vue';
+  import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
+  import timeagoMixin from '~/vue_shared/mixins/timeago';
 
   export default {
     components: {
-      TimeagoTooltiop,
+      TimeagoTooltip,
     },
+    mixins: [
+      timeagoMixin,
+    ],
     props: {
-      // @build.artifacts_expired?
-      haveArtifactsExpired: {
-        type: Boolean,
+      artifact: {
+        type: Object,
         required: true,
       },
-      // @build.has_expiring_artifacts?
-      willArtifactsExpire: {
-        type: Boolean,
-        required: true,
-      },
-      expireAt: {
-        type: String,
-        required: false,
-        default: null,
-      },
-      keepArtifactsPath: {
-        type: String,
-        required: false,
-        default: null,
-      },
-      downloadArtifactsPath: {
-        type: String,
-        required: false,
-        default: null,
+    },
+    computed: {
+      isExpired() {
+        return this.artifact.expired;
       },
-      browseArtifactsPath: {
-        type: String,
-        required: false,
-        default: null,
+      // Only when the key is `false` we can render this block
+      willExpire() {
+        return this.artifact.expired === false;
       },
     },
   };
@@ -46,21 +33,22 @@
     </div>
 
     <p
-      v-if="haveArtifactsExpired"
+      v-if="isExpired"
       class="js-artifacts-removed build-detail-row"
     >
       {{ s__('Job|The artifacts were removed') }}
     </p>
+
     <p
-      v-else-if="willArtifactsExpire"
+      v-else-if="willExpire"
       class="js-artifacts-will-be-removed build-detail-row"
     >
-      {{ s__('Job|The artifacts will be removed') }}
+      {{ s__('Job|The artifacts will be removed in') }}
     </p>
 
-    <timeago-tooltiop
-      v-if="expireAt"
-      :time="expireAt"
+    <timeago-tooltip
+      v-if="artifact.expire_at"
+      :time="artifact.expire_at"
     />
 
     <div
@@ -68,8 +56,8 @@
       role="group"
     >
       <a
-        v-if="keepArtifactsPath"
-        :href="keepArtifactsPath"
+        v-if="artifact.keep_path"
+        :href="artifact.keep_path"
         class="js-keep-artifacts btn btn-sm btn-default"
         data-method="post"
       >
@@ -77,8 +65,8 @@
       </a>
 
       <a
-        v-if="downloadArtifactsPath"
-        :href="downloadArtifactsPath"
+        v-if="artifact.download_path"
+        :href="artifact.download_path"
         class="js-download-artifacts btn btn-sm btn-default"
         download
         rel="nofollow"
@@ -87,8 +75,8 @@
       </a>
 
       <a
-        v-if="browseArtifactsPath"
-        :href="browseArtifactsPath"
+        v-if="artifact.browse_path"
+        :href="artifact.browse_path"
         class="js-browse-artifacts btn btn-sm btn-default"
       >
         {{ s__('Job|Browse') }}

app/assets/javascripts/jobs/components/commit_block.vue

 <script>
-import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
+  import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 
-export default {
-  components: {
-    ClipboardButton,
-  },
-  props: {
-    pipelineShortSha: {
-      type: String,
-      required: true,
+  export default {
+    components: {
+      ClipboardButton,
     },
-    pipelineShaPath: {
-      type: String,
-      required: true,
+    props: {
+      commit: {
+        type: Object,
+        required: true,
+      },
+      mergeRequest: {
+        type: Object,
+        required: false,
+        default: null,
+      },
+      isLastBlock: {
+        type: Boolean,
+        required: true,
+      },
     },
-    mergeRequestReference: {
-      type: String,
-      required: false,
-      default: null,
-    },
-    mergeRequestPath: {
-      type: String,
-      required: false,
-      default: null,
-    },
-    gitCommitTitlte: {
-      type: String,
-      required: true,
-    },
-  },
-};
+  };
 </script>
 <template>
-  <div class="block">
+  <div
+    :class="{
+      'block-last': isLastBlock,
+      block: !isLastBlock
+  }">
     <p>
       {{ __('Commit') }}
 
       <a
-        :href="pipelineShaPath"
+        :href="commit.commit_path"
         class="js-commit-sha commit-sha link-commit"
-      >
-        {{ pipelineShortSha }}
-      </a>
+      >{{ commit.short_id }}</a>
 
       <clipboard-button
-        :text="pipelineShortSha"
+        :text="commit.short_id"
         :title="__('Copy commit SHA to clipboard')"
+        css-class="btn btn-clipboard btn-transparent"
       />
 
       <a
-        v-if="mergeRequestPath && mergeRequestReference"
-        :href="mergeRequestPath"
+        v-if="mergeRequest"
+        :href="mergeRequest.path"
         class="js-link-commit link-commit"
-      >
-        {{ mergeRequestReference }}
-      </a>
+      >{{ mergeRequest.iid }}</a>
     </p>
 
     <p class="build-light-text append-bottom-0">
-      {{ gitCommitTitlte }}
+      {{ commit.title }}
     </p>
   </div>
 </template>

app/assets/javascripts/jobs/components/sidebar_details_block.vue

 <script>
-import timeagoMixin from '~/vue_shared/mixins/timeago';
-import { timeIntervalInWords } from '~/lib/utils/datetime_utility';
-import Icon from '~/vue_shared/components/icon.vue';
-import DetailRow from './sidebar_detail_row.vue';
+  import _ from 'underscore';
+  import timeagoMixin from '~/vue_shared/mixins/timeago';
+  import { timeIntervalInWords } from '~/lib/utils/datetime_utility';
+  import Icon from '~/vue_shared/components/icon.vue';
+  import DetailRow from './sidebar_detail_row.vue';
+  import ArtifactsBlock from './artifacts_block.vue';
+  import TriggerBlock from './trigger_block.vue';
+  import CommitBlock from './commit_block.vue';
 
-export default {
-  name: 'SidebarDetailsBlock',
-  components: {
-    DetailRow,
-    Icon,
-  },
-  mixins: [timeagoMixin],
-  props: {
-    job: {
-      type: Object,
-      required: true,
+  export default {
+    name: 'SidebarDetailsBlock',
+    components: {
+      ArtifactsBlock,
+      CommitBlock,
+      DetailRow,
+      Icon,
+      TriggerBlock,
     },
-    isLoading: {
-      type: Boolean,
-      required: true,
+    mixins: [timeagoMixin],
+    props: {
+      job: {
+        type: Object,
+        required: true,
+      },
+      isLoading: {
+        type: Boolean,
+        required: true,
+      },
+      runnerHelpUrl: {
+        type: String,
+        required: false,
+        default: '',
+      },
+      terminalPath: {
+        type: String,
+        required: false,
+        default: null,
+      },
     },
-    runnerHelpUrl: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    terminalPath: {
-      type: String,
-      required: false,
-      default: null,
-    },
-  },
-  computed: {
-    shouldRenderContent() {
-      return !this.isLoading && Object.keys(this.job).length > 0;
-    },
-    coverage() {
-      return `${this.job.coverage}%`;
-    },
-    duration() {
-      return timeIntervalInWords(this.job.duration);
-    },
-    queued() {
-      return timeIntervalInWords(this.job.queued);
-    },
-    runnerId() {
-      return `${this.job.runner.description} (#${this.job.runner.id})`;
-    },
-    retryButtonClass() {
-      let className =
-        'js-retry-button float-right btn btn-retry d-none d-md-block d-lg-block d-xl-block';
-      className +=
-        this.job.status && this.job.recoverable ? ' btn-primary' : ' btn-inverted-secondary';
-      return className;
-    },
-    hasTimeout() {
-      return this.job.metadata != null && this.job.metadata.timeout_human_readable !== null;
-    },
-    timeout() {
-      if (this.job.metadata == null) {
-        return '';
-      }
+    computed: {
+      shouldRenderContent() {
+        return !this.isLoading && Object.keys(this.job).length > 0;
+      },
+      coverage() {
+        return `${this.job.coverage}%`;
+      },
+      duration() {
+        return timeIntervalInWords(this.job.duration);
+      },
+      queued() {
+        return timeIntervalInWords(this.job.queued);
+      },
+      runnerId() {
+        return `${this.job.runner.description} (#${this.job.runner.id})`;
+      },
+      retryButtonClass() {
+        let className =
+          'js-retry-button float-right btn btn-retry d-none d-md-block d-lg-block d-xl-block';
+        className +=
+          this.job.status && this.job.recoverable ? ' btn-primary' : ' btn-inverted-secondary';
+        return className;
+      },
+      hasTimeout() {
+        return this.job.metadata != null && this.job.metadata.timeout_human_readable !== null;
+      },
+      timeout() {
+        if (this.job.metadata == null) {
+          return '';
+        }
 
-      let t = this.job.metadata.timeout_human_readable;
-      if (this.job.metadata.timeout_source !== '') {
-        t += ` (from ${this.job.metadata.timeout_source})`;
-      }
+        let t = this.job.metadata.timeout_human_readable;
+        if (this.job.metadata.timeout_source !== '') {
+          t += ` (from ${this.job.metadata.timeout_source})`;
+        }
 
-      return t;
-    },
-    renderBlock() {
-      return (
-        this.job.merge_request ||
-        this.job.duration ||
-        this.job.finished_data ||
-        this.job.erased_at ||
-        this.job.queued ||
-        this.job.runner ||
-        this.job.coverage ||
-        this.job.tags.length ||
-        this.job.cancel_path
-      );
+        return t;
+      },
+      renderBlock() {
+        return (
+          this.job.merge_request ||
+          this.job.duration ||
+          this.job.finished_data ||
+          this.job.erased_at ||
+          this.job.queued ||
+          this.job.runner ||
+          this.job.coverage ||
+          this.job.tags.length ||
+          this.job.cancel_path
+        );
+      },
+      hasArtifact() {
+        return !_.isEmpty(this.job.artifact);
+      },
+      hasTriggers() {
+        return !_.isEmpty(this.job.trigger);
+      },
+      hasStages() {
+        return (
+          this.job &&
+          this.job.pipeline &&
+          this.job.pipeline.stages &&
+          this.job.pipeline.stages.length > 0
+        ) || false;
+      },
+      commit() {
+        return this.job.pipeline.commit || {};
+      },
     },
-  },
-};
+  };
 </script>
 <template>
   <div>
@@ -229,6 +253,19 @@ export default {
           </a>
         </div>
       </div>
+      <artifacts-block
+        v-if="hasArtifact"
+        :artifact="job.artifact"
+      />
+      <trigger-block
+        v-if="hasTriggers"
+        :trigger="job.trigger"
+      />
+      <commit-block
+        :is-last-block="hasStages"
+        :commit="commit"
+        :merge-request="job.merge_request"
+      />
     </template>
     <gl-loading-icon
       v-if="isLoading"

app/assets/javascripts/jobs/components/trigger_block.vue

 <script>
   export default {
     props: {
-      shortToken: {
-        type: String,
-        required: false,
-        default: null,
-      },
-
-      variables: {
+      trigger: {
         type: Object,
-        required: false,
-        default: () => ({}),
+        required: true,
       },
     },
     data() {
@@ -20,7 +13,7 @@
     },
     computed: {
       hasVariables() {
-        return Object.keys(this.variables).length > 0;
+        return this.trigger.variables && this.trigger.variables.length > 0;
       },
     },
     methods: {
@@ -38,17 +31,18 @@
     </h4>
 
     <p
-      v-if="shortToken"
+      v-if="trigger.short_token"
       class="js-short-token"
     >
       <span class="build-light-text">
         {{ __('Token') }}
       </span>
-      {{ shortToken }}
+      {{ trigger.short_token }}
     </p>
 
     <p v-if="hasVariables">
       <button
+        v-if="!areVariablesVisible"
         type="button"
         class="btn btn-default group js-reveal-variables"
         @click="revealVariables"
@@ -63,20 +57,20 @@
       class="js-build-variables trigger-build-variables"
     >
       <template
-        v-for="(value, key) in variables"
+        v-for="variable in trigger.variables"
       >
         <dt
-          :key="`${key}-variable`"
+          :key="`${variable.key}-variable`"
           class="js-build-variable trigger-build-variable"
         >
-          {{ key }}
+          {{ variable.key }}
         </dt>
 
         <dd
-          :key="`${key}-value`"
+          :key="`${variable.key}-value`"
           class="js-build-value trigger-build-value"
         >
-          {{ value }}
+          {{ variable.value }}
         </dd>
       </template>
     </dl>

app/assets/javascripts/jobs/job_details_mediator.js

@@ -4,7 +4,6 @@ import Poll from '../lib/utils/poll';
 import JobStore from './stores/job_store';
 import JobService from './services/job_service';
 import Job from '../job';
-import handleRevealVariables from '../build_variables';
 
 export default class JobMediator {
   constructor(options = {}) {
@@ -20,7 +19,6 @@ export default class JobMediator {
 
   initBuildClass() {
     this.build = new Job();
-    handleRevealVariables();
   }
 
   fetchJob() {

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue

@@ -114,6 +114,8 @@ export default {
   },
   beforeDestroy() {
     eventHub.$off('mr.discussion.updated', this.checkStatus);
+    this.pollingInterval.destroy();
+    this.deploymentsInterval.destroy();
   },
   methods: {
     createService(store) {

app/controllers/concerns/with_performance_bar.rb

@@ -10,11 +10,7 @@ module WithPerformanceBar
   def peek_enabled?
     return false unless Gitlab::PerformanceBar.enabled?(current_user)
 
-    if RequestStore.active?
-      RequestStore.fetch(:peek_enabled) { cookie_or_default_value }
-    else
-      cookie_or_default_value
-    end
+    Gitlab::SafeRequestStore.fetch(:peek_enabled) { cookie_or_default_value }
   end
 
   private

app/controllers/projects/settings/repository_controller.rb

@@ -14,10 +14,10 @@ module Projects
         @new_deploy_token = DeployTokens::CreateService.new(@project, current_user, deploy_token_params).execute
 
         if @new_deploy_token.persisted?
-          flash.now[:notice] = s_('DeployTokens|Your new project deploy token has been created.')
+          flash[:notice] = s_('DeployTokens|Your new project deploy token has been created.')
         end
 
-        render_show
+        redirect_to action: :show
       end
 
       private

app/models/ability.rb

@@ -74,7 +74,7 @@ class Ability
     end
 
     def policy_for(user, subject = :global)
-      cache = RequestStore.active? ? RequestStore : {}
+      cache = Gitlab::SafeRequestStore.active? ? Gitlab::SafeRequestStore : {}
       DeclarativePolicy.policy_for(user, subject, cache: cache)
     end
 

app/models/concerns/bulk_member_access_load.rb

@@ -16,9 +16,9 @@ module BulkMemberAccessLoad
       key = max_member_access_for_resource_key(resource_klass, memoization_index)
       access = {}
 
-      if RequestStore.active?
-        RequestStore.store[key] ||= {}
-        access = RequestStore.store[key]
+      if Gitlab::SafeRequestStore.active?
+        Gitlab::SafeRequestStore[key] ||= {}
+        access = Gitlab::SafeRequestStore[key]
       end
 
       # Look up only the IDs we need

app/models/concerns/cacheable_attributes.rb

@@ -27,11 +27,7 @@ module CacheableAttributes
     end
 
     def cached
-      if RequestStore.active?
-        RequestStore[:"#{name}_cached_attributes"] ||= retrieve_from_cache
-      else
-        retrieve_from_cache
-      end
+      Gitlab::SafeRequestStore[:"#{name}_cached_attributes"] ||= retrieve_from_cache
     end
 
     def retrieve_from_cache

app/models/legacy_diff_note.rb

@@ -25,11 +25,7 @@ class LegacyDiffNote < Note
   end
 
   def project_repository
-    if RequestStore.active?
-      RequestStore.fetch("project:#{project_id}:repository") { self.project.repository }
-    else
-      self.project.repository
-    end
+    Gitlab::SafeRequestStore.fetch("project:#{project_id}:repository") { self.project.repository }
   end
 
   def diff_file_hash

app/models/namespace.rb

@@ -149,8 +149,8 @@ class Namespace < ActiveRecord::Base
   def find_fork_of(project)
     return nil unless project.fork_network
 
-    if RequestStore.active?
-      forks_in_namespace = RequestStore.fetch("namespaces:#{id}:forked_projects") do
+    if Gitlab::SafeRequestStore.active?
+      forks_in_namespace = Gitlab::SafeRequestStore.fetch("namespaces:#{id}:forked_projects") do
         Hash.new do |found_forks, project|
           found_forks[project] = project.fork_network.find_forks_in(projects).first
         end

app/models/project.rb

@@ -2273,11 +2273,7 @@ class Project < ActiveRecord::Base
       end
     end
 
-    if RequestStore.active?
-      RequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_collaboration") do
-        check_access.call
-      end
-    else
+    Gitlab::SafeRequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_collaboration") do
       check_access.call
     end
   end

app/views/projects/jobs/_sidebar.html.haml

@@ -3,63 +3,6 @@
     .blocks-container
       #js-details-block-vue{ data: { terminal_path: can?(current_user, :create_build_terminal, @build) && @build.has_terminal? ? terminal_project_job_path(@project, @build) : nil } }
 
-      - if can?(current_user, :read_build, @project) && (@build.artifacts? || @build.artifacts_expired?)
-        .block
-          .title
-            Job artifacts
-          - if @build.artifacts_expired?
-            %p.build-detail-row
-              The artifacts were removed
-              #{time_ago_with_tooltip(@build.artifacts_expire_at)}
-          - elsif @build.has_expiring_artifacts?
-            %p.build-detail-row
-              The artifacts will be removed
-              #{time_ago_with_tooltip(@build.artifacts_expire_at)}
-
-          - if @build.artifacts?
-            .btn-group.d-flex{ role: :group }
-              - if @build.has_expiring_artifacts? && can?(current_user, :update_build, @build)
-                = link_to keep_project_job_artifacts_path(@project, @build), class: 'btn btn-sm btn-default', method: :post do
-                  Keep
-
-              = link_to download_project_job_artifacts_path(@project, @build), rel: 'nofollow', download: '', class: 'btn btn-sm btn-default' do
-                Download
-
-              - if @build.browsable_artifacts?
-                = link_to browse_project_job_artifacts_path(@project, @build), class: 'btn btn-sm btn-default' do
-                  Browse
-
-      - if @build.trigger_request
-        .build-widget.block
-          %h4.title
-            Trigger
-
-          - if @build.trigger_request&.trigger&.short_token
-            %p
-              %span.build-light-text Token:
-              #{@build.trigger_request.trigger.short_token}
-
-          - if @build.trigger_variables.any?
-            %p
-              %button.btn.group.js-reveal-variables Reveal Variables
-
-            %dl.js-build-variables.trigger-build-variables.hide
-              - @build.trigger_variables.each do |trigger_variable|
-                %dt.js-build-variable.trigger-build-variable= trigger_variable[:key]
-                %dd.js-build-value.trigger-build-value= trigger_variable[:value]
-
-      %div{ class: (@build.pipeline.stages_count > 1 ? "block" : "block-last") }
-        %p
-          Commit
-          = link_to @build.pipeline.short_sha, project_commit_path(@project, @build.pipeline.sha), class: 'commit-sha link-commit'
-          = clipboard_button(text: @build.pipeline.short_sha, title: "Copy commit SHA to clipboard")
-          - if @build.merge_request
-            in
-            = link_to "#{@build.merge_request.to_reference}", merge_request_path(@build.merge_request), class: 'link-commit'
-
-        %p.build-light-text.append-bottom-0
-          #{@build.pipeline.git_commit_title}
-
       - if @build.pipeline.stages_count > 1
         .block-last.dropdown.build-dropdown
           %div

changelogs/unreleased/50904-move-job-page-vue.yml

+---
+title: Use Vue components and new API to render Artifacts, Trigger Variables and Commit blocks on Job page
+merge_request: 21777
+author:
+type: other

changelogs/unreleased/51747-gitlab-com-unable-to-import-a-project-that-was-just-exported.yml

+---
+title: Fix NULL pipeline import problem and pipeline user mapping issue
+merge_request: 21875
+author:
+type: fixed

changelogs/unreleased/enable-force-write-auth-keys-restore.yml

+---
+title: Enable the ability to use the force env for rebuilding authorized_keys during a restore
+merge_request: 21896
+author:
+type: fixed

doc/administration/auth/ldap.md

@@ -112,7 +112,7 @@ main:
   uid: 'sAMAccountName' # This should be the attribute, not the value that maps to uid.
 
   ##
-  ## Examples: 'america\\momo' or 'CN=Gitlab Git,CN=Users,DC=mydomain,DC=com'
+  ## Examples: 'america\momo' or 'CN=Gitlab Git,CN=Users,DC=mydomain,DC=com'
   ##
   bind_dn: '_the_full_dn_of_the_user_you_will_bind_with'
   password: '_the_password_of_the_bind_user'

doc/development/documentation/styleguide.md

@@ -251,7 +251,7 @@ below.
   (in that order) that introduced it. The above quote would be then transformed to:
 
     ```md
-    > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/1242) in GitLab 8.3.
+    > [Introduced](<link-to-issue>) in GitLab 8.3.
     ```
 
 - If the feature is only available in GitLab Enterprise Edition, don't forget to mention
@@ -259,10 +259,22 @@ below.
   the feature is available in:
 
     ```md
-    > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/1242)
-    in [GitLab Starter](https://about.gitlab.com/pricing/) 8.3.
+    > [Introduced](<link-to-issue>) in [GitLab Starter](https://about.gitlab.com/pricing/) 10.3.
     ```
 
+#### Early versions of EE
+
+If the feature was created before GitLab 9.2 (before [different EE tiers were introduced](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1851)):
+
+- Declare it as "Introduced in GitLab Enterprise Edition X.Y".
+- Note which tier the feature is available in.
+
+For example:
+
+```md
+> [Introduced](<link-to-issue>) in GitLab Enterprise Edition 9.0. Available in [GitLab Premium](https://about.gitlab.com/pricing/).
+```
+
 ### Product badges
 
 When a feature is available in EE-only tiers, add the corresponding tier according to the

doc/development/merge_request_performance_guidelines.md

@@ -168,6 +168,7 @@ user objects for every username we can remove the need for running the same
 query for every mention of `@alice`.
 
 Caching data per transaction can be done using
-[RequestStore](https://github.com/steveklabnik/request_store). Caching data in
-Redis can be done using [Rails' caching
+[RequestStore](https://github.com/steveklabnik/request_store) (use
+`Gitlab::SafeRequestStore` to avoid having to remember to check
+`RequestStore.active?`). Caching data in Redis can be done using [Rails' caching
 system](http://guides.rubyonrails.org/caching_with_rails.html).

doc/development/ordering_table_columns.md

-# Ordering Table Columns
+# Ordering Table Columns in PostgreSQL
 
 Similar to C structures the space of a table is influenced by the order of
 columns. This is because the size of columns is aligned depending on the type of
-the column. Take the following column order for example:
+the following column. Let's consider an example:
 
-* id (integer, 4 bytes)
-* name (text, variable)
-* user_id (integer, 4 bytes)
+- `id` (integer, 4 bytes)
+- `name` (text, variable)
+- `user_id` (integer, 4 bytes)
 
-Integers are aligned to the word size. This means that on a 64 bit platform the
-actual size of each column would be: 8 bytes, variable, 8 bytes. This means that
-each row will require at least 16 bytes for the two integers, and a variable
-amount for the text field. If a table has a few rows this is not an issue, but
-once you start storing millions of rows you can save space by using a different
-order. For the above example a more ideal column order would be the following:
+The first column is a 4-byte integer. The next is text of variable length. The
+`text` data type requires 1-word alignment, and on 64-bit platform, 1 word is 8
+bytes. To meet the alignment requirements, four zeros are to be added right
+after the first column, so `id` occupies 4 bytes, then 4 bytes of alignment
+padding, and only next `name` is being stored. Therefore, in this case, 8 bytes
+will be spent for storing a 4-byte integer.
 
-* id (integer, 4 bytes)
-* user_id (integer, 4 bytes)
-* name (text, variable)
+The space between rows is also subject to alignment padding. The `user_id`
+column takes only 4 bytes, and on 64-bit platform, 4 zeroes will be added for
+alignment padding, to allow storing the next row beginning with the "clear" word.
 
-In this setup the `id` and `user_id` columns can be packed together, which means
-we only need 8 bytes to store _both_ of them. This in turn each row will require
-8 bytes less of space.
+As a result, the actual size of each column would be (ommiting variable length
+data and 24-byte tuple header): 8 bytes, variable, 8 bytes. This means that
+each row will require at least 16 bytes for the two 4-byte integers. If a table
+has a few rows this is not an issue. However, once you start storing millions of
+rows you can save space by using a different order. For the above example, the
+ideal column order would be the following:
+
+- `id` (integer, 4 bytes)
+- `user_id` (integer, 4 bytes)
+- `name` (text, variable)
+
+or 
+
+- `name` (text, variable)
+- `id` (integer, 4 bytes)
+- `user_id` (integer, 4 bytes)
+
+In these examples, the `id` and `user_id` columns are packed together, which
+means we only need 8 bytes to store _both_ of them. This in turn means each row
+will require 8 bytes less space.
 
 For GitLab we require that columns of new tables are ordered based to use the
 least amount of space. An easy way of doing this is to order them based on the
-type size in descending order with variable sizes (string and text columns for
-example) at the end.
+type size in descending order with variable sizes (`text`, `varchar`, arrays,
+`json`, `jsonb`, and so on) at the end.
 
 ## Type Sizes
 
@@ -36,7 +53,7 @@ of information we will list the sizes of common types here so it's easier to
 look them up. Here "word" refers to the word size, which is 4 bytes for a 32
 bits platform and 8 bytes for a 64 bits platform.
 
-| Type             | Size                                 | Aligned To |
+| Type             | Size                                 | Alignment needed |
 |:-----------------|:-------------------------------------|:-----------|
 | smallint         | 2 bytes                              | 1 word     |
 | integer          | 4 bytes                              | 1 word     |
@@ -58,7 +75,7 @@ always be at the end of a table.
 
 ## Real Example
 
-Let's use the "events" table as an example, which currently has the following
+Let's use the `events` table as an example, which currently has the following
 layout:
 
 | Column      | Type                        | Size     |
@@ -89,8 +106,8 @@ divided into fixed size chunks as follows:
 | 8 bytes    | updated_at        |
 | 8 bytes    | action, author_id |
 
-This means that excluding the variable sized data we need at least 48 bytes per
-row.
+This means that excluding the variable sized data and tuple header, we need at
+least 8 * 6 = 48 bytes per row.
 
 We can optimise this by using the following column order instead:
 
@@ -120,8 +137,8 @@ This would produce the following chunks:
 | variable   | title              |
 | variable   | data               |
 
-Here we only need 40 bytes per row excluding the variable sized data. 8 bytes
-being saved may not sound like much, but for tables as large as the "events"
-table it does begin to matter. For example, when storing 80 000 000 rows this
-translates to a space saving of at least 610 MB: all by just changing the order
-of a few columns.
+Here we only need 40 bytes per row excluding the variable sized data and 24-byte
+tuple header. 8 bytes being saved may not sound like much, but for tables as 
+large as the `events` table it does begin to matter. For example, when storing
+80 000 000 rows this translates to a space saving of at least 610 MB, all by
+just changing the order of a few columns.

lib/banzai/filter/abstract_reference_filter.rb

@@ -296,7 +296,7 @@ module Banzai
 
       # Returns projects for the given paths.
       def find_for_paths(paths)
-        if RequestStore.active?
+        if Gitlab::SafeRequestStore.active?
           cache = refs_cache
           to_query = paths - cache.keys
 
@@ -340,7 +340,7 @@ module Banzai
       end
 
       def refs_cache
-        RequestStore["banzai_#{parent_type}_refs".to_sym] ||= {}
+        Gitlab::SafeRequestStore["banzai_#{parent_type}_refs".to_sym] ||= {}
       end
 
       def parent_type

lib/banzai/filter/external_issue_reference_filter.rb

@@ -97,9 +97,7 @@ module Banzai
       private
 
       def external_issues_cached(attribute)
-        return project.public_send(attribute) unless RequestStore.active? # rubocop:disable GitlabSecurity/PublicSend
-
-        cached_attributes = RequestStore[:banzai_external_issues_tracker_attributes] ||= Hash.new { |h, k| h[k] = {} }
+        cached_attributes = Gitlab::SafeRequestStore[:banzai_external_issues_tracker_attributes] ||= Hash.new { |h, k| h[k] = {} }
         cached_attributes[project.id][attribute] = project.public_send(attribute) if cached_attributes[project.id][attribute].nil? # rubocop:disable GitlabSecurity/PublicSend
         cached_attributes[project.id][attribute]
       end

lib/banzai/reference_parser/base_parser.rb

@@ -166,7 +166,7 @@ module Banzai
       # objects that have not yet been queried. For objects that have already
       # been queried the object is returned from the cache.
       def collection_objects_for_ids(collection, ids)
-        if RequestStore.active?
+        if Gitlab::SafeRequestStore.active?
           ids = ids.map(&:to_i)
           cache = collection_cache[collection_cache_key(collection)]
           to_query = ids - cache.keys
@@ -248,7 +248,7 @@ module Banzai
       end
 
       def collection_cache
-        RequestStore[:banzai_collection_cache] ||= Hash.new do |hash, key|
+        Gitlab::SafeRequestStore[:banzai_collection_cache] ||= Hash.new do |hash, key|
           hash[key] = {}
         end
       end

lib/banzai/request_store_reference_cache.rb

 module Banzai
   module RequestStoreReferenceCache
     def cached_call(request_store_key, cache_key, path: [])
-      if RequestStore.active?
-        cache = RequestStore[request_store_key] ||= Hash.new do |hash, key|
+      if Gitlab::SafeRequestStore.active?
+        cache = Gitlab::SafeRequestStore[request_store_key] ||= Hash.new do |hash, key|
           hash[key] = Hash.new { |h, k| h[k] = {} }
         end
 

lib/feature.rb

@@ -28,11 +28,7 @@ class Feature
     end
 
     def persisted_names
-      if RequestStore.active?
-        RequestStore[:flipper_persisted_names] ||= FlipperFeature.feature_names
-      else
-        FlipperFeature.feature_names
-      end
+      Gitlab::SafeRequestStore[:flipper_persisted_names] ||= FlipperFeature.feature_names
     end
 
     def persisted?(feature)
@@ -76,11 +72,7 @@ class Feature
     end
 
     def flipper
-      if RequestStore.active?
-        RequestStore[:flipper] ||= build_flipper_instance
-      else
-        @flipper ||= build_flipper_instance
-      end
+      @flipper ||= (Gitlab::SafeRequestStore[:flipper] ||= build_flipper_instance)
     end
 
     def build_flipper_instance

lib/gitlab/cache/request_cache.rb

@@ -26,8 +26,8 @@ module Gitlab
 
           define_method(method_name) do |*args|
             store =
-              if RequestStore.active?
-                RequestStore.store
+              if Gitlab::SafeRequestStore.active?
+                Gitlab::SafeRequestStore.store
               else
                 ivar_name = # ! and ? cannot be used as ivar name
                   "@cache_#{method_name.to_s.tr('!?', "\u2605\u2606")}"

lib/gitlab/current_settings.rb

@@ -2,11 +2,7 @@ module Gitlab
   module CurrentSettings
     class << self
       def current_application_settings
-        if RequestStore.active?
-          RequestStore.fetch(:current_application_settings) { ensure_application_settings! }
-        else
-          ensure_application_settings!
-        end
+        Gitlab::SafeRequestStore.fetch(:current_application_settings) { ensure_application_settings! }
       end
 
       def fake_application_settings(attributes = {})

lib/gitlab/diff/position.rb

@@ -101,18 +101,14 @@ module Gitlab
         return @diff_file if defined?(@diff_file)
 
         @diff_file = begin
-          if RequestStore.active?
-            key = {
-              project_id: repository.project.id,
-              start_sha: start_sha,
-              head_sha: head_sha,
-              path: file_path
-            }
-
-            RequestStore.fetch(key) { find_diff_file(repository) }
-          else
-            find_diff_file(repository)
-          end
+          key = {
+            project_id: repository.project.id,
+            start_sha: start_sha,
+            head_sha: head_sha,
+            path: file_path
+          }
+
+          Gitlab::SafeRequestStore.fetch(key) { find_diff_file(repository) }
         end
       end
 

lib/gitlab/favicon.rb

@@ -47,7 +47,7 @@ module Gitlab
       end
 
       def appearance
-        RequestStore.store[:appearance] ||= (Appearance.current || Appearance.new)
+        Gitlab::SafeRequestStore[:appearance] ||= (Appearance.current || Appearance.new)
       end
 
       def appearance_favicon

lib/gitlab/git/hook_env.rb

@@ -17,18 +17,18 @@ module Gitlab
       ].freeze
 
       def self.set(gl_repository, env)
-        return unless RequestStore.active?
+        return unless Gitlab::SafeRequestStore.active?
 
         raise "missing gl_repository" if gl_repository.blank?
 
-        RequestStore.store[:gitlab_git_env] ||= {}
-        RequestStore.store[:gitlab_git_env][gl_repository] = whitelist_git_env(env)
+        Gitlab::SafeRequestStore[:gitlab_git_env] ||= {}
+        Gitlab::SafeRequestStore[:gitlab_git_env][gl_repository] = whitelist_git_env(env)
       end
 
       def self.all(gl_repository)
-        return {} unless RequestStore.active?
+        return {} unless Gitlab::SafeRequestStore.active?
 
-        h = RequestStore.fetch(:gitlab_git_env) { {} }
+        h = Gitlab::SafeRequestStore.fetch(:gitlab_git_env) { {} }
         h.fetch(gl_repository, {})
       end
 

lib/gitlab/git/storage/circuit_breaker.rb

@@ -11,7 +11,7 @@ module Gitlab
                  to: :failure_info
 
         def self.for_storage(storage)
-          cached_circuitbreakers = RequestStore.fetch(:circuitbreaker_cache) do
+          cached_circuitbreakers = Gitlab::SafeRequestStore.fetch(:circuitbreaker_cache) do
             Hash.new do |hash, storage_name|
               hash[storage_name] = build(storage_name)
             end

lib/gitlab/git/storage/failure_info.rb

@@ -10,7 +10,7 @@ module Gitlab
             redis.del(*all_storage_keys) unless all_storage_keys.empty?
           end
 
-          RequestStore.delete(:circuitbreaker_cache)
+          Gitlab::SafeRequestStore.delete(:circuitbreaker_cache)
         end
 
         def self.load(cache_key)

lib/gitlab/git/wiki.rb

@@ -115,11 +115,7 @@ module Gitlab
       def version(commit_id)
         commit_find_proc = -> { Gitlab::Git::Commit.find(@repository, commit_id) }
 
-        if RequestStore.active?
-          RequestStore.fetch([:wiki_version_commit, commit_id]) { commit_find_proc.call }
-        else
-          commit_find_proc.call
-        end
+        Gitlab::SafeRequestStore.fetch([:wiki_version_commit, commit_id]) { commit_find_proc.call }
       end
 
       def assert_type!(object, klass)

lib/gitlab/gitaly_client.rb

@@ -316,7 +316,7 @@ module Gitlab
     # Ensures that Gitaly is not being abuse through n+1 misuse etc
     def self.enforce_gitaly_request_limits(call_site)
       # Only count limits in request-response environments (not sidekiq for example)
-      return unless RequestStore.active?
+      return unless Gitlab::SafeRequestStore.active?
 
       # This is this actual number of times this call was made. Used for information purposes only
       actual_call_count = increment_call_count("gitaly_#{call_site}_actual")
@@ -340,7 +340,7 @@ module Gitlab
     end
 
     def self.allow_n_plus_1_calls
-      return yield unless RequestStore.active?
+      return yield unless Gitlab::SafeRequestStore.active?
 
       begin
         increment_call_count(:gitaly_call_count_exception_block_depth)
@@ -351,25 +351,25 @@ module Gitlab
     end
 
     def self.get_call_count(key)
-      RequestStore.store[key] || 0
+      Gitlab::SafeRequestStore[key] || 0
     end
     private_class_method :get_call_count
 
     def self.increment_call_count(key)
-      RequestStore.store[key] ||= 0
-      RequestStore.store[key] += 1
+      Gitlab::SafeRequestStore[key] ||= 0
+      Gitlab::SafeRequestStore[key] += 1
     end
     private_class_method :increment_call_count
 
     def self.decrement_call_count(key)
-      RequestStore.store[key] -= 1
+      Gitlab::SafeRequestStore[key] -= 1
     end
     private_class_method :decrement_call_count
 
     # Returns an estimate of the number of Gitaly calls made for this
     # request
     def self.get_request_count
-      return 0 unless RequestStore.active?
+      return 0 unless Gitlab::SafeRequestStore.active?
 
       gitaly_migrate_count = get_call_count("gitaly_migrate_actual")
       gitaly_call_count = get_call_count("gitaly_call_actual")
@@ -386,28 +386,28 @@ module Gitlab
     end
 
     def self.reset_counts
-      return unless RequestStore.active?
+      return unless Gitlab::SafeRequestStore.active?
 
       %w[migrate call].each do |call_site|
-        RequestStore.store["gitaly_#{call_site}_actual"] = 0
-        RequestStore.store["gitaly_#{call_site}_permitted"] = 0
+        Gitlab::SafeRequestStore["gitaly_#{call_site}_actual"] = 0
+        Gitlab::SafeRequestStore["gitaly_#{call_site}_permitted"] = 0
       end
     end
 
     def self.add_call_details(details)
       id = details.delete(:id)
 
-      return unless id && RequestStore.active? && RequestStore.store[:peek_enabled]
+      return unless id && Gitlab::SafeRequestStore[:peek_enabled]
 
-      RequestStore.store['gitaly_call_details'] ||= {}
-      RequestStore.store['gitaly_call_details'][id] ||= {}
-      RequestStore.store['gitaly_call_details'][id].merge!(details)
+      Gitlab::SafeRequestStore['gitaly_call_details'] ||= {}
+      Gitlab::SafeRequestStore['gitaly_call_details'][id] ||= {}
+      Gitlab::SafeRequestStore['gitaly_call_details'][id].merge!(details)
     end
 
     def self.list_call_details
-      return {} unless RequestStore.active? && RequestStore.store[:peek_enabled]
+      return {} unless Gitlab::SafeRequestStore[:peek_enabled]
 
-      RequestStore.store['gitaly_call_details'] || {}
+      Gitlab::SafeRequestStore['gitaly_call_details'] || {}
     end
 
     def self.expected_server_version
@@ -445,22 +445,22 @@ module Gitlab
 
     # Count a stack. Used for n+1 detection
     def self.count_stack
-      return unless RequestStore.active?
+      return unless Gitlab::SafeRequestStore.active?
 
       stack_string = Gitlab::Profiler.clean_backtrace(caller).drop(1).join("\n")
 
-      RequestStore.store[:stack_counter] ||= Hash.new
+      Gitlab::SafeRequestStore[:stack_counter] ||= Hash.new
 
-      count = RequestStore.store[:stack_counter][stack_string] || 0
-      RequestStore.store[:stack_counter][stack_string] = count + 1
+      count = Gitlab::SafeRequestStore[:stack_counter][stack_string] || 0
+      Gitlab::SafeRequestStore[:stack_counter][stack_string] = count + 1
     end
     private_class_method :count_stack
 
     # Returns a count for the stack which called Gitaly the most times. Used for n+1 detection
     def self.max_call_count
-      return 0 unless RequestStore.active?
+      return 0 unless Gitlab::SafeRequestStore.active?
 
-      stack_counter = RequestStore.store[:stack_counter]
+      stack_counter = Gitlab::SafeRequestStore[:stack_counter]
       return 0 unless stack_counter
 
       stack_counter.values.max
@@ -469,9 +469,9 @@ module Gitlab
 
     # Returns the stacks that calls Gitaly the most times. Used for n+1 detection
     def self.max_stacks
-      return nil unless RequestStore.active?
+      return nil unless Gitlab::SafeRequestStore.active?
 
-      stack_counter = RequestStore.store[:stack_counter]
+      stack_counter = Gitlab::SafeRequestStore[:stack_counter]
       return nil unless stack_counter
 
       max = max_call_count

lib/gitlab/gitaly_client/commit_service.rb

@@ -240,22 +240,23 @@ module Gitlab
       end
 
       def find_commit(revision)
-        if RequestStore.active?
-          # We don't use RequeStstore.fetch(key) { ... } directly because `revision`
-          # can be a branch name, so we can't use it as a key as it could point
-          # to another commit later on (happens a lot in tests).
+        if Gitlab::SafeRequestStore.active?
+          # We don't use Gitlab::SafeRequestStore.fetch(key) { ... } directly
+          # because `revision` can be a branch name, so we can't use it as a key
+          # as it could point to another commit later on (happens a lot in
+          # tests).
           key = {
             storage: @gitaly_repo.storage_name,
             relative_path: @gitaly_repo.relative_path,
             commit_id: revision
           }
-          return RequestStore[key] if RequestStore.exist?(key)
+          return Gitlab::SafeRequestStore[key] if Gitlab::SafeRequestStore.exist?(key)
 
           commit = call_find_commit(revision)
           return unless commit
 
           key[:commit_id] = commit.id
-          RequestStore[key] = commit
+          Gitlab::SafeRequestStore[key] = commit
         else
           call_find_commit(revision)
         end

lib/gitlab/import_export/project_tree_restorer.rb

@@ -136,9 +136,18 @@ module Gitlab
         return if tree_hash[relation_key].blank?
 
         tree_array = [tree_hash[relation_key]].flatten
+        null_iid_pipelines = []
 
         # Avoid keeping a possible heavy object in memory once we are done with it
-        while relation_item = tree_array.shift
+        while relation_item = (tree_array.shift || null_iid_pipelines.shift)
+          if nil_iid_pipeline?(relation_key, relation_item) && tree_array.any?
+            # Move pipelines with NULL IIDs to the end
+            # so they don't clash with existing IIDs.
+            null_iid_pipelines << relation_item
+
+            next
+          end
+
           # The transaction at this level is less speedy than one single transaction
           # But we can't have it in the upper level or GC won't get rid of the AR objects
           # after we save the batch.
@@ -201,6 +210,10 @@ module Gitlab
       def excluded_keys_for_relation(relation)
         reader.attributes_finder.find_excluded_keys(relation)
       end
+
+      def nil_iid_pipeline?(relation_key, relation_item)
+        relation_key == 'pipelines' && relation_item['iid'].nil?
+      end
     end
   end
 end

lib/gitlab/import_export/relation_factory.rb

@@ -88,7 +88,6 @@ module Gitlab
         case @relation_name
         when :merge_request_diff_files       then setup_diff
         when :notes                          then setup_note
-        when 'Ci::Pipeline'                  then setup_pipeline
         end
 
         update_user_references
@@ -96,6 +95,8 @@ module Gitlab
         update_group_references
         remove_duplicate_assignees
 
+        setup_pipeline if @relation_name == 'Ci::Pipeline'
+
         reset_tokens!
         remove_encrypted_attributes!
       end

lib/gitlab/issuables_count_for_state.rb

 module Gitlab
   # Class for counting and caching the number of issuables per state.
   class IssuablesCountForState
-    # The name of the RequestStore cache key.
+    # The name of the Gitlab::SafeRequestStore cache key.
     CACHE_KEY = :issuables_count_for_state
 
     # The state values that can be safely casted to a Symbol.
@@ -10,12 +10,7 @@ module Gitlab
     # finder - The finder class to use for retrieving the issuables.
     def initialize(finder)
       @finder = finder
-      @cache =
-        if RequestStore.active?
-          RequestStore[CACHE_KEY] ||= initialize_cache
-        else
-          initialize_cache
-        end
+      @cache = Gitlab::SafeRequestStore[CACHE_KEY] ||= initialize_cache
     end
 
     def for_state_or_opened(state = nil)

lib/gitlab/logger.rb

@@ -30,7 +30,7 @@ module Gitlab
     end
 
     def self.build
-      RequestStore[self.cache_key] ||= new(self.full_log_path)
+      Gitlab::SafeRequestStore[self.cache_key] ||= new(self.full_log_path)
     end
 
     def self.full_log_path

lib/gitlab/null_request_store.rb

+# frozen_string_literal: true
+
+# Used by Gitlab::SafeRequestStore
+module Gitlab
+  # The methods `begin!`, `clear!`, and `end!` are not defined because they
+  # should only be called directly on `RequestStore`.
+  class NullRequestStore
+    def store
+      {}
+    end
+
+    def active?
+    end
+
+    def read(key)
+    end
+
+    def [](key)
+    end
+
+    def write(key, value)
+      value
+    end
+
+    def []=(key, value)
+      value
+    end
+
+    def exist?(key)
+      false
+    end
+
+    def fetch(key, &block)
+      yield
+    end
+
+    def delete(key, &block)
+      yield(key) if block_given?
+    end
+  end
+end

lib/gitlab/performance_bar/peek_query_tracker.rb

@@ -23,7 +23,7 @@ module Gitlab
         end
 
         subscribe('sql.active_record') do |_, start, finish, _, data|
-          if RequestStore.active? && RequestStore.store[:peek_enabled]
+          if Gitlab::SafeRequestStore.store[:peek_enabled]
             # data[:cached] is only available starting from Rails 5.1.0
             # https://github.com/rails/rails/blob/v5.1.0/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb#L113
             # Before that, data[:name] was set to 'CACHE'

lib/gitlab/request_context.rb

@@ -2,7 +2,7 @@ module Gitlab
   class RequestContext
     class << self
       def client_ip
-        RequestStore[:client_ip]
+        Gitlab::SafeRequestStore[:client_ip]
       end
     end
 
@@ -13,7 +13,7 @@ module Gitlab
     def call(env)
       req = Rack::Request.new(env)
 
-      RequestStore[:client_ip] = req.ip
+      Gitlab::SafeRequestStore[:client_ip] = req.ip
 
       @app.call(env)
     end

lib/gitlab/safe_request_store.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module SafeRequestStore
+    NULL_STORE = Gitlab::NullRequestStore.new
+
+    class << self
+      # These methods should always run directly against RequestStore
+      delegate :clear!, :begin!, :end!, :active?, to: :RequestStore
+
+      # These methods will run against NullRequestStore if RequestStore is disabled
+      delegate :read, :[], :write, :[]=, :exist?, :fetch, :delete, to: :store
+    end
+
+    def self.store
+      if RequestStore.active?
+        RequestStore
+      else
+        NULL_STORE
+      end
+    end
+  end
+end

lib/gitlab/temporarily_allow.rb

@@ -10,7 +10,7 @@ module Gitlab
     end
 
     def temporarily_allowed?(key)
-      if RequestStore.active?
+      if Gitlab::SafeRequestStore.active?
         temporarily_allow_request_store[key] > 0
       else
         TEMPORARILY_ALLOW_MUTEX.synchronize do
@@ -26,11 +26,11 @@ module Gitlab
     end
 
     def temporarily_allow_request_store
-      RequestStore[:temporarily_allow] ||= Hash.new(0)
+      Gitlab::SafeRequestStore[:temporarily_allow] ||= Hash.new(0)
     end
 
     def temporarily_allow_add(key, value)
-      if RequestStore.active?
+      if Gitlab::SafeRequestStore.active?
         temporarily_allow_request_store[key] += value
       else
         TEMPORARILY_ALLOW_MUTEX.synchronize do

lib/gitlab/testing/request_inspector_middleware.rb

@@ -35,11 +35,15 @@ module Gitlab
         request_headers = env_http_headers(env)
         status, headers, body = @app.call(env)
 
+        full_body = ''
+        body.each { |b| full_body << b }
+
         request = OpenStruct.new(
           url: url,
           status_code: status,
           request_headers: request_headers,
-          response_headers: headers
+          response_headers: headers,
+          body: full_body
         )
         log_request request
 

locale/gitlab.pot

@@ -4248,7 +4248,7 @@ msgstr ""
 msgid "Job|The artifacts were removed"
 msgstr ""
 
-msgid "Job|The artifacts will be removed"
+msgid "Job|The artifacts will be removed in"
 msgstr ""
 
 msgid "Job|This job is stuck, because the project doesn't have any runners online assigned to it."

package.json

@@ -35,7 +35,7 @@
     "classlist-polyfill": "^1.2.0",
     "clipboard": "^1.7.1",
     "codesandbox-api": "^0.0.18",
-    "compression-webpack-plugin": "^1.1.11",
+    "compression-webpack-plugin": "^2.0.0",
     "core-js": "^2.4.1",
     "cropper": "^2.3.0",
     "css-loader": "^1.0.0",
@@ -57,7 +57,7 @@
     "dropzone": "^4.2.0",
     "emoji-unicode-version": "^0.2.1",
     "exports-loader": "^0.7.0",
-    "file-loader": "^1.1.11",
+    "file-loader": "^2.0.0",
     "formdata-polyfill": "^3.0.11",
     "fuzzaldrin-plus": "^0.5.0",
     "glob": "^7.1.2",
@@ -72,7 +72,7 @@
     "katex": "^0.9.0",
     "marked": "^0.3.12",
     "monaco-editor": "^0.14.3",
-    "monaco-editor-webpack-plugin": "^1.5.2",
+    "monaco-editor-webpack-plugin": "^1.5.4",
     "mousetrap": "^1.4.6",
     "pikaday": "^1.6.1",
     "popper.js": "^1.14.3",
@@ -87,14 +87,14 @@
     "sortablejs": "^1.7.0",
     "sql.js": "^0.4.0",
     "stickyfilljs": "^2.0.5",
-    "style-loader": "^0.21.0",
+    "style-loader": "^0.23.0",
     "svg4everybody": "2.1.9",
     "three": "^0.84.0",
     "three-orbit-controls": "^82.1.0",
     "three-stl-loader": "^1.0.4",
     "timeago.js": "^3.0.2",
     "underscore": "^1.9.0",
-    "url-loader": "^1.0.1",
+    "url-loader": "^1.1.1",
     "visibilityjs": "^1.2.4",
     "vue": "^2.5.16",
     "vue-loader": "^15.2.4",
@@ -103,9 +103,9 @@
     "vue-template-compiler": "^2.5.16",
     "vue-virtual-scroll-list": "^1.2.5",
     "vuex": "^3.0.1",
-    "webpack": "^4.16.0",
-    "webpack-bundle-analyzer": "^2.13.1",
-    "webpack-cli": "^3.0.8",
+    "webpack": "^4.19.1",
+    "webpack-bundle-analyzer": "^3.0.2",
+    "webpack-cli": "^3.1.0",
     "webpack-stats-plugin": "^0.2.1",
     "worker-loader": "^2.0.0",
     "xterm": "^3.5.0"
@@ -113,12 +113,12 @@
   "devDependencies": {
     "axios-mock-adapter": "^1.15.0",
     "babel-eslint": "^9.0.0",
-    "babel-plugin-istanbul": "^4.1.6",
-    "babel-plugin-rewire": "^1.1.0",
+    "babel-plugin-istanbul": "^5.0.1",
+    "babel-plugin-rewire": "^1.2.0",
     "babel-template": "^6.26.0",
     "babel-types": "^6.26.0",
     "chalk": "^2.4.1",
-    "commander": "^2.15.1",
+    "commander": "^2.18.0",
     "eslint": "~5.6.0",
     "eslint-config-airbnb-base": "^13.1.0",
     "eslint-import-resolver-webpack": "^0.10.1",
@@ -143,8 +143,8 @@
     "karma-mocha-reporter": "^2.2.5",
     "karma-sourcemap-loader": "^0.3.7",
     "karma-webpack": "^4.0.0-beta.0",
-    "nodemon": "^1.18.2",
+    "nodemon": "^1.18.4",
     "prettier": "1.12.1",
-    "webpack-dev-server": "^3.1.4"
+    "webpack-dev-server": "^3.1.8"
   }
 }

spec/features/projects/artifacts/user_downloads_artifacts_spec.rb

@@ -20,23 +20,13 @@ describe "User downloads artifacts" do
     end
 
     context "via job id" do
-      set(:url) { download_project_job_artifacts_path(project, job) }
+      let(:url) { download_project_job_artifacts_path(project, job) }
 
       it_behaves_like "downloading"
     end
 
     context "via branch name and job name" do
-      set(:url) { latest_succeeded_project_artifacts_path(project, "#{pipeline.ref}/download", job: job.name) }
-
-      it_behaves_like "downloading"
-    end
-
-    context "via clicking the `Download` button" do
-      set(:url) { project_job_path(project, job) }
-
-      before do
-        click_link("Download")
-      end
+      let(:url) { latest_succeeded_project_artifacts_path(project, "#{pipeline.ref}/download", job: job.name) }
 
       it_behaves_like "downloading"
     end

spec/features/projects/jobs_spec.rb

@@ -5,7 +5,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
   let(:user) { create(:user) }
   let(:user_access_level) { :developer }
   let(:project) { create(:project, :repository) }
-  let(:pipeline) { create(:ci_pipeline, project: project) }
+  let(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) }
 
   let(:job) { create(:ci_build, :trace_live, pipeline: pipeline) }
   let(:job2) { create(:ci_build) }
@@ -20,7 +20,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
   end
 
   describe "GET /:project/jobs" do
-    let!(:job) { create(:ci_build,  pipeline: pipeline) }
+    let!(:job) { create(:ci_build, pipeline: pipeline) }
 
     context "Pending scope" do
       before do
@@ -115,22 +115,28 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
     context "Job from project" do
       let(:job) { create(:ci_build, :success, :trace_live, pipeline: pipeline) }
 
-      before do
+      it 'shows status name', :js do
         visit project_job_path(project, job)
-      end
 
-      it 'shows status name', :js do
+        wait_for_requests
+
         expect(page).to have_css('.ci-status.ci-success', text: 'passed')
       end
 
-      it 'shows commit`s data' do
-        expect(page.status_code).to eq(200)
+      it 'shows commit`s data', :js do
+        requests = inspect_requests() do
+          visit project_job_path(project, job)
+        end
+
+        wait_for_requests
+        expect(requests.first.status_code).to eq(200)
         expect(page).to have_content pipeline.sha[0..7]
-        expect(page).to have_content pipeline.git_commit_message
-        expect(page).to have_content pipeline.git_author_name
+        expect(page).to have_content pipeline.commit.title
       end
 
       it 'shows active job' do
+        visit project_job_path(project, job)
+
         expect(page).to have_selector('.build-job.active')
       end
     end
@@ -199,7 +205,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
       it { expect(page.status_code).to eq(404) }
     end
 
-    context "Download artifacts" do
+    context "Download artifacts", :js do
       before do
         job.update(legacy_artifacts_file: artifacts_file)
         visit project_job_path(project, job)
@@ -208,9 +214,22 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
       it 'has button to download artifacts' do
         expect(page).to have_content 'Download'
       end
+
+      it 'downloads the zip file when user clicks the download button' do
+        requests = inspect_requests() do
+          click_link 'Download'
+        end
+
+        artifact_request = requests.find { |req| req.url.match(%r{artifacts/download}) }
+
+        expect(artifact_request.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename="#{job.artifacts_file.filename}"})
+        expect(artifact_request.response_headers['Content-Transfer-Encoding']).to eq("binary")
+        expect(artifact_request.response_headers['Content-Type']).to eq("image/gif")
+        expect(artifact_request.body).to eq(job.artifacts_file.file.read.b)
+      end
     end
 
-    context 'Artifacts expire date' do
+    context 'Artifacts expire date', :js do
       before do
         job.update(legacy_artifacts_file: artifacts_file,
                    artifacts_expire_at: expire_at)
@@ -231,12 +250,12 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
 
         context 'when user has ability to update job' do
           it 'keeps artifacts when keep button is clicked' do
-            expect(page).to have_content 'The artifacts will be removed'
+            expect(page).to have_content 'The artifacts will be removed in'
 
             click_link 'Keep'
 
             expect(page).to have_no_link 'Keep'
-            expect(page).to have_no_content 'The artifacts will be removed'
+            expect(page).to have_no_content 'The artifacts will be removed in'
           end
         end
 
@@ -314,6 +333,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
 
       shared_examples 'expected variables behavior' do
         it 'shows variable key and value after click', :js do
+          expect(page).to have_content('Token')
           expect(page).to have_css('.js-reveal-variables')
           expect(page).not_to have_css('.js-build-variable')
           expect(page).not_to have_css('.js-build-value')
@@ -542,20 +562,26 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
     end
   end
 
-  describe "GET /:project/jobs/:id/download" do
+  describe "GET /:project/jobs/:id/download", :js do
     before do
       job.update(legacy_artifacts_file: artifacts_file)
       visit project_job_path(project, job)
+
       click_link 'Download'
     end
 
     context "Build from other project" do
       before do
         job2.update(legacy_artifacts_file: artifacts_file)
-        visit download_project_job_artifacts_path(project, job2)
       end
 
-      it { expect(page.status_code).to eq(404) }
+      it do
+        requests = inspect_requests() do
+          visit download_project_job_artifacts_path(project, job2)
+        end
+
+        expect(requests.first.status_code).to eq(404)
+      end
     end
   end
 

spec/features/projects/tree/create_directory_spec.rb

@@ -43,7 +43,7 @@ describe 'Multi-file editor new directory', :js do
     find('.js-ide-commit-mode').click
 
     find('.multi-file-commit-list-item').hover
-    first('.multi-file-discard-btn .btn').click
+    click_button 'Stage'
 
     fill_in('commit-message', with: 'commit message ide')
 

spec/features/projects/tree/create_file_spec.rb

@@ -35,7 +35,7 @@ describe 'Multi-file editor new file', :js do
     find('.js-ide-commit-mode').click
 
     find('.multi-file-commit-list-item').hover
-    first('.multi-file-discard-btn .btn').click
+    click_button 'Stage'
 
     fill_in('commit-message', with: 'commit message ide')
 

spec/javascripts/ide/components/commit_sidebar/list_item_spec.js

@@ -33,10 +33,6 @@ describe('Multi-file editor commit sidebar list item', () => {
     expect(vm.$el.querySelector('.multi-file-commit-list-path').textContent).toContain(f.path);
   });
 
-  it('renders actionn button', () => {
-    expect(vm.$el.querySelector('.multi-file-discard-btn')).not.toBeNull();
-  });
-
   it('opens a closed file in the editor when clicking the file path', done => {
     spyOn(vm, 'openPendingTab').and.callThrough();
     spyOn(router, 'push');

spec/javascripts/ide/components/repo_commit_section_spec.js

@@ -103,65 +103,6 @@ describe('RepoCommitSection', () => {
     });
   });
 
-  it('adds changed files into staged files', done => {
-    vm.$el.querySelector('.multi-file-discard-btn .btn').click();
-    vm
-      .$nextTick()
-      .then(() => vm.$el.querySelector('.multi-file-discard-btn .btn').click())
-      .then(vm.$nextTick)
-      .then(() => {
-        expect(vm.$el.querySelector('.ide-commit-list-container').textContent).toContain(
-          'There are no unstaged changes',
-        );
-      })
-      .then(done)
-      .catch(done.fail);
-  });
-
-  it('stages a single file', done => {
-    vm.$el.querySelector('.multi-file-discard-btn .btn').click();
-
-    Vue.nextTick(() => {
-      expect(
-        vm.$el
-          .querySelector('.ide-commit-list-container')
-          .querySelectorAll('.multi-file-commit-list > li').length,
-      ).toBe(1);
-
-      done();
-    });
-  });
-
-  it('discards a single file', done => {
-    vm.$el.querySelector('.multi-file-commit-list li:first-child .js-modal-primary-action').click();
-
-    Vue.nextTick(() => {
-      expect(vm.$el.querySelector('.ide-commit-list-container').textContent).not.toContain('file1');
-      expect(
-        vm.$el
-          .querySelector('.ide-commit-list-container')
-          .querySelectorAll('.multi-file-commit-list > li').length,
-      ).toBe(1);
-
-      done();
-    });
-  });
-
-  it('unstages a single file', done => {
-    vm.$el
-      .querySelectorAll('.multi-file-discard-btn')[2]
-      .querySelector('.btn')
-      .click();
-
-    Vue.nextTick(() => {
-      expect(
-        vm.$el.querySelectorAll('.ide-commit-list-container')[1].querySelectorAll('li').length,
-      ).toBe(1);
-
-      done();
-    });
-  });
-
   describe('mounted', () => {
     it('opens last opened file', () => {
       expect(store.state.openFiles.length).toBe(1);

spec/javascripts/jobs/components/artifacts_block_spec.js

@@ -11,6 +11,19 @@ describe('Artifacts block', () => {
   const timeago = getTimeago();
   const formatedDate = timeago.format(expireAt);
 
+  const expiredArtifact = {
+    expire_at: expireAt,
+    expired: true,
+  };
+
+  const nonExpiredArtifact = {
+    download_path: '/gitlab-org/gitlab-ce/-/jobs/98314558/artifacts/download',
+    browse_path: '/gitlab-org/gitlab-ce/-/jobs/98314558/artifacts/browse',
+    keep_path: '/gitlab-org/gitlab-ce/-/jobs/98314558/artifacts/keep',
+    expire_at: expireAt,
+    expired: false,
+  };
+
   afterEach(() => {
     vm.$destroy();
   });
@@ -18,100 +31,87 @@ describe('Artifacts block', () => {
   describe('with expired artifacts', () => {
     it('renders expired artifact date and info', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
+        artifact: expiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-artifacts-removed')).not.toBeNull();
       expect(vm.$el.querySelector('.js-artifacts-will-be-removed')).toBeNull();
       expect(vm.$el.textContent).toContain(formatedDate);
+      expect(vm.$el.querySelector('.js-artifacts-removed').textContent.trim()).toEqual(
+        'The artifacts were removed',
+      );
     });
   });
 
   describe('with artifacts that will expire', () => {
     it('renders will expire artifact date and info', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: false,
-        willArtifactsExpire: true,
-        expireAt,
+        artifact: nonExpiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-artifacts-removed')).toBeNull();
       expect(vm.$el.querySelector('.js-artifacts-will-be-removed')).not.toBeNull();
       expect(vm.$el.textContent).toContain(formatedDate);
+      expect(vm.$el.querySelector('.js-artifacts-will-be-removed').textContent.trim()).toEqual(
+        'The artifacts will be removed in',
+      );
     });
   });
 
-  describe('when the user can keep the artifacts', () => {
+  describe('with keep path', () => {
     it('renders the keep button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
-        keepArtifactsPath: '/keep',
+        artifact: nonExpiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-keep-artifacts')).not.toBeNull();
     });
   });
 
-  describe('when the user can not keep the artifacts', () => {
+  describe('without keep path', () => {
     it('does not render the keep button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
+        artifact: expiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-keep-artifacts')).toBeNull();
     });
   });
 
-  describe('when the user can download the artifacts', () => {
+  describe('with download path', () => {
     it('renders the download button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
-        downloadArtifactsPath: '/download',
+        artifact: nonExpiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-download-artifacts')).not.toBeNull();
     });
   });
 
-  describe('when the user can not download the artifacts', () => {
+  describe('without download path', () => {
     it('does not render the keep button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
+        artifact: expiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-download-artifacts')).toBeNull();
     });
   });
 
-  describe('when the user can browse the artifacts', () => {
+  describe('with browse path', () => {
     it('does not render the browse button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
-        browseArtifactsPath: '/browse',
+        artifact: nonExpiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-browse-artifacts')).not.toBeNull();
     });
   });
 
-  describe('when the user can not browse the artifacts', () => {
+  describe('without browse path', () => {
     it('does not render the browse button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
+        artifact: expiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-browse-artifacts')).toBeNull();

spec/javascripts/jobs/components/commit_block_spec.js

@@ -7,11 +7,16 @@ describe('Commit block', () => {
   let vm;
 
   const props = {
-    pipelineShortSha: '1f0fb84f',
-    pipelineShaPath: 'commit/1f0fb84fb6770d74d97eee58118fd3909cd4f48c',
-    mergeRequestReference: '!21244',
-    mergeRequestPath: 'merge_requests/21244',
-    gitCommitTitlte: 'Regenerate pot files',
+    commit: {
+      short_id: '1f0fb84f',
+      commit_path: 'commit/1f0fb84fb6770d74d97eee58118fd3909cd4f48c',
+      title: 'Update README.md',
+    },
+    mergeRequest: {
+      iid: '!21244',
+      path: 'merge_requests/21244',
+    },
+    isLastBlock: true,
   };
 
   afterEach(() => {
@@ -26,12 +31,18 @@ describe('Commit block', () => {
     });
 
     it('renders pipeline short sha link', () => {
-      expect(vm.$el.querySelector('.js-commit-sha').getAttribute('href')).toEqual(props.pipelineShaPath);
-      expect(vm.$el.querySelector('.js-commit-sha').textContent.trim()).toEqual(props.pipelineShortSha);
+      expect(vm.$el.querySelector('.js-commit-sha').getAttribute('href')).toEqual(
+        props.commit.commit_path,
+      );
+      expect(vm.$el.querySelector('.js-commit-sha').textContent.trim()).toEqual(
+        props.commit.short_id,
+      );
     });
 
     it('renders clipboard button', () => {
-      expect(vm.$el.querySelector('button').getAttribute('data-clipboard-text')).toEqual(props.pipelineShortSha);
+      expect(vm.$el.querySelector('button').getAttribute('data-clipboard-text')).toEqual(
+        props.commit.short_id,
+      );
     });
   });
 
@@ -41,17 +52,19 @@ describe('Commit block', () => {
         ...props,
       });
 
-      expect(vm.$el.querySelector('.js-link-commit').getAttribute('href')).toEqual(props.mergeRequestPath);
-      expect(vm.$el.querySelector('.js-link-commit').textContent.trim()).toEqual(props.mergeRequestReference);
-
+      expect(vm.$el.querySelector('.js-link-commit').getAttribute('href')).toEqual(
+        props.mergeRequest.path,
+      );
+      expect(vm.$el.querySelector('.js-link-commit').textContent.trim()).toEqual(
+        props.mergeRequest.iid,
+      );
     });
   });
 
   describe('without merge request', () => {
     it('does not render merge request', () => {
       const copyProps = Object.assign({}, props);
-      delete copyProps.mergeRequestPath;
-      delete copyProps.mergeRequestReference;
+      delete copyProps.mergeRequest;
 
       vm = mountComponent(Component, {
         ...copyProps,
@@ -67,7 +80,7 @@ describe('Commit block', () => {
         ...props,
       });
 
-      expect(vm.$el.textContent).toContain(props.gitCommitTitlte);
+      expect(vm.$el.textContent).toContain(props.commit.title);
     });
   });
 });

spec/javascripts/jobs/components/trigger_value_spec.js → spec/javascripts/jobs/components/trigger_block_spec.js

@@ -13,7 +13,9 @@ describe('Trigger block', () => {
   describe('with short token', () => {
     it('renders short token', () => {
       vm = mountComponent(Component, {
-        shortToken: '0a666b2',
+        trigger: {
+          short_token: '0a666b2',
+        },
       });
 
       expect(vm.$el.querySelector('.js-short-token').textContent).toContain('0a666b2');
@@ -22,7 +24,7 @@ describe('Trigger block', () => {
 
   describe('without short token', () => {
     it('does not render short token', () => {
-      vm = mountComponent(Component, {});
+      vm = mountComponent(Component, { trigger: {} });
 
       expect(vm.$el.querySelector('.js-short-token')).toBeNull();
     });
@@ -32,9 +34,12 @@ describe('Trigger block', () => {
     describe('reveal variables', () => {
       it('reveals variables on click', done => {
         vm = mountComponent(Component, {
-          variables: {
-            key: 'value',
-            variable: 'foo',
+          trigger: {
+            short_token: 'bd7e',
+            variables: [
+              { key: 'UPLOAD_TO_GCS', value: 'false', public: false },
+              { key: 'UPLOAD_TO_S3', value: 'true', public: false },
+            ],
           },
         });
 
@@ -44,10 +49,10 @@ describe('Trigger block', () => {
           .$nextTick()
           .then(() => {
             expect(vm.$el.querySelector('.js-build-variables')).not.toBeNull();
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('key');
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('value');
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('variable');
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('foo');
+            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('UPLOAD_TO_GCS');
+            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('false');
+            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('UPLOAD_TO_S3');
+            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('true');
           })
           .then(done)
           .catch(done.fail);
@@ -57,7 +62,7 @@ describe('Trigger block', () => {
 
   describe('without variables', () => {
     it('does not render variables', () => {
-      vm = mountComponent(Component);
+      vm = mountComponent(Component, { trigger: {} });
 
       expect(vm.$el.querySelector('.js-reveal-variables')).toBeNull();
       expect(vm.$el.querySelector('.js-build-variables')).toBeNull();

spec/javascripts/vue_mr_widget/mr_widget_options_spec.js

@@ -27,6 +27,10 @@ describe('mrWidgetOptions', () => {
     });
   });
 
+  afterEach(() => {
+    vm.$destroy();
+  });
+
   describe('data', () => {
     it('should instantiate Store and Service', () => {
       expect(vm.mr).toBeDefined();

spec/lib/banzai/filter/external_issue_reference_filter_spec.rb

@@ -79,13 +79,9 @@ describe Banzai::Filter::ExternalIssueReferenceFilter do
       expect(link).to eq helper.url_for_issue(issue_id, project, only_path: true)
     end
 
-    context 'with RequestStore enabled' do
+    context 'with RequestStore enabled', :request_store do
       let(:reference_filter) { HTML::Pipeline.new([described_class]) }
 
-      before do
-        allow(RequestStore).to receive(:active?).and_return(true)
-      end
-
       it 'queries the collection on the first call' do
         expect_any_instance_of(Project).to receive(:default_issues_tracker?).once.and_call_original
         expect_any_instance_of(Project).to receive(:external_issue_reference_pattern).once.and_call_original

spec/lib/banzai/reference_parser/base_parser_spec.rb

@@ -263,11 +263,10 @@ describe Banzai::ReferenceParser::BaseParser do
       end
     end
 
-    context 'with RequestStore enabled' do
+    context 'with RequestStore enabled', :request_store do
       before do
         cache = Hash.new { |hash, key| hash[key] = {} }
 
-        allow(RequestStore).to receive(:active?).and_return(true)
         allow(subject).to receive(:collection_cache).and_return(cache)
       end
 

spec/lib/feature_spec.rb

@@ -91,7 +91,11 @@ describe Feature do
   end
 
   describe '.flipper' do
-    shared_examples 'a memoized Flipper instance' do
+    before do
+      described_class.instance_variable_set(:@flipper, nil)
+    end
+
+    context 'when request store is inactive' do
       it 'memoizes the Flipper instance' do
         expect(Flipper).to receive(:new).once.and_call_original
 
@@ -101,16 +105,14 @@ describe Feature do
       end
     end
 
-    context 'when request store is inactive' do
-      before do
+    context 'when request store is active', :request_store do
+      it 'memoizes the Flipper instance' do
+        expect(Flipper).to receive(:new).once.and_call_original
+
+        described_class.flipper
         described_class.instance_variable_set(:@flipper, nil)
+        described_class.flipper
       end
-
-      it_behaves_like 'a memoized Flipper instance'
-    end
-
-    context 'when request store is inactive', :request_store do
-      it_behaves_like 'a memoized Flipper instance'
     end
   end
 

spec/lib/gitlab/git/hook_env_spec.rb

@@ -4,11 +4,7 @@ describe Gitlab::Git::HookEnv do
   let(:gl_repository) { 'project-123' }
 
   describe ".set" do
-    context 'with RequestStore.store disabled' do
-      before do
-        allow(RequestStore).to receive(:active?).and_return(false)
-      end
-
+    context 'with RequestStore disabled' do
       it 'does not store anything' do
         described_class.set(gl_repository, GIT_OBJECT_DIRECTORY_RELATIVE: 'foo')
 
@@ -16,11 +12,7 @@ describe Gitlab::Git::HookEnv do
       end
     end
 
-    context 'with RequestStore.store enabled' do
-      before do
-        allow(RequestStore).to receive(:active?).and_return(true)
-      end
-
+    context 'with RequestStore enabled', :request_store do
       it 'whitelist some `GIT_*` variables and stores them using RequestStore' do
         described_class.set(
           gl_repository,
@@ -41,9 +33,8 @@ describe Gitlab::Git::HookEnv do
   end
 
   describe ".all" do
-    context 'with RequestStore.store enabled' do
+    context 'with RequestStore enabled', :request_store do
       before do
-        allow(RequestStore).to receive(:active?).and_return(true)
         described_class.set(
           gl_repository,
           GIT_OBJECT_DIRECTORY_RELATIVE: 'foo',
@@ -60,7 +51,7 @@ describe Gitlab::Git::HookEnv do
   end
 
   describe ".to_env_hash" do
-    context 'with RequestStore.store enabled' do
+    context 'with RequestStore enabled', :request_store do
       using RSpec::Parameterized::TableSyntax
 
       let(:key) { 'GIT_OBJECT_DIRECTORY_RELATIVE' }
@@ -76,7 +67,6 @@ describe Gitlab::Git::HookEnv do
 
       with_them do
         before do
-          allow(RequestStore).to receive(:active?).and_return(true)
           described_class.set(gl_repository, key.to_sym => input)
         end
 
@@ -92,7 +82,7 @@ describe Gitlab::Git::HookEnv do
   end
 
   describe 'thread-safety' do
-    context 'with RequestStore.store enabled' do
+    context 'with RequestStore enabled', :request_store do
       before do
         allow(RequestStore).to receive(:active?).and_return(true)
         described_class.set(gl_repository, GIT_OBJECT_DIRECTORY_RELATIVE: 'foo')

spec/lib/gitlab/import_export/project.json

@@ -6144,7 +6144,7 @@
       "id": 36,
       "project_id": 5,
       "ref": "master",
-      "sha": "be93687618e4b132087f430a4d8fc3a609c9b77c",
+      "sha": "sha-notes",
       "before_sha": null,
       "push_data": null,
       "created_at": "2016-03-22T15:20:35.755Z",
@@ -6155,6 +6155,7 @@
       "status": "failed",
       "started_at": null,
       "finished_at": null,
+      "user_id": 9999,
       "duration": null,
       "notes": [
         {
@@ -6354,6 +6355,7 @@
     },
     {
       "id": 38,
+      "iid": 1,
       "project_id": 5,
       "ref": "master",
       "sha": "5f923865dde3436854e9ceb9cdb7815618d4e849",

spec/lib/gitlab/import_export/project_tree_restorer_spec.rb

@@ -59,7 +59,11 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
       end
 
       it 'creates a valid pipeline note' do
-        expect(Ci::Pipeline.first.notes).not_to be_empty
+        expect(Ci::Pipeline.find_by_sha('sha-notes').notes).not_to be_empty
+      end
+
+      it 'pipeline has the correct user ID' do
+        expect(Ci::Pipeline.find_by_sha('sha-notes').user_id).to eq(@user.id)
       end
 
       it 'restores pipelines with missing ref' do

spec/lib/gitlab/null_request_store_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::NullRequestStore do
+  let(:null_store) { described_class.new }
+
+  describe '#store' do
+    it 'returns an empty hash' do
+      expect(null_store.store).to eq({})
+    end
+  end
+
+  describe '#active?' do
+    it 'returns falsey' do
+      expect(null_store.active?).to be_falsey
+    end
+  end
+
+  describe '#read' do
+    it 'returns nil' do
+      expect(null_store.read('foo')).to be nil
+    end
+  end
+
+  describe '#[]' do
+    it 'returns nil' do
+      expect(null_store['foo']).to be nil
+    end
+  end
+
+  describe '#write' do
+    it 'returns the same value' do
+      expect(null_store.write('key', 'value')).to eq('value')
+    end
+  end
+
+  describe '#[]=' do
+    it 'returns the same value' do
+      expect(null_store['key'] = 'value').to eq('value')
+    end
+  end
+
+  describe '#exist?' do
+    it 'returns falsey' do
+      expect(null_store.exist?('foo')).to be_falsey
+    end
+  end
+
+  describe '#fetch' do
+    it 'returns the block result' do
+      expect(null_store.fetch('key') { 'block result' }).to eq('block result')
+    end
+  end
+
+  describe '#delete' do
+    context 'when a block is given' do
+      it 'yields the key to the block' do
+        expect do |b|
+          null_store.delete('foo', &b)
+        end.to yield_with_args('foo')
+      end
+
+      it 'returns the block result' do
+        expect(null_store.delete('foo') { |key| 'block result' }).to eq('block result')
+      end
+    end
+
+    context 'when a block is not given' do
+      it 'returns nil' do
+        expect(null_store.delete('foo')).to be nil
+      end
+    end
+  end
+end

spec/lib/gitlab/safe_request_store_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::SafeRequestStore do
+  describe '.store' do
+    context 'when RequestStore is active', :request_store do
+      it 'uses RequestStore' do
+        expect(described_class.store).to eq(RequestStore)
+      end
+    end
+
+    context 'when RequestStore is NOT active' do
+      it 'does not use RequestStore' do
+        expect(described_class.store).to be_a(Gitlab::NullRequestStore)
+      end
+    end
+  end
+
+  describe '.begin!' do
+    context 'when RequestStore is active', :request_store do
+      it 'uses RequestStore' do
+        expect(RequestStore).to receive(:begin!)
+
+        described_class.begin!
+      end
+    end
+
+    context 'when RequestStore is NOT active' do
+      it 'uses RequestStore' do
+        expect(RequestStore).to receive(:begin!)
+
+        described_class.begin!
+      end
+    end
+  end
+
+  describe '.clear!' do
+    context 'when RequestStore is active', :request_store do
+      it 'uses RequestStore' do
+        expect(RequestStore).to receive(:clear!).twice.and_call_original
+
+        described_class.clear!
+      end
+    end
+
+    context 'when RequestStore is NOT active' do
+      it 'uses RequestStore' do
+        expect(RequestStore).to receive(:clear!).and_call_original
+
+        described_class.clear!
+      end
+    end
+  end
+
+  describe '.end!' do
+    context 'when RequestStore is active', :request_store do
+      it 'uses RequestStore' do
+        expect(RequestStore).to receive(:end!).twice.and_call_original
+
+        described_class.end!
+      end
+    end
+
+    context 'when RequestStore is NOT active' do
+      it 'uses RequestStore' do
+        expect(RequestStore).to receive(:end!).and_call_original
+
+        described_class.end!
+      end
+    end
+  end
+
+  describe '.write' do
+    context 'when RequestStore is active', :request_store do
+      it 'uses RequestStore' do
+        expect do
+          described_class.write('foo', true)
+        end.to change { described_class.read('foo') }.from(nil).to(true)
+      end
+    end
+
+    context 'when RequestStore is NOT active' do
+      it 'does not use RequestStore' do
+        expect do
+          described_class.write('foo', true)
+        end.not_to change { described_class.read('foo') }.from(nil)
+      end
+    end
+  end
+
+  describe '.[]=' do
+    context 'when RequestStore is active', :request_store do
+      it 'uses RequestStore' do
+        expect do
+          described_class['foo'] = true
+        end.to change { described_class.read('foo') }.from(nil).to(true)
+      end
+    end
+
+    context 'when RequestStore is NOT active' do
+      it 'does not use RequestStore' do
+        expect do
+          described_class['foo'] = true
+        end.not_to change { described_class.read('foo') }.from(nil)
+      end
+    end
+  end
+
+  describe '.read' do
+    context 'when RequestStore is active', :request_store do
+      it 'uses RequestStore' do
+        expect do
+          RequestStore.write('foo', true)
+        end.to change { described_class.read('foo') }.from(nil).to(true)
+      end
+    end
+
+    context 'when RequestStore is NOT active' do
+      it 'does not use RequestStore' do
+        expect do
+          RequestStore.write('foo', true)
+        end.not_to change { described_class.read('foo') }.from(nil)
+
+        RequestStore.clear! # Clean up
+      end
+    end
+  end
+
+  describe '.[]' do
+    context 'when RequestStore is active', :request_store do
+      it 'uses RequestStore' do
+        expect do
+          RequestStore.write('foo', true)
+        end.to change { described_class['foo'] }.from(nil).to(true)
+      end
+    end
+
+    context 'when RequestStore is NOT active' do
+      it 'does not use RequestStore' do
+        expect do
+          RequestStore.write('foo', true)
+        end.not_to change { described_class['foo'] }.from(nil)
+
+        RequestStore.clear! # Clean up
+      end
+    end
+  end
+
+  describe '.exist?' do
+    context 'when RequestStore is active', :request_store do
+      it 'uses RequestStore' do
+        expect do
+          RequestStore.write('foo', 'not nil')
+        end.to change { described_class.exist?('foo') }.from(false).to(true)
+      end
+    end
+
+    context 'when RequestStore is NOT active' do
+      it 'does not use RequestStore' do
+        expect do
+          RequestStore.write('foo', 'not nil')
+        end.not_to change { described_class.exist?('foo') }.from(false)
+
+        RequestStore.clear! # Clean up
+      end
+    end
+  end
+
+  describe '.fetch' do
+    context 'when RequestStore is active', :request_store do
+      it 'uses RequestStore' do
+        expect do
+          described_class.fetch('foo') { 'block result' }
+        end.to change { described_class.read('foo') }.from(nil).to('block result')
+      end
+    end
+
+    context 'when RequestStore is NOT active' do
+      it 'does not use RequestStore' do
+        RequestStore.clear! # Ensure clean
+
+        expect do
+          described_class.fetch('foo') { 'block result' }
+        end.not_to change { described_class.read('foo') }.from(nil)
+
+        RequestStore.clear! # Clean up
+      end
+    end
+  end
+
+  describe '.delete' do
+    context 'when RequestStore is active', :request_store do
+      it 'uses RequestStore' do
+        described_class.write('foo', true)
+
+        expect do
+          described_class.delete('foo')
+        end.to change { described_class.read('foo') }.from(true).to(nil)
+      end
+
+      context 'when given a block and the key exists' do
+        it 'does not execute the block' do
+          described_class.write('foo', true)
+
+          expect do |b|
+            described_class.delete('foo', &b)
+          end.not_to yield_control
+        end
+      end
+
+      context 'when given a block and the key does not exist' do
+        it 'yields the key and returns the block result' do
+          result = described_class.delete('foo') { |key| "#{key} block result" }
+
+          expect(result).to eq('foo block result')
+        end
+      end
+    end
+
+    context 'when RequestStore is NOT active' do
+      before do
+        RequestStore.write('foo', true)
+      end
+
+      after do
+        RequestStore.clear! # Clean up
+      end
+
+      it 'does not use RequestStore' do
+        expect do
+          described_class.delete('foo')
+        end.not_to change { RequestStore.read('foo') }.from(true)
+      end
+
+      context 'when given a block' do
+        it 'yields the key and returns the block result' do
+          result = described_class.delete('foo') { |key| "#{key} block result" }
+
+          expect(result).to eq('foo block result')
+        end
+      end
+    end
+  end
+end

spec/models/commit_spec.rb

@@ -65,7 +65,7 @@ describe Commit do
 
       key = "Commit:author:#{commit.author_email.downcase}"
 
-      expect(RequestStore.store[key]).to eq(user)
+      expect(Gitlab::SafeRequestStore[key]).to eq(user)
       expect(commit.author).to eq(user)
     end
 

spec/views/projects/jobs/show.html.haml_spec.rb

@@ -188,40 +188,4 @@ describe 'projects/jobs/show' do
       expect(rendered).not_to have_link('New issue')
     end
   end
-
-  context 'when incomplete trigger_request is used' do
-    before do
-      build.trigger_request = FactoryBot.build(:ci_trigger_request, trigger: nil)
-    end
-
-    it 'test should not render token block' do
-      render
-
-      expect(rendered).not_to have_content('Token')
-    end
-  end
-
-  context 'when complete trigger_request is used' do
-    before do
-      build.trigger_request = FactoryBot.build(:ci_trigger_request)
-    end
-
-    it 'should render token' do
-      render
-
-      expect(rendered).to have_content('Token')
-      expect(rendered).to have_content(build.trigger_request.trigger.short_token)
-    end
-  end
-
-  describe 'commit title in sidebar' do
-    let(:commit_title) { project.commit.title }
-
-    it 'shows commit title and not show commit message' do
-      render
-
-      expect(rendered).to have_css('p.build-light-text.append-bottom-0',
-        text: /\A\n#{Regexp.escape(commit_title)}\n\Z/)
-    end
-  end
 end