Skip to content

Commit 0005a54

Browse files
MrHinshMrHinsh
committed
Refactor configuration validation to improve error reporting and remove obsolete checks
1 parent 7b1d7f0 commit 0005a54

File tree

3 files changed

+285
-22
lines changed

3 files changed

+285
-22
lines changed

‎configuration-default.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
"Endpoints": {
88
"Source": {
99
"EndpointType": "TfsTeamProjectEndpoint",
10-
"Collection": "https://dev.azure.com/nkdagility-preview/",
11-
"Project": "migrationSource1",
10+
"missing": "https://dev.azure.com/nkdagility-preview/",
11+
"missing2": "migrationSource1",
1212
"AllowCrossProjectLinking": false,
1313
"ReflectedWorkItemIdField": "Custom.ReflectedWorkItemId",
1414
"Authentication": {

‎src/MigrationTools.Host/Commands/ExecuteMigrationCommand.cs‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,19 +50,9 @@ internal override Task<int> ExecuteInternalAsync(CommandContext context, Execute
5050
int _exitCode;
5151
try
5252
{
53-
// KILL if the config is not valid
54-
// if (!VersionOptions.ConfigureOptions.IsConfigValid(Configuration))
55-
// {
56-
// CommandActivity.AddEvent(new ActivityEvent("ConfigIsNotValid"));
57-
// BoilerplateCli.ConfigIsNotValidMessage(Configuration, Log.Logger);
58-
// return Task.FromResult(-1);
59-
// }
60-
// KILL if the schema is not valid
61-
6253
if (!VersionOptions.ConfigureOptions.IsConfigSchemaValid(settings.ConfigFile))
6354
{
6455
CommandActivity.AddEvent(new ActivityEvent("ConfigIsNotValid"));
65-
BoilerplateCli.ConfigIsNotValidMessage(Configuration, Log.Logger);
6656
return Task.FromResult(-1);
6757
}
6858

‎src/MigrationTools/Options/VersionOptions.cs‎

Lines changed: 283 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
using Microsoft.Extensions.Options;
99
using Newtonsoft.Json.Linq;
1010
using Newtonsoft.Json.Schema;
11+
using Newtonsoft.Json; // for IJsonLineInfo
1112
using Serilog;
13+
using System.Text.RegularExpressions;
1214

1315
namespace MigrationTools.Options
1416
{
@@ -111,18 +113,13 @@ public static bool IsConfigSchemaValid(string configFile)
111113

112114
var schema = LoadConfigurationSchema();
113115
// Validate the configuration against the schema
114-
IList<string> messages;
115-
bool isValid = configJson.IsValid(schema, out messages);
116+
IList<ValidationError> validationErrors;
117+
bool isValid = configJson.IsValid(schema, out validationErrors);
116118
if (!isValid)
117119
{
118-
StringBuilder sb = new StringBuilder();
119-
sb.AppendLine($"Configuration file '{configFile}' is not valid against the schema.");
120-
// Log the validation errors (but still return true for backward compatibility)
121-
foreach (var message in messages)
122-
{
123-
sb.AppendLine($"{message}");
124-
}
125-
Log.Error(sb.ToString());
120+
// Build a rich, de-duplicated error report including child errors
121+
var report = BuildValidationErrorReport(configFile, validationErrors);
122+
Log.Error(report);
126123
}
127124
return isValid;
128125
}
@@ -246,6 +243,282 @@ private static JSchema LoadConfigurationSchema()
246243
return null;
247244
}
248245
}
246+
247+
#region Validation Error Helpers
248+
private static string BuildValidationErrorReport(string configFile, IEnumerable<ValidationError> rootErrors)
249+
{
250+
var sb = new StringBuilder();
251+
sb.AppendLine($"Configuration file '{configFile}' is not valid against the schema.");
252+
sb.AppendLine();
253+
sb.AppendLine("Validation errors (deduplicated & filtered):");
254+
sb.AppendLine();
255+
256+
var flattened = FlattenValidationErrors(rootErrors).ToList();
257+
// Count raw required property set occurrences before deduplication
258+
var requiredSetCounts = new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase); // key = path|prop1,prop2
259+
foreach (var fe in flattened.Where(f => f.ErrorType == ErrorType.Required))
260+
{
261+
var props = ParseRequiredProperties(fe);
262+
if (props.Count == 0) continue;
263+
var key = $"{fe.Path ?? string.Empty}|{string.Join(",", props)}";
264+
requiredSetCounts[key] = requiredSetCounts.TryGetValue(key, out var c) ? c + 1 : 1;
265+
}
266+
267+
var unique = DeduplicateValidationErrors(flattened);
268+
unique = FilterNoise(unique); // now only removes AnyOf/AllOf containers
269+
270+
// Map directly for display (keep all required variants)
271+
var displayErrors = ToDisplayErrors(configFile, unique);
272+
273+
// Build full root cause candidate list (all required sets per path)
274+
var rootCauseSetsAll = displayErrors.Where(e => e.ErrorType == ErrorType.Required)
275+
.Select(e => new { e.Path, Props = ParseRequiredPropertiesFromMessage(e.Message) })
276+
.Where(x => x.Props.Count > 0)
277+
.Select(x => new {
278+
x.Path,
279+
x.Props,
280+
Freq = requiredSetCounts.TryGetValue($"{x.Path ?? string.Empty}|{string.Join(",", x.Props)}", out var f) ? f : 1
281+
})
282+
.ToList();
283+
284+
// Filter to frequency > 1; if none qualify keep all to avoid hiding everything
285+
var rootCauseSetsFiltered = rootCauseSetsAll.Where(r => r.Freq > 1).ToList();
286+
if (rootCauseSetsFiltered.Count == 0)
287+
{
288+
rootCauseSetsFiltered = rootCauseSetsAll; // fallback
289+
}
290+
var rootCauseSets = rootCauseSetsFiltered
291+
.GroupBy(x => x.Path ?? string.Empty)
292+
.ToList();
293+
if (rootCauseSets.Count > 0)
294+
{
295+
sb.AppendLine("Likely root cause missing properties (all deduped candidate sets):");
296+
foreach (var pathGroup in rootCauseSets.OrderBy(g => g.Key, StringComparer.OrdinalIgnoreCase))
297+
{
298+
sb.AppendLine($" Path: {pathGroup.Key}");
299+
foreach (var candidate in pathGroup
300+
.Select(c => new { c.Props, FreqKey = $"{pathGroup.Key}|{string.Join(",", c.Props)}" })
301+
.GroupBy(c => string.Join(",", c.Props))
302+
.Select(g => new { Props = g.First().Props, Frequency = requiredSetCounts.TryGetValue(g.First().FreqKey, out var f) ? f : 1 })
303+
.OrderBy(c => c.Props.Count)
304+
.ThenByDescending(c => c.Frequency)
305+
.ThenBy(c => string.Join(",", c.Props), StringComparer.OrdinalIgnoreCase))
306+
{
307+
sb.AppendLine($" - missing {string.Join(", ", candidate.Props)} (freq {candidate.Frequency})");
308+
}
309+
}
310+
sb.AppendLine();
311+
}
312+
313+
foreach (var group in displayErrors
314+
.OrderBy(e => e.Path, StringComparer.OrdinalIgnoreCase)
315+
.ThenBy(e => e.ErrorType.ToString(), StringComparer.OrdinalIgnoreCase)
316+
.GroupBy(e => string.IsNullOrEmpty(e.Path) ? "<root>" : e.Path))
317+
{
318+
sb.AppendLine($"Path: {group.Key}");
319+
foreach (var error in group.OrderBy(e => e.ErrorType.ToString(), StringComparer.OrdinalIgnoreCase)
320+
.ThenBy(e => e.Message, StringComparer.OrdinalIgnoreCase))
321+
{
322+
var lineInfo = error.HasLineInfo ? $"(Ln {error.LineNumber}, Pos {error.LinePosition}) " : string.Empty;
323+
sb.AppendLine($" - {lineInfo}{error.ErrorType}: {error.Message}");
324+
}
325+
}
326+
sb.AppendLine();
327+
sb.AppendLine($"Total unique validation errors after filtering: {displayErrors.Count}");
328+
sb.AppendLine();
329+
sb.AppendLine("NOTE: A single missing property can trigger multiple branch (anyOf/allOf) mismatches. Fix the root cause lines above first.");
330+
return sb.ToString();
331+
}
332+
333+
private class DisplayValidationError
334+
{
335+
public string Path { get; set; }
336+
public ErrorType ErrorType { get; set; }
337+
public string Message { get; set; }
338+
public int LineNumber { get; set; }
339+
public int LinePosition { get; set; }
340+
public bool HasLineInfo { get; set; }
341+
}
342+
343+
private static List<DisplayValidationError> ToDisplayErrors(string configFile, List<ValidationError> errors)
344+
{
345+
JObject root = null;
346+
try { root = JObject.Parse(File.ReadAllText(configFile)); } catch { }
347+
var display = new List<DisplayValidationError>();
348+
if (root == null)
349+
{
350+
// Fallback, just map directly
351+
foreach (var e in errors)
352+
{
353+
var hasLine = ((IJsonLineInfo)e).HasLineInfo();
354+
display.Add(new DisplayValidationError
355+
{
356+
Path = e.Path,
357+
ErrorType = e.ErrorType,
358+
Message = e.Message,
359+
LineNumber = e.LineNumber,
360+
LinePosition = e.LinePosition,
361+
HasLineInfo = hasLine
362+
});
363+
}
364+
return display;
365+
}
366+
367+
foreach (var e in errors)
368+
{
369+
var hasLine = ((IJsonLineInfo)e).HasLineInfo();
370+
display.Add(new DisplayValidationError
371+
{
372+
Path = e.Path,
373+
ErrorType = e.ErrorType,
374+
Message = e.Message,
375+
LineNumber = e.LineNumber,
376+
LinePosition = e.LinePosition,
377+
HasLineInfo = hasLine
378+
});
379+
}
380+
return display;
381+
}
382+
383+
private static IEnumerable<ValidationError> FlattenValidationErrors(IEnumerable<ValidationError> errors)
384+
{
385+
var stack = new Stack<ValidationError>(errors ?? Array.Empty<ValidationError>());
386+
while (stack.Count > 0)
387+
{
388+
var current = stack.Pop();
389+
yield return current;
390+
// ChildErrors always returns a list (never null)
391+
if (current.ChildErrors.Count > 0)
392+
{
393+
foreach (var child in current.ChildErrors)
394+
{
395+
stack.Push(child);
396+
}
397+
}
398+
}
399+
}
400+
401+
private static List<ValidationError> DeduplicateValidationErrors(IEnumerable<ValidationError> errors)
402+
{
403+
var unique = new List<ValidationError>();
404+
var seen = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
405+
foreach (var e in errors)
406+
{
407+
// Compose a key that should uniquely identify a validation error occurrence
408+
var key = $"{e.Path}|{e.ErrorType}|{e.Message}";
409+
if (seen.Add(key))
410+
{
411+
unique.Add(e);
412+
}
413+
}
414+
return unique;
415+
}
416+
417+
private static List<ValidationError> FilterNoise(List<ValidationError> errors)
418+
{
419+
var toRemove = new HashSet<ValidationError>();
420+
foreach (var group in errors.GroupBy(e => e.Path ?? string.Empty))
421+
{
422+
// Remove container anyOf/allOf if we have more specific errors
423+
bool hasSpecific = group.Any(e => e.ErrorType != ErrorType.AnyOf && e.ErrorType != ErrorType.AllOf);
424+
if (hasSpecific)
425+
{
426+
foreach (var container in group.Where(e => e.ErrorType == ErrorType.AnyOf || e.ErrorType == ErrorType.AllOf))
427+
{
428+
toRemove.Add(container);
429+
}
430+
}
431+
}
432+
return errors.Where(e => !toRemove.Contains(e)).ToList();
433+
}
434+
435+
private static bool IsSinglePropertyRequiredMessage(ValidationError e)
436+
{
437+
var msg = e.Message ?? string.Empty;
438+
var m = Regex.Match(msg, @"Required properties are missing from object: (?<props>[^.]+)\.");
439+
if (!m.Success) return false;
440+
var props = m.Groups["props"].Value.Split(',').Select(p => p.Trim()).Where(p => p.Length > 0).ToList();
441+
return props.Count == 1;
442+
}
443+
444+
private static List<DisplayValidationError> IdentifyRootCauseRequiredDisplayErrors(List<DisplayValidationError> errors) => errors.Where(e => e.ErrorType == ErrorType.Required).ToList();
445+
446+
// Use the actual JSON object to collapse Required errors to the truly missing properties
447+
// Removed obsolete refinement returning ValidationError list (replaced by ToDisplayErrors)
448+
449+
// Best-effort shallow path resolution using Newtonsoft path semantics
450+
private static JToken GetTokenAtPath(JObject root, string path)
451+
{
452+
if (root == null) return null;
453+
if (string.IsNullOrEmpty(path) || path == "<root>") return root;
454+
try { return root.SelectToken(path); } catch { return null; }
455+
}
456+
457+
// Case-insensitive manual traversal fallback (handles minor casing differences or escaped segments)
458+
private static JToken GetTokenAtPathLoose(JObject root, string path)
459+
{
460+
if (root == null) return null;
461+
if (string.IsNullOrEmpty(path) || path == "<root>") return root;
462+
var current = (JToken)root;
463+
var parts = path.Split('.');
464+
foreach (var part in parts)
465+
{
466+
if (current is JObject obj)
467+
{
468+
var prop = obj.Properties().FirstOrDefault(p => string.Equals(p.Name, part, StringComparison.OrdinalIgnoreCase));
469+
if (prop == null) return null;
470+
current = prop.Value;
471+
}
472+
else
473+
{
474+
return null;
475+
}
476+
}
477+
return current;
478+
}
479+
480+
// --- End of display conversion helpers ---
481+
482+
private static List<string> ParseRequiredProperties(ValidationError e)
483+
{
484+
var msg = e.Message ?? string.Empty;
485+
var m = Regex.Match(msg, @"Required properties are missing from object: (?<props>[^.]+)\.");
486+
if (!m.Success) return new List<string>();
487+
return m.Groups["props"].Value
488+
.Split(',')
489+
.Select(p => p.Trim())
490+
.Where(p => p.Length > 0)
491+
.Distinct(StringComparer.OrdinalIgnoreCase)
492+
.OrderBy(p => p, StringComparer.OrdinalIgnoreCase)
493+
.ToList();
494+
}
495+
496+
private static List<string> ParseRequiredPropertiesFromMessage(string msg)
497+
{
498+
msg = msg ?? string.Empty;
499+
var m = Regex.Match(msg, @"Required properties are missing from object: (?<props>[^.]+)\.");
500+
if (!m.Success) return new List<string>();
501+
return m.Groups["props"].Value
502+
.Split(',')
503+
.Select(p => p.Trim())
504+
.Where(p => p.Length > 0)
505+
.Distinct(StringComparer.OrdinalIgnoreCase)
506+
.OrderBy(p => p, StringComparer.OrdinalIgnoreCase)
507+
.ToList();
508+
}
509+
510+
private static bool IsStrictSuperset(IReadOnlyCollection<string> a, IReadOnlyCollection<string> b)
511+
{
512+
if (a.Count <= b.Count) return false;
513+
return b.All(x => a.Contains(x, StringComparer.OrdinalIgnoreCase));
514+
}
515+
516+
private static bool IsStrictSubset(IReadOnlyCollection<string> a, IReadOnlyCollection<string> b)
517+
{
518+
if (a.Count >= b.Count) return false;
519+
return a.All(x => b.Contains(x, StringComparer.OrdinalIgnoreCase));
520+
}
521+
#endregion
249522
}
250523

251524

0 commit comments

Comments
 (0)