Skip to content

Commit 36984c5

Browse files
notSumit25claude
andcommitted
fix(dashboards): optimistic locking + manage-ACL on generate/stream
Adds @Version to SavedDashboard so concurrent chat turns / favorite / share / update writes on the same row fail cleanly (409) instead of racing or leaking a raw Hibernate error message. Also tightens generate/stream to require manage (not just read) access, since it creates/mutates saved dashboards. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 1ffeef7 commit 36984c5

5 files changed

Lines changed: 51 additions & 1 deletion

File tree

backend/src/main/java/com/dbaagent/controller/DashboardGenerationController.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import com.dbaagent.service.security.AccessControlService;
77
import lombok.RequiredArgsConstructor;
88
import lombok.extern.slf4j.Slf4j;
9+
import org.springframework.dao.OptimisticLockingFailureException;
910
import org.springframework.http.HttpStatus;
1011
import org.springframework.http.MediaType;
1112
import org.springframework.http.ResponseEntity;
@@ -72,7 +73,10 @@ public ResponseEntity<?> generate(@RequestBody GenerateRequest request) {
7273
@PostMapping(value = "/generate/stream", produces = MediaType.TEXT_EVENT_STREAM_VALUE)
7374
public SseEmitter generateStream(@RequestBody GenerateRequest request) {
7475
requireValid(request);
75-
accessControlService.assertCanReadConnectionContent(request.connectionId());
76+
// This path creates/updates a SavedDashboard row (beginGenerationTurn etc.)
77+
// on every call, not just reads — a VIEWER (read-only) must not be able to
78+
// mint or mutate drafts via chat.
79+
accessControlService.assertCanManageConnectionContent(request.connectionId());
7680
SseEmitter emitter = new SseEmitter(600_000L);
7781

7882
// Resolve (or create) the target dashboard and record the user's message
@@ -87,6 +91,11 @@ public SseEmitter generateStream(@RequestBody GenerateRequest request) {
8791
} catch (IllegalArgumentException | IllegalStateException e) {
8892
sendErrorAndComplete(emitter, e.getMessage());
8993
return emitter;
94+
} catch (OptimisticLockingFailureException e) {
95+
// Lost the race to another concurrent submit on the same dashboard —
96+
// same user-facing shape as the "already running" case above.
97+
sendErrorAndComplete(emitter, "A generation is already running for this dashboard.");
98+
return emitter;
9099
}
91100
// The frontend needs this id right away (not just at the end) so a
92101
// brand-new dashboard is addressable — e.g. by a reload — well before

backend/src/main/java/com/dbaagent/controller/SavedDashboardController.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import com.dbaagent.service.security.AccessControlService;
66
import lombok.extern.slf4j.Slf4j;
77
import org.springframework.beans.factory.annotation.Autowired;
8+
import org.springframework.dao.OptimisticLockingFailureException;
89
import org.springframework.http.HttpStatus;
910
import org.springframework.http.ResponseEntity;
1011
import org.springframework.web.bind.annotation.*;
@@ -25,6 +26,19 @@ public class SavedDashboardController {
2526
@Autowired
2627
private AccessControlService accessControlService;
2728

29+
// Every write method below is load-then-save on a row a background generation
30+
// turn (SavedDashboardService.beginGenerationTurn etc.) may be writing at the
31+
// same time. Without this helper, the loser's raw Hibernate message
32+
// ("Unexpected row count... where id=? and version=?") leaked straight into
33+
// the API response as a 500 instead of a clean, retryable conflict.
34+
private static ResponseEntity<Map<String, Object>> conflict(OptimisticLockingFailureException e) {
35+
log.warn("Dashboard update lost a concurrent-write race: {}", e.getMessage());
36+
Map<String, Object> body = new HashMap<>();
37+
body.put("success", false);
38+
body.put("message", "This dashboard changed elsewhere just now — please retry.");
39+
return ResponseEntity.status(HttpStatus.CONFLICT).body(body);
40+
}
41+
2842
/** Publish this dashboard to the web (opt-in, revocable public link). */
2943
@PostMapping("/{id}/share")
3044
public ResponseEntity<Map<String, Object>> enableShare(@PathVariable UUID id) {
@@ -39,6 +53,8 @@ public ResponseEntity<Map<String, Object>> enableShare(@PathVariable UUID id) {
3953
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(Map.of("success", false, "message", e.getMessage()));
4054
} catch (org.springframework.web.server.ResponseStatusException e) {
4155
throw e;
56+
} catch (OptimisticLockingFailureException e) {
57+
return conflict(e);
4258
} catch (Exception e) {
4359
log.error("Error enabling dashboard share", e);
4460
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
@@ -59,6 +75,8 @@ public ResponseEntity<Map<String, Object>> setSharePassword(@PathVariable UUID i
5975
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(Map.of("success", false, "message", e.getMessage()));
6076
} catch (org.springframework.web.server.ResponseStatusException e) {
6177
throw e;
78+
} catch (OptimisticLockingFailureException e) {
79+
return conflict(e);
6280
} catch (Exception e) {
6381
log.error("Error setting dashboard share password", e);
6482
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
@@ -79,6 +97,8 @@ public ResponseEntity<Map<String, Object>> disableShare(@PathVariable UUID id) {
7997
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(Map.of("success", false, "message", e.getMessage()));
8098
} catch (org.springframework.web.server.ResponseStatusException e) {
8199
throw e;
100+
} catch (OptimisticLockingFailureException e) {
101+
return conflict(e);
82102
} catch (Exception e) {
83103
log.error("Error disabling dashboard share", e);
84104
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
@@ -196,6 +216,8 @@ public ResponseEntity<Map<String, Object>> updateDashboard(@PathVariable UUID id
196216
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(errorResponse);
197217
} catch (org.springframework.web.server.ResponseStatusException e) {
198218
throw e;
219+
} catch (OptimisticLockingFailureException e) {
220+
return conflict(e);
199221
} catch (Exception e) {
200222
log.error("Error updating saved dashboard", e);
201223
Map<String, Object> errorResponse = new HashMap<>();
@@ -255,6 +277,8 @@ public ResponseEntity<Map<String, Object>> toggleFavorite(@PathVariable UUID id)
255277
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(errorResponse);
256278
} catch (org.springframework.web.server.ResponseStatusException e) {
257279
throw e;
280+
} catch (OptimisticLockingFailureException e) {
281+
return conflict(e);
258282
} catch (Exception e) {
259283
log.error("Error toggling favorite", e);
260284
Map<String, Object> errorResponse = new HashMap<>();

backend/src/main/java/com/dbaagent/model/SavedDashboard.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,16 @@ public boolean isSharePasswordSet() {
102102
@Column(nullable = false)
103103
private LocalDateTime updatedAt;
104104

105+
// Optimistic lock: beginGenerationTurn/appendAgentReply/completeBuildTurn/
106+
// appendErrorReply all do load-then-save on this same row, and two overlapping
107+
// turns (e.g. a slow build finishing after the user already sent a follow-up
108+
// chat) would otherwise silently lose whichever save landed first. Hibernate
109+
// bumps this on every UPDATE and rejects a save whose version is stale with
110+
// OptimisticLockException instead of overwriting.
111+
@Version
112+
@Column(nullable = false)
113+
private Long version = 0L;
114+
105115
// Jackson deserializes create/update bodies via Lombok's all-args constructor
106116
// (Spring's parameter-names module), which bypasses the field defaults and
107117
// leaves these NOT-NULL booleans null when the client omits them. Coerce here

backend/src/main/java/com/dbaagent/service/SavedDashboardService.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,12 @@ public void deleteDashboardsByConnection(String connectionId) {
253253
* kicks off the slow agent work). Marking generationStatus=RUNNING here —
254254
* not after the agent finishes — is what lets a reload mid-generation see
255255
* "still working" instead of nothing at all.
256+
*
257+
* isFreshlyRunning below is check-then-act, but SavedDashboard.version
258+
* (@Version) is the real guard: save() is `UPDATE ... WHERE version=?`, so a
259+
* racing loser gets OptimisticLockingFailureException, not a double-append
260+
* (caught in DashboardGenerationController same as the IllegalStateException
261+
* below). Verified with concurrent requests: loser rejected, zero writes.
256262
*/
257263
@Transactional
258264
public SavedDashboard beginGenerationTurn(UUID dashboardId, String connectionId, String prompt) {
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE saved_dashboards ADD COLUMN IF NOT EXISTS version BIGINT NOT NULL DEFAULT 0;

0 commit comments

Comments
 (0)