Fix for issue 1517 and correction of two tests not respecting the REST reference (impacted by the fix) - #1518
Merged
Conversation
The test UploadResponseHandlerTest#getUploadSessionOnProgressingUpload mimicked a server response with a 200/OK response code. In the REST reference (https://learn.microsoft.com/en-us/graph/api/driveitem-createuploadsession?view=graph-rest-1.0#upload-bytes-to-the-upload-session) it is stated that the response code for uploading more bytes to the upload session is 202/Accepted. The bug in #1517 is fixed. Previously the method UploadResponseHandler#handleResponse handled differently the response in these two cases: - response code 200/OK - all other cases Now the same method handles differently the response in these two cases: - response code 200/OK or 201/Created - all other cases Everything works if the REST reference is corrected and: - 200/OK is returned when an item is updated, 201/Created is returned when an item is Created - 202/Accepted is returned when more bytes are requested
Contributor
Author
|
@microsoft-github-policy-service agree |
Vincent Biret (baywet)
requested changes
Feb 21, 2024
Vincent Biret (baywet)
left a comment
Member
There was a problem hiding this comment.
Thanks for the pull request.
Can you please:
- bump the patch version in gradle.properties
- update the kiota dependencies version to 1.0.3 in gradle/dependencies.gradle
- add a changelog entry (today's date, changed section)
- update the versions in the readme.md
This will help ensure we can quickly publish the changes
Member
|
Andrew Omondi (@andrueastman) can you also have a look at this change please when compared with dotnet? |
- Updated the patch version in gradle.properies - Updated changelog.md and readme.md with the new patch version - Bumped kiota dependencies to 1.0.3 - Fixed the test UploadSliceRequestTest.putReturnsExpectedUploadSession which did not respect the response descripted in the REST reference
Contributor
Author
|
Vincent Biret (@baywet) all done. I also fixed another test case which did not mock what is written in the REST Reference |
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Signed-off-by: Vincent Biret <vibiret@microsoft.com>
Vincent Biret (baywet)
approved these changes
Feb 21, 2024
Vincent Biret (baywet)
left a comment
Member
There was a problem hiding this comment.
I just reverted some of the changes to address the root cause.
This is because the implementation was starting to drift when compared with the dotnet one
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
2 participants
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.
Changes proposed in this pull request
The test UploadResponseHandlerTest#getUploadSessionOnProgressingUpload mimicked a server response with a 200/OK response code.
In the REST reference (https://learn.microsoft.com/en-us/graph/api/driveitem-createuploadsession?view=graph-rest-1.0#upload-bytes-to-the-upload-session) it is stated that the response code for uploading more bytes to the upload session is 202/Accepted.
The bug in #1517 is fixed.
Previously the method UploadResponseHandler#handleResponse handled differently the response in these two cases:
Now the same method handles differently the response in these two cases:
Everything works if the REST reference is corrected and: