From c4c164df6a18fc34b9ef47e62db1e9de0bf42908 Mon Sep 17 00:00:00 2001 From: Jason Dove <1695733+jasongdove@users.noreply.github.com> Date: Sun, 22 Jun 2025 21:55:35 -0500 Subject: [PATCH] detect cycles in yaml sequence definitions (#2060) --- CHANGELOG.md | 2 +- .../YamlScheduling/YamlPlayoutBuilder.cs | 26 +++++++++++++++++-- .../Search/AdjGraph.cs | 7 ++++- 3 files changed, 31 insertions(+), 4 deletions(-) rename {ErsatzTV.Infrastructure => ErsatzTV.Core}/Search/AdjGraph.cs (89%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f23dd81..74a41e2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - This makes it obvious when hardware acceleration will not work as configured - Add button in schedule editor to clone schedule item - Allow YAML playout sequence definitions to reference other sequences - - Playout builder will behave in unexpected ways if nesting is too deep + - Cycles will be detected and logged, and sequences with cycles will prevent the playout from building - Add `repeat` property to YAML sequence instruction - This tells the playout builder how many times this sequence should repeat - Omitting this value is the same as setting it to `1` diff --git a/ErsatzTV.Core/Scheduling/YamlScheduling/YamlPlayoutBuilder.cs b/ErsatzTV.Core/Scheduling/YamlScheduling/YamlPlayoutBuilder.cs index d07329b9..23f9e1ec 100644 --- a/ErsatzTV.Core/Scheduling/YamlScheduling/YamlPlayoutBuilder.cs +++ b/ErsatzTV.Core/Scheduling/YamlScheduling/YamlPlayoutBuilder.cs @@ -6,6 +6,7 @@ using ErsatzTV.Core.Interfaces.Repositories; using ErsatzTV.Core.Interfaces.Scheduling; using ErsatzTV.Core.Scheduling.YamlScheduling.Handlers; using ErsatzTV.Core.Scheduling.YamlScheduling.Models; +using ErsatzTV.Core.Search; using Microsoft.Extensions.Logging; using YamlDotNet.Serialization; using YamlDotNet.Serialization.NamingConventions; @@ -127,10 +128,16 @@ public class YamlPlayoutBuilder( } } - int flattenCount = 0; + if (DetectCycle(context.Definition)) + { + logger.LogError("YAML sequence contains a cycle; unable to build playout"); + return playout; + } + + var flattenCount = 0; while (context.Definition.Playout.Any(x => x is YamlPlayoutSequenceInstruction)) { - if (flattenCount > 10) + if (flattenCount > 100) { logger.LogError( "YAML playout definition contains sequence nesting that is too deep; this introduces undefined behavior"); @@ -197,6 +204,21 @@ public class YamlPlayoutBuilder( return playout; } + private static bool DetectCycle(YamlPlayoutDefinition definition) + { + var graph = new AdjGraph(); + + foreach (YamlPlayoutSequenceItem sequence in definition.Sequence) + { + foreach (YamlPlayoutSequenceInstruction instruction in sequence.Items.OfType()) + { + graph.AddEdge(sequence.Key, instruction.Sequence); + } + } + + return graph.HasAnyCycle(); + } + private async Task GetDaysToBuild() => await configElementRepository .GetValue(ConfigElementKey.PlayoutDaysToBuild) diff --git a/ErsatzTV.Infrastructure/Search/AdjGraph.cs b/ErsatzTV.Core/Search/AdjGraph.cs similarity index 89% rename from ErsatzTV.Infrastructure/Search/AdjGraph.cs rename to ErsatzTV.Core/Search/AdjGraph.cs index b23d8fed..fa15b2b6 100644 --- a/ErsatzTV.Infrastructure/Search/AdjGraph.cs +++ b/ErsatzTV.Core/Search/AdjGraph.cs @@ -1,4 +1,4 @@ -namespace ErsatzTV.Infrastructure.Search; +namespace ErsatzTV.Core.Search; public class AdjGraph { @@ -21,6 +21,11 @@ public class AdjGraph return HasCycleImpl(from.ToLowerInvariant(), visited, stack); } + public bool HasAnyCycle() + { + return _edges.Any(edge => HasCycle(edge.From)); + } + private bool HasCycleImpl(string node, ISet visited, ISet stack) { if (stack.Contains(node))