Cache-Control header should not be Private when serving packages

[matthewskelton]

Currently, NuGetGallery leaves the Cache-Control header as Private for the "GetPackageApi" action. This means that GET requests to pull packages cannot be cached by downstream web caches (such as Squid/Varnish).

In order to allow web caches to cache GET requests for reading packages, we need a change in ApiController.cs - explicitly set the cacheability like this:

        this.Response.Cache.SetCacheability(HttpCacheability.Public);
        this.Response.Cache.SetExpires(DateTime.Now.AddDays(days));

Being able to cache GET read requests means that we can offload the load on the NuGet Gallery server. This is particularly important when running NuGet Gallery locally, with 80+ developers and upwards of 60 build pipelines.

I am happy to submit a pull request. We have this configuration working in our environment, and it works a treat with Squid - blazing fast performance from NuGet Gallery!

[half-ogre]

Sounds promising @matthewskelton. If you're happy to submit a pull request, I welcome that, and will review it. Thanks!

[matthewskelton]

Cool, I will do that. I have logged a related issue #499 too, and will get a fix ready there too, as it's related.

[pranavkm]

@matthewskelton I think sticking an OutputCache attribute on the action should suffice.

[matthewskelton]

@pranavkm Thanks for the heads-up - I'll be looking at pushing some changes tomorrow.

[matthewskelton]

@pranavkm @drewthedork I think that the semantics of NuGet package versioning (and package versioning in general) means that we should enable downstream servers/proxies to cache the package indefinitely.

You would never publish an "updated" version of SomePackage.1.3.8, because you would publish SomePackage 1.3.9 instead.

Therefore, I think that Int32.MaxValue is an appropriate cache duration time:

    [ActionName("GetPackageApi"), HttpGet]
    [OutputCache(Duration = Int32.MaxValue, Location = OutputCacheLocation.Downstream)]
    public virtual ActionResult GetPackage(string id, string version)
    ...

In terms of OutputCacheLocation, I think we should avoid telling clients that they can cache it, as we cannot predict how NuGet client needs will evolve over time, hence Downstream for this.

Tell me if you think if I'm off the mark here, otherwise, I'll push this ASAP.

[howarddierking]

closing until pull request #505 is re-opened