feat(ConnectionType) support bi-directional pagination

guides/fields/arguments.md

@@ -36,7 +36,7 @@ argument :category, !types.String
 Use `default_value: value` to provide a default value for the argument if not supplied in the query.
 
 ```ruby
-argument :category, !types.String, default_value: "Programming"
+argument :category, types.String, default_value: "Programming"
 ```
 
 Use `as: :alternateName` to use a different key from within your resolvers while

lib/graphql.rb

@@ -87,6 +87,7 @@ def self.scan_with_ragel(graphql_string)
 require "graphql/int_type"
 require "graphql/string_type"
 require "graphql/directive"
+require "graphql/name_validator"
 
 require "graphql/introspection"
 require "graphql/language"
@@ -100,6 +101,7 @@ def self.scan_with_ragel(graphql_string)
 require "graphql/analysis_error"
 require "graphql/runtime_type_error"
 require "graphql/invalid_null_error"
+require "graphql/invalid_name_error"
 require "graphql/unresolved_type_error"
 require "graphql/string_encoding_error"
 require "graphql/query"

lib/graphql/argument.rb

@@ -96,15 +96,19 @@ def prepare=(prepare_proc)
 
     NO_DEFAULT_VALUE = Object.new
     # @api private
-    def self.from_dsl(name, type = nil, description = nil, default_value: NO_DEFAULT_VALUE, as: nil, prepare: DefaultPrepare, **kwargs, &block)
+    def self.from_dsl(name, type_or_argument = nil, description = nil, default_value: NO_DEFAULT_VALUE, as: nil, prepare: DefaultPrepare, **kwargs, &block)
+      if type_or_argument.is_a?(GraphQL::Argument)
+        return type_or_argument.redefine(name: name.to_s)
+      end
+
       argument = if block_given?
         GraphQL::Argument.define(&block)
       else
         GraphQL::Argument.define(**kwargs)
       end
 
       argument.name = name.to_s
-      type && argument.type = type
+      type_or_argument && argument.type = type_or_argument
       description && argument.description = description
       if default_value != NO_DEFAULT_VALUE
         argument.default_value = default_value

lib/graphql/enum_type.rb

@@ -141,23 +141,14 @@ class EnumValue
 
       def name=(new_name)
         # Validate that the name is correct
-        unless new_name =~ /^[_a-zA-Z][_a-zA-Z0-9]*$/
-          raise(
-            GraphQL::EnumType::InvalidEnumNameError,
-            "Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but '#{new_name}' does not"
-          )
-        end
-
+        GraphQL::NameValidator.validate!(new_name)
         @name = new_name
       end
     end
 
     class UnresolvedValueError < GraphQL::Error
     end
 
-    class InvalidEnumNameError < GraphQL::Error
-    end
-
     private
 
     # Get the underlying value for this enum value

lib/graphql/invalid_name_error.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+module GraphQL
+  class InvalidNameError < GraphQL::ExecutionError
+    attr_reader :name, :valid_regex
+    def initialize(name, valid_regex)
+      @name = name
+      @valid_regex = valid_regex
+      super("Names must match #{@valid_regex.inspect} but '#{@name}' does not")
+    end
+  end
+end

lib/graphql/name_validator.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+module GraphQL
+  class NameValidator
+    VALID_NAME_REGEX = /^[_a-zA-Z][_a-zA-Z0-9]*$/
+
+    def self.validate!(name)
+      raise GraphQL::InvalidNameError.new(name, VALID_NAME_REGEX) unless valid?(name)
+    end
+
+    private
+
+    def self.valid?(name)
+      name =~ VALID_NAME_REGEX
+    end
+  end
+end

lib/graphql/object_type.rb

@@ -102,6 +102,11 @@ def implements(interfaces, inherit: false)
       dirty_ifaces.concat(interfaces)
     end
 
+    def name=(name)
+      GraphQL::NameValidator.validate!(name)
+      @name = name
+    end
+
     protected
 
     attr_reader :dirty_interfaces, :dirty_inherited_interfaces

lib/graphql/query.rb

@@ -86,6 +86,10 @@ def initialize(schema, query_string = nil, query: nil, document: nil, context: n
       @query_string = query_string || query
       @document = document
 
+      if @query_string && @document
+        raise ArgumentError, "Query should only be provided a query string or a document, not both."
+      end
+
       # A two-layer cache of type resolution:
       # { abstract_type => { value => resolved_type } }
       @resolved_types_cache = Hash.new do |h1, k1|

lib/graphql/relay/array_connection.rb

@@ -7,6 +7,30 @@ def cursor_from_node(item)
         encode(idx.to_s)
       end
 
+      def has_next_page
+        if first
+          # There are more items after these items
+          sliced_nodes.count > first
+        elsif GraphQL::Relay::ConnectionType.bidirectional_pagination && before
+          # The original array is longer than the `before` index
+          index_from_cursor(before) < nodes.length
+        else
+          false
+        end
+      end
+
+      def has_previous_page
+        if last
+          # There are items preceding the ones in this result
+          sliced_nodes.count > last
+        elsif GraphQL::Relay::ConnectionType.bidirectional_pagination && after
+          # We've paginated into the Array a bit, there are some behind us
+          index_from_cursor(after) > 0
+        else
+          false
+        end
+      end
+
       private
 
       def first

lib/graphql/relay/connection_type.rb

@@ -3,10 +3,14 @@ module GraphQL
   module Relay
     module ConnectionType
       class << self
+        # @return [Boolean] If true, connection types get a `nodes` shortcut field
         attr_accessor :default_nodes_field
+        # @return [Boolean] If true, connections check for reverse-direction `has*Page` values
+        attr_accessor :bidirectional_pagination
       end
 
       self.default_nodes_field = false
+      self.bidirectional_pagination = false
 
       # Create a connection which exposes edges of this type
       def self.create_type(wrapped_type, edge_type: wrapped_type.edge_type, edge_class: GraphQL::Relay::Edge, nodes_field: ConnectionType.default_nodes_field, &block)

lib/graphql/relay/relation_connection.rb

@@ -24,11 +24,25 @@ def cursor_from_node(item)
       end
 
       def has_next_page
-        !!(first && paged_nodes_length >= first && sliced_nodes_count > first)
+        if first
+          paged_nodes_length >= first && sliced_nodes_count > first
+        elsif GraphQL::Relay::ConnectionType.bidirectional_pagination && last
+          sliced_nodes_count > last
+        else
+          false
+        end
       end
 
       def has_previous_page
-        !!(last && paged_nodes_length >= last && sliced_nodes_count > last)
+        if last
+          paged_nodes_length >= last && sliced_nodes_count > last
+        elsif GraphQL::Relay::ConnectionType.bidirectional_pagination && after
+          # We've already paginated through the collection a bit,
+          # there are nodes behind us
+          offset_from_cursor(after) > 0
+        else
+          false
+        end
       end
 
       def first

readme.md

@@ -37,7 +37,7 @@ Or, see ["Getting Started"](https://rmosolgo.github.io/graphql-ruby/).
 
 ## Upgrade
 
-I also sell [GraphQL::Pro](http://graphql.pro) which provides several features on top of the GraphQL runtime, including [authorization](http://rmosolgo.github.io/graphql-ruby/pro/authorization), [monitoring](http://rmosolgo.github.io/graphql-ruby/pro/monitoring) plugins and [static queries](http://rmosolgo.github.io/graphql-ruby/pro/persisted_queries). Besides that, Pro customers get email support and an opportunity to support graphql-ruby's development!
+I also sell [GraphQL::Pro](http://graphql.pro) which provides several features on top of the GraphQL runtime, including [authorization](http://rmosolgo.github.io/graphql-ruby/pro/authorization), [monitoring](http://rmosolgo.github.io/graphql-ruby/pro/monitoring) plugins and [persisted queries](http://rmosolgo.github.io/graphql-ruby/operation_store/overview). Besides that, Pro customers get email support and an opportunity to support graphql-ruby's development!
 
 ## Goals
 

spec/graphql/argument_spec.rb

@@ -19,6 +19,34 @@
     assert_includes err.message, expected_error
   end
 
+  describe ".from_dsl" do
+    it "accepts an existing argument" do
+      existing = GraphQL::Argument.define do
+        name "bar"
+        type GraphQL::STRING_TYPE
+      end
+
+      arg = GraphQL::Argument.from_dsl(:foo, existing)
+
+      assert_equal "foo", arg.name
+      assert_equal GraphQL::STRING_TYPE, arg.type
+    end
+
+    it "creates an argument from dsl arguments" do
+      arg = GraphQL::Argument.from_dsl(
+        :foo,
+        GraphQL::STRING_TYPE,
+        "A Description",
+        default_value: "Bar"
+      )
+
+      assert_equal "foo", arg.name
+      assert_equal GraphQL::STRING_TYPE, arg.type
+      assert_equal "A Description", arg.description
+      assert_equal "Bar", arg.default_value
+    end
+  end
+
   it "accepts custom keywords" do
     type = GraphQL::ObjectType.define do
       name "Something"

spec/graphql/define/assign_argument_spec.rb

@@ -37,6 +37,18 @@
     assert_equal "GraphQL::Argument can't define 'blah'", err.message
   end
 
+  it "accepts an existing argument" do
+    existing = GraphQL::Argument.define do
+      name "bar"
+      type GraphQL::STRING_TYPE
+    end
+
+    arg = define_argument(:foo, existing)
+
+    assert_equal "foo", arg.name
+    assert_equal GraphQL::STRING_TYPE, arg.type
+  end
+
   def define_argument(*args)
     type = GraphQL::ObjectType.define do
       field :a, types.String do

spec/graphql/enum_type_spec.rb

@@ -39,7 +39,7 @@
 
   describe "invalid names" do
     it "rejects names with a space" do
-      assert_raises(GraphQL::EnumType::InvalidEnumNameError) {
+      assert_raises(GraphQL::InvalidNameError) {
         InvalidEnumTest = GraphQL::EnumType.define do
           name "InvalidEnumTest"
 

spec/graphql/object_type_spec.rb

@@ -17,6 +17,20 @@
     }
   end
 
+  it "doesn't allow invalid name" do
+    exception = assert_raises(GraphQL::InvalidNameError) {
+      InvalidNameObject = GraphQL::ObjectType.define do
+        name "Three Word Query"
+
+        field :id, !types.Int, "id field"
+      end
+
+      # Force evaluation
+      InvalidNameObject.name
+    }
+    assert_equal("Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but 'Three Word Query' does not", exception.message)
+  end
+
   it "has a name" do
     assert_equal("Cheese", type.name)
     type.name = "Fromage"

spec/graphql/query_spec.rb

@@ -44,6 +44,28 @@
   )}
   let(:result) { query.result }
 
+  describe "when passed both a query string and a document" do
+    it "returns an error to the client when query kwarg is used" do
+      assert_raises ArgumentError do
+        GraphQL::Query.new(
+          schema,
+          query: "{ fromSource(source: COW) { id } }",
+          document: document
+        )
+      end
+    end
+
+    it "returns an error to the client" do
+      assert_raises ArgumentError do
+        GraphQL::Query.new(
+          schema,
+          "{ fromSource(source: COW) { id } }",
+          document: document
+        )
+      end
+    end
+  end
+
   describe "when passed no query string or document" do
     it 'returns an error to the client' do
       res = GraphQL::Query.new(

spec/graphql/relay/array_connection_spec.rb

@@ -11,6 +11,10 @@ def get_last_cursor(result)
     result["data"]["rebels"]["ships"]["edges"].last["cursor"]
   end
 
+  def get_page_info(result, key = "bases")
+    result["data"]["rebels"][key]["pageInfo"]
+  end
+
   describe "results" do
     let(:query_string) {%|
       query getShips($first: Int, $after: String, $last: Int, $before: String, $nameIncludes: String){
@@ -61,6 +65,36 @@ def get_last_cursor(result)
       assert_equal("NQ==", result["data"]["rebels"]["ships"]["pageInfo"]["endCursor"])
     end
 
+    it "provides bidirectional_pagination" do
+      result = star_wars_query(query_string, "first" => 1)
+      last_cursor = get_last_cursor(result)
+
+      # When going forwards, bi-directional pagination
+      # returns `true` even for `hasPreviousPage`
+      result = star_wars_query(query_string, "first" => 1, "after" => last_cursor)
+      assert_equal(true, get_page_info(result, "ships")["hasNextPage"])
+      assert_equal(false, get_page_info(result, "ships")["hasPreviousPage"])
+
+      result = with_bidirectional_pagination {
+        star_wars_query(query_string, "first" => 3, "after" => last_cursor)
+      }
+      assert_equal(true, get_page_info(result, "ships")["hasNextPage"])
+      assert_equal(true, get_page_info(result, "ships")["hasPreviousPage"])
+
+      # When going backwards, bi-directional pagination
+      # returns true for `hasNextPage`
+      last_cursor = get_last_cursor(result)
+      result = star_wars_query(query_string, "last" => 1, "before" => last_cursor)
+      assert_equal(false, get_page_info(result, "ships")["hasNextPage"])
+      assert_equal(true, get_page_info(result, "ships")["hasPreviousPage"])
+
+      result = with_bidirectional_pagination {
+        star_wars_query(query_string, "last" => 2, "before" => last_cursor)
+      }
+      assert_equal(true, get_page_info(result, "ships")["hasNextPage"])
+      assert_equal(true, get_page_info(result, "ships")["hasPreviousPage"])
+    end
+
     it 'slices the result' do
       result = star_wars_query(query_string, "first" => 1)
       assert_equal(["X-Wing"], get_names(result))
@@ -142,10 +176,6 @@ def get_names(result)
         result["data"]["rebels"]["bases"]["edges"].map { |e| e["node"]["name"] }
       end
 
-      def get_page_info(result)
-        result["data"]["rebels"]["bases"]["pageInfo"]
-      end
-
       let(:query_string) {%|
         query getShips($first: Int, $after: String, $last: Int, $before: String){
           rebels {