Add latest changes from gitlab-org/gitlab@master

Gemfile

@@ -496,3 +496,6 @@ gem 'lockbox', '~> 0.3.3'
 
 # Email validation
 gem 'valid_email', '~> 0.1'
+
+# JSON
+gem 'json', '~> 2.3.0'

Gemfile.lock

@@ -555,7 +555,7 @@ GEM
       character_set (~> 1.1)
       regexp_parser (~> 1.1)
       regexp_property_values (~> 0.3)
-    json (1.8.6)
+    json (2.3.0)
     json-jwt (1.11.0)
       activesupport (>= 4.2)
       aes_key_wrap
@@ -1280,6 +1280,7 @@ DEPENDENCIES
   invisible_captcha (~> 0.12.1)
   jira-ruby (~> 2.0.0)
   js_regex (~> 3.1)
+  json (~> 2.3.0)
   json-schema (~> 2.8.0)
   jwt (~> 2.1.0)
   kaminari (~> 1.0)

app/assets/javascripts/diffs/store/actions.js

@@ -16,6 +16,7 @@ import {
   idleCallback,
   allDiscussionWrappersExpanded,
   prepareDiffData,
+  prepareLineForRenamedFile,
 } from './utils';
 import * as types from './mutation_types';
 import {
@@ -627,6 +628,42 @@ export const toggleFullDiff = ({ dispatch, getters, state }, filePath) => {
   }
 };
 
+export function switchToFullDiffFromRenamedFile({ commit, dispatch, state }, { diffFile }) {
+  return axios
+    .get(diffFile.context_lines_path, {
+      params: {
+        full: true,
+        from_merge_request: true,
+      },
+    })
+    .then(({ data }) => {
+      const lines = data.map((line, index) =>
+        prepareLineForRenamedFile({
+          diffViewType: state.diffViewType,
+          line,
+          diffFile,
+          index,
+        }),
+      );
+
+      commit(types.SET_DIFF_FILE_VIEWER, {
+        filePath: diffFile.file_path,
+        viewer: {
+          ...diffFile.alternate_viewer,
+          collapsed: false,
+        },
+      });
+      commit(types.SET_CURRENT_VIEW_DIFF_FILE_LINES, { filePath: diffFile.file_path, lines });
+
+      dispatch('startRenderDiffsQueue');
+    })
+    .catch(error => {
+      dispatch('receiveFullDiffError', diffFile.file_path);
+
+      throw error;
+    });
+}
+
 export const setFileCollapsed = ({ commit }, { filePath, collapsed }) =>
   commit(types.SET_FILE_COLLAPSED, { filePath, collapsed });
 

app/assets/javascripts/diffs/store/utils.js

@@ -303,6 +303,42 @@ function prepareLine(line) {
   }
 }
 
+export function prepareLineForRenamedFile({ line, diffViewType, diffFile, index = 0 }) {
+  /*
+    Renamed files are a little different than other diffs, which
+    is why this is distinct from `prepareDiffFileLines` below.
+
+    We don't get any of the diff file context when we get the diff
+    (so no "inline" vs. "parallel", no "line_code", etc.).
+
+    We can also assume that both the left and the right of each line
+    (for parallel diff view type) are identical, because the file
+    is renamed, not modified.
+
+    This should be cleaned up as part of the effort around flattening our data
+    ==> https://gitlab.com/groups/gitlab-org/-/epics/2852#note_304803402
+  */
+  const lineNumber = index + 1;
+  const cleanLine = {
+    ...line,
+    line_code: `${diffFile.file_hash}_${lineNumber}_${lineNumber}`,
+    new_line: lineNumber,
+    old_line: lineNumber,
+  };
+
+  prepareLine(cleanLine); // WARNING: In-Place Mutations!
+
+  if (diffViewType === PARALLEL_DIFF_VIEW_TYPE) {
+    return {
+      left: { ...cleanLine },
+      right: { ...cleanLine },
+      line_code: cleanLine.line_code,
+    };
+  }
+
+  return cleanLine;
+}
+
 function prepareDiffFileLines(file) {
   const inlineLines = file.highlighted_diff_lines;
   const parallelLines = file.parallel_diff_lines;

app/assets/javascripts/pipelines/components/pipelines.vue

@@ -10,7 +10,7 @@ import NavigationControls from './nav_controls.vue';
 import { getParameterByName } from '../../lib/utils/common_utils';
 import CIPaginationMixin from '../../vue_shared/mixins/ci_pagination_api_mixin';
 import PipelinesFilteredSearch from './pipelines_filtered_search.vue';
-import { ANY_TRIGGER_AUTHOR } from '../constants';
+import { ANY_TRIGGER_AUTHOR, RAW_TEXT_WARNING } from '../constants';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 export default {
@@ -258,9 +258,13 @@ export default {
       filters.forEach(filter => {
         // do not add Any for username query param, so we
         // can fetch all trigger authors
-        if (filter.value.data !== ANY_TRIGGER_AUTHOR) {
+        if (filter.type && filter.value.data !== ANY_TRIGGER_AUTHOR) {
           this.requestData[filter.type] = filter.value.data;
         }
+
+        if (!filter.type) {
+          createFlash(RAW_TEXT_WARNING, 'warning');
+        }
       });
 
       if (filters.length === 0) {

app/assets/javascripts/pipelines/constants.js

-import { __ } from '~/locale';
+import { s__, __ } from '~/locale';
 
 export const CANCEL_REQUEST = 'CANCEL_REQUEST';
 export const PIPELINES_TABLE = 'PIPELINES_TABLE';
@@ -14,3 +14,6 @@ export const TestStatus = {
 
 export const FETCH_AUTHOR_ERROR_MESSAGE = __('There was a problem fetching project users.');
 export const FETCH_BRANCH_ERROR_MESSAGE = __('There was a problem fetching project branches.');
+export const RAW_TEXT_WARNING = s__(
+  'Pipeline|Raw text search is not currently supported. Please use the available search tokens.',
+);

app/assets/javascripts/vue_shared/components/issue/related_issuable_item.vue

 <script>
 import '~/commons/bootstrap';
-import { GlTooltip, GlTooltipDirective } from '@gitlab/ui';
+import { GlIcon, GlTooltip, GlTooltipDirective } from '@gitlab/ui';
 import { sprintf } from '~/locale';
 import IssueMilestone from '../../components/issue/issue_milestone.vue';
 import IssueAssignees from '../../components/issue/issue_assignees.vue';
@@ -13,6 +13,7 @@ export default {
     IssueMilestone,
     IssueAssignees,
     CiIcon,
+    GlIcon,
     GlTooltip,
   },
   directives: {
@@ -44,6 +45,9 @@ export default {
         visibility: 'hidden',
       };
     },
+    iconClasses() {
+      return `${this.iconClass} ic-${this.iconName}`;
+    },
   },
 };
 </script>
@@ -60,13 +64,12 @@ export default {
       <!-- Title area: Status icon (XL) and title -->
       <div class="item-title d-flex align-items-xl-center mb-xl-0">
         <div ref="iconElementXL">
-          <icon
+          <gl-icon
             v-if="hasState"
             ref="iconElementXL"
-            class="mr-2"
-            :class="iconClass"
+            class="mr-2 d-block"
+            :class="iconClasses"
             :name="iconName"
-            :size="16"
             :title="stateTitle"
             :aria-label="state"
           />
@@ -74,11 +77,10 @@ export default {
         <gl-tooltip :target="() => $refs.iconElementXL">
           <span v-html="stateTitle"></span>
         </gl-tooltip>
-        <icon
+        <gl-icon
           v-if="confidential"
           v-gl-tooltip
           name="eye-slash"
-          :size="16"
           :title="__('Confidential')"
           class="confidential-icon append-right-4 align-self-baseline align-self-md-auto mt-xl-0"
           :aria-label="__('Confidential')"

app/models/merge_request.rb

@@ -866,7 +866,7 @@ class MergeRequest < ApplicationRecord
 
     check_service = MergeRequests::MergeabilityCheckService.new(self)
 
-    if async && Feature.enabled?(:async_merge_request_check_mergeability, project)
+    if async && Feature.enabled?(:async_merge_request_check_mergeability, project, default_enabled: true)
       check_service.async_execute
     else
       check_service.execute(retry_lease: false)

changelogs/unreleased/199912-ff-enable-by-default.yml

+---
+title: Enable async_merge_request_check_mergeability by default
+merge_request: 31196
+author:
+type: other

changelogs/unreleased/dont-support-raw-text-on-filter.yml

+---
+title: Show warning message to user if raw text search is used when filtering pipelines
+merge_request: 31942
+author:
+type: changed

changelogs/unreleased/json-gem-upgrade.yml

+---
+title: Upgrade json gem to 2.3.0
+merge_request: 30852
+author:
+type: performance

doc/administration/gitaly/index.md

@@ -10,6 +10,11 @@ On this page, *Gitaly server* refers to a standalone node that only runs Gitaly
 and *Gitaly client* is a GitLab Rails app node that runs all other processes
 except Gitaly.
 
+CAUTION: **Caution:**
+From GitLab 13.0, using NFS for Git repositories is deprecated. In GitLab 14.0,
+support for NFS for Git repositories is scheduled to be removed. Upgrade to
+[Gitaly Cluster](praefect.md) as soon as possible.
+
 ## Architecture
 
 Here's a high-level architecture overview of how Gitaly is used.

doc/administration/high_availability/nfs.md

@@ -7,6 +7,11 @@ type: reference
 You can view information and options set for each of the mounted NFS file
 systems by running `nfsstat -m` and `cat /etc/fstab`.
 
+CAUTION: **Caution:**
+From GitLab 13.0, using NFS for Git repositories is deprecated. In GitLab 14.0,
+support for NFS for Git repositories is scheduled to be removed. Upgrade to
+[Gitaly Cluster](../gitaly/praefect.md) as soon as possible.
+
 NOTE: **Note:** Filesystem performance has a big impact on overall GitLab
 performance, especially for actions that read or write to Git repositories. See
 [Filesystem Performance Benchmarking](../operations/filesystem_benchmarking.md)

doc/administration/reference_architectures/index.md

@@ -156,14 +156,14 @@ column.
 |-----------|-------------|----------------------------|
 | Load balancer(s) ([6](#footnotes)) | Handles load balancing, typically when you have multiple GitLab application services nodes | [Load balancer configuration](../high_availability/load_balancer.md) ([6](#footnotes))      | No |
 | Object storage service ([4](#footnotes)) | Recommended store for shared data objects | [Object Storage configuration](../object_storage.md) | No |
-| NFS ([5](#footnotes)) ([7](#footnotes)) | Shared disk storage service. Can be used as an alternative for Gitaly or Object Storage. Required for GitLab Pages | [NFS configuration](../high_availability/nfs.md) | No |
+| NFS ([5](#footnotes)) ([7](#footnotes)) | Shared disk storage service. Can be used as an alternative Object Storage. Required for GitLab Pages | [NFS configuration](../high_availability/nfs.md) | No |
 | [Consul](../../development/architecture.md#consul) ([3](#footnotes)) | Service discovery and health checks/failover | [Consul HA configuration](../high_availability/consul.md) **(PREMIUM ONLY)** | Yes |
 | [PostgreSQL](../../development/architecture.md#postgresql) | Database | [PostgreSQL configuration](https://docs.gitlab.com/omnibus/settings/database.html) | Yes |
 | [PgBouncer](../../development/architecture.md#pgbouncer) | Database connection pooler | [PgBouncer configuration](../high_availability/pgbouncer.md#running-pgbouncer-as-part-of-a-non-ha-gitlab-installation) **(PREMIUM ONLY)** | Yes |
 | Repmgr | PostgreSQL cluster management and failover | [PostgreSQL and Repmgr configuration](../high_availability/database.md) | Yes |
 | [Redis](../../development/architecture.md#redis) ([3](#footnotes))  | Key/value store for fast data lookup and caching | [Redis configuration](../high_availability/redis.md) | Yes |
 | Redis Sentinel | High availability for Redis | [Redis Sentinel configuration](../high_availability/redis.md) | Yes |
-| [Gitaly](../../development/architecture.md#gitaly) ([2](#footnotes)) ([5](#footnotes)) ([7](#footnotes))  | Provides access to Git repositories | [Gitaly configuration](../gitaly/index.md#running-gitaly-on-its-own-server) | Yes |
+| [Gitaly](../../development/architecture.md#gitaly) ([2](#footnotes)) ([7](#footnotes)) ([10](#footnotes)) | Provides access to Git repositories | [Gitaly configuration](../gitaly/index.md#running-gitaly-on-its-own-server) | Yes |
 | [Sidekiq](../../development/architecture.md#sidekiq) | Asynchronous/background jobs | [Sidekiq configuration](../high_availability/sidekiq.md) | Yes |
 | [GitLab application services](../../development/architecture.md#unicorn)([1](#footnotes)) | Unicorn/Puma, Workhorse, GitLab Shell - serves front-end requests (UI, API, Git over HTTP/SSH) | [GitLab app scaling configuration](../high_availability/gitlab.md) | Yes |
 | [Prometheus](../../development/architecture.md#prometheus) and [Grafana](../../development/architecture.md#grafana) | GitLab environment monitoring | [Monitoring node for scaling](../high_availability/monitoring_node.md) | Yes |
@@ -196,9 +196,9 @@ column.
 1. For data objects such as LFS, Uploads, Artifacts, etc. We recommend an [Object Storage service](../object_storage.md)
    over NFS where possible, due to better performance and availability.
 
-1. NFS can be used as an alternative for both repository data (replacing Gitaly) and
-   object storage but this isn't typically recommended for performance reasons. Note however it is required for
-   [GitLab Pages](https://gitlab.com/gitlab-org/gitlab-pages/issues/196).
+1. NFS can be used as an alternative for object storage but this isn't typically
+   recommended for performance reasons. Note however it is required for [GitLab
+   Pages](https://gitlab.com/gitlab-org/gitlab-pages/issues/196).
 
 1. Our architectures have been tested and validated with [HAProxy](https://www.haproxy.org/)
    as the load balancer. Although other load balancers with similar feature sets
@@ -219,3 +219,7 @@ column.
 
 1. AWS-equivalent and Azure-equivalent configurations are rough suggestions
    and may change in the future. They have not yet been tested and validated.
+
+1. From GitLab 13.0, using NFS for Git repositories is deprecated. In GitLab
+   14.0, support for NFS for Git repositories is scheduled to be removed.
+   Upgrade to [Gitaly Cluster](../gitaly/praefect.md) as soon as possible.

locale/gitlab.pot

@@ -15361,6 +15361,9 @@ msgstr ""
 msgid "Pipeline|Pipelines"
 msgstr ""
 
+msgid "Pipeline|Raw text search is not currently supported. Please use the available search tokens."
+msgstr ""
+
 msgid "Pipeline|Run Pipeline"
 msgstr ""
 

spec/frontend/diffs/store/actions_spec.js

@@ -40,6 +40,7 @@ import {
   receiveFullDiffError,
   fetchFullDiff,
   toggleFullDiff,
+  switchToFullDiffFromRenamedFile,
   setFileCollapsed,
   setExpandedDiffLines,
   setSuggestPopoverDismissed,
@@ -1252,6 +1253,87 @@ describe('DiffsStoreActions', () => {
     });
   });
 
+  describe('switchToFullDiffFromRenamedFile', () => {
+    const SUCCESS_URL = 'fakehost/context.success';
+    const ERROR_URL = 'fakehost/context.error';
+    const testFilePath = 'testpath';
+    const updatedViewerName = 'testviewer';
+    const preparedLine = { prepared: 'in-a-test' };
+    const testFile = {
+      file_path: testFilePath,
+      file_hash: 'testhash',
+      alternate_viewer: { name: updatedViewerName },
+    };
+    const updatedViewer = { name: updatedViewerName, collapsed: false };
+    const testData = [{ rich_text: 'test' }, { rich_text: 'file2' }];
+    let renamedFile;
+    let mock;
+
+    beforeEach(() => {
+      mock = new MockAdapter(axios);
+      jest.spyOn(utils, 'prepareLineForRenamedFile').mockImplementation(() => preparedLine);
+    });
+
+    afterEach(() => {
+      renamedFile = null;
+      mock.restore();
+    });
+
+    describe('success', () => {
+      beforeEach(() => {
+        renamedFile = { ...testFile, context_lines_path: SUCCESS_URL };
+        mock.onGet(SUCCESS_URL).replyOnce(200, testData);
+      });
+
+      it.each`
+        diffViewType
+        ${INLINE_DIFF_VIEW_TYPE}
+        ${PARALLEL_DIFF_VIEW_TYPE}
+      `(
+        'performs the correct mutations and starts a render queue for view type $diffViewType',
+        ({ diffViewType }) => {
+          return testAction(
+            switchToFullDiffFromRenamedFile,
+            { diffFile: renamedFile },
+            { diffViewType },
+            [
+              {
+                type: types.SET_DIFF_FILE_VIEWER,
+                payload: { filePath: testFilePath, viewer: updatedViewer },
+              },
+              {
+                type: types.SET_CURRENT_VIEW_DIFF_FILE_LINES,
+                payload: { filePath: testFilePath, lines: [preparedLine, preparedLine] },
+              },
+            ],
+            [{ type: 'startRenderDiffsQueue' }],
+          );
+        },
+      );
+    });
+
+    describe('error', () => {
+      beforeEach(() => {
+        renamedFile = { ...testFile, context_lines_path: ERROR_URL };
+        mock.onGet(ERROR_URL).reply(500);
+      });
+
+      it('dispatches the error handling action', () => {
+        const rejected = testAction(
+          switchToFullDiffFromRenamedFile,
+          { diffFile: renamedFile },
+          null,
+          [],
+          [{ type: 'receiveFullDiffError', payload: testFilePath }],
+        );
+
+        return rejected.catch(error =>
+          expect(error).toEqual(new Error('Request failed with status code 500')),
+        );
+      });
+    });
+  });
+
   describe('setFileCollapsed', () => {
     it('commits SET_FILE_COLLAPSED', done => {
       testAction(

spec/frontend/diffs/store/utils_spec.js

@@ -361,6 +361,72 @@ describe('DiffsStoreUtils', () => {
     });
   });
 
+  describe('prepareLineForRenamedFile', () => {
+    const diffFile = {
+      file_hash: 'file-hash',
+    };
+    const lineIndex = 4;
+    const sourceLine = {
+      foo: 'test',
+      rich_text: ' <p>rich</p>', // Note the leading space
+    };
+    const correctLine = {
+      foo: 'test',
+      line_code: 'file-hash_5_5',
+      old_line: 5,
+      new_line: 5,
+      rich_text: '<p>rich</p>', // Note no leading space
+      discussionsExpanded: true,
+      discussions: [],
+      hasForm: false,
+      text: undefined,
+      alreadyPrepared: true,
+    };
+    let preppedLine;
+
+    beforeEach(() => {
+      preppedLine = utils.prepareLineForRenamedFile({
+        diffViewType: INLINE_DIFF_VIEW_TYPE,
+        line: sourceLine,
+        index: lineIndex,
+        diffFile,
+      });
+    });
+
+    it('copies over the original line object to the new prepared line', () => {
+      expect(preppedLine).toEqual(
+        expect.objectContaining({
+          foo: correctLine.foo,
+          rich_text: correctLine.rich_text,
+        }),
+      );
+    });
+
+    it('correctly sets the old and new lines, plus a line code', () => {
+      expect(preppedLine.old_line).toEqual(correctLine.old_line);
+      expect(preppedLine.new_line).toEqual(correctLine.new_line);
+      expect(preppedLine.line_code).toEqual(correctLine.line_code);
+    });
+
+    it('returns a single object with the correct structure for `inline` lines', () => {
+      expect(preppedLine).toEqual(correctLine);
+    });
+
+    it('returns a nested object with "left" and "right" lines + the line code for `parallel` lines', () => {
+      preppedLine = utils.prepareLineForRenamedFile({
+        diffViewType: PARALLEL_DIFF_VIEW_TYPE,
+        line: sourceLine,
+        index: lineIndex,
+        diffFile,
+      });
+
+      expect(Object.keys(preppedLine)).toEqual(['left', 'right', 'line_code']);
+      expect(preppedLine.left).toEqual(correctLine);
+      expect(preppedLine.right).toEqual(correctLine);
+      expect(preppedLine.line_code).toEqual(correctLine.line_code);
+    });
+  });
+
   describe('prepareDiffData', () => {
     let mock;
     let preparedDiff;

spec/frontend/pipelines/pipelines_spec.js

@@ -6,7 +6,11 @@ import waitForPromises from 'helpers/wait_for_promises';
 import PipelinesComponent from '~/pipelines/components/pipelines.vue';
 import Store from '~/pipelines/stores/pipelines_store';
 import { pipelineWithStages, stageReply, users, mockSearch, branches } from './mock_data';
+import { RAW_TEXT_WARNING } from '~/pipelines/constants';
 import { GlFilteredSearch } from '@gitlab/ui';
+import createFlash from '~/flash';
+
+jest.mock('~/flash', () => jest.fn());
 
 describe('Pipelines', () => {
   const jsonFixtureName = 'pipelines/pipelines.json';
@@ -667,15 +671,18 @@ describe('Pipelines', () => {
   });
 
   describe('Pipeline filters', () => {
+    let updateContentMock;
+
     beforeEach(() => {
       mock.onGet(paths.endpoint).reply(200, pipelines);
       createComponent();
 
+      updateContentMock = jest.spyOn(wrapper.vm, 'updateContent');
+
       return waitForPromises();
     });
 
     it('updates request data and query params on filter submit', () => {
-      const updateContentMock = jest.spyOn(wrapper.vm, 'updateContent');
       const expectedQueryParams = { page: '1', scope: 'all', username: 'root', ref: 'master' };
 
       findFilteredSearch().vm.$emit('submit', mockSearch);
@@ -683,5 +690,21 @@ describe('Pipelines', () => {
       expect(wrapper.vm.requestData).toEqual(expectedQueryParams);
       expect(updateContentMock).toHaveBeenCalledWith(expectedQueryParams);
     });
+
+    it('does not add query params if raw text search is used', () => {
+      const expectedQueryParams = { page: '1', scope: 'all' };
+
+      findFilteredSearch().vm.$emit('submit', ['rawText']);
+
+      expect(wrapper.vm.requestData).toEqual(expectedQueryParams);
+      expect(updateContentMock).toHaveBeenCalledWith(expectedQueryParams);
+    });
+
+    it('displays a warning message if raw text search is used', () => {
+      findFilteredSearch().vm.$emit('submit', ['rawText']);
+
+      expect(createFlash).toHaveBeenCalledTimes(1);
+      expect(createFlash).toHaveBeenCalledWith(RAW_TEXT_WARNING, 'warning');
+    });
   });
 });

spec/lib/gitlab/json_spec.rb

@@ -17,18 +17,16 @@ RSpec.describe Gitlab::Json do
         expect(subject.parse('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
       end
 
-      # These tests will change expectations when the gem is upgraded
-
-      it "raises an error on a string" do
-        expect { subject.parse('"foo"') }.to raise_error(JSON::ParserError)
+      it "parses a string" do
+        expect(subject.parse('"foo"', legacy_mode: false)).to eq("foo")
       end
 
-      it "raises an error on a true bool" do
-        expect { subject.parse("true") }.to raise_error(JSON::ParserError)
+      it "parses a true bool" do
+        expect(subject.parse("true", legacy_mode: false)).to be(true)
       end
 
-      it "raises an error on a false bool" do
-        expect { subject.parse("false") }.to raise_error(JSON::ParserError)
+      it "parses a false bool" do
+        expect(subject.parse("false", legacy_mode: false)).to be(false)
       end
     end
 
@@ -53,6 +51,32 @@ RSpec.describe Gitlab::Json do
         expect { subject.parse("false", legacy_mode: true) }.to raise_error(JSON::ParserError)
       end
     end
+
+    context "feature flag is disabled" do
+      before do
+        stub_feature_flags(json_wrapper_legacy_mode: false)
+      end
+
+      it "parses an object" do
+        expect(subject.parse('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
+      end
+
+      it "parses an array" do
+        expect(subject.parse('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
+      end
+
+      it "parses a string" do
+        expect(subject.parse('"foo"', legacy_mode: true)).to eq("foo")
+      end
+
+      it "parses a true bool" do
+        expect(subject.parse("true", legacy_mode: true)).to be(true)
+      end
+
+      it "parses a false bool" do
+        expect(subject.parse("false", legacy_mode: true)).to be(false)
+      end
+    end
   end
 
   describe ".parse!" do
@@ -65,18 +89,16 @@ RSpec.describe Gitlab::Json do
         expect(subject.parse!('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
       end
 
-      # These tests will change expectations when the gem is upgraded
-
-      it "raises an error on a string" do
-        expect { subject.parse!('"foo"') }.to raise_error(JSON::ParserError)
+      it "parses a string" do
+        expect(subject.parse!('"foo"', legacy_mode: false)).to eq("foo")
       end
 
-      it "raises an error on a true bool" do
-        expect { subject.parse!("true") }.to raise_error(JSON::ParserError)
+      it "parses a true bool" do
+        expect(subject.parse!("true", legacy_mode: false)).to be(true)
       end
 
-      it "raises an error on a false bool" do
-        expect { subject.parse!("false") }.to raise_error(JSON::ParserError)
+      it "parses a false bool" do
+        expect(subject.parse!("false", legacy_mode: false)).to be(false)
       end
     end
 
@@ -101,6 +123,32 @@ RSpec.describe Gitlab::Json do
         expect { subject.parse!("false", legacy_mode: true) }.to raise_error(JSON::ParserError)
       end
     end
+
+    context "feature flag is disabled" do
+      before do
+        stub_feature_flags(json_wrapper_legacy_mode: false)
+      end
+
+      it "parses an object" do
+        expect(subject.parse!('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
+      end
+
+      it "parses an array" do
+        expect(subject.parse!('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
+      end
+
+      it "parses a string" do
+        expect(subject.parse!('"foo"', legacy_mode: true)).to eq("foo")
+      end
+
+      it "parses a true bool" do
+        expect(subject.parse!("true", legacy_mode: true)).to be(true)
+      end
+
+      it "parses a false bool" do
+        expect(subject.parse!("false", legacy_mode: true)).to be(false)
+      end
+    end
   end
 
   describe ".dump" do

spec/services/projects/transfer_service_spec.rb

@@ -9,18 +9,26 @@ describe Projects::TransferService do
   let(:group) { create(:group) }
   let(:project) { create(:project, :repository, :legacy_storage, namespace: user.namespace) }
 
+  subject(:execute_transfer) { described_class.new(project, user).execute(group) }
+
   context 'namespace -> namespace' do
     before do
-      allow_any_instance_of(Gitlab::UploadsTransfer)
-        .to receive(:move_project).and_return(true)
-      allow_any_instance_of(Gitlab::PagesTransfer)
-        .to receive(:move_project).and_return(true)
+      allow_next_instance_of(Gitlab::UploadsTransfer) do |service|
+        allow(service).to receive(:move_project).and_return(true)
+      end
+      allow_next_instance_of(Gitlab::PagesTransfer) do |service|
+        allow(service).to receive(:move_project).and_return(true)
+      end
+
       group.add_owner(user)
-      @result = transfer_project(project, user, group)
     end
 
-    it { expect(@result).to be_truthy }
-    it { expect(project.namespace).to eq(group) }
+    it 'updates the namespace' do
+      transfer_result = execute_transfer
+
+      expect(transfer_result).to be_truthy
+      expect(project.namespace).to eq(group)
+    end
   end
 
   context 'when transfer succeeds' do
@@ -31,26 +39,29 @@ describe Projects::TransferService do
     it 'sends notifications' do
       expect_any_instance_of(NotificationService).to receive(:project_was_moved)
 
-      transfer_project(project, user, group)
+      execute_transfer
     end
 
     it 'invalidates the user\'s personal_project_count cache' do
       expect(user).to receive(:invalidate_personal_projects_count)
 
-      transfer_project(project, user, group)
+      execute_transfer
     end
 
     it 'executes system hooks' do
-      transfer_project(project, user, group) do |service|
+      expect_next_instance_of(described_class) do |service|
         expect(service).to receive(:execute_system_hooks)
       end
+
+      execute_transfer
     end
 
     it 'moves the disk path', :aggregate_failures do
       old_path = project.repository.disk_path
       old_full_path = project.repository.full_path
 
-      transfer_project(project, user, group)
+      execute_transfer
+
       project.reload_repository!
 
       expect(project.repository.disk_path).not_to eq(old_path)
@@ -60,13 +71,13 @@ describe Projects::TransferService do
     end
 
     it 'updates project full path in .git/config' do
-      transfer_project(project, user, group)
+      execute_transfer
 
       expect(rugged_config['gitlab.fullpath']).to eq "#{group.full_path}/#{project.path}"
     end
 
     it 'updates storage location' do
-      transfer_project(project, user, group)
+      execute_transfer
 
       expect(project.project_repository).to have_attributes(
         disk_path: "#{group.full_path}/#{project.path}",
@@ -80,7 +91,7 @@ describe Projects::TransferService do
 
     def attempt_project_transfer(&block)
       expect do
-        transfer_project(project, user, group, &block)
+        execute_transfer
       end.to raise_error(ActiveRecord::ActiveRecordError)
     end
 
@@ -138,13 +149,15 @@ describe Projects::TransferService do
   end
 
   context 'namespace -> no namespace' do
-    before do
-      @result = transfer_project(project, user, nil)
-    end
+    let(:group) { nil }
 
-    it { expect(@result).to eq false }
-    it { expect(project.namespace).to eq(user.namespace) }
-    it { expect(project.errors.messages[:new_namespace].first).to eq 'Please select a new namespace for your project.' }
+    it 'does not allow the project transfer' do
+      transfer_result = execute_transfer
+
+      expect(transfer_result).to eq false
+      expect(project.namespace).to eq(user.namespace)
+      expect(project.errors.messages[:new_namespace].first).to eq 'Please select a new namespace for your project.'
+    end
   end
 
   context 'disallow transferring of project with tags' do
@@ -156,18 +169,18 @@ describe Projects::TransferService do
       project.container_repositories << container_repository
     end
 
-    subject { transfer_project(project, user, group) }
-
-    it { is_expected.to be_falsey }
+    it 'does not allow the project transfer' do
+      expect(execute_transfer).to eq false
+    end
   end
 
   context 'namespace -> not allowed namespace' do
-    before do
-      @result = transfer_project(project, user, group)
-    end
+    it 'does not allow the project transfer' do
+      transfer_result = execute_transfer
 
-    it { expect(@result).to eq false }
-    it { expect(project.namespace).to eq(user.namespace) }
+      expect(transfer_result).to eq false
+      expect(project.namespace).to eq(user.namespace)
+    end
   end
 
   context 'namespace which contains orphan repository with same projects path name' do
@@ -177,99 +190,94 @@ describe Projects::TransferService do
       group.add_owner(user)
 
       TestEnv.create_bare_repository(fake_repo_path)
-
-      @result = transfer_project(project, user, group)
     end
 
     after do
       FileUtils.rm_rf(fake_repo_path)
     end
 
-    it { expect(@result).to eq false }
-    it { expect(project.namespace).to eq(user.namespace) }
-    it { expect(project.errors[:new_namespace]).to include('Cannot move project') }
+    it 'does not allow the project transfer' do
+      transfer_result = execute_transfer
+
+      expect(transfer_result).to eq false
+      expect(project.namespace).to eq(user.namespace)
+      expect(project.errors[:new_namespace]).to include('Cannot move project')
+    end
   end
 
   context 'target namespace containing the same project name' do
     before do
       group.add_owner(user)
-      project.update(name: 'new_name')
+      create(:project, name: project.name, group: group, path: 'other')
+    end
 
-      create(:project, name: 'new_name', group: group, path: 'other')
+    it 'does not allow the project transfer' do
+      transfer_result = execute_transfer
 
-      @result = transfer_project(project, user, group)
+      expect(transfer_result).to eq false
+      expect(project.namespace).to eq(user.namespace)
+      expect(project.errors[:new_namespace]).to include('Project with same name or path in target namespace already exists')
     end
-
-    it { expect(@result).to eq false }
-    it { expect(project.namespace).to eq(user.namespace) }
-    it { expect(project.errors[:new_namespace]).to include('Project with same name or path in target namespace already exists') }
   end
 
   context 'target namespace containing the same project path' do
     before do
       group.add_owner(user)
-
       create(:project, name: 'other-name', path: project.path, group: group)
-
-      @result = transfer_project(project, user, group)
     end
 
-    it { expect(@result).to eq false }
-    it { expect(project.namespace).to eq(user.namespace) }
-    it { expect(project.errors[:new_namespace]).to include('Project with same name or path in target namespace already exists') }
+    it 'does not allow the project transfer' do
+      transfer_result = execute_transfer
+
+      expect(transfer_result).to eq false
+      expect(project.namespace).to eq(user.namespace)
+      expect(project.errors[:new_namespace]).to include('Project with same name or path in target namespace already exists')
+    end
   end
 
   context 'target namespace allows developers to create projects' do
     let(:group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) }
 
     context 'the user is a member of the target namespace with developer permissions' do
-      subject(:transfer_project_result) { transfer_project(project, user, group) }
-
       before do
         group.add_developer(user)
       end
 
       it 'does not allow project transfer to the target namespace' do
-        expect(transfer_project_result).to eq false
+        transfer_result = execute_transfer
+
+        expect(transfer_result).to eq false
         expect(project.namespace).to eq(user.namespace)
         expect(project.errors[:new_namespace]).to include('Transfer failed, please contact an admin.')
       end
     end
   end
 
-  def transfer_project(project, user, new_namespace)
-    service = Projects::TransferService.new(project, user)
-
-    yield(service) if block_given?
-
-    service.execute(new_namespace)
-  end
-
   context 'visibility level' do
-    let(:internal_group) { create(:group, :internal) }
+    let(:group) { create(:group, :internal) }
 
     before do
-      internal_group.add_owner(user)
+      group.add_owner(user)
     end
 
     context 'when namespace visibility level < project visibility level' do
-      let(:public_project) { create(:project, :public, :repository, namespace: user.namespace) }
+      let(:project) { create(:project, :public, :repository, namespace: user.namespace) }
 
       before do
-        transfer_project(public_project, user, internal_group)
+        execute_transfer
       end
 
-      it { expect(public_project.visibility_level).to eq(internal_group.visibility_level) }
+      it { expect(project.visibility_level).to eq(group.visibility_level) }
     end
 
     context 'when namespace visibility level > project visibility level' do
-      let(:private_project) { create(:project, :private, :repository, namespace: user.namespace) }
+      let(:project) { create(:project, :private, :repository, namespace: user.namespace) }
 
       before do
-        transfer_project(private_project, user, internal_group)
+        execute_transfer
       end
 
-      it { expect(private_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) }
+      it { expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) }
     end
   end
 
@@ -277,9 +285,11 @@ describe Projects::TransferService do
     it 'delegates transfer to Labels::TransferService' do
       group.add_owner(user)
 
-      expect_any_instance_of(Labels::TransferService).to receive(:execute).once.and_call_original
+      expect_next_instance_of(Labels::TransferService, user, project.group, project) do |labels_transfer_service|
+        expect(labels_transfer_service).to receive(:execute).once.and_call_original
+      end
 
-      transfer_project(project, user, group)
+      execute_transfer
     end
   end
 
@@ -287,49 +297,52 @@ describe Projects::TransferService do
     it 'delegates transfer to Milestones::TransferService' do
       group.add_owner(user)
 
-      expect(Milestones::TransferService).to receive(:new).with(user, project.group, project).and_call_original
-      expect_any_instance_of(Milestones::TransferService).to receive(:execute).once
+      expect_next_instance_of(Milestones::TransferService, user, project.group, project) do |milestones_transfer_service|
+        expect(milestones_transfer_service).to receive(:execute).once.and_call_original
+      end
 
-      transfer_project(project, user, group)
+      execute_transfer
     end
   end
 
   context 'when hashed storage in use' do
-    let!(:hashed_project) { create(:project, :repository, namespace: user.namespace) }
-    let!(:old_disk_path) { hashed_project.repository.disk_path }
+    let!(:project) { create(:project, :repository, namespace: user.namespace) }
+    let!(:old_disk_path) { project.repository.disk_path }
 
     before do
       group.add_owner(user)
     end
 
     it 'does not move the disk path', :aggregate_failures do
-      new_full_path = "#{group.full_path}/#{hashed_project.path}"
+      new_full_path = "#{group.full_path}/#{project.path}"
 
-      transfer_project(hashed_project, user, group)
-      hashed_project.reload_repository!
+      execute_transfer
 
-      expect(hashed_project.repository).to have_attributes(
+      project.reload_repository!
+
+      expect(project.repository).to have_attributes(
         disk_path: old_disk_path,
         full_path: new_full_path
       )
-      expect(hashed_project.disk_path).to eq(old_disk_path)
+      expect(project.disk_path).to eq(old_disk_path)
     end
 
     it 'does not move the disk path when the transfer fails', :aggregate_failures do
-      old_full_path = hashed_project.full_path
+      old_full_path = project.full_path
 
       expect_next_instance_of(described_class) do |service|
         allow(service).to receive(:execute_system_hooks).and_raise('foo')
       end
-      expect { transfer_project(hashed_project, user, group) }.to raise_error('foo')
 
-      hashed_project.reload_repository!
+      expect { execute_transfer }.to raise_error('foo')
 
-      expect(hashed_project.repository).to have_attributes(
+      project.reload_repository!
+
+      expect(project.repository).to have_attributes(
         disk_path: old_disk_path,
         full_path: old_full_path
       )
-      expect(hashed_project.disk_path).to eq(old_disk_path)
+      expect(project.disk_path).to eq(old_disk_path)
     end
   end
 
@@ -344,18 +357,18 @@ describe Projects::TransferService do
     end
 
     it 'refreshes the permissions of the old and new namespace' do
-      transfer_project(project, owner, group)
+      execute_transfer
 
       expect(group_member.authorized_projects).to include(project)
       expect(owner.authorized_projects).to include(project)
     end
 
     it 'only schedules a single job for every user' do
-      expect(UserProjectAccessChangedService).to receive(:new)
-        .with([owner.id, group_member.id])
-        .and_call_original
+      expect_next_instance_of(UserProjectAccessChangedService, [owner.id, group_member.id]) do |service|
+        expect(service).to receive(:execute).once.and_call_original
+      end
 
-      transfer_project(project, owner, group)
+      execute_transfer
     end
   end