Extensions and builder chaining

[soundofspace]

In some cases we want to be able to chain items and build the final item by combining all the pieces.

One such example is TlsConnectorDataBuilder, right now we support chaining multiple builders by having a field base_builders: Vec<Arc<TlsConnectorDataBuilder>> which is traversed in reverse when a field is not present in this builder.

This allows us to (efficiently) chain builder and to have pre-made builders (stored in arc) for certain uses. One thing this builder currently doesn't support is disable a specific field. This happens because a None value triggers and implicit fallback to the base builders, this could be fixed with an ExplicitNone, or something along those lines.

However since Extensions are now append only and only support modifications by using internal mutable (something that should only be used for specific use cases). We now do this:

// We can edit our current builder directly or create a new one if needed
let mut builder = req
    .extensions_mut()
    .get::<TlsConnectorDataBuilder>()
    .cloned()
    .unwrap_or_default();
builder.set_server_verify_mode(ServerVerifyMode::Disable);
req.extensions_mut().insert(builder);

A more efficient way of doing this would be to do something like this

// First builder we insert we now put inside an Arc first (into_shared_builder)
req.extensions_mut().insert(TlsConnectorDataBuilder::default().into_shared_builder())
// Cloning here is now much cheaper
let mut parent = req
    .extensions_mut()
    .get::<Arc<TlsConnectorDataBuilder>>()
    .cloned()
    .unwrap_or_default();
let builder = TlsConnectorDataBuilder::new().with_parent(parent);
req.extensions_mut().insert(builder);

But this pattern is pretty tedious and specific to this case. I however believe that we can implement this in a more generic way so that more components benefit from this. We have lots of options here, but an example could be

// Instead of one big struct with spit this config up many small ones eg:
ConnectorVerifyMode(Option<ServerVerifyMode>);
ConnectorKeyLogIntent(Option<KeyLogIntent>);

In an inefficient (to be tested) approach the connector would then do

let verify_mode = req.extensions().get::<ConnectorVerifyMode>();
let key_log_intent = req.extensions().get::<ConnectorKeyLogIntent>();

If values are None -> There was no explicit Value set
If value is ConnectorVerifyMode(None) -> Explicitly disable
If value is ConnectorVerifyMode(Some(value)) -> A value was set

We could provide a function utility to hide explicit vs implicit None if this is tedious to deal with.

In a more efficient approach we would provide a way to build a final config in a single iteration, or to get all parts in a single go. In the basic example we always iterate over all the extensions from end to start to get a value. If this is actually bad performance remains to be tested, but still getting everything needed in a single go could be useful for other uses case, but it does complicate things, and currently not convinced if it's worth it.

This logic could work along the lines

let (verify, keylog_intent) = req.extensions.get_all::<ConnectorVerifyMode, ConnectorKeyLogIntent, ...>();

or a much more complex setup such as .reduce, iterating over needed options...

It is also possible that we don't split up builders in small parts, but that instead that we come up with some sort of generic builder pattern that extends beyond the TlsConnectorDataBuilder use case, but borrows ideas from there, and that builds on top the assumptions we can now make for extensions, mostly readonly and append only. Depending how many things we store in extensions this pattern might be more efficient in some cases, this also has the advantage that it's pretty easy to get the Combined config vs having to get all pieces eg in debug logs, but also in normal logic

[GlenDC]

What you describe is more an example of an extension reducer, which is also very handy but in a way doesn't need all that much for us to support it except to:

1. support geting multiple extensions at once (can be as a tuple)
2. make use of it

With extension chaining I more ment that you want prevent by default to overwrite extensions. So when using insert it would insert it but never overwrite. Using .get() .get_mut() would give you the "last available" one. iter() would give you them all and first would give you the first. iter and first are not mutable as that would make it confusing in parent vs not parent.

Should be fairly easy to support as it would just mean we need to store a vec-like object in these hashmaps instead of the type itself. And it would solve stuff such as by accident overriding stuff.

To compensate the get + insert dance I think we could reintroduce again get_or_insert and get_or_insert_default which would both return a &mut.

An example use case is for example things like OriginalUri. That is an extension (now bound to router and thus called OriginalRouterUri, but probably to be renamed OriginalUri) which anything that modifies a uri (e.g. nested routers) could insert the current uri as a copy. This would allow you to define than an OriginalUri extractor which would use the ::first method and you would know it always return the first even if you have multiple nested routers and middleware and scared you might overwrite a previously one. This all fits in making it all easy to use without surprises as you never have to worry anymore about by accident overwriting something.

This is however different than what you are describing as what you are describing is more some reducer-like capabilities.

Both the chaining and reducer is something we should ship before releasing 0.3 as they are gonna become essential to rama users I believe.

[soundofspace]

This PR already migrates Extensions to read-only, append-only. We could indeed expose first, and iter(), but I would not re-add get_or_insert and get_or_insert_default as they both expose &mut and they would completely defeat the idea of read-only.

Instead I really do believe for cases where this is a problem we should fundamentally change how we handle these cases by:

• Creating a generic builder that can efficiently handle this case, while leaving breadcrumbs
• By splitting up larger structs into smaller ones where needed, and leaving trace of these
• A completely different way of solving it

But if we need to get something from extensions and modify it, the original items should still be there (in some way) and not modified so we have one source of truth in the breadcrumbs and don(t have to worry that components x/y might have secretly modified something

[GlenDC]

as they both expose &mut and they would completely defeat the idea of read-only.

Fair. You could also return &T instead? Not conventional but still makes it easy where you want a default.
That said.... that would again encourage bad behaviour of inserting stuff that shouldn't be inserted, so nvm...

But if we need to get something from extensions and modify it, the original items should still be there (in some way) and not modified so we have one source of truth in the breadcrumbs and don(t have to worry that components x/y might have secretly modified somethin

Yeah so that's why read-only makes sense I guess. But that's also why if you insert it shouldn't overwrite either but instead add on top. So same type but a new version.

[soundofspace]

Yeah so that's why read-only makes sense I guess. But that's also why if you insert it shouldn't overwrite either but instead add on top. So same type but a new version.

Yes insert will always be on top (last), so the focus of this issue is really: now that we have of this chain of info, how will we combine them and store them. I listed two options above, but feel free to suggest some more, otherwise I will play with those two and come back with advantages/disadvantages of both and potential new solutions found along the way

[GlenDC]

Extension wise there is in that case not much to support I think. Other than fact that you need also to provide a get_first and iter() version I suppose... That is for the support.

Another task is to actually make use of these new capabilities I suppose.
Which I guess is what you are hinting at?

In which case I think it makes sense if you make some different types that are similar to extractors but focussed per use case. That's nothing generic but really specific to the purpose. Would need to find a balance to not make it too many tpes, but neither too restrictive either. Feel free to try to come up with something for the first example impl of a use case for these new capabilities.