-
-
Notifications
You must be signed in to change notification settings - Fork 565
feat(schema): semantic nullability draft #3722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
from strawberry.schema_directive import Location, schema_directive | ||
|
||
from .field import field | ||
|
||
|
||
@schema_directive( | ||
locations=[ | ||
Location.FIELD_DEFINITION, | ||
Location.OBJECT, | ||
Location.INTERFACE, | ||
Location.SCALAR, | ||
Location.ENUM, | ||
], | ||
name="semanticNonNull", | ||
print_definition=True, | ||
) | ||
class SemanticNonNull: | ||
levels: list[int] = field(default_factory=lambda: [1]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
import textwrap | ||
from typing import List, Optional, Required | ||
|
||
import strawberry | ||
from strawberry.schema.config import StrawberryConfig | ||
|
||
|
||
def test_entities_type_when_no_type_has_keys(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue (testing): Test name doesn't match the actual test content The test name suggests it's testing entity types without keys, but it appears to be testing semantic nullability behavior. Consider renaming the test to something like |
||
@strawberry.type() | ||
class Product: | ||
upc: str | ||
name: str | ||
price: Required[int] | ||
weight: Optional[int] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Missing test cases for semantic nullability behavior The test only verifies SDL generation but doesn't test the runtime behavior of Required and Optional fields. Consider adding test cases that verify: 1) Required fields actually enforce non-null values at runtime, 2) Optional fields accept null values, 3) Error cases when null is provided for Required fields. Suggested implementation: def test_entities_type_when_no_type_has_keys():
"""Test SDL generation for types with Required and Optional fields"""
=======
def test_required_field_rejects_null():
"""Test that Required fields raise an error when null is provided"""
@strawberry.type
class Product:
upc: str
name: str
price: Required[int]
weight: Optional[int]
schema = strawberry.Schema(
query=Product,
config=StrawberryConfig(semantic_nullability_beta=True)
)
with pytest.raises(ValueError) as exc:
Product(upc="123", name="test", price=None, weight=10)
assert "price cannot be null" in str(exc.value)
def test_optional_field_accepts_null():
"""Test that Optional fields accept null values"""
@strawberry.type
class Product:
upc: str
name: str
price: Required[int]
weight: Optional[int]
# Should not raise any errors
product = Product(upc="123", name="test", price=100, weight=None)
assert product.weight is None
def test_required_field_accepts_value():
"""Test that Required fields accept non-null values"""
@strawberry.type
class Product:
upc: str
name: str
price: Required[int]
weight: Optional[int]
# Should not raise any errors
product = Product(upc="123", name="test", price=100, weight=10)
assert product.price == 100 You'll need to:
|
||
|
||
@strawberry.federation.type(extend=True) | ||
class Query: | ||
@strawberry.field | ||
def top_products(self, first: int) -> List[Product]: | ||
return [] | ||
|
||
schema = strawberry.Schema( | ||
query=Query, config=StrawberryConfig(semantic_nullability_beta=True) | ||
) | ||
|
||
expected_sdl = textwrap.dedent(""" | ||
type Product { | ||
upc: String! | ||
name: String | ||
price: Int | ||
weight: Int | ||
} | ||
|
||
extend type Query { | ||
_service: _Service! | ||
topProducts(first: Int!): [Product!]! | ||
} | ||
|
||
scalar _Any | ||
|
||
type _Service { | ||
sdl: String! | ||
} | ||
""").strip() | ||
|
||
assert str(schema) == expected_sdl | ||
|
||
query = """ | ||
query { | ||
__type(name: "_Entity") { | ||
kind | ||
possibleTypes { | ||
name | ||
} | ||
} | ||
} | ||
""" | ||
|
||
result = schema.execute_sync(query) | ||
|
||
assert not result.errors | ||
|
||
assert result.data == {"__type": None} | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Missing return statement in StrawberryRequired branch
The code sets strawberry_semantic_required_non_null but doesn't return graphql_core_type, causing it to fall through to the else clause. Add 'return graphql_core_type' after setting the attribute.