diff options
author | Stan Hu <stanhu@gmail.com> | 2018-05-11 21:19:49 +0300 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2018-05-15 15:20:14 +0300 |
commit | 3126e89eb811ae76dbb46c122251485361bb69cb (patch) | |
tree | 6a79e9756d0a180c0edaf7f306edc2cb5adf1a0f /spec | |
parent | f4ef6b474c44eb8e7034034dd95152818ae33b4a (diff) |
Add a unique and not null constraint on the project_features.project_id column
This commit has two migrations:
1. The first prunes duplicate rows in the project_features table and leaves
the row with the highest ID. Since the behavior was indeterministic before
and depended on which row the database decided to use, this change at least
makes the permissions consistent. For example, in some cases, the Wiki may
have been disabled but enabled in another entry.
2. The second adds a non-null constraint on the project_features.project_id
column.
Closes #37882
Fixes a significant part of gitlab-com/migration#408.
We found that we were overcounting Wikis because of these duplicates.
On GitLab.com, there are 56 rows with duplicate entries by project_id, and 16,661 rows with NULL project_id values.
Diffstat (limited to 'spec')
-rw-r--r-- | spec/migrations/add_unique_constraint_to_project_features_project_id_spec.rb | 59 | ||||
-rw-r--r-- | spec/models/guest_spec.rb | 6 |
2 files changed, 62 insertions, 3 deletions
diff --git a/spec/migrations/add_unique_constraint_to_project_features_project_id_spec.rb b/spec/migrations/add_unique_constraint_to_project_features_project_id_spec.rb new file mode 100644 index 00000000000..bf299b70a29 --- /dev/null +++ b/spec/migrations/add_unique_constraint_to_project_features_project_id_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20180511174224_add_unique_constraint_to_project_features_project_id.rb') + +describe AddUniqueConstraintToProjectFeaturesProjectId, :migration do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:features) { table(:project_features) } + let(:migration) { described_class.new } + + describe '#up' do + before do + (1..3).each do |i| + namespaces.create(id: i, name: "ns-test-#{i}", path: "ns-test-i#{i}") + projects.create!(id: i, name: "test-#{i}", path: "test-#{i}", namespace_id: i) + end + + features.create!(id: 1, project_id: 1) + features.create!(id: 2, project_id: 1) + features.create!(id: 3, project_id: 2) + features.create!(id: 4, project_id: 2) + features.create!(id: 5, project_id: 2) + features.create!(id: 6, project_id: 3) + end + + it 'creates a unique index and removes duplicates' do + expect(migration.index_exists?(:project_features, :project_id, unique: false, name: 'index_project_features_on_project_id')).to be true + + expect { migration.up }.to change { features.count }.from(6).to(3) + + expect(migration.index_exists?(:project_features, :project_id, unique: true, name: 'index_project_features_on_project_id')).to be true + expect(migration.index_exists?(:project_features, :project_id, name: 'index_project_features_on_project_id_unique')).to be false + + project_1_features = features.where(project_id: 1) + expect(project_1_features.count).to eq(1) + expect(project_1_features.first.id).to eq(2) + + project_2_features = features.where(project_id: 2) + expect(project_2_features.count).to eq(1) + expect(project_2_features.first.id).to eq(5) + + project_3_features = features.where(project_id: 3) + expect(project_3_features.count).to eq(1) + expect(project_3_features.first.id).to eq(6) + end + end + + describe '#down' do + it 'restores the original index' do + migration.up + + expect(migration.index_exists?(:project_features, :project_id, unique: true, name: 'index_project_features_on_project_id')).to be true + + migration.down + + expect(migration.index_exists?(:project_features, :project_id, unique: false, name: 'index_project_features_on_project_id')).to be true + expect(migration.index_exists?(:project_features, :project_id, name: 'index_project_features_on_project_id_old')).to be false + end + end +end diff --git a/spec/models/guest_spec.rb b/spec/models/guest_spec.rb index 2afdd6751a4..fc30f3056e5 100644 --- a/spec/models/guest_spec.rb +++ b/spec/models/guest_spec.rb @@ -1,9 +1,9 @@ require 'spec_helper' describe Guest do - let(:public_project) { build_stubbed(:project, :public) } - let(:private_project) { build_stubbed(:project, :private) } - let(:internal_project) { build_stubbed(:project, :internal) } + set(:public_project) { create(:project, :public) } + set(:private_project) { create(:project, :private) } + set(:internal_project) { create(:project, :internal) } describe '.can_pull?' do context 'when project is private' do |