Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions src/ModularPipelines/Engine/Execution/DependencyWaiter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ public DependencyWaiter(
/// <inheritdoc />
public async Task WaitForDependenciesAsync(ModuleState moduleState, IModuleScheduler scheduler, IServiceProvider scopedServiceProvider)
{
var dependencies = ModuleDependencyResolver.GetDependencies(moduleState.ModuleType);
// Get both attribute-based and programmatic dependencies
var dependencies = GetAllDependencies(moduleState);

foreach (var (dependencyType, ignoreIfNotRegistered) in dependencies)
{
Expand All @@ -54,11 +55,29 @@ public async Task WaitForDependenciesAsync(ModuleState moduleState, IModuleSched
$"but '{dependencyType.Name}' has not been registered in the pipeline.\n\n" +
$"Suggestions:\n" +
$" 1. Add '.AddModule<{dependencyType.Name}>()' to your pipeline configuration before '.AddModule<{moduleState.ModuleType.Name}>()'\n" +
$" 2. Use '[DependsOn<{dependencyType.Name}>(ignoreIfNotRegistered: true)]' if this dependency is optional\n" +
$" 2. Use 'deps.DependsOnOptional<{dependencyType.Name}>()' or '[DependsOn<{dependencyType.Name}>(IgnoreIfNotRegistered = true)]' if this dependency is optional\n" +
$" 3. Check for typos in the dependency type name\n" +
$" 4. Verify that '{dependencyType.Name}' is in a project referenced by your pipeline project";
throw new ModuleNotRegisteredException(message, null);
}
}
}

/// <summary>
/// Gets all dependencies for a module, combining attribute-based and programmatic dependencies.
/// </summary>
private static IEnumerable<(Type DependencyType, bool IgnoreIfNotRegistered)> GetAllDependencies(ModuleState moduleState)
{
// Attribute-based dependencies
foreach (var dep in ModuleDependencyResolver.GetDependencies(moduleState.ModuleType))
{
yield return dep;
}

// Programmatic dependencies from DeclareDependencies method
foreach (var dep in ModuleDependencyResolver.GetProgrammaticDependencies(moduleState.Module))
{
yield return dep;
}
}
}
69 changes: 68 additions & 1 deletion src/ModularPipelines/Engine/ModuleDependencyResolver.cs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
using System.Reflection;
using ModularPipelines.Attributes;
using ModularPipelines.Engine.Dependencies;
using ModularPipelines.Enums;
using ModularPipelines.Extensions;
using ModularPipelines.Models;
using ModularPipelines.Modules;

namespace ModularPipelines.Engine;

/// <summary>
/// Resolves module dependencies by inspecting DependsOn attributes.
/// Resolves module dependencies by inspecting DependsOn attributes and programmatic declarations.
/// </summary>
internal static class ModuleDependencyResolver
{
Expand Down Expand Up @@ -65,6 +67,36 @@ internal static class ModuleDependencyResolver
return GetDependencies(module.GetType());
}

/// <summary>
/// Gets programmatic dependencies declared via DeclareDependencies method on a module instance.
/// </summary>
/// <param name="module">The module instance to get programmatic dependencies from.</param>
/// <returns>An enumerable of dependency tuples (DependencyType, IgnoreIfNotRegistered).</returns>
public static IEnumerable<(Type DependencyType, bool IgnoreIfNotRegistered)> GetProgrammaticDependencies(IModule module)
{
// Use reflection to call the internal GetDeclaredDependencies method
var moduleType = module.GetType();
var method = moduleType.GetMethod("GetDeclaredDependencies",
BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public);

if (method == null)
{
yield break;
}

var dependencies = method.Invoke(module, null) as IReadOnlyList<DeclaredDependency>;

if (dependencies == null)
{
yield break;
}

foreach (var dep in dependencies)
{
yield return (dep.DependencyType, dep.IgnoreIfNotRegistered);
}
}
Comment on lines +81 to +105
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using reflection with GetMethod on every call to GetProgrammaticDependencies could impact performance in pipelines with many modules. Consider caching the MethodInfo in a ConcurrentDictionary<Type, MethodInfo?> keyed by module type to avoid repeated reflection lookups.

Additionally, since GetDeclaredDependencies is an internal method defined on Module<T>, you could instead directly call it through a cast or interface rather than using reflection, which would be both faster and type-safe.

Copilot uses AI. Check for mistakes.

