From 2f4c3cfa8c8769b6e7edee2808cf898671137b96 Mon Sep 17 00:00:00 2001
From: Clement VILLAIN <choclatefr@gmail.com>
Date: Thu, 23 May 2024 10:15:00 +0200
Subject: [PATCH] Compress duplicates rules (#843)

---
 lib/cancan/rules_compressor.rb      | 18 +++++++++++++
 spec/cancan/rule_compressor_spec.rb | 40 ++++++++++++++++++++++++++---
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/lib/cancan/rules_compressor.rb b/lib/cancan/rules_compressor.rb
index 01b9e325d..46f5a4cbc 100644
--- a/lib/cancan/rules_compressor.rb
+++ b/lib/cancan/rules_compressor.rb
@@ -11,6 +11,7 @@ def initialize(rules)
     end
 
     def compress(array)
+      array = simplify(array)
       idx = array.rindex(&:catch_all?)
       return array unless idx
 
@@ -19,5 +20,22 @@ def compress(array)
         .drop_while { |n| n.base_behavior == value.base_behavior }
         .tap { |a| a.unshift(value) unless value.cannot_catch_all? }
     end
+
+    # If we have A OR (!A AND anything ), then we can simplify to A OR anything
+    # If we have A OR (A OR anything ), then we can simplify to A OR anything
+    # If we have !A AND (A OR something), then we can simplify it to !A AND something
+    # If we have !A AND (!A AND something), then we can simplify it to !A AND something
+    #
+    # So as soon as we see a condition that is the same as the previous one,
+    # we can skip it, no matter of the base_behavior
+    def simplify(rules)
+      seen = Set.new
+      rules.reverse_each.filter_map do |rule|
+        next if seen.include?(rule.conditions)
+
+        seen.add(rule.conditions)
+        rule
+      end.reverse
+    end
   end
 end
diff --git a/spec/cancan/rule_compressor_spec.rb b/spec/cancan/rule_compressor_spec.rb
index 3f643ef24..a2e1d2953 100644
--- a/spec/cancan/rule_compressor_spec.rb
+++ b/spec/cancan/rule_compressor_spec.rb
@@ -87,8 +87,7 @@ def cannot(action, subject, args = nil)
     end
   end
 
-  # TODO: not supported yet
-  xcontext 'duplicate rules' do
+  context 'duplicate rules' do
     let(:rules) do
       [can(:read, Blog, id: 4),
        can(:read, Blog, id: 1),
@@ -99,7 +98,42 @@ def cannot(action, subject, args = nil)
     end
 
     it 'minimizes the rules, by removing duplicates' do
-      expect(described_class.new(rules).rules_collapsed).to eq [rules[0], rules[1], rules[2], rules[4]]
+      expect(described_class.new(rules).rules_collapsed).to eq [rules[0], rules[1], rules[4], rules[5]]
+    end
+  end
+
+  context 'duplicates rules with cannot' do
+    let(:rules) do
+      [can(:read, Blog, id: 1),
+       cannot(:read, Blog, id: 1)]
+    end
+
+    it 'minimizes the rules, by removing useless previous rules' do
+      expect(described_class.new(rules).rules_collapsed).to eq [rules[1]]
+    end
+  end
+
+  context 'duplicates rules with cannot and can again' do
+    let(:rules) do
+      [can(:read, Blog, id: [1, 2]),
+       cannot(:read, Blog, id: 1),
+       can(:read, Blog, id: 1)]
+    end
+
+    it 'minimizes the rules, by removing useless previous rules' do
+      expect(described_class.new(rules).rules_collapsed).to eq [rules[0], rules[2]]
+    end
+  end
+
+  context 'duplicates rules with 2 cannot' do
+    let(:rules) do
+      [can(:read, Blog),
+       cannot(:read, Blog, id: 1),
+       cannot(:read, Blog, id: 1)]
+    end
+
+    it 'minimizes the rules, by removing useless previous rules' do
+      expect(described_class.new(rules).rules_collapsed).to eq [rules[0], rules[2]]
     end
   end