Repository navigation
Conversation
|
| if base is None: | ||
| return self.backend.copy(dto, name) | ||
| return dto | ||
| return self._build_from_cached(dto, name, base, dto_config, tags=tags, backend_kwargs=backend_kwargs) |
There was a problem hiding this comment.
When an unscoped declaration already has the same fields, a later scope="schema" declaration takes this cached-build path and returns before declare_scope runs. The registry no longer stores scoped types, so relations declared afterward do not use that replacement. Another scoped declaration can also claim the same model and kind without raising ScopeConflictError.
Save the scoped declaration on the cached-build path too.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/strawchemy/dto/base.py
Line: 833
Comment:
**Schema scope is not saved**
When an unscoped declaration already has the same fields, a later `scope="schema"` declaration takes this cached-build path and returns before `declare_scope` runs. The registry no longer stores scoped types, so relations declared afterward do not use that replacement. Another scoped declaration can also claim the same model and kind without raising `ScopeConflictError`.
Save the scoped declaration on the cached-build path too.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Fixed in 06c7010: a declaration built from a cached one now declares its schema scope too, so relations use it and a second scoped declaration raises ScopeConflictError.
| dto_config.unset_sentinel, | ||
| dto_config.exclude_defaults, | ||
| ) | ||
| return (type(self), self.backend, model, base_key, node_key, config_key) |
There was a problem hiding this comment.
Generated settings are ignored
The request key omits registration, although shape matching includes it. Two generated make_input requests with the same model, name and config but different descriptions or directives therefore return the first registered type. This also happens with no_cache=True, which previously built a fresh type. The registry then skips the second request's settings.
Include registration in the request key so cached requests follow the same matching rules as shape reuse.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/strawchemy/dto/base.py
Line: 555
Comment:
**Generated settings are ignored**
The request key omits `registration`, although shape matching includes it. Two generated `make_input` requests with the same model, name and config but different descriptions or directives therefore return the first registered type. This also happens with `no_cache=True`, which previously built a fresh type. The registry then skips the second request's settings.
Include `registration` in the request key so cached requests follow the same matching rules as shape reuse.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Fixed in 42cede7: the request key includes the registration (description and directives), so a request with other settings gets its own type, with or without no_cache.
994c468 to
23b1a56
Compare
dbd4862 to
42cede7
Compare
23b1a56 to
dd887c8
Compare
9070250 to
b72d128
Compare
…oot input
BREAKING CHANGE: the nested `{Root}{Relation}Input` reached from a `pk_update_input` or `filter_update_input` now has the create shape (non-nullable columns required, generated keys optional, description "Create input"), and its own relations use create wrappers (`ToManyCreateInput`, no `remove`). Clients that sent partial nested `create` payloads under update mutations must send every required column.
… naming in the registry BREAKING CHANGE: generated types that expose the same shape under the same name are now one type, and a generated type whose shape differs from the one holding its name gets a path or numbered name instead of silently reusing it. Generated names such as `FruitOrderBy2` or `ColorDistinctOnFields` can therefore change; declare the type, or use `scope="schema"`, when a client depends on its name. `StrawberryRegistry` takes the mapper's `TypeIdentityCache`, `register_type` no longer takes `paginate`, `order`, `distinct_on` or `default_pagination`, `RegistryTypeInfo` loses the matching fields, and DTO backends no longer have `copy`.
94b36b7 to
8c2deb6
Compare
Problem
Two layers decided when two generated types are "the same", on different keys:
paginate,order,distinct_on);Each covered the other's blind spot by accident. The registry folded same-named types whatever their fields (#487, #493: the first
distinct_onlist or the first narrowed aggregate filter decided the enum for everyone), the DTO cache handed nested types built for one root to every other root (#518: nested pagination/ordering arguments depended on declaration order), and schema scope was handled twice. It also hid #494: nestedcreatepayloads under update inputs took the update shape, or the create shape, depending on which root input was declared first.Change
dto/identity.py,TypeIdentityCache), owned by the mapper and shared by every factory. It decides reuse:GraphQLFactory._register_built.scope="schema"stand-ins and raisesScopeConflictError.strawberry.type. Its dedup map (_type_map), scoped-type store and per-info option fields are gone, and so are the DTO backends'copy.createinputs of update inputs take the update shape, and leak it into create inputs #494: the related input of a relation is the payload ofcreate, so it is built in create mode under every root mode, and its back references to the root model point at a create input instead of the update root.paginate/order/distinct_onarguments.DistinctOnFieldsand aggregate argument enums take the shape of the first field or filter that builds them #493: fields with differentdistinct_onlists get their own enums, and the shared{Model}CountFieldsand argument enums are built from the mapper-wide selection rather than the declaring aggregate filter'sinclude.Breaking changes and migration
FruitOrderBy2becomesFruitOrderBy), generated types whose shape differs from the one holding a name get their own name (ColorDistinctOnFields1,ContainerType3FruitType), and the numbering of the remaining…Nnames shifts. Clients or persisted queries that spell generated names need updating; declare the type (or usescope="schema") when a client depends on its name.createpayloads underpk_update_input/filter_update_inputnow have the create shape: non-nullable columns are required, generated keys optional, description "Create input", and their own relations use create wrappers (ToManyCreateInput, noremove). Payloads that omitted required columns must now send them.paginate/order/distinct_oncarry those arguments, which adds root-prefixed nested types next to the plain ones.StrawberryRegistrytakes the mapper'sTypeIdentityCache;register_typeno longer takespaginate,order,distinct_onordefault_pagination;RegistryTypeInfoloses the matching fields;DTOBackend.copyand its overrides are removed;no_cacheonly skips the schema-scope stand-in.Tests
test_nested_relation_arguments_follow_their_root(fix: nested relation field arguments depend on the order root types are declared #518, every option, both orders),test_nested_create_payload_has_create_shapeandtest_nested_create_payload_back_reference_is_a_create_input(fix: nestedcreateinputs of update inputs take the update shape, and leak it into create inputs #494),test_generated_aggregate_count_enum_ignores_narrowed_declarationandtest_field_distinct_on_lists_get_their_own_enums(fix: generatedDistinctOnFieldsand aggregate argument enums take the shape of the first field or filter that builds them #493), andtests/unit/test_identity.pyfor shape matching and scope claims. All of them fail onfeat/scope-schema-only.field_order_by_*,order_config_*,model_field_options,paginate_and_order_combined), the splitColorDistinctOnFields1(distinct_config_with_field_override), the root-specific nested types ofpaginate_and_order_combined, and the create-shaped nested inputs oftest_mutation_schemas[update_mutation|upsert|table_mapped].Closes #494
Closes #518
Closes #493
Stacked on #517