Skip to content
This repository has been archived by the owner on Jan 24, 2021. It is now read-only.

Redesign the bootstrapper concept #2696

Open
thecodejunkie opened this issue Jan 31, 2017 · 4 comments
Open

Redesign the bootstrapper concept #2696

thecodejunkie opened this issue Jan 31, 2017 · 4 comments
Labels

Comments

@thecodejunkie
Copy link
Member

Background

The current design for INancyBootstrapper and, more importantly, the base implementation NancyBootstrapperBase have more or less been left untouched (part from a couple of additions here and there - but the over all design has remained the same) since it was introduced in the very early days of Nancy.

It has served us very well, but the time has come to start taking a look at an alternative design to help facilitate some things, such as a new feature set and to be able to integrate even better (and easier) with other environments. This is very likely to be a breaking change, hence it should be done before 2.0-RTM is released

Current design

Nancy is not really tied to the bootstrapper - it is just an additional layer put on top of the core, that helps you compose Nancy. Howerver, all you really need to do it so create an instance of INancyEngine and off you go. @grumpydev recently illustrated this in a gist where he essentially new up the entire framework without a DI container. It's still in an INancyBootstrapper implementation, but as you can see all you really need is the code in the GetEngine() method.

So what does this mean? It means that the only things that are coupled (a part from user config) to the INancyBootstrapper (and in the background, the NancyBootstrapperLocator) are our Nancy.Hosting.* packages, that uses the boostrapper to get a, fully composed, INancyEngine instance that it can use to broker requests/responses against. Nothing inside INancyEngine (which is our root-level object) is even aware of the bootstrapper. This gives us a very clean separation between the bootstrapper concept, and the actual framework. This also means that we can replace the boostrapper design, without having an impact on Nancy, the framework. The only things that needs updating are our Nancy.Hosting.* packages.

Suggested approach

A while back @khellang spiked out a proof-of-concept of an alternative design. I suggest we use this as a place to start explore a new design. Given the clean separation, of Nancy and the boostrapper, that was described above, we should be able to transplant this spike into the boostrapper-update branch and hack away. For the purpose of the experimenting, we can simply focus on a single Nancy.Hosting.* package, and fix the others once we've settled on a design - again the changes to those packages should be very small (they grab the INancyBootstapper instance from the NancyBootstrapperLocator)

Goals

Let's maintain a list of design goals that we all agree on. Please use individual comments to suggest changes / designs and once we reach consensus on something, we move it into this list.

  • Placeholder
  • Placeholder
  • Placeholder
@thecodejunkie thecodejunkie added this to the 2.0-dangermouse milestone Jan 31, 2017
@thecodejunkie
Copy link
Member Author

thecodejunkie commented Jan 31, 2017

Here are some initial thoughts

  • I would like the new design to facilitate a way for us to put the INancyContext (directly, or though some INancyContextProvider) into the container. The reason I want to do this is to make our code even cleaner and let us create a true "context everywhere" story. Currently we have to pass around the INancyContext as a parameter to all places were we want to be able to read/write values using the context. Putting it in the container would let us strip out all those parameters (except for the places where it makes sense for it to be part of the method signature) and replace it with a dependency instead.

  • I'd like for us to give the NancyInternalConfiguration concept some thoughts (both in design and as a feature). Is this something we want to keep around in the future, as a way to keep making a clear distinction between framework code and user code? Perhaps it would be enough to simply register the things in the container directly and if the user wants to swap a part out, simple override the registration by sticking their own implementation in the container?

  • With regards to NancyInternalConfiguration. We currently have a lot of things in there, but not everything. There are still registrations live in NancyBoostrapperBase<TContainer>, such as ModelValidatorFactories

  • Also with regards to NancyInternalConfiguration, I would like to make sure that only root-level objects are defined in the boostrapper itself or at least create some form of "component concept". What I mean is that we might have IRouteResolver that's a low level concept in Nancy. Our default implementation of that interface might use an IRouteParser (there's no such thing, just using it as part of my example). That interface (and implementation) is specific for our implementation and if a user creates their own IRouteResolver implementation then there's a good chance they don't care about IRouteParser. With our current we also put these satellite dependencies in NancyInternalConfiguration. Not sure how to solve/desgn this, but ideally all it should know about would be IRouteResolver and then our DefaultRouteResolver was responsible for registering it's own, implementation specific, dependencies into the container

  • This is also a good time to think about NancyBootstrapperWithRequestContainerBase and the need to keep that as its own thing. It was created in case were was a container that did not support child containers, but I think we've already established that most (all?) do these days and also that it is an essential thing for building these kinds of applications

@khellang
Copy link
Member

khellang commented Jan 31, 2017

I think we should move as much as possible away from the bootstrapper base class. It doesn't really compose very well. We keep saying that "it's as simple as implementing IBootstrapper", but in reality, the bootstrapper base implementation does quite a lot of work, and you leave a lot of (documented) features behind if you implement the interface yourself.

I did some testing related to framework configuration in the prototype here. Think that could be something to pursue? It'll be like NancyInternalConfiguration, but with a slightly easier API to work with. It'll be a one-stop-shop to add/replace/remove framework components in a discoverable way.

@thecodejunkie
Copy link
Member Author

@khellang

I think we should move as much as possible away from the bootstrapper base class. It doesn't really compose very well. We keep saying that "it's as simple as implementing IBootstrapper", but in reality, the bootstrapper base implementation does quite a lot of work, and you leave a lot of (documented) features behind if you implement the interface yourself.

Well yes and no. I think there are value in having a base class for things like the configuration (NancyInternalConfiguration and the discussion below) and hooking up things like pipelines, diagnostics and other bits and bobs. However there would be great value if that was broken up to smaller fragments so it could be customised more easily. Right now you have no way of affecting the order of things in Initialise() without duplicating all for the code in the bootstrapper. Another alternative is to maintain a sort of reference implementation, either in the code or in the wiki, to serve as an example of what needs to be done. From a point of maintainability it can get a bit worse if we don't have the base class since a change would have to be replicated to n-implementations.

I did some testing related to framework configuration in the prototype here. Think that could be something to pursue? It'll be like NancyInternalConfiguration, but with a slightly easier API to work with. It'll be a one-stop-shop to add/replace/remove framework components in a discoverable way.

I like that it gives you the concept of "namespaces" for core components 👍 I guess somewhere in the background there would be something similar to NancyInternalConfiguration that holds the default configuration for Framework.

@thecodejunkie
Copy link
Member Author

This is also a good time to think about NancyBootstrapperWithRequestContainerBase and the need to keep that as its own thing. It was created in case were was a container that did not support child containers, but I think we've already established that most (all?) do these days and also that it is an essential thing for building these kinds of applications

(added this to my initial comment to keep track)

@thecodejunkie thecodejunkie removed this from the 2.0-dangermouse milestone Jan 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants