Tweak db prepare/migrate/seed for multiple gateways (#234)

Fix a range of issues detected during manual testing with a (soon-to-be) 2.2.beta2 app with multiple DB gateways configured for the app:

- `db prepare` was failing, since the ROM/Sequel migrator needs every configured getaway to have its database created before the migrator can run. To fix this, change the order of the nested `db prepare` commands, so that `db create` and `db structure load` are run first for _every_ detected database. After that is done, move onto `db migrate` for each detected database. Finally, run `db seed`.
- Given that `db prepare` now runs commands in a mixed order (i.e. no longer a single slice at a time), change the handling of command failures such that they halt and exit the entire command immediately. This should make it easier for the user to identify and address failures, and reduces the chance of unexpected outcomes from latter steps being impacted by earlier failures.
- `db seed` should run only once for each slice, but our internal `#databases` method will yield for each database, so when a slice has multiple gateways, we were trying to seed it multiple times. To fix this, track the seeded slices and skip seeding for subsequence detected databases if the slice has already been seeded.
- Various errors were raised from the ROM/Sequel migrator if a gateway is configured but does not yet have a real migrations dir (e.g. `config/db/[gateway_name]_migrate/`). This would occur in both `db migrate` as well as `db structure dump` (which uses the migrator to know the applied migrations to include at the bottom of the structure file). To fix these, skip attempting to load the migrator for a gateway database if there is no matching migration dir found.
- Update the warnings around empty migration paths to respect the naming of migration dirs for gateways.
- Fix bug arising in `db prepare` from us accidentally assigning the `fs` as a DB command's `inflector`.

lib/hanami/cli/commands/app/db/command.rb

@@ -31,7 +31,7 @@ def initialize(
             def run_command(klass, ...)
               klass.new(
                 out: out,
-                inflector: fs,
+                inflector: inflector,
                 fs: fs,
                 system_call: system_call,
               ).call(...)

lib/hanami/cli/commands/app/db/migrate.rb

@@ -12,7 +12,7 @@ class Migrate < DB::Command
             option :gateway, required: false, desc: "Use database for gateway"
             option :target, desc: "Target migration number", aliases: ["-t"]
             option :dump, required: false, type: :boolean, default: true,
-                          desc: "Dump the database structure after migrating"
+              desc: "Dump the database structure after migrating"
 
             def call(target: nil, app: false, slice: nil, gateway: nil, dump: true, command_exit: method(:exit), **)
               databases(app: app, slice: slice, gateway: gateway).each do |database|
@@ -31,6 +31,8 @@ def call(target: nil, app: false, slice: nil, gateway: nil, dump: true, command_
             private
 
             def migrate_database(database, target:)
+              return true unless database.migrations_dir?
+
               measure "database #{database.name} migrated" do
                 if target
                   database.run_migrations(target: Integer(target))
@@ -52,24 +54,22 @@ def no_migrations?(database)
 
             def warn_on_missing_migrations_dir(database)
               out.puts <<~STR
-                WARNING: Database #{database.name} expects migrations to be located within #{relative_migrations_dir(database)} but that folder does not exist.
+                WARNING: Database #{database.name} expects migrations to be located within #{relative_migrations_path(database)} but that folder does not exist.
 
                 No database migrations can be run for this database.
               STR
             end
 
             def warn_on_empty_migrations_dir(database)
               out.puts <<~STR
-                NOTE: Empty database migrations folder (#{relative_migrations_dir(database)}) for #{database.name}
+                NOTE: Empty database migrations folder (#{relative_migrations_path(database)}) for #{database.name}
               STR
             end
 
-            def relative_migrations_dir(database)
+            def relative_migrations_path(database)
               database
-                .slice
-                .root
+                .migrations_path
                 .relative_path_from(database.slice.app.root)
-                .join("config", "db", "migrate")
                 .to_s + "/"
             end
           end

lib/hanami/cli/commands/app/db/prepare.rb

@@ -10,29 +10,55 @@ class Prepare < DB::Command
             desc "Prepare databases"
 
             def call(app: false, slice: nil, **)
-              exit_codes = []
+              command_exit = -> code { throw :command_exited, code }
+              command_exit_arg = {command_exit: command_exit}
 
+              # Since any slice may have multiple databases, we need to run the steps below in a
+              # particular order to satisfy our ROM/Sequel's migrator, which requires _all_ the
+              # databases in a slice to be created before we can use it.
+              #
+              # So before we do anything else, make sure to create/load every database first.
               databases(app: app, slice: slice).each do |database|
-                command_exit = -> code { throw :command_exited, code }
-                command_args = {slice: database.slice, command_exit: command_exit}
+                command_args = {
+                  **command_exit_arg,
+                  app: database.slice.app?,
+                  slice: database.slice,
+                  gateway: database.gateway_name.to_s
+                }
 
                 exit_code = catch :command_exited do
                   unless database.exists?
                     run_command(DB::Create, **command_args)
                     run_command(DB::Structure::Load, **command_args)
                   end
 
-                  run_command(DB::Migrate, **command_args)
-                  run_command(DB::Seed, **command_args)
                   nil
                 end
 
-                exit_codes << exit_code if exit_code
+                return exit exit_code if exit_code.to_i > 1
               end
 
-              exit_codes.each do |code|
-                break exit code if code > 0
+              # Once all databases are created, the migrator will properly load for each slice, and
+              # we can migrate each database.
+              databases(app: app, slice: slice).each do |database|
+                command_args = {
+                  **command_exit_arg,
+                  app: database.slice.app?,
+                  slice: database.slice,
+                  gateway: database.gateway_name.to_s
+                }
+
+                exit_code = catch :command_exited do
+                  run_command(DB::Migrate, **command_args)
+
+                  nil
+                end
+
+                return exit exit_code if exit_code.to_i > 1
               end
+
+              # Finally, load the seeds for the slice overall, which is a once-per-slice operation.
+              run_command(DB::Seed, app: app, slice: slice)
             end
           end
         end

lib/hanami/cli/commands/app/db/seed.rb

@@ -13,14 +13,26 @@ class Seed < DB::Command
             desc "Load seed data"
 
             def call(app: false, slice: nil, **)
+              # We use `databases` below to discover the databases throughout the app and slices. It
+              # yields every database, so in a slice with multiple gateways, we'll see multiple
+              # databases for the slice.
+              #
+              # Since `db seed` is intended to run over whole slices only (not per-gateway), keep
+              # track of the seeded slices here, so we can avoid seeding a slice multiple times.
+              seeded_slices = []
+
               databases(app: app, slice: slice).each do |database|
+                next if seeded_slices.include?(database.slice)
+
                 seeds_path = database.slice.root.join(SEEDS_PATH)
                 next unless seeds_path.file?
 
                 relative_seeds_path = seeds_path.relative_path_from(database.slice.app.root)
                 measure "seed data loaded from #{relative_seeds_path}" do
                   load seeds_path.to_s
                 end
+
+                seeded_slices << database.slice
               end
             end
           end

lib/hanami/cli/commands/app/db/utils/database.rb

@@ -162,6 +162,8 @@ def structure_file
               end
 
               def schema_migrations_sql_dump
+                return unless migrations_dir?
+
                 sql = +"INSERT INTO schema_migrations (filename) VALUES\n"
                 sql << applied_migrations.map { |v| "('#{v}')" }.join(",\n")
                 sql << ";"

spec/unit/hanami/cli/commands/app/db/prepare_spec.rb

@@ -170,12 +170,14 @@ class Comments < Hanami::DB::Relation
 
         expect(output).to include_in_order(
           "database db/app.sqlite3 created",
+          "db/app.sqlite3 structure loaded from config/db/structure.sql",
+          "database db/main.sqlite3 created",
+          "db/main.sqlite3 structure loaded from slices/main/config/db/structure.sql",
           "database db/app.sqlite3 migrated",
           "db/app.sqlite3 structure dumped to config/db/structure.sql",
-          "seed data loaded from config/db/seeds.rb",
-          "database db/main.sqlite3 created",
           "database db/main.sqlite3 migrated",
           "db/main.sqlite3 structure dumped to slices/main/config/db/structure.sql",
+          "seed data loaded from config/db/seeds.rb",
           "seed data loaded from slices/main/config/db/seeds.rb"
         )
       end
@@ -258,12 +260,14 @@ class Comments < Hanami::DB::Relation
 
         expect(output).to include_in_order(
           "database #{POSTGRES_BASE_DB_NAME}_app created",
-          "database #{POSTGRES_BASE_DB_NAME}_app migrated",
-            "#{POSTGRES_BASE_DB_NAME}_app structure dumped to config/db/structure.sql",
-          "seed data loaded from config/db/seeds.rb",
+          "#{POSTGRES_BASE_DB_NAME}_app structure loaded from config/db/structure.sql",
           "database #{POSTGRES_BASE_DB_NAME}_main created",
+          "#{POSTGRES_BASE_DB_NAME}_main structure loaded from slices/main/config/db/structure.sql",
+          "database #{POSTGRES_BASE_DB_NAME}_app migrated",
+          "#{POSTGRES_BASE_DB_NAME}_app structure dumped to config/db/structure.sql",
           "database #{POSTGRES_BASE_DB_NAME}_main migrated",
           "#{POSTGRES_BASE_DB_NAME}_main structure dumped to slices/main/config/db/structure.sql",
+          "seed data loaded from config/db/seeds.rb",
           "seed data loaded from slices/main/config/db/seeds.rb"
         )
       end
@@ -402,32 +406,28 @@ class Comments < Hanami::DB::Relation
       end
     end
 
-    it "prints errors for any prepares that fail and exits with a non-zero status" do
+    it "prints errors for any command that fails and exits immediately with a non-zero status" do
       # Fail to create the app db
       allow(system_call).to receive(:call).and_call_original
       allow(system_call)
         .to receive(:call)
-        .with(a_string_matching(/createdb.+_app/), anything)
-        .and_return Hanami::CLI::SystemCall::Result.new(exit_code: 2, out: "", err: "app-db-err")
+        .with(a_string_matching(/createdb.+_main/), anything)
+        .and_return Hanami::CLI::SystemCall::Result.new(exit_code: 2, out: "", err: "main-db-err")
 
       command.call
 
-      expect(Main::Slice["relations.comments"].to_a).to eq [{id: 1, body: "First comment"}]
-      expect { Hanami.app["relations.posts"].to_a }.to raise_error Sequel::DatabaseError
-
-      dump = File.read(Main::Slice.root.join("config", "db", "structure.sql"))
-      expect(dump).to include("CREATE TABLE public.comments")
-      expect(Hanami.app.root.join("config", "db", "structure.sql").exist?).to be false
+      expect { Hanami.app["relations.posts"] }.to raise_error Sequel::DatabaseError
+      expect { Main::Slice["relations.comments"] }.to raise_error Sequel::DatabaseError
 
       expect(output).to include_in_order(
-        "failed to create database #{POSTGRES_BASE_DB_NAME}_app",
-        "app-db-err",
-        "database #{POSTGRES_BASE_DB_NAME}_main created",
-        "database #{POSTGRES_BASE_DB_NAME}_main migrated",
-          "#{POSTGRES_BASE_DB_NAME}_main structure dumped to slices/main/config/db/structure.sql",
-        "seed data loaded from slices/main/config/db/seeds.rb"
+        "database #{POSTGRES_BASE_DB_NAME}_app created",
+        "failed to create database #{POSTGRES_BASE_DB_NAME}_main",
+        "main-db-err"
       )
 
+      expect(output).not_to include "migrated"
+      expect(output).not_to include "seed data loaded"
+
       expect(command).to have_received(:exit).with 2
     end
   end