diff --git a/examples/cookbook-plain/cookbook/ingredients/migrations/0001_initial.py b/examples/cookbook-plain/cookbook/ingredients/migrations/0001_initial.py index 04949239f..ee8cadd42 100644 --- a/examples/cookbook-plain/cookbook/ingredients/migrations/0001_initial.py +++ b/examples/cookbook-plain/cookbook/ingredients/migrations/0001_initial.py @@ -10,24 +10,46 @@ class Migration(migrations.Migration): initial = True - dependencies = [ - ] + dependencies = [] operations = [ migrations.CreateModel( - name='Category', + name="Category", fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('name', models.CharField(max_length=100)), + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("name", models.CharField(max_length=100)), ], ), migrations.CreateModel( - name='Ingredient', + name="Ingredient", fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('name', models.CharField(max_length=100)), - ('notes', models.TextField()), - ('category', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='ingredients', to='ingredients.Category')), + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("name", models.CharField(max_length=100)), + ("notes", models.TextField()), + ( + "category", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="ingredients", + to="ingredients.Category", + ), + ), ], ), ] diff --git a/examples/cookbook-plain/cookbook/ingredients/migrations/0002_auto_20161104_0050.py b/examples/cookbook-plain/cookbook/ingredients/migrations/0002_auto_20161104_0050.py index 359d4fc4c..40c7f2bfa 100644 --- a/examples/cookbook-plain/cookbook/ingredients/migrations/0002_auto_20161104_0050.py +++ b/examples/cookbook-plain/cookbook/ingredients/migrations/0002_auto_20161104_0050.py @@ -7,14 +7,12 @@ class Migration(migrations.Migration): - dependencies = [ - ('ingredients', '0001_initial'), - ] + dependencies = [("ingredients", "0001_initial")] operations = [ migrations.AlterField( - model_name='ingredient', - name='notes', + model_name="ingredient", + name="notes", field=models.TextField(blank=True, null=True), - ), + ) ] diff --git a/examples/cookbook-plain/cookbook/ingredients/migrations/0003_auto_20181018_1746.py b/examples/cookbook-plain/cookbook/ingredients/migrations/0003_auto_20181018_1746.py index 184e79e4f..3648299fd 100644 --- a/examples/cookbook-plain/cookbook/ingredients/migrations/0003_auto_20181018_1746.py +++ b/examples/cookbook-plain/cookbook/ingredients/migrations/0003_auto_20181018_1746.py @@ -5,13 +5,10 @@ class Migration(migrations.Migration): - dependencies = [ - ('ingredients', '0002_auto_20161104_0050'), - ] + dependencies = [("ingredients", "0002_auto_20161104_0050")] operations = [ migrations.AlterModelOptions( - name='category', - options={'verbose_name_plural': 'Categories'}, - ), + name="category", options={"verbose_name_plural": "Categories"} + ) ] diff --git a/examples/cookbook-plain/cookbook/recipes/migrations/0001_initial.py b/examples/cookbook-plain/cookbook/recipes/migrations/0001_initial.py index 338c71a1b..9ce12f2b1 100644 --- a/examples/cookbook-plain/cookbook/recipes/migrations/0001_initial.py +++ b/examples/cookbook-plain/cookbook/recipes/migrations/0001_initial.py @@ -10,27 +10,61 @@ class Migration(migrations.Migration): initial = True - dependencies = [ - ('ingredients', '0001_initial'), - ] + dependencies = [("ingredients", "0001_initial")] operations = [ migrations.CreateModel( - name='Recipe', + name="Recipe", fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('title', models.CharField(max_length=100)), - ('instructions', models.TextField()), + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("title", models.CharField(max_length=100)), + ("instructions", models.TextField()), ], ), migrations.CreateModel( - name='RecipeIngredient', + name="RecipeIngredient", fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('amount', models.FloatField()), - ('unit', models.CharField(choices=[('kg', 'Kilograms'), ('l', 'Litres'), ('', 'Units')], max_length=20)), - ('ingredient', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='used_by', to='ingredients.Ingredient')), - ('recipes', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='amounts', to='recipes.Recipe')), + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("amount", models.FloatField()), + ( + "unit", + models.CharField( + choices=[("kg", "Kilograms"), ("l", "Litres"), ("", "Units")], + max_length=20, + ), + ), + ( + "ingredient", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="used_by", + to="ingredients.Ingredient", + ), + ), + ( + "recipes", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="amounts", + to="recipes.Recipe", + ), + ), ], ), ] diff --git a/examples/cookbook-plain/cookbook/recipes/migrations/0002_auto_20161104_0106.py b/examples/cookbook-plain/cookbook/recipes/migrations/0002_auto_20161104_0106.py index f13539265..fb32282b2 100644 --- a/examples/cookbook-plain/cookbook/recipes/migrations/0002_auto_20161104_0106.py +++ b/examples/cookbook-plain/cookbook/recipes/migrations/0002_auto_20161104_0106.py @@ -7,19 +7,23 @@ class Migration(migrations.Migration): - dependencies = [ - ('recipes', '0001_initial'), - ] + dependencies = [("recipes", "0001_initial")] operations = [ migrations.RenameField( - model_name='recipeingredient', - old_name='recipes', - new_name='recipe', + model_name="recipeingredient", old_name="recipes", new_name="recipe" ), migrations.AlterField( - model_name='recipeingredient', - name='unit', - field=models.CharField(choices=[(b'unit', b'Units'), (b'kg', b'Kilograms'), (b'l', b'Litres'), (b'st', b'Shots')], max_length=20), + model_name="recipeingredient", + name="unit", + field=models.CharField( + choices=[ + (b"unit", b"Units"), + (b"kg", b"Kilograms"), + (b"l", b"Litres"), + (b"st", b"Shots"), + ], + max_length=20, + ), ), ] diff --git a/examples/cookbook-plain/cookbook/recipes/migrations/0003_auto_20181018_1728.py b/examples/cookbook-plain/cookbook/recipes/migrations/0003_auto_20181018_1728.py index 7a8df493b..05fed0533 100644 --- a/examples/cookbook-plain/cookbook/recipes/migrations/0003_auto_20181018_1728.py +++ b/examples/cookbook-plain/cookbook/recipes/migrations/0003_auto_20181018_1728.py @@ -5,14 +5,20 @@ class Migration(migrations.Migration): - dependencies = [ - ('recipes', '0002_auto_20161104_0106'), - ] + dependencies = [("recipes", "0002_auto_20161104_0106")] operations = [ migrations.AlterField( - model_name='recipeingredient', - name='unit', - field=models.CharField(choices=[('unit', 'Units'), ('kg', 'Kilograms'), ('l', 'Litres'), ('st', 'Shots')], max_length=20), - ), + model_name="recipeingredient", + name="unit", + field=models.CharField( + choices=[ + ("unit", "Units"), + ("kg", "Kilograms"), + ("l", "Litres"), + ("st", "Shots"), + ], + max_length=20, + ), + ) ] diff --git a/examples/cookbook/cookbook/ingredients/migrations/0001_initial.py b/examples/cookbook/cookbook/ingredients/migrations/0001_initial.py index 04949239f..ee8cadd42 100644 --- a/examples/cookbook/cookbook/ingredients/migrations/0001_initial.py +++ b/examples/cookbook/cookbook/ingredients/migrations/0001_initial.py @@ -10,24 +10,46 @@ class Migration(migrations.Migration): initial = True - dependencies = [ - ] + dependencies = [] operations = [ migrations.CreateModel( - name='Category', + name="Category", fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('name', models.CharField(max_length=100)), + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("name", models.CharField(max_length=100)), ], ), migrations.CreateModel( - name='Ingredient', + name="Ingredient", fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('name', models.CharField(max_length=100)), - ('notes', models.TextField()), - ('category', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='ingredients', to='ingredients.Category')), + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("name", models.CharField(max_length=100)), + ("notes", models.TextField()), + ( + "category", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="ingredients", + to="ingredients.Category", + ), + ), ], ), ] diff --git a/examples/cookbook/cookbook/ingredients/migrations/0002_auto_20161104_0050.py b/examples/cookbook/cookbook/ingredients/migrations/0002_auto_20161104_0050.py index 359d4fc4c..40c7f2bfa 100644 --- a/examples/cookbook/cookbook/ingredients/migrations/0002_auto_20161104_0050.py +++ b/examples/cookbook/cookbook/ingredients/migrations/0002_auto_20161104_0050.py @@ -7,14 +7,12 @@ class Migration(migrations.Migration): - dependencies = [ - ('ingredients', '0001_initial'), - ] + dependencies = [("ingredients", "0001_initial")] operations = [ migrations.AlterField( - model_name='ingredient', - name='notes', + model_name="ingredient", + name="notes", field=models.TextField(blank=True, null=True), - ), + ) ] diff --git a/examples/cookbook/cookbook/recipes/migrations/0001_initial.py b/examples/cookbook/cookbook/recipes/migrations/0001_initial.py index 338c71a1b..9ce12f2b1 100644 --- a/examples/cookbook/cookbook/recipes/migrations/0001_initial.py +++ b/examples/cookbook/cookbook/recipes/migrations/0001_initial.py @@ -10,27 +10,61 @@ class Migration(migrations.Migration): initial = True - dependencies = [ - ('ingredients', '0001_initial'), - ] + dependencies = [("ingredients", "0001_initial")] operations = [ migrations.CreateModel( - name='Recipe', + name="Recipe", fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('title', models.CharField(max_length=100)), - ('instructions', models.TextField()), + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("title", models.CharField(max_length=100)), + ("instructions", models.TextField()), ], ), migrations.CreateModel( - name='RecipeIngredient', + name="RecipeIngredient", fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('amount', models.FloatField()), - ('unit', models.CharField(choices=[('kg', 'Kilograms'), ('l', 'Litres'), ('', 'Units')], max_length=20)), - ('ingredient', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='used_by', to='ingredients.Ingredient')), - ('recipes', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='amounts', to='recipes.Recipe')), + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("amount", models.FloatField()), + ( + "unit", + models.CharField( + choices=[("kg", "Kilograms"), ("l", "Litres"), ("", "Units")], + max_length=20, + ), + ), + ( + "ingredient", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="used_by", + to="ingredients.Ingredient", + ), + ), + ( + "recipes", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="amounts", + to="recipes.Recipe", + ), + ), ], ), ] diff --git a/examples/cookbook/cookbook/recipes/migrations/0002_auto_20161104_0106.py b/examples/cookbook/cookbook/recipes/migrations/0002_auto_20161104_0106.py index f13539265..fb32282b2 100644 --- a/examples/cookbook/cookbook/recipes/migrations/0002_auto_20161104_0106.py +++ b/examples/cookbook/cookbook/recipes/migrations/0002_auto_20161104_0106.py @@ -7,19 +7,23 @@ class Migration(migrations.Migration): - dependencies = [ - ('recipes', '0001_initial'), - ] + dependencies = [("recipes", "0001_initial")] operations = [ migrations.RenameField( - model_name='recipeingredient', - old_name='recipes', - new_name='recipe', + model_name="recipeingredient", old_name="recipes", new_name="recipe" ), migrations.AlterField( - model_name='recipeingredient', - name='unit', - field=models.CharField(choices=[(b'unit', b'Units'), (b'kg', b'Kilograms'), (b'l', b'Litres'), (b'st', b'Shots')], max_length=20), + model_name="recipeingredient", + name="unit", + field=models.CharField( + choices=[ + (b"unit", b"Units"), + (b"kg", b"Kilograms"), + (b"l", b"Litres"), + (b"st", b"Shots"), + ], + max_length=20, + ), ), ] diff --git a/graphene_django/fields.py b/graphene_django/fields.py index eb1215ead..14022a969 100644 --- a/graphene_django/fields.py +++ b/graphene_django/fields.py @@ -131,32 +131,49 @@ def connection_resolver( enforce_first_or_last, root, info, - **args + **kwargs ): - 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) - - iterable = resolver(root, info, **args) - queryset = cls.resolve_queryset(connection, default_manager, info, args) - on_resolve = partial(cls.resolve_connection, connection, queryset, 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 = kwargs.get("first") + last = kwargs.get("last") + 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: + kwargs["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) + ) + + iterable = resolver(root, info, **kwargs) + queryset = cls.resolve_queryset(connection, default_manager, info, kwargs) + on_resolve = partial(cls.resolve_connection, connection, queryset, kwargs) if Promise.is_thenable(iterable): return Promise.resolve(iterable).then(on_resolve) diff --git a/graphene_django/tests/test_query.py b/graphene_django/tests/test_query.py index f24f84bca..eac4481bf 100644 --- a/graphene_django/tests/test_query.py +++ b/graphene_django/tests/test_query.py @@ -1,23 +1,22 @@ import base64 import datetime +import graphene import pytest from django.db import models +from django.db.models import Q from django.utils.functional import SimpleLazyObject +from graphene.relay import Node from py.test import raises -from django.db.models import Q - from graphql_relay import to_global_id -import graphene -from graphene.relay import Node -from ..utils import DJANGO_FILTER_INSTALLED -from ..compat import MissingType, JSONField +from ..compat import JSONField, MissingType from ..fields import DjangoConnectionField -from ..types import DjangoObjectType from ..settings import graphene_settings -from .models import Article, CNNReporter, Reporter, Film, FilmDetails +from ..types import DjangoObjectType +from ..utils import DJANGO_FILTER_INSTALLED +from .models import Article, CNNReporter, Film, FilmDetails, Reporter pytestmark = pytest.mark.django_db @@ -661,7 +660,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 @@ -702,13 +701,184 @@ 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 graphene_settings.RELAY_CONNECTION_ENFORCE_FIRST_OR_LAST = False +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 @@ -1083,7 +1253,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