Extract repository

Summary
-------

At PowerHRG we desire to implement the ability to automatically deploy
a branch as a new "review instance" of our application to our
Kubernetes cluster when a Pull Request for that branch is opened - and
subsequently clean up the instance when the PR is closed or merged -
functionality similar to Heroku's Review Apps. However, we maintain a
number of projects and do NOT want to expose this functionality for
every repository.

As we understand it, Shipit offers no mechanism at the repository
level to control which repositories can dynamically provision
stacks. It seems the Stack is the highest level concept which involves
a repository, but this remains insufficient since the Stack also
marries a particular branch and environment to the repository
concept. What we _think_ we'd like is a higher level concept which
represents the repository to which stacks could belong. This
repository could then contain its own configuration for the
project-level features we desire.

References
----------

- Shopify#960

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

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

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

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

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|

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

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

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

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