Fix for latest rails 5.2

See https://github.com/citusdata/activerecord-multi-tenant/pull/116\#issuecomment-1059230711
citusdata · May 13, 2022 · d6af5d7 · d6af5d7
1 parent f200d66
commit d6af5d7
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 14 deletions.
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -47,6 +47,14 @@ jobs:
             gemfile: 'rails_5.2'
           - ruby: '3.1'
             gemfile: 'active_record_5.2'
+          - ruby: '3.0'
+            gemfile: 'rails_5.2.3'
+          - ruby: '3.0'
+            gemfile: 'active_record_5.2.3'
+          - ruby: '3.1'
+            gemfile: 'rails_5.2.3'
+          - ruby: '3.1'
+            gemfile: 'active_record_5.2.3'
     name: Ruby ${{ matrix.ruby }} / ${{ matrix.gemfile }} ${{ (matrix.prepared_statements && 'w/ prepared statements') || '' }}
     env:
        BUNDLE_GEMFILE: gemfiles/${{ matrix.gemfile }}.gemfile

diff --git a/gemfiles/active_record_5.2.3.gemfile b/gemfiles/active_record_5.2.3.gemfile
@@ -0,0 +1,16 @@
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "appraisal"
+gem "activerecord", "~> 5.2.0", "< 5.2.4" # FIXME
+gem "i18n", "~> 0.9.5"
+gem "nokogiri", "~> 1.7.1"
+gem "nio4r", "~> 2.3.1"
+gem "sprockets", "~> 3.7.1"
+gem "byebug", "~> 11.0"
+gem "rake", "12.0.0"
+gem "redis", "3.3.3"
+gem "pry-byebug", "3.9.0"
+
+gemspec path: "../"
diff --git a/gemfiles/active_record_5.2.gemfile b/gemfiles/active_record_5.2.gemfile
@@ -3,7 +3,7 @@
 source "https://rubygems.org"
 
 gem "appraisal"
-gem "activerecord", "~> 5.2.0", "< 5.2.4" # FIXME
+gem "activerecord", "~> 5.2.0"
 gem "i18n", "~> 0.9.5"
 gem "nokogiri", "~> 1.7.1"
 gem "nio4r", "~> 2.3.1"

diff --git a/gemfiles/rails_5.2.3.gemfile b/gemfiles/rails_5.2.3.gemfile
@@ -0,0 +1,16 @@
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "appraisal"
+gem "rails", "~> 5.2.0", "< 5.2.4" # FIXME
+gem "i18n", "~> 0.9.5"
+gem "nokogiri", "~> 1.7.1"
+gem "nio4r", "~> 2.3.1"
+gem "sprockets", "~> 3.7.1"
+gem "byebug", "~> 11.0"
+gem "rake", "12.0.0"
+gem "redis", "3.3.3"
+gem "pry-byebug", "3.9.0"
+
+gemspec path: "../"
diff --git a/gemfiles/rails_5.2.gemfile b/gemfiles/rails_5.2.gemfile
@@ -3,7 +3,7 @@
 source "https://rubygems.org"
 
 gem "appraisal"
-gem "rails", "~> 5.2.0", "< 5.2.4" # FIXME
+gem "rails", "~> 5.2.0"
 gem "i18n", "~> 0.9.5"
 gem "nokogiri", "~> 1.7.1"
 gem "nio4r", "~> 2.3.1"

diff --git a/lib/activerecord-multi-tenant/query_rewriter.rb b/lib/activerecord-multi-tenant/query_rewriter.rb
@@ -295,12 +295,9 @@ def build_arel(*args)
               end
 
               node_list.select{ |n| n.is_a? Arel::Nodes::Join }.each do |node_join|
-                if (!node_join.right ||
-                    (ActiveRecord::VERSION::MAJOR == 5 &&
-                     !node_join.right.expr.right.is_a?(Arel::Attributes::Attribute)))
+                if (!node_join.right)
                   next
                 end
-
                 relation_right, relation_left = relations_from_node_join(node_join)
 
                 next unless relation_right && relation_left
@@ -322,13 +319,13 @@ def build_arel(*args)
 
     private
     def relations_from_node_join(node_join)
-      if ActiveRecord::VERSION::MAJOR == 5 || node_join.right.expr.is_a?(Arel::Nodes::Equality)
+      if node_join.right.expr.is_a?(Arel::Nodes::Equality)
         return node_join.right.expr.right.relation, node_join.right.expr.left.relation
       end
 
-      children = node_join.right.expr.children
+      children = [node_join.right.expr.children].flatten
 
-      tenant_applied = children.any?(MultiTenant::TenantEnforcementClause) || children.any?(MultiTenant::TenantJoinEnforcementClause)
+      tenant_applied = children.any?{|c| c.any?(MultiTenant::TenantEnforcementClause) || c.any?(MultiTenant::TenantJoinEnforcementClause)}
       if tenant_applied || children.empty?
         return nil, nil
       end

diff --git a/spec/activerecord-multi-tenant/model_extensions_spec.rb b/spec/activerecord-multi-tenant/model_extensions_spec.rb
@@ -528,13 +528,13 @@ def self.name
 
     MultiTenant.with(account) do
       option1 = <<-sql.strip
-        SELECT "projects".* FROM "projects" WHERE "projects"."account_id" = #{account.id} AND "projects"."id" = $1 LIMIT $2
+        SELECT "projects".* FROM "projects" WHERE "projects"."account_id" = #{account.id} AND "projects"."id" = #{project.id} LIMIT 1
       sql
       option2 = <<-sql.strip
-        SELECT "projects".* FROM "projects" WHERE "projects"."id" = $1 AND "projects"."account_id" = #{account.id} LIMIT $2
+        SELECT "projects".* FROM "projects" WHERE "projects"."id" = #{project.id} AND "projects"."account_id" = #{account.id} LIMIT 1
       sql
       option3 = <<-sql.strip
-        SELECT  "projects".* FROM "projects" WHERE "projects"."id" = $1 AND "projects"."account_id" = #{account.id} LIMIT $2
+        SELECT  "projects".* FROM "projects" WHERE "projects"."id" = #{project.id} AND "projects"."account_id" = #{account.id} LIMIT 1
       sql
 
       # Couldn't make the following line pass for some reason, so came up with an uglier alternative
@@ -548,10 +548,10 @@ def self.name
 
     MultiTenant.without do
       option1 = <<-sql.strip
-        SELECT "projects".* FROM "projects" WHERE "projects"."id" = $1 LIMIT $2
+        SELECT "projects".* FROM "projects" WHERE "projects"."id" = #{project2.id} LIMIT 1
       sql
       option2 = <<-sql.strip
-        SELECT  "projects".* FROM "projects" WHERE "projects"."id" = $1 LIMIT $2
+        SELECT  "projects".* FROM "projects" WHERE "projects"."id" = #{project2.id} LIMIT 1
       sql
 
       # Couldn't make the following line pass for some reason, so came up with an uglier alternative