Skip to content

Modernize DynamoModel.cs to C# 12 idioms #16880

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

Draft
Copilot wants to merge 4 commits into
base: master
Choose a base branch
from
+53 −55
Draft

Modernize DynamoModel.cs to C# 12 idioms #16880

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
846d5e2
Initial plan
Copilot Feb 7, 2026
378d341
Modernize collection initializations in DynamoModel.cs
Copilot Feb 7, 2026
0830006
Apply modern C# patterns (pattern matching, string interpolation) to …
Copilot Feb 7, 2026
17fcbe4
Replace string concatenation with interpolation in DynamoModel.cs
Copilot Feb 7, 2026
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
108 changes: 53 additions & 55 deletions src/DynamoCore/Models/DynamoModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@
private readonly PathManager pathManager;
private WorkspaceModel currentWorkspace;
private Timer backupFilesTimer;
private Dictionary<Guid, string> backupFilesDict = new Dictionary<Guid, string>();
private Dictionary<Guid, string> backupFilesDict = new();
internal readonly Stopwatch stopwatch = Stopwatch.StartNew();

/// <summary>
Expand Down Expand Up @@ -385,7 +385,7 @@
/// <summary>
/// The private collection of visible workspaces in Dynamo
/// </summary>
private readonly List<WorkspaceModel> _workspaces = new List<WorkspaceModel>();
private readonly List<WorkspaceModel> _workspaces = new();

/// <summary>
/// Returns collection of visible workspaces in Dynamo
Expand Down Expand Up @@ -686,7 +686,7 @@
pathManager = CreatePathManager(config);

// Ensure we have all directories in place.
var exceptions = new List<Exception>();
List<Exception> exceptions = [];
pathManager.EnsureDirectoryExistence(exceptions);

Context = config.Context;
Expand Down Expand Up @@ -830,7 +830,7 @@
// is no additional location specified. Otherwise, update pathManager.PackageDirectories to include
// PackageFolders
if (PreferenceSettings.CustomPackageFolders.Count == 0)
PreferenceSettings.CustomPackageFolders = new List<string> { BuiltInPackagesToken, pathManager.UserDataDirectory };
PreferenceSettings.CustomPackageFolders = [BuiltInPackagesToken, pathManager.UserDataDirectory];

if (!PreferenceSettings.CustomPackageFolders.Contains(BuiltInPackagesToken))
{
Expand Down Expand Up @@ -865,7 +865,7 @@
if (!string.IsNullOrEmpty(templatePath) && File.Exists(templatePath))
{
PreferenceSettings.PythonTemplateFilePath = templatePath;
Logger.Log(Resources.PythonTemplateDefinedByHost + " : " + PreferenceSettings.PythonTemplateFilePath);
Logger.Log($"{Resources.PythonTemplateDefinedByHost} : {PreferenceSettings.PythonTemplateFilePath}");
}

// Otherwise fallback to the default
Expand All @@ -885,7 +885,7 @@
else
{
// A custom python template path already exists in the DynamoSettings.xml
Logger.Log(Resources.PythonTemplateUserFile + " : " + PreferenceSettings.PythonTemplateFilePath);
Logger.Log($"{Resources.PythonTemplateUserFile} : {PreferenceSettings.PythonTemplateFilePath}");
}
pathManager.Preferences = PreferenceSettings;
PreferenceSettings.RequestUserDataFolder += pathManager.GetUserDataFolder;
Expand Down Expand Up @@ -924,7 +924,7 @@
if (this.dynamoReady)
{
DynamoReadyExtensionHandler(new ReadyParams(this),
new List<IExtension>() { extension });
[extension]);
};
};

Expand Down Expand Up @@ -956,8 +956,7 @@
AddHomeWorkspace();

AuthenticationManager = new AuthenticationManager(config.AuthProvider);
Logger.Log(string.Format("Dynamo -- Build {0}",
Assembly.GetExecutingAssembly().GetName().Version));
Logger.Log($"Dynamo -- Build {Assembly.GetExecutingAssembly().GetName().Version}");

DynamoModel.OnRequestUpdateLoadBarStatus(new SplashScreenLoadEventArgs(Resources.SplashScreenLoadNodeLibrary, 50));
InitializeNodeLibrary();
Expand Down Expand Up @@ -1032,7 +1031,7 @@
LuceneUtility.DisposeWriter();
}

GraphChecksumDictionary = new Dictionary<string, List<string>>();
GraphChecksumDictionary = new();
// This event should only be raised at the end of this method.
DynamoReady(new ReadyParams(this));

Expand All @@ -1041,7 +1040,7 @@
{
if (trustedLoc.StartsWith(programDataPath))
{
Logger.Log(("An unsafe path has been detected in Trusted Locations: " + trustedLoc));
Logger.Log($"An unsafe path has been detected in Trusted Locations: {trustedLoc}");

Check failure

Code scanning / CodeQL

Clear text storage of sensitive information High

This stores sensitive data returned by
call to method SelectSingleNode : XmlNode
Loading
as clear text.
This stores sensitive data returned by
call to method DeserializeTrustedLocations : List
Loading
as clear text.
This stores sensitive data returned by
access to local variable trustedLoc : String
Loading
as clear text.
This stores sensitive data returned by
access to local variable trustedLoc : String
Loading
as clear text.

Copilot Autofix

AI 2 months ago

In general, to fix clear-text storage issues in logs, you either (a) avoid logging the sensitive value at all, or (b) log only a redacted/derived form (such as a hash, prefix, or generic tag) that is sufficient for diagnostics without exposing the original value. For filesystem paths coming from user-controlled configuration, you typically only need to indicate that a problematic path exists and maybe show a minimally identifying portion, not the entire absolute path.

For this specific case, the best fix that preserves existing functionality is to avoid logging the full trusted location path and instead log only a safe, truncated representation. We can transform trustedLoc before passing it to Logger.Log, e.g. by logging only the last directory segment or a redacted form such as "...\folder" or "[redacted]". This keeps the information that an unsafe path exists and roughly which entry it is, while no longer storing the full user path in clear text. We only need to change the logging line in DynamoModel.cs; the rest of the flow (loading and storing trusted locations in memory and XML) is outside the identified sink and should remain unchanged.

Concretely, in src/DynamoCore/Models/DynamoModel.cs, within the constructor (or initialization method) near lines 1038–1045, replace

Logger.Log($"An unsafe path has been detected in Trusted Locations: {trustedLoc}");

with a version that uses a safe representation of trustedLoc. A simple approach that requires no new imports is to derive a short identifier, such as the last path component using Path.GetFileName(trustedLoc.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)), and log that instead. However, we must not add new imports other than standard ones, and System.IO is already imported at the top of the file, so we can safely use Path. We will compute a safeLocationDescription inline and use that in the log message. This change affects only DynamoModel.cs; no changes are required in PreferenceSettings.cs for this particular sink.

Suggested changeset 1
src/DynamoCore/Models/DynamoModel.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/DynamoCore/Models/DynamoModel.cs b/src/DynamoCore/Models/DynamoModel.cs
--- a/src/DynamoCore/Models/DynamoModel.cs
+++ b/src/DynamoCore/Models/DynamoModel.cs
@@ -1040,7 +1040,8 @@
             {
                 if (trustedLoc.StartsWith(programDataPath))
                 {
-                    Logger.Log($"An unsafe path has been detected in Trusted Locations: {trustedLoc}");
+                    var safeLocationDescription = Path.GetFileName(trustedLoc.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar));
+                    Logger.Log($"An unsafe path has been detected in Trusted Locations. Entry: {safeLocationDescription}");
                 }
             }        
         }
