summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/models/resource_event.rb (renamed from app/models/concerns/resource_event_tools.rb)25
-rw-r--r--app/models/resource_label_event.rb18
-rw-r--r--app/models/resource_milestone_event.rb12
-rw-r--r--app/models/resource_weight_event.rb19
-rw-r--r--changelogs/unreleased/207126-more-descriptive-error-messages-in-migration-helpers.yml5
-rw-r--r--doc/api/scim.md2
-rw-r--r--doc/ci/variables/README.md9
-rw-r--r--doc/ci/yaml/README.md5
-rw-r--r--doc/user/project/deploy_tokens/index.md22
-rw-r--r--doc/user/search/index.md2
-rw-r--r--lib/gitlab/database/migration_helpers.rb19
-rw-r--r--spec/lib/gitlab/database/migration_helpers_spec.rb92
-rw-r--r--spec/models/resource_label_event_spec.rb4
-rw-r--r--spec/models/resource_weight_event_spec.rb5
-rw-r--r--spec/support/shared_examples/resource_events.rb13
15 files changed, 158 insertions, 94 deletions
diff --git a/app/models/concerns/resource_event_tools.rb b/app/models/resource_event.rb
index 7226b9573e1..9b3a211ad43 100644
--- a/app/models/concerns/resource_event_tools.rb
+++ b/app/models/resource_event.rb
@@ -1,16 +1,27 @@
# frozen_string_literal: true
-module ResourceEventTools
- extend ActiveSupport::Concern
+class ResourceEvent < ApplicationRecord
+ include Gitlab::Utils::StrongMemoize
+ include Importable
- included do
- belongs_to :user
+ self.abstract_class = true
- validates :user, presence: { unless: :importing? }, on: :create
+ validates :user, presence: { unless: :importing? }, on: :create
- validate :exactly_one_issuable
+ belongs_to :user
- scope :created_after, ->(time) { where('created_at > ?', time) }
+ scope :created_after, ->(time) { where('created_at > ?', time) }
+
+ def discussion_id
+ strong_memoize(:discussion_id) do
+ Digest::SHA1.hexdigest(discussion_id_key.join("-"))
+ end
+ end
+
+ private
+
+ def discussion_id_key
+ [self.class.name, created_at, user_id]
end
def exactly_one_issuable
diff --git a/app/models/resource_label_event.rb b/app/models/resource_label_event.rb
index 59907f1b962..970d4e1e562 100644
--- a/app/models/resource_label_event.rb
+++ b/app/models/resource_label_event.rb
@@ -1,10 +1,7 @@
# frozen_string_literal: true
-class ResourceLabelEvent < ApplicationRecord
- include Importable
- include Gitlab::Utils::StrongMemoize
+class ResourceLabelEvent < ResourceEvent
include CacheMarkdownField
- include ResourceEventTools
cache_markdown_field :reference
@@ -13,8 +10,11 @@ class ResourceLabelEvent < ApplicationRecord
belongs_to :label
scope :inc_relations, -> { includes(:label, :user) }
+ scope :by_issue, ->(issue) { where(issue_id: issue.id) }
+ scope :by_merge_request, ->(merge_request) { where(merge_request_id: merge_request.id) }
validates :label, presence: { unless: :importing? }, on: :create
+ validate :exactly_one_issuable
after_save :expire_etag_cache
after_destroy :expire_etag_cache
@@ -41,12 +41,6 @@ class ResourceLabelEvent < ApplicationRecord
issue || merge_request
end
- def discussion_id(resource = nil)
- strong_memoize(:discussion_id) do
- Digest::SHA1.hexdigest(discussion_id_key.join("-"))
- end
- end
-
def project
issuable.project
end
@@ -109,10 +103,6 @@ class ResourceLabelEvent < ApplicationRecord
def resource_parent
issuable.project || issuable.group
end
-
- def discussion_id_key
- [self.class.name, created_at, user_id]
- end
end
ResourceLabelEvent.prepend_if_ee('EE::ResourceLabelEvent')
diff --git a/app/models/resource_milestone_event.rb b/app/models/resource_milestone_event.rb
index ba43a1ee363..d362ebc307a 100644
--- a/app/models/resource_milestone_event.rb
+++ b/app/models/resource_milestone_event.rb
@@ -1,10 +1,6 @@
# frozen_string_literal: true
-class ResourceMilestoneEvent < ApplicationRecord
- include Gitlab::Utils::StrongMemoize
- include Importable
- include ResourceEventTools
-
+class ResourceMilestoneEvent < ResourceEvent
belongs_to :issue
belongs_to :merge_request
belongs_to :milestone
@@ -12,6 +8,8 @@ class ResourceMilestoneEvent < ApplicationRecord
scope :by_issue, ->(issue) { where(issue_id: issue.id) }
scope :by_merge_request, ->(merge_request) { where(merge_request_id: merge_request.id) }
+ validate :exactly_one_issuable
+
enum action: {
add: 1,
remove: 2
@@ -23,8 +21,4 @@ class ResourceMilestoneEvent < ApplicationRecord
def self.issuable_attrs
%i(issue merge_request).freeze
end
-
- def resource
- issue || merge_request
- end
end
diff --git a/app/models/resource_weight_event.rb b/app/models/resource_weight_event.rb
index ab288798aed..e0cc0c87a83 100644
--- a/app/models/resource_weight_event.rb
+++ b/app/models/resource_weight_event.rb
@@ -1,26 +1,9 @@
# frozen_string_literal: true
-class ResourceWeightEvent < ApplicationRecord
- include Gitlab::Utils::StrongMemoize
-
- validates :user, presence: true
+class ResourceWeightEvent < ResourceEvent
validates :issue, presence: true
- belongs_to :user
belongs_to :issue
scope :by_issue, ->(issue) { where(issue_id: issue.id) }
- scope :created_after, ->(time) { where('created_at > ?', time) }
-
- def discussion_id(resource = nil)
- strong_memoize(:discussion_id) do
- Digest::SHA1.hexdigest(discussion_id_key.join("-"))
- end
- end
-
- private
-
- def discussion_id_key
- [self.class.name, created_at, user_id]
- end
end
diff --git a/changelogs/unreleased/207126-more-descriptive-error-messages-in-migration-helpers.yml b/changelogs/unreleased/207126-more-descriptive-error-messages-in-migration-helpers.yml
new file mode 100644
index 00000000000..36ed216a505
--- /dev/null
+++ b/changelogs/unreleased/207126-more-descriptive-error-messages-in-migration-helpers.yml
@@ -0,0 +1,5 @@
+---
+title: Improve error messages of failed migrations
+merge_request: 25457
+author:
+type: changed
diff --git a/doc/api/scim.md b/doc/api/scim.md
index cdd635d0627..eaa56b0d0dd 100644
--- a/doc/api/scim.md
+++ b/doc/api/scim.md
@@ -122,7 +122,7 @@ Parameters:
| `userName` | string | yes | Username of the user. |
| `emails` | JSON string | yes | Work email. |
| `name` | JSON string | yes | Name of the user. |
-| `meta` | string | no | Resource type (`User'). |
+| `meta` | string | no | Resource type (`User`). |
Example request:
diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md
index 643ccd45898..c768c833e7c 100644
--- a/doc/ci/variables/README.md
+++ b/doc/ci/variables/README.md
@@ -571,9 +571,12 @@ Below you can find supported syntax reference:
- `$VARIABLE =~ /^content.*/`
- `$VARIABLE_1 !~ /^content.*/` (introduced in GitLab 11.11)
- It is possible perform pattern matching against a variable and regular
- expression. Expression like this evaluates to truth if matches are found
- when using `=~`. It evaluates to truth if matches are not found when `!~` is used.
+ Variable pattern matching with regular expressions uses the
+ [RE2 regular expression syntax](https://github.com/google/re2/wiki/Syntax).
+ Expressions evaluate as `true` if:
+
+ - Matches are found when using `=~`.
+ - Matches are *not* found when using `!~`.
Pattern matching is case-sensitive by default. Use `i` flag modifier, like
`/pattern/i` to make a pattern case-insensitive.
diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md
index 8931ee43a8a..255ae3f7c13 100644
--- a/doc/ci/yaml/README.md
+++ b/doc/ci/yaml/README.md
@@ -857,7 +857,10 @@ In this example, if the first rule:
`rules:if` differs slightly from `only:variables` by accepting only a single
expression string, rather than an array of them. Any set of expressions to be
-evaluated should be conjoined into a single expression using `&&` or `||`. For example:
+evaluated should be conjoined into a single expression using `&&` or `||`, and use
+the [variable matching syntax](../variables/README.md#supported-syntax).
+
+For example:
```yaml
job:
diff --git a/doc/user/project/deploy_tokens/index.md b/doc/user/project/deploy_tokens/index.md
index 728f09ca787..4479653417c 100644
--- a/doc/user/project/deploy_tokens/index.md
+++ b/doc/user/project/deploy_tokens/index.md
@@ -9,11 +9,11 @@ at midnight UTC and that they can be only managed by [maintainers](../../permiss
## Creating a Deploy Token
-You can create as many deploy tokens as you like from the settings of your project:
+You can create as many deploy tokens as you like from the settings of your project. Alternatively, you can also create [group-scoped deploy tokens](#group-deploy-token).
1. Log in to your GitLab account.
-1. Go to the project you want to create Deploy Tokens for.
-1. Go to **Settings** > **Repository**.
+1. Go to the project (or group) you want to create Deploy Tokens for.
+1. Go to **{settings}** **Settings** > **CI / CD**.
1. Click on "Expand" on **Deploy Tokens** section.
1. Choose a name, expiry date (optional), and username (optional) for the token.
1. Choose the [desired scopes](#limiting-scopes-of-a-deploy-token).
@@ -77,6 +77,22 @@ docker login -u <username> -p <deploy_token> registry.example.com
Just replace `<username>` and `<deploy_token>` with the proper values. Then you can simply
pull images from your Container Registry.
+### Group Deploy Token
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/21765) in GitLab 12.9.
+
+A deploy token created at the group level can be used across all projects that
+belong either to the specific group or to one of its subgroups.
+
+To use a group deploy token:
+
+1. [Create](#creating-a-deploy-token) a deploy token for a group.
+1. Use it the same way you use a project deploy token when
+ [cloning a repository](#git-clone-a-repository).
+
+The scopes applied to a group deploy token (such as `read_repository`) will
+apply consistently when cloning the repository of related projects.
+
### GitLab Deploy Token
> [Introduced][ce-18414] in GitLab 10.8.
diff --git a/doc/user/search/index.md b/doc/user/search/index.md
index 70ab9af0bcc..407578fd4df 100644
--- a/doc/user/search/index.md
+++ b/doc/user/search/index.md
@@ -41,7 +41,7 @@ groups:
- [Label](../project/labels.md)
- My-reaction
- Confidential
- - Epic ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/195704) in GitLab 12.8)
+ - Epic ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/195704) in GitLab 12.9)
- Search for this text
1. Select or type the operator to use for filtering the attribute. The following operators are
available:
diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb
index f75e943671b..82a84508959 100644
--- a/lib/gitlab/database/migration_helpers.rb
+++ b/lib/gitlab/database/migration_helpers.rb
@@ -215,7 +215,7 @@ module Gitlab
fk_name = name || concurrent_foreign_key_name(source, column)
unless foreign_key_exists?(source, name: fk_name)
- raise "cannot find #{fk_name} on #{source} table"
+ raise missing_schema_object_message(source, "foreign key", fk_name)
end
disable_statement_timeout do
@@ -931,7 +931,10 @@ module Gitlab
def column_for(table, name)
name = name.to_s
- columns(table).find { |column| column.name == name }
+ column = columns(table).find { |column| column.name == name }
+ raise(missing_schema_object_message(table, "column", name)) if column.nil?
+
+ column
end
# This will replace the first occurrence of a string in a column with
@@ -1166,6 +1169,18 @@ into similar problems in the future (e.g. when new tables are created).
private
+ def missing_schema_object_message(table, type, name)
+ <<~MESSAGE
+ Could not find #{type} "#{name}" on table "#{table}" which was referenced during the migration.
+ This issue could be caused by the database schema straying from the expected state.
+
+ To resolve this issue, please verify:
+ 1. all previous migrations have completed
+ 2. the database objects used in this migration match the Rails definition in schema.rb or structure.sql
+
+ MESSAGE
+ end
+
def tables_match?(target_table, foreign_key_table)
target_table.blank? || foreign_key_table == target_table
end
diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb
index ce6e8c731e2..1fd6157ce43 100644
--- a/spec/lib/gitlab/database/migration_helpers_spec.rb
+++ b/spec/lib/gitlab/database/migration_helpers_spec.rb
@@ -383,7 +383,8 @@ describe Gitlab::Database::MigrationHelpers do
it 'raises an error' do
expect(model).to receive(:foreign_key_exists?).and_return(false)
- expect { model.validate_foreign_key(:projects, :user_id) }.to raise_error(/cannot find/)
+ error_message = /Could not find foreign key "fk_name" on table "projects"/
+ expect { model.validate_foreign_key(:projects, :user_id, name: :fk_name) }.to raise_error(error_message)
end
end
end
@@ -587,6 +588,8 @@ describe Gitlab::Database::MigrationHelpers do
end
describe '#add_column_with_default' do
+ let(:column) { Project.columns.find { |c| c.name == "id" } }
+
context 'outside of a transaction' do
context 'when a column limit is not set' do
before do
@@ -601,6 +604,9 @@ describe Gitlab::Database::MigrationHelpers do
expect(model).to receive(:change_column_default)
.with(:projects, :foo, 10)
+
+ expect(model).to receive(:column_for)
+ .with(:projects, :foo).and_return(column)
end
it 'adds the column while allowing NULL values' do
@@ -655,6 +661,7 @@ describe Gitlab::Database::MigrationHelpers do
it 'adds the column with a limit' do
allow(model).to receive(:transaction_open?).and_return(false)
allow(model).to receive(:transaction).and_yield
+ allow(model).to receive(:column_for).with(:projects, :foo).and_return(column)
allow(model).to receive(:update_column_in_batches).with(:projects, :foo, 10)
allow(model).to receive(:change_column_null).with(:projects, :foo, false)
allow(model).to receive(:change_column_default).with(:projects, :foo, 10)
@@ -721,50 +728,68 @@ describe Gitlab::Database::MigrationHelpers do
before do
allow(model).to receive(:transaction_open?).and_return(false)
- allow(model).to receive(:column_for).and_return(old_column)
end
- it 'renames a column concurrently' do
- expect(model).to receive(:check_trigger_permissions!).with(:users)
+ context 'when the column to rename exists' do
+ before do
+ allow(model).to receive(:column_for).and_return(old_column)
+ end
- expect(model).to receive(:install_rename_triggers_for_postgresql)
- .with(trigger_name, '"users"', '"old"', '"new"')
+ it 'renames a column concurrently' do
+ expect(model).to receive(:check_trigger_permissions!).with(:users)
- expect(model).to receive(:add_column)
- .with(:users, :new, :integer,
- limit: old_column.limit,
- precision: old_column.precision,
- scale: old_column.scale)
+ expect(model).to receive(:install_rename_triggers_for_postgresql)
+ .with(trigger_name, '"users"', '"old"', '"new"')
- expect(model).to receive(:change_column_default)
- .with(:users, :new, old_column.default)
+ expect(model).to receive(:add_column)
+ .with(:users, :new, :integer,
+ limit: old_column.limit,
+ precision: old_column.precision,
+ scale: old_column.scale)
- expect(model).to receive(:update_column_in_batches)
+ expect(model).to receive(:change_column_default)
+ .with(:users, :new, old_column.default)
- expect(model).to receive(:change_column_null).with(:users, :new, false)
+ expect(model).to receive(:update_column_in_batches)
- expect(model).to receive(:copy_indexes).with(:users, :old, :new)
- expect(model).to receive(:copy_foreign_keys).with(:users, :old, :new)
+ expect(model).to receive(:change_column_null).with(:users, :new, false)
+
+ expect(model).to receive(:copy_indexes).with(:users, :old, :new)
+ expect(model).to receive(:copy_foreign_keys).with(:users, :old, :new)
+
+ model.rename_column_concurrently(:users, :old, :new)
+ end
- model.rename_column_concurrently(:users, :old, :new)
+ context 'when default is false' do
+ let(:old_column) do
+ double(:column,
+ type: :boolean,
+ limit: nil,
+ default: false,
+ null: false,
+ precision: nil,
+ scale: nil)
+ end
+
+ it 'copies the default to the new column' do
+ expect(model).to receive(:change_column_default)
+ .with(:users, :new, old_column.default)
+
+ model.rename_column_concurrently(:users, :old, :new)
+ end
+ end
end
- context 'when default is false' do
- let(:old_column) do
- double(:column,
- type: :boolean,
- limit: nil,
- default: false,
- null: false,
- precision: nil,
- scale: nil)
+ context 'when the column to be renamed does not exist' do
+ before do
+ allow(model).to receive(:columns).and_return([])
end
- it 'copies the default to the new column' do
- expect(model).to receive(:change_column_default)
- .with(:users, :new, old_column.default)
+ it 'raises an error with appropriate message' do
+ expect(model).to receive(:check_trigger_permissions!).with(:users)
- model.rename_column_concurrently(:users, :old, :new)
+ error_message = /Could not find column "missing_column" on table "users"/
+ expect { model.rename_column_concurrently(:users, :missing_column, :new) }.to raise_error(error_message)
end
end
end
@@ -1133,8 +1158,9 @@ describe Gitlab::Database::MigrationHelpers do
expect(column.name).to eq('id')
end
- it 'returns nil when a column does not exist' do
- expect(model.column_for(:users, :kittens)).to be_nil
+ it 'raises an error when a column does not exist' do
+ error_message = /Could not find column "kittens" on table "users"/
+ expect { model.column_for(:users, :kittens) }.to raise_error(error_message)
end
end
diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb
index a92f5ee93e1..ca887b485a2 100644
--- a/spec/models/resource_label_event_spec.rb
+++ b/spec/models/resource_label_event_spec.rb
@@ -10,6 +10,10 @@ RSpec.describe ResourceLabelEvent, type: :model do
it_behaves_like 'having unique enum values'
+ it_behaves_like 'a resource event'
+ it_behaves_like 'a resource event for issues'
+ it_behaves_like 'a resource event for merge requests'
+
describe 'associations' do
it { is_expected.to belong_to(:user) }
it { is_expected.to belong_to(:issue) }
diff --git a/spec/models/resource_weight_event_spec.rb b/spec/models/resource_weight_event_spec.rb
index 2f00204512e..11b633e1dcf 100644
--- a/spec/models/resource_weight_event_spec.rb
+++ b/spec/models/resource_weight_event_spec.rb
@@ -3,6 +3,9 @@
require 'spec_helper'
RSpec.describe ResourceWeightEvent, type: :model do
+ it_behaves_like 'a resource event'
+ it_behaves_like 'a resource event for issues'
+
let_it_be(:user1) { create(:user) }
let_it_be(:user2) { create(:user) }
@@ -11,13 +14,11 @@ RSpec.describe ResourceWeightEvent, type: :model do
let_it_be(:issue3) { create(:issue, author: user2) }
describe 'validations' do
- it { is_expected.not_to allow_value(nil).for(:user) }
it { is_expected.not_to allow_value(nil).for(:issue) }
it { is_expected.to allow_value(nil).for(:weight) }
end
describe 'associations' do
- it { is_expected.to belong_to(:user) }
it { is_expected.to belong_to(:issue) }
end
diff --git a/spec/support/shared_examples/resource_events.rb b/spec/support/shared_examples/resource_events.rb
index d7e7349ad6c..963453666c9 100644
--- a/spec/support/shared_examples/resource_events.rb
+++ b/spec/support/shared_examples/resource_events.rb
@@ -10,8 +10,21 @@ shared_examples 'a resource event' do
let_it_be(:issue2) { create(:issue, author: user1) }
let_it_be(:issue3) { create(:issue, author: user2) }
+ describe 'importable' do
+ it { is_expected.to respond_to(:importing?) }
+ it { is_expected.to respond_to(:imported?) }
+ end
+
describe 'validations' do
it { is_expected.not_to allow_value(nil).for(:user) }
+
+ context 'when importing' do
+ before do
+ allow(subject).to receive(:importing?).and_return(true)
+ end
+
+ it { is_expected.to allow_value(nil).for(:user) }
+ end
end
describe 'associations' do