From bcc4016236587072792811dac37886a243519ea6 Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Tue, 20 Oct 2026 16:21:08 +0200 Subject: [PATCH 1/9] refactor: move the Power BI Desktop constants out of AppEnvironment The process name and the Analysis Services image name move to PowerBIDesktop as constants. A constant is compiled into its callers, so reading it no longer runs the AppEnvironment static constructor; this follows the split of AppEnvironment into small immutable types. --- src/Infrastructure/AppEnvironment.cs | 2 -- .../Configuration/Settings/StartupSettings.cs | 3 ++- src/Infrastructure/Messages/AppInstanceStartupMessage.cs | 3 ++- src/Infrastructure/Services/PowerBI/PBIDesktopService.cs | 2 +- src/Infrastructure/Services/PowerBI/PowerBIDesktop.cs | 8 ++++++++ src/Models/PBIDesktopReport.cs | 3 ++- 6 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 src/Infrastructure/Services/PowerBI/PowerBIDesktop.cs diff --git a/src/Infrastructure/AppEnvironment.cs b/src/Infrastructure/AppEnvironment.cs index e3541c20..38cc9754 100644 --- a/src/Infrastructure/AppEnvironment.cs +++ b/src/Infrastructure/AppEnvironment.cs @@ -33,8 +33,6 @@ internal static class AppEnvironment public static readonly string ApplicationRegistryApplicationTelemetryEnabledValue = "applicationTelemetryEnabled"; public static readonly string ApplicationRegistryApplicationTitleVersionHiddenValue = "applicationTitleVersionHidden"; public static readonly string ApplicationRegistryApplicationInstallFolderValue = "installFolder"; - public static readonly string PBIDesktopProcessName = "PBIDesktop"; - public static readonly string PBIDesktopSSASProcessImageName = "msmdsrv.exe"; public static readonly string[] PBIDesktopMainWindowTitleSuffixes = new string[] { // The PBIDesktop main window title is culture-specific. diff --git a/src/Infrastructure/Configuration/Settings/StartupSettings.cs b/src/Infrastructure/Configuration/Settings/StartupSettings.cs index ace85e29..28dbf6a6 100644 --- a/src/Infrastructure/Configuration/Settings/StartupSettings.cs +++ b/src/Infrastructure/Configuration/Settings/StartupSettings.cs @@ -6,6 +6,7 @@ using System.Text.Json.Serialization; using Sqlbi.Bravo.Infrastructure.Extensions; using Sqlbi.Bravo.Infrastructure.Helpers; +using Sqlbi.Bravo.Infrastructure.Services.PowerBI; namespace Sqlbi.Bravo.Infrastructure.Configuration.Settings; @@ -27,7 +28,7 @@ internal class StartupSettings public string[]? CommandLineErrors { get; set; } [JsonIgnore] - public bool IsPBIDesktopExternalTool => IsExternalTool && AppEnvironment.PBIDesktopProcessName.Equals(ParentProcessName, StringComparison.OrdinalIgnoreCase); + public bool IsPBIDesktopExternalTool => IsExternalTool && PowerBIDesktop.ProcessName.Equals(ParentProcessName, StringComparison.OrdinalIgnoreCase); [JsonIgnore] public int? ParentProcessId { get; set; } diff --git a/src/Infrastructure/Messages/AppInstanceStartupMessage.cs b/src/Infrastructure/Messages/AppInstanceStartupMessage.cs index 738bd475..8d2c1316 100644 --- a/src/Infrastructure/Messages/AppInstanceStartupMessage.cs +++ b/src/Infrastructure/Messages/AppInstanceStartupMessage.cs @@ -4,6 +4,7 @@ using Sqlbi.Bravo.Infrastructure.Configuration.Settings; using Sqlbi.Bravo.Infrastructure.Extensions; using Sqlbi.Bravo.Infrastructure.Helpers; +using Sqlbi.Bravo.Infrastructure.Services.PowerBI; using Sqlbi.Bravo.Models; using SSAS = Microsoft.AnalysisServices; @@ -33,7 +34,7 @@ internal class AppInstanceStartupMessage public string[]? CommandLineErrors { get; set; } [JsonIgnore] - public bool IsExternalTool => AppEnvironment.PBIDesktopProcessName.EqualsI(ParentProcessName); + public bool IsExternalTool => PowerBIDesktop.ProcessName.EqualsI(ParentProcessName); // Reads no user settings: a secondary instance builds its activation payload before the bootstrap. public static AppInstanceStartupMessage CreateFrom(StartupSettings settings) diff --git a/src/Infrastructure/Services/PowerBI/PBIDesktopService.cs b/src/Infrastructure/Services/PowerBI/PBIDesktopService.cs index 58b7bfe3..291f46f2 100644 --- a/src/Infrastructure/Services/PowerBI/PBIDesktopService.cs +++ b/src/Infrastructure/Services/PowerBI/PBIDesktopService.cs @@ -18,7 +18,7 @@ internal class PBIDesktopService : IPBIDesktopService { public IEnumerable GetReports(CancellationToken cancellationToken) { - var processes = ProcessHelper.GetProcessesByName(AppEnvironment.PBIDesktopProcessName); + var processes = ProcessHelper.GetProcessesByName(PowerBIDesktop.ProcessName); try { var reports = new ConcurrentBag(); diff --git a/src/Infrastructure/Services/PowerBI/PowerBIDesktop.cs b/src/Infrastructure/Services/PowerBI/PowerBIDesktop.cs new file mode 100644 index 00000000..bb011c55 --- /dev/null +++ b/src/Infrastructure/Services/PowerBI/PowerBIDesktop.cs @@ -0,0 +1,8 @@ +namespace Sqlbi.Bravo.Infrastructure.Services.PowerBI; + +internal static class PowerBIDesktop +{ + public const string ProcessName = "PBIDesktop"; + + public const string AnalysisServicesImageName = "msmdsrv.exe"; +} diff --git a/src/Models/PBIDesktopReport.cs b/src/Models/PBIDesktopReport.cs index 915a8ab5..65a1361e 100644 --- a/src/Models/PBIDesktopReport.cs +++ b/src/Models/PBIDesktopReport.cs @@ -9,6 +9,7 @@ using Sqlbi.Bravo.Infrastructure.Extensions; using Sqlbi.Bravo.Infrastructure.Helpers; using Sqlbi.Bravo.Infrastructure.Models; +using Sqlbi.Bravo.Infrastructure.Services.PowerBI; using SSAS = Microsoft.AnalysisServices; using TOM = Microsoft.AnalysisServices.Tabular; @@ -107,7 +108,7 @@ void GetConnectionMode(out string? serverName, out string? databaseName, out SSA databaseName = null; compatibilityMode = SSAS.CompatibilityMode.Unknown; - var ssasPIDs = process.GetChildrenPIDs(childProcessImageName: AppEnvironment.PBIDesktopSSASProcessImageName).ToArray(); + var ssasPIDs = process.GetChildrenPIDs(childProcessImageName: PowerBIDesktop.AnalysisServicesImageName).ToArray(); if (ssasPIDs.Length != 1) { connectionMode = PBIDesktopReportConnectionMode.UnsupportedAnalysisServicesProcessNotFound; From 9eb05b2fc33fa14472757e420922099a544e115d Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Tue, 20 Oct 2026 16:24:02 +0200 Subject: [PATCH 2/9] refactor: read the report name from the Power BI Desktop window title PowerBIDesktop.GetReportName removes the application name from a main window title. It replaces the GetPBIDesktopMainWindowTitle extension, which read the window and parsed its title in one step and returned a report name under the name of a title, so the parsing is now testable without a process. The suffixes become a private ImmutableArray instead of a public array field on AppEnvironment, which any caller could change. They are matched with an ordinal comparison: they are literal characters, not text to compare by culture. The test of the localized title formats moves from StringExtensionsTests and now checks the report name; new tests cover each separator. --- src/Infrastructure/AppEnvironment.cs | 18 --- .../Configuration/Settings/StartupSettings.cs | 3 +- .../Extensions/ProcessExtensions.cs | 20 ---- src/Infrastructure/Helpers/ProcessHelper.cs | 5 +- .../Services/PowerBI/PowerBIDesktop.cs | 47 +++++++- src/Models/PBIDesktopReport.cs | 2 +- .../Extensions/StringExtensionsTests.cs | 66 +---------- .../Services/PowerBI/PowerBIDesktopTests.cs | 106 ++++++++++++++++++ 8 files changed, 159 insertions(+), 108 deletions(-) create mode 100644 test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopTests.cs diff --git a/src/Infrastructure/AppEnvironment.cs b/src/Infrastructure/AppEnvironment.cs index 38cc9754..0798643c 100644 --- a/src/Infrastructure/AppEnvironment.cs +++ b/src/Infrastructure/AppEnvironment.cs @@ -33,24 +33,6 @@ internal static class AppEnvironment public static readonly string ApplicationRegistryApplicationTelemetryEnabledValue = "applicationTelemetryEnabled"; public static readonly string ApplicationRegistryApplicationTitleVersionHiddenValue = "applicationTitleVersionHidden"; public static readonly string ApplicationRegistryApplicationInstallFolderValue = "installFolder"; - public static readonly string[] PBIDesktopMainWindowTitleSuffixes = new string[] - { - // The PBIDesktop main window title is culture-specific. - // The suffix is not always present, for example, it is not added when the save/share function in OneDrive/SharePoint is active. - - // Different dash characters are used as a separator - // See https://github.com/sql-bi/Bravo/issues/476 - " \u002D Power BI Desktop", // Dash Punctuation - minus hyphen - " \u2013 Power BI Desktop", // Dash Punctuation - en dash - " \u2014 Power BI Desktop", // Dash Punctuation - em dash - - // The whitespace character may not be present - Swedish/sv - // See https://github.com/sql-bi/Bravo/issues/510 - "\u2013Power BI Desktop", // Dash Punctuation - en dash - - // NBSP char instead of whitespace - Latvian/lv - "\u00A0\u2014 Power BI Desktop", - }; public static readonly Color ThemeColorDark = ColorTranslator.FromHtml("#202020"); public static readonly Color ThemeColorLight = ColorTranslator.FromHtml("#F3F3F3"); public static readonly DaxLineBreakStyle FormatDaxLineBreakDefault = DaxLineBreakStyle.InitialLineBreak; diff --git a/src/Infrastructure/Configuration/Settings/StartupSettings.cs b/src/Infrastructure/Configuration/Settings/StartupSettings.cs index 28dbf6a6..171f9925 100644 --- a/src/Infrastructure/Configuration/Settings/StartupSettings.cs +++ b/src/Infrastructure/Configuration/Settings/StartupSettings.cs @@ -122,7 +122,8 @@ public static void FromCommandLineArguments(this StartupSettings settings) settings.ParentProcessId = parentProcess.Id; settings.ParentProcessName = parentProcess.ProcessName; //settings.ParentProcessMainWindowHandle = parentProcess.MainWindowHandle; - settings.ParentProcessMainWindowTitle = settings.IsPBIDesktopExternalTool ? parentProcess.GetPBIDesktopMainWindowTitle() : parentProcess.GetMainWindowTitle(); + var windowTitle = parentProcess.GetMainWindowTitle(); + settings.ParentProcessMainWindowTitle = settings.IsPBIDesktopExternalTool ? PowerBIDesktop.GetReportName(windowTitle) : windowTitle; } } parentProcess?.Dispose(); diff --git a/src/Infrastructure/Extensions/ProcessExtensions.cs b/src/Infrastructure/Extensions/ProcessExtensions.cs index 2840462d..f8950d80 100644 --- a/src/Infrastructure/Extensions/ProcessExtensions.cs +++ b/src/Infrastructure/Extensions/ProcessExtensions.cs @@ -107,24 +107,4 @@ public static string GetMainWindowTitle(this Process process) return builder.ToString(); } - - public static string? GetPBIDesktopMainWindowTitle(this Process process) - { - var windowTitle = process.GetMainWindowTitle(); - - if (windowTitle.IsNullOrWhiteSpace()) - return null; // PBIDesktop process is starting and/or the SSAS instance is not yet started and/or the model is not yet fully loaded - - foreach (var suffix in AppEnvironment.PBIDesktopMainWindowTitleSuffixes) - { - var index = windowTitle.LastIndexOf(suffix); - if (index >= 0) - { - windowTitle = windowTitle[..index]; - break; - } - } - - return windowTitle; - } } diff --git a/src/Infrastructure/Helpers/ProcessHelper.cs b/src/Infrastructure/Helpers/ProcessHelper.cs index a3a009dc..bfddc856 100644 --- a/src/Infrastructure/Helpers/ProcessHelper.cs +++ b/src/Infrastructure/Helpers/ProcessHelper.cs @@ -13,6 +13,7 @@ using System.Threading; using System.Windows.Forms; using Sqlbi.Bravo.Infrastructure.Extensions; +using Sqlbi.Bravo.Infrastructure.Services.PowerBI; namespace Sqlbi.Bravo.Infrastructure.Helpers; @@ -228,8 +229,8 @@ public static bool OpenShellExecute(string path, bool waitForStarted, [NotNullWh return false; } - // If the window title is null it means that the SSAS instance is not yet started and/or the model is not yet fully loaded - var isAvailable = process.GetPBIDesktopMainWindowTitle() is not null; + // If the report name is null it means that the SSAS instance is not yet started and/or the model is not yet fully loaded + var isAvailable = PowerBIDesktop.GetReportName(process.GetMainWindowTitle()) is not null; if (isAvailable) break; diff --git a/src/Infrastructure/Services/PowerBI/PowerBIDesktop.cs b/src/Infrastructure/Services/PowerBI/PowerBIDesktop.cs index bb011c55..8a6d51fb 100644 --- a/src/Infrastructure/Services/PowerBI/PowerBIDesktop.cs +++ b/src/Infrastructure/Services/PowerBI/PowerBIDesktop.cs @@ -1,8 +1,53 @@ -namespace Sqlbi.Bravo.Infrastructure.Services.PowerBI; +using System; +using System.Collections.Immutable; + +namespace Sqlbi.Bravo.Infrastructure.Services.PowerBI; internal static class PowerBIDesktop { public const string ProcessName = "PBIDesktop"; public const string AnalysisServicesImageName = "msmdsrv.exe"; + + // The main window title is culture-specific. The suffix is not always present, for example, it is not added when + // the save/share function in OneDrive/SharePoint is active. + private static readonly ImmutableArray s_applicationNameSuffixes = + [ + // Different dash characters are used as a separator + // See https://github.com/sql-bi/Bravo/issues/476 + " \u002D Power BI Desktop", // Dash Punctuation - minus hyphen + " \u2013 Power BI Desktop", // Dash Punctuation - en dash + " \u2014 Power BI Desktop", // Dash Punctuation - em dash + + // The whitespace character may not be present - Swedish/sv + // See https://github.com/sql-bi/Bravo/issues/510 + "\u2013Power BI Desktop", // Dash Punctuation - en dash + + // NBSP char instead of whitespace - Latvian/lv + "\u00A0\u2014 Power BI Desktop", + ]; + + /// + /// Returns the report name shown in the main window title of Power BI Desktop. + /// + /// + /// The title without the application name; when the title is empty, which happens while + /// Power BI Desktop starts its Analysis Services instance or loads the model. + /// + public static string? GetReportName(string windowTitle) + { + ArgumentNullException.ThrowIfNull(windowTitle); + + if (string.IsNullOrWhiteSpace(windowTitle)) + return null; + + foreach (var suffix in s_applicationNameSuffixes) + { + var index = windowTitle.LastIndexOf(suffix, StringComparison.Ordinal); + if (index >= 0) + return windowTitle[..index]; + } + + return windowTitle; + } } diff --git a/src/Models/PBIDesktopReport.cs b/src/Models/PBIDesktopReport.cs index 65a1361e..cc3e0b48 100644 --- a/src/Models/PBIDesktopReport.cs +++ b/src/Models/PBIDesktopReport.cs @@ -77,7 +77,7 @@ internal static PBIDesktopReport CreateFrom(Process process, bool connectionMode var report = new PBIDesktopReport { ProcessId = process.Id, - ReportName = process.GetPBIDesktopMainWindowTitle(), + ReportName = PowerBIDesktop.GetReportName(process.GetMainWindowTitle()), ServerName = null, DatabaseName = null, CompatibilityMode = SSAS.CompatibilityMode.Unknown, diff --git a/test/Bravo.Tests/Infrastructure/Extensions/StringExtensionsTests.cs b/test/Bravo.Tests/Infrastructure/Extensions/StringExtensionsTests.cs index e636c999..89073494 100644 --- a/test/Bravo.Tests/Infrastructure/Extensions/StringExtensionsTests.cs +++ b/test/Bravo.Tests/Infrastructure/Extensions/StringExtensionsTests.cs @@ -1,25 +1,12 @@ -using System; -using System.Collections; -using System.IO; -using System.Linq; -using System.Reflection; -using Sqlbi.Bravo.Infrastructure; +using Sqlbi.Bravo.Infrastructure; using Sqlbi.Bravo.Infrastructure.Extensions; using Sqlbi.Bravo.Models.FormatDax; using Xunit; -using Xunit.Abstractions; namespace Bravo.Tests.Infrastructure.Extensions; public class StringExtensionsTests { - private readonly ITestOutputHelper _output; - - public StringExtensionsTests(ITestOutputHelper output) - { - _output = output; - } - [Theory] [InlineData("", "", DaxLineBreakStyle.None)] [InlineData(null, null, DaxLineBreakStyle.None)] @@ -66,55 +53,4 @@ public void AppendApplicationVersion_Test() Assert.Equal($"Bravo for Power BI - v{AppVersion.SemanticVersion}", actual); } - - [Fact] - public void IsPBIDesktopMainWindowTitle_Test() - { - var path = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles), "Microsoft Power BI Desktop", "bin"); - - if (!Directory.Exists(path)) - { - // Skip test if the BPIDesktop folder does not exist (i.e. build agent) - return; - } - - foreach (var file in Directory.EnumerateFiles(path, "Microsoft.PowerBI.Client.Windows.Resources.dll", SearchOption.AllDirectories)) - { - _output.WriteLine("Resource file '{0}'", file); - - var assembly = Assembly.LoadFrom(file); - var name = assembly.GetManifestResourceNames().SingleOrDefault((name) => name.StartsWith($"Microsoft.PowerBI.Client.Windows.PowerBIStringResources.")); - - if (name is null) - Assert.Fail($"Resource name not found in file '{file}'"); - - using var stream = assembly.GetManifestResourceStream(name); - - if (name is null) - Assert.Fail($"Resource stream not found in file '{file}'"); - - using var reader = new System.Resources.ResourceReader(stream!); - - var found = false; - foreach (DictionaryEntry entry in reader) - { - if (entry.Key is string key && key == "PowerBIWindowTitle" && entry.Value is string value) - { - var unicodeValue = string.Join(" ", value.EnumerateRunes().Select((rune) => $"U+{rune.Value:X4}")); - _output.WriteLine("\t{0} > {1}", unicodeValue, value); - - var formattedTitle = string.Format(/*System.Globalization.CultureInfo.CurrentCulture,*/ value, "Contoso", "Power BI Desktop"); - var isSupported = AppEnvironment.PBIDesktopMainWindowTitleSuffixes.Any(formattedTitle.EndsWith); - - Assert.True(isSupported, $"Unsupported 'PowerBIWindowTitle' format string in resource file '{file}'"); - - found = true; - break; - } - } - - if (!found) - Assert.Fail($"Resource key 'PowerBIWindowTitle' not found in file '{file}'"); - } - } } diff --git a/test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopTests.cs b/test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopTests.cs new file mode 100644 index 00000000..b660f032 --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopTests.cs @@ -0,0 +1,106 @@ +using System; +using System.Collections; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Resources; +using Sqlbi.Bravo.Infrastructure.Services.PowerBI; +using Xunit; +using Xunit.Abstractions; + +namespace Bravo.Tests.Infrastructure.Services.PowerBI; + +public class PowerBIDesktopTests +{ + private readonly ITestOutputHelper _output; + + public PowerBIDesktopTests(ITestOutputHelper output) + { + _output = output; + } + + [Theory] + [InlineData("Contoso \u002D Power BI Desktop")] + [InlineData("Contoso \u2013 Power BI Desktop")] + [InlineData("Contoso \u2014 Power BI Desktop")] + [InlineData("Contoso\u2013Power BI Desktop")] + [InlineData("Contoso\u00A0\u2014 Power BI Desktop")] + public void GetReportName_TitleWithApplicationName_ReturnsTheReportName(string windowTitle) + { + Assert.Equal("Contoso", PowerBIDesktop.GetReportName(windowTitle)); + } + + [Fact] + public void GetReportName_TitleWithoutApplicationName_ReturnsTheTitle() + { + Assert.Equal("Contoso", PowerBIDesktop.GetReportName("Contoso")); + } + + [Fact] + public void GetReportName_ReportNameEndingWithApplicationName_RemovesOnlyTheLastOne() + { + var reportName = PowerBIDesktop.GetReportName("Contoso - Power BI Desktop - Power BI Desktop"); + + Assert.Equal("Contoso - Power BI Desktop", reportName); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + public void GetReportName_EmptyTitle_ReturnsNull(string windowTitle) + { + Assert.Null(PowerBIDesktop.GetReportName(windowTitle)); + } + + [Fact] + public void GetReportName_InstalledTitleFormats_ReturnsTheReportName() + { + var path = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles), "Microsoft Power BI Desktop", "bin"); + + if (!Directory.Exists(path)) + { + // Skip test if the BPIDesktop folder does not exist (i.e. build agent) + return; + } + + foreach (var file in Directory.EnumerateFiles(path, "Microsoft.PowerBI.Client.Windows.Resources.dll", SearchOption.AllDirectories)) + { + _output.WriteLine("Resource file '{0}'", file); + + var assembly = Assembly.LoadFrom(file); + var name = assembly.GetManifestResourceNames().SingleOrDefault((name) => name.StartsWith($"Microsoft.PowerBI.Client.Windows.PowerBIStringResources.")); + + if (name is null) + Assert.Fail($"Resource name not found in file '{file}'"); + + using var stream = assembly.GetManifestResourceStream(name); + + if (name is null) + Assert.Fail($"Resource stream not found in file '{file}'"); + + using var reader = new ResourceReader(stream!); + + var found = false; + foreach (DictionaryEntry entry in reader) + { + if (entry.Key is string key && key == "PowerBIWindowTitle" && entry.Value is string value) + { + var unicodeValue = string.Join(" ", value.EnumerateRunes().Select((rune) => $"U+{rune.Value:X4}")); + _output.WriteLine("\t{0} > {1}", unicodeValue, value); + + var formattedTitle = string.Format(/*System.Globalization.CultureInfo.CurrentCulture,*/ value, "Contoso", "Power BI Desktop"); + + var reportName = PowerBIDesktop.GetReportName(formattedTitle); + + Assert.True(reportName == "Contoso", $"Unsupported 'PowerBIWindowTitle' format string in resource file '{file}'"); + + found = true; + break; + } + } + + if (!found) + Assert.Fail($"Resource key 'PowerBIWindowTitle' not found in file '{file}'"); + } + } +} From 6e173b68875408089527b8ed1dba764ecf666a51 Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Tue, 20 Oct 2026 16:25:40 +0200 Subject: [PATCH 3/9] refactor: look up processes in an explicit Windows session Process.TryGetById and Process.GetParentProcess are extension members that take the session from the caller or from the process itself instead of AppEnvironment, so a caller that must not initialize AppEnvironment can use them. The WMI query of the parent lookup is unchanged. ProcessHelper.SafeGetProcessById and ProcessHelper.GetParentProcess keep their callers and delegate to them with the current session. A process found in another session, or one that has exited, is now disposed instead of being dropped with its handle open. --- .../Extensions/ProcessExtensions.cs | 86 +++++++++++++++++++ src/Infrastructure/Helpers/ProcessHelper.cs | 49 ++--------- .../Extensions/ProcessExtensionsTests.cs | 76 ++++++++++++++++ 3 files changed, 167 insertions(+), 44 deletions(-) create mode 100644 test/Bravo.Tests/Infrastructure/Extensions/ProcessExtensionsTests.cs diff --git a/src/Infrastructure/Extensions/ProcessExtensions.cs b/src/Infrastructure/Extensions/ProcessExtensions.cs index f8950d80..443343a2 100644 --- a/src/Infrastructure/Extensions/ProcessExtensions.cs +++ b/src/Infrastructure/Extensions/ProcessExtensions.cs @@ -2,6 +2,8 @@ using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Management; using System.Runtime.InteropServices; using System.Text; @@ -77,6 +79,90 @@ void GetImpl() } } + extension(Process) + { + /// + /// Gets the running process with the given identifier in the given Windows session. + /// + /// + /// when the process is not running, has exited, runs in another session or cannot be + /// opened. + /// + public static bool TryGetById(int processId, int sessionId, [NotNullWhen(true)] out Process? process) + { + process = null; + Process? candidate = null; + try + { + candidate = Process.GetProcessById(processId); // Throws ArgumentException if the process is not running + + if (candidate.SessionId != sessionId) + return false; + + if (candidate.HasExited) // Throws Win32Exception if the process cannot be opened, for example another user's + return false; + + _ = candidate.ProcessName; // Throws InvalidOperationException if the process has exited in the meantime + + process = candidate; + candidate = null; // Owned by the caller from here on, so the finally block must not dispose it + return true; + } + catch (Exception ex) when (ex is ArgumentException or InvalidOperationException or Win32Exception) + { + return false; + } + finally + { + candidate?.Dispose(); + } + } + } + + extension(Process process) + { + /// + /// Gets the parent of the process when it runs in the same Windows session; otherwise, . + /// + public Process? GetParentProcess() + { + // ManagementObjectSearcher.Get() raises a System.InvalidCastException when executed on the current thread, this regardless of the apartment state of the current thread (which is STA) + // + // System.InvalidCastException "Specified cast is not valid." + // at System.StubHelpers.InterfaceMarshaler.ConvertToNative(Object objSrc, IntPtr itfMT, IntPtr classMT, Int32 flags) + // at System.Management.SecuredIWbemServicesHandler.ExecQuery_(String strQueryLanguage, String strQuery, Int32 lFlags, IWbemContext pCtx, IEnumWbemClassObject& ppEnum) + // at System.Management.ManagementObjectSearcher.Get() + + var processId = process.Id; + var sessionId = process.SessionId; + var parentProcessId = (int?)null; + + ProcessHelper.RunOnSTAThread(GetImpl); + + if (parentProcessId is not int id) + return null; + + if (!Process.TryGetById(id, sessionId, out var parentProcess)) + return null; + + return parentProcess; + + void GetImpl() + { + var queryString = $"SELECT ParentProcessId FROM Win32_Process WHERE ProcessId = {processId} AND SessionId = {sessionId}"; + + using var searcher = new ManagementObjectSearcher(queryString); + using var collection = searcher.Get(); + using var @object = collection.OfType().SingleOrDefault(); + + if (@object is not null) + { + parentProcessId = (int)(uint)@object["ParentProcessId"]; + } + } + } + } + public static string GetMainWindowTitle(this Process process) { if (process.MainWindowTitle.Length > 0) diff --git a/src/Infrastructure/Helpers/ProcessHelper.cs b/src/Infrastructure/Helpers/ProcessHelper.cs index bfddc856..72d50766 100644 --- a/src/Infrastructure/Helpers/ProcessHelper.cs +++ b/src/Infrastructure/Helpers/ProcessHelper.cs @@ -6,7 +6,6 @@ using System.Globalization; using System.IO; using System.Linq; -using System.Management; using System.Runtime.ExceptionServices; using System.Security.Claims; using System.Security.Principal; @@ -283,33 +282,9 @@ public static IReadOnlyList GetProcessesByName(string processName) public static Process? GetParentProcess() { - // ManagementObjectSearcher.Get() raises a System.InvalidCastException when executed on the current thread, this regardless of the apartment state of the current thread (which is STA) - // - // System.InvalidCastException "Specified cast is not valid." - // at System.StubHelpers.InterfaceMarshaler.ConvertToNative(Object objSrc, IntPtr itfMT, IntPtr classMT, Int32 flags) - // at System.Management.SecuredIWbemServicesHandler.ExecQuery_(String strQueryLanguage, String strQuery, Int32 lFlags, IWbemContext pCtx, IEnumWbemClassObject& ppEnum) - // at System.Management.ManagementObjectSearcher.Get() + using var currentProcess = Process.GetCurrentProcess(); - var parentProcessId = (int?)null; - - RunOnSTAThread(GetImpl); - - var parentProcess = SafeGetProcessById(parentProcessId); - return parentProcess; - - void GetImpl() - { - var queryString = $"SELECT ParentProcessId FROM Win32_Process WHERE ProcessId = {AppEnvironment.ProcessId} AND SessionId = {AppEnvironment.SessionId}"; - - using var searcher = new ManagementObjectSearcher(queryString); - using var collection = searcher.Get(); - using var @object = collection.OfType().SingleOrDefault(); - - if (@object is not null) - { - parentProcessId = (int)(uint)@object["ParentProcessId"]; - } - } + return currentProcess.GetParentProcess(); } public static IntPtr GetCurrentProcessMainWindowHandle() @@ -334,24 +309,10 @@ public static IntPtr GetParentProcessMainWindowHandle() if (processId is null) return null; - try - { - var process = Process.GetProcessById(processId.Value); // Throws ArgumentException if the process specified by the processId parameter is not running. - - if (process.SessionId != AppEnvironment.SessionId) - return null; - - if (process.HasExited) - return null; - - _ = process.ProcessName; // Throws InvalidOperationException if the process has exited, so the requested information is not available - - return process; - } - catch (Exception ex) when (ex is ArgumentException || ex is InvalidOperationException) - { + if (!Process.TryGetById(processId.Value, AppEnvironment.SessionId, out var process)) return null; - } + + return process; } public static bool IsUserAdministrator() diff --git a/test/Bravo.Tests/Infrastructure/Extensions/ProcessExtensionsTests.cs b/test/Bravo.Tests/Infrastructure/Extensions/ProcessExtensionsTests.cs new file mode 100644 index 00000000..2ce1fa09 --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Extensions/ProcessExtensionsTests.cs @@ -0,0 +1,76 @@ +using System.Diagnostics; +using Sqlbi.Bravo.Infrastructure.Extensions; +using Xunit; + +namespace Bravo.Tests.Infrastructure.Extensions; + +public class ProcessExtensionsTests +{ + [Fact] + public void TryGetById_RunningProcessInTheSession_ReturnsTheProcess() + { + using var currentProcess = Process.GetCurrentProcess(); + + Assert.True(Process.TryGetById(currentProcess.Id, currentProcess.SessionId, out var process)); + + using (process) + { + Assert.Equal(currentProcess.Id, process.Id); + } + } + + [Fact] + public void TryGetById_ProcessInAnotherSession_ReturnsFalse() + { + using var currentProcess = Process.GetCurrentProcess(); + + Assert.False(Process.TryGetById(currentProcess.Id, currentProcess.SessionId + 1, out var process)); + Assert.Null(process); + } + + [Fact] + public void TryGetById_ProcessNotRunning_ReturnsFalse() + { + using var currentProcess = Process.GetCurrentProcess(); + var startInfo = new ProcessStartInfo("cmd.exe", "/c exit") + { + CreateNoWindow = true, + UseShellExecute = false, + }; + // The handle held until the end of the test keeps Windows from reusing the identifier for another process. + using var exitedProcess = Process.Start(startInfo); + Assert.NotNull(exitedProcess); + exitedProcess.WaitForExit(); + + Assert.False(Process.TryGetById(exitedProcess.Id, currentProcess.SessionId, out var process)); + Assert.Null(process); + } + + [Fact] + public void GetParentProcess_ChildProcess_ReturnsTheProcessThatStartedIt() + { + using var currentProcess = Process.GetCurrentProcess(); + // Redirected input keeps "pause" waiting until the test ends the process. + var startInfo = new ProcessStartInfo("cmd.exe", "/c pause") + { + CreateNoWindow = true, + UseShellExecute = false, + RedirectStandardInput = true, + }; + using var childProcess = Process.Start(startInfo); + Assert.NotNull(childProcess); + + try + { + using var parentProcess = childProcess.GetParentProcess(); + + Assert.NotNull(parentProcess); + Assert.Equal(currentProcess.Id, parentProcess.Id); + } + finally + { + childProcess.Kill(); + childProcess.WaitForExit(); + } + } +} From e895e741bd31ceba767a642bfbb697a4cd60da73 Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Tue, 20 Oct 2026 16:28:27 +0200 Subject: [PATCH 4/9] refactor: parse the startup command line without AppEnvironment StartupCommandLine.Parse turns the arguments into values without reading the process environment, so the parsing is testable. FromCommandLine then finds the process that launched Bravo with the session of the current process, and tests the MSIX package directly instead of reading AppEnvironment.DeploymentMode. A secondary instance launched with arguments, for example as a Power BI Desktop External Tool, therefore builds its activation payload without running the AppEnvironment static constructor, as one launched without arguments already did. The payload is unchanged. --- docs/design/architecture.md | 9 +- .../Settings/StartupCommandLine.cs | 86 ++++++++++++++ .../Configuration/Settings/StartupSettings.cs | 90 +++++--------- .../Host/BravoSingleInstanceTests.cs | 26 ++++ test/Bravo.Tests/Host/CallGraph.cs | 100 ++++++++++++++++ .../Settings/StartupCommandLineTests.cs | 111 ++++++++++++++++++ 6 files changed, 361 insertions(+), 61 deletions(-) create mode 100644 src/Infrastructure/Configuration/Settings/StartupCommandLine.cs create mode 100644 test/Bravo.Tests/Host/CallGraph.cs create mode 100644 test/Bravo.Tests/Infrastructure/Configuration/Settings/StartupCommandLineTests.cs diff --git a/docs/design/architecture.md b/docs/design/architecture.md index b062c2c2..3d6c5270 100644 --- a/docs/design/architecture.md +++ b/docs/design/architecture.md @@ -74,9 +74,12 @@ through a named pipe with one server instance. It disconnects between clients wi not replace that with disposal and recreation between requests: another process could claim ownership in the gap. `BravoSingleInstance.ClaimOrRedirect` claims the pipe before the bootstrap: a secondary process forwards its startup -arguments and exits without creating directories, bootstrap services, a host or a window, and without reading the -user settings file. Keep the arbitration and the activation payload free of those steps: a secondary that builds -bootstrap services can transmit stored telemetry and reads files that the primary writes. +arguments and exits without creating directories, bootstrap services, a host or a window, without reading the user +settings file, and without running the `AppEnvironment` static constructor. Keep the arbitration and the activation +payload free of those steps: a secondary that builds bootstrap services can transmit stored telemetry and reads files +that the primary writes, and the `AppEnvironment` static constructor probes the WebView2 runtime, the deployment mode +and the registry. The payload code does not reference `AppEnvironment` and takes the Windows session from the +current process. Current activation limits: diff --git a/src/Infrastructure/Configuration/Settings/StartupCommandLine.cs b/src/Infrastructure/Configuration/Settings/StartupCommandLine.cs new file mode 100644 index 00000000..f1d8ceb7 --- /dev/null +++ b/src/Infrastructure/Configuration/Settings/StartupCommandLine.cs @@ -0,0 +1,86 @@ +using System; +using System.Collections.Generic; +using System.CommandLine; +using System.CommandLine.Parsing; +using System.Linq; + +namespace Sqlbi.Bravo.Infrastructure.Configuration.Settings; + +/// +/// The arguments Bravo was started with, as a Power BI Desktop External Tool passes them. +/// +internal sealed class StartupCommandLine +{ + private StartupCommandLine() + { + } + + public bool IsEmpty { get; private init; } + + /// + /// Whether --server or --database was passed, even with a missing value. + /// + public bool HasConnectionArguments { get; private init; } + + public string? ServerName { get; private init; } + + public string? DatabaseName { get; private init; } + + public int? ParentProcessId { get; private init; } + + public IReadOnlyList Errors { get; private init; } = []; + + /// The arguments without the program name. + public static StartupCommandLine Parse(IReadOnlyList args) + { + ArgumentNullException.ThrowIfNull(args); + + if (args.Count == 0) + return new StartupCommandLine { IsEmpty = true }; + + var serverOption = new Option("--server"); + serverOption.AddAlias("-s"); + serverOption.IsRequired = true; + serverOption.Description = "Server name"; + serverOption.Arity = ArgumentArity.ExactlyOne; + + var databaseOption = new Option("--database"); + databaseOption.AddAlias("-d"); + databaseOption.IsRequired = true; + databaseOption.Description = "Database name"; + databaseOption.Arity = ArgumentArity.ExactlyOne; + + var parentProcessIdOption = new Option("--ppid"); + parentProcessIdOption.IsRequired = false; + parentProcessIdOption.Description = "Parent process ID"; + parentProcessIdOption.Arity = ArgumentArity.ExactlyOne; + + var command = new RootCommand + { + serverOption, + databaseOption, + parentProcessIdOption + }; + + var parseResult = command.Parse(args.ToArray()); + + return new StartupCommandLine + { + HasConnectionArguments = parseResult.HasOption(serverOption) || parseResult.HasOption(databaseOption), + ServerName = HasValidValue(parseResult, serverOption) ? parseResult.GetValueForOption(serverOption) : null, + DatabaseName = HasValidValue(parseResult, databaseOption) ? parseResult.GetValueForOption(databaseOption) : null, + ParentProcessId = HasValidValue(parseResult, parentProcessIdOption) ? parseResult.GetValueForOption(parentProcessIdOption) : null, + Errors = parseResult.Errors.Select((e) => e.Message).ToArray(), + }; + } + + // GetValueForOption throws when the value is missing or cannot be converted, so it is read only without an error. + private static bool HasValidValue(ParseResult parseResult, Option option) + { + var optionResult = parseResult.FindResultFor(option); + if (optionResult is null) + return false; + + return optionResult.ErrorMessage is null; + } +} diff --git a/src/Infrastructure/Configuration/Settings/StartupSettings.cs b/src/Infrastructure/Configuration/Settings/StartupSettings.cs index 171f9925..1c88caf4 100644 --- a/src/Infrastructure/Configuration/Settings/StartupSettings.cs +++ b/src/Infrastructure/Configuration/Settings/StartupSettings.cs @@ -1,6 +1,4 @@ using System; -using System.CommandLine; -using System.CommandLine.Parsing; using System.Diagnostics; using System.Linq; using System.Text.Json.Serialization; @@ -61,71 +59,47 @@ public static void FromCommandLineArguments(this StartupSettings settings) // If the file name is not available, the first element is equal to String.Empty. // In.NET 5 and later versions, for single-file publishing, the first element is the name of the host executable. var args = Environment.GetCommandLineArgs().Skip(1).ToArray(); - if (args.Length == 0) + var commandLine = StartupCommandLine.Parse(args); + + using var currentProcess = Process.GetCurrentProcess(); + + settings.FromCommandLine(commandLine, currentProcess); + } + + // Does not run the AppEnvironment static constructor: a secondary instance builds its activation payload from these + // settings before it exits. + public static void FromCommandLine(this StartupSettings settings, StartupCommandLine commandLine, Process currentProcess) + { + if (commandLine.IsEmpty) { - // No args provided settings.IsEmpty = true; return; } - var serverOption = new Option("--server"); - serverOption.AddAlias("-s"); - serverOption.IsRequired = true; - serverOption.Description = "Server name"; - serverOption.Arity = ArgumentArity.ExactlyOne; - - var databaseOption = new Option("--database"); - databaseOption.AddAlias("-d"); - databaseOption.IsRequired = true; - databaseOption.Description = "Database name"; - databaseOption.Arity = ArgumentArity.ExactlyOne; - - var parentProcessIdOption = new Option("--ppid"); - parentProcessIdOption.IsRequired = false; - parentProcessIdOption.Description = "Parent process ID"; - parentProcessIdOption.Arity = ArgumentArity.ExactlyOne; + settings.ArgumentServerName = commandLine.ServerName; + settings.ArgumentDatabaseName = commandLine.DatabaseName; + settings.IsExternalTool = commandLine.HasConnectionArguments; + settings.CommandLineErrors = commandLine.Errors.ToArray(); - var command = new RootCommand + using var parentProcess = GetParentProcess(commandLine, currentProcess); + if (parentProcess is not null) { - serverOption, - databaseOption, - parentProcessIdOption - }; - - var parseResult = command.Parse(args); - - var serverOptionResult = parseResult.FindResultFor(serverOption); - if (serverOptionResult is not null && serverOptionResult.ErrorMessage is null) - settings.ArgumentServerName = parseResult.GetValueForOption(serverOption); + settings.ParentProcessId = parentProcess.Id; + settings.ParentProcessName = parentProcess.ProcessName; + //settings.ParentProcessMainWindowHandle = parentProcess.MainWindowHandle; + var windowTitle = parentProcess.GetMainWindowTitle(); + settings.ParentProcessMainWindowTitle = settings.IsPBIDesktopExternalTool ? PowerBIDesktop.GetReportName(windowTitle) : windowTitle; + } + } - var databaseOptionResult = parseResult.FindResultFor(databaseOption); - if (databaseOptionResult is not null && databaseOptionResult.ErrorMessage is null) - settings.ArgumentDatabaseName = parseResult.GetValueForOption(databaseOption); + private static Process? GetParentProcess(StartupCommandLine commandLine, Process currentProcess) + { + if (commandLine.ParentProcessId is not int parentProcessId || !DesktopBridgeHelper.IsRunningAsMsixPackage()) + return currentProcess.GetParentProcess(); - settings.IsExternalTool = parseResult.HasOption(serverOption) || parseResult.HasOption(databaseOption); - settings.CommandLineErrors = parseResult.Errors.Select((e) => e.Message).ToArray(); + if (!Process.TryGetById(parentProcessId, currentProcess.SessionId, out var parentProcess)) + return null; - Process? parentProcess = null; - { - if (AppEnvironment.DeploymentMode == AppDeploymentMode.Packaged && parseResult.HasOption(parentProcessIdOption)) - { - var parentProcessId = parseResult.GetValueForOption(parentProcessIdOption); - parentProcess = ProcessHelper.SafeGetProcessById(parentProcessId); - } - else - { - parentProcess = ProcessHelper.GetParentProcess(); - } - - if (parentProcess is not null) - { - settings.ParentProcessId = parentProcess.Id; - settings.ParentProcessName = parentProcess.ProcessName; - //settings.ParentProcessMainWindowHandle = parentProcess.MainWindowHandle; - var windowTitle = parentProcess.GetMainWindowTitle(); - settings.ParentProcessMainWindowTitle = settings.IsPBIDesktopExternalTool ? PowerBIDesktop.GetReportName(windowTitle) : windowTitle; - } - } - parentProcess?.Dispose(); + return parentProcess; } } diff --git a/test/Bravo.Tests/Host/BravoSingleInstanceTests.cs b/test/Bravo.Tests/Host/BravoSingleInstanceTests.cs index 98d908f5..776bde39 100644 --- a/test/Bravo.Tests/Host/BravoSingleInstanceTests.cs +++ b/test/Bravo.Tests/Host/BravoSingleInstanceTests.cs @@ -4,6 +4,8 @@ using System.Threading; using Microsoft.Extensions.Logging; using Sqlbi.Bravo.Host; +using Sqlbi.Bravo.Infrastructure; +using Sqlbi.Bravo.Infrastructure.Messages; using Sqlbi.Bravo.Infrastructure.SingleInstance; using Xunit; @@ -75,4 +77,28 @@ public void ClaimOrRedirect_PrimaryUnreachable_LogsAnErrorAndReturnsNull() Assert.Contains(nameof(SingleInstanceSendStatus.OwnerUnavailable), entry.Message); Assert.Equal(1, loggerFactory.DisposeCount); } + + [Fact] + public void ClaimOrRedirect_DoesNotReferenceAppEnvironment() + { + // A secondary runs this path before it exits: the AppEnvironment static constructor probes the WebView2 + // runtime, the deployment mode and the registry, which a process that only forwards its arguments must not do. + var claimOrRedirect = typeof(BravoSingleInstance).GetMethod(nameof(BravoSingleInstance.ClaimOrRedirect), Type.EmptyTypes); + Assert.NotNull(claimOrRedirect); + + var referencedTypes = CallGraph.GetReferencedTypes(claimOrRedirect); + + Assert.DoesNotContain(typeof(AppEnvironment), referencedTypes); + } + + [Fact] + public void CallGraph_MethodThatReadsAppEnvironment_ReferencesIt() + { + var toJsonElement = typeof(AppInstanceStartupMessageExtensions).GetMethod(nameof(AppInstanceStartupMessageExtensions.ToJsonElement)); + Assert.NotNull(toJsonElement); + + var referencedTypes = CallGraph.GetReferencedTypes(toJsonElement); + + Assert.Contains(typeof(AppEnvironment), referencedTypes); + } } diff --git a/test/Bravo.Tests/Host/CallGraph.cs b/test/Bravo.Tests/Host/CallGraph.cs new file mode 100644 index 00000000..08a3404b --- /dev/null +++ b/test/Bravo.Tests/Host/CallGraph.cs @@ -0,0 +1,100 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Reflection.Emit; + +namespace Bravo.Tests.Host; + +/// +/// Reads the IL of methods to find the types whose members they reference. +/// +/// +/// Calls are followed only to methods of the same assembly, as declared: an override or an interface implementation +/// reached through virtual dispatch is not followed. Constants do not appear, because the compiler copies their value. +/// +internal static class CallGraph +{ + private static readonly Dictionary s_opCodes = typeof(OpCodes) + .GetFields(BindingFlags.Public | BindingFlags.Static) + .Select((field) => (OpCode)field.GetValue(null)!) + .ToDictionary((opCode) => (ushort)opCode.Value); + + /// + /// Returns the types whose fields or methods are referenced by or by any method of its + /// assembly that it reaches. + /// + public static IReadOnlySet GetReferencedTypes(MethodBase root) + { + var assembly = root.Module.Assembly; + var visited = new HashSet(); + var types = new HashSet(); + var pending = new Stack(); + pending.Push(root); + + while (pending.TryPop(out var method)) + { + if (!visited.Add(method)) + continue; + + foreach (var member in GetReferencedMembers(method)) + { + if (member.DeclaringType is not null) + types.Add(member.DeclaringType); + + if (member is MethodBase callee && callee.Module.Assembly == assembly) + pending.Push(callee); + } + } + + return types; + } + + private static IEnumerable GetReferencedMembers(MethodBase method) + { + var il = method.GetMethodBody()?.GetILAsByteArray(); + if (il is null) + yield break; + + var typeArguments = method.DeclaringType?.IsGenericType == true ? method.DeclaringType.GetGenericArguments() : null; + var methodArguments = method.IsGenericMethod ? method.GetGenericArguments() : null; + + var position = 0; + while (position < il.Length) + { + var opCode = ReadOpCode(il, ref position); + + if (opCode.OperandType is OperandType.InlineMethod or OperandType.InlineField) + { + var token = BitConverter.ToInt32(il, position); + var member = method.Module.ResolveMember(token, typeArguments, methodArguments); + if (member is not null) + yield return member; + } + + position += GetOperandSize(opCode, il, position); + } + } + + private static OpCode ReadOpCode(byte[] il, ref int position) + { + var value = (ushort)il[position++]; + if (value == 0xFE) + value = (ushort)(0xFE00 | il[position++]); + + return s_opCodes[value]; + } + + private static int GetOperandSize(OpCode opCode, byte[] il, int position) + { + return opCode.OperandType switch + { + OperandType.InlineNone => 0, + OperandType.ShortInlineBrTarget or OperandType.ShortInlineI or OperandType.ShortInlineVar => 1, + OperandType.InlineVar => 2, + OperandType.InlineI8 or OperandType.InlineR => 8, + OperandType.InlineSwitch => 4 + (4 * BitConverter.ToInt32(il, position)), + _ => 4, + }; + } +} diff --git a/test/Bravo.Tests/Infrastructure/Configuration/Settings/StartupCommandLineTests.cs b/test/Bravo.Tests/Infrastructure/Configuration/Settings/StartupCommandLineTests.cs new file mode 100644 index 00000000..aa833e59 --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Configuration/Settings/StartupCommandLineTests.cs @@ -0,0 +1,111 @@ +using System.Diagnostics; +using Sqlbi.Bravo.Infrastructure.Configuration.Settings; +using Xunit; + +namespace Bravo.Tests.Infrastructure.Configuration.Settings; + +public class StartupCommandLineTests +{ + [Fact] + public void Parse_NoArguments_IsEmpty() + { + var commandLine = StartupCommandLine.Parse([]); + + Assert.True(commandLine.IsEmpty); + Assert.False(commandLine.HasConnectionArguments); + Assert.Empty(commandLine.Errors); + } + + [Theory] + [InlineData("--server", "--database")] + [InlineData("-s", "-d")] + public void Parse_ServerAndDatabase_ReturnsBoth(string serverOption, string databaseOption) + { + var commandLine = StartupCommandLine.Parse([serverOption, "localhost:12345", databaseOption, "db"]); + + Assert.False(commandLine.IsEmpty); + Assert.True(commandLine.HasConnectionArguments); + Assert.Equal("localhost:12345", commandLine.ServerName); + Assert.Equal("db", commandLine.DatabaseName); + Assert.Null(commandLine.ParentProcessId); + Assert.Empty(commandLine.Errors); + } + + [Fact] + public void Parse_MissingDatabase_KeepsTheServerAndReportsAnError() + { + var commandLine = StartupCommandLine.Parse(["--server", "localhost:12345"]); + + Assert.True(commandLine.HasConnectionArguments); + Assert.Equal("localhost:12345", commandLine.ServerName); + Assert.Null(commandLine.DatabaseName); + Assert.NotEmpty(commandLine.Errors); + } + + [Fact] + public void Parse_ParentProcessId_ReturnsIt() + { + var commandLine = StartupCommandLine.Parse(["--server", "localhost:12345", "--database", "db", "--ppid", "42"]); + + Assert.Equal(42, commandLine.ParentProcessId); + Assert.Empty(commandLine.Errors); + } + + [Theory] + [InlineData("--ppid", "abc")] + [InlineData("--ppid", "")] + [InlineData("--ppid")] + public void Parse_InvalidParentProcessId_ReportsAnErrorWithoutAValue(params string[] parentProcessIdArguments) + { + string[] args = ["--server", "localhost:12345", "--database", "db", .. parentProcessIdArguments]; + + var commandLine = StartupCommandLine.Parse(args); + + Assert.Null(commandLine.ParentProcessId); + Assert.Equal("localhost:12345", commandLine.ServerName); + Assert.NotEmpty(commandLine.Errors); + } + + [Fact] + public void Parse_UnknownArgument_ReportsErrorsWithoutConnectionArguments() + { + var commandLine = StartupCommandLine.Parse([@"C:\Reports\Contoso.pbix"]); + + Assert.False(commandLine.IsEmpty); + Assert.False(commandLine.HasConnectionArguments); + Assert.Null(commandLine.ServerName); + Assert.NotEmpty(commandLine.Errors); + } + + [Fact] + public void FromCommandLine_EmptyCommandLine_SetsOnlyIsEmpty() + { + // The activation payload of a launch without arguments carries null for every other value. + var settings = new StartupSettings(); + var commandLine = StartupCommandLine.Parse([]); + using var currentProcess = Process.GetCurrentProcess(); + + settings.FromCommandLine(commandLine, currentProcess); + + Assert.True(settings.IsEmpty); + Assert.Null(settings.CommandLineErrors); + Assert.Null(settings.ParentProcessId); + } + + [Fact] + public void FromCommandLine_ConnectionArguments_SetsTheArguments() + { + var settings = new StartupSettings(); + var commandLine = StartupCommandLine.Parse(["--server", "localhost:12345", "--database", "db"]); + using var currentProcess = Process.GetCurrentProcess(); + + settings.FromCommandLine(commandLine, currentProcess); + + Assert.False(settings.IsEmpty); + Assert.True(settings.IsExternalTool); + Assert.Equal("localhost:12345", settings.ArgumentServerName); + Assert.Equal("db", settings.ArgumentDatabaseName); + Assert.NotNull(settings.CommandLineErrors); + Assert.Empty(settings.CommandLineErrors); + } +} From 2ffb00c5f7e5baa396073df7cb36177f1fa5b526 Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Tue, 20 Oct 2026 16:38:49 +0200 Subject: [PATCH 5/9] test: check the title formats of the Store installation The test of the localized window title formats also finds the Microsoft Store package of Power BI Desktop, through the documented package APIs, since its WindowsApps folder cannot be listed. The resource assemblies are loaded from their bytes: files of a package folder can be read but not loaded as images. --- test/Bravo.Tests/Bravo.Tests.csproj | 1 + .../PowerBI/PowerBIDesktopInstallation.cs | 37 +++++++++++++ .../Services/PowerBI/PowerBIDesktopTests.cs | 20 ++++--- test/Bravo.Tests/Interop/PackageInterop.cs | 55 +++++++++++++++++++ test/Bravo.Tests/NativeMethods.json | 4 ++ test/Bravo.Tests/NativeMethods.txt | 3 + 6 files changed, 112 insertions(+), 8 deletions(-) create mode 100644 test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopInstallation.cs create mode 100644 test/Bravo.Tests/Interop/PackageInterop.cs create mode 100644 test/Bravo.Tests/NativeMethods.json create mode 100644 test/Bravo.Tests/NativeMethods.txt diff --git a/test/Bravo.Tests/Bravo.Tests.csproj b/test/Bravo.Tests/Bravo.Tests.csproj index 250e2637..7ec091b6 100644 --- a/test/Bravo.Tests/Bravo.Tests.csproj +++ b/test/Bravo.Tests/Bravo.Tests.csproj @@ -9,6 +9,7 @@ + diff --git a/test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopInstallation.cs b/test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopInstallation.cs new file mode 100644 index 00000000..236839fc --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopInstallation.cs @@ -0,0 +1,37 @@ +using System; +using System.Collections.Generic; +using System.IO; +using Bravo.Tests.Interop; + +namespace Bravo.Tests.Infrastructure.Services.PowerBI; + +/// +/// Finds the Power BI Desktop installations on this machine: the MSI one and the Microsoft Store package. +/// +internal static class PowerBIDesktopInstallation +{ + private const string PackageFamilyName = "Microsoft.MicrosoftPowerBIDesktop_8wekyb3d8bbwe"; + + /// + /// Returns the bin folder of every installation. + /// + public static IEnumerable GetBinFolders() + { + var programFilesFolder = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); + var installerFolder = Path.Combine(programFilesFolder, "Microsoft Power BI Desktop", "bin"); + if (Directory.Exists(installerFolder)) + yield return installerFolder; + + // The Store package of Power BI Desktop requires Windows 10. + if (!OperatingSystem.IsWindowsVersionAtLeast(10)) + yield break; + + // The package folder is under WindowsApps, which cannot be listed, so its path comes from the package. + foreach (var packageFolder in PackageInterop.GetPackageFolders(PackageFamilyName)) + { + var binFolder = Path.Combine(packageFolder, "bin"); + if (Directory.Exists(binFolder)) + yield return binFolder; + } + } +} diff --git a/test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopTests.cs b/test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopTests.cs index b660f032..b16fcb86 100644 --- a/test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopTests.cs +++ b/test/Bravo.Tests/Infrastructure/Services/PowerBI/PowerBIDesktopTests.cs @@ -55,30 +55,34 @@ public void GetReportName_EmptyTitle_ReturnsNull(string windowTitle) [Fact] public void GetReportName_InstalledTitleFormats_ReturnsTheReportName() { - var path = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles), "Microsoft Power BI Desktop", "bin"); + var files = PowerBIDesktopInstallation.GetBinFolders() + .SelectMany((folder) => Directory.EnumerateFiles(folder, "Microsoft.PowerBI.Client.Windows.Resources.dll", SearchOption.AllDirectories)) + .ToArray(); - if (!Directory.Exists(path)) + if (files.Length == 0) { - // Skip test if the BPIDesktop folder does not exist (i.e. build agent) + // Skip test if Power BI Desktop is not installed (i.e. build agent) return; } - foreach (var file in Directory.EnumerateFiles(path, "Microsoft.PowerBI.Client.Windows.Resources.dll", SearchOption.AllDirectories)) + foreach (var file in files) { _output.WriteLine("Resource file '{0}'", file); - var assembly = Assembly.LoadFrom(file); - var name = assembly.GetManifestResourceNames().SingleOrDefault((name) => name.StartsWith($"Microsoft.PowerBI.Client.Windows.PowerBIStringResources.")); + // From bytes: the Store package folder can be read but its files cannot be loaded as images. Each load also gets + // its own context, so the MSI installation and the Store package can contain the same assembly identity. + var assembly = Assembly.Load(File.ReadAllBytes(file)); + var name = assembly.GetManifestResourceNames().SingleOrDefault((name) => name.StartsWith("Microsoft.PowerBI.Client.Windows.PowerBIStringResources.")); if (name is null) Assert.Fail($"Resource name not found in file '{file}'"); using var stream = assembly.GetManifestResourceStream(name); - if (name is null) + if (stream is null) Assert.Fail($"Resource stream not found in file '{file}'"); - using var reader = new ResourceReader(stream!); + using var reader = new ResourceReader(stream); var found = false; foreach (DictionaryEntry entry in reader) diff --git a/test/Bravo.Tests/Interop/PackageInterop.cs b/test/Bravo.Tests/Interop/PackageInterop.cs new file mode 100644 index 00000000..b5c7457e --- /dev/null +++ b/test/Bravo.Tests/Interop/PackageInterop.cs @@ -0,0 +1,55 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Runtime.Versioning; +using Windows.Win32; +using Windows.Win32.Foundation; + +namespace Bravo.Tests.Interop; + +[SupportedOSPlatform("windows8.1")] +internal static class PackageInterop +{ + /// + /// Returns the installation folders of the packages of a family installed for the current user. + /// + /// The packages cannot be read. + public static IReadOnlyList GetPackageFolders(string packageFamilyName) + { + uint count = 0; + uint bufferLength = 0; + var result = TestPInvoke.GetPackagesByPackageFamily(packageFamilyName, ref count, null, ref bufferLength, null); + if (result == WIN32_ERROR.ERROR_SUCCESS) + return []; // No package of the family is installed + + if (result != WIN32_ERROR.ERROR_INSUFFICIENT_BUFFER) + throw new Win32Exception((int)result); + + var packageFullNames = new PWSTR[count]; + var buffer = new char[bufferLength]; + result = TestPInvoke.GetPackagesByPackageFamily(packageFamilyName, ref count, packageFullNames, ref bufferLength, buffer); + if (result != WIN32_ERROR.ERROR_SUCCESS) + throw new Win32Exception((int)result); + + var folders = new List(); + foreach (var packageFullName in new string(buffer).Split('\0', StringSplitOptions.RemoveEmptyEntries)) + folders.Add(GetPackagePath(packageFullName)); + + return folders; + } + + private static string GetPackagePath(string packageFullName) + { + uint pathLength = 0; + var result = TestPInvoke.GetPackagePathByFullName(packageFullName, ref pathLength, null); + if (result != WIN32_ERROR.ERROR_INSUFFICIENT_BUFFER) + throw new Win32Exception((int)result); + + var path = new char[pathLength]; + result = TestPInvoke.GetPackagePathByFullName(packageFullName, ref pathLength, path); + if (result != WIN32_ERROR.ERROR_SUCCESS) + throw new Win32Exception((int)result); + + return new string(path, 0, (int)pathLength - 1); // The length includes the null terminator + } +} diff --git a/test/Bravo.Tests/NativeMethods.json b/test/Bravo.Tests/NativeMethods.json new file mode 100644 index 00000000..1c96c39f --- /dev/null +++ b/test/Bravo.Tests/NativeMethods.json @@ -0,0 +1,4 @@ +{ + "$schema": "https://aka.ms/CsWin32.schema.json", + "className": "TestPInvoke" +} diff --git a/test/Bravo.Tests/NativeMethods.txt b/test/Bravo.Tests/NativeMethods.txt new file mode 100644 index 00000000..abde6e4e --- /dev/null +++ b/test/Bravo.Tests/NativeMethods.txt @@ -0,0 +1,3 @@ +// Windows.Win32.Storage.Packaging.Appx +GetPackagePathByFullName +GetPackagesByPackageFamily From b5fd76fd20e2d37d61aa0d3df0d6b3ef7872671f Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Tue, 20 Oct 2026 16:54:56 +0200 Subject: [PATCH 6/9] refactor: read the process tree from a Toolhelp32 snapshot process.GetParentProcess and GetChildrenPIDs read a Toolhelp32 process snapshot instead of querying WMI. The WMI query took about 200 ms, ran on a dedicated STA thread for each call and depended on the WMI service; a snapshot takes a few milliseconds and needs no service. A snapshot that cannot be taken throws, as a failed WMI query did, and is reported by the API boundary or the global handler. ProcessTree holds the snapshot and answers the parent and child queries without native calls, so they are unit tested. ProcessInterop takes the snapshot through CsWin32. PBIDesktopService takes one snapshot for all the Power BI Desktop processes it lists. Child processes are still limited to the session of their parent, now through Process.TryGetById, which also skips those that have exited. System.Management is no longer referenced. --- src/Bravo.csproj | 1 - .../Extensions/ProcessExtensions.cs | 75 ++++-------------- .../Services/PowerBI/PBIDesktopService.cs | 4 +- .../Windows/Interop/ProcessInterop.cs | 44 +++++++++++ src/Infrastructure/Windows/ProcessTree.cs | 66 ++++++++++++++++ src/Models/PBIDesktopReport.cs | 15 +++- src/NativeMethods.txt | 8 ++ src/packages.lock.json | 12 --- .../Windows/ProcessTreeTests.cs | 76 +++++++++++++++++++ 9 files changed, 224 insertions(+), 77 deletions(-) create mode 100644 src/Infrastructure/Windows/Interop/ProcessInterop.cs create mode 100644 src/Infrastructure/Windows/ProcessTree.cs create mode 100644 test/Bravo.Tests/Infrastructure/Windows/ProcessTreeTests.cs diff --git a/src/Bravo.csproj b/src/Bravo.csproj index 7957c75b..9942425c 100644 --- a/src/Bravo.csproj +++ b/src/Bravo.csproj @@ -60,7 +60,6 @@ - diff --git a/src/Infrastructure/Extensions/ProcessExtensions.cs b/src/Infrastructure/Extensions/ProcessExtensions.cs index 443343a2..b1fb815d 100644 --- a/src/Infrastructure/Extensions/ProcessExtensions.cs +++ b/src/Infrastructure/Extensions/ProcessExtensions.cs @@ -3,11 +3,10 @@ using System.ComponentModel; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Linq; -using System.Management; using System.Runtime.InteropServices; using System.Text; using Sqlbi.Bravo.Infrastructure.Helpers; +using Sqlbi.Bravo.Infrastructure.Windows; using Sqlbi.Bravo.Infrastructure.Windows.Interop; namespace Sqlbi.Bravo.Infrastructure.Extensions; @@ -15,7 +14,7 @@ namespace Sqlbi.Bravo.Infrastructure.Extensions; internal static class ProcessExtensions { //[DebuggerStepThrough] - [Obsolete("Use WMI query")] + [Obsolete("Use GetParentProcess")] public static Process? InteropGetParent(this Process process) { Ntdll.PROCESS_BASIC_INFORMATION processInformation; @@ -43,40 +42,21 @@ internal static class ProcessExtensions return null; } - public static IEnumerable GetChildrenPIDs(this Process process, string? childProcessImageName = null) + public static IReadOnlyList GetChildrenPIDs(this Process process, ProcessTree processTree, string? childProcessImageName = null) { - // ManagementObjectSearcher.Get() raises a System.InvalidCastException when executed on the current thread, this regardless of the apartment state of the current thread (which is STA) - // - // System.InvalidCastException "Specified cast is not valid." - // at System.StubHelpers.InterfaceMarshaler.ConvertToNative(Object objSrc, IntPtr itfMT, IntPtr classMT, Int32 flags) - // at System.Management.SecuredIWbemServicesHandler.ExecQuery_(String strQueryLanguage, String strQuery, Int32 lFlags, IWbemContext pCtx, IEnumWbemClassObject& ppEnum) - // at System.Management.ManagementObjectSearcher.Get() + var sessionId = process.SessionId; - var pids = new List(); - - ProcessHelper.RunOnSTAThread(GetImpl); - - return pids; - - void GetImpl() + var childProcessIds = new List(); + foreach (var childProcessId in processTree.GetChildIds(process.Id, childProcessImageName)) { - var queryString = $"SELECT ProcessId FROM Win32_Process WHERE ParentProcessId = {process.Id} AND SessionId = {AppEnvironment.SessionId}"; - - if (childProcessImageName is not null) - queryString += $" AND Name = '{childProcessImageName}'"; - - using var searcher = new ManagementObjectSearcher(queryString); - using var collection = searcher.Get(); + if (!Process.TryGetById(childProcessId, sessionId, out var childProcess)) + continue; - foreach (var @object in collection) - { - if (@object is not null) - { - var processId = (int)(uint)@object.GetPropertyValue("ProcessId"); - pids.Add(processId); - } - } + childProcess.Dispose(); + childProcessIds.Add(childProcessId); } + + return childProcessIds; } extension(Process) @@ -126,40 +106,17 @@ public static bool TryGetById(int processId, int sessionId, [NotNullWhen(true)] /// public Process? GetParentProcess() { - // ManagementObjectSearcher.Get() raises a System.InvalidCastException when executed on the current thread, this regardless of the apartment state of the current thread (which is STA) - // - // System.InvalidCastException "Specified cast is not valid." - // at System.StubHelpers.InterfaceMarshaler.ConvertToNative(Object objSrc, IntPtr itfMT, IntPtr classMT, Int32 flags) - // at System.Management.SecuredIWbemServicesHandler.ExecQuery_(String strQueryLanguage, String strQuery, Int32 lFlags, IWbemContext pCtx, IEnumWbemClassObject& ppEnum) - // at System.Management.ManagementObjectSearcher.Get() - - var processId = process.Id; var sessionId = process.SessionId; - var parentProcessId = (int?)null; - - ProcessHelper.RunOnSTAThread(GetImpl); + var processTree = ProcessTree.Capture(); - if (parentProcessId is not int id) + var parentProcessId = processTree.GetParentId(process.Id); + if (parentProcessId is null) return null; - if (!Process.TryGetById(id, sessionId, out var parentProcess)) + if (!Process.TryGetById(parentProcessId.Value, sessionId, out var parentProcess)) return null; return parentProcess; - - void GetImpl() - { - var queryString = $"SELECT ParentProcessId FROM Win32_Process WHERE ProcessId = {processId} AND SessionId = {sessionId}"; - - using var searcher = new ManagementObjectSearcher(queryString); - using var collection = searcher.Get(); - using var @object = collection.OfType().SingleOrDefault(); - - if (@object is not null) - { - parentProcessId = (int)(uint)@object["ParentProcessId"]; - } - } } } diff --git a/src/Infrastructure/Services/PowerBI/PBIDesktopService.cs b/src/Infrastructure/Services/PowerBI/PBIDesktopService.cs index 291f46f2..2a6612a6 100644 --- a/src/Infrastructure/Services/PowerBI/PBIDesktopService.cs +++ b/src/Infrastructure/Services/PowerBI/PBIDesktopService.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using Sqlbi.Bravo.Infrastructure.Extensions; using Sqlbi.Bravo.Infrastructure.Helpers; +using Sqlbi.Bravo.Infrastructure.Windows; using Sqlbi.Bravo.Models; namespace Sqlbi.Bravo.Infrastructure.Services.PowerBI; @@ -21,11 +22,12 @@ public IEnumerable GetReports(CancellationToken cancellationTo var processes = ProcessHelper.GetProcessesByName(PowerBIDesktop.ProcessName); try { + var processTree = ProcessTree.Capture(); var reports = new ConcurrentBag(); var parallelOptions = new ParallelOptions { CancellationToken = cancellationToken }; var parallelLoop = Parallel.ForEach(processes, parallelOptions, (process) => { - var report = PBIDesktopReport.CreateFrom(process); + var report = PBIDesktopReport.CreateFrom(process, processTree); reports.Add(report); }); diff --git a/src/Infrastructure/Windows/Interop/ProcessInterop.cs b/src/Infrastructure/Windows/Interop/ProcessInterop.cs new file mode 100644 index 00000000..2cc1eefe --- /dev/null +++ b/src/Infrastructure/Windows/Interop/ProcessInterop.cs @@ -0,0 +1,44 @@ +using System.Collections.Generic; +using System.ComponentModel; +using System.Runtime.InteropServices; +using Windows.Win32; +using Windows.Win32.Foundation; +using Windows.Win32.System.Diagnostics.ToolHelp; + +namespace Sqlbi.Bravo.Infrastructure.Windows.Interop; + +internal static class ProcessInterop +{ + /// + /// Returns the processes running now, each with the identifier of the process that created it. + /// + /// The processes cannot be read. + public static IReadOnlyList GetProcesses() + { + using var snapshot = PInvoke.CreateToolhelp32Snapshot_SafeHandle(CREATE_TOOLHELP_SNAPSHOT_FLAGS.TH32CS_SNAPPROCESS, th32ProcessID: 0); + if (snapshot.IsInvalid) + throw new Win32Exception(); + + var entry = new PROCESSENTRY32W + { + dwSize = (uint)Marshal.SizeOf(), + }; + + if (!PInvoke.Process32FirstW(snapshot, ref entry)) + throw new Win32Exception(); + + var processes = new List(); + do + { + var process = new ProcessEntry((int)entry.th32ProcessID, (int)entry.th32ParentProcessID, entry.szExeFile.ToString()); + processes.Add(process); + } + while (PInvoke.Process32NextW(snapshot, ref entry)); + + var error = (WIN32_ERROR)Marshal.GetLastPInvokeError(); + if (error != WIN32_ERROR.ERROR_NO_MORE_FILES) + throw new Win32Exception((int)error); + + return processes; + } +} diff --git a/src/Infrastructure/Windows/ProcessTree.cs b/src/Infrastructure/Windows/ProcessTree.cs new file mode 100644 index 00000000..d55fd96a --- /dev/null +++ b/src/Infrastructure/Windows/ProcessTree.cs @@ -0,0 +1,66 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; +using Sqlbi.Bravo.Infrastructure.Windows.Interop; + +namespace Sqlbi.Bravo.Infrastructure.Windows; + +internal readonly record struct ProcessEntry(int ProcessId, int ParentProcessId, string ImageName); + +/// +/// The processes that were running at one moment, each with the identifier of the process that created it. +/// +/// +/// A parent identifier is recorded when the process is created and is not updated: once the parent exits, the +/// identifier can belong to an unrelated process. +/// +internal sealed class ProcessTree +{ + private readonly Dictionary _entries = []; + + public ProcessTree(IEnumerable entries) + { + ArgumentNullException.ThrowIfNull(entries); + + foreach (var entry in entries) + _ = _entries.TryAdd(entry.ProcessId, entry); + } + + /// + /// Takes a snapshot of the processes running now. + /// + /// The processes cannot be read. + public static ProcessTree Capture() + { + var entries = ProcessInterop.GetProcesses(); + + return new ProcessTree(entries); + } + + /// + /// Returns the identifier of the process that created ; when the + /// process is not in the snapshot. + /// + public int? GetParentId(int processId) + { + if (!_entries.TryGetValue(processId, out var entry)) + return null; + + return entry.ParentProcessId; + } + + /// + /// Returns the processes created by a process, optionally only those with the given image name. + /// + /// The identifier of the parent process. + /// The executable file name, such as msmdsrv.exe, compared without case. + public IReadOnlyList GetChildIds(int processId, string? imageName = null) + { + return _entries.Values + .Where((entry) => entry.ParentProcessId == processId && entry.ProcessId != processId) + .Where((entry) => imageName is null || string.Equals(entry.ImageName, imageName, StringComparison.OrdinalIgnoreCase)) + .Select((entry) => entry.ProcessId) + .ToArray(); + } +} diff --git a/src/Models/PBIDesktopReport.cs b/src/Models/PBIDesktopReport.cs index cc3e0b48..5d8b3309 100644 --- a/src/Models/PBIDesktopReport.cs +++ b/src/Models/PBIDesktopReport.cs @@ -10,6 +10,7 @@ using Sqlbi.Bravo.Infrastructure.Helpers; using Sqlbi.Bravo.Infrastructure.Models; using Sqlbi.Bravo.Infrastructure.Services.PowerBI; +using Sqlbi.Bravo.Infrastructure.Windows; using SSAS = Microsoft.AnalysisServices; using TOM = Microsoft.AnalysisServices.Tabular; @@ -65,14 +66,19 @@ public override int GetHashCode() if (process is not null) { - var report = CreateFrom(process, connectionModeEnabled); + var report = CreateFrom(process, processTree: null, connectionModeEnabled); return report; } return null; } - internal static PBIDesktopReport CreateFrom(Process process, bool connectionModeEnabled = true) + /// The Power BI Desktop process. + /// + /// A snapshot shared by the reports created together; takes one when it is needed. + /// + /// Whether to connect to the model to find its connection mode. + internal static PBIDesktopReport CreateFrom(Process process, ProcessTree? processTree = null, bool connectionModeEnabled = true) { var report = new PBIDesktopReport { @@ -108,8 +114,9 @@ void GetConnectionMode(out string? serverName, out string? databaseName, out SSA databaseName = null; compatibilityMode = SSAS.CompatibilityMode.Unknown; - var ssasPIDs = process.GetChildrenPIDs(childProcessImageName: PowerBIDesktop.AnalysisServicesImageName).ToArray(); - if (ssasPIDs.Length != 1) + var tree = processTree ?? ProcessTree.Capture(); + var ssasPIDs = process.GetChildrenPIDs(tree, childProcessImageName: PowerBIDesktop.AnalysisServicesImageName); + if (ssasPIDs.Count != 1) { connectionMode = PBIDesktopReportConnectionMode.UnsupportedAnalysisServicesProcessNotFound; return; diff --git a/src/NativeMethods.txt b/src/NativeMethods.txt index 0b3ca252..efbd74b9 100644 --- a/src/NativeMethods.txt +++ b/src/NativeMethods.txt @@ -1,2 +1,10 @@ +// Windows.Win32.Foundation +WIN32_ERROR + +// Windows.Win32.System.Diagnostics.ToolHelp +CreateToolhelp32Snapshot +Process32FirstW +Process32NextW + // Windows.Win32.System.Pipes DisconnectNamedPipe diff --git a/src/packages.lock.json b/src/packages.lock.json index cdbb7785..5d132fa8 100644 --- a/src/packages.lock.json +++ b/src/packages.lock.json @@ -147,12 +147,6 @@ "resolved": "2.0.0-beta4.22272.1", "contentHash": "1uqED/q2H0kKoLJ4+hI2iPSBSEdTuhfCYADeJrAqERmiGQ2NNacYKRNEQ+gFbU4glgVyK8rxI+ZOe1onEtr/Pg==" }, - "System.Management": { - "type": "Direct", - "requested": "[7.0.0, )", - "resolved": "7.0.0", - "contentHash": "A4jed4QUviDOm7fJNKAJObEAEkEUXmkGL/w0iyCYTzrl1rezTj8LGFHfsVst4Vb9JwFcTpboiDrvdST48avBpw==" - }, "Antlr4.Runtime.Standard": { "type": "Transitive", "resolved": "4.13.1", @@ -295,12 +289,6 @@ "resolved": "1.0.2739.15", "contentHash": "hZg7vHWBg4RIiK3rhkNqX3hcBqqoVXVrrO2BVrVOiSmEgfLa7lY64G1tWh2VY84MfHIFLCAQNwn3Rwvu+ICHyw==" }, - "System.Management": { - "type": "Direct", - "requested": "[7.0.0, )", - "resolved": "7.0.0", - "contentHash": "A4jed4QUviDOm7fJNKAJObEAEkEUXmkGL/w0iyCYTzrl1rezTj8LGFHfsVst4Vb9JwFcTpboiDrvdST48avBpw==" - }, "Microsoft.AnalysisServices.AdomdClient": { "type": "Transitive", "resolved": "19.104.1", diff --git a/test/Bravo.Tests/Infrastructure/Windows/ProcessTreeTests.cs b/test/Bravo.Tests/Infrastructure/Windows/ProcessTreeTests.cs new file mode 100644 index 00000000..63ae2a99 --- /dev/null +++ b/test/Bravo.Tests/Infrastructure/Windows/ProcessTreeTests.cs @@ -0,0 +1,76 @@ +using System; +using Sqlbi.Bravo.Infrastructure.Windows; +using Xunit; + +namespace Bravo.Tests.Infrastructure.Windows; + +public class ProcessTreeTests +{ + private static readonly ProcessTree s_tree = new( + [ + new ProcessEntry(ProcessId: 0, ParentProcessId: 0, "[System Process]"), + new ProcessEntry(ProcessId: 100, ParentProcessId: 10, "PBIDesktop.exe"), + new ProcessEntry(ProcessId: 200, ParentProcessId: 100, "msmdsrv.exe"), + new ProcessEntry(ProcessId: 300, ParentProcessId: 100, "Bravo.exe"), + new ProcessEntry(ProcessId: 400, ParentProcessId: 300, "msedgewebview2.exe"), + ]); + + [Fact] + public void GetParentId_ProcessInTheSnapshot_ReturnsItsParent() + { + Assert.Equal(100, s_tree.GetParentId(300)); + } + + [Fact] + public void GetParentId_ParentNotInTheSnapshot_ReturnsTheRecordedId() + { + // The parent exited: its identifier is still recorded on the child. + Assert.Equal(10, s_tree.GetParentId(100)); + } + + [Fact] + public void GetParentId_ProcessNotInTheSnapshot_ReturnsNull() + { + Assert.Null(s_tree.GetParentId(999)); + } + + [Fact] + public void GetChildIds_WithoutImageName_ReturnsEveryChild() + { + Assert.Equal([200, 300], s_tree.GetChildIds(100)); + } + + [Theory] + [InlineData("msmdsrv.exe")] + [InlineData("MSMDSRV.EXE")] + public void GetChildIds_WithImageName_ReturnsTheMatchingChildren(string imageName) + { + Assert.Equal([200], s_tree.GetChildIds(100, imageName)); + } + + [Fact] + public void GetChildIds_ProcessThatIsItsOwnParent_IsNotItsOwnChild() + { + Assert.Empty(s_tree.GetChildIds(0)); + } + + [Fact] + public void Constructor_DuplicateProcessId_KeepsTheFirstEntry() + { + var tree = new ProcessTree( + [ + new ProcessEntry(ProcessId: 1, ParentProcessId: 2, "a.exe"), + new ProcessEntry(ProcessId: 1, ParentProcessId: 3, "b.exe"), + ]); + + Assert.Equal(2, tree.GetParentId(1)); + } + + [Fact] + public void Capture_ContainsTheCurrentProcess() + { + var tree = ProcessTree.Capture(); + + Assert.NotNull(tree.GetParentId(Environment.ProcessId)); + } +} From 01a012ba661dfffc179af9c1c795a960841452ed Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Tue, 20 Oct 2026 16:55:37 +0200 Subject: [PATCH 7/9] fix: ignore processes whose identifier was reused A process records the identifier of its parent when it is created, and Windows reuses identifiers once a process exits. GetParentProcess and GetChildrenPIDs could therefore return an unrelated process that took the identifier of an exited parent or child. A parent must now have started before its child; a process whose start time cannot be read is treated as not found. --- .../Extensions/ProcessExtensions.cs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/Infrastructure/Extensions/ProcessExtensions.cs b/src/Infrastructure/Extensions/ProcessExtensions.cs index b1fb815d..5d5c7ff3 100644 --- a/src/Infrastructure/Extensions/ProcessExtensions.cs +++ b/src/Infrastructure/Extensions/ProcessExtensions.cs @@ -52,7 +52,12 @@ public static IReadOnlyList GetChildrenPIDs(this Process process, ProcessTr if (!Process.TryGetById(childProcessId, sessionId, out var childProcess)) continue; + var isChild = IsStartedBefore(process, childProcess); childProcess.Dispose(); + + if (!isChild) + continue; + childProcessIds.Add(childProcessId); } @@ -116,10 +121,35 @@ public static bool TryGetById(int processId, int sessionId, [NotNullWhen(true)] if (!Process.TryGetById(parentProcessId.Value, sessionId, out var parentProcess)) return null; + if (!IsStartedBefore(parentProcess, process)) + { + parentProcess.Dispose(); + return null; + } + return parentProcess; } } + // A process identifier is reused once its process exits, so the recorded parent or child can be an unrelated + // process: a real parent always started before its child. + private static bool IsStartedBefore(Process earlier, Process later) + { + try + { + // StartTime is local time, which repeats an hour when daylight saving time ends: only UTC keeps the order. + var earlierStartTime = earlier.StartTime.ToUniversalTime(); + var laterStartTime = later.StartTime.ToUniversalTime(); + + return earlierStartTime <= laterStartTime; + } + catch (Exception ex) when (ex is Win32Exception or InvalidOperationException) + { + // The start time of a process that has exited, or that cannot be opened, is unknown. + return false; + } + } + public static string GetMainWindowTitle(this Process process) { if (process.MainWindowTitle.Length > 0) From b0a9ce9447fe2d8f5fcd2a45ef0ce58dbc560bbd Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Tue, 20 Oct 2026 16:56:16 +0200 Subject: [PATCH 8/9] refactor: rename GetChildrenPIDs to GetChildProcessIds Aligns the name with the .NET naming guidelines for acronyms and compound words. No behavior change. --- src/Infrastructure/Extensions/ProcessExtensions.cs | 2 +- src/Models/PBIDesktopReport.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Infrastructure/Extensions/ProcessExtensions.cs b/src/Infrastructure/Extensions/ProcessExtensions.cs index 5d5c7ff3..2f71838c 100644 --- a/src/Infrastructure/Extensions/ProcessExtensions.cs +++ b/src/Infrastructure/Extensions/ProcessExtensions.cs @@ -42,7 +42,7 @@ internal static class ProcessExtensions return null; } - public static IReadOnlyList GetChildrenPIDs(this Process process, ProcessTree processTree, string? childProcessImageName = null) + public static IReadOnlyList GetChildProcessIds(this Process process, ProcessTree processTree, string? childProcessImageName = null) { var sessionId = process.SessionId; diff --git a/src/Models/PBIDesktopReport.cs b/src/Models/PBIDesktopReport.cs index 5d8b3309..df9abbb9 100644 --- a/src/Models/PBIDesktopReport.cs +++ b/src/Models/PBIDesktopReport.cs @@ -115,7 +115,7 @@ void GetConnectionMode(out string? serverName, out string? databaseName, out SSA compatibilityMode = SSAS.CompatibilityMode.Unknown; var tree = processTree ?? ProcessTree.Capture(); - var ssasPIDs = process.GetChildrenPIDs(tree, childProcessImageName: PowerBIDesktop.AnalysisServicesImageName); + var ssasPIDs = process.GetChildProcessIds(tree, childProcessImageName: PowerBIDesktop.AnalysisServicesImageName); if (ssasPIDs.Count != 1) { connectionMode = PBIDesktopReportConnectionMode.UnsupportedAnalysisServicesProcessNotFound; From 033ce4e62811539c4317a7a6f43b7cfefbc4a93d Mon Sep 17 00:00:00 2001 From: Alberto Spelta Date: Tue, 20 Oct 2026 17:06:17 +0200 Subject: [PATCH 9/9] refactor: remove the parent process lookups left without callers InteropGetParent, the NtQueryInformationProcess lookup kept as obsolete, ProcessHelper.GetParentProcess and GetParentProcessMainWindowHandle have no callers once process.GetParentProcess reads the Toolhelp32 snapshot, and AppEnvironment.ProcessId was read only by the WMI query. Ntdll goes with InteropGetParent, its only user. --- src/Infrastructure/AppEnvironment.cs | 3 -- .../Extensions/ProcessExtensions.cs | 31 -------------- src/Infrastructure/Helpers/ProcessHelper.cs | 17 -------- src/Infrastructure/Windows/Interop/Ntdll.cs | 40 ------------------- 4 files changed, 91 deletions(-) delete mode 100644 src/Infrastructure/Windows/Interop/Ntdll.cs diff --git a/src/Infrastructure/AppEnvironment.cs b/src/Infrastructure/AppEnvironment.cs index 0798643c..b35f183e 100644 --- a/src/Infrastructure/AppEnvironment.cs +++ b/src/Infrastructure/AppEnvironment.cs @@ -57,7 +57,6 @@ static AppEnvironment() _deploymentMode = new(() => GetDeploymentMode()); using var currentProcess = Process.GetCurrentProcess(); - ProcessId = Environment.ProcessId; SessionId = currentProcess.SessionId; ApplicationDataPath = Path.Combine(Environment.GetFolderPath(DeploymentMode == AppDeploymentMode.Packaged ? Environment.SpecialFolder.UserProfile : Environment.SpecialFolder.LocalApplicationData, Environment.SpecialFolderOption.DoNotVerify), ApplicationName); @@ -77,8 +76,6 @@ static AppEnvironment() /// public static int SessionId { get; } - public static int ProcessId { get; } - public static AppPublishMode PublishMode { get diff --git a/src/Infrastructure/Extensions/ProcessExtensions.cs b/src/Infrastructure/Extensions/ProcessExtensions.cs index 2f71838c..2229db1f 100644 --- a/src/Infrastructure/Extensions/ProcessExtensions.cs +++ b/src/Infrastructure/Extensions/ProcessExtensions.cs @@ -3,9 +3,7 @@ using System.ComponentModel; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.Runtime.InteropServices; using System.Text; -using Sqlbi.Bravo.Infrastructure.Helpers; using Sqlbi.Bravo.Infrastructure.Windows; using Sqlbi.Bravo.Infrastructure.Windows.Interop; @@ -13,35 +11,6 @@ namespace Sqlbi.Bravo.Infrastructure.Extensions; internal static class ProcessExtensions { - //[DebuggerStepThrough] - [Obsolete("Use GetParentProcess")] - public static Process? InteropGetParent(this Process process) - { - Ntdll.PROCESS_BASIC_INFORMATION processInformation; - int? retval; - try - { - retval = Ntdll.NtQueryInformationProcess(process.Handle, Ntdll.PROCESSINFOCLASS.ProcessBasicInformation, out processInformation, processInformationLength: (uint)Marshal.SizeOf(typeof(Ntdll.PROCESS_BASIC_INFORMATION)), returnLength: out _); - } - catch (Win32Exception ex) when (ex.ErrorCode == -2147467259) // System.ComponentModel.Win32Exception {"Access is denied."} - { - return null; - } - - if (retval.Value == (int)Ntdll.NTSTATUS.STATUS_SUCCESS) - { - var parentProcessId = (int)(uint)processInformation.InheritedFromUniqueProcessId; - var parentProcess = ProcessHelper.SafeGetProcessById(parentProcessId); - - return parentProcess; - } - - // TODO: How NTSTATUS codes are translated into Win32 errors ? - // throw new Win32Exception(retval); - - return null; - } - public static IReadOnlyList GetChildProcessIds(this Process process, ProcessTree processTree, string? childProcessImageName = null) { var sessionId = process.SessionId; diff --git a/src/Infrastructure/Helpers/ProcessHelper.cs b/src/Infrastructure/Helpers/ProcessHelper.cs index 72d50766..1208b336 100644 --- a/src/Infrastructure/Helpers/ProcessHelper.cs +++ b/src/Infrastructure/Helpers/ProcessHelper.cs @@ -280,13 +280,6 @@ public static IReadOnlyList GetProcessesByName(string processName) return processes; } - public static Process? GetParentProcess() - { - using var currentProcess = Process.GetCurrentProcess(); - - return currentProcess.GetParentProcess(); - } - public static IntPtr GetCurrentProcessMainWindowHandle() { using var current = Process.GetCurrentProcess(); @@ -294,16 +287,6 @@ public static IntPtr GetCurrentProcessMainWindowHandle() return current.MainWindowHandle; } - public static IntPtr GetParentProcessMainWindowHandle() - { - using var parent = GetParentProcess(); - - if (parent is not null) - return parent.MainWindowHandle; - - return IntPtr.Zero; - } - public static Process? SafeGetProcessById(int? processId) { if (processId is null) diff --git a/src/Infrastructure/Windows/Interop/Ntdll.cs b/src/Infrastructure/Windows/Interop/Ntdll.cs deleted file mode 100644 index 5091283f..00000000 --- a/src/Infrastructure/Windows/Interop/Ntdll.cs +++ /dev/null @@ -1,40 +0,0 @@ -using System; -using System.Runtime.InteropServices; - -namespace Sqlbi.Bravo.Infrastructure.Windows.Interop; - -internal static class Ntdll -{ - internal enum NTSTATUS - { - STATUS_SUCCESS = 0 - } - - internal enum PROCESSINFOCLASS - { - ProcessBasicInformation = 0, - //ProcessDebugPort = 7, - //ProcessWow64Information = 26, - //ProcessImageFileName = 27, - //ProcessBreakOnTermination = 29, - //ProcessSubsystemInformation = 75 - } - - internal struct PROCESS_BASIC_INFORMATION - { - public uint ExitStatus; - - public IntPtr PebBaseAddress; - - public UIntPtr AffinityMask; - - public int BasePriority; - - public UIntPtr UniqueProcessId; - - public UIntPtr InheritedFromUniqueProcessId; - } - - [DllImport(ExternDll.Ntdll, ExactSpelling = true, SetLastError = true)] - internal static extern int NtQueryInformationProcess(IntPtr processHandle, PROCESSINFOCLASS processInformationClass, out PROCESS_BASIC_INFORMATION processInformation, uint processInformationLength, out int returnLength); -}