damianh / LimitsMiddleware

OWIN middleware to apply limits to an OWIN pipeline.
MIT License
0 stars 0 forks source link

Add support for configurable reason phrases (with option objects) #9

Closed StefanOssendorf closed 10 years ago

StefanOssendorf commented 10 years ago

From Issue #8 (Thought of damianh) Am also thinking that I'd like to make the reason phrases configurable, so rather then overloading the UseX extension method, we could use an options object (like Nancy https://github.com/NancyFx/Nancy/blob/master/src/Nancy.Owin/NancyOptions.cs )

How does this look, shortened for berevity?

public class MaxRequestLengthContentOptions
{
    public MaxRequestLengthContentOptions(int maxContentLength)
    {
    }

    public Action<TraceEventType, string> Logger { get; set; }

    public Func<int, string> LimitReachedReasonPhrase { get; set; }

    ....
}

// leave the other overloads as they are but they can be delegated
// to this options ext method
app.MaxRequestContentLength(new MaxRequestLengthContentOptions(1000) { Logger = (eventType, message) => { ... } );

Think a spike may be in order...

StefanOssendorf commented 10 years ago

Gosh! How can i mark this as enhancment?

StefanOssendorf commented 10 years ago

See: https://github.com/StefanOssendorf/Owin.Limits/blob/master/src/Owin.Limits/MaxQueryStringLengthMiddleware.cs https://github.com/StefanOssendorf/Owin.Limits/blob/master/src/Owin.Limits/MaxQueryStringLengthOptions.cs

If it's the right direction, I implement the rest.

damianh commented 10 years ago

Looks great! Will need the corresponding method in AppBuilderExtensions of course. May also add a Logger property to MaxQueryStringLengthOptions while you are at it?

Any chance you can run the resharper code cleanup template before the PR?

StefanOssendorf commented 10 years ago

AppBuilderExtensions - of course. I'll redirect the existing two to the new one. Logger - Yep!

Resharper cleanup - Yes, but I do have to figure out how i can configure my resharper for projects. At the moment my persnal settings are messing up the bracktes and namespace declarations.

damianh commented 10 years ago

I'll update the resharper solution level settings so you shouldn't have to do anything... On 7 Apr 2014 19:09, "Stefan Ossendorf" notifications@github.com wrote:

AppBuilderExtensions - of course. I'll redirect the existing two to the new one. Logger - Yep!

Resharper cleanup - Yes, but I do have to figure out how i can configure my resharper for projects. At the moment my persnal settings are messing up the bracktes and namespace declarations.

Reply to this email directly or view it on GitHubhttps://github.com/owin-middleware/Owin.Limits/issues/9#issuecomment-39756615 .

StefanOssendorf commented 10 years ago

Damn ReSharper... I can not find a solution >_< Situation: In my "This Computer" settings i have K&R style for brace layout defined. And now i can't figure out how to tell ReSharper that the solution settings with BSD style isn't the default. Any idea? :-/

damianh commented 10 years ago

Don't worry about it. In anycase, I'll upload a sln setting file tomorrow ;)

On 10 Apr 2014 21:49, "Stefan Ossendorf" notifications@github.com wrote:

Damn ReSharper... I can not find a solution >_< Situation: In my "This Computer" settings i have K&R style for brace layout defined. And now i can't figure out how to tell ReSharper that the solution settings with BSD style isn't the default. Any idea? :-/

Reply to this email directly or view it on GitHubhttps://github.com/owin-middleware/Owin.Limits/issues/9#issuecomment-40129978 .

damianh commented 10 years ago

@StefanOssendorf Added some more explicit settings to the team shared settings file. Let me know if it works out. (Doesn't matter anyway).

StefanOssendorf commented 10 years ago

Yeah it works :-) Thank you very much :+1: But resharper modifies all files - removing all empty lines. (with cleanup code) Is that okay?

damianh commented 10 years ago

No... you've obviously heavily modified the resharper defaults. Edit the 'Team-shared' layer and tweak the settings so the formatting align. (For some reason I can't explicitly set those because I have minimally derived from the r# defaults...)

image

StefanOssendorf commented 10 years ago

Apparently. I've reseted ally m settings and now it works properly. I'll figure out how i can get both formatting styles in different solutions. Nevertheless, many thanks for your help.

StefanOssendorf commented 10 years ago

issue solved/implemented? :-)

damianh commented 10 years ago

Yup.