diff --git a/Applications/Pgan.PoracleWebNet.Api/Filters/PoracleUnsupportedExceptionFilter.cs b/Applications/Pgan.PoracleWebNet.Api/Filters/PoracleUnsupportedExceptionFilter.cs new file mode 100644 index 00000000..16e56904 --- /dev/null +++ b/Applications/Pgan.PoracleWebNet.Api/Filters/PoracleUnsupportedExceptionFilter.cs @@ -0,0 +1,41 @@ +using Microsoft.AspNetCore.Mvc.Filters; +using Pgan.PoracleWebNet.Core.Models; + +namespace Pgan.PoracleWebNet.Api.Filters; + +/// +/// Turns "your PoracleNG is too old for this" into 409 Conflict, naming the feature and what the +/// server would need. +/// +/// +/// +/// Registered globally beside the other exception filters. Without it a +/// 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. +/// +/// +/// 409 rather than 403 on purpose. The SPA's 403 branch is the disabled-feature path and keys off +/// disableKey; 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 already takes. +/// +/// +/// 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. +/// +/// +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; + } +} diff --git a/Applications/Pgan.PoracleWebNet.Api/Filters/PoracleUnsupportedResponse.cs b/Applications/Pgan.PoracleWebNet.Api/Filters/PoracleUnsupportedResponse.cs new file mode 100644 index 00000000..ff57b84b --- /dev/null +++ b/Applications/Pgan.PoracleWebNet.Api/Filters/PoracleUnsupportedResponse.cs @@ -0,0 +1,26 @@ +using Microsoft.AspNetCore.Mvc; + +namespace Pgan.PoracleWebNet.Api.Filters; + +/// +/// Single source of truth for the HTTP 409 body returned when the PoracleNG on the other end is too +/// old to serve a request. +/// +/// +/// Same reasoning as : the body is a contract the SPA reads, and +/// more than one path can produce it — the global +/// 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. +/// +internal static class PoracleUnsupportedResponse +{ + public static ObjectResult Create(string message, string feature, string requires) => new(new + { + error = message, + feature, + requires + }) + { + StatusCode = StatusCodes.Status409Conflict + }; +} diff --git a/Applications/Pgan.PoracleWebNet.Api/Program.cs b/Applications/Pgan.PoracleWebNet.Api/Program.cs index a90f0e91..7e4ae63c 100644 --- a/Applications/Pgan.PoracleWebNet.Api/Program.cs +++ b/Applications/Pgan.PoracleWebNet.Api/Program.cs @@ -190,6 +190,7 @@ options.Filters.Add(); options.Filters.Add(); options.Filters.Add(); + options.Filters.Add(); options.Filters.Add(); }); diff --git a/CHANGELOG.md b/CHANGELOG.md index 47232a85..ff302691 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)). diff --git a/Core/Pgan.PoracleWebNet.Core.Models/PoracleUnsupportedException.cs b/Core/Pgan.PoracleWebNet.Core.Models/PoracleUnsupportedException.cs new file mode 100644 index 00000000..3fa65354 --- /dev/null +++ b/Core/Pgan.PoracleWebNet.Core.Models/PoracleUnsupportedException.cs @@ -0,0 +1,66 @@ +namespace Pgan.PoracleWebNet.Core.Models; + +/// +/// The request needs something the PoracleNG on the other end does not have. +/// +/// +/// +/// Distinct from , 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. +/// +/// +/// It carries a plain feature name and the version or schema it would need, not a registry key. +/// Support is decided per feature by IPoracleServerProfileService 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. +/// +/// +/// 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. +/// +/// +public sealed class PoracleUnsupportedException : Exception +{ + /// What the user was trying to use, in their words, e.g. costume filters. + /// What the server would need, in words, e.g. PoracleNG 5.2.1 or newer. + 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; + } + + /// The feature that is missing, named as the UI names it. + public string Feature + { + get; + } + + /// What the server would need, in words, e.g. PoracleNG database migration 6. + public string Requires + { + get; + } +} diff --git a/Tests/Pgan.PoracleWebNet.Tests/Filters/PoracleUnsupportedExceptionFilterTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Filters/PoracleUnsupportedExceptionFilterTests.cs new file mode 100644 index 00000000..85d0dcbc --- /dev/null +++ b/Tests/Pgan.PoracleWebNet.Tests/Filters/PoracleUnsupportedExceptionFilterTests.cs @@ -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; + +/// +/// Verifies the global exception filter that maps to HTTP +/// 409. Like , 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. +/// +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(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(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(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); + } +} diff --git a/docs/architecture/poracleng-compatibility.md b/docs/architecture/poracleng-compatibility.md new file mode 100644 index 00000000..10f77d30 --- /dev/null +++ b/docs/architecture/poracleng-compatibility.md @@ -0,0 +1,146 @@ +# PoracleNG Version Compatibility + +PoracleWeb.NET works against any PoracleNG from 5.1.0 upwards, and switches on the extra features of a +newer one when it finds them. There is nothing to configure. + +The line that matters is **5.1.0 versus 5.2.1 and newer**. There was briefly a second question — whether +you were running PoracleNG's released `main` or its `develop` — and there no longer is: develop shipped +as 5.2.1 and merged. Anything written in terms of branches is out of date, including earlier drafts of +this page. + +Version support still matters, and always will, because self-hosters upgrade on their own schedule. The +PGAN production instance moved to 5.2.1 on 2026-08-24 (migrations 5 through 8 applied cleanly); a server +somewhere is still on 5.1.0, and one is on something older than that. + +!!! warning "5.1.0 is the minimum" + Below it, per-alarm delivery scope, the PVP mega evolution filter and the minimum time-left filter + write columns that do not exist, so those three controls save without complaint and change nothing. + PoracleWeb.NET logs an error at startup and shows the version on **Admin → Settings**. + +## How support is decided + +Three signals, probed together and cached for five minutes. The mechanics are in +[Backend Patterns](backend.md#server-capability-probe); what follows is how to choose between them. + +| Signal | Source | Answers | +|---|---|---| +| Migration number | `schema_migrations` in the Poracle database | Whether a column exists. | +| Version | `GET /health` | Which release line. | +| Capability map | `GET /health` → `capabilities` | Whether a bot or template-editor feature is compiled in. A missing key means unsupported — PoracleNG's own contract for the map. | + +**Every gate fails closed.** An unreachable server, an unread migration number and an unparseable +version all resolve to unsupported. Hiding a control that would have worked is a nuisance; showing one +that writes a column the server does not have is the silent failure this exists to prevent. + +### Prefer the migration number + +Where a column is what actually matters, gate on the schema and not on the release number. A version +string is a claim about a release line, not about this database, and the two come apart routinely: +self-hosters cherry-pick single commits, forks carry one feature and not another, and a locally built +binary reports `0.0.0` because the build flags were never injected. `PoracleServerProfile` already +treats `0.0.0` as unknown rather than ancient for that reason. + +Gate on the version only when there is nothing better to gate on. Pokecoin quest rewards are the honest +example: PoracleNG widened an allowlist on `reward_type`. No column arrived, no migration ran, no +capability key appeared. The version is the only thing that changed, so the version is what decides. + +## What depends on the server version + +| Feature | Signal | Needs | +|---|---|---| +| Costume filter on pokemon alarms | Migration | PoracleNG database migration 6 | +| Costume filter on raid alarms | Migration | PoracleNG database migration 7 | +| Pokéstop-event tracking (`incident`) | v2 API surface | PoracleNG 5.2.1 | +| Mutes | v2 API surface | PoracleNG 5.2.1 | +| Pokecoin quest rewards (`reward_type: 8`) | Version | PoracleNG 5.2.1 | +| Rule descriptions on alarm cards | Response field | v1 `allProfiles`, or any v2 read | + +Each of these carries its own small capability service, shaped like `ISummaryCapabilityService` and +resolving from `IPoracleServerProfileService`. There is deliberately no central registry: a registry +was written and abandoned, because the per-feature shape already existed and two mechanisms answering +one question is how one of them ends up being the one nobody updates. + +Costume names are the awkward one. PoracleNG loads them into its game data under `costume_{id}` keys +but publishes them nowhere -- `/api/masterdata/` offers only `monsters` and `grunts` -- so the costume +picker reads the same WatWowMap masterfile PoracleNG itself reads. The names are therefore English in +every language, and a costume too new for that file shows as its number. + +Costume values set elsewhere are already safe at every version. `TrackingFieldPreserver` and +`PoracleJsonHelper.RewriteRows` carry forward every stored field PoracleWeb.NET has no model for, so a +costume set with the bot survives a web edit on a server that has the column. + +Rule descriptions belong on this list too. A v2 tracking read takes `?include_descriptions=true` and +returns a `description` on each rule -- the same sentence PoracleNG's bot answers a `!pokemon` command +with. It is easy to conclude otherwise: `description` is not on the named `V2PokemonRule` request +schema, and the response envelope that carries it is an inline object under `rules.items` rather than a +named component, so a search of `components/schemas` finds nothing. Resolve +`V2ListOutput...Body.properties.rules.items` before believing a field is absent. + +The v1 `allProfiles` endpoint accepts the same `includeDescriptions` flag, so this one has a path on +both versions. + +Reading and deleting an existing rule is **never** gated, only creating one. A rule can already exist — +set with the bot, or left behind by a downgrade — and a row nobody can see is a row nobody can delete. + +## When a request cannot be served + +`PoracleUnsupportedException` carries a plain feature name and what the server would need, in words. The +global `PoracleUnsupportedExceptionFilter` turns it into **409 Conflict**: + +```json +{ + "error": "This PoracleNG does not support costume filters. It requires PoracleNG database migration 6.", + "feature": "costume filters", + "requires": "PoracleNG database migration 6" +} +``` + +409 rather than 403 on purpose. The 403 branch belongs to `disable_*` site settings and keys off a +`disableKey` in the body; 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 `TrackingConflictExceptionFilter` already takes. + +Throw it from the service layer, not from a controller. Quick-pick apply, profile duplicate and profile +import all reach the alarm services without passing an action that could have checked first. + +## Adding a version-gated feature + +1. **Pick the narrowest signal that predicts it**, per the rule above. Migration number if a column is + involved, capability key if PoracleNG publishes one, version only as a last resort. +2. **Write a small per-feature service over `IPoracleServerProfileService`**, shaped like + `ISummaryCapabilityService`: one method, one question, answering false when it cannot tell. There is + no capability registry and there should not be one — a registry turns every feature into a key + lookup against a table nobody reads, and the interesting part of each gate is the sentence + explaining which signal was chosen and why. +3. **Guard the service write path**, on create, update and bulk alike, and throw + `PoracleUnsupportedException(feature, requires)` with words the user can act on. +4. **Give the SPA a way to ask.** `GET /api/admin/server-profile` is admin-only, so it cannot be the + answer for a user-facing control; `GET /api/summary-schedule/capability` is the pattern to copy. +5. **Prefer hiding the control to disabling it with an explanation.** There is nothing the user can do + about their operator's PoracleNG version. +6. **Add a row to the table above.** + +## Things that look like they differ between versions and do not + +These are the ones most likely to send you writing a check that buys nothing. + +**The v1 API is frozen, and unchanged on 5.2.1.** Its release notes describe RFC 9457 +`application/problem+json` error bodies and 422 status codes, and those are real — on `/api/v2` only. +Verified by calling both surfaces on the same 5.2.1 server: a v1 tracking POST for a user that does not +exist answers `{"message":"User not found","status":"error"}`, byte-identical to 5.1.0, while a +malformed v2 path parameter answers 422 with `Content-Type: application/problem+json` and an `errors` +array. Assuming the new shapes applied everywhere cost a wrongly framed issue and pull request. +PoracleWeb.NET is on v1. + +**Do not take `active_hours` day numbering from PoracleNG's OpenAPI schema.** `V2ActiveHourEntry.day` is +declared `minimum: 0, maximum: 6` and described as "0=Sunday … 6=Saturday". The scheduler uses ISO +weekdays, Monday 1 through Sunday 7 (`isoDow` in `processor/cmd/processor/profiles.go`). The schema +therefore rejects Sunday and accepts a meaningless 0. PoracleWeb.NET validates 1 to 7, which is correct; +`ProfileController.ValidateActiveHours` is the place that would be wrong if somebody "fixed" it against +the document. + +**The `/health` capability map is not where alarm features appear.** Between 5.1.0 and 5.2.1 it gained +exactly one key, `derivedDtsTypes`, and says nothing about costume, incidents or mutes. It covers bot and +template-editor features; the migration number is what covers columns. Reaching for `Supports()` when +`HasSchema()` is the question will quietly answer false forever. diff --git a/docs/index.md b/docs/index.md index e3b28f8d..ee018134 100644 --- a/docs/index.md +++ b/docs/index.md @@ -9,7 +9,7 @@ A web application for managing Pokemon GO notification alarms through the [Porac !!! warning "PoracleNG is required" All alarm management, profile handling, and user operations are proxied through PoracleNG's REST API. [PoracleJS](https://github.com/KartulUdus/PoracleJS) is not a tested or supported configuration — some operations that rely on PoracleNG-specific endpoints will not work. - **PoracleNG 5.1.0 or newer is required.** Older servers have no column to store per-alarm delivery scope, the PVP mega evolution filter or the minimum time-left filter, so those three controls save without complaint and change nothing. PoracleWeb logs an error at startup when it finds an older server, and reports the version on Admin → Settings. + **PoracleNG 5.1.0 or newer is required.** Older servers have no column to store per-alarm delivery scope, the PVP mega evolution filter or the minimum time-left filter, so those three controls save without complaint and change nothing. PoracleWeb logs an error at startup when it finds an older server, and reports the version on Admin → Settings. Which features need a newer server than that, and how support is decided, is in [PoracleNG Version Compatibility](architecture/poracleng-compatibility.md). ## Tech Stack @@ -44,7 +44,7 @@ A web application for managing Pokemon GO notification alarms through the [Porac - **Keyboard Shortcuts** — ++question++ for help, ++bracket-left++ / ++bracket-right++ for sidebar collapse - **11 UI Languages** — Full interface translation (English, French, German, Spanish, Dutch, Italian, Portuguese, Brazilian Portuguese, Polish, Danish, Swedish). Pokemon names, types and forms follow the display language, translated by Poracle itself. What Poracle writes in your DMs is a separate choice, **Alert language**, in the user menu beside **Display language** - **Single Sign-On** — Discord and Telegram login, plus any OIDC provider ([setup](configuration/external-sso.md)), with optional [silent refresh and single logout](configuration/oidc-refresh-tokens.md) -- **Admin Panel** — User management, webhook configuration, site settings, geofence submission review +- **Admin Panel** — User management, webhook configuration, site settings, geofence submission review - **[Webhooks & Delegates](features/webhooks.md)** — Channel feeds managed as their own accounts, with named people allowed to manage one without being made an administrator - **Test Alerts** — Send a sample notification from an alarm card to preview exactly what your alerts look like (all types except Fort Changes and Max Battles) - **Weather Display** — View current in-game weather at your pin and across all tracked areas on the dashboard @@ -96,6 +96,8 @@ A web application for managing Pokemon GO notification alarms through the [Porac [:octicons-arrow-right-24: Architecture Overview](architecture/overview.md) + [:octicons-arrow-right-24: PoracleNG Version Compatibility](architecture/poracleng-compatibility.md) + - :material-map-marker-radius:{ .lg .middle } **Custom Geofences** --- diff --git a/mkdocs.yml b/mkdocs.yml index cc3e0568..842c7e65 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -85,6 +85,7 @@ nav: - Architecture: - Overview: architecture/overview.md - PoracleNG API Proxy: architecture/poracleng-proxy.md + - PoracleNG Version Compatibility: architecture/poracleng-compatibility.md - Backend Patterns: architecture/backend.md - Frontend Patterns: architecture/frontend.md - Database: architecture/database.md