refactor: remove global database state and introduce Micrate::Runner

.ameba.yml

@@ -0,0 +1,6 @@
+Naming/BlockParameterName:
+  Enabled: false
+Metrics/CyclomaticComplexity:
+  Enabled: false
+Documentation/DocumentationAdmonition:
+  Enabled: false

.gitignore

@@ -8,3 +8,4 @@
 # Libraries don't need dependency lock
 # Dependencies will be locked in application that uses them
 /shard.lock
+.agents/

README.md

@@ -108,7 +108,7 @@ This allows you to programatically use micrate's features. You'll see the `Micra
 require "micrate"
 require "pg"
 
-Micrate::DB.connection_url = "postgresql://..."
+ENV["DATABASE_URL"] = "postgresql://..."
 Micrate::Cli.run
 ```
 

examples/micrate

@@ -8,5 +8,10 @@
 require "../src/micrate"
 require "pg"
 
-Micrate::DB.connection_url = "postgresql://..."
+# The CLI uses the DATABASE_URL environment variable by default:
+# ENV["DATABASE_URL"] = "postgresql://..."
 Micrate::Cli.run
+
+# Or to run migrations programmatically:
+# runner = Micrate::Runner.new("postgresql://...")
+# runner.connect { |db| runner.up(db) }

shard.yml

@@ -27,3 +27,6 @@ development_dependencies:
   spectator:
     gitlab: arctic-fox/spectator
     version: ~> 0.11.3
+  ameba:
+    github: crystal-ameba/ameba
+    version: ~> 1.6.0

spec/migration_spec.cr

@@ -101,5 +101,5 @@ baz;")
 end
 
 def statements(migration, direction)
-  migration.statements(direction).map { |stmt| stmt.strip }
+  migration.statements(direction).map(&.strip)
 end

src/micrate.cr

@@ -1,5 +1,4 @@
 require "log"
-
 require "./micrate/*"
 
 module Micrate
@@ -13,62 +12,6 @@ module Micrate
     File.join(db_dir, "migrations")
   end
 
-  def self.dbversion(db)
-    begin
-      rows = DB.get_versions_last_first_order(db)
-      return extract_dbversion(rows)
-    rescue Exception
-      DB.create_migrations_table(db)
-      return 0
-    end
-  end
-
-  def self.up(db)
-    all_migrations = migrations_by_version
-
-    if all_migrations.size == 0
-      Log.warn { "No migrations found!" }
-      return
-    end
-
-    current = dbversion(db)
-    target = all_migrations.keys.sort.last
-    migrate(all_migrations, current, target, db)
-  end
-
-  def self.down(db)
-    all_migrations = migrations_by_version
-
-    current = dbversion(db)
-    target = previous_version(current, all_migrations.keys)
-    migrate(all_migrations, current, target, db)
-  end
-
-  def self.redo(db)
-    all_migrations = migrations_by_version
-
-    current = dbversion(db)
-    previous = previous_version(current, all_migrations.keys)
-
-    if migrate(all_migrations, current, previous, db) == :success
-      migrate(all_migrations, previous, current, db)
-    end
-  end
-
-  def self.migration_status(db) : Hash(Migration, Time?)
-    # ensure that migration table exists
-    dbversion(db)
-    migration_status(migrations_by_version.values, db)
-  end
-
-  def self.migration_status(migrations : Array(Migration), db) : Hash(Migration, Time?)
-    ({} of Migration => Time?).tap do |ret|
-      migrations.each do |m|
-        ret[m] = DB.get_migration_status(m, db)
-      end
-    end
-  end
-
   def self.create(name, dir, time)
     timestamp = time.to_s("%Y%m%d%H%M%S")
     filename = File.join(dir, "#{timestamp}_#{name}.sql")
@@ -85,50 +28,7 @@ module Micrate
     Dir.mkdir_p dir
     File.write(filename, migration_template)
 
-    return filename
-  end
-
-  def self.connection_url=(connection_url)
-    DB.connection_url = connection_url
-  end
-
-  # ---------------------------------
-  # Private
-  # ---------------------------------
-
-  private def self.migrate(all_migrations : Hash(Int, Migration), current : Int, target : Int, db)
-    direction = current < target ? :forward : :backwards
-
-    status = migration_status(all_migrations.values, db)
-    plan = migration_plan(status, current, target, direction)
-
-    if plan.empty?
-      Log.info { "No migrations to run. current version: #{current}" }
-      return :nop
-    end
-
-    Log.info { "Migrating db, current version: #{current}, target: #{target}" }
-
-    plan.each do |version|
-      migration = all_migrations[version]
-
-      # Wrap migration in a transaction
-      db.transaction do |tx|
-        migration.statements(direction).each do |stmt|
-          tx.connection.exec(stmt)
-        end
-
-        DB.record_migration(migration, direction, tx.connection)
-
-        tx.commit
-        Log.info { "OK   #{migration.name}" }
-      rescue e : Exception
-        tx.rollback
-        Log.error(exception: e) { "An error occurred executing migration #{migration.version}." }
-        return :error
-      end
-    end
-    :success
+    filename
   end
 
   private def self.verify_unordered_migrations(current, status : Hash(Int, Bool))
@@ -140,7 +40,7 @@ module Micrate
     end
   end
 
-  private def self.previous_version(current, all_versions)
+  def self.previous_version(current, all_versions)
     all_previous = all_versions.select { |version| version < current }
     if !all_previous.empty?
       return all_previous.max
@@ -150,18 +50,18 @@ module Micrate
       # the given version is (likely) valid but we didn't find
       # anything before it.
       # return value must reflect that no migrations have been applied.
-      return 0
+      0
     else
       raise "no previous version found"
     end
   end
 
-  private def self.migrations_by_version
+  def self.migrations_by_version
     Dir.entries(migrations_dir)
       .select { |name| File.file? File.join(migrations_dir, name) }
       .select { |name| /^\d+.+\.sql$/ =~ name }
       .map { |name| Migration.from_file(name) }
-      .index_by { |migration| migration.version }
+      .index_by(&.version)
   end
 
   def self.migration_plan(status : Hash(Migration, Time?), current : Int, target : Int, direction)
@@ -177,19 +77,16 @@ module Micrate
 
     if direction == :forward
       all_versions.keys
-        .sort
+        .sort!
         .select { |v| v > current && v <= target }
     else
       all_versions.keys
-        .sort
-        .reverse
+        .sort!
+        .reverse!
         .select { |v| v <= current && v > target }
     end
   end
 
-  # The most recent record for each migration specifies
-  # whether it has been applied or rolled back.
-  # The first version we find that has been applied is the current version.
   def self.extract_dbversion(rows)
     to_skip = [] of Int64
 
@@ -204,7 +101,7 @@ module Micrate
       end
     end
 
-    return 0
+    0
   end
 
   class UnorderedMigrationsException < Exception

src/micrate/cli.cr

@@ -5,66 +5,73 @@ module Micrate
     Log = ::Log.for(self)
 
     def self.drop_database
-      url = Micrate::DB.connection_url.to_s
+      url = ENV["DATABASE_URL"]? || raise "DATABASE_URL not set"
       if url.starts_with? "sqlite3:"
         path = url.gsub("sqlite3:", "")
         File.delete(path)
         Log.info { "Deleted file #{path}" }
       else
-        name = set_database_to_schema url
-        Micrate::DB.connect do |db|
+        root_url, name = extract_schema_name url
+        runner = Micrate::Runner.new(root_url)
+        runner.connect do |db|
           db.exec "DROP DATABASE IF EXISTS #{name};"
         end
         Log.info { "Dropped database #{name}" }
       end
     end
 
     def self.create_database
-      url = Micrate::DB.connection_url.to_s
+      url = ENV["DATABASE_URL"]? || raise "DATABASE_URL not set"
       if url.starts_with? "sqlite3:"
         Log.info { "For sqlite3, the database will be created during the first migration." }
       else
-        name = set_database_to_schema url
-        Micrate::DB.connect do |db|
+        root_url, name = extract_schema_name url
+        runner = Micrate::Runner.new(root_url)
+        runner.connect do |db|
           db.exec "CREATE DATABASE #{name};"
         end
         Log.info { "Created database #{name}" }
       end
     end
 
-    def self.set_database_to_schema(url)
+    def self.extract_schema_name(url)
       uri = URI.parse(url)
       if path = uri.path
-        Micrate::DB.connection_url = url.gsub(path, "/#{uri.scheme}")
-        path.gsub("/", "")
+        root_url = url.gsub(path, "/#{uri.scheme}")
+        {root_url, path.gsub("/", "")}
       else
         Log.error { "Could not determine database name" }
+        {url, ""}
       end
     end
 
     def self.run_up
-      Micrate::DB.connect do |db|
-        Micrate.up(db)
+      runner = Micrate::Runner.new
+      runner.connect do |db|
+        runner.up(db)
       end
     end
 
     def self.run_down
-      Micrate::DB.connect do |db|
-        Micrate.down(db)
+      runner = Micrate::Runner.new
+      runner.connect do |db|
+        runner.down(db)
       end
     end
 
     def self.run_redo
-      Micrate::DB.connect do |db|
-        Micrate.redo(db)
+      runner = Micrate::Runner.new
+      runner.connect do |db|
+        runner.redo(db)
       end
     end
 
     def self.run_status
-      Micrate::DB.connect do |db|
+      runner = Micrate::Runner.new
+      runner.connect do |db|
         Log.info { "Applied At                  Migration" }
         Log.info { "=======================================" }
-        Micrate.migration_status(db).each do |migration, migrated_at|
+        runner.migration_status(db).each do |migration, migrated_at|
           ts = migrated_at.nil? ? "Pending" : migrated_at.to_s
           Log.info { "%-24s -- %s\n" % [ts, migration.name] }
         end
@@ -81,9 +88,10 @@ module Micrate
     end
 
     def self.run_dbversion
-      Micrate::DB.connect do |db|
+      runner = Micrate::Runner.new
+      runner.connect do |db|
         begin
-          Log.info { Micrate.dbversion(db) }
+          Log.info { runner.dbversion(db) }
         rescue
           raise "Could not read dbversion. Please make sure the database exists and verify the connection URL."
         end