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

fix dyn and image loading from package docs folders. (and some other things) #14554

Merged
merged 11 commits into from
Nov 6, 2023
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Diagnostics;
using System.IO;
using System.Reflection;
using System.Web;
using System.Windows;
using System.Windows.Controls;
Expand Down Expand Up @@ -132,16 +133,24 @@ async void InitializeAsync()
VirtualFolderPath = string.Empty;
try
{
if (viewModel.Link != null && !string.IsNullOrEmpty(viewModel.CurrentPackageName))
//if this node is from a package then we set the virtual host path to the packages docs directory.
if (viewModel.Link != null && !string.IsNullOrEmpty(viewModel.CurrentPackageName) && viewModel.IsOwnedByPackage)
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
{
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
var absolutePath = Path.GetDirectoryName(HttpUtility.UrlDecode(viewModel.Link.AbsolutePath));
//We move two levels up so it will be located in same level than the the NodeHelpSharedDocs directory
var imagesLocation = new DirectoryInfo(absolutePath).Parent.Parent.FullName;
//Adds the NodeHelpSharedDocs directory to the path
VirtualFolderPath = Path.Combine(imagesLocation, SharedDocsDirectoryName);
VirtualFolderPath = Path.GetDirectoryName(HttpUtility.UrlDecode(viewModel.Link.AbsolutePath));
}
//if the node is not from a package, then set the virtual host path to the shared docs folder.
else if (viewModel.Link != null && !viewModel.IsOwnedByPackage)
{
VirtualFolderPath = Path.Combine(new FileInfo(Assembly.GetExecutingAssembly().Location).DirectoryName, SharedDocsDirectoryName);
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
}
//unclear what would cause this.
else
{
VirtualFolderPath = FallbackDirectoryName;
}
//TODO - the above will not handle the case that a package's images/dyns are located in the shared folder
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
//we may have to do some inspection of the package docs folder and decide to fallback in some cases, or mark the package
//in some way.
}
catch (Exception ex)
{
Expand Down
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,19 @@ private void OnInsertFile(object sender, InsertDocumentationLinkEventArgs e)

if (!DynamoSelection.Instance.Selection.Any()) return;

GroupInsertedGraph(existingGroups, e.Name);
DoEvents();

// We have selected all the nodes and notes from the inserted graph
// Now is the time to auto layout the inserted nodes
this.DynamoViewModel.GraphAutoLayoutCommand.Execute(null);
this.DynamoViewModel.FitViewCommand.Execute(false);
Dispatcher.CurrentDispatcher.BeginInvoke(() =>
{
GroupInsertedGraph(existingGroups, e.Name);
mjkkirschner marked this conversation as resolved.
Show resolved Hide resolved
});
//we want to wait for the new group to be inserted and actually rendered, so we add the layout command
//as a background priority task on the ui dispatcher.
Dispatcher.CurrentDispatcher.BeginInvoke(() =>
{
// We have selected all the nodes and notes from the inserted graph
// Now is the time to auto layout the inserted nodes
this.DynamoViewModel.GraphAutoLayoutCommand.Execute(null);
this.DynamoViewModel.FitViewCommand.Execute(false);
},DispatcherPriority.Background);
}


Expand Down Expand Up @@ -522,33 +528,5 @@ public override void Closed()
this.documentationBrowserMenuItem.IsChecked = false;
}
}

#region helper methods

/// <summary>
/// Force the Dispatcher to empty it's queue
/// </summary>
[SecurityPermission(SecurityAction.Demand, Flags = SecurityPermissionFlag.UnmanagedCode)]
public static void DoEvents()
{
var frame = new DispatcherFrame();
Dispatcher.CurrentDispatcher.Invoke(DispatcherPriority.Background,
new DispatcherOperationCallback(ExitFrame), frame);
Dispatcher.PushFrame(frame);
}

/// <summary>
/// Helper method for DispatcherUtil
/// </summary>
/// <param name="frame"></param>
/// <returns></returns>
private static object ExitFrame(object frame)
{
((DispatcherFrame)frame).Continue = false;
return null;
}

