From 7845bb026d6e03f21ba8ad80f4ba430e6207c659 Mon Sep 17 00:00:00 2001 From: Ross Oliver Date: Thu, 3 Dec 2020 13:54:55 +0000 Subject: [PATCH] Allow GET requests to be cached privately for 5m At the moment we monkey-patch the Ruby API client library to cache GET request responses for 5 minutes. It was done this way originally as ASP.NET Core was preventing us from setting `Cache-Control` headers on authenticated requests. This no longer seems to be the case and the core team appear to be actively working towards a less opinionated caching implementation. Add a custom attribute for short term (5m), private response caching that is safe to use on most GET responses. Applies it to: - All /types endpoints - All /privacy_policies endpoints - GET /teaching_events endpoints (search and upcoming) --- .../PrivateShortTermResponseCacheAttribute.cs | 13 +++++++++++++ .../Controllers/PrivacyPoliciesController.cs | 3 ++- .../Controllers/TeachingEventsController.cs | 11 +++++++---- .../Controllers/TypesController.cs | 3 ++- ...ateShortTermResponseCacheAttributeTests.cs | 19 +++++++++++++++++++ .../PrivacyPoliciesControllerTests.cs | 6 ++++++ .../TeachingEventsControllerTests.cs | 9 +++++++++ .../Controllers/TypesControllerTests.cs | 6 ++++++ 8 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 GetIntoTeachingApi/Attributes/PrivateShortTermResponseCacheAttribute.cs create mode 100644 GetIntoTeachingApiTests/Attributes/PrivateShortTermResponseCacheAttributeTests.cs diff --git a/GetIntoTeachingApi/Attributes/PrivateShortTermResponseCacheAttribute.cs b/GetIntoTeachingApi/Attributes/PrivateShortTermResponseCacheAttribute.cs new file mode 100644 index 000000000..3d33dbe0b --- /dev/null +++ b/GetIntoTeachingApi/Attributes/PrivateShortTermResponseCacheAttribute.cs @@ -0,0 +1,13 @@ +using Microsoft.AspNetCore.Mvc; + +namespace GetIntoTeachingApi.Attributes +{ + public class PrivateShortTermResponseCacheAttribute : ResponseCacheAttribute + { + public PrivateShortTermResponseCacheAttribute() + { + Location = ResponseCacheLocation.Client; + Duration = 300; + } + } +} \ No newline at end of file diff --git a/GetIntoTeachingApi/Controllers/PrivacyPoliciesController.cs b/GetIntoTeachingApi/Controllers/PrivacyPoliciesController.cs index a3a6dbea6..0a0784101 100644 --- a/GetIntoTeachingApi/Controllers/PrivacyPoliciesController.cs +++ b/GetIntoTeachingApi/Controllers/PrivacyPoliciesController.cs @@ -11,7 +11,8 @@ namespace GetIntoTeachingApi.Controllers { [Route("api/privacy_policies")] [ApiController] - [LogRequests] + [LogRequests] + [PrivateShortTermResponseCache] [Authorize] public class PrivacyPoliciesController : ControllerBase { diff --git a/GetIntoTeachingApi/Controllers/TeachingEventsController.cs b/GetIntoTeachingApi/Controllers/TeachingEventsController.cs index 2657ac6f7..6a3d6227b 100644 --- a/GetIntoTeachingApi/Controllers/TeachingEventsController.cs +++ b/GetIntoTeachingApi/Controllers/TeachingEventsController.cs @@ -15,7 +15,7 @@ namespace GetIntoTeachingApi.Controllers { [Route("api/teaching_events")] [ApiController] - [LogRequests] + [LogRequests] [Authorize] public class TeachingEventsController : ControllerBase { @@ -40,7 +40,8 @@ public TeachingEventsController( } [HttpGet] - [CrmETag] + [CrmETag] + [PrivateShortTermResponseCache] [Route("search_indexed_by_type")] [SwaggerOperation( Summary = "Searches for teaching events, returning grouped by type.", @@ -68,7 +69,8 @@ public async Task SearchIndexedByType( } [HttpGet] - [CrmETag] + [CrmETag] + [PrivateShortTermResponseCache] [Route("upcoming_indexed_by_type")] [SwaggerOperation( Summary = "Retrieves upcoming teaching events grouped by type.", @@ -83,7 +85,8 @@ public async Task SearchIndexedByType( } [HttpGet] - [CrmETag] + [CrmETag] + [PrivateShortTermResponseCache] [Route("{readableId}")] [SwaggerOperation( Summary = "Retrieves an event.", diff --git a/GetIntoTeachingApi/Controllers/TypesController.cs b/GetIntoTeachingApi/Controllers/TypesController.cs index 9915dc7d5..211857761 100644 --- a/GetIntoTeachingApi/Controllers/TypesController.cs +++ b/GetIntoTeachingApi/Controllers/TypesController.cs @@ -13,7 +13,8 @@ namespace GetIntoTeachingApi.Controllers { [Route("api/types")] [ApiController] - [LogRequests] + [LogRequests] + [PrivateShortTermResponseCache] [Authorize] public class TypesController : ControllerBase { diff --git a/GetIntoTeachingApiTests/Attributes/PrivateShortTermResponseCacheAttributeTests.cs b/GetIntoTeachingApiTests/Attributes/PrivateShortTermResponseCacheAttributeTests.cs new file mode 100644 index 000000000..83f24c4bd --- /dev/null +++ b/GetIntoTeachingApiTests/Attributes/PrivateShortTermResponseCacheAttributeTests.cs @@ -0,0 +1,19 @@ +using FluentAssertions; +using GetIntoTeachingApi.Attributes; +using Microsoft.AspNetCore.Mvc; +using Xunit; + +namespace GetIntoTeachingApiTests.Attributes +{ + public class PrivateShortTermResponseCacheAttributeTests + { + [Fact] + public void Constructor_CorrectlySetsCacheControl() + { + var cache = new PrivateShortTermResponseCacheAttribute(); + + cache.Location.Should().Be(ResponseCacheLocation.Client); + cache.Duration.Should().Be(300); + } + } +} diff --git a/GetIntoTeachingApiTests/Controllers/PrivacyPoliciesControllerTests.cs b/GetIntoTeachingApiTests/Controllers/PrivacyPoliciesControllerTests.cs index feea94b90..593d8c576 100644 --- a/GetIntoTeachingApiTests/Controllers/PrivacyPoliciesControllerTests.cs +++ b/GetIntoTeachingApiTests/Controllers/PrivacyPoliciesControllerTests.cs @@ -33,6 +33,12 @@ public void Authorize_IsPresent() public void LogRequests_IsPresent() { typeof(PrivacyPoliciesController).Should().BeDecoratedWith(); + } + + [Fact] + public void PrivateShortTermResponseCache_IsPresent() + { + typeof(PrivacyPoliciesController).Should().BeDecoratedWith(); } [Fact] diff --git a/GetIntoTeachingApiTests/Controllers/TeachingEventsControllerTests.cs b/GetIntoTeachingApiTests/Controllers/TeachingEventsControllerTests.cs index 11c9b2201..1db1dfca5 100644 --- a/GetIntoTeachingApiTests/Controllers/TeachingEventsControllerTests.cs +++ b/GetIntoTeachingApiTests/Controllers/TeachingEventsControllerTests.cs @@ -63,6 +63,15 @@ public void CrmETag_IsPresent() methods.ForEach(m => typeof(TeachingEventsController).GetMethod(m).Should().BeDecoratedWith()); } + [Fact] + public void CrmETagPrivateShortTermResponseCache_IsPresent() + { + JobStorage.Current = new Mock().Object; + var methods = new[] { "Get", "SearchIndexedByType", "UpcomingIndexedByType" }; + + methods.ForEach(m => typeof(TeachingEventsController).GetMethod(m).Should().BeDecoratedWith()); + } + [Fact] public void AddAttendee_InvalidRequest_RespondsWithValidationErrors() { diff --git a/GetIntoTeachingApiTests/Controllers/TypesControllerTests.cs b/GetIntoTeachingApiTests/Controllers/TypesControllerTests.cs index 1c2dc5e61..df024dd45 100644 --- a/GetIntoTeachingApiTests/Controllers/TypesControllerTests.cs +++ b/GetIntoTeachingApiTests/Controllers/TypesControllerTests.cs @@ -38,6 +38,12 @@ public void Authorize_IsPresent() public void LogRequests_IsPresent() { typeof(TypesController).Should().BeDecoratedWith(); + } + + [Fact] + public void PrivateShortTermResponseCache_IsPresent() + { + typeof(TypesController).Should().BeDecoratedWith(); } [Fact]