netneutrality/.junie/plans/consolidate-json-on-jackson.md

---
sessionId: session-260925-072402-zipx
---

# Requirements

### Overview & Goals
The project currently depends on **two** JSON libraries declared in `pom.xml`:

- `org.json:json:20250517` (line 220) — used widely
- `com.google.code.gson:gson:2.14.0` (line 225) — used in exactly two places

Goal: standardize on **one** JSON library across the whole codebase and remove the others from `pom.xml`.

**Recommendation: Jackson** (`com.fasterxml.jackson`). See the *Library Comparison* tab for the rationale — the decisive point is that Jackson **is already on the compile classpath** (2.21.2, transitively via JDA 6.4.1 and Javalin 7.2.3), so consolidating on it removes *both* declared dependencies instead of one.

### Scope

**In scope**
- Add an explicit, version-managed `jackson-databind` dependency.
- Migrate all `org.json` usages (`JSONObject` / `JSONArray`) to Jackson `JsonNode` / `ObjectNode` / `ArrayNode`.
- Migrate the two Gson usages to Jackson.
- Remove `org.json:json` and `com.google.code.gson:gson` from `pom.xml`.
- Update the two tests that build `JSONObject` payloads.

**Out of scope**
- Changing the *persisted JSON format* stored in the database or `state.json` — the on-the-wire/on-disk shape must stay byte-compatible in meaning so existing persisted tasks keep loading.
- Refactoring unrelated logic in the touched files (e.g. the dead loop in `BotState.saveGuildsState`).
- Replacing JDA's or Sentry's internal JSON handling.

### Functional Requirements
- The bot compiles and all existing tests pass with only one JSON library declared.
- Tasks persisted before the change still restore correctly (same field names: `id`, `lockId`, `guildId`, `channelId`, `messageId`, `emojiMapping`).
- The Javalin HTTP server keeps serializing/deserializing request and response bodies identically.
- `Settings.getGitVersion()` and `CryptoPriceCommand` keep reading the same fields from the same remote APIs.

### Non-Functional Requirements
- No new transitive dependency conflicts in the shaded jar.
- Jackson version stays aligned with the one JDA brings in, to avoid split-version issues.

# Library Comparison

### The three candidates

| | `org.json` | Gson | **Jackson** |
|---|---|---|---|
| Already on classpath? | declared | declared | **yes, transitively (JDA + Javalin)** |
| Deps removable if chosen | 1 (gson) | 1 (org.json) | **2 (both)** |
| Javalin 7 integration | none (manual) | custom `GsonJsonMapper` needed | **built-in `JavalinJackson`** |
| Tree/DOM API (needed by this codebase) | `JSONObject` | weak (`JsonObject`, clunky) | `ObjectNode` / `JsonNode` |
| POJO binding | none | good | excellent |
| Current usage count | ~9 files | 2 call sites | 0 |

### Why Jackson wins here
1. **It is already a compile dependency.** Verified via `mvn dependency:tree`:
   - `net.dv8tion:JDA:6.4.1` → `jackson-core:2.21.2`, `jackson-databind:2.21.2`
   - `io.javalin:javalin:7.2.3` → `jersey-media-json-jackson` → jackson modules
   
   So Jackson ships in the jar today whether we use it or not. Choosing `org.json` or Gson means keeping a *redundant* second parser in the artifact forever.
2. **Javalin has first-class Jackson support.** `GsonJsonMapper.java` exists purely to bridge Gson into Javalin; with Jackson that whole class is deleted and Javalin's default mapper takes over.
3. **Jackson covers both usage styles present in the codebase**: the tree-style manipulation `org.json` is used for (`ObjectNode`/`ArrayNode`) *and* the POJO serialization Gson is used for (`writeValueAsString`).

### Why not the other two
- **Gson**: the smaller migration on paper is actually the *bigger* one — all ~9 `org.json` files would move to Gson's awkward tree API, and Jackson would still be shipped unused.
- **`org.json`**: only 2 call sites to change (trivial), but it is the weakest library of the three (no POJO binding, `new JSONObject(bean)` reflection is fragile), it has no Javalin integration, and Jackson still ships unused. Choose this only if minimal diff outweighs everything else.

