Merge branch 'ld-graphql-allow-models-to-be-renamed-and-support-old-global_ids' into 'master'

Allow models used as a GlobalID to be renamed [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!62870

app/graphql/gitlab_schema.rb

@@ -51,9 +51,8 @@ class GitlabSchema < GraphQL::Schema
     end
 
     def get_type(type_name)
-      # This is a backwards compatibility hack to work around an accidentally
-      # released argument typed as EEIterationID
-      type_name = type_name.gsub(/^EE/, '') if type_name.end_with?('ID')
+      type_name = Gitlab::GlobalId::Deprecations.apply_to_graphql_name(type_name)
+
       super(type_name)
     end
 
@@ -162,10 +161,9 @@ class GitlabSchema < GraphQL::Schema
     end
   end
 
-  # This is a backwards compatibility hack to work around an accidentally
-  # released argument typed as EE{Type}ID
   def get_type(type_name)
-    type_name = type_name.gsub(/^EE/, '') if type_name.end_with?('ID')
+    type_name = Gitlab::GlobalId::Deprecations.apply_to_graphql_name(type_name)
+
     super(type_name)
   end
 end

app/graphql/types/global_id_type.rb

@@ -52,11 +52,20 @@ module Types
       @id_types ||= {}
 
       @id_types[model_class] ||= Class.new(self) do
-        graphql_name "#{model_class.name.gsub(/::/, '')}ID"
-        description <<~MD
+        model_name = model_class.name
+
+        graphql_name model_name_to_graphql_name(model_name)
+        description <<~MD.strip
           A `#{graphql_name}` is a global ID. It is encoded as a string.
 
-          An example `#{graphql_name}` is: `"#{::Gitlab::GlobalId.build(model_name: model_class.name, id: 1)}"`.
+          An example `#{graphql_name}` is: `"#{::Gitlab::GlobalId.build(model_name: model_name, id: 1)}"`.
+          #{
+            if deprecation = Gitlab::GlobalId::Deprecations.deprecation_by(model_name)
+              'The older format `"' +
+              ::Gitlab::GlobalId.build(model_name: deprecation.old_model_name, id: 1).to_s +
+              '"` was deprecated in ' +  deprecation.milestone + '.'
+            end}
+
         MD
 
         define_singleton_method(:to_s) do
@@ -69,7 +78,7 @@ module Types
 
         define_singleton_method(:as) do |new_name|
           if @renamed && graphql_name != new_name
-            raise "Conflicting names for ID of #{model_class.name}: " \
+            raise "Conflicting names for ID of #{model_name}: " \
                   "#{graphql_name} and #{new_name}"
           end
 
@@ -79,11 +88,11 @@ module Types
         end
 
         define_singleton_method(:coerce_result) do |gid, ctx|
-          global_id = ::Gitlab::GlobalId.as_global_id(gid, model_name: model_class.name)
+          global_id = ::Gitlab::GlobalId.as_global_id(gid, model_name: model_name)
 
           next global_id.to_s if suitable?(global_id)
 
-          raise GraphQL::CoercionError, "Expected a #{model_class.name} ID, got #{global_id}"
+          raise GraphQL::CoercionError, "Expected a #{model_name} ID, got #{global_id}"
         end
 
         define_singleton_method(:suitable?) do |gid|
@@ -97,9 +106,13 @@ module Types
           gid = super(string, ctx)
           next gid if suitable?(gid)
 
-          raise GraphQL::CoercionError, "#{string.inspect} does not represent an instance of #{model_class.name}"
+          raise GraphQL::CoercionError, "#{string.inspect} does not represent an instance of #{model_name}"
         end
       end
     end
+
+    def self.model_name_to_graphql_name(model_name)
+      "#{model_name.gsub(/::/, '')}ID"
+    end
   end
 end

config/initializers/global_id.rb

+# frozen_string_literal: true
+
+GlobalID.prepend(Gitlab::Patch::GlobalID)

doc/api/graphql/reference/index.md

@@ -15378,6 +15378,7 @@ An example `IssueID` is: `"gid://gitlab/Issue/1"`.
 A `IterationID` is a global ID. It is encoded as a string.
 
 An example `IterationID` is: `"gid://gitlab/Iteration/1"`.
