owin / owin.dll

OWIN defines a standard interface between .NET web servers and web applications.
http://owin.org/
Apache License 2.0
162 stars 19 forks source link

Move IAppBuilder to Microsoft.Owin #19

Closed panesofglass closed 10 years ago

panesofglass commented 10 years ago

IAppBuilder is not part of the OWIN spec and should not be included in an owin.dll. This just confuses people new to OWIN.

serialseb commented 10 years ago

I'd be happy for a Func<AppFunc,AppFunc> signature for middlewares being part of the spec. How a host initializes itself and load the middlewares is host specific, and I do not support making that into a spec. A common signature for "so i have a middleware, what do I do with it?" is something worth having in the spec and doesn't seem controversial. Then the IAppBuilder or whatever is just one builder that uses the common middleware signature without being a dependency that people all take to build their middleware.

In other words, I don't want my host to use IAppBuilder because I find and prioritize middleware in another fashion. if middlewares take dependencies on IAppBuilder I can't use them in the worst case, or I have a ref to owin.dll in the least worst case.

Have I missed anything?

panesofglass commented 10 years ago

I think we continue to have consensus on the type. Nancy's OwinHost is a class form of Func<AppFunc, AppFunc> (ctor takes the next AppFunc and Invoke is the "returned" AppFunc). @jeremydmiller, does Fubu use something similar? I assume it does since I'm fairly certain I remember you had a Russian Doll approach.

@grumpydev, I want to make sure I understand what you are saying. I think your first, recent post is in harmony with @jeremydmiller and @serialseb, that you don't think startup should be standardized. I don't think you stated anything about whether we should have a common middleware signature but spoke to the builder patterns. Is that accurate?

grumpydev commented 10 years ago

I had taken the common middleware as read, which was my mistake. I have no issue with a standardised startup/builder at some point, but I think the correct approach is to have multiple implementations and see if there's any clear "winner" when the dust settles (or not, that bit is optional), rather than go back and forth trying to spec one out.

jeremydmiller commented 10 years ago

Ryan,

FubuMVC was built around the Russian Doll model from the beginning ("Behaviors"), except that we define the chain of behaviors per request/endpoint upfront at application startup time so that we depend much less on runtime logic and have effective diagnostic visualizations of what your app does. Our model overlaps somewhat with the things I'm seeing folks try to do with OWIN middleware.

I'm not enthusiastic about IAppBuilder for bootstrapping, especially when you consider how important the open/closed "drop in a Bottle assembly and it just works" philosophy is for us. We wrap the Katana startup mechanics from users and I think we'll end up having OWIN middleware registered into FubuMVC itself so that it's easy to enforce ordering of middleware and aggregating middleware registrations from Bottles.

On Dec 3, 2013, at 6:28 AM, Ryan Riley notifications@github.com wrote:

I think we continue to have consensus on the type. Nancy's OwinHost is a class form of Func<AppFunc, AppFunc> (ctor takes the next AppFunc and Invoke is the "returned" AppFunc). @jeremydmiller, does Fubu use something similar? I assume it does since I'm fairly certain I remember you had a Russian Doll approach.

@grumpydev, I want to make sure I understand what you are saying. I think your first, recent post is in harmony with @jeremydmiller and @serialseb, that you don't think startup should be standardized. I don't think you stated anything about whether we should have a common middleware signature but spoke to the builder patterns. Is that accurate?

— Reply to this email directly or view it on GitHub.

skoon commented 10 years ago

I hope replying by email works.

The whole point of even having an OWIN.dll was to give folks something concrete to reference if they were interested in building their own middleware or app framework. If we've done the spec & OWIN.dll right, the OWIN compatible middleware should work on any other OWIN compatible host. How you load the middleware is up to the host or framework.

So to me, this represents the original Gate impetus.

So my question to the group is: Are framework/middleware written against the OWIN.dll using the IAppBuilder implementation able to run other OWIN hosts without changing the middleware/host?

If not, then I think we need to modify OWIN.dll to make that possible.

I don't think that IAppBuilder should be a standard, but I want something to be a test/dev reference.

On Dec 3, 2013, at 7:55 AM, "Jeremy D. Miller" notifications@github.com wrote:

Ryan,

FubuMVC was built around the Russian Doll model from the beginning ("Behaviors"), except that we define the chain of behaviors per request/endpoint upfront at application startup time so that we depend much less on runtime logic and have effective diagnostic visualizations of what your app does. Our model overlaps somewhat with the things I'm seeing folks try to do with OWIN middleware.

I'm not enthusiastic about IAppBuilder for bootstrapping, especially when you consider how important the open/closed "drop in a Bottle assembly and it just works" philosophy is for us. We wrap the Katana startup mechanics from users and I think we'll end up having OWIN middleware registered into FubuMVC itself so that it's easy to enforce ordering of middleware and aggregating middleware registrations from Bottles.

  • Jeremy

On Dec 3, 2013, at 6:28 AM, Ryan Riley notifications@github.com wrote:

I think we continue to have consensus on the type. Nancy's OwinHost is a class form of Func<AppFunc, AppFunc> (ctor takes the next AppFunc and Invoke is the "returned" AppFunc). @jeremydmiller, does Fubu use something similar? I assume it does since I'm fairly certain I remember you had a Russian Doll approach.

@grumpydev, I want to make sure I understand what you are saying. I think your first, recent post is in harmony with @jeremydmiller and @serialseb, that you don't think startup should be standardized. I don't think you stated anything about whether we should have a common middleware signature but spoke to the builder patterns. Is that accurate?

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub.

panesofglass commented 10 years ago

@skoon the main reason for this was the difficulty of remembering the structure of the Delegate of Doom:tm:. With the new, simpler delegate, is that still necessary?

skoon commented 10 years ago

@panesofglass I do, simply because we're in a CLR world and folks need to write against types. The new delegate is simpler, but can is that future-proof? As .NET grows, I'm sure we'll revise the delegate as new language features are added to the language. I can see your point though.

I think that folks new to OWIN are having trouble accepting that the entire spec is a delegate. CLR folks are used to getting a concrete class, or at least an interface, The concept of just taking a delegate is crazy to them. People are still amazed when I add this method to a repository.

IList<T> GetByCriteria(params Expression<Func<T, bool>>[] args)

But maybe IAppBuilder should be taken out of here and just replaced with:

namespace owin {
    using OwinDelegate = Func<IDictionary<string, object>, Task>;
}

?

Tratcher commented 10 years ago

@grumpydev We definitely need to catch our breath. This is more discussion than we've had in more than the last year combine. There's no strict rush at the moment, we'll continue to refine ideas, try them, come back with scars and wisdom, and repeat. It's at least promising that people are moving generally in the same direction on this.

@skoon The AppFunc and the IAppBuilder serve very different purposes. Also, C# doesn't share using statements like that, you have to copy them to the top of every file. I agree that .NET devs expect types, but I think the various frameworks represented here can give them the types they need. App devs should rarely have to work with raw OWIN. Actually, that's one of the things IAppBuilder was intended to accomplish, letting devs wire everything up with extension methods without ever seeing the details.

Yes there's a lot of concern over having IAppBuilder in Owin.dll, but if those just come down to the dll name then lets set those aside for now and hash through the functional parts of this discussion first. If changes need to be made then we'll want to make them all at once. As noted elsewhere, .NET's typing includes the full interface, assembly name, version, etc., with minimal interop between versions, so any changes to Owin.dll will leave us with two incompatible worlds, the old and the new.

skoon commented 10 years ago

@Tratcher Yeah, the using was just for example. To show people that the delegate is easy to use. The concern from my PoV is more centered on Microsoft "owning" the IAppBuilder interface and devs thinking that IAppBuilder is part of the spec and the one, blessed way. Even though the code is licensed to MonkeySquare. From that PoV, we would want to make the change immediate and apparent.

I'm OK with dropping this for now if everyone else is. We're kind of going in circles here, maybe we should schedule an OWIN call?

Tratcher commented 10 years ago

@skoon What specific points did you want to address?

Not sure if you've all been watching, but the other thread (https://github.com/owin/owin/issues/20) has been making good progress.

half-ogre commented 10 years ago

I know it's hard to separate them, but there are two issues here: this one, #19, about where IAppBuilder should live (if it should live at all), and #20, on whether we can settle on a middleware delegate function that will let us all play nicely together whether or not we're using IAppBuilder. I think #20 is much more important, but of course this is important too.

Regarding what @grumpydev said:

I have no issue with a standardised startup/builder at some point, but I think the correct approach is to have multiple implementations and see if there's any clear "winner" when the dust settles (or not, that bit is optional), rather than go back and forth trying to spec one out.

And what @jeremydmiller said:

I don't think that a one size fits all startup/bootstrapping mechanism will get a lot of adoption or be terribly useful. Better yet, I think you want to wait and see how this stuff is actually consumed in the various frameworks before standardizing startup and bootstrapping anyway.

I think a builder and host bootstrapping are separate concerns. Passing a builder to configure a host is one way to do bootstrapping. If we had a standard middleware delegate function, it might not even be needed in host bootstrapping, or be the best way to do it.

More importantly to me, Rack had a default builder from pretty early on, and I think it helped adoption (and middleware creation) a great deal. Is it the only builder out there? I don't think so. But it's probably the one that's used almost all the time, when what you actually need is a builder.

So I personally don't see a problem with there being a default builder that's part of the OWIN project that's driven by the OWIN community. Is IAppBuilder that thing? Certainly not where it lives today, and maybe not in the shape it is today, but that doesn't mean an Owin.Builder shouldn't exist.

An Owin.Builder existing doesn't make it the only show in town. It wouldn't be a standard (that's what a middleware delegate function would be). It wouldn't be one size fits all. It would be a well-tested and useful option for people wanting to build up their own app in a self-host scenario. (Again, I don't think builders are the way to go for frameworks and hosts atop OWIN in the first place.)

That's why I'd like to see IAppBuilder move to Owin.Builder and a different .dll name. It would then be a builder interface, not the builder interface.

Because there shouldn't be one true builder interface. There should be one true middleware delegate function.

half-ogre commented 10 years ago

We definitely need to catch our breath. This is more discussion than we've had in more than the last year combine. There's no strict rush at the moment, we'll continue to refine ideas, try them, come back with scars and wisdom, and repeat. It's at least promising that people are moving generally in the same direction on this.

There's definitely no need to rush, but neither should we sit idle. I suspect this increase in discussion matches an increase in OWIN interest, both in consumers and creators. That's why I think it's important to solve these things as early as a responsible pace accords, before it becomes even more painful to change.

App devs should rarely have to work with raw OWIN.

This is a great point for all of us to remember, but also remember that the folks on this thread are OWIN creators, not consumers. As I said above, I think a type-friendly OWIN builder that's part of the OWIN project is a good thing, but the longer this sits at Install-Package owin the more painful it will be to correct the course.

Tratcher commented 10 years ago

On a practical note....

Owin.nupkg v1.0 containing Owin.dll v1.0 containing IAppBuilder isn't going away, probably ever. Multiple shipped products and frameworks depend on it. Nuget.org prohibits taking down packages for this reason.

The best migration story you could hope for is an Owin.nupkg v2.0 that redirects via package dependency to an Owin.Builder.nupkg v2.0 containing Owin.Builder.dll v2.0 containing IAppBuilder. You'd probably also need Owin.dll v2.0 to contain a type redirect for IAppBuilder to Owin.Builder.dll if you wanted to maintain any interop/compat. You also would not be able to change the signature of IAppBuilder without breaking all the implementers and many of the consumers.

.NET Types can be a real pain sometimes... especially interfaces...

half-ogre commented 10 years ago

On a practical note....

I think this has all been practical.

Owin.nupkg v1.0 containing Owin.dll v1.0 containing IAppBuilder isn't going away, probably ever. Multiple shipped products and frameworks depend on it. Nuget.org prohibits taking down packages for this reason.

Yep. There are dirty things we could do on the NuGet side, but they are dirty and I don't think there's a need. People will stop using the old bits in time. This is one reason we have package management, to get better stuff in the hands of folks without breaking those stuck in the past.

The best migration story you could hope for is an Owin.nupkg v2.0 that redirects via package dependency to an Owin.Builder.nupkg v2.0 containing Owin.Builder.dll v2.0 containing IAppBuilder. You'd probably also need Owin.dll v2.0 to contain a type redirect for IAppBuilder to Owin.Builder.dll if you wanted to maintain any interop/compat. You also would not be able to change the signature of IAppBuilder without breaking all the implementers and many of the consumers.

Why bother? The Owin package stays 1.0 forever and eventually falls out of use. As the rest of the ecosystem moves on, so will the creators and consumers. Especially as this is just an interface, if the rest of the ecosystem stops using it, it'll fade away.

panesofglass commented 10 years ago

There's definitely no need to rush, but neither should we sit idle. I suspect this increase in discussion matches an increase in OWIN interest, both in consumers and creators. That's why I think it's important to solve these things as early as a responsible pace accords, before it becomes even more painful to change.

:+1:

I think a builder and host bootstrapping are separate concerns. Passing a builder to configure a host is one way to do bootstrapping. If we had a standard middleware delegate function, it might not even be needed in host bootstrapping, or be the best way to do it.

Agreed.

More importantly to me, Rack had a default builder from pretty early on, and I think it helped adoption (and middleware creation) a great deal. Is it the only builder out there? I don't think so. But it's probably the one that's used almost all the time, when what you actually need is a builder.

So I personally don't see a problem with there being a default builder that's part of the OWIN project that's driven by the OWIN community. Is IAppBuilder that thing? Certainly not where it lives today, and maybe not in the shape it is today, but that doesn't mean an Owin.Builder shouldn't exist.

While I don't disagree with you about the utility of a builder, Rack is a poor comparison. express for node is a better comparison. node doesn't provide any sort of builder abstraction; that's what express does. I see Katana as similar to express, as are Fix and Simple.Owin.

That said, I'm not opposed to an Owin.Builder in, say, owin-contrib. I don't know that it's core, though, since we already have several implementations, of which Katana is the foremost and most likely primary example.

loudej commented 10 years ago

Wow, what a week to be out of town!

@half-ogre came the closes to hitting the nail on the head

More importantly to me, Rack had a default builder from pretty early on, and I think it helped adoption (and middleware creation) a great deal. Is it the only builder out there? I don't think so. But it's probably the one that's used almost all the time, when what you actually need is a builder.

Regarding the name of the nupkg and dll - I do need to point out the Rack::Builder class is in the Rack gem, just like Owin.IAppBuilder is in the Owin.nupkg/Owin.dll.

I do agree there should be a standard implementation of Owin.Builder.AppBuilder class in an Owin.Builder.nupkg/Owin.Builder.dll, and a standard implementation of Startup boot class discovery in an Owin.Loader.nupkg/Owin.Loader.dll.

Those types are helpful if you are writing the host for a Startup-class style applications. If you are writing the host for a complete closed system, and you don't want site author to compose frameworks and components around your system, then you don't need any of the app builder stuff.

That said, if you don't like or need IAppBuilder, ignore it. If you do think it's a good idea, and it does apply to your scenario, use it. But I think it would be a step backwards to say we should kill it or move it because we do think it's a good idea, but we want many different ones to choose from, because we'll stick ourselves into a world where middleware, host, and site authors are waiting to see which of the "app builder" techniques has become the one people are actually using.

skoon commented 10 years ago

.NET Types can be a real pain sometimes... especially interfaces...

Aren't we glad we settled on a delegate now? ;)

bbaia commented 10 years ago

Interesting discussion, because I faced those issues while working on connect-owin. I finally ended with what @loudej described: support for both OWIN primary interface Func<IDictionary<string, object>, Task> and Owin.IAppBuilder.

node doesn't provide any sort of builder abstraction; that's what express does

node does provide a sort of builder abstraction with connect; express is based on connect. You can compare IAppBuilder with a standard implementation to connect (without its pre-defined middlewares), and Katana to express.js and others connect-based Frameworks and middlewares.

panesofglass commented 10 years ago

node does provide a sort of builder abstraction with connect; express is based on connect.

Oh, that's right! I forgot about connect. Thanks! I suppose an Owin.Builder would be something like connect, then? I would still prefer that be based on Func<AppFunc, AppFunc>, though.

half-ogre commented 10 years ago

Connect sort of conflates Rack and Rack::Builder (or OWIN and Owin.Builder, if you prefer), going a step beyond simply including the builder in the main package like Rack does.

I would still prefer that be based on Func<AppFunc, AppFunc>, though.

I still think it would be better for IAppBuilder to live in Owin.Builder than in owin.dll (Rack is Ruby-only, so we have some concerns it doesn't), but of the two things, settling on a middleware delegate function is far more important to me.

panesofglass commented 10 years ago

I still think it would be better for IAppBuilder to live in Owin.Builder than in owin.dll (Rack is Ruby-only, so we have some concerns it doesn't), but of the two things, settling on a middleware delegate function is far more important to me.

I can live with that. I also agree there is no reason to release a new owin.dll. Just let v1.0 die and start referencing Owin.Builder.dll.

I honestly just don't care about any builder abstraction. I have a feeling others, such as @markrendle, might. Again, I don't think anyone has shown that IAppBuilder is used in anything other than Katana, and since it doesn't match any OWIN signatures, it's a really confusing thing to include in an owin.dll. I have not seen any reason to not eventually move IAppBuilder to the Microsoft.Owin libraries in a future release.

If we want a common Owin.Builder, we should start that discussion. If everyone wants to adopt IAppBuilder, no sweat. Let's stick it in Owin.Builder and move on. Let's just not do that without consensus this time.

Tratcher commented 10 years ago

@panesofglass AppBuilder is all based on Func<AppFunc, AppFunc>, though as @skoon pointed out, we add types like this so we don't have to work directly against the LCD. That's why it supports a few of the derived patterns as well.

Also, with the number of desperate packages already dependent on owin.dll v1, it would not be viable for them to move one-at-a-time to a type-incompatible owin.builder.dll without a transition story like the one described above. Without type-redirect compat you're left in a chicken-or-egg position like IPv4 vs IPv6, where you don't want to move because you'll only be able to talk to those who have already moved, only worse because you won't be able to use the old one and the new one at the same time.

v1 would also take a very long time to die, and cause much confusion in the meantime, if you didn't help it along with a nuget replacement/redirect.

panesofglass commented 10 years ago

@Tratcher Perhaps I need to pose the problem a different way. Please tell me how I run the Web API middleware on anything other than Katana today. How do I run that on Fix? How do I run it on Simple.Owin? How do I use it in a F# pipeline, such as the one below?

let hello (env: IDictionary<string, obj>) = async {
    let stream = env.["owin.ResponseBody"] |> unbox<Stream>
    do! stream.WriteAsync("Hello, world!"B, 0, 13)
} |> Async.StartAsTask :> Task

let config = new HttpConfiguration()
let useWebApi next = // something taking my config and expecting a next? Except this doesn't exist.

// The following should allow me to pass my "hello" as a "next"
let app = hello |> useWebApi config
// val app : IDictionary<string, obj> -> Task
// Note that in this case, .Use is simple function composition which is built into the language.

In addition to not exposing anything directly reusable by other implementations, these are using the Microsoft.Owin wrapper types. These are convenient, yes. However, this middleware is Katana-specific.

Apologies for stating incorrectly that IAppBuilder was Katana-specific. @loudej corrected me on that point. However, I note again that I know of no other implementation using IAppBuilder, and the middelware is not OWIN middleware but Katana middleware. Thus, at this point, Katana is its own subdomain of OWIN, and I still think IAppBuilder belongs to that subdomain.

If anyone can tell me how to use most of the Katana middleware either in Fix or Simple.Owin or F#, I would happily back down on this request.

serialseb commented 10 years ago

I'm also of the opinion that I can delay further conversation and annoyance of IAppBuilder (on this list and as part of owin work anyway) until we have worked on the middleware signature as a spec.

Date: Fri, 6 Dec 2013 10:23:21 -0800 From: notifications@github.com To: owin@noreply.github.com CC: seb@serialseb.com Subject: Re: [owin] Move IAppBuilder to Microsoft.Owin (#19)

Connect sort of conflates Rack and Rack::Builder (or OWIN and Owin.Builder, if you prefer), going a step beyond simply including the builder in the main package like Rack does.

I would still prefer that be based on Func, though.

I still think it would be better for IAppBuilder to live in Owin.Builder than in owin.dll (Rack is Ruby-only, so we have some concerns it doesn't), but of the two things, settling on a middleware delegate function is far more important to me.

— Reply to this email directly or view it on GitHub.

serialseb commented 10 years ago

Same problem as for apps, this article is a good example: http://www.asp.net/aspnet/overview/owin-and-katana/owin-startup-class-detection How do I run this app without IAppBuilder? Where is the AppFunc exposed? How do I explain a host does not support IAppBuilder but is owin when the article says "Every OWIN Application has a startup class" and "The startup class shown in this tutorial can be used in every hosting application." I think we can delay the conversation until the middleware problem is solved, but right now what I'm hearing is an explanation of discontent, "We didn't sign up for that IAppBuilder and now it's required", and another camp saying "Well, it's done now, compat, microsoft has released stuff, we cant and should not remove, and it should be come part of the spec because we built it". Denying that IAppBuilder is a fail in respects to the previous points will only lead to anguish, resentment and indelible impact on the credibility of this group and its aims. It's clear we have a problem. As such, while I completely oppose any work being done on any builder of any form for now, I think we can move forward by having a spec for "Exposing AppFunc" and "MidFunc" on top of defining MidFunc. This is, I think, something we could reach consensus on, and I do believe our only current functioning governance model has been consensus.

Date: Fri, 6 Dec 2013 21:04:37 -0800 From: notifications@github.com To: owin@noreply.github.com CC: seb@serialseb.com Subject: Re: [owin] Move IAppBuilder to Microsoft.Owin (#19)

@Tratcher Perhaps I need to pose the problem a different way. Please tell me how I run the Web API middleware on anything other than Katana today. How do I run that on Fix? How do I run it on Simple.Owin? How do I use it in a F# pipeline, such as the one below?

let hello (env: IDictionary<string, obj>) = async { let stream = env.["owin.ResponseBody"] |> unbox do! stream.WriteAsync("Hello, world!"B, 0, 13) } |> Async.StartAsTask :> Task

let config = new HttpConfiguration() let useWebApi next = // something taking my config and expecting a next? Except this doesn't exist.

// The following should allow me to pass my "hello" as a "next" let app = hello |> useWebApi config

In addition to not exposing anything directly reusable by other implementations, these are using the Microsoft.Owin wrapper types. These are convenient, yes. However, this middleware is Katana-specific.

Apologies for stating incorrectly that IAppBuilder was Katana-specific. @loudej corrected me on that point. However, I note again that I know of no other implementation using IAppBuilder, and the middelware is not OWIN middleware but Katana middleware.

Thus, at this point, Katana is its own subdomain of OWIN, and I still think IAppBuilder belongs to that subdomain.

If anyone can tell me how to use most of the Katana middleware either in Fix or Simple.Owin or F#, I would happily back down on this request.

— Reply to this email directly or view it on GitHub.

loudej commented 10 years ago

The consensus which formed is that the fundamental middleware definition is Func<AppFunc, AppFunc>.

The inline representation of this is

int callCount = 0;

Func<AppFunc, AppFunc> callCounter = next => async env =>
{
    env["callCount"] = ++callCount;
    await next(env);
};

Although the actual request has the single env argument, it is simple to close over the next argument, so next was never part of the per-request signature.

Ruby was also examined to see how they kept the rackup syntax clean if their middleware are each individual lambdas. What was discovered was that their middleware were always defined as a class type. It turned out you pass a class reference when a function is expected - the class's constructor automatically becomes the target of the function and the new instance is the return value. This is the same in Ruby and JavaScript. The only thing that needs to be true of the return value in either case is that the resulting value has an Invoke method. That is true whether it is a class instance or a delegate, because all delegates have a public Invoke method.

In C# the class form of the call counter middleware looks like this

public class CallCounter
{
    private readonly AppFunc _next;
    private int _callCount;

    public CallCounter(Func<IDictionary<string, object>, Task> next)
    {
        _next = next;
    }

    public async Task Invoke(IDictionary<string, object> env)
    {
        env["callCount"] = ++_callCount;
        await _next(env);
    }
}

You can shift between these two forms freely.

Func<AppFunc, AppFunc> callCounter = next => new CallCounter(next).Invoke;

That shift also works on the MS.Owin.* components. For example:

class MyBuilder
{
    public void Add(Func<AppFunc, AppFunc> middleware)
    {
    }
}

var builder = new MyBuilder();

int callCount = 0;
builder.Add(next => async env =>
{
    env["callCount"] = ++callCount;
    await next(env);
});

var staticFileOptions = new StaticFileOptions
{
    // set properties here
};
builder.Add(next => new StaticFileMiddleware(next, staticFileOptions).Invoke);

And as you can see, the final line is pure, portable, and looks like absolute garbage. So - an extension method on IAppBuilder is offered as syntax sugar app.UseStaticFile(new StaticFileOptions { }).

In the end:

IF your builder inherits from IAppBuilder THEN your system gets syntax sugar for free ELSE your builder can do whatever it wants AND the user can type a next=> lambda to add anything they like

serialseb commented 10 years ago

I've come up with a builder specification that fits my needs well:

public static class Extensions
{
    public static Action<MidFunc> Use { get { return null; }}
    public static Action<MidFunc> Stuff(this Action<MidFunc> function)
    {
        function(MyMiddleware.Wire);
        return function;
    }
}

Date: Sat, 7 Dec 2013 08:56:20 -0800 From: notifications@github.com To: owin@noreply.github.com CC: seb@serialseb.com Subject: Re: [owin] Move IAppBuilder to Microsoft.Owin (#19)

The consensus which formed is that the fundamental middleware definition is Func<AppFunc, AppFunc>.

The inline representation of this is

int callCount = 0;

Func<AppFunc, AppFunc> callCounter = next => async env => { env["callCount"] = ++callCount; await next(env); };

Although the actual request has the single env argument, it is simple to close over the next argument, so next was never part of the per-request signature.

Ruby was also examined to see how they kept the rackup syntax clean if their middleware are each individual lambdas. What was discovered was that their middleware were always defined as a class type. It turned out you pass a class reference when a function is expected - the class's constructor automatically becomes the target of the function and the new instance is the return value. This is the same in Ruby and JavaScript. The only thing that needs to be true of the return value in either case is that the resulting value has an Invoke method. That is true whether it is a class instance or a delegate, because all delegates have a public Invoke method.

In C# the class form of the call counter middleware looks like this

public class CallCounter { private readonly AppFunc _next; private int _callCount;

public CallCounter(Func<IDictionary<string, object>, Task> next)
{
    _next = next;
}

public async Task Invoke(IDictionary<string, object> env)
{
    env["callCount"] = ++_callCount;
    await _next(env);
}

}

You can shift between these two forms freely.

Func<AppFunc, AppFunc> callCounter = next => new CallCounter(next).Invoke;

That shift also works on the MS.Owin.* components. For example:

class MyBuilder { public void Add(Func<AppFunc, AppFunc> middleware) { } }

var builder = new MyBuilder();

int callCount = 0; builder.Add(next => async env => { env["callCount"] = ++callCount; await next(env); });

var staticFileOptions = new StaticFileOptions { // set properties here }; builder.Add(next => new StaticFileMiddleware(next, staticFileOptions).Invoke);

And as you can see, the final line is pure, portable, and looks like absolute garbage. So - an extension method on IAppBuilder is offered as syntax sugar app.UseStaticFile(new StaticFileOptions { }).

In the end:

IF your builder inherits from IAppBuilder THEN your system gets syntax sugar for free

ELSE your builder can do whatever it wants AND the user can type a next=> lambda to add anything they like

— Reply to this email directly or view it on GitHub.

davidfowl commented 10 years ago

I gotta say, I've been following this thread for a while and I'm seeing a bunch of assumptions without people actually asking if things are supported or not. Twice on this same thread I've seen assumptions debunked. Owin.dll is optional, IAppBuilder does not need to be moved and it provides a very nice pattern for wiring up middleware.

The katana hosts supports a few patterns:

The IAppBuilder way:

using Microsoft.Owin;
using Owin;

[assembly: OwinStartup(typeof(MyWebApplication.Startup))]

namespace MyWebApplication
{
    public class Startup
    {
        public void Configuration(IAppBuilder app)
        {
            app.Run(async context =>
            {
                await context.Response.WriteAsync("Hello World");
            });
        }
    }
}

The pure typeless owin way:

using System;
using System.Collections.Generic;
using System.IO;
using System.Threading.Tasks;

namespace MyWebApplication
{
    using AppFunc = Func<IDictionary<string, object>, Task>;

    public class Startup
    {
        public AppFunc Configuration()
        {
            return async env =>
            {
                var writer = new StreamWriter((Stream)env["owin.ResponseBody"]);

                await writer.WriteAsync("Hello World");
                await writer.FlushAsync();
            };
        }
    }
}

Given this knowledge, your host can support whatever entrypoint it wants to typed or typeless. The katana middleware (as shown above) can work with either:

IAppBuilder

using System;
using Microsoft.Owin;
using Microsoft.Owin.StaticFiles;
using Owin;

[assembly: OwinStartup(typeof(MyWebApplication.Startup))]

namespace MyWebApplication
{
    public class Startup
    {
        public void Configuration(IAppBuilder app)
        {
            app.UseStaticFiles(new StaticFileOptions
            {
            });
        }
    }
}

OR Not

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.Owin.StaticFiles;

namespace MyWebApplication
{
    using AppFunc = Func<IDictionary<string, object>, Task>;

    public class Startup
    {
        public AppFunc Configuration()
        {
            var builder = new MyBuilder();

            builder.Add(next =>
            {
                var options = new StaticFileOptions { };
                return new StaticFileMiddleware(next, options).Invoke;
            });

            return builder.CreatePipeline();
        }
    }
}

Moving IAppBuilder doesn't solve anything. IMO it just breaks the current ecosystem built on top of it and it gains nothing while doing that.

Just to further prove that katana doesn't require any MS.Owin pieces to run the middleware dubbed as "Katana middleware":

image

Take a look at the references...

serialseb commented 10 years ago

How does that work with the example I linked from the asp.net website?

Date: Sat, 7 Dec 2013 11:12:36 -0800 From: notifications@github.com To: owin@noreply.github.com CC: seb@serialseb.com Subject: Re: [owin] Move IAppBuilder to Microsoft.Owin (#19)

I gotta say, I've been following this thread for a while and I'm seeing a bunch of assumptions without people actually asking if things are supported or not. Twice on this same thread I've seen assumptions debunked. Owin.dll is optional, IAppBuilder does not need to be moved and it provides a very nice pattern for wiring up middleware.

The katana hosts supports a few patterns:

The IAppBuilder way:

using Microsoft.Owin; using Owin;

[assembly: OwinStartup(typeof(MyWebApplication.Startup))]

namespace MyWebApplication { public class Startup { public void Configuration(IAppBuilder app) { app.Run(async context => { await context.Response.WriteAsync("Hello World"); }); } } }

The pure typeless owin way:

using System; using System.Collections.Generic; using System.IO; using System.Threading.Tasks;

namespace MyWebApplication { using AppFunc = Func<IDictionary<string, object>, Task>;

public class Startup
{
    public AppFunc Configuration()
    {
        return async env =>
        {
            var writer = new StreamWriter((Stream)env["owin.ResponseBody"]);

            await writer.WriteAsync("Hello World");
            await writer.FlushAsync();
        };
    }
}

}

Given this knowledge, your host can support whatever entrypoint it wants to typed or typeless. The katana middleware (as shown above) can work with either:

IAppBuilder

using System; using Microsoft.Owin; using Microsoft.Owin.StaticFiles; using Owin;

[assembly: OwinStartup(typeof(MyWebApplication.Startup))]

namespace MyWebApplication { public class Startup { public void Configuration(IAppBuilder app) { app.UseStaticFiles(new StaticFileOptions { }); } } }

OR Not

using System; using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.Owin.StaticFiles;

namespace MyWebApplication { using AppFunc = Func<IDictionary<string, object>, Task>;

public class Startup
{
    public AppFunc Configuration()
    {
        var builder = new MyBuilder();

        builder.Add(next =>
        {
            var options = new StaticFileOptions { };
            return new StaticFileMiddleware(next, options).Invoke;
        });

        return builder.CreatePipeline();
    }
}

}

Moving IAppBuilder doesn't solve anything. IMO it just breaks the current ecosystem built on top of it and it gains nothing while doing that.

Just to further prove that katana doesn't require any MS.Owin pieces to run the middleware dubbed as "Katana middleware":

Take a look at the references...

— Reply to this email directly or view it on GitHub.

davidfowl commented 10 years ago

@serialseb which part of the doc? There's tons of examples in there. The fact that your host may not support IAppBuilder is irrelevant, just throw an exception saying it doesn't. You don't have to use the Startup convention that katana uses and even if you did, it doesn't need to support IAppBuilder.

I guess I just don't see why we need to move or change IAppBuilder in anyway. It's additional goodness while we still support doing everything the raw way as well.

panesofglass commented 10 years ago

@davidfowl @Tratcher just mentioned above that Static Files is the first to support this. I'm very glad to know that this will be the migration forward. However, most of the middlewares I've looked at don't support this ease of interoperatbility. You also miss the point. I'm not saying IAppBuilder is bad or doesn't work. I'm saying IAppBuilder is only used in Katana stuff and is causing middleware to be built in a breaking fashion. That's not necessary, and that's the point.

The second point is that IAppBuilder is not, so far as I recall, an agreed upon interface and should be moved to its proper location. I remember we originally had an owin.dll to help people understand the delegate structure originally proposed. owin.dll was published with IAppBuilder without me knowing it. I don't know if I just missed that call or what. (I'm not saying it has to have my sign-off; I just don't recall this being a consensus among the OWIN organizers.)

davidfowl commented 10 years ago

IAppBuilder isn't only used in katana stuff (connect-owin/nancy/signalr use/support it for example) and the ecosystem is brand new so saying that only katana supports it means nothing, it was just the first one to take a bigger bet on it.

To your second point, maybe it wasn't agreed upon and I'm sure the calls fell off or people became uninterested at some point but why bother breaking the ecosystem coming up on IAppBuilder when it can be safely ignored and alternatives can be used in its place? Why can't there be another parallel stack like katana that chooses to take a bet on IAppBuilder?

It is not katana specific. It may not have been agreed upon but it doesn't make it specific to the Microsoft Owin stack. It's an option.

prabirshrestha commented 10 years ago

Owin.dll is optional, IAppBuilder does not need to be moved and it provides a very nice pattern for wiring up middleware.

The reason I have been personally against IAppBuilder being in the core owin.dll is because nothing about it is mentioned in the spec - http://owin.org/spec/owin-1.0.0.html Hence the move to Owin.Builder.dll or better to Katana.Builder.dll.

What about having something like the OWIN Key Guidelines and Common Keys for middleware, builders and starters? This also means we wouldn't need to rev the owin spec and the owin spec becomes as simple as it is now. Might be trying to draft the first version of it would be a good start?

davidfowl commented 10 years ago

It's not in the spec because it's optional (same as Owin.dll). It doesn't belong in anything called Katana*.dll (IMO).

I can get behind another guideline document that shows outlines ways to startup (if people can agree on that).

schmylan commented 10 years ago

Not a solution here, just helping shape the conversation. So we have the Katana Way :tm: and the Raw Way :tm:. Many devs' confusion would be reduced if there was an alternative to Katana. It would help unblur the lines regarding where OWIN stops and where "personal preference" begins.

Update: no not inside owin.dll, just from the communitiy.

prabirshrestha commented 10 years ago

It's not in the spec because it's optional (same as Owin.dll).

Most of us here in this discussion would know that Owin.dll is optional. But it has definitely been confusing to those new folks who jump on owin. With IAppBuilder part of it they think it is part of the owin spec.

It doesn't belong in anything called Katana*.dll (IMO).

The only problem would be if someone else writes a new builder then I would need to reference X.Builder.dll to add something like app.UseNancy(). Hence the issue #20. I don't care about the builder, I care what I can pass in the Use method as all builders will need it. This was why the Func<AppFunc,AppFunc> was brought up. https://github.com/owin/owin/issues/20#issuecomment-29743460. I'm good with having multiple too like next and options in ctor with invoke method in draft versions.

I can get behind another guideline document that shows outlines ways to startup (if people can agree on that).

I think startup would be the most difficult to agree on. but +1 for startup/middleware/builder guidelines. Middleware would definitely be the highest priority. At least starting the guidelines would be better then nothing. So is this gonna be like the OWIN common keys or just how the katana support different alternatives currently?

In the end here is what I want. So as a middleware author I would want to read the middleware guidelines and expose the api based on it, whether it is func or ctor or others. If there are multiple I can choose to expose the one that is best for me.

panesofglass commented 10 years ago

:+1: to @prabirshrestha.

@schmylan I noted several alternatives above, none of which use IAppBuilder. You can read the wiki for the list, as well.

davidfowl commented 10 years ago

@panesofglass to be pedantic, connect-owin uses an AppBuilder internally and it supports that pattern. https://github.com/bbaia/connect-owin/blob/master/src/OwinMiddleware.cs#L59

loudej commented 10 years ago

And Fix does expose IAppBuilder, although not yet in a way that's as compatible as it could be with connect-owin or microsoft.owin, https://github.com/markrendle/Fix/blob/28779d381dcc746ac339e92e6e3eed69b358bdd7/src/Fix.AppBuilder/FixAppBuilder.cs

serialseb commented 10 years ago

Self-censored unconstructive comments.

loudej commented 10 years ago

What colorful imagery! :)

I appreciate the passion as well, as a starting point it's something we all have in common.

panesofglass commented 10 years ago

@loudej LOL. Also, thank you and @davidfowl for pointing to two implementations that use the current interface.

That said, I don't know if you can really count Fix since, as you note, it sort of paints the problems in the ambiguity of the interface. One might argue that, with the looseness of the interface, we would be just as well off with another dictionary or even ExpandoObject. (Dictionary was proposed in the thread linked above.)

@serialseb let's try to keep the public discussions focused on the tech.

damianh commented 10 years ago

Everyone needs to take a deep breath. Threatening 'nuclear option' is never going to be a productive stance.

The owin nuget package shipped in November 13, 2012. Owin the project and owin.dll existed for some time before that. It seems to me that important people either took their eye off the ball / abandoned the process / dropped off the map / whatever. And now that things have gained traction, and really useful stuff is shipping, it's getting all alpha.

It's a matter of urgency that a committee (re-)forms and a proper, active, engaging governance model is put in place. If that doesn't happen then we, "the community", fail.

panesofglass commented 10 years ago

@damianh I agree the next step is restoring governance.

To your point about owin.dll, that originally existed to provide the Delegate of Doom. IAppBuilder, or its predecessor, existed in Gate, and most or all of Gate moved to Katana. IAppBuilder moved over and replaced the spec'd OWIN delegate. I wish I could find any docs or minutes on that.

One problem we must address is that a community effort will involve people engaged part time with some gaps in their availability. For this, we need excellent minutes and written dialogue, along with periods of review to allow everyone involved to respond. I think this is what broke down around the push to 1.0.

For what it is worth, I am very glad we pushed to get 1.0 out. We had a lot of things go right and some things missed. That is to be expected. We will always have some refactoring after the fact. That is how I see this current issue.

prabirshrestha commented 10 years ago

The consensus which formed is that the fundamental middleware definition is Func<AppFunc, AppFunc>.

@loudej @Tratcher might be one of you guys can start the first draft of middlewares guidelines on it.

half-ogre commented 10 years ago

It's a matter of urgency that a committee (re-)forms and a proper, active, engaging governance model is put in place. If that doesn't happen then we, "the community", fail.

+:100: to this. I don't see new points being made here. I think we've reached that time when OWIN's governors need to either raise new points to be addressed before a decision can be made, or make a decision. (And if OWIN's governance has been to reach consensus of all the many voices involved, I think it has outgrown that model.)

loudej commented 10 years ago

@half-ogre Well said. It remains to be seen if we have outgrown the governance model which was in effect. Let's put it to the test!

The issue #19 will remain open pending an actual conclusion. It will be added to the agenda for the next call, Wed 11th Dec 2013, 11am Pacific, 7pm London.

On Monday look to https://github.com/owin/owin/wiki for the call-in details and agenda. That information will also be emailed to the http-net-abstractions group.

Please do not reply to this issue if you have other suggestions for the agenda, or if you are interested but would like to postpone this topic for the following week due to conflicts. Rather, wait to share that on the wiki page or as a reply to the meeting announcement thread.

prabirshrestha commented 10 years ago

While we are on the IAppBuilder stuff.

After a chat with @davidfowl and @loudej on jabbr. They have confirmed that IAppBuilder isn't needed for the startup.

So this works for any other hosts that wants to auto load and start the app without dependencies on IAppBuilder on katana.

public class Startup {
    public AppFunc Configuration(IDictionary<string, object> startupEnv) {
        return SomeAppFunc;
    }
}

something like this is possible if you want to take dependency on IAppBuilder without exposing IAppBuildr to public.

public class Startup {
    public AppFunc Configuration(IDictionary<string, object> startupEnv) {
        var builder = new AppBuilder(startupEnv);
        return (AppFunc)builder.Build(typeof(AppFunc)); // or builder.Build() extension method.
    }
}
half-ogre commented 10 years ago

@prabirshrestha I don't think anyone is or was mistaken on whether owin.dll is required by other hosts or builders. It's about IAppBuilder being given an unwarranted primacy of place in owin.dll, and the potential for it to be perceived as a "blessed" interface as a consequence of that placement.

loudej commented 10 years ago

It is something he was mistaken about, or he wouldn't have posted it as a clarification. And owin.builder.dll is no less "blessed" than owin.dll.