From 35edffe50e142b4bae667023a8dc595386a9d6ac Mon Sep 17 00:00:00 2001 From: hokiepokedad2 <38219945+hokiepokedad2@users.noreply.github.com> Date: Mon, 24 Aug 2026 14:34:22 -0400 Subject: [PATCH] fix(pokestop-events): register the service the controller needs PokestopEventController takes IPokestopEventService by constructor injection and AddPoracleServices never registered it, so every endpoint on it threw at resolution time and answered 500. The feature has been dead since it merged in #817. Nothing in the pipeline could catch it. The service tests construct PokestopEventService directly, the controller tests mock the interface, the solution compiles, and CI is green -- a missing registration is invisible until something resolves the controller, which only happens when the application runs. It was found by using the dev instance, not by a test. ControllerDependencyRegistrationTests closes that gap: it builds the real AddPoracleServices container and asserts every controller's required constructor arguments are in it. Parameters with a default are excluded, and that distinction is the whole test. ScannerController and PokemonAvailabilityController take `IThing? thing = null` because the scanner database and the Golbat API are optional and their absence is meant to leave the field null -- those must NOT be flagged. PokestopEventController has no default, so its dependency is genuinely required. Verified by reverting the registration: exactly one case goes red, naming the controller and the service, with the other thirty-three green. Refs #806 --- .../ServiceCollectionExtensions.cs | 1 + CHANGELOG.md | 1 + .../ControllerDependencyRegistrationTests.cs | 116 ++++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 Tests/Pgan.PoracleWebNet.Tests/Configuration/ControllerDependencyRegistrationTests.cs diff --git a/Applications/Pgan.PoracleWebNet.Api/Configuration/ServiceCollectionExtensions.cs b/Applications/Pgan.PoracleWebNet.Api/Configuration/ServiceCollectionExtensions.cs index a21d9f15..99be717a 100644 --- a/Applications/Pgan.PoracleWebNet.Api/Configuration/ServiceCollectionExtensions.cs +++ b/Applications/Pgan.PoracleWebNet.Api/Configuration/ServiceCollectionExtensions.cs @@ -71,6 +71,7 @@ public static IServiceCollection AddPoracleServices(this IServiceCollection serv services.AddScoped(); services.AddScoped(); services.AddScoped(); + services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); diff --git a/CHANGELOG.md b/CHANGELOG.md index 509a7ba2..63cc589c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Pokéstop Events works at all.** The page shipped unable to answer a single request: the service behind it was never handed to the application at startup, so opening the page, adding an event or deleting one all failed the same way, with the generic server error. Nothing caught it -- the tests for that code substitute the missing piece, so they passed, and the site compiled and deployed green. A new test now builds every page's dependencies the way the running application does, which is the only place this kind of omission shows up. - **The PVP rank range is readable again on a dark-themed alarm card.** The band under a PVP alarm showed its league and nothing else, so the ranks you had set looked like they had been dropped. They were being drawn, in white, on a band that stays light in both themes. The league name gained some contrast on the way past ([#800](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/800)). - **A delegate configured in Poracle's own config can see their webhooks again.** Poracle reports a delegated webhook by the name the operator wrote in `webhook_admins`, and this site matched those strings against the webhook's URL, so a name matched nothing: the *My Webhooks* item appeared in the sidebar, the page it led to was empty, and the button on it would have been refused. Grants are now resolved to the webhook they name, whether the config names it by name or by URL, and a grant that names no webhook at all no longer puts an item in the sidebar ([#797](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/797)). - **Impersonating a delegate shows what the delegate sees.** *My Webhooks* was hidden inside an impersonation session -- the one place an admin looks to find out why someone is complaining -- while the page and its actions would both have answered. Impersonating a second account from inside that session is refused, since only one token can be held for the way back out ([#797](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/797)). diff --git a/Tests/Pgan.PoracleWebNet.Tests/Configuration/ControllerDependencyRegistrationTests.cs b/Tests/Pgan.PoracleWebNet.Tests/Configuration/ControllerDependencyRegistrationTests.cs new file mode 100644 index 00000000..7adcef1d --- /dev/null +++ b/Tests/Pgan.PoracleWebNet.Tests/Configuration/ControllerDependencyRegistrationTests.cs @@ -0,0 +1,116 @@ +using System.Reflection; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Pgan.PoracleWebNet.Api.Configuration; + +namespace Pgan.PoracleWebNet.Tests.Configuration; + +/// +/// Every controller must be constructible from the container the application actually builds. +/// +/// +/// +/// PokestopEventController shipped depending on an IPokestopEventService that +/// never registered. Every endpoint on it +/// answered 500 from the moment it merged. Nothing caught it: the unit tests construct the service +/// directly, the controller tests mock it, the solution compiles, and CI is green -- a missing +/// registration is invisible until something resolves the controller, which only happens when the +/// application runs. +/// +/// +/// So this asserts the one thing those tests cannot: that the real registration method can supply every +/// constructor argument of every controller in the API assembly. It fails when a service is added +/// without its registration, which is the mistake, rather than when a name is misspelled, which the +/// compiler already catches. +/// +/// +public class ControllerDependencyRegistrationTests +{ + /// + /// Supplied by the host rather than by AddPoracleServices, so their absence from the + /// application's own registrations is correct. + /// + private static readonly HashSet HostProvided = + [ + "IConfiguration", + "IHostEnvironment", + "IWebHostEnvironment", + "ILogger`1", + "IHttpClientFactory", + "IMemoryCache", + "IServiceProvider", + "IHttpContextAccessor", + ]; + + public static TheoryData Controllers() + { + var data = new TheoryData(); + + foreach (var controller in typeof(ServiceCollectionExtensions).Assembly + .GetTypes() + .Where(t => typeof(ControllerBase).IsAssignableFrom(t) && !t.IsAbstract) + .OrderBy(t => t.Name)) + { + data.Add(controller); + } + + return data; + } + + [Fact] + public void TheAssemblyActuallyHasControllersToCheck() + { + // A reflection filter that silently matches nothing would make every case below vacuous. + Assert.True(Controllers().Count > 15); + } + + [Theory] + [MemberData(nameof(Controllers))] + public void EveryControllerDependencyIsRegistered(Type controller) + { + var registered = BuildRegistrations(); + + var missing = controller + .GetConstructors() + .SelectMany(c => c.GetParameters()) + // A parameter with a default is the graceful-degradation pattern: the scanner DB and the + // Golbat API are both optional, their controllers take `IThing? thing = null`, and the + // container is expected to leave them null. Only a REQUIRED dependency has to be registered. + .Where(p => !p.HasDefaultValue) + .Select(p => p.ParameterType) + .Select(t => t.IsGenericType ? t.GetGenericTypeDefinition() : t) + .Where(t => t.IsInterface) + .Select(t => t.Name) + .Where(name => !HostProvided.Contains(name) && !registered.Contains(name)) + .Distinct() + .ToList(); + + Assert.True( + missing.Count == 0, + $"{controller.Name} injects {string.Join(", ", missing)}, which AddPoracleServices does not " + + "register. Every request to it would fail to resolve and answer 500."); + } + + private static HashSet BuildRegistrations() + { + 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 + .Select(d => d.ServiceType.IsGenericType + ? d.ServiceType.GetGenericTypeDefinition().Name + : d.ServiceType.Name) + .ToHashSet(StringComparer.Ordinal); + } +}