Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource#sorted_attributes behaves differently across Ruby versions #3152

Closed
robinboening opened this issue Jan 21, 2025 · 0 comments · Fixed by #3153
Closed

Resource#sorted_attributes behaves differently across Ruby versions #3152

robinboening opened this issue Jan 21, 2025 · 0 comments · Fixed by #3153

Comments

@robinboening
Copy link
Contributor

robinboening commented Jan 21, 2025

Summary

I’ve encountered inconsistent behavior in Resource#sorted_attributes based on the Ruby version being used. This method relies on Array#sort_by, and the sorting results vary with arrays of certain sizes when using different Ruby versions.

Observed Behavior

I tested the behavior with three different Ruby versions and got the following results:

Ruby Version Array Size 15 Array Size 16 Array Size 17 Array Size 20
Ruby 3.2.3 ✅ sorts as expected ✅ sorts as expected ✅ sorts as expected ✅ sorts as expected
Ruby 3.3.1 ✅ sorts as expected ✅ sorts as expected ❌ does not sort as expected ❌ does not sort as expected
Ruby 3.4.1 ✅ sorts as expected ✅ sorts as expected ❌ does not sort as expected ❌ does not sort as expected

Existing RSpec Test Case

The existing RSpec test case for Resource#sorted_attributes is valid, but only includes 4 elements, which does not detect the discrepancy observed with larger arrays. The test case can be found here.

Script to Reproduce

# 20 elements
ar20=[{:name=>"foo", :type=>:string},
 {:name=>"bar", :type=>:text},
 {name: "baz", type: :string},
 {:name=>"foo 2", :type=>:datetime},
 {:name=>"bar 2", :type=>:date},
 {:name=>"baz 2", :type=>:text},
 {:name=>"foo 3", :type=>:text},
 {:name=>"bar 3", :type=>:string},
 {:name=>"baz 3", :type=>:string},
 {:name=>"foo 4", :type=>:string},
 {:name=>"bar 4", :type=>:integer},
 {:name=>"baz 4", :type=>:date},
 {:name=>"foo 5", :type=>:date},
 {:name=>"bar 5", :type=>:date},
 {:name=>"baz 5", :type=>:date},
 {:name=>"foo 6", :type=>:date},
 {:name=>"bar 6", :type=>:date},
 {:name=>"updated_at", :type=>:datetime},
 {:name=>"name", :type=>:string},
 {:name=>"bool", :type=>:boolean}]

ar17 = ar20.drop(3) # 17 elements
ar16 = ar20.drop(4) # 16 elements
ar15 = ar20.drop(5) # 15 elements

# Original method from Alchemy
def sorted_attributes(arr)
  arr.
    sort_by  { |attr| attr[:name] == "name" ? 0 : 1 }.
    sort_by! { |attr| attr[:type] == :boolean ? 1 : 0 }.
    sort_by! { |attr| attr[:name] == "updated_at" ? 1 : 0 }
end

def test_sorted_attributes(*args)
  args.each do |a|
    res = sorted_attributes(a)

    puts "Array with #{res.size} elements"
    if res.first[:name] == "name" && res.last[:name] == "updated_at" && res[-2][:name] == "bool"
      puts "\tValid order"
    else
      puts "\tInvalid order"
    end
  end

  nil # avoid printing the array in console
end

test_sorted_attributes(ar15, ar16, ar17, ar20)

Steps to Reproduce

  1. Run the provided script and switch between the listed Ruby versions.
  2. Compare the results.

Expected Behavior

Resource#sorted_attributes should sort the attributes in a specific order:

  • The name attribute should be listed first.
  • The updated_at attribute should appear at the very last.
  • Boolean type attributes should be listed before updated_at.

The sorting behavior should remain consistent across all Ruby versions, regardless of array size.

Additional Notes

  • It appears the behavior change might be linked to Ruby's internal implementation of Array#sort_by.
  • Not directly related, but it’s worth mentioning that Array#sort_by is known for inconsistencies, as noted in this Ruby issue.
