From 49df0fc78696bf2b83afd2bf01aa51dd8bf4b4ca Mon Sep 17 00:00:00 2001 From: support Date: Wed, 14 Oct 2026 20:11:45 +0200 Subject: [PATCH 01/15] Replace Wangkanai.Detection with a user agent classifier --- Directory.Packages.props | 1 - .../RequestCustomerResolversTests.cs | 21 ++--- .../Security/Oidc/OidcWorkContextTests.cs | 4 +- .../TagHelpers/AdminTabStripTagHelperTests.cs | 16 ++-- .../UserAgent/UserAgentClassifierTests.cs | 84 +++++++++++++++++++ .../WorkContextSetterRequestResolverTests.cs | 16 ++-- .../Grand.Web.Common/Grand.Web.Common.csproj | 1 - .../ServiceCollectionExtensions.cs | 9 -- .../SearchEngineCustomerResolver.cs | 7 +- .../Startup/GrandCommonStartup.cs | 8 +- .../Admin/AdminTabStripTagHelper.cs | 11 ++- .../UserAgent/IUserAgentClassifier.cs | 17 ++++ .../UserAgent/UserAgentClassifier.cs | 42 ++++++++++ 13 files changed, 176 insertions(+), 61 deletions(-) create mode 100644 src/Tests/Grand.Web.Common.Tests/UserAgent/UserAgentClassifierTests.cs create mode 100644 src/Web/Grand.Web.Common/UserAgent/IUserAgentClassifier.cs create mode 100644 src/Web/Grand.Web.Common/UserAgent/UserAgentClassifier.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index ee3d99be6a..2f21372435 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -57,7 +57,6 @@ - diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs index 3c040570c5..c4aa4330d9 100644 --- a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs @@ -3,11 +3,11 @@ using Grand.Business.Core.Interfaces.Customers; using Grand.Domain.Customers; using Grand.Web.Common.RequestCustomerResolvers; +using Grand.Web.Common.UserAgent; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using Wangkanai.Detection.Services; namespace Grand.Web.Common.Tests; @@ -132,31 +132,24 @@ public async Task GuestCookie_RegisteredDeletedOrInactive_ReturnsNull() [TestMethod] public async Task SearchEngine_NotACrawler_ReturnsNull() { - var detection = new Mock(); - var resolver = new SearchEngineCustomerResolver(detection.Object, Mock.Of()); + var classifier = new Mock(); + classifier.SetupGet(c => c.IsCrawler).Returns(false); + var resolver = new SearchEngineCustomerResolver(classifier.Object, Mock.Of()); - //Crawler is null: the null-safe read must not throw Assert.IsNull(await resolver.Resolve()); Assert.AreEqual(RequestCustomerResolverOrder.SearchEngine, resolver.Order); - - var crawler = new Mock(); - crawler.SetupGet(c => c.IsCrawler).Returns(false); - detection.SetupGet(d => d.Crawler).Returns(crawler.Object); - Assert.IsNull(await resolver.Resolve()); } [TestMethod] public async Task SearchEngine_Crawler_ReturnsSearchEngineCustomer() { - var crawler = new Mock(); - crawler.SetupGet(c => c.IsCrawler).Returns(true); - var detection = new Mock(); - detection.SetupGet(d => d.Crawler).Returns(crawler.Object); + var classifier = new Mock(); + classifier.SetupGet(c => c.IsCrawler).Returns(true); var customers = new Mock(); customers.Setup(c => c.GetCustomerBySystemName(SystemCustomerNames.SearchEngine)) .ReturnsAsync(new Customer { Id = "crawler" }); - var resolver = new SearchEngineCustomerResolver(detection.Object, customers.Object); + var resolver = new SearchEngineCustomerResolver(classifier.Object, customers.Object); Assert.AreEqual("crawler", (await resolver.Resolve()).Id); } diff --git a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs index d1cee850de..505ee6a7e2 100644 --- a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs @@ -11,6 +11,7 @@ using Grand.Infrastructure.Configuration; using Grand.Web.Common.RequestCustomerResolvers; using Grand.Web.Common.Security.Oidc; +using Grand.Web.Common.UserAgent; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; @@ -20,7 +21,6 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using System.Security.Claims; -using Wangkanai.Detection.Services; namespace Grand.Web.Common.Tests.Security.Oidc; @@ -91,7 +91,7 @@ private TestableSetter CreateSetter() services.AddSingleton( new GuestCookieCustomerResolver(_authentication.Object, _customerService.Object, Mock.Of())); services.AddSingleton( - new SearchEngineCustomerResolver(Mock.Of(), _customerService.Object)); + new SearchEngineCustomerResolver(Mock.Of(), _customerService.Object)); services.AddSingleton( new AllowAnonymousCustomerResolver(_accessor.Object, _customerService.Object)); return new TestableSetter(_accessor.Object, _authentication.Object, Mock.Of(), diff --git a/src/Tests/Grand.Web.Common.Tests/TagHelpers/AdminTabStripTagHelperTests.cs b/src/Tests/Grand.Web.Common.Tests/TagHelpers/AdminTabStripTagHelperTests.cs index 465b9170c1..69fb25a1ed 100644 --- a/src/Tests/Grand.Web.Common.Tests/TagHelpers/AdminTabStripTagHelperTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/TagHelpers/AdminTabStripTagHelperTests.cs @@ -12,8 +12,7 @@ using Microsoft.AspNetCore.Routing; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using Wangkanai.Detection.Models; -using Wangkanai.Detection.Services; +using Grand.Web.Common.UserAgent; namespace Grand.Web.Common.Tests.TagHelpers; @@ -21,8 +20,7 @@ namespace Grand.Web.Common.Tests.TagHelpers; public class AdminTabStripTagHelperTests { private Mock _mediatorMock; - private Mock _detectionServiceMock; - private Mock _deviceServiceMock; + private Mock _userAgentMock; private ViewContext _viewContext; private Action _onPublish; @@ -37,10 +35,8 @@ public void Init() _onPublish?.Invoke(message); return Task.CompletedTask; }); - _deviceServiceMock = new Mock(); - _deviceServiceMock.Setup(x => x.Type).Returns(Device.Desktop); - _detectionServiceMock = new Mock(); - _detectionServiceMock.Setup(x => x.Device).Returns(_deviceServiceMock.Object); + _userAgentMock = new Mock(); + _userAgentMock.SetupGet(x => x.IsMobileOrTablet).Returns(false); _viewContext = new ViewContext { RouteData = new RouteData(), ViewData = new ViewDataDictionary(new EmptyModelMetadataProvider(), new ModelStateDictionary()), @@ -51,7 +47,7 @@ public void Init() private AdminTabStripTagHelper CreateStrip(string name = "product-edit", bool bindGrid = true, bool setTabPos = false) { - return new AdminTabStripTagHelper(_mediatorMock.Object, _detectionServiceMock.Object) { + return new AdminTabStripTagHelper(_mediatorMock.Object, _userAgentMock.Object) { Name = name, BindGrid = bindGrid, SetTabPos = setTabPos, @@ -166,7 +162,7 @@ public async Task Process_SetTabPos_AddsTheLeftLayoutClass() [TestMethod] public async Task Process_OnAMobileDevice_IgnoresSetTabPos() { - _deviceServiceMock.Setup(x => x.Type).Returns(Device.Mobile); + _userAgentMock.SetupGet(x => x.IsMobileOrTablet).Returns(true); var output = await Run(CreateStrip(setTabPos: true), "
a
"); Assert.AreEqual("grand-tabstrip", output.Attributes["class"].Value); diff --git a/src/Tests/Grand.Web.Common.Tests/UserAgent/UserAgentClassifierTests.cs b/src/Tests/Grand.Web.Common.Tests/UserAgent/UserAgentClassifierTests.cs new file mode 100644 index 0000000000..beab2d5aca --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/UserAgent/UserAgentClassifierTests.cs @@ -0,0 +1,84 @@ +using Grand.Web.Common.UserAgent; +using Microsoft.AspNetCore.Http; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Web.Common.Tests.UserAgent; + +[TestClass] +public class UserAgentClassifierTests +{ + private static UserAgentClassifier ClassifierFor(string userAgent) + { + var context = new DefaultHttpContext(); + if (userAgent != null) context.Request.Headers.UserAgent = userAgent; + var accessor = new Mock(); + accessor.Setup(a => a.HttpContext).Returns(context); + return new UserAgentClassifier(accessor.Object); + } + + [TestMethod] + [DataRow("Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)")] + [DataRow("Mozilla/5.0 (compatible; bingbot/2.0; +http://www.bing.com/bingbot.htm)")] + [DataRow("Mozilla/5.0 (compatible; AhrefsBot/7.0; +http://ahrefs.com/robot/)")] + [DataRow("Mozilla/5.0 (compatible; SemrushBot/7~bl; +http://www.semrush.com/bot.html)")] + [DataRow("Mozilla/5.0 AppleWebKit/537.36 (KHTML, like Gecko; compatible; GPTBot/1.2; +https://openai.com/gptbot)")] + [DataRow("Mozilla/5.0 AppleWebKit/537.36 (KHTML, like Gecko; compatible; ClaudeBot/1.0; +claudebot@anthropic.com)")] + [DataRow("Mozilla/5.0 (compatible; YandexBot/3.0; +http://yandex.com/bots)")] + [DataRow("Mozilla/5.0 (Linux; Android 5.0) AppleWebKit/537.36 (KHTML, like Gecko) Mobile Safari/537.36 (compatible; Bytespider; spider-feedback@bytedance.com)")] + [DataRow("Mozilla/5.0 (compatible; Baiduspider/2.0; +http://www.baidu.com/search/spider.html)")] + [DataRow("facebookexternalhit/1.1 (+http://www.facebook.com/externalhit_uatext.php)")] + [DataRow("meta-externalagent/1.1 (+https://developers.facebook.com/docs/sharing/webmasters/crawler)")] + [DataRow("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/120.0.0.0 Safari/537.36")] + [DataRow("Mozilla/5.0 (compatible; crawler)")] + [DataRow("Mozilla/5.0 (compatible; SomeSpider/1.0)")] + [DataRow("Scrapy/2.11.0 (+https://scrapy.org)")] + [DataRow("curl/8.4.0")] + [DataRow("Wget/1.21")] + [DataRow("python-requests/2.31.0")] + [DataRow("Go-http-client/1.1")] + [DataRow("okhttp/4.12.0")] + [DataRow("Java/17.0.2")] + [DataRow("Apache-HttpClient/4.5.14 (Java/17.0.2)")] + [DataRow("axios/1.6.0")] + [DataRow("node-fetch/1.0 (+https://github.com/bitinn/node-fetch)")] + [DataRow("")] + [DataRow(null)] + public void IsCrawler_BotsAndHttpClients_True(string userAgent) + { + Assert.IsTrue(ClassifierFor(userAgent).IsCrawler); + } + + [TestMethod] + [DataRow("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36")] + [DataRow("Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Safari/605.1.15")] + [DataRow("Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Mobile/15E148 Safari/604.1")] + [DataRow("Mozilla/5.0 (Linux; Android 10; CUBOT X30) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36")] + [DataRow("Mozilla/5.0 (X11; Linux x86_64; rv:121.0) Gecko/20100101 Firefox/121.0")] + public void IsCrawler_Browsers_False(string userAgent) + { + Assert.IsFalse(ClassifierFor(userAgent).IsCrawler); + } + + [TestMethod] + [DataRow("Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148", true)] + [DataRow("Mozilla/5.0 (iPad; CPU OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Safari/604.1", true)] + [DataRow("Mozilla/5.0 (Linux; Android 14; Pixel 8) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36", true)] + [DataRow("Mozilla/5.0 (Linux; Android 13; SM-X710) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36", true)] + [DataRow("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36", false)] + [DataRow("", false)] + public void IsMobileOrTablet(string userAgent, bool expected) + { + Assert.AreEqual(expected, ClassifierFor(userAgent).IsMobileOrTablet); + } + + [TestMethod] + public void NoHttpContext_NotACrawlerNotMobile() + { + //a scope without a request (scheduled task) is never a crawler + var classifier = new UserAgentClassifier(Mock.Of()); + + Assert.IsFalse(classifier.IsCrawler); + Assert.IsFalse(classifier.IsMobileOrTablet); + } +} diff --git a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs index 1e586ead6d..dd69af8f1c 100644 --- a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs @@ -11,13 +11,13 @@ using Grand.Domain.Tax; using Grand.Infrastructure.Configuration; using Grand.Web.Common.RequestCustomerResolvers; +using Grand.Web.Common.UserAgent; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using Wangkanai.Detection.Services; namespace Grand.Web.Common.Tests; @@ -29,7 +29,7 @@ public class WorkContextSetterRequestResolverTests private DefaultHttpContext _httpContext; private Mock _authentication; private Mock _apiAuthentication; - private Mock _detection; + private Mock _userAgent; private Mock _accessor; [TestInitialize] @@ -39,7 +39,7 @@ public void Setup() _groupService = new Mock(); _authentication = new Mock(); _apiAuthentication = new Mock(); - _detection = new Mock(); + _userAgent = new Mock(); _httpContext = new DefaultHttpContext(); _accessor = new Mock(); _accessor.Setup(a => a.HttpContext).Returns(_httpContext); @@ -69,7 +69,7 @@ private IRequestCustomerResolver[] BuiltIns() { return [ new ApiUserCustomerResolver(_apiAuthentication.Object, _accessor.Object, _customerService.Object), - new SearchEngineCustomerResolver(_detection.Object, _customerService.Object), + new SearchEngineCustomerResolver(_userAgent.Object, _customerService.Object), new GuestCookieCustomerResolver(_authentication.Object, _customerService.Object, _groupService.Object), new CookieCustomerResolver(_authentication.Object), new AllowAnonymousCustomerResolver(_accessor.Object, _customerService.Object) @@ -87,9 +87,7 @@ private void ArrangeAllBuiltInsMatch() _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(guid.ToString()); _customerService.Setup(c => c.GetCustomerByGuid(guid)) .ReturnsAsync(new Customer { Id = "guest", Active = true }); - var crawler = new Mock(); - crawler.SetupGet(c => c.IsCrawler).Returns(true); - _detection.SetupGet(d => d.Crawler).Returns(crawler.Object); + _userAgent.SetupGet(u => u.IsCrawler).Returns(true); _customerService.Setup(c => c.GetCustomerBySystemName(SystemCustomerNames.SearchEngine)) .ReturnsAsync(new Customer { Id = "crawler", IsSystemAccount = true }); _apiAuthentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync(new Customer { Id = "api", Active = true }); @@ -140,7 +138,7 @@ public async Task BuiltIns_ApiUser_WinsWhenNothingElseMatches() _httpContext.SetEndpoint(null); _authentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync((Customer)null); _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync((string)null); - _detection.SetupGet(d => d.Crawler).Returns((ICrawlerService)null); + _userAgent.SetupGet(u => u.IsCrawler).Returns(false); var customer = await CreateSetter(BuiltIns()).CurrentCustomerForTest(new Store()); Assert.AreEqual("api", customer.Id); } @@ -150,7 +148,7 @@ public async Task BuiltIns_NothingMatches_CreatesNewGuest() { _authentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync((Customer)null); _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync((string)null); - _detection.SetupGet(d => d.Crawler).Returns((ICrawlerService)null); + _userAgent.SetupGet(u => u.IsCrawler).Returns(false); _apiAuthentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync((Customer)null); _customerService.Setup(c => c.InsertGuestCustomer(It.IsAny())) .ReturnsAsync((Customer c) => c); diff --git a/src/Web/Grand.Web.Common/Grand.Web.Common.csproj b/src/Web/Grand.Web.Common/Grand.Web.Common.csproj index 53d8c644ad..f2542a14d6 100644 --- a/src/Web/Grand.Web.Common/Grand.Web.Common.csproj +++ b/src/Web/Grand.Web.Common/Grand.Web.Common.csproj @@ -11,7 +11,6 @@ - diff --git a/src/Web/Grand.Web.Common/Infrastructure/ServiceCollectionExtensions.cs b/src/Web/Grand.Web.Common/Infrastructure/ServiceCollectionExtensions.cs index 7c6dd6fbee..f1757c054a 100644 --- a/src/Web/Grand.Web.Common/Infrastructure/ServiceCollectionExtensions.cs +++ b/src/Web/Grand.Web.Common/Infrastructure/ServiceCollectionExtensions.cs @@ -302,13 +302,4 @@ public static void AddWebEncoder(this IServiceCollection services) }); } - - /// - /// Adds services for detection device - /// - /// Collection of service descriptors - public static void AddDetectionDevice(this IServiceCollection services) - { - services.AddDetection(); - } } \ No newline at end of file diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs index 14ed77830e..176b887048 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs @@ -1,6 +1,6 @@ using Grand.Business.Core.Interfaces.Customers; using Grand.Domain.Customers; -using Wangkanai.Detection.Services; +using Grand.Web.Common.UserAgent; namespace Grand.Web.Common.RequestCustomerResolvers; @@ -8,15 +8,14 @@ namespace Grand.Web.Common.RequestCustomerResolvers; /// Crawlers run as the SearchEngine system customer /// public class SearchEngineCustomerResolver( - IDetectionService detectionService, + IUserAgentClassifier userAgentClassifier, ICustomerService customerService) : IRequestCustomerResolver { public int Order => RequestCustomerResolverOrder.SearchEngine; public async Task Resolve() { - var isCrawler = detectionService.Crawler?.IsCrawler; - if (!isCrawler.GetValueOrDefault()) return null; + if (!userAgentClassifier.IsCrawler) return null; return await customerService.GetCustomerBySystemName(SystemCustomerNames.SearchEngine); } diff --git a/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs b/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs index 45815a79cd..2029022aea 100644 --- a/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs +++ b/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs @@ -2,6 +2,7 @@ using Grand.Infrastructure.Configuration; using Grand.Web.Common.Infrastructure; using Grand.Web.Common.Routing; +using Grand.Web.Common.UserAgent; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.ResponseCompression; @@ -68,8 +69,8 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config //add WebEncoderOptions services.AddWebEncoder(); - //add detection device - services.AddDetectionDevice(); + //classify the request's user agent (crawler, mobile) + services.AddScoped(); //add routing services.AddRouting(options => @@ -122,9 +123,6 @@ public void Configure(WebApplication application, IWebHostEnvironment webHostEnv if (!performanceConfig.IgnoreUsePoweredByMiddleware) application.UsePoweredBy(); - //add responsive middleware (for detection) - application.UseDetection(); - //use routing application.UseRouting(); } diff --git a/src/Web/Grand.Web.Common/TagHelpers/Admin/AdminTabStripTagHelper.cs b/src/Web/Grand.Web.Common/TagHelpers/Admin/AdminTabStripTagHelper.cs index ae00c72220..31e66c0ebb 100644 --- a/src/Web/Grand.Web.Common/TagHelpers/Admin/AdminTabStripTagHelper.cs +++ b/src/Web/Grand.Web.Common/TagHelpers/Admin/AdminTabStripTagHelper.cs @@ -1,12 +1,11 @@ using Grand.Mediator; using Grand.Web.Common.Events; +using Grand.Web.Common.UserAgent; using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.ViewFeatures; using Microsoft.AspNetCore.Razor.TagHelpers; using System.Text.Json; -using Wangkanai.Detection.Models; -using Wangkanai.Detection.Services; namespace Grand.Web.Common.TagHelpers.Admin; @@ -19,13 +18,13 @@ namespace Grand.Web.Common.TagHelpers.Admin; [HtmlTargetElement("admin-tabstrip")] public class AdminTabStripTagHelper : TagHelper { - private readonly IDetectionService _detectionService; + private readonly IUserAgentClassifier _userAgentClassifier; private readonly IMediator _mediator; - public AdminTabStripTagHelper(IMediator mediator, IDetectionService detectionService) + public AdminTabStripTagHelper(IMediator mediator, IUserAgentClassifier userAgentClassifier) { _mediator = mediator; - _detectionService = detectionService; + _userAgentClassifier = userAgentClassifier; } [HtmlAttributeName("SetTabPos")] public bool SetTabPos { get; set; } = false; @@ -41,7 +40,7 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu ViewContext.ViewData[typeof(AdminTabContentTagHelper).FullName] = new List(); _ = await output.GetChildContentAsync(); var list = (List)ViewContext.ViewData[typeof(AdminTabContentTagHelper).FullName]; - if (_detectionService.Device.Type == Device.Mobile || _detectionService.Device.Type == Device.Tablet) + if (_userAgentClassifier.IsMobileOrTablet) SetTabPos = false; var selectedTabIndex = GetSelectedTabIndex(); diff --git a/src/Web/Grand.Web.Common/UserAgent/IUserAgentClassifier.cs b/src/Web/Grand.Web.Common/UserAgent/IUserAgentClassifier.cs new file mode 100644 index 0000000000..9a1a6d1d9b --- /dev/null +++ b/src/Web/Grand.Web.Common/UserAgent/IUserAgentClassifier.cs @@ -0,0 +1,17 @@ +namespace Grand.Web.Common.UserAgent; + +/// +/// What the current request's User-Agent says about the client +/// +public interface IUserAgentClassifier +{ + /// + /// A crawler, a scraper or a plain HTTP client - including a missing User-Agent + /// + bool IsCrawler { get; } + + /// + /// A phone or a tablet + /// + bool IsMobileOrTablet { get; } +} diff --git a/src/Web/Grand.Web.Common/UserAgent/UserAgentClassifier.cs b/src/Web/Grand.Web.Common/UserAgent/UserAgentClassifier.cs new file mode 100644 index 0000000000..5096c70815 --- /dev/null +++ b/src/Web/Grand.Web.Common/UserAgent/UserAgentClassifier.cs @@ -0,0 +1,42 @@ +using System.Text.RegularExpressions; +using Microsoft.AspNetCore.Http; + +namespace Grand.Web.Common.UserAgent; + +/// +/// Classifies the current request's User-Agent once per request +/// +public class UserAgentClassifier(IHttpContextAccessor httpContextAccessor) : IUserAgentClassifier +{ + //user agents longer than this are cut before matching + private const int MaxLength = 512; + + //"bot" not preceded by "cu": CUBOT is an Android phone brand + private static readonly Regex CrawlerPattern = new( + "(? _isCrawler ??= Classify(userAgent => + string.IsNullOrWhiteSpace(userAgent) || CrawlerPattern.IsMatch(userAgent)); + + public bool IsMobileOrTablet => _isMobileOrTablet ??= Classify(userAgent => + !string.IsNullOrWhiteSpace(userAgent) && MobileOrTabletPattern.IsMatch(userAgent)); + + private bool Classify(Func rule) + { + var request = httpContextAccessor.HttpContext?.Request; + if (request == null) return false; + + var userAgent = request.Headers.UserAgent.ToString(); + if (userAgent.Length > MaxLength) userAgent = userAgent[..MaxLength]; + return rule(userAgent); + } +} From 77af4542f52ad6c6190fee35d0d6befd90a1a7e4 Mon Sep 17 00:00:00 2001 From: support Date: Wed, 14 Oct 2026 20:19:24 +0200 Subject: [PATCH 02/15] Resolve the guest cookie only to an active guest --- .../RequestCustomerResolversTests.cs | 34 +++++++++++++++++++ .../GuestCookieCustomerResolver.cs | 6 ++-- .../GuestCustomerRules.cs | 28 +++++++++++++++ 3 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRules.cs diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs index c4aa4330d9..8065ffd3ed 100644 --- a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs @@ -207,6 +207,40 @@ public async Task ApiUser_Authenticated_ReturnsCustomer() Assert.AreEqual("api", (await resolver.Resolve()).Id); } + [TestMethod] + public async Task GuestCookie_SystemAccountGuid_ReturnsNull() + { + //Anonymous and SearchEngine belong to the Guests group + var guid = Guid.NewGuid(); + var authentication = new Mock(); + authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(guid.ToString()); + var customers = new Mock(); + customers.Setup(c => c.GetCustomerByGuid(guid)) + .ReturnsAsync(new Customer { Id = "anonymous", Active = true, IsSystemAccount = true }); + + var resolver = new GuestCookieCustomerResolver(authentication.Object, customers.Object, + Mock.Of()); + + Assert.IsNull(await resolver.Resolve()); + } + + [TestMethod] + [DataRow("00000000-0000-0000-0000-000000000000")] + [DataRow("0f8fad5bd9cb469fa16570867728950e")] + [DataRow("{0f8fad5b-d9cb-469f-a165-70867728950e}")] + public async Task GuestCookie_EmptyOrNonCanonicalGuid_NotLookedUp(string cookie) + { + var authentication = new Mock(); + authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(cookie); + var customers = new Mock(); + + var resolver = new GuestCookieCustomerResolver(authentication.Object, customers.Object, + Mock.Of()); + + Assert.IsNull(await resolver.Resolve()); + customers.Verify(c => c.GetCustomerByGuid(It.IsAny()), Times.Never); + } + private static IHttpContextAccessor AccessorWith(HttpContext context) { var accessor = new Mock(); diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs index 9b0962899d..66ea5f1c64 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs @@ -17,11 +17,11 @@ public class GuestCookieCustomerResolver( public async Task Resolve() { - var guid = await authenticationService.GetCustomerGuid(); - if (string.IsNullOrEmpty(guid) || !Guid.TryParse(guid, out var customerGuid)) return null; + var cookie = await authenticationService.GetCustomerGuid(); + if (!GuestCustomerRules.TryParseCookie(cookie, out var customerGuid)) return null; var customerByGuid = await customerService.GetCustomerByGuid(customerGuid); - if (customerByGuid is { Deleted: false, Active: true } && !await groupService.IsRegistered(customerByGuid)) + if (customerByGuid != null && await GuestCustomerRules.IsAcceptableGuest(customerByGuid, groupService)) return customerByGuid; return null; diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRules.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRules.cs new file mode 100644 index 0000000000..bb175ca7f4 --- /dev/null +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRules.cs @@ -0,0 +1,28 @@ +using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Domain.Customers; + +namespace Grand.Web.Common.RequestCustomerResolvers; + +/// +/// What a guest cookie may resolve to +/// +public static class GuestCustomerRules +{ + /// + /// The cookie holds a GUID in the format the store writes ("D"), never Guid.Empty + /// + public static bool TryParseCookie(string value, out Guid customerGuid) + { + return Guid.TryParseExact(value, "D", out customerGuid) && customerGuid != Guid.Empty; + } + + /// + /// An active guest: not deleted, not registered and not a system account (Anonymous and SearchEngine are + /// in the Guests group too) + /// + public static async Task IsAcceptableGuest(Customer customer, IGroupService groupService) + { + return customer is { Deleted: false, Active: true, IsSystemAccount: false } + && !await groupService.IsRegistered(customer); + } +} From cdbfc31b150c529f63b3915147257be1ea835ac8 Mon Sep 17 00:00:00 2001 From: support Date: Wed, 14 Oct 2026 20:25:48 +0200 Subject: [PATCH 03/15] Skip activity tracking for system accounts --- .../Filters/CustomerActivityAttributeTests.cs | 75 +++++++++++++++++++ .../Filters/CustomerActivityAttribute.cs | 6 +- 2 files changed, 80 insertions(+), 1 deletion(-) create mode 100644 src/Tests/Grand.Web.Common.Tests/Filters/CustomerActivityAttributeTests.cs diff --git a/src/Tests/Grand.Web.Common.Tests/Filters/CustomerActivityAttributeTests.cs b/src/Tests/Grand.Web.Common.Tests/Filters/CustomerActivityAttributeTests.cs new file mode 100644 index 0000000000..69207f40ff --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/Filters/CustomerActivityAttributeTests.cs @@ -0,0 +1,75 @@ +using Grand.Business.Core.Interfaces.Customers; +using Grand.Data; +using Grand.Domain.Customers; +using Grand.Infrastructure; +using Grand.SharedKernel.Extensions; +using Grand.Web.Common.Filters; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using System.Linq.Expressions; +using System.Net; + +namespace Grand.Web.Common.Tests.Filters; + +[TestClass] +public class CustomerActivityAttributeTests +{ + private Mock _customerService; + private Mock _contextAccessor; + private Mock _workContext; + + [TestInitialize] + public void Setup() + { + //the test App_Data/Settings.cfg has a connection string, so the filter sees an installed database + DataSettingsManager.Initialize(Path.Combine("", CommonPath.AppData, CommonPath.SettingsFile)); + _customerService = new Mock(); + _workContext = new Mock(); + _contextAccessor = new Mock(); + _contextAccessor.SetupGet(c => c.WorkContext).Returns(_workContext.Object); + } + + private async Task RunGet(Customer current) + { + _workContext.SetupGet(w => w.CurrentCustomer).Returns(current); + var httpContext = new DefaultHttpContext(); + httpContext.Request.Method = HttpMethods.Get; + httpContext.Request.Scheme = "https"; + httpContext.Request.Host = new HostString("shop.test"); + httpContext.Request.Path = "/product"; + httpContext.Connection.RemoteIpAddress = IPAddress.Parse("10.0.0.1"); + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var executing = new ActionExecutingContext(actionContext, new List(), + new Dictionary(), null); + var filter = new CustomerActivityAttribute.CustomerActivityFilter(_customerService.Object, + _contextAccessor.Object, new CustomerSettings { StoreLastVisitedPage = true }); + + await filter.OnActionExecutionAsync(executing, + () => Task.FromResult(new ActionExecutedContext(actionContext, new List(), null))); + } + + [TestMethod] + public async Task SystemAccount_NothingWritten() + { + await RunGet(new Customer { Id = "searchengine", IsSystemAccount = true }); + + _customerService.Verify(c => c.UpdateCustomerField(It.IsAny(), + It.IsAny>>(), It.IsAny()), Times.Never); + _customerService.Verify(c => c.UpdateCustomerField(It.IsAny(), + It.IsAny>>(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task Guest_ActivityWritten() + { + await RunGet(new Customer { Id = "guest", Active = true }); + + _customerService.Verify(c => c.UpdateCustomerField(It.IsAny(), + It.IsAny>>(), It.IsAny()), Times.AtLeastOnce); + } +} diff --git a/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs b/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs index f5ae2d36d4..f4f72f3093 100644 --- a/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs +++ b/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs @@ -26,7 +26,7 @@ public CustomerActivityAttribute() : base(typeof(CustomerActivityFilter)) /// /// Represents a filter that saves last customer activity date /// - private class CustomerActivityFilter( + internal class CustomerActivityFilter( ICustomerService customerService, IContextAccessor contextAccessor, CustomerSettings customerSettings) : IAsyncActionFilter @@ -53,6 +53,10 @@ public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionE if (!customerSettings.StoreLastVisitedPage) return; + //system accounts (SearchEngine, Anonymous) are shared by every request they serve + if (contextAccessor.WorkContext.CurrentCustomer.IsSystemAccount) + return; + //update last activity date if (contextAccessor.WorkContext.CurrentCustomer.LastActivityDateUtc.AddMinutes(3.0) < DateTime.UtcNow) await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCustomer, x => x.LastActivityDateUtc, From 9d53e59b3f84678bf85d4e49832d88f050af47aa Mon Sep 17 00:00:00 2001 From: support Date: Wed, 14 Oct 2026 20:31:17 +0200 Subject: [PATCH 04/15] Add guest customer persistence for unsaved guests --- .../GuestCustomerPersistenceTests.cs | 135 ++++++++++++++++++ .../GuestCustomerPersistence.cs | 64 +++++++++ .../IGuestCustomerPersistence.cs | 30 ++++ 3 files changed, 229 insertions(+) create mode 100644 src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs create mode 100644 src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs create mode 100644 src/Web/Grand.Web.Common/RequestCustomerResolvers/IGuestCustomerPersistence.cs diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs new file mode 100644 index 0000000000..5b91e5c280 --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs @@ -0,0 +1,135 @@ +using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Business.Core.Interfaces.Customers; +using Grand.Domain.Customers; +using Grand.Web.Common.RequestCustomerResolvers; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Web.Common.Tests.RequestCustomerResolvers; + +[TestClass] +public class GuestCustomerPersistenceTests +{ + private Mock _customers; + private Mock _groups; + private GuestCustomerPersistence _persistence; + + [TestInitialize] + public void Setup() + { + _customers = new Mock(); + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())).ReturnsAsync((Customer c) => c); + _groups = new Mock(); + _persistence = new GuestCustomerPersistence(_customers.Object, _groups.Object, + NullLogger.Instance); + } + + [TestMethod] + public void IsTransient_OnlyForTheTransientId() + { + Assert.IsFalse(_persistence.IsTransient("g1")); + + _persistence.SetTransient(new Customer { Id = "g1" }); + + Assert.IsTrue(_persistence.IsTransient("g1")); + Assert.IsFalse(_persistence.IsTransient("other")); + Assert.IsFalse(_persistence.IsTransient(null)); + Assert.IsFalse(_persistence.IsTransient("")); + } + + [TestMethod] + public async Task EnsurePersisted_InsertsOnceAndClearsTransient() + { + var guest = new Customer { Id = "g1" }; + _persistence.SetTransient(guest); + + await _persistence.EnsurePersisted(); + await _persistence.EnsurePersisted(); + + _customers.Verify(c => c.InsertGuestCustomer(guest), Times.Once); + Assert.IsNull(_persistence.Transient); + Assert.IsFalse(_persistence.IsTransient("g1")); + } + + [TestMethod] + public async Task EnsurePersisted_NoTransient_DoesNothing() + { + await _persistence.EnsurePersisted(); + + _customers.Verify(c => c.InsertGuestCustomer(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task EnsurePersisted_ConcurrentCalls_InsertOnce() + { + var gate = new TaskCompletionSource(); + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())).Returns(gate.Task); + _persistence.SetTransient(new Customer { Id = "g1" }); + + var first = _persistence.EnsurePersisted(); + var second = _persistence.EnsurePersisted(); + gate.SetResult(new Customer { Id = "g1" }); + await Task.WhenAll(first, second); + + _customers.Verify(c => c.InsertGuestCustomer(It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task Promote_Inserts() + { + var guest = new Customer { Id = "g1", CustomerGuid = Guid.NewGuid() }; + + Assert.AreSame(guest, await _persistence.Promote(guest)); + } + + [TestMethod] + public async Task Promote_ParallelRequestWon_ReturnsTheExistingGuest() + { + //the unique CustomerGuid index rejected the second insert of the same cookie + var guid = Guid.NewGuid(); + var existing = new Customer { Id = "first", CustomerGuid = guid, Active = true }; + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())) + .ThrowsAsync(new InvalidOperationException("duplicate key")); + _customers.Setup(c => c.GetCustomerByGuid(guid)).ReturnsAsync(existing); + + var result = await _persistence.Promote(new Customer { Id = "second", CustomerGuid = guid }); + + Assert.AreSame(existing, result); + } + + [TestMethod] + public async Task Promote_GuidTurnedOutToBeRegistered_ReturnsNull() + { + var guid = Guid.NewGuid(); + var admin = new Customer { Id = "admin", CustomerGuid = guid, Active = true }; + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())) + .ThrowsAsync(new InvalidOperationException("duplicate key")); + _customers.Setup(c => c.GetCustomerByGuid(guid)).ReturnsAsync(admin); + _groups.Setup(g => g.IsRegistered(admin)).ReturnsAsync(true); + + Assert.IsNull(await _persistence.Promote(new Customer { Id = "x", CustomerGuid = guid })); + } + + [TestMethod] + public async Task Promote_GuidTurnedOutToBeSystemAccount_ReturnsNull() + { + var guid = Guid.NewGuid(); + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())) + .ThrowsAsync(new InvalidOperationException("duplicate key")); + _customers.Setup(c => c.GetCustomerByGuid(guid)).ReturnsAsync( + new Customer { Id = "searchengine", CustomerGuid = guid, Active = true, IsSystemAccount = true }); + + Assert.IsNull(await _persistence.Promote(new Customer { Id = "x", CustomerGuid = guid })); + } + + [TestMethod] + public async Task Promote_InsertFailsForAnotherReason_Throws() + { + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())) + .ThrowsAsync(new InvalidOperationException("db down")); + + await Assert.ThrowsExactlyAsync(() => + _persistence.Promote(new Customer { Id = "x", CustomerGuid = Guid.NewGuid() })); + } +} diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs new file mode 100644 index 0000000000..79c11eded5 --- /dev/null +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs @@ -0,0 +1,64 @@ +using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Business.Core.Interfaces.Customers; +using Grand.Domain.Customers; +using Microsoft.Extensions.Logging; + +namespace Grand.Web.Common.RequestCustomerResolvers; + +/// +/// Takes the undecorated customer service: the decorator depends on this class +/// +public class GuestCustomerPersistence( + ICustomerService customerService, + IGroupService groupService, + ILogger logger) : IGuestCustomerPersistence +{ + private Task _persisting; + + public Customer Transient { get; private set; } + + public void SetTransient(Customer customer) + { + Transient = customer; + _persisting = null; + } + + public bool IsTransient(string customerId) + { + return Transient != null && !string.IsNullOrEmpty(customerId) && Transient.Id == customerId; + } + + public Task EnsurePersisted() + { + if (Transient == null) return Task.CompletedTask; + + //writes of one request may run concurrently: they share the single insert + return _persisting ??= Persist(Transient); + } + + private async Task Persist(Customer guest) + { + await customerService.InsertGuestCustomer(guest); + Transient = null; + } + + public async Task Promote(Customer guest) + { + try + { + return await customerService.InsertGuestCustomer(guest); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + //the unique CustomerGuid index: a parallel request of the same browser saved this cookie first + var existing = await customerService.GetCustomerByGuid(guest.CustomerGuid); + if (existing == null) throw; + + if (await GuestCustomerRules.IsAcceptableGuest(existing, groupService)) return existing; + + logger.LogWarning("Guest cookie {CustomerGuid} belongs to a customer that is not a guest; a new guest is issued", + guest.CustomerGuid); + return null; + } + } +} diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/IGuestCustomerPersistence.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/IGuestCustomerPersistence.cs new file mode 100644 index 0000000000..daa5b7cc94 --- /dev/null +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/IGuestCustomerPersistence.cs @@ -0,0 +1,30 @@ +using Grand.Domain.Customers; + +namespace Grand.Web.Common.RequestCustomerResolvers; + +/// +/// The guest of this request that is not saved yet: saved on its first write, or by a later request that +/// returns its cookie +/// +public interface IGuestCustomerPersistence +{ + /// + /// The unsaved guest of this request; null once saved or when the customer came from the database + /// + Customer Transient { get; } + + void SetTransient(Customer customer); + + bool IsTransient(string customerId); + + /// + /// Saves the transient guest once; later calls do nothing + /// + Task EnsurePersisted(); + + /// + /// Saves a guest for a returned cookie whose GUID no customer has. Returns the saved guest, the guest a + /// parallel request saved first, or null when the GUID turned out to belong to a customer that is not a guest + /// + Task Promote(Customer guest); +} From 71484272c5f8e96547a29631ff9964efbf2fc4c7 Mon Sep 17 00:00:00 2001 From: support Date: Wed, 14 Oct 2026 20:39:32 +0200 Subject: [PATCH 05/15] Save the unsaved guest before the first write to it --- .../TransientGuestCustomerServiceTests.cs | 122 ++++++++++ .../GuestCustomerRegistration.cs | 32 +++ .../TransientGuestCustomerService.cs | 216 ++++++++++++++++++ .../Startup/GuestCustomerStartup.cs | 28 +++ 4 files changed, 398 insertions(+) create mode 100644 src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs create mode 100644 src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs create mode 100644 src/Web/Grand.Web.Common/RequestCustomerResolvers/TransientGuestCustomerService.cs create mode 100644 src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs new file mode 100644 index 0000000000..41aa903e63 --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs @@ -0,0 +1,122 @@ +using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Business.Core.Interfaces.Customers; +using Grand.Domain.Customers; +using Grand.Domain.Orders; +using Grand.Web.Common.RequestCustomerResolvers; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using System.Linq.Expressions; + +namespace Grand.Web.Common.Tests.RequestCustomerResolvers; + +[TestClass] +public class TransientGuestCustomerServiceTests +{ + private Mock _inner; + private Mock _persistence; + private TransientGuestCustomerService _service; + private readonly Customer _transient = new() { Id = "transient" }; + + [TestInitialize] + public void Setup() + { + _inner = new Mock(); + _persistence = new Mock(); + _persistence.Setup(p => p.IsTransient("transient")).Returns(true); + _persistence.SetupGet(p => p.Transient).Returns(_transient); + _persistence.Setup(p => p.EnsurePersisted()).Returns(Task.CompletedTask); + _service = new TransientGuestCustomerService(_inner.Object, _persistence.Object); + } + + [TestMethod] + public async Task UpdateCustomerFieldById_OnTransient_PersistsThenWrites() + { + //AffiliateAttribute writes the affiliate id by customer id on the landing GET + var order = new List(); + _persistence.Setup(p => p.EnsurePersisted()).Callback(() => order.Add("persist")).Returns(Task.CompletedTask); + _inner.Setup(i => i.UpdateCustomerField("transient", It.IsAny>>(), "aff")) + .Callback(() => order.Add("write")).Returns(Task.CompletedTask); + + await _service.UpdateCustomerField("transient", x => x.AffiliateId, "aff"); + + CollectionAssert.AreEqual(new[] { "persist", "write" }, order); + } + + [TestMethod] + public async Task UpdateUserField_OnTransient_Persists() + { + await _service.UpdateUserField(_transient, SystemCustomerFieldNames.LanguageId, "pl", "store"); + + _persistence.Verify(p => p.EnsurePersisted(), Times.Once); + _inner.Verify(i => i.UpdateUserField(_transient, SystemCustomerFieldNames.LanguageId, "pl", "store"), Times.Once); + } + + [TestMethod] + public async Task InsertShoppingCartItem_OnTransient_Persists() + { + await _service.InsertShoppingCartItem("transient", new ShoppingCartItem()); + + _persistence.Verify(p => p.EnsurePersisted(), Times.Once); + } + + [TestMethod] + public async Task Write_OnAnotherCustomer_DoesNotPersist() + { + await _service.UpdateUserField(new Customer { Id = "other" }, "key", "value"); + await _service.UpdateCustomerField("other", x => x.AffiliateId, "aff"); + + _persistence.Verify(p => p.EnsurePersisted(), Times.Never); + } + + [TestMethod] + public async Task Reads_NeverPersist() + { + await _service.GetCustomerByGuid(Guid.NewGuid()); + await _service.GetCustomerBySystemName("x"); + + _persistence.Verify(p => p.EnsurePersisted(), Times.Never); + } + + [TestMethod] + public async Task GetCustomerById_OfTransient_ReturnsTransientWithoutQuery() + { + Assert.AreSame(_transient, await _service.GetCustomerById("transient")); + _inner.Verify(i => i.GetCustomerById(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task InsertGuestCustomer_PassesThrough() + { + var guest = new Customer(); + _inner.Setup(i => i.InsertGuestCustomer(guest)).ReturnsAsync(guest); + + Assert.AreSame(guest, await _service.InsertGuestCustomer(guest)); + _persistence.Verify(p => p.EnsurePersisted(), Times.Never); + } + + [TestMethod] + public void Registration_DecoratesCustomerServiceAndSharesPersistencePerScope() + { + var services = new ServiceCollection(); + services.AddScoped(); + services.AddSingleton(Mock.Of()); + services.AddSingleton(typeof(ILogger<>), typeof(NullLogger<>)); + + services.AddTransientGuestCustomers(); + + using var provider = services.BuildServiceProvider(new ServiceProviderOptions { ValidateScopes = true }); + using var scope = provider.CreateScope(); + Assert.IsInstanceOfType(scope.ServiceProvider.GetRequiredService(), + typeof(TransientGuestCustomerService)); + Assert.AreSame(scope.ServiceProvider.GetRequiredService(), + scope.ServiceProvider.GetRequiredService()); + Assert.AreEqual(1, services.Count(d => d.ServiceType == typeof(ICustomerService))); + } + + //any concrete ICustomerService with a parameterless constructor, as the business layer registers one + private class FakeCustomerService() + : TransientGuestCustomerService(Mock.Of(), Mock.Of()); +} diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs new file mode 100644 index 0000000000..e79c696d5b --- /dev/null +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs @@ -0,0 +1,32 @@ +using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Business.Core.Interfaces.Customers; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Grand.Web.Common.RequestCustomerResolvers; + +public static class GuestCustomerRegistration +{ + /// + /// Decorates the registered customer service so that the request's unsaved guest is saved before its first + /// write. Call after the business layer has registered ICustomerService + /// + public static IServiceCollection AddTransientGuestCustomers(this IServiceCollection services) + { + var registered = services.LastOrDefault(d => d.ServiceType == typeof(ICustomerService)); + if (registered?.ImplementationType == null) return services; + + var innerType = registered.ImplementationType; + services.Remove(registered); + services.Add(new ServiceDescriptor(innerType, innerType, registered.Lifetime)); + + services.AddScoped(sp => new GuestCustomerPersistence( + (ICustomerService)sp.GetRequiredService(innerType), + sp.GetRequiredService(), + sp.GetRequiredService>())); + services.Add(new ServiceDescriptor(typeof(ICustomerService), sp => new TransientGuestCustomerService( + (ICustomerService)sp.GetRequiredService(innerType), + sp.GetRequiredService()), registered.Lifetime)); + return services; + } +} diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/TransientGuestCustomerService.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/TransientGuestCustomerService.cs new file mode 100644 index 0000000000..b74b4618e2 --- /dev/null +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/TransientGuestCustomerService.cs @@ -0,0 +1,216 @@ +using Grand.Business.Core.Interfaces.Customers; +using Grand.Domain; +using Grand.Domain.Common; +using Grand.Domain.Customers; +using Grand.Domain.Orders; +using System.Linq.Expressions; + +namespace Grand.Web.Common.RequestCustomerResolvers; + +/// +/// Saves the request's unsaved guest before the first write to it. Every customer write is a partial update by +/// id, which silently does nothing on a document that does not exist yet +/// +public class TransientGuestCustomerService(ICustomerService inner, IGuestCustomerPersistence persistence) + : ICustomerService +{ + private async Task Persist(string customerId) + { + if (persistence.IsTransient(customerId)) await persistence.EnsurePersisted(); + } + + #region Reads + + public Task> GetAllCustomers(DateTime? createdFromUtc = null, DateTime? createdToUtc = null, + string affiliateId = "", string vendorId = "", string storeId = "", string ownerId = "", + string salesEmployeeId = "", string[] customerGroupIds = null, string[] customerTagIds = null, + string email = null, string username = null, string firstName = null, string lastName = null, + string company = null, string phone = null, string zipPostalCode = null, + bool loadOnlyWithShoppingCart = false, ShoppingCartType? sct = null, int pageIndex = 0, + int pageSize = int.MaxValue, Expression> orderBySelector = null) + { + return inner.GetAllCustomers(createdFromUtc, createdToUtc, affiliateId, vendorId, storeId, ownerId, + salesEmployeeId, customerGroupIds, customerTagIds, email, username, firstName, lastName, company, phone, + zipPostalCode, loadOnlyWithShoppingCart, sct, pageIndex, pageSize, orderBySelector); + } + + public Task> GetOnlineCustomers(DateTime lastActivityFromUtc, string[] customerGroupIds, + string storeId = "", string salesEmployeeId = "", int pageIndex = 0, int pageSize = int.MaxValue) + { + return inner.GetOnlineCustomers(lastActivityFromUtc, customerGroupIds, storeId, salesEmployeeId, pageIndex, + pageSize); + } + + public Task GetCountOnlineShoppingCart(DateTime lastActivityFromUtc, string storeId = "", + string salesEmployeeId = "") + { + return inner.GetCountOnlineShoppingCart(lastActivityFromUtc, storeId, salesEmployeeId); + } + + public Task GetCustomerById(string customerId) + { + //the request's own unsaved guest, e.g. reloaded by a handler on the first request + return persistence.IsTransient(customerId) + ? Task.FromResult(persistence.Transient) + : inner.GetCustomerById(customerId); + } + + public Task> GetCustomersByIds(string[] customerIds) => inner.GetCustomersByIds(customerIds); + + public Task GetCustomerByGuid(Guid customerGuid) => inner.GetCustomerByGuid(customerGuid); + + public Task GetCustomerByEmail(string email, string storeId = "") => + inner.GetCustomerByEmail(email, storeId); + + public Task GetCustomerBySystemName(string systemName) => inner.GetCustomerBySystemName(systemName); + + public Task GetCustomerByUsername(string username, string storeId = "") => + inner.GetCustomerByUsername(username, storeId); + + #endregion + + #region Inserts + + public Task InsertGuestCustomer(Customer customer) => inner.InsertGuestCustomer(customer); + + public Task InsertCustomer(Customer customer) => inner.InsertCustomer(customer); + + public Task DeleteGuestCustomers(DateTime? createdFromUtc, DateTime? createdToUtc, + bool onlyWithoutShoppingCart) => + inner.DeleteGuestCustomers(createdFromUtc, createdToUtc, onlyWithoutShoppingCart); + + #endregion + + #region Writes to one customer + + public async Task UpdateCustomer(Customer customer) + { + await Persist(customer?.Id); + await inner.UpdateCustomer(customer); + } + + public async Task UpdateUserField(Customer customer, string key, TPropType value, string storeId = "") + { + await Persist(customer?.Id); + await inner.UpdateUserField(customer, key, value, storeId); + } + + public async Task DeleteCustomer(Customer customer, bool hard = false) + { + await Persist(customer?.Id); + await inner.DeleteCustomer(customer, hard); + } + + public async Task UpdateCustomerField(Customer customer, Expression> expression, T value) + { + await Persist(customer?.Id); + await inner.UpdateCustomerField(customer, expression, value); + } + + public async Task UpdateCustomerField(string customerId, Expression> expression, T value) + { + await Persist(customerId); + await inner.UpdateCustomerField(customerId, expression, value); + } + + public async Task UpdateActive(Customer customer) + { + await Persist(customer?.Id); + await inner.UpdateActive(customer); + } + + public async Task UpdateContributions(Customer customer) + { + await Persist(customer?.Id); + await inner.UpdateContributions(customer); + } + + public async Task UpdateCustomerLastLoginDate(Customer customer) + { + await Persist(customer?.Id); + await inner.UpdateCustomerLastLoginDate(customer); + } + + public async Task UpdateCustomerInAdminPanel(Customer customer) + { + await Persist(customer?.Id); + await inner.UpdateCustomerInAdminPanel(customer); + } + + public async Task ResetCheckoutData(Customer customer, string storeId, bool clearCouponCodes = false, + bool clearCheckoutAttributes = false, bool clearLoyaltyPoints = true, bool clearShipping = true, + bool clearPayment = true) + { + await Persist(customer?.Id); + await inner.ResetCheckoutData(customer, storeId, clearCouponCodes, clearCheckoutAttributes, + clearLoyaltyPoints, clearShipping, clearPayment); + } + + public async Task InsertCustomerGroupInCustomer(CustomerGroup customerGroup, string customerId) + { + await Persist(customerId); + await inner.InsertCustomerGroupInCustomer(customerGroup, customerId); + } + + public async Task DeleteCustomerGroupInCustomer(CustomerGroup customerGroup, string customerId) + { + await Persist(customerId); + await inner.DeleteCustomerGroupInCustomer(customerGroup, customerId); + } + + public async Task DeleteAddress(Address address, string customerId) + { + await Persist(customerId); + await inner.DeleteAddress(address, customerId); + } + + public async Task InsertAddress(Address address, string customerId) + { + await Persist(customerId); + await inner.InsertAddress(address, customerId); + } + + public async Task UpdateAddress(Address address, string customerId) + { + await Persist(customerId); + await inner.UpdateAddress(address, customerId); + } + + public async Task UpdateBillingAddress(Address address, string customerId) + { + await Persist(customerId); + await inner.UpdateBillingAddress(address, customerId); + } + + public async Task UpdateShippingAddress(Address address, string customerId) + { + await Persist(customerId); + await inner.UpdateShippingAddress(address, customerId); + } + + public async Task ClearShoppingCartItem(string customerId, IList cart) + { + await Persist(customerId); + await inner.ClearShoppingCartItem(customerId, cart); + } + + public async Task DeleteShoppingCartItem(string customerId, ShoppingCartItem shoppingCartItem) + { + await Persist(customerId); + await inner.DeleteShoppingCartItem(customerId, shoppingCartItem); + } + + public async Task InsertShoppingCartItem(string customerId, ShoppingCartItem shoppingCartItem) + { + await Persist(customerId); + await inner.InsertShoppingCartItem(customerId, shoppingCartItem); + } + + public async Task UpdateShoppingCartItem(string customerId, ShoppingCartItem shoppingCartItem) + { + await Persist(customerId); + await inner.UpdateShoppingCartItem(customerId, shoppingCartItem); + } + + #endregion +} diff --git a/src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs b/src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs new file mode 100644 index 0000000000..06789c8a52 --- /dev/null +++ b/src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs @@ -0,0 +1,28 @@ +using Grand.Infrastructure; +using Grand.Web.Common.RequestCustomerResolvers; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; + +namespace Grand.Web.Common.Startup; + +/// +/// Saves a guest only once it returns its cookie or something is written to it +/// +public class GuestCustomerStartup : IStartupApplication +{ + public void ConfigureServices(IServiceCollection services, IConfiguration configuration) + { + services.AddTransientGuestCustomers(); + } + + public void Configure(WebApplication application, IWebHostEnvironment webHostEnvironment) + { + } + + //after Grand.Business.Customers (100) registers ICustomerService + public int Priority => 1100; + + public bool BeforeConfigure => true; +} From 7bde4dd023b0c819afecfa09f00e332ba0c01857 Mon Sep 17 00:00:00 2001 From: support Date: Wed, 14 Oct 2026 20:44:19 +0200 Subject: [PATCH 06/15] Register the guest customer decorator after the business services --- .../TransientGuestCustomerServiceTests.cs | 17 +++++++++++++++-- .../GuestCustomerRegistration.cs | 8 +++++--- .../Startup/GuestCustomerStartup.cs | 4 ++-- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs index 41aa903e63..a5735ea398 100644 --- a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs @@ -109,13 +109,26 @@ public void Registration_DecoratesCustomerServiceAndSharesPersistencePerScope() using var provider = services.BuildServiceProvider(new ServiceProviderOptions { ValidateScopes = true }); using var scope = provider.CreateScope(); - Assert.IsInstanceOfType(scope.ServiceProvider.GetRequiredService(), - typeof(TransientGuestCustomerService)); + Assert.AreEqual(typeof(TransientGuestCustomerService), + scope.ServiceProvider.GetRequiredService().GetType()); Assert.AreSame(scope.ServiceProvider.GetRequiredService(), scope.ServiceProvider.GetRequiredService()); Assert.AreEqual(1, services.Count(d => d.ServiceType == typeof(ICustomerService))); } + [TestMethod] + public void Registration_NoCustomerService_Throws() + { + Assert.ThrowsExactly(() => new ServiceCollection().AddTransientGuestCustomers()); + } + + [TestMethod] + public void Startup_RunsInSecondPass() + { + //second pass, after Grand.Business.Customers registers ICustomerService + Assert.IsFalse(new Grand.Web.Common.Startup.GuestCustomerStartup().BeforeConfigure); + } + //any concrete ICustomerService with a parameterless constructor, as the business layer registers one private class FakeCustomerService() : TransientGuestCustomerService(Mock.Of(), Mock.Of()); diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs index e79c696d5b..311bb3d6d7 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs @@ -14,11 +14,13 @@ public static class GuestCustomerRegistration public static IServiceCollection AddTransientGuestCustomers(this IServiceCollection services) { var registered = services.LastOrDefault(d => d.ServiceType == typeof(ICustomerService)); - if (registered?.ImplementationType == null) return services; + if (registered?.ImplementationType == null) + throw new InvalidOperationException( + "The guest customer decorator needs a type-based ICustomerService registration made before it."); var innerType = registered.ImplementationType; services.Remove(registered); - services.Add(new ServiceDescriptor(innerType, innerType, registered.Lifetime)); + services.Add(new ServiceDescriptor(innerType, innerType, ServiceLifetime.Scoped)); services.AddScoped(sp => new GuestCustomerPersistence( (ICustomerService)sp.GetRequiredService(innerType), @@ -26,7 +28,7 @@ public static IServiceCollection AddTransientGuestCustomers(this IServiceCollect sp.GetRequiredService>())); services.Add(new ServiceDescriptor(typeof(ICustomerService), sp => new TransientGuestCustomerService( (ICustomerService)sp.GetRequiredService(innerType), - sp.GetRequiredService()), registered.Lifetime)); + sp.GetRequiredService()), ServiceLifetime.Scoped)); return services; } } diff --git a/src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs b/src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs index 06789c8a52..29d670befe 100644 --- a/src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs +++ b/src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs @@ -21,8 +21,8 @@ public void Configure(WebApplication application, IWebHostEnvironment webHostEnv { } - //after Grand.Business.Customers (100) registers ICustomerService + //second pass (BeforeConfigure false), after Grand.Business.Customers (100) registers ICustomerService public int Priority => 1100; - public bool BeforeConfigure => true; + public bool BeforeConfigure => false; } From 86d85a06bd9fdf193de4aa49468a79a749278f1c Mon Sep 17 00:00:00 2001 From: support Date: Thu, 15 Oct 2026 07:07:44 +0200 Subject: [PATCH 07/15] Save a new guest only when it returns its cookie or is written to --- .../Filters/CustomerActivityAttributeTests.cs | 16 ++- .../WorkContextSetterRequestResolverTests.cs | 99 +++++++++++++++++++ .../Filters/CustomerActivityAttribute.cs | 26 ++--- src/Web/Grand.Web.Common/WorkContextSetter.cs | 53 ++++++++-- 4 files changed, 174 insertions(+), 20 deletions(-) diff --git a/src/Tests/Grand.Web.Common.Tests/Filters/CustomerActivityAttributeTests.cs b/src/Tests/Grand.Web.Common.Tests/Filters/CustomerActivityAttributeTests.cs index 69207f40ff..4b6a307c9c 100644 --- a/src/Tests/Grand.Web.Common.Tests/Filters/CustomerActivityAttributeTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Filters/CustomerActivityAttributeTests.cs @@ -4,6 +4,7 @@ using Grand.Infrastructure; using Grand.SharedKernel.Extensions; using Grand.Web.Common.Filters; +using Grand.Web.Common.RequestCustomerResolvers; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Abstractions; @@ -22,6 +23,7 @@ public class CustomerActivityAttributeTests private Mock _customerService; private Mock _contextAccessor; private Mock _workContext; + private Mock _persistence; [TestInitialize] public void Setup() @@ -30,6 +32,7 @@ public void Setup() DataSettingsManager.Initialize(Path.Combine("", CommonPath.AppData, CommonPath.SettingsFile)); _customerService = new Mock(); _workContext = new Mock(); + _persistence = new Mock(); _contextAccessor = new Mock(); _contextAccessor.SetupGet(c => c.WorkContext).Returns(_workContext.Object); } @@ -47,7 +50,7 @@ private async Task RunGet(Customer current) var executing = new ActionExecutingContext(actionContext, new List(), new Dictionary(), null); var filter = new CustomerActivityAttribute.CustomerActivityFilter(_customerService.Object, - _contextAccessor.Object, new CustomerSettings { StoreLastVisitedPage = true }); + _contextAccessor.Object, new CustomerSettings { StoreLastVisitedPage = true }, _persistence.Object); await filter.OnActionExecutionAsync(executing, () => Task.FromResult(new ActionExecutedContext(actionContext, new List(), null))); @@ -72,4 +75,15 @@ public async Task Guest_ActivityWritten() _customerService.Verify(c => c.UpdateCustomerField(It.IsAny(), It.IsAny>>(), It.IsAny()), Times.AtLeastOnce); } + + [TestMethod] + public async Task TransientGuest_NothingWritten() + { + _persistence.Setup(p => p.IsTransient("unsaved")).Returns(true); + + await RunGet(new Customer { Id = "unsaved", Active = true }); + + _customerService.Verify(c => c.UpdateCustomerField(It.IsAny(), + It.IsAny>>(), It.IsAny()), Times.Never); + } } diff --git a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs index dd69af8f1c..6fe9b6150b 100644 --- a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs @@ -31,6 +31,7 @@ public class WorkContextSetterRequestResolverTests private Mock _apiAuthentication; private Mock _userAgent; private Mock _accessor; + private Mock _persistence; [TestInitialize] public void Setup() @@ -42,6 +43,7 @@ public void Setup() _userAgent = new Mock(); _httpContext = new DefaultHttpContext(); _accessor = new Mock(); + _persistence = new Mock(); _accessor.Setup(a => a.HttpContext).Returns(_httpContext); } @@ -56,6 +58,16 @@ private TestableSetter CreateSetter(params IRequestCustomerResolver[] resolvers) new TaxSettings(), new AppConfig(), services.BuildServiceProvider()); } + private TestableSetter CreateSetterWithPersistence() + { + var services = new ServiceCollection(); + services.AddSingleton(_persistence.Object); + return new TestableSetter(_accessor.Object, _authentication.Object, Mock.Of(), + _customerService.Object, Mock.Of(), + Mock.Of(), Mock.Of(), Mock.Of(), + new TaxSettings(), new AppConfig(), services.BuildServiceProvider()); + } + private static Mock ResolverMock(int order, Customer result) { var mock = new Mock(); @@ -160,6 +172,93 @@ public async Task BuiltIns_NothingMatches_CreatesNewGuest() _authentication.Verify(a => a.SetCustomerGuid(customer.CustomerGuid), Times.Once); } + [TestMethod] + public async Task Get_NoCookie_TransientGuestNotInserted() + { + _httpContext.Request.Method = HttpMethods.Get; + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store { Id = "s1" }); + + Assert.AreEqual("s1", customer.StoreId); + _persistence.Verify(p => p.SetTransient(customer), Times.Once); + _customerService.Verify(c => c.InsertGuestCustomer(It.IsAny()), Times.Never); + _authentication.Verify(a => a.SetCustomerGuid(customer.CustomerGuid), Times.Once); + } + + [TestMethod] + public async Task Post_NoCookie_GuestInserted() + { + _httpContext.Request.Method = HttpMethods.Post; + _customerService.Setup(c => c.InsertGuestCustomer(It.IsAny())).ReturnsAsync((Customer c) => c); + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store { Id = "s1" }); + + _customerService.Verify(c => c.InsertGuestCustomer(customer), Times.Once); + _persistence.Verify(p => p.SetTransient(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task ReturnedCookieOfUnsavedGuest_Promoted() + { + _httpContext.Request.Method = HttpMethods.Get; + var guid = Guid.NewGuid(); + _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(guid.ToString()); + _customerService.Setup(c => c.GetCustomerByGuid(guid)).ReturnsAsync((Customer)null); + var promoted = new Customer { Id = "promoted", CustomerGuid = guid, Active = true }; + _persistence.Setup(p => p.Promote(It.Is(c => c.CustomerGuid == guid && c.StoreId == "s1"))) + .ReturnsAsync(promoted); + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store { Id = "s1" }); + + Assert.AreSame(promoted, customer); + _authentication.Verify(a => a.SetCustomerGuid(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task CookieOfExistingCustomer_NeverPromoted_NewGuestIssued() + { + //an administrator's GUID planted in the guest cookie + _httpContext.Request.Method = HttpMethods.Get; + var adminGuid = Guid.NewGuid(); + _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(adminGuid.ToString()); + _customerService.Setup(c => c.GetCustomerByGuid(adminGuid)) + .ReturnsAsync(new Customer { Id = "admin", CustomerGuid = adminGuid, Active = true }); + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store { Id = "s1" }); + + Assert.AreNotEqual("admin", customer.Id); + Assert.AreNotEqual(adminGuid, customer.CustomerGuid); + _persistence.Verify(p => p.Promote(It.IsAny()), Times.Never); + _authentication.Verify(a => a.SetCustomerGuid(customer.CustomerGuid), Times.Once); + } + + [TestMethod] + public async Task PromotionRefused_NewGuestIssued() + { + _httpContext.Request.Method = HttpMethods.Get; + var guid = Guid.NewGuid(); + _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(guid.ToString()); + _persistence.Setup(p => p.Promote(It.IsAny())).ReturnsAsync((Customer)null); + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store { Id = "s1" }); + + Assert.AreNotEqual(guid, customer.CustomerGuid); + _persistence.Verify(p => p.SetTransient(customer), Times.Once); + } + + [TestMethod] + public async Task BackgroundTask_NeverTransient() + { + _accessor.Setup(a => a.HttpContext).Returns((HttpContext)null); + _customerService.Setup(c => c.GetCustomerBySystemName(SystemCustomerNames.BackgroundTask)) + .ReturnsAsync(new Customer { Id = "task", IsSystemAccount = true }); + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store()); + + Assert.AreEqual("task", customer.Id); + _persistence.Verify(p => p.SetTransient(It.IsAny()), Times.Never); + } + [TestMethod] public async Task BackgroundTask_NoHttpContext_ResolversAreNotAsked() { diff --git a/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs b/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs index f4f72f3093..143ba5286e 100644 --- a/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs +++ b/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs @@ -2,6 +2,7 @@ using Grand.Data; using Grand.Domain.Customers; using Grand.Infrastructure; +using Grand.Web.Common.RequestCustomerResolvers; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; @@ -29,7 +30,8 @@ public CustomerActivityAttribute() : base(typeof(CustomerActivityFilter)) internal class CustomerActivityFilter( ICustomerService customerService, IContextAccessor contextAccessor, - CustomerSettings customerSettings) : IAsyncActionFilter + CustomerSettings customerSettings, + IGuestCustomerPersistence guestCustomerPersistence) : IAsyncActionFilter { #region Methods @@ -53,13 +55,15 @@ public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionE if (!customerSettings.StoreLastVisitedPage) return; - //system accounts (SearchEngine, Anonymous) are shared by every request they serve - if (contextAccessor.WorkContext.CurrentCustomer.IsSystemAccount) + //system accounts are shared by every request they serve; an unsaved guest is saved by a write, + //which activity tracking must not trigger + var customer = contextAccessor.WorkContext.CurrentCustomer; + if (customer.IsSystemAccount || guestCustomerPersistence.IsTransient(customer.Id)) return; //update last activity date - if (contextAccessor.WorkContext.CurrentCustomer.LastActivityDateUtc.AddMinutes(3.0) < DateTime.UtcNow) - await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCustomer, x => x.LastActivityDateUtc, + if (customer.LastActivityDateUtc.AddMinutes(3.0) < DateTime.UtcNow) + await customerService.UpdateCustomerField(customer, x => x.LastActivityDateUtc, DateTime.UtcNow); //get current IP address @@ -67,11 +71,11 @@ await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCus //update customer's IP address if (!string.IsNullOrEmpty(currentIpAddress) && - !currentIpAddress.Equals(contextAccessor.WorkContext.CurrentCustomer.LastIpAddress, + !currentIpAddress.Equals(customer.LastIpAddress, StringComparison.OrdinalIgnoreCase)) { - contextAccessor.WorkContext.CurrentCustomer.LastIpAddress = currentIpAddress; - await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCustomer, x => x.LastIpAddress, + customer.LastIpAddress = currentIpAddress; + await customerService.UpdateCustomerField(customer, x => x.LastIpAddress, currentIpAddress); } @@ -81,10 +85,10 @@ await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCus return; //save new one if don't match - if (!pageUrl.Equals(contextAccessor.WorkContext.CurrentCustomer.LastVisitedPage, StringComparison.OrdinalIgnoreCase)) + if (!pageUrl.Equals(customer.LastVisitedPage, StringComparison.OrdinalIgnoreCase)) { - contextAccessor.WorkContext.CurrentCustomer.LastVisitedPage = pageUrl; - await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCustomer, x => x.LastVisitedPage, pageUrl); + customer.LastVisitedPage = pageUrl; + await customerService.UpdateCustomerField(customer, x => x.LastVisitedPage, pageUrl); } } diff --git a/src/Web/Grand.Web.Common/WorkContextSetter.cs b/src/Web/Grand.Web.Common/WorkContextSetter.cs index 6f4777d571..b3cb972d8b 100644 --- a/src/Web/Grand.Web.Common/WorkContextSetter.cs +++ b/src/Web/Grand.Web.Common/WorkContextSetter.cs @@ -217,6 +217,49 @@ private async Task GetBackgroundTaskCustomer() } private async Task CreateCustomerGuest(Store store) + { + //resolved here, not injected: it lives in the request scope next to the decorated customer service + var persistence = _serviceProvider.GetService(); + + //a browser returning the cookie of a guest that was never saved: save it now under that GUID + var returningGuid = persistence != null ? await ReturningGuestGuid() : null; + if (returningGuid.HasValue) + { + var promoted = await persistence.Promote(BuildGuest(store, returningGuid.Value)); + if (promoted != null) return promoted; + } + + var customer = BuildGuest(store, Guid.NewGuid()); + if (persistence != null && IsSafeRequest()) + //saved on its first write, or by the next request that returns the cookie + persistence.SetTransient(customer); + else + customer = await _customerService.InsertGuestCustomer(customer); + + await _authenticationService.SetCustomerGuid(customer.CustomerGuid); + return customer; + } + + /// + /// The guest cookie's GUID when no customer has it; an existing customer (registered, administrator, system, + /// deleted) is never taken over + /// + private async Task ReturningGuestGuid() + { + var cookie = await _authenticationService.GetCustomerGuid(); + if (!GuestCustomerRules.TryParseCookie(cookie, out var customerGuid)) return null; + + return await _customerService.GetCustomerByGuid(customerGuid) == null ? customerGuid : null; + } + + private bool IsSafeRequest() + { + var method = _httpContextAccessor.HttpContext?.Request.Method; + return method != null && (HttpMethods.IsGet(method) || HttpMethods.IsHead(method) || + HttpMethods.IsOptions(method)); + } + + private Customer BuildGuest(Store store, Guid customerGuid) { var userFields = new List(); @@ -234,20 +277,14 @@ private async Task CreateCustomerGuest(Store store) StoreId = store.Id }); - var customer = new Customer { - CustomerGuid = Guid.NewGuid(), + return new Customer { + CustomerGuid = customerGuid, Active = true, StoreId = store.Id, LastActivityDateUtc = DateTime.UtcNow, LastIpAddress = _httpContextAccessor?.HttpContext?.Connection?.RemoteIpAddress?.ToString(), UserFields = userFields }; - - customer = await _customerService.InsertGuestCustomer(customer); - - //set customer cookie - await _authenticationService.SetCustomerGuid(customer.CustomerGuid); - return customer; } /// From 4b3e5d22a5717528f4bed9806025c9b10e70d2e1 Mon Sep 17 00:00:00 2001 From: support Date: Thu, 15 Oct 2026 07:11:30 +0200 Subject: [PATCH 08/15] Make the customer GUID index unique --- .../Services/InstallationService.cs | 2 +- .../2.5/MigrationCustomerGuidUniqueIndex.cs | 54 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs diff --git a/src/Modules/Grand.Module.Installer/Services/InstallationService.cs b/src/Modules/Grand.Module.Installer/Services/InstallationService.cs index e61e4557ba..b7813a5c39 100644 --- a/src/Modules/Grand.Module.Installer/Services/InstallationService.cs +++ b/src/Modules/Grand.Module.Installer/Services/InstallationService.cs @@ -482,7 +482,7 @@ await dbContext.CreateIndex(_customerRepository, await dbContext.CreateIndex(_customerRepository, OrderBuilder.Create().Ascending(x => x.LastActivityDateUtc), "LastActivityDateUtc_1"); await dbContext.CreateIndex(_customerRepository, OrderBuilder.Create().Ascending(x => x.CustomerGuid), - "CustomerGuid_1"); + "CustomerGuid_1", true); await dbContext.CreateIndex(_customerRepository, OrderBuilder.Create().Ascending(x => x.Email), "Email_1"); //compound lookup indexes supporting per-store customer identity (uniqueness of Email/Username + StoreId diff --git a/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs new file mode 100644 index 0000000000..04d5be1251 --- /dev/null +++ b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs @@ -0,0 +1,54 @@ +using Grand.Data; +using Grand.Domain.Customers; +using Grand.Infrastructure.Migrations; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Grand.Module.Migration.Migrations._2._5; + +public class MigrationCustomerGuidUniqueIndex : IMigration +{ + private const string IndexName = "CustomerGuid_1"; + + public int Priority => 0; + public DbVersion Version => new(2, 5); + public Guid Identity => new("f0c98624-441d-42b6-95ae-429bfb64d266"); + public string Name => "Unique index on Customer.CustomerGuid"; + + public bool UpgradeProcess(IServiceProvider serviceProvider) + { + var repository = serviceProvider.GetRequiredService>(); + var databaseContext = serviceProvider.GetRequiredService(); + var logService = serviceProvider.GetRequiredService>(); + + try + { + var duplicates = repository.Table + .GroupBy(x => x.CustomerGuid) + .Where(x => x.Count() > 1) + .Select(x => x.Key) + .Take(20) + .ToList(); + + //duplicates must not stop the upgrade - keep the non-unique index for the lookups and report them + if (duplicates.Count != 0) + { + logService.LogError( + "The index {IndexName} stays non-unique - these customer GUIDs are used more than once: {Guids}. Remove the duplicates and recreate the index as unique", + IndexName, string.Join(", ", duplicates)); + return true; + } + + databaseContext.DeleteIndex(repository, IndexName).GetAwaiter().GetResult(); + databaseContext.CreateIndex(repository, + OrderBuilder.Create().Ascending(x => x.CustomerGuid), IndexName, true) + .GetAwaiter().GetResult(); + } + catch (Exception ex) + { + logService.LogError(ex, "UpgradeProcess - CustomerGuidUniqueIndex (2.5)"); + } + + return true; + } +} From 0bb0a8ebcf52819b826e4bc4524572383345ba13 Mon Sep 17 00:00:00 2001 From: support Date: Thu, 15 Oct 2026 07:12:44 +0200 Subject: [PATCH 09/15] Mark store selector links nofollow --- .../Grand.Web/Views/Shared/Partials/Selector_Currency.cshtml | 2 +- .../Grand.Web/Views/Shared/Partials/Selector_Language.cshtml | 2 +- .../Grand.Web/Views/Shared/Partials/Selector_TaxType.cshtml | 4 ++-- src/Web/Grand.Web/Views/Shared/Partials/Selector_Theme.cshtml | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Currency.cshtml b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Currency.cshtml index e10689880a..c715f0ce8c 100644 --- a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Currency.cshtml +++ b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Currency.cshtml @@ -38,7 +38,7 @@ @foreach (var item in currencies) { var className = item.Selected ? "active" : ""; -
  • @item.Text
    • +
  • @item.Text
    • } diff --git a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Language.cshtml b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Language.cshtml index a776ea7c09..f9be99364b 100644 --- a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Language.cshtml +++ b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Language.cshtml @@ -22,7 +22,7 @@ @foreach (var lang in availableLanguages) { var className = lang.Id == contextAccessor.WorkContext.WorkingLanguage.Id ? "active" : ""; -
  • +
  • @lang.Name
    • } diff --git a/src/Web/Grand.Web/Views/Shared/Partials/Selector_TaxType.cshtml b/src/Web/Grand.Web/Views/Shared/Partials/Selector_TaxType.cshtml index 51075fb068..62e8562b0f 100644 --- a/src/Web/Grand.Web/Views/Shared/Partials/Selector_TaxType.cshtml +++ b/src/Web/Grand.Web/Views/Shared/Partials/Selector_TaxType.cshtml @@ -19,10 +19,10 @@
    diff --git a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Theme.cshtml b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Theme.cshtml index 78111a5c88..b3449d9c37 100644 --- a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Theme.cshtml +++ b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Theme.cshtml @@ -25,7 +25,7 @@ @foreach (var theme in availablThemes) { var className = theme.Name == themeContext?.GetCurrentTheme() ? "active" : ""; -
  • +
  • @(theme.Title ?? theme.Name)
    • } From 223c2880b56c4a3477384455a86bf2b5fd9db78f Mon Sep 17 00:00:00 2001 From: support Date: Thu, 15 Oct 2026 07:16:12 +0200 Subject: [PATCH 10/15] Keep the customer GUID index when the unique index cannot be created --- .../2.5/MigrationCustomerGuidUniqueIndex.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs index 04d5be1251..6bf3958572 100644 --- a/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs +++ b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs @@ -21,6 +21,16 @@ public bool UpgradeProcess(IServiceProvider serviceProvider) var databaseContext = serviceProvider.GetRequiredService(); var logService = serviceProvider.GetRequiredService>(); + //LiteDB names indexes by field, so the named drop and recreate below do not apply to it + //and EnsureIndex never alters an existing index - new installs create the index unique + if (DataSettingsManager.Instance.LoadSettings().DbProvider != DbProvider.MongoDB) + { + logService.LogInformation( + "The index {IndexName} stays non-unique on this database provider (new installs create it unique)", + IndexName); + return true; + } + try { var duplicates = repository.Table @@ -43,6 +53,12 @@ public bool UpgradeProcess(IServiceProvider serviceProvider) databaseContext.CreateIndex(repository, OrderBuilder.Create().Ascending(x => x.CustomerGuid), IndexName, true) .GetAwaiter().GetResult(); + + //when the unique index exists this is a rejected, swallowed no-op; when the unique create failed + //it restores the lookup index + databaseContext.CreateIndex(repository, + OrderBuilder.Create().Ascending(x => x.CustomerGuid), IndexName) + .GetAwaiter().GetResult(); } catch (Exception ex) { From daae76d3c0ea084ce689aad2d55cf1ea9b4c2a67 Mon Sep 17 00:00:00 2001 From: support Date: Thu, 15 Oct 2026 07:42:15 +0200 Subject: [PATCH 11/15] Give the unsaved guest its Guests group --- .../Services/CustomerService.cs | 3 ++- .../Services/CustomerServiceTests.cs | 13 ++++++++++ .../GuestCustomerPersistenceTests.cs | 24 +++++++++++++++---- .../WorkContextSetterRequestResolverTests.cs | 1 + .../GuestCustomerPersistence.cs | 7 +++++- .../IGuestCustomerPersistence.cs | 2 +- src/Web/Grand.Web.Common/WorkContextSetter.cs | 2 +- 7 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/Business/Grand.Business.Customers/Services/CustomerService.cs b/src/Business/Grand.Business.Customers/Services/CustomerService.cs index 143fe78201..5ab0d10a1a 100644 --- a/src/Business/Grand.Business.Customers/Services/CustomerService.cs +++ b/src/Business/Grand.Business.Customers/Services/CustomerService.cs @@ -327,7 +327,8 @@ public virtual async Task InsertGuestCustomer(Customer customer) { SystemName = SystemCustomerGroupNames.Guests }); if (guestGroup == null) throw new GrandException("'Guests' group could not be loaded"); - customer.Groups.Add(guestGroup.Id); + if (!customer.Groups.Contains(guestGroup.Id)) + customer.Groups.Add(guestGroup.Id); await _customerRepository.InsertAsync(customer); diff --git a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs index ee12c930ae..c592b7e3c5 100644 --- a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs +++ b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs @@ -333,6 +333,19 @@ public async Task InsertGuestCustomerTest() Assert.IsTrue(_repository.Table.Any(x => x.StoreId == "1")); } + [TestMethod] + public async Task InsertGuestCustomer_GroupAlreadyPresent_AddedOnce() + { + _mediatorMock.Setup(x => x.Send(It.IsAny(), default)) + .Returns(Task.FromResult(new CustomerGroup { Id = "guests" })); + var customer = new Customer { CustomerGuid = Guid.NewGuid(), Active = true }; + customer.Groups.Add("guests"); + + await _customerService.InsertGuestCustomer(customer); + + CollectionAssert.AreEqual(new[] { "guests" }, customer.Groups.ToArray()); + } + [TestMethod] public async Task InsertCustomerTest() { diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs index 5b91e5c280..8b733d6998 100644 --- a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs @@ -26,11 +26,11 @@ public void Setup() } [TestMethod] - public void IsTransient_OnlyForTheTransientId() + public async Task IsTransient_OnlyForTheTransientId() { Assert.IsFalse(_persistence.IsTransient("g1")); - _persistence.SetTransient(new Customer { Id = "g1" }); + await _persistence.SetTransient(new Customer { Id = "g1" }); Assert.IsTrue(_persistence.IsTransient("g1")); Assert.IsFalse(_persistence.IsTransient("other")); @@ -38,11 +38,27 @@ public void IsTransient_OnlyForTheTransientId() Assert.IsFalse(_persistence.IsTransient("")); } + [TestMethod] + public async Task SetTransient_AddsGuestsGroup() + { + _groups.Setup(g => g.GetCustomerGroupBySystemName(SystemCustomerGroupNames.Guests)) + .ReturnsAsync(new CustomerGroup { Id = "guests" }); + var fresh = new Customer { Id = "g1" }; + var already = new Customer { Id = "g2" }; + already.Groups.Add("guests"); + + await _persistence.SetTransient(fresh); + await _persistence.SetTransient(already); + + CollectionAssert.AreEqual(new[] { "guests" }, fresh.Groups.ToArray()); + CollectionAssert.AreEqual(new[] { "guests" }, already.Groups.ToArray()); + } + [TestMethod] public async Task EnsurePersisted_InsertsOnceAndClearsTransient() { var guest = new Customer { Id = "g1" }; - _persistence.SetTransient(guest); + await _persistence.SetTransient(guest); await _persistence.EnsurePersisted(); await _persistence.EnsurePersisted(); @@ -65,7 +81,7 @@ public async Task EnsurePersisted_ConcurrentCalls_InsertOnce() { var gate = new TaskCompletionSource(); _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())).Returns(gate.Task); - _persistence.SetTransient(new Customer { Id = "g1" }); + await _persistence.SetTransient(new Customer { Id = "g1" }); var first = _persistence.EnsurePersisted(); var second = _persistence.EnsurePersisted(); diff --git a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs index 6fe9b6150b..2bd7d91864 100644 --- a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs @@ -44,6 +44,7 @@ public void Setup() _httpContext = new DefaultHttpContext(); _accessor = new Mock(); _persistence = new Mock(); + _persistence.Setup(p => p.SetTransient(It.IsAny())).Returns(Task.CompletedTask); _accessor.Setup(a => a.HttpContext).Returns(_httpContext); } diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs index 79c11eded5..5c0778437e 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs @@ -17,8 +17,13 @@ public class GuestCustomerPersistence( public Customer Transient { get; private set; } - public void SetTransient(Customer customer) + public async Task SetTransient(Customer customer) { + //the Guests group is normally added on insert; permission checks read it from the unsaved customer + var guestsGroup = await groupService.GetCustomerGroupBySystemName(SystemCustomerGroupNames.Guests); + if (guestsGroup != null && !customer.Groups.Contains(guestsGroup.Id)) + customer.Groups.Add(guestsGroup.Id); + Transient = customer; _persisting = null; } diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/IGuestCustomerPersistence.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/IGuestCustomerPersistence.cs index daa5b7cc94..d6d1f9ffc7 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/IGuestCustomerPersistence.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/IGuestCustomerPersistence.cs @@ -13,7 +13,7 @@ public interface IGuestCustomerPersistence ///
    Customer Transient { get; } - void SetTransient(Customer customer); + Task SetTransient(Customer customer); bool IsTransient(string customerId); diff --git a/src/Web/Grand.Web.Common/WorkContextSetter.cs b/src/Web/Grand.Web.Common/WorkContextSetter.cs index b3cb972d8b..96e75bc8d6 100644 --- a/src/Web/Grand.Web.Common/WorkContextSetter.cs +++ b/src/Web/Grand.Web.Common/WorkContextSetter.cs @@ -232,7 +232,7 @@ private async Task CreateCustomerGuest(Store store) var customer = BuildGuest(store, Guid.NewGuid()); if (persistence != null && IsSafeRequest()) //saved on its first write, or by the next request that returns the cookie - persistence.SetTransient(customer); + await persistence.SetTransient(customer); else customer = await _customerService.InsertGuestCustomer(customer); From edd374af8822b028faf10425fd2d8c70c6d2571a Mon Sep 17 00:00:00 2001 From: support Date: Thu, 15 Oct 2026 07:58:57 +0200 Subject: [PATCH 12/15] Treat only named crawlers as search engines --- .../RequestCustomerResolversTests.cs | 20 ++++++++++++-- .../Security/Oidc/OidcWorkContextTests.cs | 2 +- .../UserAgent/UserAgentClassifierTests.cs | 26 +++++++++---------- .../WorkContextSetterRequestResolverTests.cs | 21 +++++++++++++-- .../SearchEngineCustomerResolver.cs | 8 +++++- .../UserAgent/IUserAgentClassifier.cs | 2 +- .../UserAgent/UserAgentClassifier.cs | 5 ++-- 7 files changed, 61 insertions(+), 23 deletions(-) diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs index 8065ffd3ed..bd2683160a 100644 --- a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs @@ -134,7 +134,8 @@ public async Task SearchEngine_NotACrawler_ReturnsNull() { var classifier = new Mock(); classifier.SetupGet(c => c.IsCrawler).Returns(false); - var resolver = new SearchEngineCustomerResolver(classifier.Object, Mock.Of()); + var resolver = new SearchEngineCustomerResolver(classifier.Object, Mock.Of(), + AccessorWith(new DefaultHttpContext())); Assert.IsNull(await resolver.Resolve()); Assert.AreEqual(RequestCustomerResolverOrder.SearchEngine, resolver.Order); @@ -149,11 +150,26 @@ public async Task SearchEngine_Crawler_ReturnsSearchEngineCustomer() customers.Setup(c => c.GetCustomerBySystemName(SystemCustomerNames.SearchEngine)) .ReturnsAsync(new Customer { Id = "crawler" }); - var resolver = new SearchEngineCustomerResolver(classifier.Object, customers.Object); + var resolver = new SearchEngineCustomerResolver(classifier.Object, customers.Object, + AccessorWith(new DefaultHttpContext())); Assert.AreEqual("crawler", (await resolver.Resolve()).Id); } + [TestMethod] + public async Task SearchEngine_AuthorizationHeader_ReturnsNull() + { + var classifier = new Mock(); + classifier.SetupGet(c => c.IsCrawler).Returns(true); + var customers = new Mock(); + var context = new DefaultHttpContext(); + context.Request.Headers.Authorization = "Bearer token"; + var resolver = new SearchEngineCustomerResolver(classifier.Object, customers.Object, AccessorWith(context)); + + Assert.IsNull(await resolver.Resolve()); + customers.Verify(c => c.GetCustomerBySystemName(It.IsAny()), Times.Never); + } + [TestMethod] public async Task ApiUser_NoApiCustomer_ReturnsNull() { diff --git a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs index 505ee6a7e2..1257bec63a 100644 --- a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs @@ -91,7 +91,7 @@ private TestableSetter CreateSetter() services.AddSingleton( new GuestCookieCustomerResolver(_authentication.Object, _customerService.Object, Mock.Of())); services.AddSingleton( - new SearchEngineCustomerResolver(Mock.Of(), _customerService.Object)); + new SearchEngineCustomerResolver(Mock.Of(), _customerService.Object, _accessor.Object)); services.AddSingleton( new AllowAnonymousCustomerResolver(_accessor.Object, _customerService.Object)); return new TestableSetter(_accessor.Object, _authentication.Object, Mock.Of(), diff --git a/src/Tests/Grand.Web.Common.Tests/UserAgent/UserAgentClassifierTests.cs b/src/Tests/Grand.Web.Common.Tests/UserAgent/UserAgentClassifierTests.cs index beab2d5aca..a46ad8d821 100644 --- a/src/Tests/Grand.Web.Common.Tests/UserAgent/UserAgentClassifierTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/UserAgent/UserAgentClassifierTests.cs @@ -29,9 +29,20 @@ private static UserAgentClassifier ClassifierFor(string userAgent) [DataRow("Mozilla/5.0 (compatible; Baiduspider/2.0; +http://www.baidu.com/search/spider.html)")] [DataRow("facebookexternalhit/1.1 (+http://www.facebook.com/externalhit_uatext.php)")] [DataRow("meta-externalagent/1.1 (+https://developers.facebook.com/docs/sharing/webmasters/crawler)")] - [DataRow("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/120.0.0.0 Safari/537.36")] [DataRow("Mozilla/5.0 (compatible; crawler)")] [DataRow("Mozilla/5.0 (compatible; SomeSpider/1.0)")] + public void IsCrawler_NamedCrawlers_True(string userAgent) + { + Assert.IsTrue(ClassifierFor(userAgent).IsCrawler); + } + + [TestMethod] + [DataRow("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36")] + [DataRow("Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Safari/605.1.15")] + [DataRow("Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Mobile/15E148 Safari/604.1")] + [DataRow("Mozilla/5.0 (Linux; Android 10; CUBOT X30) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36")] + [DataRow("Mozilla/5.0 (X11; Linux x86_64; rv:121.0) Gecko/20100101 Firefox/121.0")] + [DataRow("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/120.0.0.0 Safari/537.36")] [DataRow("Scrapy/2.11.0 (+https://scrapy.org)")] [DataRow("curl/8.4.0")] [DataRow("Wget/1.21")] @@ -44,18 +55,7 @@ private static UserAgentClassifier ClassifierFor(string userAgent) [DataRow("node-fetch/1.0 (+https://github.com/bitinn/node-fetch)")] [DataRow("")] [DataRow(null)] - public void IsCrawler_BotsAndHttpClients_True(string userAgent) - { - Assert.IsTrue(ClassifierFor(userAgent).IsCrawler); - } - - [TestMethod] - [DataRow("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36")] - [DataRow("Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Safari/605.1.15")] - [DataRow("Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Mobile/15E148 Safari/604.1")] - [DataRow("Mozilla/5.0 (Linux; Android 10; CUBOT X30) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36")] - [DataRow("Mozilla/5.0 (X11; Linux x86_64; rv:121.0) Gecko/20100101 Firefox/121.0")] - public void IsCrawler_Browsers_False(string userAgent) + public void IsCrawler_BrowsersAndHttpClients_False(string userAgent) { Assert.IsFalse(ClassifierFor(userAgent).IsCrawler); } diff --git a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs index 2bd7d91864..42297678e8 100644 --- a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs @@ -78,11 +78,11 @@ private static Mock ResolverMock(int order, Customer r } //registered in a scrambled order on purpose: the setter must sort by Order - private IRequestCustomerResolver[] BuiltIns() + private IRequestCustomerResolver[] BuiltIns(IUserAgentClassifier classifier = null) { return [ new ApiUserCustomerResolver(_apiAuthentication.Object, _accessor.Object, _customerService.Object), - new SearchEngineCustomerResolver(_userAgent.Object, _customerService.Object), + new SearchEngineCustomerResolver(classifier ?? _userAgent.Object, _customerService.Object, _accessor.Object), new GuestCookieCustomerResolver(_authentication.Object, _customerService.Object, _groupService.Object), new CookieCustomerResolver(_authentication.Object), new AllowAnonymousCustomerResolver(_accessor.Object, _customerService.Object) @@ -133,6 +133,23 @@ public async Task BuiltIns_GuestCookie_BeatsSearchEngine() Assert.AreEqual("guest", customer.Id); } + [TestMethod] + [DataRow("python-requests/2.31.0")] + [DataRow("")] + public async Task BuiltIns_ApiClientWithRealClassifier_ResolvesToApiCustomer(string userAgent) + { + _httpContext.Request.Headers.UserAgent = userAgent; + _httpContext.Request.Headers.Authorization = "Bearer token"; + _customerService.Setup(c => c.GetCustomerBySystemName(SystemCustomerNames.SearchEngine)) + .ReturnsAsync(new Customer { Id = "crawler", IsSystemAccount = true }); + _apiAuthentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync(new Customer { Id = "api", Active = true }); + + var customer = await CreateSetter(BuiltIns(new UserAgentClassifier(_accessor.Object))) + .CurrentCustomerForTest(new Store()); + + Assert.AreEqual("api", customer.Id); + } + [TestMethod] public async Task BuiltIns_SearchEngine_BeatsApiUser() { diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs index 176b887048..9d68f737e5 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs @@ -1,6 +1,7 @@ using Grand.Business.Core.Interfaces.Customers; using Grand.Domain.Customers; using Grand.Web.Common.UserAgent; +using Microsoft.AspNetCore.Http; namespace Grand.Web.Common.RequestCustomerResolvers; @@ -9,12 +10,17 @@ namespace Grand.Web.Common.RequestCustomerResolvers; /// public class SearchEngineCustomerResolver( IUserAgentClassifier userAgentClassifier, - ICustomerService customerService) : IRequestCustomerResolver + ICustomerService customerService, + IHttpContextAccessor httpContextAccessor) : IRequestCustomerResolver { public int Order => RequestCustomerResolverOrder.SearchEngine; public async Task Resolve() { + //an authenticated API client is never treated as a crawler + var request = httpContextAccessor.HttpContext?.Request; + if (request != null && !string.IsNullOrEmpty(request.Headers.Authorization.ToString())) return null; + if (!userAgentClassifier.IsCrawler) return null; return await customerService.GetCustomerBySystemName(SystemCustomerNames.SearchEngine); diff --git a/src/Web/Grand.Web.Common/UserAgent/IUserAgentClassifier.cs b/src/Web/Grand.Web.Common/UserAgent/IUserAgentClassifier.cs index 9a1a6d1d9b..bdbda0dc70 100644 --- a/src/Web/Grand.Web.Common/UserAgent/IUserAgentClassifier.cs +++ b/src/Web/Grand.Web.Common/UserAgent/IUserAgentClassifier.cs @@ -6,7 +6,7 @@ namespace Grand.Web.Common.UserAgent; public interface IUserAgentClassifier { /// - /// A crawler, a scraper or a plain HTTP client - including a missing User-Agent + /// A crawler identified by its User-Agent. A missing User-Agent or a plain HTTP client is not a crawler /// bool IsCrawler { get; } diff --git a/src/Web/Grand.Web.Common/UserAgent/UserAgentClassifier.cs b/src/Web/Grand.Web.Common/UserAgent/UserAgentClassifier.cs index 5096c70815..920e2024ec 100644 --- a/src/Web/Grand.Web.Common/UserAgent/UserAgentClassifier.cs +++ b/src/Web/Grand.Web.Common/UserAgent/UserAgentClassifier.cs @@ -13,8 +13,7 @@ public class UserAgentClassifier(IHttpContextAccessor httpContextAccessor) : IUs //"bot" not preceded by "cu": CUBOT is an Android phone brand private static readonly Regex CrawlerPattern = new( - "(? _isCrawler ??= Classify(userAgent => - string.IsNullOrWhiteSpace(userAgent) || CrawlerPattern.IsMatch(userAgent)); + !string.IsNullOrWhiteSpace(userAgent) && CrawlerPattern.IsMatch(userAgent)); public bool IsMobileOrTablet => _isMobileOrTablet ??= Classify(userAgent => !string.IsNullOrWhiteSpace(userAgent) && MobileOrTabletPattern.IsMatch(userAgent)); From 234e4cb7f5be60ef43ea9dd0484afd83495715f9 Mon Sep 17 00:00:00 2001 From: support Date: Thu, 15 Oct 2026 07:59:50 +0200 Subject: [PATCH 13/15] Skip recently viewed products for an unsaved guest --- .../ProductControllerPreviewTests.cs | 33 +++++++++++++++++-- .../Controllers/ProductController.cs | 16 ++++++--- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/src/Tests/Grand.Web.Tests/Controllers/ProductControllerPreviewTests.cs b/src/Tests/Grand.Web.Tests/Controllers/ProductControllerPreviewTests.cs index 40fa9564e7..6c1854c494 100644 --- a/src/Tests/Grand.Web.Tests/Controllers/ProductControllerPreviewTests.cs +++ b/src/Tests/Grand.Web.Tests/Controllers/ProductControllerPreviewTests.cs @@ -9,6 +9,7 @@ using Grand.Domain.Permissions; using Grand.Infrastructure; using Grand.Mediator; +using Grand.Web.Common.RequestCustomerResolvers; using Grand.Web.Controllers; using Grand.Web.Features.Models.Products; using Grand.Web.Models.Catalog; @@ -33,6 +34,8 @@ public class ProductControllerPreviewTests private Mock _mediatorMock; private Mock _permissionServiceMock; private Mock _productServiceMock; + private Mock _guestPersistenceMock; + private Mock _recentlyViewedMock; [TestInitialize] public void Init() @@ -45,6 +48,8 @@ public void Init() _aclServiceMock = new Mock(); _permissionServiceMock = new Mock(); _mediatorMock = new Mock(); + _guestPersistenceMock = new Mock(); + _recentlyViewedMock = new Mock(); var workContextMock = new Mock(); workContextMock.Setup(x => x.CurrentCustomer).Returns(_customer); @@ -67,13 +72,14 @@ public void Init() _productServiceMock.Object, contextAccessorMock.Object, new Mock().Object, - new Mock().Object, + _recentlyViewedMock.Object, new Mock().Object, _aclServiceMock.Object, _permissionServiceMock.Object, _mediatorMock.Object, new CatalogSettings { AllowViewUnpublishedProductPage = false }, - new CaptchaSettings()) { + new CaptchaSettings(), + _guestPersistenceMock.Object) { ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext { Request = { Method = "GET" } } } @@ -137,4 +143,27 @@ public async Task ProductDetails_AvailabilityEnded_CatalogManagerCanPreview() Assert.IsInstanceOfType(result.Result); } + + [TestMethod] + public async Task ProductDetails_UnsavedGuest_DoesNotRecordRecentlyViewed() + { + GivenProduct(new Product { Id = "p1", Published = true, VisibleIndividually = true }); + _guestPersistenceMock.Setup(x => x.IsTransient(_customer.Id)).Returns(true); + + await _controller.ProductDetails("p1"); + + _recentlyViewedMock.Verify(x => x.AddProductToRecentlyViewedList(It.IsAny(), It.IsAny()), + Times.Never); + } + + [TestMethod] + public async Task ProductDetails_SavedCustomer_RecordsRecentlyViewed() + { + GivenProduct(new Product { Id = "p1", Published = true, VisibleIndividually = true }); + _guestPersistenceMock.Setup(x => x.IsTransient(_customer.Id)).Returns(false); + + await _controller.ProductDetails("p1"); + + _recentlyViewedMock.Verify(x => x.AddProductToRecentlyViewedList(_customer.Id, "p1"), Times.Once); + } } diff --git a/src/Web/Grand.Web/Controllers/ProductController.cs b/src/Web/Grand.Web/Controllers/ProductController.cs index 4d7769ccdb..4f7ac0c1be 100644 --- a/src/Web/Grand.Web/Controllers/ProductController.cs +++ b/src/Web/Grand.Web/Controllers/ProductController.cs @@ -16,6 +16,7 @@ using Grand.Web.Common.Controllers; using Grand.Web.Common.Extensions; using Grand.Web.Common.Filters; +using Grand.Web.Common.RequestCustomerResolvers; using Grand.Web.Common.Security.Captcha; using Grand.Web.Events; using Grand.Web.Extensions; @@ -44,7 +45,8 @@ public ProductController( IPermissionService permissionService, IMediator mediator, CatalogSettings catalogSettings, - CaptchaSettings captchaSettings + CaptchaSettings captchaSettings, + IGuestCustomerPersistence guestCustomerPersistence ) { _productService = productService; @@ -57,6 +59,7 @@ CaptchaSettings captchaSettings _mediator = mediator; _catalogSettings = catalogSettings; _captchaSettings = captchaSettings; + _guestCustomerPersistence = guestCustomerPersistence; } #endregion @@ -262,6 +265,7 @@ public virtual async Task GetDatesForMonth(string productId, int private readonly IMediator _mediator; private readonly CatalogSettings _catalogSettings; private readonly CaptchaSettings _captchaSettings; + private readonly IGuestCustomerPersistence _guestCustomerPersistence; #endregion @@ -320,8 +324,9 @@ public virtual async Task> ProductDetails(stri var productLayoutViewPath = await _mediator.Send(new GetProductLayoutViewPath { ProductLayoutId = product.ProductLayoutId }); - //save as recently viewed - await _recentlyViewedProductsService.AddProductToRecentlyViewedList(customer.Id, product.Id); + //save as recently viewed - not for the guest that is not saved yet: its rows would stay orphaned + if (!_guestCustomerPersistence.IsTransient(customer.Id)) + await _recentlyViewedProductsService.AddProductToRecentlyViewedList(customer.Id, product.Id); //display "edit" (manage) link if (await _permissionService.Authorize(StandardPermission.ManageAccessAdminPanel, customer) && @@ -535,8 +540,9 @@ public virtual async Task QuickView(string productId) var productLayoutViewPath = await _mediator.Send(new GetProductLayoutViewPath { ProductLayoutId = product.ProductLayoutId }); - //save as recently viewed - await _recentlyViewedProductsService.AddProductToRecentlyViewedList(customer.Id, product.Id); + //save as recently viewed - not for the guest that is not saved yet: its rows would stay orphaned + if (!_guestCustomerPersistence.IsTransient(customer.Id)) + await _recentlyViewedProductsService.AddProductToRecentlyViewedList(customer.Id, product.Id); _ = _productService.IncrementProductField(product, x => x.Viewed, 1); From 651a40023cfb79f401c2863636be55ca7a011982 Mon Sep 17 00:00:00 2001 From: support Date: Thu, 15 Oct 2026 08:01:52 +0200 Subject: [PATCH 14/15] Log the promotion failure and pin the customer service surface --- .../TransientGuestCustomerServiceTests.cs | 31 +++++++++++++++++++ .../GuestCustomerPersistence.cs | 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs index a5735ea398..2528e5ab8f 100644 --- a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs @@ -129,6 +129,37 @@ public void Startup_RunsInSecondPass() Assert.IsFalse(new Grand.Web.Common.Startup.GuestCustomerStartup().BeforeConfigure); } + private static readonly string[] NonPersistingMembers = [ + "GetAllCustomers", "GetOnlineCustomers", "GetCountOnlineShoppingCart", "GetCustomerById", + "GetCustomersByIds", "GetCustomerByGuid", "GetCustomerByEmail", "GetCustomerBySystemName", + "GetCustomerByUsername", "InsertGuestCustomer", "InsertCustomer", "DeleteGuestCustomers" + ]; + + //every one of these calls Persist(...) in TransientGuestCustomerService before it writes + private static readonly string[] PersistingWrites = [ + "UpdateCustomer", "UpdateUserField`1", "DeleteCustomer", "UpdateCustomerField`1", "UpdateActive", + "UpdateContributions", "UpdateCustomerLastLoginDate", "UpdateCustomerInAdminPanel", "ResetCheckoutData", + "InsertCustomerGroupInCustomer", "DeleteCustomerGroupInCustomer", "DeleteAddress", "InsertAddress", + "UpdateAddress", "UpdateBillingAddress", "UpdateShippingAddress", "ClearShoppingCartItem", + "DeleteShoppingCartItem", "InsertShoppingCartItem", "UpdateShoppingCartItem" + ]; + + [TestMethod] + public void CustomerServiceSurface_IsExactlyTheKnownSet() + { + var actual = typeof(ICustomerService).GetMethods() + .Select(m => m.IsGenericMethodDefinition ? $"{m.Name}`{m.GetGenericArguments().Length}" : m.Name) + .Distinct() + .OrderBy(n => n) + .ToArray(); + var expected = NonPersistingMembers.Concat(PersistingWrites).OrderBy(n => n).ToArray(); + + CollectionAssert.AreEqual(expected, actual, + "ICustomerService changed. Decide whether the new member writes a customer; if it does, wrap it with " + + "Persist(...) in TransientGuestCustomerService, add a test for it and list it in PersistingWrites, " + + "otherwise list it in NonPersistingMembers."); + } + //any concrete ICustomerService with a parameterless constructor, as the business layer registers one private class FakeCustomerService() : TransientGuestCustomerService(Mock.Of(), Mock.Of()); diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs index 5c0778437e..0df6291f16 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs @@ -61,7 +61,7 @@ public async Task Promote(Customer guest) if (await GuestCustomerRules.IsAcceptableGuest(existing, groupService)) return existing; - logger.LogWarning("Guest cookie {CustomerGuid} belongs to a customer that is not a guest; a new guest is issued", + logger.LogWarning(ex, "Guest cookie {CustomerGuid} belongs to a customer that is not a guest; a new guest is issued", guest.CustomerGuid); return null; } From 9080293e30e4f01971d8757b27c23de0cbabb4ab Mon Sep 17 00:00:00 2001 From: support Date: Thu, 15 Oct 2026 12:16:02 +0200 Subject: [PATCH 15/15] Save unsaved guests with a GET filter instead of decorating ICustomerService The decorator replaced the registered ICustomerService, which overrides a core service and broke startup for factory registrations. GET actions that write to the current customer are marked [PersistGuestCustomer] instead (SetLanguage, SetCurrency, SetTaxType); AffiliateAttribute saves the guest before it stores the affiliate. - the CustomerGuid migration makes the index unique on LiteDB too and logs an error when the Mongo index did not end up unique - a failed guest insert is retried by the next write instead of rethrown --- .../2.5/MigrationCustomerGuidUniqueIndex.cs | 89 +++++--- .../MigrationCustomerGuidUniqueIndexTests.cs | 55 +++++ .../PersistGuestCustomerAttributeTests.cs | 102 +++++++++ .../GuestCustomerPersistenceTests.cs | 20 +- .../TransientGuestCustomerServiceTests.cs | 166 -------------- .../Filters/AffiliateAttribute.cs | 9 +- .../Filters/PersistGuestCustomerAttribute.cs | 31 +++ .../GuestCustomerPersistence.cs | 6 +- .../GuestCustomerRegistration.cs | 34 --- .../TransientGuestCustomerService.cs | 216 ------------------ .../Startup/GrandCommonStartup.cs | 4 + .../Startup/GuestCustomerStartup.cs | 28 --- src/Web/Grand.Web.Common/WorkContextSetter.cs | 2 +- .../Grand.Web/Controllers/CommonController.cs | 3 + 14 files changed, 280 insertions(+), 485 deletions(-) create mode 100644 src/Tests/Grand.Modules.Tests/Services/Migrations/MigrationCustomerGuidUniqueIndexTests.cs create mode 100644 src/Tests/Grand.Web.Common.Tests/Filters/PersistGuestCustomerAttributeTests.cs delete mode 100644 src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs create mode 100644 src/Web/Grand.Web.Common/Filters/PersistGuestCustomerAttribute.cs delete mode 100644 src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs delete mode 100644 src/Web/Grand.Web.Common/RequestCustomerResolvers/TransientGuestCustomerService.cs delete mode 100644 src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs diff --git a/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs index 6bf3958572..6698da3813 100644 --- a/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs +++ b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs @@ -1,8 +1,10 @@ using Grand.Data; +using Grand.Data.Mongo; using Grand.Domain.Customers; using Grand.Infrastructure.Migrations; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using MongoDB.Driver; namespace Grand.Module.Migration.Migrations._2._5; @@ -10,6 +12,9 @@ public class MigrationCustomerGuidUniqueIndex : IMigration { private const string IndexName = "CustomerGuid_1"; + //LiteDB names an index after its field and ignores the given name + private const string LiteDbIndexName = "CustomerGuid"; + public int Priority => 0; public DbVersion Version => new(2, 5); public Guid Identity => new("f0c98624-441d-42b6-95ae-429bfb64d266"); @@ -21,50 +26,66 @@ public bool UpgradeProcess(IServiceProvider serviceProvider) var databaseContext = serviceProvider.GetRequiredService(); var logService = serviceProvider.GetRequiredService>(); - //LiteDB names indexes by field, so the named drop and recreate below do not apply to it - //and EnsureIndex never alters an existing index - new installs create the index unique - if (DataSettingsManager.Instance.LoadSettings().DbProvider != DbProvider.MongoDB) + try { - logService.LogInformation( - "The index {IndexName} stays non-unique on this database provider (new installs create it unique)", - IndexName); - return true; + MakeUnique(repository, databaseContext, logService); } - - try + catch (Exception ex) { - var duplicates = repository.Table - .GroupBy(x => x.CustomerGuid) - .Where(x => x.Count() > 1) - .Select(x => x.Key) - .Take(20) - .ToList(); + logService.LogError(ex, "UpgradeProcess - CustomerGuidUniqueIndex (2.5)"); + } - //duplicates must not stop the upgrade - keep the non-unique index for the lookups and report them - if (duplicates.Count != 0) - { - logService.LogError( - "The index {IndexName} stays non-unique - these customer GUIDs are used more than once: {Guids}. Remove the duplicates and recreate the index as unique", - IndexName, string.Join(", ", duplicates)); - return true; - } + return true; + } - databaseContext.DeleteIndex(repository, IndexName).GetAwaiter().GetResult(); - databaseContext.CreateIndex(repository, - OrderBuilder.Create().Ascending(x => x.CustomerGuid), IndexName, true) - .GetAwaiter().GetResult(); + /// + /// Recreates the CustomerGuid index as unique. Returns false when it stays non-unique + /// + public static bool MakeUnique(IRepository repository, IDatabaseContext databaseContext, ILogger logService) + { + var duplicates = repository.Table + .GroupBy(x => x.CustomerGuid) + .Where(x => x.Count() > 1) + .Select(x => x.Key) + .Take(20) + .ToList(); - //when the unique index exists this is a rejected, swallowed no-op; when the unique create failed - //it restores the lookup index - databaseContext.CreateIndex(repository, - OrderBuilder.Create().Ascending(x => x.CustomerGuid), IndexName) - .GetAwaiter().GetResult(); + //duplicates must not stop the upgrade - keep the non-unique index for the lookups and report them + if (duplicates.Count != 0) + { + logService.LogError( + "The index {IndexName} stays non-unique - these customer GUIDs are used more than once: {Guids}. Remove the duplicates and recreate the index as unique", + IndexName, string.Join(", ", duplicates)); + return false; } - catch (Exception ex) + + var mongoRepository = repository as MongoRepository; + var keys = OrderBuilder.Create().Ascending(x => x.CustomerGuid); + + databaseContext.DeleteIndex(repository, mongoRepository != null ? IndexName : LiteDbIndexName) + .GetAwaiter().GetResult(); + databaseContext.CreateIndex(repository, keys, IndexName, true).GetAwaiter().GetResult(); + + //when the unique index exists this is a rejected, swallowed no-op; when the unique create failed + //it restores the lookup index + databaseContext.CreateIndex(repository, keys, IndexName).GetAwaiter().GetResult(); + + //CreateIndex swallows its errors: a duplicate saved by a running instance after the check above + //would leave the index non-unique without a trace + if (mongoRepository != null && !IsUnique(mongoRepository)) { - logService.LogError(ex, "UpgradeProcess - CustomerGuidUniqueIndex (2.5)"); + logService.LogError( + "The index {IndexName} could not be created unique and stays non-unique. Remove duplicate customer GUIDs and recreate the index as unique", + IndexName); + return false; } return true; } + + private static bool IsUnique(MongoRepository repository) + { + return repository.Collection.Indexes.List().ToList() + .Any(x => x["name"] == IndexName && x.Contains("unique") && x["unique"].ToBoolean()); + } } diff --git a/src/Tests/Grand.Modules.Tests/Services/Migrations/MigrationCustomerGuidUniqueIndexTests.cs b/src/Tests/Grand.Modules.Tests/Services/Migrations/MigrationCustomerGuidUniqueIndexTests.cs new file mode 100644 index 0000000000..2643ec598b --- /dev/null +++ b/src/Tests/Grand.Modules.Tests/Services/Migrations/MigrationCustomerGuidUniqueIndexTests.cs @@ -0,0 +1,55 @@ +using Grand.Data.LiteDb; +using Grand.Data.Tests; +using Grand.Domain.Customers; +using Grand.Module.Migration.Migrations._2._5; +using Grand.SharedKernel; +using LiteDB; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Grand.Modules.Tests.Services.Migrations; + +[TestClass] +public class MigrationCustomerGuidUniqueIndexTests +{ + private LiteDatabase _database; + private LiteDBRepository _repository; + private LiteDBContext _context; + + [TestInitialize] + public void Setup() + { + _database = new LiteDatabase(new MemoryStream()); + _repository = new LiteDBRepository(_database, new AuditInfoProvider()); + _context = new LiteDBContext(_database); + //an upgraded install: the lookup index exists, not unique + _database.GetCollection(nameof(Customer)).EnsureIndex(x => x.CustomerGuid); + } + + [TestCleanup] + public void Cleanup() => _database.Dispose(); + + [TestMethod] + public async Task LiteDb_RecreatesTheIndexUnique() + { + var guid = Guid.NewGuid(); + await _repository.InsertAsync(new Customer { CustomerGuid = guid }); + + Assert.IsTrue(MigrationCustomerGuidUniqueIndex.MakeUnique(_repository, _context, NullLogger.Instance)); + + await Assert.ThrowsExactlyAsync(() => _repository.InsertAsync(new Customer { CustomerGuid = guid })); + } + + [TestMethod] + public async Task LiteDb_Duplicates_KeepTheIndexNonUnique() + { + var guid = Guid.NewGuid(); + await _repository.InsertAsync(new Customer { CustomerGuid = guid }); + await _repository.InsertAsync(new Customer { CustomerGuid = guid }); + + Assert.IsFalse(MigrationCustomerGuidUniqueIndex.MakeUnique(_repository, _context, NullLogger.Instance)); + + await _repository.InsertAsync(new Customer { CustomerGuid = guid }); + Assert.AreEqual(3, _repository.Table.Count(x => x.CustomerGuid == guid)); + } +} diff --git a/src/Tests/Grand.Web.Common.Tests/Filters/PersistGuestCustomerAttributeTests.cs b/src/Tests/Grand.Web.Common.Tests/Filters/PersistGuestCustomerAttributeTests.cs new file mode 100644 index 0000000000..c4157d811c --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/Filters/PersistGuestCustomerAttributeTests.cs @@ -0,0 +1,102 @@ +using Grand.Business.Core.Interfaces.Customers; +using Grand.Data; +using Grand.Domain.Affiliates; +using Grand.Domain.Customers; +using Grand.Infrastructure; +using Grand.SharedKernel.Extensions; +using Grand.Web.Common.Filters; +using Grand.Web.Common.RequestCustomerResolvers; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using System.Linq.Expressions; + +namespace Grand.Web.Common.Tests.Filters; + +[TestClass] +public class PersistGuestCustomerAttributeTests +{ + private readonly List _order = []; + private Mock _persistence; + + [TestInitialize] + public void Setup() + { + //the test App_Data/Settings.cfg has a connection string, so the filters see an installed database + DataSettingsManager.Initialize(Path.Combine("", CommonPath.AppData, CommonPath.SettingsFile)); + _persistence = new Mock(); + _persistence.Setup(p => p.EnsurePersisted()).Callback(() => _order.Add("persist")) + .Returns(Task.CompletedTask); + } + + private static (ActionExecutingContext, ActionExecutionDelegate) Context(string query, Action onAction) + { + var httpContext = new DefaultHttpContext(); + httpContext.Request.Method = HttpMethods.Get; + httpContext.Request.QueryString = new QueryString(query); + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var executing = new ActionExecutingContext(actionContext, new List(), + new Dictionary(), null); + return (executing, () => + { + onAction(); + return Task.FromResult(new ActionExecutedContext(actionContext, new List(), null)); + }); + } + + [TestMethod] + public async Task PersistsTheGuestBeforeTheAction() + { + var (executing, next) = Context("", () => _order.Add("action")); + var filter = new PersistGuestCustomerAttribute.PersistGuestCustomerFilter(_persistence.Object); + + await filter.OnActionExecutionAsync(executing, next); + + CollectionAssert.AreEqual(new[] { "persist", "action" }, _order); + } + + [TestMethod] + public async Task Affiliate_LandingWithAffiliateId_PersistsThenWrites() + { + var customer = new Customer { Id = "g1" }; + var (filter, customerService) = AffiliateFilter(customer, + new Affiliate { Id = "aff", Active = true }); + customerService.Setup(c => c.UpdateCustomerField("g1", It.IsAny>>(), "aff")) + .Callback(() => _order.Add("write")).Returns(Task.CompletedTask); + var (executing, next) = Context("?affiliateid=aff", () => { }); + + await filter.OnActionExecutionAsync(executing, next); + + CollectionAssert.AreEqual(new[] { "persist", "write" }, _order); + } + + [TestMethod] + public async Task Affiliate_InactiveAffiliate_NothingPersisted() + { + var (filter, _) = AffiliateFilter(new Customer { Id = "g1" }, new Affiliate { Id = "aff", Active = false }); + var (executing, next) = Context("?affiliateid=aff", () => { }); + + await filter.OnActionExecutionAsync(executing, next); + + _persistence.Verify(p => p.EnsurePersisted(), Times.Never); + } + + private (AffiliateAttribute.AffiliateFilter, Mock) AffiliateFilter(Customer current, + Affiliate affiliate) + { + var workContext = new Mock(); + workContext.SetupGet(w => w.CurrentCustomer).Returns(current); + var contextAccessor = new Mock(); + contextAccessor.SetupGet(c => c.WorkContext).Returns(workContext.Object); + var affiliateService = new Mock(); + affiliateService.Setup(a => a.GetAffiliateById(affiliate.Id)).ReturnsAsync(affiliate); + var customerService = new Mock(); + + return (new AffiliateAttribute.AffiliateFilter(affiliateService.Object, customerService.Object, + contextAccessor.Object, _persistence.Object), customerService); + } +} diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs index 8b733d6998..1adbff6e4c 100644 --- a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs @@ -1,4 +1,4 @@ -using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Business.Core.Interfaces.Common.Directory; using Grand.Business.Core.Interfaces.Customers; using Grand.Domain.Customers; using Grand.Web.Common.RequestCustomerResolvers; @@ -91,6 +91,24 @@ public async Task EnsurePersisted_ConcurrentCalls_InsertOnce() _customers.Verify(c => c.InsertGuestCustomer(It.IsAny()), Times.Once); } + [TestMethod] + public async Task EnsurePersisted_AfterFailedInsert_RetriesOnTheNextWrite() + { + var guest = new Customer { Id = "g1" }; + _customers.SetupSequence(c => c.InsertGuestCustomer(guest)) + .ThrowsAsync(new TimeoutException("db blip")) + .ReturnsAsync(guest); + await _persistence.SetTransient(guest); + + await Assert.ThrowsExactlyAsync(() => _persistence.EnsurePersisted()); + Assert.IsTrue(_persistence.IsTransient("g1")); + + await _persistence.EnsurePersisted(); + + _customers.Verify(c => c.InsertGuestCustomer(guest), Times.Exactly(2)); + Assert.IsNull(_persistence.Transient); + } + [TestMethod] public async Task Promote_Inserts() { diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs deleted file mode 100644 index 2528e5ab8f..0000000000 --- a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/TransientGuestCustomerServiceTests.cs +++ /dev/null @@ -1,166 +0,0 @@ -using Grand.Business.Core.Interfaces.Common.Directory; -using Grand.Business.Core.Interfaces.Customers; -using Grand.Domain.Customers; -using Grand.Domain.Orders; -using Grand.Web.Common.RequestCustomerResolvers; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.VisualStudio.TestTools.UnitTesting; -using Moq; -using System.Linq.Expressions; - -namespace Grand.Web.Common.Tests.RequestCustomerResolvers; - -[TestClass] -public class TransientGuestCustomerServiceTests -{ - private Mock _inner; - private Mock _persistence; - private TransientGuestCustomerService _service; - private readonly Customer _transient = new() { Id = "transient" }; - - [TestInitialize] - public void Setup() - { - _inner = new Mock(); - _persistence = new Mock(); - _persistence.Setup(p => p.IsTransient("transient")).Returns(true); - _persistence.SetupGet(p => p.Transient).Returns(_transient); - _persistence.Setup(p => p.EnsurePersisted()).Returns(Task.CompletedTask); - _service = new TransientGuestCustomerService(_inner.Object, _persistence.Object); - } - - [TestMethod] - public async Task UpdateCustomerFieldById_OnTransient_PersistsThenWrites() - { - //AffiliateAttribute writes the affiliate id by customer id on the landing GET - var order = new List(); - _persistence.Setup(p => p.EnsurePersisted()).Callback(() => order.Add("persist")).Returns(Task.CompletedTask); - _inner.Setup(i => i.UpdateCustomerField("transient", It.IsAny>>(), "aff")) - .Callback(() => order.Add("write")).Returns(Task.CompletedTask); - - await _service.UpdateCustomerField("transient", x => x.AffiliateId, "aff"); - - CollectionAssert.AreEqual(new[] { "persist", "write" }, order); - } - - [TestMethod] - public async Task UpdateUserField_OnTransient_Persists() - { - await _service.UpdateUserField(_transient, SystemCustomerFieldNames.LanguageId, "pl", "store"); - - _persistence.Verify(p => p.EnsurePersisted(), Times.Once); - _inner.Verify(i => i.UpdateUserField(_transient, SystemCustomerFieldNames.LanguageId, "pl", "store"), Times.Once); - } - - [TestMethod] - public async Task InsertShoppingCartItem_OnTransient_Persists() - { - await _service.InsertShoppingCartItem("transient", new ShoppingCartItem()); - - _persistence.Verify(p => p.EnsurePersisted(), Times.Once); - } - - [TestMethod] - public async Task Write_OnAnotherCustomer_DoesNotPersist() - { - await _service.UpdateUserField(new Customer { Id = "other" }, "key", "value"); - await _service.UpdateCustomerField("other", x => x.AffiliateId, "aff"); - - _persistence.Verify(p => p.EnsurePersisted(), Times.Never); - } - - [TestMethod] - public async Task Reads_NeverPersist() - { - await _service.GetCustomerByGuid(Guid.NewGuid()); - await _service.GetCustomerBySystemName("x"); - - _persistence.Verify(p => p.EnsurePersisted(), Times.Never); - } - - [TestMethod] - public async Task GetCustomerById_OfTransient_ReturnsTransientWithoutQuery() - { - Assert.AreSame(_transient, await _service.GetCustomerById("transient")); - _inner.Verify(i => i.GetCustomerById(It.IsAny()), Times.Never); - } - - [TestMethod] - public async Task InsertGuestCustomer_PassesThrough() - { - var guest = new Customer(); - _inner.Setup(i => i.InsertGuestCustomer(guest)).ReturnsAsync(guest); - - Assert.AreSame(guest, await _service.InsertGuestCustomer(guest)); - _persistence.Verify(p => p.EnsurePersisted(), Times.Never); - } - - [TestMethod] - public void Registration_DecoratesCustomerServiceAndSharesPersistencePerScope() - { - var services = new ServiceCollection(); - services.AddScoped(); - services.AddSingleton(Mock.Of()); - services.AddSingleton(typeof(ILogger<>), typeof(NullLogger<>)); - - services.AddTransientGuestCustomers(); - - using var provider = services.BuildServiceProvider(new ServiceProviderOptions { ValidateScopes = true }); - using var scope = provider.CreateScope(); - Assert.AreEqual(typeof(TransientGuestCustomerService), - scope.ServiceProvider.GetRequiredService().GetType()); - Assert.AreSame(scope.ServiceProvider.GetRequiredService(), - scope.ServiceProvider.GetRequiredService()); - Assert.AreEqual(1, services.Count(d => d.ServiceType == typeof(ICustomerService))); - } - - [TestMethod] - public void Registration_NoCustomerService_Throws() - { - Assert.ThrowsExactly(() => new ServiceCollection().AddTransientGuestCustomers()); - } - - [TestMethod] - public void Startup_RunsInSecondPass() - { - //second pass, after Grand.Business.Customers registers ICustomerService - Assert.IsFalse(new Grand.Web.Common.Startup.GuestCustomerStartup().BeforeConfigure); - } - - private static readonly string[] NonPersistingMembers = [ - "GetAllCustomers", "GetOnlineCustomers", "GetCountOnlineShoppingCart", "GetCustomerById", - "GetCustomersByIds", "GetCustomerByGuid", "GetCustomerByEmail", "GetCustomerBySystemName", - "GetCustomerByUsername", "InsertGuestCustomer", "InsertCustomer", "DeleteGuestCustomers" - ]; - - //every one of these calls Persist(...) in TransientGuestCustomerService before it writes - private static readonly string[] PersistingWrites = [ - "UpdateCustomer", "UpdateUserField`1", "DeleteCustomer", "UpdateCustomerField`1", "UpdateActive", - "UpdateContributions", "UpdateCustomerLastLoginDate", "UpdateCustomerInAdminPanel", "ResetCheckoutData", - "InsertCustomerGroupInCustomer", "DeleteCustomerGroupInCustomer", "DeleteAddress", "InsertAddress", - "UpdateAddress", "UpdateBillingAddress", "UpdateShippingAddress", "ClearShoppingCartItem", - "DeleteShoppingCartItem", "InsertShoppingCartItem", "UpdateShoppingCartItem" - ]; - - [TestMethod] - public void CustomerServiceSurface_IsExactlyTheKnownSet() - { - var actual = typeof(ICustomerService).GetMethods() - .Select(m => m.IsGenericMethodDefinition ? $"{m.Name}`{m.GetGenericArguments().Length}" : m.Name) - .Distinct() - .OrderBy(n => n) - .ToArray(); - var expected = NonPersistingMembers.Concat(PersistingWrites).OrderBy(n => n).ToArray(); - - CollectionAssert.AreEqual(expected, actual, - "ICustomerService changed. Decide whether the new member writes a customer; if it does, wrap it with " + - "Persist(...) in TransientGuestCustomerService, add a test for it and list it in PersistingWrites, " + - "otherwise list it in NonPersistingMembers."); - } - - //any concrete ICustomerService with a parameterless constructor, as the business layer registers one - private class FakeCustomerService() - : TransientGuestCustomerService(Mock.Of(), Mock.Of()); -} diff --git a/src/Web/Grand.Web.Common/Filters/AffiliateAttribute.cs b/src/Web/Grand.Web.Common/Filters/AffiliateAttribute.cs index 847267932c..2ce94c6dbe 100644 --- a/src/Web/Grand.Web.Common/Filters/AffiliateAttribute.cs +++ b/src/Web/Grand.Web.Common/Filters/AffiliateAttribute.cs @@ -2,6 +2,7 @@ using Grand.Data; using Grand.Domain.Affiliates; using Grand.Infrastructure; +using Grand.Web.Common.RequestCustomerResolvers; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; @@ -24,9 +25,10 @@ public AffiliateAttribute() : base(typeof(AffiliateFilter)) /// /// Represents a filter that checks and updates affiliate of customer /// - private class AffiliateFilter(IAffiliateService affiliateService, + internal class AffiliateFilter(IAffiliateService affiliateService, ICustomerService customerService, - IContextAccessor contextAccessor) : IAsyncActionFilter + IContextAccessor contextAccessor, + IGuestCustomerPersistence guestCustomerPersistence) : IAsyncActionFilter { #region Methods @@ -84,6 +86,9 @@ private async Task SetCustomerAffiliateId(Affiliate affiliate) if (affiliate.Id == contextAccessor.WorkContext.CurrentCustomer.AffiliateId) return; + //a landing page with ?affiliateid= is usually the guest's first, unsaved request + await guestCustomerPersistence.EnsurePersisted(); + //update affiliate identifier contextAccessor.WorkContext.CurrentCustomer.AffiliateId = affiliate.Id; await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCustomer.Id, x => x.AffiliateId, diff --git a/src/Web/Grand.Web.Common/Filters/PersistGuestCustomerAttribute.cs b/src/Web/Grand.Web.Common/Filters/PersistGuestCustomerAttribute.cs new file mode 100644 index 0000000000..4bb4bfe8b2 --- /dev/null +++ b/src/Web/Grand.Web.Common/Filters/PersistGuestCustomerAttribute.cs @@ -0,0 +1,31 @@ +using Grand.Web.Common.RequestCustomerResolvers; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; + +namespace Grand.Web.Common.Filters; + +/// +/// Saves the request's unsaved guest before a GET action that writes to the current customer. A guest of a +/// GET request is not in the database yet, and a customer write is a partial update by id that silently does +/// nothing on a document that does not exist. Requests of other methods save the guest when it is created +/// +public class PersistGuestCustomerAttribute : TypeFilterAttribute +{ + public PersistGuestCustomerAttribute() : base(typeof(PersistGuestCustomerFilter)) + { + } + + #region Filter + + internal class PersistGuestCustomerFilter(IGuestCustomerPersistence guestCustomerPersistence) + : IAsyncActionFilter + { + public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) + { + await guestCustomerPersistence.EnsurePersisted(); + await next(); + } + } + + #endregion +} diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs index 0df6291f16..de5964e999 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs @@ -5,9 +5,6 @@ namespace Grand.Web.Common.RequestCustomerResolvers; -/// -/// Takes the undecorated customer service: the decorator depends on this class -/// public class GuestCustomerPersistence( ICustomerService customerService, IGroupService groupService, @@ -37,6 +34,9 @@ public Task EnsurePersisted() { if (Transient == null) return Task.CompletedTask; + //a failed insert is retried by the next write instead of rethrowing the cached failure + if (_persisting is { IsFaulted: true } or { IsCanceled: true }) _persisting = null; + //writes of one request may run concurrently: they share the single insert return _persisting ??= Persist(Transient); } diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs deleted file mode 100644 index 311bb3d6d7..0000000000 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRegistration.cs +++ /dev/null @@ -1,34 +0,0 @@ -using Grand.Business.Core.Interfaces.Common.Directory; -using Grand.Business.Core.Interfaces.Customers; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; - -namespace Grand.Web.Common.RequestCustomerResolvers; - -public static class GuestCustomerRegistration -{ - /// - /// Decorates the registered customer service so that the request's unsaved guest is saved before its first - /// write. Call after the business layer has registered ICustomerService - /// - public static IServiceCollection AddTransientGuestCustomers(this IServiceCollection services) - { - var registered = services.LastOrDefault(d => d.ServiceType == typeof(ICustomerService)); - if (registered?.ImplementationType == null) - throw new InvalidOperationException( - "The guest customer decorator needs a type-based ICustomerService registration made before it."); - - var innerType = registered.ImplementationType; - services.Remove(registered); - services.Add(new ServiceDescriptor(innerType, innerType, ServiceLifetime.Scoped)); - - services.AddScoped(sp => new GuestCustomerPersistence( - (ICustomerService)sp.GetRequiredService(innerType), - sp.GetRequiredService(), - sp.GetRequiredService>())); - services.Add(new ServiceDescriptor(typeof(ICustomerService), sp => new TransientGuestCustomerService( - (ICustomerService)sp.GetRequiredService(innerType), - sp.GetRequiredService()), ServiceLifetime.Scoped)); - return services; - } -} diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/TransientGuestCustomerService.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/TransientGuestCustomerService.cs deleted file mode 100644 index b74b4618e2..0000000000 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/TransientGuestCustomerService.cs +++ /dev/null @@ -1,216 +0,0 @@ -using Grand.Business.Core.Interfaces.Customers; -using Grand.Domain; -using Grand.Domain.Common; -using Grand.Domain.Customers; -using Grand.Domain.Orders; -using System.Linq.Expressions; - -namespace Grand.Web.Common.RequestCustomerResolvers; - -/// -/// Saves the request's unsaved guest before the first write to it. Every customer write is a partial update by -/// id, which silently does nothing on a document that does not exist yet -/// -public class TransientGuestCustomerService(ICustomerService inner, IGuestCustomerPersistence persistence) - : ICustomerService -{ - private async Task Persist(string customerId) - { - if (persistence.IsTransient(customerId)) await persistence.EnsurePersisted(); - } - - #region Reads - - public Task> GetAllCustomers(DateTime? createdFromUtc = null, DateTime? createdToUtc = null, - string affiliateId = "", string vendorId = "", string storeId = "", string ownerId = "", - string salesEmployeeId = "", string[] customerGroupIds = null, string[] customerTagIds = null, - string email = null, string username = null, string firstName = null, string lastName = null, - string company = null, string phone = null, string zipPostalCode = null, - bool loadOnlyWithShoppingCart = false, ShoppingCartType? sct = null, int pageIndex = 0, - int pageSize = int.MaxValue, Expression> orderBySelector = null) - { - return inner.GetAllCustomers(createdFromUtc, createdToUtc, affiliateId, vendorId, storeId, ownerId, - salesEmployeeId, customerGroupIds, customerTagIds, email, username, firstName, lastName, company, phone, - zipPostalCode, loadOnlyWithShoppingCart, sct, pageIndex, pageSize, orderBySelector); - } - - public Task> GetOnlineCustomers(DateTime lastActivityFromUtc, string[] customerGroupIds, - string storeId = "", string salesEmployeeId = "", int pageIndex = 0, int pageSize = int.MaxValue) - { - return inner.GetOnlineCustomers(lastActivityFromUtc, customerGroupIds, storeId, salesEmployeeId, pageIndex, - pageSize); - } - - public Task GetCountOnlineShoppingCart(DateTime lastActivityFromUtc, string storeId = "", - string salesEmployeeId = "") - { - return inner.GetCountOnlineShoppingCart(lastActivityFromUtc, storeId, salesEmployeeId); - } - - public Task GetCustomerById(string customerId) - { - //the request's own unsaved guest, e.g. reloaded by a handler on the first request - return persistence.IsTransient(customerId) - ? Task.FromResult(persistence.Transient) - : inner.GetCustomerById(customerId); - } - - public Task> GetCustomersByIds(string[] customerIds) => inner.GetCustomersByIds(customerIds); - - public Task GetCustomerByGuid(Guid customerGuid) => inner.GetCustomerByGuid(customerGuid); - - public Task GetCustomerByEmail(string email, string storeId = "") => - inner.GetCustomerByEmail(email, storeId); - - public Task GetCustomerBySystemName(string systemName) => inner.GetCustomerBySystemName(systemName); - - public Task GetCustomerByUsername(string username, string storeId = "") => - inner.GetCustomerByUsername(username, storeId); - - #endregion - - #region Inserts - - public Task InsertGuestCustomer(Customer customer) => inner.InsertGuestCustomer(customer); - - public Task InsertCustomer(Customer customer) => inner.InsertCustomer(customer); - - public Task DeleteGuestCustomers(DateTime? createdFromUtc, DateTime? createdToUtc, - bool onlyWithoutShoppingCart) => - inner.DeleteGuestCustomers(createdFromUtc, createdToUtc, onlyWithoutShoppingCart); - - #endregion - - #region Writes to one customer - - public async Task UpdateCustomer(Customer customer) - { - await Persist(customer?.Id); - await inner.UpdateCustomer(customer); - } - - public async Task UpdateUserField(Customer customer, string key, TPropType value, string storeId = "") - { - await Persist(customer?.Id); - await inner.UpdateUserField(customer, key, value, storeId); - } - - public async Task DeleteCustomer(Customer customer, bool hard = false) - { - await Persist(customer?.Id); - await inner.DeleteCustomer(customer, hard); - } - - public async Task UpdateCustomerField(Customer customer, Expression> expression, T value) - { - await Persist(customer?.Id); - await inner.UpdateCustomerField(customer, expression, value); - } - - public async Task UpdateCustomerField(string customerId, Expression> expression, T value) - { - await Persist(customerId); - await inner.UpdateCustomerField(customerId, expression, value); - } - - public async Task UpdateActive(Customer customer) - { - await Persist(customer?.Id); - await inner.UpdateActive(customer); - } - - public async Task UpdateContributions(Customer customer) - { - await Persist(customer?.Id); - await inner.UpdateContributions(customer); - } - - public async Task UpdateCustomerLastLoginDate(Customer customer) - { - await Persist(customer?.Id); - await inner.UpdateCustomerLastLoginDate(customer); - } - - public async Task UpdateCustomerInAdminPanel(Customer customer) - { - await Persist(customer?.Id); - await inner.UpdateCustomerInAdminPanel(customer); - } - - public async Task ResetCheckoutData(Customer customer, string storeId, bool clearCouponCodes = false, - bool clearCheckoutAttributes = false, bool clearLoyaltyPoints = true, bool clearShipping = true, - bool clearPayment = true) - { - await Persist(customer?.Id); - await inner.ResetCheckoutData(customer, storeId, clearCouponCodes, clearCheckoutAttributes, - clearLoyaltyPoints, clearShipping, clearPayment); - } - - public async Task InsertCustomerGroupInCustomer(CustomerGroup customerGroup, string customerId) - { - await Persist(customerId); - await inner.InsertCustomerGroupInCustomer(customerGroup, customerId); - } - - public async Task DeleteCustomerGroupInCustomer(CustomerGroup customerGroup, string customerId) - { - await Persist(customerId); - await inner.DeleteCustomerGroupInCustomer(customerGroup, customerId); - } - - public async Task DeleteAddress(Address address, string customerId) - { - await Persist(customerId); - await inner.DeleteAddress(address, customerId); - } - - public async Task InsertAddress(Address address, string customerId) - { - await Persist(customerId); - await inner.InsertAddress(address, customerId); - } - - public async Task UpdateAddress(Address address, string customerId) - { - await Persist(customerId); - await inner.UpdateAddress(address, customerId); - } - - public async Task UpdateBillingAddress(Address address, string customerId) - { - await Persist(customerId); - await inner.UpdateBillingAddress(address, customerId); - } - - public async Task UpdateShippingAddress(Address address, string customerId) - { - await Persist(customerId); - await inner.UpdateShippingAddress(address, customerId); - } - - public async Task ClearShoppingCartItem(string customerId, IList cart) - { - await Persist(customerId); - await inner.ClearShoppingCartItem(customerId, cart); - } - - public async Task DeleteShoppingCartItem(string customerId, ShoppingCartItem shoppingCartItem) - { - await Persist(customerId); - await inner.DeleteShoppingCartItem(customerId, shoppingCartItem); - } - - public async Task InsertShoppingCartItem(string customerId, ShoppingCartItem shoppingCartItem) - { - await Persist(customerId); - await inner.InsertShoppingCartItem(customerId, shoppingCartItem); - } - - public async Task UpdateShoppingCartItem(string customerId, ShoppingCartItem shoppingCartItem) - { - await Persist(customerId); - await inner.UpdateShoppingCartItem(customerId, shoppingCartItem); - } - - #endregion -} diff --git a/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs b/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs index 2029022aea..569669d024 100644 --- a/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs +++ b/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs @@ -1,6 +1,7 @@ using Grand.Infrastructure; using Grand.Infrastructure.Configuration; using Grand.Web.Common.Infrastructure; +using Grand.Web.Common.RequestCustomerResolvers; using Grand.Web.Common.Routing; using Grand.Web.Common.UserAgent; using Microsoft.AspNetCore.Builder; @@ -72,6 +73,9 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config //classify the request's user agent (crawler, mobile) services.AddScoped(); + //a guest is saved only once it returns its cookie or an action marked [PersistGuestCustomer] writes to it + services.AddScoped(); + //add routing services.AddRouting(options => { diff --git a/src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs b/src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs deleted file mode 100644 index 29d670befe..0000000000 --- a/src/Web/Grand.Web.Common/Startup/GuestCustomerStartup.cs +++ /dev/null @@ -1,28 +0,0 @@ -using Grand.Infrastructure; -using Grand.Web.Common.RequestCustomerResolvers; -using Microsoft.AspNetCore.Builder; -using Microsoft.AspNetCore.Hosting; -using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.DependencyInjection; - -namespace Grand.Web.Common.Startup; - -/// -/// Saves a guest only once it returns its cookie or something is written to it -/// -public class GuestCustomerStartup : IStartupApplication -{ - public void ConfigureServices(IServiceCollection services, IConfiguration configuration) - { - services.AddTransientGuestCustomers(); - } - - public void Configure(WebApplication application, IWebHostEnvironment webHostEnvironment) - { - } - - //second pass (BeforeConfigure false), after Grand.Business.Customers (100) registers ICustomerService - public int Priority => 1100; - - public bool BeforeConfigure => false; -} diff --git a/src/Web/Grand.Web.Common/WorkContextSetter.cs b/src/Web/Grand.Web.Common/WorkContextSetter.cs index 96e75bc8d6..aeba6b8a8c 100644 --- a/src/Web/Grand.Web.Common/WorkContextSetter.cs +++ b/src/Web/Grand.Web.Common/WorkContextSetter.cs @@ -218,7 +218,7 @@ private async Task GetBackgroundTaskCustomer() private async Task CreateCustomerGuest(Store store) { - //resolved here, not injected: it lives in the request scope next to the decorated customer service + //resolved here, not injected: it belongs to the request scope var persistence = _serviceProvider.GetService(); //a browser returning the cookie of a guest that was never saved: save it now under that GUID diff --git a/src/Web/Grand.Web/Controllers/CommonController.cs b/src/Web/Grand.Web/Controllers/CommonController.cs index e0a36bd6cd..139d23312e 100644 --- a/src/Web/Grand.Web/Controllers/CommonController.cs +++ b/src/Web/Grand.Web/Controllers/CommonController.cs @@ -144,6 +144,7 @@ public virtual IActionResult ExternalAuthenticationError(IEnumerable err [HttpGet] [PublicStore(true)] [DenySystemAccount] + [PersistGuestCustomer] public virtual async Task SetLanguage( [FromServices] AppConfig config, [FromServices] ICustomerService customerService, @@ -211,6 +212,7 @@ public virtual IActionResult InternalRedirect(string url, bool permanentRedirect [DenySystemAccount] [PublicStore(true)] [HttpGet] + [PersistGuestCustomer] public virtual async Task SetCurrency( [FromServices] ICurrencyService currencyService, [FromServices] ICustomerService customerService, @@ -289,6 +291,7 @@ void SetStoreCookie(Domain.Stores.Store store) //available even when navigation is not allowed [PublicStore(true)] [HttpGet] + [PersistGuestCustomer] public virtual async Task SetTaxType( [FromServices] TaxSettings taxSettings, [FromServices] ICustomerService customerService,