json-api-dotnet/JsonApiDotNetCore

Custom filters in own project (unit tests)

Closed this issue · 6 comments

@bkoelman :
Thank you for your work with #1286 which will enable us to implement custom filters in our project. One such filter being filter=IsWithinBBox(geometry, Xmin, Xmax, Ymin, Ymax)

What is your suggestion on how we unit test our custom filters? I really like the simple and clear test you have here; Set up a reader and call .Read(parameterName, parameterValue).

I tried to replicate the test in our project, but unfortunately for us your tests depend on internal classes e.g. ResourceFactory. It'd be really nice if we could have unit tests as clean as yours. Does it make sense to you to expose the needed internals for this purpose?

Well, that one is easy to solve:

private static IResourceFactory CreateResourceFactory()
{
    var services = new ServiceCollection();
    services.AddJsonApi();
    using ServiceProvider serviceProvider = services.BuildServiceProvider();

    return serviceProvider.GetRequiredService<IResourceFactory>();
}

Superfically easy but it it seems to me overkill to have to fully build and resource manage a service provider and dependency inject to get the same testable conditions. Would it not more be more practial to make ResourceFactory public, or is that deemed too dangerous?

It's extremely cheap. The returned IResourceFactory can even be made static, unless corner-case features are used that aren't even documented. It's fine to dispose the container right away, because IResourceFactory doesn't implement IDisposable, so it survives when the container is disposed.

Alternatively, it's easy to roll your own, assuming you don't need these corner-case features, such as injected parameters and abstract types:

using System.Linq.Expressions;
using System.Reflection;
using JsonApiDotNetCore.Resources;

public sealed class SimpleResourceFactory : IResourceFactory
{
    public IIdentifiable CreateInstance(Type resourceClrType)
    {
        return (IIdentifiable)Activator.CreateInstance(resourceClrType)!;
    }

    public TResource CreateInstance<TResource>()
        where TResource : IIdentifiable
    {
        return Activator.CreateInstance<TResource>();
    }

    public NewExpression CreateNewExpression(Type resourceClrType)
    {
        ConstructorInfo constructor = resourceClrType.GetConstructors().First();
        return Expression.New(constructor, []);
    }
}

I'd like to keep the built-in one non-public because it may change in the future, or we could provide multiple implementations and choose the most efficient one.

Thanks @bkoelman, it makes sense.

@klavspc Can this be closed?

@klavspc Can this be closed?

Sure, and thanks for your help and quick reply