Feat/hangfire add timerange attribute

[kuberto773]

No description provided.

[Copilot]

Pull request overview

This PR adds a new AllowedExecutionTimeRangeAttribute to the Hangfire infrastructure library that restricts background job execution to specific time windows. However, the PR also includes an incorrect framework version upgrade that must be addressed.

Changes:

• Added AllowedExecutionTimeRangeAttribute to control when Hangfire jobs can execute based on time ranges
• Upgraded target framework from net9.0 to net10.0 (CRITICAL: this version does not exist)
• Updated Hangfire.Core package from version 1.8.22 to 1.8.23

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
Madev.Utils.Infrastructure.Hangfire.csproj	Upgrades target framework to non-existent net10.0 and updates Hangfire.Core to 1.8.23
Attributes/AllowedExecutionTimeRangeAttribute.cs	New filter attribute that restricts job execution to configurable time ranges with hardcoded Central European timezone

Comments suppressed due to low confidence (7)

src/Madev.Utils.Infrastructure.Hangfire/Attributes/AllowedExecutionTimeRangeAttribute.cs:44

• The method JobIsAllowedInActualTime throws an exception if timeFrom equals timeTo, but this check happens during job execution rather than in the constructor. This means invalid configurations won't be detected until the job tries to run. Consider validating this in the constructor and throwing an ArgumentException there instead.

        if (_timeFrom == _timeTo)
            throw new Exception("Duration cannot be 0h0m0s");

src/Madev.Utils.Infrastructure.Hangfire/Attributes/AllowedExecutionTimeRangeAttribute.cs:21

• Missing input validation for the constructor parameters. The timeFrom and timeTo strings should be validated to ensure they match the expected "HH:mm:ss" format before calling ParseExact. Without validation, ParseExact will throw a generic FormatException with unclear error messages if the format is incorrect. Consider wrapping the ParseExact calls in a try-catch block and throwing an ArgumentException with a clear message explaining the expected format.

    public AllowedExecutionTimeRangeAttribute(string timeFrom, string timeTo)
    {
        _timeFrom = DateTime.ParseExact(timeFrom, "HH:mm:ss", CultureInfo.InvariantCulture).TimeOfDay;
        _timeTo = DateTime.ParseExact(timeTo, "HH:mm:ss", CultureInfo.InvariantCulture).TimeOfDay;
    }

src/Madev.Utils.Infrastructure.Hangfire/Attributes/AllowedExecutionTimeRangeAttribute.cs:34

• Creating an ArgumentOutOfRangeException and using it as a FailedState is misleading. ArgumentOutOfRangeException is meant for parameter validation, not for business logic failures. Consider creating a custom exception type or using a more appropriate exception type for this scenario.

            context.CandidateState = new FailedState(new ArgumentOutOfRangeException($"It is not allowed to perform the task at {dateTimeNow}"))

src/Madev.Utils.Infrastructure.Hangfire/Attributes/AllowedExecutionTimeRangeAttribute.cs:67

• The method GetTimeInCentralEuropeStandardTime is public but appears to be an implementation detail. According to the existing DisableMultipleQueuedItemsFilterAttribute pattern in the codebase, helper methods should be private. Consider making this method private unless there's a specific reason for it to be public.

    public TimeSpan GetTimeInCentralEuropeStandardTime(DateTime dateTime)

src/Madev.Utils.Infrastructure.Hangfire/Attributes/AllowedExecutionTimeRangeAttribute.cs:41

• The method JobIsAllowedInActualTime is public but appears to be an implementation detail. According to the existing DisableMultipleQueuedItemsFilterAttribute pattern in the codebase where helper methods like ActiveProcess are private, consider making this method private unless there's a specific reason for it to be public.

    public bool JobIsAllowedInActualTime(TimeSpan now)

src/Madev.Utils.Infrastructure.Hangfire/Attributes/AllowedExecutionTimeRangeAttribute.cs:69

• The variable name 'infotime' does not follow C# naming conventions. It should be 'timeZoneInfo' or 'timeZone' to be more descriptive and follow camelCase naming conventions.

        TimeZoneInfo infotime = TimeZoneInfo.FindSystemTimeZoneById("Central Europe Standard Time");

src/Madev.Utils.Infrastructure.Hangfire/Attributes/AllowedExecutionTimeRangeAttribute.cs:30

• The method GetTimeInCentralEuropeStandardTime uses DateTime.Now which is the local system time, then converts it to Central European time. This creates a dependency on the system's local time zone configuration and could lead to incorrect behavior. Consider accepting DateTime as a parameter with a default of DateTime.UtcNow, or use DateTimeOffset for more reliable timezone handling. Additionally, the method should accept a DateTime parameter to make it testable.

        var dateTimeNow = GetTimeInCentralEuropeStandardTime(DateTime.Now);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The TimeZoneInfo.FindSystemTimeZoneById call can throw TimeZoneNotFoundException if "Central Europe Standard Time" is not found on the system. This is particularly problematic on non-Windows systems where timezone identifiers differ (e.g., Linux uses "Europe/Prague" or "Europe/Berlin" instead). Consider using TimeZoneInfo.TryFindSystemTimeZoneById or providing a fallback mechanism.

Suggested change

	public TimeSpan GetTimeInCentralEuropeStandardTime(DateTime dateTime)
	{
	TimeZoneInfo infotime = TimeZoneInfo.FindSystemTimeZoneById("Central Europe Standard Time");
	return TimeZoneInfo.ConvertTimeBySystemTimeZoneId(dateTime, infotime.Id).TimeOfDay;
	private static readonly string[] CentralEuropeTimeZoneIds =
	{
	"Central Europe Standard Time", // Windows
	"Europe/Prague", // IANA (example CET/CEST)
	"Europe/Berlin" // IANA (alternative CET/CEST)
	};
	public TimeSpan GetTimeInCentralEuropeStandardTime(DateTime dateTime)
	{
	TimeZoneInfo infoTime = null;
	foreach (var timeZoneId in CentralEuropeTimeZoneIds)
	{
	try
	{
	infoTime = TimeZoneInfo.FindSystemTimeZoneById(timeZoneId);
	break;
	}
	catch (TimeZoneNotFoundException)
	{
	// Try next candidate ID
	}
	catch (InvalidTimeZoneException)
	{
	// Try next candidate ID
	}
	}
	if (infoTime == null)
	{
	infoTime = TimeZoneInfo.Local;
	}
	return TimeZoneInfo.ConvertTime(dateTime, infoTime).TimeOfDay;

[Copilot]

The hardcoded timezone "Central Europe Standard Time" makes this attribute unusable for applications running in other timezones. Consider accepting the timezone as a constructor parameter with a default value, allowing consumers to specify their timezone requirements.