From 6d1601eb2b40b2ce0509ef8c36ef9e8373bc6ca8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9nich=20Bon=20=C4=86iri=C4=87?= Date: Thu, 11 Jun 2026 00:58:01 -0600 Subject: [PATCH 1/2] fix(core): modernize crystal syntax and ameba linting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add and run ameba for linting. - Resolve not_nil! and syntax/style issues reported by ameba. - Ignore pedantic .ameba.yml rules like BlockParameterName. - Rename set_database_to_schema to extract_schema_name for accuracy. Co-developed-by: Gemini AI Signed-off-by: Rénich Bon Ćirić --- .ameba.yml | 6 ++++++ shard.yml | 3 +++ spec/migration_spec.cr | 2 +- src/micrate.cr | 28 +++++++++++++--------------- src/micrate/cli.cr | 6 +++--- src/micrate/db.cr | 17 ++++++++--------- src/micrate/migration.cr | 4 ++-- 7 files changed, 36 insertions(+), 30 deletions(-) create mode 100644 .ameba.yml diff --git a/.ameba.yml b/.ameba.yml new file mode 100644 index 0000000..ff0dedb --- /dev/null +++ b/.ameba.yml @@ -0,0 +1,6 @@ +Naming/BlockParameterName: + Enabled: false +Metrics/CyclomaticComplexity: + Enabled: false +Documentation/DocumentationAdmonition: + Enabled: false diff --git a/shard.yml b/shard.yml index 9243197..e981846 100644 --- a/shard.yml +++ b/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 diff --git a/spec/migration_spec.cr b/spec/migration_spec.cr index 25e88fd..da2cfc7 100644 --- a/spec/migration_spec.cr +++ b/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 diff --git a/src/micrate.cr b/src/micrate.cr index c6d3522..639dd33 100644 --- a/src/micrate.cr +++ b/src/micrate.cr @@ -14,13 +14,11 @@ module Micrate 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 + rows = DB.get_versions_last_first_order(db) + extract_dbversion(rows) + rescue Exception + DB.create_migrations_table(db) + 0 end def self.up(db) @@ -32,7 +30,7 @@ module Micrate end current = dbversion(db) - target = all_migrations.keys.sort.last + target = all_migrations.keys.sort!.last migrate(all_migrations, current, target, db) end @@ -85,7 +83,7 @@ module Micrate Dir.mkdir_p dir File.write(filename, migration_template) - return filename + filename end def self.connection_url=(connection_url) @@ -150,7 +148,7 @@ 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 @@ -161,7 +159,7 @@ module Micrate .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,12 +175,12 @@ 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 @@ -204,7 +202,7 @@ module Micrate end end - return 0 + 0 end class UnorderedMigrationsException < Exception diff --git a/src/micrate/cli.cr b/src/micrate/cli.cr index cba6243..1036b79 100644 --- a/src/micrate/cli.cr +++ b/src/micrate/cli.cr @@ -11,7 +11,7 @@ module Micrate File.delete(path) Log.info { "Deleted file #{path}" } else - name = set_database_to_schema url + name = extract_schema_name url Micrate::DB.connect do |db| db.exec "DROP DATABASE IF EXISTS #{name};" end @@ -24,7 +24,7 @@ module Micrate 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 + name = extract_schema_name url Micrate::DB.connect do |db| db.exec "CREATE DATABASE #{name};" end @@ -32,7 +32,7 @@ module Micrate 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}") diff --git a/src/micrate/db.cr b/src/micrate/db.cr index b6606a9..bc86ac1 100644 --- a/src/micrate/db.cr +++ b/src/micrate/db.cr @@ -11,13 +11,11 @@ module Micrate end def self.connect - validate_connection_url - ::DB.connect(self.connection_url.not_nil!) + ::DB.connect(valid_connection_url) end - def self.connect(&block) - validate_connection_url - ::DB.open self.connection_url.not_nil! do |db| + def self.connect(&) + ::DB.open valid_connection_url do |db| yield db end end @@ -50,14 +48,15 @@ module Micrate end private def self.dialect - validate_connection_url - @@dialect ||= Dialect.from_connection_url(self.connection_url.not_nil!) + @@dialect ||= Dialect.from_connection_url(valid_connection_url) end - private def self.validate_connection_url - if !self.connection_url + private def self.valid_connection_url : String + url = self.connection_url + if !url raise "No database connection URL is configured. Please set the DATABASE_URL environment variable." end + url end end end diff --git a/src/micrate/migration.cr b/src/micrate/migration.cr index b474154..823da77 100644 --- a/src/micrate/migration.cr +++ b/src/micrate/migration.cr @@ -77,8 +77,8 @@ module Micrate def self.from_version(version) file_name = Dir.entries(Micrate.migrations_dir) - .find { |name| name.starts_with? version.to_s } - .not_nil! + .find(&.starts_with?(version.to_s)) + self.from_file(file_name) end end From 69055eea9039fe9ce4cdbd92fe25f06d80fd5f33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9nich=20Bon=20=C4=86iri=C4=87?= Date: Thu, 11 Jun 2026 01:11:11 -0600 Subject: [PATCH 2/2] refactor: remove global database state and introduce Micrate::Runner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaced `Micrate::DB` and its global `@@connection_url` and `@@dialect` with `Micrate::Runner` context objects. This encapsulates connection configuration and allows for concurrent programmatic database operations. Signed-off-by: Rénich Bon Ćirić Co-developed-by: Gemini AI --- .gitignore | 1 + README.md | 2 +- examples/micrate | 7 ++- src/micrate.cr | 105 +------------------------------------ src/micrate/cli.cr | 44 +++++++++------- src/micrate/db.cr | 126 +++++++++++++++++++++++++++++++++++++-------- 6 files changed, 141 insertions(+), 144 deletions(-) diff --git a/.gitignore b/.gitignore index 42eb3be..847951b 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ # Libraries don't need dependency lock # Dependencies will be locked in application that uses them /shard.lock +.agents/ diff --git a/README.md b/README.md index e6cf863..6102666 100644 --- a/README.md +++ b/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 ``` diff --git a/examples/micrate b/examples/micrate index 70bc5cd..1267472 100755 --- a/examples/micrate +++ b/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) } diff --git a/src/micrate.cr b/src/micrate.cr index 639dd33..97bf90f 100644 --- a/src/micrate.cr +++ b/src/micrate.cr @@ -1,5 +1,4 @@ require "log" - require "./micrate/*" module Micrate @@ -13,60 +12,6 @@ module Micrate File.join(db_dir, "migrations") end - def self.dbversion(db) - rows = DB.get_versions_last_first_order(db) - extract_dbversion(rows) - rescue Exception - DB.create_migrations_table(db) - 0 - 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") @@ -86,49 +31,6 @@ module Micrate 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 - end - private def self.verify_unordered_migrations(current, status : Hash(Int, Bool)) migrations = status.select { |version, is_applied| !is_applied && version < current } .keys @@ -138,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 @@ -154,7 +56,7 @@ module Micrate 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 } @@ -185,9 +87,6 @@ module Micrate 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 diff --git a/src/micrate/cli.cr b/src/micrate/cli.cr index 1036b79..0a53aa0 100644 --- a/src/micrate/cli.cr +++ b/src/micrate/cli.cr @@ -5,14 +5,15 @@ 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 = extract_schema_name 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}" } @@ -20,12 +21,13 @@ module Micrate 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 = extract_schema_name 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}" } @@ -35,36 +37,41 @@ module Micrate 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 diff --git a/src/micrate/db.cr b/src/micrate/db.cr index bc86ac1..266dff6 100644 --- a/src/micrate/db.cr +++ b/src/micrate/db.cr @@ -2,42 +2,45 @@ require "db" require "./db/*" module Micrate - module DB - class_getter connection_url : String? { ENV["DATABASE_URL"]? } + class Runner + getter connection_url : String - def self.connection_url=(connection_url) - @@dialect = nil - @@connection_url = connection_url + def initialize(connection_url : String? = ENV["DATABASE_URL"]?) + url = connection_url + if !url + raise "No database connection URL is configured. Please set the DATABASE_URL environment variable." + end + @connection_url = url end - def self.connect - ::DB.connect(valid_connection_url) + def connect + ::DB.connect(@connection_url) end - def self.connect(&) - ::DB.open valid_connection_url do |db| + def connect(&) + ::DB.open(@connection_url) do |db| yield db end end - def self.get_versions_last_first_order(db) + def get_versions_last_first_order(db) db.query_all "SELECT version_id, is_applied from micrate_db_version ORDER BY id DESC", as: {Int64, Bool} end - def self.create_migrations_table(db) + def create_migrations_table(db) dialect.query_create_migrations_table(db) end - def self.record_migration(migration, direction, db) + def record_migration(migration, direction, db) is_applied = direction == :forward dialect.query_record_migration(migration, is_applied, db) end - def self.exec(statement, db) + def exec(statement, db) db.exec(statement) end - def self.get_migration_status(migration, db) : Time? + def get_migration_status(migration, db) : Time? rows = dialect.query_migration_status(migration, db) if !rows.empty? && rows[0][1] @@ -47,16 +50,97 @@ module Micrate end end - private def self.dialect - @@dialect ||= Dialect.from_connection_url(valid_connection_url) + private getter dialect : DB::Dialect do + DB::Dialect.from_connection_url(@connection_url) end - private def self.valid_connection_url : String - url = self.connection_url - if !url - raise "No database connection URL is configured. Please set the DATABASE_URL environment variable." + def dbversion(db) + rows = get_versions_last_first_order(db) + Micrate.extract_dbversion(rows) + rescue Exception + create_migrations_table(db) + 0 + end + + def up(db) + all_migrations = Micrate.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 down(db) + all_migrations = Micrate.migrations_by_version + + current = dbversion(db) + target = Micrate.previous_version(current, all_migrations.keys) + migrate(all_migrations, current, target, db) + end + + def redo(db) + all_migrations = Micrate.migrations_by_version + + current = dbversion(db) + previous = Micrate.previous_version(current, all_migrations.keys) + + if migrate(all_migrations, current, previous, db) == :success + migrate(all_migrations, previous, current, db) + end + end + + def migration_status(db) : Hash(Migration, Time?) + # ensure that migration table exists + dbversion(db) + migration_status(Micrate.migrations_by_version.values, db) + end + + def migration_status(migrations : Array(Migration), db) : Hash(Migration, Time?) + ({} of Migration => Time?).tap do |ret| + migrations.each do |m| + ret[m] = get_migration_status(m, db) + end + end + end + + private def migrate(all_migrations : Hash(Int, Migration), current : Int, target : Int, db) + direction = current < target ? :forward : :backwards + + status = migration_status(all_migrations.values, db) + plan = Micrate.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 + + 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 - url + :success end end end