Aspnetboilerplate: Why is /GetScripts not cachable and/or cached?

Created on 26 Jul 2018  ·  26Comments  ·  Source: aspnetboilerplate/aspnetboilerplate

Hi,

  • ASP.NET 4.6.1
  • ABP 3.7.2
  • MVC without ZERO (ie permissions, menu etc). Basically we GetScripts() for generating service proxies

Now while I realise it is generated at runtime, in my case it is 100% the same all of the time. And as a result it is returned for every single page request.

So why can it not be cached internally with a circuit breaker as needed?

We use the following circuit breaker code to deal with physical files on disk where we hash the last changed file time:

```c#
///


///
/// Use this class as follows to create cache busing URL's:
///

///
///
///

///

///
///
/// By adding this snippet of XML to the web.config’s
///
/// section, we instruct IIS 7+ to intercept all URLs with a
/// folder name containing “v=[md5Hash]” and rewrite the URL to the original file path.
///

///
///
///
///
///
///

///

///
/// You need to run the AppPool in Integrated Pipeline mode for the section to have any effect.
///

///

public static class OzCpCacheBusterHelper
{
///
/// Constructs a cache busting path in the form of /original/path/to/file.css/v-xxx where
/// xxx is the MD5 hash of the last write timestamp of the physical file located at
/// \WebRoot\original\path\to\file.css
///

/// Relative path to the file.
/// Path to the CDN version of the file.
/// Returns a valid cache busting path to the original resource.
public static string FingerPrint(string aRootRelativePath, string aCdnPath = "")
{
//Return the CDN path if we have one assigned and we are not debugging
if (!string.IsNullOrEmpty(aCdnPath) && !HttpContext.Current.IsDebuggingEnabled)
{
return aCdnPath;
}

        //Should the relative path not already exist in the cache then lets add it
        if (HttpRuntime.Cache[aRootRelativePath] == null)
        {
            //Determine both absolute and relative paths
            string relative = VirtualPathUtility.ToAbsolute("~" + aRootRelativePath);
            string absolute = HostingEnvironment.MapPath(relative);

            if (!File.Exists(absolute))
            {
                throw new FileNotFoundException("File not found", absolute);
            }

            //Construct a file path in the form {relative}/v-{Md5 Of Last Write Time}/ to ensure we have unique path 
            DateTime fileLastWriteTime = File.GetLastWriteTime(absolute);
            int index = relative.LastIndexOf('/');
            string result = relative.Insert(index, "/v-" + Md5($"{fileLastWriteTime:yyy-MM-dd_hh:mm:ss.fff}"));
            HttpRuntime.Cache.Insert(aRootRelativePath, result, new CacheDependency(absolute));
        }
        return HttpRuntime.Cache[aRootRelativePath] as string;
    }

    /// <summary>
    ///     Computes an MD5 hash of the input value
    /// </summary>
    /// <param name="aInput">String value to hash.</param>
    /// <returns>Returns an MD5 hash of the input as a string.</returns>
    private static string Md5(string aInput)
    {
        //Compute the hash
        MD5 md5 = MD5.Create();
        byte[] inputBytes = Encoding.ASCII.GetBytes(aInput);
        byte[] hash = md5.ComputeHash(inputBytes);

        //Convert byte array to string
        StringBuilder sb = new StringBuilder();
        foreach (byte hashCharacter in hash)
        {
            sb.Append(hashCharacter.ToString("X2"));
        }
        return sb.ToString();
    }
}

```

That way clients receive a cached copy until the file is updated. I would think something similair could be applied to the generated contents of GetScripts() ie. An MD5 of its contents and if unchanged then the content is returned from a HttpRuntime.Cache[] entry.

The time to perform the above MD5 and cache inspection would be well worth it as I am guessing that would be less than the time it takes to deliver GetScripts() on the wire as well as the saved bytes on the wire.

Thoughts?

enhancement high

Most helpful comment

@ismcagdas You guys already cache it at the server level, so how is it any different if you were to cache the requests lol. Just invalidate it when someone changes a permission or localization in the UI. You already do it now for the local cache. The other option is to break out things that can be cached and what can not. I assume the app service js stuff can be.

https://stackoverflow.com/questions/1167890/how-to-programmatically-clear-outputcache-for-controller-action-method

All 26 comments

Are you using asp net 4.x or asp net core?

