From 30556f35fb23c1f7025d5edc4fc351507573ad7f Mon Sep 17 00:00:00 2001 From: Grigoriy Sterin Date: Tue, 12 May 2020 13:24:55 -0400 Subject: [PATCH] JOOQ DSL example --- build.gradle | 27 +++ .../WorkspaceManagerJdbcConfiguration.java | 8 + .../terra/workspace/db/DataReferenceDao.java | 218 ++++++++---------- .../bio/terra/workspace/db/WorkspaceDao.java | 79 +++---- .../workspace/db/DataReferenceDaoTest.java | 29 ++- 5 files changed, 184 insertions(+), 177 deletions(-) diff --git a/build.gradle b/build.gradle index 755b2e61e5..895c9ecb41 100644 --- a/build.gradle +++ b/build.gradle @@ -12,6 +12,7 @@ plugins { id 'com.google.cloud.tools.jib' version '1.8.0' id 'org.openapi.generator' version '4.2.3' id 'com.diffplug.gradle.spotless' version '3.27.2' + id "nu.studer.jooq" version "4.2" } group = 'bio.terra.workspace' @@ -34,6 +35,7 @@ dependencies { compile(group: 'org.broadinstitute.dsde.workbench', name: 'sam-client_2.12', version: '0.1-11a7002') implementation group: 'org.springframework.boot', name: 'spring-boot-starter-data-jdbc' implementation group: 'org.springframework.boot', name: 'spring-boot-starter-web' + implementation group: 'org.springframework.boot', name: 'spring-boot-starter-jooq' implementation group: 'org.postgresql', name: 'postgresql', version: '42.1.4' implementation group: 'org.apache.commons', name: 'commons-dbcp2', version: '2.7.0' implementation group: 'org.apache.commons', name: 'commons-pool2', version: '2.8.0' @@ -41,6 +43,9 @@ dependencies { implementation group: 'org.liquibase' , name: 'liquibase-core', version: '3.8.6' implementation group: 'org.webjars', name: 'swagger-ui', version: '3.24.0' + compile 'org.jooq:jooq' + jooqRuntime 'org.postgresql:postgresql' + // -- OpenAPI CodeGen dependencies -- // TODO: this version of swagger-annotations is old, but the code gen is still relying on it // There is an open bug tracking the fix: https://github.com/OpenAPITools/openapi-generator/issues/4245 @@ -109,3 +114,25 @@ spotless { } } +jooq { + version '3.13.2' + main(sourceSets.main) { + jdbc { + driver = 'org.postgresql.Driver' + url = 'jdbc:postgresql://localhost:5432/testdb' + user = 'dbuser' + password = 'dbpwd' + } + generator { + name = 'org.jooq.codegen.DefaultGenerator' + database { + name = 'org.jooq.meta.postgres.PostgresDatabase' + inputSchema = 'public' + } + target { + packageName = 'bio.terra.workspace.db.generated' + } + } + } +} + diff --git a/src/main/java/bio/terra/workspace/app/configuration/WorkspaceManagerJdbcConfiguration.java b/src/main/java/bio/terra/workspace/app/configuration/WorkspaceManagerJdbcConfiguration.java index 691356cad5..7d709f204f 100644 --- a/src/main/java/bio/terra/workspace/app/configuration/WorkspaceManagerJdbcConfiguration.java +++ b/src/main/java/bio/terra/workspace/app/configuration/WorkspaceManagerJdbcConfiguration.java @@ -1,5 +1,8 @@ package bio.terra.workspace.app.configuration; +import org.jooq.DSLContext; +import org.jooq.SQLDialect; +import org.jooq.impl.DSL; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.context.annotation.Bean; @@ -44,4 +47,9 @@ public void setUpgradeOnStart(boolean upgradeOnStart) { public PlatformTransactionManager getTransactionManager() { return new DataSourceTransactionManager(getDataSource()); } + + @Bean + public DSLContext dslContext() { + return DSL.using(this.getDataSource(), SQLDialect.POSTGRES); + } } diff --git a/src/main/java/bio/terra/workspace/db/DataReferenceDao.java b/src/main/java/bio/terra/workspace/db/DataReferenceDao.java index 858db35e89..07f0da123f 100644 --- a/src/main/java/bio/terra/workspace/db/DataReferenceDao.java +++ b/src/main/java/bio/terra/workspace/db/DataReferenceDao.java @@ -1,35 +1,32 @@ package bio.terra.workspace.db; -import bio.terra.workspace.app.configuration.WorkspaceManagerJdbcConfiguration; +import static bio.terra.workspace.db.generated.tables.WorkspaceDataReference.*; +import static bio.terra.workspace.db.generated.tables.WorkspaceResource.*; + import bio.terra.workspace.common.exception.DataReferenceNotFoundException; +import bio.terra.workspace.db.generated.tables.WorkspaceDataReference; +import bio.terra.workspace.db.generated.tables.WorkspaceResource; import bio.terra.workspace.generated.model.DataReferenceDescription; import bio.terra.workspace.generated.model.DataReferenceList; import bio.terra.workspace.generated.model.ResourceDescription; -import java.sql.ResultSet; -import java.sql.SQLException; -import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.UUID; -import java.util.stream.Collectors; -import org.apache.commons.lang3.StringUtils; +import org.jooq.*; import org.openapitools.jackson.nullable.JsonNullable; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.dao.EmptyResultDataAccessException; -import org.springframework.jdbc.core.RowMapper; -import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; -import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; import org.springframework.stereotype.Component; @Component public class DataReferenceDao { - private final NamedParameterJdbcTemplate jdbcTemplate; + private final DSLContext dslContext; + + private static final WorkspaceDataReference referenceTable = WORKSPACE_DATA_REFERENCE.as("referenceTable"); + private static final WorkspaceResource resourceTable = WORKSPACE_RESOURCE.as("resourceTable"); @Autowired - public DataReferenceDao(WorkspaceManagerJdbcConfiguration jdbcConfiguration) { - this.jdbcTemplate = new NamedParameterJdbcTemplate(jdbcConfiguration.getDataSource()); + public DataReferenceDao(DSLContext dslContext) { + this.dslContext = dslContext; } public String createDataReference( @@ -41,148 +38,121 @@ public String createDataReference( String cloningInstructions, JsonNullable referenceType, JsonNullable reference) { - String sql = - "INSERT INTO workspace_data_reference (workspace_id, reference_id, name, resource_id, credential_id, cloning_instructions, reference_type, reference) VALUES " - + "(:workspace_id, :reference_id, :name, :resource_id, :credential_id, :cloning_instructions, :reference_type, cast(:reference AS json))"; - - Map paramMap = new HashMap<>(); - paramMap.put("workspace_id", workspaceId.toString()); - paramMap.put("reference_id", referenceId.toString()); - paramMap.put("name", name); - paramMap.put("cloning_instructions", cloningInstructions); - paramMap.put("credential_id", credentialId.orElse(null)); - paramMap.put("resource_id", resourceId.orElse(null)); - paramMap.put("reference_type", referenceType.orElse(null)); - paramMap.put("reference", reference.orElse(null)); - - jdbcTemplate.update(sql, paramMap); + + dslContext + .insertInto(referenceTable) + .columns( + referenceTable.WORKSPACE_ID, + referenceTable.REFERENCE_ID, + referenceTable.NAME, + referenceTable.CLONING_INSTRUCTIONS, + referenceTable.CREDENTIAL_ID, + referenceTable.RESOURCE_ID, + referenceTable.REFERENCE_TYPE, + referenceTable.REFERENCE) + .values( + workspaceId.toString(), + referenceId.toString(), + name, + cloningInstructions, + credentialId.orElse(null), + resourceId.isPresent() ? resourceId.get().toString() : null, + referenceType.orElse(null), + JSON.valueOf(reference.orElse(null))) + .execute(); + return referenceId.toString(); } public DataReferenceDescription getDataReference(UUID referenceId) { - String sql = - "SELECT workspace_id, reference_id, name, resource_id, credential_id, cloning_instructions, reference_type, reference from workspace_data_reference where reference_id = :id"; - - Map paramMap = new HashMap<>(); - paramMap.put("id", referenceId.toString()); - - try { - return jdbcTemplate.queryForObject(sql, paramMap, new DataReferenceMapper()); - } catch (EmptyResultDataAccessException e) { + DataReferenceDescription dataReference = + dslContext + .selectFrom(referenceTable) + .where(referenceTable.REFERENCE_ID.eq(referenceId.toString())) + .fetchOne(new DataReferenceMapper()); + + if (dataReference != null) { + return dataReference; + } else { throw new DataReferenceNotFoundException("Data Reference not found."); } } public boolean isControlled(UUID referenceId) { - String sql = - "SELECT CASE WHEN resource_id IS NULL THEN 'false' ELSE 'true' END FROM workspace_data_reference where reference_id = :id"; - - Map paramMap = new HashMap<>(); - paramMap.put("id", referenceId.toString()); - - try { - return jdbcTemplate.queryForObject(sql, paramMap, Boolean.class).booleanValue(); - } catch (EmptyResultDataAccessException e) { + Record1 rec = + dslContext + .select(referenceTable.RESOURCE_ID) + .from(referenceTable) + .where(referenceTable.REFERENCE_ID.eq(referenceId.toString())) + .fetchOne(); + if (rec != null) { + return rec.get(referenceTable.RESOURCE_ID) != null; + } else { throw new DataReferenceNotFoundException("Data Reference not found."); } } public boolean deleteDataReference(UUID referenceId) { - Map paramMap = new HashMap(); - paramMap.put("id", referenceId.toString()); int rowsAffected = - jdbcTemplate.update( - "DELETE FROM workspace_data_reference WHERE reference_id = :id", paramMap); + dslContext + .delete(referenceTable) + .where(referenceTable.REFERENCE_ID.eq(referenceId.toString())) + .execute(); return rowsAffected > 0; } public DataReferenceList enumerateDataReferences( String workspaceId, String owner, int offset, int limit) { - List whereClauses = new ArrayList<>(); - whereClauses.add("(ref.workspace_id = :id)"); - whereClauses.add(uncontrolledOrVisibleResourcesClause("resource", "ref")); - String filterSql = combineWhereClauses(whereClauses); - String sql = - "SELECT ref.workspace_id, ref.reference_id, ref.name, ref.resource_id, ref.credential_id, ref.cloning_instructions, ref.reference_type, ref.reference," - + " resource.resource_id, resource.associated_app, resource.is_visible, resource.owner, resource.attributes" - + " FROM workspace_data_reference AS ref" - + " LEFT JOIN workspace_resource AS resource ON ref.resource_id = resource.resource_id" - + filterSql - + " ORDER BY ref.reference_id" - + " OFFSET :offset" - + " LIMIT :limit"; - MapSqlParameterSource params = new MapSqlParameterSource(); - params.addValue("id", workspaceId); - params.addValue("owner", owner); - params.addValue("offset", offset); - params.addValue("limit", limit); List resultList = - jdbcTemplate.query(sql, params, new DataReferenceMapper()); + dslContext + .select() + .from(referenceTable) + .leftOuterJoin(resourceTable) + .on(referenceTable.RESOURCE_ID.eq(resourceTable.RESOURCE_ID)) + .where(referenceTable.WORKSPACE_ID.eq(workspaceId)) + .and( + referenceTable + .RESOURCE_ID + .isNull() + .or(resourceTable.IS_VISIBLE.eq(true).or(resourceTable.OWNER.eq(owner)))) + .orderBy(referenceTable.REFERENCE_ID) + .offset(offset) + .limit(limit) + .fetch(new DataReferenceMapper()); return new DataReferenceList().resources(resultList); } - private static class ResourceDescriptionMapper implements RowMapper { - public ResourceDescription mapRow(ResultSet rs, int rowNum) throws SQLException { + private static class ResourceDescriptionMapper implements RecordMapper { + @Override + public ResourceDescription map(Record rec) { return new ResourceDescription() - .workspaceId(UUID.fromString(rs.getString("workspace_id"))) - .resourceId(UUID.fromString(rs.getString("resource_id"))) - .isVisible(rs.getBoolean("is_visible")) - .owner(rs.getString("owner")) - .attributes(rs.getString("attributes")); + .workspaceId(UUID.fromString(rec.get(resourceTable.WORKSPACE_ID))) + .resourceId(UUID.fromString(rec.get(resourceTable.RESOURCE_ID))) + .isVisible(rec.get(resourceTable.IS_VISIBLE)) + .owner(rec.get(resourceTable.OWNER)) + .attributes(rec.get(resourceTable.ATTRIBUTES).toString()); } } - private static class DataReferenceMapper implements RowMapper { - public DataReferenceDescription mapRow(ResultSet rs, int rowNum) throws SQLException { + private static class DataReferenceMapper + implements RecordMapper { + @Override + public DataReferenceDescription map(Record rec) { ResourceDescriptionMapper resourceDescriptionMapper = new ResourceDescriptionMapper(); + String resourceId = rec.get(referenceTable.RESOURCE_ID); return new DataReferenceDescription() - .workspaceId(UUID.fromString(rs.getString("workspace_id"))) - .referenceId(UUID.fromString(rs.getString("reference_id"))) - .name(rs.getString("name")) - .resourceDescription( - rs.getString("resource_id") == null - ? null - : resourceDescriptionMapper.mapRow(rs, rowNum)) - .credentialId(rs.getString("credential_id")) + .workspaceId(UUID.fromString(rec.get(referenceTable.WORKSPACE_ID))) + .referenceId(UUID.fromString(rec.get(referenceTable.REFERENCE_ID))) + .name(rec.get(referenceTable.NAME)) + .resourceDescription(resourceId == null ? null : resourceDescriptionMapper.map(rec)) + .credentialId(rec.get(referenceTable.CREDENTIAL_ID)) .cloningInstructions( DataReferenceDescription.CloningInstructionsEnum.fromValue( - rs.getString("cloning_instructions"))) + rec.get(referenceTable.CLONING_INSTRUCTIONS))) .referenceType( - DataReferenceDescription.ReferenceTypeEnum.fromValue(rs.getString("reference_type"))) - .reference(rs.getString("reference")); + DataReferenceDescription.ReferenceTypeEnum.fromValue( + rec.get(referenceTable.REFERENCE_TYPE))) + .reference(rec.get(referenceTable.REFERENCE).toString()); } } - - // Returns a SQL condition as a string accepts both uncontrolled data references and visible - // controlled references. Uncontrolled references are not tracked as resources, and their - // existence is always visible to all workspace readers. - public static String uncontrolledOrVisibleResourcesClause( - String resourceTableAlias, String referenceTableAlias) { - return "((" - + referenceTableAlias - + ".resource_id IS NULL) OR " - + visibleResourcesClause(resourceTableAlias) - + ")"; - } - - // Returns a SQL condition as a string that filters out invisible controlled references. - // References are considered 'invisible' if the is_visible column of the corresponding resource - // is false AND the resource owner is not the user issuing the query. - // Uncontrolled references are not tracked as resources, and their existence is always visible - // to all workspace readers. - public static String visibleResourcesClause(String resourceTableAlias) { - return "(" - + resourceTableAlias - + ".is_visible = true OR " - + resourceTableAlias - + ".owner = :owner)"; - } - - // Combines a list of String SQL conditions with the delimiter `" AND "` to create a single - // SQL `WHERE` clause. Ignores null and empty strings. - public static String combineWhereClauses(List clauses) { - return " WHERE (" - + clauses.stream().filter(StringUtils::isNotBlank).collect(Collectors.joining(" AND ")) - + ")"; - } } diff --git a/src/main/java/bio/terra/workspace/db/WorkspaceDao.java b/src/main/java/bio/terra/workspace/db/WorkspaceDao.java index da536bd8f8..c60b314737 100644 --- a/src/main/java/bio/terra/workspace/db/WorkspaceDao.java +++ b/src/main/java/bio/terra/workspace/db/WorkspaceDao.java @@ -1,17 +1,17 @@ package bio.terra.workspace.db; -import bio.terra.workspace.app.configuration.WorkspaceManagerJdbcConfiguration; +import static bio.terra.workspace.db.generated.tables.Workspace.*; + import bio.terra.workspace.common.exception.DuplicateWorkspaceException; import bio.terra.workspace.common.exception.WorkspaceNotFoundException; +import bio.terra.workspace.db.generated.tables.records.WorkspaceRecord; import bio.terra.workspace.generated.model.WorkspaceDescription; -import java.util.HashMap; -import java.util.Map; import java.util.UUID; +import org.jooq.DSLContext; +import org.jooq.exception.DataAccessException; +import org.jooq.exception.SQLStateClass; import org.openapitools.jackson.nullable.JsonNullable; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.dao.DuplicateKeyException; -import org.springframework.dao.EmptyResultDataAccessException; -import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Isolation; import org.springframework.transaction.annotation.Propagation; @@ -19,63 +19,56 @@ @Component public class WorkspaceDao { - private final NamedParameterJdbcTemplate jdbcTemplate; + private final DSLContext dslContext; @Autowired - public WorkspaceDao(WorkspaceManagerJdbcConfiguration jdbcConfiguration) { - jdbcTemplate = new NamedParameterJdbcTemplate(jdbcConfiguration.getDataSource()); + public WorkspaceDao(DSLContext dslContext) { + this.dslContext = dslContext; } @Transactional(propagation = Propagation.REQUIRED, isolation = Isolation.SERIALIZABLE) public String createWorkspace(UUID workspaceId, JsonNullable spendProfile) { - String sql = - "INSERT INTO workspace (workspace_id, spend_profile, profile_settable) values " - + "(:id, :spend_profile, :spend_profile_settable)"; - - Map paramMap = new HashMap<>(); - paramMap.put("id", workspaceId.toString()); - paramMap.put("spend_profile", spendProfile.orElse(null)); - paramMap.put("spend_profile_settable", !spendProfile.isPresent()); - try { - jdbcTemplate.update(sql, paramMap); - } catch (DuplicateKeyException e) { - throw new DuplicateWorkspaceException( - "Workspace " + workspaceId.toString() + " already exists.", e); + dslContext + .insertInto(WORKSPACE) + .columns(WORKSPACE.WORKSPACE_ID, WORKSPACE.SPEND_PROFILE, WORKSPACE.PROFILE_SETTABLE) + .values( + workspaceId.toString(), + spendProfile.isPresent() ? spendProfile.get().toString() : null, + !spendProfile.isPresent()) + .execute(); + } catch (DataAccessException ex) { + if (ex.sqlStateClass() == SQLStateClass.C23_INTEGRITY_CONSTRAINT_VIOLATION) { + throw new DuplicateWorkspaceException("Workspace " + workspaceId.toString() + " already exists.", ex); + } + throw ex; } + return workspaceId.toString(); } @Transactional(propagation = Propagation.REQUIRED, isolation = Isolation.SERIALIZABLE) public boolean deleteWorkspace(UUID workspaceId) { - Map paramMap = new HashMap(); - paramMap.put("id", workspaceId.toString()); int rowsAffected = - jdbcTemplate.update("DELETE FROM workspace WHERE workspace_id = :id", paramMap); + dslContext + .delete(WORKSPACE) + .where(WORKSPACE.WORKSPACE_ID.eq(workspaceId.toString())) + .execute(); return rowsAffected > 0; } public WorkspaceDescription getWorkspace(String id) { - String sql = "SELECT * FROM workspace where workspace_id = (:id)"; - - Map paramMap = new HashMap<>(); - paramMap.put("id", id); - - try { - Map queryOutput = jdbcTemplate.queryForMap(sql, paramMap); - + WorkspaceRecord rec = + dslContext.selectFrom(WORKSPACE).where(WORKSPACE.WORKSPACE_ID.eq(id)).fetchOne(); + if (rec != null) { WorkspaceDescription desc = new WorkspaceDescription(); - desc.setId(UUID.fromString(queryOutput.get("workspace_id").toString())); - - if (queryOutput.getOrDefault("spend_profile", null) == null) { - desc.setSpendProfile(JsonNullable.undefined()); - } else { - desc.setSpendProfile( - JsonNullable.of(UUID.fromString(queryOutput.get("spend_profile").toString()))); - } - + desc.setId(UUID.fromString(rec.getWorkspaceId())); + desc.setSpendProfile( + rec.getSpendProfile() != null + ? JsonNullable.of(UUID.fromString(rec.getSpendProfile())) + : JsonNullable.undefined()); return desc; - } catch (EmptyResultDataAccessException e) { + } else { throw new WorkspaceNotFoundException("Workspace not found."); } } diff --git a/src/test/java/bio/terra/workspace/db/DataReferenceDaoTest.java b/src/test/java/bio/terra/workspace/db/DataReferenceDaoTest.java index 0dc28a127d..977023080b 100644 --- a/src/test/java/bio/terra/workspace/db/DataReferenceDaoTest.java +++ b/src/test/java/bio/terra/workspace/db/DataReferenceDaoTest.java @@ -15,6 +15,8 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import java.util.UUID; +import org.jooq.exception.DataAccessException; +import org.jooq.exception.SQLStateClass; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -95,15 +97,22 @@ public void createReferenceWithoutWorkspaceFails() throws Exception { assertThrows( DataIntegrityViolationException.class, () -> { - dataReferenceDao.createDataReference( - referenceId, - UUID.randomUUID(), // non-existing workspace ID - name, - JsonNullable.undefined(), - JsonNullable.of(credentialId), - cloningInstructions, - JsonNullable.of(referenceType), - JsonNullable.of(reference.toString())); + try { + dataReferenceDao.createDataReference( + referenceId, + UUID.randomUUID(), // non-existing workspace ID + name, + JsonNullable.undefined(), + JsonNullable.of(credentialId), + cloningInstructions, + JsonNullable.of(referenceType), + JsonNullable.of(reference)); + } catch (DataAccessException ex) { + if (ex.sqlStateClass() == SQLStateClass.C23_INTEGRITY_CONSTRAINT_VIOLATION) { + throw new DataIntegrityViolationException("test"); + } + throw ex; + } }); } @@ -119,7 +128,7 @@ public void verifyGetDataReference() { JsonNullable.of(credentialId), cloningInstructions, JsonNullable.of(referenceType), - JsonNullable.of(reference.toString())); + JsonNullable.of(reference)); DataReferenceDescription result = dataReferenceDao.getDataReference(referenceId); assertThat(result.getWorkspaceId(), equalTo(workspaceId));