Ensure apply also validates the plan before calling apply (#1814)

* Ensure apply also validates the plan before calling apply

* fix tests compilation

Author: Mpdreamz

src/tooling/docs-assembler/Cli/DeployCommands.cs

@@ -44,7 +44,7 @@ public async Task<int> Plan(
 		string environment,
 		string s3BucketName,
 		string @out = "",
-		float deleteThreshold = 0.2f,
+		float? deleteThreshold = null,
 		Cancel ctx = default
 	)
 	{
@@ -56,20 +56,21 @@ public async Task<int> Plan(
 		var fs = new FileSystem();
 		var assembleContext = new AssembleContext(assemblyConfiguration, configurationContext, environment, collector, fs, fs, null, null);
 		var s3Client = new AmazonS3Client();
-		IDocsSyncPlanStrategy planner = new AwsS3SyncPlanStrategy(logFactory, s3Client, s3BucketName, assembleContext);
-		var plan = await planner.Plan(ctx);
+		var planner = new AwsS3SyncPlanStrategy(logFactory, s3Client, s3BucketName, assembleContext);
+		var plan = await planner.Plan(deleteThreshold, ctx);
 		_logger.LogInformation("Total files to sync: {TotalFiles}", plan.TotalSyncRequests);
 		_logger.LogInformation("Total files to delete: {DeleteCount}", plan.DeleteRequests.Count);
 		_logger.LogInformation("Total files to add: {AddCount}", plan.AddRequests.Count);
 		_logger.LogInformation("Total files to update: {UpdateCount}", plan.UpdateRequests.Count);
 		_logger.LogInformation("Total files to skip: {SkipCount}", plan.SkipRequests.Count);
 		_logger.LogInformation("Total local source files: {TotalSourceFiles}", plan.TotalSourceFiles);
 		_logger.LogInformation("Total remote source files: {TotalSourceFiles}", plan.TotalRemoteFiles);
-		var validationResult = planner.Validate(plan, deleteThreshold);
+		var validator = new DocsSyncPlanValidator(logFactory);
+		var validationResult = validator.Validate(plan);
 		if (!validationResult.Valid)
 		{
 			await githubActionsService.SetOutputAsync("plan-valid", "false");
-			collector.EmitError(@out, $"Plan is invalid, delete ratio: {validationResult.DeleteRatio}, threshold: {validationResult.DeleteThreshold} over {plan.TotalSyncRequests:N0} files while plan has {plan.DeleteRequests:N0} deletions");
+			collector.EmitError(@out, $"Plan is invalid, delete ratio: {validationResult.DeleteRatio}, threshold: {validationResult.DeleteThreshold} over {plan.TotalRemoteFiles:N0} remote files while plan has {plan.DeleteRequests:N0} deletions");
 			await collector.StopAsync(ctx);
 			return collector.Errors;
 		}
@@ -92,11 +93,7 @@ public async Task<int> Plan(
 	/// <param name="s3BucketName">The S3 bucket name to deploy to</param>
 	/// <param name="planFile">The path to the plan file to apply</param>
 	/// <param name="ctx"></param>
-	public async Task<int> Apply(
-		string environment,
-		string s3BucketName,
-		string planFile,
-		Cancel ctx = default)
+	public async Task<int> Apply(string environment, string s3BucketName, string planFile, Cancel ctx = default)
 	{
 		AssignOutputLogger();
 		await using var collector = new ConsoleDiagnosticsCollector(logFactory, githubActionsService)
@@ -111,7 +108,6 @@ public async Task<int> Apply(
 			ConcurrentServiceRequests = Environment.ProcessorCount * 2,
 			MinSizeBeforePartUpload = S3EtagCalculator.PartSize
 		});
-		IDocsSyncApplyStrategy applier = new AwsS3SyncApplyStrategy(logFactory, s3Client, transferUtility, s3BucketName, assembleContext, collector);
 		if (!File.Exists(planFile))
 		{
 			collector.EmitError(planFile, "Plan file does not exist.");
@@ -133,6 +129,15 @@ public async Task<int> Apply(
 			await collector.StopAsync(ctx);
 			return collector.Errors;
 		}
+		var validator = new DocsSyncPlanValidator(logFactory);
+		var validationResult = validator.Validate(plan);
+		if (!validationResult.Valid)
+		{
+			collector.EmitError(planFile, $"Plan is invalid, delete ratio: {validationResult.DeleteRatio}, threshold: {validationResult.DeleteThreshold} over {plan.TotalRemoteFiles:N0} remote files while plan has {plan.DeleteRequests:N0} deletions");
+			await collector.StopAsync(ctx);
+			return collector.Errors;
+		}
+		var applier = new AwsS3SyncApplyStrategy(logFactory, s3Client, transferUtility, s3BucketName, assembleContext, collector);
 		await applier.Apply(plan, ctx);
 		await collector.StopAsync(ctx);
 		return collector.Errors;

src/tooling/docs-assembler/Deploying/AwsS3SyncPlanStrategy.cs

@@ -82,8 +82,6 @@ public class AwsS3SyncPlanStrategy(
 )
 	: IDocsSyncPlanStrategy
 {
-	private readonly ILogger<AwsS3SyncPlanStrategy> _logger = logFactory.CreateLogger<AwsS3SyncPlanStrategy>();
-
 	private readonly IS3EtagCalculator _s3EtagCalculator = calculator ?? new S3EtagCalculator(logFactory, context.ReadFileSystem);
 
 	private bool IsSymlink(string path)
@@ -92,7 +90,7 @@ private bool IsSymlink(string path)
 		return fileInfo.LinkTarget != null;
 	}
 
-	public async Task<SyncPlan> Plan(Cancel ctx = default)
+	public async Task<SyncPlan> Plan(float? deleteThreshold, Cancel ctx = default)
 	{
 		var remoteObjects = await ListObjects(ctx);
 		var localObjects = context.OutputDirectory.GetFiles("*", SearchOption.AllDirectories)
@@ -158,6 +156,7 @@ await Parallel.ForEachAsync(localObjects, ctx, async (localFile, token) =>
 
 		return new SyncPlan
 		{
+			DeleteThresholdDefault = deleteThreshold,
 			TotalRemoteFiles = remoteObjects.Count,
 			TotalSourceFiles = localObjects.Length,
 			DeleteRequests = deleteRequests.ToList(),
@@ -168,40 +167,6 @@ await Parallel.ForEachAsync(localObjects, ctx, async (localFile, token) =>
 		};
 	}
 
-	/// <inheritdoc />
-	public PlanValidationResult Validate(SyncPlan plan, float deleteThreshold)
-	{
-		if (plan.TotalSourceFiles == 0)
-		{
-			_logger.LogError("No files to sync");
-			return new(false, 1.0f, deleteThreshold);
-		}
-
-		var deleteRatio = (float)plan.DeleteRequests.Count / plan.TotalRemoteFiles;
-		if (plan.TotalRemoteFiles == 0)
-		{
-			_logger.LogInformation("No files discovered in S3, assuming a clean bucket resetting delete threshold to `0.0' as our plan should not have ANY deletions");
-			deleteThreshold = 0.0f;
-		}
-		// if the total remote files are less than or equal to 100, we enforce a higher ratio of 0.8
-		// this allows newer assembled documentation to be in a higher state of flux
-		if (plan.TotalRemoteFiles <= 100)
-			deleteThreshold = Math.Max(deleteThreshold, 0.8f);
-
-		// if the total remote files are less than or equal to 1000, we enforce a higher ratio of 0.5
-		// this allows newer assembled documentation to be in a higher state of flux
-		else if (plan.TotalRemoteFiles <= 1000)
-			deleteThreshold = Math.Max(deleteThreshold, 0.5f);
-
-		if (deleteRatio > deleteThreshold)
-		{
-			_logger.LogError("Delete ratio is {Ratio} which is greater than the threshold of {Threshold}", deleteRatio, deleteThreshold);
-			return new(false, deleteRatio, deleteThreshold);
-		}
-
-		return new(true, deleteRatio, deleteThreshold);
-	}
-
 	private async Task<Dictionary<string, S3Object>> ListObjects(Cancel ctx = default)
 	{
 		var listBucketRequest = new ListObjectsV2Request

src/tooling/docs-assembler/Deploying/DocsSync.cs

@@ -9,9 +9,8 @@ namespace Documentation.Assembler.Deploying;
 
 public interface IDocsSyncPlanStrategy
 {
-	Task<SyncPlan> Plan(Cancel ctx = default);
+	Task<SyncPlan> Plan(float? deleteThreshold, Cancel ctx = default);
 
-	PlanValidationResult Validate(SyncPlan plan, float deleteThreshold);
 }
 public record PlanValidationResult(bool Valid, float DeleteRatio, float DeleteThreshold);
 
@@ -52,6 +51,10 @@ public record SkipRequest : SyncRequest
 
 public record SyncPlan
 {
+	/// The user-specified delete threshold
+	[JsonPropertyName("deletion_threshold_default")]
+	public required float? DeleteThresholdDefault { get; init; }
+
 	/// The total number of source files that were located in the build output
 	[JsonPropertyName("total_source_files")]
 	public required int TotalSourceFiles { get; init; }

src/tooling/docs-assembler/Deploying/DocsSyncPlanValidator.cs

@@ -0,0 +1,57 @@
+// Licensed to Elasticsearch B.V under one or more agreements.
+// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
+// See the LICENSE file in the project root for more information
+
+using Microsoft.Extensions.Logging;
+
+namespace Documentation.Assembler.Deploying;
+
+public class DocsSyncPlanValidator(ILoggerFactory logFactory)
+{
+	private readonly ILogger<AwsS3SyncPlanStrategy> _logger = logFactory.CreateLogger<AwsS3SyncPlanStrategy>();
+
+	public PlanValidationResult Validate(SyncPlan plan)
+	{
+		if (plan.DeleteThresholdDefault is not null)
+			_logger.LogInformation("Using user-specified delete threshold of {Threshold}", plan.DeleteThresholdDefault);
+
+		var deleteThreshold = plan.DeleteThresholdDefault ?? 0.2f;
+		if (plan.TotalSourceFiles == 0)
+		{
+			_logger.LogError("No files to sync");
+			return new(false, 1.0f, deleteThreshold);
+		}
+
+		var deleteRatio = (float)plan.DeleteRequests.Count / plan.TotalRemoteFiles;
+		if (plan.TotalRemoteFiles == 0)
+		{
+			_logger.LogInformation("No files discovered in S3, assuming a clean bucket resetting delete threshold to `0.0' as our plan should not have ANY deletions");
+			deleteThreshold = 0.0f;
+		}
+		// if the total remote files are less than or equal to 100, we enforce a higher ratio of 0.8
+		// this allows newer assembled documentation to be in a higher state of flux
+		if (plan.TotalRemoteFiles <= 100)
+		{
+			_logger.LogInformation("Plan has less than 100 total remote files ensuring delete threshold is at minimum 0.8");
+			deleteThreshold = Math.Max(deleteThreshold, 0.8f);
+		}
+
+		// if the total remote files are less than or equal to 1000, we enforce a higher ratio of 0.5
+		// this allows newer assembled documentation to be in a higher state of flux
+		else if (plan.TotalRemoteFiles <= 1000)
+		{
+			_logger.LogInformation("Plan has less than 1000 but more than a 100 total remote files ensuring delete threshold is at minimum 0.5");
+			deleteThreshold = Math.Max(deleteThreshold, 0.5f);
+		}
+
+		if (deleteRatio > deleteThreshold)
+		{
+			_logger.LogError("Delete ratio is {Ratio} which is greater than the threshold of {Threshold}", deleteRatio, deleteThreshold);
+			return new(false, deleteRatio, deleteThreshold);
+		}
+
+		return new(true, deleteRatio, deleteThreshold);
+	}
+
+
+}

tests/docs-assembler.Tests/src/docs-assembler.Tests/DocsSyncTests.cs

@@ -10,6 +10,7 @@
 using Elastic.Documentation.Configuration;
 using Elastic.Documentation.Configuration.Assembler;
 using Elastic.Documentation.Diagnostics;
+using Elastic.Documentation.Tooling.Diagnostics;
 using FakeItEasy;
 using FluentAssertions;
 using Microsoft.Extensions.Logging;
@@ -62,7 +63,7 @@ public async Task TestPlan()
 		var planStrategy = new AwsS3SyncPlanStrategy(new LoggerFactory(), mockS3Client, "fake", context);
 
 		// Act
-		var plan = await planStrategy.Plan(ctx: Cancel.None);
+		var plan = await planStrategy.Plan(null, Cancel.None);
 
 		// Assert
 
@@ -108,7 +109,7 @@ public async Task ValidateAdditionsPlan(
 		bool valid
 	)
 	{
-		var (planStrategy, plan) = await SetupS3SyncContextSetup(localFiles, remoteFiles);
+		var (validator, _, plan) = await SetupS3SyncContextSetup(localFiles, remoteFiles, deleteThreshold);
 
 		// Assert
 
@@ -118,7 +119,7 @@ bool valid
 		plan.AddRequests.Count.Should().Be(totalFilesToAdd);
 		plan.DeleteRequests.Count.Should().Be(totalFilesToRemove);
 
-		var validationResult = planStrategy.Validate(plan, deleteThreshold);
+		var validationResult = validator.Validate(plan);
 		if (plan.TotalSyncRequests <= 100)
 			validationResult.DeleteThreshold.Should().Be(Math.Max(deleteThreshold, 0.8f));
 		else if (plan.TotalSyncRequests <= 1000)
@@ -146,7 +147,7 @@ public async Task ValidateUpdatesPlan(
 		bool valid
 	)
 	{
-		var (planStrategy, plan) = await SetupS3SyncContextSetup(localFiles, remoteFiles, "different-etag");
+		var (validator, _, plan) = await SetupS3SyncContextSetup(localFiles, remoteFiles, deleteThreshold, "different-etag");
 
 		// Assert
 
@@ -156,7 +157,7 @@ bool valid
 		plan.UpdateRequests.Count.Should().Be(totalFilesToUpdate);
 		plan.DeleteRequests.Count.Should().Be(totalFilesToRemove);
 
-		var validationResult = planStrategy.Validate(plan, deleteThreshold);
+		var validationResult = validator.Validate(plan);
 		if (plan.TotalSyncRequests <= 100)
 			validationResult.DeleteThreshold.Should().Be(Math.Max(deleteThreshold, 0.8f));
 		else if (plan.TotalSyncRequests <= 1000)
@@ -165,8 +166,8 @@ bool valid
 		validationResult.Valid.Should().Be(valid, $"Delete ratio is {validationResult.DeleteRatio} when maximum is {validationResult.DeleteThreshold}");
 	}
 
-	private static async Task<(AwsS3SyncPlanStrategy planStrategy, SyncPlan plan)> SetupS3SyncContextSetup(
-		int localFiles, int remoteFiles, string etag = "etag")
+	private static async Task<(DocsSyncPlanValidator validator, AwsS3SyncPlanStrategy planStrategy, SyncPlan plan)> SetupS3SyncContextSetup(
+		int localFiles, int remoteFiles, float? deleteThreshold = null, string etag = "etag")
 	{
 		// Arrange
 		IReadOnlyCollection<IDiagnosticsOutput> diagnosticsOutputs = [];
@@ -204,8 +205,9 @@ bool valid
 		var planStrategy = new AwsS3SyncPlanStrategy(new LoggerFactory(), mockS3Client, "fake", context, mockEtagCalculator);
 
 		// Act
-		var plan = await planStrategy.Plan(ctx: Cancel.None);
-		return (planStrategy, plan);
+		var plan = await planStrategy.Plan(deleteThreshold, Cancel.None);
+		var validator = new DocsSyncPlanValidator(new LoggerFactory());
+		return (validator, planStrategy, plan);
 	}
 
 	[Fact]
@@ -233,6 +235,7 @@ public async Task TestApply()
 		var context = new AssembleContext(config, configurationContext, "dev", collector, fileSystem, fileSystem, null, checkoutDirectory);
 		var plan = new SyncPlan
 		{
+			DeleteThresholdDefault = null,
 			TotalRemoteFiles = 0,
 			TotalSourceFiles = 5,
 			TotalSyncRequests = 6,