EOF
@@ -1040,7 +1040,8 @@
{
if (trustedLoc.StartsWith(programDataPath))
{
Logger.Log($"An unsafe path has been detected in Trusted Locations: {trustedLoc}");
var safeLocationDescription = Path.GetFileName(trustedLoc.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar));
Logger.Log($"An unsafe path has been detected in Trusted Locations. Entry: {safeLocationDescription}");
}
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
}
}
Expand Down Expand Up @@ -1193,7 +1192,7 @@
if (!string.IsNullOrEmpty(pathManager.PythonTemplateFilePath) && File.Exists(pathManager.PythonTemplateFilePath))
{
PreferenceSettings.PythonTemplateFilePath = pathManager.PythonTemplateFilePath;
Logger.Log(Resources.PythonTemplateAppData + " : " + PreferenceSettings.PythonTemplateFilePath);
Logger.Log($"{Resources.PythonTemplateAppData} : {PreferenceSettings.PythonTemplateFilePath}");
}

// Otherwise the OOTB hard-coded template is applied
Expand Down Expand Up @@ -1248,7 +1247,7 @@

private IEnumerable<IExtension> LoadExtensions()
{
var extensions = new List<IExtension>();
List<IExtension> extensions = [];
foreach (var dir in pathManager.ExtensionsDirectories)
{
extensions.AddRange(ExtensionManager.ExtensionLoader.LoadDirectory(dir));
Expand All @@ -1260,8 +1259,7 @@
{
ExtensionManager.Remove(ext);

var logSource = ext as ILogSource;
if (logSource != null)
if (ext is ILogSource logSource)
logSource.MessageLogged -= LogMessage;
}

Expand Down Expand Up @@ -1293,7 +1291,7 @@
}
catch (Exception ex)
{
logger.Log(ex.Message + " : " + ex.StackTrace);
logger.Log($"{ex.Message} : {ex.StackTrace}");
return;
}
}
Expand All @@ -1311,7 +1309,7 @@
}
catch (Exception ex)
{
logger.Log(ex.Message + " : " + ex.StackTrace);
logger.Log($"{ex.Message} : {ex.StackTrace}");
return;
}

