fix(pokestop-events): register the service the controller needs - #825
Merged
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pokéstop Events has been dead since #817 merged. Every endpoint answers 500.
PokestopEventControllertakesIPokestopEventServiceby constructor injection.AddPoracleServicesregistersIPoracleIncidentProxybut never the service, so the controller cannot be resolved and every request — list, add, edit, delete — fails the same way.Why nothing caught it
PokestopEventServicedirectly.IPokestopEventService.A missing registration is invisible until something resolves the controller, which only happens when the application actually runs. This was found by using the dev instance, not by any test — visible in the container log as
InvalidOperationException: Unable to resolve service for type … IPokestopEventService, and in the UI as "An unexpected server error occurred" on an empty page.The guard
ControllerDependencyRegistrationTestsbuilds the realAddPoracleServicescontainer and asserts every controller's required constructor arguments are in it.The word "required" is the whole test.
ScannerControllerandPokemonAvailabilityControllertakeIScannerService? scannerService = nullandIPokemonAvailabilityService? … = null, because the scanner database and the Golbat API are optional and their absence is meant to leave the field null — that is the documented graceful-degradation design and must not be flagged.PokestopEventControllerhas no default, so its dependency is genuinely required.My first version of the test did not make that distinction and produced three false positives, which is worth knowing if it ever needs extending.
It also carries a case asserting the reflection filter finds more than fifteen controllers, so a filter that silently matched nothing could not make every other case vacuously pass.
Verified red first
Reverting just the registration line turns exactly one case red — naming the controller and the missing service — with the other thirty-three green.
Full suite: 2410 passing.
Worth doing after merge
:betawill redeploy on the dev instance; the Pokéstop Events page should be opened there to confirm it now answers, since that is the only place this class of failure shows.