Skip to content

Add upload status tracking and per-upload change detection (issue #205) - #208

Merged
willr3 merged 1 commit into
Hyperfoil:mainfrom
stalep:issue_205_upload_status
Aug 6, 2026
Merged

Add upload status tracking and per-upload change detection (issue #205)#208
willr3 merged 1 commit into
Hyperfoil:mainfrom
stalep:issue_205_upload_status

Conversation

@stalep

@stalep stalep commented Jul 17, 2026

Copy link
Copy Markdown
Member

Adds REST endpoint for polling upload processing status and retrieving change detection results triggered by specific uploads.

New endpoint:
GET /api/folder/upload/{uploadId}/status
Returns UploadStatus: state (PROCESSING/COMPLETED/FAILED), durationMs,
error message, and list of Change records with changeType enum.

Implementation:

  • UploadService: in-memory bounded tracker map (max 1000 entries, 10-min retention after completion). Observes ChangeDetectedEvent to capture per-upload detection results (dispatch=true only).
  • UploadStatus/Change DTOs in api package. Change.ChangeType enum maps from NodeType detection types.
  • NodeType.toChangeType() converts detection node types to Change enum.
  • ChangeDetectedEvent: added rootValueId and nodeType fields so the UploadService observer can attribute changes to specific uploads.
  • FolderService.upload(): registers with UploadService for both the normal and empty-folder (no work) paths.

@stalep
stalep requested review from Arjun-Parmani and willr3 July 17, 2026 09:08
Comment thread src/main/java/io/hyperfoil/tools/h5m/svc/UploadService.java Outdated
Comment thread src/test/java/io/hyperfoil/tools/h5m/rest/RestEndpointTest.java Outdated
@stalep
stalep force-pushed the issue_205_upload_status branch 3 times, most recently from bf1f202 to d123c69 Compare July 24, 2026 11:39
@stalep
stalep force-pushed the issue_205_upload_status branch from d123c69 to cc46f8f Compare July 28, 2026 18:41
@willr3

willr3 commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

I am trying to understand the workflow that will make use of this PR. My theory is the following:

  1. Upload data to h5m and record the returned id
  2. Poll h5m for resulting calculations to complete.
  3. Report any Changes that were detected as a result of uploading the data.

Assuming that is the workflow we are targeting then I think there are some aspects of the proposed solution that we should consider

  1. The timeout on the cache means the "changes caused by the upload" are only available at that enpoint for a fixed time (10 minutes) and retrieving that information after that time will rely on the ValueService api
  2. The UploadService exists for tracking the processing of an upload but perhaps it should be merged with the ProcessingService to support monitoring both upload and recalculation triggered processing?
  3. The Change object appears to be a Value with additional information about the Node that created the Value. Is that additional information necessary for the workflow? If not then I supsect we can avoid the complexity of the additional entities and use the ValueService to provide the requisite information (perhaps with the ability to filter for change detection nodes?)

@stalep

stalep commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

I am trying to understand the workflow that will make use of this PR. My theory is the following:

1. Upload data to h5m and record the returned id

2. Poll h5m for resulting calculations to complete.

3. Report any Changes that were detected as a result of uploading the data.

Basically yes.

Assuming that is the workflow we are targeting then I think there are some aspects of the proposed solution that we should consider

1. The timeout on the cache means the "changes caused by the upload" are only available at that enpoint for a fixed time (10 minutes) and retrieving that information after that time will rely on the ValueService api

Yes, I acknowledge it's limiting atm.

2. The `UploadService` exists for tracking the processing of an upload but perhaps it should be merged with the `ProcessingService` to support monitoring both upload and recalculation triggered processing?

Imo they have different lifecycles, ProcessingService manages persisted ProcessingTrackerEntity for crash recovery, while UploadService is intentionally in-memory with short retention for real-time polling.

3. The `Change` object appears to be a `Value` with additional information about the `Node` that created the `Value`. Is that additional information necessary for the workflow? If not then I supsect we can avoid the complexity of the additional entities and use the `ValueService` to provide the requisite information (perhaps with the ability to filter for change detection nodes?)

I agree it simplifies the h5m codebase to just return detection value IDs and let clients query details via ValueService. However, I think that shifts the complexity to the API consumer and makes the user experience worse. The intended workflow for a CI integration is:

  1. Upload data
  2. Poll for completion
  3. Immediately see what changed and why — in a single response

With List detectionValueIds, the client would need to make N additional API calls to fetch each value, then look up the node for each to understand the detection type. For a CI system that just wants to know "did this upload trigger any regressions?", the Change summary gives enough context (node name, detection type, value ID) in one response without extra round-trips.

I took another look at this pr and how it "works" with the current notification system regarding changes. I'm thinking that the "push/pull" change system has diverged a bit. I'm looking at changing this pr now such that they have the "same" backend system.

@stalep
stalep force-pushed the issue_205_upload_status branch 2 times, most recently from b319450 to 5885110 Compare August 5, 2026 09:53

@willr3 willr3 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I do not think we should have sql to fetch Values outside of the ValueService.

Comment thread src/main/java/io/hyperfoil/tools/h5m/svc/UploadService.java Outdated
@stalep
stalep force-pushed the issue_205_upload_status branch 3 times, most recently from 76d8eca to 1656252 Compare August 5, 2026 17:39
Comment thread src/main/java/io/hyperfoil/tools/h5m/rest/ValueResource.java Outdated
Comment thread src/main/java/io/hyperfoil/tools/h5m/rest/ValueResource.java Outdated
@stalep
stalep force-pushed the issue_205_upload_status branch from 1656252 to 0b6d82b Compare August 5, 2026 19:25

@willr3 willr3 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

two minor changes so that we use consistent language with users

Comment thread src/main/java/io/hyperfoil/tools/h5m/event/ChangeNotification.java Outdated
Comment thread src/main/java/io/hyperfoil/tools/h5m/notification/WebhookPlugin.java Outdated
…uery (issue Hyperfoil#205)

Separate upload processing status from value querying, following
REST best practices (Will's feedback):

Processing status endpoint:
  GET /api/processing/{id} → {id, state, error}
  Backed by WorkService.getTracker() with DB fallback (checks
  if root value exists to infer COMPLETED after tracker cleanup).

Value descendants endpoint:
  GET /api/value/{id}/descendants?detection=true → List<Value>
  Returns detection node values scoped to an upload (root value).
  Uses in-memory cache (populated by ChangeDetectedEvent observer
  in ValueService) with DB fallback via recursive CTE query.
  Without ?detection=true, returns all descendant values.

Eliminated components:
  - UploadService — replaced by ValueService cache + WorkService
    tracker. Processing state from WorkService.getTracker(),
    detection values from ValueService.getDetectionDescendants().
  - UploadStatus record — processing status is a simple
    ProcessingStatus record on ValueResource.
  - FolderResource.getUploadStatus() — moved to ValueResource.

Change record stays as internal event type:
  - Used by ChangeDetectedEvent → NotificationService → plugins
  - Not exposed in REST API — pull consumers get Value DTOs
  - Push consumers (webhooks) get enriched Change records

Detection value cache in ValueService:
  - ConcurrentHashMap<Long, List<Value>> keyed by root value ID
  - Populated by @observes ChangeDetectedEvent observer
  - 10-minute retention, max 1000 entries
  - getDetectionDescendants() checks cache, falls back to DB
@stalep
stalep force-pushed the issue_205_upload_status branch from 0b6d82b to 441ff94 Compare August 6, 2026 00:35
@willr3
willr3 merged commit 48d5b67 into Hyperfoil:main Aug 6, 2026
2 checks passed
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.

3 participants