Add validation around constructor args.

[Jark]

This fixes #85, unfortunately we have to execute the Func args once to do the validation properly.

I think ideally we'd do a simplification to get rid of public void RegisterController<T>(params object[] args) and public void RegisterController<T>(Func<object[]> args) by using dependency injection. And preferably also the ability to create singleton controllers, I don't see what the use case for singleton controllers would be anyway. I guess it does make the initial play with restup easier.

An example of the design with dependency injection would be:

[RestController]
public sealed class ParameterController
{
    private readonly IParameterStore parameterStore;

    public ParameterController(IParameterStore parameterStore)
    {
        this.parameterStore = parameterStore;
    }

    [UriFormat("/parameter/{id}/{propName}")]
    public IGetResponse GetValue(int id, string propName)
    {
        object value;
        if (parameterStore.TryGet(id, propName, out value))
        {
            return new GetResponse(GetResponse.ResponseStatus.OK, new { Id = id, Prop = propName, Value = value });
        }

        return new GetResponse(GetResponse.ResponseStatus.NotFound, new { Id = id, Prop = propName });
    }
}

public interface IParameterStore
{
    bool TryGet(int id, string propName, out object value);
}

public  interface IDependencyResolver : IDisposable
{
    object GetService(Type serviceType);
    void Register(Type serviceType, Func<object> activator);
}

And then the initialisation of the http server with rest would be:

// setup stores to backend processes
var store = new ParameterStore();

// setup rest route handler
var restRouteHandler = new RestRouteHandler();
restRouteHandler.DependencyResolver.Register(typeof (IParameterStore), () => store);
restRouteHandler.RegisterController<ParameterController>();

// setup http server config
var config = new HttpServerConfiguration()
    .RegisterRoute(restRouteHandler)
    .ListenOnPort(80);

// create & start server
var httpServer = new HttpServer(config);
httpServer.StartServerAsync().Wait();

Let me know what you think about the code change and the design above. If the design needs work then maybe we can create a separate issue for this.

[tomkuijsten]

I don't see why we would want to remove singleton controllers, all my controllers are singletons and I don't see why I would want PerCall controllers. You see this as a bad practice?

Executing the Func is really not what I would want to do, but I see that there is no other way in the current implementation. Problem is that it's unexpected behavior from the end-user's perspective (I wouldn't expect it to happen). Another problem is that this will only solve part of the problem, the Func can return a different args array later on. Would be weird, I agree, but never the less possible,

I like the idea of dependency injection, but I think we should create another issue/branch for that to elaborate a bit more on the architecture.

When I remove the TypeComparer, all tests pass. Can be removed completely?

[Jark]

Hi @tomkuijsten,

1 - I've removed the TypeComparer, since Type apparently does implement Equals / GetHashCode.

2 - What do do with the RegisterController<T>(Func<object[]> args) I think I should either remove the validation for this particular method, because like you say, users wouldn't expect it to be executed it once. Or, we can obsolete the RegisterController<T>(Func<object[]> args) method, do you know of any good use case where you would use the Func<object[]> instead of the params object[] method?

3 - Getting rid of singleton controllers I worded this a bit poorly, I meant doing away with the InstanceCreationType stuff. I think how a controller is instantiated shouldn't necessary need to be defined by the user of the library. The library should be free to instantiate 10s of the same controllers if this is necessary from a processing perspective.

Generally speaking you wouldn't want to directly keep state in the controller class, because the it should only handle requests and not need to store state. You just need to have some way to access state, which is why the RegisterController takes parameters to pass into the constructor.

There are some reasons why the library could decide to change how/when to instantiate controllers:

Imagine if we were to use a different threading model with for instance one worker thread per processor core to ensure maximum parallel processing capability. Each of these app domains would hold an instance of every controller so that when bad exceptions happen only on one thread.

If we would decide to optimise for memory at one point, then there could be a case where we instantiate a controller for a couple of calls and then maybe dispose of it again.

This all boils down to less configuration and less thinking needed from a user perspective.

4 - Dependency injection

Agreed, should be a different issue and branch.

[tomkuijsten]

1. Thanks
2. I can't really think of a scenario that would validate the usage of the Func overload. The only reason you would use that, is if parameters would change over time. This however, is very error prone, what if you use it with a Singleton controller, those changed constructor parameters will never be used. I agree on making it Obsolete. It would still be possible to change parameters by passing an object which is under your control (eg passing a MyControllerConfiguration object), if you really want to.
3. I follow you arguments and I think you do have a point. Let's put it in a separate issue (if you want to work on it right now).
4. Ok

[tomkuijsten]

Thanks for the addition, fine with me to merge it, agree?

[Jark]

Yeah, I'm ok with merging. Sorry, forgot to press "Comment" after typing my message here yesterday.