diff --git a/Applications/Pgan.PoracleWebNet.Api/Filters/PoracleRequestRefusedExceptionFilter.cs b/Applications/Pgan.PoracleWebNet.Api/Filters/PoracleRequestRefusedExceptionFilter.cs new file mode 100644 index 00000000..71cc2f17 --- /dev/null +++ b/Applications/Pgan.PoracleWebNet.Api/Filters/PoracleRequestRefusedExceptionFilter.cs @@ -0,0 +1,37 @@ +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; +using Pgan.PoracleWebNet.Core.Models; + +namespace Pgan.PoracleWebNet.Api.Filters; + +/// +/// Reports a refusal from PoracleNG's human, profile, area and location routes as the caller's problem. +/// +/// +/// Registered globally beside , which does the same job for the +/// tracking routes. Same body shape as every other refusal in this API -- { "error": "..." } -- so +/// the SPA's interceptor needs nothing new to show it. +/// +public sealed class PoracleRequestRefusedExceptionFilter : IActionFilter +{ + public void OnActionExecuting(ActionExecutingContext context) + { + } + + public void OnActionExecuted(ActionExecutedContext context) + { + if (context.Exception is not PoracleRequestRefusedException ex) + { + return; + } + + context.Result = new ObjectResult(new + { + error = ex.Message, + }) + { + StatusCode = ex.StatusCode, + }; + context.ExceptionHandled = true; + } +} diff --git a/Applications/Pgan.PoracleWebNet.Api/Program.cs b/Applications/Pgan.PoracleWebNet.Api/Program.cs index 7e4ae63c..1096326e 100644 --- a/Applications/Pgan.PoracleWebNet.Api/Program.cs +++ b/Applications/Pgan.PoracleWebNet.Api/Program.cs @@ -188,6 +188,7 @@ options.Filters.Add(); options.Filters.Add(); options.Filters.Add(); + options.Filters.Add(); options.Filters.Add(); options.Filters.Add(); options.Filters.Add(); diff --git a/CHANGELOG.md b/CHANGELOG.md index 02ceabcc..382befdd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **A refused profile, area, place or account change now says what was wrong with it.** Saving a profile with no number, a location that is not a location, an area list Poracle could not read, or banning a user through an older path all came back as *An unexpected error occurred* -- the wording this site uses when it genuinely does not know -- and were recorded as server faults. Poracle had explained every one of them; the explanation was being thrown away one layer below the screen. It now reaches the dialog that asked, in Poracle's own words, whether the server answered in its long-standing format or the newer one 5.2.1 introduced. The alarm pages were fixed this way some time ago; this is the same fix on the half of the site it never reached. +- **Switching to a profile that is no longer there stops signing you out.** Any *not found* answer from Poracle was read as *your account has been deleted*, which is what one of them means. A missing profile is not, and neither is a route an older Poracle does not have -- but both ended the session and sent you back to the login page, and one of them did it while a profile duplicate was tidying up after itself, hiding whatever had actually gone wrong. A deleted account still signs you out, exactly as before. - **An underscore in a name survives the rule sentence on an alarm card.** The sentence was having every `*`, `_` and backtick stripped out of it, not only the ones acting as Discord emphasis. Nothing Poracle renders today contains one, so there is no visible change: this is the case that had not arrived yet. Poracle interpolates template names, areas and saved-place labels into that sentence without escaping them, so an area called `north_side` would have reached the card as `northside`. Emphasis is now removed only where it is paired, and where an underscore sits between two word characters it is left alone -- which is what Discord does with it too ([#819](https://github.com/PGAN-Dev/PoracleWeb.NET/pull/819)). - **Pokéstop Events works at all.** The page shipped unable to answer a single request: the service behind it was never handed to the application at startup, so opening the page, adding an event or deleting one all failed the same way, with the generic server error. Nothing caught it -- the tests for that code substitute the missing piece, so they passed, and the site compiled and deployed green. A new test now builds every page's dependencies the way the running application does, which is the only place this kind of omission shows up. - **The PVP rank range is readable again on a dark-themed alarm card.** The band under a PVP alarm showed its league and nothing else, so the ranks you had set looked like they had been dropped. They were being drawn, in white, on a band that stays light in both themes. The league name gained some contrast on the way past ([#800](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/800)). diff --git a/Core/Pgan.PoracleWebNet.Core.Models/PoracleRequestRefusedException.cs b/Core/Pgan.PoracleWebNet.Core.Models/PoracleRequestRefusedException.cs new file mode 100644 index 00000000..4bc6c037 --- /dev/null +++ b/Core/Pgan.PoracleWebNet.Core.Models/PoracleRequestRefusedException.cs @@ -0,0 +1,50 @@ +namespace Pgan.PoracleWebNet.Core.Models; + +/// +/// PoracleNG refused a human, profile, area or location request and said why. +/// +/// +/// +/// PoracleHumanProxy handled 404 and one 409 and let everything else reach +/// EnsureSuccessStatusCode(), so a 400 such as "state is required (true/false)" or "profile_no must +/// be specified" arrived at the browser as 500 "An unexpected error occurred" and was logged as a server +/// fault. This is the same defect #539 closed on the tracking proxy, on the half of the surface that fix +/// did not reach. +/// +/// +/// Deliberately not : that type is documented as an alarm rejection +/// and shares an alarm-worded fallback with it, so reusing it would answer "Poracle rejected the alarm." +/// to someone renaming a profile or saving a place. One narrowly-named exception per failure shape is the +/// pattern the rest of the filters already follow. +/// +/// +/// carries the answer through because not every refusal is a 400: PoracleNG +/// answers 404 "Profile not found" for a profile number that does not exist, which is the caller naming +/// something absent, not the account being gone. +/// +/// +public sealed class PoracleRequestRefusedException : Exception +{ + public PoracleRequestRefusedException(string message) + : base(message) + { + } + + public PoracleRequestRefusedException(string message, int statusCode) + : base(message) => this.StatusCode = statusCode; + + public PoracleRequestRefusedException() + { + } + + public PoracleRequestRefusedException(string message, Exception innerException) + : base(message, innerException) + { + } + + /// The status the API should answer. 400 unless PoracleNG named something absent. + public int StatusCode + { + get; init; + } = 400; +} diff --git a/Core/Pgan.PoracleWebNet.Core.Services/PoracleHumanProxy.cs b/Core/Pgan.PoracleWebNet.Core.Services/PoracleHumanProxy.cs index b760d57a..9ee24c95 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/PoracleHumanProxy.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/PoracleHumanProxy.cs @@ -9,6 +9,9 @@ namespace Pgan.PoracleWebNet.Core.Services; public class PoracleHumanProxy(HttpClient httpClient, IConfiguration configuration) : IPoracleHumanProxy { + /// What to say when PoracleNG refused and explained nothing usable. + private const string Unexplained = "Poracle rejected the request."; + private readonly HttpClient _httpClient = httpClient; private readonly string _apiAddress = configuration["Poracle:ApiAddress"] ?? string.Empty; private readonly string _apiSecret = configuration["Poracle:ApiSecret"] ?? string.Empty; @@ -17,23 +20,75 @@ public class PoracleHumanProxy(HttpClient httpClient, IConfiguration configurati /// URL-encodes a userId for safe path construction. Webhook IDs are full URLs /// containing slashes that would break routing without encoding. /// + private static string Encode(string userId) => Uri.EscapeDataString(userId); + /// - /// Turns PoracleNG's "user not found" into something the API can answer 401 to. + /// Turns a refusal from PoracleNG into the answer it deserves, and lets everything else throw. /// /// - /// A JWT outlives the account it names. Without this, every lookup for a deleted user threw an - /// HttpRequestException that the global handler flattened into a 500, so the SPA -- which signs out - /// only on 401 -- left the user in an app where every page failed. See #584. + /// + /// Only 404 was read here, and one 409 on the delete-place path. Everything else fell through to + /// EnsureSuccessStatusCode(), whose the global handler + /// flattens into 500 "An unexpected error occurred" -- so "state is required (true/false)", + /// "profile_no must be specified" and "invalid latitude" all reached the user as a server fault and + /// were logged as one. #539 fixed exactly this on the tracking proxy and never reached here. + /// + /// + /// 422 is the same refusal wearing a different number. Verified live: the v1 human and profile routes + /// answer 400 on 5.1.0 and 5.2.1 alike, but 5.2.1's /api/v2/humans surface answers RFC 9457 + /// problem+json at 422 for the same mistakes. Matching only 400 would re-open this the day a call + /// site moves to v2. + /// + /// + /// A 404 is account-gone only when PoracleNG says so. It also answers 404 "Profile not found" when a + /// profile number does not exist, and gin answers a plaintext "404 page not found" for a route this + /// build does not have -- treating either as a dead account signed the user out of a working session, + /// and in ProfileOverviewService's restore-the-profile finally it did so while + /// swallowing the real failure. Both verified against 5.1.0 and 5.2.1. + /// /// - private static void EnsureAccountStillExists(HttpResponseMessage response) + private static async Task EnsureAcceptedAsync(HttpResponseMessage response) { - if (response.StatusCode == HttpStatusCode.NotFound) + if (response.IsSuccessStatusCode) + { + return; + } + + var payload = await response.Content.ReadAsStringAsync(); + + switch (response.StatusCode) { - throw new AccountGoneException(); + case HttpStatusCode.NotFound when NamesAMissingAccount(payload): + // A JWT outlives the account it names. Without this, every lookup for a deleted user threw + // an HttpRequestException that the global handler flattened into a 500, so the SPA -- which + // signs out only on 401 -- left the user in an app where every page failed. See #584. + throw new AccountGoneException(); + + case HttpStatusCode.NotFound: + throw new PoracleRequestRefusedException( + PoracleProblemDetails.Describe(payload, Unexplained), (int)HttpStatusCode.NotFound); + + case HttpStatusCode.BadRequest: + case HttpStatusCode.UnprocessableEntity: + throw new PoracleRequestRefusedException(PoracleProblemDetails.Describe(payload, Unexplained)); + + default: + response.EnsureSuccessStatusCode(); + return; } } - private static string Encode(string userId) => Uri.EscapeDataString(userId); + /// + /// True when a 404 body is PoracleNG saying the account itself is gone rather than something in it. + /// + /// + /// v1 answers {"message":"User not found"}; the v2 surface and the tracking routes say + /// "human not found". Anything else at 404 names a profile, a place or a route. + /// + private static bool NamesAMissingAccount(string? payload) => + payload is not null + && (payload.Contains("user not found", StringComparison.OrdinalIgnoreCase) + || payload.Contains("human not found", StringComparison.OrdinalIgnoreCase)); public async Task GetHumanAsync(string userId) { @@ -58,22 +113,19 @@ private static void EnsureAccountStillExists(HttpResponseMessage response) public async Task CreateHumanAsync(JsonElement body) { var response = await this.SendAsync(HttpMethod.Post, "/api/humans", body.GetRawText()); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } public async Task StartAsync(string userId) { var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/start"); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } public async Task StopAsync(string userId) { var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/stop"); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } public async Task AdminDisabledAsync(string userId, bool disabled) @@ -86,23 +138,20 @@ public async Task AdminDisabledAsync(string userId, bool disabled) state = disabled }); var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/adminDisabled", body); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } public async Task SetLocationAsync(string userId, double lat, double lon) { var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/setLocation/{lat}/{lon}"); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } public async Task SetAreasAsync(string userId, string[] areas) { var body = JsonSerializer.Serialize(areas); var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/setAreas", body); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } public async Task GetAreasAsync(string userId) => @@ -113,15 +162,13 @@ public async Task SetAreasAsync(string userId, string[] areas) public async Task SwitchProfileAsync(string userId, int profileNo) { var response = await this.SendAsync(HttpMethod.Post, $"/api/humans/{Encode(userId)}/switchProfile/{profileNo}"); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } public async Task GetProfilesAsync(string userId) { var response = await this.SendAsync(HttpMethod.Get, $"/api/profiles/{Encode(userId)}"); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); var json = await response.Content.ReadAsStringAsync(); using var doc = JsonDocument.Parse(json); @@ -131,29 +178,25 @@ public async Task GetProfilesAsync(string userId) public async Task AddProfileAsync(string userId, JsonElement body) { var response = await this.SendAsync(HttpMethod.Post, $"/api/profiles/{Encode(userId)}/add", body.GetRawText()); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } public async Task UpdateProfileAsync(string userId, JsonElement body) { var response = await this.SendAsync(HttpMethod.Post, $"/api/profiles/{Encode(userId)}/update", body.GetRawText()); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } public async Task DeleteProfileAsync(string userId, int profileNo) { var response = await this.SendAsync(HttpMethod.Delete, $"/api/profiles/{Encode(userId)}/byProfileNo/{profileNo}"); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } public async Task CopyProfileAsync(string userId, int fromProfileNo, int toProfileNo) { var response = await this.SendAsync(HttpMethod.Post, $"/api/profiles/{Encode(userId)}/copy/{fromProfileNo}/{toProfileNo}"); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } public async Task CheckLocationAsync(string userId, double lat, double lon) @@ -173,8 +216,7 @@ public async Task CopyProfileAsync(string userId, int fromProfileNo, int toProfi public async Task GetPlacesAsync(string userId) { var response = await this.SendAsync(HttpMethod.Get, $"/api/humans/{Encode(userId)}/locations"); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); var json = await response.Content.ReadAsStringAsync(); using var doc = JsonDocument.Parse(json); @@ -225,8 +267,7 @@ public async Task GetPlacesAsync(string userId) var response = await this.SendAsync( HttpMethod.Post, $"/api/humans/{Encode(userId)}/locations/add", body); - EnsureAccountStillExists(response); - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); // A rejected label is reported inside a 200: PoracleNG answers per row so a batch can partly // succeed. Treating the 200 as success stored nothing and told the user it worked. @@ -255,8 +296,9 @@ public async Task DeletePlaceAsync(string userId, string label) { var response = await this.SendAsync( HttpMethod.Post, $"/api/humans/{Encode(userId)}/locations/{Encode(label)}/delete"); - EnsureAccountStillExists(response); + // Read before the general refusal path: a 409 here names the alarms still pointing at the place, + // which is the difference between "could not delete" and knowing what to repoint first. if (response.StatusCode == HttpStatusCode.Conflict) { var conflict = await response.Content.ReadAsStringAsync(); @@ -269,7 +311,7 @@ public async Task DeletePlaceAsync(string userId, string label) throw new PlaceInUseException(rules); } - response.EnsureSuccessStatusCode(); + await EnsureAcceptedAsync(response); } private async Task SendAsync(HttpMethod method, string path, string? body = null) diff --git a/Core/Pgan.PoracleWebNet.Core.Services/PoracleProblemDetails.cs b/Core/Pgan.PoracleWebNet.Core.Services/PoracleProblemDetails.cs index b206d285..34afe119 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/PoracleProblemDetails.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/PoracleProblemDetails.cs @@ -39,11 +39,17 @@ internal static class PoracleProblemDetails /// Reads an explanation out of a response body. Never throws: an unreadable body still has to produce /// something to show, and the alternative is a 500 for a request PoracleNG already described. /// - public static string Describe(string? body) + public static string Describe(string? body) => Describe(body, Unexplained); + + /// + /// As , but says something other than "alarm" when the body explains + /// nothing. The human, profile, area and location routes refuse requests that are not alarms. + /// + public static string Describe(string? body, string fallback) { if (string.IsNullOrWhiteSpace(body)) { - return Unexplained; + return fallback; } JsonElement root; @@ -56,12 +62,12 @@ public static string Describe(string? body) { // Not JSON. gin's plaintext "404 page not found" lands here, as does an HTML error page from // whatever proxy sits in front. Short bodies are still better than nothing. - return body.Length > 300 ? Unexplained : body.Trim(); + return body.Length > 300 ? fallback : body.Trim(); } if (root.ValueKind != JsonValueKind.Object) { - return Unexplained; + return fallback; } var fieldErrors = FieldErrors(root); @@ -81,7 +87,7 @@ public static string Describe(string? body) } } - return Unexplained; + return fallback; } /// True when this body is PoracleNG's problem+json rather than the v1 shape. diff --git a/Tests/Pgan.PoracleWebNet.Tests/Filters/PoracleRequestRefusedExceptionFilterTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Filters/PoracleRequestRefusedExceptionFilterTests.cs new file mode 100644 index 00000000..f317e1fb --- /dev/null +++ b/Tests/Pgan.PoracleWebNet.Tests/Filters/PoracleRequestRefusedExceptionFilterTests.cs @@ -0,0 +1,84 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using Pgan.PoracleWebNet.Api.Filters; +using Pgan.PoracleWebNet.Core.Models; + +namespace Pgan.PoracleWebNet.Tests.Filters; + +/// +/// The global filter that answers a PoracleNG refusal on the human, profile, area and location routes. +/// +public class PoracleRequestRefusedExceptionFilterTests +{ + private static ActionExecutedContext BuildContext(Exception ex) + { + var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()); + return new ActionExecutedContext(actionContext, [], controller: null!) { Exception = ex }; + } + + private static object? Read(object value, string property) => + value.GetType().GetProperty(property)?.GetValue(value); + + [Fact] + public void AnswersFourHundredCarryingPoraclesOwnWording() + { + var context = BuildContext(new PoracleRequestRefusedException("state is required (true/false)")); + + new PoracleRequestRefusedExceptionFilter().OnActionExecuted(context); + + var result = Assert.IsType(context.Result); + Assert.Equal(StatusCodes.Status400BadRequest, result.StatusCode); + Assert.True(context.ExceptionHandled); + Assert.Equal("state is required (true/false)", Read(result.Value!, "error")); + } + + [Fact] + public void KeepsTheStatusTheRefusalCarries() + { + // "Profile not found" is a 404 upstream and stays one here. Flattening it to 400 would say the + // request was malformed when it named something that is simply not there. + var context = BuildContext(new PoracleRequestRefusedException("Profile not found", 404)); + + new PoracleRequestRefusedExceptionFilter().OnActionExecuted(context); + + Assert.Equal(StatusCodes.Status404NotFound, Assert.IsType(context.Result).StatusCode); + } + + [Fact] + public void UsesTheSameBodyShapeAsEveryOtherRefusal() + { + // The SPA's interceptor reads `error`. A different key here would show a blank snackbar. + var context = BuildContext(new PoracleRequestRefusedException("invalid latitude")); + + new PoracleRequestRefusedExceptionFilter().OnActionExecuted(context); + + Assert.NotNull(Read(Assert.IsType(context.Result).Value!, "error")); + } + + [Fact] + public void LeavesTheAccountGoneFilterAlone() + { + // Every filter runs on every request. Answering 400 for a dead account would cost the SPA the 401 + // it signs out on -- the defect #584 closed. + var context = BuildContext(new AccountGoneException()); + + new PoracleRequestRefusedExceptionFilter().OnActionExecuted(context); + + Assert.Null(context.Result); + Assert.False(context.ExceptionHandled); + } + + [Fact] + public void IgnoresOtherExceptions() + { + var context = BuildContext(new InvalidOperationException("unrelated")); + + new PoracleRequestRefusedExceptionFilter().OnActionExecuted(context); + + Assert.Null(context.Result); + Assert.False(context.ExceptionHandled); + } +} diff --git a/Tests/Pgan.PoracleWebNet.Tests/Services/PoracleHumanProxyRefusalTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Services/PoracleHumanProxyRefusalTests.cs new file mode 100644 index 00000000..e53f97a9 --- /dev/null +++ b/Tests/Pgan.PoracleWebNet.Tests/Services/PoracleHumanProxyRefusalTests.cs @@ -0,0 +1,365 @@ +using System.Net; +using System.Text; +using System.Text.Json; +using Microsoft.Extensions.Configuration; +using Pgan.PoracleWebNet.Core.Models; +using Pgan.PoracleWebNet.Core.Services; + +namespace Pgan.PoracleWebNet.Tests.Services; + +/// +/// What the human, profile, area and location proxy does when PoracleNG refuses. +/// +/// +/// +/// Every status and body quoted here was taken from a live PoracleNG: 5.1.0 on :3040 and 5.2.1 on :3042, +/// probed with deliberately invalid requests. The v1 routes this proxy calls answer 400 identically on +/// both; the 422 bodies come from 5.2.1's /api/v2/humans surface, which the same mistakes reach +/// once a call site moves over. +/// +/// +/// Half of these assert what must keep working rather than what must now fail. A refusal filter that also +/// swallows the account-gone 404 costs the SPA its sign-out; one that swallows the delete-place 409 costs +/// the user the list of alarms blocking the delete. Both are cheaper to catch here than in production. +/// +/// +public class PoracleHumanProxyRefusalTests +{ + private static PoracleHumanProxy CreateSut(MockHandler handler) => + new(new HttpClient(handler), new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Poracle:ApiAddress"] = "http://localhost:3030", + ["Poracle:ApiSecret"] = "test-secret", + }) + .Build()); + + private static PoracleHumanProxy Refusing(HttpStatusCode status, string body) => + CreateSut(new MockHandler(status, body)); + + private static JsonElement Body(string json) => JsonDocument.Parse(json).RootElement; + + // ────────────────────────────────────────────────────────────── + // 400 on the v1 routes -- the caller's mistake, reported as one + // ────────────────────────────────────────────────────────────── + + [Fact] + public async Task AdminDisabledPassesPoracleWordingThroughOn400() + { + // Live: POST /api/humans/{id}/adminDisabled {"nope":1} on 5.1.0 and 5.2.1. + var sut = Refusing(HttpStatusCode.BadRequest, """{"message":"state is required (true/false)","status":"error"}"""); + + var ex = await Assert.ThrowsAsync( + () => sut.AdminDisabledAsync("user1", true)); + + Assert.Equal("state is required (true/false)", ex.Message); + Assert.Equal(400, ex.StatusCode); + } + + [Fact] + public async Task SetLocationPassesPoracleWordingThroughOn400() + { + // Live: POST /api/humans/{id}/setLocation/abc/def. + var sut = Refusing(HttpStatusCode.BadRequest, """{"message":"invalid latitude","status":"error"}"""); + + var ex = await Assert.ThrowsAsync( + () => sut.SetLocationAsync("user1", 0, 0)); + + Assert.Equal("invalid latitude", ex.Message); + } + + [Fact] + public async Task SetAreasPassesPoracleWordingThroughOn400() + { + // Live: POST /api/humans/{id}/setAreas with an object instead of an array. + var sut = Refusing( + HttpStatusCode.BadRequest, + """{"message":"json: cannot unmarshal object into Go value of type []string","status":"error"}"""); + + var ex = await Assert.ThrowsAsync( + () => sut.SetAreasAsync("user1", ["area1"])); + + Assert.Contains("cannot unmarshal object", ex.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task CreateHumanPassesPoracleWordingThroughOn400() + { + var sut = Refusing(HttpStatusCode.BadRequest, """{"message":"id and name are required","status":"error"}"""); + + var ex = await Assert.ThrowsAsync( + () => sut.CreateHumanAsync(Body("""{"garbage":true}"""))); + + Assert.Equal("id and name are required", ex.Message); + } + + [Fact] + public async Task UpdateProfilePassesPoracleWordingThroughOn400() + { + var sut = Refusing(HttpStatusCode.BadRequest, """{"message":"profile_no must be specified","status":"error"}"""); + + var ex = await Assert.ThrowsAsync( + () => sut.UpdateProfileAsync("user1", Body("""{"bad":1}"""))); + + Assert.Equal("profile_no must be specified", ex.Message); + } + + [Fact] + public async Task AddProfilePassesPoracleWordingThroughOn400() + { + var sut = Refusing(HttpStatusCode.BadRequest, """{"message":"invalid request body","status":"error"}"""); + + await Assert.ThrowsAsync( + () => sut.AddProfileAsync("user1", Body("""{"bad":1}"""))); + } + + [Fact] + public async Task DeleteProfilePassesPoracleWordingThroughOn400() + { + var sut = Refusing(HttpStatusCode.BadRequest, """{"message":"invalid profile_no","status":"error"}"""); + + var ex = await Assert.ThrowsAsync( + () => sut.DeleteProfileAsync("user1", 1)); + + Assert.Equal("invalid profile_no", ex.Message); + } + + [Fact] + public async Task CopyProfilePassesPoracleWordingThroughOn400() + { + var sut = Refusing(HttpStatusCode.BadRequest, """{"message":"invalid from profile number","status":"error"}"""); + + var ex = await Assert.ThrowsAsync( + () => sut.CopyProfileAsync("user1", 1, 2)); + + Assert.Equal("invalid from profile number", ex.Message); + } + + [Fact] + public async Task SwitchProfilePassesPoracleWordingThroughOn400() + { + var sut = Refusing(HttpStatusCode.BadRequest, """{"message":"invalid profile number","status":"error"}"""); + + var ex = await Assert.ThrowsAsync( + () => sut.SwitchProfileAsync("user1", 1)); + + Assert.Equal("invalid profile number", ex.Message); + } + + [Fact] + public async Task AddPlacePassesPoracleWordingThroughOn400() + { + var sut = Refusing(HttpStatusCode.BadRequest, """{"message":"invalid request body","status":"error"}"""); + + var ex = await Assert.ThrowsAsync( + () => sut.AddPlaceAsync("user1", new SavedPlace { Label = "home", Latitude = 1, Longitude = 2 })); + + Assert.Equal("invalid request body", ex.Message); + } + + // ────────────────────────────────────────────────────────────── + // 422 -- the same refusal wearing v2's number + // ────────────────────────────────────────────────────────────── + + [Fact] + public async Task ReadsProblemJsonFieldErrorsOn422() + { + // Live: POST /api/v2/humans/{id}/admin-disable {"nope":1} on 5.2.1. + var sut = Refusing(HttpStatusCode.UnprocessableEntity, """ + {"title":"Unprocessable Entity","status":422,"detail":"validation failed", + "errors":[{"message":"expected required property disabled to be present","location":"body"}, + {"message":"unexpected property","location":"body.nope"}]} + """); + + var ex = await Assert.ThrowsAsync( + () => sut.AdminDisabledAsync("user1", true)); + + Assert.Contains("expected required property disabled to be present", ex.Message, StringComparison.Ordinal); + Assert.Contains("nope: unexpected property", ex.Message, StringComparison.Ordinal); + Assert.Equal(400, ex.StatusCode); + } + + [Fact] + public async Task ReadsProblemJsonDetailWhenThereAreNoFieldErrors() + { + var sut = Refusing( + HttpStatusCode.UnprocessableEntity, + """{"title":"Unprocessable Entity","status":422,"detail":"latitude must be between -90 and 90"}"""); + + var ex = await Assert.ThrowsAsync( + () => sut.SetLocationAsync("user1", 999, 0)); + + Assert.Equal("latitude must be between -90 and 90", ex.Message); + } + + [Fact] + public async Task SaysSomethingOtherThanAlarmWhenTheRefusalExplainsNothing() + { + // PoracleProblemDetails' own fallback names an alarm, which is wrong for a profile or a place. + var sut = Refusing(HttpStatusCode.BadRequest, "{}"); + + var ex = await Assert.ThrowsAsync( + () => sut.UpdateProfileAsync("user1", Body("{}"))); + + Assert.DoesNotContain("alarm", ex.Message, StringComparison.OrdinalIgnoreCase); + Assert.NotEmpty(ex.Message); + } + + // ────────────────────────────────────────────────────────────── + // 404 -- account gone, and the two 404s that are not + // ────────────────────────────────────────────────────────────── + + [Fact] + public async Task AMissingAccountIsStillAccountGone() + { + // The SPA signs out only on 401, and AccountGoneExceptionFilter is what produces it. Live wording + // from both 5.1.0 and 5.2.1. + var sut = Refusing(HttpStatusCode.NotFound, """{"message":"User not found","status":"error"}"""); + + await Assert.ThrowsAsync(() => sut.StartAsync("deleted-user")); + } + + [Theory] + [InlineData("/stop")] + [InlineData("/setAreas")] + [InlineData("/profiles")] + public async Task EveryWritePathStillReportsAMissingAccountAsGone(string path) + { + var sut = Refusing(HttpStatusCode.NotFound, """{"message":"User not found","status":"error"}"""); + + Task Call() => path switch + { + "/stop" => sut.StopAsync("deleted-user"), + "/setAreas" => sut.SetAreasAsync("deleted-user", ["area1"]), + _ => sut.GetProfilesAsync("deleted-user"), + }; + + await Assert.ThrowsAsync(Call); + } + + [Fact] + public async Task TheV2WordingForAMissingAccountCountsToo() + { + var sut = Refusing(HttpStatusCode.NotFound, """{"title":"Not Found","status":404,"detail":"human not found"}"""); + + await Assert.ThrowsAsync(() => sut.StopAsync("deleted-user")); + } + + [Fact] + public async Task AMissingProfileIsNotAMissingAccount() + { + // Live: POST /api/humans/{id}/switchProfile/99 answers 404 "Profile not found" on both servers. + // Calling that account-gone signed the user out of a working session -- and inside + // ProfileOverviewService's restore-the-profile finally, it did so while hiding the real failure. + var sut = Refusing(HttpStatusCode.NotFound, """{"message":"Profile not found","status":"error"}"""); + + var ex = await Assert.ThrowsAsync( + () => sut.SwitchProfileAsync("user1", 99)); + + Assert.Equal("Profile not found", ex.Message); + Assert.Equal(404, ex.StatusCode); + } + + [Fact] + public async Task ARouteThisBuildDoesNotHaveIsNotAMissingAccount() + { + // gin answers a plaintext body for an absent route. Signing the user out over it told them their + // account was deleted when the truth was an older PoracleNG. + var sut = Refusing(HttpStatusCode.NotFound, "404 page not found"); + + var ex = await Assert.ThrowsAsync( + () => sut.CopyProfileAsync("user1", 1, 2)); + + Assert.Equal(404, ex.StatusCode); + Assert.Contains("404 page not found", ex.Message, StringComparison.Ordinal); + } + + // ────────────────────────────────────────────────────────────── + // What must keep working + // ────────────────────────────────────────────────────────────── + + [Fact] + public async Task APlaceStillInUseIsStillAConflictNamingTheAlarms() + { + var handler = new MockHandler( + HttpStatusCode.Conflict, + """{"message":"location in use","referencing_rules":["monster 25","raid 5"]}"""); + var sut = CreateSut(handler); + + var ex = await Assert.ThrowsAsync(() => sut.DeletePlaceAsync("user1", "home")); + + Assert.Equal(["monster 25", "raid 5"], ex.ReferencingRules); + } + + [Fact] + public async Task AServerFaultIsStillAServerFault() + { + // 500, 502 and the rest are not the caller's problem and must not be dressed up as a 400. + var sut = Refusing(HttpStatusCode.InternalServerError, "{}"); + + await Assert.ThrowsAsync(() => sut.SetAreasAsync("user1", ["area1"])); + } + + [Fact] + public async Task AConflictOutsideDeletePlaceIsStillAServerFault() + { + // Only the delete-place route gives 409 a meaning. Nothing else should start reading one. + var sut = Refusing(HttpStatusCode.Conflict, "{}"); + + await Assert.ThrowsAsync(() => sut.CreateHumanAsync(Body("{}"))); + } + + [Fact] + public async Task A200StillSucceedsOnEveryPathThisTouches() + { + var handler = new MockHandler(HttpStatusCode.OK, """{"status":"ok"}"""); + var sut = CreateSut(handler); + + await sut.CreateHumanAsync(Body("""{"id":"u1","name":"n"}""")); + await sut.StartAsync("user1"); + await sut.StopAsync("user1"); + await sut.AdminDisabledAsync("user1", true); + await sut.SetLocationAsync("user1", 1, 2); + await sut.SetAreasAsync("user1", ["area1"]); + await sut.SwitchProfileAsync("user1", 1); + await sut.AddProfileAsync("user1", Body("""{"name":"p"}""")); + await sut.UpdateProfileAsync("user1", Body("""{"profile_no":1}""")); + await sut.DeleteProfileAsync("user1", 1); + await sut.CopyProfileAsync("user1", 1, 2); + await sut.DeletePlaceAsync("user1", "home"); + + Assert.Null(await sut.AddPlaceAsync("user1", new SavedPlace { Label = "home" })); + } + + [Fact] + public async Task AddPlaceStillReportsTheRefusalHiddenInsideA200() + { + // PoracleNG answers per row so a batch can partly succeed. This is not a status-code refusal and + // must not turn into one. + var handler = new MockHandler(HttpStatusCode.OK, """{"results":[{"error":"label already used"}]}"""); + var sut = CreateSut(handler); + + Assert.Equal("label already used", await sut.AddPlaceAsync("user1", new SavedPlace { Label = "home" })); + } + + [Fact] + public async Task ReadsThatAnswerNullOnFailureStillAnswerNull() + { + // GetHumanAsync and CheckLocationAsync are read paths whose callers branch on null. Turning their + // failures into throws would break HumanService, ProfileService and TestAlertService at once. + var sut = Refusing(HttpStatusCode.BadRequest, """{"message":"invalid latitude","status":"error"}"""); + + Assert.Null(await sut.GetHumanAsync("user1")); + Assert.Null(await sut.CheckLocationAsync("user1", 999, 999)); + Assert.Null(await sut.GetAreasAsync("user1")); + } + + private sealed class MockHandler(HttpStatusCode statusCode, string responseBody) : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) => + Task.FromResult(new HttpResponseMessage(statusCode) + { + Content = new StringContent(responseBody, Encoding.UTF8, "application/json"), + }); + } +}