Conversation
Adds a transport-agnostic GraphQL-lite bridge over existing CoreEx.Data dynamic querying (QueryArgs/PagingArgs), with: - CoreEx.Data.GraphQL: engine, DI-driven root registration, selection-set to JsonFilter projection, arg mapping, and schema/discovery generation. - CoreEx (src): IGraphQLEngine/GraphQLEngineResult/GraphQLEngineError contracts. - CoreEx.AspNetCore: MapCoreExGraphQLLite minimal-API hosting bridge. - Contoso.Products sample wiring + integration tests. - Unit tests for the engine, arg mapper and value converter. - Updated coreex-conventions.instructions.md with explicit private-method and access-level XML doc requirements. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… aliases at every depth Closes the two most impactful GraphQL-ecosystem interop gaps found in review: - __typename is now resolvable at root and any nested selection depth. Without this, any standard client (Apollo Client, Relay, urql) that auto-injects __typename into every selection set would get a spurious error on every single request. - Fragment spreads/inline fragments now produce an explicit FRAGMENTS_NOT_SUPPORTED error instead of being silently skipped, which previously risked returning incomplete data with a misleading 200/no-errors response. - Field aliases are now honored at every selection depth (not just root) via a new GraphQLResponseShaper reshaping pass, fixing a real correctness bug where nested aliases were silently ignored. Updated README non-goals to accurately describe remaining gaps (no standard introspection/SDL, no directives/interfaces/unions) and added unit test coverage for all three fixes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… paging
Replace the initial filter/orderby/skip/take GraphQL argument convention
with GraphQL-idiomatic structured input:
- where: field-keyed operator objects (or bare-scalar equality shorthand)
composed via and/or/not, translated 1:1 to the OData-esque filter string
consumed by the entity's existing QueryArgsConfig (GraphQLFilterTranslator).
- orderBy: a list of field/direction objects, translated to the equivalent
orderby string (GraphQLOrderByTranslator).
- Relay Cursor Connections paging (edges { node cursor } pageInfo totalCount)
via first/after forward pagination, backed by an opaque offset cursor
(GraphQLCursor). last/before are explicitly rejected (out of scope for v1).
Both translators are pure syntax rewrites with zero field/operator
legality checks by design -- real validation happens downstream, exactly
as it does today for the REST $filter/$orderby query strings, via the
same QueryArgsConfig parse pipeline. This preserves exact behavioral
parity with the existing OData-esque querying rather than introducing a
second, parallel validation surface.
Also:
- GraphQLSchemaBuilder discovery document now reports where/orderBy shapes
(sourced from QueryArgsConfig.ToJsonSchema()) and the fixed Connection/
Edge/PageInfo shape for query roots.
- GraphQLEngine.MapException maps GraphQLArgumentTranslationException to
an ARGUMENT_ERROR code.
- Full unit test rewrite/expansion (78 tests) covering translators, cursor
round-tripping, and engine-level where/orderBy/paging behavior against a
real QueryArgsConfig.Parse pipeline (not string hacks).
- Sample Contoso.Products GraphQL-lite integration tests updated to the new
query/response shape; verified against live Postgres infra.
- README updated for the new capabilities and non-goals (no last/before,
no standard introspection).
The item-root GraphQL-lite test suite only covered the not-found path; add the missing success-path counterpart, asserting the same known seeded product (sku/text) that the REST Product_Get_Found test already verifies, for parity coverage across both bridges.
The repo's .editorconfig naming rule (async_methods_async_suffix) requires all async methods -- test methods included -- to end with 'Async'. Several GraphQLEngineTests methods contained 'Async' mid-name (e.g. describing the method under test) but did not end with it, triggering IDE1006 naming warnings. Renamed all 23 affected test methods to end with the required suffix; no behavioral change.
Add src/CoreEx.Data.GraphQL/AGENTS.md matching the established per-package AI Usage Guide pattern (registration, query syntax example, Do Not rules, Further Reading), and cross-link it from the README's new 'AI Usage Guide' section, consistent with every other src/*/AGENTS.md-bearing package. Also reviewed the README end-to-end against the current implementation (GraphQLEngine, GraphQLLiteOptions, GraphQLQueryRoot/ GraphQLItemRoot, all Internal/* translators/resolvers, and the CoreEx.AspNetCore MapCoreExGraphQLLite hosting bridge) -- confirmed accurate, no content changes required beyond the new AI Usage Guide section.
…ntroller, and capabilities docs - coreex-expert.agent.md: add CoreEx.Data.GraphQL row to per-package AI usage guide table - coreex-host-setup.instructions.md: add optional AddCoreExGraphQLLite/MapCoreExGraphQLLite registration to API host table and key points - coreex-api-controllers.instructions.md: cross-link GraphQL-lite as an optional alternative to a REST query endpoint - docs/capabilities.md: add GraphQL-lite Query Bridge subsection under Data Access & Persistence Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…older-placement convention - coreex-repositories.instructions.md: document the public-visibility exception for QueryArgsConfig types backing a CoreEx.Data.GraphQL root, and the colocation-with-repository convention (5+ config escalation threshold) vs. a dedicated Query/ subfolder - ProductQueryArgsConfig.cs: add the required one-line justification comment for its public visibility, per the new convention Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Parse GraphQL int/float literals with InvariantCulture (was CurrentCulture, causing 9.99 to be misparsed as 999 on e.g. de-DE machines). - Require a selection set for object-typed leaf fields (item roots, node, and nested complex properties) instead of silently returning the whole unfiltered DTO; raises a new SELECTION_REQUIRED error. - Throw GraphQLArgumentTranslationException for an undefined \ reference instead of silently resolving to null, and make sure this (and any other argument-conversion error) is caught and mapped to an ARGUMENT_ERROR in ExecuteAsync's main loop rather than propagating unhandled - a real bug this fix exposed that had no prior coverage. - Guard GetInt against unchecked long-to-int truncation, throwing instead of silently wrapping out-of-range values. - Reject duplicate root-level response-key aliases (DUPLICATE_FIELD) rather than letting the last selection silently win. - Add a needsItems flag to BuildConnectionPagingArgs so a totalCount-only selection caps Take at 1 instead of over-fetching a full page of rows that get discarded. - Key the GraphQLTypeShape field-map cache by (Type, JsonSerializerOptions) instead of Type alone, so a second engine with different JSON options cannot get the wrong cached shape. - Let OperationCanceledException propagate out of the engine's root resolvers instead of being swallowed into an EXECUTION_ERROR. Adds regression tests for all of the above. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…edService pattern
The sample's Program.cs was simplified to resolve scoped dependencies via
CoreEx.ExecutionContext.GetRequiredService<T>() (reads the ambient scoped
service provider set by UseExecutionContext() middleware, which every
CoreEx host already registers) instead of an explicit
IHttpContextAccessor/HttpContext.RequestServices dance. Bring the
GraphQL-lite README, AGENTS.md, XML doc remarks, host-setup instructions,
and samples/docs/hosts-layer.md usage snippets in line with it.
Also corrects docs/capabilities.md's GraphQL-lite example, which had
drifted from the shipped API: AddCoreExGraphQLLite takes a two-parameter
(GraphQLLiteOptions, IServiceProvider) delegate, AddQuery/AddGet take
(QueryArgs?, PagingArgs?, ct) / (args dictionary, ct) - not a wrapped
RequestOptions object or a typed id parameter - and orderBy uses the
field-name-as-key shape ({ name: ASC }), not { field: "name" }.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ExecuteAsync_ResolverThrowsOperationCanceled_PropagatesRatherThanBecomingAnEngineError was a void test method calling ThrowAsync(...) without awaiting the returned Task, so NUnit never observed the assertion outcome - it would pass even if the OperationCanceledException were swallowed into an EXECUTION_ERROR result. Made the method async Task and awaited the assertion (and added the Async suffix per convention). Verified by temporarily reverting the production fix this test targets: it now fails as expected, then passes once reverted back. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces CoreEx.Data.GraphQL, a transport-agnostic “GraphQL-lite” query bridge that translates GraphQL where/orderBy and Relay-style paging onto the existing QueryArgsConfig + QueryArgs/PagingArgs pipeline, plus a minimal-API hosting bridge in CoreEx.AspNetCore and sample usage/tests/docs.
Changes:
- Add new
CoreEx.Data.GraphQLpackage (engine, translators, selection/projection, schema discovery) and newCoreEx-level contracts/result envelope types. - Add ASP.NET Core minimal-API bridge (
MapCoreExGraphQLLite) and wire it into the Contoso Products sample with matching integration tests. - Update solution/test filters, dependency versions, and documentation/instructions to reflect the new capability and registration pattern.
Reviewed changes
Copilot reviewed 54 out of 54 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/CoreEx.Data.GraphQL.Test.Unit/Model/PersonQueryArgsConfig.cs | Minimal QueryArgsConfig used by unit tests to exercise end-to-end translation/parsing. |
| tests/CoreEx.Data.GraphQL.Test.Unit/Model/Person.cs | Test DTO shape for nested selection/projection tests. |
| tests/CoreEx.Data.GraphQL.Test.Unit/Internal/GraphQLValueConverterTests.cs | Unit coverage for argument conversion (literals, variables, culture, overflow). |
| tests/CoreEx.Data.GraphQL.Test.Unit/Internal/GraphQLOrderByTranslatorTests.cs | Unit coverage for orderBy translation and validation errors. |
| tests/CoreEx.Data.GraphQL.Test.Unit/Internal/GraphQLFilterTranslatorTests.cs | Unit coverage for where translation and operand escaping/validation errors. |
| tests/CoreEx.Data.GraphQL.Test.Unit/Internal/GraphQLCursorTests.cs | Unit coverage for cursor encoding/decoding behavior. |
| tests/CoreEx.Data.GraphQL.Test.Unit/Internal/GraphQLArgsMapperTests.cs | Unit coverage for mapping GraphQL args to QueryArgs/PagingArgs (incl. count-only path). |
| tests/CoreEx.Data.GraphQL.Test.Unit/GraphQLEngineTests.cs | End-to-end engine tests (projection, paging, schema, errors, cancellation). |
| tests/CoreEx.Data.GraphQL.Test.Unit/CoreEx.Data.GraphQL.Test.Unit.csproj | New multi-targeted unit test project for GraphQL-lite engine. |
| src/CoreEx/Data/GraphQL/IGraphQLEngine.cs | New transport-agnostic engine contract in CoreEx. |
| src/CoreEx/Data/GraphQL/GraphQLEngineResult.cs | CoreEx-level result envelope for GraphQL-over-HTTP-style responses. |
| src/CoreEx/Data/GraphQL/GraphQLEngineError.cs | CoreEx-level error envelope (message/path/extensions). |
| src/CoreEx.Data.GraphQL/README.md | Package documentation (capabilities, usage, non-goals). |
| src/CoreEx.Data.GraphQL/Internal/GraphQLValueConverter.cs | AST + variables → CLR argument conversion and typed accessors. |
| src/CoreEx.Data.GraphQL/Internal/GraphQLTypeShape.cs | Reflected DTO field shape caching keyed by serializer options. |
| src/CoreEx.Data.GraphQL/Internal/GraphQLSelectionResolver.cs | Selection-set validation + JsonFilter include-path flattening. |
| src/CoreEx.Data.GraphQL/Internal/GraphQLSchemaBuilder.cs | Discovery document builder composing QueryArgsConfig schema + DTO field shape. |
| src/CoreEx.Data.GraphQL/Internal/GraphQLResponseShaper.cs | Alias-aware reshaping plus __typename injection post-JsonFilter. |
| src/CoreEx.Data.GraphQL/Internal/GraphQLOrderByTranslator.cs | GraphQL orderBy list-of-objects → OData-esque order-by string. |
| src/CoreEx.Data.GraphQL/Internal/GraphQLFilterTranslator.cs | GraphQL where input object → OData-esque filter string. |
| src/CoreEx.Data.GraphQL/Internal/GraphQLCursor.cs | Offset-based cursor codec for Relay forward pagination. |
| src/CoreEx.Data.GraphQL/Internal/GraphQLConnectionResolver.cs | Validates/records fixed Connection/Edge/PageInfo selection shapes. |
| src/CoreEx.Data.GraphQL/Internal/GraphQLArgumentTranslationException.cs | Dedicated exception type for GraphQL arg-shape translation failures. |
| src/CoreEx.Data.GraphQL/Internal/GraphQLArgsMapper.cs | Maps resolved args into QueryArgs + paging semantics (incl. count-only optimization). |
| src/CoreEx.Data.GraphQL/GraphQLServiceCollectionExtensions.cs | DI registration extension for singleton IGraphQLEngine. |
| src/CoreEx.Data.GraphQL/GraphQLQueryRoot.cs | Query-root descriptor binding root name + config + resolver. |
| src/CoreEx.Data.GraphQL/GraphQLLiteOptions.cs | Options container for explicit root registration (AddQuery/AddGet). |
| src/CoreEx.Data.GraphQL/GraphQLItemRoot.cs | Single-item root descriptor binding root name + resolver. |
| src/CoreEx.Data.GraphQL/GraphQLEngine.cs | Concrete engine implementation (parse, execute roots, apply projection, map errors). |
| src/CoreEx.Data.GraphQL/GlobalUsing.cs | Global usings for the new package. |
| src/CoreEx.Data.GraphQL/CoreEx.Data.GraphQL.csproj | New package project definition and dependencies. |
| src/CoreEx.Data.GraphQL/AGENTS.md | AI usage guide for the new package (registration and do-not rules). |
| src/CoreEx.AspNetCore/GlobalUsing.cs | Adds global using for GraphQL-lite engine contract types. |
| src/CoreEx.AspNetCore/CoreExAspNetCoreExtensions.GraphQLLite.cs | Minimal-API bridge endpoint mapping GraphQL-over-HTTP envelope to IGraphQLEngine. |
| samples/tests/Contoso.Products.Test.Api/ReadTests.ProductGraphQLLite.cs | Sample integration tests proving parity with REST query behavior. |
| samples/src/Contoso.Products.Infrastructure/Repositories/ProductQueryArgsConfig.cs | Makes config public for host-level GraphQL-lite root registration. |
| samples/src/Contoso.Products.Api/Program.cs | Registers engine roots and maps /api/products/query endpoint. |
| samples/src/Contoso.Products.Api/GlobalUsing.cs | Adds global usings for GraphQL-lite registration and config access. |
| samples/src/Contoso.Products.Api/Contoso.Products.Api.csproj | Adds project reference to CoreEx.Data.GraphQL. |
| samples/docs/patterns.md | Documents GraphQL-lite query bridge as an API pattern. |
| samples/docs/hosts-layer.md | Documents host wiring and usage constraints for the /query bridge. |
| README.md | Adds the new package to the repo/package catalog. |
| docs/capabilities.md | Adds “GraphQL-lite Query Bridge” capability documentation. |
| Directory.Packages.props | Adds GraphQL-Parser dependency version. |
| CoreEx.slnx | Adds new project + tests to solution index. |
| CoreEx.sln | Adds new projects to the Visual Studio solution. |
| CoreEx.Core.Test.Parallel.slnf | Includes new unit test project in the parallel test filter. |
| CoreEx.Core.slnf | Includes new project + tests in core solution filter. |
| AGENTS.md | Updates agent entry-point doc to include new data/query packages. |
| .github/instructions/coreex-repositories.instructions.md | Documents QueryArgsConfig visibility exception for GraphQL-lite host registration. |
| .github/instructions/coreex-host-setup.instructions.md | Documents GraphQL-lite registration and mapping APIs for hosts. |
| .github/instructions/coreex-conventions.instructions.md | Updates documentation conventions guidance (incl. private methods, test exceptions). |
| .github/instructions/coreex-api-controllers.instructions.md | Notes optional GraphQL-lite surface alongside REST query endpoints. |
| .github/agents/coreex-expert.agent.md | Adds new package guide link for the expert agent references. |
Comments suppressed due to low confidence (1)
src/CoreEx.Data.GraphQL/Internal/GraphQLValueConverter.cs:94
GetBoolreturnsnullfor unsupported types, which can silently ignore invalid inputs (especially viavariables) rather than returning anARGUMENT_ERROR. Consider throwingGraphQLArgumentTranslationExceptionfor any non-null, non-boolean value.
…late package assets - nuget-publish.ps1: add src\CoreEx.Data.GraphQL to ProjectsToPublish (was missing, so it never got packed/published). - CoreEx.Template: add CoreEx.Data.GraphQL AGENTS.md to the coreex-ai docs-cache copy list, and a CoreEx.Data.GraphQL PackageVersion entry to the scaffolded solution's central _Directory.Packages.props so consumers can add the package with a resolvable version. - Update the docs-sync skill file lists and package-guide counts (16 -> 17) across SKILL.md/README.md/coreex-ai-workflows.md/agents/README.md to include CoreEx.Data.GraphQL.md. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 61 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
src/CoreEx/Data/GraphQL/GraphQLEngineResult.cs:36
GraphQLEngineResult.Failureis declared asparams IEnumerable<GraphQLEngineError>but call sites pass a singleGraphQLEngineError(e.g.Failure(NewError(...))), which won’t compile. Useparams GraphQLEngineError[](or a non-paramsIEnumerable<...>parameter) so the call sites are valid.
src/CoreEx.Data.GraphQL/GraphQLEngine.cs:138- When
edges/nodeare aliased (supported byGraphQLConnectionResolver), the errorPathcurrently uses the literal field names ("edges","node") rather than the response keys (aliases). This makes errors point at the wrong JSON path for clients using aliases; use the resolved aliases in theerrorPathpassed toGraphQLSelectionResolver.Resolve.
src/CoreEx.Data.GraphQL/Internal/GraphQLValueConverter.cs:79 GetIntreturnsnullfor unsupported value types, which can silently turn an invalid argument (e.g.first: "abc"or a wrong-typed variable) into “not specified”, falling back to defaults or even aNOT_FOUND. For GraphQL arguments this should be anARGUMENT_ERRORso callers get deterministic feedback.
A single IGraphQLEngine instance dispatches to an arbitrary set of registered roots (AddQuery/AddGet), so the entity is identified by the request body's query, not the URL. Nesting the route under /products/ implied REST-style one-endpoint-per-resource scoping, contradicting the engine's multi-root design and the MapCoreExGraphQLLite default of '/query'. Renamed across the sample host, its integration tests, and the READMEs/AGENTS.md/hosts-layer.md that documented the old path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 61 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
src/CoreEx/Data/GraphQL/GraphQLEngineResult.cs:36
GraphQLEngineResult.Failureis declared asparams IEnumerable<GraphQLEngineError>but all call sites pass a singleGraphQLEngineError(e.g.GraphQLEngine.ExecuteAsync), which will not compile. The params type should beGraphQLEngineError[](or accept a singleIEnumerable<GraphQLEngineError>withoutparams).
src/CoreEx.Data.GraphQL/README.md:70- The Key types table says
IGraphQLEngine,GraphQLEngineResult, andGraphQLEngineErrorare in theCoreEx.Datanamespace, but the types added in this PR are inCoreEx.Data.GraphQL. This should be corrected so consumers can locate the API quickly.
…tInt/GetBool validation, DateOnly/TimeOnly scalars, doc fixes) - GraphQLValueConverter.GetInt/GetBool now throw GraphQLArgumentTranslationException for invalid/unsupported values instead of silently returning null. - GraphQLTypeShape now classifies DateOnly/TimeOnly as scalar, matching QueryFilterFieldConfigBase. - GraphQLSelectionResolver, GraphQLConnectionResolver and GraphQLEngine now build GraphQLEngineError.Path from the client's requested alias (not the raw field name) at every nesting level, so error paths always match the actual response JSON. - Fixed stale single-arg AddCoreExGraphQLLite(o => ...) examples in the AspNetCore XML remarks and GraphQL README to the actual two-arg (o, sp) => ... signature. - Added regression tests for each fix. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 62 out of 62 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
src/CoreEx/Data/GraphQL/GraphQLEngineResult.cs:36
GraphQLEngineResult.Failurecurrently takesparams IEnumerable<GraphQLEngineError>which makes call sites likeGraphQLEngineResult.Failure(NewError(...))a compile error (a singleGraphQLEngineErroris not anIEnumerable<GraphQLEngineError>). Change this overload to acceptparams GraphQLEngineError[](or a singleIEnumerable<GraphQLEngineError>withoutparams).
src/CoreEx.Data.GraphQL/README.md:20- README describes the
IGraphQLEnginecontract as being in theCoreEx.Datanamespace, but the actual contract isCoreEx.Data.GraphQL(in theCoreExproject). This is likely to mislead consumers and should be corrected.
src/CoreEx.Data.GraphQL/README.md:65 - The Key types table lists the wrong
AddCoreExGraphQLLitesignature; the extension takesAction<GraphQLLiteOptions, IServiceProvider>(as shown in the usage snippet), notAction<GraphQLLiteOptions>.
src/CoreEx.Data.GraphQL/README.md:70 - The Key types table says
IGraphQLEngine/GraphQLEngineResult/GraphQLEngineErrorare in theCoreEx.Datanamespace, but they are defined inCoreEx.Data.GraphQL. Aligning this avoids confusion when searching/importing types.
…signature docs, unused response param) - README incorrectly stated IGraphQLEngine/GraphQLEngineResult/GraphQLEngineError live in the CoreEx.Data namespace; they are declared under CoreEx.Data.GraphQL. Fixed both mentions. - README Key types table still showed the stale single-arg AddCoreExGraphQLLite(IServiceCollection, Action<GraphQLLiteOptions>) signature; updated to the actual two-arg Action<GraphQLLiteOptions, IServiceProvider>. - Removed the unused HttpResponse response minimal-API parameter from MapCoreExGraphQLLite (the handler always writes via request.HttpContext). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…d-name validation, docs) - GraphQLArgsMapper.BuildConnectionPagingArgs now returns a RequiresTotalCountForHasNextPage flag and forces IsCountRequested when PagingArgs.MaximumTake is so small that the usual one-item over-fetch is structurally impossible (MaximumTake <= first). GraphQLEngine falls back to a TotalCount-based hasNextPage derivation in that case. Added a regression test for MaximumTake = 1. - Added GraphQLNameValidator to validate where/orderBy field-name dictionary keys against the GraphQL Name grammar before composing OData-esque filter/order-by strings, closing an injection/malformed-string risk when arguments arrive via JSON variables. - GraphQLConnectionResolver's "unknown field" error messages now list __typename alongside the other allowed edge/pageInfo/root fields. - Doc/wording fixes: GlobalUsing.cs ordering, AddGet cref signature drift (Service Collection extensions + GraphQLItemRoot), GraphQLCursor remarks (offset: not arrayconnection:), and GraphQLOrderByTranslator remarks recommending single-key-per-item orderBy objects. - Made MovementQueryArgsConfig/OrderQueryArgsConfig public, aligning the samples with the "QueryArgsConfig is always public" guidance in coreex-repositories.instructions.md. - Reverted non-functional CoreEx.sln header churn (BOM-only line, VisualStudioVersion suffix). - Fixed ReadTests.ProductGraphQLLite's where/orderBy variable types to the actual generated ProductLiteWhereInput/ProductLiteOrderByInput names. - Added Products integration tests exercising the standard client-tooling introspection query (fragments + all IntrospectionQuery fields) and __type(name:) lookups, validating GraphQL schema discovery works end-to-end over HTTP. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ERROR; harden AddGet id validation - GraphQLEngine.MapException now classifies ArgumentException (and subtypes) and KeyNotFoundException as ARGUMENT_ERROR rather than the generic EXECUTION_ERROR, since resolver delegates commonly throw these standard .NET types to reject a missing/invalid argument. - Contoso.Products.Api's AddGet<Product> resolver now validates the 'id' argument explicitly via TryGetValue + a non-empty string pattern, throwing ArgumentException instead of relying on an indexer + null-forgiving lookup that could surface as an opaque EXECUTION_ERROR or an unhandled NullReferenceException. - Updated the matching README.md/AGENTS.md/docs/capabilities.md/samples/docs/hosts-layer.md/ coreex-host-setup.instructions.md code samples to the safer pattern. - Added GraphQLEngineTests coverage for the new ARGUMENT_ERROR mappings and confirmed the EXECUTION_ERROR fallback still applies to unmapped exception types. - Refactored GraphQLLite_Introspection_StandardClientQuery_Succeeds to load its query from an embedded resource (Resources/ReadTests/ProductGraphQLLite_StandardIntrospectionQuery.graphql) instead of an inline C# raw string literal, matching existing test conventions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…Filter JsonFilter.CreateDictionary demoted a field to 'intermediary' (non-leaf, dropped from Include output) whenever any other requested field's name was a raw string prefix of it, with no path-segment-boundary check. This meant requesting sibling scalar fields such as 'category'/'categoryText' (a common CoreEx reference-data Code/Text pair) silently dropped 'category' from the result, regardless of request order, since 'categoryText' textually starts with 'category'. Add IsProperDescendantPath, which additionally requires the character immediately following the shorter path to be '.' or '[' (a genuine nested path-segment boundary) before demoting. This affects any consumer of JsonFilter.TryJsonFilter, including REST \ projection and GraphQL-lite field selection. Add regression tests covering both the internal CreateDictionary heuristic and the end-to-end TryJsonFilter behaviour for the category/categoryText case. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…hread selected fields into list-root QueryArgs; endpoint configuration hook - GraphQLIntrospectionSchemaBuilder: AddGet (item) root fields now advertise includeText/includeInactive arguments alongside the existing id: ID!, matching the behaviour GraphQLEngine already honours for item roots via GraphQLArgsMapper.BuildQueryArgs. Previously this worked at runtime but was invisible to schema-aware tooling (Postman, Nitro, Apollo Sandbox) since it wasn't declared in introspection. - GraphQLEngine: list-root execution now sets queryArgs.IncludeFields from the resolved GraphQL selection paths, so the underlying QueryArgs-driven data layer can project only the fields actually selected by the request. - GraphQLQueryRoot: log query/paging args at Debug level (when enabled) for easier local troubleshooting of list-root invocations. - PagingArgs/QueryArgs: add ToString() overrides to support the new debug logging with a readable summary. - MapCoreExGraphQLLite: accept an optional configure callback so callers can further customise the mapped endpoint (e.g. auth, rate limiting), and tag the endpoint with a display name/"GraphQL" group for discoverability. Update README/AGENTS docs and GraphQLEngineTests to reflect the item-root schema change. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…esponse ExecuteAsync captures jsonOptions = JsonDefaults.SerializerOptions for per-request serialization, but the final JsonSerializer.SerializeToElement call for the top-level data object omitted it and fell back to the default serializer options, which could produce a response JSON shape inconsistent with the rest of the engine's output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 73 out of 73 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/CoreEx.Data.GraphQL/Internal/GraphQLOrderByTranslator.cs:37
orderBylist items are documented as "each object should specify a single field" because input-object field order is not spec-guaranteed; however the current implementation allows multi-key objects and relies on dictionary enumeration order, which can produce nondeterministic precedence. Consider rejecting multi-key objects with a clear translation error so precedence is always well-defined.
…marks wording, align Skip immutability check - Move Microsoft.Extensions.Logging to GlobalUsing.cs instead of a file-level using in GraphQLQueryRoot.cs. - Reword ProductQueryArgsConfig remarks: all sample QueryArgsConfig types are public, so drop the misleading 'not the usual internal' framing. - Align PagingArgs.Skip to call CheckImmutable before assignment, matching Take/IsCountRequested. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…resolver try/catch BuildQueryArgs(args) (used for includeText/includeInactive translation) was called outside the try/catch in ExecuteItemRootAsync, so a malformed argument value (e.g. includeText passed a non-boolean) threw a GraphQLArgumentTranslationException that escaped ExecuteAsync and aborted the whole request, instead of being mapped to a per-field ARGUMENT_ERROR like every other item/list-root argument-translation failure. Moved the call inside the existing try block, matching ExecuteQueryRootAsync's equivalent BuildQueryArgs call. Added a regression test that reproduces the previous unhandled-throw behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…e complex struct properties GraphQLTypeShape.BuildFieldMap and GraphQLIntrospectionSchemaBuilder.EnsureObjectType both used `elementType ?? property.PropertyType` / `node.ElementType ?? node.PropertyType` to pick the type to recurse into for a complex property. ElementType is only populated for collections, so a non-collection nullable complex struct property (e.g. Money? Total) fell through to the raw declared PropertyType - Nullable<Money> - and reflection recursed into Nullable<T>'s own HasValue/Value properties instead of Money's Amount/Currency. This broke both selection-set validation (spurious UNKNOWN_FIELD errors for genuine struct properties) and introspection (registering a bogus "Nullable`1" OBJECT type). Unwrap via Nullable.GetUnderlyingType(...) ?? propertyType at both call sites. Added a GraphQLEngineTests regression test (readonly record struct Money, nullable Invoice.Total property) that reproduces the previous UNKNOWN_FIELD failure. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment on lines
+66
to
+73
| private void ThrowIfReservedOrDuplicate(string name) | ||
| { | ||
| if (name.StartsWith("__", StringComparison.Ordinal)) | ||
| throw new ArgumentException($"Root field name '{name}' is reserved for GraphQL introspection; names starting with '__' are not permitted.", nameof(name)); | ||
|
|
||
| if (_queryRoots.ContainsKey(name) || _itemRoots.ContainsKey(name)) | ||
| throw new ArgumentException($"A root field named '{name}' is already registered.", nameof(name)); | ||
| } |
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
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.
Summary
Adds
CoreEx.Data.GraphQL— a transport-agnostic bridge that lets a native GraphQL query drive an entity's existingQueryArgsConfig-basedQueryAsync/GetAsyncpipeline, with no parallel query engine and no new authorization surface. GraphQLwhere/orderBysyntax is rewritten 1:1 onto the OData-esqueQueryArgs/PagingArgsstrings already understood byQueryFilterParser/QueryOrderByParser, so field/operator legality continues to be enforced by the same battle-tested parser used by REST$query.Key capabilities
where/orderBytranslated syntactically to the OData-esque equivalent, then parsed by the existing, unmodifiedQueryFilterParser/QueryOrderByParser— exact operator/field parity with$query, including whateverQueryArgsConfigexposes (startswith(),ge, etc.).edges,pageInfo,totalCount) bridged ontoPagingArgs, including atotalCount-only fast path that avoids over-fetching rows.JsonFilter, honoring aliases and__typenameat every depth; fragments are rejected loudly rather than silently ignored. The resolved selection is also fed intoQueryArgs.IncludeFieldsfor list roots, so the underlyingQueryArgsConfig-driven data layer can project only what was actually selected.includeText/includeInactiveref-data options, supported (and schema-advertised) on both list (AddQuery) and single-item (AddGet) roots — an item-rootproduct(id: "...", includeText: true) { category categoryText }works the same way a list root does.__schema/__type(name:)/__typenameare real and built once from the registered roots. Each query root'swhere/orderByare described as fully-typed<Item>WhereInput/<Item>OrderByInputINPUT_OBJECTgraphs (derived automatically fromQueryArgsConfig.ToJsonSchema()), so schema-aware tooling (Postman, Nitro, Apollo Sandbox, GraphiQL) gets full autocomplete with no extra configuration.Namegrammar, so the constructed OData string can't be broken out of.ArgumentException,KeyNotFoundException,GraphQLArgumentTranslationException,QueryFilterParserException,QueryOrderByParserException,ValidationException,NotFoundException, and unknown-field/unknown-root errors all map to distinct{ message, path, extensions.code }shapes instead of a generic, unhelpfulEXECUTION_ERROR.MapCoreExGraphQLLiteaccepts an optionalconfigurecallback for further endpoint customisation (auth, rate limiting, etc.), and tags the endpoint for discoverability.IGraphQLEngine,GraphQLEngineResult,GraphQLEngineError) lives in coreCoreEx; the implementation lives inCoreEx.Data.GraphQL; the minimal-API HTTP bridge (MapCoreExGraphQLLite) lives inCoreEx.AspNetCore. Dependencies point the right way.Hardening found via iterative review
Across several rounds of independent/automated review, the following were found and fixed with regression tests: culture-sensitive float literal parsing; unprojected object-typed leaf fields silently returning the whole DTO; undefined GraphQL variable references silently resolving to
null; silentlong→intoverflow in argument conversion; last-wins duplicate response-key aliases;totalCount-only over-fetching; a type-shape cache key that ignoredJsonSerializerOptions;OperationCanceledExceptionbeing swallowed into a field error instead of propagating; reserved/duplicate root-name registration; malformed numeric literal handling; ahasNextPageedge case; and the final response JSON silently falling back to default (rather than the configured)JsonSerializerOptions.While hardening this feature, a genuine pre-existing, unrelated
CoreEx.Json.JsonFilterbug was also found and fixed: sibling scalar fields where one name is a raw string prefix of another (e.g. the common ref-datacategory/categoryTextCode/Text pair) were incorrectly treated as a nested-path relationship and the shorter field silently dropped fromInclude-filtered output — this affected REST$fieldsprojection too, not just GraphQL-lite.Docs
src/CoreEx.Data.GraphQL/README.mdandAGENTS.md, kept current with every capability above (introspection,includeTexton item roots, etc.).docs/capabilities.md,samples/docs/hosts-layer.md,samples/docs/patterns.md,.github/instructions/coreex-host-setup.instructions.md,.github/instructions/coreex-api-controllers.instructions.md, and.github/agents/coreex-expert.agent.mdupdated with the registration API and theCoreEx.ExecutionContext.GetRequiredService<T>()scoped-resolution pattern..github/instructions/coreex-repositories.instructions.mdcodifies theQueryArgsConfigvisibility exception and folder-placement convention established while building this feature.Testing
CoreEx.Data.GraphQL.Test.Unittests passing across net8/9/10.Contoso.Products.Test.ApiGraphQLLite_*integration tests passing across net8/9/10, including a spec-compliant introspection test driven by a standard client-tooling introspection query (embedded resource).CoreEx.Test.Unitpassing across net8/9/10 (includes theJsonFilterregression coverage above).CoreEx.slnbuild clean (0 warnings, 0 errors).