From 522458420df786eb653009d616127e1060b3ab9b Mon Sep 17 00:00:00 2001 From: Sean Hall Date: Sat, 1 Aug 2020 11:49:05 -0600 Subject: WIXFEAT:5319 Remove IVariables since they were a leaky abstraction. --- src/WixToolset.Mba.Core/Engine.cs | 290 ++++++++++++---------------------- src/WixToolset.Mba.Core/IEngine.cs | 95 +++++++---- src/WixToolset.Mba.Core/IVariables.cs | 27 ---- src/balutil/balutil.cpp | 4 +- src/balutil/inc/balutil.h | 4 +- 5 files changed, 171 insertions(+), 249 deletions(-) delete mode 100644 src/WixToolset.Mba.Core/IVariables.cs diff --git a/src/WixToolset.Mba.Core/Engine.cs b/src/WixToolset.Mba.Core/Engine.cs index 2c544f29..98427cfa 100644 --- a/src/WixToolset.Mba.Core/Engine.cs +++ b/src/WixToolset.Mba.Core/Engine.cs @@ -18,10 +18,6 @@ namespace WixToolset.Mba.Core private static readonly string normalizeVersionFormatString = "{0} must be less than or equal to " + UInt16.MaxValue; private IBootstrapperEngine engine; - private Variables numericVariables; - private Variables secureStringVariables; - private Variables stringVariables; - private Variables versionVariables; /// /// Creates a new instance of the container class. @@ -30,135 +26,6 @@ namespace WixToolset.Mba.Core internal Engine(IBootstrapperEngine engine) { this.engine = engine; - - // Wrap the calls to get and set numeric variables. - this.numericVariables = new Variables( - delegate(string name) - { - long value; - int ret = this.engine.GetVariableNumeric(name, out value); - if (NativeMethods.S_OK != ret) - { - throw new Win32Exception(ret); - } - - return value; - }, - delegate(string name, long value) - { - this.engine.SetVariableNumeric(name, value); - }, - delegate(string name) - { - long value; - int ret = this.engine.GetVariableNumeric(name, out value); - - return NativeMethods.E_NOTFOUND != ret; - } - ); - - // Wrap the calls to get and set string variables using SecureStrings. - this.secureStringVariables = new Variables( - delegate(string name) - { - var pUniString = this.getStringVariable(name, out var length); - try - { - return this.convertToSecureString(pUniString, length); - } - finally - { - if (IntPtr.Zero != pUniString) - { - Marshal.FreeCoTaskMem(pUniString); - } - } - }, - delegate(string name, SecureString value) - { - IntPtr pValue = Marshal.SecureStringToCoTaskMemUnicode(value); - try - { - this.engine.SetVariableString(name, pValue, true); - } - finally - { - Marshal.FreeCoTaskMem(pValue); - } - }, - delegate(string name) - { - return this.containsVariable(name); - } - ); - - // Wrap the calls to get and set string variables. - this.stringVariables = new Variables( - delegate(string name) - { - int length; - IntPtr pUniString = this.getStringVariable(name, out length); - try - { - return Marshal.PtrToStringUni(pUniString, length); - } - finally - { - if (IntPtr.Zero != pUniString) - { - Marshal.FreeCoTaskMem(pUniString); - } - } - }, - delegate(string name, string value) - { - IntPtr pValue = Marshal.StringToCoTaskMemUni(value); - try - { - this.engine.SetVariableString(name, pValue, true); - } - finally - { - Marshal.FreeCoTaskMem(pValue); - } - }, - delegate(string name) - { - return this.containsVariable(name); - } - ); - - // Wrap the calls to get and set version variables. - this.versionVariables = new Variables( - delegate(string name) - { - long value; - int ret = this.engine.GetVariableVersion(name, out value); - if (NativeMethods.S_OK != ret) - { - throw new Win32Exception(ret); - } - - return LongToVersion(value); - }, - delegate(string name, Version value) - { - long version = VersionToLong(value); - this.engine.SetVariableVersion(name, version); - }, - delegate(string name) - { - long value; - int ret = this.engine.GetVariableVersion(name, out value); - - return NativeMethods.E_NOTFOUND != ret; - } - ); - } - - public IVariables NumericVariables - { - get { return this.numericVariables; } } public int PackageCount @@ -172,21 +39,6 @@ namespace WixToolset.Mba.Core } } - public IVariables SecureStringVariables - { - get { return this.secureStringVariables; } - } - - public IVariables StringVariables - { - get { return this.stringVariables; } - } - - public IVariables VersionVariables - { - get { return this.versionVariables; } - } - public void Apply(IntPtr hwndParent) { this.engine.Apply(hwndParent); @@ -197,6 +49,13 @@ namespace WixToolset.Mba.Core this.engine.CloseSplashScreen(); } + public bool ContainsVariable(string name) + { + int capacity = 0; + int ret = this.engine.GetVariableString(name, IntPtr.Zero, ref capacity); + return NativeMethods.E_NOTFOUND != ret; + } + public void Detect() { this.Detect(IntPtr.Zero); @@ -275,6 +134,61 @@ namespace WixToolset.Mba.Core return sb.ToString(); } + public long GetVariableNumeric(string name) + { + int ret = this.engine.GetVariableNumeric(name, out long value); + if (NativeMethods.S_OK != ret) + { + throw new Win32Exception(ret); + } + + return value; + } + + public SecureString GetVariableSecureString(string name) + { + var pUniString = this.getStringVariable(name, out var length); + try + { + return this.convertToSecureString(pUniString, length); + } + finally + { + if (IntPtr.Zero != pUniString) + { + Marshal.FreeCoTaskMem(pUniString); + } + } + } + + public string GetVariableString(string name) + { + int length; + IntPtr pUniString = this.getStringVariable(name, out length); + try + { + return Marshal.PtrToStringUni(pUniString, length); + } + finally + { + if (IntPtr.Zero != pUniString) + { + Marshal.FreeCoTaskMem(pUniString); + } + } + } + + public Version GetVariableVersion(string name) + { + int ret = this.engine.GetVariableVersion(name, out long value); + if (NativeMethods.S_OK != ret) + { + throw new Win32Exception(ret); + } + + return LongToVersion(value); + } + public void LaunchApprovedExe(IntPtr hwndParent, string approvedExeForElevationId, string arguments) { this.LaunchApprovedExe(hwndParent, approvedExeForElevationId, arguments, 0); @@ -310,6 +224,43 @@ namespace WixToolset.Mba.Core this.engine.SetDownloadSource(packageOrContainerId, payloadId, url, user, password); } + public void SetVariable(string name, long value) + { + this.engine.SetVariableNumeric(name, value); + } + + public void SetVariable(string name, SecureString value, bool formatted) + { + IntPtr pValue = Marshal.SecureStringToCoTaskMemUnicode(value); + try + { + this.engine.SetVariableString(name, pValue, formatted); + } + finally + { + Marshal.FreeCoTaskMem(pValue); + } + } + + public void SetVariable(string name, string value, bool formatted) + { + IntPtr pValue = Marshal.StringToCoTaskMemUni(value); + try + { + this.engine.SetVariableString(name, pValue, formatted); + } + finally + { + Marshal.FreeCoTaskMem(pValue); + } + } + + public void SetVariable(string name, Version value) + { + long version = VersionToLong(value); + this.engine.SetVariableVersion(name, version); + } + public int SendEmbeddedError(int errorCode, string message, int uiHint) { int result = 0; @@ -329,49 +280,6 @@ namespace WixToolset.Mba.Core this.engine.Quit(exitCode); } - internal sealed class Variables : IVariables - { - // .NET 2.0 does not support Func or Action. - internal delegate T Getter(string name); - internal delegate void Setter(string name, T value); - - private Getter getter; - private Setter setter; - private Predicate contains; - - internal Variables(Getter getter, Setter setter, Predicate contains) - { - this.getter = getter; - this.setter = setter; - this.contains = contains; - } - - public T this[string name] - { - get { return this.getter(name); } - set { this.setter(name, value); } - } - - public bool Contains(string name) - { - return this.contains(name); - } - } - - /// - /// Gets whether the variable given by exists. - /// - /// The name of the variable to check. - /// True if the variable given by exists; otherwise, false. - internal bool containsVariable(string name) - { - int capacity = 0; - IntPtr pValue = IntPtr.Zero; - int ret = this.engine.GetVariableString(name, pValue, ref capacity); - - return NativeMethods.E_NOTFOUND != ret; - } - /// /// Gets the variable given by as a string. /// diff --git a/src/WixToolset.Mba.Core/IEngine.cs b/src/WixToolset.Mba.Core/IEngine.cs index 5ddc4176..84d93bb0 100644 --- a/src/WixToolset.Mba.Core/IEngine.cs +++ b/src/WixToolset.Mba.Core/IEngine.cs @@ -8,38 +8,11 @@ namespace WixToolset.Mba.Core public interface IEngine { - /// - /// Gets or sets numeric variables for the engine. - /// - IVariables NumericVariables { get; } - /// /// Gets the number of packages in the bundle. /// int PackageCount { get; } - /// - /// Gets or sets string variables for the engine using SecureStrings. - /// - IVariables SecureStringVariables { get; } - - /// - /// Gets or sets string variables for the engine. - /// - IVariables StringVariables { get; } - - /// - /// Gets or sets variables for the engine. - /// - /// The class can keep track of when the build and revision fields are undefined, but the engine can't. - /// Therefore, the build and revision fields must be defined when setting a variable. - /// Use the NormalizeVersion method to make sure the engine can accept the Version. - /// - /// To keep track of versions without build or revision fields, use StringVariables instead. - /// - /// The given was invalid. - IVariables VersionVariables { get; } - /// /// Install the packages. /// @@ -52,6 +25,13 @@ namespace WixToolset.Mba.Core /// void CloseSplashScreen(); + /// + /// Checks if a variable exists in the engine. + /// + /// The name of the variable. + /// Whether the variable exists. + bool ContainsVariable(string name); + /// /// Determine if all installation conditions are fulfilled. /// @@ -94,6 +74,30 @@ namespace WixToolset.Mba.Core /// A Win32 error occurred. string FormatString(string format); + /// + /// Gets numeric variables for the engine. + /// + /// The name of the variable. + long GetVariableNumeric(string name); + + /// + /// Gets string variables for the engine using SecureStrings. + /// + /// The name of the variable. + SecureString GetVariableSecureString(string name); + + /// + /// Gets string variables for the engine. + /// + /// The name of the variable. + string GetVariableString(string name); + + /// + /// Gets variables for the engine. + /// + /// The name of the variable. + Version GetVariableVersion(string name); + /// /// Launches a preapproved executable elevated. As long as the engine already elevated, there will be no UAC prompt. /// @@ -152,6 +156,43 @@ namespace WixToolset.Mba.Core /// The password for proxy authentication. void SetDownloadSource(string packageOrContainerId, string payloadId, string url, string user, string password); + /// + /// Sets numeric variables for the engine. + /// + /// The name of the variable. + /// The value to set. + void SetVariable(string name, long value); + + /// + /// Sets string variables for the engine using SecureStrings. + /// + /// The name of the variable. + /// The value to set. + /// False if the value is a literal string. + void SetVariable(string name, SecureString value, bool formatted); + + /// + /// Sets string variables for the engine. + /// + /// The name of the variable. + /// The value to set. + /// False if the value is a literal string. + void SetVariable(string name, string value, bool formatted); + + /// + /// Sets variables for the engine. + /// + /// The class can keep track of when the build and revision fields are undefined, but the engine can't. + /// Therefore, the build and revision fields must be defined when setting a variable. + /// Use the NormalizeVersion method to make sure the engine can accept the Version. + /// + /// To keep track of versions without build or revision fields, use StringVariables instead. + /// + /// The name of the variable. + /// The value to set. + /// The given was invalid. + void SetVariable(string name, Version value); + /// /// Sends error message when embedded. /// diff --git a/src/WixToolset.Mba.Core/IVariables.cs b/src/WixToolset.Mba.Core/IVariables.cs deleted file mode 100644 index c7994f0b..00000000 --- a/src/WixToolset.Mba.Core/IVariables.cs +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information. - -namespace WixToolset.Mba.Core -{ - using System; - - /// - /// An accessor for numeric, string, and version variables for the engine. - /// - public interface IVariables - { - /// - /// Gets or sets the variable given by . - /// - /// The name of the variable to get/set. - /// The value of the given variable. - /// An error occurred getting the variable. - T this[string name] { get; set; } - - /// - /// Gets whether the variable given by exists. - /// - /// The name of the variable to check. - /// True if the variable given by exists; otherwise, false. - bool Contains(string name); - } -} diff --git a/src/balutil/balutil.cpp b/src/balutil/balutil.cpp index ebfaede4..faca70f5 100644 --- a/src/balutil/balutil.cpp +++ b/src/balutil/balutil.cpp @@ -167,7 +167,7 @@ LExit: } -DAPI_(BOOL) BalStringVariableExists( +DAPI_(BOOL) BalVariableExists( __in_z LPCWSTR wzVariable ) { @@ -183,7 +183,7 @@ DAPI_(BOOL) BalStringVariableExists( hr = vpEngine->GetVariableString(wzVariable, NULL, &cch); LExit: - return E_MOREDATA == hr; // string exists only if there are more than zero characters in the variable. + return E_NOTFOUND != hr; } diff --git a/src/balutil/inc/balutil.h b/src/balutil/inc/balutil.h index b718e48b..affa4925 100644 --- a/src/balutil/inc/balutil.h +++ b/src/balutil/inc/balutil.h @@ -108,10 +108,10 @@ DAPI_(HRESULT) BalSetNumericVariable( ); /******************************************************************* -BalStringVariableExists - checks if a string variable exists in the engine. +BalVariableExists - checks if a variable exists in the engine. ********************************************************************/ -DAPI_(BOOL) BalStringVariableExists( +DAPI_(BOOL) BalVariableExists( __in_z LPCWSTR wzVariable ); -- cgit v1.2.3-55-g6feb