Merge pull request #1853 from riganti/redirect-queue-block

Block postbacks for 5s after redirects

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<IHttpRedirectService>().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<IHttpRedirectService>().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);
     }
 

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()

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<any> {
     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<DotvvmRedirectEventArgs> {
@@ -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;
 }

src/Framework/Framework/Resources/Scripts/tests/eventArgs.test.ts

@@ -174,7 +174,8 @@ const fetchDefinitions = {
     postbackRedirect: async <T>(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 <T>(url: string, init: RequestInit) => {

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 = <HTMLAnchorElement> 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();
 }

src/Framework/Framework/Resources/Scripts/utils/promise.ts

@@ -1,2 +1,3 @@
 /** Runs the callback in the next event loop cycle */ 
 export const defer = <T>(callback: () => T) => Promise.resolve().then(callback)
+export const delay = (ms: number) => new Promise(resolve => setTimeout(resolve, ms))

src/Framework/Framework/ViewModel/Serialization/DefaultViewModelSerializer.cs

@@ -258,14 +258,15 @@ public JObject BuildResourcesJson(IDotvvmRequestContext context, Func<string, bo
         /// <summary>
         /// Serializes the redirect action.
         /// </summary>
-        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();
             result["url"] = url;
             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);
         }
 

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);
+        }
+    }
+}

src/Samples/Common/Views/FeatureSamples/Redirect/RedirectPostbackConcurrency.dothtml

@@ -0,0 +1,33 @@
+@viewModel DotVVM.Samples.BasicSamples.ViewModels.FeatureSamples.Redirect.RedirectPostbackConcurrencyViewModel, DotVVM.Samples.Common
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+    <title>Hello from DotVVM!</title>
+</head>
+<body>
+    <div IncludeInPage={resource: IsEmptyPage}>
+        <dot:RouteLink RouteName={resource: Context.Route.RouteName}  Query-empty=false>Back</dot:RouteLink>
+    </div>
+    <div class="container" IncludeInPage={resource: !IsEmptyPage}>
+        <h1>Redirect and postback concurrency test</h1>
+        <p>
+            Testing Concurrency=Deny / Concurrency=Queue with redirect and file returns.
+        </p>
+        <p>First, we have a set of buttons incrementing a static variable, each takes about 2sec and redirects to a blank page afterwards</p>
+        <p>GlobalCounter = <span data-ui="counter" InnerText={value: Counter} /></p>
+        <p>MiniCounter(Concurrency=Deny) = <dot:Button data-ui=minicounter Text={value: MiniCounter} PostBack.Concurrency=Deny Click={staticCommand: MiniCounter = MiniCounter + 1} /></p>
+
+        <p>
+            <dot:Button data-ui="inc-default" Click={command: DelayIncrement()} PostBack.Concurrency=Default>Increment (Concurrency=Default)</dot:Button>
+            <dot:Button data-ui="inc-deny" Click={command: DelayIncrement()} PostBack.Concurrency=Deny>Increment (Concurrency=Deny)</dot:Button>
+            <dot:Button data-ui="inc-queue" Click={command: DelayIncrement()} PostBack.Concurrency=Queue>Increment (Concurrency=Queue)</dot:Button>
+        </p>
+
+        <p>We also test that returning files does not block the page forever, </p>
+
+        <p>
+            <dot:Button data-ui="file-std" Click={command: GetFileStandard()}>Standard return file</dot:Button>
+            <dot:Button data-ui="file-custom" Click={command: GetFileCustom()}>Custom return file (will have delay before page works)</dot:Button>
+        </p>
+    </div>
+</body>
+</html>

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);
+            });
+        }
     }
 }