some review comments

fix compression fix 1 test
saintentropy · Nov 15, 2023 · 5859718 · 5859718
1 parent 67a66eb
commit 5859718
Show file tree

Hide file tree

Showing 5 changed files with 145 additions and 101 deletions.
diff --git a/src/DynamoCore/Models/DynamoModel.cs b/src/DynamoCore/Models/DynamoModel.cs
@@ -1242,7 +1242,7 @@ private void EngineController_TraceReconcliationComplete(TraceReconciliationEven
         {
             Debug.WriteLine("TRACE RECONCILIATION: {0} total serializables were orphaned.", obj.CallsiteToOrphanMap.SelectMany(kvp => kvp.Value).Count());
 
-            // The orphans will come back here as a dictionary of lists of strings jeyed by their callsite id.
+            // The orphans will come back here as a dictionary of lists of strings keyed by their callsite id.
             // This dictionary gets redistributed into a dictionary keyed by the workspace id.
 
             var workspaceOrphanMap = new Dictionary<Guid, List<string>>();

diff --git a/src/Engine/ProtoCore/Lang/CallSite.cs b/src/Engine/ProtoCore/Lang/CallSite.cs
@@ -262,12 +262,12 @@ public CallSite(int classScope, string methodName,
         #region public methods
 
         /// <summary>
-        /// Load the serialised data provided into this callsite's trace cache
+        /// Load the serialized data provided into this callsite's trace cache
         /// </summary>
         /// <param name="serializedTraceData">The data to load</param>
         public void LoadSerializedDataIntoTraceCache(string serializedTraceData)
         {
-            if (serializedTraceData == null)
+            if (serializedTraceData == null || CheckIfTraceDataIsLegacySOAPFormat(serializedTraceData))
             {
                 beforeFirstRunSerializables = new List<string>();
                 return;
@@ -277,8 +277,8 @@ public void LoadSerializedDataIntoTraceCache(string serializedTraceData)
             try
             {
                 //Optional Compression / Decompression
-                serializedTraceData = DecompressSerializedTraceData(serializedTraceData);
-                newTraceData = JsonConvert.DeserializeObject<List<SingleRunTraceData>>(serializedTraceData);
+                var decompressedTraceData = DecompressSerializedTraceData(serializedTraceData);
+                newTraceData = JsonConvert.DeserializeObject<List<SingleRunTraceData>>(decompressedTraceData);
             }
             catch
             {
@@ -453,14 +453,16 @@ public string GetTraceDataToSave()
         /// </summary>
         /// <param name="json"></param>
         /// <returns></returns>
-        public static string CompressSerializedTraceData(string json)
+        private static string CompressSerializedTraceData(string json)
         {
             byte[] dataToCompress = Encoding.UTF8.GetBytes(json);
 
             using (var memoryStream = new MemoryStream())
             using (var gzipStream = new GZipStream(memoryStream, CompressionLevel.Optimal))
             {
                     gzipStream.Write(dataToCompress, 0, dataToCompress.Length);
+                    //we ned to flush the gzip stream to force write to be done BEFORE we access the memory stream.
+                    gzipStream.Close();
                     return Convert.ToBase64String(memoryStream.ToArray());
             }
         }
@@ -562,6 +564,21 @@ private void UpdateCallsiteExecutionState(Object callsiteData, RuntimeCore runti
                 }
             }*/
         }
+        //TODO write a unit test for this.
+        private static bool CheckIfTraceDataIsLegacySOAPFormat(string base64EncodedTraceData)
+        {
+            var data = Convert.FromBase64String(base64EncodedTraceData);
+            if (data.Length > 71)
+            {
+                var header = Encoding.UTF8.GetString(data, 0, 72);
+                if (header == @"<SOAP-ENV:Envelope xmlns:xsi=""http://www.w3.org/2001/XMLSchema-instance""")
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
 
         #endregion
 
@@ -1878,7 +1895,7 @@ public static StackValue PerformReturnTypeCoerce(FunctionEndPoint functionEndPoi
         public static IList<string> GetAllSerializablesFromSingleRunTraceData(
             RawTraceData callSiteData)
         {
-            if (callSiteData.Data == null)
+            if (callSiteData.Data == null || CheckIfTraceDataIsLegacySOAPFormat(callSiteData.Data))
             {
                 return new List<string>();
             }
@@ -1892,7 +1909,7 @@ public static IList<string> GetAllSerializablesFromSingleRunTraceData(
             }
             catch
             {
-                //Do nothing.  Old Format data.
+                //Do nothing.
             }
 
             if (traceData == null)

diff --git a/src/Engine/ProtoCore/RuntimeData.cs b/src/Engine/ProtoCore/RuntimeData.cs
@@ -348,7 +348,7 @@ private string GetAndRemoveTraceDataForNode(Guid nodeGuid, string callsiteID)
                 }
             }
 
-            // For backword compatibility: old dyn file doesn't have CallSiteID
+            // For backward compatibility: old dyn file doesn't have CallSiteID
             // attribute, so the call site id will be empty string.
             if (callsiteTraceData == null && !string.IsNullOrEmpty(callsiteID))
             {

diff --git a/test/Engine/FFITarget/MinimalTraceTargets.cs b/test/Engine/FFITarget/MinimalTraceTargets.cs
@@ -118,7 +118,6 @@ public void Dispose()
     }
 
 
-    [Serializable]
     internal class IDHolder
     {
         [JsonProperty] 
@@ -131,14 +130,8 @@ public IDHolder()
     }
 
 
-    [Serializable]
     internal class DummyDataHolder
     {
-        public void GetObjectData(SerializationInfo info, StreamingContext context)
-        {
-            throw new NotImplementedException();
-        }
-
         public DummyDataHolder() { }
     }