From 72baac0d33da3d1e73488a4ed660baf878972f9e Mon Sep 17 00:00:00 2001 From: Craig Long Date: Mon, 13 Nov 2023 12:39:17 -0500 Subject: [PATCH] PR comments --- src/Libraries/CoreNodeModels/Remember.cs | 8 +-- src/Libraries/CoreNodes/Data.cs | 66 ++++++++++++------- .../Properties/Resources.Designer.cs | 15 ++++- .../CoreNodes/Properties/Resources.en-US.resx | 7 +- .../CoreNodes/Properties/Resources.resx | 7 +- test/DynamoCoreTests/DSCoreDataTests.cs | 19 ++++-- 6 files changed, 84 insertions(+), 38 deletions(-) diff --git a/src/Libraries/CoreNodeModels/Remember.cs b/src/Libraries/CoreNodeModels/Remember.cs index 527ee7001ad..ba4c846940d 100644 --- a/src/Libraries/CoreNodeModels/Remember.cs +++ b/src/Libraries/CoreNodeModels/Remember.cs @@ -13,18 +13,16 @@ namespace CoreNodeModels [NodeDescription(nameof(Properties.Resources.RememberDescription), typeof(Properties.Resources))] [NodeCategory("Core.Data")] [InPortNames(">")] - [InPortTypes("object")] + [InPortTypes("var[]..[]")] [InPortDescriptions(typeof(Properties.Resources), nameof(Properties.Resources.RememberInputToolTip))] [OutPortNames(">")] - [OutPortTypes("object")] + [OutPortTypes("var[]..[]")] [OutPortDescriptions(typeof(Properties.Resources), nameof(Properties.Resources.RememberOuputToolTip))] [IsDesignScriptCompatible] - [DynamoServices.RegisterForTrace] public class Remember : NodeModel { private string cache = ""; - [JsonProperty("Cache")] public string Cache { get { return cache; } @@ -55,7 +53,7 @@ private void OnPropertyChanged(object sender, PropertyChangedEventArgs e) { switch (e.PropertyName) { - case "State": + case nameof(State): if (State == ElementState.Warning) { Cache = ""; diff --git a/src/Libraries/CoreNodes/Data.cs b/src/Libraries/CoreNodes/Data.cs index 6391c3b7660..2be115245d8 100644 --- a/src/Libraries/CoreNodes/Data.cs +++ b/src/Libraries/CoreNodes/Data.cs @@ -98,7 +98,7 @@ private static object DynamoJObjectToNative(JObject jObject) case "dynamo.geometry:mesh-1.0.0": return Mesh.FromJson(jObject.ToString()); - //types supported by Goemetry.FromJson + //types supported by Geometry.FromJson case "autodesk.math:point3d-1.0.0": case "dynamo.geometry:sab-1.0.0": case "dynamo.geometry:tsm-1.0.0": @@ -110,34 +110,56 @@ private static object DynamoJObjectToNative(JObject jObject) //Dynamo types case "dynamo.graphics:color-1.0.0": - return Color.ByARGB( - (int)jObject["A"], - (int)jObject["R"], - (int)jObject["G"], - (int)jObject["B"]); + try + { + return Color.ByARGB( + (int)jObject["A"], + (int)jObject["R"], + (int)jObject["G"], + (int)jObject["B"]); + } + catch { + throw new FormatException(string.Format(Properties.Resources.Exception_Deserialize_Bad_Format, typeof(Color).FullName)); + } + #if _WINDOWS case "dynamo.graphics:png-1.0.0": + jObject.TryGetValue(ImageFormat.Png.ToString(), out var value); if (value != null) { - var stream = Convert.FromBase64String(value.ToString()); - - Bitmap bitmap; - using (var ms = new MemoryStream(stream)) - bitmap = new Bitmap(Bitmap.FromStream(ms)); - - return bitmap; + try + { + var stream = Convert.FromBase64String(value.ToString()); + + Bitmap bitmap; + using (var ms = new MemoryStream(stream)) + bitmap = new Bitmap(Bitmap.FromStream(ms)); + + return bitmap; + } + catch { + //Pass through to the next throw + } } -#endif + throw new FormatException(string.Format(Properties.Resources.Exception_Deserialize_Bad_Format, "dynamo.graphics:png-1.0.0")); +#else return null; - +#endif case "dynamo.data:location-1.0.0": - return DynamoUnits.Location.ByLatitudeAndLongitude( - (double)jObject["Latitude"], - (double)jObject["Longitude"], - (string)jObject["Name"]); + try + { + return DynamoUnits.Location.ByLatitudeAndLongitude( + (double)jObject["Latitude"], + (double)jObject["Longitude"], + (string)jObject["Name"]); + } + catch + { + throw new FormatException(string.Format(Properties.Resources.Exception_Deserialize_Bad_Format, typeof(DynamoUnits.Location).FullName)); + } default: return null; @@ -171,7 +193,7 @@ public static string StringifyJSON([ArbitraryDimensionArrayImport] object values new ColorConveter(), new LocationConverter(), #if _WINDOWS - new ImageConverter(), + new PNGImageConverter(), #endif }); } @@ -325,7 +347,7 @@ public override bool CanConvert(Type objectType) #if NET6_0_OR_GREATER [SupportedOSPlatform("windows")] #endif - private class ImageConverter : JsonConverter + private class PNGImageConverter : JsonConverter { public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) { @@ -404,7 +426,7 @@ public static Dictionary Remember([ArbitraryDimensionArrayImport } catch { - throw new NotSupportedException(Properties.Resources.Exception_Deserialize_Unsupported_Type); + throw new NotSupportedException(Properties.Resources.Exception_Deserialize_Unsupported_Cache); } return new Dictionary diff --git a/src/Libraries/CoreNodes/Properties/Resources.Designer.cs b/src/Libraries/CoreNodes/Properties/Resources.Designer.cs index 96db1723081..26f019238be 100644 --- a/src/Libraries/CoreNodes/Properties/Resources.Designer.cs +++ b/src/Libraries/CoreNodes/Properties/Resources.Designer.cs @@ -133,11 +133,20 @@ internal static string EnumDateOfWeekWednesday { } /// - /// Looks up a localized string similar to The stored data can not be rebuilt.. + /// Looks up a localized string similar to The json for the type {0} was not formatted correctly. /// - internal static string Exception_Deserialize_Unsupported_Type { + internal static string Exception_Deserialize_Bad_Format { get { - return ResourceManager.GetString("Exception_Deserialize_Unsupported_Type", resourceCulture); + return ResourceManager.GetString("Exception_Deserialize_Bad_Format", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The stored data can not be loaded.. + /// + internal static string Exception_Deserialize_Unsupported_Cache { + get { + return ResourceManager.GetString("Exception_Deserialize_Unsupported_Cache", resourceCulture); } } diff --git a/src/Libraries/CoreNodes/Properties/Resources.en-US.resx b/src/Libraries/CoreNodes/Properties/Resources.en-US.resx index dc1c3222e18..dbcca25c411 100644 --- a/src/Libraries/CoreNodes/Properties/Resources.en-US.resx +++ b/src/Libraries/CoreNodes/Properties/Resources.en-US.resx @@ -141,8 +141,11 @@ Wednesday - - The stored data can not be rebuilt. + + The json for the type {0} was not formatted correctly + + + The stored data can not be loaded. Cannot store data of type {0}. diff --git a/src/Libraries/CoreNodes/Properties/Resources.resx b/src/Libraries/CoreNodes/Properties/Resources.resx index 43b8b5490c4..770037c270b 100644 --- a/src/Libraries/CoreNodes/Properties/Resources.resx +++ b/src/Libraries/CoreNodes/Properties/Resources.resx @@ -141,8 +141,11 @@ Wednesday - - The stored data can not be rebuilt. + + The json for the type {0} was not formatted correctly + + + The stored data can not be loaded. Cannot store data of type {0}. diff --git a/test/DynamoCoreTests/DSCoreDataTests.cs b/test/DynamoCoreTests/DSCoreDataTests.cs index af67e748c27..7fb581cd6e5 100644 --- a/test/DynamoCoreTests/DSCoreDataTests.cs +++ b/test/DynamoCoreTests/DSCoreDataTests.cs @@ -203,7 +203,8 @@ public void RoundTripForBoundingBoxReturnsSameResult() // Verify objects match when serializing / de-serializing geometry type AssertPreviewValue("abb39e07-db08-45cf-9438-478defffbf68", true); - // Verify current failure cases are all false + // Currently we do not support oriented BB. + // This test will verify current unsupported cases AssertPreviewValue("eb9130a1-309c-492a-9679-28ad5ef8fddf", true); } @@ -243,6 +244,7 @@ public void RoundTripForVectorReturnsSameResult() AssertPreviewValue("71efc8c5c0c74189901707c30e6d5903", true); } + //Waiting for upstream PR so that we can mark as [Category("UnitTests")] [Test] [Category("Failure")] public void RoundTripForArcReturnsSameResult() @@ -254,7 +256,9 @@ public void RoundTripForArcReturnsSameResult() // Verify objects match when serializing / de-serializing geometry type AssertPreviewValue("71efc8c5c0c74189901707c30e6d5903", true); - // Verify current failure cases are all false + // A known issue is that Arcs do not deserialize with the same start angle value. + // It is always zero although the curve topology is identical. + // This will verify the current known edge case. AssertPreviewValue("82304dd5025948f8a5644a84a32d58d4", true); } @@ -293,7 +297,9 @@ public void RoundTripForEllipseArcReturnsSameResult() // Verify objects match when serializing / de-serializing geometry type AssertPreviewValue("a29aa179c7ae4069a6d9c6d2055ab845", true); - // Verify current failure cases are all false + // A known issue is that EllipseArcs do not deserialize with the same start angle value. + // It is always zero although the curve topology is identical. + // This will verify the current known edge case. AssertPreviewValue("a73925f57d2c44d7994a2c4d77bf8581", true); } @@ -308,7 +314,10 @@ public void RoundTripForHelixReturnsSameResult() // Verify objects match when serializing / de-serializing geometry type AssertPreviewValue("b6a4919b3dd94eb79a7f0435d941d235", true); - // Verify current failure cases are all false + // A known issue is that Helix do not deserialize with the same type. + // It is always converted to nurbscurve (Same as SAB serialization). + // When the spiral GeoemtrySchema type is finalized we use it to support helix. + // This will verify the current known unsupported case. AssertPreviewValue("1bbd147b429c43ab8fe46a00d691a024", true); } @@ -336,6 +345,7 @@ public void RoundTripForNurbsCurveReturnsSameResult() AssertPreviewValue("423356e2c8f84e00aa6c50e9bdb72c98", true); } + //Waiting for upstream PR so that we can mark as [Category("UnitTests")] [Test] [Category("Failure")] public void RoundTripForPolyCurveReturnsSameResult() @@ -384,6 +394,7 @@ public void RoundTripForPointReturnsSameResult() AssertPreviewValue("71efc8c5c0c74189901707c30e6d5903", true); } + //Waiting for upstream PR so that we can mark as [Category("UnitTests")] [Test] [Category("Failure")] public void RoundTripForCylinderReturnsSameResult()