Repository navigation
Fix three bugs: ApiVersion update, CancellationToken passthrough, cache key collisions - #26
Merged
Merged
Conversation
ApiServiceConfig.UpdateFrom() was missing the ApiVersion property update, so runtime config updates via the custom/updateApiConfig LSP notification could not change it.
Handle() was passing CancellationToken.None to the schema fetch operation instead of the incoming cancellationToken, preventing proper cancellation.
PostJsonAsync methods used GetHashCode() on serialized JSON payloads as cache key components. GetHashCode() is not deterministic across .NET runtimes and can produce collisions. Replaced with SHA-256 hash.
There was a problem hiding this comment.
Pull request overview
Fixes three correctness issues in the server’s API/config handling and schema generation command flow: config updates now include API version, schema fetch respects caller cancellation, and API response caching keys are made deterministic and collision-resistant.
Changes:
- Update
ApiServiceConfig.UpdateFromto propagateApiVersionwhen provided. - Pass the incoming
CancellationTokenthrough to dynamic schema fetching. - Replace non-deterministic
GetHashCode()cache-key hashing with deterministic SHA-256 hashing for POST payloads.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Server/Services/Api/ApiServiceConfig.cs | Ensures ApiVersion is updated consistently with other config fields in UpdateFrom. |
| Server/Services/Api/ApiService.cs | Makes POST cache keys deterministic by hashing serialized payloads with SHA-256. |
| Server/Handlers/CodeActionHandler/GenerateDynamicSchemaCommandHandler.cs | Fixes cancellation propagation by passing the handler token into FetchSchemaAsync. |
resolveSdkNupkgPath now checks the sibling Connectors-NET-SDK repo's build output (../../Connectors-NET-SDK/src/*/bin/Debug/*.nupkg) as a fallback when no explicit sdkNupkgPath setting or workspace SDK/ folder is found. This mirrors the existing sibling server DLL discovery pattern. Also updates launch.json to open the Connectors-NET-Samples workspace in the Extension Development Host for proper end-to-end testing. Extracted findNewestNupkgInDir helper to reduce duplication.
- Reuse pre-serialized JSON for POST body instead of re-serializing via PostAsJsonAsync - Split launch.json into basic config and opt-in config with Samples workspace path Co-authored-by: Dobby <dobby@microsoft.com>
Use SerializeToUtf8Bytes + ByteArrayContent so payload bytes are allocated once and shared between SHA-256 hashing and HTTP body. Co-authored-by: Dobby <dobby@microsoft.com>
Co-authored-by: Dobby <dobby@microsoft.com>
ApiVersion now defaults to string.Empty (like other properties) so UpdateFrom partial updates work correctly. EffectiveApiVersion property applies the 2018-07-01-preview default at the point of use. Co-authored-by: Dobby <dobby@microsoft.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
Fixes three bugs identified during PR #4 review.
Bug fixes
ApiServiceConfig.UpdateFrom missing ApiVersion (Closes ApiServiceConfig.UpdateFrom does not update ApiVersion #7)
GenerateDynamicSchemaCommandHandler ignores CancellationToken (Closes GenerateDynamicSchemaCommandHandler ignores CancellationToken #6)
ApiService cache key uses GetHashCode (Closes ApiService cache key uses GetHashCode which can cause collisions #5)
Testing