refactor: move upsert logic into a plugin

Author: bethesque

DEVELOPER_SETUP.md

@@ -45,28 +45,22 @@ To make the barrier to entry as low as possible, the mysql2 and pg gems are not
 
 You can set the `PACT_BROKER_DATABASE_URL` environment variable to use a postgres/mysql database using the format `driver://username:password@host:port/database` eg. `postgres://pact_broker:password@localhost/pact_broker`. Ensure you have set `INSTALL_MYSQL=true` or `INSTALL_PG=true` and run `bundle install` to make sure the required gems are present.
 
-## Running the tests with mysql
+## Running the tests with postgres and mysql
 
 ```
-docker-compose -f docker-compose-test-mysql.yml up --build --remove-orphans
+docker-compose -f docker-compose-test.yml up --build --remove-orphans
 
 # in separate console window...
-docker-compose -f docker-compose-test-mysql.yml run --rm tests bash
-
-# inside the tests container
-bundle exec rake
-```
-
-## Running the tests with postgres
-
-```
-docker-compose -f docker-compose-test-postgres.yml up --build --remove-orphans
+docker-compose -f docker-compose-test.yml run --rm postgres-tests bash
 
 # in separate console window...
-docker-compose -f docker-compose-test-postgres.yml run --rm tests bash
+docker-compose -f docker-compose-test.yml run --rm mysql-tests bash
 
 # inside the tests container
 bundle exec rake
+
+# if you don't want to run the whole rake test suite, init the db first
+/home/init-db.sh
 ```
 ## Running the tests
 

docker-compose-test-mysql.yml

@@ -0,0 +1,100 @@
+# This doesn't work yet
+version: "3"
+
+services:
+  postgres:
+    image: postgres
+    healthcheck:
+      test: psql postgres --command "select 1" -U postgres
+    ports:
+      - "5432:5432"
+    environment:
+      POSTGRES_USER: postgres
+      POSTGRES_PASSWORD: postgres
+      POSTGRES_DB: postgres
+
+  init-postgres:
+    build: .
+    depends_on:
+      - postgres
+    environment:
+      DATABASE_ADAPTER: docker_compose_postgres
+      PACT_BROKER_HIDE_PACTFLOW_MESSAGES: 'true'
+      INSTALL_PG: 'true'
+    volumes:
+      - ./lib:/home/lib
+      - ./db:/home/db
+      - ./tasks:/home/tasks
+      - ./Rakefile:/home/Rakefile
+      - ./config:/home/config
+    command: dockerize --wait tcp://postgres:5432 /home/init-db.sh
+
+  postgres-tests:
+    build: .
+    depends_on:
+      - postgres
+    environment:
+      DATABASE_ADAPTER: docker_compose_postgres
+      PACT_BROKER_HIDE_PACTFLOW_MESSAGES: 'true'
+      INSTALL_PG: 'true'
+    volumes:
+      - ./lib:/home/lib
+      - ./spec:/home/spec
+      - ./db:/home/db
+      - ./config.ru:/home/config.ru
+      - ./tasks:/home/tasks
+      - ./Rakefile:/home/Rakefile
+      - ./config:/home/config
+      - ./.rspec:/home/.rspec
+      - ./.rubocop:/home/.rubocop
+      - ./.gitignore:/home/.gitignore
+      - ./public:/home/public
+
+  mysql:
+    image: mysql:5.7.28
+    command: --default-authentication-plugin=mysql_native_password
+    environment:
+      MYSQL_ROOT_PASSWORD: pact_broker
+      MYSQL_DATABASE: pact_broker
+      MYSQL_USER: pact_broker
+      MYSQL_PASSWORD: pact_broker
+    ports:
+      - "3306:3306"
+
+  init-mysql:
+    build: .
+    depends_on:
+      - mysql
+    environment:
+      DATABASE_ADAPTER: docker_compose_mysql
+      PACT_BROKER_HIDE_PACTFLOW_MESSAGES: 'true'
+      INSTALL_MYSQL: 'true'
+    volumes:
+      - ./lib:/home/lib
+      - ./db:/home/db
+      - ./tasks:/home/tasks
+      - ./Rakefile:/home/Rakefile
+      - ./config:/home/config
+    command: dockerize --wait tcp://mysql:3306 /home/init-db.sh
+
+  mysql-tests:
+    build: .
+    depends_on:
+      - mysql
+    environment:
+      DATABASE_ADAPTER: docker_compose_mysql
+      PACT_BROKER_HIDE_PACTFLOW_MESSAGES: 'true'
+      INSTALL_MYSQL: 'true'
+    volumes:
+      - ./lib:/home/lib
+      - ./spec:/home/spec
+      - ./db:/home/db
+      - ./config.ru:/home/config.ru
+      - ./tasks:/home/tasks
+      - ./Rakefile:/home/Rakefile
+      - ./config:/home/config
+      - ./.rspec:/home/.rspec
+      - ./.rubocop:/home/.rubocop
+      - ./.gitignore:/home/.gitignore
+      - ./public:/home/public
+

docker-compose-test.yml

@@ -4,17 +4,13 @@
 module PactBroker
   module Pacts
     class LatestPactPublicationIdForConsumerVersion < Sequel::Model(:latest_pact_publication_ids_for_consumer_versions)
-
+      set_primary_key [:provider_id, :consumer_version_id]
       unrestrict_primary_key
