Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DYN-6394: Add legacy trace warning when opening workspace #14628

Merged
merged 41 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
64b4706
remove coreclr-ncalc references
aparajit-pratap Aug 23, 2023
4a82ae7
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Aug 25, 2023
dbc3e12
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Aug 30, 2023
81ace20
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Aug 31, 2023
a7cafdc
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Sep 6, 2023
55c654d
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Sep 6, 2023
7ede2a4
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Sep 7, 2023
c43a02d
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Sep 8, 2023
1951d1a
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Sep 20, 2023
cd100b1
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Sep 21, 2023
c04c193
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Sep 25, 2023
35edbdb
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Oct 16, 2023
5c6a4a3
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Oct 18, 2023
1641149
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Oct 19, 2023
eff8d9b
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Oct 20, 2023
8415e17
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Oct 23, 2023
8a3368b
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Oct 30, 2023
3350614
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Nov 6, 2023
be0a127
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Nov 10, 2023
a3d62e9
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Nov 15, 2023
a3c806e
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Nov 17, 2023
e93499d
Merge branch 'master' of github.com:DynamoDS/Dynamo
aparajit-pratap Nov 20, 2023
37d470c
show legacy trace warning
aparajit-pratap Nov 20, 2023
9c09ab3
cleanup
aparajit-pratap Nov 20, 2023
946e529
get workspace version correctly
aparajit-pratap Nov 20, 2023
5a9d878
update warning
aparajit-pratap Nov 20, 2023
1dd5c3c
add warning for xml DYNs
aparajit-pratap Nov 20, 2023
e26d682
add unit test
aparajit-pratap Nov 20, 2023
1b38c4d
cleanup
aparajit-pratap Nov 20, 2023
a2a6256
review comments
aparajit-pratap Nov 21, 2023
a39ae7f
Merge branch 'master' of github.com:DynamoDS/Dynamo into dyn-6394
aparajit-pratap Nov 22, 2023
77cc25d
update tests
aparajit-pratap Nov 22, 2023
16d50d4
revert test fixture change
aparajit-pratap Nov 22, 2023
2d96ffd
Merge branch 'master' of github.com:DynamoDS/Dynamo into dyn-6394
aparajit-pratap Nov 22, 2023
97d5987
refactor, add test
aparajit-pratap Nov 22, 2023
3e0cbd5
cleanup
aparajit-pratap Nov 22, 2023
d8e2dc2
rename
aparajit-pratap Nov 22, 2023
71eb83c
review comments
aparajit-pratap Nov 22, 2023
e5a56ed
update tests
aparajit-pratap Nov 27, 2023
456c20e
update tests
aparajit-pratap Nov 27, 2023
0c03faf
revert unchanged test file
aparajit-pratap Nov 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions src/DynamoCore/Graph/Nodes/NodeCategories.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
Expand All @@ -8,6 +8,7 @@
using Dynamo.Configuration;
using Dynamo.Engine;
using Dynamo.Graph.Nodes.CustomNodes;
using Dynamo.Graph.Workspaces;
using Dynamo.Library;
using ProtoCore;

Expand Down Expand Up @@ -282,7 +283,7 @@ internal static string GetDocumentXmlPath(XmlDocument document)
/// <returns>Returns a dictionary of deserialized node-data-list pairs
/// loaded from the given XmlDocument.</returns>
internal static IEnumerable<KeyValuePair<Guid, List<CallSite.RawTraceData>>>
LoadTraceDataFromXmlDocument(XmlDocument document)
LoadTraceDataFromXmlDocument(XmlDocument document, out bool containsTraceData)
{
if (document == null)
throw new ArgumentNullException("document");
Expand All @@ -301,7 +302,10 @@ where childNode.Name.Equals(sessionXmlTagName)

var loadedData = new Dictionary<Guid, List<CallSite.RawTraceData>>();
if (!query.Any()) // There's no data, return empty dictionary.
{
containsTraceData = false;
return loadedData;
}

XmlElement sessionElement = query.ElementAt(0);
foreach (XmlElement nodeElement in sessionElement.ChildNodes)
Expand All @@ -313,14 +317,12 @@ where childNode.Name.Equals(sessionXmlTagName)
var callsiteId = string.Empty;
if (child.HasAttribute(Configurations.CallSiteID))
{
callsiteId = child.GetAttribute(Configurations.CallSiteID);
containsTraceData = true;
return loadedData;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so now, we just return an empty list no matter what, but we make a note that there was at least 1 node with trace data?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed the variable from containsTraceData to containsLegacyTraceData to make it clearer. This flag is only used to trigger the toast notification when opening an old workspace.

var traceData = child.InnerText;
callsiteTraceData.Add(new CallSite.RawTraceData(callsiteId, traceData));
}
loadedData.Add(guid, callsiteTraceData);
}

containsTraceData = false;
return loadedData;
}

Expand Down
10 changes: 8 additions & 2 deletions src/DynamoCore/Graph/Workspaces/SerializationConverters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -679,9 +679,12 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
#region Restore trace data
// Trace Data
Dictionary<Guid, List<CallSite.RawTraceData>> loadedTraceData = new Dictionary<Guid, List<CallSite.RawTraceData>>();
bool containsTraceData = false;
// Restore trace data if bindings are present in json
if (obj["Bindings"] != null && obj["Bindings"].Children().Count() > 0)
{
containsTraceData = true;
aparajit-pratap marked this conversation as resolved.
Show resolved Hide resolved

JEnumerable<JToken> bindings = obj["Bindings"].Children();

// Iterate through bindings to extract nodeID's and bindingData (callsiteId & traceData)
Expand Down Expand Up @@ -725,7 +728,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
if (obj.TryGetValue(nameof(HomeWorkspaceModel.Thumbnail), StringComparison.OrdinalIgnoreCase, out JToken thumbnail))
homeWorkspace.Thumbnail = thumbnail.ToString();

// GraphDocumentaionLink
// GraphDocumentationLink
if (obj.TryGetValue(nameof(HomeWorkspaceModel.GraphDocumentationURL), StringComparison.OrdinalIgnoreCase, out JToken helpLink))
{
if (Uri.TryCreate(helpLink.ToString(), UriKind.Absolute, out Uri uri))
Expand All @@ -738,6 +741,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
// If there is a active linter serialized in the graph we set it to the active linter else set the default None.
SetActiveLinter(obj);


ws = homeWorkspace;
}

Expand All @@ -746,7 +750,9 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
ws.ExternalFiles = externalFiles;
if (obj.TryGetValue(nameof(WorkspaceModel.Author), StringComparison.OrdinalIgnoreCase, out JToken author))
ws.Author = author.ToString();


ws.ContainsTraceData = containsTraceData;

return ws;
}

Expand Down
2 changes: 2 additions & 0 deletions src/DynamoCore/Graph/Workspaces/WorkspaceModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,8 @@ internal int CurrentPasteOffset
}
}

internal bool ContainsTraceData { get; set; }

internal bool ScaleFactorChanged = false;

/// <summary>
Expand Down
24 changes: 15 additions & 9 deletions src/DynamoCore/Models/DynamoModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2320,15 +2320,17 @@ private bool OpenJsonFile(
workspace.FromJsonGraphId = string.IsNullOrEmpty(filePath) ? WorkspaceModel.ComputeGraphIdFromJson(fileContents) : "";
workspace.ScaleFactor = dynamoPreferences.ScaleFactor;

// NOTE: This is to handle the case of opening a JSON file that does not have a version string
// This logic may not be correct, need to decide the importance of versioning early JSON files
string versionString = dynamoPreferences.Version;
if (versionString == null)
versionString = AssemblyHelper.GetDynamoVersion().ToString();
workspace.WorkspaceVersion = new System.Version(versionString);

HomeWorkspaceModel homeWorkspace = workspace as HomeWorkspaceModel;
if (homeWorkspace != null)
// This is to handle the case of opening a JSON file that does not have a version string
workspace.WorkspaceVersion = dynamoPreferences.Version ==
null ? AssemblyHelper.GetDynamoVersion() : new Version(dynamoPreferences.Version);

if (workspace.ContainsTraceData && workspace.WorkspaceVersion < new Version(3, 0, 0))
{
OnRequestNotification(Resources.LegacyTraceDataWarning, true);
aparajit-pratap marked this conversation as resolved.
Show resolved Hide resolved
}

if (workspace is HomeWorkspaceModel homeWorkspace)
{
homeWorkspace.EnableLegacyPolyCurveBehavior ??= PreferenceSettings.Instance.DefaultEnableLegacyPolyCurveBehavior;

Expand Down Expand Up @@ -2451,12 +2453,16 @@ private bool OpenXmlHomeWorkspace(
{
var nodeGraph = NodeGraph.LoadGraphFromXml(xmlDoc, NodeFactory);
Guid deterministicId = GuidUtility.Create(GuidUtility.UrlNamespace, workspaceInfo.Name);

var loadedTraceData = Utils.LoadTraceDataFromXmlDocument(xmlDoc, out var containsTraceData);
if(containsTraceData) OnRequestNotification(Resources.LegacyTraceDataWarning, true);

var newWorkspace = new HomeWorkspaceModel(
deterministicId,
EngineController,
Scheduler,
NodeFactory,
Utils.LoadTraceDataFromXmlDocument(xmlDoc),
loadedTraceData,
nodeGraph.Nodes,
nodeGraph.Notes,
nodeGraph.Annotations,
Expand Down
6 changes: 3 additions & 3 deletions src/DynamoCore/Models/DynamoModelEvents.cs
Original file line number Diff line number Diff line change
Expand Up @@ -541,12 +541,12 @@ internal void OnRequestPythonReset(string pythonEngine)
/// <summary>
/// This event is used to raise a toast notification from the DynamoViewModel
/// </summary>
internal event Action<string> RequestNotification;
internal void OnRequestNotification(string notification)
internal event Action<string, bool> RequestNotification;
internal void OnRequestNotification(string notification, bool stayOpen = false)
{
if (RequestNotification != null)
{
RequestNotification(notification);
RequestNotification(notification, stayOpen);
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/DynamoCore/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/DynamoCore/Properties/Resources.en-US.resx
Original file line number Diff line number Diff line change
Expand Up @@ -908,4 +908,7 @@ This package likely contains an assembly that is blocked. You will need to load
<data name="FormulaMigrated" xml:space="preserve">
<value>Formula node has been deprecated. It has been automatically migrated to a CodeBlock node. Note that results may vary after the migration depending on lacing options selected on the original Formula node. Appropriate replication guides might need to be applied to the CodeBlock node script.</value>
</data>
<data name="LegacyTraceDataWarning" xml:space="preserve">
<value>This workspace contains element binding data in a legacy format that is no longer supported in Dynamo 3.0 and higher versions. Element binding will work however, if any new trace data generated during graph execution is saved with the workspace.</value>
aparajit-pratap marked this conversation as resolved.
Show resolved Hide resolved
</data>
</root>
3 changes: 3 additions & 0 deletions src/DynamoCore/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -911,4 +911,7 @@ This package likely contains an assembly that is blocked. You will need to load
<data name="FormulaMigrated" xml:space="preserve">
<value>Formula node has been deprecated. It has been automatically migrated to a CodeBlock node. Note that results may vary after the migration depending on lacing options selected on the original Formula node. Appropriate replication guides might need to be applied to the CodeBlock node script.</value>
</data>
<data name="LegacyTraceDataWarning" xml:space="preserve">
<value>This workspace contains element binding data in a legacy format that is no longer supported in Dynamo 3.0 and higher versions. Element binding will work however, if any new trace data generated during graph execution is saved with the workspace.</value>
</data>
</root>
4 changes: 2 additions & 2 deletions src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1907,9 +1907,9 @@ private void model_ComputeModelDeserialized()
/// Create a toast notification with a notification sent by the DynamoModel
/// </summary>
/// <param name="notification"></param>
private void model_RequestNotification(string notification)
private void model_RequestNotification(string notification, bool stayOpen = false)
{
this.MainGuideManager.CreateRealTimeInfoWindow(notification);
this.MainGuideManager.CreateRealTimeInfoWindow(notification, stayOpen);
}

/// <summary>
Expand Down
13 changes: 13 additions & 0 deletions test/DynamoCoreTests/CallsiteTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -334,5 +334,18 @@ public void CanDetectLegacyTraceFormat()
"PFNPQVAtRU5WOkVudmVsb3BlIHhtbG5zOnhzaT0iaHR0cDovL3d3dyY2hlbWFzLm1pY3Jvc29mdC5jb20vc29hcC9lbmNvZGluZy";
Assert.True(CheckIfTraceDataIsLegacySOAPFormat(legacyTraceData));
}

[Test]
public void CanWarnAboutLegacyTraceData()
{
var counter = 99;
CurrentDynamoModel.RequestNotification += (_, _) => { counter++; };

// Dyn file contains SOAP formatted trace data.
var ws = Open<HomeWorkspaceModel>(TestDirectory, callsiteDir,
"element_binding_customNodes_modified_multiple_pre3.0.dyn");

Assert.AreEqual(100, counter);
}
}
}
6 changes: 3 additions & 3 deletions test/DynamoCoreTests/UtilityTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,14 @@ public void LoadTraceDataFromXmlDocument00()
Assert.Throws<ArgumentNullException>(() =>
{
// Test method call without a valid XmlDocument.
Graph.Nodes.Utilities.LoadTraceDataFromXmlDocument(null);
Graph.Nodes.Utilities.LoadTraceDataFromXmlDocument(null, out _);
});

Assert.Throws<ArgumentException>(() =>
{
// Test XmlDocument without a document element.
XmlDocument document = new XmlDocument();
Graph.Nodes.Utilities.LoadTraceDataFromXmlDocument(document);
Graph.Nodes.Utilities.LoadTraceDataFromXmlDocument(document, out _);
});
}

Expand All @@ -321,7 +321,7 @@ public void LoadTraceDataFromXmlDocument01()
{
XmlDocument document = new XmlDocument();
document.AppendChild(document.CreateElement("RootElement"));
outputs = Graph.Nodes.Utilities.LoadTraceDataFromXmlDocument(document);
outputs = Graph.Nodes.Utilities.LoadTraceDataFromXmlDocument(document, out _);
});

Assert.IsNotNull(outputs);
Expand Down
Loading
Loading