diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fe31515..17dafbf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,11 +20,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Nothing on screen: the last code that could write an alarm straight to Poracle's database has been taken out.** Every alarm write has gone through PoracleNG's API since 2.0, so its deduplication, its field defaults and its immediate state reload all run -- but the database tables were still mapped in code beside it, one line away from being used again. That mapping is gone, along with a set of profile methods nothing had called since the same migration. The two places that still reach the alarm tables directly are unchanged and are there for reasons written down beside them. + - **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)). - **Pokemon alarm edits are written through PoracleNG 5.2.1’s strict `/api/v2` surface, where the server can tell an edit apart from a takeover.** Nothing changes on screen. What changes is underneath: an edit now addresses the rule by its id, so Poracle refuses outright if the uid is not yours or if the result would duplicate an alarm you already have, instead of PoracleWeb.NET having to work that out from a success response and undo it afterwards. Poracle also explains a rejected filter field by name now, so the message on the dialog says which one. Anything older than 5.2.0 keeps the surface it has always used, unchanged, and so do the other nine alarm types; an edit carrying anything the new surface cannot express takes the old path rather than failing. Set `PORACLE_TRACKING_API_VERSION` to `v1` or `v2` to pin it ([#805](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/805)). ### Fixed +- **Turning on auto-delete for invasions stops turning it on for your Pokéstop Events too.** Showcase, Kecleon and Gold Stop rules live in the same table invasions do, and the invasion switch on the Cleaning page was sweeping them up: it read every row of that table, set the auto-delete bit on all of them and wrote them back, so a switch that never mentions Pokéstop Events was quietly changing them. The invasion list and the dashboard both draw that line already; cleaning did not. It now leaves them alone -- unless the Pokéstop Events page is switched off entirely, in which case the invasion switch is the only control you have over those rules and it keeps them, as it always did. - **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 beside them is darker now too -- it was legible, but thin ([#800](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/800)). - **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. @@ -36,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Documentation +- **Why this site is not going to start requiring PoracleNG 5.2.0 is now written down.** Moving to Poracle's newer API was the obvious moment to make 5.2.0 a hard minimum and delete the older code paths, and it was refused: a self-hoster on 5.1.0 would lose their Areas page, their saved places, the location pin, delegated webhooks and all alarm editing, and almost nothing would be gained in exchange, because the newer surface mostly renames things rather than adding them. The compatibility page now carries that reasoning, and the enhancement-request page carries the nine findings behind it -- each one confirmed by calling a 5.2.1 server rather than reading its source, including two that stop whole features moving: a Sunday schedule cannot be expressed on the new API at all, and an invasion rule for a named grunt cannot be read back and written again. +- **A workaround this site has carried for months turns out to rest on something that does not happen.** Per-alarm delivery scope is written through a class built around Poracle refusing a private geofence name. Tested properly for the first time -- with a real user-drawn geofence rather than a made-up name, from an account proven to have no admin rights -- 5.1.0 and 5.2.1 both accepted it and stored it, on both API surfaces. The workaround stays, because it produces the right result either way and nobody should delete one on an unexplained result, but the finding is recorded with its method so whoever removes it does so on evidence. - **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 merged and shipped, so the real question is 5.1.0 versus 5.2.0 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. - **The documentation has been audited against the code and brought up to date with everything that landed this cycle.** Nine changes shipped and none of them were written down, so the pages a self-hoster reads described a site with ten alarm types, five quest reward tabs, no costume filter and no way to quiet anything. Corrected throughout: the alarm type tables and feature lists now carry all eleven types, the test-alert coverage arithmetic is right in the three places that state it, the scope chip is on seven card types rather than six, and the claim that a Pokemon edit keeps its rule id is gone -- it stopped being true when edits moved to the newer write surface. **Quiet periods have a page of their own**, covering what they are, how they differ from *Pause Alerts*, why one rule cannot be quieted on its own, and that a restart of the Poracle processor clears them. Costume filters, PokéCoin rewards, Pokéstop Events, rule descriptions and repeating schedule rules are all written up where they belong, each with the version or database migration it needs. - **Every version requirement in the docs said 5.2.1 and every gate in the code says 5.2.0.** 5.2.1 is the release production happens to run, not the release that added the features, so a self-hoster on exactly 5.2.0 was told four working features were unavailable to them. Corrected on the compatibility page, and the page now names the three constants it is describing so the next drift is visible. diff --git a/CLAUDE.md b/CLAUDE.md index 0908585a..123ee39a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -97,8 +97,8 @@ Pgan.PoracleWebNet.slnx - **PoracleNG handles**: field defaults (template, PVP, size, etc.), dedup detection, immediate state reload on every mutation, grunt_type normalization, area dual-writes on profile switch. ### Repository Layer -- Repositories remain for **non-alarm** data: `HumanRepository` (admin bulk ops only -- `GetAllAsync`, `DeleteUserAsync`), `ProfileRepository` (admin bulk ops and non-active profile cleanup in `UserGeofenceService`), and all PoracleWeb-owned tables (`SiteSettingRepository`, `WebhookDelegateRepository`, `UserGeofenceRepository`, `QuickPickDefinitionRepository`, `QuickPickAppliedStateRepository`). Single-user human and profile reads/writes are fully proxied through `IPoracleHumanProxy`. -- **Removed**: 8 alarm repository classes (MonsterRepository, RaidRepository, etc.), `BaseRepository`, `PoracleUnitOfWork`, `IUnitOfWork`, and all alarm repository interfaces. `EnsureNotNullDefaults()` is no longer needed for alarm writes (PoracleNG handles NULL defaults). +- Repositories remain for **non-alarm** data: `HumanRepository` (admin bulk ops only -- `GetAllAsync`, `DeleteUserAsync`), `ProfileRepository` (down to `RenameAsync` and `UpdateAsync` -- the two writes PoracleNG's API cannot serve), and all PoracleWeb-owned tables (`SiteSettingRepository`, `WebhookDelegateRepository`, `UserGeofenceRepository`, `QuickPickDefinitionRepository`, `QuickPickAppliedStateRepository`). Single-user human and profile reads/writes are fully proxied through `IPoracleHumanProxy`. +- **Removed**: 8 alarm repository classes (MonsterRepository, RaidRepository, etc.), `BaseRepository`, `PoracleUnitOfWork`, `IUnitOfWork`, and all alarm repository interfaces. `PoracleContext` no longer maps the eight alarm tables at all -- the DbSets, the entity classes and their `OnModelCreating` configuration are gone, so a direct alarm write is not one `DbSet` away. `IUserAreaDualWriter` reaches those tables by raw SQL over a validated table name, not through EF. `EnsureNotNullDefaults()` is no longer needed for alarm writes (PoracleNG handles NULL defaults). ### Manual Mapping Extensions - **No AutoMapper dependency.** All mappings use static extension methods in `Core.Mappings/`. diff --git a/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IProfileRepository.cs b/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IProfileRepository.cs index 0f511667..ee23cf14 100644 --- a/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IProfileRepository.cs +++ b/Core/Pgan.PoracleWebNet.Core.Abstractions/Repositories/IProfileRepository.cs @@ -2,11 +2,12 @@ namespace Pgan.PoracleWebNet.Core.Abstractions.Repositories; +/// +/// The two profile writes PoracleNG's API cannot serve. Everything else about profiles — reads, +/// create, delete, copy, switch — goes through IPoracleHumanProxy. +/// public interface IProfileRepository { - public Task> GetByUserAsync(string userId); - public Task GetByUserAndProfileNoAsync(string userId, int profileNo); - public Task CreateAsync(Profile profile); public Task UpdateAsync(Profile profile); /// /// Renames a profile, touching only profiles.name. @@ -20,6 +21,4 @@ public interface IProfileRepository /// /// false if no such profile exists. public Task RenameAsync(string userId, int profileNo, string name); - - public Task DeleteAsync(string userId, int profileNo); } diff --git a/Core/Pgan.PoracleWebNet.Core.Abstractions/Services/IProfileService.cs b/Core/Pgan.PoracleWebNet.Core.Abstractions/Services/IProfileService.cs index 5caf1b7e..47b9a27a 100644 --- a/Core/Pgan.PoracleWebNet.Core.Abstractions/Services/IProfileService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Abstractions/Services/IProfileService.cs @@ -6,8 +6,5 @@ public interface IProfileService { public Task> GetByUserAsync(string userId); public Task GetByUserAndProfileNoAsync(string userId, int profileNo); - public Task CreateAsync(Profile profile); - public Task UpdateAsync(Profile profile); - public Task DeleteAsync(string userId, int profileNo); public Task CopyAsync(string userId, int fromProfileNo, int toProfileNo); } diff --git a/Core/Pgan.PoracleWebNet.Core.Repositories/ProfileRepository.cs b/Core/Pgan.PoracleWebNet.Core.Repositories/ProfileRepository.cs index d63d9bc2..3f2af19a 100644 --- a/Core/Pgan.PoracleWebNet.Core.Repositories/ProfileRepository.cs +++ b/Core/Pgan.PoracleWebNet.Core.Repositories/ProfileRepository.cs @@ -12,31 +12,6 @@ public class ProfileRepository(PoracleContext context) : IProfileRepository { private readonly PoracleContext _context = context; - public async Task> GetByUserAsync(string userId) - { - var entities = await this._context.Profiles - .Where(p => p.Id == userId) - .ToListAsync(); - - return entities.Select(e => e.ToModel()); - } - - public async Task GetByUserAndProfileNoAsync(string userId, int profileNo) - { - var entity = await this._context.Profiles - .FirstOrDefaultAsync(p => p.Id == userId && p.ProfileNo == profileNo); - - return entity is null ? null : entity.ToModel(); - } - - public async Task CreateAsync(Profile profile) - { - var entity = profile.ToEntity(); - this._context.Profiles.Add(entity); - await this._context.SaveChangesAsync(); - return entity.ToModel(); - } - public async Task UpdateAsync(Profile profile) { var entity = await this._context.Profiles @@ -63,19 +38,4 @@ public async Task RenameAsync(string userId, int profileNo, string name) await this._context.SaveChangesAsync(); return true; } - - public async Task DeleteAsync(string userId, int profileNo) - { - var entity = await this._context.Profiles - .FirstOrDefaultAsync(p => p.Id == userId && p.ProfileNo == profileNo); - - if (entity is null) - { - return false; - } - - this._context.Profiles.Remove(entity); - await this._context.SaveChangesAsync(); - return true; - } } diff --git a/Core/Pgan.PoracleWebNet.Core.Services/CleaningService.cs b/Core/Pgan.PoracleWebNet.Core.Services/CleaningService.cs index a5013546..4d0f7e69 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/CleaningService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/CleaningService.cs @@ -36,7 +36,7 @@ public async Task> GetCleanStatusAsync(string userId, i ["raids"] = AllClean(allTracking, "raid"), ["eggs"] = AllClean(allTracking, "egg"), ["quests"] = AllClean(allTracking, "quest"), - ["invasions"] = AllClean(allTracking, "invasion"), + ["invasions"] = AllClean(allTracking, "invasion", await this.OwnRowPredicateAsync("invasion")), ["lures"] = AllClean(allTracking, "lure"), ["nests"] = AllClean(allTracking, "nest"), ["gyms"] = AllClean(allTracking, "gym"), @@ -72,6 +72,35 @@ public async Task ToggleCleanMaxBattlesAsync(string userId, int profileNo, await this.ToggleCleanAsync("maxbattle", userId, clean); + /// + /// Which rows of a tracking type this page owns. + /// + /// + /// Invasion and incident rules share one table, and PoracleWeb reads invasions over v1, which does + /// not partition them. So a clean toggle over "invasion" swept up the user's Kecleon, Showcase and + /// Gold Stop rules, set the auto-delete bit on them and POSTed them back through the invasion + /// endpoint -- a write to rows that belong to the Pokestop Events page, from a switch that never + /// mentions it. and DashboardService both draw this line at the + /// same boundary; cleaning did not. + /// + /// + /// Conditional for the same reason as : when the Pokestop Events + /// surface is switched off there is no other page holding those rows, and the invasion switch is + /// the only control the user has over them. + /// + private async Task> OwnRowPredicateAsync(string type) + { + if (type != "invasion" || !await this._featureGate.IsEnabledAsync(DisableFeatureKeys.PokestopEvents)) + { + return _ => true; + } + + return row => !PokestopEventTypes.IsEventName( + row.TryGetProperty("grunt_type", out var gt) && gt.ValueKind == JsonValueKind.String + ? gt.GetString() + : null); + } + /// /// Workaround: PoracleNG has no bulk clean toggle endpoint. We fetch all alarms of the type, /// set the clean field on each, and POST them back via CreateAsync (which upserts by UID). @@ -99,10 +128,18 @@ private async Task ToggleCleanAsync(string type, string userId, int clean) return 0; } - var count = trackingJson.GetArrayLength(); + var isOurs = await this.OwnRowPredicateAsync(type); + var ownRows = trackingJson.EnumerateArray().Where(isOurs).ToList(); + + if (ownRows.Count == 0) + { + return 0; + } + + var count = ownRows.Count; var updatedAlarms = new JsonArray(); - foreach (var alarm in trackingJson.EnumerateArray()) + foreach (var alarm in ownRows) { var dict = JsonSerializer.Deserialize>(alarm.GetRawText())!; @@ -121,7 +158,7 @@ private async Task ToggleCleanAsync(string type, string userId, int clean) // upsert on uid and must not be deleted. if (InsertOnlyTypes.Contains(type)) { - var uids = trackingJson.EnumerateArray() + var uids = ownRows .Where(a => a.TryGetProperty("uid", out var u) && u.ValueKind == JsonValueKind.Number) .Select(a => a.GetProperty("uid").GetInt32()) .ToList(); @@ -135,7 +172,8 @@ private async Task ToggleCleanAsync(string type, string userId, int clean) catch { // Put the originals back rather than leaving the user with no alarms at all. - await this._trackingProxy.CreateAsync(type, userId, trackingJson); + await this._trackingProxy.CreateAsync( + type, userId, JsonSerializer.SerializeToElement(ownRows)); throw; } @@ -167,15 +205,24 @@ await BulkUidRemap.ApplyAsync( /// Checks whether all items in a tracking array have clean == true or clean == 1. /// Returns false if the array is empty or missing. /// - private static bool AllClean(JsonElement root, string key) + private static bool AllClean(JsonElement root, string key, Func? isOurs = null) { if (!root.TryGetProperty(key, out var arr) || arr.ValueKind != JsonValueKind.Array || arr.GetArrayLength() == 0) { return false; } + var any = false; + foreach (var item in arr.EnumerateArray()) { + if (isOurs != null && !isOurs(item)) + { + continue; + } + + any = true; + if (!item.TryGetProperty("clean", out var cleanVal)) { return false; @@ -200,6 +247,7 @@ private static bool AllClean(JsonElement root, string key) } } - return true; + // An array holding nothing but somebody else's rows is the empty case, not the all-clean one. + return any; } } diff --git a/Core/Pgan.PoracleWebNet.Core.Services/ProfileService.cs b/Core/Pgan.PoracleWebNet.Core.Services/ProfileService.cs index 730f577a..fdb5b0e7 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/ProfileService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/ProfileService.cs @@ -1,22 +1,17 @@ using System.Text.Json; -using Pgan.PoracleWebNet.Core.Abstractions.Repositories; using Pgan.PoracleWebNet.Core.Abstractions.Services; using Pgan.PoracleWebNet.Core.Models; namespace Pgan.PoracleWebNet.Core.Services; /// -/// Proxy-first service for profile reads. Create/Update/Delete are already proxied by -/// ProfileController via IPoracleHumanProxy; this service provides reads for -/// LocationController, UserGeofenceService, and ProfileController.GetAll. -/// IProfileRepository is kept for non-active profile operations in UserGeofenceService. +/// Profile reads and copy, all served by PoracleNG. Create, delete and switch are proxied directly by +/// ProfileController through IPoracleHumanProxy; rename and the area/coordinate write go straight to +/// the database through IProfileRepository, because PoracleNG's API serves neither. /// -public class ProfileService( - IProfileRepository repository, - IPoracleHumanProxy humanProxy) : IProfileService +public class ProfileService(IPoracleHumanProxy humanProxy) : IProfileService { - private readonly IProfileRepository _repository = repository; private readonly IPoracleHumanProxy _humanProxy = humanProxy; public async Task> GetByUserAsync(string userId) @@ -80,12 +75,6 @@ private async Task> WithActiveProfileAsync(string userId, L return profiles.Find(p => p.ProfileNo == profileNo); } - public async Task CreateAsync(Profile profile) => await this._repository.CreateAsync(profile); - - public async Task UpdateAsync(Profile profile) => await this._repository.UpdateAsync(profile); - - public async Task DeleteAsync(string userId, int profileNo) => await this._repository.DeleteAsync(userId, profileNo); - public async Task CopyAsync(string userId, int fromProfileNo, int toProfileNo) => await this._humanProxy.CopyProfileAsync(userId, fromProfileNo, toProfileNo); diff --git a/Core/Pgan.PoracleWebNet.Core.Services/UserGeofenceService.cs b/Core/Pgan.PoracleWebNet.Core.Services/UserGeofenceService.cs index 6cfc663e..04aaf5b2 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/UserGeofenceService.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/UserGeofenceService.cs @@ -15,6 +15,7 @@ public partial class UserGeofenceService( IPoracleApiProxy poracleApiProxy, IPoracleHumanProxy humanProxy, IHumanRepository humanRepository, + IHumanService humanService, IUserAreaDualWriter areaWriter, IDiscordNotificationService discordNotificationService, IFeatureGate featureGate, @@ -28,6 +29,7 @@ public partial class UserGeofenceService( private readonly IPoracleApiProxy _poracleApiProxy = poracleApiProxy; private readonly IPoracleHumanProxy _humanProxy = humanProxy; private readonly IHumanRepository _humanRepository = humanRepository; + private readonly IHumanService _humanService = humanService; private readonly IUserAreaDualWriter _areaWriter = areaWriter; private readonly IDiscordNotificationService _discordNotificationService = discordNotificationService; private readonly IFeatureGate _featureGate = featureGate; @@ -503,9 +505,19 @@ public async Task SubmitForReviewAsync(string humanId, string koji /// private async Task BuildSubmissionPostAsync(UserGeofence geofence, GeofenceReviewState state) { - // Profile-agnostic on purpose. GetByIdAndProfileAsync(id, 1) also filters on current_profile_no, so - // it finds nobody whose active profile isn't #1 -- which is most people, since the default is 0. - var human = await this._humanRepository.GetByIdAsync(geofence.HumanId); + // Read through the service, which asks PoracleNG rather than the database. Its own failure is + // caught here rather than left to the caller, so the owner's display name degrades on its own + // like every other piece of this card -- the post still goes out, with the mention carrying the + // identity instead of a name. + Human? human = null; + try + { + human = await this._humanService.GetByIdAsync(geofence.HumanId); + } + catch (Exception ex) + { + LogOwnerNameLookupFailed(this._logger, ex, geofence.HumanId); + } double[][]? polygon = null; if (!string.IsNullOrEmpty(geofence.PolygonJson)) @@ -957,6 +969,9 @@ private async Task ReloadGeofencesSafeAsync() [LoggerMessage(Level = LogLevel.Information, Message = "Admin {AdminId} deleted geofence '{KojiName}' (ID {Id}, status: {Status})")] private static partial void LogAdminDeletedGeofence(ILogger logger, string adminId, string kojiName, int id, string status); + [LoggerMessage(Level = LogLevel.Warning, Message = "Could not read the owner of a geofence submission ({HumanId}); the review card will show the mention without a name")] + private static partial void LogOwnerNameLookupFailed(ILogger logger, Exception ex, string humanId); + [LoggerMessage(Level = LogLevel.Warning, Message = "Failed to fetch static map for geofence '{KojiName}'")] private static partial void LogStaticMapFetchFailed(ILogger logger, Exception ex, string kojiName); diff --git a/Core/Pgan.PoracleWebNet.Core.Services/UserOwnedOverrideAreaProxy.cs b/Core/Pgan.PoracleWebNet.Core.Services/UserOwnedOverrideAreaProxy.cs index a54c85ab..9e72618c 100644 --- a/Core/Pgan.PoracleWebNet.Core.Services/UserOwnedOverrideAreaProxy.cs +++ b/Core/Pgan.PoracleWebNet.Core.Services/UserOwnedOverrideAreaProxy.cs @@ -18,6 +18,15 @@ namespace Pgan.PoracleWebNet.Core.Services; /// setAreas, which strips silently, this one rejects. /// /// +/// That premise did not reproduce on 2026-08-24. Tested with a real user-drawn fence +/// carrying userSelectable: false, from a human proven non-admin by the same fence being stripped +/// from setAreas in the same session: 5.1.0 v1, 5.2.1 v1 and 5.2.1 v2 all accepted it and stored +/// it verbatim, as they did a fence name that does not exist. Why is unknown -- the filter may be +/// conditional on configuration those deployments do not set. The class stays because it produces the +/// correct stored value either way, and because nobody should delete a workaround on an unexplained +/// negative. See docs/poracleng-v2-review.md, "override_areas re-test". +/// +/// /// Matching never consults userSelectable — resolveOverride hands the rule's areas /// straight to areaOverlap, a name comparison against the fences the spawn fell in. So the fix /// is to send PoracleNG only the names it will accept, then write the full list into the row diff --git a/Data/Pgan.PoracleWebNet.Data/Entities/EggEntity.cs b/Data/Pgan.PoracleWebNet.Data/Entities/EggEntity.cs deleted file mode 100644 index 63d8e69d..00000000 --- a/Data/Pgan.PoracleWebNet.Data/Entities/EggEntity.cs +++ /dev/null @@ -1,76 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using System.ComponentModel.DataAnnotations.Schema; - -namespace Pgan.PoracleWebNet.Data.Entities; - -[Table("egg")] -public class EggEntity -{ - [Key] - [DatabaseGenerated(DatabaseGeneratedOption.Identity)] - [Column("uid")] - public int Uid - { - get; set; - } - - [Column("id")] - public string Id { get; set; } = null!; - - [Column("ping")] - public string Ping { get; set; } = string.Empty; - - [Column("distance")] - public int Distance - { - get; set; - } - - [Column("team")] - public int Team { get; set; } = 4; - - [Column("level")] - public int Level - { - get; set; - } - - [Column("clean")] - public int Clean - { - get; set; - } - - [Column("template")] - public string? Template - { - get; set; - } - - [Column("exclusive")] - public int Exclusive - { - get; set; - } - - [Column("gym_id")] - public string? GymId - { - get; set; - } - - [Column("rsvp_changes")] - public int RsvpChanges - { - get; set; - } - - [Column("profile_no")] - public int ProfileNo { get; set; } = 1; - - [ForeignKey("Id")] - public HumanEntity? Human - { - get; set; - } -} diff --git a/Data/Pgan.PoracleWebNet.Data/Entities/GymEntity.cs b/Data/Pgan.PoracleWebNet.Data/Entities/GymEntity.cs deleted file mode 100644 index 72614c6f..00000000 --- a/Data/Pgan.PoracleWebNet.Data/Entities/GymEntity.cs +++ /dev/null @@ -1,73 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using System.ComponentModel.DataAnnotations.Schema; - -namespace Pgan.PoracleWebNet.Data.Entities; - -[Table("gym")] -public class GymEntity -{ - [Key] - [DatabaseGenerated(DatabaseGeneratedOption.Identity)] - [Column("uid")] - public int Uid - { - get; set; - } - - [Column("id")] - public string Id { get; set; } = null!; - - [Column("ping")] - public string Ping { get; set; } = string.Empty; - - [Column("distance")] - public int Distance - { - get; set; - } - - [Column("team")] - public int Team - { - get; set; - } - - [Column("slot_changes")] - public int SlotChanges - { - get; set; - } - - [Column("clean")] - public int Clean - { - get; set; - } - - [Column("template")] - public string? Template - { - get; set; - } - - [Column("battle_changes")] - public int BattleChanges - { - get; set; - } - - [Column("gym_id")] - public string? GymId - { - get; set; - } - - [Column("profile_no")] - public int ProfileNo { get; set; } = 1; - - [ForeignKey("Id")] - public HumanEntity? Human - { - get; set; - } -} diff --git a/Data/Pgan.PoracleWebNet.Data/Entities/HumanEntity.cs b/Data/Pgan.PoracleWebNet.Data/Entities/HumanEntity.cs index 0d5fb7ca..f47c7e72 100644 --- a/Data/Pgan.PoracleWebNet.Data/Entities/HumanEntity.cs +++ b/Data/Pgan.PoracleWebNet.Data/Entities/HumanEntity.cs @@ -86,12 +86,4 @@ public string? BlockedAlerts } public ICollection Profiles { get; set; } = []; - public ICollection Monsters { get; set; } = []; - public ICollection Raids { get; set; } = []; - public ICollection Eggs { get; set; } = []; - public ICollection Quests { get; set; } = []; - public ICollection Invasions { get; set; } = []; - public ICollection Lures { get; set; } = []; - public ICollection Nests { get; set; } = []; - public ICollection Gyms { get; set; } = []; } diff --git a/Data/Pgan.PoracleWebNet.Data/Entities/InvasionEntity.cs b/Data/Pgan.PoracleWebNet.Data/Entities/InvasionEntity.cs deleted file mode 100644 index 6784bb54..00000000 --- a/Data/Pgan.PoracleWebNet.Data/Entities/InvasionEntity.cs +++ /dev/null @@ -1,61 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using System.ComponentModel.DataAnnotations.Schema; - -namespace Pgan.PoracleWebNet.Data.Entities; - -[Table("invasion")] -public class InvasionEntity -{ - [Key] - [DatabaseGenerated(DatabaseGeneratedOption.Identity)] - [Column("uid")] - public int Uid - { - get; set; - } - - [Column("id")] - public string Id { get; set; } = null!; - - [Column("ping")] - public string Ping { get; set; } = string.Empty; - - [Column("distance")] - public int Distance - { - get; set; - } - - [Column("gender")] - public int Gender - { - get; set; - } - - [Column("grunt_type")] - public string? GruntType - { - get; set; - } - - [Column("clean")] - public int Clean - { - get; set; - } - - [Column("template")] - public string? Template - { - get; set; - } - - [Column("profile_no")] - public int ProfileNo { get; set; } = 1; - - [ForeignKey("Id")] - public HumanEntity? Human - { - get; set; - } -} diff --git a/Data/Pgan.PoracleWebNet.Data/Entities/LureEntity.cs b/Data/Pgan.PoracleWebNet.Data/Entities/LureEntity.cs deleted file mode 100644 index 12f17164..00000000 --- a/Data/Pgan.PoracleWebNet.Data/Entities/LureEntity.cs +++ /dev/null @@ -1,55 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using System.ComponentModel.DataAnnotations.Schema; - -namespace Pgan.PoracleWebNet.Data.Entities; - -[Table("lures")] -public class LureEntity -{ - [Key] - [DatabaseGenerated(DatabaseGeneratedOption.Identity)] - [Column("uid")] - public int Uid - { - get; set; - } - - [Column("id")] - public string Id { get; set; } = null!; - - [Column("ping")] - public string Ping { get; set; } = string.Empty; - - [Column("distance")] - public int Distance - { - get; set; - } - - [Column("lure_id")] - public int LureId - { - get; set; - } - - [Column("clean")] - public int Clean - { - get; set; - } - - [Column("template")] - public string? Template - { - get; set; - } - - [Column("profile_no")] - public int ProfileNo { get; set; } = 1; - - [ForeignKey("Id")] - public HumanEntity? Human - { - get; set; - } -} diff --git a/Data/Pgan.PoracleWebNet.Data/Entities/MonsterEntity.cs b/Data/Pgan.PoracleWebNet.Data/Entities/MonsterEntity.cs deleted file mode 100644 index 3ab84c37..00000000 --- a/Data/Pgan.PoracleWebNet.Data/Entities/MonsterEntity.cs +++ /dev/null @@ -1,163 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using System.ComponentModel.DataAnnotations.Schema; - -namespace Pgan.PoracleWebNet.Data.Entities; - -[Table("monsters")] -public class MonsterEntity -{ - [Key] - [DatabaseGenerated(DatabaseGeneratedOption.Identity)] - [Column("uid")] - public int Uid - { - get; set; - } - - [Column("id")] - public string Id { get; set; } = null!; - - [Column("pokemon_id")] - public int PokemonId - { - get; set; - } - - [Column("ping")] - public string Ping { get; set; } = string.Empty; - - [Column("distance")] - public int Distance - { - get; set; - } - - [Column("min_iv")] - public int MinIv - { - get; set; - } - - [Column("max_iv")] - public int MaxIv { get; set; } = 100; - - [Column("min_cp")] - public int MinCp - { - get; set; - } - - [Column("max_cp")] - public int MaxCp { get; set; } = 9000; - - [Column("min_level")] - public int MinLevel - { - get; set; - } - - [Column("max_level")] - public int MaxLevel { get; set; } = 55; - - [Column("min_weight")] - public int MinWeight - { - get; set; - } - - [Column("max_weight")] - public int MaxWeight { get; set; } = 9000000; - - [Column("atk")] - public int Atk - { - get; set; - } - - [Column("def")] - public int Def - { - get; set; - } - - [Column("sta")] - public int Sta - { - get; set; - } - - [Column("max_atk")] - public int MaxAtk { get; set; } = 15; - - [Column("max_def")] - public int MaxDef { get; set; } = 15; - - [Column("max_sta")] - public int MaxSta { get; set; } = 15; - - [Column("pvp_ranking_worst")] - public int PvpRankingWorst { get; set; } = 4096; - - [Column("pvp_ranking_best")] - public int PvpRankingBest - { - get; set; - } - - [Column("pvp_ranking_min_cp")] - public int PvpRankingMinCp - { - get; set; - } - - [Column("pvp_ranking_league")] - public int PvpRankingLeague - { - get; set; - } - - [Column("pvp_ranking_cap")] - public int PvpRankingCap - { - get; set; - } - - [Column("form")] - public int Form - { - get; set; - } - - [Column("size")] - public int Size { get; set; } = -1; - - [Column("max_size")] - public int MaxSize { get; set; } = 5; - - [Column("gender")] - public int Gender - { - get; set; - } - - [Column("clean")] - public int Clean - { - get; set; - } - - [Column("template")] - public string? Template - { - get; set; - } - - [Column("profile_no")] - public int ProfileNo { get; set; } = 1; - - [ForeignKey("Id")] - public HumanEntity? Human - { - get; set; - } -} diff --git a/Data/Pgan.PoracleWebNet.Data/Entities/NestEntity.cs b/Data/Pgan.PoracleWebNet.Data/Entities/NestEntity.cs deleted file mode 100644 index 37672c50..00000000 --- a/Data/Pgan.PoracleWebNet.Data/Entities/NestEntity.cs +++ /dev/null @@ -1,67 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using System.ComponentModel.DataAnnotations.Schema; - -namespace Pgan.PoracleWebNet.Data.Entities; - -[Table("nests")] -public class NestEntity -{ - [Key] - [DatabaseGenerated(DatabaseGeneratedOption.Identity)] - [Column("uid")] - public int Uid - { - get; set; - } - - [Column("id")] - public string Id { get; set; } = null!; - - [Column("ping")] - public string Ping { get; set; } = string.Empty; - - [Column("distance")] - public int Distance - { - get; set; - } - - [Column("pokemon_id")] - public int PokemonId - { - get; set; - } - - [Column("min_spawn_avg")] - public int MinSpawnAvg - { - get; set; - } - - [Column("form")] - public int Form - { - get; set; - } - - [Column("clean")] - public int Clean - { - get; set; - } - - [Column("template")] - public string? Template - { - get; set; - } - - [Column("profile_no")] - public int ProfileNo { get; set; } = 1; - - [ForeignKey("Id")] - public HumanEntity? Human - { - get; set; - } -} diff --git a/Data/Pgan.PoracleWebNet.Data/Entities/QuestEntity.cs b/Data/Pgan.PoracleWebNet.Data/Entities/QuestEntity.cs deleted file mode 100644 index 561218d7..00000000 --- a/Data/Pgan.PoracleWebNet.Data/Entities/QuestEntity.cs +++ /dev/null @@ -1,73 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using System.ComponentModel.DataAnnotations.Schema; - -namespace Pgan.PoracleWebNet.Data.Entities; - -[Table("quest")] -public class QuestEntity -{ - [Key] - [DatabaseGenerated(DatabaseGeneratedOption.Identity)] - [Column("uid")] - public int Uid - { - get; set; - } - - [Column("id")] - public string Id { get; set; } = null!; - - [Column("ping")] - public string Ping { get; set; } = string.Empty; - - [Column("distance")] - public int Distance - { - get; set; - } - - [Column("reward")] - public int Reward - { - get; set; - } - - [Column("reward_type")] - public int RewardType - { - get; set; - } - - [Column("shiny")] - public int Shiny - { - get; set; - } - - [Column("clean")] - public int Clean - { - get; set; - } - - [Column("template")] - public string? Template - { - get; set; - } - - [Column("profile_no")] - public int ProfileNo { get; set; } = 1; - - [Column("form")] - public int Form - { - get; set; - } - - [ForeignKey("Id")] - public HumanEntity? Human - { - get; set; - } -} diff --git a/Data/Pgan.PoracleWebNet.Data/Entities/RaidEntity.cs b/Data/Pgan.PoracleWebNet.Data/Entities/RaidEntity.cs deleted file mode 100644 index 199e435f..00000000 --- a/Data/Pgan.PoracleWebNet.Data/Entities/RaidEntity.cs +++ /dev/null @@ -1,94 +0,0 @@ -using System.ComponentModel.DataAnnotations; -using System.ComponentModel.DataAnnotations.Schema; - -namespace Pgan.PoracleWebNet.Data.Entities; - -[Table("raid")] -public class RaidEntity -{ - [Key] - [DatabaseGenerated(DatabaseGeneratedOption.Identity)] - [Column("uid")] - public int Uid - { - get; set; - } - - [Column("id")] - public string Id { get; set; } = null!; - - [Column("pokemon_id")] - public int PokemonId - { - get; set; - } - - [Column("ping")] - public string Ping { get; set; } = string.Empty; - - [Column("distance")] - public int Distance - { - get; set; - } - - [Column("team")] - public int Team { get; set; } = 4; - - [Column("level")] - public int Level - { - get; set; - } - - [Column("form")] - public int Form - { - get; set; - } - - [Column("clean")] - public int Clean - { - get; set; - } - - [Column("template")] - public string? Template - { - get; set; - } - - [Column("move")] - public int Move { get; set; } = 9000; - - [Column("evolution")] - public int Evolution { get; set; } = 9000; - - [Column("exclusive")] - public int Exclusive - { - get; set; - } - - [Column("gym_id")] - public string? GymId - { - get; set; - } - - [Column("rsvp_changes")] - public int RsvpChanges - { - get; set; - } - - [Column("profile_no")] - public int ProfileNo { get; set; } = 1; - - [ForeignKey("Id")] - public HumanEntity? Human - { - get; set; - } -} diff --git a/Data/Pgan.PoracleWebNet.Data/PoracleContext.cs b/Data/Pgan.PoracleWebNet.Data/PoracleContext.cs index a59d8b8e..f03608e5 100644 --- a/Data/Pgan.PoracleWebNet.Data/PoracleContext.cs +++ b/Data/Pgan.PoracleWebNet.Data/PoracleContext.cs @@ -3,18 +3,20 @@ namespace Pgan.PoracleWebNet.Data; +/// +/// The Poracle database, read and written only where PoracleNG's API cannot serve the operation. +/// +/// +/// There are deliberately no alarm entities here. Every tracking write goes through PoracleNG so that +/// its dedup, defaults and state reload run; the two places that still reach the alarm tables directly +/// (IUserAreaDualWriter) do so with raw SQL over a validated table name, not through EF. Mapping +/// the tables again would make a direct alarm write one DbSet away, which is the thing the 2.0 +/// migration existed to prevent. +/// public class PoracleContext(DbContextOptions options) : DbContext(options) { public DbSet Humans => this.Set(); public DbSet Profiles => this.Set(); - public DbSet Monsters => this.Set(); - public DbSet Raids => this.Set(); - public DbSet Eggs => this.Set(); - public DbSet Quests => this.Set(); - public DbSet Invasions => this.Set(); - public DbSet Lures => this.Set(); - public DbSet Nests => this.Set(); - public DbSet Gyms => this.Set(); public DbSet PwebSettings => this.Set(); protected override void OnModelCreating(ModelBuilder modelBuilder) @@ -33,63 +35,5 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) // Ensure pweb_settings.value can hold JSON blobs (quick pick definitions, applied states) modelBuilder.Entity().Property(e => e.Value).HasColumnType("longtext"); - - // Set default values for NOT NULL text columns across all alarm entities - modelBuilder.Entity().Property(e => e.Ping).HasDefaultValue(""); - modelBuilder.Entity().Property(e => e.Ping).HasDefaultValue(""); - modelBuilder.Entity().Property(e => e.Ping).HasDefaultValue(""); - modelBuilder.Entity().Property(e => e.Ping).HasDefaultValue(""); - modelBuilder.Entity().Property(e => e.Ping).HasDefaultValue(""); - modelBuilder.Entity().Property(e => e.Ping).HasDefaultValue(""); - modelBuilder.Entity().Property(e => e.Ping).HasDefaultValue(""); - modelBuilder.Entity().Property(e => e.Ping).HasDefaultValue(""); - - // Human -> Monsters relationship - modelBuilder.Entity() - .HasOne(m => m.Human) - .WithMany(h => h.Monsters) - .HasForeignKey(m => m.Id); - - // Human -> Raids relationship - modelBuilder.Entity() - .HasOne(r => r.Human) - .WithMany(h => h.Raids) - .HasForeignKey(r => r.Id); - - // Human -> Eggs relationship - modelBuilder.Entity() - .HasOne(e => e.Human) - .WithMany(h => h.Eggs) - .HasForeignKey(e => e.Id); - - // Human -> Quests relationship - modelBuilder.Entity() - .HasOne(q => q.Human) - .WithMany(h => h.Quests) - .HasForeignKey(q => q.Id); - - // Human -> Invasions relationship - modelBuilder.Entity() - .HasOne(i => i.Human) - .WithMany(h => h.Invasions) - .HasForeignKey(i => i.Id); - - // Human -> Lures relationship - modelBuilder.Entity() - .HasOne(l => l.Human) - .WithMany(h => h.Lures) - .HasForeignKey(l => l.Id); - - // Human -> Nests relationship - modelBuilder.Entity() - .HasOne(n => n.Human) - .WithMany(h => h.Nests) - .HasForeignKey(n => n.Id); - - // Human -> Gyms relationship - modelBuilder.Entity() - .HasOne(g => g.Human) - .WithMany(h => h.Gyms) - .HasForeignKey(g => g.Id); } } diff --git a/Tests/Pgan.PoracleWebNet.Tests/Configuration/ControllerDependencyRegistrationTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Configuration/ControllerDependencyRegistrationTests.cs index 7adcef1d..79e9c516 100644 --- a/Tests/Pgan.PoracleWebNet.Tests/Configuration/ControllerDependencyRegistrationTests.cs +++ b/Tests/Pgan.PoracleWebNet.Tests/Configuration/ControllerDependencyRegistrationTests.cs @@ -41,6 +41,15 @@ public class ControllerDependencyRegistrationTests "IMemoryCache", "IServiceProvider", "IHttpContextAccessor", + "IServiceScopeFactory", + "ILoggerProvider", + "IConfigureOptions`1", + "IPostConfigureOptions`1", + "IValidateOptions`1", + "IOptionsChangeTokenSource`1", + "IOptionsFactory`1", + "IOptionsMonitorCache`1", + "IMetricsListener", ]; public static TheoryData Controllers() @@ -113,4 +122,93 @@ private static HashSet BuildRegistrations() : d.ServiceType.Name) .ToHashSet(StringComparer.Ordinal); } + + /// + /// The same check one level deeper: every interface a registered implementation asks for must also + /// be registered. + /// + /// + /// The controller sweep above only sees a controller's own constructor. A service that gains a + /// dependency -- UserGeofenceService taking IHumanService, say -- is invisible to it, + /// and an unregistered one there fails exactly the same way: the controller resolves, its service + /// does not, and the endpoint answers 500. + /// + [Fact] + public void EveryRegisteredImplementationsDependenciesAreRegistered() + { + var services = BuildServices(); + var registered = services + .Select(d => d.ServiceType.IsGenericType + ? d.ServiceType.GetGenericTypeDefinition().Name + : d.ServiceType.Name) + .ToHashSet(StringComparer.Ordinal); + + var missing = new List(); + + foreach (var implementation in services + .Select(d => d.ImplementationType) + .Where(t => t is not null && !t.IsAbstract && IsOurs(t)) + .Distinct() + .Cast()) + { + var ctor = implementation.GetConstructors() + .OrderByDescending(c => c.GetParameters().Length) + .FirstOrDefault(); + + if (ctor is null) + { + continue; + } + + foreach (var name in ctor.GetParameters() + .Where(p => !p.HasDefaultValue) + // An IEnumerable always resolves -- to an empty sequence when nothing implements T, + // which is the silent failure, so the element type is what has to be registered. + .Select(p => Unwrap(p.ParameterType)) + .Where(t => t.IsInterface) + .Select(t => t.IsGenericType ? t.GetGenericTypeDefinition().Name : t.Name) + .Where(name => !HostProvided.Contains(name) && !registered.Contains(name))) + { + missing.Add($"{implementation.Name} -> {name}"); + } + } + + Assert.True( + missing.Count == 0, + "AddPoracleServices registers implementations whose own dependencies it does not register: " + + string.Join(", ", missing.Distinct())); + } + + /// + /// Only types this solution owns. Framework registrations bring their own graph and their own + /// platform rules -- AddDataProtection registers KeyManagementOptionsSetup, which + /// takes an IRegistryPolicyResolver that exists on Windows and not on Linux. Walking those + /// asserts something about .NET rather than about this application, and it answers differently on a + /// developer machine and on CI, which is how this test first failed. + /// + private static bool IsOurs(Type type) => + type.Assembly.GetName().Name?.StartsWith("Pgan.PoracleWebNet", StringComparison.Ordinal) == true; + + private static Type Unwrap(Type type) => + type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IEnumerable<>) + ? type.GetGenericArguments()[0] + : type; + + private static ServiceCollection BuildServices() + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddPoracleServices(new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Poracle:ApiAddress"] = "http://localhost:3030", + ["Poracle:ApiSecret"] = "test-secret", + ["Jwt:Secret"] = "test-secret-that-is-long-enough-for-hmac-sha256-signing", + ["ConnectionStrings:PoracleDb"] = "server=localhost;database=poracle;user=root;password=x", + ["ConnectionStrings:PoracleWebDb"] = "server=localhost;database=poracle_web;user=root;password=x", + }) + .Build()); + + return services; + } } diff --git a/Tests/Pgan.PoracleWebNet.Tests/Services/CleaningServiceTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Services/CleaningServiceTests.cs index 5496474d..74047b67 100644 --- a/Tests/Pgan.PoracleWebNet.Tests/Services/CleaningServiceTests.cs +++ b/Tests/Pgan.PoracleWebNet.Tests/Services/CleaningServiceTests.cs @@ -541,4 +541,88 @@ public async Task StatusNoLongerReportsFortChanges() Assert.DoesNotContain("fortchanges", status.Keys); } + + /// + /// Kecleon, Showcase and Gold Stop rules live in the invasion table but belong to the Pokestop + /// Events page. The invasion clean switch used to rewrite them too. + /// + [Fact] + public async Task ToggleCleanInvasionsLeavesPokestopEventRowsAlone() + { + this._featureGate.Setup(g => g.IsEnabledAsync(DisableFeatureKeys.PokestopEvents)).ReturnsAsync(true); + this._proxy.Setup(p => p.GetByUserAsync("invasion", "u1")).ReturnsAsync(CreateJsonArray( + new { uid = 1, clean = 0, grunt_type = "water" }, + new { uid = 2, clean = 0, grunt_type = "kecleon" }, + new { uid = 3, clean = 0, grunt_type = "showcase" })); + + JsonElement sent = default; + this._proxy.Setup(p => p.CreateAsync("invasion", "u1", It.IsAny())) + .Callback((_, _, body) => sent = body.Clone()) + .ReturnsAsync(new TrackingCreateResult([], 0, 1, 0)); + + Assert.Equal(1, await this._sut.ToggleCleanInvasionsAsync("u1", 1, 1)); + Assert.Equal([1], sent.EnumerateArray().Select(r => r.GetProperty("uid").GetInt32())); + } + + /// + /// The legitimate case the partition must not break: with the Pokestop Events surface switched off + /// there is no other page holding those rows, so the invasion switch still owns them. + /// + [Fact] + public async Task ToggleCleanInvasionsIncludesEventRowsWhenThatPageIsOff() + { + this._featureGate.Setup(g => g.IsEnabledAsync(DisableFeatureKeys.PokestopEvents)).ReturnsAsync(false); + this._proxy.Setup(p => p.GetByUserAsync("invasion", "u1")).ReturnsAsync(CreateJsonArray( + new { uid = 1, clean = 0, grunt_type = "water" }, + new { uid = 2, clean = 0, grunt_type = "kecleon" })); + + JsonElement sent = default; + this._proxy.Setup(p => p.CreateAsync("invasion", "u1", It.IsAny())) + .Callback((_, _, body) => sent = body.Clone()) + .ReturnsAsync(new TrackingCreateResult([], 0, 2, 0)); + + Assert.Equal(2, await this._sut.ToggleCleanInvasionsAsync("u1", 1, 1)); + Assert.Equal([1, 2], sent.EnumerateArray().Select(r => r.GetProperty("uid").GetInt32())); + } + + [Fact] + public async Task ToggleCleanInvasionsWritesNothingWhenOnlyEventRowsExist() + { + this._featureGate.Setup(g => g.IsEnabledAsync(DisableFeatureKeys.PokestopEvents)).ReturnsAsync(true); + this._proxy.Setup(p => p.GetByUserAsync("invasion", "u1")).ReturnsAsync(CreateJsonArray( + new { uid = 2, clean = 0, grunt_type = "kecleon" })); + + Assert.Equal(0, await this._sut.ToggleCleanInvasionsAsync("u1", 1, 1)); + this._proxy.Verify(p => p.CreateAsync("invasion", "u1", It.IsAny()), Times.Never); + } + + [Fact] + public async Task GetCleanStatusIgnoresPokestopEventRowsForInvasions() + { + this._featureGate.Setup(g => g.IsEnabledAsync(DisableFeatureKeys.PokestopEvents)).ReturnsAsync(true); + var json = JsonSerializer.SerializeToElement(new + { + invasion = new object[] + { + new { uid = 1, clean = 1, grunt_type = "water" }, + new { uid = 2, clean = 0, grunt_type = "showcase" }, + }, + }); + this._proxy.Setup(p => p.GetAllTrackingAsync("u1")).ReturnsAsync(json); + + Assert.True((await this._sut.GetCleanStatusAsync("u1", 1))["invasions"]); + } + + [Fact] + public async Task GetCleanStatusReportsNotCleanWhenOnlyEventRowsExist() + { + this._featureGate.Setup(g => g.IsEnabledAsync(DisableFeatureKeys.PokestopEvents)).ReturnsAsync(true); + var json = JsonSerializer.SerializeToElement(new + { + invasion = new object[] { new { uid = 2, clean = 1, grunt_type = "showcase" } }, + }); + this._proxy.Setup(p => p.GetAllTrackingAsync("u1")).ReturnsAsync(json); + + Assert.False((await this._sut.GetCleanStatusAsync("u1", 1))["invasions"]); + } } diff --git a/Tests/Pgan.PoracleWebNet.Tests/Services/ProfileServiceTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Services/ProfileServiceTests.cs index 6c39610d..cf8b4321 100644 --- a/Tests/Pgan.PoracleWebNet.Tests/Services/ProfileServiceTests.cs +++ b/Tests/Pgan.PoracleWebNet.Tests/Services/ProfileServiceTests.cs @@ -1,6 +1,5 @@ using System.Text.Json; using Moq; -using Pgan.PoracleWebNet.Core.Abstractions.Repositories; using Pgan.PoracleWebNet.Core.Abstractions.Services; using Pgan.PoracleWebNet.Core.Models; using Pgan.PoracleWebNet.Core.Services; @@ -9,12 +8,10 @@ namespace Pgan.PoracleWebNet.Tests.Services; public class ProfileServiceTests { - private readonly Mock _repository = new(); private readonly Mock _humanProxy = new(); private readonly ProfileService _sut; - public ProfileServiceTests() => this._sut = new ProfileService( - this._repository.Object, this._humanProxy.Object); + public ProfileServiceTests() => this._sut = new ProfileService(this._humanProxy.Object); [Fact] public async Task GetByUserAsyncReturnsProfilesFromProxy() @@ -76,48 +73,10 @@ public async Task GetByUserAndProfileNoAsyncReturnsNullWhenNotFound() }); this._humanProxy.Setup(p => p.GetProfilesAsync("u1")).ReturnsAsync(proxyResponse); - // Profile 99 doesn't exist — proxy returns profiles but none match - // Falls back to DB which also returns null - this._repository.Setup(r => r.GetByUserAndProfileNoAsync("u1", 99)).ReturnsAsync((Profile?)null); - + // Profile 99 does not exist and there is no DB fallback: the proxy's answer is the answer. Assert.Null(await this._sut.GetByUserAndProfileNoAsync("u1", 99)); } - [Fact] - public async Task CreateAsyncDelegates() - { - var profile = new Profile { Id = "u1", ProfileNo = 3, Name = "New" }; - this._repository.Setup(r => r.CreateAsync(profile)).ReturnsAsync(profile); - - var result = await this._sut.CreateAsync(profile); - - Assert.Equal("New", result.Name); - this._repository.Verify(r => r.CreateAsync(profile), Times.Once); - } - - [Fact] - public async Task UpdateAsyncDelegates() - { - var profile = new Profile { Id = "u1", ProfileNo = 1, Name = "Updated" }; - this._repository.Setup(r => r.UpdateAsync(profile)).ReturnsAsync(profile); - await this._sut.UpdateAsync(profile); - this._repository.Verify(r => r.UpdateAsync(profile), Times.Once); - } - - [Fact] - public async Task DeleteAsyncReturnsTrue() - { - this._repository.Setup(r => r.DeleteAsync("u1", 2)).ReturnsAsync(true); - Assert.True(await this._sut.DeleteAsync("u1", 2)); - } - - [Fact] - public async Task DeleteAsyncReturnsFalse() - { - this._repository.Setup(r => r.DeleteAsync("u1", 99)).ReturnsAsync(false); - Assert.False(await this._sut.DeleteAsync("u1", 99)); - } - [Fact] public async Task CopyAsyncCallsProxy() { diff --git a/Tests/Pgan.PoracleWebNet.Tests/Services/UserGeofenceServiceTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Services/UserGeofenceServiceTests.cs index e8965f97..a2241a7b 100644 --- a/Tests/Pgan.PoracleWebNet.Tests/Services/UserGeofenceServiceTests.cs +++ b/Tests/Pgan.PoracleWebNet.Tests/Services/UserGeofenceServiceTests.cs @@ -16,6 +16,7 @@ public class UserGeofenceServiceTests private readonly Mock _poracleApiProxy = new(); private readonly Mock _humanProxy = new(); private readonly Mock _humanRepo = new(); + private readonly Mock _humanService = new(); private readonly Mock _areaWriter = new(); private readonly Mock _discordNotificationService = new(); private readonly Mock _featureGate = new(); @@ -34,6 +35,7 @@ public UserGeofenceServiceTests() this._poracleApiProxy.Object, this._humanProxy.Object, this._humanRepo.Object, + this._humanService.Object, this._areaWriter.Object, this._discordNotificationService.Object, this._featureGate.Object, @@ -304,7 +306,7 @@ public async Task SubmitForReviewAsyncUpdatesStatusToPendingReview() var geofence = new UserGeofence { Id = 1, HumanId = "u1", KojiName = "downtown", Status = "active", PolygonJson = "[[1,2],[3,4],[5,6]]" }; this._repository.Setup(r => r.GetByKojiNameAsync("downtown")).ReturnsAsync(geofence); this._repository.Setup(r => r.UpdateAsync(It.IsAny())).ReturnsAsync((UserGeofence g) => g); - this._humanRepo.Setup(r => r.GetByIdAsync("u1")).ReturnsAsync(new Human { Id = "u1", Name = "TestUser" }); + this._humanService.Setup(h => h.GetByIdAsync("u1")).ReturnsAsync(new Human { Id = "u1", Name = "TestUser" }); this._discordNotificationService.Setup(d => d.CreateGeofenceSubmissionPostAsync(It.IsAny())) .ReturnsAsync((string?)null); @@ -314,6 +316,28 @@ public async Task SubmitForReviewAsyncUpdatesStatusToPendingReview() Assert.NotNull(result.SubmittedAt); } + /// + /// The owner's display name now comes from PoracleNG rather than the database, so a PoracleNG + /// outage must not take the review card down with it -- the post still goes out and the mention + /// carries the identity. + /// + [Fact] + public async Task SubmitForReviewAsyncStillPostsWhenTheOwnerLookupFails() + { + var geofence = new UserGeofence { Id = 1, HumanId = "u1", KojiName = "downtown", Status = "active", PolygonJson = "[[1,2],[3,4],[5,6]]" }; + this._repository.Setup(r => r.GetByKojiNameAsync("downtown")).ReturnsAsync(geofence); + this._repository.Setup(r => r.UpdateAsync(It.IsAny())).ReturnsAsync((UserGeofence g) => g); + this._humanService.Setup(h => h.GetByIdAsync("u1")).ThrowsAsync(new HttpRequestException("Connection refused")); + this._discordNotificationService.Setup(d => d.CreateGeofenceSubmissionPostAsync( + It.Is(p => p.UserId == "u1" && p.UserName == null))) + .ReturnsAsync("thread_456"); + + var result = await this._sut.SubmitForReviewAsync("u1", "downtown"); + + Assert.Equal("pending_review", result.Status); + Assert.Equal("thread_456", result.DiscordThreadId); + } + [Fact] public async Task SubmitForReviewAsyncThrowsWhenNotOwned() { @@ -346,7 +370,7 @@ public async Task SubmitForReviewAsyncSavesDiscordThreadId() var geofence = new UserGeofence { Id = 1, HumanId = "u1", KojiName = "downtown", DisplayName = "Downtown", GroupName = "City", Status = "active", PolygonJson = "[[1,2],[3,4],[5,6]]" }; this._repository.Setup(r => r.GetByKojiNameAsync("downtown")).ReturnsAsync(geofence); this._repository.Setup(r => r.UpdateAsync(It.IsAny())).ReturnsAsync((UserGeofence g) => g); - this._humanRepo.Setup(r => r.GetByIdAsync("u1")).ReturnsAsync(new Human { Id = "u1", Name = "TestUser" }); + this._humanService.Setup(h => h.GetByIdAsync("u1")).ReturnsAsync(new Human { Id = "u1", Name = "TestUser" }); this._discordNotificationService.Setup(d => d.CreateGeofenceSubmissionPostAsync( It.Is(p => p.UserId == "u1" && p.UserName == "TestUser" && p.DisplayName == "Downtown" && p.GroupName == "City" && p.PublicName == "downtown"))) diff --git a/docs/architecture/poracleng-compatibility.md b/docs/architecture/poracleng-compatibility.md index 0950e68e..00c0301a 100644 --- a/docs/architecture/poracleng-compatibility.md +++ b/docs/architecture/poracleng-compatibility.md @@ -23,6 +23,32 @@ somewhere is still on 5.1.0, and one is on something older than that. 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**. +## The v2 migration does not raise the floor + +Moving to PoracleNG's `/api/v2` surface was the obvious moment to make 5.2.0 a hard minimum and delete +the v1 code paths. It was considered and refused, so here is the reasoning in one place rather than +scattered through commit messages. + +Dropping v1 would cost a self-hoster on 5.1.0 their Areas page, their saved places, the location pin, +delegated-webhook resolution and all alarm editing. That is the application, not a feature. + +What it would buy is smaller than it looks. Most of the movable v2 operations are renames with no +behaviour change at all. The genuine gains — the lure edit no longer having to delete and recreate the +row, an upstream 409 refusing an alarm collision instead of PoracleWeb reconstructing it from a 200 — +are workaround *removals*, and the fallback keeps them: the v2 path skips the workaround, the v1 path +keeps it. Nothing user-visible is unlocked by deleting v1. + +Two things follow. Reads deliberately stay on v1, including the tempting full snapshot at +`GET /api/v2/humans/{id}/tracking`; the moment reads move, the internal currency stops being v1-shaped +and the fallback stops being free. And an operation that genuinely has no v1 equivalent — +`PUT /v2/humans/{id}/locations/{label}` is the only one — gets a capability gate and degrades to the +old flow, the pattern `MuteCapabilityService` already sets, rather than forcing a floor for one feature. + +Two v2 surfaces cannot serve PoracleWeb at all yet: invasion, whose reads omit the targeting field for a +named grunt, and bulk distance, which has no batch write. So v1 has to stay in the codebase regardless. +Revisit the floor when upstream closes both — see +[the v2 findings](../poracleng-enhancement-requests.md#v2-findings-for-an-upstream-report). + ## How support is decided Three signals, probed together and cached for five minutes. The mechanics are in diff --git a/docs/poracleng-enhancement-requests.md b/docs/poracleng-enhancement-requests.md index 3bb2bcb5..c1d73446 100644 --- a/docs/poracleng-enhancement-requests.md +++ b/docs/poracleng-enhancement-requests.md @@ -226,6 +226,127 @@ the hook list already in hand. --- +## v2 findings, for an upstream report + +Nine things found while planning the v1-to-v2 migration against **PoracleNG 5.2.1**. Every one below +was confirmed by calling a running 5.2.1 instance on 2026-08-24, not by reading source; the requests +above were mostly derived from source and are older. Two of them decide whether whole surfaces can +move to v2 at all. + +### `active_hours.day` is bounded 0-6 while the scheduler reads ISO 1-7 + +The v2 schema declares `day` as `minimum: 0, maximum: 6` and the migration guide documents it as +"0 = Sunday". The scheduler does not agree: `isoDow` in `processor/cmd/processor/profiles.go` uses ISO +weekdays, Monday 1 through Sunday 7, and nothing translates between the two. + +``` +PATCH /api/v2/humans/{id}/profiles/1 {"active_hours":[{"day":7,...}]} + -> 422 "expected number <= 6" at body.active_hours[0].day +PATCH /api/v2/humans/{id}/profiles/1 {"active_hours":[{"day":0,...}]} + -> 200 +``` + +So through v2 a Sunday schedule cannot be expressed at all, and the value that *is* accepted matches +no weekday. The official migration guide propagates the error, so any client following it writes +schedules that never fire. + +**Consequence here:** v2 `active_hours` writes are blocked. PoracleWeb's own 1-7 validation is correct +and stays. + +### v2 invasion reads omit the targeting field for a named grunt, so GET then PUT is impossible + +v2 requires exactly one of `type_id`, `grunt_id`, `everything`, `boss` on a write. For a rule whose +`grunt_type` is a *type* name it returns `type_id` and round-trips fine. For a rule whose `grunt_type` +is a named grunt it returns **no targeting field of any kind**: + +| stored `grunt_type` | v1 read | v2 read | +|---|---|---| +| `water` | `grunt_type: "water"` | `type_id: 11` | +| `blanche` | `grunt_type: "blanche"` | *(nothing)* | +| `player team leader` | `grunt_type: "player team leader"` | *(nothing)* | +| `npc 0` | `grunt_type: "npc 0"` | *(nothing)* | + +Handing a v2 read straight back to a v2 write therefore fails: + +``` +PUT /api/v2/humans/{id}/tracking/invasion/757 + -> 422 "exactly one of type_id, grunt_id, everything, boss must be set" +``` + +PoracleNG holds the forward name-to-id map and exposes no endpoint for it, and PoracleWeb stores only +the name, so the id cannot be reconstructed on the client either. **Consequence here:** invasion is +excluded from the v2 migration in both directions until a read returns `grunt_id`. + +### `override_areas` is not validated, on either version or either surface + +This is the one that most needs a second opinion, because it contradicts what PoracleWeb was built +around. See [the verification note](poracleng-v2-review.md#override_areas-re-test-2026-08-24) for the +full method. In short: a non-admin's `override_areas` was stored verbatim on 5.1.0 v1, on 5.2.1 v1 and +on 5.2.1 v2 — for a real user-drawn fence carrying `userSelectable: false`, and for a fence name that +does not exist at all. The `setAreas` filter on the same human, in the same session, stripped the same +fence name, so the human was demonstrably non-admin and the filter was demonstrably live. + +There is a privacy edge if this holds: a crafted call can scope a rule to another user's private +geofence name, which leaks nothing by itself but does let one account key alerts off another's area. +Unreachable through PoracleWeb's UI, since #544 stopped those names being listed anywhere. + +### `language` validation depends on configuration the client cannot read + +`POST /api/v2/humans/{id}/language` accepted `"zz"` with 200 and stored it. On a deployment that +configures `general.available_languages` the same call is refused — which is +[#194](https://github.com/jfberry/PoracleNG/issues/194) above, still open. The write also lowercases: +`"DE"` stores `de`. Worth documenting, since a client sending a stored casing back gets a different +string than it sent. + +### `/health` carries no applied-migration number + +`/health` returns capabilities, status and version. It does not say which schema migration has been +applied, and that is a different fact from the version: a 5.2.1 binary pointed at a database whose +migrations did not run reports 5.2.1 and behaves like an older one. PoracleWeb reads +`schema_migrations` directly for exactly this reason — it is the last read-only dependency +`PoracleContext` has. One integer on `/health` would delete `PoracleSchemaVersionReader`, its +interface, its registration and that dependency. + +### No list-humans and no delete-human, on either version + +``` +GET /api/v2/humans -> 404 +GET /api/v2/humans?type=webhook -> 404 +GET /api/humans -> 404 +DELETE /api/v2/humans/{id} -> 404 +``` + +Every human route is single-`{id}`. The admin user list and account deletion therefore keep +`HumanRepository` and its direct database access alive. A `?type=webhook` filter alone would close the +delegated-webhook half of it. + +### Profile create returns no `profile_no` + +`POST /api/v2/humans/{id}/profiles` answers `{"status":"ok"}`. PoracleNG assigns the lowest free +number rather than max + 1, so the caller cannot predict it and must snapshot the list, create, re-read +and diff — on names that are not unique. Returning the created resource, as the rest of v2 does, would +remove that dance. + +### PATCH profile cannot write name, area or coordinates + +``` +PATCH /api/v2/humans/{id}/profiles/1 {"name":"renamed"} + -> 422 "expected required property active_hours to be present" at body + -> 422 "unexpected property" at body.name +``` + +`active_hours` is required and is the only writable field. There is no rename endpoint on either +version, so `IProfileRepository.RenameAsync`'s direct database write stays. + +### v2 `setAreas` keeps the v1 `userSelectable` filter + +Re-confirmed on 5.2.1 rather than taken from source. `POST /api/v2/humans/{id}/areas` with +`["", "aberdeen"]` stored `["aberdeen"]` — silently, 200, no warning, exactly as +v1 does. This is the [trusted setAreas](#trusted-setareas-bypass-userselectable-filter) ask above, and v2 +does not close it. + +--- + ## Summary Table | Gap | Priority | Workaround in Use | Status | diff --git a/docs/poracleng-v2-review.md b/docs/poracleng-v2-review.md index 9f9a532e..2e98b3b8 100644 --- a/docs/poracleng-v2-review.md +++ b/docs/poracleng-v2-review.md @@ -176,3 +176,47 @@ The three **High** asks (trusted setAreas, admin list, batch resolve) are the ga **Phase 4 — new capabilities:** `incident` (four-layer, distinct dictionary), invasion two-axis, `pvp_ranking_evolution`, `fort.include_empty` reconcile, saved-locations editor, `blocked_alerts` consumption. Effort: L. **Phase 5 — gated on bulk PATCH:** replace fetch-modify-POST across 8 alarm services + `CleaningService` with one PATCH; reconcile `poracleng-enhancement-requests.md` with v2. Effort: M. + + +--- + +## `override_areas` re-test, 2026-08-24 + +The claim in the Executive Summary above — that `validateOverrideFields` refuses a `userSelectable: +false` fence with a 400, "the single most important blocker" — **did not reproduce.** It was a source +read against an open pull request, and it has never been confirmed by calling a server. It has now been +tested against two, and both accept. + +**Method.** A real user-drawn geofence was inserted into the dev `poracle_web` database so PoracleWeb's +feed served it with `userSelectable: false` and `displayInMatches: false` — the same shape a user's own +geofence takes. Both PoracleNG instances read that one feed, so both saw it after a reload. A human with +no admin role and no area restriction was created on each. This matters: the earlier probe used a fence +name that did not exist, which never reaches the filter being tested. + +**Control, run on the same human in the same session.** `setAreas` with +`["v2probe-fence", "aberdeen"]` stored `["aberdeen"]` on 5.1.0 v1 and on 5.2.1 v2. So the human was +demonstrably non-admin and the `userSelectable` filter was demonstrably live. + +**Result.** With the filter proven live on the areas surface, the same fence name was accepted and +stored verbatim in `override_areas` by every write path tested: + +| Server | Surface | `override_areas` sent | Stored | +|---|---|---|---| +| 5.1.0 | v1 `POST /api/tracking/pokemon/{id}` | `["v2probe-fence"]` | `["v2probe-fence"]` | +| 5.2.1 | v1 `POST /api/tracking/pokemon/{id}` | `["v2probe-fence"]` | `["v2probe-fence"]` | +| 5.2.1 | v2 `POST /api/v2/humans/{id}/tracking/pokemon` | `["v2probe-fence"]` | `["v2probe-fence"]` | +| 5.1.0 | v1 | `["definitely-not-a-real-fence"]` | stored verbatim | +| 5.2.1 | v2 | `["definitely-not-a-real-fence"]` | stored verbatim | + +All probe rows, the probe humans and the probe geofence were deleted afterwards and both instances +reloaded; the feed is back to its 793 admin fences. + +**What this does and does not settle.** It settles that `UserOwnedOverrideAreaProxy`'s stated premise is +wrong for the exact request shape PoracleWeb sends, on both versions. It does not settle *why* — the +filter may be conditional on configuration this deployment does not set, or the validation may have been +removed. So the class stays: it is currently sending PoracleNG a filtered list and then writing the full +list to the row itself, which produces the correct stored value whether or not validation exists. The +finding is recorded so that whoever removes it does so on evidence rather than on the doc comment. + +It also does **not** weaken blocker 1. The `setAreas` control above re-confirms it on 5.2.1 v2: +`IUserAreaDualWriter` and its `HACK: trusted-set-areas` sites stay on direct DB.