Vojtech Szocs has uploaded a new change for review. Change subject: webadmin: Fix UI plugin REST API / Engine session refresh issue ......................................................................
webadmin: Fix UI plugin REST API / Engine session refresh issue Patch http://gerrit.ovirt.org/#/c/35185/ ensured that UI plugin REST API session uses the existing Engine user session, instead of creating a separate one. This caused REST API session heartbeat mechanism to eventually keep the Engine user session alive, effectively preventing GUI from auto-logout due to GUI user's inactivity. This patch syncs REST API session with Engine user session: upon each backend call that refreshes Engine session (action or query with refresh=true), REST API session is refreshed. REST API session refresh is done max. once per 1 minute to prevent flooding the backend. Change-Id: I2ffd3198239040f67313238a837af98db16e79ea Bug-Url: https://bugzilla.redhat.com/1172726 Bug-Url: https://bugzilla.redhat.com/1168842 Signed-off-by: Vojtech Szocs <vsz...@redhat.com> --- M frontend/webadmin/modules/frontend/exclude-filters.xml A frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/EngineSessionRefreshed.java M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java M frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendActionTest.java M frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendTest.java M frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManagerTest.java M frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java 7 files changed, 89 insertions(+), 12 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/10/36710/1 diff --git a/frontend/webadmin/modules/frontend/exclude-filters.xml b/frontend/webadmin/modules/frontend/exclude-filters.xml index ae17cae..3b9ca50 100644 --- a/frontend/webadmin/modules/frontend/exclude-filters.xml +++ b/frontend/webadmin/modules/frontend/exclude-filters.xml @@ -38,6 +38,18 @@ <Bug code="UuF"/> </Match> + <!-- + findbugs complains about unused field(s) in this class. + This is a GWT class and used to generate code. + + findbugs reason: + UuF: Unused field (UUF_UNUSED_FIELD) + --> + <Match> + <Class name="org.ovirt.engine.ui.frontend.communication.EngineSessionRefreshed" /> + <Bug code="UuF"/> + </Match> + <Match> <Class name="org.ovirt.engine.ui.frontend.AsyncQuery" /> <Field name="converterCallback"/> diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/EngineSessionRefreshed.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/EngineSessionRefreshed.java new file mode 100644 index 0000000..8f1c57c --- /dev/null +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/EngineSessionRefreshed.java @@ -0,0 +1,11 @@ +package org.ovirt.engine.ui.frontend.communication; + +import com.gwtplatform.dispatch.annotation.GenEvent; + +/** + * Event triggered when the Engine session is refreshed, i.e. due to executing backend action or query. + */ +@GenEvent +public class EngineSessionRefreshed { + +} diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java index 2c00dd2..58b9e61 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java @@ -6,13 +6,20 @@ import org.ovirt.engine.core.common.action.LoginUserParameters; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdcReturnValueBase; +import org.ovirt.engine.core.common.queries.VdcQueryParametersBase; +import com.google.gwt.event.shared.EventBus; import com.google.inject.Inject; /** * This class is a singleton and manages how {@code VdcOperation}s are added to the queue to be processed. */ public class VdcOperationManager { + + /** + * Event bus to propagate events related to operation processing. + */ + private final EventBus eventBus; /** * The operation queue. It can hold any kind of VdcOperation. @@ -34,7 +41,8 @@ * @param operationProcessor the operation processor. */ @Inject - public VdcOperationManager(final OperationProcessor operationProcessor) { + public VdcOperationManager(final EventBus eventBus, final OperationProcessor operationProcessor) { + this.eventBus = eventBus; this.processor = operationProcessor; } @@ -73,6 +81,10 @@ if (isAllowedToExecute) { if (operationCanBeAdded && operationQueue.add(operation)) { processor.processOperation(this); + + if (engineSessionRefreshed(operation)) { + EngineSessionRefreshedEvent.fire(eventBus); + } } } @@ -169,4 +181,22 @@ public void retrieveFromHttpSession(final String key, final StorageCallback callback) { processor.retrieveFromHttpSession(key, callback); } + + /** + * Returns {@code true} if execution of given operation caused the Engine session to be refreshed. + */ + boolean engineSessionRefreshed(VdcOperation<?, ?> operation) { + // Actions always refresh the Engine session + if (operation.isAction()) { + return true; + } + + // Queries optionally refresh the Engine session + else if (((VdcQueryParametersBase) operation.getParameter()).getRefresh()) { + return true; + } + + return false; + } + } diff --git a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendActionTest.java b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendActionTest.java index b3256dc..97cf835 100644 --- a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendActionTest.java +++ b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendActionTest.java @@ -116,7 +116,7 @@ when(mockXsrfRpcRequestBuilder.getXsrfToken()).thenReturn(new XsrfToken("Something")); //$NON-NLS-1$ OperationProcessor operationProcessor = new OperationProcessor(communicationsProvider); operationProcessor.setScheduler(fakeScheduler); - VdcOperationManager operationsManager = new VdcOperationManager(operationProcessor); + VdcOperationManager operationsManager = new VdcOperationManager(mockEventBus, operationProcessor); operationsManager.setLoggedIn(true); frontend = new Frontend(operationsManager, mockCanDoActionErrorsTranslator, mockVdsmErrorsTranslator, mockEventBus); diff --git a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendTest.java b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendTest.java index 189c228..5d7ad90 100644 --- a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendTest.java +++ b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/FrontendTest.java @@ -109,7 +109,7 @@ when(mockXsrfRpcRequestBuilder.getXsrfToken()).thenReturn(new XsrfToken("Something")); //$NON-NLS-1$ OperationProcessor operationProcessor = new OperationProcessor(communicationsProvider); operationProcessor.setScheduler(fakeScheduler); - VdcOperationManager operationsManager = new VdcOperationManager(operationProcessor); + VdcOperationManager operationsManager = new VdcOperationManager(mockEventBus, operationProcessor); operationsManager.setLoggedIn(true); frontend = new Frontend(operationsManager, mockCanDoActionErrorsTranslator, mockVdsmErrorsTranslator, mockEventBus); diff --git a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManagerTest.java b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManagerTest.java index 6fa0b92..ef0839b 100644 --- a/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManagerTest.java +++ b/frontend/webadmin/modules/frontend/src/test/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManagerTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -23,12 +24,18 @@ import org.ovirt.engine.core.common.queries.VdcQueryParametersBase; import org.ovirt.engine.core.common.queries.VdcQueryType; +import com.google.gwt.event.shared.EventBus; + @RunWith(MockitoJUnitRunner.class) public class VdcOperationManagerTest { VdcOperationManager testManager; @Mock OperationProcessor mockOperationProcessor; + + @Mock + EventBus mockEventBus; + @Captor ArgumentCaptor<VdcOperation<VdcActionType, LoginUserParameters>> loginOperationCaptor; @@ -36,7 +43,7 @@ @Before public void setUp() throws Exception { final VdcReturnValueBase result = new VdcReturnValueBase(); - testManager = new VdcOperationManager(mockOperationProcessor); + testManager = new VdcOperationManager(mockEventBus, mockOperationProcessor); final VdcReturnValueBase loginResult = new VdcReturnValueBase(); LoginUserParameters params = new LoginUserParameters("test", "test", //$NON-NLS-1$ //$NON-NLS-2$ "test"); //$NON-NLS-1$ @@ -70,6 +77,7 @@ new VdcActionParametersBase(), null); testManager.addOperation(testOperation); verify(mockOperationProcessor).processOperation(testManager); + verify(mockEventBus).fireEvent(any(EngineSessionRefreshedEvent.class)); assertEquals("Operations must match", testOperation, testManager.pollOperation()); //$NON-NLS-1$ } @@ -79,10 +87,12 @@ VdcQueryParametersBase>(VdcQueryType.Search, new VdcQueryParametersBase(), null); testManager.addOperation(testOperation); verify(mockOperationProcessor).processOperation(testManager); + verify(mockEventBus).fireEvent(any(EngineSessionRefreshedEvent.class)); // Second add, shouldn't add and generate an event. testManager.addOperation(testOperation); // Verify it is only called once (from before) verify(mockOperationProcessor).processOperation(testManager); + verify(mockEventBus).fireEvent(any(EngineSessionRefreshedEvent.class)); } @Test @@ -101,6 +111,7 @@ operationList.add(testOperation3); testManager.addOperationList(operationList); verify(mockOperationProcessor, times(3)).processOperation(testManager); + verify(mockEventBus, times(2)).fireEvent(any(EngineSessionRefreshedEvent.class)); assertEquals("First poll should be action", testManager.pollOperation(), testOperation1); //$NON-NLS-1$ assertEquals("Second poll should be query", testManager.pollOperation(), testOperation2); //$NON-NLS-1$ assertNull("Third poll should be null", testManager.pollOperation()); //$NON-NLS-1$ diff --git a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java index d6ead3f..d68fc0f 100644 --- a/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java +++ b/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java @@ -5,6 +5,8 @@ import org.ovirt.engine.ui.common.utils.HttpUtils; import org.ovirt.engine.ui.frontend.Frontend; +import org.ovirt.engine.ui.frontend.communication.EngineSessionRefreshedEvent; +import org.ovirt.engine.ui.frontend.communication.EngineSessionRefreshedEvent.EngineSessionRefreshedHandler; import org.ovirt.engine.ui.frontend.communication.StorageCallback; import org.ovirt.engine.ui.frontend.utils.BaseContextPathData; @@ -38,7 +40,7 @@ * <p> * Triggers {@link RestApiSessionAcquiredEvent} upon acquiring or reusing REST API session. */ -public class RestApiSessionManager { +public class RestApiSessionManager implements EngineSessionRefreshedHandler { private static class RestApiRequestCallback implements RequestCallback { @@ -81,6 +83,8 @@ private String restApiSessionTimeout = DEFAULT_SESSION_TIMEOUT; private String restApiSessionId; + private boolean refreshRestApiSession = false; + @Inject public RestApiSessionManager(EventBus eventBus) { this.eventBus = eventBus; @@ -89,6 +93,15 @@ // send authentication headers to URLs ending in api/, otherwise it will send them to URLs ending in /, and // this causes problems in other applications, for example in the reports application. this.restApiBaseUrl = BaseContextPathData.getInstance().getPath() + "api/"; //$NON-NLS-1$ + + eventBus.addHandler(EngineSessionRefreshedEvent.getType(), this); + } + + @Override + public void onEngineSessionRefreshed(EngineSessionRefreshedEvent event) { + if (restApiSessionId != null) { + refreshRestApiSession = true; + } } public void setSessionTimeout(String sessionTimeout) { @@ -140,18 +153,18 @@ Scheduler.get().scheduleFixedDelay(new RepeatingCommand() { @Override public boolean execute() { - String sessionId = getSessionId(); + boolean sessionInUse = restApiSessionId != null; - if (sessionId != null) { + if (sessionInUse && refreshRestApiSession) { // The browser takes care of sending JSESSIONID cookie for this request automatically sendRequest(createRequest(null), new RestApiRequestCallback()); - // The session is still in use, proceed with the heartbeat - return true; - } else { - // The session has been released, cancel the heartbeat - return false; + // Reset the refresh flag + refreshRestApiSession = false; } + + // Proceed with the heartbeat only when the session is still in use + return sessionInUse; } }, SESSION_HEARTBEAT_MS); } -- To view, visit http://gerrit.ovirt.org/36710 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2ffd3198239040f67313238a837af98db16e79ea Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Vojtech Szocs <vsz...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
