-
Notifications
You must be signed in to change notification settings - Fork 176
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix Function Serialization and Input Validation (#1290)
- Fix incorrect range check on max watermark for the GetInstanceBatches*Async activities - Remove $type from durable function extension input/output representation - Re-enable tests for PAAS to prevent future regressions
- Loading branch information
Showing
11 changed files
with
146 additions
and
24 deletions.
There are no files selected for viewing
This file contains 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
25 changes: 25 additions & 0 deletions
25
...icrosoft.Health.Dicom.Operations.Client/Serialization/MessageSerializerSettingsFactory.cs
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| // ------------------------------------------------------------------------------------------------- | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. | ||
| // ------------------------------------------------------------------------------------------------- | ||
|
|
||
| using Microsoft.Azure.WebJobs.Extensions.DurableTask; | ||
| using Newtonsoft.Json; | ||
|
|
||
| namespace Microsoft.Health.Dicom.Operations.Client.Serialization | ||
| { | ||
| // TODO: Migrate to common package | ||
| internal class MessageSerializerSettingsFactory : IMessageSerializerSettingsFactory | ||
| { | ||
| public JsonSerializerSettings CreateJsonSerializerSettings() | ||
| { | ||
| // Based on the framework settings: | ||
| // https://github.com/Azure/azure-functions-durable-extension/blob/v2.6.0/src/WebJobs.Extensions.DurableTask/MessageSerializerSettingsFactory.cs | ||
| return new JsonSerializerSettings | ||
| { | ||
| TypeNameHandling = TypeNameHandling.None, | ||
| DateParseHandling = DateParseHandling.None, | ||
| }; | ||
| } | ||
| } | ||
| } |
This file contains 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
This file contains 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
This file contains 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
65 changes: 65 additions & 0 deletions
65
....Health.Dicom.Operations.UnitTests/Serialization/MessageSerializerSettingsFactoryTests.cs
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| // ------------------------------------------------------------------------------------------------- | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. | ||
| // ------------------------------------------------------------------------------------------------- | ||
|
|
||
| using System.Collections.Generic; | ||
| using Microsoft.Health.Dicom.Core.Features.ExtendedQueryTag; | ||
| using Microsoft.Health.Dicom.Core.Features.Model; | ||
| using Microsoft.Health.Dicom.Operations.Indexing.Models; | ||
| using Microsoft.Health.Dicom.Operations.Serialization; | ||
| using Newtonsoft.Json; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.Health.Dicom.Operations.UnitTests.Serialization | ||
| { | ||
| public class MessageSerializerSettingsFactoryTests | ||
| { | ||
| private readonly static JsonSerializerSettings JsonSerializerSettings = new MessageSerializerSettingsFactory().CreateJsonSerializerSettings(); | ||
|
|
||
| [Fact] | ||
| public void GivenPreviouslySerializedMessage_WhenDeserializingWithNewSettings_ThenDeserializeSuccessfully() | ||
| { | ||
| var queryTags = new List<ExtendedQueryTagStoreEntry> | ||
| { | ||
| new ExtendedQueryTagStoreEntry(1, "01", "DT", "foo", QueryTagLevel.Instance, ExtendedQueryTagStatus.Adding, QueryStatus.Enabled, 0), | ||
| new ExtendedQueryTagStoreEntry(2, "02", "DT", "bar", QueryTagLevel.Study, ExtendedQueryTagStatus.Adding, QueryStatus.Enabled, 0), | ||
| }; | ||
| var range = new WatermarkRange(5, 10); | ||
| const int threadCount = 7; | ||
|
|
||
| var before = new ReindexBatchArguments(queryTags, range, threadCount); | ||
| string json = JsonConvert.SerializeObject( | ||
| before, | ||
| new JsonSerializerSettings | ||
| { | ||
| TypeNameHandling = TypeNameHandling.Objects, | ||
| TypeNameAssemblyFormatHandling = TypeNameAssemblyFormatHandling.Simple, | ||
| }); | ||
|
|
||
| ReindexBatchArguments actual = JsonConvert.DeserializeObject<ReindexBatchArguments>(json, JsonSerializerSettings); | ||
|
|
||
| Assert.Equal(before.QueryTags, actual.QueryTags, new TagEntryComparer()); | ||
| Assert.Equal(before.ThreadCount, actual.ThreadCount); | ||
| Assert.Equal(before.WatermarkRange, actual.WatermarkRange); | ||
| } | ||
|
|
||
| private sealed class TagEntryComparer : IEqualityComparer<ExtendedQueryTagStoreEntry> | ||
| { | ||
| public bool Equals(ExtendedQueryTagStoreEntry x, ExtendedQueryTagStoreEntry y) | ||
| => x.ErrorCount == y.ErrorCount | ||
| || x.Key == y.Key | ||
| || x.Level == y.Level | ||
| || x.Path == y.Path | ||
| || x.PrivateCreator == y.PrivateCreator | ||
| || x.QueryStatus == y.QueryStatus | ||
| || x.Status == y.Status | ||
| || x.VR == y.VR; | ||
|
|
||
| public int GetHashCode(ExtendedQueryTagStoreEntry obj) | ||
| { | ||
| throw new System.NotImplementedException(); | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains 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
This file contains 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
This file contains 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
25 changes: 25 additions & 0 deletions
25
src/Microsoft.Health.Dicom.Operations/Serialization/MessageSerializerSettingsFactory.cs
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| // ------------------------------------------------------------------------------------------------- | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License (MIT). See LICENSE in the repo root for license information. | ||
| // ------------------------------------------------------------------------------------------------- | ||
|
|
||
| using Microsoft.Azure.WebJobs.Extensions.DurableTask; | ||
| using Newtonsoft.Json; | ||
|
|
||
| namespace Microsoft.Health.Dicom.Operations.Serialization | ||
| { | ||
| // TODO: Migrate to common package | ||
| internal class MessageSerializerSettingsFactory : IMessageSerializerSettingsFactory | ||
| { | ||
| public JsonSerializerSettings CreateJsonSerializerSettings() | ||
| { | ||
| // Based on the framework settings: | ||
| // https://github.com/Azure/azure-functions-durable-extension/blob/v2.6.0/src/WebJobs.Extensions.DurableTask/MessageSerializerSettingsFactory.cs | ||
| return new JsonSerializerSettings | ||
| { | ||
| TypeNameHandling = TypeNameHandling.None, | ||
| DateParseHandling = DateParseHandling.None, | ||
| }; | ||
| } | ||
| } | ||
| } |
This file contains 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