#endregion

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Reflection;
using System.Web;
using System.Windows;
using System.Windows.Documents;
using Dynamo.Core;
using Dynamo.DocumentationBrowser.Properties;
using Dynamo.Logging;
Expand Down Expand Up @@ -84,25 +85,24 @@ private set
}

private Uri link;
private string graphPath;
private string content;
private string currentPackageName;


/// <summary>
/// Package Name
/// Package Name of the current node for docs display, if this node is from a package.
/// </summary>
internal string CurrentPackageName
{
get
{
return currentPackageName;
}
set
{
currentPackageName = value;
}
}
internal string CurrentPackageName { get; set; }
/// <summary>
/// True if the current node for docs display is owned by a package.
/// </summary>
internal bool IsOwnedByPackage { get; set; }
/// <summary>
/// Name of the current node's sample dyn for docs display, this is the currently always the node name.
/// </summary>
internal string CurrentGraphName { get; set; }

//path to the current node's sample dyn, usually extracted from the .md file.
internal string GraphPath { get; set; }

private MarkdownHandler MarkdownHandlerInstance => markdownHandler ?? (markdownHandler = new MarkdownHandler());
public bool HasContent => !string.IsNullOrWhiteSpace(this.content);
Expand Down Expand Up @@ -202,33 +202,40 @@ private void HandleLocalResource(OpenDocumentationLinkEventArgs e)
try
{
string targetContent;
string graph;
string graphPath;
string graphName;
bool ownedByPackage = false;
string packageName = string.Empty;
Uri link;

switch (e)
{
case OpenNodeAnnotationEventArgs openNodeAnnotationEventArgs:
packageName = openNodeAnnotationEventArgs.PackageName;
ownedByPackage = !string.IsNullOrEmpty(openNodeAnnotationEventArgs.PackageName);

var mdLink = packageManagerDoc.GetAnnotationDoc(
openNodeAnnotationEventArgs.MinimumQualifiedName,
openNodeAnnotationEventArgs.PackageName);

link = string.IsNullOrEmpty(mdLink) ? new Uri(String.Empty, UriKind.Relative) : new Uri(mdLink);
graph = GetGraphLinkFromMDLocation(link);
graphPath = GetGraphLinkFromMDLocation(link, ownedByPackage);
targetContent = CreateNodeAnnotationContent(openNodeAnnotationEventArgs);
graphName = openNodeAnnotationEventArgs.MinimumQualifiedName;

break;

case OpenDocumentationLinkEventArgs openDocumentationLink:
link = openDocumentationLink.Link;
graph = GetGraphLinkFromMDLocation(link);
graphPath = GetGraphLinkFromMDLocation(link, false);
targetContent = ResourceUtilities.LoadContentFromResources(openDocumentationLink.Link.ToString(), GetType().Assembly);
graphName = null;
break;

default:
// Navigate to unsupported
targetContent = null;
graph = null;
graphPath = null;
link = null;
graphName = null;
break;
Expand All @@ -241,8 +248,10 @@ private void HandleLocalResource(OpenDocumentationLinkEventArgs e)
else
{
this.content = targetContent;
this.graphPath = graph;
this.currentPackageName = graphName;
this.GraphPath = graphPath;
IsOwnedByPackage = ownedByPackage;
CurrentPackageName = packageName;
CurrentGraphName = graphName;
this.Link = link;
}
}
Expand All @@ -264,13 +273,13 @@ private void HandleLocalResource(OpenDocumentationLinkEventArgs e)
this.shouldLoadDefaultContent = false;
}

private string GetGraphLinkFromMDLocation(Uri link)
private string GetGraphLinkFromMDLocation(Uri link,bool isOwnedByPackage)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for curiosity you already defined the property IsOwnedByPackage then why passing it as a parameter to a method?

Copy link
Member Author

@mjkkirschner mjkkirschner Nov 3, 2023

Choose a reason for hiding this comment

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

Good eye, and this is very important because unfortunately the singleton nature of the Docs browser model makes it very easy to access data which has not yet been set correctly.

For example before I made this change it was possible for me to reference isOwnedByPackage before it had even been calculated for the node docs request we were parsing. By using the param as a dependency it's clear to callers that the method will be using it and it should be up to date.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth making it a class member in that case?

