diff --git a/graphene_django/fields.py b/graphene_django/fields.py index 67559aa87..ccc61dec5 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -179,26 +179,43 @@ def connection_resolver( info, **args ): + # By current `connection_from_list_slice` implementation, + # `last` means last N items in the selection, + # and when use `last` with `first`, `last` means last N items in first N items. + first = args.get("first") last = args.get("last") - if enforce_first_or_last: - assert first or last, ( - "You must provide a `first` or `last` value to properly paginate the `{}` connection." - ).format(info.field_name) - - if max_limit: - if first: - assert first <= max_limit, ( - "Requesting {} records on the `{}` connection exceeds the `first` limit of {} records." - ).format(first, info.field_name, max_limit) - args["first"] = min(first, max_limit) - - if last: - assert last <= max_limit, ( - "Requesting {} records on the `{}` connection exceeds the `last` limit of {} records." - ).format(last, info.field_name, max_limit) - args["last"] = min(last, max_limit) + if first is not None and first <= 0: + raise ValueError( + "`first` argument must be positive, got `{first}`".format(first=first) + ) + if last is not None and last <= 0: + raise ValueError( + "`last` argument must be positive, got `{last}`".format(last=last) + ) + if enforce_first_or_last and not (first or last): + raise ValueError( + "You must provide a `first` or `last` value " + "to properly paginate the `{info.field_name}` connection.".format( + info=info + ) + ) + + if not max_limit: + pass + elif first is None and last is None: + args["first"] = max_limit + else: + count = min(i for i in (first, last) if i) + if count > max_limit: + raise ValueError( + ( + "Requesting {count} records " + "on the `{info.field_name}` connection " + "exceeds the limit of {max_limit} records." + ).format(count=count, info=info, max_limit=max_limit) + ) # eventually leads to DjangoObjectType's get_queryset (accepts queryset) # or a resolve_foo (does not accept queryset) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index 3881ed87a..5def5e408 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -13,6 +13,7 @@ from ..compat import JSONField, MissingType from ..fields import DjangoConnectionField +from ..settings import graphene_settings from ..types import DjangoObjectType from ..utils import DJANGO_FILTER_INSTALLED from .models import Article, CNNReporter, Film, FilmDetails, Reporter @@ -681,7 +682,7 @@ class Query(graphene.ObjectType): assert len(result.errors) == 1 assert str(result.errors[0]) == ( "Requesting 101 records on the `allReporters` connection " - "exceeds the `first` limit of 100 records." + "exceeds the limit of 100 records." ) assert result.data == expected @@ -722,11 +723,182 @@ class Query(graphene.ObjectType): assert len(result.errors) == 1 assert str(result.errors[0]) == ( "Requesting 101 records on the `allReporters` connection " - "exceeds the `last` limit of 100 records." + "exceeds the limit of 100 records." ) assert result.data == expected +def test_should_not_error_if_last_and_first_not_greater_than_max(): + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = 1 + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + r = Reporter.objects.create( + first_name="John", last_name="Doe", email="johndoe@example.com", a_choice=1 + ) + + schema = graphene.Schema(query=Query) + query = """ + query NodeFilteringQuery { + allReporters(first: 999999, last: 1) { + edges { + node { + id + } + } + } + } + """ + + expected = {"allReporters": {"edges": [{"node": {"id": "UmVwb3J0ZXJUeXBlOjE="}}]}} + + result = schema.execute(query) + assert not result.errors + assert result.data == expected + + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = 100 + + +def test_should_error_if_negative_first(): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query NodeFilteringQuery { + allReporters(first: -100, last: 200) { + edges { + node { + id + } + } + } + } + """ + + expected = {"allReporters": None} + + result = schema.execute(query) + assert len(result.errors) == 1 + assert str(result.errors[0]) == "`first` argument must be positive, got `-100`" + assert result.data == expected + + +def test_should_error_if_negative_last(): + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + schema = graphene.Schema(query=Query) + query = """ + query NodeFilteringQuery { + allReporters(first: 200, last: -100) { + edges { + node { + id + } + } + } + } + """ + + expected = {"allReporters": None} + + result = schema.execute(query) + assert len(result.errors) == 1 + assert str(result.errors[0]) == "`last` argument must be positive, got `-100`" + assert result.data == expected + + +def test_max_limit_is_zero(): + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = 0 + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + r = Reporter.objects.create( + first_name="John", last_name="Doe", email="johndoe@example.com", a_choice=1 + ) + + schema = graphene.Schema(query=Query) + query = """ + query NodeFilteringQuery { + allReporters(first: 99999999) { + edges { + node { + id + } + } + } + } + """ + + expected = {"allReporters": {"edges": [{"node": {"id": "UmVwb3J0ZXJUeXBlOjE="}}]}} + + result = schema.execute(query) + assert not result.errors + assert result.data == expected + + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = 100 + + +def test_max_limit_is_none(): + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = None + + class ReporterType(DjangoObjectType): + class Meta: + model = Reporter + interfaces = (Node,) + + class Query(graphene.ObjectType): + all_reporters = DjangoConnectionField(ReporterType) + + r = Reporter.objects.create( + first_name="John", last_name="Doe", email="johndoe@example.com", a_choice=1 + ) + + schema = graphene.Schema(query=Query) + query = """ + query NodeFilteringQuery { + allReporters(first: 99999999) { + edges { + node { + id + } + } + } + } + """ + + expected = {"allReporters": {"edges": [{"node": {"id": "UmVwb3J0ZXJUeXBlOjE="}}]}} + + result = schema.execute(query) + assert not result.errors + assert result.data == expected + + graphene_settings.RELAY_CONNECTION_MAX_LIMIT = 100 + + def test_should_query_promise_connectionfields(): from promise import Promise @@ -1229,7 +1401,7 @@ class Meta: class Query(graphene.ObjectType): films = DjangoConnectionField(FilmType) - def resolve_films(root, info): + def resolve_films(root, info, **kwargs): qs = Film.objects.prefetch_related("reporters") return qs