+The older format `"gid://gitlab/EEIteration/1"` was deprecated in 13.3.
 
 ### `IterationsCadenceID`
 

lib/gitlab/global_id/deprecations.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module GlobalId
+    module Deprecations
+      Deprecation = Struct.new(:old_model_name, :new_model_name, :milestone, keyword_init: true)
+
+      # Contains the deprecations in place.
+      # Example:
+      #
+      #   DEPRECATIONS = [
+      #     Deprecation.new(old_model_name: 'PrometheusService', new_model_name: 'Integrations::Prometheus', milestone: '14.0')
+      #   ].freeze
+      DEPRECATIONS = [
+        # This works around an accidentally released argument named as `"EEIterationID"` in 7000489db.
+        Deprecation.new(old_model_name: 'EEIteration', new_model_name: 'Iteration', milestone: '13.3')
+      ].freeze
+
+      # Maps of the DEPRECATIONS Hash for quick access.
+      OLD_NAME_MAP = DEPRECATIONS.index_by(&:old_model_name).freeze
+      NEW_NAME_MAP = DEPRECATIONS.index_by(&:new_model_name).freeze
+      OLD_GRAPHQL_NAME_MAP = DEPRECATIONS.index_by do |d|
+        Types::GlobalIDType.model_name_to_graphql_name(d.old_model_name)
+      end.freeze
+
+      def self.deprecated?(old_model_name)
+        OLD_NAME_MAP.key?(old_model_name)
+      end
+
+      def self.deprecation_for(old_model_name)
+        OLD_NAME_MAP[old_model_name]
+      end
+
+      def self.deprecation_by(new_model_name)
+        NEW_NAME_MAP[new_model_name]
+      end
+
+      # Returns the new `graphql_name` (Type#graphql_name) of a deprecated GID,
+      # or the `graphql_name` argument given if no deprecation applies.
+      def self.apply_to_graphql_name(graphql_name)
+        return graphql_name unless deprecation = OLD_GRAPHQL_NAME_MAP[graphql_name]
+
+        Types::GlobalIDType.model_name_to_graphql_name(deprecation.new_model_name)
+      end
+    end
+  end
+end

lib/gitlab/patch/global_id.rb

+# frozen_string_literal: true
+
+# To support GlobalID arguments that present a model with its old "deprecated" name
+# we alter GlobalID so it will correctly find the record with its new model name.
+module Gitlab
+  module Patch
+    module GlobalID
+      def initialize(gid, options = {})
+        super
+
+        if deprecation = Gitlab::GlobalId::Deprecations.deprecation_for(model_name)
+          @new_model_name = deprecation.new_model_name
+        end
+      end
+
+      def model_name
+        new_model_name || super
+      end
+
+      private
+
+      attr_reader :new_model_name
+    end
+  end
+end

spec/graphql/types/global_id_type_spec.rb

@@ -3,6 +3,10 @@
 require 'spec_helper'
 
 RSpec.describe Types::GlobalIDType do
+  include ::Gitlab::Graphql::Laziness
+  include GraphqlHelpers
+  include GlobalIDDeprecationHelpers
+
   let_it_be(:project) { create(:project) }
 
   let(:gid) { project.to_global_id }
@@ -97,6 +101,142 @@ RSpec.describe Types::GlobalIDType do
       expect { type.coerce_isolated_input(invalid_gid) }
         .to raise_error(GraphQL::CoercionError, /does not represent an instance of Project/)
     end
