From 9b1ceb38cbec545ab75fa94dd0e25f8670a8f235 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 16:55:12 +0200 Subject: [PATCH 01/20] don't print stacktrace when assert fails --- .../java/lcsb/mapviewer/common/UnitTestFailedWatcher.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/commons/src/main/java/lcsb/mapviewer/common/UnitTestFailedWatcher.java b/commons/src/main/java/lcsb/mapviewer/common/UnitTestFailedWatcher.java index cf8337839..833ee1a0c 100644 --- a/commons/src/main/java/lcsb/mapviewer/common/UnitTestFailedWatcher.java +++ b/commons/src/main/java/lcsb/mapviewer/common/UnitTestFailedWatcher.java @@ -6,6 +6,8 @@ import org.junit.runner.Description; public class UnitTestFailedWatcher extends TestWatcher { @Override protected void failed(Throwable e, Description description) { - e.printStackTrace(); + if (!(e instanceof AssertionError)) { + e.printStackTrace(); + } } } -- GitLab From f84e94a29e53358a302cbf485ff4ce4a307a4ad8 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 16:55:36 +0200 Subject: [PATCH 02/20] provide info about deprecated columns in API --- .../projects/overlays/OverlayRestImpl.java | 17 +++++++++ .../services/utils/ColorSchemaReader.java | 34 +++++++++++++++-- .../web/OverlayControllerIntegrationTest.java | 37 +++++++++++++++++-- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java index 46a834171..0ff7457c8 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java @@ -12,6 +12,7 @@ import org.springframework.transaction.annotation.Transactional; import lcsb.mapviewer.api.*; import lcsb.mapviewer.common.Pair; +import lcsb.mapviewer.common.exception.InvalidStateException; import lcsb.mapviewer.common.geometry.ColorParser; import lcsb.mapviewer.model.Project; import lcsb.mapviewer.model.cache.FileEntry; @@ -26,6 +27,8 @@ import lcsb.mapviewer.persist.dao.cache.UploadedFileEntryDao; import lcsb.mapviewer.persist.dao.map.LayoutDao; import lcsb.mapviewer.services.interfaces.ILayoutService; import lcsb.mapviewer.services.interfaces.ILayoutService.CreateLayoutParams; +import lcsb.mapviewer.services.utils.ColorSchemaReader; +import lcsb.mapviewer.services.utils.data.ColorSchemaColumn; @Transactional @Service @@ -78,6 +81,8 @@ public class OverlayRestImpl extends BaseRestImpl { result.put("description", overlay.getDescription()); result.put("publicOverlay", overlay.isPublicLayout()); result.put("defaultOverlay", overlay.isDefaultOverlay()); + + result.put("deprecatedColumns", getDeprecatedColumns(overlay)); result.put("googleLicenseConsent", overlay.isGoogleLicenseConsent()); List> images = new ArrayList<>(); List imageList = new ArrayList<>(overlay.getDataOverlayImageLayers()); @@ -97,6 +102,18 @@ public class OverlayRestImpl extends BaseRestImpl { return result; } + private String getDeprecatedColumns(Layout overlay) { + try { + String result = ""; + for (ColorSchemaColumn column : new ColorSchemaReader().getDeprecatedColumns(overlay)) { + result += column.name() + ","; + } + return result; + } catch (IOException | InvalidColorSchemaException e) { + throw new InvalidStateException(e); + } + } + public List> getOverlayElements(String projectId, int overlayId, String columns) throws QueryException { diff --git a/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java b/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java index 72200ad59..a77487a00 100644 --- a/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java +++ b/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java @@ -1,10 +1,9 @@ package lcsb.mapviewer.services.utils; -import java.awt.*; +import java.awt.Color; import java.io.*; import java.lang.reflect.Field; import java.util.*; -import java.util.List; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; @@ -12,8 +11,7 @@ import org.apache.logging.log4j.Logger; import lcsb.mapviewer.annotation.services.MiriamConnector; import lcsb.mapviewer.common.*; -import lcsb.mapviewer.common.exception.InvalidArgumentException; -import lcsb.mapviewer.common.exception.NotImplementedException; +import lcsb.mapviewer.common.exception.*; import lcsb.mapviewer.common.geometry.ColorParser; import lcsb.mapviewer.converter.model.celldesigner.species.SpeciesMapping; import lcsb.mapviewer.converter.zip.ZipEntryFileFactory; @@ -808,4 +806,32 @@ public class ColorSchemaReader { } return readColorSchema(new ByteArrayInputStream(inputData), params); } + + public List getDeprecatedColumns(Layout overlay) + throws IOException, InvalidColorSchemaException { + List result = new ArrayList<>(); + BufferedReader br = new BufferedReader( + new InputStreamReader(new ByteArrayInputStream(overlay.getInputData().getFileContent()))); + + String line = br.readLine(); + while (line != null && line.startsWith("#")) { + line = br.readLine(); + } + String[] columns = line.split("\t"); + + Map schemaColumns = new HashMap<>(); + parseColumns(columns, schemaColumns, ColorSchemaType.GENERIC); + for (ColorSchemaColumn column : schemaColumns.keySet()) { + try { + Field f = ColorSchemaColumn.class.getField(column.name()); + if (column.getDepractedColumnName() != null || f.isAnnotationPresent(Deprecated.class)) { + result.add(column); + } + } catch (NoSuchFieldException | SecurityException e) { + throw new InvalidStateException(e); + } + } + return result; + } + } diff --git a/web/src/test/java/lcsb/mapviewer/web/OverlayControllerIntegrationTest.java b/web/src/test/java/lcsb/mapviewer/web/OverlayControllerIntegrationTest.java index f0dfd1ee1..705f1000d 100644 --- a/web/src/test/java/lcsb/mapviewer/web/OverlayControllerIntegrationTest.java +++ b/web/src/test/java/lcsb/mapviewer/web/OverlayControllerIntegrationTest.java @@ -1,6 +1,6 @@ package lcsb.mapviewer.web; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -34,6 +34,7 @@ import lcsb.mapviewer.model.user.User; import lcsb.mapviewer.persist.dao.map.LayoutDao; import lcsb.mapviewer.services.interfaces.IModelService; import lcsb.mapviewer.services.interfaces.IUserService; +import lcsb.mapviewer.services.utils.data.ColorSchemaColumn; @RunWith(SpringJUnit4ClassRunner.class) @Transactional @@ -422,9 +423,8 @@ public class OverlayControllerIntegrationTest extends ControllerIntegrationTest .getAsJsonArray().size()); } - private Layout createOverlay(User admin) { + private Layout createOverlay(User admin, String content) { UploadedFileEntry file = new UploadedFileEntry(); - String content = "elementIdentifier\tvalue\n\t-1"; file.setFileContent(content.getBytes()); file.setLength(content.getBytes().length); @@ -436,6 +436,10 @@ public class OverlayControllerIntegrationTest extends ControllerIntegrationTest return overlay; } + private Layout createOverlay(User admin) { + return createOverlay(admin, "elementIdentifier\tvalue\n\t-1"); + } + @Test public void testGetReactionsForOverlay() throws Exception { User admin = createAdmin(TEST_ADMIN_LOGIN, TEST_ADMIN_PASSWORD); @@ -1028,4 +1032,31 @@ public class OverlayControllerIntegrationTest extends ControllerIntegrationTest .andExpect(status().is2xxSuccessful()); } + @Test + public void testDeprecatedOverlayInformation() throws Exception { + createAdmin(TEST_ADMIN_LOGIN, TEST_ADMIN_PASSWORD); + + Layout overlay = createOverlay(null, + "elementIdentifier\t" + ColorSchemaColumn.REACTION_IDENTIFIER + "\tvalue\n\t\t-1"); + overlay.setPublicLayout(true); + layoutDao.update(overlay); + + MockHttpSession session = createSession(TEST_ADMIN_LOGIN, TEST_ADMIN_PASSWORD); + + RequestBuilder request = get( + "/projects/" + TEST_PROJECT + "/overlays/" + overlay.getId() + "/") + .contentType(MediaType.APPLICATION_FORM_URLENCODED) + .session(session); + + String response = mockMvc.perform(request) + .andExpect(status().is2xxSuccessful()) + .andReturn().getResponse().getContentAsString(); + String deprecatedColumns = new JsonParser() + .parse(response) + .getAsJsonObject().get("deprecatedColumns").getAsString(); + + assertNotNull(deprecatedColumns); + assertFalse(deprecatedColumns.isEmpty()); + } + } -- GitLab From 1c4e1dc8af1c9e5dd82001d8068784ef33388fba Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 17:12:00 +0200 Subject: [PATCH 03/20] returned value should be a list not a string --- .../mapviewer/api/projects/overlays/OverlayRestImpl.java | 7 ++++--- .../mapviewer/web/OverlayControllerIntegrationTest.java | 9 ++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java index 0ff7457c8..9bc011253 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java @@ -102,12 +102,13 @@ public class OverlayRestImpl extends BaseRestImpl { return result; } - private String getDeprecatedColumns(Layout overlay) { + private List getDeprecatedColumns(Layout overlay) { try { - String result = ""; + List result = new ArrayList<>(); for (ColorSchemaColumn column : new ColorSchemaReader().getDeprecatedColumns(overlay)) { - result += column.name() + ","; + result.add(column.name()); } + logger.debug(result); return result; } catch (IOException | InvalidColorSchemaException e) { throw new InvalidStateException(e); diff --git a/web/src/test/java/lcsb/mapviewer/web/OverlayControllerIntegrationTest.java b/web/src/test/java/lcsb/mapviewer/web/OverlayControllerIntegrationTest.java index 705f1000d..5368aef97 100644 --- a/web/src/test/java/lcsb/mapviewer/web/OverlayControllerIntegrationTest.java +++ b/web/src/test/java/lcsb/mapviewer/web/OverlayControllerIntegrationTest.java @@ -1,6 +1,6 @@ package lcsb.mapviewer.web; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -1051,12 +1051,11 @@ public class OverlayControllerIntegrationTest extends ControllerIntegrationTest String response = mockMvc.perform(request) .andExpect(status().is2xxSuccessful()) .andReturn().getResponse().getContentAsString(); - String deprecatedColumns = new JsonParser() + int deprecatedColumns = new JsonParser() .parse(response) - .getAsJsonObject().get("deprecatedColumns").getAsString(); + .getAsJsonObject().get("deprecatedColumns").getAsJsonArray().size(); - assertNotNull(deprecatedColumns); - assertFalse(deprecatedColumns.isEmpty()); + assertEquals(2, deprecatedColumns); } } -- GitLab From bb9cd9acec5f297b77ee3b939ea7a49a9b99f4ea Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 18:01:47 +0200 Subject: [PATCH 04/20] InvalidArgumentException shold rresult in 500 --- rest-api/src/main/java/lcsb/mapviewer/api/BaseController.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/BaseController.java b/rest-api/src/main/java/lcsb/mapviewer/api/BaseController.java index e5e295b8d..2698fdcf0 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/BaseController.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/BaseController.java @@ -41,8 +41,7 @@ public abstract class BaseController { } else if (e instanceof QueryException || e instanceof HttpMessageNotReadableException || e instanceof MissingServletRequestParameterException - || e instanceof HttpMediaTypeNotSupportedException - || e instanceof IllegalArgumentException) { + || e instanceof HttpMediaTypeNotSupportedException) { logger.error(e, e); return createErrorResponse("Query server error.", e.getMessage(), new HttpHeaders(), HttpStatus.BAD_REQUEST); } else if (e instanceof ServletRequestBindingException && e.getMessage().contains(Configuration.AUTH_TOKEN)) { -- GitLab From 0a7b2d9d7038c15c3d01e5553fc109d4a696cc48 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 18:03:04 +0200 Subject: [PATCH 05/20] file upload didn't work --- .../mapviewer/api/files/FileController.java | 2 +- .../mapviewer/services/impl/FileService.java | 12 +++++++ .../services/interfaces/IFileService.java | 3 ++ ...llerIntegrationTestWithoutTransaction.java | 36 +++++++++++++++++-- 4 files changed, 50 insertions(+), 3 deletions(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java b/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java index fee30ce28..4b4a99b8a 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java @@ -40,7 +40,7 @@ public class FileController extends BaseController { return fileRest.getFile(id); } - @PreAuthorize("@fileService.getById(#id)?.owner?.login == authentication.name") + @PreAuthorize("@fileService.getOwnerByFileId(#id)?.login == authentication.name") @PostMapping(value = "/{id}:uploadContent") public Map uploadContent(@PathVariable(value = "id") String id, @RequestBody byte[] data) throws ObjectNotFoundException { diff --git a/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java b/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java index 8519510c3..97ff42bdc 100644 --- a/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java +++ b/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java @@ -5,6 +5,7 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import lcsb.mapviewer.model.cache.UploadedFileEntry; +import lcsb.mapviewer.model.user.User; import lcsb.mapviewer.persist.dao.cache.UploadedFileEntryDao; import lcsb.mapviewer.services.interfaces.IFileService; @@ -24,4 +25,15 @@ public class FileService implements IFileService { return uploadedFileEntryDao.getById(id); } + @Override + public User getOwnerByFileId(int id) { + UploadedFileEntry entry = uploadedFileEntryDao.getById(id); + if (entry != null) { + uploadedFileEntryDao.getById(id).getOwner().getLogin(); + return uploadedFileEntryDao.getById(id).getOwner(); + } else { + return null; + } + } + } diff --git a/service/src/main/java/lcsb/mapviewer/services/interfaces/IFileService.java b/service/src/main/java/lcsb/mapviewer/services/interfaces/IFileService.java index 809e597a3..4ea629807 100644 --- a/service/src/main/java/lcsb/mapviewer/services/interfaces/IFileService.java +++ b/service/src/main/java/lcsb/mapviewer/services/interfaces/IFileService.java @@ -1,9 +1,12 @@ package lcsb.mapviewer.services.interfaces; import lcsb.mapviewer.model.cache.UploadedFileEntry; +import lcsb.mapviewer.model.user.User; public interface IFileService { UploadedFileEntry getById(int id); + User getOwnerByFileId(int id); + } diff --git a/web/src/test/java/lcsb/mapviewer/web/FileControllerIntegrationTestWithoutTransaction.java b/web/src/test/java/lcsb/mapviewer/web/FileControllerIntegrationTestWithoutTransaction.java index 9d32a6cf1..2b351c909 100644 --- a/web/src/test/java/lcsb/mapviewer/web/FileControllerIntegrationTestWithoutTransaction.java +++ b/web/src/test/java/lcsb/mapviewer/web/FileControllerIntegrationTestWithoutTransaction.java @@ -8,18 +8,23 @@ import java.util.Arrays; import org.apache.http.client.entity.UrlEncodedFormEntity; import org.apache.http.message.BasicNameValuePair; import org.apache.http.util.EntityUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpSession; -import org.springframework.test.annotation.Rollback; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.web.servlet.RequestBuilder; +import com.google.gson.JsonParser; + @RunWith(SpringJUnit4ClassRunner.class) -@Rollback public class FileControllerIntegrationTestWithoutTransaction extends ControllerIntegrationTest { + Logger logger = LogManager.getLogger(); + private static final String BUILT_IN_TEST_ADMIN_PASSWORD = "admin"; private static final String BUILT_IN_TEST_ADMIN_LOGIN = "admin"; @@ -42,4 +47,31 @@ public class FileControllerIntegrationTestWithoutTransaction extends ControllerI mockMvc.perform(request) .andExpect(status().is4xxClientError()); } + + @Test + public void createAndAppendToExistingFile() throws Exception { + MockHttpSession session = createSession(BUILT_IN_TEST_ADMIN_LOGIN, BUILT_IN_TEST_ADMIN_PASSWORD); + + String body = EntityUtils.toString(new UrlEncodedFormEntity(Arrays.asList( + new BasicNameValuePair("filename", "unknown.txt"), + new BasicNameValuePair("length", "29")))); + + RequestBuilder request = post("/files/") + .contentType(MediaType.APPLICATION_FORM_URLENCODED) + .content(body) + .session(session); + + String response = mockMvc.perform(request) + .andExpect(status().is2xxSuccessful()).andReturn().getResponse().getContentAsString(); + int id = new JsonParser().parse(response).getAsJsonObject().get("id").getAsInt(); + + String fileContent = "elementIdentifier\tvalue\nxx\t-1"; + + RequestBuilder postContentRequest = post("/files/" + id + ":uploadContent") + .content(fileContent) + .session(session); + + mockMvc.perform(postContentRequest) + .andExpect(status().is2xxSuccessful()); + } } -- GitLab From 11936eacd7f4c63197e7982caccff4e572c61488 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 18:07:57 +0200 Subject: [PATCH 06/20] information about deprecated columns is available for overlays with source --- .../projects/overlays/OverlayRestImpl.java | 1 - .../services/utils/ColorSchemaReader.java | 34 ++++++++++--------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java index 9bc011253..30d861064 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java @@ -108,7 +108,6 @@ public class OverlayRestImpl extends BaseRestImpl { for (ColorSchemaColumn column : new ColorSchemaReader().getDeprecatedColumns(overlay)) { result.add(column.name()); } - logger.debug(result); return result; } catch (IOException | InvalidColorSchemaException e) { throw new InvalidStateException(e); diff --git a/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java b/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java index a77487a00..7eb4e7430 100644 --- a/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java +++ b/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java @@ -810,25 +810,27 @@ public class ColorSchemaReader { public List getDeprecatedColumns(Layout overlay) throws IOException, InvalidColorSchemaException { List result = new ArrayList<>(); - BufferedReader br = new BufferedReader( - new InputStreamReader(new ByteArrayInputStream(overlay.getInputData().getFileContent()))); + if (overlay.getInputData() != null) { + BufferedReader br = new BufferedReader( + new InputStreamReader(new ByteArrayInputStream(overlay.getInputData().getFileContent()))); - String line = br.readLine(); - while (line != null && line.startsWith("#")) { - line = br.readLine(); - } - String[] columns = line.split("\t"); + String line = br.readLine(); + while (line != null && line.startsWith("#")) { + line = br.readLine(); + } + String[] columns = line.split("\t"); - Map schemaColumns = new HashMap<>(); - parseColumns(columns, schemaColumns, ColorSchemaType.GENERIC); - for (ColorSchemaColumn column : schemaColumns.keySet()) { - try { - Field f = ColorSchemaColumn.class.getField(column.name()); - if (column.getDepractedColumnName() != null || f.isAnnotationPresent(Deprecated.class)) { - result.add(column); + Map schemaColumns = new HashMap<>(); + parseColumns(columns, schemaColumns, ColorSchemaType.GENERIC); + for (ColorSchemaColumn column : schemaColumns.keySet()) { + try { + Field f = ColorSchemaColumn.class.getField(column.name()); + if (column.getDepractedColumnName() != null || f.isAnnotationPresent(Deprecated.class)) { + result.add(column); + } + } catch (NoSuchFieldException | SecurityException e) { + throw new InvalidStateException(e); } - } catch (NoSuchFieldException | SecurityException e) { - throw new InvalidStateException(e); } } return result; -- GitLab From f6e7573fbaee9c04322f4853fe9d2ccd080b5740 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 18:45:21 +0200 Subject: [PATCH 07/20] single column data doesn't use columns --- .../services/utils/ColorSchemaReader.java | 20 ++++++++++--------- .../services/utils/ColorSchemaReaderTest.java | 12 +++++++++-- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java b/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java index 7eb4e7430..2f5ba0faa 100644 --- a/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java +++ b/service/src/main/java/lcsb/mapviewer/services/utils/ColorSchemaReader.java @@ -819,17 +819,19 @@ public class ColorSchemaReader { line = br.readLine(); } String[] columns = line.split("\t"); + if (columns.length > 1) { - Map schemaColumns = new HashMap<>(); - parseColumns(columns, schemaColumns, ColorSchemaType.GENERIC); - for (ColorSchemaColumn column : schemaColumns.keySet()) { - try { - Field f = ColorSchemaColumn.class.getField(column.name()); - if (column.getDepractedColumnName() != null || f.isAnnotationPresent(Deprecated.class)) { - result.add(column); + Map schemaColumns = new HashMap<>(); + parseColumns(columns, schemaColumns, ColorSchemaType.GENERIC); + for (ColorSchemaColumn column : schemaColumns.keySet()) { + try { + Field f = ColorSchemaColumn.class.getField(column.name()); + if (column.getDepractedColumnName() != null || f.isAnnotationPresent(Deprecated.class)) { + result.add(column); + } + } catch (NoSuchFieldException | SecurityException e) { + throw new InvalidStateException(e); } - } catch (NoSuchFieldException | SecurityException e) { - throw new InvalidStateException(e); } } } diff --git a/service/src/test/java/lcsb/mapviewer/services/utils/ColorSchemaReaderTest.java b/service/src/test/java/lcsb/mapviewer/services/utils/ColorSchemaReaderTest.java index 4592e44bb..f58cdd492 100644 --- a/service/src/test/java/lcsb/mapviewer/services/utils/ColorSchemaReaderTest.java +++ b/service/src/test/java/lcsb/mapviewer/services/utils/ColorSchemaReaderTest.java @@ -2,11 +2,10 @@ package lcsb.mapviewer.services.utils; import static org.junit.Assert.*; -import java.awt.*; +import java.awt.Color; import java.io.*; import java.nio.charset.StandardCharsets; import java.util.*; -import java.util.List; import org.apache.commons.io.output.ByteArrayOutputStream; import org.apache.logging.log4j.LogManager; @@ -16,6 +15,7 @@ import org.junit.*; import lcsb.mapviewer.commands.ColorExtractor; import lcsb.mapviewer.commands.ColorModelCommand; import lcsb.mapviewer.common.TextFileUtils; +import lcsb.mapviewer.model.cache.UploadedFileEntry; import lcsb.mapviewer.model.map.MiriamData; import lcsb.mapviewer.model.map.layout.*; import lcsb.mapviewer.model.map.model.Model; @@ -326,4 +326,12 @@ public class ColorSchemaReaderTest extends ServiceTestFunctions { assertNotNull(schemas.iterator().next().getModelName()); } + @Test + public void testGetDeprecatedColumns() throws Exception { + Layout overlay = new Layout(); + UploadedFileEntry file = new UploadedFileEntry(); + file.setFileContent("blabla\nbasd\n".getBytes("UTF-8")); + overlay.setInputData(file); + assertEquals(0, reader.getDeprecatedColumns(overlay).size()); + } } -- GitLab From 84794ae19d86546cf51bd834ff79667fe1a25f54 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 18:45:40 +0200 Subject: [PATCH 08/20] show warning about deprecated columns in frontend --- .../src/main/js/gui/leftPanel/OverlayPanel.js | 13 +++++++++++- .../src/main/js/map/data/DataOverlay.js | 21 +++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/frontend-js/src/main/js/gui/leftPanel/OverlayPanel.js b/frontend-js/src/main/js/gui/leftPanel/OverlayPanel.js index e26912cc9..1c6454fa0 100644 --- a/frontend-js/src/main/js/gui/leftPanel/OverlayPanel.js +++ b/frontend-js/src/main/js/gui/leftPanel/OverlayPanel.js @@ -318,7 +318,17 @@ OverlayPanel.prototype.createOverlayRow = function (overlay, checked, disabled) OverlayPanel.prototype.overlayToDataRow = function (overlay, checked, disabled) { var result = []; result[0] = overlay.getOrder(); - result[1] = overlay.getName(); + if (overlay.getDeprecatedColumns() !== undefined && overlay.getDeprecatedColumns() !== null && overlay.getDeprecatedColumns().length > 0) { + + result[1] = "
" + + "" + overlay.getName() + "
" + } else { + result[1] = overlay.getName(); + } if (overlay.getInputDataAvailable()) { if (disabled) { @@ -351,6 +361,7 @@ OverlayPanel.prototype.overlayToDataRow = function (overlay, checked, disabled) ""; } } + logger.debug(result); return result; }; diff --git a/frontend-js/src/main/js/map/data/DataOverlay.js b/frontend-js/src/main/js/map/data/DataOverlay.js index 16692fff0..8e5a78f53 100644 --- a/frontend-js/src/main/js/map/data/DataOverlay.js +++ b/frontend-js/src/main/js/map/data/DataOverlay.js @@ -26,6 +26,7 @@ function DataOverlay(overlayId, name) { this.setImagesDirectory(object.images); this.setDescription(object.description); this.setCreator(object.creator); + this.setDeprecatedColumns(object.deprecatedColumns); this.setContent(object.content); this.setFilename(object.filename); this.setPublicOverlay(object.publicOverlay); @@ -328,6 +329,22 @@ DataOverlay.prototype.setContent = function (content) { this._content = content; }; +/** + * + * @returns {string[]} + */ +DataOverlay.prototype.getDeprecatedColumns = function () { + return this._deprecatedColumns; +}; + +/** + * + * @param {string[]} deprecatedColumns + */ +DataOverlay.prototype.setDeprecatedColumns = function (deprecatedColumns) { + this._deprecatedColumns = deprecatedColumns; +}; + /** * * @returns {number} @@ -380,7 +397,7 @@ DataOverlay.prototype.setType = function (type) { * * @param {boolean} value */ -DataOverlay.prototype.setGoogleLicenseConsent = function(value) { +DataOverlay.prototype.setGoogleLicenseConsent = function (value) { this._googleLicenseConsent = value; }; @@ -388,7 +405,7 @@ DataOverlay.prototype.setGoogleLicenseConsent = function(value) { * * @returns {boolean} */ -DataOverlay.prototype.isGoogleLicenseConsent = function() { +DataOverlay.prototype.isGoogleLicenseConsent = function () { return this._googleLicenseConsent; }; -- GitLab From 987c5c83cab4b1c59e8592b27d25a75c42a40e49 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 18:47:31 +0200 Subject: [PATCH 09/20] changelog updated --- CHANGELOG | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 1d02c5026..6bfcc91cb 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -19,6 +19,9 @@ minerva (14.0.0~alpha.0) unstable; urgency=low about type of database that identifies the target (#66) * Small improvement: redundant 'references' field in gene variants data overlay is now deprecated (#850) + * Small improvement: information about deprecated columns in data overlay is + visible in overlay list (#838) + * Small improvement: publication list is resizable (#740) * Bug fix: export to CellDesigner of reaction with two modifiers connected with boolean operator resulted was skipping some layout information * Bug fix: reaction in SBGNML file containing two products was improperly -- GitLab From 486b27d9bb4620074018acc1c58872a699ff17d1 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 19:08:50 +0200 Subject: [PATCH 10/20] we need to compare to 'true' because we might get null on the left side which cannot be resolved to boolean --- .../api/projects/overlays/OverlayController.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayController.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayController.java index fd8a5c604..f48006cbc 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayController.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayController.java @@ -60,7 +60,7 @@ public class OverlayController extends BaseController { @PreAuthorize("hasAuthority('IS_ADMIN')" + " or hasAuthority('IS_CURATOR') and hasAuthority('READ_PROJECT:' + #projectId)" + " or hasAuthority('READ_PROJECT:' + #projectId) and " + - " (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout)") + " (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)") @GetMapping(value = "/{overlayId}/models/{modelId}/bioEntities/") public List> getOverlayElements( @PathVariable(value = "projectId") String projectId, @@ -72,7 +72,7 @@ public class OverlayController extends BaseController { @PreAuthorize("hasAuthority('IS_ADMIN')" + " or hasAuthority('IS_CURATOR') and hasAuthority('READ_PROJECT:' + #projectId)" + " or hasAuthority('READ_PROJECT:' + #projectId) and " + - " (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout)") + " (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)") @GetMapping(value = "/{overlayId}/models/{modelId}/bioEntities/reactions/{reactionId}/") public Map getFullReaction( @PathVariable(value = "projectId") String projectId, @@ -88,7 +88,7 @@ public class OverlayController extends BaseController { @PreAuthorize("hasAuthority('IS_ADMIN')" + " or hasAuthority('IS_CURATOR') and hasAuthority('READ_PROJECT:' + #projectId)" + " or hasAuthority('READ_PROJECT:' + #projectId) and " + - " (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout)") + " (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)") @GetMapping(value = "/{overlayId}/models/{modelId}/bioEntities/elements/{elementId}/") public Map getFullSpecies( @PathVariable(value = "projectId") String projectId, @@ -147,7 +147,7 @@ public class OverlayController extends BaseController { @PreAuthorize("hasAuthority('IS_ADMIN')" + " or hasAuthority('IS_CURATOR') and hasAuthority('READ_PROJECT:' + #projectId)" + " or hasAuthority('READ_PROJECT:' + #projectId) and " + - " (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout)") + " (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)") @GetMapping(value = "/{overlayId}:downloadSource") public ResponseEntity getOverlaySource( @PathVariable(value = "projectId") String projectId, -- GitLab From 1ea69afe2a1920dcdd1947e7f0dd212c72293e56 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 19:12:06 +0200 Subject: [PATCH 11/20] the user might be null --- .../main/java/lcsb/mapviewer/services/impl/FileService.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java b/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java index 97ff42bdc..8690ac68f 100644 --- a/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java +++ b/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java @@ -28,8 +28,9 @@ public class FileService implements IFileService { @Override public User getOwnerByFileId(int id) { UploadedFileEntry entry = uploadedFileEntryDao.getById(id); - if (entry != null) { - uploadedFileEntryDao.getById(id).getOwner().getLogin(); + if (entry != null && entry.getOwner() != null) { + //it's lazy initialized + entry.getOwner().getLogin(); return uploadedFileEntryDao.getById(id).getOwner(); } else { return null; -- GitLab From 9f2292ea919c6e110f40a52c0b21b31e2ee3c264 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 19:20:43 +0200 Subject: [PATCH 12/20] when type of the argument is wrong return 400 --- rest-api/src/main/java/lcsb/mapviewer/api/BaseController.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/BaseController.java b/rest-api/src/main/java/lcsb/mapviewer/api/BaseController.java index 2698fdcf0..b286c43cd 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/BaseController.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/BaseController.java @@ -14,6 +14,7 @@ import org.springframework.web.HttpMediaTypeNotSupportedException; import org.springframework.web.bind.MissingServletRequestParameterException; import org.springframework.web.bind.ServletRequestBindingException; import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; @@ -41,7 +42,8 @@ public abstract class BaseController { } else if (e instanceof QueryException || e instanceof HttpMessageNotReadableException || e instanceof MissingServletRequestParameterException - || e instanceof HttpMediaTypeNotSupportedException) { + || e instanceof HttpMediaTypeNotSupportedException + || e instanceof MethodArgumentTypeMismatchException) { logger.error(e, e); return createErrorResponse("Query server error.", e.getMessage(), new HttpHeaders(), HttpStatus.BAD_REQUEST); } else if (e instanceof ServletRequestBindingException && e.getMessage().contains(Configuration.AUTH_TOKEN)) { -- GitLab From a22bbce1f8af1748108bc9706e17e7c0b6e43b04 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 19:23:17 +0200 Subject: [PATCH 13/20] we should properly handle nulls --- .../mapviewer/api/files/FileController.java | 2 +- .../mapviewer/api/files/FileRestImpl.java | 2 +- .../mapviewer/services/impl/FileService.java | 20 ++++++++++--------- .../services/interfaces/IFileService.java | 4 ++-- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java b/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java index 4b4a99b8a..506434442 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java @@ -42,7 +42,7 @@ public class FileController extends BaseController { @PreAuthorize("@fileService.getOwnerByFileId(#id)?.login == authentication.name") @PostMapping(value = "/{id}:uploadContent") - public Map uploadContent(@PathVariable(value = "id") String id, @RequestBody byte[] data) + public Map uploadContent(@PathVariable(value = "id") Integer id, @RequestBody byte[] data) throws ObjectNotFoundException { return fileRest.uploadContent(id, data); } diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/files/FileRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/files/FileRestImpl.java index 636cf3c00..fe478a55e 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/files/FileRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/files/FileRestImpl.java @@ -60,7 +60,7 @@ public class FileRestImpl extends BaseRestImpl { return result; } - public Map uploadContent(String id, byte[] data) throws ObjectNotFoundException { + public Map uploadContent(Integer id, byte[] data) throws ObjectNotFoundException { int fileId = Integer.valueOf(id); UploadedFileEntry fileEntry = uploadedFileEntryDao.getById(fileId); if (fileEntry == null) { diff --git a/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java b/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java index 8690ac68f..0b94799a4 100644 --- a/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java +++ b/service/src/main/java/lcsb/mapviewer/services/impl/FileService.java @@ -21,20 +21,22 @@ public class FileService implements IFileService { } @Override - public UploadedFileEntry getById(int id) { + public UploadedFileEntry getById(Integer id) { return uploadedFileEntryDao.getById(id); } @Override - public User getOwnerByFileId(int id) { - UploadedFileEntry entry = uploadedFileEntryDao.getById(id); - if (entry != null && entry.getOwner() != null) { - //it's lazy initialized - entry.getOwner().getLogin(); - return uploadedFileEntryDao.getById(id).getOwner(); - } else { - return null; + public User getOwnerByFileId(Integer id) { + if (id != null) { + UploadedFileEntry entry = uploadedFileEntryDao.getById(id); + if (entry != null && entry.getOwner() != null) { + // it's lazy initialized + entry.getOwner().getLogin(); + return uploadedFileEntryDao.getById(id).getOwner(); + } } + return null; + } } diff --git a/service/src/main/java/lcsb/mapviewer/services/interfaces/IFileService.java b/service/src/main/java/lcsb/mapviewer/services/interfaces/IFileService.java index 4ea629807..5598fecf5 100644 --- a/service/src/main/java/lcsb/mapviewer/services/interfaces/IFileService.java +++ b/service/src/main/java/lcsb/mapviewer/services/interfaces/IFileService.java @@ -5,8 +5,8 @@ import lcsb.mapviewer.model.user.User; public interface IFileService { - UploadedFileEntry getById(int id); + UploadedFileEntry getById(Integer id); - User getOwnerByFileId(int id); + User getOwnerByFileId(Integer id); } -- GitLab From d9130144e523df96d479eec7e5fcfb59863eaba6 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 19:28:04 +0200 Subject: [PATCH 14/20] when fetching with null id return null --- .../src/main/java/lcsb/mapviewer/persist/dao/BaseDao.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/persist/src/main/java/lcsb/mapviewer/persist/dao/BaseDao.java b/persist/src/main/java/lcsb/mapviewer/persist/dao/BaseDao.java index 5cb391fb1..efb3df7af 100644 --- a/persist/src/main/java/lcsb/mapviewer/persist/dao/BaseDao.java +++ b/persist/src/main/java/lcsb/mapviewer/persist/dao/BaseDao.java @@ -254,7 +254,10 @@ public abstract class BaseDao { * @return object width identifier given as parameter */ @SuppressWarnings("unchecked") - public T getById(int id) { + public T getById(Integer id) { + if (id == null) { + return null; + } List list = getSession() .createQuery(" from " + this.clazz.getSimpleName() + " where id=:id " + removableAndStatemant()) .setParameter("id", id).list(); -- GitLab From ac7c1d8a22c96e889756b92a27108a7b5620b1dd Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 19:33:16 +0200 Subject: [PATCH 15/20] check if type is proper reference genome type --- .../mapviewer/api/genomics/ReferenceGenomeRestImpl.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/genomics/ReferenceGenomeRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/genomics/ReferenceGenomeRestImpl.java index d6271f6d2..b260d3c77 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/genomics/ReferenceGenomeRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/genomics/ReferenceGenomeRestImpl.java @@ -168,7 +168,12 @@ public class ReferenceGenomeRestImpl extends BaseRestImpl { public List> getReferenceGenomeVersions(String organismId, String type) throws QueryException { - ReferenceGenomeType genomeType = ReferenceGenomeType.valueOf(type); + ReferenceGenomeType genomeType = null; + try { + genomeType = ReferenceGenomeType.valueOf(type); + } catch (IllegalArgumentException e) { + throw new QueryException("Invalid type: " + type); + } MiriamData organism; if (organismId != null && !organismId.isEmpty()) { organism = new MiriamData(MiriamType.TAXONOMY, organismId); -- GitLab From 318148e97a99ae6487028518992b1aa443723309 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 19:33:48 +0200 Subject: [PATCH 16/20] file ids are integers --- .../lcsb/mapviewer/api/files/FileController.java | 2 +- .../java/lcsb/mapviewer/api/files/FileRestImpl.java | 13 ++++++++----- .../lcsb/mapviewer/api/files/FileRestImplTest.java | 6 +++--- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java b/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java index 506434442..1f9ec07f9 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/files/FileController.java @@ -36,7 +36,7 @@ public class FileController extends BaseController { @PostAuthorize("hasAuthority('IS_ADMIN') or returnObject['owner'] == authentication.name") @GetMapping(value = "/{id}") - public Map getFile(@PathVariable(value = "id") String id) throws ObjectNotFoundException { + public Map getFile(@PathVariable(value = "id") Integer id) throws ObjectNotFoundException { return fileRest.getFile(id); } diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/files/FileRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/files/FileRestImpl.java index fe478a55e..4a3073d7e 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/files/FileRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/files/FileRestImpl.java @@ -15,6 +15,7 @@ import lcsb.mapviewer.common.exception.InvalidStateException; import lcsb.mapviewer.model.cache.UploadedFileEntry; import lcsb.mapviewer.model.user.User; import lcsb.mapviewer.persist.dao.cache.UploadedFileEntryDao; +import lcsb.mapviewer.services.interfaces.IFileService; @Transactional @Service @@ -22,9 +23,12 @@ public class FileRestImpl extends BaseRestImpl { private UploadedFileEntryDao uploadedFileEntryDao; + private IFileService fileService; + @Autowired - public FileRestImpl(UploadedFileEntryDao uploadedFileEntryDao) { + public FileRestImpl(UploadedFileEntryDao uploadedFileEntryDao, IFileService fileService) { this.uploadedFileEntryDao = uploadedFileEntryDao; + this.fileService = fileService; } public Map createFile(String filename, String length, User user) { @@ -35,15 +39,14 @@ public class FileRestImpl extends BaseRestImpl { entry.setOwner(user); uploadedFileEntryDao.add(entry); try { - return getFile(entry.getId() + ""); + return getFile(entry.getId()); } catch (ObjectNotFoundException e) { throw new InvalidStateException(e); } } - public Map getFile(String id) throws ObjectNotFoundException { - int fileId = Integer.valueOf(id); - UploadedFileEntry fileEntry = uploadedFileEntryDao.getById(fileId); + public Map getFile(Integer id) throws ObjectNotFoundException { + UploadedFileEntry fileEntry = fileService.getById(id); if (fileEntry == null) { throw new ObjectNotFoundException("Object not found"); } diff --git a/rest-api/src/test/java/lcsb/mapviewer/api/files/FileRestImplTest.java b/rest-api/src/test/java/lcsb/mapviewer/api/files/FileRestImplTest.java index 413ae7ead..1f2d50ba0 100644 --- a/rest-api/src/test/java/lcsb/mapviewer/api/files/FileRestImplTest.java +++ b/rest-api/src/test/java/lcsb/mapviewer/api/files/FileRestImplTest.java @@ -46,14 +46,14 @@ public class FileRestImplTest extends RestTestFunctions { byte[] dataChunkMerged = ArrayUtils.addAll(dataChunk, dataChunk2); Map result = fileRestImpl.createFile("test.txt", "100", user); int id = (Integer) result.get("id"); - fileRestImpl.uploadContent(id + "", dataChunk); + fileRestImpl.uploadContent(id, dataChunk); UploadedFileEntry file = uploadedFileEntryDao.getById(id); assertEquals(100, file.getLength()); assertEquals(2, file.getFileContent().length); assertArrayEquals(dataChunk, file.getFileContent()); - fileRestImpl.uploadContent(id + "", dataChunk2); + fileRestImpl.uploadContent(id, dataChunk2); assertEquals(100, file.getLength()); assertEquals(4, file.getFileContent().length); @@ -70,7 +70,7 @@ public class FileRestImplTest extends RestTestFunctions { int id = (Integer) result.get("id"); try { - fileRestImpl.uploadContent(id + "", dataChunk); + fileRestImpl.uploadContent(id, dataChunk); } finally { uploadedFileEntryDao.getById(id); UploadedFileEntry file = uploadedFileEntryDao.getById(id); -- GitLab From a14d8c8f69dc6ffe5a88752959a10a6627c8e2ff Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 19:38:47 +0200 Subject: [PATCH 17/20] this method doesn't improved super method --- .../main/java/lcsb/mapviewer/persist/dao/map/ModelDao.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/persist/src/main/java/lcsb/mapviewer/persist/dao/map/ModelDao.java b/persist/src/main/java/lcsb/mapviewer/persist/dao/map/ModelDao.java index 28b9f7f15..395398d9c 100644 --- a/persist/src/main/java/lcsb/mapviewer/persist/dao/map/ModelDao.java +++ b/persist/src/main/java/lcsb/mapviewer/persist/dao/map/ModelDao.java @@ -40,12 +40,6 @@ public class ModelDao extends BaseDao { super.delete(model); } - @Override - public ModelData getById(int id) { - ModelData result = super.getById(id); - return result; - } - /** * Return the latest model for the project with a given project identifier. * -- GitLab From 97cccedb73ea50566cd63e97366108cb5c3f100a Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 19:45:45 +0200 Subject: [PATCH 18/20] modelId should be numeric --- .../main/java/lcsb/mapviewer/api/BaseRestImpl.java | 10 +++++++--- .../mapviewer/api/projects/ProjectController.java | 2 +- .../mapviewer/api/projects/ProjectRestImpl.java | 4 ++-- .../api/projects/models/ModelController.java | 5 +++-- .../api/projects/models/ModelRestImpl.java | 13 ++++++++----- .../models/parameters/ParametersRestImpl.java | 4 ++-- .../api/projects/models/units/UnitsRestImpl.java | 2 +- 7 files changed, 24 insertions(+), 16 deletions(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/BaseRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/BaseRestImpl.java index b7be8ff5a..bc9c4366a 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/BaseRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/BaseRestImpl.java @@ -8,6 +8,7 @@ import javax.xml.transform.*; import javax.xml.transform.stream.StreamResult; import javax.xml.transform.stream.StreamSource; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.math.NumberUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -160,10 +161,9 @@ public abstract class BaseRestImpl { * @param modelId * list of model identifiers separated by "," or '*' when all models * should be returned - * @throws ObjectNotFoundException - * thrown when data for given identifiers doesn't exist + * @throws QueryException */ - protected List getModels(String projectId, String modelId) throws ObjectNotFoundException { + protected List getModels(String projectId, String modelId) throws QueryException { Model model = modelService.getLastModelByProjectId(projectId); if (model == null) { throw new ObjectNotFoundException("Project with given id doesn't exist"); @@ -172,6 +172,10 @@ public abstract class BaseRestImpl { if (!modelId.equals("*")) { for (String str : modelId.split(",")) { + if (!StringUtils.isNumeric(str)) { + throw new QueryException("Invalid modelId: " + modelId); + } + Model submodel = model.getSubmodelById(Integer.valueOf(str)); if (submodel == null) { throw new ObjectNotFoundException("Model with given id doesn't exist"); diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/ProjectController.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/ProjectController.java index 02801a3c1..081f60fa2 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/ProjectController.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/ProjectController.java @@ -114,7 +114,7 @@ public class ProjectController extends BaseController { @PreAuthorize("hasAnyAuthority('IS_ADMIN', 'READ_PROJECT:' + #projectId)") @GetMapping(value = "/{projectId}/statistics") - public Object getStatistics(@PathVariable(value = "projectId") String projectId) throws ObjectNotFoundException { + public Object getStatistics(@PathVariable(value = "projectId") String projectId) throws QueryException { return projectController.getStatistics(projectId); } 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 00139e1e0..63814e80e 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 @@ -195,7 +195,7 @@ public class ProjectRestImpl extends BaseRestImpl { return project.getInputData(); } - public Map getStatistics(String projectId) throws ObjectNotFoundException { + public Map getStatistics(String projectId) throws QueryException { Map result = new TreeMap<>(); Map elementAnnotations = new TreeMap<>(); @@ -624,7 +624,7 @@ public class ProjectRestImpl extends BaseRestImpl { return null; } - public List> getSubmapConnections(String projectId) throws ObjectNotFoundException { + public List> getSubmapConnections(String projectId) throws QueryException { List> result = new ArrayList<>(); List models = getModels(projectId, "*"); List elements = new ArrayList<>(); diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/ModelController.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/ModelController.java index dabceafea..de89361e9 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/ModelController.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/ModelController.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.util.List; import java.util.Map; +import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; @@ -37,7 +38,7 @@ public class ModelController extends BaseController { @PreAuthorize("hasAnyAuthority('IS_ADMIN', 'READ_PROJECT:' + #projectId)") @GetMapping(value = "/") public List> getModels(@PathVariable(value = "projectId") String projectId) - throws ObjectNotFoundException { + throws QueryException { return modelController.getModels(projectId); } @@ -45,7 +46,7 @@ public class ModelController extends BaseController { @GetMapping(value = "/{modelId:.+}") public Object getModel( @PathVariable(value = "modelId") String modelId, - @PathVariable(value = "projectId") String projectId) throws ObjectNotFoundException { + @PathVariable(value = "projectId") String projectId) throws QueryException { if (modelId.equals("*")) { return modelController.getModels(projectId); } else { diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/ModelRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/ModelRestImpl.java index 15c1affd9..da7ac0174 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/ModelRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/ModelRestImpl.java @@ -1,12 +1,12 @@ package lcsb.mapviewer.api.projects.models; -import java.awt.*; +import java.awt.Color; import java.awt.geom.*; import java.io.*; import java.util.*; -import java.util.List; import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.beans.factory.annotation.Autowired; @@ -59,7 +59,7 @@ public class ModelRestImpl extends BaseRestImpl { this.layoutService = layoutService; } - public List> getModels(String projectId) throws ObjectNotFoundException { + public List> getModels(String projectId) throws QueryException { Project project = getProjectService().getProjectByProjectId(projectId); if (project == null) { throw new ObjectNotFoundException("Project with given id doesn't exist"); @@ -67,7 +67,10 @@ public class ModelRestImpl extends BaseRestImpl { return createData(project); } - public Map getModel(String projectId, String modelId) { + public Map getModel(String projectId, String modelId) throws QueryException { + if (!StringUtils.isNumeric(modelId)) { + throw new QueryException("Invalid modelId: " + modelId); + } Model model = getModelService().getLastModelByProjectId(projectId); Model submodel = model.getSubmodelById(modelId); if (submodel == null) { @@ -95,7 +98,7 @@ public class ModelRestImpl extends BaseRestImpl { } } - private List> createData(Project project) { + private List> createData(Project project) throws QueryException { List> result = new ArrayList<>(); Model model = getModelService().getLastModelByProjectId(project.getProjectId()); if (model != null) { diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/parameters/ParametersRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/parameters/ParametersRestImpl.java index 62ef329cc..3e2c3fc1b 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/parameters/ParametersRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/parameters/ParametersRestImpl.java @@ -36,7 +36,7 @@ public class ParametersRestImpl extends BaseRestImpl { } private Set getParametersFromProject(String projectId, String modelId) - throws ObjectNotFoundException { + throws QueryException { List models = getModels(projectId, modelId); Set parameters = new LinkedHashSet<>(); @@ -52,7 +52,7 @@ public class ParametersRestImpl extends BaseRestImpl { } private Set getGlobalParametersFromProject(String projectId, String modelId) - throws ObjectNotFoundException { + throws QueryException { List models = getModels(projectId, modelId); Set parameters = new LinkedHashSet<>(); diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/units/UnitsRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/units/UnitsRestImpl.java index b276f8795..692ba9791 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/units/UnitsRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/models/units/UnitsRestImpl.java @@ -59,7 +59,7 @@ public class UnitsRestImpl extends BaseRestImpl { return result; } - public List> getUnits(String projectId, String modelId) throws ObjectNotFoundException { + public List> getUnits(String projectId, String modelId) throws QueryException { List> result = new ArrayList<>(); List models = getModels(projectId, modelId); for (Model model : models) { -- GitLab From 5e1475d2cfa0327ad9b8eb748991aae221bd4daa Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 20:03:47 +0200 Subject: [PATCH 19/20] overlayId is numeric --- .../projects/overlays/OverlayController.java | 14 ++++++------- .../projects/overlays/OverlayRestImpl.java | 21 ++++++++----------- .../overlays/OverlayRestImplTest.java | 4 ++-- .../services/impl/LayoutService.java | 2 +- .../services/interfaces/ILayoutService.java | 2 +- 5 files changed, 20 insertions(+), 23 deletions(-) diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayController.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayController.java index f48006cbc..3424a2acf 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayController.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayController.java @@ -53,7 +53,7 @@ public class OverlayController extends BaseController { @GetMapping(value = "/{overlayId}/") public Map getOverlayById( @PathVariable(value = "projectId") String projectId, - @PathVariable(value = "overlayId") String overlayId) throws QueryException { + @PathVariable(value = "overlayId") Integer overlayId) throws QueryException { return overlayRestImp.getOverlayById(projectId, overlayId); } @@ -64,7 +64,7 @@ public class OverlayController extends BaseController { @GetMapping(value = "/{overlayId}/models/{modelId}/bioEntities/") public List> getOverlayElements( @PathVariable(value = "projectId") String projectId, - @PathVariable(value = "overlayId") String overlayId, + @PathVariable(value = "overlayId") Integer overlayId, @RequestParam(value = "columns", defaultValue = "") String columns) throws QueryException { return overlayRestImp.getOverlayElements(projectId, Integer.valueOf(overlayId), columns); } @@ -77,7 +77,7 @@ public class OverlayController extends BaseController { public Map getFullReaction( @PathVariable(value = "projectId") String projectId, @PathVariable(value = "modelId") String modelId, - @PathVariable(value = "overlayId") String overlayId, + @PathVariable(value = "overlayId") Integer overlayId, @PathVariable(value = "reactionId") String reactionId, @RequestParam(value = "columns", defaultValue = "") String columns) throws QueryException, NumberFormatException, ObjectNotFoundException { @@ -93,7 +93,7 @@ public class OverlayController extends BaseController { public Map getFullSpecies( @PathVariable(value = "projectId") String projectId, @PathVariable(value = "modelId") String modelId, - @PathVariable(value = "overlayId") String overlayId, + @PathVariable(value = "overlayId") Integer overlayId, @PathVariable(value = "elementId") String reactionId, @RequestParam(value = "columns", defaultValue = "") String columns) throws QueryException, NumberFormatException, ObjectNotFoundException { @@ -126,7 +126,7 @@ public class OverlayController extends BaseController { @DeleteMapping(value = "/{overlayId}") public Map removeOverlay( @PathVariable(value = "projectId") String projectId, - @PathVariable(value = "overlayId") String overlayId) throws QueryException, IOException { + @PathVariable(value = "overlayId") Integer overlayId) throws QueryException, IOException { return overlayRestImp.removeOverlay(projectId, overlayId); } @@ -136,7 +136,7 @@ public class OverlayController extends BaseController { @PatchMapping(value = "/{overlayId}") public Map updateOverlay( @RequestBody String body, - @PathVariable(value = "overlayId") String overlayId, + @PathVariable(value = "overlayId") Integer overlayId, @PathVariable(value = "projectId") String projectId) throws QueryException, IOException { Map node = parseBody(body); @@ -151,7 +151,7 @@ public class OverlayController extends BaseController { @GetMapping(value = "/{overlayId}:downloadSource") public ResponseEntity getOverlaySource( @PathVariable(value = "projectId") String projectId, - @PathVariable(value = "overlayId") String overlayId) + @PathVariable(value = "overlayId") Integer overlayId) throws QueryException { FileEntry file = overlayRestImp.getOverlaySource(projectId, overlayId); diff --git a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java index 30d861064..ae80ae0db 100644 --- a/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java +++ b/rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImpl.java @@ -147,28 +147,27 @@ public class OverlayRestImpl extends BaseRestImpl { return result; } - public Map getOverlayById(String projectId, String overlayId) + public Map getOverlayById(String projectId, Integer overlayId) throws QueryException { Model model = getModelService().getLastModelByProjectId(projectId); if (model == null) { throw new QueryException("Project with given id doesn't exist"); } - Layout overlay = layoutService.getLayoutById(Integer.valueOf(overlayId)); + Layout overlay = layoutService.getLayoutById(overlayId); if (overlay == null) { throw new QueryException("Overlay with given id doesn't exist"); } return overlayToMap(overlay); } - public FileEntry getOverlaySource(String projectId, String overlayId) + public FileEntry getOverlaySource(String projectId, Integer overlayId) throws QueryException { Model model = getModelService().getLastModelByProjectId(projectId); if (model == null) { throw new QueryException("Project with given id doesn't exist"); } try { - int id = Integer.parseInt(overlayId); - Layout layout = layoutService.getLayoutById(id); + Layout layout = layoutService.getLayoutById(overlayId); if (layout == null) { throw new QueryException("Invalid overlay id"); } @@ -180,13 +179,12 @@ public class OverlayRestImpl extends BaseRestImpl { } } - public Map updateOverlay(String overlayId, Map overlayData) throws QueryException { + public Map updateOverlay(Integer overlayId, Map overlayData) throws QueryException { if (overlayData == null) { throw new QueryException("overlay field cannot be undefined"); } try { - int id = Integer.parseInt(overlayId); - Layout layout = layoutService.getLayoutById(id); + Layout layout = layoutService.getLayoutById(overlayId); if (layout == null) { throw new ObjectNotFoundException("overlay doesn't exist"); } @@ -232,15 +230,14 @@ public class OverlayRestImpl extends BaseRestImpl { } } - public Map removeOverlay(String projectId, String overlayId) + public Map removeOverlay(String projectId, Integer overlayId) throws QueryException, IOException { Project project = getProjectService().getProjectByProjectId(projectId); if (project == null) { throw new ObjectNotFoundException("Project with given id doesn't exist"); } try { - int id = Integer.parseInt(overlayId); - Layout layout = layoutService.getLayoutById(id); + Layout layout = layoutService.getLayoutById(overlayId); if (layout == null) { throw new ObjectNotFoundException("Overlay doesn't exist"); } @@ -321,7 +318,7 @@ public class OverlayRestImpl extends BaseRestImpl { layout.setOrderIndex(count); layoutService.updateLayout(layout); - return getOverlayById(projectId, layout.getId() + ""); + return getOverlayById(projectId, layout.getId()); } catch (InvalidColorSchemaException e) { throw new QueryException(e.getMessage(), e); } diff --git a/rest-api/src/test/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImplTest.java b/rest-api/src/test/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImplTest.java index 40702e8a5..3c7048d91 100644 --- a/rest-api/src/test/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImplTest.java +++ b/rest-api/src/test/java/lcsb/mapviewer/api/projects/overlays/OverlayRestImplTest.java @@ -66,7 +66,7 @@ public class OverlayRestImplTest extends RestTestFunctions { Map result = overlayRest.addOverlay(projectId, "x", "desc", "s1", null, null, ColorSchemaType.GENERIC.name(), "true", null); - String id = result.get("idObject").toString(); + Integer id = Integer.valueOf(result.get("idObject").toString()); Map data = new HashMap<>(); data.put("name", "xyz"); result = overlayRest.updateOverlay(id, data); @@ -84,7 +84,7 @@ public class OverlayRestImplTest extends RestTestFunctions { Map result = overlayRest.addOverlay(projectId, "x", "desc", "s1", null, null, ColorSchemaType.GENERIC.name(), "true", null); - String id = result.get("idObject").toString(); + Integer id = Integer.valueOf(result.get("idObject").toString()); Map data = new HashMap<>(); data.put("name", ""); result = overlayRest.updateOverlay(id, data); diff --git a/service/src/main/java/lcsb/mapviewer/services/impl/LayoutService.java b/service/src/main/java/lcsb/mapviewer/services/impl/LayoutService.java index 83cc8fe1e..ddd3c9947 100644 --- a/service/src/main/java/lcsb/mapviewer/services/impl/LayoutService.java +++ b/service/src/main/java/lcsb/mapviewer/services/impl/LayoutService.java @@ -469,7 +469,7 @@ public class LayoutService implements ILayoutService { } @Override - public Layout getLayoutById(int overlayId) { + public Layout getLayoutById(Integer overlayId) { return layoutDao.getById(overlayId); } diff --git a/service/src/main/java/lcsb/mapviewer/services/interfaces/ILayoutService.java b/service/src/main/java/lcsb/mapviewer/services/interfaces/ILayoutService.java index 0e97cb538..a0da91ebe 100644 --- a/service/src/main/java/lcsb/mapviewer/services/interfaces/ILayoutService.java +++ b/service/src/main/java/lcsb/mapviewer/services/interfaces/ILayoutService.java @@ -142,7 +142,7 @@ public interface ILayoutService { */ List getLayoutsByProject(Project project); - Layout getLayoutById(int overlayId); + Layout getLayoutById(Integer overlayId); void setLayoutDao(LayoutDao layoutDao); -- GitLab From f73ef15be1e380d14a896326baccb8ca81d745b4 Mon Sep 17 00:00:00 2001 From: Piotr Gawron Date: Thu, 25 Jul 2019 20:15:11 +0200 Subject: [PATCH 20/20] we allow only nonegative ids --- .../api/projects/models/parameters/ParametersRestImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest-api/src/test/java/lcsb/mapviewer/api/projects/models/parameters/ParametersRestImplTest.java b/rest-api/src/test/java/lcsb/mapviewer/api/projects/models/parameters/ParametersRestImplTest.java index 30a62541b..381443df7 100644 --- a/rest-api/src/test/java/lcsb/mapviewer/api/projects/models/parameters/ParametersRestImplTest.java +++ b/rest-api/src/test/java/lcsb/mapviewer/api/projects/models/parameters/ParametersRestImplTest.java @@ -29,7 +29,7 @@ public class ParametersRestImplTest extends RestTestFunctions { @Test(expected = ObjectNotFoundException.class) public void testGetParameterForNonExistingModel() throws Exception { parametersRestImpl.getParameter(configurationService.getConfigurationValue(ConfigurationElementType.DEFAULT_MAP), - "-1", "-1"); + "0", "0"); } @Test(expected = ObjectNotFoundException.class) -- GitLab