Skip to content

Add DisplayName functionality #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
NiSHoW wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add DisplayName functionality #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3099f47
add displayname functionality
NiSHoW Jun 14, 2023
8fb70c9
update sample for use display name
NiSHoW Jun 14, 2023
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
2 changes: 2 additions & 0 deletions samples/Hangfire.Microservices.NewsletterService/NewsletterSender.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ namespace Hangfire.Microservices.NewsletterService
[Queue("newsletter")]
public sealed class NewsletterSender
{

[JobDisplayName("Newsletter {0}")]
public static void Execute(long campaignId)
{
Console.WriteLine($"Processing newsletter '{campaignId}'");
Expand Down
2 changes: 2 additions & 0 deletions samples/Hangfire.Microservices.OrdersService/OrderSubmitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

namespace Hangfire.Microservices.OrdersService
{
[Queue("orders")]
public class OrderSubmitter
{
[JobDisplayName("order {0} - status {1}")]
public void Execute(long orderId, string status)
{
Console.WriteLine($"Submitting order {orderId} with status {status}");
Expand Down
9 changes: 8 additions & 1 deletion src/Hangfire.DynamicJobs/DynamicJob.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,15 @@ public DynamicJob(
[NotNull] string method,
[CanBeNull] string parameterTypes,
[CanBeNull] string args,
[CanBeNull] JobFilterAttribute[] filters)
[CanBeNull] JobFilterAttribute[] filters,
[CanBeNull] JobDisplayNameAttribute displayName)
{
Type = type ?? throw new ArgumentNullException(nameof(type));
Method = method ?? throw new ArgumentNullException(nameof(method));
ParameterTypes = parameterTypes;
Args = args;
Filters = filters;
DisplayName = displayName;
}

[NotNull]
Expand All @@ -46,6 +48,11 @@ public DynamicJob(
[JsonProperty("f", NullValueHandling = NullValueHandling.Ignore, ItemTypeNameHandling = TypeNameHandling.Auto)]
public JobFilterAttribute[] Filters { get; }

//TODO: Check if is better save display name as resolved string (maybe when you use location resources can be cause crash?)
[CanBeNull]
[JsonProperty("dn", NullValueHandling = NullValueHandling.Ignore, ItemTypeNameHandling = TypeNameHandling.Auto)]
public JobDisplayNameAttribute DisplayName { get; }

@odinserj odinserj Jul 4, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should expose the DisplayName property as a string one and calculate its value when creating a dynamic job. Such descriptions are often based on argument values (by using placeholders like {0} passed to the String.Format method), and since argument types can be unavailable at runtime, we shouldn't depend on them.


[PublicAPI]
[DynamicJobDisplayName]
public static object Execute(DynamicJob dynamicJob, PerformContext context)
Expand Down
29 changes: 29 additions & 0 deletions src/Hangfire.DynamicJobs/DynamicJobDisplayNameAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Please see the LICENSE file for the licensing details.

using System;
using System.Globalization;
using Hangfire.Annotations;
using Hangfire.Common;
using Hangfire.Dashboard;
Expand All @@ -23,6 +24,13 @@ public override string Format([NotNull] DashboardContext context, [NotNull] Job
{
if (arg is DynamicJob dynamicJob)
{
//create display name from JobDisplayNameAttribute
if (dynamicJob.DisplayName != null)
{
var displayName = GenerateDisplayName(dynamicJob);
return displayName;
}

return $"Dynamic: {ExtractTypeName(dynamicJob.Type, out _, out _)}.{dynamicJob.Method}";
}
}
Expand Down Expand Up @@ -60,5 +68,26 @@ internal static string ExtractTypeName(string type, out string @namespace, out s

return type;
}



internal static string GenerateDisplayName(DynamicJob job)
{

var text = job.DisplayName.DisplayName;

//code from format method of JobDisplayNameAttribute with private properties (cache/resources)
//if (ResourceType != null)
//{
// text = _resourceManagerCache.GetOrAdd(ResourceType, InitResourceManager).GetString(DisplayName, CultureInfo.CurrentUICulture);
// if (string.IsNullOrEmpty(text))
// {
// text = DisplayName;
// }
//}
string[] arguments = SerializationHelper.Deserialize<string[]>(job.Args);
return string.Format(CultureInfo.CurrentCulture, text, arguments);
}

}
}
20 changes: 19 additions & 1 deletion src/Hangfire.DynamicJobs/DynamicJobRecurringJobManagerExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Threading.Tasks;
using Hangfire.Annotations;
using Hangfire.Common;
Expand Down Expand Up @@ -138,11 +139,28 @@ private static Job ToDynamicJob([NotNull] Job job, [CanBeNull] IEnumerable<JobFi
if (job == null) throw new ArgumentNullException(nameof(job));

var invocationData = InvocationData.SerializeJob(job);

//get display name form original job
var jobDisplayNameAttribute = job.Method.GetCustomAttributes<JobDisplayNameAttribute>(inherit: true).FirstOrDefault();
//TODO: maybe is better resolve name here and serialize as string?

//get queue filter from original job
//// Job.Method.GetMethodFilterAttributes() inaccessibile
//// ReflectedAttributeCache.GetMethodFilterAttributes(job.Method); inaccessible
var typeFiltes = job.Type.GetTypeInfo().GetCustomAttributes(typeof(QueueAttribute), inherit: true).Cast<QueueAttribute>();
var methodFiltes = job.Method.GetCustomAttributes(typeof(QueueAttribute), inherit: true).Cast<QueueAttribute>();

//generate list of filters for dynamicjob
var dynamicfilters = new List<JobFilterAttribute>(filters?.ToArray() ?? Enumerable.Empty<JobFilterAttribute>());

@odinserj odinserj Jul 4, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should concentrate on description only in this pull request. Automatic filter attribute acquisition is great, but it has some gotchas – since arguments (or their types) of a target method can be unavailable at runtime, this solution will have a lot of limitations. Currently Hangfire.DynamicJobs extension requires all the filters to be passed explicitly, and this is the only obvious solution I see without hidden problems.

dynamicfilters.AddRange(typeFiltes);
dynamicfilters.AddRange(methodFiltes);

var dynamicJob = new DynamicJob(invocationData.Type,
invocationData.Method,
!String.IsNullOrEmpty(invocationData.ParameterTypes) ? invocationData.ParameterTypes : null,
invocationData.Arguments,
filters?.ToArray());
dynamicfilters?.ToArray(),
jobDisplayNameAttribute);

return Job.FromExpression(() => DynamicJob.Execute(dynamicJob, default));
}
Expand Down