Skip to content

[DT-3253] Filter document fetches by entity-allowed categories #2901

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kevinmarete merged 4 commits into from
May 12, 2026
Merged

[DT-3253] Filter document fetches by entity-allowed categories #2901

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1f07742
Include categories
kevinmarete May 11, 2026
6acb758
Add tests
kevinmarete May 11, 2026
67a9a38
Update src/main/java/org/broadinstitute/consent/http/db/FileStorageOb…
kevinmarete May 11, 2026
396f0df
Fix spotless error
kevinmarete May 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions src/main/java/org/broadinstitute/consent/http/db/FileStorageObjectDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import org.broadinstitute.consent.http.models.FileStorageObject;
import org.jdbi.v3.sqlobject.config.RegisterRowMapper;
import org.jdbi.v3.sqlobject.customizer.Bind;
import org.jdbi.v3.sqlobject.customizer.BindList;
import org.jdbi.v3.sqlobject.customizer.BindList.EmptyHandling;
import org.jdbi.v3.sqlobject.statement.GetGeneratedKeys;
import org.jdbi.v3.sqlobject.statement.SqlQuery;
import org.jdbi.v3.sqlobject.statement.SqlUpdate;
Expand Down Expand Up @@ -146,6 +148,34 @@ default FileStorageObject findById(Integer fileStorageObjectId) {
FileStorageObject findActiveFileByIdAndEntityId(
@Bind("entityId") String entityId, @Bind("fileStorageObjectId") Integer fileStorageObjectId);

/** Returns the file only when it belongs to the entity and one of the allowed categories. */
@RegisterRowMapper(FileStorageObjectMapperWithFSOPrefix.class)
@SqlQuery(
"""
SELECT
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.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,
fso.deleted AS fso_deleted,
fso.delete_user_id AS fso_delete_user_id
FROM file_storage_object fso
WHERE fso.entity_id = :entityId
AND fso.file_storage_object_id = :fileStorageObjectId
AND (fso.deleted = false OR fso.deleted IS NULL)
AND fso.category IN (<categories>)
""")
FileStorageObject findActiveFileByIdAndEntityIdAndCategories(
@Bind("entityId") String entityId,
@Bind("fileStorageObjectId") Integer fileStorageObjectId,
@BindList(value = "categories", onEmpty = EmptyHandling.NULL_STRING) List<String> categories);

/**
* Returns ALL file records for the entity, including soft-deleted ones. Callers are responsible
* for filtering on {@code deleted} if needed.
Expand Down Expand Up @@ -193,4 +223,29 @@ List<FileStorageObject> findFilesByEntityIdAndCategory(
ORDER BY fso.create_date DESC
""")
List<FileStorageObject> findFileMetadataByEntityId(@Bind("entityId") String entityId);

@UseRowMapper(FileStorageObjectMapperWithFSOPrefix.class)
@SqlQuery(
"""
SELECT
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.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,
fso.deleted AS fso_deleted,
fso.delete_user_id AS fso_delete_user_id
FROM file_storage_object fso
WHERE fso.entity_id = :entityId
AND fso.category IN (<categories>)
ORDER BY fso.create_date DESC
""")
List<FileStorageObject> findFileMetadataByEntityIdAndCategories(
@Bind("entityId") String entityId,
@BindList(value = "categories", onEmpty = EmptyHandling.NULL_STRING) List<String> categories);
}
46 changes: 41 additions & 5 deletions src/main/java/org/broadinstitute/consent/http/service/FileStorageObjectService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public FileStorageObject getDocument(
checkAccess(user, entity, entityId, null, OperationType.READ);

String resolvedEntityId = resolveEntityId(user, documentEntity, entityId);
return fetchMetadataByEntityIdAndId(resolvedEntityId, fileStorageObjectId);
return fetchMetadataByEntityIdAndId(resolvedEntityId, fileStorageObjectId, documentEntity);
}

/**
Expand All @@ -134,7 +134,8 @@ public FileStorageObject getDocumentFile(
User user, String entity, String entityId, Integer fileStorageObjectId) {
DocumentEntity documentEntity = requireDocumentEntity(entity);
String resolvedEntityId = resolveEntityId(user, documentEntity, entityId);
FileStorageObject fso = fetchMetadataByEntityIdAndId(resolvedEntityId, fileStorageObjectId);
FileStorageObject fso =
fetchMetadataByEntityIdAndId(resolvedEntityId, fileStorageObjectId, documentEntity);

checkAccess(user, entity, entityId, fso.getCategory(), OperationType.READ);

Expand Down Expand Up @@ -167,7 +168,7 @@ public List<FileStorageObject> listDocuments(User user, String entity, String en
checkAccess(user, entity, entityId, null, OperationType.READ);

String resolvedEntityId = resolveEntityId(user, documentEntity, entityId);
return fetchAllMetadataByEntityId(resolvedEntityId);
return fetchAllMetadataByEntityId(resolvedEntityId, documentEntity);
}

/**
Expand All @@ -188,7 +189,7 @@ public FileStorageObject updateDocumentCategory(

String resolvedEntityId = resolveEntityId(user, documentEntity, entityId);
FileStorageObject fileStorageObject =
fetchMetadataByEntityIdAndId(resolvedEntityId, fileStorageObjectId);
fetchMetadataByEntityIdAndId(resolvedEntityId, fileStorageObjectId, documentEntity);

Instant updateDate = Instant.now();
fileStorageObjectDAO.updateCategory(fileStorageObjectId, category.getValue(), user.getUserId());
Expand Down Expand Up @@ -219,7 +220,7 @@ public FileStorageObject deleteDocument(
DocumentEntity documentEntity = requireDocumentEntity(entity);
String resolvedEntityId = resolveEntityId(user, documentEntity, entityId);
FileStorageObject fileStorageObject =
fetchMetadataByEntityIdAndId(resolvedEntityId, fileStorageObjectId);
fetchMetadataByEntityIdAndId(resolvedEntityId, fileStorageObjectId, documentEntity);

checkAccess(user, entity, entityId, fileStorageObject.getCategory(), OperationType.WRITE);

Expand Down Expand Up @@ -503,6 +504,22 @@ public FileStorageObject fetchMetadataByEntityIdAndId(
return fileStorageObject;
}
Comment thread
kevinmarete marked this conversation as resolved.

/**
* Fetches metadata for an active file constrained to categories valid for the requested entity.
* This prevents records for another document domain from being returned when IDs overlap.
*/
public FileStorageObject fetchMetadataByEntityIdAndId(
String entityId, Integer fileStorageObjectId, DocumentEntity entity)
throws NotFoundException {
FileStorageObject fileStorageObject =
fileStorageObjectDAO.findActiveFileByIdAndEntityIdAndCategories(
entityId, fileStorageObjectId, allowedCategoryValuesForEntity(entity));
if (fileStorageObject == null) {
throw new NotFoundException("File not found");
}
return fileStorageObject;
}

/**
* Returns ALL file metadata for the entity, including soft-deleted records. Callers should
* inspect {@link FileStorageObject#getDeleted()} and filter as appropriate for their use case.
Expand All @@ -511,6 +528,13 @@ public List<FileStorageObject> fetchAllMetadataByEntityId(String entityId) {
return fileStorageObjectDAO.findFileMetadataByEntityId(entityId);
}

/** Returns file metadata scoped to categories valid for the requested entity. */
public List<FileStorageObject> fetchAllMetadataByEntityId(
String entityId, DocumentEntity entity) {
return fileStorageObjectDAO.findFileMetadataByEntityIdAndCategories(
entityId, allowedCategoryValuesForEntity(entity));
}

/**
* Returns ALL files for the entity (active + deleted), populated with GCS content. Callers should
* inspect {@link FileStorageObject#getDeleted()} and filter as appropriate.
Expand Down Expand Up @@ -597,6 +621,18 @@ private boolean isCategoryAllowedForEntity(DocumentEntity entity, FileCategory c
};
}

private List<String> allowedCategoryValuesForEntity(DocumentEntity entity) {
return switch (entity) {
case DAR ->
List.of(
FileCategory.IRB_COLLABORATION_LETTER.getValue(),
FileCategory.DATA_USE_LETTER.getValue());
Comment thread
kevinmarete marked this conversation as resolved.
case DAC -> List.of(FileCategory.DATA_ACCESS_AGREEMENT.getValue());
case DATASET -> List.of(FileCategory.NIH_INSTITUTIONAL_CERTIFICATION.getValue());
case STUDY -> List.of(FileCategory.ALTERNATIVE_DATA_SHARING_PLAN.getValue());
};
}

private Integer parseNumericEntityId(String entityId) {
try {
return Integer.valueOf(entityId);
Expand Down
38 changes: 38 additions & 0 deletions src/test/java/org/broadinstitute/consent/http/db/FileStorageObjectDAOTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,30 @@ void testFindFileMetadataByEntityId() {
filesFound.forEach(file -> assertEquals(entityId, file.getEntityId()));
}

@Test
void testFindFileMetadataByEntityIdAndCategoriesFiltersCategory() {
String entityId = randomAlphabetic(10);

FileStorageObject matching1 =
createFileStorageObject(entityId, FileCategory.IRB_COLLABORATION_LETTER);
FileStorageObject matching2 = createFileStorageObject(entityId, FileCategory.DATA_USE_LETTER);
createFileStorageObject(entityId, FileCategory.DATA_ACCESS_AGREEMENT);
createFileStorageObject(randomAlphabetic(8), FileCategory.IRB_COLLABORATION_LETTER);

List<FileStorageObject> filesFound =
fileStorageObjectDAO.findFileMetadataByEntityIdAndCategories(
entityId,
List.of(
FileCategory.IRB_COLLABORATION_LETTER.getValue(),
FileCategory.DATA_USE_LETTER.getValue()));

assertEquals(2, filesFound.size());
List<Integer> foundIds =
filesFound.stream().map(FileStorageObject::getFileStorageObjectId).toList();
assertTrue(foundIds.contains(matching1.getFileStorageObjectId()));
assertTrue(foundIds.contains(matching2.getFileStorageObjectId()));
}

@Test
void testFindActiveFileByIdAndEntityId() {
String entityId = randomAlphabetic(10);
Expand Down Expand Up @@ -309,6 +333,20 @@ void testFindActiveFileByIdAndEntityIdWrongEntityReturnsNull() {
assertNull(found);
}

@Test
void testFindActiveFileByIdAndEntityIdAndCategoriesWrongCategoryReturnsNull() {
String entityId = randomAlphabetic(10);
FileStorageObject file = createFileStorageObject(entityId, FileCategory.DATA_ACCESS_AGREEMENT);

FileStorageObject found =
fileStorageObjectDAO.findActiveFileByIdAndEntityIdAndCategories(
entityId,
file.getFileStorageObjectId(),
List.of(FileCategory.IRB_COLLABORATION_LETTER.getValue()));

assertNull(found);
}

@Test
void testCreateFile() {
String fileName = randomAlphabetic(10);
Expand Down
Loading
Loading