Break Ordering into Ascending/Descending nodes, allow reversal

lib/arel/nodes.rb

@@ -11,6 +11,8 @@
 
 # unary
 require 'arel/nodes/unary'
+require 'arel/nodes/ascending'
+require 'arel/nodes/descending'
 require 'arel/nodes/unqualified_column'
 require 'arel/nodes/with'
 
@@ -19,7 +21,6 @@
 require 'arel/nodes/equality'
 require 'arel/nodes/in' # Why is this subclassed from equality?
 require 'arel/nodes/join_source'
-require 'arel/nodes/ordering'
 require 'arel/nodes/delete_statement'
 require 'arel/nodes/table_alias'
 require 'arel/nodes/infix_operation'

lib/arel/nodes/ascending.rb

+module Arel
+  module Nodes
+    class Ascending < Ordering
+
+      def reverse
+        Descending.new(expr)
+      end
+
+      def direction
+        :asc
+      end
+
+      def ascending?
+        true
+      end
+
+      def descending?
+        false
+      end
+
+    end
+  end
+end

lib/arel/nodes/descending.rb

+module Arel
+  module Nodes
+    class Descending < Ordering
+
+      def reverse
+        Ascending.new(expr)
+      end
+
+      def direction
+        :desc
+      end
+
+      def ascending?
+        false
+      end
+
+      def descending?
+        true
+      end
+
+    end
+  end
+end

lib/arel/nodes/ordering.rb

 module Arel
   module Nodes
-    class Ordering < Arel::Nodes::Binary
-      alias :expr :left
-      alias :direction :right
-
-      def initialize expr, direction = :asc
-        super
-      end
-
-      def ascending?
-        direction == :asc
-      end
-
-      def descending?
-        direction == :desc
-      end
+    class Ordering < Unary
     end
   end
 end

lib/arel/nodes/unary.rb

@@ -18,6 +18,7 @@ def initialize expr
       Not
       Offset
       On
+      Ordering
       Top
       Lock
       DistinctOn

lib/arel/order_predications.rb

@@ -2,11 +2,11 @@ module Arel
   module OrderPredications
 
     def asc
-      Nodes::Ordering.new self, :asc
+      Nodes::Ascending.new self
     end
 
     def desc
-      Nodes::Ordering.new self, :desc
+      Nodes::Descending.new self
     end
 
   end

lib/arel/visitors/depth_first.rb

@@ -22,6 +22,7 @@ def unary o
       alias :visit_Arel_Nodes_Not               :unary
       alias :visit_Arel_Nodes_Offset            :unary
       alias :visit_Arel_Nodes_On                :unary
+      alias :visit_Arel_Nodes_Ordering          :unary
       alias :visit_Arel_Nodes_Top               :unary
       alias :visit_Arel_Nodes_UnqualifiedColumn :unary
 
@@ -75,7 +76,6 @@ def binary o
       alias :visit_Arel_Nodes_NotEqual           :binary
       alias :visit_Arel_Nodes_NotIn              :binary
       alias :visit_Arel_Nodes_Or                 :binary
-      alias :visit_Arel_Nodes_Ordering           :binary
       alias :visit_Arel_Nodes_OuterJoin          :binary
       alias :visit_Arel_Nodes_TableAlias         :binary
       alias :visit_Arel_Nodes_Values             :binary

lib/arel/visitors/dot.rb

@@ -30,7 +30,6 @@ def accept object
       private
       def visit_Arel_Nodes_Ordering o
         visit_edge o, "expr"
-        visit_edge o, "direction"
       end
 
       def visit_Arel_Nodes_TableAlias o

lib/arel/visitors/to_sql.rb

@@ -203,8 +203,12 @@ def visit_Arel_Nodes_Grouping o
         "(#{visit o.expr})"
       end
 
-      def visit_Arel_Nodes_Ordering o
-        "#{visit o.expr} #{o.descending? ? 'DESC' : 'ASC'}"
+      def visit_Arel_Nodes_Ascending o
+        "#{visit o.expr} ASC"
+      end
+
+      def visit_Arel_Nodes_Descending o
+        "#{visit o.expr} DESC"
       end
 
       def visit_Arel_Nodes_Group o

test/attributes/test_attribute.rb

@@ -619,9 +619,9 @@ module Attributes
       end
 
       describe '#asc' do