+
+    context 'with a deprecation' do
+      around(:all) do |example|
+        # Unset all previously memoized GlobalIDTypes to allow us to define one
+        # that will use the constants stubbed in the `before` block.
+        previous_id_types = Types::GlobalIDType.instance_variable_get(:@id_types)
+        Types::GlobalIDType.instance_variable_set(:@id_types, {})
+
+        example.run
+      ensure
+        Types::GlobalIDType.instance_variable_set(:@id_types, previous_id_types)
+      end
+
+      before do
+        deprecation = Gitlab::GlobalId::Deprecations::Deprecation.new(old_model_name: 'OldIssue', new_model_name: 'Issue', milestone: '10.0')
+
+        stub_global_id_deprecations(deprecation)
+      end
+
+      let_it_be(:issue) { create(:issue) }
+
+      let!(:type) { ::Types::GlobalIDType[::Issue] }
+      let(:deprecated_gid) { Gitlab::GlobalId.build(model_name: 'OldIssue', id: issue.id) }
+      let(:deprecating_gid) { Gitlab::GlobalId.build(model_name: 'Issue', id: issue.id) }
+
+      it 'appends the description with a deprecation notice for the old Global ID' do
+        expect(type.to_graphql.description).to include('The older format `"gid://gitlab/OldIssue/1"` was deprecated in 10.0')
+      end
+
+      describe 'coercing input against the type (parsing the Global ID string when supplied as an argument)' do
+        subject(:result) { type.coerce_isolated_input(gid.to_s) }
+
+        context 'when passed the deprecated Global ID' do
+          let(:gid) { deprecated_gid }
+
+          it 'changes the model_name to the new model name' do
+            expect(result.model_name).to eq('Issue')
+          end
+
+          it 'changes the model_class to the new model class' do
+            expect(result.model_class).to eq(Issue)
+          end
+
+          it 'can find the correct resource' do
+            expect(result.find).to eq(issue)
+          end
+
+          it 'can find the correct resource loaded through GitlabSchema' do
+            expect(force(GitlabSchema.object_from_id(result, expected_class: Issue))).to eq(issue)
+          end
+        end
+
+        context 'when passed the Global ID that is deprecating another' do
+          let(:gid) { deprecating_gid }
+
+          it 'works as normal' do
+            expect(result).to have_attributes(
+              model_class: Issue,
+              model_name: 'Issue',
+              find: issue,
+              to_s: gid.to_s
+            )
+          end
+        end
+      end
+
+      describe 'coercing the result against the type (producing the Global ID string when used in a field)' do
+        context 'when passed the deprecated Global ID' do
+          let(:gid) { deprecated_gid }
+
+          it 'works, but does not result in matching the new Global ID', :aggregate_failures do
+            # Note, this would normally never happen in real life as the object being parsed
+            # by the field would not produce the GlobalID of the deprecated model. This test
+            # proves that it is technically possible for the deprecated GlobalID to be
+            # considered parsable for the type, as opposed to raising a `GraphQL::CoercionError`.
+            expect(type.coerce_isolated_result(gid)).not_to eq(issue.to_global_id.to_s)
+            expect(type.coerce_isolated_result(gid)).to eq(gid.to_s)
+          end
+        end
+
+        context 'when passed the Global ID that is deprecating another' do
+          let(:gid) { deprecating_gid }
+
+          it 'works as normal' do
+            expect(type.coerce_isolated_result(gid)).to eq(issue.to_global_id.to_s)
+          end
+        end
+      end
+
+      describe 'executing against the schema' do
+        let(:query_result) do
+          context = { current_user: issue.project.owner }
+          variables = { 'id' => gid }
+
+          run_with_clean_state(query, context: context, variables: variables).to_h
+        end
+
+        shared_examples 'a query that works with old and new GIDs' do
+          let(:query) do
+            <<-GQL
+            query($id: #{argument_name}!) {
+              issue(id: $id) {
+                id
+              }
+            }
+            GQL
+          end
+
+          subject { query_result.dig('data', 'issue', 'id') }
+
+          context 'when the argument value is the new GID' do
+            let(:gid) { Gitlab::GlobalId.build(model_name: 'Issue', id: issue.id) }
+
+            it { is_expected.to be_present }
+          end
+
+          context 'when the argument value is the old GID' do
+            let(:gid) { Gitlab::GlobalId.build(model_name: 'OldIssue', id: issue.id) }
+
+            it { is_expected.to be_present }
+          end
+        end
+
+        context 'when the query signature includes the old type name' do
+          let(:argument_name) { 'OldIssueID' }
+
+          it_behaves_like 'a query that works with old and new GIDs'
+        end
+
+        context 'when the query signature includes the new type name' do
+          let(:argument_name) { 'IssueID' }
+
+          it_behaves_like 'a query that works with old and new GIDs'
+        end
+      end
+    end
   end
 
   describe 'a parameterized type with a namespace' do
@@ -231,4 +371,10 @@ RSpec.describe Types::GlobalIDType do
       end
     end
   end
