Skip to content

Commit 55f9afe

Browse files
committed
Addressed copilot pr comments
1 parent 575a295 commit 55f9afe

7 files changed

Lines changed: 49 additions & 11 deletions

File tree

azure-blob-payloads/build.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ test {
7272
}
7373
}
7474

75-
// Integration tests require DTS emulator (default localhost:8080) and Azurite on localhost:10000.
75+
// Integration tests require DTS emulator (default localhost:4001) and Azurite on localhost:10000.
7676
task integrationTest(type: Test) {
7777
useJUnitPlatform {
7878
includeTags 'integration'

azure-blob-payloads/src/main/java/com/microsoft/durabletask/azureblobpayloads/LargePayloadClientExtensions.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
import com.microsoft.durabletask.DurableTaskGrpcClientBuilder;
66

7+
import java.util.Objects;
8+
79
/**
810
* Extension methods for enabling externalized payload storage on a {@link DurableTaskGrpcClientBuilder}.
911
* <p>
@@ -27,6 +29,8 @@ private LargePayloadClientExtensions() {
2729
public static DurableTaskGrpcClientBuilder useExternalizedPayloads(
2830
DurableTaskGrpcClientBuilder builder,
2931
LargePayloadStorageOptions options) {
32+
Objects.requireNonNull(builder, "builder must not be null");
33+
Objects.requireNonNull(options, "options must not be null");
3034
PayloadStore store = new BlobPayloadStore(options);
3135
builder.addInterceptor(new LargePayloadInterceptor(store, options));
3236
return builder;
@@ -47,6 +51,9 @@ public static DurableTaskGrpcClientBuilder useExternalizedPayloads(
4751
DurableTaskGrpcClientBuilder builder,
4852
PayloadStore store,
4953
LargePayloadStorageOptions options) {
54+
Objects.requireNonNull(builder, "builder must not be null");
55+
Objects.requireNonNull(store, "store must not be null");
56+
Objects.requireNonNull(options, "options must not be null");
5057
builder.addInterceptor(new LargePayloadInterceptor(store, options));
5158
return builder;
5259
}

azure-blob-payloads/src/main/java/com/microsoft/durabletask/azureblobpayloads/LargePayloadStorageOptions.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ public int getThresholdBytes() {
5858
* @throws IllegalArgumentException if the value exceeds 1 MiB
5959
*/
6060
public LargePayloadStorageOptions setThresholdBytes(int thresholdBytes) {
61+
if (thresholdBytes < 0) {
62+
throw new IllegalArgumentException(
63+
"Payload storage threshold cannot be negative.");
64+
}
6165
if (thresholdBytes > ONE_MIB) {
6266
throw new IllegalArgumentException(
6367
"Payload storage threshold cannot exceed 1 MiB (" + ONE_MIB + " bytes).");
@@ -84,6 +88,10 @@ public int getMaxPayloadBytes() {
8488
* @return this options object
8589
*/
8690
public LargePayloadStorageOptions setMaxPayloadBytes(int maxPayloadBytes) {
91+
if (maxPayloadBytes <= 0) {
92+
throw new IllegalArgumentException(
93+
"Maximum payload size must be a positive number of bytes.");
94+
}
8795
this.maxPayloadBytes = maxPayloadBytes;
8896
return this;
8997
}

azure-blob-payloads/src/main/java/com/microsoft/durabletask/azureblobpayloads/LargePayloadWorkerExtensions.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
import com.microsoft.durabletask.DurableTaskGrpcWorkerBuilder;
66

7+
import java.util.Objects;
8+
79
/**
810
* Extension methods for enabling externalized payload storage on a {@link DurableTaskGrpcWorkerBuilder}.
911
* <p>
@@ -29,6 +31,8 @@ private LargePayloadWorkerExtensions() {
2931
public static DurableTaskGrpcWorkerBuilder useExternalizedPayloads(
3032
DurableTaskGrpcWorkerBuilder builder,
3133
LargePayloadStorageOptions options) {
34+
Objects.requireNonNull(builder, "builder must not be null");
35+
Objects.requireNonNull(options, "options must not be null");
3236
PayloadStore store = new BlobPayloadStore(options);
3337
builder.addInterceptor(new LargePayloadInterceptor(store, options));
3438
builder.setSupportsLargePayloads(true);
@@ -50,6 +54,9 @@ public static DurableTaskGrpcWorkerBuilder useExternalizedPayloads(
5054
DurableTaskGrpcWorkerBuilder builder,
5155
PayloadStore store,
5256
LargePayloadStorageOptions options) {
57+
Objects.requireNonNull(builder, "builder must not be null");
58+
Objects.requireNonNull(store, "store must not be null");
59+
Objects.requireNonNull(options, "options must not be null");
5360
builder.addInterceptor(new LargePayloadInterceptor(store, options));
5461
builder.setSupportsLargePayloads(true);
5562
return builder;

azure-blob-payloads/src/test/java/com/microsoft/durabletask/azureblobpayloads/LargePayloadIntegrationTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
* <p>
2525
* These tests require:
2626
* <ul>
27-
* <li>DTS emulator running on localhost:8080:
28-
* {@code docker run --name durabletask-emulator -p 8080:8080 -d mcr.microsoft.com/dts/dts-emulator:latest}</li>
27+
* <li>DTS emulator running on localhost:4001:
28+
* {@code docker run --name durabletask-emulator -p 4001:8080 -d mcr.microsoft.com/dts/dts-emulator:latest}</li>
2929
* <li>Azurite running on localhost:10000:
3030
* {@code docker run --name azurite -p 10000:10000 -p 10001:10001 -p 10002:10002 -d mcr.microsoft.com/azure-storage/azurite}</li>
3131
* </ul>
@@ -42,7 +42,7 @@ public class LargePayloadIntegrationTest {
4242
private static final String EMULATOR_ENDPOINT =
4343
System.getenv("DTS_ENDPOINT") != null
4444
? System.getenv("DTS_ENDPOINT")
45-
: "http://localhost:8080";
45+
: "http://localhost:4001";
4646

4747
private DurableTaskGrpcWorker worker;
4848
private DurableTaskClient client;

client/src/main/java/com/microsoft/durabletask/DurableTaskGrpcWorker.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -654,9 +654,11 @@ private void completeOrchestratorTaskWithChunking(OrchestratorResponse response)
654654
while (isPartial) {
655655
OrchestratorResponse.Builder chunk = OrchestratorResponse.newBuilder()
656656
.setInstanceId(response.getInstanceId())
657-
.setCustomStatus(response.getCustomStatus())
658657
.setCompletionToken(response.getCompletionToken())
659658
.setRequiresHistory(response.getRequiresHistory());
659+
if (response.hasCustomStatus()) {
660+
chunk.setCustomStatus(response.getCustomStatus());
661+
}
660662

661663
while (actionsCompleted < allActions.size()) {
662664
OrchestratorAction nextAction = allActions.get(actionsCompleted);

samples/src/main/java/io/durabletask/samples/LargePayloadSample.java

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,13 @@ public static void main(String[] args) throws InterruptedException, TimeoutExcep
5555
"PAYLOAD_STORAGE_CONNECTION_STRING",
5656
"UseDevelopmentStorage=true");
5757

58-
int payloadSizeBytes = Integer.parseInt(
59-
envOrDefault("PAYLOAD_SIZE_BYTES", "1572864")); // 1.5 MiB
58+
int payloadSizeBytes = intEnvOrDefault("PAYLOAD_SIZE_BYTES", 1572864); // 1.5 MiB
6059

61-
int externalizeThresholdBytes = Integer.parseInt(
62-
envOrDefault("EXTERNALIZE_THRESHOLD_BYTES", "900000"));
60+
int externalizeThresholdBytes = intEnvOrDefault("EXTERNALIZE_THRESHOLD_BYTES", 900000);
6361

6462
logger.info(String.format(
65-
"Config: payloadSize=%d bytes, threshold=%d bytes, storage=%s",
66-
payloadSizeBytes, externalizeThresholdBytes, storageConnectionString));
63+
"Config: payloadSize=%d bytes, threshold=%d bytes",
64+
payloadSizeBytes, externalizeThresholdBytes));
6765

6866
// --- Payload storage options ---
6967
LargePayloadStorageOptions payloadOptions = new LargePayloadStorageOptions()
@@ -183,4 +181,20 @@ private static String envOrDefault(String key, String defaultValue) {
183181
String value = System.getenv(key);
184182
return (value != null && !value.isEmpty()) ? value : defaultValue;
185183
}
184+
185+
private static int intEnvOrDefault(String key, int defaultValue) {
186+
String rawValue = System.getenv(key);
187+
if (rawValue == null || rawValue.isEmpty()) {
188+
return defaultValue;
189+
}
190+
try {
191+
return Integer.parseInt(rawValue);
192+
} catch (NumberFormatException e) {
193+
logger.warning(String.format(
194+
"Invalid integer value for %s: '%s'. Using default value: %d.",
195+
key, rawValue, defaultValue));
196+
return defaultValue;
197+
}
198+
}
199+
186200
}

0 commit comments

Comments
 (0)