diff --git a/.github/workflows/active-record-multi-tenant-tests.yml b/.github/workflows/active-record-multi-tenant-tests.yml index 1d7e09c2..9e5ed6c0 100644 --- a/.github/workflows/active-record-multi-tenant-tests.yml +++ b/.github/workflows/active-record-multi-tenant-tests.yml @@ -51,13 +51,16 @@ jobs: - rails-6.0 - rails-6.1 - rails-7.0 + - rails-7.1 - active-record-6.0 - active-record-6.1 - active-record-7.0 + - active-record-7.1 citus_version: - '10' - '11' - + - '12' + name: Ruby ${{ matrix.ruby }}/${{ matrix.gemfile }} / Citus ${{ matrix.citus_version }} env: APPRAISAL: ${{ matrix.appraisal }} diff --git a/.rubocop.yml b/.rubocop.yml index de3b54a4..cb334d8b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -7,21 +7,29 @@ AllCops: - 'node_modules/**/*' - 'Vagrantfile' TargetRubyVersion: 3.0 + SuggestExtensions: false + NewCops: enable -Style/FrozenStringLiteralComment: +Gemspec/DevelopmentDependencies: Enabled: false -Style/Documentation: - Exclude: - - '**/*.rb' +Lint/ConstantDefinitionInBlock: Enabled: false -Lint/ConstantDefinitionInBlock: +Lint/EmptyBlock: Enabled: false Style/ClassAndModuleChildren: Enabled: false +Style/Documentation: + Exclude: + - '**/*.rb' + Enabled: false + +Style/DocumentDynamicEvalDefinition: + Enabled: false + Metrics/BlockLength: Max: 650 diff --git a/Appraisals b/Appraisals index abac38c4..7b1f9086 100644 --- a/Appraisals +++ b/Appraisals @@ -1,3 +1,5 @@ +# frozen_string_literal: true + appraise 'rails-6.0' do gem 'rails', '~> 6.0.3' end @@ -10,6 +12,10 @@ appraise 'rails-7.0' do gem 'rails', '~> 7.0.0' end +appraise 'rails-7.1' do + gem 'rails', '~> 7.1.0' +end + appraise 'active-record-6.0' do gem 'activerecord', '~> 6.0.3' end @@ -21,3 +27,7 @@ end appraise 'active-record-7.0' do gem 'activerecord', '~> 7.0.0' end + +appraise 'active-record-7.1' do + gem 'activerecord', '~> 7.1.0' +end diff --git a/CHANGELOG.md b/CHANGELOG.md index d5f205c5..bb95949d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,18 @@ # Changelog +## 2.4.0 2023-09-22 +* Adds citus 12 to test matrix (#210) +* Adds Support for rails 7.1 (#208) +* Fix missing scope in habtm.rb (#207) +* Update logic inside the tenant_klass_defined? method (#202) + +## 2.3.0 2023-06-05 +* Adds has_and_belongs_to_many feature with tenant (#193) +* Removes eol ruby versions +* Adds documentation in ReadTheDocs platform (#196) +* Organizes badges in documentation and README.md (#197) +* Wrap ActiveRecord::Base with ActiveSupport.on_load (#199) + ## 2.2.0 2022-12-06 * Handle changing tenant from `nil` to a value [#173](https://github.com/citusdata/activerecord-multi-tenant/pull/173) * Allow Partitioned tables to be created without a primary key [#172](https://github.com/citusdata/activerecord-multi-tenant/pull/172) diff --git a/Gemfile b/Gemfile index fe65166c..3ad7db9f 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + source 'https://rubygems.org' gemspec diff --git a/README.md b/README.md index a3320e8c..34e97d8b 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,5 @@ -# activerecord-multi-tenant [![Build Status](https://github.com/citusdata/activerecord-multi-tenant/actions/workflows/active-record-multi-tenant-tests.yml/badge.svg)](https://github.com/citusdata/activerecord-multi-tenant/actions/workflows/active-record-multi-tenant-tests.yml) [![codecov](https://codecov.io/gh/citusdata/activerecord-multi-tenant/branch/master/graph/badge.svg?token=rw0TsEk4Ld)](https://codecov.io/gh/citusdata/activerecord-multi-tenant) [ ![Gems Version](https://img.shields.io/gem/v/activerecord-multi-tenant.svg)](https://rubygems.org/gems/activerecord-multi-tenant)[ ![Gem Download Count](https://img.shields.io/gem/dt/activerecord-multi-tenant.svg)](https://rubygems.org/gems/activerecord-multi-tenant) [![Documentation Status](https://readthedocs.org/projects/activerecord-multi-tenant/badge/?version=latest)](https://activerecord-multi-tenant.readthedocs.io/en/latest/?badge=latest) +# activerecord-multi-tenant +[![Build Status](https://github.com/citusdata/activerecord-multi-tenant/actions/workflows/active-record-multi-tenant-tests.yml/badge.svg)](https://github.com/citusdata/activerecord-multi-tenant/actions/workflows/active-record-multi-tenant-tests.yml) [![codecov](https://codecov.io/gh/citusdata/activerecord-multi-tenant/branch/master/graph/badge.svg?token=rw0TsEk4Ld)](https://codecov.io/gh/citusdata/activerecord-multi-tenant) [ ![Gems Version](https://img.shields.io/gem/v/activerecord-multi-tenant.svg)](https://rubygems.org/gems/activerecord-multi-tenant)[ ![Gem Download Count](https://img.shields.io/gem/dt/activerecord-multi-tenant.svg)](https://rubygems.org/gems/activerecord-multi-tenant) [![Documentation Status](https://readthedocs.org/projects/activerecord-multi-tenant/badge/?version=latest)](https://activerecord-multi-tenant.readthedocs.io/en/latest/?badge=latest) Introduction Post: https://www.citusdata.com/blog/2017/01/05/easily-scale-out-multi-tenant-apps/ diff --git a/Rakefile b/Rakefile index fb8d39a3..e5ff0a2a 100644 --- a/Rakefile +++ b/Rakefile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'rubygems' require 'bundler/setup' require 'bundler/gem_tasks' diff --git a/activerecord-multi-tenant.gemspec b/activerecord-multi-tenant.gemspec index a90fc884..0811991c 100644 --- a/activerecord-multi-tenant.gemspec +++ b/activerecord-multi-tenant.gemspec @@ -1,18 +1,20 @@ +# frozen_string_literal: true + $LOAD_PATH.push File.expand_path('lib', __dir__) require 'activerecord-multi-tenant/version' Gem::Specification.new do |spec| spec.name = 'activerecord-multi-tenant' spec.version = MultiTenant::VERSION - spec.summary = 'ActiveRecord/Rails integration for multi-tenant databases, '\ - 'in particular the Citus extension for PostgreSQL' + spec.summary = 'ActiveRecord/Rails integration for multi-tenant databases, ' \ + 'in particular the Citus extension for PostgreSQL' spec.description = '' spec.authors = ['Citus Data'] spec.email = 'engage@citusdata.com' spec.required_ruby_version = '>= 3.0.0' + spec.metadata = { 'rubygems_mfa_required' => 'true' } spec.files = `git ls-files`.split("\n") - spec.test_files = `git ls-files -- {spec}/*`.split("\n") spec.require_paths = ['lib'] spec.homepage = 'https://github.com/citusdata/activerecord-multi-tenant' spec.license = 'MIT' diff --git a/docs/requirements.txt b/docs/requirements.txt index 296e53c1..37de70e5 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -8,7 +8,7 @@ alabaster==0.7.13 # via sphinx babel==2.12.1 # via sphinx -certifi==2023.5.7 +certifi==2023.7.22 # via requests charset-normalizer==3.1.0 # via requests @@ -58,5 +58,5 @@ sphinxcontrib-serializinghtml==1.1.5 # via sphinx sphinxnotes-strike==1.2 # via -r requirements.in -urllib3==2.0.2 +urllib3==2.0.7 # via requests diff --git a/lib/activerecord-multi-tenant/copy_from_client.rb b/lib/activerecord-multi-tenant/copy_from_client.rb index 786c7622..8df5b606 100644 --- a/lib/activerecord-multi-tenant/copy_from_client.rb +++ b/lib/activerecord-multi-tenant/copy_from_client.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module MultiTenant # Designed to be mixed into an ActiveRecord model to provide # a copy_from_client method that allows for efficient bulk insertion of diff --git a/lib/activerecord-multi-tenant/delete_operations.rb b/lib/activerecord-multi-tenant/delete_operations.rb index 3508ad54..b2bfbf3d 100644 --- a/lib/activerecord-multi-tenant/delete_operations.rb +++ b/lib/activerecord-multi-tenant/delete_operations.rb @@ -1,31 +1,34 @@ +# frozen_string_literal: true + module Arel module ActiveRecordRelationExtension - def delete_all(conditions = nil) - tenant_id = MultiTenant.current_tenant_id - tenant_key = MultiTenant.partition_key(MultiTenant.current_tenant_class) + # Overrides the delete_all method to include tenant scoping + def delete_all + # Call the original delete_all method if the current tenant is identified by an ID + return super if MultiTenant.current_tenant_is_id? || MultiTenant.current_tenant.nil? + tenant_key = MultiTenant.partition_key(MultiTenant.current_tenant_class) + tenant_id = MultiTenant.current_tenant_id arel = eager_loading? ? apply_join_dependency.arel : build_arel arel.source.left = table - group_values_arel_columns = arel_columns(group_values.uniq) - having_clause_ast = having_clause.ast unless having_clause.empty? - stmt = arel.compile_delete(table[primary_key], having_clause_ast, group_values_arel_columns) - - if tenant_id - tenant_condition = table[tenant_key.downcase].eq(tenant_id) - account_condition = table["account_id"].eq(tenant_id) - conditions = Arel::Nodes::And.new([tenant_condition, conditions].compact) - puts "conditions: #{conditions.to_sql}" - puts "tenant_id: #{tenant_id}" + if tenant_id && klass.column_names.include?(tenant_key) + # Check if the tenant key is present in the model's column names + tenant_condition = table[tenant_key].eq(tenant_id) + # Add the tenant condition to the arel query if it is not already present + unless arel.constraints.any? { |node| node.to_sql.include?(tenant_condition.to_sql) } + arel = arel.where(tenant_condition) + end end - puts "stmt klass: #{stmt.class}" - - if conditions - stmt.where(conditions) - end + subquery = arel.clone + subquery.projections.clear + subquery = subquery.project(table[primary_key]) + in_condition = Arel::Nodes::In.new(table[primary_key], subquery.ast) + stmt = Arel::DeleteManager.new.from(table) + stmt.wheres = [in_condition] - puts "stmtt: #{stmt.to_sql}" + # Execute the delete statement using the connection and return the result klass.connection.delete(stmt, "#{klass} Delete All").tap { reset } end end diff --git a/lib/activerecord-multi-tenant/fast_truncate.rb b/lib/activerecord-multi-tenant/fast_truncate.rb index 27b61708..fdd5d97d 100644 --- a/lib/activerecord-multi-tenant/fast_truncate.rb +++ b/lib/activerecord-multi-tenant/fast_truncate.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Truncates only the tables that have been modified, according to sequence # values # Faster alternative to DatabaseCleaner.clean_with(:truncation, pre_count: true) diff --git a/lib/activerecord-multi-tenant/habtm.rb b/lib/activerecord-multi-tenant/habtm.rb index 8c0edf94..31618451 100644 --- a/lib/activerecord-multi-tenant/habtm.rb +++ b/lib/activerecord-multi-tenant/habtm.rb @@ -8,9 +8,9 @@ module ActiveRecord module Associations module ClassMethods # rubocop:disable Naming/PredicateName - def has_and_belongs_to_many_with_tenant(name, options = {}, &extension) + def has_and_belongs_to_many_with_tenant(name, scope = nil, **options, &extension) # rubocop:enable Naming/PredicateName - has_and_belongs_to_many_without_tenant(name, **options, &extension) + has_and_belongs_to_many_without_tenant(name, scope, **options, &extension) middle_reflection = _reflections[name.to_s].through_reflection join_model = middle_reflection.klass @@ -34,11 +34,10 @@ def has_and_belongs_to_many_with_tenant(name, options = {}, &extension) # This method sets the tenant_id on the join table and executes before creation of the join table record. define_method :tenant_set do - if tenant_enabled - raise MultiTenant::MissingTenantError, 'Tenant Id is not set' unless MultiTenant.current_tenant_id + return unless tenant_enabled + raise MultiTenant::MissingTenantError, 'Tenant Id is not set' unless MultiTenant.current_tenant_id - send("#{tenant_column}=", MultiTenant.current_tenant_id) - end + send("#{tenant_column}=", MultiTenant.current_tenant_id) end end end diff --git a/lib/activerecord-multi-tenant/migrations.rb b/lib/activerecord-multi-tenant/migrations.rb index 237e5202..cd069705 100644 --- a/lib/activerecord-multi-tenant/migrations.rb +++ b/lib/activerecord-multi-tenant/migrations.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module MultiTenant module MigrationExtensions def create_distributed_table(table_name, partition_key) @@ -66,22 +68,19 @@ def citus_version ActiveRecord::Migration.include MultiTenant::MigrationExtensions if defined?(ActiveRecord::Migration) -module ActiveRecord - module ConnectionAdapters # :nodoc: - module SchemaStatements - alias orig_create_table create_table - - def create_table(table_name, options = {}, &block) - ret = orig_create_table(table_name, **options.except(:partition_key), &block) - if options[:id] != false && options[:partition_key] && options[:partition_key].to_s != 'id' - execute "ALTER TABLE #{table_name} DROP CONSTRAINT #{table_name}_pkey" - execute "ALTER TABLE #{table_name} ADD PRIMARY KEY(\"#{options[:partition_key]}\", id)" - end - ret +module MultiTenant + module SchemaStatementsExtensions + def create_table(table_name, options = {}, &block) + ret = super(table_name, **options.except(:partition_key), &block) + if options[:id] != false && options[:partition_key] && options[:partition_key].to_s != 'id' + execute "ALTER TABLE #{table_name} DROP CONSTRAINT #{table_name}_pkey" + execute "ALTER TABLE #{table_name} ADD PRIMARY KEY(\"#{options[:partition_key]}\", id)" end + ret end end end +ActiveRecord::ConnectionAdapters::SchemaStatements.prepend(MultiTenant::SchemaStatementsExtensions) module ActiveRecord class SchemaDumper diff --git a/lib/activerecord-multi-tenant/model_extensions.rb b/lib/activerecord-multi-tenant/model_extensions.rb index 0ec84f98..f2f464a7 100644 --- a/lib/activerecord-multi-tenant/model_extensions.rb +++ b/lib/activerecord-multi-tenant/model_extensions.rb @@ -1,4 +1,6 @@ -require_relative './multi_tenant' +# frozen_string_literal: true + +require_relative 'multi_tenant' module MultiTenant # Extension to the model to allow scoping of models to the current tenant. This is done by adding @@ -6,7 +8,7 @@ module MultiTenant # model declaration. # Adds scoped_by_tenant? partition_key, primary_key and inherited methods to the model module ModelExtensionsClassMethods - DEFAULT_ID_FIELD = 'id'.freeze + DEFAULT_ID_FIELD = 'id' # executes when multi_tenant method is called in the model. This method adds the following # methods to the model that calls it. # scoped_by_tenant? - returns true if the model is scoped by tenant @@ -67,7 +69,7 @@ def inherited(subclass) partition_key = @partition_key # Create an implicit belongs_to association only if tenant class exists - if MultiTenant.tenant_klass_defined?(tenant_name) + if MultiTenant.tenant_klass_defined?(tenant_name, options) belongs_to tenant_name, **options.slice(:class_name, :inverse_of, :optional) .merge(foreign_key: options[:partition_key]) end @@ -76,7 +78,7 @@ def inherited(subclass) after_initialize proc { |record| if MultiTenant.current_tenant_id && (!record.attribute_present?(partition_key) || record.public_send(partition_key.to_sym).nil?) - record.public_send("#{partition_key}=".to_sym, MultiTenant.current_tenant_id) + record.public_send(:"#{partition_key}=", MultiTenant.current_tenant_id) end } @@ -103,7 +105,7 @@ def inherited(subclass) tenant_id end - if MultiTenant.tenant_klass_defined?(tenant_name) + if MultiTenant.tenant_klass_defined?(tenant_name, options) define_method "#{tenant_name}=" do |model| super(model) if send("#{partition_key}_changed?") && persisted? && !send("#{partition_key}_was").nil? @@ -188,17 +190,19 @@ def inherited(subclass) end # skips statement caching for classes that is Multi-tenant or has a multi-tenant relation -class ActiveRecord::Associations::Association - alias skip_statement_cache_orig skip_statement_cache? +module MultiTenant + module AssociationExtensions + def skip_statement_cache?(*scope) + return true if klass.respond_to?(:scoped_by_tenant?) && klass.scoped_by_tenant? - def skip_statement_cache?(*scope) - return true if klass.respond_to?(:scoped_by_tenant?) && klass.scoped_by_tenant? + if reflection.through_reflection + through_klass = reflection.through_reflection.klass + return true if through_klass.respond_to?(:scoped_by_tenant?) && through_klass.scoped_by_tenant? + end - if reflection.through_reflection - through_klass = reflection.through_reflection.klass - return true if through_klass.respond_to?(:scoped_by_tenant?) && through_klass.scoped_by_tenant? + super(*scope) end - - skip_statement_cache_orig(*scope) end end + +ActiveRecord::Associations::Association.prepend(MultiTenant::AssociationExtensions) diff --git a/lib/activerecord-multi-tenant/multi_tenant.rb b/lib/activerecord-multi-tenant/multi_tenant.rb index b2edb592..e03008dc 100644 --- a/lib/activerecord-multi-tenant/multi_tenant.rb +++ b/lib/activerecord-multi-tenant/multi_tenant.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'active_support/current_attributes' module MultiTenant @@ -5,12 +7,17 @@ class Current < ::ActiveSupport::CurrentAttributes attribute :tenant end - def self.tenant_klass_defined?(tenant_name) - !!tenant_name.to_s.classify.safe_constantize + def self.tenant_klass_defined?(tenant_name, options = {}) + class_name = if options[:class_name].present? + options[:class_name] + else + tenant_name.to_s.classify + end + !!class_name.safe_constantize end def self.partition_key(tenant_name) - "#{tenant_name}_id" + "#{tenant_name.to_s.underscore}_id" end # rubocop:disable Style/ClassVars @@ -58,9 +65,9 @@ def self.multi_tenant_model_for_arel(arel) return nil unless arel.respond_to?(:ast) if arel.ast.relation.is_a? Arel::Nodes::JoinSource - MultiTenant.multi_tenant_model_for_table(arel.ast.relation.left.table_name) + MultiTenant.multi_tenant_model_for_table(TableNode.table_name(arel.ast.relation.left)) else - MultiTenant.multi_tenant_model_for_table(arel.ast.relation.table_name) + MultiTenant.multi_tenant_model_for_table(TableNode.table_name(arel.ast.relation)) end end @@ -123,39 +130,21 @@ def self.without(&block) # Wrap calls to any of `method_names` on an instance Class `klass` with MultiTenant.with # when `'owner'` (evaluated in context of the klass instance) is a ActiveRecord model instance that is multi-tenant # Instruments the methods provided with previously set Multitenant parameters - # In Ruby 2 using splat (*) operator with `&block` is not supported, so we need to use `method(...)` syntax # TODO: Could not understand the use of owner here. Need to check - if Gem::Version.create(RUBY_VERSION) < Gem::Version.new('3.0.0') - def self.wrap_methods(klass, owner, *method_names) - method_names.each do |method_name| - original_method_name = :"_mt_original_#{method_name}" - klass.class_eval <<-CODE, __FILE__, __LINE__ + 1 - alias_method :#{original_method_name}, :#{method_name} - def #{method_name}(*args, &block) - if MultiTenant.multi_tenant_model_for_table(#{owner}.class.table_name).present? && #{owner}.persisted? && MultiTenant.current_tenant_id.nil? && #{owner}.class.respond_to?(:partition_key) && #{owner}.attributes.include?(#{owner}.class.partition_key) - MultiTenant.with(#{owner}.public_send(#{owner}.class.partition_key)) { #{original_method_name}(*args, &block) } - else - #{original_method_name}(*args, &block) - end - end - CODE - end - end - else - def self.wrap_methods(klass, owner, *method_names) - method_names.each do |method_name| - original_method_name = :"_mt_original_#{method_name}" - klass.class_eval <<-CODE, __FILE__, __LINE__ + 1 - alias_method :#{original_method_name}, :#{method_name} + def self.wrap_methods(klass, owner, *method_names) + mod = Module.new + klass.prepend(mod) + + method_names.each do |method_name| + mod.module_eval <<-CODE, __FILE__, __LINE__ + 1 def #{method_name}(...) if MultiTenant.multi_tenant_model_for_table(#{owner}.class.table_name).present? && #{owner}.persisted? && MultiTenant.current_tenant_id.nil? && #{owner}.class.respond_to?(:partition_key) && #{owner}.attributes.include?(#{owner}.class.partition_key) - MultiTenant.with(#{owner}.public_send(#{owner}.class.partition_key)) { #{original_method_name}(...) } + MultiTenant.with(#{owner}.public_send(#{owner}.class.partition_key)) { super } else - #{original_method_name}(...) + super end end - CODE - end + CODE end end diff --git a/lib/activerecord-multi-tenant/query_monitor.rb b/lib/activerecord-multi-tenant/query_monitor.rb index 85b555f7..8af77b35 100644 --- a/lib/activerecord-multi-tenant/query_monitor.rb +++ b/lib/activerecord-multi-tenant/query_monitor.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # Add generic warning when queries fail and there is no tenant set # To handle this case, a QueryMonitor hook is created and registered # to sql.active_record. This hook will log a warning when a query fails diff --git a/lib/activerecord-multi-tenant/query_rewriter.rb b/lib/activerecord-multi-tenant/query_rewriter.rb index da1f3d5d..f516ffa1 100644 --- a/lib/activerecord-multi-tenant/query_rewriter.rb +++ b/lib/activerecord-multi-tenant/query_rewriter.rb @@ -1,5 +1,7 @@ +# frozen_string_literal: true + require 'active_record' -require_relative './arel_visitors_depth_first' unless Arel::Visitors.const_defined?(:DepthFirst) +require_relative 'arel_visitors_depth_first' unless Arel::Visitors.const_defined?(:DepthFirst) # Iterates AST and adds tenant enforcement clauses to all relations module MultiTenant @@ -83,7 +85,7 @@ def visit_Arel_Attributes_Attribute(*args) def visit_Arel_Nodes_Equality(obj, *args) if obj.left.is_a?(Arel::Attributes::Attribute) - table_name = obj.left.relation.table_name + table_name = MultiTenant::TableNode.table_name(obj.left.relation) model = MultiTenant.multi_tenant_model_for_table(table_name) if model.present? && obj.left.name.to_s == model.partition_key.to_s @current_context.visited_handled_relation(obj.left.relation) @@ -101,7 +103,7 @@ def visit_MultiTenant_TenantJoinEnforcementClause(obj, *) end def visit_Arel_Table(obj, _collector = nil) - @current_context.visited_relation(obj) if tenant_relation?(obj.table_name) + @current_context.visited_relation(obj) if tenant_relation?(MultiTenant::TableNode.table_name(obj)) end alias visit_Arel_Nodes_TableAlias visit_Arel_Table @@ -179,7 +181,9 @@ class BaseTenantEnforcementClause < Arel::Nodes::Node def initialize(tenant_attribute) super() @tenant_attribute = tenant_attribute - @tenant_model = MultiTenant.multi_tenant_model_for_table(tenant_attribute.relation.table_name) + @tenant_model = MultiTenant.multi_tenant_model_for_table( + MultiTenant::TableNode.table_name(tenant_attribute.relation) + ) end def to_s @@ -215,7 +219,7 @@ class TenantJoinEnforcementClause < BaseTenantEnforcementClause def initialize(tenant_attribute, table_left) super(tenant_attribute) @table_left = table_left - @model_left = MultiTenant.multi_tenant_model_for_table(table_left.table_name) + @model_left = MultiTenant.multi_tenant_model_for_table(MultiTenant::TableNode.table_name(table_left)) end private @@ -241,7 +245,7 @@ def visit_MultiTenant_TenantJoinEnforcementClause(obj, collector) module DatabaseStatements def join_to_update(update, *args) update = super(update, *args) - model = MultiTenant.multi_tenant_model_for_table(update.ast.relation.table_name) + model = MultiTenant.multi_tenant_model_for_table(MultiTenant::TableNode.table_name(update.ast.relation)) if model.present? && !MultiTenant.with_write_only_mode_enabled? && MultiTenant.current_tenant_id.present? update.where(MultiTenant::TenantEnforcementClause.new(model.arel_table[model.partition_key])) end @@ -250,7 +254,7 @@ def join_to_update(update, *args) def join_to_delete(delete, *args) delete = super(delete, *args) - model = MultiTenant.multi_tenant_model_for_table(delete.ast.left.table_name) + model = MultiTenant.multi_tenant_model_for_table(MultiTenant::TableNode.table_name(delete.ast.left)) if model.present? && !MultiTenant.with_write_only_mode_enabled? && MultiTenant.current_tenant_id.present? delete.where(MultiTenant::TenantEnforcementClause.new(model.arel_table[model.partition_key])) end @@ -280,13 +284,10 @@ def delete(arel, name = nil, binds = []) Arel::Visitors::ToSql.include(MultiTenant::TenantValueVisitor) -require 'active_record/relation' -module ActiveRecord - module QueryMethods - alias build_arel_orig build_arel - - def build_arel(*args) - arel = build_arel_orig(*args) +module MultiTenant + module QueryMethodsExtensions + def build_arel(*) + arel = super unless MultiTenant.with_write_only_mode_enabled? visitor = MultiTenant::ArelTenantVisitor.new(arel) @@ -295,7 +296,7 @@ def build_arel(*args) node = context.arel_node context.unhandled_relations.each do |relation| - model = MultiTenant.multi_tenant_model_for_table(relation.arel_table.table_name) + model = MultiTenant.multi_tenant_model_for_table(MultiTenant::TableNode.table_name(relation.arel_table)) if MultiTenant.current_tenant_id enforcement_clause = MultiTenant::TenantEnforcementClause.new(relation.arel_table[model.partition_key]) @@ -330,8 +331,8 @@ def build_arel(*args) next unless relation_right && relation_left - model_right = MultiTenant.multi_tenant_model_for_table(relation_left.table_name) - model_left = MultiTenant.multi_tenant_model_for_table(relation_right.table_name) + model_right = MultiTenant.multi_tenant_model_for_table(MultiTenant::TableNode.table_name(relation_left)) + model_left = MultiTenant.multi_tenant_model_for_table(MultiTenant::TableNode.table_name(relation_right)) next unless model_right && model_left join_enforcement_clause = MultiTenant::TenantJoinEnforcementClause.new( @@ -370,7 +371,9 @@ def relations_from_node_join(node_join) end end -require 'active_record/base' +require 'active_record/relation' +ActiveRecord::QueryMethods.prepend(MultiTenant::QueryMethodsExtensions) + module MultiTenantFindBy def cached_find_by_statement(key, &block) return super unless respond_to?(:scoped_by_tenant?) && scoped_by_tenant? @@ -380,4 +383,6 @@ def cached_find_by_statement(key, &block) end end -ActiveRecord::Base.singleton_class.prepend(MultiTenantFindBy) +ActiveSupport.on_load(:active_record) do |base| + base.singleton_class.prepend(MultiTenantFindBy) +end diff --git a/lib/activerecord-multi-tenant/sidekiq.rb b/lib/activerecord-multi-tenant/sidekiq.rb index 6bbb5d29..0bca5257 100644 --- a/lib/activerecord-multi-tenant/sidekiq.rb +++ b/lib/activerecord-multi-tenant/sidekiq.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'sidekiq/client' # Adds methods to handle tenant information both in the client and server. diff --git a/lib/activerecord-multi-tenant/table_node.rb b/lib/activerecord-multi-tenant/table_node.rb new file mode 100644 index 00000000..9431b06c --- /dev/null +++ b/lib/activerecord-multi-tenant/table_node.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module MultiTenant + module TableNode + # Return table name + def self.table_name(node) + # NOTE: Arel::Nodes::Table#table_name is removed in Rails 7.1 + if node.is_a?(Arel::Nodes::TableAlias) + node.table_name + else + node.name + end + end + end +end diff --git a/lib/activerecord-multi-tenant/version.rb b/lib/activerecord-multi-tenant/version.rb index 480a0194..20299e68 100644 --- a/lib/activerecord-multi-tenant/version.rb +++ b/lib/activerecord-multi-tenant/version.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module MultiTenant - VERSION = '2.2.0'.freeze + VERSION = '2.4.0' end diff --git a/lib/activerecord_multi_tenant.rb b/lib/activerecord_multi_tenant.rb index 2c785152..09a29a0c 100644 --- a/lib/activerecord_multi_tenant.rb +++ b/lib/activerecord_multi_tenant.rb @@ -8,6 +8,7 @@ require_relative 'activerecord-multi-tenant/multi_tenant' require_relative 'activerecord-multi-tenant/query_rewriter' require_relative 'activerecord-multi-tenant/query_monitor' +require_relative 'activerecord-multi-tenant/table_node' require_relative 'activerecord-multi-tenant/version' require_relative 'activerecord-multi-tenant/habtm' require_relative 'activerecord-multi-tenant/delete_operations' diff --git a/spec/activerecord-multi-tenant/associations_spec.rb b/spec/activerecord-multi-tenant/associations_spec.rb index 731e7b58..d74cd24c 100644 --- a/spec/activerecord-multi-tenant/associations_spec.rb +++ b/spec/activerecord-multi-tenant/associations_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe MultiTenant, 'Association methods' do diff --git a/spec/activerecord-multi-tenant/model_extensions_spec.rb b/spec/activerecord-multi-tenant/model_extensions_spec.rb index 2ae94f8a..a49af989 100644 --- a/spec/activerecord-multi-tenant/model_extensions_spec.rb +++ b/spec/activerecord-multi-tenant/model_extensions_spec.rb @@ -131,6 +131,13 @@ def self.name it { expect(@posts).to eq([@post1]) } end + describe 'inspect method filters senstive column values' do + it 'filters senstive value' do + account = Account.new(name: 'foo', password: 'baz') + expect(account.inspect).to eq '#' + end + end + # Scoping models describe 'Project.all should be scoped to the current tenant if set' do before do diff --git a/spec/activerecord-multi-tenant/multi_tenant_spec.rb b/spec/activerecord-multi-tenant/multi_tenant_spec.rb index 0cae7227..f2f9fca9 100644 --- a/spec/activerecord-multi-tenant/multi_tenant_spec.rb +++ b/spec/activerecord-multi-tenant/multi_tenant_spec.rb @@ -4,7 +4,7 @@ RSpec.describe MultiTenant do describe '.load_current_tenant!' do - let(:fake_tenant) { OpenStruct.new(id: 1) } + let(:fake_tenant) { double(id: 1) } let(:mock_klass) { double(find: fake_tenant) } before do @@ -64,4 +64,86 @@ end end end + + describe '.tenant_klass_defined?' do + context 'without options' do + before(:all) do + class SampleTenant < ActiveRecord::Base + multi_tenant :sample_tenant + end + end + + it 'return true with valid tenant_name' do + expect(MultiTenant.tenant_klass_defined?(:sample_tenant)).to eq(true) + end + + it 'return false with invalid_tenant_name' do + invalid_tenant_name = :tenant + expect(MultiTenant.tenant_klass_defined?(invalid_tenant_name)).to eq(false) + end + end + + context 'with options' do + context 'and valid class_name' do + it 'return true' do + class SampleTenant < ActiveRecord::Base + multi_tenant :tenant + end + + tenant_name = :tenant + options = { + class_name: 'SampleTenant' + } + expect(MultiTenant.tenant_klass_defined?(tenant_name, options)).to eq(true) + end + + it 'return true when tenant class is nested' do + module SampleModule + class SampleNestedTenant < ActiveRecord::Base + multi_tenant :tenant + end + # rubocop:disable Layout/TrailingWhitespace + # Trailing whitespace is intentionally left here + + class AnotherTenant < ActiveRecord::Base + end + # rubocop:enable Layout/TrailingWhitespace + end + tenant_name = :tenant + options = { + class_name: 'SampleModule::SampleNestedTenant' + } + expect(MultiTenant.tenant_klass_defined?(tenant_name, options)).to eq(true) + end + end + end + end + + describe '.wrap_methods' do + context 'when method is already prepended' do + it 'is not an stack error' do + klass = Class.new do + def hello + 'hello' + end + end + + klass.prepend(Module.new do + def hello + "#{super} world" + end + + def owner + Class.new(ActiveRecord::Base) do + self.table_name = 'accounts' + end.new + end + end) + + MultiTenant.wrap_methods(klass, :owner, :hello) + + expect(klass.new.hello).to eq('hello world') + end + end + end end diff --git a/spec/activerecord-multi-tenant/query_rewriter_spec.rb b/spec/activerecord-multi-tenant/query_rewriter_spec.rb index 9c506fea..2b6410e3 100644 --- a/spec/activerecord-multi-tenant/query_rewriter_spec.rb +++ b/spec/activerecord-multi-tenant/query_rewriter_spec.rb @@ -57,7 +57,6 @@ end it 'delete_all the records' do - expected_query = <<-SQL.strip DELETE FROM "projects" WHERE "projects"."id" IN (SELECT "projects"."id" FROM "projects" @@ -65,6 +64,7 @@ and "managers"."account_id" = :account_id WHERE "projects"."account_id" = :account_id ) + AND "projects"."account_id" = :account_id SQL expect do @@ -72,12 +72,11 @@ Project.joins(:manager).delete_all end end.to change { Project.count }.from(3).to(1) - query_index = 0 - @queries.each_with_index do |actual_query, index| + + @queries.each do |actual_query| next unless actual_query.include?('DELETE FROM ') expect(format_sql(actual_query)).to eq(format_sql(expected_query.gsub(':account_id', account.id.to_s))) - query_index += 1 end end diff --git a/spec/activerecord-multi-tenant/record_callback_spec.rb b/spec/activerecord-multi-tenant/record_callback_spec.rb index 4a7e64f0..816fb678 100644 --- a/spec/activerecord-multi-tenant/record_callback_spec.rb +++ b/spec/activerecord-multi-tenant/record_callback_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' class ProjectWithCallbacks < ActiveRecord::Base diff --git a/spec/activerecord-multi-tenant/record_finding_spec.rb b/spec/activerecord-multi-tenant/record_finding_spec.rb index 99873d1b..d5b46074 100644 --- a/spec/activerecord-multi-tenant/record_finding_spec.rb +++ b/spec/activerecord-multi-tenant/record_finding_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe MultiTenant, 'Record finding' do diff --git a/spec/activerecord-multi-tenant/sidekiq_spec.rb b/spec/activerecord-multi-tenant/sidekiq_spec.rb index b47dee90..30b9c792 100644 --- a/spec/activerecord-multi-tenant/sidekiq_spec.rb +++ b/spec/activerecord-multi-tenant/sidekiq_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' require 'sidekiq/client' require 'activerecord-multi-tenant/sidekiq' diff --git a/spec/schema.rb b/spec/schema.rb index edc136cb..517cf965 100644 --- a/spec/schema.rb +++ b/spec/schema.rb @@ -9,6 +9,7 @@ t.column :name, :string t.column :subdomain, :string t.column :domain, :string + t.column :password, :string end create_table :projects, force: true, partition_key: :account_id do |t| @@ -173,8 +174,8 @@ class Project < ActiveRecord::Base class Manager < ActiveRecord::Base multi_tenant :account belongs_to :project - has_and_belongs_to_many :tasks, { tenant_column: :account_id, tenant_enabled: true, - tenant_class_name: 'Account' } + has_and_belongs_to_many :tasks, tenant_column: :account_id, tenant_enabled: true, + tenant_class_name: 'Account' end class Task < ActiveRecord::Base diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9ea5c333..28628ff3 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,6 +18,7 @@ add_group 'Lib', '/lib' # Include the lib directory for coverage puts "Tracked files: #{SimpleCov.tracked_files}" end + SimpleCov.minimum_coverage 80 require 'simplecov-cobertura' SimpleCov.formatter = SimpleCov::Formatter::CoberturaFormatter @@ -27,13 +28,29 @@ require 'action_controller/railtie' require 'rspec/rails' +module MultiTenantTest + class Application < Rails::Application; end +end + +# Specifies columns which shouldn't be exposed while calling #inspect. +ActiveSupport.on_load(:active_record) do + self.filter_attributes += MultiTenantTest::Application.config.filter_parameters +end + require 'activerecord_multi_tenant' +# It's necessary for testing the filtering of senstive column values in ActiveRecord. +# Refer to "describe 'inspect method filters senstive column values'" +# +# To verify that ActiveSupport.on_load(:active_record) is not being unnecessarily invoked, +# this line should be placed after "require 'activerecord_multi_tenant'" and before ActiveRecord::Base is called. +MultiTenantTest::Application.config.filter_parameters = [:password] + require 'bundler' Bundler.require(:default, :development) -require_relative './support/format_sql' +require_relative 'support/format_sql' -dbconfig = YAML.safe_load(IO.read(File.join(File.dirname(__FILE__), 'database.yml'))) +dbconfig = YAML.safe_load_file(File.join(File.dirname(__FILE__), 'database.yml')) ActiveRecord::Base.logger = Logger.new(File.join(File.dirname(__FILE__), 'debug.log')) ActiveRecord::Base.establish_connection(dbconfig['test']) @@ -55,13 +72,6 @@ end end -module MultiTenantTest - class Application < Rails::Application; end -end - -MultiTenantTest::Application.config.secret_token = 'x' * 40 -MultiTenantTest::Application.config.secret_key_base = 'y' * 40 - # rubocop:disable Lint/UnusedMethodArgument # changing the name of the parameter breaks tests def with_belongs_to_required_by_default(&block)