Merge pull request #15346 from lpichler/refactor_combine_filtered_ids

Refactor combine_filtered_ids in RBAC

lib/rbac/filterer.rb

@@ -358,50 +358,31 @@ def calc_filtered_ids(scope, user_filters, user, miq_group, scope_tenant_filter)
 
       combine_filtered_ids(u_filtered_ids, b_filtered_ids, m_filtered_ids, d_filtered_ids, scope_tenant_filter.try(:ids))
     end
+
+    #
+    # Algorithm: b_intersection_m        = (b_filtered_ids INTERSECTION m_filtered_ids)
+    #            u_union_d_union_b_and_m = u_filtered_ids UNION d_filtered_ids UNION b_intersection_m
+    #            filter                  = u_union_d_union_b_and_m INTERSECTION tenant_filter_ids
     #
-    # Algorithm: filter = u_filtered_ids UNION (b_filtered_ids INTERSECTION m_filtered_ids)
-    #            filter = (filter UNION d_filtered_ids)
-    #            filter = filter INTERSECTION tenant_filter_ids if tenant_filter_ids is not nil
-    # a nil as input for any field means it does not apply
+    # a nil as input for any field means it DOES NOT apply the operation(INTERSECTION, UNION)
     # a nil as output means there is not filter
     #
     # @param u_filtered_ids [nil|Array<Integer>] self service user owned objects
     # @param b_filtered_ids [nil|Array<Integer>] objects that belong to parent
     # @param m_filtered_ids [nil|Array<Integer>] managed filter object ids
     # @param d_filtered_ids [nil|Array<Integer>] ids from descendants
     # @param tenant_filter_ids [nil|Array<Integer>] ids
-    # @return nil if filters do not aply
+    # @return nil if filters do not apply
     # @return [Array<Integer>] target ids for filter
 
     def combine_filtered_ids(u_filtered_ids, b_filtered_ids, m_filtered_ids, d_filtered_ids, tenant_filter_ids)
-      filtered_ids =
-        if b_filtered_ids.nil?
-          m_filtered_ids
-        elsif m_filtered_ids.nil?
-          b_filtered_ids
-        else
-          b_filtered_ids & m_filtered_ids
-        end
+      intersection = ->(operand1, operand2) { [operand1, operand2].compact.reduce(&:&) }
+      union        = ->(operand1, operand2, operand3 = nil) { [operand1, operand2, operand3].compact.reduce(&:|) }
 
-      if u_filtered_ids.kind_of?(Array)
-        filtered_ids ||= []
-        filtered_ids += u_filtered_ids
-      end
+      b_intersection_m                 = intersection.call(b_filtered_ids, m_filtered_ids)
+      u_union_d_union_b_intersection_m = union.call(u_filtered_ids, d_filtered_ids, b_intersection_m)
 
-      if filtered_ids.kind_of?(Array)
-        filtered_ids += d_filtered_ids if d_filtered_ids.kind_of?(Array)
-        filtered_ids.uniq!
-      elsif d_filtered_ids.kind_of?(Array) && d_filtered_ids.present?
-        filtered_ids = d_filtered_ids
-      end
-
-      if filtered_ids.kind_of?(Array) && tenant_filter_ids
-        filtered_ids & tenant_filter_ids.to_a
-      elsif filtered_ids.nil? && tenant_filter_ids.kind_of?(Array) && tenant_filter_ids.present?
-        tenant_filter_ids
-      else
-        filtered_ids
-      end
+      intersection.call(u_union_d_union_b_intersection_m, tenant_filter_ids)
     end
 
     # @param parent_class [Class] Class of parent (e.g. Host)

spec/lib/rbac/filterer_spec.rb

@@ -1,4 +1,59 @@
 describe Rbac::Filterer do
+  describe '.combine_filtered_ids' do
+    # Algorithm (from Rbac::Filterer.combine_filtered_ids):
+    # b_intersection_m        = (belongsto_filtered_ids INTERSECTION managed_filtered_ids)
+    # u_union_d_union_b_and_m = user_filtered_ids UNION descendant_filtered_ids UNION belongsto_filtered_ids
+    # filter                  = u_union_d_union_b_and_m INTERSECTION tenant_filter_ids
+
+    def combine_filtered_ids(user_filtered_ids, belongsto_filtered_ids, managed_filtered_ids, descendant_filtered_ids, tenant_filter_ids)
+      Rbac::Filterer.new.send(:combine_filtered_ids, user_filtered_ids, belongsto_filtered_ids, managed_filtered_ids, descendant_filtered_ids, tenant_filter_ids)
+    end
+
+    it 'only user filter(self service user)' do
+      expect(combine_filtered_ids([1, 2], nil, nil, nil, nil)).to match_array([1, 2])
+    end
+
+    it 'only belongs to filter(Host & Cluster filter)' do
+      expect(combine_filtered_ids(nil, [1, 2], nil, nil, nil)).to match_array([1, 2])
+    end
+
+    it 'only managed filter(tags)' do
+      expect(combine_filtered_ids(nil, nil, [1, 2], nil, nil)).to match_array([1, 2])
+    end
+
+    it 'only descendants filter' do
+      expect(combine_filtered_ids(nil, nil, nil, [1, 2], nil)).to match_array([1, 2])
+    end
+
+    it 'only tenant filter' do
+      expect(combine_filtered_ids(nil, nil, nil, nil, [1, 2])).to match_array([1, 2])
+    end
+
+    it 'belongs to and tenant filter' do
+      expect(combine_filtered_ids(nil, [1, 2], nil, nil, [2, 3])).to match_array([2])
+    end
+
+    it 'belongs to and managed filters(Host & Cluster filter and tags)' do
+      expect(combine_filtered_ids(nil, [1, 2], [2, 3], nil, nil)).to match_array([2])
+    end
+
+    it 'user filter, belongs to and managed filters(self service user, Host & Cluster filter and tags)' do
+      expect(combine_filtered_ids([1], [2, 3], [3, 4], nil, nil)).to match_array([1, 3])
+    end
+
+    it 'user filter, belongs to, managed filters and descendants filter(self service user, Host & Cluster filter and tags)' do
+      expect(combine_filtered_ids([1], [2, 3], [3, 4], [5, 6], nil)).to match_array([1, 3, 5, 6])
+    end
+
+    it 'user filter, belongs to managed filters, descendants filter and tenant filter(self service user, Host & Cluster filter and tags)' do
+      expect(combine_filtered_ids([1], [2, 3], [3, 4], [5, 6], [1, 6])).to match_array([1, 6])
+    end
+
+    it 'belongs to managed filters, descendants filter and tenant filter(self service user, Host & Cluster filter and tags)' do
+      expect(combine_filtered_ids(nil, [2, 3], [3, 4], [5, 6], [1, 6])).to match_array([6])
+    end
+  end
+
   before { allow(User).to receive_messages(:server_timezone => "UTC") }
 
   let(:default_tenant)     { Tenant.seed }