diff --git a/.gitlab/issue_templates/Deprecations.md b/.gitlab/issue_templates/Deprecations.md index ef6764c3621..49f1129fe89 100644 --- a/.gitlab/issue_templates/Deprecations.md +++ b/.gitlab/issue_templates/Deprecations.md @@ -1,4 +1,4 @@ -For guidance on the overall deprecations, removals and breaking changes workflow, please visit [Breaking changes, deprecations, and removing features](https://about.gitlab.com/handbook/product/gitlab-the-product/#breaking-changes-deprecations-and-removing-features) +For guidance on the overall deprecations, removals and breaking changes workflow, please visit [Breaking changes, deprecations, and removing features](https://about.gitlab.com/handbook/product/gitlab-the-product/#deprecations-removals-and-breaking-changes) @@ -11,7 +11,7 @@ It is recommended that you link to the documentation. The description of the deprecation should state what actions the user should take to rectify the behavior. If the deprecation is scheduled for an upcoming release, the content should remain in the deprecations documentation page until it has been completed. For example, if a deprecation is announced in 14.9 and scheduled to be completed in 15.0, the same content would be included in the documentation for 14.9, 14.10, and 15.0. -**If this issue proposes a breaking change outside a major release XX.0, you need to get approval from your manager and request collaboration from Product Operations on communication. Be sure to follow the guidance [here](https://about.gitlab.com/handbook/product/gitlab-the-product/#breaking-changes-deprecations-and-removing-features.)** +**If this issue proposes a breaking change outside a major release XX.0, you need to get approval from your manager and request collaboration from Product Operations on communication. Be sure to follow the guidance [here](https://about.gitlab.com/handbook/product/gitlab-the-product/#deprecations-removals-and-breaking-changes.)** --> diff --git a/.gitlab/merge_request_templates/Removals.md b/.gitlab/merge_request_templates/Removals.md index afe95f853bc..6f31f3cefd2 100644 --- a/.gitlab/merge_request_templates/Removals.md +++ b/.gitlab/merge_request_templates/Removals.md @@ -18,7 +18,7 @@ If there is no relevant deprecation issue, hit pause and: Removals must be [announced as deprecations](https://about.gitlab.com/handbook/marketing/blog/release-posts/#deprecations) at least 2 milestones in advance of the planned removal date. -If the removal creates a [breaking change](https://about.gitlab.com/handbook/product/gitlab-the-product/#breaking-changes-deprecations-and-removing-features), it can only be removed in a major "XX.0" release. +If the removal creates a [breaking change](https://about.gitlab.com/handbook/product/gitlab-the-product/#deprecations-removals-and-breaking-changes), it can only be removed in a major "XX.0" release. **By the 10th**: Assign this MR to these team members as reviewers, and for approval: diff --git a/config/initializers/sawyer_patch.rb b/config/initializers/sawyer_patch.rb index 34d2843d165..2a946cf0f7d 100644 --- a/config/initializers/sawyer_patch.rb +++ b/config/initializers/sawyer_patch.rb @@ -6,47 +6,21 @@ module SawyerClassPatch def attr_accessor(*attrs) attrs.each do |attribute| class_eval do - # rubocop:disable Gitlab/ModuleWithInstanceVariables - if method_defined?(attribute) || method_defined?("#{attribute}=") || method_defined?("#{attribute}?") - define_method attribute do - raise Sawyer::Error, - "Sawyer method \"#{attribute}\" overlaps Ruby method. Convert to a hash to access the attribute." - end + define_method attribute do + raise Sawyer::Error, + "Sawyer method \"#{attribute}\" access is forbidden. Convert to a hash to access the attribute." + end - define_method "#{attribute}=" do |value| - raise Sawyer::Error, - "Sawyer method \"#{attribute}\" overlaps Ruby method. Convert to a hash to access the attribute." - end + define_method "#{attribute}=" do |value| + raise Sawyer::Error, + "Sawyer method \"#{attribute}=\" access is forbidden. Convert to a hash to access the attribute." + end - define_method "#{attribute}?" do - raise Sawyer::Error, - "Sawyer method \"#{attribute}\" overlaps Ruby method. Convert to a hash to access the attribute." - end - else - define_method attribute do - Gitlab::Import::Logger.warn( - Gitlab::ApplicationContext.current.merge( - { - message: 'Sawyer attribute called', - attribute: attribute, - caller: Gitlab::BacktraceCleaner.clean_backtrace(caller) - } - ) - ) - - @attrs[attribute.to_sym] - end - - define_method "#{attribute}=" do |value| - @attrs[attribute.to_sym] = value - end - - define_method "#{attribute}?" do - !!@attrs[attribute.to_sym] - end + define_method "#{attribute}?" do + raise Sawyer::Error, + "Sawyer method \"#{attribute}?\" overlaps Ruby method. Convert to a hash to access the attribute." end end - # rubocop:enable Gitlab/ModuleWithInstanceVariables end end end diff --git a/doc/api/graphql/index.md b/doc/api/graphql/index.md index 40e1ed115a3..151d8750ad9 100644 --- a/doc/api/graphql/index.md +++ b/doc/api/graphql/index.md @@ -85,7 +85,7 @@ process would pose significant risk. ### Deprecation and removal process The deprecation and removal process for the GitLab GraphQL API aligns with the wider GitLab -[deprecation process](https://about.gitlab.com/handbook/product/gitlab-the-product/#breaking-changes-deprecations-and-removing-features). +[deprecation process](https://about.gitlab.com/handbook/product/gitlab-the-product/#deprecations-removals-and-breaking-changes). Parts of the schema marked for removal from the GitLab GraphQL API are first [deprecated](https://about.gitlab.com/handbook/product/gitlab-the-product/#deprecation) diff --git a/spec/initializers/sawyer_patch_spec.rb b/spec/initializers/sawyer_patch_spec.rb index b3c10e63460..9fcdc9583aa 100644 --- a/spec/initializers/sawyer_patch_spec.rb +++ b/spec/initializers/sawyer_patch_spec.rb @@ -1,36 +1,33 @@ # frozen_string_literal: true + require 'spec_helper' require 'sawyer' require_relative '../../config/initializers/sawyer_patch' RSpec.describe 'sawyer_patch' do - it 'raises error when acessing a method that overlaps a Ruby method' do + it 'raises error when acessing Sawyer Resource dynamic methods' do sawyer_resource = Sawyer::Resource.new( Sawyer::Agent.new(''), { to_s: 'Overriding method', + nil?: 'value', + login: 'Login', user: { to_s: 'Overriding method', name: 'User name' } } ) - error_message = 'Sawyer method "to_s" overlaps Ruby method. Convert to a hash to access the attribute.' - expect { sawyer_resource.to_s }.to raise_error(Sawyer::Error, error_message) - expect { sawyer_resource.to_s? }.to raise_error(Sawyer::Error, error_message) - expect { sawyer_resource.to_s = 'new value' }.to raise_error(Sawyer::Error, error_message) - expect { sawyer_resource.user.to_s }.to raise_error(Sawyer::Error, error_message) - expect(sawyer_resource.user.name).to eq('User name') - end - - it 'raises error when acessing a boolean method that overlaps a Ruby method' do - sawyer_resource = Sawyer::Resource.new( - Sawyer::Agent.new(''), - { - nil?: 'value' - } - ) - + expect { sawyer_resource.to_s }.to raise_error(Sawyer::Error) + expect { sawyer_resource.to_s? }.to raise_error(Sawyer::Error) + expect { sawyer_resource.to_s = 'new value' }.to raise_error(Sawyer::Error) expect { sawyer_resource.nil? }.to raise_error(Sawyer::Error) + expect { sawyer_resource.user.to_s }.to raise_error(Sawyer::Error) + expect { sawyer_resource.login }.to raise_error(Sawyer::Error) + expect { sawyer_resource.login? }.to raise_error(Sawyer::Error) + expect { sawyer_resource.login = 'New value' }.to raise_error(Sawyer::Error) + expect { sawyer_resource.user.name }.to raise_error(Sawyer::Error) + expect { sawyer_resource.user.name? }.to raise_error(Sawyer::Error) + expect { sawyer_resource.user.name = 'New value' }.to raise_error(Sawyer::Error) end it 'raises error when acessing a method that expects an argument' do @@ -45,47 +42,8 @@ RSpec.describe 'sawyer_patch' do } ) - expect(sawyer_resource.user).to eq('value') - expect { sawyer_resource.user = 'New user' }.to raise_error(ArgumentError) expect { sawyer_resource == true }.to raise_error(ArgumentError) expect { sawyer_resource != true }.to raise_error(ArgumentError) expect { sawyer_resource + 1 }.to raise_error(ArgumentError) end - - it 'does not raise error if is not an overlapping method' do - sawyer_resource = Sawyer::Resource.new( - Sawyer::Agent.new(''), - { - count_total: 1, - user: { name: 'User name' } - } - ) - - expect(sawyer_resource.count_total).to eq(1) - expect(sawyer_resource.count_total?).to eq(true) - expect(sawyer_resource.count_total + 1).to eq(2) - sawyer_resource.count_total = 3 - expect(sawyer_resource.count_total).to eq(3) - expect(sawyer_resource.user.name).to eq('User name') - end - - it 'logs when a sawyer resource dynamic method is called' do - sawyer_resource = Sawyer::Resource.new( - Sawyer::Agent.new(''), - { - count_total: 1, - user: { name: 'User name' } - } - ) - expected_attributes = [] - allow(Gitlab::Import::Logger).to receive(:warn) do |params| - expected_attributes.push(params[:attribute]) - end - - sawyer_resource.count_total - sawyer_resource.user - sawyer_resource.user.name - - expect(expected_attributes).to match_array(%i[count_total user user name]) - end end