{
if (link == null || link.Equals(new Uri(String.Empty, UriKind.Relative))) return string.Empty;
try
{
string graphPath = DynamoGraphFromMDFilePath(link.AbsolutePath);
return File.Exists(graphPath) ? graphPath : null;
string gp = DynamoGraphFromMDFilePath(link.AbsolutePath, isOwnedByPackage);
return File.Exists(gp) ? gp : null;
}
catch (Exception)
{
Expand Down Expand Up @@ -315,7 +324,7 @@ private void OnCurrentMdFileChanged(object sender, FileSystemEventArgs e)
var nodeAnnotationArgs = openDocumentationLinkEventArgs as OpenNodeAnnotationEventArgs;
this.content = CreateNodeAnnotationContent(nodeAnnotationArgs);
this.Link = new Uri(e.FullPath);
this.graphPath = GetGraphLinkFromMDLocation(this.Link);
this.GraphPath = GetGraphLinkFromMDLocation(this.Link,nodeAnnotationArgs.PackageName != string.Empty);
}

private string CreateNodeAnnotationContent(OpenNodeAnnotationEventArgs e)
Expand Down Expand Up @@ -427,14 +436,15 @@ internal void InsertGraph()

if (raiseInsertGraph != null)
{
if (graphPath != null)
if (GraphPath != null)
{
var graphName = this.currentPackageName ?? Path.GetFileNameWithoutExtension(graphPath);
raiseInsertGraph(this, new InsertDocumentationLinkEventArgs(graphPath, graphName));
var graphName = CurrentPackageName ?? Path.GetFileNameWithoutExtension(GraphPath);
raiseInsertGraph(this, new InsertDocumentationLinkEventArgs(GraphPath, graphName));
}
else
{
raiseInsertGraph(this, new InsertDocumentationLinkEventArgs(Resources.FileNotFoundFailureMessage, DynamoGraphFromMDFilePath(this.Link.AbsolutePath)));
raiseInsertGraph(this, new InsertDocumentationLinkEventArgs(Resources.FileNotFoundFailureMessage,
DynamoGraphFromMDFilePath(this.Link.AbsolutePath,IsOwnedByPackage)));
return;
}
}
Expand All @@ -443,12 +453,20 @@ internal void InsertGraph()
internal delegate void InsertDocumentationLinkEventHandler(object sender, InsertDocumentationLinkEventArgs e);
internal event InsertDocumentationLinkEventHandler HandleInsertFile;

private string DynamoGraphFromMDFilePath(string path)
private string DynamoGraphFromMDFilePath(string path, bool IsOwnedByPackage)
{
path = HttpUtility.UrlDecode(path);
var rootLevelDir = Path.GetDirectoryName(path);
var imagesLocation = Path.Combine(new DirectoryInfo(rootLevelDir).Parent.Parent.FullName, DocumentationBrowserView.SharedDocsDirectoryName);
return Path.Combine(imagesLocation, Path.GetFileNameWithoutExtension(path)) + ".dyn";
if (!IsOwnedByPackage)
{

var sharedDocsLocation = Path.Combine(new FileInfo(Assembly.GetExecutingAssembly().Location).DirectoryName,
DocumentationBrowserView.SharedDocsDirectoryName);
return Path.Combine(sharedDocsLocation, Path.GetFileNameWithoutExtension(path)) + ".dyn";
}
else
{
return Path.Combine(Path.GetDirectoryName(path), Path.GetFileNameWithoutExtension(path)) + ".dyn";
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ private MLNodeAutoCompletionResponse GetMLNodeAutocompleteResults(string request
{
try
{
var uri = DynamoUtilities.PathHelper.getServiceBackendAddress(this, nodeAutocompleteMLEndpoint);
var uri = DynamoUtilities.PathHelper.GetServiceBackendAddress(this, nodeAutocompleteMLEndpoint);
var client = new RestClient(uri);
var request = new RestRequest(string.Empty,Method.Post);

Expand Down
2 changes: 1 addition & 1 deletion src/DynamoPackages/PackageLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
using System.IO;
using System.Linq;
using System.Reflection;
using Dynamo.Configuration;
using System.Runtime.Loader;
using Dynamo.Core;
using Dynamo.Exceptions;
using Dynamo.Extensions;
Expand Down
2 changes: 1 addition & 1 deletion src/DynamoPackages/PackageManagerExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public void Dispose()
/// </summary>
public void Startup(StartupParams startupParams)
{
string url = DynamoUtilities.PathHelper.getServiceBackendAddress(this, "packageManagerAddress");
string url = DynamoUtilities.PathHelper.GetServiceBackendAddress(this, "packageManagerAddress");

OnMessageLogged(LogMessage.Info("Dynamo will use the package manager server at : " + url));

Expand Down
2 changes: 1 addition & 1 deletion src/DynamoUtilities/PathHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ internal static bool IsSubDirectoryOfDirectory(string subdirectory, string direc
/// <param name="serviceKey">Service or feature for which the address is being requested.
/// It should match the key specified in the config file.</param>
/// <returns>Path that will be used to fetch resources</returns>
public static string getServiceBackendAddress(object o, string serviceKey)
public static string GetServiceBackendAddress(object o, string serviceKey)
{
string url = null;
if (o != null)
Expand Down
4 changes: 2 additions & 2 deletions src/Notifications/NotificationCenterController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ private void AddNotifications(List<NotificationItemModel> notifications)

private void RequestNotifications()
{
var uri = DynamoUtilities.PathHelper.getServiceBackendAddress(this, "notificationAddress");
var uri = DynamoUtilities.PathHelper.GetServiceBackendAddress(this, "notificationAddress");
HttpWebRequest request = (HttpWebRequest)WebRequest.Create(uri);
request.AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate;

Expand Down Expand Up @@ -310,7 +310,7 @@ public void RefreshNotifications(string url="") {
InvokeJS(@"window.RequestNotifications('" + url + "');");
}
else {
InvokeJS(@"window.RequestNotifications('" + DynamoUtilities.PathHelper.getServiceBackendAddress(this, "notificationAddress") + "');");
InvokeJS(@"window.RequestNotifications('" + DynamoUtilities.PathHelper.GetServiceBackendAddress(this, "notificationAddress") + "');");
}
}
}
Expand Down
19 changes: 1 addition & 18 deletions test/DynamoCoreWpfTests/DynamoViewTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,26 +40,11 @@ protected override void GetLibrariesToPreload(List<string> libraries)
libraries.Add("FFITarget.dll");
}

public override void Open(string path)
{
base.Open(path);

DispatcherUtil.DoEvents();
}

public override void Run()
{
base.Run();

DispatcherUtil.DoEvents();
}

[Test]
public void FooterNotificationControlTest()
{
// Arrange
Open(@"UI\ZoomNodeColorStates.dyn");

var workspace = ViewModel.Model.CurrentWorkspace as HomeWorkspaceModel;
Debug.Assert(workspace != null, nameof(workspace) + " != null");
workspace.Run();
Expand Down Expand Up @@ -116,7 +101,6 @@ public void OpeningWorkspaceWithTclsrustWarning()
// Open workspace with test mode as false, to verify trust warning.
DynamoModel.IsTestMode = false;
Open(@"core\CustomNodes\TestAdd.dyn");

Assert.IsTrue(ViewModel.FileTrustViewModel.ShowWarningPopup);

// Close workspace
Expand All @@ -138,7 +122,7 @@ public void ElementBinding_SaveAs()
var filePath = Path.Combine(GetTestDirectory(ExecutingDirectory), pathInTestsDir);

// Always start with a fresh workspace with no binding data for this test.
File.Copy(prebindingPath, filePath);
File.Copy(prebindingPath, filePath,true);
OpenAndRun(pathInTestsDir);

// Assert that the node doesn't have trace data the first time it's run.
Expand Down Expand Up @@ -176,7 +160,6 @@ public void TestToastNotificationClosingBehavior()
{
var preferencesWindow = new PreferencesView(View);
preferencesWindow.Show();
DispatcherUtil.DoEvents();
string selectedLanguage = (string)((ComboBox)preferencesWindow.FindName("LanguageCmb")).SelectedItem;
var english = Configurations.SupportedLocaleDic.FirstOrDefault(x => x.Value == "en-US").Key;
var spanish = Configurations.SupportedLocaleDic.FirstOrDefault(x => x.Value == "es-ES").Key;
Expand Down
Loading
Loading