Expand Down Expand Up @@ -1426,16 +1424,15 @@
///
private void OnAsyncTaskStateChanged(DynamoScheduler sender, TaskStateChangedEventArgs e)
{
var updateTask = e.Task as UpdateGraphAsyncTask;
switch (e.CurrentState)
{
case TaskStateChangedEventArgs.State.ExecutionStarting:
if (updateTask != null)
if (e.Task is UpdateGraphAsyncTask updateTask)
ExecutionEvents.OnGraphPreExecution(new ExecutionSession(updateTask, this, geometryFactoryPath));
break;

case TaskStateChangedEventArgs.State.ExecutionCompleted:
if (updateTask != null)
if (e.Task is UpdateGraphAsyncTask updateTask2)
{
// Record execution time for update graph task.
long start = e.Task.ExecutionStartTime.TickCount;
Expand All @@ -1445,10 +1442,10 @@
//don't attempt to send these events unless GA is active or ADP will actually record these events.
if (Logging.Analytics.ReportingAnalytics)
{
if (updateTask.ModifiedNodes != null && updateTask.ModifiedNodes.Any())
if (updateTask2.ModifiedNodes != null && updateTask2.ModifiedNodes.Any())
{
// Send analytics for each of modified nodes so they are counted individually
foreach (var node in updateTask.ModifiedNodes)
foreach (var node in updateTask2.ModifiedNodes)
{
// Tracking node execution as generic event
// it is distinguished with the legacy aggregated performance event
Expand All @@ -1462,7 +1459,7 @@

Debug.WriteLine(String.Format(Resources.EvaluationCompleted, executionTimeSpan));

ExecutionEvents.OnGraphPostExecution(new ExecutionSession(updateTask, this, geometryFactoryPath));
ExecutionEvents.OnGraphPostExecution(new ExecutionSession(updateTask2, this, geometryFactoryPath));
}
break;
}
Expand Down Expand Up @@ -1792,8 +1789,9 @@
return;
}

var nodes = new List<TypeLoadData>();
Loader.LoadNodesFromAssembly(assem, Context, nodes, new List<TypeLoadData>());
List<TypeLoadData> nodes = [];
List<TypeLoadData> errors = [];
Loader.LoadNodesFromAssembly(assem, Context, nodes, errors);

LoadNodeModels(nodes, true);
}
Expand Down Expand Up @@ -2443,8 +2441,7 @@
// The following logic to start periodic evaluation will need to be moved
// inside of the HomeWorkspaceModel's constructor. It cannot be there today
// as it causes an immediate crash due to the above ResetEngine call.
var hws = ws as HomeWorkspaceModel;
if (hws != null)
if (ws is HomeWorkspaceModel hws)
{
// TODO: #4258
// Remove this ResetEngine call when multiple home workspaces is supported.
Expand Down Expand Up @@ -2491,8 +2488,8 @@
CustomNodeManager,
this.LinterManager);

workspace.FileName = string.IsNullOrEmpty(filePath)? string.Empty : filePath;
workspace.FromJsonGraphId = string.IsNullOrEmpty(filePath) ? WorkspaceModel.ComputeGraphIdFromJson(fileContents) : string.Empty;
workspace.FileName = string.IsNullOrEmpty(filePath) ? "" : filePath;
workspace.FromJsonGraphId = string.IsNullOrEmpty(filePath) ? WorkspaceModel.ComputeGraphIdFromJson(fileContents) : "";
workspace.ScaleFactor = dynamoPreferences.ScaleFactor;
workspace.IsTemplate = isTemplate;
if (!IsTestMode && !IsHeadless)
Expand Down Expand Up @@ -2689,7 +2686,7 @@
// when the last auto-save operation happens. Now the IDs will be used to know
// whether some workspaces have already been backed up. If so, those workspaces won't be
// backed up again.
var tempDict = new Dictionary<Guid, string>(backupFilesDict);
Dictionary<Guid, string> tempDict = new(backupFilesDict);
backupFilesDict.Clear();
PreferenceSettings.BackupFiles.Clear();
foreach (var workspace in Workspaces)
Expand All @@ -2710,7 +2707,7 @@
var savePath = pathManager.GetBackupFilePath(workspace);
OnRequestWorkspaceBackUpSave(savePath, true);
backupFilesDict[workspace.Guid] = savePath;
Logger.Log(Resources.BackupSavedMsg + ": " + savePath);
Logger.Log($"{Resources.BackupSavedMsg}: {savePath}");
}
PreferenceSettings.BackupFiles.AddRange(backupFilesDict.Values);
});
Expand Down Expand Up @@ -2801,7 +2798,7 @@

internal void UngroupModel(List<ModelBase> modelsToUngroup)
{
var emptyGroup = new List<ModelBase>();
List<ModelBase> emptyGroup = [];
var annotations = Workspaces.SelectMany(ws => ws.Annotations);
foreach (var model in modelsToUngroup)
{
Expand Down Expand Up @@ -2880,7 +2877,7 @@
var selectedGroup = workspaceAnnotations.FirstOrDefault(x => x.GUID == hostGroupGuid);
if (selectedGroup is null) return;

var modelsToModify = new List<ModelBase>();
List<ModelBase> modelsToModify = [];
modelsToModify.AddRange(modelsToAdd);
modelsToModify.Add(selectedGroup);

Expand Down Expand Up @@ -3108,15 +3105,17 @@
if (!(el is NodeModel))
continue;

var node = el as NodeModel;
var connectors =
node.InPorts.Concat(node.OutPorts).SelectMany(port => port.Connectors)
.Where(
connector =>
connector.End != null && connector.End.Owner.IsSelected
&& !ClipBoard.Contains(connector));
if (el is NodeModel node)
{
var connectors =
node.InPorts.Concat(node.OutPorts).SelectMany(port => port.Connectors)
.Where(
connector =>
connector.End != null && connector.End.Owner.IsSelected
&& !ClipBoard.Contains(connector));

ClipBoard.AddRange(connectors);
ClipBoard.AddRange(connectors);
}
}
}

Expand Down Expand Up @@ -3159,11 +3158,11 @@

