Skip to content

Commit

Permalink
Properly persist run level property bags (#1136)
Browse files Browse the repository at this point in the history
  • Loading branch information
@michaelcfanning
michaelcfanning authored Nov 15, 2018
1 parent 9d2f619 commit 5841151
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 36 deletions.
111 changes: 87 additions & 24 deletions src/Sarif.UnitTests/Baseline/ResultMatching/SarifLogResultMatcherTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,15 @@ public class SarifLogResultMatcherTests
{
private readonly ITestOutputHelper output;

private readonly SarifLogResultMatcher baseliner = new SarifLogResultMatcher(
exactResultMatchers: new [] { ExactMatchers.ExactResultMatcherFactory.GetIdenticalResultMatcher(considerPropertyBagsWhenComparing: true) },
private static readonly SarifLogResultMatcher s_preserveMostRecentPropertyBagMatcher = new SarifLogResultMatcher(
exactResultMatchers: new [] { ExactMatchers.ExactResultMatcherFactory.GetIdenticalResultMatcher(considerPropertyBagsWhenComparing: false) },
heuristicMatchers: null,
propertyBagMergeBehaviors: DictionaryMergeBehavior.InitializeFromPrevious);
propertyBagMergeBehaviors: DictionaryMergeBehavior.InitializeFromMostRecent);

private static readonly SarifLogResultMatcher s_preserveOldestPropertyBagMatcher = new SarifLogResultMatcher(
exactResultMatchers: new[] { ExactMatchers.ExactResultMatcherFactory.GetIdenticalResultMatcher(considerPropertyBagsWhenComparing: false) },
heuristicMatchers: null,
propertyBagMergeBehaviors: DictionaryMergeBehavior.InitializeFromOldest);

public SarifLogResultMatcherTests(ITestOutputHelper outputHelper)
{
Expand Down Expand Up @@ -51,7 +56,7 @@ public void SarifLogResultMatcher_BaselinesTwoSimpleSarifLogs()
result.CorrelationGuid = Guid.NewGuid().ToString();
}

SarifLog calculatedNextBaseline = baseliner.Match(new SarifLog[] { baselineLog }, new SarifLog[] { currentLog }).First();
SarifLog calculatedNextBaseline = s_preserveOldestPropertyBagMatcher.Match(new SarifLog[] { baselineLog }, new SarifLog[] { currentLog }).First();

calculatedNextBaseline.Runs.Should().HaveCount(1);

Expand Down Expand Up @@ -110,15 +115,83 @@ public void SarifLogResultMatcher_BaselinesSarifLogsWithProperties()
baselineLog.Runs[0].SetProperty(uniqueToBaselinePropertyName, uniqueToBaselinePropertyValue);
currentLog.Runs[0].SetProperty(uniqueToCurrentPropertyName, uniqueToCurrentPropertyValue);

SarifLog calculatedNextBaseline = baseliner.Match(new SarifLog[] { baselineLog }, new SarifLog[] { currentLog }).First();
SarifLog calculatedNextBaseline = s_preserveOldestPropertyBagMatcher.Match(new SarifLog[] { baselineLog }, new SarifLog[] { currentLog }).First();

string value = null;

// The default property bag matching behavior is to retain the property bag in its entirety from the baseline
calculatedNextBaseline.Runs[0].Properties.Should().NotBeNull();
calculatedNextBaseline.Runs[0].Properties.Count.Should().Be(2);

calculatedNextBaseline.Runs[0].GetProperty(sharedPropertyName).Should().Be(currentSharedPropertyValue);
calculatedNextBaseline.Runs[0].GetProperty(uniqueToBaselinePropertyName).Should().Be(uniqueToBaselinePropertyValue);
calculatedNextBaseline.Runs[0].TryGetProperty(uniqueToCurrentPropertyName, out string value).Should().BeFalse();
calculatedNextBaseline.Runs[0].TryGetProperty(uniqueToCurrentPropertyName, out value).Should().BeFalse();

calculatedNextBaseline = s_preserveMostRecentPropertyBagMatcher.Match(new SarifLog[] { baselineLog }, new SarifLog[] { currentLog }).First();

// The default property bag matching behavior is to retain the property bag in its entirety from the baseline
calculatedNextBaseline.Runs[0].Properties.Should().NotBeNull();
calculatedNextBaseline.Runs[0].Properties.Count.Should().Be(2);

calculatedNextBaseline.Runs[0].GetProperty(sharedPropertyName).Should().Be(currentSharedPropertyValue);
calculatedNextBaseline.Runs[0].GetProperty(uniqueToCurrentPropertyName).Should().Be(uniqueToCurrentPropertyValue);
calculatedNextBaseline.Runs[0].TryGetProperty(uniqueToBaselinePropertyName, out value).Should().BeFalse();
}

[Fact]
public void SarifLogResultMatcher_MatchMultipleCurrentLogsWithNoBaseline()
{
Random random = RandomSarifLogGenerator.GenerateRandomAndLog(this.output);

SarifLog mostRecentLog = RandomSarifLogGenerator.GenerateSarifLogWithRuns(random, 1);
SarifLog oldestLog = mostRecentLog.DeepClone();

string sharedPropertyName = nameof(sharedPropertyName);
string currentSharedPropertyValue = Guid.NewGuid().ToString();

string uniqueToMostRecentPropertyName = nameof(uniqueToMostRecentPropertyName);
string uniqueToMostRecentPropertyValue = Guid.NewGuid().ToString();

string uniqueToOldestPropertyName = nameof(uniqueToOldestPropertyName);
string uniqueToOldestPropertyValue = Guid.NewGuid().ToString();

mostRecentLog.Runs[0].SetProperty(sharedPropertyName, currentSharedPropertyValue);
oldestLog.Runs[0].SetProperty(sharedPropertyName, currentSharedPropertyValue);

mostRecentLog.Runs[0].SetProperty(uniqueToMostRecentPropertyName, uniqueToMostRecentPropertyValue);
oldestLog.Runs[0].SetProperty(uniqueToOldestPropertyName, uniqueToOldestPropertyValue);

SarifLog calculatedNextBaseline = s_preserveOldestPropertyBagMatcher.Match(
previousLogs: null,
currentLogs: new SarifLog[] { oldestLog, mostRecentLog }).First();

calculatedNextBaseline.Runs[0].Properties.Should().NotBeNull();
calculatedNextBaseline.Runs[0].Properties.Count.Should().Be(2);

string value = null;

// The default property bag matching behavior is to retain the oldest property bag available. Since we have no
// baseline in this test, we expect the preserved property bag to be associated with the first (i.e., oldest)
// run in the currentLogs property

calculatedNextBaseline.Runs[0].GetProperty(sharedPropertyName).Should().Be(currentSharedPropertyValue);
calculatedNextBaseline.Runs[0].TryGetProperty(uniqueToOldestPropertyName, out value).Should().BeTrue();
calculatedNextBaseline.Runs[0].GetProperty(uniqueToOldestPropertyName).Should().Be(uniqueToOldestPropertyValue);

calculatedNextBaseline.Runs[0].TryGetProperty(uniqueToMostRecentPropertyName, out value).Should().BeFalse();

calculatedNextBaseline = s_preserveMostRecentPropertyBagMatcher.Match(
previousLogs: null,
currentLogs: new SarifLog[] { oldestLog, mostRecentLog }).First();

calculatedNextBaseline.Runs[0].Properties.Should().NotBeNull();
calculatedNextBaseline.Runs[0].Properties.Count.Should().Be(2);

calculatedNextBaseline.Runs[0].GetProperty(sharedPropertyName).Should().Be(currentSharedPropertyValue);
calculatedNextBaseline.Runs[0].TryGetProperty(uniqueToMostRecentPropertyName, out value).Should().BeTrue();
calculatedNextBaseline.Runs[0].GetProperty(uniqueToMostRecentPropertyName).Should().Be(uniqueToMostRecentPropertyValue);

calculatedNextBaseline.Runs[0].TryGetProperty(uniqueToOldestPropertyName, out value).Should().BeFalse();
}

[Fact]
Expand All @@ -138,7 +211,7 @@ public void SarifLogResultMatcher_CurrentLogEmpty_AllAbsent()
result.CorrelationGuid = Guid.NewGuid().ToString();
}

SarifLog calculatedNextBaseline = baseliner.Match(new SarifLog[] { baselineLog }, new SarifLog[] { currentLog }).First();
SarifLog calculatedNextBaseline = s_preserveOldestPropertyBagMatcher.Match(new SarifLog[] { baselineLog }, new SarifLog[] { currentLog }).First();

calculatedNextBaseline.Runs.Should().HaveCount(1);

Expand All @@ -161,7 +234,7 @@ public void SarifLogResultMatcher_PreviousLogNull_WorksAsExpected()
SarifLog currentLog = RandomSarifLogGenerator.GenerateSarifLogWithRuns(random, 1);
currentLog.Runs[0].Id = new RunAutomationDetails { InstanceGuid = Guid.NewGuid().ToString() };

SarifLog calculatedNextBaseline = baseliner.Match(null, new SarifLog[] { currentLog }).First();
SarifLog calculatedNextBaseline = s_preserveOldestPropertyBagMatcher.Match(null, new SarifLog[] { currentLog }).First();

calculatedNextBaseline.Runs.Should().HaveCount(1);

Expand Down Expand Up @@ -197,7 +270,7 @@ public void SarifLogResultMatcher_PreviousLogEmpty_WorksAsExpected()
SarifLog currentLog = RandomSarifLogGenerator.GenerateSarifLogWithRuns(random, 1);
currentLog.Runs[0].Id = new RunAutomationDetails { InstanceGuid = Guid.NewGuid().ToString() };

