diff --git a/src/Framework/Framework/Hosting/DotvvmRequestContextExtensions.cs b/src/Framework/Framework/Hosting/DotvvmRequestContextExtensions.cs index 913347ffb7..8bc46a5b7a 100644 --- a/src/Framework/Framework/Hosting/DotvvmRequestContextExtensions.cs +++ b/src/Framework/Framework/Hosting/DotvvmRequestContextExtensions.cs @@ -141,8 +141,8 @@ public static void RedirectToRoutePermanent(this IDotvvmRequestContext context, context.RedirectToUrlPermanent(url, replaceInHistory, allowSpaRedirect); } - public static void SetRedirectResponse(this IDotvvmRequestContext context, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false) => - context.Configuration.ServiceProvider.GetRequiredService().WriteRedirectResponse(context.HttpContext, url, statusCode, replaceInHistory, allowSpaRedirect); + public static void SetRedirectResponse(this IDotvvmRequestContext context, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false, string? downloadName = null) => + context.Configuration.ServiceProvider.GetRequiredService().WriteRedirectResponse(context.HttpContext, url, statusCode, replaceInHistory, allowSpaRedirect, downloadName); internal static Task SetCachedViewModelMissingResponse(this IDotvvmRequestContext context) { @@ -262,8 +262,10 @@ public static async Task ReturnFileAsync(this IDotvvmRequestContext context, Str AttachmentDispositionType = attachmentDispositionType ?? "attachment" }; + var downloadAttribute = attachmentDispositionType == "inline" ? null : fileName; + var generatedFileId = await returnedFileStorage.StoreFileAsync(stream, metadata).ConfigureAwait(false); - context.SetRedirectResponse(context.TranslateVirtualPath("~/dotvvmReturnedFile?id=" + generatedFileId)); + context.SetRedirectResponse(context.TranslateVirtualPath("~/dotvvmReturnedFile?id=" + generatedFileId), downloadName: downloadAttribute); throw new DotvvmInterruptRequestExecutionException(InterruptReason.ReturnFile, fileName); } diff --git a/src/Framework/Framework/Hosting/HttpRedirectService.cs b/src/Framework/Framework/Hosting/HttpRedirectService.cs index d1aef7f12f..c29281b6c7 100644 --- a/src/Framework/Framework/Hosting/HttpRedirectService.cs +++ b/src/Framework/Framework/Hosting/HttpRedirectService.cs @@ -25,14 +25,13 @@ namespace DotVVM.Framework.Hosting { public interface IHttpRedirectService { - void WriteRedirectResponse(IHttpContext httpContext, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false); + void WriteRedirectResponse(IHttpContext httpContext, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false, string? downloadName = null); } public class DefaultHttpRedirectService: IHttpRedirectService { - public void WriteRedirectResponse(IHttpContext httpContext, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false) + public void WriteRedirectResponse(IHttpContext httpContext, string url, int statusCode = (int)HttpStatusCode.Redirect, bool replaceInHistory = false, bool allowSpaRedirect = false, string? downloadName = null) { - if (DotvvmRequestContext.DetermineRequestType(httpContext) is DotvvmRequestType.Navigate) { httpContext.Response.Headers["Location"] = url; @@ -43,7 +42,7 @@ public void WriteRedirectResponse(IHttpContext httpContext, string url, int stat httpContext.Response.StatusCode = 200; httpContext.Response.ContentType = "application/json"; httpContext.Response - .WriteAsync(DefaultViewModelSerializer.GenerateRedirectActionResponse(url, replaceInHistory, allowSpaRedirect)) + .WriteAsync(DefaultViewModelSerializer.GenerateRedirectActionResponse(url, replaceInHistory, allowSpaRedirect, downloadName)) .GetAwaiter().GetResult(); // ^ we just wait for this Task. Redirect API never was async and the response size is small enough that we can't quite safely wait for the result // .GetAwaiter().GetResult() preserves stack traces across async calls, thus I like it more that .Wait() diff --git a/src/Framework/Framework/Resources/Scripts/postback/redirect.ts b/src/Framework/Framework/Resources/Scripts/postback/redirect.ts index bad3b21184..fb28132976 100644 --- a/src/Framework/Framework/Resources/Scripts/postback/redirect.ts +++ b/src/Framework/Framework/Resources/Scripts/postback/redirect.ts @@ -1,17 +1,23 @@ import * as events from '../events'; import * as magicNavigator from '../utils/magic-navigator' import { handleSpaNavigationCore } from "../spa/spa"; +import { delay } from '../utils/promise'; + export function performRedirect(url: string, replace: boolean, allowSpa: boolean): Promise { if (replace) { location.replace(url); - return Promise.resolve(); } else if (compileConstants.isSpa && allowSpa) { return handleSpaNavigationCore(url); } else { magicNavigator.navigate(url); - return Promise.resolve(); } + + // When performing redirect, we pretend that the request takes additional X second to avoid + // double submit with Postback.Concurrency=Deny or Queue. + // We do not want to block the page forever, as the redirect might just return a file (or HTTP 204/205), + // and the page will continue to live. + return delay(5_000); } export async function handleRedirect(options: PostbackOptions, resultObject: any, response: Response, replace: boolean = false): Promise { @@ -28,7 +34,12 @@ export async function handleRedirect(options: PostbackOptions, resultObject: any } events.redirect.trigger(redirectArgs); - await performRedirect(url, replace, resultObject.allowSpa); + const downloadFileName = resultObject.download + if (downloadFileName != null) { + magicNavigator.navigate(url, downloadFileName) + } else { + await performRedirect(url, replace, resultObject.allowSpa); + } return redirectArgs; } diff --git a/src/Framework/Framework/Resources/Scripts/tests/eventArgs.test.ts b/src/Framework/Framework/Resources/Scripts/tests/eventArgs.test.ts index 097fe5c478..d6c29aa268 100644 --- a/src/Framework/Framework/Resources/Scripts/tests/eventArgs.test.ts +++ b/src/Framework/Framework/Resources/Scripts/tests/eventArgs.test.ts @@ -174,7 +174,8 @@ const fetchDefinitions = { postbackRedirect: async (url: string, init: RequestInit) => { return { action: "redirect", - url: "/newUrl" + url: "/newUrl", + download: "some-file" // say it's a file, so that DotVVM does not block postback queue after the test } as any; }, @@ -213,7 +214,8 @@ const fetchDefinitions = { return { action: "redirect", url: "/newUrl", - allowSpa: true + allowSpa: true, + download: "some-file" } as any; }, spaNavigateError: async (url: string, init: RequestInit) => { diff --git a/src/Framework/Framework/Resources/Scripts/utils/magic-navigator.ts b/src/Framework/Framework/Resources/Scripts/utils/magic-navigator.ts index 60e3486ab1..b97ca9dc46 100644 --- a/src/Framework/Framework/Resources/Scripts/utils/magic-navigator.ts +++ b/src/Framework/Framework/Resources/Scripts/utils/magic-navigator.ts @@ -1,10 +1,15 @@ let fakeAnchor: HTMLAnchorElement | undefined; -export function navigate(url: string) { +export function navigate(url: string, downloadName: string | null | undefined = null) { if (!fakeAnchor) { fakeAnchor = document.createElement("a"); fakeAnchor.style.display = "none"; document.body.appendChild(fakeAnchor); } + if (downloadName == null) { + fakeAnchor.removeAttribute("download"); + } else { + fakeAnchor.download = downloadName + } fakeAnchor.href = url; fakeAnchor.click(); } diff --git a/src/Framework/Framework/Resources/Scripts/utils/promise.ts b/src/Framework/Framework/Resources/Scripts/utils/promise.ts index a860c54669..c702c2f86e 100644 --- a/src/Framework/Framework/Resources/Scripts/utils/promise.ts +++ b/src/Framework/Framework/Resources/Scripts/utils/promise.ts @@ -1,2 +1,3 @@ /** Runs the callback in the next event loop cycle */ export const defer = (callback: () => T) => Promise.resolve().then(callback) +export const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)) diff --git a/src/Framework/Framework/ViewModel/Serialization/DefaultViewModelSerializer.cs b/src/Framework/Framework/ViewModel/Serialization/DefaultViewModelSerializer.cs index eb35dc99f4..db51e3f283 100644 --- a/src/Framework/Framework/ViewModel/Serialization/DefaultViewModelSerializer.cs +++ b/src/Framework/Framework/ViewModel/Serialization/DefaultViewModelSerializer.cs @@ -258,7 +258,7 @@ public JObject BuildResourcesJson(IDotvvmRequestContext context, Func /// Serializes the redirect action. /// - public static string GenerateRedirectActionResponse(string url, bool replace, bool allowSpa) + public static string GenerateRedirectActionResponse(string url, bool replace, bool allowSpa, string? downloadName) { // create result object var result = new JObject(); @@ -266,6 +266,7 @@ public static string GenerateRedirectActionResponse(string url, bool replace, bo result["action"] = "redirect"; if (replace) result["replace"] = true; if (allowSpa) result["allowSpa"] = true; + if (downloadName is object) result["download"] = downloadName; return result.ToString(Formatting.None); } diff --git a/src/Samples/Common/ViewModels/FeatureSamples/Redirect/RedirectPostbackConcurrencyViewModel.cs b/src/Samples/Common/ViewModels/FeatureSamples/Redirect/RedirectPostbackConcurrencyViewModel.cs new file mode 100644 index 0000000000..75f53ba0b1 --- /dev/null +++ b/src/Samples/Common/ViewModels/FeatureSamples/Redirect/RedirectPostbackConcurrencyViewModel.cs @@ -0,0 +1,65 @@ +using System.Diagnostics.Metrics; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using DotVVM.Core.Storage; +using DotVVM.Framework.ViewModel; + +namespace DotVVM.Samples.BasicSamples.ViewModels.FeatureSamples.Redirect +{ + class RedirectPostbackConcurrencyViewModel : DotvvmViewModelBase + { + public static int GlobalCounter = 0; + private readonly IReturnedFileStorage returnedFileStorage; + + [Bind(Direction.ServerToClient)] + public int Counter { get; set; } = GlobalCounter; + + public int MiniCounter { get; set; } = 0; + + [FromQuery("empty")] + public bool IsEmptyPage { get; set; } = false; + [FromQuery("loadDelay")] + public int LoadDelay { get; set; } = 0; + + public RedirectPostbackConcurrencyViewModel(IReturnedFileStorage returnedFileStorage) + { + this.returnedFileStorage = returnedFileStorage; + } + public override async Task Init() + { + await Task.Delay(LoadDelay); // delay to enable user to click DelayIncrement button between it succeeding and loading the next page + await base.Init(); + } + + public async Task DelayIncrement() + { + await Task.Delay(1000); + + Interlocked.Increment(ref GlobalCounter); + + Context.RedirectToRoute(Context.Route.RouteName, query: new { empty = true, loadDelay = 2000 }); + } + + public async Task GetFileStandard() + { + await Context.ReturnFileAsync("test file"u8.ToArray(), "test.txt", "text/plain"); + } + + public async Task GetFileCustom() + { + var metadata = new ReturnedFileMetadata() + { + FileName = "test_custom.txt", + MimeType = "text/plain", + AttachmentDispositionType = "attachment" + }; + + var stream = new MemoryStream("test custom file"u8.ToArray()); + var generatedFileId = await returnedFileStorage.StoreFileAsync(stream, metadata).ConfigureAwait(false); + + var url = Context.TranslateVirtualPath("~/dotvvmReturnedFile?id=" + generatedFileId); + Context.RedirectToUrl(url); + } + } +} diff --git a/src/Samples/Common/Views/FeatureSamples/Redirect/RedirectPostbackConcurrency.dothtml b/src/Samples/Common/Views/FeatureSamples/Redirect/RedirectPostbackConcurrency.dothtml new file mode 100644 index 0000000000..f9838fcdc6 --- /dev/null +++ b/src/Samples/Common/Views/FeatureSamples/Redirect/RedirectPostbackConcurrency.dothtml @@ -0,0 +1,33 @@ +@viewModel DotVVM.Samples.BasicSamples.ViewModels.FeatureSamples.Redirect.RedirectPostbackConcurrencyViewModel, DotVVM.Samples.Common + + + Hello from DotVVM! + + +
+ Back +
+
+

Redirect and postback concurrency test

+

+ Testing Concurrency=Deny / Concurrency=Queue with redirect and file returns. +

+

First, we have a set of buttons incrementing a static variable, each takes about 2sec and redirects to a blank page afterwards

+

GlobalCounter =

+

MiniCounter(Concurrency=Deny) =

+ +

+ Increment (Concurrency=Default) + Increment (Concurrency=Deny) + Increment (Concurrency=Queue) +

+ +

We also test that returning files does not block the page forever,

+ +

+ Standard return file + Custom return file (will have delay before page works) +

+
+ + diff --git a/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs b/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs index ff8f0ccc1b..83323d2379 100644 --- a/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs +++ b/src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs @@ -327,6 +327,7 @@ public partial class SamplesRouteUrls public const string FeatureSamples_Redirect_RedirectionHelpers_PageC = "FeatureSamples/Redirect/RedirectionHelpers_PageC"; public const string FeatureSamples_Redirect_RedirectionHelpers_PageD = "FeatureSamples/Redirect/RedirectionHelpers_PageD"; public const string FeatureSamples_Redirect_RedirectionHelpers_PageE = "FeatureSamples/Redirect/RedirectionHelpers_PageE"; + public const string FeatureSamples_Redirect_RedirectPostbackConcurrency = "FeatureSamples/Redirect/RedirectPostbackConcurrency"; public const string FeatureSamples_Redirect_Redirect_StaticCommand = "FeatureSamples/Redirect/Redirect_StaticCommand"; public const string FeatureSamples_RenderSettingsModeServer_RenderSettingModeServerProperty = "FeatureSamples/RenderSettingsModeServer/RenderSettingModeServerProperty"; public const string FeatureSamples_RenderSettingsModeServer_RepeaterCollectionExchange = "FeatureSamples/RenderSettingsModeServer/RepeaterCollectionExchange"; diff --git a/src/Samples/Tests/Tests/Feature/RedirectTests.cs b/src/Samples/Tests/Tests/Feature/RedirectTests.cs index 70af6f5cc2..ef9b12b62e 100644 --- a/src/Samples/Tests/Tests/Feature/RedirectTests.cs +++ b/src/Samples/Tests/Tests/Feature/RedirectTests.cs @@ -1,7 +1,11 @@ using System; using System.Linq; +using System.Threading; using DotVVM.Samples.Tests.Base; using DotVVM.Testing.Abstractions; +using OpenQA.Selenium; +using Riganti.Selenium.Core; +using Riganti.Selenium.Core.Abstractions; using Riganti.Selenium.DotVVM; using Xunit; using Xunit.Abstractions; @@ -69,6 +73,94 @@ public void Feature_Redirect_RedirectionHelpers() Assert.Matches($"{SamplesRouteUrls.FeatureSamples_Redirect_RedirectionHelpers_PageE}/1221\\?x=a", currentUrl.LocalPath + currentUrl.Query); }); } - + + bool TryClick(IElementWrapper element) + { + if (element is null) return false; + try + { + element.Click(); + return true; + } + catch (StaleElementReferenceException) + { + return false; + } + } + + [Fact] + public void Feature_Redirect_RedirectPostbackConcurrency() + { + RunInAllBrowsers(browser => { + browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_Redirect_RedirectPostbackConcurrency); + + int globalCounter() => int.Parse(browser.First("counter", SelectByDataUi).GetText()); + + var initialCounter = globalCounter(); + for (int i = 0; i < 20; i++) + { + TryClick(browser.FirstOrDefault("inc-default", SelectByDataUi)); + Thread.Sleep(1); + } + browser.WaitFor(() => Assert.Contains("empty=true", browser.CurrentUrl, StringComparison.OrdinalIgnoreCase), 7000, "Redirect did not happen"); + browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_Redirect_RedirectPostbackConcurrency); + + // must increment at least 20 times, otherwise delays are too short + Assert.Equal(globalCounter(), initialCounter + 20); + + initialCounter = globalCounter(); + var clickCount = 0; + while (TryClick(browser.FirstOrDefault("inc-deny", SelectByDataUi))) + { + clickCount++; + Thread.Sleep(1); + } + Assert.InRange(clickCount, 3, int.MaxValue); + browser.WaitFor(() => Assert.Contains("empty=true", browser.CurrentUrl, StringComparison.OrdinalIgnoreCase), timeout: 500, "Redirect did not happen"); + + browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_Redirect_RedirectPostbackConcurrency); + Assert.Equal(globalCounter(), initialCounter + 1); // only one click was allowed + + initialCounter = globalCounter(); + clickCount = 0; + while (TryClick(browser.FirstOrDefault("inc-queue", SelectByDataUi))) + { + clickCount++; + Thread.Sleep(1); + } + + Assert.InRange(clickCount, 3, int.MaxValue); + browser.WaitFor(() => Assert.Contains("empty=true", browser.CurrentUrl, StringComparison.OrdinalIgnoreCase), timeout: 500, "Redirect did not happen"); + + browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_Redirect_RedirectPostbackConcurrency); + Assert.Equal(globalCounter(), initialCounter + 1); // only one click was allowed + }); + } + + [Fact] + public void Feature_Redirect_RedirectPostbackConcurrencyFileReturn() + { + RunInAllBrowsers(browser => { + browser.NavigateToUrl(SamplesRouteUrls.FeatureSamples_Redirect_RedirectPostbackConcurrency); + + void increment(int timeout) + { + browser.WaitFor(() => { + var original = int.Parse(browser.First("minicounter", SelectByDataUi).GetText()); + browser.First("minicounter", SelectByDataUi).Click(); + AssertUI.TextEquals(browser.First("minicounter", SelectByDataUi), (original + 1).ToString()); + }, timeout, "Could not increment minicounter in given timeout (postback queue is blocked)"); + } + + increment(3000); + + browser.First("file-std", SelectByDataUi).Click(); + increment(3000); + + browser.First("file-custom", SelectByDataUi).Click(); + // longer timeout, because DotVVM blocks postback queue for 5s after redirects to debounce any further requests + increment(15000); + }); + } } }