Update DTFx.AzureStorage v2 Queue to use Base64 Encoding #1154
Conversation
| options = options ?? new QueueClientOptions(); | ||
| options.MessageEncoding = QueueMessageEncoding.Base64; |
There was a problem hiding this comment.
@nytian: I usually like to follow the "rule" that, if we're doing the same work in 3 places or more, we should probably refactor that logic into a common helper function.
Otherwise, I see the following risk: we could end up adding a new method to create a queue client, and then forget to add this "base64" encoding trick.
Can you look into a refactoring of these 3 methods where they call common logic to ensure the client options is always populated with base64 encoding?
test/DurableTask.AzureStorage.Tests/AzureStorageScenarioTests.cs
Outdated
Show resolved
Hide resolved
test/DurableTask.AzureStorage.Tests/AzureStorageScenarioTests.cs
Outdated
Show resolved
Hide resolved
| @@ -2312,6 +2314,40 @@ public async Task ScheduledStart_Activity_GetStatus_Returns_ScheduledStart(bool | |||
| } | |||
| } | |||
|
|
|||
| /// <summary> | |||
There was a problem hiding this comment.
can you please also add a test that this can still read messages sent by DTFX.AS V2.0.0 (which doesn't specify encoding)? Or do we already have that test?
| // Check the message can be successfully read without any error | ||
| Assert.IsNotNull(content); | ||
| Assert.IsInstanceOfType<string>(content); |
There was a problem hiding this comment.
can you please also check the contents of the message? Want to make sure it's de-serialized to the right string, and not a random string (which could happen under the wrong encoding)
|
Close as these changes are no longer needed. |
DTFx.AzureStorage v1.x and v2.x with different version Azure Storage SDK has different default encoding style. This pull request updates the queue encoding mechanism in the DTFx.AzureStorage v2 to use Base 64 Encoding style. This change ensures that the queue messages created by DTFx.AzureStorage v2 are forward compatible with those created by DTFx.AzureStorage v1.
Unit test has been added.