Skip to content

Milestone 1 complete: Kayak removed, HttpListener in place — follow-on clean-up#121

Merged
hibri merged 3 commits intomasterfrom
copilot/review-code-for-next-steps
Mar 28, 2026
Merged

Milestone 1 complete: Kayak removed, HttpListener in place — follow-on clean-up#121
hibri merged 3 commits intomasterfrom
copilot/review-code-for-next-steps

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 28, 2026

PR #120 fully achieved the first milestone (Kayak → HttpListener). This PR confirms that and addresses the immediate follow-on hygiene items surfaced during the review.

Changes

  • README — replace deprecated WebClient.DownloadString with HttpClient.GetStringAsync + async/await; fix "Eample" typo
  • RequestHandlerFactory.ClearHandlers — remove dead no-op that allocated a List<RequestHandler> and discarded it without assigning
  • FileResponseBody.GetBytes — fix CA2022: FileStream.Read can return fewer bytes than requested; replace with ReadExactly (available since .NET 7)
// Before — partial read possible
fileStream.Read(buffer, 0, (int)fileInfo.Length);

// After — guaranteed full read
fileStream.ReadExactly(buffer, 0, (int)fileInfo.Length);
Original prompt

Review the code for next steps. Analyse if the first milestone has been achieved


⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.

Copilot AI and others added 2 commits March 28, 2026 16:10
… no-op

Co-authored-by: hibri <122442+hibri@users.noreply.github.com>
…uestHandlerFactory blank lines

Agent-Logs-Url: https://github.com/hibri/HttpMock/sessions/98c11885-a97f-46fd-b392-354806586e77

Co-authored-by: hibri <122442+hibri@users.noreply.github.com>
Copilot AI changed the title [WIP] Review code for milestone achievement Milestone 1 complete: Kayak removed, HttpListener in place — follow-on clean-up Mar 28, 2026
Copilot AI requested a review from hibri March 28, 2026 16:15
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Follow-on cleanup after the Kayak → HttpListener milestone: removes dead code, fixes a file-read correctness issue, and modernizes README sample code.

Changes:

  • Update README sample to use HttpClient.GetStringAsync with async/await, and fix a typo (“Eample” → “Example”).
  • Remove a no-op RequestHandlerFactory.ClearHandlers() method.
  • Replace FileStream.Read with ReadExactly in FileResponseBody.GetBytes() to ensure full-buffer reads.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
README.md Modernizes sample HTTP call to async HttpClient and fixes a typo.
src/HttpMock/RequestHandlerFactory.cs Removes unused/broken public API method (ClearHandlers).
src/HttpMock/FileResponseBody.cs Uses ReadExactly to avoid partial reads when loading file responses.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 33 to 37
public RequestHandler CustomVerb(string path, string verb) {
return CreateHandler(path, verb);
}

public void ClearHandlers() {
new List<RequestHandler>();
}



private RequestHandler CreateHandler(string path, string method) {
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the public RequestHandlerFactory.ClearHandlers() method is a breaking change for any external consumers that may be calling it (even if it was previously a no-op). Consider keeping the method as a backwards-compatible wrapper (e.g., delegating to _requestProcessor.ClearHandlers()), marking it [Obsolete], and removing it in the next major version instead of deleting it outright.

Copilot uses AI. Check for mistakes.
.OK();

string result = new WebClient().DownloadString("http://localhost:9191/endpoint");
string result = await new HttpClient().GetStringAsync("http://localhost:9191/endpoint");
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

new HttpClient() is created inline and never disposed in the example. Even in sample code this can encourage patterns that lead to socket exhaustion; prefer a single reused HttpClient instance or a using/using var around it in the snippet.

Suggested change
string result = await new HttpClient().GetStringAsync("http://localhost:9191/endpoint");
using var httpClient = new HttpClient();
string result = await httpClient.GetStringAsync("http://localhost:9191/endpoint");

Copilot uses AI. Check for mistakes.
Comment on lines +26 to 27
fileStream.ReadExactly(buffer, 0, (int)fileInfo.Length);
int length = (int)fileInfo.Length;
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since buffer is already sized to the file length, consider calling ReadExactly using buffer.Length (and avoiding repeated (int)fileInfo.Length casts). This removes a potential mismatch if the file size changes between allocation and read, and keeps the code simpler.

Suggested change
fileStream.ReadExactly(buffer, 0, (int)fileInfo.Length);
int length = (int)fileInfo.Length;
fileStream.ReadExactly(buffer, 0, buffer.Length);
int length = buffer.Length;

Copilot uses AI. Check for mistakes.
@hibri hibri marked this pull request as ready for review March 28, 2026 20:54
@hibri hibri merged commit 0ab896f into master Mar 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants