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.

Sign up for GitHub

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-2149 DummyNode unresolved node messages are confusing #10051

Merged
merged 6 commits into from
Oct 16, 2019
Merged

DYN-2149 DummyNode unresolved node messages are confusing #10051

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
759041c
First commit
reddyashish Oct 11, 2019
d983d58
Change the warning message for the node model nodes
reddyashish Oct 14, 2019
a8c992f
Displaying the type name of the node model node.
reddyashish Oct 15, 2019
d14b311
Using the whole namespace to check for zerotouch nodes.
reddyashish Oct 15, 2019
69d0cab
Adding test
reddyashish Oct 15, 2019
2de8a87
some more comments
reddyashish Oct 16, 2019
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
72 changes: 62 additions & 10 deletions src/DynamoCore/Graph/Nodes/DummyNode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public DummyNode()
LegacyNodeName = "Dynamo.Graph.Nodes.DummyNode";
LegacyFullName = LegacyNodeName;
LegacyAssembly = string.Empty;
FunctionName = string.Empty;
NodeNature = Nature.Unresolved;
Description = GetDescription();
ShouldDisplayPreviewCore = false;
Expand Down Expand Up @@ -142,6 +143,52 @@ public DummyNode(
UpdatePorts();
}

/// <summary>
/// This function creates DummyNode with specified number of ports.
/// </summary>
/// <param name="id">Id of the original node</param>
/// <param name="inputCount">Number of input ports</param>
/// <param name="outputCount">Number of output ports</param>
/// <param name="legacyAssembly">Assembly of the node</param>
/// <param name="functionName">Function name of the node</param>
/// <param name="originalElement">Original JSON description of the node</param>
public DummyNode(
string id,
int inputCount,
int outputCount,
string legacyAssembly,
string functionName,
JObject originalElement)
{
GUID = new Guid(id);

InputCount = inputCount;
OutputCount = outputCount;

string legacyName = "Unresolved";
Copy link
Contributor

@QilongTang QilongTang Oct 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@mjkkirschner @aparajit-pratap I cant remember, do we localize node names?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just found out that we do not localize node names, so we should be fine here

LegacyNodeName = legacyName;
LegacyFullName = legacyName;
Name = legacyName;
FunctionName = functionName;

OriginalNodeContent = originalElement;

LegacyAssembly = legacyAssembly;
NodeNature = DummyNode.Nature.Unresolved;

Description = GetDescription();
ShouldDisplayPreviewCore = false;

if (originalElement != null)
{
var legacyFullName = originalElement["FunctionSignature"];
if (legacyFullName != null)
LegacyFullName = legacyFullName.ToString();
}

UpdatePorts();
}

private void LoadNode(XmlNode nodeElement)
{
XmlElement originalElement = OriginalXmlNodeContent;
Expand Down Expand Up @@ -320,29 +367,29 @@ internal string GetDescription()
{
if (NodeNature == Nature.Deprecated)
{
if (string.IsNullOrEmpty(LegacyAssembly))
if (string.IsNullOrEmpty(FunctionName))
{
const string format = "Node of type '{0}' is now deprecated";
return string.Format(format, LegacyNodeName);
const string format = "Node from assembly '{0}' is now deprecated.";
return string.Format(format, LegacyAssembly);
}
else
{
const string format = "Node of type '{0}' ({1}) is now deprecated";
return string.Format(format, LegacyNodeName, LegacyAssembly);
const string format = "Node '{0}' is now deprecated";
return string.Format(format, FunctionName);
}
}

if (NodeNature == Nature.Unresolved)
{
if (string.IsNullOrEmpty(LegacyAssembly))
if (string.IsNullOrEmpty(FunctionName))
{
const string format = "Node of type '{0}' cannot be resolved";
return string.Format(format, LegacyNodeName);
const string format = "Node from assembly '{0}' cannot be resolved.";
return string.Format(format, LegacyAssembly);
}
else
{
const string format = "Node of type '{0}' ({1}) cannot be resolved";
return string.Format(format, LegacyNodeName, LegacyAssembly);
Copy link
Member

Choose a reason for hiding this comment

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

is LegacyNodeName used anywhere anymore? If not maybe you want to obsolete it?

const string format = "Node '{0}' cannot be resolved.";
return string.Format(format, FunctionName);
}
}

Expand Down Expand Up @@ -370,6 +417,11 @@ internal string GetDescription()
/// </summary>
public string LegacyAssembly { get; private set; }

/// <summary>
/// Returns the node's function name
/// </summary>
public string FunctionName { get; private set; }
Copy link
Member

Choose a reason for hiding this comment

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

any reason not to overload the LegacyNodeName - maybe note that this is property is only valid for ZeroTouch dummy nodes?


/// <summary>
/// Returns the original node DSFunction description or UI node type
/// </summary>
Expand Down
29 changes: 23 additions & 6 deletions src/DynamoCore/Graph/Workspaces/SerializationConverters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,28 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
{
NodeModel node = null;

String typeName = String.Empty;
String functionName = String.Empty;
String assemblyName = String.Empty;

var obj = JObject.Load(reader);
Type type = null;

try
{
type = Type.GetType(obj["$type"].Value<string>());
typeName = obj["$type"].Value<string>().Split(',').FirstOrDefault();

if (typeName.Contains("ZeroTouch"))
Copy link
Member

Choose a reason for hiding this comment

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

can we be specific about this- it's possible anyone could make a nodeModel and name it ZeroTouch lets use our entire namespace and type name - including the assembly name would be fine as well for precision.

{
// If it is a zero touch node, then get the whole function name including the namespace.
functionName = obj["FunctionSignature"].Value<string>().Split('@').FirstOrDefault().Trim();
Copy link
Member

Choose a reason for hiding this comment

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

do FunctionSignatures always have the @ token? What if it's not there, what happens?

}
// we get the assembly name from the type string for the node model nodes.
else
{
assemblyName = obj["$type"].Value<string>().Split(',').Skip(1).FirstOrDefault().Trim();
}
}
catch(Exception e)
{
Expand All @@ -115,9 +132,8 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
{
List<Assembly> resultList;

var typeName = obj["$type"].Value<string>().Split(',').FirstOrDefault();
// This assemblyName does not usually contain version information...
var assemblyName = obj["$type"].Value<string>().Split(',').Skip(1).FirstOrDefault().Trim();
assemblyName = obj["$type"].Value<string>().Split(',').Skip(1).FirstOrDefault().Trim();
if (assemblyName != null)
{
if(this.loadedAssemblies.TryGetValue(assemblyName, out resultList))
Expand Down Expand Up @@ -159,7 +175,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
// If type is still null at this point return a dummy node
if (type == null)
{
node = CreateDummyNode(obj, assemblyLocation, inPorts, outPorts);
node = CreateDummyNode(obj, assemblyName, functionName, inPorts, outPorts);
}
// Attempt to create a valid node using the type
else if (type == typeof(Function))
Expand Down Expand Up @@ -223,7 +239,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
// Use the functionDescriptor to try and restore the proper node if possible
if (functionDescriptor == null)
{
node = CreateDummyNode(obj, assemblyLocation, inPorts, outPorts);
node = CreateDummyNode(obj, assemblyName, functionName, inPorts, outPorts);
}
else
{
Expand Down Expand Up @@ -286,7 +302,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
return node;
}

private DummyNode CreateDummyNode(JObject obj, string assemblyLocation, PortModel[] inPorts, PortModel[] outPorts)
private DummyNode CreateDummyNode(JObject obj, string legacyAssembly, string functionName, PortModel[] inPorts, PortModel[] outPorts)
{
var inputcount = inPorts.Count();
var outputcount = outPorts.Count();
Expand All @@ -295,7 +311,8 @@ private DummyNode CreateDummyNode(JObject obj, string assemblyLocation, PortMode
obj["Id"].ToString(),
inputcount,
outputcount,
assemblyLocation,
legacyAssembly,
functionName,
obj);
}

Expand Down