Log entry added, non-closed resources are correctly handled, file header collections are stateless, hard-coded constants deleted relying on the application configuration file

Migration scripts combined

Author: alex-odysseus

src/main/java/org/ohdsi/webapi/statistic/controller/StatisticController.java

@@ -8,6 +8,8 @@
 import org.ohdsi.webapi.statistic.dto.SourceExecutionDto;
 import org.ohdsi.webapi.statistic.dto.SourceExecutionsDto;
 import org.ohdsi.webapi.statistic.service.StatisticService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.stereotype.Controller;
 
@@ -30,6 +32,9 @@
 @Path("/statistic/")
 @ConditionalOnProperty(value = "audit.trail.enabled", havingValue = "true")
 public class StatisticController {
+    
+    private static final Logger log = LoggerFactory.getLogger(StatisticController.class);
+    
     private StatisticService service;
 
     public enum ResponseFormat {
@@ -40,8 +45,16 @@ public enum ResponseFormat {
         add(new String[]{"Date", "Source", "Execution Type"});
     }};
 
+    private static final List<String[]> EXECUTION_STATISTICS_CSV_RESULT_HEADER_WITH_USER_ID = new ArrayList<String[]>() {{
+        add(new String[]{"Date", "Source", "Execution Type", "User ID"});
+    }};
+    
     private static final List<String[]> ACCESS_TRENDS_CSV_RESULT_HEADER = new ArrayList<String[]>() {{
-        add(new String[]{"Date", "Endpoint", "UserID"});
+        add(new String[]{"Date", "Endpoint"});
+    }};
+    
+    private static final List<String[]> ACCESS_TRENDS_CSV_RESULT_HEADER_WITH_USER_ID = new ArrayList<String[]>() {{
+        add(new String[]{"Date", "Endpoint", "User ID"});
     }};
 
     public StatisticController(StatisticService service) {
@@ -93,31 +106,30 @@ public Response accessStatistics(AccessTrendsStatisticsRequest accessTrendsStati
     }
 
     private Response prepareExecutionResultResponse(List<SourceExecutionDto> executions, String filename, boolean showUserInformation) {
-        updateExecutionStatisticsHeader(showUserInformation);
         List<String[]> data = executions.stream()
                 .map(execution -> showUserInformation
-                        ? new String[]{execution.getExecutionDate(), execution.getSourceName(), execution.getExecutionName(), execution.getUserID()}
+                        ? new String[]{execution.getExecutionDate(), execution.getSourceName(), execution.getExecutionName(), execution.getUserId()}
                         : new String[]{execution.getExecutionDate(), execution.getSourceName(), execution.getExecutionName()}
                 )
                 .collect(Collectors.toList());
-        return prepareResponse(data, filename, EXECUTION_STATISTICS_CSV_RESULT_HEADER);
+        return prepareResponse(data, filename, showUserInformation ? EXECUTION_STATISTICS_CSV_RESULT_HEADER_WITH_USER_ID : EXECUTION_STATISTICS_CSV_RESULT_HEADER);
     }
 
     private Response prepareAccessTrendsResponse(List<AccessTrendDto> trends, String filename, boolean showUserInformation) {
-        updateAccessTrendsHeader(showUserInformation);
         List<String[]> data = trends.stream()
                 .map(trend -> showUserInformation
                         ? new String[]{trend.getExecutionDate().toString(), trend.getEndpointName(), trend.getUserID()}
                         : new String[]{trend.getExecutionDate().toString(), trend.getEndpointName()}
                 )
                 .collect(Collectors.toList());
-        return prepareResponse(data, filename, ACCESS_TRENDS_CSV_RESULT_HEADER);
+        return prepareResponse(data, filename, showUserInformation ? ACCESS_TRENDS_CSV_RESULT_HEADER_WITH_USER_ID : ACCESS_TRENDS_CSV_RESULT_HEADER);
     }
 
     private Response prepareResponse(List<String[]> data, String filename, List<String[]> header) {
-        try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
+        try (ByteArrayOutputStream baos = new ByteArrayOutputStream();
             StringWriter sw = new StringWriter();
-            CSVWriter csvWriter = new CSVWriter(sw, ',', CSVWriter.DEFAULT_QUOTE_CHARACTER, CSVWriter.DEFAULT_ESCAPE_CHARACTER);
+            CSVWriter csvWriter = new CSVWriter(sw, ',', CSVWriter.DEFAULT_QUOTE_CHARACTER, CSVWriter.DEFAULT_ESCAPE_CHARACTER)) {
+
             csvWriter.writeAll(header);
             csvWriter.writeAll(data);
             csvWriter.flush();
@@ -129,28 +141,11 @@ private Response prepareResponse(List<String[]> data, String filename, List<Stri
                     .header("Content-Disposition", String.format("attachment; filename=\"%s\"", filename))
                     .build();
         } catch (Exception ex) {
+            log.error("An error occurred while building a response");
             throw new RuntimeException(ex);
         }
     }
 
-    private void updateExecutionStatisticsHeader(boolean showUserInformation) {
-        EXECUTION_STATISTICS_CSV_RESULT_HEADER.clear();
-        if (showUserInformation) {
-            EXECUTION_STATISTICS_CSV_RESULT_HEADER.add(new String[]{"Date", "Source", "Execution Type", "User ID"});
-        } else {
-            EXECUTION_STATISTICS_CSV_RESULT_HEADER.add(new String[]{"Date", "Source", "Execution Type"});
-        }
-    }
-
-    private void updateAccessTrendsHeader(boolean showUserInformation) {
-        ACCESS_TRENDS_CSV_RESULT_HEADER.clear();
-        if (showUserInformation) {
-            ACCESS_TRENDS_CSV_RESULT_HEADER.add(new String[]{"Date", "Endpoint", "UserID"});
-        } else {
-            ACCESS_TRENDS_CSV_RESULT_HEADER.add(new String[]{"Date", "Endpoint"});
-        }
-    }
-
     public static final class ExecutionStatisticsRequest {
         // Format - yyyy-MM-dd
         String startDate;

src/main/java/org/ohdsi/webapi/statistic/dto/SourceExecutionDto.java

@@ -1,19 +1,16 @@
 package org.ohdsi.webapi.statistic.dto;
 
-import java.time.Instant;
-import java.time.LocalDate;
-
 public class SourceExecutionDto {
     private String sourceName;
     private String executionName;
     private String executionDate;
-    private String userID;
+    private String userId;
 
-    public SourceExecutionDto(String sourceName, String executionName, String executionDate, String userID) {
+    public SourceExecutionDto(String sourceName, String executionName, String executionDate, String userId) {
         this.sourceName = sourceName;
         this.executionName = executionName;
         this.executionDate = executionDate;
-        this.userID = userID;
+        this.userId = userId;
     }
 
     public String getSourceName() {
@@ -40,11 +37,11 @@ public void setExecutionDate(String executionDate) {
         this.executionDate = executionDate;
     }
 
-    public String getUserID() {
-        return userID;
+    public String getUserId() {
+        return userId;
     }
 
-    public void setUserID(String userID) {
-        this.userID = userID;
+    public void setUserId(String userId) {
+        this.userId = userId;
     }
 }

src/main/java/org/ohdsi/webapi/statistic/service/StatisticService.java

@@ -37,14 +37,12 @@ public class StatisticService {
     protected final Logger LOG = LoggerFactory.getLogger(getClass());
 
     @Value("${audit.trail.log.file}")
-    // TODO remove value
-    private String absoluteLogFileName = "/tmp/atlas/audit/audit.log";
+    private String absoluteLogFileName;
 
     private String logFileName;
 
     @Value("${audit.trail.log.file.pattern}")
-    // TODO remove value
-    private String absoluteLogFileNamePattern = "/tmp/atlas/audit/audit-%d{yyyy-MM-dd}-%i.log";
+    private String absoluteLogFileNamePattern;
 
     private String logFileNamePattern;
 
@@ -102,6 +100,10 @@ public class StatisticService {
     }
 
     public StatisticService() {
+        if (absoluteLogFileName == null || absoluteLogFileNamePattern == null) {
+            throw new RuntimeException("Application statistics can't operate because of missing configuration values for the audit trail log file or its pattern");
+        }
+        
         logFileName = new File(absoluteLogFileName).getName();
         logFileNamePattern = new File(absoluteLogFileNamePattern).getName();
 

src/main/resources/db/migration/postgresql/V2.14.0.20231206190600__fixing_statistics_enpoint_permission_method.sql

@@ -1,15 +1,15 @@
 INSERT INTO ${ohdsiSchema}.sec_permission(id, value, description) VALUES
-    (nextval('${ohdsiSchema}.sec_permission_id_seq'), 'statistic:executions:get', 'Source execution statistics permission');
+    (nextval('${ohdsiSchema}.sec_permission_id_seq'), 'statistic:executions:post', 'Source execution statistics permission');
 
 INSERT INTO ${ohdsiSchema}.sec_permission(id, value, description) VALUES
     (nextval('${ohdsiSchema}.sec_permission_id_seq'), 'statistic:accesstrends:post', 'Access trends statistics permission');
 
 INSERT INTO ${ohdsiSchema}.sec_role_permission(id, role_id, permission_id)
 SELECT nextval('${ohdsiSchema}.sec_role_permission_sequence'), sr.id, sp.id
 FROM ${ohdsiSchema}.sec_permission SP, ${ohdsiSchema}.sec_role sr
-WHERE sp.value IN ('statistic:executions:get') AND sr.name IN ('admin');
+WHERE sp.value IN ('statistic:executions:post') AND sr.name IN ('admin');
 
 INSERT INTO ${ohdsiSchema}.sec_role_permission(id, role_id, permission_id)
 SELECT nextval('${ohdsiSchema}.sec_role_permission_sequence'), sr.id, sp.id
 FROM ${ohdsiSchema}.sec_permission SP, ${ohdsiSchema}.sec_role sr
-WHERE sp.value IN ('statistic:accesstrends:post') AND sr.name IN ('admin');
+WHERE sp.value IN ('statistic:accesstrends:post') AND sr.name IN ('admin');