add support for generic methods where the generic types may not be in…

[sage-nathan-jones]

… the method's argument list. Maintains backward compatibility with jobs already serialized before this change.

Addresses issue #893

[codecov-io]

Codecov Report

Merging #894 into master will increase coverage by 0.06%. The diff coverage is 93.33%.

Impacted Files	Coverage Δ
src/Hangfire.Core/Common/Job.cs	89.53% <100%> (+0.12%)	:arrow_up:
src/Hangfire.Core/Storage/InvocationData.cs	73.91% <100%> (+1.18%)	:arrow_up:
src/Hangfire.Core/Common/TypeExtensions.cs	98.64% <83.33%> (-1.36%)	:arrow_down:
src/Hangfire.Core/QueueAttribute.cs	100% <0%> (ø)	:arrow_up:
src/Hangfire.Core/AutomaticRetryAttribute.cs	80.28% <0%> (+0.28%)	:arrow_up:
src/Hangfire.SqlServer/SqlServerDistributedLock.cs	82.75% <0%> (+0.83%)	:arrow_up:

[sage-nathan-jones]

Ok, so I renamed the variables and moved around the logic as you suggested. I removed the code that was in place to check genericTypes against genericArguments since collapsing them to the same variable prohibits that (this was the .Zip() call.) This was a sanity test in any event.

I also changed the .Aggregate() call to just be a foreach loop. This is still required in my opinion because without it, you will get the NullReferenceException during the MakeGenericMethod() call (which was what our original problem was.) This check has to be done after any sort of sniffing from TypeMatchRecursive().

[sage-nathan-jones]

I've made the requested changes. It has the original behavior of throwing a NullReferenceObject from MethodInfo.MakeGeneric()

[odinserj]

@sage-nathan-jones thanks a lot for your hard work! @aidmsu, to what branch should we merge these changes?

[aidmsu]

@odinserj @sage-nathan-jones I found one more problem in the PR. Since this is bugfix I think we should merge these changes into master after fixing it.

[odinserj]

@sage-nathan-jones, how are you doing? We realised that fix for this issue is a bit more complicated, because some storages (for example Hangfire.Redis.StackExchange) are using properties of the InvocationData class directly, without calls to Serialize or Deserialize. Therefore we can't add any properties here.

As a workaround, we can include the information regarding generic arguments to the Method property itself, for example MyGenericMethod<System.String, System.Int32>, but this will require additional parsing and much more effort for backward compatibility.

I would like to know if you still want to complete this issue. If not – not a problem!

[sage-nathan-jones]

I will look at it this week. The simple cases should be straightforward, I worry about robustness... but since it is just flat out broken now, I guess it will be better than nothing.

Get Outlook for iOShttps://aka.ms/o0ukef

---

From: Sergey Odinokov [email protected] Sent: Monday, July 10, 2017 12:00:23 PM To: HangfireIO/Hangfire Cc: Jones, Nathan; Mention Subject: Re: [HangfireIO/Hangfire] add support for generic methods where the generic types may not be in… (#894)

@sage-nathan-joneshttps://github.com/sage-nathan-jones, how are you doing? We realised that fix for this issue is a bit more complicated, because some storages (for example Hangfire.Redis.StackExchange) are using properties of the InvocationData class directly, without calls to Serialize or Deserialize. Therefore we can't add any properties here.

As a workaround, we can include the information regarding generic arguments to the Method property itself, for example MyGenericMethod<System.String, System.Int32>, but this will require additional parsing and much more effort for backward compatibility.

I would like to know if you still want to complete this issue. If not – not a problem!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/HangfireIO/Hangfire/pull/894#issuecomment-314151657, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AbeRz3BjhOQC-tAGqae98cqTnj1sozEEks5sMkqXgaJpZM4NhVec.

---

Attention: This email has arrived from the internet. Please use caution when opening attachments. Please follow all appropriate security policies when opening any email from a non-trusted source. If you feel this message is suspect please contact your local security representative or Help Desk for review. See you at #SageSummit Tour 2017. Learn Morehttps://www.sage.com/sage-summit/ Paris | Madrid | Melbourne | Berlin | Johannesburg | Singapore | London | Atlanta | Toronto

Sage Summit goes local. Business Builders, Build On!

The information contained in this email transmission may constitute confidential information. If you are not the intended recipient, please take notice that reuse of the information is prohibited.