Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
using Microsoft.AspNetCore.Mvc.Filters;
using Pgan.PoracleWebNet.Core.Models;

namespace Pgan.PoracleWebNet.Api.Filters;

/// <summary>
/// Turns "your PoracleNG is too old for this" into 409 Conflict, naming the feature and what the
/// server would need.
/// </summary>
/// <remarks>
/// <para>
/// Registered globally beside the other exception filters. Without it a
/// <see cref="PoracleUnsupportedException"/> surfaces as a 500, and before the exception existed the
/// user saw whatever PoracleNG said — usually a column name, which tells them nothing they can act on.
/// </para>
/// <para>
/// 409 rather than 403 on purpose. The SPA's 403 branch is the disabled-feature path and keys off
/// <c>disableKey</c>; borrowing it would put an operator's switch and a version shortfall behind the
/// same toast, and the version shortfall has the one detail worth reading in it. 409 has no
/// interceptor branch, so it falls through to the caller, which shows the message beside the control
/// that caused it — the same route <see cref="TrackingConflictExceptionFilter"/> already takes.
/// </para>
/// <para>
/// A filter rather than a controller pre-check because the throw comes from the service layer:
/// quick-pick apply, profile duplicate and profile import all reach the alarm services without
/// passing an action that could have checked first.
/// </para>
/// </remarks>
public sealed class PoracleUnsupportedExceptionFilter : IExceptionFilter
{
public void OnException(ExceptionContext context)
{
if (context.Exception is not PoracleUnsupportedException ex)
{
return;
}

context.Result = PoracleUnsupportedResponse.Create(ex.Message, ex.Feature, ex.Requires);
context.ExceptionHandled = true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using Microsoft.AspNetCore.Mvc;

namespace Pgan.PoracleWebNet.Api.Filters;

/// <summary>
/// Single source of truth for the HTTP 409 body returned when the PoracleNG on the other end is too
/// old to serve a request.
/// </summary>
/// <remarks>
/// Same reasoning as <see cref="FeatureDisabledResponse"/>: the body is a contract the SPA reads, and
/// more than one path can produce it — the global <see cref="PoracleUnsupportedExceptionFilter"/>
/// today, a controller-level pre-check the first time one is worth having. Keeping the shape in one
/// place is what stops the two answering the same failure differently.
/// </remarks>
internal static class PoracleUnsupportedResponse
{
public static ObjectResult Create(string message, string feature, string requires) => new(new
{
error = message,
feature,
requires
})
{
StatusCode = StatusCodes.Status409Conflict
};
}
1 change: 1 addition & 0 deletions Applications/Pgan.PoracleWebNet.Api/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@
options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.AlarmValidationExceptionFilter>();
options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.TrackingRuleNotFoundExceptionFilter>();
options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.AccountGoneExceptionFilter>();
options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.PoracleUnsupportedExceptionFilter>();
options.Filters.Add<Pgan.PoracleWebNet.Api.Filters.BlockedAccountFilter>();
});

Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- **A request an older Poracle cannot serve now says which feature it was and what the server would need.** Nothing throws it yet, so there is no visible change today: it is the answer waiting for the first control that depends on a newer PoracleNG than the one an instance is pointed at. Until now such a request came back either in Poracle's own wording, which names a database column, or as a plain server error. It now answers with the feature named as this site names it and the version or database migration that would serve it, in words, beside the control that asked -- rather than borrowing the disabled-by-your-administrator wording, which would be untrue and would send the reader looking for a switch nobody turned off.

### Documentation

- **The PoracleNG version compatibility page has been rewritten, and its premise replaced.** It described PoracleNG as having two long-lived branches and this site supporting both; develop shipped as 5.2.1 and merged, so the real question is 5.1.0 versus 5.2.1 and newer. It now covers how support is decided, why a database migration number is a better thing to gate on than a version string, which features need a newer server, how to add another, and two traps worth knowing: PoracleNG's v1 API is unchanged on 5.2.1 -- the new error format and status codes in its release notes apply to v2 only -- and its OpenAPI document numbers the days of the week differently from the scheduler that reads them.
### Changed

- **A refused alarm explains itself the same way whichever Poracle surface answered.** The v2 write path already read PoracleNG's newer RFC 9457 error bodies and named the individual field it refused; the older v1 path, still the one most installs use, was reading only the older shape and answering a validation refusal as though the server had broken. Both paths now share one reader, and where many fields are refused at once the message names the first few and counts the rest rather than rendering a dozen clauses into a snackbar ([#803](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/803)).
Expand Down
66 changes: 66 additions & 0 deletions Core/Pgan.PoracleWebNet.Core.Models/PoracleUnsupportedException.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
namespace Pgan.PoracleWebNet.Core.Models;

/// <summary>
/// The request needs something the PoracleNG on the other end does not have.
/// </summary>
/// <remarks>
/// <para>
/// Distinct from <see cref="FeatureDisabledException"/>, which means an administrator switched
/// something off. Nobody switched anything off here: the request is reasonable and the server is
/// simply an older PoracleNG. Without this the failure arrives either as PoracleNG's own wording,
/// which names a column rather than a feature, or as a generic 500.
/// </para>
/// <para>
/// It carries a plain feature name and the version or schema it would need, not a registry key.
/// Support is decided per feature by <c>IPoracleServerProfileService</c> and the small services on
/// top of it, so there is nothing central to look a key up in — and the words are what the user
/// reads, so they belong at the throw site where somebody knows what the feature is called.
/// </para>
/// <para>
/// Thrown from the service layer rather than checked in a controller so the paths that reach a
/// service without passing a controller action are covered too: quick-pick apply, profile duplicate
/// and profile import all fan out across the alarm services directly. That is the #565 shape.
/// </para>
/// </remarks>
public sealed class PoracleUnsupportedException : Exception
{
/// <param name="feature">What the user was trying to use, in their words, e.g. <c>costume filters</c>.</param>
/// <param name="requires">What the server would need, in words, e.g. <c>PoracleNG 5.2.1 or newer</c>.</param>
public PoracleUnsupportedException(string feature, string requires)
: base($"This PoracleNG does not support {feature}. It requires {requires}.")
{
this.Feature = feature;
this.Requires = requires;
}

public PoracleUnsupportedException() : base("This PoracleNG does not support the requested feature.")
{
this.Feature = string.Empty;
this.Requires = string.Empty;
}

public PoracleUnsupportedException(string message) : base(message)
{
this.Feature = string.Empty;
this.Requires = string.Empty;
}

public PoracleUnsupportedException(string message, Exception innerException)
: base(message, innerException)
{
this.Feature = string.Empty;
this.Requires = string.Empty;
}

/// <summary>The feature that is missing, named as the UI names it.</summary>
public string Feature
{
get;
}

/// <summary>What the server would need, in words, e.g. <c>PoracleNG database migration 6</c>.</summary>
public string Requires
{
get;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
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;

/// <summary>
/// Verifies the global exception filter that maps <see cref="PoracleUnsupportedException"/> to HTTP
/// 409. Like <see cref="FeatureDisabledExceptionFilterTests"/>, this is the safety net for
/// service-to-service callers (quick-pick apply, profile import and duplicate) that never pass a
/// controller action that could have pre-checked.
/// </summary>
public class PoracleUnsupportedExceptionFilterTests
{
private static ExceptionContext BuildContext(Exception ex)
{
var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor());
return new ExceptionContext(actionContext, []) { Exception = ex };
}

private static object? Read(object value, string property) =>
value.GetType().GetProperty(property)?.GetValue(value);

[Fact]
public void MapsUnsupportedExceptionTo409NamingTheFeatureAndWhatItNeeds()
{
var context = BuildContext(new PoracleUnsupportedException("costume filters", "PoracleNG database migration 6"));
var sut = new PoracleUnsupportedExceptionFilter();

sut.OnException(context);

var result = Assert.IsType<ObjectResult>(context.Result);
Assert.Equal(StatusCodes.Status409Conflict, result.StatusCode);
Assert.True(context.ExceptionHandled);
Assert.NotNull(result.Value);
Assert.Equal("costume filters", Read(result.Value, "feature"));
Assert.Equal("PoracleNG database migration 6", Read(result.Value, "requires"));
}

[Fact]
public void ErrorTextSaysWhichFeatureAndWhichVersion()
{
// The whole point of the surface: the reader learns what they asked for and what would serve it,
// instead of PoracleNG's own wording, which names a column.
var context = BuildContext(new PoracleUnsupportedException("pokecoin quest rewards", "PoracleNG 5.2.1 or newer"));

new PoracleUnsupportedExceptionFilter().OnException(context);

var error = Read(Assert.IsType<ObjectResult>(context.Result).Value!, "error") as string;
Assert.NotNull(error);
Assert.Contains("pokecoin quest rewards", error);
Assert.Contains("5.2.1", error);
}

[Fact]
public void DoesNotBorrowTheDisabledFeatureContract()
{
// 403-with-disableKey is the administrator-switched-it-off path and the SPA's 403 branch keys off
// it. An old server is not a disabled feature, and answering with that shape would put the two
// behind the same toast -- losing the one detail here worth reading.
var context = BuildContext(new PoracleUnsupportedException("mutes", "PoracleNG 5.2.1 or newer"));

new PoracleUnsupportedExceptionFilter().OnException(context);

var result = Assert.IsType<ObjectResult>(context.Result);
Assert.NotEqual(StatusCodes.Status403Forbidden, result.StatusCode);
Assert.Null(Read(result.Value!, "disableKey"));
}

[Fact]
public void IgnoresOtherExceptions()
{
var context = BuildContext(new InvalidOperationException("unrelated"));
var sut = new PoracleUnsupportedExceptionFilter();

sut.OnException(context);

Assert.Null(context.Result);
Assert.False(context.ExceptionHandled);
}

[Fact]
public void LeavesTheDisabledFeatureFilterAlone()
{
// Both filters run on every request. Each must decline the other's exception, or whichever is
// registered first answers for both.
var context = BuildContext(new FeatureDisabledException("disable_mons"));

new PoracleUnsupportedExceptionFilter().OnException(context);

Assert.Null(context.Result);
Assert.False(context.ExceptionHandled);
}
}
Loading
Loading