Add recursive flag to prevent unwanted deletion of parents while they're already being deleted (#1378)

When a tab was being deleted, for instance, the last block that got
deleted would cascade to delete its parent tab, even though the
`DeleteTab` function was already going to do this. This would produce DB
collisions that would put the app into a bad state. Now we pass a
`recursive` flag that is used to determine whether to perform the
recursive close of the parents.

Also updates `DeleteWorkspace` so that named workspaces will always be
deleted if they're empty, even if `force` is false
This commit is contained in:
Evan Simkowitz 2024-12-03 21:39:06 -05:00 committed by GitHub
parent 2bd7579cd5
commit a72d3e5c7a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 23 additions and 13 deletions

View File

@ -121,7 +121,7 @@ func (svc *ObjectService) DeleteBlock(uiContext waveobj.UIContext, blockId strin
ctx, cancelFn := context.WithTimeout(context.Background(), DefaultTimeout) ctx, cancelFn := context.WithTimeout(context.Background(), DefaultTimeout)
defer cancelFn() defer cancelFn()
ctx = waveobj.ContextWithUpdates(ctx) ctx = waveobj.ContextWithUpdates(ctx)
err := wcore.DeleteBlock(ctx, blockId) err := wcore.DeleteBlock(ctx, blockId, true)
if err != nil { if err != nil {
return nil, fmt.Errorf("error deleting block: %w", err) return nil, fmt.Errorf("error deleting block: %w", err)
} }

View File

@ -169,7 +169,7 @@ func (svc *WorkspaceService) CloseTab(ctx context.Context, workspaceId string, t
blockcontroller.StopBlockController(blockId) blockcontroller.StopBlockController(blockId)
} }
}() }()
newActiveTabId, err := wcore.DeleteTab(ctx, workspaceId, tabId) newActiveTabId, err := wcore.DeleteTab(ctx, workspaceId, tabId, true)
if err != nil { if err != nil {
return nil, nil, fmt.Errorf("error closing tab: %w", err) return nil, nil, fmt.Errorf("error closing tab: %w", err)
} }

View File

@ -99,7 +99,11 @@ func createBlockObj(ctx context.Context, tabId string, blockDef *waveobj.BlockDe
}) })
} }
func DeleteBlock(ctx context.Context, blockId string) error { // Must delete all blocks individually first.
// Also deletes LayoutState.
// recursive: if true, will recursively close parent tab, window, workspace, if they are empty.
// Returns new active tab id, error.
func DeleteBlock(ctx context.Context, blockId string, recursive bool) error {
block, err := wstore.DBMustGet[*waveobj.Block](ctx, blockId) block, err := wstore.DBMustGet[*waveobj.Block](ctx, blockId)
if err != nil { if err != nil {
return fmt.Errorf("error getting block: %w", err) return fmt.Errorf("error getting block: %w", err)
@ -109,7 +113,7 @@ func DeleteBlock(ctx context.Context, blockId string) error {
} }
if len(block.SubBlockIds) > 0 { if len(block.SubBlockIds) > 0 {
for _, subBlockId := range block.SubBlockIds { for _, subBlockId := range block.SubBlockIds {
err := DeleteBlock(ctx, subBlockId) err := DeleteBlock(ctx, subBlockId, recursive)
if err != nil { if err != nil {
return fmt.Errorf("error deleting subblock %s: %w", subBlockId, err) return fmt.Errorf("error deleting subblock %s: %w", subBlockId, err)
} }
@ -123,14 +127,14 @@ func DeleteBlock(ctx context.Context, blockId string) error {
parentORef := waveobj.ParseORefNoErr(block.ParentORef) parentORef := waveobj.ParseORefNoErr(block.ParentORef)
if parentORef.OType == waveobj.OType_Tab { if parentORef.OType == waveobj.OType_Tab {
if parentBlockCount == 0 { if parentBlockCount == 0 && recursive {
// if parent tab has no blocks, delete the tab // if parent tab has no blocks, delete the tab
log.Printf("DeleteBlock: parent tab has no blocks, deleting tab %s", parentORef.OID) log.Printf("DeleteBlock: parent tab has no blocks, deleting tab %s", parentORef.OID)
parentWorkspaceId, err := wstore.DBFindWorkspaceForTabId(ctx, parentORef.OID) parentWorkspaceId, err := wstore.DBFindWorkspaceForTabId(ctx, parentORef.OID)
if err != nil { if err != nil {
return fmt.Errorf("error finding workspace for tab to delete %s: %w", parentORef.OID, err) return fmt.Errorf("error finding workspace for tab to delete %s: %w", parentORef.OID, err)
} }
newActiveTabId, err := DeleteTab(ctx, parentWorkspaceId, parentORef.OID) newActiveTabId, err := DeleteTab(ctx, parentWorkspaceId, parentORef.OID, true)
if err != nil { if err != nil {
return fmt.Errorf("error deleting tab %s: %w", parentORef.OID, err) return fmt.Errorf("error deleting tab %s: %w", parentORef.OID, err)
} }

View File

@ -120,6 +120,8 @@ func CreateWindow(ctx context.Context, winSize *waveobj.WinSize, workspaceId str
return GetWindow(ctx, windowId) return GetWindow(ctx, windowId)
} }
// CloseWindow closes a window and deletes its workspace if it is empty and not named.
// If fromElectron is true, it does not send an event to Electron.
func CloseWindow(ctx context.Context, windowId string, fromElectron bool) error { func CloseWindow(ctx context.Context, windowId string, fromElectron bool) error {
log.Printf("CloseWindow %s\n", windowId) log.Printf("CloseWindow %s\n", windowId)
window, err := GetWindow(ctx, windowId) window, err := GetWindow(ctx, windowId)

View File

@ -28,19 +28,22 @@ func CreateWorkspace(ctx context.Context, name string, icon string, color string
return ws, nil return ws, nil
} }
// If force is true, it will delete even if workspace is named.
// If workspace is empty, it will be deleted, even if it is named.
// Returns true if workspace was deleted, false if it was not deleted.
func DeleteWorkspace(ctx context.Context, workspaceId string, force bool) (bool, error) { func DeleteWorkspace(ctx context.Context, workspaceId string, force bool) (bool, error) {
log.Printf("DeleteWorkspace %s\n", workspaceId) log.Printf("DeleteWorkspace %s\n", workspaceId)
workspace, err := wstore.DBMustGet[*waveobj.Workspace](ctx, workspaceId) workspace, err := wstore.DBMustGet[*waveobj.Workspace](ctx, workspaceId)
if err != nil { if err != nil {
return false, fmt.Errorf("error getting workspace: %w", err) return false, fmt.Errorf("error getting workspace: %w", err)
} }
if workspace.Name != "" && workspace.Icon != "" && !force { if workspace.Name != "" && workspace.Icon != "" && !force && len(workspace.TabIds) > 0 {
log.Printf("Ignoring DeleteWorkspace for workspace %s as it is named\n", workspaceId) log.Printf("Ignoring DeleteWorkspace for workspace %s as it is named\n", workspaceId)
return false, nil return false, nil
} }
for _, tabId := range workspace.TabIds { for _, tabId := range workspace.TabIds {
log.Printf("deleting tab %s\n", tabId) log.Printf("deleting tab %s\n", tabId)
_, err := DeleteTab(ctx, workspaceId, tabId) _, err := DeleteTab(ctx, workspaceId, tabId, false)
if err != nil { if err != nil {
return false, fmt.Errorf("error closing tab: %w", err) return false, fmt.Errorf("error closing tab: %w", err)
} }
@ -104,8 +107,9 @@ func CreateTab(ctx context.Context, workspaceId string, tabName string, activate
// Must delete all blocks individually first. // Must delete all blocks individually first.
// Also deletes LayoutState. // Also deletes LayoutState.
// recursive: if true, will recursively close parent window, workspace, if they are empty.
// Returns new active tab id, error. // Returns new active tab id, error.
func DeleteTab(ctx context.Context, workspaceId string, tabId string) (string, error) { func DeleteTab(ctx context.Context, workspaceId string, tabId string, recursive bool) (string, error) {
ws, _ := wstore.DBGet[*waveobj.Workspace](ctx, workspaceId) ws, _ := wstore.DBGet[*waveobj.Workspace](ctx, workspaceId)
if ws == nil { if ws == nil {
return "", fmt.Errorf("workspace not found: %q", workspaceId) return "", fmt.Errorf("workspace not found: %q", workspaceId)
@ -117,7 +121,7 @@ func DeleteTab(ctx context.Context, workspaceId string, tabId string) (string, e
// close blocks (sends events + stops block controllers) // close blocks (sends events + stops block controllers)
for _, blockId := range tab.BlockIds { for _, blockId := range tab.BlockIds {
err := DeleteBlock(ctx, blockId) err := DeleteBlock(ctx, blockId, false)
if err != nil { if err != nil {
return "", fmt.Errorf("error deleting block %s: %w", blockId, err) return "", fmt.Errorf("error deleting block %s: %w", blockId, err)
} }
@ -141,7 +145,7 @@ func DeleteTab(ctx context.Context, workspaceId string, tabId string) (string, e
wstore.DBDelete(ctx, waveobj.OType_Tab, tabId) wstore.DBDelete(ctx, waveobj.OType_Tab, tabId)
wstore.DBDelete(ctx, waveobj.OType_LayoutState, tab.LayoutState) wstore.DBDelete(ctx, waveobj.OType_LayoutState, tab.LayoutState)
if newActiveTabId == "" { if newActiveTabId == "" && recursive {
windowId, err := wstore.DBFindWindowForWorkspaceId(ctx, workspaceId) windowId, err := wstore.DBFindWindowForWorkspaceId(ctx, workspaceId)
if err != nil { if err != nil {
return newActiveTabId, fmt.Errorf("unable to find window for workspace id %v: %w", workspaceId, err) return newActiveTabId, fmt.Errorf("unable to find window for workspace id %v: %w", workspaceId, err)

View File

@ -489,7 +489,7 @@ func (ws *WshServer) FileAppendIJsonCommand(ctx context.Context, data wshrpc.Com
} }
func (ws *WshServer) DeleteSubBlockCommand(ctx context.Context, data wshrpc.CommandDeleteBlockData) error { func (ws *WshServer) DeleteSubBlockCommand(ctx context.Context, data wshrpc.CommandDeleteBlockData) error {
err := wcore.DeleteBlock(ctx, data.BlockId) err := wcore.DeleteBlock(ctx, data.BlockId, false)
if err != nil { if err != nil {
return fmt.Errorf("error deleting block: %w", err) return fmt.Errorf("error deleting block: %w", err)
} }
@ -505,7 +505,7 @@ func (ws *WshServer) DeleteBlockCommand(ctx context.Context, data wshrpc.Command
if tabId == "" { if tabId == "" {
return fmt.Errorf("no tab found for block") return fmt.Errorf("no tab found for block")
} }
err = wcore.DeleteBlock(ctx, data.BlockId) err = wcore.DeleteBlock(ctx, data.BlockId, true)
if err != nil { if err != nil {
return fmt.Errorf("error deleting block: %w", err) return fmt.Errorf("error deleting block: %w", err)
} }