From ffb30e3a1a23114e022b6bda39017a3917046faa Mon Sep 17 00:00:00 2001 From: Omotola Date: Wed, 23 Aug 2023 17:26:04 -0700 Subject: [PATCH] Fix to remove default off detectors from experiments (#739) Fix to remove default off detectors from experiments --- .../DetectorRestrictions.cs | 3 + .../Experiments/ExperimentService.cs | 13 +++- .../Experiments/IExperimentService.cs | 7 +++ .../Services/BcdeScanExecutionService.cs | 9 +-- .../Services/DetectorProcessingService.cs | 2 + .../Services/IDetectorProcessingService.cs | 2 +- .../Experiments/ExperimentServiceTests.cs | 63 +++++++++++++++++++ 7 files changed, 93 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Orchestrator/DetectorRestrictions.cs b/src/Microsoft.ComponentDetection.Orchestrator/DetectorRestrictions.cs index d1285304..51d59cb1 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/DetectorRestrictions.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/DetectorRestrictions.cs @@ -1,6 +1,7 @@ namespace Microsoft.ComponentDetection.Orchestrator; using System.Collections.Generic; +using Microsoft.ComponentDetection.Contracts; public class DetectorRestrictions { @@ -9,4 +10,6 @@ public class DetectorRestrictions public IEnumerable ExplicitlyEnabledDetectorIds { get; set; } public IEnumerable AllowedDetectorCategories { get; set; } + + public IEnumerable DisabledDetectors { get; set; } } diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs index 46eef35c..ea8be1d1 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs @@ -108,6 +108,18 @@ public class ExperimentService : IExperimentService } } + public void RemoveUnwantedExperimentsbyDetectors(IEnumerable detectors) + { + var experimentsToRemove = this.experiments + .Where(x => detectors.Any(detector => x.Key.IsInControlGroup(detector) || x.Key.IsInExperimentGroup(detector))) + .Select(x => x.Key).ToList(); + + foreach (var config in experimentsToRemove.Where(config => this.experiments.TryRemove(config, out _))) + { + this.logger.LogDebug("Removing {Experiment} from active experiments", config.Name); + } + } + /// public async Task FinishAsync() { @@ -125,7 +137,6 @@ public class ExperimentService : IExperimentService { var controlComponents = experiment.ControlGroupComponents; var experimentComponents = experiment.ExperimentGroupComponents; - this.logger.LogInformation( "Experiment {Experiment} finished with {ControlCount} components in the control group and {ExperimentCount} components in the experiment group", config.Name, diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentService.cs index 958d45df..7f689275 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentService.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Experiments/IExperimentService.cs @@ -1,5 +1,6 @@ namespace Microsoft.ComponentDetection.Orchestrator.Experiments; +using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.ComponentDetection.Common.DependencyGraph; using Microsoft.ComponentDetection.Contracts; @@ -23,4 +24,10 @@ public interface IExperimentService /// /// A representing the asynchronous operation. Task FinishAsync(); + + /// + /// Removes any experimentsthat contains a detector that is not needed. + /// + /// List of all detectors. + void RemoveUnwantedExperimentsbyDetectors(IEnumerable detectors); } diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Services/BcdeScanExecutionService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Services/BcdeScanExecutionService.cs index bbce4c43..f980d462 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Services/BcdeScanExecutionService.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Services/BcdeScanExecutionService.cs @@ -1,4 +1,4 @@ -namespace Microsoft.ComponentDetection.Orchestrator.Services; +namespace Microsoft.ComponentDetection.Orchestrator.Services; using System; using System.Collections.Generic; @@ -38,14 +38,15 @@ public class BcdeScanExecutionService : IBcdeScanExecutionService using var scope = this.logger.BeginScope("Executing BCDE scan"); var detectorRestrictions = this.GetDetectorRestrictions(detectionArguments); - var detectors = this.detectorRestrictionService.ApplyRestrictions(detectorRestrictions, this.detectors).ToImmutableList(); + var restrictedDetectors = this.detectorRestrictionService.ApplyRestrictions(detectorRestrictions, this.detectors).ToImmutableList(); + detectorRestrictions.DisabledDetectors = this.detectors.Except(restrictedDetectors).ToList(); this.logger.LogDebug("Finished applying restrictions to detectors."); - var processingResult = await this.detectorProcessingService.ProcessDetectorsAsync(detectionArguments, detectors, detectorRestrictions); + var processingResult = await this.detectorProcessingService.ProcessDetectorsAsync(detectionArguments, restrictedDetectors, detectorRestrictions); var scanResult = this.graphTranslationService.GenerateScanResultFromProcessingResult(processingResult, detectionArguments); - scanResult.DetectorsInScan = detectors.Select(x => ConvertToContract(x)).ToList(); + scanResult.DetectorsInScan = restrictedDetectors.Select(x => ConvertToContract(x)).ToList(); scanResult.ResultCode = processingResult.ResultCode; return scanResult; diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs index 2206606f..cb488d1d 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Services/DetectorProcessingService.cs @@ -52,6 +52,8 @@ public class DetectorProcessingService : IDetectorProcessingService ? this.GenerateDirectoryExclusionPredicate(detectionArguments.SourceDirectory.ToString(), detectionArguments.DirectoryExclusionList, detectionArguments.DirectoryExclusionListObsolete, allowWindowsPaths: false, ignoreCase: false) : this.GenerateDirectoryExclusionPredicate(detectionArguments.SourceDirectory.ToString(), detectionArguments.DirectoryExclusionList, detectionArguments.DirectoryExclusionListObsolete, allowWindowsPaths: true, ignoreCase: true); + this.experimentService.RemoveUnwantedExperimentsbyDetectors(detectorRestrictions.DisabledDetectors); + IEnumerable> scanTasks = detectors .Select(async detector => { diff --git a/src/Microsoft.ComponentDetection.Orchestrator/Services/IDetectorProcessingService.cs b/src/Microsoft.ComponentDetection.Orchestrator/Services/IDetectorProcessingService.cs index 4aa7621d..ce3027ca 100644 --- a/src/Microsoft.ComponentDetection.Orchestrator/Services/IDetectorProcessingService.cs +++ b/src/Microsoft.ComponentDetection.Orchestrator/Services/IDetectorProcessingService.cs @@ -1,4 +1,4 @@ -namespace Microsoft.ComponentDetection.Orchestrator.Services; +namespace Microsoft.ComponentDetection.Orchestrator.Services; using System.Collections.Generic; using System.Threading.Tasks; diff --git a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs index e399579f..f466f19e 100644 --- a/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs +++ b/test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs @@ -8,6 +8,7 @@ using FluentAssertions; using Microsoft.ComponentDetection.Common.DependencyGraph; using Microsoft.ComponentDetection.Contracts; using Microsoft.ComponentDetection.Contracts.BcdeModels; +using Microsoft.ComponentDetection.Detectors.NuGet; using Microsoft.ComponentDetection.Orchestrator.ArgumentSets; using Microsoft.ComponentDetection.Orchestrator.Experiments; using Microsoft.ComponentDetection.Orchestrator.Experiments.Configs; @@ -262,4 +263,66 @@ public class ExperimentServiceTests x => x.ProcessExperimentAsync(this.experimentConfigMock.Object, It.IsAny()), Times.Never()); } + + [TestMethod] + public async Task RecordDetectorRun_CheckUnwantedDetectors_RemoveExperimentAsync() + { + var components = ExperimentTestUtils.CreateRandomComponents(); + + var service = new ExperimentService( + new[] { this.experimentConfigMock.Object }, + new[] { this.experimentProcessorMock.Object }, + this.graphTranslationServiceMock.Object, + this.loggerMock.Object); + this.SetupGraphMock(components); + + var detectorList = new List + { + new NuGetComponentDetector( + new Mock().Object, + new Mock().Object, + new Mock>().Object), this.detectorMock.Object, + }; + + service.RemoveUnwantedExperimentsbyDetectors(detectorList); + + service.RecordDetectorRun(this.detectorMock.Object, this.componentRecorder, this.detectionArgsMock.Object); + + await service.FinishAsync(); + + this.experimentProcessorMock.Verify( + x => x.ProcessExperimentAsync(this.experimentConfigMock.Object, It.IsAny()), + Times.Never()); + } + + [TestMethod] + public async Task RecordDetectorRun_CheckUnwantedDetectors_KeepExperimentAsync() + { + var components = ExperimentTestUtils.CreateRandomComponents(); + + var service = new ExperimentService( + new[] { this.experimentConfigMock.Object }, + new[] { this.experimentProcessorMock.Object }, + this.graphTranslationServiceMock.Object, + this.loggerMock.Object); + this.SetupGraphMock(components); + + var detectorList = new List + { + new NuGetComponentDetector( + new Mock().Object, + new Mock().Object, + new Mock>().Object), + }; + + service.RemoveUnwantedExperimentsbyDetectors(detectorList); + + service.RecordDetectorRun(this.detectorMock.Object, this.componentRecorder, this.detectionArgsMock.Object); + + await service.FinishAsync(); + + this.experimentProcessorMock.Verify( + x => x.ProcessExperimentAsync(this.experimentConfigMock.Object, It.IsAny()), + Times.Once()); + } }