/// <summary>
/// Gets all dependencies declared on a module type via DependsOn attributes,
/// including both static (attribute-based) and dynamic (runtime-added) dependencies.
Expand All @@ -89,4 +121,39 @@ internal static class ModuleDependencyResolver
}
}
}

/// <summary>
/// Gets all dependencies declared on a module instance, including:
/// - Static dependencies from DependsOn attributes
/// - Programmatic dependencies from DeclareDependencies method
/// - Dynamic dependencies from the registry
/// </summary>
public static IEnumerable<(Type DependencyType, bool IgnoreIfNotRegistered)> GetAllDependencies(
IModule module,
IEnumerable<Type> availableModuleTypes,
IModuleDependencyRegistry? dynamicRegistry = null)
{
var moduleType = module.GetType();

// Static dependencies from attributes
foreach (var dep in GetDependencies(moduleType, availableModuleTypes))
{
yield return dep;
}

// Programmatic dependencies from DeclareDependencies method
foreach (var dep in GetProgrammaticDependencies(module))
{
yield return dep;
}

// Dynamic dependencies from registration
if (dynamicRegistry != null)
{
foreach (var dynamicDep in dynamicRegistry.GetDynamicDependencies(moduleType))
{
yield return (dynamicDep, false);
}
}
}
}
25 changes: 23 additions & 2 deletions src/ModularPipelines/Engine/ModuleScheduler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ public void InitializeModules(IEnumerable<IModule> modules)
}

// Use the overload that includes dynamic dependencies from registration events
var dependencies = ModuleDependencyResolver.GetAllDependencies(moduleType, availableModuleTypes, _dependencyRegistry);
// and programmatic dependencies from DeclareDependencies method
var dependencies = ModuleDependencyResolver.GetAllDependencies(state.Module, availableModuleTypes, _dependencyRegistry);

foreach (var (dependencyType, ignoreIfNotRegistered) in dependencies)
{
Expand Down Expand Up @@ -202,7 +203,9 @@ public void AddModule(IModule module)
{
// Resolve dependencies inside write lock to prevent race conditions
// where _moduleStates could change between resolution and processing
var dependencies = ModuleDependencyResolver.GetDependencies(moduleType);
var dependencies = ConcatDependencies(
ModuleDependencyResolver.GetDependencies(moduleType),
ModuleDependencyResolver.GetProgrammaticDependencies(module));

foreach (var (dependencyType, ignoreIfNotRegistered) in dependencies)
{
Expand Down Expand Up @@ -590,4 +593,22 @@ private bool CanExecuteRespectingConstraints(ModuleState moduleState)
// Delegate constraint checking to the evaluator
return _constraintEvaluator.CanQueue(moduleState, activeModules);
}

/// <summary>
/// Concatenates two dependency enumerables into a single sequence.
/// </summary>
private static IEnumerable<(Type DependencyType, bool IgnoreIfNotRegistered)> ConcatDependencies(
IEnumerable<(Type DependencyType, bool IgnoreIfNotRegistered)> first,
IEnumerable<(Type DependencyType, bool IgnoreIfNotRegistered)> second)
{
foreach (var dep in first)
{
yield return dep;
}

foreach (var dep in second)
{
yield return dep;
}
}
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This manual concatenation of two enumerables could be simplified using LINQ's Concat method, which is more idiomatic and reduces code:

private static IEnumerable<(Type DependencyType, bool IgnoreIfNotRegistered)> ConcatDependencies(
    IEnumerable<(Type DependencyType, bool IgnoreIfNotRegistered)> first,
    IEnumerable<(Type DependencyType, bool IgnoreIfNotRegistered)> second)
{
    return first.Concat(second);
}

Or better yet, inline the concatenation at the call site since it's only used once and is straightforward.

Copilot uses AI. Check for mistakes.
}
30 changes: 30 additions & 0 deletions src/ModularPipelines/Enums/DependencyType.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
using System.Text.Json.Serialization;

namespace ModularPipelines.Enums;

/// <summary>
/// Defines the type of dependency between modules.
/// </summary>
[JsonConverter(typeof(JsonStringEnumConverter<DependencyType>))]
public enum DependencyType
{
/// <summary>
/// Required dependency. The dependent module will fail if this dependency is not registered.
/// </summary>
Required = 0,

/// <summary>
/// Optional dependency. The dependent module will not fail if this dependency is not registered.
/// </summary>
Optional = 1,

/// <summary>
/// Lazy dependency. The dependency is only executed if explicitly awaited within the module.
/// </summary>
Lazy = 2,

/// <summary>
/// Conditional dependency. The dependency is only active if a condition is met.
/// </summary>
Conditional = 3,
}
Comment on lines +21 to +34
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DependencyType.Lazy enum value is defined but never actually checked or used in the execution engine. A search of the codebase shows it's only set in DeclaredDependency.Lazy() but never read or acted upon by the scheduler or dependency resolver.

This means lazy dependencies behave identically to optional dependencies (both have IgnoreIfNotRegistered = true), making the distinction meaningless. Either:

  1. Remove the Lazy enum value and DependsOnLazy methods, documenting that users should use DependsOnOptional instead
  2. Implement actual lazy evaluation semantics where lazy dependencies are only triggered when explicitly awaited

Currently, the code creates an illusion of different dependency types without providing the promised behavior.

Copilot uses AI. Check for mistakes.
39 changes: 39 additions & 0 deletions src/ModularPipelines/Models/DeclaredDependency.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
using ModularPipelines.Enums;

namespace ModularPipelines.Models;

/// <summary>
/// Represents a dependency declared programmatically via <see cref="Modules.IDependencyDeclaration"/>.
/// </summary>
/// <param name="DependencyType">The type of the module being depended on.</param>
/// <param name="Type">The type of dependency (Required, Optional, Lazy, Conditional).</param>
/// <param name="IgnoreIfNotRegistered">Whether to ignore this dependency if not registered.</param>
public readonly record struct DeclaredDependency(
Type DependencyType,
DependencyType Type,
bool IgnoreIfNotRegistered)
{
/// <summary>
/// Creates a required dependency.
/// </summary>
public static DeclaredDependency Required(Type type) =>
new(type, Enums.DependencyType.Required, false);

/// <summary>
/// Creates an optional dependency.
/// </summary>
public static DeclaredDependency Optional(Type type) =>
new(type, Enums.DependencyType.Optional, true);

/// <summary>
/// Creates a lazy dependency.
/// </summary>
public static DeclaredDependency Lazy(Type type) =>
new(type, Enums.DependencyType.Lazy, true);

/// <summary>
/// Creates a conditional dependency.
/// </summary>
public static DeclaredDependency Conditional(Type type) =>
new(type, Enums.DependencyType.Conditional, false);
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The property names in this record are confusing. The property DependencyType is of type Type (representing the module type), while the property Type is of type DependencyType enum (representing whether it's Required, Optional, etc.). This naming is backwards and makes the code harder to understand.

Consider renaming to make the purpose clearer:

  • DependencyTypeModuleType
  • TypeDependencyKind or Kind

This would make usage more intuitive: dep.ModuleType for the module type and dep.Kind for whether it's required/optional/lazy/conditional.

Suggested change
/// <param name="DependencyType">The type of the module being depended on.</param>
/// <param name="Type">The type of dependency (Required, Optional, Lazy, Conditional).</param>
/// <param name="IgnoreIfNotRegistered">Whether to ignore this dependency if not registered.</param>
public readonly record struct DeclaredDependency(
Type DependencyType,
DependencyType Type,
bool IgnoreIfNotRegistered)
{
/// <summary>
/// Creates a required dependency.
/// </summary>
public static DeclaredDependency Required(Type type) =>
new(type, Enums.DependencyType.Required, false);
/// <summary>
/// Creates an optional dependency.
/// </summary>
public static DeclaredDependency Optional(Type type) =>
new(type, Enums.DependencyType.Optional, true);
/// <summary>
/// Creates a lazy dependency.
/// </summary>
public static DeclaredDependency Lazy(Type type) =>
new(type, Enums.DependencyType.Lazy, true);
/// <summary>
/// Creates a conditional dependency.
/// </summary>
public static DeclaredDependency Conditional(Type type) =>
new(type, Enums.DependencyType.Conditional, false);
/// <param name="ModuleType">The type of the module being depended on.</param>
/// <param name="DependencyKind">The type of dependency (Required, Optional, Lazy, Conditional).</param>
/// <param name="IgnoreIfNotRegistered">Whether to ignore this dependency if not registered.</param>
public readonly record struct DeclaredDependency(
Type ModuleType,
DependencyType DependencyKind,
bool IgnoreIfNotRegistered)
{
/// <summary>
/// Backwards-compatible alias for <see cref="ModuleType"/>.
/// </summary>
[Obsolete("Use ModuleType instead.")]
public Type DependencyType => ModuleType;
/// <summary>
/// Backwards-compatible alias for <see cref="DependencyKind"/>.
/// </summary>
[Obsolete("Use DependencyKind instead.")]
public DependencyType Type => DependencyKind;
/// <summary>
/// Creates a required dependency.
/// </summary>
public static DeclaredDependency Required(Type moduleType) =>
new(moduleType, Enums.DependencyType.Required, false);
/// <summary>
/// Creates an optional dependency.
/// </summary>
public static DeclaredDependency Optional(Type moduleType) =>
new(moduleType, Enums.DependencyType.Optional, true);
/// <summary>
/// Creates a lazy dependency.
/// </summary>
public static DeclaredDependency Lazy(Type moduleType) =>
new(moduleType, Enums.DependencyType.Lazy, true);
/// <summary>
/// Creates a conditional dependency.
/// </summary>
public static DeclaredDependency Conditional(Type moduleType) =>
new(moduleType, Enums.DependencyType.Conditional, false);

Copilot uses AI. Check for mistakes.
}
111 changes: 111 additions & 0 deletions src/ModularPipelines/Modules/DependencyDeclaration.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
using ModularPipelines.Enums;
using ModularPipelines.Models;

namespace ModularPipelines.Modules;

/// <summary>
/// Implementation of <see cref="IDependencyDeclaration"/> for collecting programmatic dependencies.
/// </summary>
/// <remarks>
/// <para>
/// <b>Thread Safety:</b> This class is not thread-safe. It is designed to be used
/// during the synchronous <see cref="Module{T}.DeclareDependencies"/> method call
/// and should not be accessed from multiple threads.
/// </para>
/// </remarks>
internal sealed class DependencyDeclaration : IDependencyDeclaration
{
private readonly List<DeclaredDependency> _dependencies = new();

/// <summary>
/// Gets the declared dependencies.
/// </summary>
public IReadOnlyList<DeclaredDependency> Dependencies => _dependencies;

/// <inheritdoc />
public IDependencyDeclaration DependsOn<TModule>() where TModule : IModule
{
return DependsOn(typeof(TModule));
}

/// <inheritdoc />
public IDependencyDeclaration DependsOn(Type moduleType)
{
ValidateModuleType(moduleType);
_dependencies.Add(DeclaredDependency.Required(moduleType));
return this;
}

/// <inheritdoc />
public IDependencyDeclaration DependsOnOptional<TModule>() where TModule : IModule
{
return DependsOnOptional(typeof(TModule));
}

/// <inheritdoc />
public IDependencyDeclaration DependsOnOptional(Type moduleType)
{
ValidateModuleType(moduleType);
_dependencies.Add(DeclaredDependency.Optional(moduleType));
return this;
}

/// <inheritdoc />
public IDependencyDeclaration DependsOnIf<TModule>(bool condition) where TModule : IModule
{
return DependsOnIf(typeof(TModule), condition);
}

/// <inheritdoc />
public IDependencyDeclaration DependsOnIf<TModule>(Func<bool> predicate) where TModule : IModule
{
ArgumentNullException.ThrowIfNull(predicate);
return DependsOnIf(typeof(TModule), predicate());
}

/// <inheritdoc />
public IDependencyDeclaration DependsOnIf(Type moduleType, bool condition)
{
if (!condition)
{
return this;
}

ValidateModuleType(moduleType);
_dependencies.Add(DeclaredDependency.Conditional(moduleType));
return this;
}

/// <inheritdoc />
public IDependencyDeclaration DependsOnIf(Type moduleType, Func<bool> predicate)
{
ArgumentNullException.ThrowIfNull(predicate);
return DependsOnIf(moduleType, predicate());
}

/// <inheritdoc />
public IDependencyDeclaration DependsOnLazy<TModule>() where TModule : IModule
{
return DependsOnLazy(typeof(TModule));
}

/// <inheritdoc />
public IDependencyDeclaration DependsOnLazy(Type moduleType)
{
ValidateModuleType(moduleType);
_dependencies.Add(DeclaredDependency.Lazy(moduleType));
return this;
}

private static void ValidateModuleType(Type moduleType)
{
ArgumentNullException.ThrowIfNull(moduleType);

if (!moduleType.IsAssignableTo(typeof(IModule)))
{
throw new ArgumentException(
$"{moduleType.FullName} is not a Module (does not implement IModule)",
nameof(moduleType));
}
}
}
Loading
Loading