From 3f584cb2eee11a8060cc819e07108d388b2884c7 Mon Sep 17 00:00:00 2001
From: pkupczyk <piotr.kupczyk@id.ethz.ch>
Date: Mon, 14 May 2018 20:33:23 +0200
Subject: [PATCH] SSDM-5852 : V3 API: Query - fixes after FJE's code review: -
added missing database label to V3 Query class - added missing check to V3
createQueries method
---
.../html/test/test-create.js | 3 ++-
.../html/test/test-search.js | 6 ++---
.../executor/query/CreateQueryExecutor.java | 4 +++
.../v3/translator/query/QueryTranslator.java | 12 +++++++++
.../cisd/openbis/plugin/query/server/DAO.java | 9 +++++--
.../resources/api/v3/as/dto/query/Query.js | 7 +++++
.../systemtest/asapi/v3/CreateQueryTest.java | 26 +++++++++++++++++++
.../systemtest/asapi/v3/GetQueryTest.java | 1 +
.../generic/asapi/v3/dto/query/Query.java | 16 ++++++++++++
.../generic/sharedapi/v3/dictionary.txt | 2 ++
10 files changed, 80 insertions(+), 6 deletions(-)
diff --git a/js-test/servers/common/core-plugins/tests/1/as/webapps/openbis-v3-api-test/html/test/test-create.js b/js-test/servers/common/core-plugins/tests/1/as/webapps/openbis-v3-api-test/html/test/test-create.js
index 4496270c6fd..bf6c1b11188 100644
--- a/js-test/servers/common/core-plugins/tests/1/as/webapps/openbis-v3-api-test/html/test/test-create.js
+++ b/js-test/servers/common/core-plugins/tests/1/as/webapps/openbis-v3-api-test/html/test/test-create.js
@@ -841,7 +841,8 @@ define(
c.assertNotNull(query.getPermId(), "Perm Id");
c.assertEqual(query.getName(), queryCreation.getName(), "Name");
c.assertEqual(query.getDescription(), queryCreation.getDescription(), "Description");
- c.assertEqual(query.getDatabaseId().getName(), queryCreation.getDatabaseId().getName(), "Database");
+ c.assertEqual(query.getDatabaseId().getName(), queryCreation.getDatabaseId().getName(), "Database id");
+ c.assertEqual(query.getDatabaseLabel(), "openBIS meta data", "Database label");
c.assertEqual(query.getQueryType(), queryCreation.getQueryType(), "Query type");
c.assertEqual(query.getEntityTypeCodePattern(), queryCreation.getEntityTypeCodePattern(), "Entity type code pattern");
c.assertEqual(query.getSql(), queryCreation.getSql(), "Sql");
diff --git a/js-test/servers/common/core-plugins/tests/1/as/webapps/openbis-v3-api-test/html/test/test-search.js b/js-test/servers/common/core-plugins/tests/1/as/webapps/openbis-v3-api-test/html/test/test-search.js
index 4ee2f1f7744..28af639aeb9 100644
--- a/js-test/servers/common/core-plugins/tests/1/as/webapps/openbis-v3-api-test/html/test/test-search.js
+++ b/js-test/servers/common/core-plugins/tests/1/as/webapps/openbis-v3-api-test/html/test/test-search.js
@@ -1897,7 +1897,7 @@ define([ 'jquery', 'underscore', 'openbis', 'test/openbis-execute-operations', '
creation.setName(c.generateId("query"));
creation.setDatabaseId(new c.QueryDatabaseName("openbisDB"));
creation.setQueryType(c.QueryType.GENERIC);
- creation.setSql("some sql");
+ creation.setSql("select * from spaces");
var fSearch = function(facade) {
return facade.createQueries([creation]).then(function(techIds) {
@@ -1922,7 +1922,7 @@ define([ 'jquery', 'underscore', 'openbis', 'test/openbis-execute-operations', '
creation.setName(c.generateId("query"));
creation.setDatabaseId(new c.QueryDatabaseName("openbisDB"));
creation.setQueryType(c.QueryType.GENERIC);
- creation.setSql("some sql");
+ creation.setSql("select * from spaces");
var fSearch = function(facade) {
return facade.createQueries([creation]).then(function(techIds) {
@@ -1948,7 +1948,7 @@ define([ 'jquery', 'underscore', 'openbis', 'test/openbis-execute-operations', '
creation.setDatabaseId(new c.QueryDatabaseName("openbisDB"));
creation.setQueryType(c.QueryType.EXPERIMENT);
creation.setEntityTypeCodePattern(c.generateId("pattern"))
- creation.setSql("some sql");
+ creation.setSql("select * from experiments where perm_id = ${key}");
var fSearch = function(facade) {
return facade.createQueries([creation]).then(function(techIds) {
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/query/CreateQueryExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/query/CreateQueryExecutor.java
index 0c81dfdcd98..10890db55a1 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/query/CreateQueryExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/query/CreateQueryExecutor.java
@@ -41,6 +41,7 @@ import ch.systemsx.cisd.common.exceptions.UserFailureException;
import ch.systemsx.cisd.openbis.generic.server.business.bo.DataAccessExceptionTranslator;
import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory;
import ch.systemsx.cisd.openbis.generic.shared.dto.QueryPE;
+import ch.systemsx.cisd.openbis.plugin.query.server.DAO;
/**
* @author pkupczyk
@@ -114,6 +115,9 @@ public class CreateQueryExecutor extends AbstractCreateEntityExecutor<QueryCreat
{
throw new UserFailureException("Sql cannot be empty.");
}
+
+ DAO.checkQuery(creation.getSql());
+
if (creation.getQueryType() == null)
{
throw new UserFailureException("Query type cannot be null.");
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/query/QueryTranslator.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/query/QueryTranslator.java
index 68170d1d82b..e1276238ac6 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/query/QueryTranslator.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/translator/query/QueryTranslator.java
@@ -30,6 +30,8 @@ import ch.ethz.sis.openbis.generic.asapi.v3.dto.query.id.QueryTechId;
import ch.ethz.sis.openbis.generic.server.asapi.v3.translator.AbstractCachingTranslator;
import ch.ethz.sis.openbis.generic.server.asapi.v3.translator.TranslationContext;
import ch.ethz.sis.openbis.generic.server.asapi.v3.translator.TranslationResults;
+import ch.systemsx.cisd.openbis.plugin.query.shared.DatabaseDefinition;
+import ch.systemsx.cisd.openbis.plugin.query.shared.IQueryDatabaseDefinitionProvider;
/**
* @author pkupczyk
@@ -47,6 +49,9 @@ public class QueryTranslator extends AbstractCachingTranslator<Long, Query, Quer
@Autowired
private IQueryBaseTranslator baseTranslator;
+ @Autowired
+ private IQueryDatabaseDefinitionProvider databaseProvider;
+
@Override
protected Set<Long> shouldTranslate(TranslationContext context, Collection<Long> queryIds, QueryFetchOptions fetchOptions)
{
@@ -86,6 +91,13 @@ public class QueryTranslator extends AbstractCachingTranslator<Long, Query, Quer
result.setName(baseRecord.name);
result.setDescription(baseRecord.description);
result.setDatabaseId(new QueryDatabaseName(baseRecord.database));
+
+ DatabaseDefinition database = databaseProvider.getDefinition(baseRecord.database);
+ if (database != null)
+ {
+ result.setDatabaseLabel(database.getLabel());
+ }
+
result.setQueryType(QueryType.valueOf(baseRecord.queryType));
result.setEntityTypeCodePattern(baseRecord.entityTypeCodePattern);
result.setSql(baseRecord.sql);
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/query/server/DAO.java b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/query/server/DAO.java
index 1147785a4e2..6a9ab0b5cb6 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/plugin/query/server/DAO.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/plugin/query/server/DAO.java
@@ -149,8 +149,7 @@ public class DAO extends SimpleJdbcDaoSupport implements IDAO
afterPropertiesSet();
}
- @Override
- public TableModel query(String sqlQuery, QueryParameterBindings bindingsOrNull)
+ public static void checkQuery(String sqlQuery)
{
if (sqlQuery.toLowerCase().trim().startsWith("select") == false)
{
@@ -162,6 +161,12 @@ public class DAO extends SimpleJdbcDaoSupport implements IDAO
throw new UserFailureException("Sorry, only one query statement is allowed: "
+ "A ';' somewhere in the middle has been found.");
}
+ }
+
+ @Override
+ public TableModel query(String sqlQuery, QueryParameterBindings bindingsOrNull)
+ {
+ checkQuery(sqlQuery);
PreparedStatementCallback callback = new PreparedStatementCallback()
{
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/public/resources/api/v3/as/dto/query/Query.js b/openbis/source/java/ch/systemsx/cisd/openbis/public/resources/api/v3/as/dto/query/Query.js
index 95334af69fc..b1483ca0bec 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/public/resources/api/v3/as/dto/query/Query.js
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/public/resources/api/v3/as/dto/query/Query.js
@@ -13,6 +13,7 @@ define([ "stjs", "util/Exceptions" ], function(stjs, exceptions) {
prototype.name = null;
prototype.description = null;
prototype.databaseId = null;
+ prototype.databaseLabel = null;
prototype.queryType = null;
prototype.entityTypeCodePattern = null;
prototype.sql = null;
@@ -51,6 +52,12 @@ define([ "stjs", "util/Exceptions" ], function(stjs, exceptions) {
prototype.setDatabaseId = function(databaseId) {
this.databaseId = databaseId;
};
+ prototype.getDatabaseLabel = function() {
+ return this.databaseLabel;
+ };
+ prototype.setDatabaseLabel = function(databaseLabel) {
+ this.databaseLabel = databaseLabel;
+ };
prototype.getQueryType = function() {
return this.queryType;
};
diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/CreateQueryTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/CreateQueryTest.java
index fbfc720e19d..1fd83aa361a 100644
--- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/CreateQueryTest.java
+++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/CreateQueryTest.java
@@ -218,4 +218,30 @@ public class CreateQueryTest extends AbstractQueryTest
}, "Sql cannot be empty");
}
+ @Test
+ public void testCreateWithNonSelectSql()
+ {
+ assertUserFailureException(new IDelegatedAction()
+ {
+ @Override
+ public void execute()
+ {
+ QueryCreation creation = testCreation();
+ creation.setSql("update spaces set code = 'YOU_HAVE_BEEN_HACKED' where code = 'CISD'");
+ createQuery(TEST_USER, PASSWORD, creation);
+ }
+ }, "Sorry, only select statements are allowed");
+
+ assertUserFailureException(new IDelegatedAction()
+ {
+ @Override
+ public void execute()
+ {
+ QueryCreation creation = testCreation();
+ creation.setSql("select * from spaces; update spaces set code = 'YOU_HAVE_BEEN_HACKED' where code = 'CISD'");
+ createQuery(TEST_USER, PASSWORD, creation);
+ }
+ }, "Sorry, only one query statement is allowed: A ';' somewhere in the middle has been found.");
+ }
+
}
diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/GetQueryTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/GetQueryTest.java
index bd81b1beebe..afe058139b0 100644
--- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/GetQueryTest.java
+++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/GetQueryTest.java
@@ -201,6 +201,7 @@ public class GetQueryTest extends AbstractQueryTest
assertEquals(query.getName(), creation.getName());
assertEquals(query.getDescription(), creation.getDescription());
assertEquals(query.getDatabaseId(), creation.getDatabaseId());
+ assertEquals(query.getDatabaseLabel(), "openBIS meta data");
assertEquals(query.getEntityTypeCodePattern(), creation.getEntityTypeCodePattern());
assertEquals(query.getQueryType(), creation.getQueryType());
assertEquals(query.isPublic(), creation.isPublic());
diff --git a/openbis_api/source/java/ch/ethz/sis/openbis/generic/asapi/v3/dto/query/Query.java b/openbis_api/source/java/ch/ethz/sis/openbis/generic/asapi/v3/dto/query/Query.java
index 9672df058c8..df2332a1cad 100644
--- a/openbis_api/source/java/ch/ethz/sis/openbis/generic/asapi/v3/dto/query/Query.java
+++ b/openbis_api/source/java/ch/ethz/sis/openbis/generic/asapi/v3/dto/query/Query.java
@@ -55,6 +55,9 @@ public class Query implements Serializable, IDescriptionHolder, IModificationDat
@JsonProperty
private IQueryDatabaseId databaseId;
+ @JsonProperty
+ private String databaseLabel;
+
@JsonProperty
private QueryType queryType;
@@ -142,6 +145,19 @@ public class Query implements Serializable, IDescriptionHolder, IModificationDat
this.databaseId = databaseId;
}
+ // Method automatically generated with DtoGenerator
+ @JsonIgnore
+ public String getDatabaseLabel()
+ {
+ return databaseLabel;
+ }
+
+ // Method automatically generated with DtoGenerator
+ public void setDatabaseLabel(String databaseLabel)
+ {
+ this.databaseLabel = databaseLabel;
+ }
+
// Method automatically generated with DtoGenerator
@JsonIgnore
public QueryType getQueryType()
diff --git a/openbis_api/sourceTest/java/ch/ethz/sis/openbis/generic/sharedapi/v3/dictionary.txt b/openbis_api/sourceTest/java/ch/ethz/sis/openbis/generic/sharedapi/v3/dictionary.txt
index e3f060bc6a0..b8413bd9a42 100644
--- a/openbis_api/sourceTest/java/ch/ethz/sis/openbis/generic/sharedapi/v3/dictionary.txt
+++ b/openbis_api/sourceTest/java/ch/ethz/sis/openbis/generic/sharedapi/v3/dictionary.txt
@@ -2194,6 +2194,8 @@ with Entity Type Code Pattern
with Parameters
with Query Type
with Sql
+get Database Label
+set Database Label
get Data Set Types
Get Data Set Types Operation
--
GitLab