From c25b6ea3eefc24901af7b79708d71269598abc9c Mon Sep 17 00:00:00 2001
From: Piotr Gawron <piotr.gawron@uni.lu>
Date: Mon, 26 Aug 2019 15:51:00 +0200
Subject: [PATCH] sql exception in add project doesn't hung the process
---
CHANGELOG | 1 +
.../api/projects/ProjectRestImpl.java | 5 +-
.../services/impl/ProjectService.java | 156 ++++++++++--------
.../web/ControllerIntegrationTest.java | 7 +
...llerIntegrationTestWithoutTransaction.java | 29 +++-
5 files changed, 123 insertions(+), 75 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index 8d21a978c9..ee5360ff15 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -24,6 +24,7 @@ minerva (14.0.0~beta.0) unstable; urgency=low
reaction (#908)
* Bug fix: user with modify access to the project can edit it in admin panel
(#901)
+ * Bug fix: creating project with too long name hung (#916)
-- Piotr Gawron <piotr.gawron@uni.lu> Mon, 21 Aug 2019 21:00:00 +0200
diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/ProjectRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/ProjectRestImpl.java
index 43103b52b0..56a3eed9cf 100644
--- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/ProjectRestImpl.java
+++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/ProjectRestImpl.java
@@ -334,9 +334,11 @@ public class ProjectRestImpl extends BaseRestImpl {
throws QueryException, IOException, SecurityException {
Project project = getProjectService().getProjectByProjectId(projectId);
if (project != null) {
- logger.debug(project.getProjectId());
throw new ObjectExistsException("Project with given id already exists");
}
+ if (projectId.length() > 255) {
+ throw new QueryException("projectId is too long");
+ }
CreateProjectParams params = new CreateProjectParams();
String directory = computePathForProject(projectId, path);
String fileId = getFirstValue(data.get("file-id"));
@@ -721,6 +723,7 @@ public class ProjectRestImpl extends BaseRestImpl {
public String content;
@SuppressWarnings("unused")
public String level;
+
public LogEntry(int id, String content, String level) {
this.id = id;
this.content = content;
diff --git a/service/src/main/java/lcsb/mapviewer/services/impl/ProjectService.java b/service/src/main/java/lcsb/mapviewer/services/impl/ProjectService.java
index cb26f8ee29..8b9bf83a11 100644
--- a/service/src/main/java/lcsb/mapviewer/services/impl/ProjectService.java
+++ b/service/src/main/java/lcsb/mapviewer/services/impl/ProjectService.java
@@ -316,101 +316,108 @@ public class ProjectService implements IProjectService {
}
@Override
- public void createProject(final CreateProjectParams params) throws SecurityException {
+ public void createProject(final CreateProjectParams params) {
// this count down is used to wait for asynchronous thread to initialize
// data in the db (probably it would be better to move the initialization to
// main thread)
final CountDownLatch waitForInitialData = new CountDownLatch(1);
+ final List<Throwable> problems = new ArrayList<>();
+
Thread computations = new Thread(new Runnable() {
@Override
public void run() {
- if (params.isAsync()) {
- // because we are running this in separate thread we need to open a
- // new session for db connection
- dbUtils.createSessionForCurrentThread();
- }
-
- Project project = createProjectFromParams(params);
- projectDao.add(project);
- if (params.isAsync()) {
- projectDao.commit();
- }
- waitForInitialData.countDown();
- double[] outOfMemoryBuffer;
- MinervaLoggerAppender appender = MinervaLoggerAppender.createAppender();
try {
- logger.debug("Running: " + params.getProjectId() + "; " + params.getProjectFile());
- outOfMemoryBuffer = new double[OUT_OF_MEMORY_BACKUP_BUFFER_SIZE];
- for (int i = 0; i < OUT_OF_MEMORY_BACKUP_BUFFER_SIZE; i++) {
- outOfMemoryBuffer[i] = Math.random() * OUT_OF_MEMORY_BACKUP_BUFFER_SIZE;
+ if (params.isAsync()) {
+ // because we are running this in separate thread we need to open a
+ // new session for db connection
+ dbUtils.createSessionForCurrentThread();
}
- UploadedFileEntry file = params.getProjectFile();
- project.setInputData(file);
-
- createModel(params, project);
- Model originalModel = project.getModels().iterator().next().getModel();
- new CreateHierarchyCommand(originalModel, generator.computeZoomLevels(originalModel),
- generator.computeZoomFactor(originalModel)).execute();
- for (Model model : originalModel.getSubmodels()) {
- new CreateHierarchyCommand(model, generator.computeZoomLevels(model), generator.computeZoomFactor(model))
- .execute();
+ Project project = createProjectFromParams(params);
+ projectDao.add(project);
+ if (params.isAsync()) {
+ projectDao.commit();
}
+ waitForInitialData.countDown();
+ double[] outOfMemoryBuffer;
+ MinervaLoggerAppender appender = MinervaLoggerAppender.createAppender();
+ try {
+ logger.debug("Running: " + params.getProjectId() + "; " + params.getProjectFile());
+ outOfMemoryBuffer = new double[OUT_OF_MEMORY_BACKUP_BUFFER_SIZE];
+ for (int i = 0; i < OUT_OF_MEMORY_BACKUP_BUFFER_SIZE; i++) {
+ outOfMemoryBuffer[i] = Math.random() * OUT_OF_MEMORY_BACKUP_BUFFER_SIZE;
+ }
- createImages(project, params);
+ UploadedFileEntry file = params.getProjectFile();
+ project.setInputData(file);
- for (Layout layout : project.getLayouts()) {
- for (DataOverlayImageLayer imageLayer : layout.getDataOverlayImageLayers()) {
- String[] tmp = imageLayer.getDirectory().split("[\\\\/]");
- imageLayer.setDirectory(tmp[tmp.length - 1]);
+ createModel(params, project);
+ Model originalModel = project.getModels().iterator().next().getModel();
+ new CreateHierarchyCommand(originalModel, generator.computeZoomLevels(originalModel),
+ generator.computeZoomFactor(originalModel)).execute();
+ for (Model model : originalModel.getSubmodels()) {
+ new CreateHierarchyCommand(model, generator.computeZoomLevels(model), generator.computeZoomFactor(model))
+ .execute();
}
- }
- projectDao.update(project);
+ createImages(project, params);
- if (params.isAnalyzeAnnotations()) {
- analyzeAnnotations(originalModel, params);
- }
- MinervaLoggerAppender.unregisterLogEventStorage(appender);
- project.addLoggingInfo(appender);
+ for (Layout layout : project.getLayouts()) {
+ for (DataOverlayImageLayer imageLayer : layout.getDataOverlayImageLayers()) {
+ String[] tmp = imageLayer.getDirectory().split("[\\\\/]");
+ imageLayer.setDirectory(tmp[tmp.length - 1]);
+ }
+ }
- if (params.isCacheModel()) {
- cacheData(originalModel, params);
- }
+ projectDao.update(project);
- updateProjectStatus(project, ProjectStatus.DONE, IProgressUpdater.MAX_PROGRESS, params);
- if (project.getNotifyEmail() != null && !project.getNotifyEmail().equals("")) {
- try {
- sendSuccesfullEmail(originalModel);
- } catch (MessagingException e) {
- logger.error(e, e);
+ if (params.isAnalyzeAnnotations()) {
+ analyzeAnnotations(originalModel, params);
}
- }
+ MinervaLoggerAppender.unregisterLogEventStorage(appender);
+ project.addLoggingInfo(appender);
- logger.info("Project " + project.getProjectId() + " created successfully.");
- } catch (HibernateException e) {
- outOfMemoryBuffer = null;
- logger.error("Problem with database", e);
- handleHibernateExceptionReporting(params, e);
- } catch (Exception e) {
- outOfMemoryBuffer = null;
- handleCreateProjectException(params, e);
- } catch (OutOfMemoryError oome) {
- // release some memory
- outOfMemoryBuffer = null;
- logger.error("Out of memory", oome);
- if (project != null) {
- project.setErrors("Out of memory: " + oome.getMessage());
- }
- updateProjectStatus(project, ProjectStatus.FAIL, IProgressUpdater.MAX_PROGRESS, params);
- } finally {
- if (params.isAsync()) {
- // close the transaction for this thread
- dbUtils.closeSessionForCurrentThread();
+ if (params.isCacheModel()) {
+ cacheData(originalModel, params);
+ }
+
+ updateProjectStatus(project, ProjectStatus.DONE, IProgressUpdater.MAX_PROGRESS, params);
+ if (project.getNotifyEmail() != null && !project.getNotifyEmail().equals("")) {
+ try {
+ sendSuccesfullEmail(originalModel);
+ } catch (MessagingException e) {
+ logger.error(e, e);
+ }
+ }
+
+ logger.info("Project " + project.getProjectId() + " created successfully.");
+ } catch (HibernateException e) {
+ outOfMemoryBuffer = null;
+ logger.error("Problem with database", e);
+ handleHibernateExceptionReporting(params, e);
+ } catch (Exception e) {
+ outOfMemoryBuffer = null;
+ handleCreateProjectException(params, e);
+ } catch (OutOfMemoryError oome) {
+ // release some memory
+ outOfMemoryBuffer = null;
+ logger.error("Out of memory", oome);
+ if (project != null) {
+ project.setErrors("Out of memory: " + oome.getMessage());
+ }
+ updateProjectStatus(project, ProjectStatus.FAIL, IProgressUpdater.MAX_PROGRESS, params);
+ } finally {
+ if (params.isAsync()) {
+ // close the transaction for this thread
+ dbUtils.closeSessionForCurrentThread();
+ }
+ MinervaLoggerAppender.unregisterLogEventStorage(appender);
}
- MinervaLoggerAppender.unregisterLogEventStorage(appender);
+ } catch (Throwable t) {
+ problems.add(t);
+ waitForInitialData.countDown();
}
}
@@ -424,7 +431,10 @@ public class ProjectService implements IProjectService {
try {
waitForInitialData.await();
} catch (InterruptedException e1) {
- logger.error(e1, e1);
+ problems.add(e1);
+ }
+ if (problems.size() > 0) {
+ throw new RuntimeException(problems.get(0));
}
}
diff --git a/web/src/test/java/lcsb/mapviewer/web/ControllerIntegrationTest.java b/web/src/test/java/lcsb/mapviewer/web/ControllerIntegrationTest.java
index 3983113dc8..ad197888d8 100644
--- a/web/src/test/java/lcsb/mapviewer/web/ControllerIntegrationTest.java
+++ b/web/src/test/java/lcsb/mapviewer/web/ControllerIntegrationTest.java
@@ -249,6 +249,13 @@ abstract public class ControllerIntegrationTest {
});
}
+ protected void removeFileInSeparateThread(UploadedFileEntry fileEntry) throws Exception {
+ callInSeparateThread(() -> {
+ fileDao.delete(fileDao.getById(fileEntry.getId()));
+ return null;
+ });
+ }
+
protected UploadedFileEntry createFileInSeparateThread(String content, User user) throws Exception {
return callInSeparateThread(() -> {
return createFile(content, user);
diff --git a/web/src/test/java/lcsb/mapviewer/web/ProjectControllerIntegrationTestWithoutTransaction.java b/web/src/test/java/lcsb/mapviewer/web/ProjectControllerIntegrationTestWithoutTransaction.java
index a1f33cbaa9..0ce88ed33a 100644
--- a/web/src/test/java/lcsb/mapviewer/web/ProjectControllerIntegrationTestWithoutTransaction.java
+++ b/web/src/test/java/lcsb/mapviewer/web/ProjectControllerIntegrationTestWithoutTransaction.java
@@ -62,7 +62,7 @@ public class ProjectControllerIntegrationTestWithoutTransaction extends Controll
mockMvc.perform(request).andExpect(status().is2xxSuccessful());
} finally {
- callInSeparateThread(()->{
+ callInSeparateThread(() -> {
try {
waitForProjectToFinishLoading(TEST_PROJECT);
} catch (InterruptedException e) {
@@ -74,4 +74,31 @@ public class ProjectControllerIntegrationTestWithoutTransaction extends Controll
}
}
+ @Test
+ public void addInvalidProject() throws Exception {
+ User admin = userService.getUserByLogin(BUILT_IN_ADMIN_LOGIN);
+ UploadedFileEntry fileEntry = createFileInSeparateThread(
+ new String(Files.readAllBytes(Paths.get("./src/test/resources/generic.xml")), "UTF-8"),
+ admin);
+ try {
+ String invalidId = "aaaaaaaaxvncnbvscbnmcnbmccbnsbnsdsnbmdsvbnmsdvnbmsdbmnbndvmsbnmsvdnbmnmbdsvnbmdsvxncbmbnmscbnzdnbnabnsbnamsdbmnsadbmnasdbnmnbmsadbnmasdnbasdbnmsadnbnbmsadbnmadsnbmadsnbnbsadnbmadsbnndsabnbmdasbnmdsajqwrhgjrwhjghgjwerghjwreghwewnjnnbbbnbnbmbnbnzcmnnbmzcnmbcsbnmcsnbcnbzmnbczxnbmczxnbmcxznbcnxbmznbmxzcnbzcxnnbcxznbmzcnbczxnbmnbzcxnbmcznnczbnbzcnbmzcbnmbncznbcznbcz";
+
+ String body = EntityUtils.toString(new UrlEncodedFormEntity(Arrays.asList(
+ new BasicNameValuePair("file-id", String.valueOf(fileEntry.getId())),
+ new BasicNameValuePair("mapCanvasType", "OPEN_LAYERS"),
+ new BasicNameValuePair("parser",
+ "lcsb.mapviewer.converter.model.celldesigner.CellDesignerXmlParser"))));
+
+ RequestBuilder request = post("/projects/" + invalidId)
+ .contentType(MediaType.APPLICATION_FORM_URLENCODED)
+ .content(body)
+ .session(createSession(BUILT_IN_ADMIN_LOGIN, BUILT_IN_ADMIN_PASSWORD));
+
+ mockMvc.perform(request).andExpect(status().isBadRequest());
+
+ } finally {
+ removeFileInSeparateThread(fileEntry);
+ }
+ }
+
}
--
GitLab