Conversation
…fix Windows build compatibility
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds first-class Durable Functions “durable HTTP” support to the Java SDK by introducing request/response models, retry/auth configuration, and orchestration helpers, along with unit and end-to-end coverage.
Changes:
- Introduces
DurableHttpAPIs and supporting models (DurableHttpRequest/Response, retry options, token sources) in the core client SDK. - Adds unit tests validating JSON wire compatibility (TimeSpan formatting, managed identity options, retry options, request/response serialization).
- Adds an Azure Functions E2E function + test, and centralizes JUnit dependencies at the root Gradle level.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| samples-azure-functions/build.gradle | Removes per-module JUnit deps (now provided centrally). |
| endtoendtests/src/test/java/com/functions/EndToEndTests.java | Adds E2E test for durable HTTP orchestration. |
| endtoendtests/src/main/java/com/functions/CallHttpFunction.java | Adds Functions endpoints/orchestrator that performs a durable HTTP GET. |
| endtoendtests/build.gradle | Removes per-module JUnit deps (now provided centrally). |
| client/src/test/java/com/microsoft/durabletask/TimeSpanHelperTest.java | Adds unit tests for .NET TimeSpan ↔ Java Duration conversion. |
| client/src/test/java/com/microsoft/durabletask/ManagedIdentityTokenSourceTest.java | Adds unit tests for managed identity token source behavior/JSON. |
| client/src/test/java/com/microsoft/durabletask/ManagedIdentityOptionsTest.java | Adds unit tests for managed identity options JSON shape. |
| client/src/test/java/com/microsoft/durabletask/HttpRetryOptionsTest.java | Adds unit tests for HTTP retry options behavior/JSON. |
| client/src/test/java/com/microsoft/durabletask/DurableHttpResponseTest.java | Adds unit tests for durable HTTP response model/JSON. |
| client/src/test/java/com/microsoft/durabletask/DurableHttpRequestTest.java | Adds unit tests for durable HTTP request model/JSON and MI scenarios. |
| client/src/main/java/com/microsoft/durabletask/TokenSource.java | Adds polymorphic base type for durable HTTP auth token sources. |
| client/src/main/java/com/microsoft/durabletask/TimeSpanHelper.java | Adds internal utility for TimeSpan wire-format conversions. |
| client/src/main/java/com/microsoft/durabletask/TaskOrchestrationContext.java | Adds convenience callHttp(...) default methods on orchestration context. |
| client/src/main/java/com/microsoft/durabletask/ManagedIdentityTokenSource.java | Adds managed identity token source model with resource normalization. |
| client/src/main/java/com/microsoft/durabletask/ManagedIdentityOptions.java | Adds managed identity options model for authority host/tenant. |
| client/src/main/java/com/microsoft/durabletask/HttpRetryOptions.java | Adds durable HTTP retry policy model with TimeSpan serialization helpers. |
| client/src/main/java/com/microsoft/durabletask/DurableHttpResponse.java | Adds durable HTTP response model. |
| client/src/main/java/com/microsoft/durabletask/DurableHttpRequest.java | Adds durable HTTP request model incl. timeout/retry/token source. |
| client/src/main/java/com/microsoft/durabletask/DurableHttp.java | Adds orchestrator-side helper for invoking the built-in HTTP activity. |
| client/build.gradle | Improves JDK11 test runtime selection and Windows executable handling; removes local JUnit deps. |
| build.gradle | Centralizes JUnit 5 dependencies for all Java subprojects. |
| azuremanaged/build.gradle | Aligns test runtime selection and removes per-module JUnit deps. |
| azurefunctions/src/test/java/com/microsoft/durabletask/azurefunctions/DurableHttpTest.java | Adds unit tests ensuring DurableHttp delegates to callActivity as expected. |
| azurefunctions/build.gradle | Adds test toolchain config and Mockito deps to support new unit tests. |
You can also share your feedback on Copilot code review. Take the survey.
client/src/main/java/com/microsoft/durabletask/HttpRetryOptions.java
Outdated
Show resolved
Hide resolved
client/src/main/java/com/microsoft/durabletask/TimeSpanHelper.java
Outdated
Show resolved
Hide resolved
client/src/main/java/com/microsoft/durabletask/DurableHttpResponse.java
Dismissed
Show dismissed
Hide dismissed
YunchuWang
left a comment
YunchuWang
left a comment
There was a problem hiding this comment.
Code Review Summary
Overall risk: Moderate
Merge recommendation: Needs changes (at least F1/F2 should be addressed first)
This PR adds comprehensive callHttp support for the Java Durable Task SDK. The overall design is solid - good wire-format compatibility with .NET, strong test coverage, and clean API surface. A few issues should be fixed before merge.
Findings
- High (P1): 1 -
normalizeResourceprefix matching too broad (F1) - Medium (P2): 1 -
asynchronousPatternEnableddefault mismatch on deserialization (F2) - Low (P3): 2 - duplicated JDK path logic in build files (F3),
setMaxRetryIntervalmissing validation (F4) - Nit (P4): 1 -
normalizeResourcecase-sensitive matching (F5)
See inline comments for details on F1, F2, and F4.
[F3] JDK path resolution logic duplicated across 3 build.gradle files (Low / P3)
The ~15-line JDK path detection block (rawJdkPath, isWindows, exeSuffix) is copy-pasted identically in client/build.gradle, azurefunctions/build.gradle, and azuremanaged/build.gradle. Consider extracting to a shared Gradle script (e.g., gradle/jdk-config.gradle) and using apply from: in each module. Not blocking merge but worth a follow-up.
[F5] normalizeResource is case-sensitive (Nit / P4)
startsWith in ManagedIdentityTokenSource.normalizeResource() is case-sensitive. If a user passes HTTPS://MANAGEMENT.CORE.WINDOWS.NET, it won't match. In practice users almost always use lowercase, but using resource.toLowerCase(Locale.ROOT).startsWith(base) would be more robust.
Positive notes: Excellent test coverage (constructor, serialization, round-trip, cross-extension parity tests), correct defensive copies with unmodifiable maps, proper polymorphic Jackson setup for TokenSource, and correct .NET TimeSpan format handling in TimeSpanHelper.
client/src/main/java/com/microsoft/durabletask/ManagedIdentityTokenSource.java
Outdated
Show resolved
Hide resolved
I will make this fix in a separate PR. |
Issue describing the changes in this PR
Adds support for Calls to HTTP endpoints to the Java SDK.
Pull request checklist
CHANGELOG.md