Skip to content

Commit d465439

Browse files
committed
Rollback changes to the 'delete on upload' changes to support explicit definition in code, profile component-level and command line override.
1 parent ceb7a06 commit d465439

9 files changed

Lines changed: 230 additions & 43 deletions

File tree

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
3.3.6
1+
3.3.7

src/VirtualClient/VirtualClient.Contracts/FileUploadDescriptor.cs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,10 @@ public class FileUploadDescriptor
3636
/// <param name="contentType">The web content type for the blob (e.g. text/plain, application/octet-stream).</param>
3737
/// <param name="filePath">The path to the file containing the content to upload.</param>
3838
/// <param name="manifest">Properties that define a manifest (e.g. metadata) for the file and its contents.</param>
39+
/// <param name="deleteOnUpload">True/false whether the file should be deleted upon being successfully uploaded. Default = false.</param>
40+
/// <param name="includeManifest">True/false whether a manifest should be included with the file uploaded.</param>
3941
[JsonConstructor]
40-
public FileUploadDescriptor(string blobPath, string containerName, string contentEncoding, string contentType, string filePath, IDictionary<string, IConvertible> manifest = null)
42+
public FileUploadDescriptor(string blobPath, string containerName, string contentEncoding, string contentType, string filePath, IDictionary<string, IConvertible> manifest = null, bool deleteOnUpload = false, bool includeManifest = false)
4143
{
4244
blobPath.ThrowIfNullOrWhiteSpace(nameof(blobPath));
4345
containerName.ThrowIfNullOrWhiteSpace(nameof(containerName));
@@ -51,7 +53,9 @@ public FileUploadDescriptor(string blobPath, string containerName, string conten
5153
this.ContainerName = containerName;
5254
this.ContentEncoding = contentEncoding;
5355
this.ContentType = contentType;
56+
this.DeleteOnUpload = deleteOnUpload;
5457
this.FilePath = filePath;
58+
this.IncludeManifest = includeManifest;
5559
this.Manifest = new Dictionary<string, IConvertible>(StringComparer.OrdinalIgnoreCase);
5660

5761
if (manifest?.Any() == true)
@@ -90,12 +94,24 @@ public FileUploadDescriptor(string blobPath, string containerName, string conten
9094
[JsonProperty(PropertyName = "contentType", Required = Required.Always)]
9195
public string ContentType { get; }
9296

97+
/// <summary>
98+
/// True/false whether the file should be deleted upon being successfully uploaded.
99+
/// </summary>
100+
[JsonProperty(PropertyName = "deleteOnUpload", Required = Required.Always)]
101+
public bool DeleteOnUpload { get; set; }
102+
93103
/// <summary>
94104
/// The full path to the file to upload.
95105
/// </summary>
96106
[JsonProperty(PropertyName = "filePath", Required = Required.Always)]
97107
public string FilePath { get; }
98108

109+
/// <summary>
110+
/// True/false whether a manifest should be included alongside the file uploaded.
111+
/// </summary>
112+
[JsonProperty(PropertyName = "includeManifest", Required = Required.Always)]
113+
public bool IncludeManifest { get; set; }
114+
99115
/// <summary>
100116
/// Manifest/metadata information related to the file and. its contents
101117
/// </summary>

src/VirtualClient/VirtualClient.Contracts/VirtualClientComponent.cs

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ namespace VirtualClient.Contracts
55
{
66
using System;
77
using System.Collections.Generic;
8+
using System.Globalization;
89
using System.Linq;
910
using System.Reflection;
1011
using System.Runtime.InteropServices;
@@ -197,18 +198,36 @@ public string ContentPathTemplate
197198

198199
/// <summary>
199200
/// True/false whether log files upload request processing should be deferred
200-
/// for handling later when a content store is defined. Default = false.
201+
/// for handling later when a content store is defined.
201202
/// </summary>
202-
public bool DeferUploads
203+
public bool? DeferUpload
203204
{
204205
get
205206
{
206-
return this.Parameters.GetValue<bool>(nameof(this.DeferUploads), false);
207+
this.Parameters.TryGetValue(nameof(this.DeferUpload), out IConvertible deferUpload);
208+
return deferUpload?.ToBoolean(CultureInfo.InvariantCulture);
207209
}
208210

209211
protected set
210212
{
211-
this.Parameters[nameof(this.DeferUploads)] = false;
213+
this.Parameters[nameof(this.DeferUpload)] = value;
214+
}
215+
}
216+
217+
/// <summary>
218+
/// True/false whether log files should be deleted upon being successfully uploaded.
219+
/// </summary>
220+
public bool? DeleteOnUpload
221+
{
222+
get
223+
{
224+
this.Parameters.TryGetValue(nameof(this.DeleteOnUpload), out IConvertible delete);
225+
return delete?.ToBoolean(CultureInfo.InvariantCulture);
226+
}
227+
228+
protected set
229+
{
230+
this.Parameters[nameof(this.DeleteOnUpload)] = false;
212231
}
213232
}
214233

@@ -320,6 +339,23 @@ protected set
320339
}
321340
}
322341

342+
/// <summary>
343+
/// True/false whether a manifest should be included with file uploads.
344+
/// </summary>
345+
public bool? IncludeManifestOnUpload
346+
{
347+
get
348+
{
349+
this.Parameters.TryGetValue(nameof(this.IncludeManifestOnUpload), out IConvertible includeManifest);
350+
return includeManifest?.ToBoolean(CultureInfo.InvariantCulture);
351+
}
352+
353+
protected set
354+
{
355+
this.Parameters[nameof(this.IncludeManifestOnUpload)] = value;
356+
}
357+
}
358+
323359
/// <summary>
324360
/// Metadata provided to the application on the command line.
325361
/// </summary>

src/VirtualClient/VirtualClient.Contracts/VirtualClientComponentExtensions.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,16 @@ public static FileUploadDescriptor CreateFileUploadDescriptor(this VirtualClient
146146
component.ContentPathTemplate,
147147
subPath);
148148

149+
if (component.DeleteOnUpload == true)
150+
{
151+
descriptor.DeleteOnUpload = true;
152+
}
153+
154+
if (component.IncludeManifestOnUpload == true)
155+
{
156+
descriptor.IncludeManifest = true;
157+
}
158+
149159
return descriptor;
150160
}
151161

src/VirtualClient/VirtualClient.Core.UnitTests/VirtualClientLoggingExtensionsTests.cs

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ namespace VirtualClient
1414
using Moq;
1515
using NUnit.Framework;
1616
using VirtualClient.Common;
17+
using VirtualClient.Common.Contracts;
1718
using VirtualClient.Common.Telemetry;
19+
using VirtualClient.Contracts;
1820

1921
[TestFixture]
2022
[Category("Unit")]
@@ -551,5 +553,131 @@ public async Task LogProcessDetailsExtensionLogsToTheExpectedStandardErrorToFile
551553
Assert.IsTrue(standardErrorConfirmed, "Standard error not confirmed.");
552554
}
553555
}
556+
557+
[Test]
558+
public async Task LogProcessDetailsExtensionUploadsLogFilesByDefaultWhenAContentStoreIsProvided()
559+
{
560+
bool confirmed = false;
561+
Mock<IProcessProxy> mockProcess = new Mock<IProcessProxy>().Setup();
562+
563+
using (TestExecutor component = new TestExecutor(this))
564+
{
565+
this.FileSystem
566+
.Setup(fs => fs.File.WriteAllTextAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()))
567+
.Callback<string, string, CancellationToken>((path, content, token) =>
568+
{
569+
if (path.StartsWith(this.PlatformSpecifics.ContentUploadsDirectory))
570+
{
571+
confirmed = true;
572+
FileUploadDescriptor descriptor = content.FromJson<FileUploadDescriptor>();
573+
Assert.IsNotNull(descriptor);
574+
}
575+
})
576+
.Returns(Task.CompletedTask);
577+
578+
await component.LogProcessDetailsAsync(mockProcess.Object, EventContext.None, logToTelemetry: false, logToFile: true);
579+
Assert.IsTrue(confirmed);
580+
}
581+
}
582+
583+
[Test]
584+
public async Task LogProcessDetailsExtensionSupportsDeferringLogFileUploads()
585+
{
586+
bool deferred = true;
587+
Mock<IProcessProxy> mockProcess = new Mock<IProcessProxy>().Setup();
588+
589+
using (TestExecutor component = new TestExecutor(this))
590+
{
591+
component.Parameters[nameof(component.DeferUpload)] = true;
592+
593+
this.FileSystem
594+
.Setup(fs => fs.File.WriteAllTextAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()))
595+
.Callback<string, string, CancellationToken>((path, content, token) =>
596+
{
597+
if (path.StartsWith(this.PlatformSpecifics.ContentUploadsDirectory))
598+
{
599+
deferred = false;
600+
}
601+
})
602+
.Returns(Task.CompletedTask);
603+
604+
await component.LogProcessDetailsAsync(mockProcess.Object, EventContext.None, logToTelemetry: false, logToFile: true);
605+
Assert.IsTrue(deferred);
606+
}
607+
}
608+
609+
[Test]
610+
public async Task LogProcessDetailsExtensionDeletesLogFilesOnUploadsOnlyWhenExpected()
611+
{
612+
bool confirmed = false;
613+
bool shouldBeDeleted = false;
614+
Mock<IProcessProxy> mockProcess = new Mock<IProcessProxy>().Setup();
615+
616+
using (TestExecutor component = new TestExecutor(this))
617+
{
618+
// By default, log files should NOT be deleted on upload.
619+
this.FileSystem
620+
.Setup(fs => fs.File.WriteAllTextAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()))
621+
.Callback<string, string, CancellationToken>((path, content, token) =>
622+
{
623+
if (path.StartsWith(this.PlatformSpecifics.ContentUploadsDirectory))
624+
{
625+
confirmed = true;
626+
FileUploadDescriptor descriptor = content.FromJson<FileUploadDescriptor>();
627+
Assert.IsNotNull(descriptor);
628+
Assert.AreEqual(shouldBeDeleted, descriptor.DeleteOnUpload, "Log file delete does not match expectation.");
629+
}
630+
})
631+
.Returns(Task.CompletedTask);
632+
633+
await component.LogProcessDetailsAsync(mockProcess.Object, EventContext.None, logToTelemetry: false, logToFile: true);
634+
Assert.IsTrue(confirmed);
635+
636+
// If requested, however, log files should be deleted on upload.
637+
confirmed = false;
638+
shouldBeDeleted = true;
639+
component.Parameters[nameof(component.DeleteOnUpload)] = true;
640+
641+
await component.LogProcessDetailsAsync(mockProcess.Object, EventContext.None, logToTelemetry: false, logToFile: true);
642+
Assert.IsTrue(confirmed);
643+
}
644+
}
645+
646+
[Test]
647+
public async Task LogProcessDetailsExtensionIncludesAManifestWithLogFilesOnUploadsOnlyWhenExpected()
648+
{
649+
bool confirmed = false;
650+
bool shouldBeIncluded = false;
651+
Mock<IProcessProxy> mockProcess = new Mock<IProcessProxy>().Setup();
652+
653+
using (TestExecutor component = new TestExecutor(this))
654+
{
655+
// By default, a manifest should NOT be included with log file uploads.
656+
this.FileSystem
657+
.Setup(fs => fs.File.WriteAllTextAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<CancellationToken>()))
658+
.Callback<string, string, CancellationToken>((path, content, token) =>
659+
{
660+
if (path.StartsWith(this.PlatformSpecifics.ContentUploadsDirectory))
661+
{
662+
confirmed = true;
663+
FileUploadDescriptor descriptor = content.FromJson<FileUploadDescriptor>();
664+
Assert.IsNotNull(descriptor);
665+
Assert.AreEqual(shouldBeIncluded, descriptor.IncludeManifest, "Manifest inclusion does not match expectation.");
666+
}
667+
})
668+
.Returns(Task.CompletedTask);
669+
670+
await component.LogProcessDetailsAsync(mockProcess.Object, EventContext.None, logToTelemetry: false, logToFile: true);
671+
Assert.IsTrue(confirmed);
672+
673+
// If requested, however, log files should be deleted on upload.
674+
confirmed = false;
675+
shouldBeIncluded = true;
676+
component.Parameters[nameof(component.IncludeManifestOnUpload)] = true;
677+
678+
await component.LogProcessDetailsAsync(mockProcess.Object, EventContext.None, logToTelemetry: false, logToFile: true);
679+
Assert.IsTrue(confirmed);
680+
}
681+
}
554682
}
555683
}