+
+  describe '.model_name_to_graphql_name' do
+    it 'returns a graphql name for the given model name' do
+      expect(described_class.model_name_to_graphql_name('DesignManagement::Design')).to eq('DesignManagementDesignID')
+    end
+  end
 end

spec/initializers/global_id_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'global_id' do
+  it 'prepends `Gitlab::Patch::GlobalID`' do
+    expect(GlobalID.ancestors).to include(Gitlab::Patch::GlobalID)
+  end
+
+  it 'patches GlobalID to find aliased models when a deprecation exists' do
+    allow(Gitlab::GlobalId::Deprecations).to receive(:deprecation_for).and_call_original
+    allow(Gitlab::GlobalId::Deprecations).to receive(:deprecation_for).with('Issue').and_return(double(new_model_name: 'Project'))
+    project = create(:project)
+    gid_string = Gitlab::GlobalId.build(model_name: Issue.name, id: project.id).to_s
+
+    expect(GlobalID.new(gid_string)).to have_attributes(
+      to_s: gid_string,
+      model_name: 'Project',
+      model_class: Project,
+      find: project
+    )
+  end
+
+  it 'works as normal when no deprecation exists' do
+    issue = create(:issue)
+    gid_string = Gitlab::GlobalId.build(model_name: Issue.name, id: issue.id).to_s
+
+    expect(GlobalID.new(gid_string)).to have_attributes(
+      to_s: gid_string,
+      model_name: 'Issue',
+      model_class: Issue,
+      find: issue
+    )
+  end
+end

spec/lib/gitlab/global_id/deprecations_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::GlobalId::Deprecations do
+  include GlobalIDDeprecationHelpers
+
+  let_it_be(:deprecation_1) { described_class::Deprecation.new(old_model_name: 'Foo::Model', new_model_name: 'Bar', milestone: '9.0') }
+  let_it_be(:deprecation_2) { described_class::Deprecation.new(old_model_name: 'Baz', new_model_name: 'Qux::Model', milestone: '10.0') }
+
+  before do
+    stub_global_id_deprecations(deprecation_1, deprecation_2)
+  end
+
+  describe '.deprecated?' do
+    it 'returns a boolean to signal if model name has a deprecation', :aggregate_failures do
+      expect(described_class.deprecated?('Foo::Model')).to eq(true)
+      expect(described_class.deprecated?('Qux::Model')).to eq(false)
+    end
+  end
+
+  describe '.deprecation_for' do
+    it 'returns the deprecation for the model if it exists', :aggregate_failures do
+      expect(described_class.deprecation_for('Foo::Model')).to eq(deprecation_1)
+      expect(described_class.deprecation_for('Qux::Model')).to be_nil
+    end
+  end
+
+  describe '.deprecation_by' do
+    it 'returns the deprecation by the model if it exists', :aggregate_failures do
+      expect(described_class.deprecation_by('Foo::Model')).to be_nil
+      expect(described_class.deprecation_by('Qux::Model')).to eq(deprecation_2)
+    end
+  end
+
+  describe '.apply_to_graphql_name' do
+    it 'returns the corresponding graphql_name of the GID for the new model', :aggregate_failures do
+      expect(described_class.apply_to_graphql_name('FooModelID')).to eq('BarID')
+      expect(described_class.apply_to_graphql_name('BazID')).to eq('QuxModelID')
+    end
+
+    it 'returns the same value if there is no deprecation' do
+      expect(described_class.apply_to_graphql_name('ProjectID')).to eq('ProjectID')
+    end
+  end
+end

spec/support/helpers/global_id_deprecation_helpers.rb

+# frozen_string_literal: true
+
+module GlobalIDDeprecationHelpers
+  def stub_global_id_deprecations(*deprecations)
+    old_name_map = deprecations.index_by(&:old_model_name)
+    new_name_map = deprecations.index_by(&:new_model_name)
+    old_graphql_name_map = deprecations.index_by { |d| Types::GlobalIDType.model_name_to_graphql_name(d.old_model_name) }
+
+    stub_const('Gitlab::GlobalId::Deprecations::OLD_NAME_MAP', old_name_map)
+    stub_const('Gitlab::GlobalId::Deprecations::NEW_NAME_MAP', new_name_map)
+    stub_const('Gitlab::GlobalId::Deprecations::OLD_GRAPHQL_NAME_MAP', old_graphql_name_map)
+  end
+end