Need guidance for resource hooks

[crgolden]

Description

I have been trying to get Resource Hooks working in v4.0.0-alpha3. I have followed the steps in the docs, but the hook is never hit.

The only way I have actually gotten it to work is to add an explicit call to AddScoped<IResourceHookContainer<WeatherForecast>, WeatherForecastDefinition>(), then inject the IResourceHookContainer<WeatherForecast> directly into the service. But that seems wrong - looking at the source it should be registered automatically using open generics.

I also run into problems trying to call BeforeCreate because I would then have to magically have a IAffectedResources object somehow. Which seems only to be available internally through some TypeHelper extension methods and internal derived classes.

Getting this to work is a blocker for me using this library in production code because I have to be able to authenticate and authorize at the request level.

Anyway, I have posted a simple repository that reproduces the behavior: https://github.com/crgolden/JsonApiDotNetCoreIssue. Please take a look and let me know what I'm doing wrong.

Thanks!
...

Environment

• JsonApiDotNetCore Version: 4.0.0-alpha3
• Other Relevant Package Versions: not sure

[maurei]

You were very close. The hooks you defined weren't executed because your WeatherForecastDefinition wasn't registered with the DI container. Add the following:

...
services.AddScoped<ResourceDefinition<WeatherForecast>, WeatherForecastDefinition>()
...

I realise that this piece of information is hidden away deep in the docs. Not even the resource hooks docs but the ResourceDefinition docs that is referred to by the resource docs all the way at the bottom of that page:

Prior to the introduction of auto-discovery, you needed to register the ResourceDefinition on the container yourself

You're not using auto-discovery because you're using AddJsonApi<TDbContext>() so you still have to register your resource definition manually. (Note that the overload with TDbContext is basically a subset of the auto-discovery. Also note to self: that overload is used in the getting started document: need to update that.

For auto registration of your custom services I would recommend you to use a startup of the form

services.AddJsonApi(options =>
{
...
    options.EnableResourceHooks = true; // I'm going to make a PR to default to true
...
},
discovery => discovery.AddCurrentAssembly());

I need to update the docs on this.

Let me know if you have any more questions!

[crgolden]

@maurei Thanks for the response!

Your solution worked for the simple example I shared. Then I spent several hours banging my head against the wall trying to figure out why I still couldn't get it to work in an existing application.

Finally, I started recreating the existing application entirely from scratch to pinpoint the issue. It turns out that, as soon as the entities are moved to a separate assembly, the resource hooks no longer work. That is a big gotcha.

I updated my sample repo to reproduce the problem. Comment the WeatherForecast in the "Entities" project and uncomment the one in the "JsonApiDotNetCoreIssue" project and the resource hooks start working again.

This would be a blocking issue for me, as I can hardly guarantee the entities will always be in the same assembly. They may even be coming from a NuGet package, etc.

[wisepotato]

Thanks for your interest in JADNC, i'm going to check what's going on in your repo.

[maurei]

@crgolden Thanks for reporting this. There is a problem with detecting which hooks have been implemented for a particular resource when that resource lives in another assembly, see HooksDiscovery.cs

I'm going to look into it in more detail tomorrow, probably this will require a fix which we will release in -alpha4.

On a side-note: I'd recommend you to check out to the develop branch and use that locally in your projects until alpha4 has been released because it comes with an upgrade of .net core 3 and various other breaking changes

[wisepotato]

or, @maurei checks it. Because he's 2fast4me

[maurei]

The detection issue has been resolved in #617. This fix will be released in alpha4 today or tomorrow.

Apart from that, there was also another issue with your setup: the usage of entity-resource split. Resource hooks does not work with that. ER split will be removed in alpha4 (see #553). I have submitted a PR in your sample project that introduces the fix of #617 as a custom override and gets rid of the ER split.

If you have any more questions be sure to ask

[crgolden]

@maurei Thanks for taking the time to help me with that issue. I really appreciate it.

Do you guys have a timeline for the v4 release? Using dev branches and alpha versions is all fine and good in my personal GitHub repos, but I can't use them in the corporate environment where I'm really trying to implement this package.

[maurei]

@crgolden I'm waiting for #618 to be approved after which I will immediately release v4.0.0-alpha4 which you can use in your corporate projects. After that no more alpha releases, we'll be aiming for the full release of v4 (unless new immediate bugs show up). But for that I want to completely integrate it our own corporate projects first to beta (battle) test it, and write a migration guide for people coming from v3.1