diff --git a/CHANGELOG.md b/CHANGELOG.md index 424d0035..338d6ed0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +# 7.0.1 + +- Fixed issues: + - Issue: [attempting to get WorkItemItemTypes without a project causes an UnAuthorizedException but the http code is OK #107](https://github.com/hkarthik7/azure-devops-java-sdk/issues/107) + - `ErrorResponseHandler` was calling `context.request().getRequestUri()` to build the 401 error message. This triggered a fresh `LocationService` OPTIONS lookup that overwrote `ResponseHandler.apiResponse` with 200 OK, hiding the real 401 from callers of `ResponseHandler.getResponse()`. Fixed by using `context.response().request().uri()` (the URI from the already-sent `HttpResponse`) instead. # 7.1.0 - Added support for **Dashboard API**. diff --git a/azd/src/main/java/org/azd/abstractions/handlers/ErrorResponseHandler.java b/azd/src/main/java/org/azd/abstractions/handlers/ErrorResponseHandler.java index 93955381..902d86d9 100644 --- a/azd/src/main/java/org/azd/abstractions/handlers/ErrorResponseHandler.java +++ b/azd/src/main/java/org/azd/abstractions/handlers/ErrorResponseHandler.java @@ -36,11 +36,15 @@ public CompletableFuture handleAsync(ResponseContext context) { } if (status == 401) { + // Use the URI from the already-sent HttpResponse rather than calling + // context.request().getRequestUri(), which triggers a fresh LocationService + // lookup (OPTIONS request) that overwrites ResponseHandler.apiResponse with + // 200 OK — hiding the real 401 from callers of ResponseHandler.getResponse(). return CompletableFuture.failedFuture( new AzDException( ApiExceptionTypes.UnAuthorizedException.toString(), "Given token doesn't have access to resource '" - + context.request().getRequestUri() + "'." + + context.response().request().uri() + "'." ) ); } diff --git a/azd/src/test/java/org/azd/unittests/WorkItemTrackingRequestBuilderTest.java b/azd/src/test/java/org/azd/unittests/WorkItemTrackingRequestBuilderTest.java index bbd96308..362cfe01 100644 --- a/azd/src/test/java/org/azd/unittests/WorkItemTrackingRequestBuilderTest.java +++ b/azd/src/test/java/org/azd/unittests/WorkItemTrackingRequestBuilderTest.java @@ -7,6 +7,8 @@ import org.azd.authentication.PersonalAccessTokenCredential; import org.azd.common.types.JsonPatchDocument; import org.azd.enums.*; +import org.azd.abstractions.ResponseHandler; +import org.azd.enums.HttpStatusCode; import org.azd.exceptions.AzDException; import org.azd.helpers.StreamHelper; import org.azd.http.ClientRequest; @@ -24,6 +26,8 @@ import java.util.Map; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; public class WorkItemTrackingRequestBuilderTest { private static final SerializerContext serializer = InstanceFactory.createSerializerContext(); @@ -456,4 +460,42 @@ public void shouldUpdateAFormattedCommentForGivenWorkItem() throws AzDException w.comments().update("# This is an updated comment.", comment.getId(), 2177, CommentFormat.markdown); } + + /** + * Regression test for issue #107. + * + * When a 401 is returned (e.g. calling workItemTypes().list() without a project), + * ResponseHandler.getResponse() must still reflect the 401 status after the exception + * is thrown. Previously, ErrorResponseHandler called context.request().getRequestUri() + * to build the error message, which triggered a fresh LocationService OPTIONS call that + * overwrote ResponseHandler.apiResponse with 200 OK before the exception reached the + * caller. + */ + @Test + public void shouldRetainUnauthorizedStatusInResponseHandlerWhenProjectIsAbsent() { + String dir = System.getProperty("user.dir"); + var file = new File(dir + "/src/test/java/org/azd/_unitTest.json"); + MockParameters m; + try { + m = InstanceFactory.createSerializerContext().deserialize(file, MockParameters.class); + } catch (AzDException e) { + return; // skip if config not available + } + // Build a client WITHOUT project — mirrors the reporter's code in issue #107. + var pat = new PersonalAccessTokenCredential( + Instance.BASE_INSTANCE.append(m.getO()), m.getT()); + var noProjectClient = AzDService.builder().authentication(pat).buildClient(); + + try { + noProjectClient.workItemTracking().workItemTypes().list(); + } catch (AzDException e) { + // Exception is expected. The key assertion: ResponseHandler must NOT have + // been overwritten with 200 OK by the internal location-discovery request. + var response = ResponseHandler.getResponse(); + assertNotNull("ResponseHandler.getResponse() must not be null after a failed call", response); + assertNotEquals( + "ResponseHandler.getResponse() must not be OK after a 401 — issue #107", + HttpStatusCode.OK, response.getStatusCode()); + } + } }