src/VirtualClient/VirtualClient.Core/VirtualClientLoggingExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ await RetryPolicies.FileOperations.ExecuteAsync(async () =>
408408
await fileSystem.File.WriteAllTextAsync(logFilePath, outputBuilder.ToString());
409409
});
410410

411-
if (upload && !component.DeferUploads && component.TryGetContentStoreManager(out IBlobManager blobManager))
411+
if (upload && component.DeferUpload != true && component.TryGetContentStoreManager(out IBlobManager blobManager))
412412
{
413413
string effectiveToolName = component.GetLogDirectoryName(processDetails.ToolName);
414414

src/VirtualClient/VirtualClient.Main/profiles/EXECUTE-SCRIPT.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
"Parameters": {
2828
"Scenario": "$.Parameters.Scenario",
2929
"Command": "$.Parameters.Command",
30-
"DeferUploads": true,
30+
"DeferUpload": true,
3131
"EnvironmentVariables": "EXPERIMENT_ID={ExperimentId},EXECUTION_SYSTEM={ExecutionSystem}",
3232
"LogFileName": "$.Parameters.LogFileName",
3333
"LogFolderName": "$.Parameters.LogFolderName",

src/VirtualClient/VirtualClient.Main/profiles/MONITORS-FILE-UPLOAD.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,15 @@
55
"SupportedOperatingSystems": "Azure Linux,CentOS,Debian,RedHat,Suse,Ubuntu,Windows"
66
},
77
"Parameters": {
8-
"IncludeManifest": false
8+
"DeleteOnUpload": null,
9+
"IncludeManifestOnUpload": null
910
},
1011
"Monitors": [
1112
{
1213
"Type": "FileUploadMonitor",
1314
"Parameters": {
14-
"Scenario": "UploadContent",
15-
"DeleteLogs": false,
16-
"IncludeManifest": "$.Parameters.IncludeManifest"
15+
"DeleteOnUpload": "$.Parameters.DeleteOnUpload",
16+
"IncludeManifestOnUpload": "$.Parameters.IncludeManifestOnUpload"
1717
}
1818
}
1919
]

0 commit comments

Comments
 (0)