Check if JSON before Parsing in mismatch check (#456)

microsoft · Jan 4, 2023 · 8675d3f · 8675d3f
1 parent 78d3798
commit 8675d3f
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 16 deletions.
diff --git a/src/PAModel/MsAppTest.cs b/src/PAModel/MsAppTest.cs
@@ -57,7 +57,7 @@ public static bool TestClone(string pathToMsApp)
             (CanvasDocument doc1, var errors) = CanvasDocument.LoadFromMsapp(pathToMsApp);
             errors.ThrowOnErrors();
 
-            var docClone = new CanvasDocument(doc1); 
+            var docClone = new CanvasDocument(doc1);
 
             return HasNoDeltas(doc1, docClone, strict: true);
         }
@@ -154,7 +154,7 @@ public static bool StressTest(string pathToMsApp)
                     string outFile = temp1.FullPath;
 
                     var log = TextWriter.Null;
-                    
+
                     // MsApp --> Model
                     CanvasDocument msapp;
                     ErrorContainer errors = new ErrorContainer();
@@ -188,8 +188,8 @@ public static bool StressTest(string pathToMsApp)
                     using (var tempDir = new TempDir())
                     {
                         string outSrcDir = tempDir.Dir;
-                        errors = msapp.SaveToSources(outSrcDir, verifyOriginalPath : pathToMsApp);
-                        errors.ThrowOnErrors();                 
+                        errors = msapp.SaveToSources(outSrcDir, verifyOriginalPath: pathToMsApp);
+                        errors.ThrowOnErrors();
                     }
                 } // end using
 
@@ -245,10 +245,11 @@ public static bool Compare(string pathToZip1, string pathToZip2, TextWriter log,
         public static void CompareChecksums(string pathToZip, TextWriter log, Dictionary<string, byte[]> comp, bool first, ErrorContainer errorContainer)
         {
             // Path to the directory where we are creating the normalized form
-             string normFormDir = ".\\diffFiles";
+            string normFormDir = ".\\diffFiles";
 
             // Create directory if doesn't exist
-            if (!Directory.Exists(normFormDir)) {
+            if (!Directory.Exists(normFormDir))
+            {
                 Directory.CreateDirectory(normFormDir);
             }
 
@@ -261,7 +262,7 @@ public static void CompareChecksums(string pathToZip, TextWriter log, Dictionary
                     {
                         continue;
                     }
-                    
+
                     // Do easy diffs
                     {
                         if (first)
@@ -276,18 +277,41 @@ public static void CompareChecksums(string pathToZip, TextWriter log, Dictionary
                                 bool same = newContents.SequenceEqual(originalContents);
 
                                 if (!same)
-                                { 
-                                    var jsonDictionary1 = FlattenJson(originalContents);
-                                    var jsonDictionary2 = FlattenJson(newContents);
+                                {
+
+                                    bool isJson = true;
+
+                                    // Catch in case of originalContents/newContents not being JSON
+                                    try
+                                    {
+                                        JsonDocument.Parse(originalContents);
+                                        JsonDocument.Parse(newContents);
+                                    }
+                                    catch
+                                    {
+                                        isJson = false;
+                                    }
 
-                                    // Add JSONMismatch error if JSON property was changed or removed
-                                    CheckPropertyChangedRemoved(jsonDictionary1, jsonDictionary2, errorContainer, "");
+                                    if (isJson)
+                                    {
+                                        var jsonDictionary1 = FlattenJson(originalContents);
+                                        var jsonDictionary2 = FlattenJson(newContents);
+
+                                        // Add JSONMismatch error if JSON property was changed or removed
+                                        CheckPropertyChangedRemoved(jsonDictionary1, jsonDictionary2, errorContainer, "");
+
+                                        // Add JSONMismatch error if JSON property was added
+                                        CheckPropertyAdded(jsonDictionary1, jsonDictionary2, errorContainer, "");
+                                    }
 
-                                    // Add JSONMismatch error if JSON property was added
-                                    CheckPropertyAdded(jsonDictionary1, jsonDictionary2, errorContainer, "");
 #if DEBUG
                                     DebugMismatch(entry, originalContents, newContents, normFormDir);
 #endif
+
+                                    if (!isJson)
+                                    {
+                                        throw new ArgumentException($"Mismatch detected in non-Json properties: " + entry.FullName);
+                                    }
                                 }
 
                                 comp.Remove(entry.FullName);
@@ -310,7 +334,7 @@ public static Dictionary<string, JsonElement> FlattenJson(byte[] json)
                 var jsonObject = document.RootElement.EnumerateObject().SelectMany(property => GetLeaves(null, property));
                 return jsonObject.ToDictionary(key => key.Path, value => value.Property.Value.Clone());
             }
-                
+
         }
 
         public static IEnumerable<(string Path, JsonProperty Property)> GetLeaves(string path, JsonProperty property)
@@ -357,7 +381,7 @@ public static Dictionary<string, JsonElement> FlattenJson(byte[] json)
         public static IEnumerable<(string Path, JsonProperty Property)> FlattenArray(string path, JsonElement array)
         {
             List<(string arrayPath, JsonProperty arrayProperty)> enumeratedObjects = new List<(string arrayPath, JsonProperty arrayProperty)>();
-            
+
             int index = 0;
 
             foreach (var member in array.EnumerateArray())

diff --git a/src/PAModelTests/Apps/ImageApp.msapp b/src/PAModelTests/Apps/ImageApp.msapp
diff --git a/src/PAModelTests/Apps/ImageApp_SwitchNames.msapp b/src/PAModelTests/Apps/ImageApp_SwitchNames.msapp
diff --git a/src/PAModelTests/ChecksumTests.cs b/src/PAModelTests/ChecksumTests.cs
@@ -7,6 +7,7 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Reflection;
+using System.Reflection.PortableExecutable;
 using System.Text;
 
 namespace PAModelTests

diff --git a/src/PAModelTests/ErrorTests.cs b/src/PAModelTests/ErrorTests.cs
@@ -94,5 +94,19 @@ public void TestJSONValueChanged(string file1, string file2, string file3)
             // Confirm that the unit tests have the expected output
             Assert.Equal(File.ReadAllText(path3), errorContainer.ToString());
         }
+
+        [Theory]
+        [InlineData("ImageApp_SwitchNames.msapp", "ImageApp.msapp")]
+        [InlineData("ImageApp.msapp", "ImageApp_SwitchNames.msapp")]
+        public void CompareChecksum_ImageNotReadAsJSONTest(string app1, string app2)
+        {
+            var pathToZip1 = Path.Combine(Environment.CurrentDirectory, "Apps", app1);
+            var pathToZip2 = Path.Combine(Environment.CurrentDirectory, "Apps", app2);
+
+            // When there's a file content mismatch on non-JSON files,
+            // we must throw an error and not use JSON to compare non JSON-files
+            var exception = Assert.Throws<ArgumentException>(() => MsAppTest.Compare(pathToZip1, pathToZip2, Console.Out));
+            Assert.Equal("Mismatch detected in non-Json properties: Assets\\Images\\1556681b-11bd-4d72-9b17-4f884fb4b465.png", exception.Message);
+        }
     }
 }