Skip to content

Send out web socket event when malware scanner finishes#435

Open
schiwekM wants to merge 6 commits into
mainfrom
feat/websockets-for-malware-scanning
Open

Send out web socket event when malware scanner finishes#435
schiwekM wants to merge 6 commits into
mainfrom
feat/websockets-for-malware-scanning

Conversation

@schiwekM
Copy link
Copy Markdown
Contributor

Note: Does not yet fully work with Fiori elements because somehow a table row cannot be targeted by the side effect event. Event is however received.

@schiwekM schiwekM requested a review from a team as a code owner April 28, 2026 13:07
@hyperspace-insights
Copy link
Copy Markdown
Contributor

Summary

The following content is AI-generated and provides a summary of the pull request:


Emit WebSocket Event When Malware Scanner Finishes

New Feature

✨ Automatically emits an attachmentStatusChanged WebSocket event after malware scanning completes, enabling SAP Fiori Elements UIs to refresh automatically without manual page reload. The event is emitted for draft-enabled entities when the service has WebSockets enabled.

Note: Full Fiori Elements integration (table row targeting via side effect) is not yet complete, but the WebSocket event is correctly received.

Changes

  • CHANGELOG.md: Added entry documenting the new attachmentStatusChanged WebSocket event feature.
  • README.md: Added a tip explaining that when WebSockets and draft are enabled, the plugin automatically emits the attachmentStatusChanged event upon scan completion, triggering UI refresh.
  • lib/plugin.js: Extended unfoldModel to:
    • Auto-generate an attachmentStatusChanged event definition in each service that has attachment compositions.
    • Add @Common.SideEffects annotations to attachment entities linking the WebSocket event to status property and statusNav entity refreshes.
    • Handle both inferred CSN (UI annotations) and runtime CSN flavors.
    • Register unfoldModel on the compile.for.runtime hook in addition to the existing compile hook.
  • srv/attachments/basic.js: Wrapped malware scan emission in cds.spawn() to decouple scanning from the request lifecycle, avoiding blocking the HTTP response.
  • srv/malware-scanner/malwareScanner.js: After updating the scan status for draft-enabled entities, spawns an async task that queries the active/draft record and emits the attachmentStatusChanged event on the parent service with a structured sideEffectSource path.
  • tests/incidents-app/package.json: Added @cap-js-community/websocket as a dependency for the test app.
  • tests/incidents-app/srv/services.cds: Annotated ProcessorService with @ws, @odata, and WebSocket base URL/channel annotations to enable WebSocket support in the test app.
  • tests/unit/websocket.test.js: New unit test suite covering updateStatus event emission scenarios (draft vs. active, missing records, non-draft entities) and unfoldModel annotation/event generation correctness.

  • 🔄 Regenerate and Update Summary
  • ✏️ Insert as PR Description (deletes this comment)
  • 🗑️ Delete comment
PR Bot Information

Version: 1.20.33

  • Correlation ID: 08a912ba-f14b-4cb8-ba1e-9069d2b01d40
  • Event Trigger: pull_request.opened
  • File Content Strategy: Full file content
  • Output Template: Default Template
  • LLM: anthropic--claude-4.6-sonnet
  • Summary Prompt: Default Prompt

Copy link
Copy Markdown
Contributor

@hyperspace-insights hyperspace-insights Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR introduces a useful WebSocket event feature for malware scan completion, but has several correctness issues: unhandled errors in fire-and-forget cds.spawn blocks, a fragile isActiveEntity derivation that can produce wrong OData paths, potential mismatch when updateStatus is called with a complex CQL where expression rather than plain keys, and a test spy that overwrites spawnCallback on each call — masking assertions on the first spawn invocation.

PR Bot Information

Version: 1.20.33

  • Correlation ID: 08a912ba-f14b-4cb8-ba1e-9069d2b01d40
  • Event Trigger: pull_request.opened
  • File Content Strategy: Full file content
  • LLM: anthropic--claude-4.6-sonnet

Comment thread srv/malware-scanner/malwareScanner.js Outdated
)
const result = draft ?? active
if (result) {
const isActiveEntity = !draft
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic Error: isActiveEntity is derived from whether a draft row exists, but this is unreliable

const isActiveEntity = !draft means: if a draft record exists, isActiveEntity=false; otherwise true. However, draft is the result of SELECT.one.from(target.drafts).where(where) — if the draft row simply doesn't exist in the DB at query time (e.g., was already activated), this will incorrectly emit IsActiveEntity=true even if the attachment actually belongs to an active entity reached via draft activation. The logic works for the happy path but conflates "no draft row" with "is active entity", which can produce a wrong sideEffectSource path and fail to trigger the UI side-effect on the correct entity.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

Comment on lines +203 to +206
const [active, draft] = await Promise.all([
SELECT.one.from(target).where(where),
SELECT.one.from(target.drafts).where(where),
])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: updateStatus is called with a complex CQL expression array for where (not a plain key object) when scanning completes, but the spawned SELECT.one.from(target).where(where) and SELECT.one.from(target.drafts).where(where) will receive that complex expression on the initial updateStatus("Scanning") call and then again on the final status call with a hash-qualified expression. The CQL where array including or and xpr nodes may not be supported by all CDS DB adapters in the same way for SELECT, potentially returning no result and silently skipping the event emission.

Consider querying by the original plain keys object (passed as a separate parameter, or stored before the compound expression is built) rather than the raw where argument which can be an arbitrary CQL expression.


Please provide feedback on the review comment by checking the appropriate box:

  • 🌟 Awesome comment, a human might have missed that.
  • ✅ Helpful comment
  • 🤷 Neutral
  • ❌ This comment is not helpful

Comment thread srv/attachments/basic.js Outdated
Comment thread lib/plugin.js
Comment thread tests/unit/websocket.test.js
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.

1 participant