Skip to content

[ZEPPELIN-6456] Share websocket operation contract - #5406

Open
namuuCY wants to merge 1 commit into
apache:masterfrom
namuuCY:master
Open

[ZEPPELIN-6456] Share websocket operation contract#5406
namuuCY wants to merge 1 commit into
apache:masterfrom
namuuCY:master

Conversation

@namuuCY

@namuuCY namuuCY commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

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 from zeppelin-server. The Angular SDK also maintained a separate websocket operation enum, allowing the Java and TypeScript definitions to drift.

This PR treats Java Message.OP as the wire-operation authority and adds fail-closed CI validation that compares it directly with the TypeScript OP enum. It does not generate either definition and does not introduce a third complete operation list. Frontend-only events such as PARAGRAPH_STATUS are marked inline with @frontendOnly.

The change also corrects LIST_UPDATE_NOTE_JOBS to 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.

  • For a wire operation, add the same name and string value to both Java Message.OP and TypeScript OP. This applies even when the Angular SDK does not actively use that operation.
  • For an SDK-local event, add it only to TypeScript OP and document it with an @frontendOnly JSDoc tag.
  • Add an operation to MessageSendDataTypeMap or MessageReceiveDataTypeMap only 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

  • Compare Java Message.OP directly with TypeScript OP
  • Mark frontend-only operations next to their TypeScript declarations
  • Correct the LIST_UPDATE_NOTE_JOBS direction
  • Add Java compatibility tests
  • Add checker regression tests
  • Add Angular contract validation to CI
  • Preserve existing wire operation names and values

What is the Jira issue?

How should this be tested?

Run the focused Java contract tests:

./mvnw test -pl zeppelin-common -Dtest=MessageContractTest

Validate the Java/TypeScript websocket contracts and checker regression tests:

cd zeppelin-web-angular
npm run check:websocket-contract

Build the Angular SDK:

cd zeppelin-web-angular
npm run build-project:sdk

Run the Apache license check:

./mvnw clean org.apache.rat:apache-rat-plugin:check -Prat

Screenshots (if appropriate)

N/A — this PR does not change the UI.

Questions:

  • Do the license files need to be updated? No.
  • Are there breaking changes for older versions? No. Existing wire operation names and values remain unchanged.
  • Does this need documentation? No user-facing documentation is required. The direct contract checker and inline frontend-only marker document the maintenance rules.

jongyoul
jongyoul previously approved these changes Aug 10, 2026
@jongyoul
jongyoul self-requested a review August 10, 2026 02:33
@jongyoul

Copy link
Copy Markdown
Member

@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:

  • Java Message.OP
  • websocket-operations.json
  • TypeScript OP

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.

LIST_UPDATE_NOTE_JOBS appears to illustrate this risk. It is classified as bidirectional in the new contract, but the server has no incoming handler for it and only sends it to clients. The definitions agree with each other, but they do not accurately describe the server behavior.

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.

@namuuCY

namuuCY commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@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 Message.OP as the authority for wire-operation names. CI compares it directly with the TypeScript OP enum. Frontend-only events are marked next to their TypeScript declarations with @frontendOnly, so there is no separate exception list.

I also corrected LIST_UPDATE_NOTE_JOBS to server-to-client and removed its stale entry from MessageSendDataTypeMap.

The checker is intentionally read-only. When adding a wire operation, developers must add the same name and string value to both Java Message.OP and TypeScript OP; otherwise CI reports the missing or extra operation. SDK-local events are added only to TypeScript and marked with @frontendOnly.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants