More specs

.github/workflows/migration-tests.yml

@@ -111,7 +111,7 @@ jobs:
         if: steps.app-cache.outputs.cache-hit != 'true'
         run: rm -rf tmp/app-cache/uploads && cp -r public/uploads tmp/app-cache/uploads
 
-      - name: Check IntermediateDB schema
+      - name: Validate IntermediateDB schema
         run: migrations/bin/cli schema validate
 
       - name: RSpec

migrations/config/intermediate_db.yml

@@ -3,7 +3,7 @@
 output:
   schema_file: "db/intermediate_db_schema/100-base-schema.sql"
   models_directory: "lib/database/intermediate_db"
-  models_namespace: Migrations::Database::IntermediateDB
+  models_namespace: ::Migrations::Database::IntermediateDB
 
 schema:
   tables:

migrations/lib/database/intermediate_db/upload.rb

@@ -17,76 +17,86 @@ module Migrations::Database::IntermediateDB
       VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)
     SQL
 
-    class << self
-      def create_for_file!(
+    def self.create_for_file!(
+      path:,
+      filename: nil,
+      type: nil,
+      description: nil,
+      origin: nil,
+      user_id: nil
+    )
+      create!(
+        id: ::Migrations::ID.hash(path),
+        filename: filename || File.basename(path),
         path:,
-        filename: nil,
-        type: nil,
-        description: nil,
-        origin: nil,
-        user_id: nil
+        type:,
+        description:,
+        origin:,
+        user_id:,
       )
-        create!(
-          id: ::Migrations::ID.hash(path),
-          filename: filename || File.basename(path),
-          path:,
-          type:,
-          description:,
-          origin:,
-          user_id:,
-        )
-      end
+    end
 
-      def create_for_url!(url:, filename:, type: nil, description: nil, origin: nil, user_id: nil)
-        create!(
-          id: ::Migrations::ID.hash(url),
-          filename:,
-          url:,
-          type:,
-          description:,
-          origin:,
-          user_id:,
-        )
-      end
-
-      def create_for_data!(data:, filename:, type: nil, description: nil, origin: nil, user_id: nil)
-        create!(
-          id: ::Migrations::ID.hash(data),
-          filename:,
-          data: ::Migrations::Database.to_blob(data),
-          type:,
-          description:,
-          origin:,
-          user_id:,
-        )
-      end
-
-      private
-
-      def create!(
-        id:,
+    def self.create_for_url!(
+      url:,
+      filename:,
+      type: nil,
+      description: nil,
+      origin: nil,
+      user_id: nil
+    )
+      create!(
+        id: ::Migrations::ID.hash(url),
         filename:,
-        path: nil,
-        data: nil,
-        url: nil,
-        type: nil,
-        description: nil,
-        origin: nil,
-        user_id: nil
+        url:,
+        type:,
+        description:,
+        origin:,
+        user_id:,
+      )
+    end
+
+    def self.create_for_data!(
+      data:,
+      filename:,
+      type: nil,
+      description: nil,
+      origin: nil,
+      user_id: nil
+    )
+      create!(
+        id: ::Migrations::ID.hash(data),
+        filename:,
+        data: ::Migrations::Database.to_blob(data),
+        type:,
+        description:,
+        origin:,
+        user_id:,
+      )
+    end
+
+    def self.create!(
+      id:,
+      filename:,
+      path: nil,
+      data: nil,
+      url: nil,
+      type: nil,
+      description: nil,
+      origin: nil,
+      user_id: nil
+    )
+      ::Migrations::Database::IntermediateDB.insert(
+        SQL,
+        id,
+        filename,
+        path,
+        data,
+        url,
+        type,
+        description,
+        origin,
+        user_id,
       )
-        ::Migrations::Database::IntermediateDB.insert(
-          SQL,
-          id,
-          filename,
-          path,
-          data,
-          url,
-          type,
-          description,
-          origin,
-          user_id,
-        )
-      end
     end
   end
 end

migrations/lib/database/schema/config_validator.rb

@@ -15,9 +15,11 @@ module Migrations::Database::Schema
       @errors.clear
 
       validate_with_json_schema(config)
-      return if has_errors?
+      return self if has_errors?
 
       validate_config(config)
+
+      self
     end
 
     def has_errors?
@@ -37,25 +39,28 @@ module Migrations::Database::Schema
     end
 
     def validate_config(config)
-      validate_plugins(config)
+      validate_output_config(config)
       validate_schema_config(config)
+      validate_plugins(config)
     end
 
-    def validate_plugins(config)
-      if (plugin_names = config[:plugins]).nil?
-        @errors << "Plugin configuration not found"
-        return
-      end
+    def validate_output_config(config)
+      output_config = config[:output]
 
-      all_plugin_names = Discourse.plugins.map(&:name)
+      schema_file_path = File.dirname(output_config[:schema_file])
+      schema_file_path = File.expand_path(schema_file_path, ::Migrations.root_path)
+      @errors << "Directory of `schema_file` does not exist" if !Dir.exist?(schema_file_path)
 
-      if (additional_plugins = all_plugin_names.difference(plugin_names)).any?
-        @errors << "Additional plugins installed. Uninstall them or add to configuration: #{additional_plugins.join(", ")}"
-      end
+      models_directory = File.expand_path(output_config[:models_directory], ::Migrations.root_path)
+      @errors << "`models_directory` does not exist" if !Dir.exist?(models_directory)
 
-      if (missing_plugins = plugin_names.difference(all_plugin_names)).any?
-        @errors << "Configured plugins not installed: #{missing_plugins.join(", ")}"
-      end
+      existing_namespace =
+        begin
+          Object.const_get(output_config[:models_namespace]).is_a?(Module)
+        rescue NameError
+          false
+        end
+      @errors << "`models_namespace` is not defined" if !existing_namespace
     end
 
     def validate_schema_config(config)
@@ -72,8 +77,8 @@ module Migrations::Database::Schema
 
       schema_config
         .dig(:global, :tables, :exclude)
-        .sort
-        .each do |table_name|
+        &.sort
+        &.each do |table_name|
           if !existing_table_names.delete?(table_name)
             @errors << "Excluded table does not exist: #{table_name}"
           end
@@ -90,5 +95,18 @@ module Migrations::Database::Schema
         @errors << "Table missing from configuration file: #{table_name}"
       end
     end
+
+    def validate_plugins(config)
+      plugin_names = config[:plugins]
+      all_plugin_names = Discourse.plugins.map(&:name)
+
+      if (additional_plugins = all_plugin_names.difference(plugin_names)).any?
+        @errors << "Additional plugins installed. Uninstall them or add to configuration: #{additional_plugins.sort.join(", ")}"
+      end
+
+      if (missing_plugins = plugin_names.difference(all_plugin_names)).any?
+        @errors << "Configured plugins not installed: #{missing_plugins.sort.join(", ")}"
+      end
+    end
   end
 end

migrations/lib/database/schema/loader.rb

@@ -2,8 +2,6 @@
 
 module Migrations::Database::Schema
   class Loader
-    attr_reader :errors
-
     def initialize(schema_config)
       @schema_config = schema_config
       @db = ActiveRecord::Base.connection

migrations/spec/lib/database/intermediate_db/upload_spec.rb

@@ -0,0 +1,5 @@
+# frozen_string_literal: true
+
+RSpec.describe ::Migrations::Database::IntermediateDB::Upload do
+  it_behaves_like "a database entity"
+end

migrations/spec/lib/database/intermediate_db_spec.rb

@@ -94,4 +94,25 @@ RSpec.describe ::Migrations::Database::IntermediateDB do
       end
     end
   end
+
+  it "checks that manually created entities are tested" do
+    Dir[
+      File.join(::Migrations.root_path, "lib", "database", "intermediate_db", "*.rb")
+    ].each do |path|
+      next if File.read(path).include?("auto-generated")
+
+      spec_path =
+        File.join(
+          ::Migrations.root_path,
+          "spec",
+          "lib",
+          "database",
+          "intermediate_db",
+          "#{File.basename(path, ".rb")}_spec.rb",
+        )
+
+      expect(File.exist?(spec_path)).to eq(true), "#{spec_path} is missing"
+      expect(File.read(spec_path)).to include('it_behaves_like "a database entity"')
+    end
+  end
 end

migrations/spec/lib/database/migrator_spec.rb

@@ -9,15 +9,7 @@ RSpec.describe ::Migrations::Database::Migrator do
     ignore_errors: false
   )
     if migrations_directory
-      migrations_path =
-        File.join(
-          ::Migrations.root_path,
-          "spec",
-          "support",
-          "fixtures",
-          "schema",
-          migrations_directory,
-        )
+      migrations_path = File.join(fixture_root, "schema", migrations_directory)
     end
 
     temp_path = storage_path = Dir.mktmpdir if storage_path.nil?

migrations/spec/lib/database/schema/config_validator_spec.rb

@@ -0,0 +1,127 @@
+# frozen_string_literal: true
+
+RSpec.describe ::Migrations::Database::Schema::ConfigValidator do
+  subject(:validator) { described_class.new }
+
+  def minimal_config
+    {
+      output: {
+        schema_file: "db/intermediate_db_schema/100-base-schema.sql",
+        models_directory: "lib/database/intermediate_db",
+        models_namespace: "Migrations::Database::IntermediateDB",
+      },
+      schema: {
+        tables: {
+          users: {
+            columns: {
+              include: %w[id username],
+            },
+          },
+        },
+        global: {
+          columns: {
+          },
+          tables: {
+          },
+        },
+      },
+      plugins: [],
+    }
+  end
+
+  before do
+    allow(ActiveRecord::Base.connection).to receive(:tables).and_return(["users"])
+    allow(ActiveRecord::Base.connection).to receive(:columns).with("users").and_return(
+      [
+        instance_double(
+          ActiveRecord::ConnectionAdapters::PostgreSQL::Column,
+          name: "id",
+          type: :integer,
+        ),
+        instance_double(
+          ActiveRecord::ConnectionAdapters::PostgreSQL::Column,
+          name: "username",
+          type: :string,
+        ),
+      ],
+    )
+  end
+
+  it "validates the minimal config" do
+    expect(validator.validate(minimal_config)).to_not have_errors
+    expect(validator.errors).to be_empty
+  end
+
+  it "validates the config against JSON schema" do
+    expect(validator.validate({})).to have_errors
+    expect(validator.errors).to contain_exactly(
+      "object at root is missing required properties: output, schema, plugins",
+    )
+
+    incomplete_config = minimal_config.except(:plugins)
+    incomplete_config[:schema].except!(:global)
+    expect(validator.validate(incomplete_config)).to have_errors
+    expect(validator.errors).to contain_exactly(
+      "object at `/schema` is missing required properties: global",
+      "object at root is missing required properties: plugins",
+    )
+
+    invalid_config = minimal_config
+    invalid_config[:output][:models_namespace] = 123
+    expect(validator.validate(invalid_config)).to have_errors
+    expect(validator.errors).to contain_exactly(
+      "value at `/output/models_namespace` is not a string",
+    )
+  end
+
+  context "with output config" do
+    it "checks if directory of `schema_file` exists" do
+      config = minimal_config
+      config[:output][:schema_file] = "foo/bar/100-base-schema.sql"
+      expect(validator.validate(config)).to have_errors
+      expect(validator.errors).to contain_exactly("Directory of `schema_file` does not exist")
+    end
+
+    it "checks if `models_directory` exists" do
+      config = minimal_config
+      config[:output][:models_directory] = "foo/bar"
+      expect(validator.validate(config)).to have_errors
+      expect(validator.errors).to contain_exactly("`models_directory` does not exist")
+    end
+
+    it "checks if `models_namespace` is an existing namespace" do
+      config = minimal_config
+      config[:output][:models_namespace] = "::Foo::Bar::IntermediateDB"
+      expect(validator.validate(config)).to have_errors
+      expect(validator.errors).to contain_exactly("`models_namespace` is not defined")
+    end
+  end
+
+  context "with plugins config" do
+    before do
+      allow(Discourse).to receive(:plugins).and_return(
+        [
+          instance_double(Plugin::Instance, name: "footnote"),
+          instance_double(Plugin::Instance, name: "chat"),
+          instance_double(Plugin::Instance, name: "poll"),
+        ],
+      )
+    end
+
+    it "detects if a configured plugin is missing" do
+      config = minimal_config
+      config[:plugins] = %w[foo poll bar chat footnote]
+      expect(validator.validate(config)).to have_errors
+      expect(validator.errors).to contain_exactly("Configured plugins not installed: bar, foo")
+    end
+
+    it "detects if an active plugin isn't configured" do
+      config = minimal_config
+      config[:plugins] = %w[poll]
+      expect(validator.validate(config)).to have_errors
+      expect(validator.errors).to contain_exactly(
+        "Additional plugins installed. Uninstall them or add to configuration: chat, footnote",
+      )
+    end
+  end
+end

migrations/spec/support/helpers.rb

@@ -5,3 +5,7 @@ def reset_memoization(instance, *variables)
     instance.remove_instance_variable(var) if instance.instance_variable_defined?(var)
   end
 end
+
+def fixture_root
+  @fixture_root ||= File.join(::Migrations.root_path, "spec", "support", "fixtures")
+end