robinboening added a commit to robinboening/alchemy_cms that referenced this issue Jan 21, 2025
Chaining sort_by methods caused issues because each subsequent sort_by effectively overrides the previous sort order by re-sorting the entire array based solely on the new condition. This can disrupt the prioritization of earlier sorting criteria.

Interestingly, chaining multiple sort_by calls has been working as expected in Ruby versions prior to 3.3.1, as the underlying behavior of Array#sort_by appears to have been consistent in preserving the prioritization of earlier steps.

Switching to a single sort_by call ensures the logic remains robust and compatible across Ruby versions, including 3.3.1.

Fixes AlchemyCMS#3152
robinboening added a commit to robinboening/alchemy_cms that referenced this issue Jan 23, 2025
Chaining sort_by methods caused issues because each subsequent sort_by effectively overrides the previous sort order by re-sorting the entire array based solely on the new condition. This can disrupt the prioritization of earlier sorting criteria.

Interestingly, chaining multiple sort_by calls has been working as expected in Ruby versions prior to 3.3.1, as the underlying behavior of Array#sort_by appears to have been consistent in preserving the prioritization of earlier steps.

Switching to a single sort_by call ensures the logic remains robust and compatible across Ruby versions, including 3.3.1.

Fixes AlchemyCMS#3152
alchemycms-bot pushed a commit that referenced this issue Jan 24, 2025
Chaining sort_by methods caused issues because each subsequent sort_by effectively overrides the previous sort order by re-sorting the entire array based solely on the new condition. This can disrupt the prioritization of earlier sorting criteria.

Interestingly, chaining multiple sort_by calls has been working as expected in Ruby versions prior to 3.3.1, as the underlying behavior of Array#sort_by appears to have been consistent in preserving the prioritization of earlier steps.

Switching to a single sort_by call ensures the logic remains robust and compatible across Ruby versions, including 3.3.1.

Fixes #3152

(cherry picked from commit 5d397f5)
alchemycms-bot pushed a commit that referenced this issue Jan 24, 2025
Chaining sort_by methods caused issues because each subsequent sort_by effectively overrides the previous sort order by re-sorting the entire array based solely on the new condition. This can disrupt the prioritization of earlier sorting criteria.

Interestingly, chaining multiple sort_by calls has been working as expected in Ruby versions prior to 3.3.1, as the underlying behavior of Array#sort_by appears to have been consistent in preserving the prioritization of earlier steps.

Switching to a single sort_by call ensures the logic remains robust and compatible across Ruby versions, including 3.3.1.

Fixes #3152

(cherry picked from commit 5d397f5)
alchemycms-bot pushed a commit that referenced this issue Jan 24, 2025
Chaining sort_by methods caused issues because each subsequent sort_by effectively overrides the previous sort order by re-sorting the entire array based solely on the new condition. This can disrupt the prioritization of earlier sorting criteria.

Interestingly, chaining multiple sort_by calls has been working as expected in Ruby versions prior to 3.3.1, as the underlying behavior of Array#sort_by appears to have been consistent in preserving the prioritization of earlier steps.

Switching to a single sort_by call ensures the logic remains robust and compatible across Ruby versions, including 3.3.1.

Fixes #3152

(cherry picked from commit 5d397f5)
alchemycms-bot pushed a commit that referenced this issue Jan 24, 2025
Chaining sort_by methods caused issues because each subsequent sort_by effectively overrides the previous sort order by re-sorting the entire array based solely on the new condition. This can disrupt the prioritization of earlier sorting criteria.

Interestingly, chaining multiple sort_by calls has been working as expected in Ruby versions prior to 3.3.1, as the underlying behavior of Array#sort_by appears to have been consistent in preserving the prioritization of earlier steps.

Switching to a single sort_by call ensures the logic remains robust and compatible across Ruby versions, including 3.3.1.

Fixes #3152

(cherry picked from commit 5d397f5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant