From e4fe78c960772c0240b0e2622199961163752292 Mon Sep 17 00:00:00 2001 From: Khash Sajadi Date: Wed, 10 Jan 2024 11:06:07 -0800 Subject: [PATCH] ensure sort works consistently across platforms --- lib/roleback/rule.rb | 12 ++++++------ lib/roleback/rule_book.rb | 10 +++++++++- spec/rule_book_spec.rb | 32 ++++++++++++++++---------------- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/lib/roleback/rule.rb b/lib/roleback/rule.rb index 9d20bce..8ef259a 100644 --- a/lib/roleback/rule.rb +++ b/lib/roleback/rule.rb @@ -50,13 +50,13 @@ def conflicts_with?(rule) # scope_weight = 100 # resource_weight = 10 # outcome_weight = 1 - # scope_value = 0 if scope == ANY, 1 otherwise - # resource_value = 0 if resource == ANY, 1 otherwise - # outcome_value = 0 if outcome == ALLOW, 1 otherwise + # scope_value = 1 if scope == ANY, 2 otherwise + # resource_value = 1 if resource == ANY, 2 otherwise + # outcome_value = 1 if outcome == ALLOW, 2 otherwise def numerical_value - scope_value = @scope == ::Roleback::ANY ? 0 : 1 - resource_value = @resource == ::Roleback::ANY ? 0 : 1 - outcome_value = @outcome == ::Roleback::ALLOW ? 0 : 1 + scope_value = @scope == ::Roleback::ANY ? 1 : 2 + resource_value = @resource == ::Roleback::ANY ? 1 : 2 + outcome_value = @outcome == ::Roleback::ALLOW ? 1 : 2 (scope_value * 100) + (resource_value * 10) + (outcome_value * 1) end diff --git a/lib/roleback/rule_book.rb b/lib/roleback/rule_book.rb index e339f90..7ee3b83 100644 --- a/lib/roleback/rule_book.rb +++ b/lib/roleback/rule_book.rb @@ -96,7 +96,15 @@ def self.sort(rules) # sort the rules rules.sort! do |a, b| - b.numerical_value <=> a.numerical_value + an = a.numerical_value + bn = b.numerical_value + + # if the numerical values are the same, sort by key, otherwise sort by numerical value + if an == bn + a.key <=> b.key + else + bn <=> an + end end end diff --git a/spec/rule_book_spec.rb b/spec/rule_book_spec.rb index 5f9dd8f..29712db 100644 --- a/spec/rule_book_spec.rb +++ b/spec/rule_book_spec.rb @@ -140,7 +140,7 @@ rule1 = ::Roleback::Rule.new(role: role1, resource: resource, scope: scope, action: :show, outcome: ::Roleback::DENY) # any allow - rule2 = ::Roleback::Rule.new(role: role1, resource: ::Roleback::ANY, scope: ::Roleback::ANY, action: :show, outcome: ::Roleback::ALLOW) + rule2 = ::Roleback::Rule.new(role: role1, resource: ::Roleback.any, scope: ::Roleback::any, action: :show, outcome: ::Roleback::ALLOW) rule_book1.add(rule1) rule_book1.add(rule2) @@ -201,23 +201,23 @@ expect(rules.length).to eq(18) - # make sure the rules are sorted correctly based on the following list + # make sure the rules are sorted correctly based on the following values: expect(sorted_rules[0].to_s).to eq('api:/users/work->deny') - expect(sorted_rules[1].to_s).to eq('api:/users/rest->allow') - expect(sorted_rules[2].to_s).to eq('api:/users/create->allow') - expect(sorted_rules[3].to_s).to eq('api:/users/show->allow') - expect(sorted_rules[4].to_s).to eq('api:/users/update->allow') - expect(sorted_rules[5].to_s).to eq('api:/users/delete->allow') - expect(sorted_rules[6].to_s).to eq('api:/users/index->allow') - expect(sorted_rules[7].to_s).to eq('api:/users/new->allow') - expect(sorted_rules[8].to_s).to eq('api:/users/edit->allow') + expect(sorted_rules[1].to_s).to eq('api:/users/create->allow') + expect(sorted_rules[2].to_s).to eq('api:/users/delete->allow') + expect(sorted_rules[3].to_s).to eq('api:/users/edit->allow') + expect(sorted_rules[4].to_s).to eq('api:/users/index->allow') + expect(sorted_rules[5].to_s).to eq('api:/users/new->allow') + expect(sorted_rules[6].to_s).to eq('api:/users/rest->allow') + expect(sorted_rules[7].to_s).to eq('api:/users/show->allow') + expect(sorted_rules[8].to_s).to eq('api:/users/update->allow') expect(sorted_rules[9].to_s).to eq('*:/users/create->allow') - expect(sorted_rules[10].to_s).to eq('*:/users/show->allow') - expect(sorted_rules[11].to_s).to eq('*:/users/update->allow') - expect(sorted_rules[12].to_s).to eq('*:/users/delete->allow') - expect(sorted_rules[13].to_s).to eq('*:/users/index->allow') - expect(sorted_rules[14].to_s).to eq('*:/users/new->allow') - expect(sorted_rules[15].to_s).to eq('*:/users/edit->allow') + expect(sorted_rules[10].to_s).to eq('*:/users/delete->allow') + expect(sorted_rules[11].to_s).to eq('*:/users/edit->allow') + expect(sorted_rules[12].to_s).to eq('*:/users/index->allow') + expect(sorted_rules[13].to_s).to eq('*:/users/new->allow') + expect(sorted_rules[14].to_s).to eq('*:/users/show->allow') + expect(sorted_rules[15].to_s).to eq('*:/users/update->allow') expect(sorted_rules[16].to_s).to eq('*:/users/work->allow') expect(sorted_rules[17].to_s).to eq('*:/*/see->allow') end