aglinxinyuan opened a new pull request, #7099:
URL: https://github.com/apache/texera/pull/7099
### What changes were proposed in this PR?
The websocket model tier had no test anywhere — `find amber/src/test -path
'*websocket*'` returned nothing. The uncovered classes are mostly bare case
classes, so the thing actually worth pinning is their Jackson polymorphic
registration, which is a **cross-language contract**:
| Where | What it says |
|---|---|
| `TexeraWebSocketRequest` / `TexeraWebSocketEvent` | `@JsonTypeInfo(use =
Id.NAME, include = As.PROPERTY, property = "type")` |
| every `@JsonSubTypes.Type` entry | no `name =` — and there is no
`@JsonTypeName` in the package |
| ⇒ resulting wire id | Jackson's `TypeNameIdResolver` fallback = **the bare
simple class name** |
| `workflow-websocket.interface.ts` | the Angular map keys on exactly those
strings |
So renaming a Scala class — or adding an explicit `name =` — silently
changes the wire protocol, with no compile error on either side. Nothing
guarded that.
```
client ──{"type":"ResultPaginationRequest", …}──▶
WorkflowWebsocketResource:81
objectMapper.readValue(msg,
classOf[TexeraWebSocketRequest])
server ──{"type":"PaginatedResultEvent", …}──▶ SessionState:60
sendText(writeValueAsString(msg))
```
Three specs, all driving the **same `JSONUtils.objectMapper` production
uses** (a fresh `new ObjectMapper()` would test fiction — without
`DefaultScalaModule` none of these case classes bind at all):
- **`TexeraWebSocketRequestSpec`** — deserializes all 13 registered subtypes
through `classOf[TexeraWebSocketRequest]`; asserts the registered id set
*against the annotation itself*, so the expected list is anchored to production
rather than to another literal; asserts no entry is explicitly named; rejects
an unknown and a missing type id. Also pins `ResultPaginationRequest`'s three
default arguments — the live path, since the client declares `columnOffset` /
`columnLimit` / `columnSearch` optional. If `DefaultScalaModule` were ever
dropped, `columnLimit` would bind to `0` and result pagination would silently
return zero columns.
- **`TexeraWebSocketEventSpec`** — events are **serialize-only** (server →
client; nothing in `main` ever reads one back), so it asserts the emitted
discriminator for the 11 registered subtypes *and* the 5 that are produced but
unregistered, round-trips the 8 that are symmetric, and pins that the effective
inclusion rule is `NON_ABSENT` and not `NON_EMPTY` (the result panel reads
`updates` / `tableStats` unconditionally).
- **`PaginatedResultEventSpec`** — the companion `apply`'s field projection,
with `pageIndex` ≠ `pageSize` and `requestID` ≠ `operatorID` so any
transposition fails.
Deliberately **not** asserted, and noted in-file so the reasoning survives:
- `EditingTimeCompilationRequest.toLogicalPlanPojo` — no production caller;
the type stays in the registry assertions (registry membership *is* the
contract) but its dead method is not pinned.
- that the 5 unregistered events *fail* to deserialize — they do today, but
asserting a failure would turn this suite red the moment someone harmlessly
registers them.
Two divergences found while writing this, reported rather than encoded as
expectations:
- **`SkipTupleRequest` field-name mismatch** — the client sends `{ workers
}` (`execute-workflow.service.ts:313`) while the case class declares
`workerIds`, and the shared mapper never disables `FAIL_ON_UNKNOWN_PROPERTIES`,
so the real client frame would be rejected. Latent only because the handler
throws `"skipping tuple is temporarily disabled"` before reading the field.
- **`ExecutionResultService:498`** does `slice(columnOffset, columnOffset +
columnLimit)`; with the default `columnLimit = Int.MaxValue` and any
`columnOffset > 0` that sum overflows negative and yields no columns.
Unreachable today because the frontend's `useCache` short-circuit keeps the
all-columns request off the wire.
### Any related issues, documentation, discussions?
Closes #7097
### How was this PR tested?
Three new specs, 24 tests, run locally against Java 17:
```
sbt "WorkflowExecutionService/testOnly
org.apache.texera.web.model.websocket.request.TexeraWebSocketRequestSpec
org.apache.texera.web.model.websocket.event.TexeraWebSocketEventSpec
org.apache.texera.web.model.websocket.event.PaginatedResultEventSpec"
```
```
[info] Suites: completed 3, aborted 0
[info] Tests: succeeded 24, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
```
`Test/scalafmtCheck` and `Test/scalafix --check` both `[success]`. No
production file is touched — the diff is three new test files.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
