Enhance external resolver system - #7
Conversation
WalkthroughThe change adds dynamic cross-service resolvers and integrates them with Mongo query execution. Resolve stages batch external keys, cache results per query, populate nested BSON fields, and support dependency-injection registration. Documentation adds configuration and request examples. ChangesExternal resolver support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant QueryEndpoint
participant MongoQueryAdapter
participant ExternalServiceResolver
participant DownstreamOxQLService
Client->>QueryEndpoint: Submit query with resolve stage
QueryEndpoint->>MongoQueryAdapter: Execute query
MongoQueryAdapter->>ExternalServiceResolver: Resolve batched keys
ExternalServiceResolver->>DownstreamOxQLService: POST OxQL key query
DownstreamOxQLService-->>ExternalServiceResolver: Return matching items
ExternalServiceResolver-->>MongoQueryAdapter: Return resolved values
MongoQueryAdapter-->>QueryEndpoint: Return enriched documents
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/docs/cross-service-resolver.md`:
- Around line 83-86: Update the resolver query construction around inList to
serialize each key as a properly escaped OxQL string literal before joining the
values. Use a trusted string serializer or query builder so quotes and escape
characters cannot alter the query grammar; keep URL encoding applied to the
completed query.
- Around line 256-261: The cross-service resolver example must pass
request-specific resolvers into query execution instead of only storing them
locally. In src/docs/cross-service-resolver.md lines 256-261, add or use a
request-scoped execution path that accepts resolvers before
queryService.ExecuteAsync; update README.md line 415 to document that request
only after the execution path is implemented.
- Around line 186-193: Update the service-target handling around the serviceMap
loop and HttpClient.BaseAddress assignment to enforce server-side admission
before creating the client: resolve each source through a server-managed
registry or validate it against a configured allowlist, reject private,
loopback, link-local, and metadata endpoints, and ensure redirects cannot escape
the approved targets. Do not rely solely on the existing HTTP-versus-HTTPS
check.
- Around line 93-94: Update the response deserialization in the cross-service
resolver to use the query response contract returned by the endpoint, rather
than deserializing a top-level List<JsonElement>. Access and enumerate the
contract’s Items collection for result documents, preserving the existing
cancellation token and downstream item-processing flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b61fd2e0-4ee8-4d0b-a670-5e98837c0e9c
📒 Files selected for processing (8)
README.mdsrc/OxQL.Mongo/MongoQueryAdapter.cssrc/OxQL.Mongo/MongoQueryExecutor.cssrc/OxQL.Mongo/ServiceCollectionExtensions.cssrc/OxQL.Tests/Fakes/FakeExternalResolver.cssrc/OxQL.Tests/Mongo/MongoQueryAdapterResolveTests.cssrc/README.mdsrc/docs/cross-service-resolver.md
| // Build: match id in ["key1","key2"] | ||
| var inList = string.Join(", ", keys.Select(k => $"\"{k}\"")); | ||
| var query = $"match id in [{inList}]"; | ||
| var url = $"/oxql/query?q={HttpUtility.UrlEncode(query)}"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Escape each resolver key before building the OxQL query.
Line 84 inserts raw keys inside quoted OxQL literals. A key containing quotes or escape characters can change the remote query. URL encoding at Line 86 does not protect the OxQL grammar.
Serialize each key as a string literal with a trusted serializer or query builder before joining the list.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/docs/cross-service-resolver.md` around lines 83 - 86, Update the resolver
query construction around inList to serialize each key as a properly escaped
OxQL string literal before joining the values. Use a trusted string serializer
or query builder so quotes and escape characters cannot alter the query grammar;
keep URL encoding applied to the completed query.
| var items = await response.Content | ||
| .ReadFromJsonAsync<List<JsonElement>>(cancellationToken: cancellationToken) ?? []; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Deserialize the query response envelope.
The endpoint at Lines 249-264 returns Results.Ok(result). The Mongo execution path returns a QueryResponse object with result documents in Items. ReadFromJsonAsync<List<JsonElement>> expects a top-level array, so a normal remote OxQL response will throw JsonException.
Deserialize the response contract and enumerate its Items collection.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/docs/cross-service-resolver.md` around lines 93 - 94, Update the response
deserialization in the cross-service resolver to use the query response contract
returned by the endpoint, rather than deserializing a top-level
List<JsonElement>. Access and enumerate the contract’s Items collection for
result documents, preserving the existing cancellation token and downstream
item-processing flow.
| foreach (var (source, baseAddress) in serviceMap) | ||
| { | ||
| if (!_options.AllowHttp && baseAddress.Scheme.Equals("http", StringComparison.OrdinalIgnoreCase)) | ||
| throw new InvalidOperationException( | ||
| $"Plain HTTP is not allowed for '{source}'. Use HTTPS."); | ||
|
|
||
| var client = _httpClientFactory.CreateClient($"oxql-external-{source}"); | ||
| client.BaseAddress = baseAddress; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Apply a server-side target admission policy.
services is caller-controlled, and this factory assigns each supplied URI to HttpClient.BaseAddress. HTTPS does not prevent requests to internal HTTPS services, loopback endpoints, or cloud metadata endpoints. This creates an SSRF path.
Resolve service names through a server-managed registry, or enforce an allowlist and block private, loopback, link-local, and redirect targets.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/docs/cross-service-resolver.md` around lines 186 - 193, Update the
service-target handling around the serviceMap loop and HttpClient.BaseAddress
assignment to enforce server-side admission before creating the client: resolve
each source through a server-managed registry or validate it against a
configured allowlist, reject private, loopback, link-local, and metadata
endpoints, and ensure redirects cannot escape the approved targets. Do not rely
solely on the existing HTTP-versus-HTTPS check.
| var serviceMap = ServiceRegistryParser.Parse(services); | ||
| var resolvers = factory.CreateResolvers(serviceMap); | ||
|
|
||
| // Register resolvers into your execution context, then run the query | ||
| var request = BuildQueryRequest(q); | ||
| var result = await queryService.ExecuteAsync(request, ct); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Integrate request-specific resolvers with query execution.
DynamicResolverFactory.CreateResolvers returns resolvers only into a local variable. The Mongo adapter receives its resolver set during DI construction, so it cannot use these request-specific resolvers.
src/docs/cross-service-resolver.md#L256-L261: create a request-scoped execution path that receivesresolversbeforequeryService.ExecuteAsync.README.md#L415-L415: document the request only after that execution path is implemented.
📍 Affects 2 files
src/docs/cross-service-resolver.md#L256-L261(this comment)README.md#L415-L415
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/docs/cross-service-resolver.md` around lines 256 - 261, The cross-service
resolver example must pass request-specific resolvers into query execution
instead of only storing them locally. In src/docs/cross-service-resolver.md
lines 256-261, add or use a request-scoped execution path that accepts resolvers
before queryService.ExecuteAsync; update README.md line 415 to document that
request only after the execution path is implemented.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation