WIXFEAT:5319 Remove IVariables since they were a leaky abstraction.
Sean Hall committed
Aug 1, 2020 at 11:49 UTC
522458420df786eb653009d616127e1060b3ab9b
5 files changed
+171
-249
src/WixToolset.Mba.Core/Engine.cs
+99
-191
@@ -18,10 +18,6 @@ namespace WixToolset.Mba.Core
18
private static readonly string normalizeVersionFormatString = "{0} must be less than or equal to " + UInt16.MaxValue;
19
20
private IBootstrapperEngine engine;
21
- private Variables<long> numericVariables;
22
- private Variables<SecureString> secureStringVariables;
23
- private Variables<string> stringVariables;
24
- private Variables<Version> versionVariables;
21
22
/// <summary>
23
/// Creates a new instance of the <see cref="Engine"/> container class.
@@ -30,135 +26,6 @@ namespace WixToolset.Mba.Core
26
internal Engine(IBootstrapperEngine engine)
27
{
28
this.engine = engine;
33
-
34
- // Wrap the calls to get and set numeric variables.
35
- this.numericVariables = new Variables<long>(
36
- delegate(string name)
37
- {
38
- long value;
39
- int ret = this.engine.GetVariableNumeric(name, out value);
40
- if (NativeMethods.S_OK != ret)
41
- {
42
- throw new Win32Exception(ret);
43
- }
44
-
45
- return value;
46
- },
47
- delegate(string name, long value)
48
- {
49
- this.engine.SetVariableNumeric(name, value);
50
- },
51
- delegate(string name)
52
- {
53
- long value;
54
- int ret = this.engine.GetVariableNumeric(name, out value);
55
-
56
- return NativeMethods.E_NOTFOUND != ret;
57
- }
58
- );
59
-
60
- // Wrap the calls to get and set string variables using SecureStrings.
61
- this.secureStringVariables = new Variables<SecureString>(
62
- delegate(string name)
63
- {
64
- var pUniString = this.getStringVariable(name, out var length);
65
- try
66
- {
67
- return this.convertToSecureString(pUniString, length);
68
- }
69
- finally
70
- {
71
- if (IntPtr.Zero != pUniString)
72
- {
73
- Marshal.FreeCoTaskMem(pUniString);
74
- }
75
- }
76
- },
77
- delegate(string name, SecureString value)
78
- {
79
- IntPtr pValue = Marshal.SecureStringToCoTaskMemUnicode(value);
80
- try
81
- {
82
- this.engine.SetVariableString(name, pValue, true);
83
- }
84
- finally
85
- {
86
- Marshal.FreeCoTaskMem(pValue);
87
- }
88
- },
89
- delegate(string name)
90
- {
91
- return this.containsVariable(name);
92
- }
93
- );
94
-
95
- // Wrap the calls to get and set string variables.
96
- this.stringVariables = new Variables<string>(
97
- delegate(string name)
98
- {
99
- int length;
100
- IntPtr pUniString = this.getStringVariable(name, out length);
101
- try
102
- {
103
- return Marshal.PtrToStringUni(pUniString, length);
104
- }
105
- finally
106
- {
107
- if (IntPtr.Zero != pUniString)
108
- {
109
- Marshal.FreeCoTaskMem(pUniString);
110
- }
111
- }
112
- },
113
- delegate(string name, string value)
114
- {
115
- IntPtr pValue = Marshal.StringToCoTaskMemUni(value);
116
- try
117
- {
118
- this.engine.SetVariableString(name, pValue, true);
119
- }
120
- finally
121
- {
122
- Marshal.FreeCoTaskMem(pValue);
123
- }
124
- },
125
- delegate(string name)
126
- {
127
- return this.containsVariable(name);
128
- }
129
- );
130
-
131
- // Wrap the calls to get and set version variables.
132
- this.versionVariables = new Variables<Version>(
133
- delegate(string name)
134
- {
135
- long value;
136
- int ret = this.engine.GetVariableVersion(name, out value);
137
- if (NativeMethods.S_OK != ret)
138
- {
139
- throw new Win32Exception(ret);
140
- }
141
-
142
- return LongToVersion(value);
143
- },
144
- delegate(string name, Version value)
145
- {
146
- long version = VersionToLong(value);
147
- this.engine.SetVariableVersion(name, version);
148
- },
149
- delegate(string name)
150
- {
151
- long value;
152
- int ret = this.engine.GetVariableVersion(name, out value);
153
-
154
- return NativeMethods.E_NOTFOUND != ret;
155
- }
156
- );
157
- }
158
-
159
- public IVariables<long> NumericVariables
160
- {
161
- get { return this.numericVariables; }
29
}
30
31
public int PackageCount
@@ -172,21 +39,6 @@ namespace WixToolset.Mba.Core
39
}
40
}
41
175
- public IVariables<SecureString> SecureStringVariables
176
- {
177
- get { return this.secureStringVariables; }
178
- }
179
-
180
- public IVariables<string> StringVariables
181
- {
182
- get { return this.stringVariables; }
183
- }
184
-
185
- public IVariables<Version> VersionVariables
186
- {
187
- get { return this.versionVariables; }
188
- }
189
-
42
public void Apply(IntPtr hwndParent)
43
{
44
this.engine.Apply(hwndParent);
@@ -197,6 +49,13 @@ namespace WixToolset.Mba.Core
49
this.engine.CloseSplashScreen();
50
}
51
52
+ public bool ContainsVariable(string name)
53
+ {
54
+ int capacity = 0;
55
+ int ret = this.engine.GetVariableString(name, IntPtr.Zero, ref capacity);
56
+ return NativeMethods.E_NOTFOUND != ret;
57
+ }
58
+
59
public void Detect()
60
{
61
this.Detect(IntPtr.Zero);
@@ -275,6 +134,61 @@ namespace WixToolset.Mba.Core
134
return sb.ToString();
135
}
136
137
+ public long GetVariableNumeric(string name)
138
+ {
139
+ int ret = this.engine.GetVariableNumeric(name, out long value);
140
+ if (NativeMethods.S_OK != ret)
141
+ {
142
+ throw new Win32Exception(ret);
143
+ }
144
+
145
+ return value;
146
+ }
147
+
148
+ public SecureString GetVariableSecureString(string name)
149
+ {
150
+ var pUniString = this.getStringVariable(name, out var length);
151
+ try
152
+ {
153
+ return this.convertToSecureString(pUniString, length);
154
+ }
155
+ finally
156
+ {
157
+ if (IntPtr.Zero != pUniString)
158
+ {
159
+ Marshal.FreeCoTaskMem(pUniString);
160
+ }
161
+ }
162
+ }
163
+
164
+ public string GetVariableString(string name)
165
+ {
166
+ int length;
167
+ IntPtr pUniString = this.getStringVariable(name, out length);
168
+ try
169
+ {
170
+ return Marshal.PtrToStringUni(pUniString, length);
171
+ }
172
+ finally
173
+ {
174
+ if (IntPtr.Zero != pUniString)
175
+ {
176
+ Marshal.FreeCoTaskMem(pUniString);
177
+ }
178
+ }
179
+ }
180
+
181
+ public Version GetVariableVersion(string name)
182
+ {
183
+ int ret = this.engine.GetVariableVersion(name, out long value);
184
+ if (NativeMethods.S_OK != ret)
185
+ {
186
+ throw new Win32Exception(ret);
187
+ }
188
+
189
+ return LongToVersion(value);
190
+ }
191
+
192
public void LaunchApprovedExe(IntPtr hwndParent, string approvedExeForElevationId, string arguments)
193
{
194
this.LaunchApprovedExe(hwndParent, approvedExeForElevationId, arguments, 0);
@@ -310,6 +224,43 @@ namespace WixToolset.Mba.Core
224
this.engine.SetDownloadSource(packageOrContainerId, payloadId, url, user, password);
225
}
226
227
+ public void SetVariable(string name, long value)
228
+ {
229
+ this.engine.SetVariableNumeric(name, value);
230
+ }
231
+
232
+ public void SetVariable(string name, SecureString value, bool formatted)
233
+ {
234
+ IntPtr pValue = Marshal.SecureStringToCoTaskMemUnicode(value);
235
+ try
236
+ {
237
+ this.engine.SetVariableString(name, pValue, formatted);
238
+ }
239
+ finally
240
+ {
241
+ Marshal.FreeCoTaskMem(pValue);
242
+ }
243
+ }
244
+
245
+ public void SetVariable(string name, string value, bool formatted)
246
+ {
247
+ IntPtr pValue = Marshal.StringToCoTaskMemUni(value);
248
+ try
249
+ {
250
+ this.engine.SetVariableString(name, pValue, formatted);
251
+ }
252
+ finally
253
+ {
254
+ Marshal.FreeCoTaskMem(pValue);
255
+ }
256
+ }
257
+
258
+ public void SetVariable(string name, Version value)
259
+ {
260
+ long version = VersionToLong(value);
261
+ this.engine.SetVariableVersion(name, version);
262
+ }
263
+
264
public int SendEmbeddedError(int errorCode, string message, int uiHint)
265
{
266
int result = 0;
@@ -329,49 +280,6 @@ namespace WixToolset.Mba.Core
280
this.engine.Quit(exitCode);
281
}
282
332
- internal sealed class Variables<T> : IVariables<T>
333
- {
334
- // .NET 2.0 does not support Func<T, TResult> or Action<T1, T2>.
335
- internal delegate T Getter(string name);
336
- internal delegate void Setter(string name, T value);
337
-
338
- private Getter getter;
339
- private Setter setter;
340
- private Predicate<string> contains;
341
-
342
- internal Variables(Getter getter, Setter setter, Predicate<string> contains)
343
- {
344
- this.getter = getter;
345
- this.setter = setter;
346
- this.contains = contains;
347
- }
348
-
349
- public T this[string name]
350
- {
351
- get { return this.getter(name); }
352
- set { this.setter(name, value); }
353
- }
354
-
355
- public bool Contains(string name)
356
- {
357
- return this.contains(name);
358
- }
359
- }
360
-
361
- /// <summary>
362
- /// Gets whether the variable given by <paramref name="name"/> exists.
363
- /// </summary>
364
- /// <param name="name">The name of the variable to check.</param>
365
- /// <returns>True if the variable given by <paramref name="name"/> exists; otherwise, false.</returns>
366
- internal bool containsVariable(string name)
367
- {
368
- int capacity = 0;
369
- IntPtr pValue = IntPtr.Zero;
370
- int ret = this.engine.GetVariableString(name, pValue, ref capacity);
371
-
372
- return NativeMethods.E_NOTFOUND != ret;
373
- }
374
-
283
/// <summary>
284
/// Gets the variable given by <paramref name="name"/> as a string.
285
/// </summary>
src/WixToolset.Mba.Core/IEngine.cs
+68
-27
@@ -8,38 +8,11 @@ namespace WixToolset.Mba.Core
8
9
public interface IEngine
10
{
11
- /// <summary>
12
- /// Gets or sets numeric variables for the engine.
13
- /// </summary>
14
- IVariables<long> NumericVariables { get; }
15
-
11
/// <summary>
12
/// Gets the number of packages in the bundle.
13
/// </summary>
14
int PackageCount { get; }
15
21
- /// <summary>
22
- /// Gets or sets string variables for the engine using SecureStrings.
23
- /// </summary>
24
- IVariables<SecureString> SecureStringVariables { get; }
25
-
26
- /// <summary>
27
- /// Gets or sets string variables for the engine.
28
- /// </summary>
29
- IVariables<string> StringVariables { get; }
30
-
31
- /// <summary>
32
- /// Gets or sets <see cref="Version"/> variables for the engine.
33
- ///
34
- /// The <see cref="Version"/> class can keep track of when the build and revision fields are undefined, but the engine can't.
35
- /// Therefore, the build and revision fields must be defined when setting a <see cref="Version"/> variable.
36
- /// Use the NormalizeVersion method to make sure the engine can accept the Version.
37
- ///
38
- /// To keep track of versions without build or revision fields, use StringVariables instead.
39
- /// </summary>
40
- /// <exception cref="OverflowException">The given <see cref="Version"/> was invalid.</exception>
41
- IVariables<Version> VersionVariables { get; }
42
-
16
/// <summary>
17
/// Install the packages.
18
/// </summary>
@@ -52,6 +25,13 @@ namespace WixToolset.Mba.Core
25
/// </summary>
26
void CloseSplashScreen();
27
28
+ /// <summary>
29
+ /// Checks if a variable exists in the engine.
30
+ /// </summary>
31
+ /// <param name="name">The name of the variable.</param>
32
+ /// <returns>Whether the variable exists.</returns>
33
+ bool ContainsVariable(string name);
34
+
35
/// <summary>
36
/// Determine if all installation conditions are fulfilled.
37
/// </summary>
@@ -94,6 +74,30 @@ namespace WixToolset.Mba.Core
74
/// <exception cref="Win32Exception">A Win32 error occurred.</exception>
75
string FormatString(string format);
76
77
+ /// <summary>
78
+ /// Gets numeric variables for the engine.
79
+ /// </summary>
80
+ /// <param name="name">The name of the variable.</param>
81
+ long GetVariableNumeric(string name);
82
+
83
+ /// <summary>
84
+ /// Gets string variables for the engine using SecureStrings.
85
+ /// </summary>
86
+ /// <param name="name">The name of the variable.</param>
87
+ SecureString GetVariableSecureString(string name);
88
+
89
+ /// <summary>
90
+ /// Gets string variables for the engine.
91
+ /// </summary>
92
+ /// <param name="name">The name of the variable.</param>
93
+ string GetVariableString(string name);
94
+
95
+ /// <summary>
96
+ /// Gets <see cref="Version"/> variables for the engine.
97
+ /// </summary>
98
+ /// <param name="name">The name of the variable.</param>
99
+ Version GetVariableVersion(string name);
100
+
101
/// <summary>
102
/// Launches a preapproved executable elevated. As long as the engine already elevated, there will be no UAC prompt.
103
/// </summary>
@@ -152,6 +156,43 @@ namespace WixToolset.Mba.Core
156
/// <param name="password">The password for proxy authentication.</param>
157
void SetDownloadSource(string packageOrContainerId, string payloadId, string url, string user, string password);
158
159
+ /// <summary>
160
+ /// Sets numeric variables for the engine.
161
+ /// </summary>
162
+ /// <param name="name">The name of the variable.</param>
163
+ /// <param name="value">The value to set.</param>
164
+ void SetVariable(string name, long value);
165
+
166
+ /// <summary>
167
+ /// Sets string variables for the engine using SecureStrings.
168
+ /// </summary>
169
+ /// <param name="name">The name of the variable.</param>
170
+ /// <param name="value">The value to set.</param>
171
+ /// <param name="formatted">False if the value is a literal string.</param>
172
+ void SetVariable(string name, SecureString value, bool formatted);
173
+
174
+ /// <summary>
175
+ /// Sets string variables for the engine.
176
+ /// </summary>
177
+ /// <param name="name">The name of the variable.</param>
178
+ /// <param name="value">The value to set.</param>
179
+ /// <param name="formatted">False if the value is a literal string.</param>
180
+ void SetVariable(string name, string value, bool formatted);
181
+
182
+ /// <summary>
183
+ /// Sets <see cref="Version"/> variables for the engine.
184
+ ///
185
+ /// The <see cref="Version"/> class can keep track of when the build and revision fields are undefined, but the engine can't.
186
+ /// Therefore, the build and revision fields must be defined when setting a <see cref="Version"/> variable.
187
+ /// Use the NormalizeVersion method to make sure the engine can accept the Version.
188
+ ///
189
+ /// To keep track of versions without build or revision fields, use StringVariables instead.
190
+ /// </summary>
191
+ /// <param name="name">The name of the variable.</param>
192
+ /// <param name="value">The value to set.</param>
193
+ /// <exception cref="OverflowException">The given <see cref="Version"/> was invalid.</exception>
194
+ void SetVariable(string name, Version value);
195
+
196
/// <summary>
197
/// Sends error message when embedded.
198
/// </summary>
src/WixToolset.Mba.Core/IVariables.cs
deleted
-27
@@ -1,27 +0,0 @@
1
-// 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.
2
-
3
-namespace WixToolset.Mba.Core
4
-{
5
- using System;
6
-
7
- /// <summary>
8
- /// An accessor for numeric, string, and version variables for the engine.
9
- /// </summary>
10
- public interface IVariables<T>
11
- {
12
- /// <summary>
13
- /// Gets or sets the variable given by <paramref name="name"/>.
14
- /// </summary>
15
- /// <param name="name">The name of the variable to get/set.</param>
16
- /// <returns>The value of the given variable.</returns>
17
- /// <exception cref="Exception">An error occurred getting the variable.</exception>
18
- T this[string name] { get; set; }
19
-
20
- /// <summary>
21
- /// Gets whether the variable given by <paramref name="name"/> exists.
22
- /// </summary>
23
- /// <param name="name">The name of the variable to check.</param>
24
- /// <returns>True if the variable given by <paramref name="name"/> exists; otherwise, false.</returns>
25
- bool Contains(string name);
26
- }
27
-}
src/balutil/balutil.cpp
+2
-2
@@ -167,7 +167,7 @@ LExit:
167
}
168
169
170
-DAPI_(BOOL) BalStringVariableExists(
170
+DAPI_(BOOL) BalVariableExists(
171
__in_z LPCWSTR wzVariable
172
)
173
{
@@ -183,7 +183,7 @@ DAPI_(BOOL) BalStringVariableExists(
183
hr = vpEngine->GetVariableString(wzVariable, NULL, &cch);
184
185
LExit:
186
- return E_MOREDATA == hr; // string exists only if there are more than zero characters in the variable.
186
+ return E_NOTFOUND != hr;
187
}
188
189
src/balutil/inc/balutil.h
+2
-2
@@ -108,10 +108,10 @@ DAPI_(HRESULT) BalSetNumericVariable(
108
);
109
110
/*******************************************************************
111
-BalStringVariableExists - checks if a string variable exists in the engine.
111
+BalVariableExists - checks if a variable exists in the engine.
112
113
********************************************************************/
114
-DAPI_(BOOL) BalStringVariableExists(
114
+DAPI_(BOOL) BalVariableExists(
115
__in_z LPCWSTR wzVariable
116
);
117