Refactor QueryArgsConfig into reusable per-entity classes, add query AI-enablement docs - #171
Merged
Conversation
…AI-enablement docs
- Introduce QueryArgsConfig<TSelf> (CRTP) with a lazy-instantiated Default
singleton, replacing the private static readonly QueryArgsConfig fields
previously declared inline in each repository.
- Extract Order/Movement/Product query configuration into dedicated
{Name}QueryArgsConfig classes under Infrastructure/Repositories/.
- Fix OrderQueryArgsConfig: drop the incorrect WithDefaultModelPrefix("Order")
now that Order queries run directly against the entity with no anonymous
projection (unlike Product, which does project and correctly keeps its
prefix).
- Add WithNoModelPrefix() to clear a field's model prefix; WithModelPrefix(string?)
no longer silently accepts null (throws), removing prior ambiguity between
set and clear.
- Rename MovementRepository.SaveAndGetAllMutatedMovements -> ...Async to
follow async naming convention.
- Update coreex-repository / coreex-test-api skills and instructions with an
explicit interview step before generating query configuration (fields,
operators, model mapping are business decisions the agent must collect,
not infer), plus field-type/operator reference tables and expanded query
test guidance.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors CoreEx QueryArgs configuration into reusable per-entity classes (via a CRTP-based QueryArgsConfig<TSelf>.Default singleton) and updates the repository/testing guidance to make query configuration and query tests more explicit and repeatable across entities.
Changes:
- Introduces
QueryArgsConfig<TSelf>with a lazyDefaultinstance, and addsWithNoModelPrefix()to explicitly clear a field model prefix. - Extracts Product/Movement/Order query configuration into dedicated
*QueryArgsConfigclasses and updates repositories to use the newDefaultpattern. - Expands
coreex-repository/coreex-test-apiguidance to require an explicit “interview” step for query field/operator decisions and adds richer query testing patterns.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/CoreEx.Data/Querying/QueryFilterFieldConfigBaseT.cs | Tightens model-prefix APIs (no implicit null) and adds an explicit “clear prefix” helper. |
| src/CoreEx.Data/Querying/QueryArgsConfigT.cs | Adds CRTP QueryArgsConfig<TSelf> with lazy Default singleton instance. |
| src/CoreEx.Data/Querying/QueryArgsConfig.cs | Formatting-only change (trailing newline). |
| src/CoreEx.Data/Querying/IQueryFilterFieldConfig.cs | Clarifies ModelPrefix semantics in documentation. |
| samples/src/Contoso.Products.Infrastructure/Repositories/ProductRepository.cs | Switches query parsing/schema to ProductQueryArgsConfig.Default. |
| samples/src/Contoso.Products.Infrastructure/Repositories/ProductQueryArgsConfig.cs | New extracted Product query config (filter/order-by). |
| samples/src/Contoso.Products.Infrastructure/Repositories/MovementRepository.cs | Renames internal async helper and switches query parsing/schema to MovementQueryArgsConfig.Default. |
| samples/src/Contoso.Products.Infrastructure/Repositories/MovementQueryArgsConfig.cs | New extracted Movement query config (filter/order-by). |
| samples/src/Contoso.Orders.Infrastructure/Repositories/OrderRepository.cs | Switches query parsing to OrderQueryArgsConfig.Default. |
| samples/src/Contoso.Orders.Infrastructure/Repositories/OrderQueryArgsConfig.cs | New extracted Order query config (filter/order-by), removing the prior model prefix and trimming operators. |
| .github/skills/coreex-test-api/references/workflow.md | Expands “Query” test expectations and provides seed/test patterns and examples. |
| .github/skills/coreex-repository/SKILL.md | Updates skill interview prompts to collect explicit query field/operator/mapping decisions. |
| .github/skills/coreex-repository/references/workflow.md | Reworks repository workflow to use per-entity QueryArgsConfig<TSelf> classes and adds operator/type reference guidance. |
| .github/instructions/coreex-repositories.instructions.md | Updates repository instructions to prefer QueryArgsConfig<TSelf> and adds field/operator/mapping guidance. |
Comments suppressed due to low confidence (1)
src/CoreEx.Data/Querying/QueryFilterFieldConfigBaseT.cs:30
- The
<returns>XML doc forWithNoModelPrefixsays it returnsQueryFilterParser, but the method returnsTSelf. Updating this keeps the fluent API documentation accurate.
/// <summary>
/// Clears (overrides) the optional <see cref="QueryFilterFieldConfigBase.ModelPrefix"/> to be used where referencing the underlying <see cref="IQueryable{T}"/> model.
/// </summary>
/// <returns>The <see cref="QueryFilterParser"/> to support fluent-style method-chaining.</returns>
public TSelf WithNoModelPrefix()
…lPrefix non-nullable Addresses PR review comment: <returns> on both methods incorrectly referenced QueryFilterParser instead of the actual TSelf return type, and WithModelPrefix''s modelPrefix parameter was declared nullable despite ThrowIfNullOrEmpty() throwing on null/empty at runtime. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
WithNoModelPrefix() only exists on filter-field configs (QueryFilterFieldConfigBase<TSelf>). Order-by fields (QueryOrderByFieldConfig) still clear their prefix via WithModelPrefix(null), since that type was not changed by this PR. Update both doc locations to distinguish the two so generated order-by override code does not attempt to call a method that does not exist. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…g for consistency QueryOrderByFieldConfig.WithModelPrefix previously accepted a nullable modelPrefix used both to set and to clear the prefix, unlike the filter-field config where WithModelPrefix is non-nullable and clearing is a separate WithNoModelPrefix() method. Aligns the two: - WithModelPrefix(string modelPrefix) is now non-nullable and throws on null/empty via ThrowIfNullOrEmpty(), matching QueryFilterFieldConfigBase<TSelf>. - Added WithNoModelPrefix() to explicitly clear the prefix. - QueryOrderByFieldConfig.ModelPrefix now defaults from QueryOrderByParser.DefaultModelPrefix via a property initializer (same approach QueryFilterFieldConfigBase already uses), rather than via an internal call to the now-throwing WithModelPrefix(DefaultModelPrefix) in QueryOrderByParser.AddField, which would have thrown whenever no default prefix was configured. - Simplify the two doc locations (coreex-repositories.instructions.md, coreex-repository/references/workflow.md) that previously had to call out the filter-vs-order-by inconsistency now that both configs share the same WithModelPrefix / WithNoModelPrefix pattern. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ference - Field type reference tables in both coreex-repositories.instructions.md and coreex-repository/references/workflow.md wrongly stated that AddField<string> defaults to "Comparison + string functions". QueryFilterFieldConfigBase actually defaults string fields to QueryFilterOperator.ComparisonOperators only; StringFunctions (startswith/endswith/contains) must be added explicitly via .WithOperators(...). Corrected both tables. - SKILL.md frontmatter description still referenced QueryArgsConfig; updated to QueryArgsConfig<TSelf> to match the now-standard per-entity pattern. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
QueryArgsConfig<TSelf>(CRTP) with a lazy-instantiatedDefaultsingleton, replacing theprivate static readonly QueryArgsConfigfields previously declared inline in each repository.{Name}QueryArgsConfigclasses underInfrastructure/Repositories/.OrderQueryArgsConfig: drop the incorrectWithDefaultModelPrefix("Order")— Order queries run directly against the entity with no anonymous projection, unlike Product, which does project and correctly keeps its prefix.WithNoModelPrefix()to clear a field's model prefix;WithModelPrefix(string?)no longer silently accepts null (throws), removing the prior ambiguity between "set" and "clear".MovementRepository.SaveAndGetAllMutatedMovements→...Asyncto follow the async naming convention.coreex-repository/coreex-test-apiskills and instructions with an explicit interview step before generating query configuration (fields, operators, model mapping are business decisions the agent must collect, not infer), plus field-type/operator reference tables and expanded query test guidance.Notes
OrderQueryArgsConfig.CustomerIdnow only supportsEqualityOperators(previously also hadStartsWith) — Orders is a WIP domain percopilot-instructions.mdand has no query test coverage yet, so this is a scope trim rather than a regression against any tested behaviour.ProductQueryArgsConfig/MovementQueryArgsConfigare exercised end-to-end byReadTests.ProductQuery.cs/ReadTests.MovementQuery.cs.Validation
dotnet build CoreEx.sln— succeeded, 0 errors.dotnet test tests\CoreEx.Data.Test.Unit— 120/120 passed on net8.0/net9.0/net10.0.