From ecbae9e64712b363d66de84a50e49adde58086b4 Mon Sep 17 00:00:00 2001 From: Boaz Yaniv Date: Tue, 27 Sep 2016 21:14:43 +0900 Subject: [PATCH 1/4] Added basic support for filtering views in Middleware using shared code. --- .../connection_adapters/abstract_adapter.rb | 11 +- .../connection_adapters/postgresql_adapter.rb | 23 +++- spec/data_sources_spec.rb | 103 ++++++++++++++++++ spec/spec_helper.rb | 6 + .../helpers.rb} | 55 ++++------ 5 files changed, 164 insertions(+), 34 deletions(-) create mode 100644 spec/data_sources_spec.rb rename spec/{tables_only_spec.rb => support/helpers.rb} (52%) diff --git a/lib/schema_plus/compatibility/active_record/connection_adapters/abstract_adapter.rb b/lib/schema_plus/compatibility/active_record/connection_adapters/abstract_adapter.rb index da1c89d..8195af4 100644 --- a/lib/schema_plus/compatibility/active_record/connection_adapters/abstract_adapter.rb +++ b/lib/schema_plus/compatibility/active_record/connection_adapters/abstract_adapter.rb @@ -2,11 +2,20 @@ module SchemaPlus::Compatibility module ActiveRecord module ConnectionAdapters module AbstractAdapter + @@internal_tables = [] + @@internal_tables << ::ActiveRecord::InternalMetadata.table_name if defined? ::ActiveRecord::InternalMetadata + def user_tables_only t = tables_only - t.delete ::ActiveRecord::InternalMetadata.table_name if defined? ::ActiveRecord::InternalMetadata + t.delete_if {|table_name| @@internal_tables.include? table_name} t end + + def user_views_only + # Out of the currently supported databases, only PostgreSQL has internal views. + # Postgres can override this function, but the rest of the adapters can just return the views as is. + views if respond_to? :views + end end end end diff --git a/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb b/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb index b26997a..9314927 100644 --- a/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb +++ b/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb @@ -3,12 +3,31 @@ module ActiveRecord module ConnectionAdapters module PostgreSQLAdapter def tables_only - select_values(<<-SQL, "SCHEMA") + _pg_relations(%w{r}) # (r)elation/table + end + + def user_views_only + _pg_relations(%w{v m}, _filter_user_data_sources_sql) # (v)iew, (m)aterialized view + end + + # Filter for user data sources + def _filter_user_data_sources_sql + sql = "AND c.relname NOT LIKE 'pg\\_%'" + sql += " AND schemaname != 'postgis'" if adapter_name == 'PostGIS' + sql + end + + # This private method tries the model the way ActiveRecord queries PostgreSQL relations, but allow for modifications + # by middleware. + def _pg_relations(rel_types, extra_sql = '') + rel_type_query = rel_types.map{|t| "'#{t}'"}.join(',') + select_values(<<-SQL, 'SCHEMA') SELECT c.relname FROM pg_class c LEFT JOIN pg_namespace n ON n.oid = c.relnamespace - WHERE c.relkind IN ('r') -- (r)elation/table + WHERE c.relkind IN (#{rel_type_query}) AND n.nspname = ANY (current_schemas(false)) + #{extra_sql} SQL end end diff --git a/spec/data_sources_spec.rb b/spec/data_sources_spec.rb new file mode 100644 index 0000000..16e7ff5 --- /dev/null +++ b/spec/data_sources_spec.rb @@ -0,0 +1,103 @@ +# encoding: utf-8 +require 'spec_helper' +require 'support/helpers' + +describe 'tables_only' do + let(:connection) { ActiveRecord::Base.connection } + + around(:each) do |example| + create_dummy_data_sources &example + end + + it 'does not cause deprecation' do + expect(ActiveSupport::Deprecation).not_to receive(:warn) + connection.tables_only + end + + let(:ar_internal_tables) { + if ActiveRecord::VERSION::MAJOR >= 5 + [ActiveRecord::InternalMetadata.table_name] + else + [] + end + } + + it 'lists all tables' do + expect(connection.tables_only).to match_array %w[t1 t2] + ar_internal_tables + end + + it "user_tables_only doesn't list internal tables" do + expect(connection.user_tables_only).to match_array %w[t1 t2] + end + +end + +# Only test ActiveRecord versions which support views (5.0+) +if ActiveRecord::Base.connection.respond_to? :views + describe 'user_views_only' do + let(:connection) { ActiveRecord::Base.connection } + + around(:each) do |example| + create_dummy_data_sources &example + end + + if DataSourceHelpers.is_postgresql? + it 'lists all views and materialized views' do + expect(connection.user_views_only).to match_array %w[v1 v2 mv1 mv2] + end + else + it 'lists all views' do + expect(connection.user_views_only).to match_array %w[v1 v2] + end + end + end +end + + +if DataSourceHelpers.is_postgresql? + describe '_pg_relations' do + let(:connection) { ActiveRecord::Base.connection } + + around(:each) do |example| + create_dummy_data_sources &example + end + + it 'lists (r)elations (tables)' do + result = (connection._pg_relations(%w[r])) + expect(result).to include *%w[t1 t2] + expect(result).not_to include *%w[v1 v2] + expect(result).not_to include *%w[mv1 mv2] + end + + it 'lists (v)iews' do + result = (connection._pg_relations(%w[v])) + expect(result).to include *%w[v1 v2] + expect(result).not_to include *%w[mv1 mv2] + expect(result).not_to include *%w[t1 t2] + end + + it 'lists (m)aterialized views' do + result = (connection._pg_relations(%w[m])) + expect(result).to include *%w[mv1 mv2] + expect(result).not_to include *%w[v1 v2] + expect(result).not_to include *%w[t1 t2] + end + + it 'lists (r)elations and (v)iews' do + result = (connection._pg_relations(%w[r v])) + expect(result).to include *%w[t1 t2 v1 v2] + expect(result).not_to include *%w[mv1 mv2] + end + + it 'lists (r)elations, (v)iews and (m)aterialized views' do + result = (connection._pg_relations(%w[r v m])) + expect(result).to include *%w[t1 t2 v1 v2 mv1 mv2] + end + + it 'supports extra SQL code' do + result = (connection._pg_relations(%w[r v m], "AND c.relname NOT LIKE '%2'")) + expect(result).to include *%w[t1 v1 mv1] + expect(result).not_to include *%w[t2 v2 mv2] + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6bd6dcd..3c7da05 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -9,9 +9,15 @@ require 'active_record' require 'schema_plus_compatibility' require 'schema_dev/rspec' +require 'support/helpers' SchemaDev::Rspec.setup +RSpec.configure do |config| + config.include DataSourceHelpers +end + Dir[File.dirname(__FILE__) + "/support/**/*.rb"].each {|f| require f} SimpleCov.command_name "[ruby#{RUBY_VERSION}-activerecord#{::ActiveRecord.version}-#{ActiveRecord::Base.connection.adapter_name}]" + diff --git a/spec/tables_only_spec.rb b/spec/support/helpers.rb similarity index 52% rename from spec/tables_only_spec.rb rename to spec/support/helpers.rb index 92048b6..0734208 100644 --- a/spec/tables_only_spec.rb +++ b/spec/support/helpers.rb @@ -1,19 +1,27 @@ -# encoding: utf-8 -require 'spec_helper' - -describe "tables_only" do - let(:connection) { ActiveRecord::Base.connection } +module DataSourceHelpers + def create_dummy_view(v, t) + drop_dummy_view v + connection.execute "CREATE VIEW #{v} AS SELECT * FROM #{t}" + end def drop_dummy_view(v) connection.execute "DROP VIEW IF EXISTS #{v}" end - def create_dummy_view(v, t) - drop_dummy_view v - connection.execute "CREATE VIEW #{v} AS SELECT * FROM #{t}" + def create_dummy_materialized_view(v, t) + if DataSourceHelpers.is_postgresql? + drop_dummy_materialized_view v + connection.execute "CREATE MATERIALIZED VIEW #{v} AS SELECT * FROM #{t}" + end + end + + def drop_dummy_materialized_view(v) + if DataSourceHelpers.is_postgresql? + connection.execute "DROP MATERIALIZED VIEW IF EXISTS #{v}" + end end - around(:each) do |example| + def create_dummy_data_sources begin connection.tables_only.each do |table| connection.drop_table table, force: :cascade @@ -23,35 +31,20 @@ def create_dummy_view(v, t) connection.create_table :t2 create_dummy_view :v1, :t1 create_dummy_view :v2, :t2 - example.run + create_dummy_materialized_view :mv1, :t1 + create_dummy_materialized_view :mv2, :t2 + yield ensure drop_dummy_view :v1 drop_dummy_view :v2 + drop_dummy_materialized_view :mv1 + drop_dummy_materialized_view :mv2 connection.drop_table :t1, if_exists: true connection.drop_table :t2, if_exists: true end end - it "does not cause deprecation" do - expect(ActiveSupport::Deprecation).not_to receive(:warn) - connection.tables_only - end - - let(:ar_internal_tables) { - if ActiveRecord::VERSION::MAJOR >= 5 - [ActiveRecord::InternalMetadata.table_name] - else - [] - end - } - - it "lists all tables" do - expect(connection.tables_only).to match_array %w[t1 t2] + ar_internal_tables - end - - it "user_tables_only doesn't list internal tables" do - expect(connection.user_tables_only).to match_array %w[t1 t2] + def self.is_postgresql? + ActiveRecord::Base.connection.instance_of? ActiveRecord::ConnectionAdapters::PostgreSQLAdapter end - - end From 9eca9bbbae3e350985cbc65a66f2f9e8e809732f Mon Sep 17 00:00:00 2001 From: Boaz Yaniv Date: Thu, 20 Oct 2016 09:57:02 +0900 Subject: [PATCH 2/4] Added _pg_relations_sql --- .../connection_adapters/postgresql_adapter.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb b/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb index 9314927..e2ec57a 100644 --- a/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb +++ b/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb @@ -17,17 +17,25 @@ def _filter_user_data_sources_sql sql end + def _pg_relations(rel_types, extra_sql = nil) + sql = _pg_relations_sql(rel_types) + if extra_sql + sql << "\n" + sql << extra_sql + end + select_values(sql, 'SCHEMA') + end + # This private method tries the model the way ActiveRecord queries PostgreSQL relations, but allow for modifications # by middleware. - def _pg_relations(rel_types, extra_sql = '') + def _pg_relations_sql(rel_types) rel_type_query = rel_types.map{|t| "'#{t}'"}.join(',') - select_values(<<-SQL, 'SCHEMA') + <<-SQL SELECT c.relname FROM pg_class c LEFT JOIN pg_namespace n ON n.oid = c.relnamespace WHERE c.relkind IN (#{rel_type_query}) AND n.nspname = ANY (current_schemas(false)) - #{extra_sql} SQL end end From dcccb6c6cf41dd8bc7d6eae39e31710685bb1adf Mon Sep 17 00:00:00 2001 From: Boaz Yaniv Date: Sat, 29 Oct 2016 12:57:24 +0900 Subject: [PATCH 3/4] Bumped version --- lib/schema_plus/compatibility/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/schema_plus/compatibility/version.rb b/lib/schema_plus/compatibility/version.rb index bafa049..e705d4f 100644 --- a/lib/schema_plus/compatibility/version.rb +++ b/lib/schema_plus/compatibility/version.rb @@ -1,5 +1,5 @@ module SchemaPlus module Compatibility - VERSION = "0.2.0" + VERSION = "0.3.0" end end From 872f9a4b78cae36340167b13884bbc0654217213 Mon Sep 17 00:00:00 2001 From: Boaz Yaniv Date: Sun, 30 Oct 2016 13:54:15 +0900 Subject: [PATCH 4/4] Removed leading 'AND' from Postgres views SQL filter --- .../active_record/connection_adapters/mysql2_adapter.rb | 2 +- .../connection_adapters/postgresql_adapter.rb | 7 +++---- spec/data_sources_spec.rb | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/schema_plus/compatibility/active_record/connection_adapters/mysql2_adapter.rb b/lib/schema_plus/compatibility/active_record/connection_adapters/mysql2_adapter.rb index 6d7ad0f..0e06c90 100644 --- a/lib/schema_plus/compatibility/active_record/connection_adapters/mysql2_adapter.rb +++ b/lib/schema_plus/compatibility/active_record/connection_adapters/mysql2_adapter.rb @@ -3,7 +3,7 @@ module ActiveRecord module ConnectionAdapters module Mysql2Adapter def tables_only - select_values("SHOW FULL TABLES WHERE table_type != 'VIEW'", "SCHEMA") + select_values("SHOW FULL TABLES WHERE table_type != 'VIEW'", 'SCHEMA') end end end diff --git a/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb b/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb index e2ec57a..c19bd1a 100644 --- a/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb +++ b/lib/schema_plus/compatibility/active_record/connection_adapters/postgresql_adapter.rb @@ -12,16 +12,15 @@ def user_views_only # Filter for user data sources def _filter_user_data_sources_sql - sql = "AND c.relname NOT LIKE 'pg\\_%'" - sql += " AND schemaname != 'postgis'" if adapter_name == 'PostGIS' + sql = "c.relname NOT LIKE 'pg\\_%'" + sql += " AND n.nspname != 'postgis'" if adapter_name == 'PostGIS' sql end def _pg_relations(rel_types, extra_sql = nil) sql = _pg_relations_sql(rel_types) if extra_sql - sql << "\n" - sql << extra_sql + sql << "\nAND (#{extra_sql})" end select_values(sql, 'SCHEMA') end diff --git a/spec/data_sources_spec.rb b/spec/data_sources_spec.rb index 16e7ff5..c18b1e0 100644 --- a/spec/data_sources_spec.rb +++ b/spec/data_sources_spec.rb @@ -95,7 +95,7 @@ end it 'supports extra SQL code' do - result = (connection._pg_relations(%w[r v m], "AND c.relname NOT LIKE '%2'")) + result = (connection._pg_relations(%w[r v m], "c.relname NOT LIKE '%2'")) expect(result).to include *%w[t1 v1 mv1] expect(result).not_to include *%w[t2 v2 mv2] end