From 273d9fd92312ea2b107f80dc2c5a7f6d4498fd58 Mon Sep 17 00:00:00 2001 From: anhefti Date: Wed, 17 Feb 2021 10:21:19 +0100 Subject: [PATCH 1/2] Fixed master and created tests --- .../dao/impl/WebserviceInfoDAOImpl.java | 66 ++++++++++---- .../session/impl/ExamSessionControlTask.java | 6 -- .../config/application-ws.properties | 1 + .../integration/api/admin/WebserviceTest.java | 85 +++++++++++++++++++ 4 files changed, 136 insertions(+), 22 deletions(-) create mode 100644 src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/WebserviceTest.java diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/WebserviceInfoDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/WebserviceInfoDAOImpl.java index 325e57ef..d0cbf95b 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/WebserviceInfoDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/WebserviceInfoDAOImpl.java @@ -35,13 +35,16 @@ public class WebserviceInfoDAOImpl implements WebserviceInfoDAO { private final WebserviceServerInfoRecordMapper webserviceServerInfoRecordMapper; private final long masterDelayTimeThreshold; + private final boolean forceMaster; public WebserviceInfoDAOImpl( final WebserviceServerInfoRecordMapper webserviceServerInfoRecordMapper, + @Value("${sebserver.webservice.forceMaster:false}") final boolean forceMaster, @Value("${sebserver.webservice.master.delay.threshold:10000}") final long masterDelayTimeThreshold) { this.webserviceServerInfoRecordMapper = webserviceServerInfoRecordMapper; this.masterDelayTimeThreshold = masterDelayTimeThreshold; + this.forceMaster = forceMaster; } @Transactional @@ -68,18 +71,24 @@ public class WebserviceInfoDAOImpl implements WebserviceInfoDAO { if (masters != null && !masters.isEmpty()) { if (masters.size() > 1) { + log.error("There are more then one master registered: ", masters); log.info("Reset masters and set this webservice as new master"); - masters.stream() - .forEach(masterRec -> this.webserviceServerInfoRecordMapper.updateByPrimaryKeySelective( - new WebserviceServerInfoRecord(masterRec.getId(), null, null, 0, 0L))); - this.setMasterTo(uuid); - return true; + + masters.stream().forEach(masterRec -> this.webserviceServerInfoRecordMapper + .updateByPrimaryKeySelective( + new WebserviceServerInfoRecord( + masterRec.getId(), + null, + null, + 0, + 0L))); + return this.setMasterTo(uuid); } final WebserviceServerInfoRecord masterRec = masters.get(0); if (masterRec.getUuid().equals(uuid)) { - // this webservice was the master and update time to remain being master + // This webservice is the master. Update time-stamp to remain being master final long now = Utils.getMillisecondsNow(); this.webserviceServerInfoRecordMapper.updateByPrimaryKeySelective( new WebserviceServerInfoRecord(masterRec.getId(), null, null, null, now)); @@ -91,20 +100,18 @@ public class WebserviceInfoDAOImpl implements WebserviceInfoDAO { return true; } else { // Another webservice is master. Check if still alive... + // Force this service to become master if the other master is not alive anymore + // Or if this service is forced to be the master service final long now = Utils.getMillisecondsNow(); final long lastUpdateSince = now - masterRec.getUpdateTime(); - if (lastUpdateSince > this.masterDelayTimeThreshold) { - log.info("Change webservice master form uuid: {} to uuid: {}", masterRec.getUuid(), uuid); - this.webserviceServerInfoRecordMapper.updateByPrimaryKeySelective( - new WebserviceServerInfoRecord(masterRec.getId(), null, null, 0, 0L)); - setMasterTo(uuid); + if (lastUpdateSince > this.masterDelayTimeThreshold || this.forceMaster) { + forceMaster(uuid, masterRec.getUuid(), masterRec.getId()); return true; } } } else { - // if we have no master yet - setMasterTo(uuid); - return true; + // We have no master yet so set this as master service + return setMasterTo(uuid); } return false; @@ -115,14 +122,41 @@ public class WebserviceInfoDAOImpl implements WebserviceInfoDAO { } } - private void setMasterTo(final String uuid) { + private void forceMaster(final String uuid, final String otherUUID, final Long otherId) { + + log.info("Change webservice master form uuid: {} to uuid: {}", otherUUID, uuid); + + this.webserviceServerInfoRecordMapper.updateByPrimaryKeySelective( + new WebserviceServerInfoRecord(otherId, null, null, 0, 0L)); + setMasterTo(uuid); + } + + private boolean setMasterTo(final String uuid) { log.info("Set webservice {} as master", uuid); final long now = Utils.getMillisecondsNow(); - this.webserviceServerInfoRecordMapper.updateByExampleSelective( + + // check if this is registered + final List entries = this.webserviceServerInfoRecordMapper.selectByExample() + .where(WebserviceServerInfoRecordDynamicSqlSupport.uuid, SqlBuilder.isEqualTo(uuid)) + .build() + .execute(); + + if (entries == null || entries.isEmpty()) { + log.warn("The webservice with uuid: {} is not registered and cannot become a master", uuid); + return false; + } + + final Integer execute = this.webserviceServerInfoRecordMapper.updateByExampleSelective( new WebserviceServerInfoRecord(null, null, null, 1, now)) .where(WebserviceServerInfoRecordDynamicSqlSupport.uuid, SqlBuilder.isEqualTo(uuid)) .build() .execute(); + if (execute == null || execute.intValue() <= 0) { + log.error("Failed to update webservice with uuid: {} to become master", uuid); + return false; + } + + return true; } @Transactional diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionControlTask.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionControlTask.java index 48992209..f60add00 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionControlTask.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamSessionControlTask.java @@ -49,12 +49,6 @@ class ExamSessionControlTask implements DisposableBean { private final String examTaskCron; private final long pingUpdateRate; - // TODO considering to have some caching of running exams end dates here to improve performance - // the end check task has than only to first update missing running exams and then - // check the exam ending within the cached end date of the exam. if an exam has ended by - // applying the check to the cached value, the process can double-check if the end date has - // no change and update if needed or end the exam and remove from cache. - protected ExamSessionControlTask( final ExamDAO examDAO, final SEBClientConnectionService sebClientConnectionService, diff --git a/src/main/resources/config/application-ws.properties b/src/main/resources/config/application-ws.properties index efa28906..fcd19e39 100644 --- a/src/main/resources/config/application-ws.properties +++ b/src/main/resources/config/application-ws.properties @@ -32,6 +32,7 @@ sebserver.webservice.api.admin.clientSecret=${sebserver.password} sebserver.webservice.internalSecret=${sebserver.password} ### webservice networking +sebserver.webservice.forceMaster=false sebserver.webservice.distributed=false sebserver.webservice.http.external.scheme=https sebserver.webservice.http.external.servername= diff --git a/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/WebserviceTest.java b/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/WebserviceTest.java new file mode 100644 index 00000000..3770935c --- /dev/null +++ b/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/admin/WebserviceTest.java @@ -0,0 +1,85 @@ +/* + * Copyright (c) 2021 ETH Zürich, Educational Development and Technology (LET) + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ + +package ch.ethz.seb.sebserver.webservice.integration.api.admin; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; + +import ch.ethz.seb.sebserver.webservice.WebserviceInfo; +import ch.ethz.seb.sebserver.webservice.servicelayer.dao.WebserviceInfoDAO; + +public class WebserviceTest extends AdministrationAPIIntegrationTester { + + private static final String WEBSERVICE_1 = "WEBSERVICE_1"; + private static final String WEBSERVICE_2 = "WEBSERVICE_2"; + + @Autowired + private WebserviceInfoDAO webserviceInfoDAO; + @Autowired + private WebserviceInfo webserviceInfo; + + @Before + public void init() { + this.webserviceInfoDAO.unregister(this.webserviceInfo.getWebserviceUUID()); + this.webserviceInfoDAO.register(WEBSERVICE_1, "0.0.0.1"); + this.webserviceInfoDAO.register(WEBSERVICE_2, "0.0.0.1"); + } + + @After + public void cleanup() { + this.webserviceInfoDAO.unregister(WEBSERVICE_1); + this.webserviceInfoDAO.unregister(WEBSERVICE_2); + this.webserviceInfoDAO.register( + this.webserviceInfo.getWebserviceUUID(), + this.webserviceInfo.getLocalHostAddress()); + } + + @Test + public void testFistBecomeMaster() { + assertTrue(this.webserviceInfoDAO.isMaster(WEBSERVICE_1)); + assertFalse(this.webserviceInfoDAO.isMaster(WEBSERVICE_2)); + assertTrue(this.webserviceInfoDAO.isMaster(WEBSERVICE_1)); + assertTrue(this.webserviceInfoDAO.isMaster(WEBSERVICE_1)); + } + + @Test + public void testUnregister_OtherBecomeMaster() { + assertTrue(this.webserviceInfoDAO.isMaster(WEBSERVICE_1)); + assertTrue(this.webserviceInfoDAO.unregister(WEBSERVICE_1)); + assertFalse(this.webserviceInfoDAO.isMaster(WEBSERVICE_1)); + assertTrue(this.webserviceInfoDAO.isMaster(WEBSERVICE_2)); + } + + @Test + public void testOtherBecomeMasterAfterTimout() { + assertTrue(this.webserviceInfoDAO.isMaster(WEBSERVICE_1)); + assertFalse(this.webserviceInfoDAO.isMaster(WEBSERVICE_2)); + + try { + Thread.sleep(5000); + } catch (final InterruptedException e) { + } + + // Still not master + assertFalse(this.webserviceInfoDAO.isMaster(WEBSERVICE_2)); + + try { + Thread.sleep(6000); + } catch (final InterruptedException e) { + } + + assertTrue(this.webserviceInfoDAO.isMaster(WEBSERVICE_2)); + } + +} From 8c29d7e8f24df397a600d57d1e7d440dff12c412 Mon Sep 17 00:00:00 2001 From: anhefti Date: Wed, 17 Feb 2021 11:34:56 +0100 Subject: [PATCH 2/2] fixed import settings and tests --- .../gui/content/SEBExamConfigImportPopup.java | 52 ++++++++++++------- .../gui/content/SEBExamConfigList.java | 2 +- .../gui/content/SEBSettingsForm.java | 12 +++-- .../gui/integration/GuiIntegrationTest.java | 8 +++ .../api/exam/ExamAPIIntegrationTester.java | 7 +++ 5 files changed, 57 insertions(+), 24 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBExamConfigImportPopup.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBExamConfigImportPopup.java index 1b52f458..a717dcae 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBExamConfigImportPopup.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBExamConfigImportPopup.java @@ -68,9 +68,19 @@ public class SEBExamConfigImportPopup { this.pageService = pageService; } - public Function importFunction(final boolean newConfig) { + public Function importFunction() { + return importFunction(null); + } + + public Function importFunction(final Supplier tabSelectionSupplier) { return action -> { + final boolean newConfig = tabSelectionSupplier == null || tabSelectionSupplier.get() == null; + final PageContext context = (tabSelectionSupplier != null) + ? action.pageContext() + .withAttribute(SEBSettingsForm.ATTR_VIEW_INDEX, tabSelectionSupplier.get()) + : action.pageContext(); + final ModalInputDialog> dialog = new ModalInputDialog>( action.pageContext().getParent().getShell(), @@ -79,7 +89,7 @@ public class SEBExamConfigImportPopup { final ImportFormContext importFormContext = new ImportFormContext( this.pageService, - action.pageContext(), + context, newConfig); dialog.open( @@ -147,18 +157,6 @@ public class SEBExamConfigImportPopup { if (!configuration.hasError()) { context.publishInfo(SEBExamConfigForm.FORM_IMPORT_CONFIRM_TEXT_KEY); - final PageAction action = (newConfig) - ? this.pageService.pageActionBuilder(context) - .newAction(ActionDefinition.SEB_EXAM_CONFIG_IMPORT_TO_NEW_CONFIG) - .create() - : this.pageService.pageActionBuilder(context) - .newAction(ActionDefinition.SEB_EXAM_CONFIG_MODIFY) - .create(); - - this.pageService.firePageEvent( - new ActionEvent(action), - action.pageContext()); - } else { final Exception error = configuration.getError(); if (error instanceof RestCallError) { @@ -177,14 +175,15 @@ public class SEBExamConfigImportPopup { .notifyImportError(EntityType.CONFIGURATION_NODE, error); } }); - return true; + + } else { + formHandle.getContext().notifyError( + SEBExamConfigForm.FORM_TITLE, + configuration.getError()); } - formHandle.getContext().notifyError( - SEBExamConfigForm.FORM_TITLE, - configuration.getError()); - } + reloadPage(newConfig, context); return true; } else { formHandle.getContext().publishPageMessage( @@ -194,11 +193,26 @@ public class SEBExamConfigImportPopup { return false; } catch (final Exception e) { + reloadPage(newConfig, formHandle.getContext()); formHandle.getContext().notifyError(SEBExamConfigForm.FORM_TITLE, e); return true; } } + private void reloadPage(final boolean newConfig, final PageContext context) { + final PageAction action = (newConfig) + ? this.pageService.pageActionBuilder(context) + .newAction(ActionDefinition.SEB_EXAM_CONFIG_IMPORT_TO_NEW_CONFIG) + .create() + : this.pageService.pageActionBuilder(context) + .newAction(ActionDefinition.SEB_EXAM_CONFIG_MODIFY) + .create(); + + this.pageService.firePageEvent( + new ActionEvent(action), + action.pageContext()); + } + private boolean checkInput(final FormHandle formHandle, final boolean newConfig, final Form form) { if (newConfig) { diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBExamConfigList.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBExamConfigList.java index d5eef79b..a549a49e 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBExamConfigList.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBExamConfigList.java @@ -170,7 +170,7 @@ public class SEBExamConfigList implements TemplateComposer { .newAction(ActionDefinition.SEB_EXAM_CONFIG_IMPORT_TO_NEW_CONFIG) .withSelect( configTable.getGrantedSelection(this.currentUser, NO_MODIFY_PRIVILEGE_ON_OTHER_INSTITUTION), - this.sebExamConfigImportPopup.importFunction(true), + this.sebExamConfigImportPopup.importFunction(), EMPTY_SELECTION_TEXT_KEY) .noEventPropagation() .publishIf(examConfigGrant::im); diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBSettingsForm.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBSettingsForm.java index f4c2f96f..75509107 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBSettingsForm.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/SEBSettingsForm.java @@ -69,6 +69,8 @@ public class SEBSettingsForm implements TemplateComposer { private static final Logger log = LoggerFactory.getLogger(SEBSettingsForm.class); + public static final String ATTR_VIEW_INDEX = "VIEW_INDEX"; + private static final String VIEW_TEXT_KEY_PREFIX = "sebserver.examconfig.props.form.views."; private static final String KEY_SAVE_TO_HISTORY_SUCCESS = @@ -194,7 +196,7 @@ public class SEBSettingsForm implements TemplateComposer { } // set selection if available - final String viewIndex = pageContext.getAttribute("VIEW_INDEX"); + final String viewIndex = pageContext.getAttribute(ATTR_VIEW_INDEX); if (StringUtils.isNotBlank(viewIndex)) { try { tabFolder.setSelection(Integer.parseInt(viewIndex)); @@ -217,7 +219,7 @@ public class SEBSettingsForm implements TemplateComposer { .call() .onError(t -> notifyErrorOnSave(t, pageContext)); return action.withAttribute( - "VIEW_INDEX", + ATTR_VIEW_INDEX, String.valueOf(tabFolder.getSelectionIndex())); }) .withSuccess(KEY_SAVE_TO_HISTORY_SUCCESS) @@ -232,7 +234,7 @@ public class SEBSettingsForm implements TemplateComposer { .call() .getOrThrow(); return action.withAttribute( - "VIEW_INDEX", + ATTR_VIEW_INDEX, String.valueOf(tabFolder.getSelectionIndex())); }) .withSuccess(KEY_UNDO_SUCCESS) @@ -254,7 +256,9 @@ public class SEBSettingsForm implements TemplateComposer { .newAction(ActionDefinition.SEB_EXAM_CONFIG_IMPORT_TO_EXISTING_CONFIG) .withEntityKey(entityKey) - .withExec(this.sebExamConfigImportPopup.importFunction(false)) + .withExec(this.sebExamConfigImportPopup.importFunction( + () -> String.valueOf(tabFolder.getSelectionIndex()))) + .noEventPropagation() .publishIf(() -> examConfigGrant.iw() && !readonly && !isAttachedToExam) .newAction(ActionDefinition.SEB_EXAM_CONFIG_VIEW_PROP) diff --git a/src/test/java/ch/ethz/seb/sebserver/gui/integration/GuiIntegrationTest.java b/src/test/java/ch/ethz/seb/sebserver/gui/integration/GuiIntegrationTest.java index 4bc87b44..5ff8dada 100644 --- a/src/test/java/ch/ethz/seb/sebserver/gui/integration/GuiIntegrationTest.java +++ b/src/test/java/ch/ethz/seb/sebserver/gui/integration/GuiIntegrationTest.java @@ -36,6 +36,8 @@ import ch.ethz.seb.sebserver.gui.service.remote.webservice.api.RestServiceImpl; import ch.ethz.seb.sebserver.gui.service.remote.webservice.auth.OAuth2AuthorizationContextHolder; import ch.ethz.seb.sebserver.gui.service.remote.webservice.auth.SEBServerAuthorizationContext; import ch.ethz.seb.sebserver.gui.service.remote.webservice.auth.WebserviceURIService; +import ch.ethz.seb.sebserver.webservice.WebserviceInfo; +import ch.ethz.seb.sebserver.webservice.servicelayer.dao.WebserviceInfoDAO; @RunWith(SpringRunner.class) @SpringBootTest( @@ -59,13 +61,19 @@ public abstract class GuiIntegrationTest { protected JSONMapper jsonMapper; @Autowired protected FilterChainProxy springSecurityFilterChain; + @Autowired + private WebserviceInfoDAO webserviceInfoDAO; + @Autowired + private WebserviceInfo webserviceInfo; protected MockMvc mockMvc; @Before public void setup() { + this.webserviceInfoDAO.unregister(this.webserviceInfo.getWebserviceUUID()); this.mockMvc = MockMvcBuilders.webAppContextSetup(this.wac) .addFilter(this.springSecurityFilterChain).build(); + } protected OAuth2AuthorizationContextHolder getAuthorizationContextHolder() { diff --git a/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/exam/ExamAPIIntegrationTester.java b/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/exam/ExamAPIIntegrationTester.java index 1757b7f0..e53e9d4e 100644 --- a/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/exam/ExamAPIIntegrationTester.java +++ b/src/test/java/ch/ethz/seb/sebserver/webservice/integration/api/exam/ExamAPIIntegrationTester.java @@ -53,6 +53,8 @@ import ch.ethz.seb.sebserver.SEBServer; import ch.ethz.seb.sebserver.WebSecurityConfig; import ch.ethz.seb.sebserver.gbl.api.API; import ch.ethz.seb.sebserver.gbl.api.JSONMapper; +import ch.ethz.seb.sebserver.webservice.WebserviceInfo; +import ch.ethz.seb.sebserver.webservice.servicelayer.dao.WebserviceInfoDAO; import ch.ethz.seb.sebserver.webservice.servicelayer.sebconfig.ClientConfigService; import ch.ethz.seb.sebserver.webservice.weblayer.oauth.AdminAPIClientDetails; import ch.ethz.seb.sebserver.webservice.weblayer.oauth.WebClientDetailsService; @@ -77,6 +79,10 @@ public abstract class ExamAPIIntegrationTester { protected JSONMapper jsonMapper; @Autowired protected FilterChainProxy springSecurityFilterChain; + @Autowired + private WebserviceInfoDAO webserviceInfoDAO; + @Autowired + private WebserviceInfo webserviceInfo; protected MockMvc mockMvc; @@ -88,6 +94,7 @@ public abstract class ExamAPIIntegrationTester { @Before public void setup() { + this.webserviceInfoDAO.unregister(this.webserviceInfo.getWebserviceUUID()); this.mockMvc = MockMvcBuilders.webAppContextSetup(this.wac) .addFilter(this.springSecurityFilterChain).build(); Mockito.when(this.webClientDetailsService.loadClientByClientId(Mockito.anyString())).thenReturn(