.NET 4.x (updated original post)

https://github.com/aspnetboilerplate/aspnetboilerplate/blob/19b10b4e21d2b716e17f860ea74e30b8a64ea8d8/src/Abp.Web.Api/WebApi/Controllers/Dynamic/Scripting/ScriptProxyManager.cs#L17

NET 4.x ABP is a dictionary to store script information. It is a program cache.
I don't feel it will affect performance.

It affects network performance because it is served up on every request even when the content has not changed as the template I have says:

<!-- Dynamic ABP scripts - runtime creation and can not be bundled -->
<script src="/AbpScripts/GetScripts" type="text/javascript"></script>

No need to keep serving up content that is unchanged.

@ismcagdas is there a place in ABP where I can intercept the content of GetScripts() and implement my own caching?

So how do I replace the existing implementation with my own? Replacing a service does not appear to be relevant here.

Now that you have a custom controller, you only need to customize a route url.

Replace the url of the get script with your routing url address. No need to replace any service.

Same question asked in AspNetZero:
https://github.com/aspnetzero/aspnet-zero-core/issues/1319

"AbpScripts/GetScripts contains settings, localization values (might be changed on the UI) etc.. That is why it is not good to cache this.

However, we can cache AbpServiceProxies/GetAll because it is only updated when we add a new app service.
"

"AbpScripts/GetScripts contains settings, localization values (might be changed on the UI) etc.. That is why it is not good to cache this."

Well not really. By all means send something when it changes but for the most part it will pretty much remain unchanged once the settings and localization values have been configured. After all once I add the Afrikaans for OK I am not going to change it daily. Once I configure the SMTP settings they are not going to change daily. Weekly perhaps but in that week there is no point sending out the same thing unchanged for the entire week.

@natiki as you probably know; settings & permissions are already cached. It does not read from database for every request. Bu we can make additional optimizations, yes.

So just for completeness I am including what I am seeing. As you can see it is a stock standard deployment and the consequences of a 6.7k 1s worth of data coming back each request means things add up pretty quickly.

image

GetScripts.txt

Hi guys, just wanted to chip in here as I've noticed an issue affecting our front end.

I had @natiki's issue long ago, and fixed it by adding this to nmy Global.asax (ye 'old ASP 4.7)

protected override void Application_EndRequest(object sender, EventArgs e)
        {
            if (Context.Request.Url.ToString().Contains("/AbpServiceProxies/GetAll") || Context.Request.Url.ToString().Contains("/AbpScripts/GetScripts"))
            {
                //workaround for ABP's caching
                Response.Headers["Cache-Control"] = "public";
                Response.Headers["Expires"] = DateTime.Now.AddDays(1).ToString();
            }


        }

However,
Since GetScripts contains permissions for the logged in user, what I was observing was a users permissions being carried onto another user after logging out and then in again.

Obviously this is happening because my front end was caching GetScripts and causing horrible side affects when a user logs out, and another user logs in.

The way I will fix this for now is by appending the uid as a blank url parameter. But ideally there would be a better way to force a re-get of GetScripts when a user logs in.

tbh I might just disable it, and put it back to how it was - since permissions etc can be changed on the fly at least for GetScripts.

As we don't have permissions issues we did end up caching it in our MVC applications by changing our templates to call:

Our Layout controller then has /layout/getscripts:

```c#
///


/// Allows us to cache the ABP GetScripts() call which is slow and unchanging.
///

/// Returns the result of /AbpScripts/GetScripts
[OutputCache(Duration = 300)]
public virtual ActionResult GetScripts()
{
//Return they JS we got from the ABP end point via Flurl
return new JavaScriptResult
{
Script = OzCrGetScriptsHelper.GetScripts(Request.RequestContext.HttpContext, ApplicationSettings.Optomizations.BundlingAndMinification.Value)
};
}

And we have a common method to minify it after retrieving it from the original end point /abscripts/getscripts:

```c#
namespace OzCruisingPresentation.Mvc.Helpers.GetScripts
{
    /// <summary>
    ///     Class to manage the retrieval of the scripts returned from ABP\GetScripts so that we can minify it etc.
    /// </summary>
    public static class OzCrGetScriptsHelper
    {
        /// <summary>
        ///     Gets the content of ABP\GetScripts and optionally minifies it.
        /// </summary>
        /// <param name="aHttpContext">Context the request is running in.</param>
        /// <param name="aBundlingAndMinificationEnabled">Based on whether we are optomising or not.</param>
        /// <returns>Returns the javascript optionally minified.</returns>
        public static string GetScripts(HttpContextBase aHttpContext, bool aBundlingAndMinificationEnabled)
        {
            //#TODO (DE) 2018-08-21: Improve this once the authors have fixed:
            //#TODO (DE) 2018-08-21: https://github.com/aspnetboilerplate/aspnetboilerplate/issues/3673
            //#TODO (DE) 2018-08-21: https://github.com/tmenier/Flurl/issues/365   //Don't like the proposed solution. This will be sorted when we move to the developer localised domains

            //Get the contents of what ABP needs to return
            string result;
            if (OzCpSiteLocationHelper.IsDeveloperMachine() || aHttpContext.Request.Url.Host.Equals("localhost", StringComparison.OrdinalIgnoreCase))
                result = AsyncHelper.RunSync(() => $"http://{aHttpContext.Request.Url.Host}/abpscripts/getscripts".GetStringAsync());
            else
                result = AsyncHelper.RunSync(() => $"https://{aHttpContext.Request.Url.Host}/abpscripts/getscripts".GetStringAsync());

            if (aBundlingAndMinificationEnabled)
            {
                UglifyResult resultMinified = Uglify.Js(result);

                return resultMinified.HasErrors ? result : resultMinified.Code;
            }

            //Return what we have constructed
            return result;
        }
    }
}

Note: The SSL complication can be avoided ultimately by either not using SSL, not using Flurl, Using Flurl and implementing a custom certificate handler.

@hikalkan Hopefully you can add a module configuration that allows minification of any dynamically generated scripts as well.

We'll re-evaluate this in the v4.0 milestone.

ABP uses in memory cache to cache values like settings, permissions etc... So, generating the scripts on the server-side will be very fast.

For the data transferred to client, most of the sections in GetScripts response are changing while the app is running (session, localization, authorization, navigation, settings, timing).

Because of that, it is not possible to cache the output of this Action. However, if those are not changing for the target app, you can create a custom Controller and Action like @natiki did and use OutputCache attribute to cache the generated script.

@ismcagdas I would like to request that this gets an additional reconsideration from the ABP team. The issue here is not the fact that it is fast to generate on the server, it is the fact that it is transferred for every request. I would further argue that of

"(session, localization, authorization, navigation, settings, timing)"

the only thing that has a high probability of changing every minute would be session based stuff.

Once the app is localized, authorized, settings entered and time local set it will not change that often. They will change sure but definitely not every minute. So based on this I think there is still a very good case for the fact that this should be cached.

Furthermore, while it may be fast on the server side the TTFB in my case was still over half a second (~700ms) and I have a very small GetScripts output. And if that is blocking requests then it affects overall application performance.

So, any chance this (caching of GetScripts()) can be reconsidered to be part of the framework?

@natiki I think using a custom controller like you do is a better choice. You will be still using the entire framework code for generating the scripts. If we implement caching on the Framework level, I'm sure we will have many problem reports because of this. So, I think, this must be done on purpose by the developers who need it.

@ismcagdas You guys already cache it at the server level, so how is it any different if you were to cache the requests lol. Just invalidate it when someone changes a permission or localization in the UI. You already do it now for the local cache. The other option is to break out things that can be cached and what can not. I assume the app service js stuff can be.

https://stackoverflow.com/questions/1167890/how-to-programmatically-clear-outputcache-for-controller-action-method

In my case the localization data represents 90% of the script file weight, and it would be a real upgrade to have the file cached client side it there's no modification on localization data.

In our case 90% of the 7mb are the app services.

@ismcagdas Great to see that this has made it back to open status. I can concur with the above that about 95% of the data returned is unchanged once the application has started up.

@natiki yes we decided to try this again 😄

Any news on this topic ? Maybe in the 5.0 release ?

Thanks @NicolasPerego, we will reconsider this.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

worthy7 picture worthy7  ·  30Comments

alexeynikitin picture alexeynikitin  ·  27Comments

dotnetshadow picture dotnetshadow  ·  36Comments

alandillon picture alandillon  ·  28Comments

vitorlacerda picture vitorlacerda  ·  28Comments