From f73e0abb919b34c1ac4bffbdef55fe483c6a77f0 Mon Sep 17 00:00:00 2001 From: Gregory Rushton Date: Wed, 4 Sep 2024 10:03:44 -0400 Subject: [PATCH] DCJ-570: Add Associated DAA and DAA File info to Get All DACs (#2397) --- .../consent/http/db/DacDAO.java | 88 ++++++++---- .../consent/http/db/mapper/DacReducer.java | 2 +- .../db/mapper/FileStorageObjectMapper.java | 12 +- .../consent/http/db/DacDAOTest.java | 130 +++++++++--------- 4 files changed, 138 insertions(+), 94 deletions(-) diff --git a/src/main/java/org/broadinstitute/consent/http/db/DacDAO.java b/src/main/java/org/broadinstitute/consent/http/db/DacDAO.java index 381641c8df..d61cfd9868 100644 --- a/src/main/java/org/broadinstitute/consent/http/db/DacDAO.java +++ b/src/main/java/org/broadinstitute/consent/http/db/DacDAO.java @@ -6,7 +6,7 @@ import java.util.Set; import org.broadinstitute.consent.http.db.mapper.DacMapper; import org.broadinstitute.consent.http.db.mapper.DacReducer; -import org.broadinstitute.consent.http.db.mapper.FileStorageObjectMapper; +import org.broadinstitute.consent.http.db.mapper.FileStorageObjectMapperWithFSOPrefix; import org.broadinstitute.consent.http.db.mapper.RoleMapper; import org.broadinstitute.consent.http.db.mapper.UserRoleMapper; import org.broadinstitute.consent.http.db.mapper.UserWithRolesMapper; @@ -29,7 +29,7 @@ import org.jdbi.v3.sqlobject.transaction.Transactional; @RegisterRowMapper(DacMapper.class) -@RegisterRowMapper(FileStorageObjectMapper.class) +@RegisterRowMapper(FileStorageObjectMapperWithFSOPrefix.class) public interface DacDAO extends Transactional { String QUERY_FIELD_SEPARATOR = ", "; @@ -39,17 +39,53 @@ public interface DacDAO extends Transactional { * * @return List */ + @RegisterBeanMapper(value = DataAccessAgreement.class, prefix = "daa") + @RegisterBeanMapper(value = FileStorageObjectDAO.class) @RegisterBeanMapper(value = Dac.class) @RegisterBeanMapper(value = Dataset.class) @UseRowReducer(DacReducer.class) @SqlQuery(""" - SELECT dac.dac_id, dac.email, dac.name, dac.description, d.dataset_id, d.name AS dataset_name, - DATE(d.create_date) AS dataset_create_date, d.object_id, d.active, d.needs_approval, - d.alias AS dataset_alias, d.create_user_id, d.update_date AS dataset_update_date, - d.update_user_id, d.data_use AS dataset_data_use, d.sharing_plan_document, - d.sharing_plan_document_name + SELECT + dac.dac_id, + dac.email, + dac.name, + dac.description, + d.dataset_id, + d.name AS dataset_name, + DATE(d.create_date) AS dataset_create_date, + d.object_id, + d.active, + d.needs_approval, + d.alias AS dataset_alias, + d.create_user_id, + d.update_date AS dataset_update_date, + d.update_user_id, + d.data_use AS dataset_data_use, + d.sharing_plan_document, + d.sharing_plan_document_name, + daa.daa_id AS daa_daa_id, + daa.create_user_id AS daa_create_user_id, + daa.create_date AS daa_create_date, + daa.update_user_id AS daa_update_user_id, + daa.update_date AS daa_update_date, + daa.initial_dac_id AS daa_initial_dac_id, + fso.file_storage_object_id AS fso_file_storage_object_id, + fso.entity_id AS fso_entity_id, + fso.file_name AS fso_file_name, + fso.category AS fso_category, + fso.gcs_file_uri AS fso_gcs_file_uri, + fso.media_type AS fso_media_type, + fso.deleted AS fso_deleted, + fso.delete_user_id AS fso_delete_user_id, + fso.create_date AS fso_create_date, + fso.create_user_id AS fso_create_user_id, + fso.update_date AS fso_update_date, + fso.update_user_id AS fso_update_user_id FROM dac - LEFT OUTER JOIN dataset d ON dac.dac_id = d.dac_id + LEFT JOIN dataset d ON dac.dac_id = d.dac_id + LEFT JOIN dac_daa dd ON dac.dac_id = dd.dac_id + LEFT JOIN data_access_agreement daa ON dd.daa_id = daa.daa_id + LEFT JOIN file_storage_object fso ON daa.daa_id::text = fso.entity_id """) List findAll(); @@ -96,24 +132,24 @@ public interface DacDAO extends Transactional { @UseRowReducer(DacReducer.class) @SqlQuery(""" SELECT dac.*, - daa.daa_id as daa_daa_id, - daa.create_user_id as daa_create_user_id, - daa.create_date as daa_create_date, - daa.update_user_id as daa_update_user_id, - daa.update_date as daa_update_date, - daa.initial_dac_id as daa_initial_dac_id, - fso.file_storage_object_id AS file_storage_object_id, - fso.entity_id AS entity_id, - fso.file_name AS file_name, - fso.category AS category, - fso.gcs_file_uri AS gcs_file_uri, - fso.media_type AS media_type, - fso.create_date AS create_date, - fso.create_user_id AS create_user_id, - fso.update_date AS update_date, - fso.update_user_id AS update_user_id, - fso.deleted AS deleted, - fso.delete_user_id AS delete_user_id + daa.daa_id as daa_daa_id, + daa.create_user_id as daa_create_user_id, + daa.create_date as daa_create_date, + daa.update_user_id as daa_update_user_id, + daa.update_date as daa_update_date, + daa.initial_dac_id as daa_initial_dac_id, + fso.file_storage_object_id AS fso_file_storage_object_id, + fso.entity_id AS fso_entity_id, + fso.file_name AS fso_file_name, + fso.category AS fso_category, + fso.gcs_file_uri AS fso_gcs_file_uri, + fso.media_type AS fso_media_type, + fso.deleted AS fso_deleted, + fso.delete_user_id AS fso_delete_user_id, + fso.create_date AS fso_create_date, + fso.create_user_id AS fso_create_user_id, + fso.update_date AS fso_update_date, + fso.update_user_id AS fso_update_user_id FROM dac LEFT JOIN dac_daa dd ON dac.dac_id = dd.dac_id LEFT JOIN data_access_agreement daa ON dd.daa_id = daa.daa_id diff --git a/src/main/java/org/broadinstitute/consent/http/db/mapper/DacReducer.java b/src/main/java/org/broadinstitute/consent/http/db/mapper/DacReducer.java index b195219652..73c513e57f 100644 --- a/src/main/java/org/broadinstitute/consent/http/db/mapper/DacReducer.java +++ b/src/main/java/org/broadinstitute/consent/http/db/mapper/DacReducer.java @@ -33,7 +33,7 @@ public void accumulate(Map container, RowView rowView) { daa = rowView.getRow(DataAccessAgreement.class); } - if (rowView.getColumn("file_storage_object_id", String.class) != null) { + if (hasNonZeroColumn(rowView, "fso_file_storage_object_id")) { FileStorageObject fso = rowView.getRow(FileStorageObject.class); daa.setFile(fso); } diff --git a/src/main/java/org/broadinstitute/consent/http/db/mapper/FileStorageObjectMapper.java b/src/main/java/org/broadinstitute/consent/http/db/mapper/FileStorageObjectMapper.java index 18bb16576a..57be44d659 100644 --- a/src/main/java/org/broadinstitute/consent/http/db/mapper/FileStorageObjectMapper.java +++ b/src/main/java/org/broadinstitute/consent/http/db/mapper/FileStorageObjectMapper.java @@ -8,10 +8,12 @@ import java.util.Objects; import org.broadinstitute.consent.http.enumeration.FileCategory; import org.broadinstitute.consent.http.models.FileStorageObject; +import org.broadinstitute.consent.http.util.ConsentLogger; import org.jdbi.v3.core.mapper.RowMapper; import org.jdbi.v3.core.statement.StatementContext; -public class FileStorageObjectMapper implements RowMapper, RowMapperHelper { +public class FileStorageObjectMapper implements RowMapper, RowMapperHelper, + ConsentLogger { @Override public FileStorageObject map(ResultSet r, StatementContext statementContext) throws SQLException { @@ -30,17 +32,21 @@ public FileStorageObject map(ResultSet r, StatementContext statementContext) thr } if (hasColumn(r, addPrefix("gcs_file_uri"))) { + String value = r.getString(addPrefix("gcs_file_uri")); try { - file.setBlobId(BlobId.fromGsUtilUri(r.getString(addPrefix("gcs_file_uri")))); + file.setBlobId(BlobId.fromGsUtilUri(value)); } catch (Exception e) { + logException("Error parsing blob id: %s for fso id: %s".formatted(value, file.getFileStorageObjectId()), e); file.setBlobId(null); } } if (hasColumn(r, addPrefix("category"))) { + String value = r.getString(addPrefix("category")); try { - file.setCategory(FileCategory.findValue(r.getString(addPrefix("category")))); + file.setCategory(FileCategory.findValue(value)); } catch (Exception e) { + logException("Error parsing file category: %s for fso id: %s".formatted(value, file.getFileStorageObjectId()), e); file.setCategory(null); } } diff --git a/src/test/java/org/broadinstitute/consent/http/db/DacDAOTest.java b/src/test/java/org/broadinstitute/consent/http/db/DacDAOTest.java index 6dc01c898b..3050e4a7f4 100644 --- a/src/test/java/org/broadinstitute/consent/http/db/DacDAOTest.java +++ b/src/test/java/org/broadinstitute/consent/http/db/DacDAOTest.java @@ -6,6 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import jakarta.ws.rs.core.MediaType; import java.sql.Timestamp; import java.time.Instant; import java.util.ArrayList; @@ -18,6 +19,7 @@ import java.util.UUID; import org.apache.commons.lang3.RandomStringUtils; import org.apache.commons.lang3.RandomUtils; +import org.broadinstitute.consent.http.enumeration.FileCategory; import org.broadinstitute.consent.http.enumeration.OrganizationType; import org.broadinstitute.consent.http.enumeration.PropertyType; import org.broadinstitute.consent.http.enumeration.UserRoles; @@ -43,7 +45,8 @@ class DacDAOTest extends DAOTestHelper { @Test void testInsertWithoutEmail() { - Dac dac = insertDac(); + Integer dacId = createRandomDAC(); + Dac dac = dacDAO.findById(dacId); assertNotNull(dac); } @@ -89,40 +92,27 @@ void testFindAllDACUsersBySearchString_case2() { @Test void testFindAllNoDatasets() { - Integer id = dacDAO.createDac( - "Test_" + RandomStringUtils.random(20, true, true), - "Test_" + RandomStringUtils.random(20, true, true), - new Date()); - Integer id2 = dacDAO.createDac( - "Test_" + RandomStringUtils.random(20, true, true), - "Test_" + RandomStringUtils.random(20, true, true), - new Date()); - User user = createUser(); + Integer dacId1 = createRandomDAC(); + Integer dacId2 = createRandomDAC(); List dacs = dacDAO.findAll(); Dac dac1 = dacs.get(0); - assertEquals(id, dac1.getDacId()); + assertEquals(dacId1, dac1.getDacId()); assertEquals(0, dac1.getDatasetIds().size()); assertNull(dac1.getAssociatedDaa()); Dac dac2 = dacs.get(1); - assertEquals(id2, dac2.getDacId()); + assertEquals(dacId2, dac2.getDacId()); assertEquals(0, dac2.getDatasetIds().size()); assertNull(dac2.getAssociatedDaa()); } @Test void testFindAllWithDataset() { - Integer id = dacDAO.createDac( - "Test_" + RandomStringUtils.random(20, true, true), - "Test_" + RandomStringUtils.random(20, true, true), - new Date()); - Integer id2 = dacDAO.createDac( - "Test_" + RandomStringUtils.random(20, true, true), - "Test_" + RandomStringUtils.random(20, true, true), - new Date()); + Integer dacId1 = createRandomDAC(); + Integer dacId2 = createRandomDAC(); User user = createUser(); - Integer datasetId = datasetDAO.insertDataset(RandomStringUtils.random(20, true, true), new Timestamp(new Date().getTime()), user.getUserId(), RandomStringUtils.random(20, true, true), new DataUseBuilder().setGeneralUse(true).build().toString(), id); + Integer datasetId = datasetDAO.insertDataset(RandomStringUtils.random(20, true, true), new Timestamp(new Date().getTime()), user.getUserId(), RandomStringUtils.random(20, true, true), new DataUseBuilder().setGeneralUse(true).build().toString(), dacId1); List list = new ArrayList<>(); DatasetProperty dsp = new DatasetProperty(); dsp.setDataSetId(datasetId); @@ -131,35 +121,48 @@ void testFindAllWithDataset() { dsp.setCreateDate(new Date()); list.add(dsp); datasetDAO.insertDatasetProperties(list); - Integer datasetId2 = datasetDAO.insertDataset(RandomStringUtils.random(20, true, true), new Timestamp(new Date().getTime()), user.getUserId(), RandomStringUtils.random(20, true, true), new DataUseBuilder().setGeneralUse(true).build().toString(), id2); - Dataset dataset1 = datasetDAO.findDatasetById(datasetId); - Dataset dataset2 = datasetDAO.findDatasetById(datasetId2); + Integer datasetId2 = datasetDAO.insertDataset(RandomStringUtils.random(20, true, true), new Timestamp(new Date().getTime()), user.getUserId(), RandomStringUtils.random(20, true, true), new DataUseBuilder().setGeneralUse(true).build().toString(), dacId2); + List dacs = dacDAO.findAll(); Dac dac1 = dacs.get(0); List datasetIds = dac1.getDatasetIds(); - assertEquals(id, dac1.getDacId()); + assertEquals(dacId1, dac1.getDacId()); assertEquals(1, datasetIds.size()); assertEquals(datasetId, datasetIds.get(0)); assertNull(dac1.getAssociatedDaa()); Dac dac2 = dacs.get(1); List datasetIds2 = dac2.getDatasetIds(); - assertEquals(id2, dac2.getDacId()); + assertEquals(dacId2, dac2.getDacId()); assertEquals(1, datasetIds2.size()); assertEquals(datasetId2, datasetIds2.get(0)); assertNull(dac2.getAssociatedDaa()); } @Test - void testFindByIdNoDaa() { - Integer id = dacDAO.createDac( - "Test_" + RandomStringUtils.random(20, true, true), - "Test_" + RandomStringUtils.random(20, true, true), - new Date()); + void testFindAllWithDAAs() { User user = createUser(); - Integer daaId = daaDAO.createDaa(user.getUserId(), new Date().toInstant(), user.getUserId(), new Date().toInstant(), id); - DataAccessAgreement daa = daaDAO.findById(daaId); + + Integer dacId1 = createRandomDAC(); + Integer daaId1 = daaDAO.createDaa(user.getUserId(), new Date().toInstant(), user.getUserId(), new Date().toInstant(), dacId1); + createFSO(user.getUserId(), daaId1); + daaDAO.createDacDaaRelation(dacId1, daaId1); + + Integer dacId2 = createRandomDAC(); + Integer daaId2 = daaDAO.createDaa(user.getUserId(), new Date().toInstant(), user.getUserId(), new Date().toInstant(), dacId2); + createFSO(user.getUserId(), daaId2); + daaDAO.createDacDaaRelation(dacId2, daaId2); + + dacDAO.findAll().forEach(dac -> { + assertNotNull(dac.getAssociatedDaa()); + assertNotNull(dac.getAssociatedDaa().getFile()); + }); + } + + @Test + void testFindByIdNoDaa() { + Integer id = createRandomDAC(); Dac dac = dacDAO.findById(id); assertEquals(id, dac.getDacId()); assertNull(dac.getAssociatedDaa()); @@ -167,10 +170,7 @@ void testFindByIdNoDaa() { @Test void testFindByIdWithDaa() { - Integer id = dacDAO.createDac( - "Test_" + RandomStringUtils.random(20, true, true), - "Test_" + RandomStringUtils.random(20, true, true), - new Date()); + Integer id = createRandomDAC(); User user = createUser(); Integer daaId = daaDAO.createDaa(user.getUserId(), new Date().toInstant(), user.getUserId(), new Date().toInstant(), id); DataAccessAgreement daa = daaDAO.findById(daaId); @@ -188,10 +188,7 @@ void testFindByIdWithDaa() { @Test void testCreateDac() { - Integer id = dacDAO.createDac( - "Test_" + RandomStringUtils.random(20, true, true), - "Test_" + RandomStringUtils.random(20, true, true), - new Date()); + Integer id = createRandomDAC(); Dac dac = dacDAO.findById(id); assertEquals(dac.getDacId(), id); } @@ -199,9 +196,9 @@ void testCreateDac() { @Test void testUpdateDacWithoutEmail() { String newValue = "New Value"; - Dac dac = insertDac(); - dacDAO.updateDac(newValue, newValue, new Date(), dac.getDacId()); - Dac updatedDac = dacDAO.findById(dac.getDacId()); + Integer dacId = createRandomDAC(); + dacDAO.updateDac(newValue, newValue, new Date(), dacId); + Dac updatedDac = dacDAO.findById(dacId); assertEquals(updatedDac.getName(), newValue); assertEquals(updatedDac.getDescription(), newValue); @@ -411,15 +408,7 @@ void testFindDacsForCollectionId() { Collection results = dacDAO.findDacsForCollectionId(collection.getDarCollectionId()); assertEquals(1, results.size()); - assertTrue(results.stream().map(d -> d.getDacId()).toList().contains(dac.getDacId())); - } - - private Dac insertDac() { - Integer id = dacDAO.createDac( - "Test_" + RandomStringUtils.random(20, true, true), - "Test_" + RandomStringUtils.random(20, true, true), - new Date()); - return dacDAO.findById(id); + assertTrue(results.stream().map(Dac::getDacId).toList().contains(dac.getDacId())); } private Dac insertDacWithEmail() { @@ -433,10 +422,7 @@ private Dac insertDacWithEmail() { } private Dac createDac() { - Integer id = dacDAO.createDac( - "Test_" + RandomStringUtils.random(20, true, true), - "Test_" + RandomStringUtils.random(20, true, true), - new Date()); + Integer id = createRandomDAC(); return dacDAO.findById(id); } @@ -467,17 +453,15 @@ private DarCollection createDarCollection() { Integer collection_id = darCollectionDAO.insertDarCollection(darCode, user.getUserId(), new Date()); Dataset dataset = createDataset(); - DataAccessRequest dar = createDataAccessRequest(user.getUserId(), collection_id, darCode); + DataAccessRequest dar = createDataAccessRequest(user.getUserId(), collection_id); dataAccessRequestDAO.insertDARDatasetRelation(dar.getReferenceId(), dataset.getDataSetId()); - createDataAccessRequest(user.getUserId(), collection_id, darCode); + createDataAccessRequest(user.getUserId(), collection_id); return darCollectionDAO.findDARCollectionByCollectionId(collection_id); } - private DataAccessRequest createDataAccessRequest(Integer userId, Integer collectionId, - String darCode) { + private DataAccessRequest createDataAccessRequest(Integer userId, Integer collectionId) { DataAccessRequestData data = new DataAccessRequestData(); data.setProjectTitle("Project Title: " + RandomStringUtils.random(50, true, false)); - data.setDarCode(darCode); DatasetEntry entry = new DatasetEntry(); entry.setKey("key"); entry.setValue("value"); @@ -496,7 +480,7 @@ private DataAccessRequest createDataAccessRequest(Integer userId, Integer collec return dataAccessRequestDAO.findByReferenceId(referenceId); } - private DataAccessRequest createDataAccessRequestInCollectionWithDataset( + private void createDataAccessRequestInCollectionWithDataset( DarCollection collection, Dataset d ) { @@ -513,7 +497,6 @@ private DataAccessRequest createDataAccessRequestInCollectionWithDataset( new DataAccessRequestData() ); dataAccessRequestDAO.insertDARDatasetRelation(randomUUID, d.getDataSetId()); - return dataAccessRequestDAO.findByReferenceId(randomUUID); } private Dataset createDataset() { @@ -550,4 +533,23 @@ private Dataset createDatasetWithDac(Integer dacId) { return datasetDAO.findDatasetById(id); } + private Integer createRandomDAC() { + return dacDAO.createDac( + "Test_" + RandomStringUtils.random(20, true, true), + "Test_" + RandomStringUtils.random(20, true, true), + new Date()); + } + + private void createFSO(Integer userId, Integer daaId) { + fileStorageObjectDAO.insertNewFile( + RandomStringUtils.randomAlphabetic(10), + FileCategory.DATA_ACCESS_AGREEMENT.getValue(), + RandomStringUtils.randomAlphabetic(10), + MediaType.TEXT_PLAIN_TYPE.getType(), + daaId.toString(), + userId, + Instant.now() + ); + + } }