Skip to content
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -537,11 +537,7 @@ MailMessageDAO providesMailMessageDAO() {

@Provides
MetricsService providesMetricsService() {
return new MetricsService(
providesDatasetDAO(),
providesDataAccessRequestDAO(),
providesDARCollectionDAO(),
providesElectionDAO());
return new MetricsService(providesDatasetDAO(), providesDataAccessRequestDAO());
}

@Provides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,49 @@ AND dar.collection_id NOT IN (
""")
List<DataAccessRequest> findApprovedDARsByDatasetId(@Bind("datasetId") Integer datasetId);

/**
* This query finds DARs submitted on dar-dataset combinations where the most recent vote is true
* similar to {@link #findApprovedDARsByDatasetId(Integer datasetId) findApprovedDARsByDatasetId}.
* The primary difference is that we want to include expired DARs to show in dataset usage
* metrics.
*
* @param datasetId The dataset id
* @return List of approved DARs for the dataset
*/
@UseRowReducer(DataAccessRequestReducer.class)
@SqlQuery(
"""
SELECT dar.id, dar.reference_id, dar.collection_id, dar.parent_id, dar.user_id,
dar.create_date, dar.submission_date, dar.update_date, dar.data, dar.era_commons_id,
dar.approving_so_id, dar.approving_so_timestamp, dar.requires_so_approval,
dar.closeout_so_approval_timestamp, dar.closeout_approving_so_id,
dd.dataset_id, collection.dar_code
FROM data_access_request dar
LEFT JOIN dar_collection collection on collection.collection_id = dar.collection_id
INNER JOIN dar_dataset dd ON dd.reference_id = dar.reference_id
INNER JOIN (
SELECT DISTINCT e.reference_id, e.dataset_id, LAST_VALUE(v.vote)
OVER(
PARTITION BY e.reference_id, e.dataset_id
ORDER BY v.create_date
RANGE BETWEEN
UNBOUNDED PRECEDING AND
UNBOUNDED FOLLOWING
) last_vote
FROM election e
INNER JOIN vote v ON e.election_id = v.election_id AND v.vote IS NOT NULL
AND LOWER(e.election_type) = 'dataaccess'
AND LOWER(v.type) IN ('final', 'radar_approve')) final_access_vote ON final_access_vote.reference_id = dar.reference_id AND final_access_vote.dataset_id = dd.dataset_id
WHERE dd.dataset_id = :datasetId
AND dar.submission_date IS NOT NULL
AND final_access_vote.last_vote = TRUE
AND (LOWER(dar.data->>'status') != 'archived' OR dar.data->>'status' IS NULL)
-- Exclude DARs that have a closeoutSupplement
AND data ->> 'closeoutSupplement' IS NULL
""")
List<DataAccessRequest> findSummaryMetricApprovedDARsByDatasetIdIncludesExpired(
@Bind("datasetId") Integer datasetId);

/**
* This query finds dataset ids on dar-dataset combinations where the most recent vote is true.
* This includes datasets that are a part of expired DARs, UNLIKE findApprovedDARsByDatasetId. The
Expand Down Expand Up @@ -305,11 +348,6 @@ void updateDataByReferenceId(
""")
void deleteByReferenceId(@Bind("referenceId") String referenceId);

@SqlUpdate("DELETE FROM data_access_request WHERE reference_id IN (<referenceIds>)")
void deleteByReferenceIds(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by fix: unused

@BindList(value = "referenceIds", onEmpty = EmptyHandling.NULL_STRING)
List<String> referenceIds);

@SqlUpdate(
"""
UPDATE data_access_request
Expand Down Expand Up @@ -494,16 +532,6 @@ INSERT INTO dar_dataset (reference_id, dataset_id)
""")
void deleteDARDatasetRelationByReferenceId(@Bind("referenceId") String referenceId);

/**
* Delete rows which have a referenceId that is in the list referenceIds
*
* @param referenceIds List<String>
*/
@SqlUpdate("DELETE FROM dar_dataset WHERE reference_id in (<referenceIds>)")
void deleteDARDatasetRelationByReferenceIds(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by fix: unused

@BindList(value = "referenceIds", onEmpty = EmptyHandling.NULL_STRING)
List<String> referenceIds);

/**
* Returns all dataset_ids that match any of the referenceIds inside the "referenceIds" list
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,25 +151,6 @@ List<Election> findOpenElectionsByReferenceIds(
@BindList(value = "referenceIds", onEmpty = EmptyHandling.NULL_STRING)
List<String> referenceIds);

@SqlQuery(
"""
SELECT distinct *
FROM election e
INNER JOIN
(SELECT reference_id, MAX(create_date) max_date
FROM election e WHERE LOWER(e.election_type) = LOWER(:type)
GROUP BY reference_id) election_view
ON election_view.max_date = e.create_date
AND election_view.reference_id = e.reference_id
WHERE e.reference_id in (<referenceIds>)
AND LOWER(e.election_type) = LOWER(:type)
""")
@UseRowMapper(SimpleElectionMapper.class)
List<Election> findLastElectionsByReferenceIdsAndType(
@BindList(value = "referenceIds", onEmpty = EmptyHandling.NULL_STRING)
List<String> referenceIds,
@Bind("type") String type);

@SqlQuery(
"""
SELECT e.* FROM election e
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package org.broadinstitute.consent.http.models;

import java.sql.Timestamp;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;

public record DarMetricsSummary(
Timestamp updateDate,
String projectTitle,
String darCode,
String nonTechRus,
String referenceId,
Boolean expired) {

public DarMetricsSummary(DataAccessRequest dar) {
this(
dar != null ? dar.getUpdateDate() : null,
dar != null && dar.getData() != null ? dar.getData().getProjectTitle() : null,
dar != null ? dar.getDarCode() : null,
dar != null && dar.getData() != null ? dar.getData().getNonTechRus() : null,
dar != null ? dar.getReferenceId() : null,
computeExpired(dar));
}

private static Boolean computeExpired(DataAccessRequest dar) {
// If the DAR or its submission date is null, we consider it expired for metrics purposes
if (dar == null || dar.getSubmissionDate() == null) {
return true;
}
LocalDateTime oneYearAgo = LocalDateTime.now().minusYears(1);
ZonedDateTime zonedDateTime = oneYearAgo.atZone(ZoneId.systemDefault());
Timestamp lastYear = Timestamp.from(zonedDateTime.toInstant());
return dar.getSubmissionDate().before(lastYear);
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

computeExpired uses a different expiration definition than DataAccessRequest (which sets expired using EXPIRATION_DURATION_MILLIS / 365 days when mapping submission_date). Using LocalDateTime.now().minusYears(1) can diverge around leap years and makes the summary potentially inconsistent with dar.getExpired(). Consider deriving this field from dar.getExpired() (and only special-case null DARs if needed) to keep expiration logic consistent across the API.

Suggested change
// If the DAR or its submission date is null, we consider it expired for metrics purposes
if (dar == null || dar.getSubmissionDate() == null) {
return true;
}
LocalDateTime oneYearAgo = LocalDateTime.now().minusYears(1);
ZonedDateTime zonedDateTime = oneYearAgo.atZone(ZoneId.systemDefault());
Timestamp lastYear = Timestamp.from(zonedDateTime.toInstant());
return dar.getSubmissionDate().before(lastYear);
// If the DAR is null, we consider it expired for metrics purposes.
// Otherwise, rely on the expiration logic defined in DataAccessRequest.
if (dar == null) {
return true;
}
return dar.getExpired();

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In short, we are doing this but with one additional check to ensure that submission date isn't also null.

}
}
Original file line number Diff line number Diff line change
@@ -1,21 +1,10 @@
package org.broadinstitute.consent.http.models;

import java.util.List;
import org.broadinstitute.consent.http.service.MetricsService.DarMetricsSummary;

public class DatasetMetrics {

private Dataset dataset;
private List<DarMetricsSummary> dars;
private List<Election> elections;

public Dataset getDataset() {
return dataset;
}

public void setDataset(Dataset dataset) {
this.dataset = dataset;
}

public List<DarMetricsSummary> getDars() {
return dars;
Expand All @@ -24,12 +13,4 @@ public List<DarMetricsSummary> getDars() {
public void setDars(List<DarMetricsSummary> dars) {
this.dars = dars;
}

public List<Election> getElections() {
return elections;
}

public void setElections(List<Election> elections) {
this.elections = elections;
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
package org.broadinstitute.consent.http.resources;

import com.google.inject.Inject;
import io.dropwizard.auth.Auth;
import jakarta.annotation.security.PermitAll;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.Response;
import java.util.List;
import org.broadinstitute.consent.http.models.DarMetricsSummary;
import org.broadinstitute.consent.http.models.DatasetMetrics;
import org.broadinstitute.consent.http.models.DuosUser;
import org.broadinstitute.consent.http.service.MetricsService;

@Path("/metrics")
@Path("{api : (api/)?}metrics")
public class MetricsResource extends Resource {

private final MetricsService metricsService;
Expand All @@ -19,6 +24,12 @@ public MetricsResource(MetricsService metricsService) {
this.metricsService = metricsService;
}

/**
* @deprecated
* @param datasetId the id of the dataset for which to generate metrics
* @return Response containing DatasetMetrics for the given datasetId
*/
@Deprecated(forRemoval = true, since = "2026-02-23")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be removed when the front end is updated to use the GET /api/metrics/dar-summaries/{datasetId} API

@GET
@Path("/dataset/{datasetId}")
@Produces("application/json")
Expand All @@ -30,4 +41,19 @@ public Response getDatasetMetricsData(@PathParam("datasetId") Integer datasetId)
return createExceptionResponse(e);
}
}