### Cost
Jackson is the largest diff (~11 files) but the only option that actually reduces what is bundled.

# Technical Design

### Current Implementation

**Gson (2 sites — trivial):**
- `http/server/GsonJsonMapper.java` — implements Javalin's `JsonMapper` over a static `Gson`.
- `http/WebModule.java:256` — `RequestBody.create(new Gson().toJson(params), ...)` for POST bodies.

**`org.json` (9 files):**
- `core/tasks/PersistableTask.java` — interface declares `void restore(JSONObject payload)`.
- `core/tasks/impl/AbstractPersistableTask.java:69-80` — `restore(JSONObject)` no-op and `toPayload()` using **`new JSONObject(this)` bean introspection** plus `id` / `lockId`.
- `core/tasks/impl/EmojiTask.java:39-51` — `restore` reads `guildId`, `channelId`, `messageId`, and the `emojiMapping` object.
- `core/tasks/persistence/PersistableTaskRegistry.java`, `db/service/TaskService.java` — parse/handle payload JSON.
- `http/WebModule.java:133-143` — `getAsObject()` / `getAsArray()` returning `JSONObject` / `JSONArray`.
- `utils/Settings.java:104-109` — `getAsArray()`, `isNull(0)`, `getJSONObject(0).getString("tag_name")`.
- `command/crypto/CryptoPriceCommand.java` — reads the crypto API response.
- `bootstrap/BotState.java:46-81` — builds a `state.json` dump.
- Tests: `PersistableTaskRegistryTest`, `EmojiReactionRoleTaskPersistenceTest`.

`tasks/reaction/EmojiReactionRoleTask.java` itself has **no** JSON imports — it is affected only indirectly through `EmojiTask`.

### Key Decisions
1. **Jackson as the single library**, pinned to the version JDA supplies (2.21.2) via an explicit `<dependency>` on `jackson-databind` so the build does not silently depend on a transitive.
2. **One shared `ObjectMapper`** in a small `utils/Json.java` holder — `ObjectMapper` is thread-safe and expensive to create; today the code does `new Gson()` per request.
3. **Replace `new JSONObject(this)` bean introspection with explicit payload building.** Jackson's `valueToTree(this)` would walk every getter on the task hierarchy (including ones returning JDA objects) and can blow up. Instead `toPayload()` builds an `ObjectNode` explicitly and tasks with extra state override a `writePayload(ObjectNode)` hook — mirroring the existing `restore` hook. This keeps the emitted field set identical to today's (`id`, `lockId`, `guildId`, `channelId`, `messageId`, `emojiMapping`, `name`) but makes it deterministic.
4. **Delete `GsonJsonMapper`** and rely on Javalin's built-in Jackson mapper rather than porting the class to Jackson.
5. **Keep tree-style code tree-style.** No POJO/DTO redesign in this task — `JSONObject` → `ObjectNode`, `JSONArray` → `ArrayNode`, `optString(k, null)` → `node.path(k).asText(null)`.

### Data Models / Contracts

```java
// utils/Json.java (new)
public final class Json {
    private static final ObjectMapper MAPPER = new ObjectMapper();
    public static ObjectMapper mapper();
    public static ObjectNode object();          // MAPPER.createObjectNode()
    public static ArrayNode array();
    public static ObjectNode readObject(String json);   // wraps JsonProcessingException
    public static ArrayNode  readArray(String json);
    public static String write(Object value);
}
```

Interface change in `PersistableTask`:
```java
- void restore(JSONObject payload);
+ void restore(ObjectNode payload);
```

New hook in `AbstractPersistableTask`:
```java
protected void writePayload(ObjectNode payload) { /* no-op */ }

@Override
public String toPayload() {
    ObjectNode payload = Json.object();
    payload.put("id", getId());
    payload.put("lockId", getLockId());
    payload.put("name", getName());
    writePayload(payload);
    return payload.toString();
}
```

`EmojiTask` gains the symmetric writer:
```java
@Override protected void writePayload(ObjectNode payload) {
    payload.put("guildId", guildId);
    payload.put("channelId", channelId);
    payload.put("messageId", messageId);
    ObjectNode mapping = payload.putObject("emojiMapping");
    emojiMapping.forEach(mapping::put);
}
```

`WebModule` response accessors:
```java
public ObjectNode getAsObject();   // Json.readObject(body)
public ArrayNode  getAsArray();    // Json.readArray(body)
```
Both wrap Jackson's checked `JsonProcessingException` into an unchecked exception so the existing call sites' signatures are unchanged.

### File Structure

**Added**
- `src/main/java/me/bobert/netneutrality/utils/Json.java`

**Removed**
- `src/main/java/me/bobert/netneutrality/http/server/GsonJsonMapper.java`

**Modified**
- `pom.xml` — drop `org.json:json` + `com.google.code.gson:gson`, add `jackson-databind`
- `core/tasks/PersistableTask.java`
- `core/tasks/impl/AbstractPersistableTask.java`
- `core/tasks/impl/EmojiTask.java`
- `core/tasks/persistence/PersistableTaskRegistry.java`
- `db/service/TaskService.java`
- `http/WebModule.java`
- wherever `GsonJsonMapper` was registered on the Javalin config (`http/server/*`)
- `utils/Settings.java`
- `command/crypto/CryptoPriceCommand.java`
- `bootstrap/BotState.java`
- `src/test/java/.../PersistableTaskRegistryTest.java`
- `src/test/java/.../EmojiReactionRoleTaskPersistenceTest.java`

### Architecture Diagram

```mermaid
graph LR
  A[Task persistence] --> J[Json utils - ObjectMapper]
  B[WebModule HTTP client] --> J
  C[Settings / CryptoPriceCommand] --> B
  D[BotState dump] --> J
  E[Javalin server] --> F[JavalinJackson built-in]
  J --> G[jackson-databind]
  F --> G
```

### Risks
- **Behavioural drift in `toPayload()`**: `new JSONObject(this)` reflects over *all* getters. If any persisted consumer relies on an incidental field, the explicit writer could drop it. Mitigation: inspect the DB payload column / existing tests and keep the field set identical.
- **Null handling differs**: `JSONObject.put(k, null)` removes the key; `ObjectNode.put(k, (String) null)` writes a JSON `null`. `EmojiTask.restore` uses `optString(k, null)`, so writers must skip nulls to stay equivalent.
- **`JSONArray.isNull(0)` has no direct Jackson twin** — `Settings.getGitVersion()` must instead check `array.isEmpty() || !array.get(0).hasNonNull("tag_name")`.
- **Checked exceptions**: Jackson parsing throws `JsonProcessingException`; must be wrapped to avoid rippling `throws` through call sites.
- **Shade plugin**: confirm no relocation/filter rules in `pom.xml` reference `org.json` or `gson`.

# Testing

### Validation Approach
- `./mvnw -o clean verify` must pass with the two dependencies removed — a leftover `org.json`/`gson` import then becomes a hard compile error, which is the primary completeness check.
- Grep the whole `src` tree for `org.json`, `com.google.gson`, `JSONObject`, `JSONArray`, `Gson` and confirm zero hits.
- `./mvnw -o dependency:tree` to confirm `org.json:json` and `com.google.code.gson:gson` are fully gone (not even transitive) and only one Jackson version is resolved.

### Key Scenarios
- **Task round-trip**: `EmojiReactionRoleTaskPersistenceTest` — a task is persisted via `toPayload()` and restored via `restore(...)` with `guildId` / `channelId` / `messageId` / `emojiMapping` intact.
- **Registry restore**: `PersistableTaskRegistryTest` — a payload built in the test is resolved to the right task type and hydrated.
- **Backward compatibility**: add an assertion that a payload string in the *old* `org.json` format still restores correctly, proving pre-existing DB rows keep working.
- **HTTP client**: `WebModule.getAsObject()` / `getAsArray()` parse a sample body; the POST path produces the same JSON body as Gson did for a `Map<String,String>`.

### Edge Cases
- Missing / null `guildId` in a payload → `restore` yields `null`, not the string `"null"`.
- Empty array from the GitLab releases endpoint → `Settings.getGitVersion()` returns `"0.0.0"` instead of throwing.
- Malformed response body → the wrapped unchecked exception surfaces with a useful message rather than an NPE.

### Test Changes
- Update the two existing test files to build payloads with `ObjectNode`.
- Add a legacy-payload restore test in `EmojiReactionRoleTaskPersistenceTest`.
- Optionally add a small `JsonTest` covering `Json.readObject` / `readArray` error wrapping.

# Delivery Steps

### ✓ Step 1: Introduce Jackson and the shared Json utility
Jackson is an explicit dependency and a single shared `ObjectMapper` is available to the codebase.

- Add `com.fasterxml.jackson.core:jackson-databind` to `pom.xml`, pinned to the version already resolved via JDA (2.21.2), next to the existing `org.json`/`gson` entries (which stay for now so the build keeps compiling).
- Create `src/main/java/me/bobert/netneutrality/utils/Json.java` holding a static thread-safe `ObjectMapper` plus helpers: `mapper()`, `object()`, `array()`, `readObject(String)`, `readArray(String)`, `write(Object)`.
- Have `readObject`/`readArray` wrap Jackson's checked `JsonProcessingException` in an unchecked exception so existing call-site signatures stay unchanged.
- Verify no shade-plugin relocation or filter rules in `pom.xml` reference `org.json` or `gson`.

### ✓ Step 2: Migrate the Gson call sites to Jackson
Gson is no longer referenced anywhere in the source tree.

- Delete `http/server/GsonJsonMapper.java` and remove its registration from the Javalin configuration, relying on Javalin 7's built-in Jackson `JsonMapper` instead.
- Replace `new Gson().toJson(params)` at `http/WebModule.java:256` with `Json.write(params)`, reusing the shared mapper instead of allocating per request.
- Confirm Javalin request/response serialization still behaves identically for the existing endpoints.

### ✓ Step 3: Migrate the HTTP client and its consumers
`WebModule` returns Jackson nodes and its callers read them.

- Change `WebModule.getAsObject()` to return `ObjectNode` and `getAsArray()` to return `ArrayNode` (lines 133-143), parsing through `Json`.
- Update `utils/Settings.java:104-109`: replace `JSONArray`/`isNull(0)`/`getJSONObject(0).getString("tag_name")` with an `ArrayNode` emptiness check and `get(0).path("tag_name").asText()`, preserving the `"0.0.0"` fallback.
- Update `command/crypto/CryptoPriceCommand.java` to read the crypto API response via `JsonNode` accessors.
- Remove the now-unused `org.json` imports from these three files.

### ✓ Step 4: Migrate the task persistence layer
Tasks serialize and restore through Jackson while keeping the existing on-disk payload shape.

- Change `PersistableTask.restore(JSONObject)` to `restore(ObjectNode)`.
- In `AbstractPersistableTask`, replace the `new JSONObject(this)` bean-introspection `toPayload()` with an explicit `ObjectNode` builder writing `id`, `lockId` and `name`, plus a new `protected void writePayload(ObjectNode)` hook; keep `restore` a no-op default.
- In `EmojiTask`, rewrite `restore` (lines 39-51) using `payload.path(...).asText(null)` and iteration over the `emojiMapping` node, and implement `writePayload` to emit `guildId`, `channelId`, `messageId` and `emojiMapping`, skipping nulls to match `optString(k, null)` semantics.
- Update `PersistableTaskRegistry` and `db/service/TaskService` to parse stored payload strings via `Json.readObject`.
- Confirm `tasks/reaction/EmojiReactionRoleTask` still compiles unchanged against the new `EmojiTask` contract.

### ✓ Step 5: Update tests and drop the old dependencies
`org.json` and Gson are removed from `pom.xml` and the full build passes.

- Rewrite payload construction in `PersistableTaskRegistryTest` and `EmojiReactionRoleTaskPersistenceTest` using `ObjectNode`.
- Add a regression test restoring a payload written in the legacy `org.json` format to prove pre-existing database rows still load.
- Migrate the `state.json` dump in `bootstrap/BotState.java:46-81` to `ObjectNode`/`ArrayNode`.
- Remove the `org.json:json` and `com.google.code.gson:gson` dependencies from `pom.xml`.
- Run `./mvnw -o clean verify`, then `dependency:tree` to confirm both libraries are fully absent and a single Jackson version is resolved.