docs: add implementation plan for record-based output (phase out DTO layer)#437
Draft
docs: add implementation plan for record-based output (phase out DTO layer)#437
Conversation
Collaborator
Author
|
Se https://github.com/sikt-no/graphitron/blob/26e651f49167017692e243ccd2820b6eb88a5fd7/docs/plan-phase-out-dto-layer.md for et forsøk på å forklare hva jeg forsøker å få til. |
…ng code touched) Adds the new record-based architecture as pure additive change on top of current main. No existing code is modified. The new architecture lives in no.sikt.graphitron.record.* and supporting packages. New production files: - GraphitronField/GraphitronType sealed interface hierarchy (field + type packages) - GraphitronSchemaBuilder: schema → GraphitronField classification - GraphitronSchemaValidator: validates the classified schema - JooqCatalog: thin jOOQ catalog wrapper for column/FK lookup - GraphitronFetcherFactory (graphitron-common): runtime fetcher factory - WatcherMojo (graphitron-maven-plugin): dev-mode watcher goal New test files: - GraphitronSchemaBuilderTest: @EnumSource truth-table tests for all field types - Validation tests for all 35 sealed interface leaf types (record/validation/) Plan and taxonomy docs: - docs/plan-phase-out-dto-layer.md - docs/field-taxonomy.md pom.xml: set compiler release to 21 (required for switch-pattern expressions used in GraphitronSchemaBuilder and GraphitronSchemaValidator). https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
alf
force-pushed
the
claude/review-dto-phase-out-OUTkz
branch
from
26e651f to
ceb76df
Compare
Adds a validation check that warns when a list or connection field is backed by a PK-less table and carries neither @defaultOrder nor @orderby enum values — a condition that makes result ordering non-deterministic. Changes: - QueryField.TableQueryField: add returnTypeName field so the validator can look up the return type's jOOQ table at validation time - GraphitronSchemaValidator.validateDeterministicOrdering(): new helper that checks List/Connection cardinality for PK-less tables with no ordering spec and emits a ValidationError - FieldValidationTestHelper.inQuerySchemaWithReturnType(): new helper that places a QueryField in a two-type schema (Query + return type), giving the validator access to the return type's table metadata - TableQueryFieldNonDeterministicOrderingValidationTest: 6 @EnumSource cases covering PK-less list, PK-less connection, PK-less with @defaultOrder, PK-less with @orderby, table-with-PK, and single cardinality (no check needed) - TableQueryFieldValidationTest: add returnTypeName="Film" to all existing construction sites (FILM has a PK → no new warnings) This replaces the excluded NonDeterministicOrderingValidationTest, which was testing the old ProcessedDefinitionsValidator via schema files. The new test operates entirely within the record architecture. https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
…nTest The validator looks up the return type to check the table's primary key for non-deterministic ordering. Without the return type in the schema the PK check silently skips — which masks any future test case that expects a non-deterministic ordering error. Switch inQuerySchema → inQuerySchemaWithReturnType(FILM_TYPE) so the Film type (which has a PK) is visible to the validator. All existing cases are unaffected since FILM has a PK and all List cases carry a defaultOrder or orderByValues. https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
A TableQueryField whose returnTypeName is absent from the schema is a
broken schema — not a condition to silently skip. The validator now
emits an error ("return type '...' does not exist in the schema") and
short-circuits further checks for that field.
Also refactors TableQueryFieldNonDeterministicOrderingValidationTest to
store the full GraphitronSchema in each enum constant (built at class
load time) so the UNKNOWN_RETURN_TYPE case can use inQuerySchema without
the return type, while all other cases use inQuerySchemaWithReturnType.
https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
Update docs/plan-phase-out-dto-layer.md to reflect five divergences between the plan's design sketches and the code that was implemented: - GraphitronType: add ErrorType to permits list and record definition - TableType: replace tableName field with NodeRef node; SQL name is now always on TableRef.tableName() for both resolved and unresolved variants - TableRef: ResolvedTable gains a leading tableName field; UnresolvedTable gains the same tableName field (was no-arg); tableName() is added to the interface so callers never need to reach into the parent type - TableInterfaceType: remove tableName field (now on TableRef); add List<ParticipantRef> participants - InterfaceType / UnionType: add List<ParticipantRef> participants - Update GraphitronSchemaBuilder example to match new constructor signatures - TableQueryField: document returnTypeName field added for non-deterministic ordering validation https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
Document Maven and Docker setup for agent sessions: - Maven 3.9.11 + Java 21 are pre-installed; ~/.m2/settings.xml carries a session-scoped HTTPS proxy — do not edit - `service docker start` fails (ulimit restriction); use `dockerd ... &` instead; document the correct invocation - Note which tests require Docker (TestContainers) vs which don't https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
Bugs fixed: - InterfaceTypeValidationTest, UnionTypeValidationTest: ALL_BOUND cases used UnresolvedTable inside BoundParticipant, contradicting the 'all bound' claim; replaced with ResolvedTable (ACTOR / CATEGORY) for all participants - Five mutation field tests: parentTypeName was "Query" instead of "Mutation"; add inMutationSchema() helper to FieldValidationTestHelper and use it - UNKNOWN_RETURN_TYPE case: was in the non-deterministic ordering test but tests the return-type-existence invariant; moved to TableQueryFieldValidationTest as a standalone @test so it can use a different schema setup Structural consistency: - TableQueryFieldNonDeterministicOrderingValidationTest: fields are now accessed via schema() / errors() accessor methods (private final fields, not package-private); add comment explaining why ValidatorCase is not implemented Coverage gaps closed: - TableQueryFieldValidationTest, TableFieldValidationTest: add Connection cardinality cases with unresolved index and unresolved primary key - TableMethodQueryField, TableInterfaceQueryField, InterfaceQueryField, UnionQueryField, TableInterfaceField, InterfaceField, UnionField: each had only a single VALID case; add LIST_UNRESOLVED_INDEX and LIST_UNRESOLVED_PRIMARY_KEY cases; rename VALID description from "always valid" to "single cardinality — valid" - TableMethodFieldValidationTest: add UNRESOLVED_KEY_AND_CONDITION case - ComputedFieldValidationTest, ServiceFieldValidationTest: add UNRESOLVED_KEY and UNRESOLVED_KEY_AND_CONDITION cases https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
…pers
The validator reads only the field's own data for all but two field types
(TableQueryField and NodeIdReferenceField). Remove the redundant
inQuerySchema/inMutationSchema/inTableTypeSchema wrappers from 28 test methods
and add a validate(GraphitronField) helper that builds the minimal schema
from the field's own parentTypeName and name.
- FieldValidationTestHelper: add validate(GraphitronField), keep inQuerySchema
(used by TableQueryFieldValidationTest.unknownReturnType), keep
inTableTypeSchema (used by NodeIdReferenceFieldValidationTest), remove
inMutationSchema and the previously used-only-in-tests table wrapper
- NodeIdFieldValidationTest: switch to validate(tc.field()) — the validator
reads field.node() directly; remove the custom schema() enum method
- All other field tests: validate(inXxxSchema("...", tc.field())) → validate(tc.field())
https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
…lookups The validator previously reached back into the schema for two field types. Both are fixed by embedding the resolved data in the field itself: TableQueryField: replace String returnTypeName with ReturnTypeRef (sealed: UnresolvedReturnType | ResolvedReturnType). ResolvedReturnType carries the jOOQ Table<?> for the PK-presence check; null when the table is unresolved. The validator switches on the field's ReturnTypeRef directly. NodeTypeRef.ResolvedNodeType: was an empty marker record. Now carries ResolvedTable targetTable and ResolvedTable parentTable (either may be null when the respective table is unresolved). The validator uses these for implicit FK counting and explicit path verification without touching the schema. GraphitronSchemaValidator: validateField loses the schema parameter; the schema loop no longer passes it. resolvedTableFor() is deleted. FieldValidationTestHelper: all schema-building helpers specific to field validation (inQuerySchema, inQuerySchemaWithReturnType, inTableTypeSchema, inTableTypeSchemaWithNodeTarget) are removed — every field now validates via validate(GraphitronField). TableQueryFieldNonDeterministicOrderingValidationTest and NodeIdReferenceFieldValidationTest both implement ValidatorCase. https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
… properties only ReturnTypeRef is redesigned: ResolvedReturnType replaced by TableBoundReturnType (carrying TableRef) and OtherReturnType, eliminating nullable Table<?> fields. Every non-scalar-returning field now carries a ReturnTypeRef so validators and generators have full return-type information without consulting the schema. NodeTypeRef is narrowed to carry only @node directive properties (NodeDirective) rather than the resolved tables of the target and parent types. The FK-validation data that was embedded in ResolvedNodeType now lives on NodeIdReferenceField as separate targetType (ReturnTypeRef) and parentTable (ResolvedTable) fields, following the same pattern as TableField and TableQueryField. The builder is updated with resolveReturnType() / resolveNodeType() helpers that populate the new fields, and the validator switches on TableBoundReturnType instead of ResolvedReturnType. All 38 validation test files are updated to match the new constructors. https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
…detection Replace the old FieldCardinality enum with a sealed FieldWrapper hierarchy that properly models all GraphQL type wrappers: - FieldWrapper.Single(boolean nullable) - FieldWrapper.List(boolean listNullable, boolean itemNullable, DefaultOrderSpec, List<OrderByEnumValueSpec>) - FieldWrapper.Connection(boolean connectionNullable, boolean itemNullable, DefaultOrderSpec, List<OrderByEnumValueSpec>) Key changes: - FieldWrapper tracks nullability at every level (outer and item), which was lost in the old FieldCardinality model - Connection detection is structural: checks for edges.node pattern in the GraphQL schema rather than relying on the "Connection" name suffix - For connection fields, returnType now resolves to the element type (edges.node) rather than the connection wrapper type — providing the correct jOOQ table ref - The same element-type resolution applies to @tableMethod fields with connection return types - FieldCardinality.java deleted; no remaining references https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
A field's GraphQL return type is a single concept: wrapper(elementType). Separating them into ReturnTypeRef + FieldWrapper was redundant and forced callers to always use both together. Merging them gives a single, complete description of a field's return type. Changes: - ReturnTypeRef gains a FieldWrapper wrapper() accessor; all three variants (TableBoundReturnType, OtherReturnType, UnresolvedReturnType) carry the wrapper as a record component - ChildField: TableField, TableMethodField, TableInterfaceField, InterfaceField, UnionField lose their separate `cardinality` field - QueryField: TableQueryField, TableMethodQueryField, TableInterfaceQueryField, InterfaceQueryField, UnionQueryField lose their separate `cardinality` field - GraphitronSchemaBuilder.resolveReturnType() now takes a FieldWrapper and embeds it directly; all call sites pass buildWrapper(fieldDef) or Single(true) for reference targets (NodeIdReferenceField.targetType) - GraphitronSchemaValidator replaces field.cardinality() with field.returnType().wrapper() throughout - All test files updated: wrapper construction moved inside ReturnTypeRef; field construction test-helpers (filmReturn, actorReturn) replace single shared constants to allow per-test wrapper variation https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
Replace the stale FieldCardinality section with the current design: - ReturnTypeRef now embeds FieldWrapper, documenting that a field's return type and its wrapper are a single unified concept - FieldWrapper documented with nullability (listNullable/itemNullable/ connectionNullable) and structural connection detection via edges.node - All FieldCardinality references in the generating stream and ordering deliverable sections updated to FieldWrapper https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
Replace all FieldCardinality references with current terminology: - Query fields table: 'FieldCardinality property' → 'Single / List / Connection' wrapper embedded in returnType; ServiceQueryField now documented as always-single - Child fields table: 'Cardinality is a FieldCardinality spec property' → 'Wrapper embedded in returnType' for TableField, TableMethodField, TableInterfaceField, InterfaceField, UnionField https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
…rtyField, TableInterfaceField, InterfaceField, UnionField
- Add DIR_SERVICE ("service") and DIR_EXTERNAL_FIELD ("externalField") constants
and validate them against the schema at build time.
- ResultType (@record) parents now dispatch to classifyChildFieldOnResultType():
@service → ServiceField, any other field → PropertyField (columnName from
@field(name:) or the GraphQL field name).
- TableType parents: @service → ServiceField and @externalfield → ComputedField
are checked before @tableMethod so the more-specific directives win.
- classifyObjectReturnChildField() now handles TableInterfaceType → TableInterfaceField,
InterfaceType → InterfaceField, UnionType → UnionField; the NestingField fallback
still covers plain unclassified object types.
- Builder tests: update UNCLASSIFIED_ON_RESULT_TYPE → PROPERTY_FIELD_ON_RESULT_TYPE
(now asserts PropertyField, not UnclassifiedField) and add new enum cases for every
P3 field type including ServiceField, ComputedField, InterfaceField, UnionField,
TableInterfaceField, and ServiceField on ResultType parents.
https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
…fication New data types: - ArgumentSpec record: name, typeName, nonNull, list, lookupKey, orderBy, conditionArg - InputFieldSpec record: name, typeName, nonNull, list, lookupKey, orderBy, columnName, javaName - InputType added to the GraphitronType sealed hierarchy (input object types) Record changes: - TableField gains List<ArgumentSpec> arguments - ServiceField gains List<ArgumentSpec> arguments + List<String> contextArguments - TableMethodField gains List<String> contextArguments Builder changes: - InputType classification via buildInputType() / buildInputFieldSpec() (@notGenerated fields excluded; @field name/javaName captured) - parseArguments() builds ArgumentSpec from each GraphQLArgument (@lookupKey, @orderby, @condition flags detected) - parseContextArguments() extracts contextArguments from @service / @tableMethod - New constants: DIR_LOOKUP_KEY, DIR_ORDER_BY, DIR_CONDITION, ARG_CONTEXT_ARGUMENTS (all validated in validateDirectiveSchema) Validator changes: - validateType() switch gains InputType case - validateInputType(): each InputFieldSpec.typeName checked against known types - validateArguments(): each ArgumentSpec.typeName checked against known types - validateTableField / validateServiceField updated to pass types map through Tests: - ArgumentParsingCase enum: TableField args, @lookupKey/@orderby detection, ServiceField.contextArguments, TableMethodField.contextArguments - InputTypeCase enum: scalar fields, @field(name:), @notGenerated exclusion, list types - ArgumentValidationTest: builtin scalars OK, known InputType OK, unknown type → error - InputTypeValidationTest: scalar fields OK, unknown type → error, nested input OK - TableField / ServiceField / TableMethodField validation tests updated for new fields https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
… hierarchies - Remove lookupKey from ArgumentSpec (it is a field-level classifier, not per-argument semantic); update all call sites - Add arguments/contextArguments parameters to QueryField subtypes: LookupQueryField, TableQueryField, TableMethodQueryField, ServiceQueryField - Add classifyQueryField() and classifyMutationField() in GraphitronSchemaBuilder to classify Query/Mutation root fields into the sealed hierarchy; lookup detection uses hasLookupKeyAnywhere() which recurses into input type fields - Add validateLookupQueryField() in GraphitronSchemaValidator enforcing: single return cardinality, no @orderby args, no @condition args - Update all affected tests: GraphitronSchemaBuilderTest (15 new RootFieldCase enum cases), LookupQueryFieldValidationTest (7 cases), plus updated constructors in TableQueryField*, TableMethodQueryField*, ServiceQueryField*, and ArgumentValidation tests https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
- Mark P3, P4, P5 as ✓ in deliverables table - Remove lookupKey from ArgumentSpec and InputFieldSpec definitions; add note explaining it is a field-level classifier only — storing it per-arg would mislead generator authors into treating @lookupKey args differently - Add P5 section documenting classification priority, lookupKey semantics, LookupQueryField validation constraints, and mutation classification - Remove superseded @lookupKey validation rule (was: must not appear on non-scalar types; replaced by lookup field validation in validator) - Fix Level 1 test example to match actual LookupQueryFieldValidationTest shape https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
- Add ExternalRef record to carry ExternalCodeReference data (className + methodName) from @service and @tableMethod directives - Add serviceRef: ExternalRef to ServiceQueryField, ServiceMutationField, ServiceField — stores the @service(service:) reference - Add tableMethodRef: ExternalRef to TableMethodQueryField, TableMethodField — stores the @tableMethod(tableMethodReference:) reference - Add arguments: List<ArgumentSpec> to all MutationField variants; add contextArguments/serviceRef to ServiceMutationField (was missing both) - Add arguments: List<ArgumentSpec> to TableMethodQueryField and TableMethodField (contextArguments fields already had them, but regular arguments were absent) - Add discriminatorValue: String to ParticipantRef.BoundParticipant — captures @Discriminator(value:) from implementing types; null when absent - Change InputFieldSpec.javaName: String → javaNamePresent: boolean — @field(javaName:) is deprecated; store presence only for validation warning, matching the existing pattern in ColumnField - Fix ComputedField Javadoc: @computed → @externalfield - Update builder to extract and populate all new fields; add DIR_DISCRIMINATOR and associated constants; update validateDirectiveSchema assertions - Update all affected tests to use new constructor signatures https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
lookupKey is a field-level classifier, not a per-input-field semantic. hasLookupKeyAnywhere() reads directly from the live GraphQL schema (GraphQLInputObjectType), never from InputFieldSpec, so storing it there was dead code that would mislead future generator authors. Also update plan to reflect all recent changes: ExternalRef, serviceRef, tableMethodRef, discriminatorValue on BoundParticipant, arguments on MutationField variants, javaNamePresent semantics. https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
- Add InputType to GraphitronType permits list and D1 code snippet - Add MultitableReferenceField to ChildField permits list - ParticipantRef section was already added in previous session https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
- Remove UnresolvedReturnType from ReturnTypeRef: graphql-java guarantees all field return type references are valid; directive-argument string type refs (e.g. @nodeid typeName) fall through to OtherReturnType gracefully - Remove redundant type-existence checks from validateArguments/validateInputType: graphql-java already enforces this at schema assembly time; custom scalars and enums were incorrectly flagged as unknown types - Standardize arguments/contextArguments order: TableMethodQueryField and TableMethodField now put arguments before contextArguments, matching the service-family convention - Fix UnresolvedKeyRef("") to use a descriptive placeholder message - Remove ARG_SORT_FIELD_NAME duplicate constant (same value as ARG_NAME) - Replace Collectors.toList() with .toList() in argStringList for immutability - Fix @computed → @externalfield in ConditionOnlyRef Javadoc - Fix "warning" → "error" in InputFieldSpec Javadoc for javaName deprecation - Remove duplicate @code{arguments} sentence from ServiceQueryField Javadoc - Add comment explaining ConstructorField is intentionally deferred to UnclassifiedField - Remove test cases that tested now-impossible states https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
…NoNodeDirectiveType Directive argument string values (like @nodeid typeName:) are not validated by graphql-java, so the referenced type may genuinely not exist. Distinguishing 'type not in schema' from 'type exists but lacks @node' allows the validator to produce targeted error messages: - "type 'X' does not exist in the schema" (NotFoundNodeType) - "type 'X' does not have @node" (NoNodeDirectiveType) instead of the previous catch-all "does not exist or does not have @node". resolveNodeType() now checks schema.getType() first to detect the missing-type case before consulting the classified types map. https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
Replace duplicated code blocks with links to source files and concise prose. Key changes: - All taxonomy sections now link to the actual .java files - ReturnTypeRef: removed UnresolvedReturnType (never existed in code) - Added NodeTypeRef section (3-way split, rationale for string-arg validation) - JooqCatalog: replaced large code block with description + link - P4 validation: removed stale type-existence check items (graphql-java guarantees these) - P4 InputFieldSpec: "warning" → "error" for javaNamePresent - FieldWrapper, GraphitronSchema: keep non-obvious design notes, add links https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
- GraphitronType tree: add ErrorType and InputType (were missing) - GraphitronField tree: add MultitableReferenceField to ChildField permits - Add source file links to GraphitronType and GraphitronField sections - ComputedField: @computed → @externalfield (correct directive name) - Child fields intro: remove stale "sourceContext property" claim; source context is derived at generation time, not stored on the record - MultitableReferenceField: add to child field table and matrix as unsupported https://claude.ai/code/session_012BHSnrKwDL1zddc2ico7pD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds docs/plan-phase-out-dto-layer.md — the design document for the
recordBasedOutputfeature flag and the new generator pipeline(Steps 0–8: ConditionWrapper, ServiceWrapper, JooqQueryClassGenerator,
DataLoaderClassGenerator, RuntimeWiringClassGenerator,
OperationRuntimeWiringClassGenerator, RecordWiringClassGenerator,
orchestration). Replaces the append-based snapshot workaround in the
Claude plan file with proper git history.
https://claude.ai/code/session_01HGqpHE7fXNuwBiM8fvpVia