Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip allocating new HttpContextWrapper for every LayoutRenderer #782

Open
snakefoot opened this issue May 29, 2022 · 2 comments
Open

Skip allocating new HttpContextWrapper for every LayoutRenderer #782

snakefoot opened this issue May 29, 2022 · 2 comments
Milestone

Comments

@snakefoot
Copy link
Contributor

snakefoot commented May 29, 2022

It is silly that we have this code:

    public class DefaultHttpContextAccessor : IHttpContextAccessor
    {
        public HttpContextBase HttpContext
        {
            get
            {
                var httpContext = System.Web.HttpContext.Current;
                if (httpContext == null)
                    return null;
                return new HttpContextWrapper(httpContext); // Unnecessary allocation
            }
        }
    }

Would be better just using System.Web.HttpContext-instance directly, since allocation of HttpContextWrapper does not seem to provide any value (Since there is TestInvolvingAspNetHttpContext)

@snakefoot snakefoot added this to the 6.0 milestone May 29, 2022
@snakefoot snakefoot added performance ASP.NET 4 ASP.NET MVC Classic labels May 29, 2022
@bakgerman
Copy link
Contributor

I attempted this, had many build errors, once those were resolved many unit tests fail since NSubstitute will not work on HttpContext, the methods are not virtual. Since HttpContextBase or HttpContextWrapper is not an HttpContext I am pondering how to leave NLog.Web using HttpContext directly but have NLog.Web.Tests use HttpContextBase, have not reached an answer yet.

@snakefoot
Copy link
Contributor Author

snakefoot commented Aug 13, 2023

Maybe just a manual mock like this:

        HttpContext httpContext = new HttpContext(new MyWorkerRequest());

        public class MyWorkerRequest : SimpleWorkerRequest
        {
            public MyWorkerRequest()
                :base("/", "/", "/", "", new StringWriter(CultureInfo.InvariantCulture))
            {
            }

            public override bool IsEntireEntityBodyIsPreloaded() => true;
            public override byte[] GetPreloadedEntityBody() => Array.Empty<byte>();
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants