[ZEPPELIN-6456] Share websocket operation contract - #5406
Conversation
|
@namuuCY Thank you for working on this. I understand and agree with the intention of preventing the Java and TypeScript websocket operation definitions from drifting. My concern is that the current approach requires the same operation list to be maintained manually in three places:
Although the CI check verifies that these definitions remain synchronized, this seems to add another source that developers must update whenever an operation changes. It may therefore increase the maintenance cost rather than reduce it. The check also cannot detect a mistake if the same incorrect information is added consistently to all three places.
Could you please reconsider this design and explore whether there is a simpler way to prevent the Java and TypeScript definitions from drifting without introducing a third complete operation list? I do not have a particular implementation to require, and I would be very interested in hearing your ideas. I am requesting changes because I think we should resolve this maintenance concern before merging. The intention of the PR is valuable, and I appreciate you taking the time to work on it. |
|
@jongyoul Thank you — you are right that the manifest became a third manually maintained operation list, and that the direction check was circular rather than behavioral. I removed the manifest files and now treat Java I also corrected The checker is intentionally read-only. When adding a wire operation, developers must add the same name and string value to both Java I removed direction metadata from this check and do not infer directions from arbitrary code usage. If behavioral direction validation is added later, it should be based on the server's actual handlers and emissions rather than another manually maintained declaration. |
What is this PR for?
The Java server and clients already share
org.apache.zeppelin.common.Message, but the class still contained a stale comment claiming it was copied fromzeppelin-server. The Angular SDK also maintained a separate websocket operation enum, allowing the Java and TypeScript definitions to drift.This PR treats Java
Message.OPas the wire-operation authority and adds fail-closed CI validation that compares it directly with the TypeScriptOPenum. It does not generate either definition and does not introduce a third complete operation list. Frontend-only events such asPARAGRAPH_STATUSare marked inline with@frontendOnly.The change also corrects
LIST_UPDATE_NOTE_JOBSto server-to-client and removes its stale TypeScript send-map entry. CI does not infer directions from arbitrary code usage; any future direction validation should be based on the server's actual handlers and emissions.Adding a websocket operation
The checker is intentionally read-only and fails when only one side of the contract changes.
Message.OPand TypeScriptOP. This applies even when the Angular SDK does not actively use that operation.OPand document it with an@frontendOnlyJSDoc tag.MessageSendDataTypeMaporMessageReceiveDataTypeMaponly when the Angular SDK sends or receives it.If only one side is updated, CI reports the missing or extra operation. Updating both definitions in the same PR restores the contract check.
What type of PR is it?
Refactoring
Todos
Message.OPdirectly with TypeScriptOPLIST_UPDATE_NOTE_JOBSdirectionWhat is the Jira issue?
How should this be tested?
Run the focused Java contract tests:
./mvnw test -pl zeppelin-common -Dtest=MessageContractTestValidate the Java/TypeScript websocket contracts and checker regression tests:
cd zeppelin-web-angular npm run check:websocket-contractBuild the Angular SDK:
cd zeppelin-web-angular npm run build-project:sdkRun the Apache license check:
Screenshots (if appropriate)
N/A — this PR does not change the UI.
Questions: