🔄 Merge pull request Ambro17#18 from Ambro17/add-max-field-arguments-rule

Add max field arguments rule

Author: Ambro17

schemadiff/changes/field.py

@@ -1,4 +1,4 @@
-from graphql import is_non_null_type
+from graphql import is_non_null_type, GraphQLField
 
 from schemadiff.changes import Change, Criticality, is_safe_type_change
 
@@ -71,13 +71,14 @@ def path(self):
 
 
 class FieldArgumentAdded(Change):
-    def __init__(self, parent, field_name, argument_name, arg_type):
+    def __init__(self, parent, field_name: str, field: GraphQLField, argument_name, arg_type):
         self.criticality = Criticality.safe('Adding an optional argument is a safe change')\
                            if not is_non_null_type(arg_type.type)\
                            else Criticality.breaking("Adding a required argument to an existing field is a breaking "
                                                    "change because it will break existing uses of this field")
         self.parent = parent
         self.field_name = field_name
+        self.field = field
         self.argument_name = argument_name
         self.arg_type = arg_type
 

schemadiff/changes/object.py

@@ -1,13 +1,16 @@
+from graphql import GraphQLField, GraphQLObjectType
+
 from schemadiff.changes import Change, Criticality
 
 
 class ObjectTypeFieldAdded(Change):
 
     criticality = Criticality.safe()
 
-    def __init__(self, parent, field_name):
+    def __init__(self, parent: GraphQLObjectType, field_name, field: GraphQLField):
         self.parent = parent
         self.field_name = field_name
+        self.field = field
         self.description = parent.fields[field_name].description
 
     @property

schemadiff/diff/field.py

@@ -10,8 +10,8 @@
 
 class Field:
 
-    def __init__(self, type_, name, old_field, new_field):
-        self.type_ = type_
+    def __init__(self, parent, name, old_field, new_field):
+        self.parent = parent
         self.field_name = name
         self.old_field = old_field
         self.new_field = new_field
@@ -22,31 +22,31 @@ def diff(self):
         changes = []
 
         if self.old_field.description != self.new_field.description:
-            changes.append(FieldDescriptionChanged(self.type_, self.field_name, self.old_field, self.new_field))
+            changes.append(FieldDescriptionChanged(self.parent, self.field_name, self.old_field, self.new_field))
 
         if self.old_field.deprecation_reason != self.new_field.deprecation_reason:
-            changes.append(FieldDeprecationReasonChanged(self.type_, self.field_name, self.old_field, self.new_field))
+            changes.append(FieldDeprecationReasonChanged(self.parent, self.field_name, self.old_field, self.new_field))
 
         if str(self.old_field.type) != str(self.new_field.type):
-            changes.append(FieldTypeChanged(self.type_, self.field_name, self.old_field, self.new_field))
+            changes.append(FieldTypeChanged(self.parent, self.field_name, self.old_field, self.new_field))
 
         added = self.new_args - self.old_args
         removed = self.old_args - self.new_args
 
         changes.extend(
-            FieldArgumentAdded(self.type_, self.field_name, arg_name, self.new_field.args[arg_name])
+            FieldArgumentAdded(self.parent, self.field_name, self.new_field, arg_name, self.new_field.args[arg_name])
             for arg_name in added
         )
         changes.extend(
-            FieldArgumentRemoved(self.type_, self.field_name, arg_name)
+            FieldArgumentRemoved(self.parent, self.field_name, arg_name)
             for arg_name in removed
         )
 
         common_arguments = self.common_arguments()
         for arg_name in common_arguments:
             old_arg = self.old_field.args[arg_name]
             new_arg = self.new_field.args[arg_name]
-            changes += Argument(self.type_, self.field_name, arg_name, old_arg, new_arg).diff() or []
+            changes += Argument(self.parent, self.field_name, arg_name, old_arg, new_arg).diff() or []
 
         return changes
 