-        it 'should create an Ordering node' do
+        it 'should create an Ascending node' do
           relation = Table.new(:users)
-          relation[:id].asc.must_be_kind_of Nodes::Ordering
+          relation[:id].asc.must_be_kind_of Nodes::Ascending
         end
 
         it 'should generate ASC in sql' do
@@ -635,9 +635,9 @@ module Attributes
       end
 
       describe '#desc' do
-        it 'should create an Ordering node' do
+        it 'should create a Descending node' do
           relation = Table.new(:users)
-          relation[:id].desc.must_be_kind_of Nodes::Ordering
+          relation[:id].desc.must_be_kind_of Nodes::Descending
         end
 
         it 'should generate DESC in sql' do

test/nodes/test_ascending.rb

+require 'helper'
+
+module Arel
+  module Nodes
+    class TestAscending < MiniTest::Unit::TestCase
+      def test_construct
+        ascending = Ascending.new 'zomg'
+        assert_equal 'zomg', ascending.expr
+      end
+
+      def test_reverse
+        ascending = Ascending.new 'zomg'
+        descending = ascending.reverse
+        assert_kind_of Descending, descending
+        assert_equal ascending.expr, descending.expr
+      end
+
+      def test_direction
+        ascending = Ascending.new 'zomg'
+        assert_equal :asc, ascending.direction
+      end
+
+      def test_ascending?
+        ascending = Ascending.new 'zomg'
+        assert ascending.ascending?
+      end
+
+      def test_descending?
+        ascending = Ascending.new 'zomg'
+        assert !ascending.descending?
+      end
+    end
+  end
+end

test/nodes/test_descending.rb

+require 'helper'
+
+module Arel
+  module Nodes
+    class TestDescending < MiniTest::Unit::TestCase
+      def test_construct
+        descending = Descending.new 'zomg'
+        assert_equal 'zomg', descending.expr
+      end
+
+      def test_reverse
+        descending = Descending.new 'zomg'
+        ascending = descending.reverse
+        assert_kind_of Ascending, ascending
+        assert_equal descending.expr, ascending.expr
+      end
+
+      def test_direction
+        descending = Descending.new 'zomg'
+        assert_equal :desc, descending.direction
+      end
+
+      def test_ascending?
+        descending = Descending.new 'zomg'
+        assert !descending.ascending?
+      end
+
+      def test_descending?
+        descending = Descending.new 'zomg'
+        assert descending.descending?
+      end
+    end
+  end
+end

test/nodes/test_infix_operation.rb

@@ -21,9 +21,9 @@ def test_operation_alias
       def test_opertaion_ordering
         operation = InfixOperation.new :+, 1, 2
         ordering = operation.desc
-        assert_kind_of Ordering, ordering
+        assert_kind_of Descending, ordering
         assert_equal operation, ordering.expr
-        assert_equal :desc, ordering.direction
+        assert ordering.descending?
       end
     end
   end

test/visitors/test_depth_first.rb

@@ -28,6 +28,7 @@ def test_raises_with_object
         Arel::Nodes::On,
         Arel::Nodes::Grouping,
         Arel::Nodes::Offset,
+        Arel::Nodes::Ordering,
         Arel::Nodes::Having,
         Arel::Nodes::StringJoin,
         Arel::Nodes::UnqualifiedColumn,
@@ -104,7 +105,6 @@ def test_outer_join
         Arel::Nodes::Values,
         Arel::Nodes::As,
         Arel::Nodes::DeleteStatement,
-        Arel::Nodes::Ordering,
         Arel::Nodes::JoinSource,
       ].each do |klass|
         define_method("test_#{klass.name.gsub('::', '_')}") do

test/visitors/test_dot.rb

@@ -33,6 +33,7 @@ def test_named_function
         Arel::Nodes::On,
         Arel::Nodes::Grouping,
         Arel::Nodes::Offset,
+        Arel::Nodes::Ordering,
         Arel::Nodes::Having,
         Arel::Nodes::UnqualifiedColumn,
         Arel::Nodes::Top,
@@ -63,7 +64,6 @@ def test_named_function
         Arel::Nodes::Values,
         Arel::Nodes::As,
         Arel::Nodes::DeleteStatement,
-        Arel::Nodes::Ordering,
         Arel::Nodes::JoinSource,
       ].each do |klass|
         define_method("test_#{klass.name.gsub('::', '_')}") do