diff --git a/src/Runner.Worker/ActionCommandManager.cs b/src/Runner.Worker/ActionCommandManager.cs index 1691d0eb0..e77bf8136 100644 --- a/src/Runner.Worker/ActionCommandManager.cs +++ b/src/Runner.Worker/ActionCommandManager.cs @@ -646,7 +646,8 @@ namespace GitHub.Runner.Worker string keyToExclude = Constants.Runner.InternalTelemetryIssueDataKey; var filteredDictionaryEntries = command.Properties - .Where(kvp => !string.Equals(kvp.Key, keyToExclude, StringComparison.OrdinalIgnoreCase)); + .Where(kvp => !string.Equals(kvp.Key, keyToExclude, StringComparison.OrdinalIgnoreCase)) + .ToList(); var metadata = new IssueMetadata(issueCategory, false, null, filteredDictionaryEntries); var issue = context.CreateIssue(this.Type, command.Data, metadata, true); diff --git a/src/Sdk/DTWebApi/WebApi/Issue.cs b/src/Sdk/DTWebApi/WebApi/Issue.cs index 3566a3d54..2235dd861 100644 --- a/src/Sdk/DTWebApi/WebApi/Issue.cs +++ b/src/Sdk/DTWebApi/WebApi/Issue.cs @@ -101,11 +101,6 @@ namespace GitHub.DistributedTask.WebApi m_serializedData = null; } - [DataMember(Name = "Data", EmitDefaultValue = false, Order = 4)] - private IDictionary m_serializedData; - - private IDictionary m_data; - /// /// DataContractSerializer bypasses all constructor logic and inline initialization! /// This method takes the place of a workhorse constructor for baseline initialization. @@ -113,7 +108,14 @@ namespace GitHub.DistributedTask.WebApi /// private void EnsureInitialized() { + //Note that ?? is a short-circuiting operator. m_data = m_data ?? new Dictionary(StringComparer.OrdinalIgnoreCase); } + + [DataMember(Name = "Data", EmitDefaultValue = false, Order = 4)] + private IDictionary m_serializedData; + + private IDictionary m_data; + } } diff --git a/src/Sdk/DTWebApi/WebApi/TimelineRecord.cs b/src/Sdk/DTWebApi/WebApi/TimelineRecord.cs index 51317d171..e42607e10 100644 --- a/src/Sdk/DTWebApi/WebApi/TimelineRecord.cs +++ b/src/Sdk/DTWebApi/WebApi/TimelineRecord.cs @@ -10,69 +10,76 @@ namespace GitHub.DistributedTask.WebApi public sealed class TimelineRecord { public TimelineRecord() + : this(null) { - this.Attempt = 1; } private TimelineRecord(TimelineRecord recordToBeCloned) { - this.Attempt = recordToBeCloned.Attempt; - this.ChangeId = recordToBeCloned.ChangeId; - this.CurrentOperation = recordToBeCloned.CurrentOperation; - this.FinishTime = recordToBeCloned.FinishTime; - this.Id = recordToBeCloned.Id; - this.Identifier = recordToBeCloned.Identifier; - this.LastModified = recordToBeCloned.LastModified; - this.Location = recordToBeCloned.Location; - this.Name = recordToBeCloned.Name; - this.Order = recordToBeCloned.Order; - this.ParentId = recordToBeCloned.ParentId; - this.PercentComplete = recordToBeCloned.PercentComplete; - this.RecordType = recordToBeCloned.RecordType; - this.Result = recordToBeCloned.Result; - this.ResultCode = recordToBeCloned.ResultCode; - this.StartTime = recordToBeCloned.StartTime; - this.State = recordToBeCloned.State; - this.TimelineId = recordToBeCloned.TimelineId; - this.WorkerName = recordToBeCloned.WorkerName; - this.RefName = recordToBeCloned.RefName; - this.ErrorCount = recordToBeCloned.ErrorCount; - this.WarningCount = recordToBeCloned.WarningCount; - this.NoticeCount = recordToBeCloned.NoticeCount; - this.AgentPlatform = recordToBeCloned.AgentPlatform; - - if (recordToBeCloned.Log != null) + this.EnsureInitialized(); + if (recordToBeCloned != null) { - this.Log = new TaskLogReference + this.Attempt = recordToBeCloned.Attempt; + this.ChangeId = recordToBeCloned.ChangeId; + this.CurrentOperation = recordToBeCloned.CurrentOperation; + this.FinishTime = recordToBeCloned.FinishTime; + this.Id = recordToBeCloned.Id; + this.Identifier = recordToBeCloned.Identifier; + this.LastModified = recordToBeCloned.LastModified; + this.Location = recordToBeCloned.Location; + this.Name = recordToBeCloned.Name; + this.Order = recordToBeCloned.Order; + this.ParentId = recordToBeCloned.ParentId; + this.PercentComplete = recordToBeCloned.PercentComplete; + this.RecordType = recordToBeCloned.RecordType; + this.Result = recordToBeCloned.Result; + this.ResultCode = recordToBeCloned.ResultCode; + this.StartTime = recordToBeCloned.StartTime; + this.State = recordToBeCloned.State; + this.TimelineId = recordToBeCloned.TimelineId; + this.WorkerName = recordToBeCloned.WorkerName; + this.RefName = recordToBeCloned.RefName; + this.ErrorCount = recordToBeCloned.ErrorCount; + this.WarningCount = recordToBeCloned.WarningCount; + this.NoticeCount = recordToBeCloned.NoticeCount; + this.AgentPlatform = recordToBeCloned.AgentPlatform; + + if (recordToBeCloned.Log != null) { - Id = recordToBeCloned.Log.Id, - Location = recordToBeCloned.Log.Location, - }; - } + this.Log = new TaskLogReference + { + Id = recordToBeCloned.Log.Id, + Location = recordToBeCloned.Log.Location, + }; + } - if (recordToBeCloned.Details != null) - { - this.Details = new TimelineReference + if (recordToBeCloned.Details != null) { - ChangeId = recordToBeCloned.Details.ChangeId, - Id = recordToBeCloned.Details.Id, - Location = recordToBeCloned.Details.Location, - }; - } + this.Details = new TimelineReference + { + ChangeId = recordToBeCloned.Details.ChangeId, + Id = recordToBeCloned.Details.Id, + Location = recordToBeCloned.Details.Location, + }; + } - if (recordToBeCloned.m_issues?.Count> 0) - { - this.Issues.AddRange(recordToBeCloned.Issues.Select(i => i.Clone())); - } + if (recordToBeCloned.m_issues?.Count > 0) + { + m_issues.AddRange(recordToBeCloned.m_issues.Select(i => i.Clone())); + } - if (recordToBeCloned.m_previousAttempts?.Count > 0) - { - this.PreviousAttempts.AddRange(recordToBeCloned.PreviousAttempts); - } + if (recordToBeCloned.m_previousAttempts?.Count > 0) + { + m_previousAttempts.AddRange(recordToBeCloned.m_previousAttempts); + } - if (recordToBeCloned.m_variables?.Count > 0) - { - this.m_variables = recordToBeCloned.Variables.ToDictionary(k => k.Key, v => v.Value.Clone()); + if (recordToBeCloned.m_variables?.Count > 0) + { + // Don't pave over the case-insensitive Dictionary we initialized above. + foreach (var kvp in recordToBeCloned.m_variables) { + m_variables[kvp.Key] = kvp.Value.Clone(); + } + } } } @@ -234,10 +241,6 @@ namespace GitHub.DistributedTask.WebApi { get { - if (m_issues == null) - { - m_issues = new List(); - } return m_issues; } } @@ -274,22 +277,14 @@ namespace GitHub.DistributedTask.WebApi { get { - if (m_previousAttempts == null) - { - m_previousAttempts = new List(); - } return m_previousAttempts; } } - public IDictionary Variables + public IDictionary Variables { get { - if (m_variables == null) - { - m_variables = new Dictionary(StringComparer.OrdinalIgnoreCase); - } return m_variables; } } @@ -299,11 +294,32 @@ namespace GitHub.DistributedTask.WebApi return new TimelineRecord(this); } + [OnDeserialized] + private void OnDeserialized(StreamingContext context) + { + this.EnsureInitialized(); + } + + /// + /// DataContractSerializer bypasses all constructor logic and inline initialization! + /// This method takes the place of a workhorse constructor for baseline initialization. + /// The expectation is for this logic to be accessible to constructors and also to the OnDeserialized helper. + /// + private void EnsureInitialized() + { + //Note that ?? is a short-circuiting operator. + m_issues = m_issues ?? new List(); + m_variables = m_variables ?? new Dictionary(StringComparer.OrdinalIgnoreCase); + m_previousAttempts = m_previousAttempts ?? new List(); + this.Attempt = Math.Max(this.Attempt, 1); + } + + [DataMember(Name = "Issues", EmitDefaultValue = false, Order = 60)] private List m_issues; [DataMember(Name = "Variables", EmitDefaultValue = false, Order = 80)] - private Dictionary m_variables; + private Dictionary m_variables; [DataMember(Name = "PreviousAttempts", EmitDefaultValue = false, Order = 120)] private List m_previousAttempts;