Consume URI in PUT response for GitHub-owned storage for GEI #1371
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the multipart upload flow to consume the JSON body returned by the final PUT "complete" request and use its uri field instead of synthesizing the GEI URI.
- Tests now mock and assert against the full JSON response, including the
uriproperty. ArchiveUploaderis refactored to parse the JSON response inCompleteUploadand return theuriinstead of extracting the GUID from the query string.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ArchiveUploadersTests.cs | Adds JObject response mocks and updates assertions to use the returned uri. |
| ArchiveUploader.cs | Removes manual GUID parsing, parses JSON in CompleteUpload, and returns the uri. |
Comments suppressed due to low confidence (2)
src/Octoshift/Services/ArchiveUploader.cs:81
- [nitpick] Returning the GEI URI as a string may lead to formatting issues downstream. Consider having
UploadMultipartreturn aUriobject directly for stronger typing.
return geiUri.ToString();
src/Octoshift/Services/ArchiveUploader.cs:136
- Parsing and immediately using the
urifield without validation may throw on unexpected responses. Consider checking for missing or invaliduriand logging a clearer error if parsing fails.
var responseData = JObject.Parse(response);
| var completeUploadResponse = new JObject | ||
| { | ||
| ["guid"] = guid, | ||
| ["node_id"] = "global-relay-id", | ||
| ["name"] = archiveName, | ||
| ["size"] = largeContent.Length, | ||
| ["uri"] = geiUri, | ||
| ["created_at"] = "2025-06-23T17:13:02.818-07:00" | ||
| }; |
There was a problem hiding this comment.
The completeUploadResponse initialization is duplicated across multiple tests. Consider extracting it into a common helper or fixture to reduce repetition.
| var completeUploadResponse = new JObject | |
| { | |
| ["guid"] = guid, | |
| ["node_id"] = "global-relay-id", | |
| ["name"] = archiveName, | |
| ["size"] = largeContent.Length, | |
| ["uri"] = geiUri, | |
| ["created_at"] = "2025-06-23T17:13:02.818-07:00" | |
| }; | |
| var completeUploadResponse = CreateCompleteUploadResponse(guid, archiveName, largeContent.Length, geiUri); |
Copilot uses AI. Check for mistakes.
Unit Test Results 1 files 1 suites 20s ⏱️ Results for commit ff927dd. ♻️ This comment has been updated with latest results. |
|
Closes https://github.ghe.com/github/octoshift/issues/10765!
This PR consumes the new JSON response from the PUT "complete" request for the multipart upload flow when uploading archives to GitHub-owned storage for GEI!
This is an example of a response from the PUT response:
{ "guid": "363b2659-b8a3-4878-bfff-eed4bcb54d35", "node_id": "MA_kgDaACQzNjNiMjY1OS1iOGEzLTQ4NzgtYmZmZi1lZWQ0YmNiNTRkMzU", "name": "git-archive.tar.gz", "size": 33287, "uri": "gei://archive/363b2659-b8a3-4878-bfff-eed4bcb54d35", "created_at": "2024-11-13T12:35:45.761-08:00" }With the changes in this PR, the JSON body ☝️ is consumed, and the GEI URI doesn't need to be interpolated anymore:
gh-gei/src/Octoshift/Services/ArchiveUploader.cs
Lines 82 to 85 in f9ee191
Here's an example of these changes at work:
ThirdPartyNotices.txt(if applicable)