From 1cb31ae863f27d9f9bdee8889c62991b53a5f872 Mon Sep 17 00:00:00 2001 From: gindibay Date: Fri, 2 Jun 2023 18:19:42 +0300 Subject: [PATCH 1/2] Adds initial implementation but still failing --- .../delete_operations.rb | 35 +++++++++++++++++++ lib/activerecord_multi_tenant.rb | 1 + .../query_rewriter_spec.rb | 28 +++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 lib/activerecord-multi-tenant/delete_operations.rb diff --git a/lib/activerecord-multi-tenant/delete_operations.rb b/lib/activerecord-multi-tenant/delete_operations.rb new file mode 100644 index 00000000..3508ad54 --- /dev/null +++ b/lib/activerecord-multi-tenant/delete_operations.rb @@ -0,0 +1,35 @@ +module Arel + module ActiveRecordRelationExtension + def delete_all(conditions = nil) + tenant_id = MultiTenant.current_tenant_id + tenant_key = MultiTenant.partition_key(MultiTenant.current_tenant_class) + + 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}" + end + + puts "stmt klass: #{stmt.class}" + + if conditions + stmt.where(conditions) + end + + puts "stmtt: #{stmt.to_sql}" + klass.connection.delete(stmt, "#{klass} Delete All").tap { reset } + end + end +end + +# Patch ActiveRecord::Relation with the extension module +ActiveRecord::Relation.prepend(Arel::ActiveRecordRelationExtension) diff --git a/lib/activerecord_multi_tenant.rb b/lib/activerecord_multi_tenant.rb index 1b9b606d..09a29a0c 100644 --- a/lib/activerecord_multi_tenant.rb +++ b/lib/activerecord_multi_tenant.rb @@ -11,3 +11,4 @@ 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/query_rewriter_spec.rb b/spec/activerecord-multi-tenant/query_rewriter_spec.rb index 5bc280cb..9c506fea 100644 --- a/spec/activerecord-multi-tenant/query_rewriter_spec.rb +++ b/spec/activerecord-multi-tenant/query_rewriter_spec.rb @@ -45,12 +45,40 @@ let!(:manager1) { Manager.create(name: 'Manager 1', project: project1, account: account) } let!(:manager2) { Manager.create(name: 'Manager 2', project: project2, account: account) } + before(:each) do + @queries = [] + ActiveSupport::Notifications.subscribe('sql.active_record') do |_name, _started, _finished, _unique_id, payload| + @queries << payload[:sql] + end + end + + after(:each) do + ActiveSupport::Notifications.unsubscribe('sql.active_record') + end + it 'delete_all the records' do + + expected_query = <<-SQL.strip + DELETE FROM "projects" WHERE "projects"."id" IN + (SELECT "projects"."id" FROM "projects" + INNER JOIN "managers" ON "managers"."project_id" = "projects"."id" + and "managers"."account_id" = :account_id + WHERE "projects"."account_id" = :account_id + ) + SQL + expect do MultiTenant.with(account) do 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| + 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 it 'delete_all the records without a current tenant' do From 60bfde1ea655189dec7f41fbba59b3adaf19bf21 Mon Sep 17 00:00:00 2001 From: amit909singh Date: Sun, 3 Dec 2023 05:36:32 -0800 Subject: [PATCH 2/2] Users/amit909sin/issue 195 (#219) * Fix the tenant scoping for delete_all --- .../delete_operations.rb | 41 ++++++++++--------- lib/activerecord-multi-tenant/multi_tenant.rb | 2 +- .../query_rewriter_spec.rb | 7 ++-- 3 files changed, 26 insertions(+), 24 deletions(-) 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/multi_tenant.rb b/lib/activerecord-multi-tenant/multi_tenant.rb index 91abba8d..e03008dc 100644 --- a/lib/activerecord-multi-tenant/multi_tenant.rb +++ b/lib/activerecord-multi-tenant/multi_tenant.rb @@ -17,7 +17,7 @@ def self.tenant_klass_defined?(tenant_name, options = {}) end def self.partition_key(tenant_name) - "#{tenant_name}_id" + "#{tenant_name.to_s.underscore}_id" end # rubocop:disable Style/ClassVars 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