diff --git a/edge-agent/src/Microsoft.Azure.Devices.Edge.Agent.IoTHub/EdgeAgentConnection.cs b/edge-agent/src/Microsoft.Azure.Devices.Edge.Agent.IoTHub/EdgeAgentConnection.cs index 3a75094e22b..423f272201b 100644 --- a/edge-agent/src/Microsoft.Azure.Devices.Edge.Agent.IoTHub/EdgeAgentConnection.cs +++ b/edge-agent/src/Microsoft.Azure.Devices.Edge.Agent.IoTHub/EdgeAgentConnection.cs @@ -403,8 +403,9 @@ async Task GetTwinFunc() async Task ApplyPatchAsync(TwinCollection desiredProperties, TwinCollection patch) { try + { - string mergedJson = JsonEx.Merge(desiredProperties, patch, true); + string mergedJson = JsonEx.Merge(desiredProperties, patch, true, "createOptions"); desiredProperties = new TwinCollection(mergedJson); Events.LogDesiredPropertiesAfterPatch(desiredProperties); this.desiredProperties = Option.Some(desiredProperties); diff --git a/edge-util/src/Microsoft.Azure.Devices.Edge.Util/JsonEx.cs b/edge-util/src/Microsoft.Azure.Devices.Edge.Util/JsonEx.cs index e8eb2ae2c69..a17084d4448 100644 --- a/edge-util/src/Microsoft.Azure.Devices.Edge.Util/JsonEx.cs +++ b/edge-util/src/Microsoft.Azure.Devices.Edge.Util/JsonEx.cs @@ -35,15 +35,15 @@ public static T Get(this JObject obj, string key) return token.Value(); } - public static string Merge(object baseline, object patch, bool treatNullAsDelete) + public static string Merge(object baseline, object patch, bool treatNullAsDelete, string chunkedProperty = "") { JToken baselineToken = JToken.FromObject(baseline); JToken patchToken = JToken.FromObject(patch); - JToken mergedToken = Merge(baselineToken, patchToken, treatNullAsDelete); + JToken mergedToken = Merge(baselineToken, patchToken, treatNullAsDelete, chunkedProperty); return mergedToken.ToString(); } - public static JToken Merge(JToken baselineToken, JToken patchToken, bool treatNullAsDelete) + public static JToken Merge(JToken baselineToken, JToken patchToken, bool treatNullAsDelete, string chunkedProperty = "") { // Reached the leaf JValue if (patchToken.Type != JTokenType.Object || baselineToken.Type != JTokenType.Object) @@ -55,6 +55,16 @@ public static JToken Merge(JToken baselineToken, JToken patchToken, bool treatNu var baseline = (JObject)baselineToken; var result = new JObject(baseline); + // Collect the chunked (for example createOptionsXX) keys that exist in the patch + HashSet patchChunkedNames = new HashSet(); + + if (!string.IsNullOrEmpty(chunkedProperty)) { + patchChunkedNames = patch.Properties() + .Where(p => IsChunkedName(chunkedProperty, p.Name)) + .Select(p => p.Name) + .ToHashSet(StringComparer.Ordinal); + } + foreach (JProperty patchProp in patch.Properties()) { if (IsValidToken(patchProp.Value)) @@ -62,7 +72,7 @@ public static JToken Merge(JToken baselineToken, JToken patchToken, bool treatNu JProperty baselineProp = baseline.Property(patchProp.Name); if (baselineProp != null && patchProp.Value.Type != JTokenType.Null) { - JToken nestedResult = Merge(baselineProp.Value, patchProp.Value, treatNullAsDelete); + JToken nestedResult = Merge(baselineProp.Value, patchProp.Value, treatNullAsDelete, chunkedProperty); result[patchProp.Name] = nestedResult; } else // decide whether to remove or add the patch key @@ -83,9 +93,42 @@ public static JToken Merge(JToken baselineToken, JToken patchToken, bool treatNu } } + // Clean up result from non-existing chunked properties. + if (!string.IsNullOrEmpty(chunkedProperty)) { + var resultToRemove = result.Properties() + .Where(p => + IsChunkedName(chunkedProperty, p.Name) && + (patchChunkedNames.Count == 0 || + !patchChunkedNames.Contains(p.Name))) + .Select(p => p.Name) + .ToList(); + + foreach (var name in resultToRemove) + { + result.Remove(name); + } + } return result; } + private static bool IsChunkedName(string chunkedName, string propertyName) + { + if (!propertyName.StartsWith(chunkedName, StringComparison.Ordinal)) + { + return false; + } + + // Require exactly two digits after chunked property (for example "createOptionsXX") + if (propertyName.Length != chunkedName.Length + 2) + { + return false; + } + + string suffix = propertyName.Substring(chunkedName.Length); // e.g. "01", "15" + + return int.TryParse(suffix, out int n) && n >= 0 && n <= 99; + } + public static bool IsValidToken(JToken token) => ValidDiffTypes.Any(t => t == token.Type); public static string Diff(object from, object to) diff --git a/edge-util/test/Microsoft.Azure.Devices.Edge.Util.Test/JsonExTest.cs b/edge-util/test/Microsoft.Azure.Devices.Edge.Util.Test/JsonExTest.cs index ab37499b0fa..41b468b754b 100644 --- a/edge-util/test/Microsoft.Azure.Devices.Edge.Util.Test/JsonExTest.cs +++ b/edge-util/test/Microsoft.Azure.Devices.Edge.Util.Test/JsonExTest.cs @@ -258,6 +258,165 @@ public void TestMergeAllCases() Assert.True(JToken.DeepEquals(resultCollection, JToken.FromObject(nestedEmptyBaseline))); } + [Fact] + public void TestMergeCreateOptions_remove_unused() + { + // Arrange + var baseline = new + { + module = new + { + level0 = "nochange", + level1 = "value1", + settings = new + { + createOptions = "some-create", + createOptions01 = "-options" + } + }, + }; + + var patch = new + { + module = new + { + level0 = "nochange", + level1 = "value2", + settings = new + { + createOptions = "some-create-option-that-is-not-chucked" + } + }, + }; + + var merged = new + { + module = new + { + level0 = "nochange", + level1 = "value2", + settings = new + { + createOptions = "some-create-option-that-is-not-chucked" + } + }, + }; + // Assert + JToken resultCollection = JsonEx.Merge(JToken.FromObject(baseline), JToken.FromObject(patch), true, "createOptions"); + + // Assert + Assert.True(JToken.DeepEquals(JToken.FromObject(resultCollection), JToken.FromObject(merged)), resultCollection.ToString()); + } + + [Fact] + public void TestMergeCreateOptions_remove_02_kepp_01() + { + // Arrange + var baseline = new + { + module = new + { + level0 = "nochange", + level1 = "value1", + settings = new + { + createOptions = "some-create", + createOptions01 = "-options", + createOptions02 = "-that-is-old" + } + }, + }; + + var patch = new + { + module = new + { + level0 = "nochange", + level1 = "value2", + settings = new + { + createOptions = "some-create", + createOptions01 = "-options-that-is-new" + } + }, + }; + + var merged = new + { + module = new + { + level0 = "nochange", + level1 = "value2", + settings = new + { + createOptions = "some-create", + createOptions01 = "-options-that-is-new" + } + }, + }; + // Assert + JToken resultCollection = JsonEx.Merge(JToken.FromObject(baseline), JToken.FromObject(patch), true, "createOptions"); + + // Assert + Assert.True(JToken.DeepEquals(JToken.FromObject(resultCollection), JToken.FromObject(merged)), resultCollection.ToString()); + } + + [Fact] + public void TestMergeCreateOptions3_do_not_remove_since_both_have_01_02() + { + // Arrange + var baseline = new + { + module = new + { + level0 = "nochange", + level1 = "value1", + settings = new + { + createOptions = "some-create", + createOptions01 = "-options", + createOptions02 = "-that-is-old" + } + }, + }; + + var patch = new + { + module = new + { + level0 = "nochange", + level1 = "value2", + settings = new + { + createOptions = "some-create", + createOptions01 = "-options-that-is-new", + createOptions02 = "-and-even-longer" + } + }, + }; + + var merged = new + { + module = new + { + level0 = "nochange", + level1 = "value2", + settings = new + { + createOptions = "some-create", + createOptions01 = "-options-that-is-new", + createOptions02 = "-and-even-longer" + } + }, + }; + // Assert + JToken resultCollection = JsonEx.Merge(JToken.FromObject(baseline), JToken.FromObject(patch), true, "createOptions"); + + // Assert + Assert.True(JToken.DeepEquals(JToken.FromObject(resultCollection), JToken.FromObject(merged)), resultCollection.ToString()); + } + + [Fact] public void TestDiffAllCases() {