Support non-dbo schemas in schema dumper (#1201)

Author: aidanharan

Diff for: CHANGELOG.md

@@ -1,3 +1,9 @@
+## Unreleased
+
+#### Added
+
+- [#](https://github.com/rails-sqlserver/activerecord-sqlserver-adapter/pull/) Support non-dbo schemas in schema dumper.
+
 ## v7.1.4
 
 #### Fixed

Diff for: lib/active_record/connection_adapters/sqlserver/schema_dumper.rb

@@ -39,6 +39,17 @@ def schema_collation(column)
         def default_primary_key?(column)
           super && column.is_identity?
         end
+
+        def schemas(stream)
+          schema_names = @connection.schema_names
+
+          if schema_names.any?
+            schema_names.sort.each do |name|
+              stream.puts "  create_schema #{name.inspect}"
+            end
+            stream.puts
+          end
+        end
       end
     end
   end

Diff for: lib/active_record/connection_adapters/sqlserver/schema_statements.rb

@@ -393,19 +393,37 @@ def drop_schema(schema_name)
           execute "DROP SCHEMA [#{schema_name}]"
         end
 
+        # Returns an array of schema names.
+        def schema_names
+          sql = <<~SQL.squish
+           SELECT name
+            FROM sys.schemas
+            WHERE
+            name NOT LIKE 'db_%' AND
+            name NOT IN ('INFORMATION_SCHEMA', 'sys')
+          SQL
+
+          query_values(sql, "SCHEMA")
+        end
+
         private
 
         def data_source_sql(name = nil, type: nil)
-          scope = quoted_scope name, type: type
+          scope = quoted_scope(name, type: type)
 
-          table_name = lowercase_schema_reflection_sql 'TABLE_NAME'
-          database = scope[:database].present? ? "#{scope[:database]}." : ""
+          table_schema  = lowercase_schema_reflection_sql('TABLE_SCHEMA')
+          table_name    = lowercase_schema_reflection_sql('TABLE_NAME')
+          database      = scope[:database].present? ? "#{scope[:database]}." : ""
           table_catalog = scope[:database].present? ? quote(scope[:database]) : "DB_NAME()"
 
-          sql = "SELECT #{table_name}"
+          sql = "SELECT "
+          sql += " CASE"
+          sql += "  WHEN #{table_schema} = 'dbo' THEN #{table_name}"
+          sql += "  ELSE CONCAT(#{table_schema}, '.', #{table_name})"
+          sql += " END"
           sql += " FROM #{database}INFORMATION_SCHEMA.TABLES WITH (NOLOCK)"
           sql += " WHERE TABLE_CATALOG = #{table_catalog}"
-          sql += " AND TABLE_SCHEMA = #{quote(scope[:schema])}"
+          sql += " AND TABLE_SCHEMA = #{quote(scope[:schema])}" if scope[:schema]
           sql += " AND TABLE_NAME = #{quote(scope[:name])}" if scope[:name]
           sql += " AND TABLE_TYPE = #{quote(scope[:type])}" if scope[:type]
           sql += " ORDER BY #{table_name}"
@@ -414,9 +432,10 @@ def data_source_sql(name = nil, type: nil)
 
         def quoted_scope(name = nil, type: nil)
           identifier = SQLServer::Utils.extract_identifiers(name)
+
           {}.tap do |scope|
             scope[:database] = identifier.database if identifier.database
-            scope[:schema] = identifier.schema || "dbo"
+            scope[:schema] = identifier.schema || "dbo" if name.present?
             scope[:name] = identifier.object if identifier.object
             scope[:type] = type if type
           end

Diff for: test/cases/schema_dumper_test_sqlserver.rb

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "cases/helper_sqlserver"
+require "stringio"
 
 class SchemaDumperTestSQLServer < ActiveRecord::TestCase
   before { all_tables }
@@ -141,7 +142,7 @@ class SchemaDumperTestSQLServer < ActiveRecord::TestCase
   it "honor nonstandard primary keys" do
     generate_schema_for_table("movies") do |output|
       match = output.match(%r{create_table "movies"(.*)do})
-      assert_not_nil(match, "nonstandardpk table not found")
+      assert_not_nil(match, "non-standard primary key table not found")
       assert_match %r(primary_key: "movieid"), match[1], "non-standard primary key not preserved"
     end
   end
@@ -159,14 +160,32 @@ class SchemaDumperTestSQLServer < ActiveRecord::TestCase
     _(output.scan('t.integer "unique_field"').length).must_equal(1)
   end
 
+  it "schemas are dumped and tables names only include non-default schema" do
+    stream = StringIO.new
+    ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection_pool, stream)
+    generated_schema = stream.string
+
+    # Only generate non-default schemas. Default schema is 'dbo'.
+    assert_not_includes generated_schema, 'create_schema "dbo"'
+    assert_includes generated_schema, 'create_schema "test"'
+    assert_includes generated_schema, 'create_schema "test2"'
+
+    # Only non-default schemas should be included in table names. Default schema is 'dbo'.
+    assert_includes generated_schema, 'create_table "accounts"'
+    assert_includes generated_schema, 'create_table "test.aliens"'
+    assert_includes generated_schema, 'create_table "test2.sst_schema_test_multiple_schema"'
+  end
+
   private
 
   def generate_schema_for_table(*table_names)
-    require "stringio"
+    previous_ignore_tables = ActiveRecord::SchemaDumper.ignore_tables
+    ActiveRecord::SchemaDumper.ignore_tables = all_tables - table_names
 
     stream = StringIO.new
     ActiveRecord::SchemaDumper.ignore_tables = all_tables - table_names
-    ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, stream)
+    ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection_pool, stream)
+
     @generated_schema = stream.string
     yield @generated_schema if block_given?
     @schema_lines = Hash.new
@@ -177,6 +196,8 @@ def generate_schema_for_table(*table_names)
       @schema_lines[Regexp.last_match[1]] = SchemaLine.new(line)
     end
     @generated_schema
+  ensure
+    ActiveRecord::SchemaDumper.ignore_tables = previous_ignore_tables
   end
 
   def line(column_name)