schemadiff/diff/object_type.py

@@ -9,8 +9,8 @@ def __init__(self, old, new):
         self.old = old
         self.new = new
 
-        self.old_fields = set(old.fields)
-        self.new_fields = set(new.fields)
+        self.old_field_names = set(old.fields)
+        self.new_field_names = set(new.fields)
 
         self.old_interfaces = set(old.interfaces)
         self.new_interfaces = set(new.interfaces)
@@ -19,9 +19,9 @@ def diff(self):
         changes = []
 
         # Added and removed fields
-        added = self.new_fields - self.old_fields
-        removed = self.old_fields - self.new_fields
-        changes.extend(ObjectTypeFieldAdded(self.new, field_name) for field_name in added)
+        added = self.new_field_names - self.old_field_names
+        removed = self.old_field_names - self.new_field_names
+        changes.extend(ObjectTypeFieldAdded(self.new, field_name, self.new.fields[field_name]) for field_name in added)
         changes.extend(ObjectTypeFieldRemoved(self.new, field_name, self.old.fields[field_name])
                        for field_name in removed)
 
@@ -31,15 +31,15 @@ def diff(self):
         changes.extend(NewInterfaceImplemented(interface, self.new) for interface in added)
         changes.extend(DroppedInterfaceImplementation(interface, self.new) for interface in removed)
 
-        for field in self.common_fields():
-            old_field = self.old.fields[field]
-            new_field = self.new.fields[field]
-            changes += Field(self.new, field, old_field, new_field).diff() or []
+        for field_name in self.common_fields():
+            old_field = self.old.fields[field_name]
+            new_field = self.new.fields[field_name]
+            changes += Field(self.new, field_name, old_field, new_field).diff() or []
 
         return changes
 
     def common_fields(self):
-        return self.old_fields & self.new_fields
+        return self.old_field_names & self.new_field_names
 
     def added_interfaces(self):
         """Compare interfaces equality by name. Internal diffs are solved later"""

schemadiff/validation_rules/__init__.py

@@ -4,7 +4,7 @@
 from graphql import GraphQLObjectType
 
 from schemadiff.changes.enum import EnumValueAdded, EnumValueDescriptionChanged
-from schemadiff.changes.field import FieldDescriptionChanged
+from schemadiff.changes.field import FieldDescriptionChanged, FieldArgumentAdded
 from schemadiff.changes.object import ObjectTypeFieldAdded
 from schemadiff.changes.type import AddedType, TypeDescriptionChanged
 
@@ -62,8 +62,6 @@ def is_valid(self) -> bool:
         else:
             return type_has_description
 
-        return True
-
     @property
     def message(self):
         return f"{self.change.message} without a description for {self.change.path} (rule: `{self.name}`)."
@@ -144,3 +142,5 @@ def is_valid(self) -> bool:
     @property
     def message(self):
         return f"Description for enum value `{self.change.name}` was removed (rule: `{self.name}`)"
+
+

tests/test_validation_rules.py

@@ -2,6 +2,8 @@
 from graphql import build_schema as schema
 
 from schemadiff.changes import Change, Criticality
+from schemadiff.changes.field import FieldArgumentAdded
+from schemadiff.changes.object import ObjectTypeFieldAdded
 from schemadiff.validation import evaluate_rules, ValidationResult, ValidationError
 from schemadiff.validation_rules import (
     ValidationRule,
@@ -230,3 +232,137 @@ def test_schema_added_field_no_desc():
     )
     assert diff[0].path == 'AddedType.other'
 
+
+def test_cant_create_mutation_with_more_than_10_arguments():
+    schema_restrictions = ['field-has-too-many-arguments']
+
+    class FieldHasTooManyArguments(ValidationRule):
+        """Restrict adding fields with too many top level arguments"""
+
+        name = "field-has-too-many-arguments"
+        limit = 10
+
+        def is_valid(self) -> bool:
+            if not isinstance(self.change, (ObjectTypeFieldAdded, FieldArgumentAdded)):
+                return True
+
+            if len(self.args) > self.limit:
+                return False
+            else:
+                return True
+
+        @property
+        def args(self):
+            return self.change.field.args or {}
+
+        @property
+        def message(self):
+            return f"Field `{self.change.parent.name}.{self.change.field_name}` has too many arguments " \
+                   f"({len(self.args)}>{self.limit}). Rule: {self.name}"
+
+    old_schema = schema("""
+    schema {
+        mutation: Mutation
+    }
+
+    type Mutation {
+        field: Int
+    }
+    """)
+
+    new_schema = schema("""
+    schema {
+        mutation: Mutation
+    }
+
+    type Mutation {
+        field: Int
+        mutation_with_too_many_args(
+            a1: Int, a2: Int, a3: Int, a4: Int, a5: Int, a6: Int, a7: Int, a8: Int, a9: Int, a10: Int, a11: Int 
+        ): Int
+    }
+    """)
+
+    diff = Schema(old_schema, new_schema).diff()
+    # Type Int was also added but its ignored because its a primitive.
+    assert diff and len(diff) == 1
+    error_msg = (
+        'Field `Mutation.mutation_with_too_many_args` has too many arguments (11>10). '
+        'Rule: field-has-too-many-arguments'
+    )
+    assert evaluate_rules(diff, schema_restrictions) == ValidationResult(
+        ok=False,
+        errors=[ValidationError(error_msg)]
+    )
+    assert diff[0].path == 'Mutation.mutation_with_too_many_args'
+
+
+def test_cant_add_arguments_to_mutation_if_exceeds_10_args():
+    schema_restrictions = ['field-has-too-many-arguments']
+
+    class FieldHasTooManyArguments(ValidationRule):
+        """Restrict adding fields with too many top level arguments"""
+
+        name = "field-has-too-many-arguments"
+        limit = 10
+
+        def is_valid(self) -> bool:
+            if not isinstance(self.change, (ObjectTypeFieldAdded, FieldArgumentAdded)):
+                return True
+
+            if len(self.args) > self.limit:
+                return False
+            else:
+                return True
+
+        @property
+        def args(self):
+            return self.change.field.args or {}
+
+        @property
+        def message(self):
+            return f"Field `{self.change.parent.name}.{self.change.field_name}` has too many arguments " \
+                   f"({len(self.args)}>{self.limit}). Rule: {self.name}"
+
+    old_schema = schema("""
+    schema {
+        mutation: Mutation
+    }
+
+    type Mutation {
+        field: Int
+        mutation_with_too_many_args(
+            a1: Int, a2: Int, a3: Int, a4: Int, a5: Int, a6: Int, a7: Int, a8: Int, a9: Int, a10: Int 
+        ): Int
+    }
+    """)
+
+    new_schema = schema("""
+    schema {
+        mutation: Mutation
+    }
+
+    type Mutation {
+        field: Int
+        mutation_with_too_many_args(
+            a1: Int, a2: Int, a3: Int, a4: Int, a5: Int, a6: Int, a7: Int, a8: Int, a9: Int, a10: Int, a11: Int 
+        ): Int
+    }
+    """)
+
+    diff = Schema(old_schema, new_schema).diff()
+    # Type Int was also added but its ignored because its a primitive.
+    assert diff and len(diff) == 1
+    error_msg = (
+        'Field `Mutation.mutation_with_too_many_args` has too many arguments (11>10). '
+        'Rule: field-has-too-many-arguments'
+    )
+    assert evaluate_rules(diff, schema_restrictions) == ValidationResult(
+        ok=False,
+        errors=[ValidationError(error_msg)]
+    )
+    assert diff[0].path == 'Mutation.mutation_with_too_many_args'
+
+
+def test_mutation_input_fields_cant_share_prefix():
+    pass