Refactor: Introduce service layer and thin down controller layer#207
Merged
anderslindho merged 8 commits intoChannelFinder:masterfrom Apr 10, 2026
Merged
Refactor: Introduce service layer and thin down controller layer#207anderslindho merged 8 commits intoChannelFinder:masterfrom
anderslindho merged 8 commits intoChannelFinder:masterfrom
Conversation
anderslindho
requested review from
domonkos-ess,
georgweiss,
imretoth-ess,
jacomago,
shroffk,
simon-ess and
tynanford
anderslindho
force-pushed
the
thin-controller
branch
4 times, most recently
from
3105641 to
2082fad
Compare
- Add LegacyApiProperties @ConfigurationProperties bean for channelfinder.legacy.service-root, normalizing leading/trailing slashes; defaults to ChannelFinder. - Remove hardcoded CFResourceDescriptors constants from all six API interface @RequestMapping declarations; interfaces now carry only HTTP method contracts. - Inject scroll resource URI via @value in ChannelRepository to avoid static constant coupling. - Document the property in application.properties.
anderslindho
force-pushed
the
thin-controller
branch
2 times, most recently
from
fb0a338 to
59eebe9
Compare
jacomago
reviewed
Apr 7, 2026
Contributor
jacomago
left a comment
jacomago
left a comment
There was a problem hiding this comment.
I don't particularly like the name legacy, I would prefer v0 or something clearer. Legacy is confusing as if you have two things called legacy or old, you don't know what the sequence is.
imretoth-ess
reviewed
Apr 8, 2026
anderslindho
force-pushed
the
thin-controller
branch
from
59eebe9 to
7afee66
Compare
Contributor
|
Weird you manage to have both code coverage and integration tests working and now they are failing again... |
…egation All business logic (authorization, validation, orchestration) moves out of controllers into dedicated application services. Controllers now contain only constructor injection and single-line delegation calls. - Add ChannelService, TagService, PropertyService, InfoService, ChannelScrollService. - Move processAllChannels scroll orchestration and CF_ADMIN auth check into ChannelProcessorService; remove IChannelScroll dependency from its controller. - Add seven domain exception types (ChannelNotFoundException, TagNotFoundException, PropertyNotFoundException, *ValidationException, UnauthorizedException). - Add ChannelFinderExceptionHandler (@RestControllerAdvice) mapping domain exceptions to HTTP status codes. - Remove validateChannelRequest/validateTagRequest/validatePropertyRequest from API interfaces; validation now lives entirely in the service layer. - Switch all six controllers to constructor injection. - Update ChannelProcessorServiceTest to match the expanded constructor signature.
Introduce a web package hierarchy that makes the structural intent clear and leaves room for a future web.v1 package alongside it. - rest.api.* -> web.v0.api.* - rest.controller.* -> web.v0.controller.* - ChannelFinderExceptionHandler -> web (cross-cutting, not legacy-specific) Update all test imports accordingly. No behaviour change.
Replace the implicit per-controller @crossorigin annotations with an explicit CorsConfigurationSource bean in WebSecurityConfig, backed by a configurable cors.allowed-origins property (default: *). - Remove @crossorigin from all legacy controllers (committed as fixup). - Add corsConfigurationSource() bean and wire it into the filter chain. - Document the property in application.properties.
…n errors Bulk requests of 1500+ channels with rich property payloads exceeded the default ES circuit-breaker limit (~102 MiB), causing 429/413 rejections in CI. Split channel inserts into batches of 1000 and throw on bulk errors rather than logging and continuing with a silently empty index. Same fail-fast treatment applied to checkBulkResponse (properties/tags).
anderslindho
force-pushed
the
thin-controller
branch
from
7afee66 to
8cb3de5
Compare
Unmapped exceptions (NPE, unexpected RuntimeException, etc.) bypassed the advice entirely and fell through to Spring's default error handler, which can leak a full stack trace to the client depending on server configuration. - Add catch-all @ExceptionHandler(Exception.class) returning 500 with a generic message; log at SEVERE with the full exception cause - Downgrade 4xx handler log level from SEVERE to WARNING — client errors are not server faults and SEVERE drowns out real failures - Pass ex as third argument to logger.log in all handlers so stack traces are preserved rather than swallowed
Contributor
Author
The fall-back exception handler 🙃 FWIW I always had either both passing or both failing, in case you are insinuating that the tests historically have been flaky. |
anderslindho
force-pushed
the
thin-controller
branch
from
8cb3de5 to
d1c071d
Compare
jacomago
previously approved these changes
Apr 10, 2026
Contributor
jacomago
left a comment
jacomago
left a comment
There was a problem hiding this comment.
I would prefer anything you added called legacy be v0 instead.
The advice is scoped to web.v0.controller and returns ResponseStatusException, which is the pre-Spring 6 style. Marking it @deprecated and naming it V0ExceptionHandler makes room for a global ChannelFinderExceptionHandler using ProblemDetail (RFC 9457) when the v1 API lands — the absence of that file is an intentional prompt to add it.
anderslindho
dismissed stale reviews from jacomago and imretoth-ess
via
45c756a
anderslindho
force-pushed
the
thin-controller
branch
from
d1c071d to
45c756a
Compare
|
jacomago
approved these changes
Apr 10, 2026
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
3 participants
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.
Moves towards a clean controller -> service -> repository layering as the foundation for a future versioned REST API.