Merge pull request #4 from Kaligo/fix/db_connection_leak

Fix DB connection leak

bin/setup

@@ -3,6 +3,8 @@ set -euo pipefail
 IFS=$'\n\t'
 set -vx
 
+brew install pkg-config
+
 bundle install
 
 # Do any other automated setup that you need to do here

lib/sequel_data/migrate/migrator.rb

@@ -15,39 +15,41 @@ def initialize(config)
       end
 
       def migrate
-        db = connect_database
-        dataset = ensure_table_exists(db)
-
-        already_migrated = dataset.select_map(column).to_set
-        migration_files = fetch_migration_files.reject { |file| already_migrated.include?(File.basename(file)) }.sort
-        migrations = fetch_migrations(migration_files)
-
-        migrations.zip(migration_files).each do |migration, file|
-          db.log_info("Begin applying migration file #{file}")
-          migration.new.up
-          set_migration_version(db, file)
-          db.log_info("Finished applying migration version #{file}")
+        connect_database do |db|
+          dataset = ensure_table_exists(db)
+
+          already_migrated = dataset.select_map(column).to_set
+          migration_files = fetch_migration_files.reject { |file| already_migrated.include?(File.basename(file)) }.sort
+          migrations = fetch_migrations(migration_files)
+
+          migrations.zip(migration_files).each do |migration, file|
+            db.log_info("Begin applying migration file #{file}")
+            migration.new.up
+            set_migration_version(db, file)
+            db.log_info("Finished applying migration version #{file}")
+          end
         end
       end
 
       def rollback(step = 1)
-        db = connect_database
-        dataset = ensure_table_exists(db)
-
-        already_migrated = dataset.select_map(column).to_set
-        migration_files = fetch_migration_files.select do |file|
-          already_migrated.include?(File.basename(file))
-        end.sort.reverse!
-        migrations = fetch_migrations(migration_files)
-
-        migrations.zip(migration_files).each do |migration, file|
-          step -= 1
-          break if step.negative?
-
-          db.log_info("Begin rolling back migration file #{file}")
-          migration.new.down
-          remove_migration_version(db, file)
-          db.log_info("Finished rolling back migration version #{file}")
+        connect_database do |db|
+          dataset = ensure_table_exists(db)
+
+          already_migrated = dataset.select_map(column).to_set
+          migration_files = fetch_migration_files.select do |file|
+            already_migrated.include?(File.basename(file))
+          end.sort.reverse!
+          migrations = fetch_migrations(migration_files)
+
+          migrations.zip(migration_files).each do |migration, file|
+            step -= 1
+            break if step.negative?
+
+            db.log_info("Begin rolling back migration file #{file}")
+            migration.new.down
+            remove_migration_version(db, file)
+            db.log_info("Finished rolling back migration version #{file}")
+          end
         end
       end
 
@@ -63,10 +65,10 @@ def table
         :data_migrations
       end
 
-      def connect_database
+      def connect_database(&block)
         raise ConfigurationError, "db_configuration is not set" if config.db_configuration.host.nil?
 
-        Sequel.connect(config.db_configuration.host)
+        Sequel.connect(config.db_configuration.host, &block)
       end
 
       def ensure_table_exists(db)

sig/sequel_data/migrate.rbs

@@ -54,7 +54,7 @@ module SequelData
       attr_reader config: untyped
       def column: -> :version
       def table: -> :data_migrations
-      def connect_database: -> untyped
+      def connect_database: () { (untyped db) -> void } -> untyped
       def ensure_table_exists: (untyped db) -> untyped
       def fetch_migration_files: -> Array[String]
       def load_migration_file: (String file) -> untyped

spec/sequel_data/migrate/migrator_spec.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+RSpec::Matchers.define_negated_matcher :not_change, :change
+
 RSpec.describe SequelData::Migrate::Migrator do
   let(:db) { DB }
   let(:config) do
@@ -115,6 +117,18 @@
         }.from(["20010101010205_second_migration.rb"]).to(versions)
       end
     end
+
+    context "when migrations are applied" do
+      after { db.drop_table(:data_migrations) }
+
+      it "does not leave extra connections open" do
+        expect do
+          migrator.migrate
+        end.to(not_change do
+          Sequel::DATABASES.size
+        end)
+      end
+    end
   end
 
   describe "#rollback" do
@@ -189,5 +203,19 @@
         }.from(["20010101010203_initial_migration.rb", "20010101010205_second_migration.rb"]).to([])
       end
     end
+
+    context "when rollbacks are applied" do
+      before { migrator.migrate }
+
+      after { db.drop_table(:data_migrations) }
+
+      it "does not leave extra connections open" do
+        expect do
+          migrator.rollback
+        end.to(not_change do
+          Sequel::DATABASES.size
+        end)
+      end
+    end
   end
 end