diff --git a/web/pgadmin/tools/sqleditor/static/js/components/sections/MainToolBar.jsx b/web/pgadmin/tools/sqleditor/static/js/components/sections/MainToolBar.jsx index 04908e2e2ff..1a7b2646eca 100644 --- a/web/pgadmin/tools/sqleditor/static/js/components/sections/MainToolBar.jsx +++ b/web/pgadmin/tools/sqleditor/static/js/components/sections/MainToolBar.jsx @@ -50,7 +50,14 @@ function changeQueryExecutionSettings(type, api, transId, value) { let url = url_for(`sqleditor.${type}`, { 'trans_id': transId, }); - return api.post(url, JSON.stringify(value)); + return api.post(url, JSON.stringify(value)).then((res)=>{ + // The server answers 200 with status false when it cannot check the + // transaction or connection, in which case nothing was changed. + if(res?.data?.data?.status === false) { + throw new Error(res.data.data.result || gettext('Failed to change the execute option.')); + } + return res; + }); } // Execute options that also live on the server side transaction object, so @@ -84,6 +91,12 @@ export function getChangedExecutionOptions(restored, current) { export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddToMacros}) { const eventBus = useContext(QueryToolEventsContext); const queryToolCtx = useContext(QueryToolContext); + /* A connection change replaces the transaction, so callbacks that outlive + a render read the current context from here rather than capturing it. */ + const queryToolCtxRef = React.useRef(queryToolCtx); + React.useLayoutEffect(()=>{ + queryToolCtxRef.current = queryToolCtx; + }, [queryToolCtx]); const queryToolConnCtx = useContext(QueryToolConnectionContext); const modalId = MODAL_DIALOGS.QT_CONFIRMATIONS; @@ -107,6 +120,13 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT const [pendingExecOptions, setPendingExecOptions] = useState(null); // The number of restored execute options still being sent to the server. const [applyingExecOptions, setApplyingExecOptions] = useState(0); + /* Each execute option request rewrites the whole transaction object kept in + the session, so they are sent one at a time, chained on this promise, so + that a later request cannot overwrite an earlier one's change. */ + const execOptionsQueueRef = React.useRef(Promise.resolve()); + // Whether an execute option changed from the menu is still being sent. + const [changingExecOption, setChangingExecOption] = useState(false); + const changingExecOptionRef = React.useRef(false); const {isSaveToolDataEnabled} = useApplicationState() ?? {}; /* Menu button refs */ const saveAsMenuRef = React.useRef(null); @@ -168,22 +188,57 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT explain(true); }, [explain]); + const sendExecOption = useCallback((option, value)=>{ + // Start from the queue so that a synchronous failure is handled too. + // Read the context when the request is sent, as it may be queued. + const request = execOptionsQueueRef.current + .then(()=>{ + const {api, params} = queryToolCtxRef.current; + return changeQueryExecutionSettings(option, api, params.trans_id, value); + }); + execOptionsQueueRef.current = request.catch(()=>{/* handled by the caller */}); + return request; + }, []); + const checkMenuClick = useCallback((e)=>{ - setCheckedMenuItems((prev)=>{ - let newVal = !prev[e.value]; - if (e.value === 'server_cursor') { - queryToolCtx.updateServerCursor({server_cursor: newVal}); + if(EXECUTION_OPTIONS.includes(e.value)) { + if(changingExecOptionRef.current) { + return; } - if(e.value === 'auto_commit' || e.value === 'auto_rollback' || e.value === 'server_cursor') { - changeQueryExecutionSettings(e.value, queryToolCtx.api, queryToolCtx.params.trans_id, newVal) - .catch ((error)=>{ - newVal = prev[e.value]; - eventBus.fireEvent(QUERY_TOOL_EVENTS.HANDLE_API_ERROR, error, { - checkTransaction: true, - }); + const option = e.value; + const oldVal = Boolean(checkedMenuItemsRef.current[option]); + const newVal = !oldVal; + const setOption = (val)=>{ + setCheckedMenuItems((prev)=>({...prev, [option]: val})); + if(option === 'server_cursor') { + queryToolCtxRef.current.updateServerCursor({server_cursor: val}); + } + }; + changingExecOptionRef.current = true; + setChangingExecOption(true); + setOption(newVal); + sendExecOption(option, newVal) + .catch((error)=>{ + setOption(oldVal); + // Save the reverted value so the tab state matches the server. + if(isSaveToolDataEnabled?.('sqleditor')) { + eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_QUERY_TOOL_DATA); + } + eventBus.fireEvent(QUERY_TOOL_EVENTS.HANDLE_API_ERROR, error, { + checkTransaction: true, }); + }) + .finally(()=>{ + changingExecOptionRef.current = false; + setChangingExecOption(false); + }); + if(isSaveToolDataEnabled?.('sqleditor')) { + eventBus.fireEvent(QUERY_TOOL_EVENTS.TRIGGER_SAVE_QUERY_TOOL_DATA); } - + return; + } + setCheckedMenuItems((prev)=>{ + let newVal = !prev[e.value]; let otherVars = {}; if (e.value === 'explain_serialize_binary' && newVal) { otherVars = { 'explain_serialize_text': false }; @@ -242,9 +297,7 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT const options = Object.entries(pendingExecOptions); setApplyingExecOptions((prev)=>prev + options.length); options.forEach(([option, value])=>{ - // Start from a promise so that a synchronous failure is handled too. - Promise.resolve() - .then(()=>changeQueryExecutionSettings(option, queryToolCtx.api, queryToolCtx.params.trans_id, value)) + sendExecOption(option, value) .catch((error)=>{ setCheckedMenuItems((prev)=>({...prev, [option]: !value})); if(option === 'server_cursor') { @@ -759,11 +812,11 @@ export function MainToolBar({containerRef, onFilterClick, onManageMacros, onAddT label={gettext('Execute Options Menu')} > {gettext('Auto commit?')} + disabled={changingExecOption} onClick={checkMenuClick}>{gettext('Auto commit?')} {gettext('Auto rollback on error?')} + disabled={changingExecOption} onClick={checkMenuClick}>{gettext('Auto rollback on error?')} {gettext('Use server cursor?')} + disabled={changingExecOption} onClick={checkMenuClick}>{gettext('Use server cursor?')} { ); queryToolCtx.params.is_query_tool = isQueryTool; const result = render(ui(connected)); - return {...result, setConnected: (conn) => result.rerender(ui(conn))}; + return { + ...result, + setConnected: (conn, ctx) => { + if(ctx) { + queryToolCtx = ctx; + } + result.rerender(ui(conn)); + }, + }; }; beforeEach(() => { @@ -160,6 +168,21 @@ describe('MainToolBar toolbar options restore', () => { expect(queryToolCtx.api.post).toHaveBeenCalledWith('/mock/sqleditor.server_cursor/1234', 'true'); }); + it('sends restored execute options to the transaction current when connected', async () => { + const {setConnected} = renderToolBar(false); + await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull()); + + act(() => { + eventBus.fireEvent(QUERY_TOOL_EVENTS.RESTORE_TOOLBAR_OPTIONS, {auto_commit: false}); + }); + await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current.auto_commit).toBe(false)); + + // The transaction is replaced before the pending option is sent. + setConnected(true, {...queryToolCtx, params: {...queryToolCtx.params, trans_id: 5678}}); + await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledTimes(1)); + expect(queryToolCtx.api.post).toHaveBeenCalledWith('/mock/sqleditor.auto_commit/5678', 'false'); + }); + it('reverts and saves a restored execute option the server rejects', async () => { queryToolCtx.api.post = jest.fn(() => Promise.reject(new Error('failed'))); const fireEvent = jest.spyOn(eventBus, 'fireEvent'); @@ -202,11 +225,13 @@ describe('MainToolBar toolbar options restore', () => { act(() => { eventBus.fireEvent(QUERY_TOOL_EVENTS.RESTORE_TOOLBAR_OPTIONS, {auto_commit: false, auto_rollback: true}); }); - await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledTimes(2)); + await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledTimes(1)); await waitFor(() => expectGated(true)); - // Still disabled until every option has reached the server. + // Still disabled until every option has reached the server, and the + // options are sent one at a time. await act(async () => { resolvers[0]({data: {}}); }); + await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledTimes(2)); expectGated(true); await act(async () => { resolvers[1]({data: {}}); }); @@ -327,3 +352,136 @@ describe('MainToolBar toolbar options restore', () => { expect(queryToolCtx.api.post).not.toHaveBeenCalled(); }); }); + +describe('MainToolBar execute options menu', () => { + let eventBus, queryToolCtx; + const ThemedMainToolBar = withTheme(MainToolBar); + + const renderToolBar = () => { + const containerRef = React.createRef(); + const ui = (ctx) => ( + + + +
+ {/* noop */}} + onManageMacros={()=>{/* noop */}} onAddToMacros={()=>{/* noop */}} /> +
+ + + + ); + const result = render(ui(queryToolCtx)); + return {...result, setContext: (ctx) => result.rerender(ui(ctx))}; + }; + + const menuItem = (label) => document.querySelector(`li[data-label="${label}"]`); + + const openMenuAndClick = async (label) => { + if(!menuItem(label)) { + await act(async () => { + fireDomEvent.click(document.querySelector('button[data-label="Execute options"]')); + }); + await waitFor(() => expect(menuItem(label)).not.toBeNull()); + } + await act(async () => { fireDomEvent.click(menuItem(label)); }); + }; + + beforeEach(() => { + eventBus = new EventBus(); + queryToolCtx = { + params: {trans_id: 1234, is_query_tool: true, macros: []}, + preferences: {sqleditor: {...sqleditorPrefs}, editor: {}}, + api: {post: jest.fn(() => Promise.resolve({data: {data: {status: true}}}))}, + modal: {}, + toolbarOptionsRef: {current: null}, + updateServerCursor: jest.fn(), + }; + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('sends a changed execute option to the server', async () => { + renderToolBar(); + await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull()); + + await openMenuAndClick('Use server cursor?'); + await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledWith('/mock/sqleditor.server_cursor/1234', 'true')); + await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current.server_cursor).toBe(true)); + expect(queryToolCtx.updateServerCursor).toHaveBeenLastCalledWith({server_cursor: true}); + }); + + it('sends execute options to the current transaction after a connection change', async () => { + const {setContext} = renderToolBar(); + await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull()); + + // A connection change replaces the transaction with a new one. + const newCtx = { + ...queryToolCtx, + params: {...queryToolCtx.params, trans_id: 5678}, + updateServerCursor: jest.fn(), + }; + setContext(newCtx); + + await openMenuAndClick('Use server cursor?'); + await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledWith('/mock/sqleditor.server_cursor/5678', 'true')); + expect(queryToolCtx.api.post).not.toHaveBeenCalledWith('/mock/sqleditor.server_cursor/1234', expect.anything()); + expect(newCtx.updateServerCursor).toHaveBeenLastCalledWith({server_cursor: true}); + expect(queryToolCtx.updateServerCursor).not.toHaveBeenCalled(); + }); + + it('reverts an execute option when the request fails', async () => { + queryToolCtx.api.post = jest.fn(() => Promise.reject(new Error('failed'))); + const fireEvent = jest.spyOn(eventBus, 'fireEvent'); + renderToolBar(); + await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull()); + + await openMenuAndClick('Use server cursor?'); + await waitFor(() => { + expect(fireEvent).toHaveBeenCalledWith(QUERY_TOOL_EVENTS.HANDLE_API_ERROR, expect.any(Error), {checkTransaction: true}); + }); + await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current.server_cursor).toBe(false)); + expect(queryToolCtx.updateServerCursor).toHaveBeenLastCalledWith({server_cursor: false}); + expect(menuItem('Use server cursor?')).toHaveAttribute('data-checked', 'false'); + fireEvent.mockRestore(); + }); + + it('reverts an execute option when the server returns status false', async () => { + queryToolCtx.api.post = jest.fn(() => Promise.resolve({data: {data: {status: false, result: 'Not connected'}}})); + const fireEvent = jest.spyOn(eventBus, 'fireEvent'); + renderToolBar(); + await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull()); + + await openMenuAndClick('Auto commit?'); + await waitFor(() => { + expect(fireEvent).toHaveBeenCalledWith(QUERY_TOOL_EVENTS.HANDLE_API_ERROR, + expect.objectContaining({message: 'Not connected'}), {checkTransaction: true}); + }); + await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current.auto_commit).toBe(true)); + fireEvent.mockRestore(); + }); + + it('disables the execute options whilst a change is being sent', async () => { + let resolvePost; + queryToolCtx.api.post = jest.fn(() => new Promise((resolve) => { resolvePost = resolve; })); + renderToolBar(); + await waitFor(() => expect(queryToolCtx.toolbarOptionsRef.current).not.toBeNull()); + + await openMenuAndClick('Auto commit?'); + await waitFor(() => expect(queryToolCtx.api.post).toHaveBeenCalledTimes(1)); + ['Auto commit?', 'Auto rollback on error?', 'Use server cursor?'].forEach((label) => { + expect(menuItem(label)).toHaveAttribute('aria-disabled', 'true'); + }); + + // A click whilst the request is in flight is ignored. + await act(async () => { fireDomEvent.click(menuItem('Auto rollback on error?')); }); + expect(queryToolCtx.api.post).toHaveBeenCalledTimes(1); + + await act(async () => { resolvePost({data: {data: {status: true}}}); }); + await waitFor(() => expect(menuItem('Auto rollback on error?')).not.toHaveAttribute('aria-disabled', 'true')); + expect(queryToolCtx.toolbarOptionsRef.current.auto_commit).toBe(false); + expect(queryToolCtx.toolbarOptionsRef.current.auto_rollback).toBe(false); + }); +});