//make a lookup table to store the guids of the
//old models and the guids of their pasted versions
var modelLookup = new Dictionary<Guid, ModelBase>();
Dictionary<Guid, ModelBase> modelLookup = [];

//make a list of all newly created models so that their
//creations can be recorded in the undo recorder.
var createdModels = new List<ModelBase>();
List<ModelBase> createdModels = [];

var nodes = ClipBoard.OfType<NodeModel>();
var connectors = ClipBoard.OfType<ConnectorModel>();
Expand All @@ -3177,7 +3176,7 @@
var xmlDoc = new XmlDocument();

// Create the new NodeModel's
var newNodeModels = new List<NodeModel>();
List<NodeModel> newNodeModels = [];
using (CurrentWorkspace.BeginDelayedGraphExecution())
{
foreach (var node in nodes)
Expand Down Expand Up @@ -3212,7 +3211,7 @@
}

// Create the new NoteModel's
var newNoteModels = new List<NoteModel>();
List<NoteModel> newNoteModels = [];
foreach (var note in notes)
{
var noteModel = new NoteModel(note.X, note.Y, note.Text, Guid.NewGuid());
Expand Down Expand Up @@ -3285,7 +3284,7 @@
//Grouping depends on the selected node models.
//so adding the group after nodes / notes are added to workspace.
//select only those nodes that are part of a group.
var newAnnotations = new List<AnnotationModel>();
List<AnnotationModel> newAnnotations = [];
foreach (var annotation in annotations.OrderByDescending(a => a.HasNestedGroups))
{
if (modelLookup.ContainsKey(annotation.GUID))
Expand Down Expand Up @@ -3341,9 +3340,9 @@
private AnnotationModel CreateAnnotationModel(
AnnotationModel model, Dictionary<Guid, ModelBase> modelLookup)
{
var annotationNodeModel = new List<NodeModel>();
var annotationNoteModel = new List<NoteModel>();
var annotationAnnotationModels = new List<AnnotationModel>();
List<NodeModel> annotationNodeModel = [];
List<NoteModel> annotationNoteModel = [];
List<AnnotationModel> annotationAnnotationModels = [];
// some models can be deleted after copying them,
// so they need to be in pasted annotation as well
var modelsToRestore = model.DeletedModelBases.Intersect(ClipBoard);
Expand Down Expand Up @@ -3396,8 +3395,7 @@
/// <param name="parameters">The object to add to the selection.</param>
public void AddToSelection(object parameters)
{
var selectable = parameters as ISelectable;
if (selectable != null)
if (parameters is ISelectable selectable)
{
DynamoSelection.Instance.Selection.AddUnique(selectable);
}
Expand Down Expand Up @@ -3471,7 +3469,7 @@
where !displayString.Contains("GetType")
select string.IsNullOrEmpty(function.Namespace)
? ""
: function.Namespace + "." + function.Signature + "\n"));
: $"{function.Namespace}.{function.Signature}\n"));

var sb = string.Join("\n", descriptions);

Expand Down Expand Up @@ -3764,7 +3762,7 @@

private void InsertConnectors(IEnumerable<ConnectorModel> connectors)
{
List<ConnectorModel> newConnectors = new List<ConnectorModel>();
List<ConnectorModel> newConnectors = [];

foreach (var connectorModel in connectors)
{
Expand All @@ -3787,7 +3785,7 @@

private List<NoteModel> GetInsertedNotes(IEnumerable<ExtraAnnotationViewInfo> viewInfoAnnotations)
{
List<NoteModel> result = new List<NoteModel>();
List<NoteModel> result = [];

foreach (var annotation in viewInfoAnnotations)
{
Expand All @@ -3808,7 +3806,7 @@

internal static void RecordUndoModels(WorkspaceModel workspace, List<ModelBase> undoItems)
{
var userActionDictionary = new Dictionary<ModelBase, UndoRedoRecorder.UserAction>();
Dictionary<ModelBase, UndoRedoRecorder.UserAction> userActionDictionary = [];
//Add models that were newly created
foreach (var undoItem in undoItems)
{
Expand Down
Loading