Abstract range tests behind new class

The RelativePositioning::Range class is reponsible for inferring
the other end of ranges, testing if the range is open and detecting
if an object is contained within the range (allowing us to avoid some
operations).

lib/gitlab/relative_positioning/item_context.rb

@@ -18,6 +18,10 @@ module Gitlab
         @ignoring = ignoring
       end
 
+      def positioned?
+        relative_position.present?
+      end
+
       def min_relative_position
         strong_memoize(:min_relative_position) { calculate_relative_position('MIN') }
       end

lib/gitlab/relative_positioning/mover.rb

@@ -25,25 +25,23 @@ module Gitlab
       end
 
       def move(object, first, last)
-        raise ArgumentError unless object && (first || last) && (first != last)
-        # Moving a object next to itself is a no-op
-        return if object == first || object == last
+        raise ArgumentError, 'object is required' unless object
 
         lhs = context(first, ignoring: object)
         rhs = context(last, ignoring: object)
         focus = context(object)
-
-        lhs ||= rhs.lhs_neighbour
-        rhs ||= lhs.rhs_neighbour
-
-        if lhs.nil?
-          move_to_range_start(focus, rhs.relative_position)
-        elsif rhs.nil?
-          move_to_range_end(focus, lhs.relative_position)
+        range = RelativePositioning.range(lhs, rhs)
+
+        if range.cover?(focus)
+          # Moving a object already within a range is a no-op
+        elsif range.open_on_left?
+          move_to_range_start(focus, range.rhs.relative_position)
+        elsif range.open_on_right?
+          move_to_range_end(focus, range.lhs.relative_position)
         else
-          pos_left, pos_right = create_space_between(lhs, rhs)
+          pos_left, pos_right = create_space_between(range)
           desired_position = position_between(pos_left, pos_right)
-          focus.place_at_position(desired_position, lhs)
+          focus.place_at_position(desired_position, range.lhs)
         end
       end
 
@@ -95,16 +93,16 @@ module Gitlab
         context.object.relative_position = new_pos
       end
 
-      def create_space_between(lhs, rhs)
-        pos_left = lhs&.relative_position
-        pos_right = rhs&.relative_position
+      def create_space_between(range)
+        pos_left = range.lhs&.relative_position
+        pos_right = range.rhs&.relative_position
 
         return [pos_left, pos_right] unless gap_too_small?(pos_left, pos_right)
 
-        gap = rhs.create_space_left
+        gap = range.rhs.create_space_left
         [pos_left - gap.delta, pos_right]
       rescue NoSpaceLeft
-        gap = lhs.create_space_right
+        gap = range.lhs.create_space_right
         [pos_left, pos_right + gap.delta]
       end
 

lib/gitlab/relative_positioning/range.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module RelativePositioning
+    IllegalRange = Class.new(ArgumentError)
+
+    class Range
+      attr_reader :lhs, :rhs
+
+      def open_on_left?
+        lhs.nil?
+      end
+
+      def open_on_right?
+        rhs.nil?
+      end
+
+      def cover?(item_context)
+        return false unless item_context
+        return false unless item_context.positioned?
+        return true if item_context.object == lhs&.object
+        return true if item_context.object == rhs&.object
+
+        pos = item_context.relative_position
+
+        return lhs.relative_position < pos if open_on_right?
+        return pos < rhs.relative_position if open_on_left?
+
+        lhs.relative_position < pos && pos < rhs.relative_position
+      end
+
+      def ==(other)
+        other.is_a?(RelativePositioning::Range) && lhs == other.lhs && rhs == other.rhs
+      end
+    end
+
+    def self.range(lhs, rhs)
+      if lhs && rhs
+        ClosedRange.new(lhs, rhs)
+      elsif lhs
+        StartingFrom.new(lhs)
+      elsif rhs
+        EndingAt.new(rhs)
+      else
+        raise IllegalRange, 'One of rhs or lhs must be provided' unless lhs && rhs
+      end
+    end
+
+    class ClosedRange < RelativePositioning::Range
+      def initialize(lhs, rhs)
+        @lhs, @rhs = lhs, rhs
+        raise IllegalRange, 'Either lhs or rhs is missing' unless lhs && rhs
+        raise IllegalRange, 'lhs and rhs cannot be the same object' if lhs == rhs
+      end
+    end
+
+    class StartingFrom < RelativePositioning::Range
+      include Gitlab::Utils::StrongMemoize
+
+      def initialize(lhs)
+        @lhs = lhs
+        raise IllegalRange, 'lhs is required' unless lhs
+      end
+
+      def rhs
+        strong_memoize(:rhs) { lhs.rhs_neighbour }
+      end
+    end
+
+    class EndingAt < RelativePositioning::Range
+      include Gitlab::Utils::StrongMemoize
+
+      def initialize(rhs)
+        @rhs = rhs
+        raise IllegalRange, 'rhs is required' unless rhs
+      end
+
+      def lhs
+        strong_memoize(:lhs) { rhs.lhs_neighbour }
+      end
+    end
+  end
+end

spec/lib/gitlab/relative_positioning/range_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::RelativePositioning::Range do
+  item_a = OpenStruct.new(relative_position: 100, object: :x, positioned?: true)
+  item_b = OpenStruct.new(relative_position: 200, object: :y, positioned?: true)
+
+  before do
+    allow(item_a).to receive(:lhs_neighbour) { nil }
+    allow(item_a).to receive(:rhs_neighbour) { item_b }
+
+    allow(item_b).to receive(:lhs_neighbour) { item_a }
+    allow(item_b).to receive(:rhs_neighbour) { nil }
+  end
+
+  describe 'RelativePositioning.range' do
+    it 'raises if lhs and rhs are nil' do
+      expect { Gitlab::RelativePositioning.range(nil, nil) }.to raise_error(ArgumentError)
+    end
+
+    it 'raises an error if there is no extent' do
+      expect { Gitlab::RelativePositioning.range(item_a, item_a) }.to raise_error(ArgumentError)
+    end
+
+    it 'constructs a closed range when both termini are provided' do
+      range = Gitlab::RelativePositioning.range(item_a, item_b)
+      expect(range).to be_a_kind_of(Gitlab::RelativePositioning::Range)
+      expect(range).to be_a_kind_of(Gitlab::RelativePositioning::ClosedRange)
+    end
+
+    it 'constructs a starting-from range when only the LHS is provided' do
+      range = Gitlab::RelativePositioning.range(item_a, nil)
+      expect(range).to be_a_kind_of(Gitlab::RelativePositioning::Range)
+      expect(range).to be_a_kind_of(Gitlab::RelativePositioning::StartingFrom)
+    end
+
+    it 'constructs an ending-at range when only the RHS is provided' do
+      range = Gitlab::RelativePositioning.range(nil, item_b)
+      expect(range).to be_a_kind_of(Gitlab::RelativePositioning::Range)
+      expect(range).to be_a_kind_of(Gitlab::RelativePositioning::EndingAt)
+    end
+  end
+
+  it 'infers neighbours correctly' do
+    starting_at_a = Gitlab::RelativePositioning.range(item_a, nil)
+    ending_at_b = Gitlab::RelativePositioning.range(nil, item_b)
+
+    expect(starting_at_a).to eq(ending_at_b)
+  end
+
+  describe '#open_on_left?' do
+    where(:lhs, :rhs, :expected_result) do
+      [
+        [item_a, item_b, false],
+        [item_a, nil, false],
+        [nil, item_b, false],
+        [item_b, nil, false],
+        [nil, item_a, true]
+      ]
+    end
+
+    with_them do
+      it 'is true if there is no LHS terminus' do
+        range = Gitlab::RelativePositioning.range(lhs, rhs)
+
+        expect(range.open_on_left?).to be(expected_result)
+      end
+    end
+  end
+
+  describe '#open_on_right?' do
+    where(:lhs, :rhs, :expected_result) do
+      [
+        [item_a, item_b, false],
+        [item_a, nil, false],
+        [nil, item_b, false],
+        [item_b, nil, true],
+        [nil, item_a, false]
+      ]
+    end
+
+    with_them do
+      it 'is true if there is no RHS terminus' do
+        range = Gitlab::RelativePositioning.range(lhs, rhs)
+
+        expect(range.open_on_right?).to be(expected_result)
+      end
+    end
+  end
+
+  describe '#cover?' do
+    item_c = OpenStruct.new(relative_position: 150, object: :z, positioned?: true)
+    item_d = OpenStruct.new(relative_position: 050, object: :w, positioned?: true)
+    item_e = OpenStruct.new(relative_position: 250, object: :r, positioned?: true)
+    item_f = OpenStruct.new(positioned?: false)
+    item_ax = OpenStruct.new(relative_position: 100, object: :not_x, positioned?: true)
+    item_bx = OpenStruct.new(relative_position: 200, object: :not_y, positioned?: true)
+
+    where(:lhs, :rhs, :item, :expected_result) do
+      [
+        [item_a, item_b, item_a, true],
+        [item_a, item_b, item_b, true],
+        [item_a, item_b, item_c, true],
+        [item_a, item_b, item_d, false],
+        [item_a, item_b, item_e, false],
+        [item_a, item_b, item_ax, false],
+        [item_a, item_b, item_bx, false],
+        [item_a, item_b, item_f, false],
+        [item_a, item_b, nil, false],
+
+        [nil, item_b, item_a, true],
+        [nil, item_b, item_b, true],
+        [nil, item_b, item_c, true],
+        [nil, item_b, item_d, false],
+        [nil, item_b, item_e, false],
+        [nil, item_b, item_ax, false],
+        [nil, item_b, item_bx, false],
+        [nil, item_b, item_f, false],
+        [nil, item_b, nil, false],
+
+        [item_a, nil, item_a, true],
+        [item_a, nil, item_b, true],
+        [item_a, nil, item_c, true],
+        [item_a, nil, item_d, false],
+        [item_a, nil, item_e, false],
+        [item_a, nil, item_ax, false],
+        [item_a, nil, item_bx, false],
+        [item_a, nil, item_f, false],
+        [item_a, nil, nil, false],
+
+        [nil, item_a, item_a, true],
+        [nil, item_a, item_b, false],
+        [nil, item_a, item_c, false],
+        [nil, item_a, item_d, true],
+        [nil, item_a, item_e, false],
+        [nil, item_a, item_ax, false],
+        [nil, item_a, item_bx, false],
+        [nil, item_a, item_f, false],
+        [nil, item_a, nil, false],
+
+        [item_b, nil, item_a, false],
+        [item_b, nil, item_b, true],
+        [item_b, nil, item_c, false],
+        [item_b, nil, item_d, false],
+        [item_b, nil, item_e, true],
+        [item_b, nil, item_ax, false],
+        [item_b, nil, item_bx, false],
+        [item_b, nil, item_f, false],
+        [item_b, nil, nil, false]
+      ]
+    end
+
+    with_them do
+      it 'is true when the object is within the bounds of the range' do
+        range = Gitlab::RelativePositioning.range(lhs, rhs)
+
+        expect(range.cover?(item)).to be(expected_result)
+      end
+    end
+  end
+end