Migrate HTTP server to Javalin and remove legacy handlers
Replaced the existing HttpServer implementation and associated handler classes (`AbstractHandler`, `ActionHandler`, `GuildHandler`, `RootHandler`) with a Javalin-based routing system. Improved modularity by introducing `ActionRoutes` and `GuildRoutes` for route definitions, and centralized JSON mapping using `GsonJsonMapper`. Simplified caching via the new `ResponseCache` utility, updated `WebModule` for cleaner timeout handling, and extended test coverage for API endpoints.
This commit is contained in:
anthorob committed
1 parent
5f6265db01
commit
01bfe22d22
14 files changed
+779
-384
No files matched your search
@@ -0,0 +1,188 @@
|
||||
---
|
||||
sessionId: session-260923-111902-5uwc
|
||||
---
|
||||
|
||||
# Requirements
|
||||
|
||||
### Overview & Goals
|
||||
Replace the hand-rolled `com.sun.net.httpserver` stack in `me.bobert.netneutrality.http.server` with Javalin, so the bot's embedded API needs far fewer custom classes and gains correct status codes, JSON handling and error responses. Additionally simplify the outbound `WebModule` HTTP client.
|
||||
|
||||
### Scope
|
||||
**In scope**
|
||||
- `WebApi` becomes a thin Javalin-based `Lifecycle` component (same priority 70, same `NETNEUTRALITY_API_PORT` setting).
|
||||
- Delete `AbstractHandler`, `RootHandler`, `ActionHandler`, `GuildHandler`; their logic moves into small route-registration units.
|
||||
- Standardise the API layer on **Gson** — remove `org.json` (`JSONObject`/`JSONArray`) usage from the HTTP server package.
|
||||
- Correct HTTP semantics: 404 for unknown guilds/paths, 405 for wrong methods, 500 on exceptions, UTF-8 safe bodies.
|
||||
- Simplify `WebModule` (outbound OkHttp wrapper): extract the static cache, remove duplicated/unused members.
|
||||
|
||||
**Out of scope**
|
||||
- Changing the public URL shape of existing endpoints (`/`, `/actions/*`, `/guilds/*`) — paths and payload keys stay backwards compatible.
|
||||
- Authentication/authorisation for the API.
|
||||
- Migrating `org.json` usage elsewhere in the project (e.g. task classes using `WebModule.getAsObject()`).
|
||||
|
||||
### Functional Requirements
|
||||
- `GET /` → HTML liveness page (unchanged text).
|
||||
- `GET /actions/status` → `{"health":...,"ping":...}`.
|
||||
- `POST /actions/shutdown` → runs the shutdown hook and exits (accept `GET` too for compatibility if desired).
|
||||
- `GET /actions/settings/current/{guildId}` → `{"status":"ok","settings":{...}}`, or 404 + `{"status":"error"}` for an unknown guild.
|
||||
- `POST /actions/settings/reload/{guildId}` → reloads the guild setting cache, `{"status":"true"}`; 404 for an unknown guild.
|
||||
- `GET /guilds/owner` → array of `{guild_id, owner_id}`.
|
||||
- `GET /guilds/channels/{guildId}` and `GET /guilds/roles/{guildId}` → arrays of id/name objects; 404 for an unknown guild.
|
||||
- Requests arriving before JDA is ready must return 503 rather than throwing an NPE.
|
||||
|
||||
### Non-Functional Requirements
|
||||
- Startup/shutdown stays deterministic through `ApplicationContext` — `stop()` must release the port.
|
||||
- Shaded jar must still run (`maven-shade-plugin`); Javalin/Jetty service files need merging.
|
||||
- No behavioural regression for existing consumers of the endpoints.
|
||||
|
||||
# Technical Design
|
||||
|
||||
### Current Implementation
|
||||
- `WebApi.java` (92 lines) implements `Lifecycle` (priority 70), creates an `HttpServer`, registers three contexts and a fixed pool built from `BaseThreadFactory("web-api")`. Constructed in `App.java:154`.
|
||||
- `AbstractHandler` offers `sendStringResponse`/`sendJsonResponse`; both hardcode status 200 and use `str.length()` instead of byte length (breaks non-ASCII).
|
||||
- `ActionHandler` / `GuildHandler` route via `if (path.startsWith(...))` chains and parse ids with `getRequestURI().toString().split("/")`; they mix Gson and `org.json`, never check the HTTP method, and call `jda.getGuildById()` twice in places.
|
||||
- `WebModule` is an unrelated *outbound* OkHttp wrapper with a builder, a static LRU cache and a `CachedResponse` record; it builds a brand-new `OkHttpClient` per request and has three redundant timeout setters plus unused `elapsedTime`/`errorMessage` accessors.
|
||||
|
||||
### Key Decisions
|
||||
1. **Javalin** (6.x, JDK 17+ / Java 25 compatible) as the embedded server — removes routing, method matching, error mapping and content-type handling code.
|
||||
2. **Gson only** in the HTTP layer: register a `GsonJsonMapper` on the Javalin config so `ctx.json(obj)` works with plain records/maps; `org.json` disappears from `http/server`.
|
||||
3. **Route groups instead of handler classes**: `ActionRoutes` and `GuildRoutes` expose a single `static void register(Javalin app, deps...)`, so `AbstractHandler`, `RootHandler`, `ActionHandler`, `GuildHandler` are all deleted (4 classes → 2 tiny registrars).
|
||||
4. **Guild lookup guard**: one private helper resolves `{guildId}` → `Guild` or throws `NotFoundResponse`, replacing repeated null checks.
|
||||
5. **WebModule**: keep the builder API used by callers, but move the cache into a package-private `ResponseCache`, share a single static `OkHttpClient` and derive per-request timeouts with `newBuilder()`.
|
||||
|
||||
### Proposed Changes
|
||||
**`http/server/WebApi.java`**
|
||||
```java
|
||||
public class WebApi implements Lifecycle {
|
||||
private Javalin app;
|
||||
@Override public int priority() { return 70; }
|
||||
@Override public void start() {
|
||||
int port = Integer.parseInt(settings.get("NETNEUTRALITY_API_PORT", "9000"));
|
||||
app = Javalin.create(cfg -> cfg.jsonMapper(new GsonJsonMapper()));
|
||||
app.get("/", ctx -> ctx.html(LIVENESS_HTML));
|
||||
ActionRoutes.register(app, jdaSupplier, guildServices, shutdownHook);
|
||||
GuildRoutes.register(app, jdaSupplier);
|
||||
app.exception(Exception.class, (e, ctx) -> { log.error(...); ctx.status(500).json(Map.of("status","error")); });
|
||||
app.start(port);
|
||||
}
|
||||
@Override public void stop() { if (app != null) { app.stop(); app = null; } }
|
||||
}
|
||||
```
|
||||
|
||||
**`http/server/GuildRoutes.java`** — `/guilds/owner`, `/guilds/channels/{guildId}`, `/guilds/roles/{guildId}` mapped to `List<Map<String,String>>` and returned via `ctx.json(...)`.
|
||||
|
||||
**`http/server/ActionRoutes.java`** — `/actions/status`, `/actions/shutdown`, `/actions/settings/current/{guildId}`, `/actions/settings/reload/{guildId}`.
|
||||
|
||||
**`http/server/GsonJsonMapper.java`** — ~15 lines implementing Javalin's `JsonMapper` over a shared `Gson`.
|
||||
|
||||
**`http/WebModule.java`** — shared client, cache extracted, redundant setters/accessors dropped; public builder methods used by callers remain.
|
||||
|
||||
### Data Models / Contracts
|
||||
Small records replace ad-hoc JSON building, serialized by Gson with the existing key names:
|
||||
```java
|
||||
record GuildOwner(String guild_id, String owner_id) {}
|
||||
record NamedEntity(String channel_id, String channel_name) {}
|
||||
record StatusResponse(String health, String ping) {}
|
||||
```
|
||||
|
||||
### File Structure
|
||||
```
|
||||
src/main/java/me/bobert/netneutrality/http/
|
||||
WebModule.java (modified)
|
||||
server/WebApi.java (rewritten, Javalin)
|
||||
server/GsonJsonMapper.java (new)
|
||||
server/ActionRoutes.java (new)
|
||||
server/GuildRoutes.java (new)
|
||||
server/AbstractHandler.java (deleted)
|
||||
server/RootHandler.java (deleted)
|
||||
server/ActionHandler.java (deleted)
|
||||
server/GuildHandler.java (deleted)
|
||||
pom.xml (+ io.javalin:javalin, shade transformer)
|
||||
```
|
||||
|
||||
### Architecture Diagram
|
||||
```mermaid
|
||||
graph LR
|
||||
App-->ApplicationContext
|
||||
ApplicationContext-->WebApi
|
||||
WebApi-->Javalin
|
||||
WebApi-->GsonJsonMapper
|
||||
Javalin-->ActionRoutes
|
||||
Javalin-->GuildRoutes
|
||||
ActionRoutes-->GuildServiceProvider
|
||||
GuildRoutes-->JDA
|
||||
```
|
||||
|
||||
### Risks
|
||||
- **Shaded jar**: Jetty/Javalin rely on `META-INF/services`; the shade plugin needs `ServicesResourceTransformer`, otherwise startup fails only in the packaged build.
|
||||
- **Jar size / SLF4J binding**: Jetty logs through SLF4J; project already has log4j + slf4j — check for duplicate bindings.
|
||||
- **JDA not ready**: `jdaSupplier.get()` may be null; guard with a 503 mapping.
|
||||
- `System.exit(0)` in the shutdown route must run after the response is flushed.
|
||||
|
||||
# Testing
|
||||
|
||||
### Validation Approach
|
||||
Build with `./mvnw -q verify` (checkstyle included), then start the app/`WebApi` on an ephemeral port and exercise every endpoint with `curl`, comparing payload shape against the pre-refactor behaviour.
|
||||
|
||||
### Key Scenarios
|
||||
- `GET /` returns 200 and the HTML liveness text.
|
||||
- `GET /actions/status` returns 200 `application/json` with `health` and `ping`.
|
||||
- `GET /guilds/owner` returns a JSON array with `guild_id`/`owner_id` keys.
|
||||
- `GET /guilds/channels/{id}` and `/guilds/roles/{id}` return arrays with `channel_id`/`channel_name`.
|
||||
- `GET /actions/settings/current/{id}` returns `{"status":"ok","settings":{...}}`.
|
||||
|
||||
### Edge Cases
|
||||
- Unknown guild id → 404 with `{"status":"error"}` (previously 200).
|
||||
- Unknown path → 404 (previously echoed the path with 200).
|
||||
- Wrong method on a route → 405.
|
||||
- Non-ASCII content in settings values round-trips correctly (old `str.length()` bug).
|
||||
- `Lifecycle.stop()` frees the port — a second `start()` on the same port succeeds.
|
||||
|
||||
### Test Changes
|
||||
- Add a lightweight JUnit test starting `WebApi` on port 0 with a mocked `Supplier<JDA>` and `GuildServiceProvider`, hitting the routes with Javalin's test client or `java.net.http.HttpClient`.
|
||||
- Add a `WebModule` test covering cache hit/expiry after the cache extraction.
|
||||
|
||||
# Delivery Steps
|
||||
|
||||
### ✓ Step 1: Add Javalin dependency and Gson JSON mapper
|
||||
The project builds with Javalin available and a Gson-backed JSON mapper ready for use.
|
||||
|
||||
- Add `io.javalin:javalin` (6.x) to `pom.xml` dependencies.
|
||||
- Add `ServicesResourceTransformer` to the `maven-shade-plugin` configuration so Jetty/Javalin service files survive shading.
|
||||
- Create `http/server/GsonJsonMapper.java` implementing Javalin's `JsonMapper` over a shared `Gson` instance.
|
||||
- Verify `./mvnw -q package` succeeds and the shaded jar still starts.
|
||||
|
||||
### ✓ Step 2: Rewrite WebApi as a Javalin Lifecycle component
|
||||
`WebApi` starts and stops a Javalin server while keeping its Lifecycle contract and the root endpoint.
|
||||
|
||||
- Replace the `HttpServer`/`ExecutorService` fields with a `Javalin app` field.
|
||||
- Build the instance in `start()` with the `GsonJsonMapper`, reading `NETNEUTRALITY_API_PORT` from `SettingsProvider` as today.
|
||||
- Register `GET /` returning the liveness HTML and delete `RootHandler.java`.
|
||||
- Add global `exception` and `error` handlers returning 500/404 JSON, plus a 503 guard when `jdaSupplier.get()` is null.
|
||||
- Implement `stop()` via `app.stop()`; keep `priority() == 70` and the `App.java` construction site unchanged.
|
||||
|
||||
### ✓ Step 3: Migrate guild endpoints to GuildRoutes
|
||||
All `/guilds/*` endpoints are served by Javalin route lambdas with proper status codes and Gson serialization.
|
||||
|
||||
- Create `http/server/GuildRoutes.register(Javalin, Supplier<JDA>)`.
|
||||
- Map `GET /guilds/owner`, `GET /guilds/channels/{guildId}`, `GET /guilds/roles/{guildId}` using path params instead of `split("/")`.
|
||||
- Return `List<record>` payloads via `ctx.json(...)`, preserving the existing `guild_id`/`owner_id`/`channel_id`/`channel_name` keys.
|
||||
- Resolve guilds through a shared helper that throws `NotFoundResponse` for unknown ids (single `getGuildById` call).
|
||||
- Delete `GuildHandler.java`.
|
||||
|
||||
### ✓ Step 4: Migrate action endpoints to ActionRoutes
|
||||
All `/actions/*` endpoints run on Javalin, with `org.json` fully removed from the HTTP server package.
|
||||
|
||||
- Create `http/server/ActionRoutes.register(Javalin, Supplier<JDA>, GuildServiceProvider, Runnable)`.
|
||||
- Map `/actions/status`, `/actions/shutdown`, `/actions/settings/current/{guildId}`, `/actions/settings/reload/{guildId}`.
|
||||
- Replace `JSONObject`/`JSONArray` with Gson-serialized maps/records keeping the same response keys.
|
||||
- Ensure the shutdown route flushes its response before running the hook and exiting.
|
||||
- Delete `ActionHandler.java` and `AbstractHandler.java`, and add endpoint tests starting `WebApi` on port 0 with mocked JDA/guild services.
|
||||
|
||||
### ✓ Step 5: Simplify the outbound WebModule client
|
||||
`WebModule` keeps its builder API but sheds duplicated state and its inlined cache.
|
||||
|
||||
- Extract the static LRU cache and `CachedResponse` into a package-private `ResponseCache` class in `http/`.
|
||||
- Share a single static `OkHttpClient` and derive per-request timeouts with `newBuilder()` instead of constructing a client per build.
|
||||
- Collapse the redundant `timeout`/`connectTimeout`/`readTimeout`/`writeTimeout` setters and remove unused accessors (`getErrorMessage`, `elapsedTime` exposure) after checking call sites.
|
||||
- Add a unit test covering cache hit, expiry and POST bypass; verify existing `WebModule` callers still compile.
|
||||
Reference in new issue
Block a user