@SuppressWarnings("unused")
@GET
@Path("/dar-summaries/{datasetId}")
@Produces("application/json")
@PermitAll
public Response getDarSummaryData(
@Auth DuosUser user, @PathParam("datasetId") Integer datasetId) {
try {
List<DarMetricsSummary> summaries = metricsService.generateDarSummaries(datasetId);
return Response.ok().entity(summaries).build();
} catch (Exception e) {
return createExceptionResponse(e);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,115 +1,40 @@
package org.broadinstitute.consent.http.service;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.inject.Inject;
import jakarta.ws.rs.NotFoundException;
import java.sql.Timestamp;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Function;
import java.util.stream.Collectors;
import org.broadinstitute.consent.http.db.DarCollectionDAO;
import org.broadinstitute.consent.http.db.DataAccessRequestDAO;
import org.broadinstitute.consent.http.db.DatasetDAO;
import org.broadinstitute.consent.http.db.ElectionDAO;
import org.broadinstitute.consent.http.models.DarCollection;
import org.broadinstitute.consent.http.models.DarMetricsSummary;
import org.broadinstitute.consent.http.models.DataAccessRequest;
import org.broadinstitute.consent.http.models.Dataset;
import org.broadinstitute.consent.http.models.DatasetMetrics;
import org.broadinstitute.consent.http.models.Election;

public class MetricsService {

private final DatasetDAO dataSetDAO;
private final DataAccessRequestDAO darDAO;
private final DarCollectionDAO darCollectionDAO;
private final ElectionDAO electionDAO;

@Inject
public MetricsService(
DatasetDAO dataSetDAO,
DataAccessRequestDAO darDAO,
DarCollectionDAO darCollectionDAO,
ElectionDAO electionDAO) {
public MetricsService(DatasetDAO dataSetDAO, DataAccessRequestDAO darDAO) {
this.dataSetDAO = dataSetDAO;
this.darDAO = darDAO;
this.darCollectionDAO = darCollectionDAO;
this.electionDAO = electionDAO;
}

public static class DarMetricsSummary {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted to the models package.


final Timestamp updateDate;
@JsonProperty final String projectTitle;
@JsonProperty final String darCode;
@JsonProperty final String nonTechRus;
@JsonProperty final String referenceId;

public DarMetricsSummary(DataAccessRequest dar, String darCode) {
if (dar != null && dar.data != null) {
this.updateDate = dar.getUpdateDate();
this.projectTitle = dar.data.getProjectTitle();
this.darCode = darCode;
this.nonTechRus = dar.data.getNonTechRus();
this.referenceId = dar.getReferenceId();
} else {
this.updateDate = null;
this.projectTitle = null;
this.darCode = null;
this.nonTechRus = null;
this.referenceId = null;
}
}
}

public DatasetMetrics generateDatasetMetrics(Integer datasetId) {

DatasetMetrics metrics = new DatasetMetrics();
List<DarMetricsSummary> darMetricsSummaries = generateDarSummaries(datasetId);
metrics.setDars(darMetricsSummaries);
return metrics;
}

// get datasetDTO with properties and data use restrictions
public List<DarMetricsSummary> generateDarSummaries(Integer datasetId) {
Dataset dataset = dataSetDAO.findDatasetById(datasetId);
if (dataset == null) {
throw new NotFoundException("Dataset with specified ID does not exist.");
}

// find dars with the given datasetId in their list of datasetIds, datasetId is a String so it
// can be converted to jsonb in query
// convert all dars into smaller objects that only contain the information needed
List<DataAccessRequest> dars = darDAO.findApprovedDARsByDatasetId(datasetId);
List<Integer> darCollectionIds = dars.stream().map(DataAccessRequest::getCollectionId).toList();
List<DarCollection> darCollections =
darCollectionIds.isEmpty()
? List.of()
: darCollectionDAO.findDARCollectionByCollectionIds(darCollectionIds);
Map<Integer, DarCollection> collectionMap =
darCollections.stream()
.collect(Collectors.toMap(DarCollection::getDarCollectionId, Function.identity()));

List<DarMetricsSummary> darInfo =
dars.stream()
.map(
dar -> {
DarCollection collection = collectionMap.get(dar.getCollectionId());
String darCode = Objects.nonNull(collection) ? collection.getDarCode() : null;
return new DarMetricsSummary(dar, darCode);
})
.collect(Collectors.toList());

// if there are associated dars, find associated access elections so we know how many and which
// dars are approved/denied
List<String> referenceIds =
dars.stream().map(dar -> (dar.referenceId)).collect(Collectors.toList());
if (!referenceIds.isEmpty()) {
List<Election> elections =
electionDAO.findLastElectionsByReferenceIdsAndType(referenceIds, "DataAccess");
metrics.setElections(elections);
} else {
metrics.setElections(Collections.emptyList());
}
metrics.setDataset(dataset);
metrics.setDars(darInfo);
return metrics;
List<DataAccessRequest> dars =
darDAO.findSummaryMetricApprovedDARsByDatasetIdIncludesExpired(datasetId);
return dars.stream().map(DarMetricsSummary::new).toList();
}
}
Loading
Loading