SarifLog calculatedNextBaseline = baseliner.Match(new SarifLog[0], new SarifLog[] { currentLog }).First();
SarifLog calculatedNextBaseline = s_preserveOldestPropertyBagMatcher.Match(new SarifLog[0], new SarifLog[] { currentLog }).First();

calculatedNextBaseline.Runs.Should().HaveCount(1);

Expand Down Expand Up @@ -259,7 +332,7 @@ public void SarifLogResultMatcher_MultipleLogsDuplicateData_WorksAsExpected()
}
}
};
SarifLog result = baseliner.Match(new SarifLog[0], new SarifLog[] { current1, current2 }).First();
SarifLog result = s_preserveOldestPropertyBagMatcher.Match(new SarifLog[0], new SarifLog[] { current1, current2 }).First();

result.Runs[0].Files.Should().HaveCount(1);

Expand Down Expand Up @@ -298,7 +371,7 @@ public void SarifLogResultMatcher_MultipleLogsInvalidData_ThrowsInvalidOperation
}
}
};
Assert.Throws<InvalidOperationException>(() => baseliner.Match(new SarifLog[0], new SarifLog[] { current1, current2 }));
Assert.Throws<InvalidOperationException>(() => s_preserveOldestPropertyBagMatcher.Match(new SarifLog[0], new SarifLog[] { current1, current2 }));

}

Expand All @@ -315,23 +388,13 @@ public void SarifLogResultMatcher_PreservesPropertiesProperly ()
SetPropertyOnAllFileAndResultObjects(baselineLog, "Key", baselinePropertyValue);
SetPropertyOnAllFileAndResultObjects(currentLog, "Key", currentPropertyValue);

// Retain property bag values from baseline items
var matcher = new SarifLogResultMatcher(
exactResultMatchers: new IResultMatcher[] { ExactMatchers.ExactResultMatcherFactory.GetIdenticalResultMatcher(considerPropertyBagsWhenComparing: false) },
heuristicMatchers: null,
propertyBagMergeBehaviors: DictionaryMergeBehavior.InitializeFromPrevious);

SarifLog matchedLog = matcher.Match(baselineLog.DeepClone(), currentLog.DeepClone());
SarifLog matchedLog = s_preserveOldestPropertyBagMatcher.Match(baselineLog.DeepClone(), currentLog.DeepClone());
matchedLog.Runs[0].Results.Where((r) => { return r.GetProperty("Key") == baselinePropertyValue; }).Count().Should().Be(matchedLog.Runs[0].Results.Count);
matchedLog.Runs[0].Files.Values.Where((r) => { return r.GetProperty("Key") == baselinePropertyValue; }).Count().Should().Be(matchedLog.Runs[0].Files.Count);

matcher = new SarifLogResultMatcher(
exactResultMatchers: new IResultMatcher[] { ExactMatchers.ExactResultMatcherFactory.GetIdenticalResultMatcher(considerPropertyBagsWhenComparing: false) },
heuristicMatchers: null,
propertyBagMergeBehaviors: DictionaryMergeBehavior.InitializeFromCurrent);

// Retain property bag values from most current run
matchedLog = matcher.Match(baselineLog.DeepClone(), currentLog.DeepClone());
matchedLog = s_preserveMostRecentPropertyBagMatcher.Match(baselineLog.DeepClone(), currentLog.DeepClone());
matchedLog.Runs[0].Results.Where((r) => { return r.GetProperty("Key") == currentPropertyValue; }).Count().Should().Be(matchedLog.Runs[0].Results.Count);
matchedLog.Runs[0].Files.Values.Where((r) => { return r.GetProperty("Key") == currentPropertyValue; }).Count().Should().Be(matchedLog.Runs[0].Files.Count);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Sarif/Baseline/ResultMatching/DataStructures/MatchedResults.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public Result CalculateNewBaselineResult(DictionaryMergeBehavior propertyBagMerg
ResultMatchingProperties = MergeDictionaryPreferFirst(ResultMatchingProperties, OriginalResultMatchingProperties);

if (PreviousResult != null &&
propertyBagMergeBehavior == DictionaryMergeBehavior.InitializeFromPrevious)
propertyBagMergeBehavior == DictionaryMergeBehavior.InitializeFromOldest)
{
result.Properties = PreviousResult.Result.Properties;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Sarif/Baseline/ResultMatching/DictionaryMergeBehaviors.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ public enum DictionaryMergeBehavior
{
None = 0, // By default, we will always exclusively preserve the earliest
// properties that have been generated. We will not attempt any merging.
InitializeFromPrevious = None,
InitializeFromCurrent = 0x1, // On setting this bit, we will discard earlier properties
InitializeFromOldest = None,
InitializeFromMostRecent = 0x1,// On setting this bit, we will discard earlier properties
// in favor of those that are most recently generated.
}
}
25 changes: 16 additions & 9 deletions src/Sarif/Baseline/ResultMatching/SarifLogMatcher.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,14 +222,21 @@ private SarifLog ConstructSarifLogFromMatchedResults(
// We flow the baseline instance id forward (which becomes the
// baseline guid for the merged log)
run.BaselineInstanceGuid = previousRuns.First().Id?.InstanceGuid;
}


// We preserve either the most current or the oldest property bag associated with
// a run. Note that this implies ordering of current and previous runs with
// the most recent runs being at the front of the enumerable set. This is
// a non-obvious subtlety that we should look at resolving in a cleaned up design
properties = PropertyBagMergeBehavior.HasFlag(DictionaryMergeBehavior.InitializeFromCurrent)
? currentRuns.First().Properties
: previousRuns.Last().Properties;
if (PropertyBagMergeBehavior.HasFlag(DictionaryMergeBehavior.InitializeFromOldest))
{
// Find the 'oldest' log file and initialize properties from that log property bag
properties = previousRuns.FirstOrDefault() != null
? previousRuns.First().Properties
: currentRuns.First().Properties;
}
else
{
// Find the most recent log file instance and retain its property bag
// Find the 'oldest' log file and initialize properties from that log property bag
properties = currentRuns.Last().Properties;
}

properties = properties ?? new Dictionary<string, SerializedPropertyInfo>();
Expand Down Expand Up @@ -297,7 +304,7 @@ private SarifLog ConstructSarifLogFromMatchedResults(
invocations.AddRange(currentRun.Invocations);
}

if (PropertyBagMergeBehavior == DictionaryMergeBehavior.InitializeFromCurrent)
if (PropertyBagMergeBehavior == DictionaryMergeBehavior.InitializeFromMostRecent)
{
properties = currentRun.Properties;
}
Expand Down Expand Up @@ -381,7 +388,7 @@ internal static void MergeDictionaryInto<T, S>(

if (basePropertyBagHolder != null)
{
basePropertyBagHolder.Properties = propertyBagMergeBehavior == DictionaryMergeBehavior.InitializeFromCurrent
basePropertyBagHolder.Properties = propertyBagMergeBehavior == DictionaryMergeBehavior.InitializeFromMostRecent
? propertiesToMerge
: baseProperties;
}
Expand Down

0 comments on commit 5841151

Please sign in to comment.