diff --git a/app/models/shipit/repository.rb b/app/models/shipit/repository.rb new file mode 100644 index 000000000..41645f0d5 --- /dev/null +++ b/app/models/shipit/repository.rb @@ -0,0 +1,33 @@ +module Shipit + class Repository < ApplicationRecord + OWNER_MAX_SIZE = 39 + private_constant :OWNER_MAX_SIZE + + NAME_MAX_SIZE = 100 + private_constant :NAME_MAX_SIZE + + validates :name, uniqueness: {scope: %i(owner), case_sensitive: false, + message: 'cannot be used more than once'} + validates :owner, :name, presence: true, ascii_only: true + validates :owner, format: {with: /\A[a-z0-9_\-\.]+\z/}, length: {maximum: OWNER_MAX_SIZE} + validates :name, format: {with: /\A[a-z0-9_\-\.]+\z/}, length: {maximum: NAME_MAX_SIZE} + + has_many :stacks, dependent: :destroy + + def name=(n) + super(n&.downcase) + end + + def owner=(o) + super(o&.downcase) + end + + def http_url + Shipit.github.url("#{owner}/#{name}") + end + + def git_url + "https://#{Shipit.github.domain}/#{owner}/#{name}.git" + end + end +end diff --git a/app/models/shipit/stack.rb b/app/models/shipit/stack.rb index b4c2a54a0..83c55d91c 100644 --- a/app/models/shipit/stack.rb +++ b/app/models/shipit/stack.rb @@ -40,6 +40,7 @@ def blank? has_many :hooks, dependent: :destroy has_many :api_clients, dependent: :destroy belongs_to :lock_author, class_name: :User, optional: true + belongs_to :repository def lock_author(*) super || AnonymousUser.new diff --git a/db/migrate/20191209231045_create_shipit_repositories.rb b/db/migrate/20191209231045_create_shipit_repositories.rb new file mode 100644 index 000000000..8e9023f3c --- /dev/null +++ b/db/migrate/20191209231045_create_shipit_repositories.rb @@ -0,0 +1,12 @@ +class CreateShipitRepositories < ActiveRecord::Migration[6.0] + def change + create_table :repositories do |t| + t.string :owner, limit: 100, null: false + t.string :name, limit: 39, null: false + + t.timestamps + end + + add_index :repositories, ["owner", "name"], name: "repository_unicity", unique: true + end +end diff --git a/db/migrate/20191209231307_add_repository_reference_to_stacks.rb b/db/migrate/20191209231307_add_repository_reference_to_stacks.rb new file mode 100644 index 000000000..ed53bb916 --- /dev/null +++ b/db/migrate/20191209231307_add_repository_reference_to_stacks.rb @@ -0,0 +1,41 @@ +class AddRepositoryReferenceToStacks < ActiveRecord::Migration[6.0] + def up + add_reference :stacks, :repository + + repositories = Shipit::Stack.distinct.select(:repo_owner, :repo_name).pluck(:repo_owner, :repo_name) + repositories.map do |repo_owner, repo_name| + repository = Shipit::Repository.create_or_find_by( + owner: repo_owner, + name: repo_name, + ) + + stacks = Shipit::Stack.where(repo_owner: repository.owner, repo_name: repository.name) + stacks.update(repository: repository) + end + + change_column :stacks, :repository_id, :integer, null: false + + remove_index :stacks, name: "stack_unicity" + add_index :stacks, ["repository_id", "environment"], name: "stack_unicity", unique: true + change_column :stacks, :repo_owner, :string, null: true + change_column :stacks, :repo_name, :string, null: true + remove_column :stacks, :repo_owner + remove_column :stacks, :repo_name + end + + def down + add_column :stacks, :repo_name, :string, limit: 100 + add_column :stacks, :repo_owner, :string, limit: 39 + remove_index :stacks, name: "stack_unicity" + add_index :stacks, ["repo_owner", "repo_name", "environment"], name: "stack_unicity", unique: true + + Shipit::Repository.find_each do |repository| + repository.stacks.update_all(repo_owner: repository.owner, repo_name: repository.name) + end + + change_column :stacks, :repo_name, :string, null: false + change_column :stacks, :repo_owner, :string, null: false + + remove_reference :stacks, :repository + end +end diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index 47b258fa3..ee0980f40 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_05_02_020249) do +ActiveRecord::Schema.define(version: 2019_12_09_231307) do create_table "api_clients", force: :cascade do |t| t.text "permissions", limit: 65535 @@ -190,9 +190,15 @@ t.index ["user_id"], name: "index_deploy_statuses_on_user_id" end + create_table "repositories", force: :cascade do |t| + t.string "owner", limit: 100, null: false + t.string "name", limit: 39, null: false + t.datetime "created_at", precision: 6, null: false + t.datetime "updated_at", precision: 6, null: false + t.index ["owner", "name"], name: "repository_unicity", unique: true + end + create_table "stacks", force: :cascade do |t| - t.string "repo_name", limit: 100, null: false - t.string "repo_owner", limit: 39, null: false t.string "environment", limit: 50, default: "production", null: false t.datetime "created_at" t.datetime "updated_at" @@ -211,7 +217,9 @@ t.datetime "locked_since" t.boolean "merge_queue_enabled", default: false, null: false t.datetime "last_deployed_at" - t.index ["repo_owner", "repo_name", "environment"], name: "stack_unicity", unique: true + t.integer "repository_id", null: false + t.index ["repository_id", "environment"], name: "stack_unicity", unique: true + t.index ["repository_id"], name: "index_stacks_on_repository_id" end create_table "statuses", force: :cascade do |t| diff --git a/test/fixtures/shipit/repositories.yml b/test/fixtures/shipit/repositories.yml new file mode 100644 index 000000000..3596b35cf --- /dev/null +++ b/test/fixtures/shipit/repositories.yml @@ -0,0 +1,19 @@ +shipit: + owner: shopify + name: shipit-engine + +cyclimse: + owner: george + name: cyclimse + +foo-bar: + owner: shopify + name: foo-bar + +soc: + owner: "shopify" + name: "soc" + +check_runs: + owner: shopify + name: check_runs diff --git a/test/fixtures/shipit/stacks.yml b/test/fixtures/shipit/stacks.yml index b1f69f641..1a69c28b3 100644 --- a/test/fixtures/shipit/stacks.yml +++ b/test/fixtures/shipit/stacks.yml @@ -1,6 +1,5 @@ shipit: - repo_owner: "shopify" - repo_name: "shipit-engine" + repository: shipit environment: "production" branch: master ignore_ci: false @@ -54,8 +53,7 @@ shipit: updated_at: <%= 8.days.ago.to_s(:db) %> shipit_canaries: - repo_owner: "shopify" - repo_name: "shipit-engine" + repository: shipit environment: "canaries" branch: master ignore_ci: false @@ -114,8 +112,7 @@ shipit_canaries: updated_at: <%= 8.days.ago.to_s(:db) %> cyclimse: - repo_owner: george - repo_name: cyclimse + repository: cyclimse environment: production branch: master ignore_ci: false @@ -148,8 +145,7 @@ cyclimse: updated_at: <%= 8.days.ago.to_s(:db) %> undeployed_stack: - repo_owner: "shopify" - repo_name: "foo-bar" + repository: foo-bar environment: "production" branch: master ignore_ci: true @@ -185,8 +181,7 @@ undeployed_stack: updated_at: <%= 8.days.ago.to_s(:db) %> soc: - repo_owner: "shopify" - repo_name: "soc" + repository: soc environment: "production" branch: master tasks_count: 0 @@ -220,16 +215,14 @@ soc: updated_at: <%= 8.days.ago.to_s(:db) %> check_runs: - repo_owner: shopify - repo_name: check_runs + repository: check_runs environment: production branch: master tasks_count: 0 undeployed_commits_count: 1 shipit_undeployed: - repo_owner: "shopify" - repo_name: "shipit-engine" + repository: shipit environment: "undeployed" branch: master ignore_ci: true @@ -284,8 +277,7 @@ shipit_undeployed: updated_at: <%= 8.days.ago.to_s(:db) %> shipit_single: - repo_owner: "shopify" - repo_name: "shipit-engine" + repository: shipit environment: "single" branch: master ignore_ci: false @@ -322,8 +314,7 @@ shipit_single: last_deployed_at: <%= 8.days.ago.to_s(:db) %> shipit_stats: - repo_owner: "shopify" - repo_name: "shipit-engine" + repository: shipit environment: "stats" branch: master ignore_ci: false diff --git a/test/models/shipit/repository_test.rb b/test/models/shipit/repository_test.rb new file mode 100644 index 000000000..4326474ba --- /dev/null +++ b/test/models/shipit/repository_test.rb @@ -0,0 +1,65 @@ +require 'test_helper' + +module Shipit + class RepositoryTest < ActiveSupport::TestCase + setup do + @repository = shipit_repositories(:shipit) + end + + test "owner, and name uniqueness is enforced" do + clone = Repository.new(@repository.attributes.except('id')) + refute clone.save + assert_equal ["cannot be used more than once"], clone.errors[:name] + end + + test "owner, name, and environment can only be ASCII" do + @repository.update(owner: 'héllò', name: 'wørld') + refute_predicate @repository, :valid? + end + + test "owner and name are case insensitive" do + assert_no_difference -> { Repository.count } do + error = assert_raises ActiveRecord::RecordInvalid do + Repository.create!( + owner: @repository.owner.upcase, + name: @repository.name.upcase, + ) + end + assert_equal 'Validation failed: Name cannot be used more than once', error.message + end + + new_repository = Repository.create!(owner: 'FOO', name: 'BAR') + assert_equal new_repository, Repository.find_by(owner: 'foo', name: 'bar') + end + + test "owner is automatically downcased" do + @repository.owner = 'George' + assert_equal 'george', @repository.owner + end + + test "name is automatically downcased" do + @repository.name = 'Cyclim.se' + assert_equal 'cyclim.se', @repository.name + end + + test "owner cannot contain a `/`" do + assert @repository.valid? + @repository.owner = 'foo/bar' + refute @repository.valid? + end + + test "name cannot contain a `/`" do + assert @repository.valid? + @repository.name = 'foo/bar' + refute @repository.valid? + end + + test "http_url" do + assert_equal "https://github.com/#{@repository.owner}/#{@repository.name}", @repository.http_url + end + + test "git_url" do + assert_equal "https://github.com/#{@repository.owner}/#{@repository.name}.git", @repository.git_url + end + end +end diff --git a/test/models/stacks_test.rb b/test/models/stacks_test.rb index 56ef169a5..f443e39be 100644 --- a/test/models/stacks_test.rb +++ b/test/models/stacks_test.rb @@ -9,43 +9,6 @@ def setup GithubHook.any_instance.stubs(:teardown!) end - test "repo_owner, repo_name and environment uniqueness is enforced" do - clone = Stack.new(@stack.attributes.except('id')) - refute clone.save - assert_equal ["cannot be used more than once with this environment"], clone.errors[:repo_name] - end - - test "repo_owner, repo_name, and environment can only be ASCII" do - @stack.update(repo_owner: 'héllò', repo_name: 'wørld', environment: 'pródüctïòn') - refute_predicate @stack, :valid? - end - - test "repo_owner and repo_name are case insensitive" do - assert_no_difference -> { Stack.count } do - error = assert_raises ActiveRecord::RecordInvalid do - Stack.create!( - repo_owner: @stack.repo_owner.upcase, - repo_name: @stack.repo_name.upcase, - environment: @stack.environment, - ) - end - assert_equal 'Validation failed: Repo name cannot be used more than once with this environment', error.message - end - - new_stack = Stack.create!(repo_owner: 'FOO', repo_name: 'BAR') - assert_equal new_stack, Stack.find_by(repo_owner: 'foo', repo_name: 'bar') - end - - test "repo_owner is automatically downcased" do - @stack.repo_owner = 'George' - assert_equal 'george', @stack.repo_owner - end - - test "repo_name is automatically downcased" do - @stack.repo_name = 'Cyclim.se' - assert_equal 'cyclim.se', @stack.repo_name - end - test "branch defaults to master" do @stack.branch = "" assert @stack.save @@ -64,18 +27,6 @@ def setup assert_equal 'foo:bar', @stack.environment end - test "repo_owner cannot contain a `/`" do - assert @stack.valid? - @stack.repo_owner = 'foo/bar' - refute @stack.valid? - end - - test "repo_name cannot contain a `/`" do - assert @stack.valid? - @stack.repo_name = 'foo/bar' - refute @stack.valid? - end - test "repo_http_url" do assert_equal "https://github.com/#{@stack.repo_owner}/#{@stack.repo_name}", @stack.repo_http_url end