Consider making attributes non-sealed

[LordJZ]

This issue was previously discussed in #503. TL;DR: job filter attributes such as QueueAttribute are sealed and it makes it unnecessarily hard to use Hangfire in some cases.

There are many use-cases for inheritable job filters:

• Let end-users create short-hands for common scenarios
  • Imagine a [CommonQueue] and [SpecialQueue] being short for [Queue("common")] and [Queue("special")]
• Let end-users extend the behavior of existing filters, rather then creating new ones
  • Changing the order of the Queue attribute
  • Running custom code before and/or after the filter is invoked (this does not even require virtual functions)
• Let end-users enforce code quality policies
  • Imagine a "no direct 3rd-party code references" policy that lets you better assess feature impact and scope

Note that it is of course still possible to implement all use cases without inheriting from existing filters. You could wrap (aggregate) attribute classes, or copy/paste code, or even recompile Hangfire itself (e.g. to fix internal references in ctors in Ace). Since the project is open-source and you also provide source code for closed-source components, everything is effectively public but inconvenient to use. It would be much easier to just have the filter classes un-sealed.

[odinserj]

Thanks for reporting this. The idea of using inheritance to provide common constructor arguments and change the ordering seems useful indeed. The sealed attribute is added simply because it's much easier to remove it than to add it. However there's a chance that one will face with an unexpected consequences of using inheritance.

There are different levels of filter's scope – method, type and global, and it's possible to specify the same filter on multiple levels. Each filter also have the AllowMultiple property that's usually disabled for job filters. And when it's disabled, all the duplicates are removed from the resulting filter chain, where most narrow scope gets the preference. But since duplicate detection is based on concrete type, we'll get subtle behaviour when inheritance is implemented.

The simplest example – consider you've overridden the AutomaticRetryAttribute with MyAutomaticRetryAttribute and applied it to some jobs. You expect that MyAutomaticRetryAttribute will be used now, but actually both AutomaticRetryAttribute and MyAutomaticRetryAttribute will be invoked, because the former is added globally by default. Of course you can tweak the Order property, but anyway there will be possible problems for other filters, e.g. ThrottlingAttribute.

So unfortunately there's no simple solution for this problem. For QueueAttribute it's relatively easy to make your own filter, since it's a one-line job filter. Can you tell me what logic you need to implement for other filters? It's easy to declare everything extensible, but it's much harder to make everything consistent after doing this step.

[odinserj]

But I don't see any negative consequences for filters that support multiple instances, like MutexAttribute, SemaphoreAttribute, SlidingWindowAttribute, FixedWindowAttribute and DynamicWindowAttribute, so we can remove the sealed keyword from them.

[LordJZ]

Thank you -- the type-uniqueness case is indeed interesting. One could argue that the unique filters should be job properties, like Queue in the newer version of Hangfire, but that's a long shot from now and out of scope of this issue.

Can you tell me what logic you need to implement for other filters?

I have my own Queue where I plan to add more complex logic for queue selection. I also provide my own AutomaticRetryAttribute with IsRetryAttempt(IState)/IsRetryAttemptReason(string) methods exposed to be used in a polling routine that lets one await a job for completion. Basically makes Hangfire a Task.Run on steroids.

But I don't see any negative consequences for filters that support multiple instances, like MutexAttribute, SemaphoreAttribute, SlidingWindowAttribute, FixedWindowAttribute and DynamicWindowAttribute, so we can remove the sealed keyword from them.

Thanks, that would be great. These are exactly the attributes with which I had the most difficulties.

[odinserj]

Thank you -- the type-uniqueness case is indeed interesting. One could argue that the unique filters should be job properties, like Queue in the newer version of Hangfire, but that's a long shot from now and out of scope of this issue.

Actually this is possible with a custom job filter provider. Perhaps I will write something to handle this out of box.

Thanks, that would be great. These are exactly the attributes with which I had the most difficulties.

Done, updates available in Hangfire.Throttling 1.3.1.

[LordJZ]

Done, updates available in Hangfire.Throttling 1.3.1.

Great thanks. I can confirm inheriting [Mutex} works for me, including GetDescription etc.