+      plugin :upsert, identifying_columns: [:provider_id, :consumer_version_id]
 
       dataset_module do
         include PactBroker::Repositories::Helpers
       end
-
-      def upsert
-        before_create
-        self.class.upsert(to_hash, [:provider_id, :consumer_version_id])
-      end
     end
   end
 end

lib/pact_broker/pacts/latest_pact_publication_id_for_consumer_version.rb

@@ -7,7 +7,6 @@
 module PactBroker
   module Pacts
     class PactPublication < Sequel::Model(:pact_publications)
-      UNIQUE_CONSTRAINT_KEYS = [:consumer_version_id, :provider_id, :revision_number].freeze
 
       extend Forwardable
 
@@ -22,6 +21,9 @@ class PactPublication < Sequel::Model(:pact_publications)
       one_to_one(:latest_verification, class: "PactBroker::Verifications::LatestVerificationForPactVersion", key: :pact_version_id, primary_key: :pact_version_id)
       one_to_many(:head_pact_tags, class: "PactBroker::Tags::HeadPactTag", primary_key: :id, key: :pact_publication_id)
 
+      plugin :upsert, identifying_columns: [:consumer_version_id, :provider_id, :revision_number]
+      plugin :timestamps, update_on_create: true
+
       dataset_module do
         include PactBroker::Repositories::Helpers
 
@@ -158,21 +160,12 @@ def to_version_domain_lightweight
         OpenStruct.new(number: consumer_version.number, pacticipant: consumer, order: consumer_version.order)
       end
 
-      def upsert
-        before_create
-        params = to_hash.merge(created_at: Sequel.datetime_class.now)
-        self.id = self.class.upsert(params, UNIQUE_CONSTRAINT_KEYS).id
-        self.refresh
-      end
-
       private
 
       def cached_domain_for_delegation
         @domain_object ||= to_domain
       end
     end
-
-    PactPublication.plugin :timestamps, update_on_create: true
   end
 end
 

lib/pact_broker/pacts/pact_publication.rb

@@ -6,6 +6,8 @@ module PactBroker
   module Pacts
     class PactVersion < Sequel::Model(:pact_versions)
       plugin :timestamps
+      plugin :upsert, identifying_columns: [:consumer_id, :provider_id, :sha]
+
       one_to_many :pact_publications, reciprocal: :pact_version
       one_to_many :verifications, reciprocal: :verification, order: :id, class: "PactBroker::Domain::Verification"
       one_to_one :latest_verification, class: "PactBroker::Verifications::LatestVerificationForPactVersion", key: :pact_version_id, primary_key: :id
@@ -79,11 +81,6 @@ def verified_successfully_by_any_provider_version?
           .join(:versions, Sequel[:versions][:id] => Sequel[:verifications][:provider_version_id])
           .any?
       end
-
-      def upsert
-        self.before_create
-        self.class.upsert(to_hash, [:consumer_id, :provider_id, :sha])
-      end
     end
   end
 end

lib/pact_broker/pacts/pact_version.rb

@@ -51,24 +51,6 @@ def select_for_subquery column
           select(column)
         end
       end
-
-      def upsert row, unique_key_names, columns_to_update = nil
-        if postgres?
-          insert_conflict(update: row, target: unique_key_names).insert(row)
-        elsif mysql?
-          update_cols = columns_to_update || (row.keys - unique_key_names)
-          on_duplicate_key_update(*update_cols).insert(row)
-        else
-          # Sqlite
-          key = row.reject{ |k, v| !unique_key_names.include?(k) }
-          if where(key).count == 0
-            insert(row)
-          else
-            where(key).update(row)
-          end
-        end
-        model.where(row.select{ |key, _| unique_key_names.include?(key)}).single_record
-      end
     end
   end
 end

lib/pact_broker/repositories/helpers.rb

@@ -3,17 +3,14 @@
 module PactBroker
   module Verifications
     class LatestVerificationIdForPactVersionAndProviderVersion < Sequel::Model(:latest_verification_id_for_pact_version_and_provider_version)
-
       unrestrict_primary_key
+      set_primary_key [:pact_version_id, :provider_version_id]
+
+      plugin :upsert, identifying_columns: [:pact_version_id, :provider_version_id]
 
       dataset_module do
         include PactBroker::Repositories::Helpers
       end
-
-      def upsert
-        before_create
-        self.class.upsert(to_hash, [:pact_version_id, :provider_version_id])
-      end
     end
   end
 end

lib/pact_broker/verifications/latest_verification_id_for_pact_version_and_provider_version.rb

@@ -6,9 +6,6 @@
 # Rather than re-writing the whole save method and all the hooks and validation logic in it,
 # it naughtily overrides the private _insert_dataset.
 
-# MySQL does not return the id of the original row when a duplicate is inserted,
-# so we have to manually find the original record an load it into the model.
-
 module Sequel
   module Plugins
     module InsertIgnore
@@ -28,7 +25,6 @@ def insert_ignore(opts = {})
           load_values_from_previously_inserted_object unless id
           self
         rescue Sequel::NoExistingObject
-          # MySQL. Ruining it for everyone.
           load_values_from_previously_inserted_object
         end
 
@@ -40,7 +36,7 @@ def load_values_from_previously_inserted_object
           refresh
         end
 
-        # Does the job for Sqlite and Postgres
+        # naughty override of Sequel private method
         def _insert_dataset
           super.insert_ignore
         end