<=> operator should return nil when invoked with an incomparable object (#38772).

Patch by Go MAEDA.

git-svn-id: https://svn.redmine.org/redmine/trunk@22269 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb
index 24300c6d2db50099392cf165d7116bba7afccbc4..be9cf550f57b3f3f26ec1b864ec914e0c0639161 100644 (file)
--- a/app/models/custom_field.rb
+++ b/app/models/custom_field.rb
@@ -263,6 +263,8 @@ class CustomField < ActiveRecord::Base
   end
 
   def <=>(field)
+    return nil unless field.is_a?(CustomField)
+
     position <=> field.position
   end
 

diff --git a/app/models/enumeration.rb b/app/models/enumeration.rb
index 762cddc9a10ffdd7273a5763d6bb38968b89bc6a..f2f63372a8b26793ed4342335020915e0ef2f0f5 100644 (file)
--- a/app/models/enumeration.rb
+++ b/app/models/enumeration.rb
@@ -91,6 +91,8 @@ class Enumeration < ActiveRecord::Base
   end
 
   def <=>(enumeration)
+    return nil unless enumeration.is_a?(Enumeration)
+
     position <=> enumeration.position
   end
 

diff --git a/app/models/issue.rb b/app/models/issue.rb
index d6d6c7c8b2430f05189af12cc5ebf7de82c198e2..e23b02647a8e25d778a498c4044b9ecaa8fb39eb 100644 (file)
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -1435,9 +1435,9 @@ class Issue < ActiveRecord::Base
   end
 
   def <=>(issue)
-    if issue.nil?
-      -1
-    elsif root_id != issue.root_id
+    return nil unless issue.is_a?(Issue)
+
+    if root_id != issue.root_id
       (root_id || 0) <=> (issue.root_id || 0)
     else
       (lft || 0) <=> (issue.lft || 0)

diff --git a/app/models/issue_category.rb b/app/models/issue_category.rb
index 626bb1449875506dc5a8c6f2b1cafe7cb0ceaae2..eadb44891f2e042041a172245fdd0506101d8a0b 100644 (file)
--- a/app/models/issue_category.rb
+++ b/app/models/issue_category.rb
@@ -43,6 +43,8 @@ class IssueCategory < ActiveRecord::Base
   end
 
   def <=>(category)
+    return nil unless category.is_a?(IssueCategory)
+
     name <=> category.name
   end
 

diff --git a/app/models/issue_relation.rb b/app/models/issue_relation.rb
index f33b755688a3f382aa1da7336ffae5262185e81d..2caaca9badd9a3a366093636cd5299ead8b44602 100644 (file)
--- a/app/models/issue_relation.rb
+++ b/app/models/issue_relation.rb
@@ -198,6 +198,8 @@ class IssueRelation < ActiveRecord::Base
   end
 
   def <=>(relation)
+    return nil unless relation.is_a?(IssueRelation)
+
     r = TYPES[self.relation_type][:order] <=> TYPES[relation.relation_type][:order]
     r == 0 ? id <=> relation.id : r
   end

diff --git a/app/models/issue_status.rb b/app/models/issue_status.rb
index c41e7a739963534a67ccbb6da6b9ef2965c4e08c..3080fea8a3cad557ca3da8ad27433b53616f01cd 100644 (file)
--- a/app/models/issue_status.rb
+++ b/app/models/issue_status.rb
@@ -82,6 +82,8 @@ class IssueStatus < ActiveRecord::Base
   end
 
   def <=>(status)
+    return nil unless status.is_a?(IssueStatus)
+
     position <=> status.position
   end
 

diff --git a/app/models/member.rb b/app/models/member.rb
index 21c62340862cad17460aceca64dd7ee942560999..9e623d5f30bedbbbb7e5b7b4254ba388b1971d27 100644 (file)
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -81,6 +81,8 @@ class Member < ActiveRecord::Base
   end
 
   def <=>(member)
+    return nil unless member.is_a?(Member)
+
     a, b = roles.sort, member.roles.sort
     if a == b
       if principal

diff --git a/app/models/principal.rb b/app/models/principal.rb
index e62b5f320ea9b6472d1894ff94951693d42c52c6..4cce97e26f997827aa96615ef42ad82dc5952bd4 100644 (file)
--- a/app/models/principal.rb
+++ b/app/models/principal.rb
@@ -151,9 +151,11 @@ class Principal < ActiveRecord::Base
   end
 
   def <=>(principal)
-    if principal.nil?
-      -1
-    elsif self.class.name == principal.class.name
+    # avoid an error when sorting members without roles (#10053)
+    return -1 if principal.nil?
+    return nil unless principal.is_a?(Principal)
+
+    if self.class.name == principal.class.name
       self.to_s.casecmp(principal.to_s)
     else
       # groups after users

diff --git a/app/models/project.rb b/app/models/project.rb
index 5f45896fce0e5ecce5fb9a6aba0dfacd78dafe3f..11f4a953da2475b9cef0ba26e74e1501bdc93f45 100644 (file)
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -671,6 +671,8 @@ class Project < ActiveRecord::Base
   end
 
   def <=>(project)
+    return nil unless project.is_a?(Project)
+
     name.casecmp(project.name)
   end
 

diff --git a/app/models/repository.rb b/app/models/repository.rb
index be10fa3cfa05a7937b306ac721f6e55594b38d34..bc570f2f8d853a93752218b491d0da2edcfb8800 100644 (file)
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -141,6 +141,8 @@ class Repository < ActiveRecord::Base
   end
 
   def <=>(repository)
+    return nil unless repository.is_a?(Repository)
+
     if is_default?
       -1
     elsif repository.is_default?

diff --git a/app/models/role.rb b/app/models/role.rb
index 604bcd71275f39007582bf60824dc5faaf11ed3a..078419c607c0e71cec9c9fe2f02d6d6ec312c8b9 100644 (file)
--- a/app/models/role.rb
+++ b/app/models/role.rb
@@ -155,14 +155,14 @@ class Role < ActiveRecord::Base
   end
 
   def <=>(role)
-    if role
-      if builtin == role.builtin
-        position <=> role.position
-      else
-        builtin <=> role.builtin
-      end
+    # returns -1 for nil since r2726
+    return -1 if role.nil?
+    return nil unless role.is_a?(Role)
+
+    if builtin == role.builtin
+      position <=> role.position
     else
-      -1
+      builtin <=> role.builtin
     end
   end
 

diff --git a/app/models/tracker.rb b/app/models/tracker.rb
index bfec75e70b12762da574a1a1d9ecbc16b2b3f8a6..ef2f48b04bdd9f66355374500bcb99847e20df2e 100644 (file)
--- a/app/models/tracker.rb
+++ b/app/models/tracker.rb
@@ -93,6 +93,8 @@ class Tracker < ActiveRecord::Base
   def to_s; name end
 
   def <=>(tracker)
+    return nil unless tracker.is_a?(Tracker)
+
     position <=> tracker.position
   end
 

diff --git a/app/models/version.rb b/app/models/version.rb
index f5ac64b745cc090f6db9b69f942f3fe649edb6ee..77228826ceb21279d03e7338de2ea6492bbe0852 100644 (file)
--- a/app/models/version.rb
+++ b/app/models/version.rb
@@ -316,6 +316,8 @@ class Version < ActiveRecord::Base
   # Versions are sorted by effective_date and name
   # Those with no effective_date are at the end, sorted by name
   def <=>(version)
+    return nil unless version.is_a?(Version)
+
     if self.effective_date
       if version.effective_date
         if self.effective_date == version.effective_date

diff --git a/lib/redmine/plugin.rb b/lib/redmine/plugin.rb
index b7f2433543f60f88aaec52e8cdbd2081016608b8..79e310f3194a1220ed84ccb805932cb1b17f3551 100644 (file)
--- a/lib/redmine/plugin.rb
+++ b/lib/redmine/plugin.rb
@@ -187,6 +187,8 @@ module Redmine
     end
 
     def <=>(plugin)
+      return nil unless plugin.is_a?(Plugin)
+
       self.id.to_s <=> plugin.id.to_s
     end
 

diff --git a/lib/redmine/themes.rb b/lib/redmine/themes.rb
index a73184090eeecf5c31d941919f41292a1439396e..ff51f00832a5c56c89d7857c7127a2880cb70040 100644 (file)
--- a/lib/redmine/themes.rb
+++ b/lib/redmine/themes.rb
@@ -61,6 +61,8 @@ module Redmine
       end
 
       def <=>(theme)
+        return nil unless theme.is_a?(Theme)
+
         name <=> theme.name
       end
 

diff --git a/test/unit/enumeration_test.rb b/test/unit/enumeration_test.rb
index 25dfb14c490c0026c0579480fb38272622dc44d4..a9d0e8eb33a8f4453bdd3545922554c14de8af50 100644 (file)
--- a/test/unit/enumeration_test.rb
+++ b/test/unit/enumeration_test.rb
@@ -179,4 +179,10 @@ class EnumerationTest < ActiveSupport::TestCase
     override.destroy
     assert_equal [1, 2, 3], [a, b, c].map(&:reload).map(&:position)
   end
+
+  def test_spaceship_operator_with_incomparable_value_should_return_nil
+    e = Enumeration.first
+    assert_nil e <=> nil
+    assert_nil e <=> 'foo'
+  end
 end