From fc243f90c81c9cc8cc1ded8a8160a27ffee08282 Mon Sep 17 00:00:00 2001 From: KFilippopolitis Date: Tue, 15 Sep 2026 14:23:43 +0300 Subject: [PATCH 01/10] refactor: remove dead code and simplify shared helpers Drop unused dataset mapping helpers, Lombok/Gson/Spring Data REST annotations, and blank-line noise. Collapse the Logger level switch into one formatter and reuse a static ObjectMapper in JsonConverters. --- .../AuthenticationEventListener.java | 2 - .../java/hbp/mip/datamodel/DataModelDTO.java | 134 ------------------ .../hbp/mip/experiment/ExperimentDAO.java | 37 ----- .../experiment/ExperimentSpecifications.java | 17 +-- src/main/java/hbp/mip/user/UserDAO.java | 8 -- .../java/hbp/mip/user/UserRepository.java | 2 - src/main/java/hbp/mip/utils/ClaimUtils.java | 2 - src/main/java/hbp/mip/utils/HTTPUtil.java | 10 +- .../java/hbp/mip/utils/JsonConverters.java | 7 +- src/main/java/hbp/mip/utils/Logger.java | 22 +-- 10 files changed, 14 insertions(+), 227 deletions(-) diff --git a/src/main/java/hbp/mip/configurations/AuthenticationEventListener.java b/src/main/java/hbp/mip/configurations/AuthenticationEventListener.java index 7de16e61b..8d56fb2ee 100644 --- a/src/main/java/hbp/mip/configurations/AuthenticationEventListener.java +++ b/src/main/java/hbp/mip/configurations/AuthenticationEventListener.java @@ -1,13 +1,11 @@ package hbp.mip.configurations; import hbp.mip.utils.Logger; -import lombok.extern.slf4j.Slf4j; import org.springframework.context.event.EventListener; import org.springframework.security.authentication.event.AuthenticationSuccessEvent; import org.springframework.security.core.Authentication; import org.springframework.stereotype.Component; -@Slf4j @Component public class AuthenticationEventListener { diff --git a/src/main/java/hbp/mip/datamodel/DataModelDTO.java b/src/main/java/hbp/mip/datamodel/DataModelDTO.java index c989ddcc9..2e4cd2fa5 100644 --- a/src/main/java/hbp/mip/datamodel/DataModelDTO.java +++ b/src/main/java/hbp/mip/datamodel/DataModelDTO.java @@ -5,15 +5,8 @@ import lombok.NoArgsConstructor; import lombok.Setter; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; public record DataModelDTO( String code, @@ -26,133 +19,6 @@ public record DataModelDTO( Map> datasetsVariables ) { - - public DataModelDTO withDatasets(Map> datasetVariablesByDataset, - List availableDatasets, - List datasetEnumerationsOverride) { - // Find the datasets enumeration if it exists in variables or groups or use override - List datasets = datasetEnumerationsOverride != null - ? datasetEnumerationsOverride - : findDatasetEnumerations(this.variables, this.groups); - List filteredDatasets = filterDatasetsByAvailability(datasets, availableDatasets); - Map> datasetsVariables = normaliseDatasetVariables(datasetVariablesByDataset, filteredDatasets); - // Return a new instance of DataModelDTO with datasets and variables set appropriately - return new DataModelDTO(this.code, this.version, this.label, this.longitudinal, this.variables, this.groups, filteredDatasets, datasetsVariables); - } - - private static List findDatasetEnumerations(List variables, List groups) { - // Check the top-level variables list for a dataset code - Optional> datasetEnumerations = findDatasetInVariables(variables); - if (datasetEnumerations.isPresent()) { - return datasetEnumerations.get(); - } - - // If not found in the top level, search recursively in the groups - if (groups == null) { - return null; // Return null if no dataset variable is found - } - - for (DataModelGroupDTO group : groups) { - if (group == null) { - continue; - } - - datasetEnumerations = findDatasetInGroup(group); - if (datasetEnumerations.isPresent()) { - return datasetEnumerations.get(); - } - } - return null; // Return null if no dataset variable is found - } - - private static Optional> findDatasetInVariables(List variables) { - if (variables == null) { - return Optional.empty(); - } - - return variables.stream() - .filter(variable -> "dataset".equals(variable.getCode())) - .map(CommonDataElementDTO::getEnumerations) - .findFirst(); - } - - private static Optional> findDatasetInGroup(DataModelGroupDTO group) { - if (group == null) { - return Optional.empty(); - } - - // First check the variables in the group - Optional> datasetEnumerations = findDatasetInVariables(group.variables()); - if (datasetEnumerations.isPresent()) { - return datasetEnumerations; - } - - // Recursively search in subgroups - List subgroups = group.groups(); - if (subgroups == null) { - return Optional.empty(); - } - - for (DataModelGroupDTO subgroup : subgroups) { - datasetEnumerations = findDatasetInGroup(subgroup); - if (datasetEnumerations.isPresent()) { - return datasetEnumerations; - } - } - return Optional.empty(); - } - - private static Map> normaliseDatasetVariables(Map> datasetVariablesByDataset, - List datasets) { - if (datasetVariablesByDataset == null || datasetVariablesByDataset.isEmpty()) { - return Collections.emptyMap(); - } - - Set allowedDatasets = datasets == null - ? Collections.emptySet() - : datasets.stream().map(EnumerationDTO::code).collect(Collectors.toCollection(HashSet::new)); - - Map> normalisedVariables = new HashMap<>(); - datasetVariablesByDataset.forEach((datasetCode, variables) -> { - if (datasetCode == null) { - return; - } - if (!allowedDatasets.isEmpty() && !allowedDatasets.contains(datasetCode)) { - return; - } - List safeVariables = (variables == null || variables.isEmpty()) - ? Collections.emptyList() - : Collections.unmodifiableList(new ArrayList<>(variables)); - normalisedVariables.put(datasetCode, safeVariables); - }); - - if (normalisedVariables.isEmpty()) { - return Collections.emptyMap(); - } - - return Collections.unmodifiableMap(normalisedVariables); - } - - private static List filterDatasetsByAvailability(List datasets, List availableDatasets) { - if (datasets == null) { - return null; - } - if (availableDatasets == null || availableDatasets.isEmpty()) { - return datasets; - } - - Set allowedCodes = new HashSet<>(availableDatasets); - List filtered = datasets.stream() - .filter(dataset -> allowedCodes.contains(dataset.code())) - .toList(); - - if (filtered.isEmpty()) { - return Collections.emptyList(); - } - - return filtered; - } - public record DataModelGroupDTO( String code, String label, diff --git a/src/main/java/hbp/mip/experiment/ExperimentDAO.java b/src/main/java/hbp/mip/experiment/ExperimentDAO.java index 89455a8cc..39d93ddc5 100644 --- a/src/main/java/hbp/mip/experiment/ExperimentDAO.java +++ b/src/main/java/hbp/mip/experiment/ExperimentDAO.java @@ -1,76 +1,59 @@ package hbp.mip.experiment; -import com.google.gson.annotations.Expose; import hbp.mip.user.UserDAO; import jakarta.persistence.*; -import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; -import java.text.SimpleDateFormat; import java.util.Date; import java.util.UUID; @Entity @Data @NoArgsConstructor -@AllArgsConstructor @Table(name = "`experiment`") public class ExperimentDAO { - @Expose @Id @Column(columnDefinition = "uuid", updatable = false) private UUID uuid; - @Expose @Column(columnDefinition = "TEXT") private String name; - @Expose @ManyToOne @JoinColumn(name = "created_by_username", columnDefinition = "CHARACTER VARYING") private UserDAO createdBy; - @Expose @Column(columnDefinition = "TEXT") @Enumerated(EnumType.STRING) private Status status; - @Expose @Column(columnDefinition = "TEXT") private String result; - @Expose @Column(columnDefinition = "TIMESTAMP WITHOUT TIME ZONE") private Date finished; - @Expose @Column(columnDefinition = "TEXT") private String algorithm; - @Expose @Column(name = "algorithm_id", columnDefinition = "TEXT") private String algorithmId; - @Expose @Column(columnDefinition = "TIMESTAMP WITHOUT TIME ZONE") private Date created = new Date(); - @Expose @Column(columnDefinition = "TIMESTAMP WITHOUT TIME ZONE") private Date updated; - @Expose @Column(columnDefinition = "BOOLEAN") private boolean shared = false; - @Expose @Column(name = "mip_version", columnDefinition = "TEXT") private String mipVersion; // Whether the experiment's result have been viewed by its owner - @Expose @Column(columnDefinition = "BOOLEAN") private boolean viewed = false; @@ -80,24 +63,4 @@ public enum Status { success } - @Override - public String toString() { - String finishedDT = ""; - if (this.finished != null) - finishedDT = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS").format(this.finished); - - return "ExperimentDAO(uuid=" + this.uuid + - ", name=" + this.name + - ", createdBy=" + this.createdBy + - ", status=" + this.status + - ", result=" + this.result + - ", finished=" + finishedDT + - ", algorithm=" + this.algorithm + - ", algorithmId=" + this.algorithmId + - ", created=" + new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS").format(this.created) + - ", updated=" + this.updated + - ", shared=" + this.shared + - ", mipVersion=" + this.mipVersion + - ", viewed=" + this.viewed + ")"; - } } diff --git a/src/main/java/hbp/mip/experiment/ExperimentSpecifications.java b/src/main/java/hbp/mip/experiment/ExperimentSpecifications.java index fecc00a9e..c1c60648e 100644 --- a/src/main/java/hbp/mip/experiment/ExperimentSpecifications.java +++ b/src/main/java/hbp/mip/experiment/ExperimentSpecifications.java @@ -13,32 +13,19 @@ public class ExperimentSpecifications { public static class ExperimentWithName implements Specification { private final String name; - private String regExp; public ExperimentWithName(String name) { this.name = name; - this.regExp = name; } public Predicate toPredicate(@NonNull Root root, @NonNull CriteriaQuery criteriaQuery, @NonNull CriteriaBuilder cb) { if (name == null) { return cb.isTrue(cb.literal(true)); - } else { - regExp = (name.contains("%") ? name : name + "%"); } - return cb.like(cb.lower(root.get("name")), this.regExp.toLowerCase()); - } - - @Override - public @NonNull Specification and(Specification other) { - return Specification.super.and(other); - } - - @Override - public @NonNull Specification or(Specification other) { - return Specification.super.or(other); + String regExp = (name.contains("%") ? name : name + "%"); + return cb.like(cb.lower(root.get("name")), regExp.toLowerCase()); } } diff --git a/src/main/java/hbp/mip/user/UserDAO.java b/src/main/java/hbp/mip/user/UserDAO.java index ea29599ee..7cddd9c15 100644 --- a/src/main/java/hbp/mip/user/UserDAO.java +++ b/src/main/java/hbp/mip/user/UserDAO.java @@ -1,36 +1,28 @@ package hbp.mip.user; -import com.google.gson.annotations.Expose; import jakarta.persistence.Column; import jakarta.persistence.Entity; import jakarta.persistence.Id; import jakarta.persistence.Table; -import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; @Entity @Data @NoArgsConstructor -@AllArgsConstructor @Table(name = "`user`") public class UserDAO { @Id - @Expose private String username; - @Expose @Column(name = "subject_id") private String subjectId; - @Expose private String fullname; - @Expose private String email; - @Expose @Column(name = "agree_nda") private Boolean agreeNDA; diff --git a/src/main/java/hbp/mip/user/UserRepository.java b/src/main/java/hbp/mip/user/UserRepository.java index d83ad87e4..9fadd5215 100644 --- a/src/main/java/hbp/mip/user/UserRepository.java +++ b/src/main/java/hbp/mip/user/UserRepository.java @@ -1,9 +1,7 @@ package hbp.mip.user; import org.springframework.data.repository.CrudRepository; -import org.springframework.data.rest.core.annotation.RestResource; -@RestResource(exported = false) public interface UserRepository extends CrudRepository { UserDAO findByUsername(String username); } diff --git a/src/main/java/hbp/mip/utils/ClaimUtils.java b/src/main/java/hbp/mip/utils/ClaimUtils.java index e872aea8b..6b013c47d 100644 --- a/src/main/java/hbp/mip/utils/ClaimUtils.java +++ b/src/main/java/hbp/mip/utils/ClaimUtils.java @@ -47,7 +47,6 @@ private static ArrayList getAuthorityRoles(Authentication authentication public void validateAccessRightsOnDatasets(Authentication authentication, List experimentDatasets, Logger logger) { - if (experimentDatasets == null || experimentDatasets.isEmpty()) { String errorMessage = "At least one dataset must be provided."; logger.warn(errorMessage); @@ -75,7 +74,6 @@ public boolean validateAccessRightsOnALLExperiments(Authentication authenticatio public List getAuthorizedDataModels(Logger logger, Authentication authentication, List allDataModels) { - ArrayList authorities = getAuthorityRoles(authentication); if (hasRoleAccess(authorities, allDatasetsAllowedClaim, logger)) { diff --git a/src/main/java/hbp/mip/utils/HTTPUtil.java b/src/main/java/hbp/mip/utils/HTTPUtil.java index 45f0d0d09..5306edd33 100644 --- a/src/main/java/hbp/mip/utils/HTTPUtil.java +++ b/src/main/java/hbp/mip/utils/HTTPUtil.java @@ -16,23 +16,19 @@ private HTTPUtil() { } public static void sendGet(String url, StringBuilder resp) throws IOException { - sendHTTP(url, "", resp, "GET", null); + sendHTTP(url, "", resp, "GET"); } public static int sendPost(String url, String query, StringBuilder resp) throws IOException { - return sendHTTP(url, query, resp, "POST", null); + return sendHTTP(url, query, resp, "POST"); } - private static int sendHTTP(String url, String query, StringBuilder resp, String httpVerb, String authorization) + private static int sendHTTP(String url, String query, StringBuilder resp, String httpVerb) throws IOException { URL obj = new URL(url); HttpURLConnection con = (HttpURLConnection) obj.openConnection(); - if (authorization != null) { - con.setRequestProperty("Authorization", authorization); - } - if (!"GET".equals(httpVerb)) { con.setRequestMethod(httpVerb); if (query != null && !query.isEmpty()) { diff --git a/src/main/java/hbp/mip/utils/JsonConverters.java b/src/main/java/hbp/mip/utils/JsonConverters.java index fb8eaf5ff..698ff512b 100644 --- a/src/main/java/hbp/mip/utils/JsonConverters.java +++ b/src/main/java/hbp/mip/utils/JsonConverters.java @@ -8,12 +8,11 @@ public class JsonConverters { private static final Gson gson = new Gson(); + private static final ObjectMapper objectMapper = new ObjectMapper(); - public static String convertObjectToJsonString(Object object) { - ObjectMapper mapper = new ObjectMapper(); - //Converting the Object to JSONString + public static String convertObjectToJsonString(Object object) { try { - return mapper.writeValueAsString(object); + return objectMapper.writeValueAsString(object); } catch (JsonProcessingException e) { return e.getMessage(); } diff --git a/src/main/java/hbp/mip/utils/Logger.java b/src/main/java/hbp/mip/utils/Logger.java index 917f40351..6227c017d 100644 --- a/src/main/java/hbp/mip/utils/Logger.java +++ b/src/main/java/hbp/mip/utils/Logger.java @@ -1,9 +1,7 @@ package hbp.mip.utils; -import org.apache.logging.log4j.LoggingException; import org.slf4j.LoggerFactory; - public class Logger { private static final org.slf4j.Logger logger = LoggerFactory.getLogger(Logger.class); @@ -15,31 +13,23 @@ public Logger(String username, String endpoint) { this.endpoint = endpoint; } - private void logUserAction(String message, String logLevel){ - String logMessage = "User -> " + username + " , Endpoint -> " + endpoint + " , Info -> " + message; - - switch (logLevel) { - case "ERROR" -> logger.error(logMessage); - case "WARNING" -> logger.warn(logMessage); - case "INFO" -> logger.info(logMessage); - case "DEBUG" -> logger.debug(logMessage); - default -> throw new LoggingException("Not supported loglevel: " + logLevel); - } + private String formatMessage(String message) { + return "User -> " + username + " , Endpoint -> " + endpoint + " , Info -> " + message; } public void error(String message) { - logUserAction(message, "ERROR"); + logger.error(formatMessage(message)); } public void warn(String message) { - logUserAction(message, "WARNING"); + logger.warn(formatMessage(message)); } public void info(String message) { - logUserAction(message, "INFO"); + logger.info(formatMessage(message)); } public void debug(String message) { - logUserAction(message, "DEBUG"); + logger.debug(formatMessage(message)); } } From bd32a02d921011463086706098e0a085464f5765 Mon Sep 17 00:00:00 2001 From: KFilippopolitis Date: Tue, 15 Sep 2026 14:23:59 +0300 Subject: [PATCH 02/10] refactor(algorithm): replace algorithms API with analysis and specifications endpoints - Delete AlgorithmService/AlgorithmsAPI, the disabled-algorithms resource, and the now-unused resource/stream helpers. - Add AnalysisAPI/AnalysisService for execution and SpecificationsAPI/SpecificationsService for Exaflow inputdata, preprocessing and algorithm specifications. - Route persisted and transient experiments through AnalysisRequestDTO, read stored analysis JSON directly, and drop the legacy algorithm payload adapter and custom deserializer. - Stamp experiments with the configured MIP version instead of the request body. - Replace the reflection-based non-editable-field check with explicit null checks. - Cover the new request/response shapes with focused tests. --- config/disabledAlgorithms.json | 4 - .../mip/algorithm/AlgorithmRequestDTO.java | 32 --- .../hbp/mip/algorithm/AlgorithmService.java | 128 ------------ .../algorithm/AlgorithmSpecificationDTO.java | 71 +------ .../java/hbp/mip/algorithm/AlgorithmsAPI.java | 36 ---- .../hbp/mip/algorithm/AlgorithmsSpecs.java | 15 -- .../java/hbp/mip/algorithm/AnalysisAPI.java | 51 +++++ .../AnalysisInputDataSpecificationDTO.java | 9 + .../hbp/mip/algorithm/AnalysisRequestDTO.java | 42 ++++ .../hbp/mip/algorithm/AnalysisService.java | 85 ++++++++ .../algorithm/InputDataSpecificationDTO.java | 13 ++ .../algorithm/ParameterSpecificationDTO.java | 24 +++ .../PreprocessingOutputSpecificationDTO.java | 6 + .../PreprocessingStepSpecificationDTO.java | 18 ++ .../hbp/mip/algorithm/SpecificationsAPI.java | 59 ++++++ .../mip/algorithm/SpecificationsService.java | 58 ++++++ .../hbp/mip/experiment/ExperimentDTO.java | 64 +++--- .../experiment/ExperimentExecutionDTO.java | 18 +- .../mip/experiment/ExperimentRepository.java | 10 +- .../hbp/mip/experiment/ExperimentService.java | 196 +++++++----------- .../hbp/mip/utils/CustomResourceLoader.java | 21 -- .../hbp/mip/utils/InputStreamConverter.java | 22 -- .../AlgorithmSpecificationDTOTest.java | 165 +++++++-------- ...AnalysisRequestDTODeserializationTest.java | 50 +++++ .../AnalysisServiceRequestIdTest.java | 38 ++++ .../ExperimentServiceMipVersionTest.java | 102 +++++++++ 26 files changed, 748 insertions(+), 589 deletions(-) delete mode 100644 config/disabledAlgorithms.json delete mode 100644 src/main/java/hbp/mip/algorithm/AlgorithmRequestDTO.java delete mode 100644 src/main/java/hbp/mip/algorithm/AlgorithmService.java delete mode 100644 src/main/java/hbp/mip/algorithm/AlgorithmsAPI.java delete mode 100644 src/main/java/hbp/mip/algorithm/AlgorithmsSpecs.java create mode 100644 src/main/java/hbp/mip/algorithm/AnalysisAPI.java create mode 100644 src/main/java/hbp/mip/algorithm/AnalysisInputDataSpecificationDTO.java create mode 100644 src/main/java/hbp/mip/algorithm/AnalysisRequestDTO.java create mode 100644 src/main/java/hbp/mip/algorithm/AnalysisService.java create mode 100644 src/main/java/hbp/mip/algorithm/InputDataSpecificationDTO.java create mode 100644 src/main/java/hbp/mip/algorithm/ParameterSpecificationDTO.java create mode 100644 src/main/java/hbp/mip/algorithm/PreprocessingOutputSpecificationDTO.java create mode 100644 src/main/java/hbp/mip/algorithm/PreprocessingStepSpecificationDTO.java create mode 100644 src/main/java/hbp/mip/algorithm/SpecificationsAPI.java create mode 100644 src/main/java/hbp/mip/algorithm/SpecificationsService.java delete mode 100644 src/main/java/hbp/mip/utils/CustomResourceLoader.java delete mode 100644 src/main/java/hbp/mip/utils/InputStreamConverter.java create mode 100644 src/test/java/hbp/mip/algorithm/AnalysisRequestDTODeserializationTest.java create mode 100644 src/test/java/hbp/mip/algorithm/AnalysisServiceRequestIdTest.java create mode 100644 src/test/java/hbp/mip/experiment/ExperimentServiceMipVersionTest.java diff --git a/config/disabledAlgorithms.json b/config/disabledAlgorithms.json deleted file mode 100644 index 6fc4a5d46..000000000 --- a/config/disabledAlgorithms.json +++ /dev/null @@ -1,4 +0,0 @@ -[ - "THREE_C", - "KAPLAN_MEIER" -] \ No newline at end of file diff --git a/src/main/java/hbp/mip/algorithm/AlgorithmRequestDTO.java b/src/main/java/hbp/mip/algorithm/AlgorithmRequestDTO.java deleted file mode 100644 index 0a61a5df4..000000000 --- a/src/main/java/hbp/mip/algorithm/AlgorithmRequestDTO.java +++ /dev/null @@ -1,32 +0,0 @@ -package hbp.mip.algorithm; - -import hbp.mip.experiment.ExperimentExecutionDTO; -import lombok.extern.slf4j.Slf4j; - -import java.util.*; - -@Slf4j -public record AlgorithmRequestDTO( - String request_id, - InputDataRequestDTO inputdata, - Map parameters, - Map preprocessing -) { - - public static AlgorithmRequestDTO create(UUID experimentUUID,ExperimentExecutionDTO.AlgorithmExecutionDTO algorithmExecutionDTO){ - return new AlgorithmRequestDTO( - experimentUUID.toString(), - algorithmExecutionDTO.inputdata(), - algorithmExecutionDTO.parameters(), - algorithmExecutionDTO.preprocessing() - ); - } - - public record InputDataRequestDTO(String data_model, List datasets, List x, List y, - List validation_datasets, - FilterRequestDTO filters) { - } - - public record FilterRequestDTO(String condition, List rules) { - } -} diff --git a/src/main/java/hbp/mip/algorithm/AlgorithmService.java b/src/main/java/hbp/mip/algorithm/AlgorithmService.java deleted file mode 100644 index c4d0c17f9..000000000 --- a/src/main/java/hbp/mip/algorithm/AlgorithmService.java +++ /dev/null @@ -1,128 +0,0 @@ -package hbp.mip.algorithm; - -import com.google.gson.Gson; -import com.google.gson.reflect.TypeToken; -import hbp.mip.utils.CustomResourceLoader; -import hbp.mip.utils.HTTPUtil; -import hbp.mip.utils.Logger; -import hbp.mip.utils.Exceptions.InternalServerError; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.core.io.Resource; -import org.springframework.scheduling.annotation.Async; -import org.springframework.scheduling.annotation.EnableAsync; -import org.springframework.scheduling.annotation.EnableScheduling; -import org.springframework.scheduling.annotation.Scheduled; -import org.springframework.stereotype.Service; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -import static hbp.mip.utils.InputStreamConverter.convertInputStreamToString; - -@Service -@EnableScheduling -public class AlgorithmService { - - private static final Gson gson = new Gson(); - - private final AlgorithmsSpecs algorithmsSpecs; - private final CustomResourceLoader resourceLoader; - - @Value("${files.disabledAlgorithms_json}") - private String disabledAlgorithmsFilePath; - - @Value("${services.exaflow.algorithmsUrl}") - private String exaflowAlgorithmsUrl; - - public AlgorithmService(AlgorithmsSpecs algorithmsSpecs, CustomResourceLoader resourceLoader) { - this.algorithmsSpecs = algorithmsSpecs; - this.resourceLoader = resourceLoader; - } - - public List getAlgorithms(Logger logger) { - - List exaflowAlgorithms = getExaflowAlgorithms(logger); - - List disabledAlgorithms = getDisabledAlgorithms(logger); - logger.debug("Disabled algorithms: " + disabledAlgorithms); - - // Remove any disabled algorithm - ArrayList enabledAlgorithms = new ArrayList<>(); - for (AlgorithmSpecificationDTO algorithm : exaflowAlgorithms) { - if (!disabledAlgorithms.contains(algorithm.name())) { - enabledAlgorithms.add(algorithm); - } - } - - logger.debug("Disabled " + (exaflowAlgorithms.size() - enabledAlgorithms.size()) + " algorithms."); - return enabledAlgorithms; - } - - /** - * This method gets all the available exaflow algorithms and removes the - * disabled. - * - * @return a list of ExaflowAlgorithmSpecificationDTO or null if something - * fails - */ - private List getExaflowAlgorithms(Logger logger) { - List algorithms; - StringBuilder response = new StringBuilder(); - try { - HTTPUtil.sendGet(exaflowAlgorithmsUrl, response); - algorithms = gson.fromJson( - response.toString(), - new TypeToken>() { - }.getType()); - } catch (Exception e) { - logger.error("Could not fetch exaflow algorithms: " + e.getMessage()); - throw new InternalServerError("Could not fetch exaflow algorithms."); - } - - if (algorithms == null || algorithms.isEmpty()) { - String errorMessage = "Exaflow algorithms response was empty."; - logger.error(errorMessage); - throw new InternalServerError(errorMessage); - } - - logger.debug("Fetched " + algorithms.size() + " exaflow algorithms."); - algorithmsSpecs.setAlgorithms(algorithms); - return algorithms; - } - - @EnableAsync - public static class AlgorithmAggregator { - - private final AlgorithmService algorithmService; - - public AlgorithmAggregator(AlgorithmService algorithmService) { - this.algorithmService = algorithmService; - } - - @Async - @Scheduled(fixedDelayString = "${services.algorithmsUpdateInterval}000") - public void scheduleFixedRateTaskAsync() { - algorithmService.getExaflowAlgorithms(new Logger("AlgorithmAggregator", "(GET) /algorithms")); - } - } - - /** - * Fetches the disabled algorithms from a .json file - * - * @return a list with their names - */ - private List getDisabledAlgorithms(Logger logger) { - Resource resource = resourceLoader.getResource(disabledAlgorithmsFilePath); - try { - return gson.fromJson( - convertInputStreamToString(resource.getInputStream()), - new TypeToken>() { - }.getType()); - } catch (IOException e) { - logger.error("Could not load the disabled algorithms. Exception: " + e.getMessage()); - return Collections.emptyList(); - } - } -} diff --git a/src/main/java/hbp/mip/algorithm/AlgorithmSpecificationDTO.java b/src/main/java/hbp/mip/algorithm/AlgorithmSpecificationDTO.java index 445783520..c53e1e192 100644 --- a/src/main/java/hbp/mip/algorithm/AlgorithmSpecificationDTO.java +++ b/src/main/java/hbp/mip/algorithm/AlgorithmSpecificationDTO.java @@ -1,7 +1,5 @@ package hbp.mip.algorithm; -import com.google.gson.annotations.SerializedName; - import java.util.Collections; import java.util.List; import java.util.Map; @@ -14,71 +12,18 @@ public record AlgorithmSpecificationDTO( String documentation, String type, List flags, - ExaflowAlgorithmInputdataSpecificationDTO inputdata, - Map parameters, - List preprocessing) { + InputDataSpecificationDTO y, + InputDataSpecificationDTO x, + boolean requires_validation_datasets, + Map parameters, + List required_preprocessing) { @Override - public Map parameters() { + public Map parameters() { return Objects.requireNonNullElse(parameters, Collections.emptyMap()); } @Override - public List preprocessing() { - return Objects.requireNonNullElse(preprocessing, Collections.emptyList()); - } - - public record AlgorithmParameterSpecificationDTO( - String label, - String desc, - List types, - String required, - String multiple, - String min, - String max, - @SerializedName("default") String default_value, - AlgorithmEnumDTO enums, - AlgorithmEnumDTO dict_keys_enums, - AlgorithmEnumDTO dict_values_enums, - String dict_values_type - - ) { - public record AlgorithmEnumDTO( - String type, - List source) { - } - } - - public record ExaflowAlgorithmInputdataSpecificationDTO( - AlgorithmInputDataDetailSpecificationDTO x, - AlgorithmInputDataDetailSpecificationDTO y, - AlgorithmInputDataDetailSpecificationDTO data_model, - AlgorithmInputDataDetailSpecificationDTO datasets, - AlgorithmInputDataDetailSpecificationDTO validation_datasets, - AlgorithmInputDataDetailSpecificationDTO filter) { - } - - public record AlgorithmInputDataDetailSpecificationDTO( - String label, - String desc, - List types, - List stattypes, - String required, - Integer min_count, - Integer max_count - - ) { - } - - public record TransformerSpecificationDTO( - String name, - String label, - String desc, - String documentation, - Integer order, - Map parameters) { - @Override - public Map parameters() { - return Objects.requireNonNullElse(parameters, Collections.emptyMap()); - } + public List required_preprocessing() { + return Objects.requireNonNullElse(required_preprocessing, Collections.emptyList()); } } diff --git a/src/main/java/hbp/mip/algorithm/AlgorithmsAPI.java b/src/main/java/hbp/mip/algorithm/AlgorithmsAPI.java deleted file mode 100644 index 0570a0257..000000000 --- a/src/main/java/hbp/mip/algorithm/AlgorithmsAPI.java +++ /dev/null @@ -1,36 +0,0 @@ -package hbp.mip.algorithm; - -import hbp.mip.user.ActiveUserService; -import hbp.mip.utils.Logger; -import org.springframework.http.ResponseEntity; -import org.springframework.security.core.Authentication; -import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RestController; - -import java.util.List; - -import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; - -@RestController -@RequestMapping(value = "/algorithms", produces = {APPLICATION_JSON_VALUE}) -public class AlgorithmsAPI { - - private final AlgorithmService algorithmService; - - private final ActiveUserService activeUserService; - - public AlgorithmsAPI(ActiveUserService activeUserService, AlgorithmService algorithmService) { - this.activeUserService = activeUserService; - this.algorithmService = algorithmService; - } - - @GetMapping - public ResponseEntity> getAlgorithms(Authentication authentication) { - Logger logger = new Logger(activeUserService.getActiveUser(authentication).username(), "(GET) /algorithms"); - logger.info("Request for algorithms."); - List algorithms = algorithmService.getAlgorithms(logger); - logger.info("Algorithms returned: " + algorithms.size()); - return ResponseEntity.ok(algorithms); - } -} diff --git a/src/main/java/hbp/mip/algorithm/AlgorithmsSpecs.java b/src/main/java/hbp/mip/algorithm/AlgorithmsSpecs.java deleted file mode 100644 index eb1447d3d..000000000 --- a/src/main/java/hbp/mip/algorithm/AlgorithmsSpecs.java +++ /dev/null @@ -1,15 +0,0 @@ -package hbp.mip.algorithm; - -import lombok.Getter; -import lombok.Setter; -import org.springframework.stereotype.Component; - -import java.util.ArrayList; -import java.util.List; - -@Getter -@Setter -@Component -public class AlgorithmsSpecs { - private List algorithms = new ArrayList<>(); -} diff --git a/src/main/java/hbp/mip/algorithm/AnalysisAPI.java b/src/main/java/hbp/mip/algorithm/AnalysisAPI.java new file mode 100644 index 000000000..210fe3866 --- /dev/null +++ b/src/main/java/hbp/mip/algorithm/AnalysisAPI.java @@ -0,0 +1,51 @@ +package hbp.mip.algorithm; + +import hbp.mip.user.ActiveUserService; +import hbp.mip.utils.ClaimUtils; +import hbp.mip.utils.Logger; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.ResponseEntity; +import org.springframework.security.core.Authentication; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; + +@RestController +@RequestMapping(value = "/analysis", produces = { APPLICATION_JSON_VALUE }) +public class AnalysisAPI { + + private final AnalysisService analysisService; + private final ActiveUserService activeUserService; + private final ClaimUtils claimUtils; + + @Value("${authentication.enabled}") + private boolean authenticationIsEnabled; + + public AnalysisAPI( + ActiveUserService activeUserService, + AnalysisService analysisService, + ClaimUtils claimUtils) { + this.activeUserService = activeUserService; + this.analysisService = analysisService; + this.claimUtils = claimUtils; + } + + @PostMapping(consumes = APPLICATION_JSON_VALUE) + public ResponseEntity runAnalysis( + Authentication authentication, + @RequestBody AnalysisRequestDTO analysisRequest) { + Logger logger = new Logger(activeUserService.getActiveUser(authentication).username(), "(POST) /analysis"); + logger.info("Request for analysis execution."); + if (authenticationIsEnabled && analysisRequest.inputdata() != null) { + claimUtils.validateAccessRightsOnDatasets( + authentication, + analysisRequest.inputdata().datasets(), + logger); + } + AnalysisService.AnalysisResultDTO result = analysisService.runAnalysis(analysisRequest, logger); + return ResponseEntity.status(result.code()).body(result.body()); + } +} diff --git a/src/main/java/hbp/mip/algorithm/AnalysisInputDataSpecificationDTO.java b/src/main/java/hbp/mip/algorithm/AnalysisInputDataSpecificationDTO.java new file mode 100644 index 000000000..ebd080d99 --- /dev/null +++ b/src/main/java/hbp/mip/algorithm/AnalysisInputDataSpecificationDTO.java @@ -0,0 +1,9 @@ +package hbp.mip.algorithm; + +public record AnalysisInputDataSpecificationDTO( + InputDataSpecificationDTO data_model, + InputDataSpecificationDTO datasets, + InputDataSpecificationDTO filters, + InputDataSpecificationDTO variables, + InputDataSpecificationDTO validation_datasets) { +} diff --git a/src/main/java/hbp/mip/algorithm/AnalysisRequestDTO.java b/src/main/java/hbp/mip/algorithm/AnalysisRequestDTO.java new file mode 100644 index 000000000..1c157d8b4 --- /dev/null +++ b/src/main/java/hbp/mip/algorithm/AnalysisRequestDTO.java @@ -0,0 +1,42 @@ +package hbp.mip.algorithm; + +import java.util.List; +import java.util.Map; +import java.util.UUID; + +public record AnalysisRequestDTO( + String request_id, + AnalysisInputDataDTO inputdata, + List preprocessing, + AnalysisAlgorithmDTO algorithm, + Map flags) { + + public static AnalysisRequestDTO withRequestId(UUID experimentUuid, AnalysisRequestDTO analysis) { + return new AnalysisRequestDTO( + experimentUuid.toString(), + analysis.inputdata(), + analysis.preprocessing(), + analysis.algorithm(), + analysis.flags()); + } + + public record AnalysisInputDataDTO( + String data_model, + List datasets, + List validation_datasets, + Map filters, + List variables) { + } + + public record AnalysisPreprocessingStepDTO( + String name, + Map parameters) { + } + + public record AnalysisAlgorithmDTO( + String name, + List x, + List y, + Map parameters) { + } +} diff --git a/src/main/java/hbp/mip/algorithm/AnalysisService.java b/src/main/java/hbp/mip/algorithm/AnalysisService.java new file mode 100644 index 000000000..dc6c75bc5 --- /dev/null +++ b/src/main/java/hbp/mip/algorithm/AnalysisService.java @@ -0,0 +1,85 @@ +package hbp.mip.algorithm; + +import hbp.mip.utils.Exceptions.BadRequestException; +import hbp.mip.utils.Exceptions.InternalServerError; +import hbp.mip.utils.HTTPUtil; +import hbp.mip.utils.JsonConverters; +import hbp.mip.utils.Logger; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Service; + +import java.io.IOException; +import java.util.Map; +import java.util.UUID; + +@Service +public class AnalysisService { + + private static final String GENERIC_ERROR_MESSAGE = + "Something went wrong. Please inform the system administrator or try again later."; + + private final String analysisUrl; + + public AnalysisService(@Value("${services.exaflow.analysisUrl}") String analysisUrl) { + this.analysisUrl = analysisUrl; + } + + public AnalysisResultDTO runAnalysis(UUID requestId, AnalysisRequestDTO analysisRequest, Logger logger) { + AnalysisRequestDTO requestWithId = AnalysisRequestDTO.withRequestId(requestId, analysisRequest); + String requestBody = JsonConverters.convertObjectToJsonString(requestWithId); + + logger.debug("Exaflow analysis request, endpoint: " + analysisUrl); + logger.debug("Exaflow analysis request, body: " + requestBody); + + int responseCode; + var responseBody = new StringBuilder(); + try { + responseCode = HTTPUtil.sendPost(analysisUrl, requestBody, responseBody); + } catch (IOException e) { + logger.error("Could not run the exaflow analysis: " + e.getMessage()); + throw new InternalServerError(e.getMessage()); + } + + Object result = convertResponseToAnalysisResult(logger, responseCode, responseBody); + return new AnalysisResultDTO(responseCode, result); + } + + public AnalysisResultDTO runAnalysis(AnalysisRequestDTO analysisRequest, Logger logger) { + UUID requestId = analysisRequest.request_id() != null + ? parseRequestId(analysisRequest.request_id(), logger) + : UUID.randomUUID(); + return runAnalysis(requestId, analysisRequest, logger); + } + + private static UUID parseRequestId(String requestId, Logger logger) { + try { + return UUID.fromString(requestId); + } catch (IllegalArgumentException e) { + String errorMessage = "Invalid request_id: must be a valid UUID."; + logger.warn(errorMessage); + throw new BadRequestException(errorMessage); + } + } + + private static Object convertResponseToAnalysisResult(Logger logger, int code, StringBuilder responseBody) { + return switch (code) { + case 200 -> JsonConverters.convertJsonStringToObject(responseBody.toString(), Object.class); + case 400, 460, 461, 462, 512, 513 -> convertExaflowResponseToAnalysisResult(responseBody.toString()); + case 500 -> convertExaflowResponseToAnalysisResult(GENERIC_ERROR_MESSAGE); + default -> handleUnexpectedResponseCode(logger, code); + }; + } + + private static Map convertExaflowResponseToAnalysisResult(String resultBody) { + return Map.of("data", resultBody, "type", "text/plain+error"); + } + + private static Object handleUnexpectedResponseCode(Logger logger, int code) { + String errorMessage = "Exaflow execution responded with an unexpected status code: " + code; + logger.error(errorMessage); + throw new InternalServerError(errorMessage); + } + + public record AnalysisResultDTO(int code, Object body) { + } +} diff --git a/src/main/java/hbp/mip/algorithm/InputDataSpecificationDTO.java b/src/main/java/hbp/mip/algorithm/InputDataSpecificationDTO.java new file mode 100644 index 000000000..9ac945ba8 --- /dev/null +++ b/src/main/java/hbp/mip/algorithm/InputDataSpecificationDTO.java @@ -0,0 +1,13 @@ +package hbp.mip.algorithm; + +import java.util.List; + +public record InputDataSpecificationDTO( + String label, + String desc, + List types, + List stattypes, + Boolean required, + Integer min_count, + Integer max_count) { +} diff --git a/src/main/java/hbp/mip/algorithm/ParameterSpecificationDTO.java b/src/main/java/hbp/mip/algorithm/ParameterSpecificationDTO.java new file mode 100644 index 000000000..6d7a71d0c --- /dev/null +++ b/src/main/java/hbp/mip/algorithm/ParameterSpecificationDTO.java @@ -0,0 +1,24 @@ +package hbp.mip.algorithm; + +import com.google.gson.annotations.SerializedName; + +import java.util.List; + +public record ParameterSpecificationDTO( + String label, + String desc, + List types, + Boolean required, + Boolean multiple, + @SerializedName("default") Object default_value, + ParameterEnumSpecificationDTO enums, + ParameterEnumSpecificationDTO dict_keys_enums, + ParameterEnumSpecificationDTO dict_values_enums, + String dict_values_type, + Double min, + Double max) { + public record ParameterEnumSpecificationDTO( + String type, + List source) { + } +} diff --git a/src/main/java/hbp/mip/algorithm/PreprocessingOutputSpecificationDTO.java b/src/main/java/hbp/mip/algorithm/PreprocessingOutputSpecificationDTO.java new file mode 100644 index 000000000..ba133c733 --- /dev/null +++ b/src/main/java/hbp/mip/algorithm/PreprocessingOutputSpecificationDTO.java @@ -0,0 +1,6 @@ +package hbp.mip.algorithm; + +public record PreprocessingOutputSpecificationDTO( + String type, + String code_parameter) { +} diff --git a/src/main/java/hbp/mip/algorithm/PreprocessingStepSpecificationDTO.java b/src/main/java/hbp/mip/algorithm/PreprocessingStepSpecificationDTO.java new file mode 100644 index 000000000..77221c9c7 --- /dev/null +++ b/src/main/java/hbp/mip/algorithm/PreprocessingStepSpecificationDTO.java @@ -0,0 +1,18 @@ +package hbp.mip.algorithm; + +import java.util.Collections; +import java.util.Map; +import java.util.Objects; + +public record PreprocessingStepSpecificationDTO( + String name, + String desc, + String documentation, + String label, + Map parameters, + PreprocessingOutputSpecificationDTO output) { + @Override + public Map parameters() { + return Objects.requireNonNullElse(parameters, Collections.emptyMap()); + } +} diff --git a/src/main/java/hbp/mip/algorithm/SpecificationsAPI.java b/src/main/java/hbp/mip/algorithm/SpecificationsAPI.java new file mode 100644 index 000000000..d7df8b661 --- /dev/null +++ b/src/main/java/hbp/mip/algorithm/SpecificationsAPI.java @@ -0,0 +1,59 @@ +package hbp.mip.algorithm; + +import hbp.mip.user.ActiveUserService; +import hbp.mip.utils.Logger; +import org.springframework.http.ResponseEntity; +import org.springframework.security.core.Authentication; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +import java.util.List; + +import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; + +@RestController +@RequestMapping(value = "/specifications", produces = { APPLICATION_JSON_VALUE }) +public class SpecificationsAPI { + + private final SpecificationsService specificationsService; + private final ActiveUserService activeUserService; + + public SpecificationsAPI(ActiveUserService activeUserService, SpecificationsService specificationsService) { + this.activeUserService = activeUserService; + this.specificationsService = specificationsService; + } + + @GetMapping("/inputdata") + public ResponseEntity getInputdataSpecification(Authentication authentication) { + Logger logger = new Logger( + activeUserService.getActiveUser(authentication).username(), + "(GET) /specifications/inputdata"); + logger.info("Request for inputdata specification."); + AnalysisInputDataSpecificationDTO specification = specificationsService.getInputdataSpecification(logger); + return ResponseEntity.ok(specification); + } + + @GetMapping("/preprocessing") + public ResponseEntity> getPreprocessingSpecifications( + Authentication authentication) { + Logger logger = new Logger( + activeUserService.getActiveUser(authentication).username(), + "(GET) /specifications/preprocessing"); + logger.info("Request for preprocessing specifications."); + List specifications = + specificationsService.getPreprocessingSpecifications(logger); + return ResponseEntity.ok(specifications); + } + + @GetMapping("/algorithms") + public ResponseEntity> getAlgorithmSpecifications(Authentication authentication) { + Logger logger = new Logger( + activeUserService.getActiveUser(authentication).username(), + "(GET) /specifications/algorithms"); + logger.info("Request for algorithm specifications."); + List specifications = specificationsService.getAlgorithmSpecifications(logger); + logger.info("Algorithm specifications returned: " + specifications.size()); + return ResponseEntity.ok(specifications); + } +} diff --git a/src/main/java/hbp/mip/algorithm/SpecificationsService.java b/src/main/java/hbp/mip/algorithm/SpecificationsService.java new file mode 100644 index 000000000..ff469f35a --- /dev/null +++ b/src/main/java/hbp/mip/algorithm/SpecificationsService.java @@ -0,0 +1,58 @@ +package hbp.mip.algorithm; + +import com.google.gson.reflect.TypeToken; +import hbp.mip.utils.Exceptions.InternalServerError; +import hbp.mip.utils.HTTPUtil; +import hbp.mip.utils.JsonConverters; +import hbp.mip.utils.Logger; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Service; + +import java.lang.reflect.Type; +import java.util.List; + +@Service +public class SpecificationsService { + + private static final Type PREPROCESSING_SPECIFICATIONS_TYPE = + new TypeToken>() { + }.getType(); + private static final Type ALGORITHM_SPECIFICATIONS_TYPE = new TypeToken>() { + }.getType(); + + @Value("${services.exaflow.inputdataSpecUrl}") + private String inputdataSpecUrl; + + @Value("${services.exaflow.preprocessingSpecUrl}") + private String preprocessingSpecUrl; + + @Value("${services.exaflow.algorithmsSpecUrl}") + private String algorithmsSpecUrl; + + public AnalysisInputDataSpecificationDTO getInputdataSpecification(Logger logger) { + return fetch(inputdataSpecUrl, AnalysisInputDataSpecificationDTO.class, "inputdata specification", logger); + } + + public List getPreprocessingSpecifications(Logger logger) { + return fetch(preprocessingSpecUrl, PREPROCESSING_SPECIFICATIONS_TYPE, "preprocessing specifications", logger); + } + + public List getAlgorithmSpecifications(Logger logger) { + return fetch(algorithmsSpecUrl, ALGORITHM_SPECIFICATIONS_TYPE, "algorithm specifications", logger); + } + + private T fetch(String url, Type typeOfT, String what, Logger logger) { + StringBuilder response = new StringBuilder(); + try { + HTTPUtil.sendGet(url, response); + T specification = JsonConverters.convertJsonStringToObject(response.toString(), typeOfT); + if (specification == null || (specification instanceof List list && list.isEmpty())) { + throw new InternalServerError("Exaflow " + what + " response was empty."); + } + return specification; + } catch (Exception e) { + logger.error("Could not fetch exaflow " + what + ": " + e.getMessage()); + throw new InternalServerError("Could not fetch exaflow " + what + "."); + } + } +} diff --git a/src/main/java/hbp/mip/experiment/ExperimentDTO.java b/src/main/java/hbp/mip/experiment/ExperimentDTO.java index 61aa32612..19408b270 100644 --- a/src/main/java/hbp/mip/experiment/ExperimentDTO.java +++ b/src/main/java/hbp/mip/experiment/ExperimentDTO.java @@ -1,5 +1,6 @@ package hbp.mip.experiment; +import hbp.mip.algorithm.AnalysisRequestDTO; import hbp.mip.user.UserDTO; import hbp.mip.utils.JsonConverters; @@ -7,36 +8,35 @@ import java.util.UUID; public record ExperimentDTO( - UUID uuid, - String name, - UserDTO createdBy, - Date created, - Date updated, - Date finished, - Boolean shared, - Boolean viewed, - Object result, - ExperimentDAO.Status status, - ExperimentExecutionDTO.AlgorithmExecutionDTO algorithm, - String mipVersion) { - public ExperimentDTO(ExperimentDAO experimentDAO, boolean includeResult) { - this( - experimentDAO.getUuid(), - experimentDAO.getName(), - new UserDTO(experimentDAO.getCreatedBy()), - experimentDAO.getCreated(), - experimentDAO.getUpdated(), - experimentDAO.getFinished(), - experimentDAO.isShared(), - experimentDAO.isViewed(), - includeResult - ? JsonConverters.convertJsonStringToObject( - String.valueOf(experimentDAO.getResult()), - Object.class) - : null, - experimentDAO.getStatus(), - JsonConverters.convertJsonStringToObject(experimentDAO.getAlgorithm(), - ExperimentExecutionDTO.AlgorithmExecutionDTO.class), - experimentDAO.getMipVersion()); - } + UUID uuid, + String name, + UserDTO createdBy, + Date created, + Date updated, + Date finished, + Boolean shared, + Boolean viewed, + Object result, + ExperimentDAO.Status status, + AnalysisRequestDTO analysis, + String mipVersion) { + public ExperimentDTO(ExperimentDAO experimentDAO, boolean includeResult) { + this( + experimentDAO.getUuid(), + experimentDAO.getName(), + new UserDTO(experimentDAO.getCreatedBy()), + experimentDAO.getCreated(), + experimentDAO.getUpdated(), + experimentDAO.getFinished(), + experimentDAO.isShared(), + experimentDAO.isViewed(), + includeResult + ? JsonConverters.convertJsonStringToObject( + String.valueOf(experimentDAO.getResult()), + Object.class) + : null, + experimentDAO.getStatus(), + JsonConverters.convertJsonStringToObject(experimentDAO.getAlgorithm(), AnalysisRequestDTO.class), + experimentDAO.getMipVersion()); + } } diff --git a/src/main/java/hbp/mip/experiment/ExperimentExecutionDTO.java b/src/main/java/hbp/mip/experiment/ExperimentExecutionDTO.java index ee088eb62..77a01db60 100644 --- a/src/main/java/hbp/mip/experiment/ExperimentExecutionDTO.java +++ b/src/main/java/hbp/mip/experiment/ExperimentExecutionDTO.java @@ -1,20 +1,8 @@ package hbp.mip.experiment; -import hbp.mip.algorithm.AlgorithmRequestDTO; - -import java.util.Map; -import java.util.UUID; +import hbp.mip.algorithm.AnalysisRequestDTO; public record ExperimentExecutionDTO( - UUID uuid, - String name, - AlgorithmExecutionDTO algorithm, - String mipVersion) { - public record AlgorithmExecutionDTO( - String name, - AlgorithmRequestDTO.InputDataRequestDTO inputdata, - Map parameters, - Map preprocessing) { - - } + String name, + AnalysisRequestDTO analysis) { } diff --git a/src/main/java/hbp/mip/experiment/ExperimentRepository.java b/src/main/java/hbp/mip/experiment/ExperimentRepository.java index cf5181942..1674cc58a 100644 --- a/src/main/java/hbp/mip/experiment/ExperimentRepository.java +++ b/src/main/java/hbp/mip/experiment/ExperimentRepository.java @@ -8,12 +8,10 @@ import hbp.mip.utils.Logger; import org.springframework.data.jpa.repository.JpaSpecificationExecutor; import org.springframework.data.repository.CrudRepository; -import org.springframework.data.rest.core.annotation.RestResource; import java.util.Date; import java.util.UUID; -@RestResource(exported = false) public interface ExperimentRepository extends CrudRepository, JpaSpecificationExecutor { ExperimentDAO findByUuid(UUID experimentUuid); @@ -39,16 +37,16 @@ default ExperimentDAO loadExperiment(String uuid, Logger logger) { } default ExperimentDAO createExperimentInTheDatabase(ExperimentExecutionDTO experimentExecutionDTO, UserDTO user, - Logger logger) { + String mipVersion, Logger logger) { ExperimentDAO experimentDAO = new ExperimentDAO(); UUID experimentUuid = UUID.randomUUID(); experimentDAO.setUuid(experimentUuid); experimentDAO.setCreatedBy(new UserDAO(user)); - experimentDAO.setAlgorithm(JsonConverters.convertObjectToJsonString(experimentExecutionDTO.algorithm())); - experimentDAO.setAlgorithmId(experimentExecutionDTO.algorithm().name()); + experimentDAO.setAlgorithm(JsonConverters.convertObjectToJsonString(experimentExecutionDTO.analysis())); + experimentDAO.setAlgorithmId(experimentExecutionDTO.analysis().algorithm().name()); experimentDAO.setName(experimentExecutionDTO.name()); - experimentDAO.setMipVersion(experimentExecutionDTO.mipVersion()); + experimentDAO.setMipVersion(mipVersion); experimentDAO.setStatus(ExperimentDAO.Status.pending); try { diff --git a/src/main/java/hbp/mip/experiment/ExperimentService.java b/src/main/java/hbp/mip/experiment/ExperimentService.java index fe9f0e20a..0eb66fe6b 100644 --- a/src/main/java/hbp/mip/experiment/ExperimentService.java +++ b/src/main/java/hbp/mip/experiment/ExperimentService.java @@ -1,6 +1,7 @@ package hbp.mip.experiment; -import hbp.mip.algorithm.AlgorithmRequestDTO; +import hbp.mip.algorithm.AnalysisRequestDTO; +import hbp.mip.algorithm.AnalysisService; import hbp.mip.user.ActiveUserService; import hbp.mip.user.UserDTO; import hbp.mip.utils.*; @@ -11,7 +12,6 @@ import org.springframework.security.core.Authentication; import org.springframework.stereotype.Service; -import java.io.IOException; import java.util.*; import static hbp.mip.utils.JsonConverters.convertObjectToJsonString; @@ -20,66 +20,29 @@ public class ExperimentService { private static final int MAX_PAGE_SIZE = 50; - private static final String GENERIC_ERROR_MESSAGE = "Something went wrong. Please inform the system administrator or try again later."; private final ActiveUserService activeUserService; private final ClaimUtils claimUtils; private final ExperimentRepository experimentRepository; - - @Value("${services.exaflow.algorithmsUrl}") - private String exaflowAlgorithmsUrl; - - @Value("${authentication.enabled}") - private boolean authenticationIsEnabled; - - public ExperimentService(ActiveUserService activeUserService, ClaimUtils claimUtils, - ExperimentRepository experimentRepository) { + private final AnalysisService analysisService; + private final boolean authenticationIsEnabled; + private final String mipVersion; + + public ExperimentService( + ActiveUserService activeUserService, + ClaimUtils claimUtils, + ExperimentRepository experimentRepository, + AnalysisService analysisService, + @Value("${authentication.enabled}") boolean authenticationIsEnabled, + @Value("${mip.version}") String mipVersion) { this.activeUserService = activeUserService; this.claimUtils = claimUtils; this.experimentRepository = experimentRepository; + this.analysisService = analysisService; + this.authenticationIsEnabled = authenticationIsEnabled; + this.mipVersion = mipVersion; } - private static Object convertResponseToAlgorithmResults(Logger logger, int code, StringBuilder responseBody) { - return switch (code) { - case 200 -> JsonConverters.convertJsonStringToObject(responseBody.toString(), Object.class); - case 400, 460, 461, 512, 513 -> convertExaflowResponseToAlgorithmResult(responseBody.toString()); - case 500 -> convertExaflowResponseToAlgorithmResult(GENERIC_ERROR_MESSAGE); - default -> handleUnexpectedResponseCode(logger, code); - }; - } - - private static Map convertExaflowResponseToAlgorithmResult(String resultBody) { - return Map.of("data", resultBody, "type", "text/plain+error"); - } - - private static Object handleUnexpectedResponseCode(Logger logger, int code) { - String errorMessage = "Exaflow execution responded with an unexpected status code: " + code; - logger.error(errorMessage); - throw new InternalServerError(errorMessage); - } - - /** - * The getExperiments will retrieve the experiments from database according to - * the filters. - * - * @param name is optional, in case it is required to filter the - * experiments by name - * @param algorithm is optional, in case it is required to filter the - * experiments by algorithm name - * @param shared is optional, in case it is required to filter the - * experiments by shared - * @param viewed is optional, in case it is required to filter the - * experiments by viewed - * @param includeShared is optional, in case it is required to retrieve the - * experiment that is shared - * @param page is the page that is required to be retrieved - * @param size is the size of each page - * @param orderBy is the column that is required to ordered by - * @param descending is a boolean to determine if the experiments will be - * ordered by descending or ascending order - * @param logger contains username and the endpoint. - * @return a map experiments - */ public ExperimentsDTO getExperiments(Authentication authentication, String name, String algorithm, Boolean shared, Boolean viewed, boolean includeShared, boolean mine, boolean notMine, int page, int size, String orderBy, Boolean descending, @@ -120,7 +83,6 @@ private Specification buildExperimentSpecification(Authentication spec = Specification.where(new ExperimentSpecifications.NotMyExperiment(user.username())) .and(new ExperimentSpecifications.SharedExperiment(true)); } else if (hasAccessRights) { - // Spring Data JPA no longer accepts null in Specification.where(...). spec = (root, query, criteriaBuilder) -> criteriaBuilder.conjunction(); } else { spec = Specification.where(new ExperimentSpecifications.MyExperiment(user.username())) @@ -164,37 +126,48 @@ private void validateExperimentAccess(Authentication authentication, ExperimentD public ExperimentDTO createExperiment(Authentication authentication, ExperimentExecutionDTO experimentExecutionDTO, Logger logger) { - algorithmParametersLogging(experimentExecutionDTO, logger); + requireAnalysisPayload(experimentExecutionDTO, logger); + analysisParametersLogging(experimentExecutionDTO, logger); validateDatasetAccess(authentication, experimentExecutionDTO, logger); ExperimentDAO experimentDAO = experimentRepository.createExperimentInTheDatabase(experimentExecutionDTO, - activeUserService.getActiveUser(authentication), logger); - runAlgorithmInBackground(experimentDAO, experimentExecutionDTO, logger); + activeUserService.getActiveUser(authentication), mipVersion, logger); + runAnalysisInBackground(experimentDAO, experimentExecutionDTO, logger); return new ExperimentDTO(experimentDAO, false); } + private void requireAnalysisPayload(ExperimentExecutionDTO experimentExecutionDTO, Logger logger) { + if (experimentExecutionDTO.analysis() == null) { + String errorMessage = "Missing required analysis payload."; + logger.warn(errorMessage); + throw new BadRequestException(errorMessage); + } + } + private void validateDatasetAccess(Authentication authentication, ExperimentExecutionDTO experimentExecutionDTO, Logger logger) { if (authenticationIsEnabled) { claimUtils.validateAccessRightsOnDatasets(authentication, - experimentExecutionDTO.algorithm().inputdata().datasets(), logger); + experimentExecutionDTO.analysis().inputdata().datasets(), logger); } } - private void runAlgorithmInBackground(ExperimentDAO experimentDAO, ExperimentExecutionDTO experimentExecutionDTO, + private void runAnalysisInBackground(ExperimentDAO experimentDAO, ExperimentExecutionDTO experimentExecutionDTO, Logger logger) { new Thread(() -> { try { - logger.debug("Experiment's algorithm execution started in a background thread."); - ExperimentAlgorithmResultDTO resultDTO = runExaflowAlgorithm(experimentDAO.getUuid(), - experimentExecutionDTO, logger); - experimentDAO.setResult(convertObjectToJsonString(resultDTO.result())); + logger.debug("Experiment analysis execution started in a background thread."); + AnalysisService.AnalysisResultDTO resultDTO = analysisService.runAnalysis( + experimentDAO.getUuid(), + experimentExecutionDTO.analysis(), + logger); + experimentDAO.setResult(convertObjectToJsonString(resultDTO.body())); experimentDAO .setStatus(resultDTO.code() >= 400 ? ExperimentDAO.Status.error : ExperimentDAO.Status.success); } catch (Exception e) { - logger.error("Exaflow algorithm execution failed: " + e.getMessage()); + logger.error("Exaflow analysis execution failed: " + e.getMessage()); experimentDAO.setStatus(ExperimentDAO.Status.error); } experimentRepository.finishExperiment(experimentDAO, logger); @@ -204,17 +177,21 @@ private void runAlgorithmInBackground(ExperimentDAO experimentDAO, ExperimentExe public ExperimentDTO runTransientExperiment(Authentication authentication, ExperimentExecutionDTO experimentExecutionDTO, Logger logger) { - algorithmParametersLogging(experimentExecutionDTO, logger); + requireAnalysisPayload(experimentExecutionDTO, logger); + analysisParametersLogging(experimentExecutionDTO, logger); validateDatasetAccess(authentication, experimentExecutionDTO, logger); UUID uuid = UUID.randomUUID(); - ExperimentAlgorithmResultDTO algorithmResult = runExaflowAlgorithm(uuid, experimentExecutionDTO, logger); + AnalysisService.AnalysisResultDTO analysisResult = analysisService.runAnalysis( + uuid, + experimentExecutionDTO.analysis(), + logger); return new ExperimentDTO(uuid, experimentExecutionDTO.name(), null, null, null, null, null, null, - algorithmResult.result(), - algorithmResult.code() >= 400 ? ExperimentDAO.Status.error : ExperimentDAO.Status.success, - experimentExecutionDTO.algorithm(), - experimentExecutionDTO.mipVersion()); + analysisResult.body(), + analysisResult.code() >= 400 ? ExperimentDAO.Status.error : ExperimentDAO.Status.success, + experimentExecutionDTO.analysis(), + mipVersion); } public ExperimentDTO updateExperiment(UserDTO user, String uuid, ExperimentDTO experiment, Logger logger) { @@ -277,12 +254,17 @@ private void checkDeleteAuthorization(UserDTO user, ExperimentDAO experimentDAO, } private void verifyNonEditableFieldsAreNotBeingModified(ExperimentDTO experimentDTO, Logger logger) { - List.of("uuid", "algorithm", "created", "updated", "finished", "createdBy", "result", "status") - .forEach(field -> throwNonEditableExceptionIfNotNull(getFieldValue(experimentDTO, field), field, - logger)); + throwIfNonNull(experimentDTO.uuid(), "uuid", logger); + throwIfNonNull(experimentDTO.createdBy(), "createdBy", logger); + throwIfNonNull(experimentDTO.created(), "created", logger); + throwIfNonNull(experimentDTO.updated(), "updated", logger); + throwIfNonNull(experimentDTO.finished(), "finished", logger); + throwIfNonNull(experimentDTO.result(), "result", logger); + throwIfNonNull(experimentDTO.status(), "status", logger); + throwIfNonNull(experimentDTO.analysis(), "analysis", logger); } - private void throwNonEditableExceptionIfNotNull(Object field, String nonEditableField, Logger logger) { + private void throwIfNonNull(Object field, String nonEditableField, Logger logger) { if (field != null) { String errorMessage = "Tried to edit non-editable field: " + nonEditableField; logger.warn(errorMessage); @@ -290,73 +272,39 @@ private void throwNonEditableExceptionIfNotNull(Object field, String nonEditable } } - private Object getFieldValue(ExperimentDTO experimentDTO, String fieldName) { - try { - var field = ExperimentDTO.class.getDeclaredField(fieldName); - field.setAccessible(true); - return field.get(experimentDTO); - } catch (NoSuchFieldException | IllegalAccessException e) { - return null; - } - } - - private void algorithmParametersLogging(ExperimentExecutionDTO experimentExecutionDTO, Logger logger) { - ExperimentExecutionDTO.AlgorithmExecutionDTO algorithm = experimentExecutionDTO.algorithm(); + private void analysisParametersLogging(ExperimentExecutionDTO experimentExecutionDTO, Logger logger) { + AnalysisRequestDTO analysis = experimentExecutionDTO.analysis(); StringBuilder parametersLogMessage = new StringBuilder(); - Optional.ofNullable(algorithm.parameters()).ifPresent(parameters -> parameters.forEach((paramName, + Optional.ofNullable(analysis.algorithm().parameters()).ifPresent(parameters -> parameters.forEach((paramName, paramValue) -> parametersLogMessage.append(" ").append(paramName).append(" -> ").append(paramValue))); - Optional.ofNullable(algorithm.preprocessing()).ifPresent(preprocessing -> preprocessing - .forEach((name, value) -> parametersLogMessage.append(" ").append(name).append(" -> ").append(value))); + Optional.ofNullable(analysis.preprocessing()).ifPresent(preprocessing -> preprocessing + .forEach(step -> parametersLogMessage.append(" ").append(step.name()).append(" -> ") + .append(step.parameters()))); - if (algorithm.inputdata() != null) { - AlgorithmRequestDTO.InputDataRequestDTO inputData = algorithm.inputdata(); + if (analysis.inputdata() != null) { + AnalysisRequestDTO.AnalysisInputDataDTO inputData = analysis.inputdata(); parametersLogMessage.append(" Input Data Model: ").append(inputData.data_model()); Optional.ofNullable(inputData.datasets()) .ifPresent(datasets -> parametersLogMessage.append(" Datasets: ").append(datasets)); - Optional.ofNullable(inputData.x()) + Optional.ofNullable(inputData.variables()) + .ifPresent(variables -> parametersLogMessage.append(" Variables: ").append(variables)); + + Optional.ofNullable(analysis.algorithm().x()) .ifPresent(xVars -> parametersLogMessage.append(" X Variables: ").append(xVars)); - Optional.ofNullable(inputData.y()) + Optional.ofNullable(analysis.algorithm().y()) .ifPresent(yVars -> parametersLogMessage.append(" Y Variables: ").append(yVars)); if (inputData.filters() != null) { - AlgorithmRequestDTO.FilterRequestDTO filters = inputData.filters(); - parametersLogMessage.append(" Filter Condition: ").append(filters.condition()); - - Optional.ofNullable(filters.rules()) - .ifPresent(rules -> parametersLogMessage.append(" Filter Rules: ").append(rules)); + parametersLogMessage.append(" Filters: ").append(inputData.filters()); } } - logger.debug("Algorithm " + algorithm.name() + " execution starting with parameters: " + parametersLogMessage); - } - - private ExperimentAlgorithmResultDTO runExaflowAlgorithm(UUID uuid, ExperimentExecutionDTO experimentExecutionDTO, - Logger logger) { - String algorithmEndpoint = exaflowAlgorithmsUrl + "/" + experimentExecutionDTO.algorithm().name(); - var requestBody = convertObjectToJsonString( - AlgorithmRequestDTO.create(uuid, experimentExecutionDTO.algorithm())); - - logger.debug("Exaflow algorithm request, endpoint: " + algorithmEndpoint); - logger.debug("Exaflow algorithm request, body: " + requestBody); - - int responseCode; - var responseBody = new StringBuilder(); - try { - responseCode = HTTPUtil.sendPost(algorithmEndpoint, requestBody, responseBody); - } catch (IOException e) { - logger.error("Could not run the exaflow algorithm: " + e.getMessage()); - throw new InternalServerError(e.getMessage()); - } - - Object result = convertResponseToAlgorithmResults(logger, responseCode, responseBody); - return new ExperimentAlgorithmResultDTO(responseCode, result); - } - - record ExperimentAlgorithmResultDTO(int code, Object result) { + logger.debug("Analysis " + analysis.algorithm().name() + " execution starting with parameters: " + + parametersLogMessage); } } diff --git a/src/main/java/hbp/mip/utils/CustomResourceLoader.java b/src/main/java/hbp/mip/utils/CustomResourceLoader.java deleted file mode 100644 index 89552146c..000000000 --- a/src/main/java/hbp/mip/utils/CustomResourceLoader.java +++ /dev/null @@ -1,21 +0,0 @@ -package hbp.mip.utils; - -import lombok.NonNull; -import org.springframework.context.ResourceLoaderAware; -import org.springframework.core.io.Resource; -import org.springframework.core.io.ResourceLoader; -import org.springframework.stereotype.Component; - -@Component -public class CustomResourceLoader implements ResourceLoaderAware { - - private ResourceLoader resourceLoader; - - public void setResourceLoader(@NonNull ResourceLoader resourceLoader) { - this.resourceLoader = resourceLoader; - } - - public Resource getResource(String resourceLocation) { - return resourceLoader.getResource(resourceLocation); - } -} \ No newline at end of file diff --git a/src/main/java/hbp/mip/utils/InputStreamConverter.java b/src/main/java/hbp/mip/utils/InputStreamConverter.java deleted file mode 100644 index 0926d6fa6..000000000 --- a/src/main/java/hbp/mip/utils/InputStreamConverter.java +++ /dev/null @@ -1,22 +0,0 @@ -package hbp.mip.utils; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.nio.charset.StandardCharsets; - -public class InputStreamConverter { - - // Pure Java - // https://stackoverflow.com/questions/309424/how-do-i-read-convert-an-inputstream-into-a-string-in-java - public static String convertInputStreamToString(InputStream inputStream) throws IOException { - ByteArrayOutputStream result = new ByteArrayOutputStream(); - byte[] buffer = new byte[1024]; - int length; - while ((length = inputStream.read(buffer)) != -1) { - result.write(buffer, 0, length); - } - - return result.toString(StandardCharsets.UTF_8); - } -} diff --git a/src/test/java/hbp/mip/algorithm/AlgorithmSpecificationDTOTest.java b/src/test/java/hbp/mip/algorithm/AlgorithmSpecificationDTOTest.java index 73f79bbe8..9affcd9af 100644 --- a/src/test/java/hbp/mip/algorithm/AlgorithmSpecificationDTOTest.java +++ b/src/test/java/hbp/mip/algorithm/AlgorithmSpecificationDTOTest.java @@ -4,7 +4,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; -import hbp.mip.experiment.ExperimentExecutionDTO; import org.junit.jupiter.api.Test; import java.lang.reflect.Type; @@ -20,22 +19,19 @@ class AlgorithmSpecificationDTOTest { private final ObjectMapper objectMapper = new ObjectMapper(); @Test - void deserializesOutlierDictionaryMetadataAndPreprocessingOrder() throws Exception { + void deserializesHeadAlgorithmSpecificationShape() throws Exception { String payload = """ [ { - "name": "outlier_report", - "label": "Outlier Report", - "desc": "Detect outliers.", + "name": "linear_regression", + "label": "Linear Regression", + "desc": "Short description.", "documentation": "Long algorithm documentation.", - "type": "stats", + "type": "exareme3", "flags": ["beta"], - "inputdata": { - "data_model": { "label": "Data model", "desc": "", "types": ["text"] }, - "datasets": { "label": "Datasets", "desc": "", "types": ["text"] }, - "validation_datasets": { "label": "Validation datasets", "desc": "", "types": ["text"], "required": false, "min_count": 0, "max_count": 2 }, - "y": { "label": "Y", "desc": "", "types": ["real"], "required": true, "min_count": 1, "max_count": 3 } - }, + "y": { "label": "Y", "desc": "", "types": ["real"], "required": true, "min_count": 1, "max_count": 1 }, + "x": { "label": "X", "desc": "", "types": ["real"], "required": true, "min_count": 1 }, + "requires_validation_datasets": false, "parameters": { "folds": { "label": "Folds", @@ -51,16 +47,7 @@ void deserializesOutlierDictionaryMetadataAndPreprocessingOrder() throws Excepti "dict_values_type": "real" } }, - "preprocessing": [ - { - "name": "outlier_winsorizer", - "label": "Outlier Winsorizer", - "desc": "Clip outliers.", - "documentation": "Long preprocessing documentation.", - "order": 2, - "parameters": {} - } - ] + "required_preprocessing": [] } ] """; @@ -70,99 +57,95 @@ void deserializesOutlierDictionaryMetadataAndPreprocessingOrder() throws Excepti List algorithms = gson.fromJson(payload, algorithmListType); AlgorithmSpecificationDTO algorithm = algorithms.getFirst(); - AlgorithmSpecificationDTO.AlgorithmParameterSpecificationDTO folds = algorithm.parameters().get("folds"); + ParameterSpecificationDTO folds = algorithm.parameters().get("folds"); assertThat(folds.dict_values_type()).isEqualTo("real"); assertThat(folds.dict_keys_enums().type()).isEqualTo("input_var_names"); assertThat(folds.dict_keys_enums().source()).containsExactly("x", "y"); assertThat(algorithm.documentation()).isEqualTo("Long algorithm documentation."); - assertThat(algorithm.type()).isEqualTo("stats"); + assertThat(algorithm.type()).isEqualTo("exareme3"); assertThat(algorithm.flags()).containsExactly("beta"); - assertThat(algorithm.inputdata().y().min_count()).isEqualTo(1); - assertThat(algorithm.inputdata().y().max_count()).isEqualTo(3); - assertThat(algorithm.inputdata().validation_datasets().max_count()).isEqualTo(2); - assertThat(folds.min()).isEqualTo("0"); - assertThat(folds.max()).isEqualTo("10"); - assertThat(algorithm.preprocessing().getFirst().order()).isEqualTo(2); - assertThat(algorithm.preprocessing().getFirst().documentation()).isEqualTo("Long preprocessing documentation."); + assertThat(algorithm.y().min_count()).isEqualTo(1); + assertThat(algorithm.y().max_count()).isEqualTo(1); + assertThat(algorithm.requires_validation_datasets()).isFalse(); + assertThat(folds.min()).isEqualTo(0.0); + assertThat(folds.max()).isEqualTo(10.0); JsonNode serialized = objectMapper.readTree(objectMapper.writeValueAsString(algorithm)); assertThat(serialized.at("/documentation").asText()).isEqualTo("Long algorithm documentation."); assertThat(serialized.at("/parameters/folds/dict_values_type").asText()).isEqualTo("real"); - assertThat(serialized.at("/preprocessing/0/documentation").asText()).isEqualTo("Long preprocessing documentation."); - assertThat(serialized.at("/preprocessing/0/order").asInt()).isEqualTo(2); + assertThat(serialized.at("/requires_validation_datasets").asBoolean()).isFalse(); } @Test - void keepsDocumentationOptionalWhenMissing() { - String payload = """ + void deserializesInputdataAndPreprocessingSpecifications() { + String inputdataPayload = """ + { + "data_model": { "label": "Data model", "desc": "", "types": ["text"], "required": true }, + "datasets": { "label": "Datasets", "desc": "", "types": ["text"], "required": true, "min_count": 1 }, + "filters": { "label": "Filters", "desc": "", "types": ["jsonObject"], "required": false }, + "variables": { "label": "Variables", "desc": "", "types": ["real", "int", "text"], "required": true, "min_count": 1 } + } + """; + String preprocessingPayload = """ [ { - "name": "canonical_optional_algorithm", - "label": "Canonical Optional Algorithm", - "desc": "Canonical short description.", - "inputdata": { - "data_model": { "label": "Data model", "desc": "", "types": ["text"] }, - "datasets": { "label": "Datasets", "desc": "", "types": ["text"] }, - "y": { "label": "Y", "desc": "", "types": ["real"], "required": true } - }, + "name": "categorical_column_creator", + "label": "Categorical Column Creator", + "desc": "Create a derived column.", + "documentation": "Long preprocessing documentation.", "parameters": {}, - "preprocessing": [ - { - "name": "canonical_preprocessing", - "label": "Canonical Preprocessing", - "desc": "Canonical preprocessing summary.", - "order": 4, - "parameters": {} - } - ] + "output": { + "type": "new_categorical_column", + "code_parameter": "code" + } } ] """; - Type algorithmListType = new TypeToken>() { - }.getType(); - List algorithms = gson.fromJson(payload, algorithmListType); - - AlgorithmSpecificationDTO algorithm = algorithms.getFirst(); + AnalysisInputDataSpecificationDTO inputdata = gson.fromJson(inputdataPayload, AnalysisInputDataSpecificationDTO.class); + List preprocessing = gson.fromJson( + preprocessingPayload, + new TypeToken>() { + }.getType()); - assertThat(algorithm.documentation()).isNull(); - assertThat(algorithm.preprocessing().getFirst().documentation()).isNull(); + assertThat(inputdata.variables().min_count()).isEqualTo(1); + assertThat(preprocessing.getFirst().output().type()).isEqualTo("new_categorical_column"); + assertThat(preprocessing.getFirst().output().code_parameter()).isEqualTo("code"); } @Test - void serializesNestedOutlierPreprocessingUnchangedForExaflowRequest() throws Exception { - Map outlierWinsorizer = new LinkedHashMap<>(); - outlierWinsorizer.put("strategies", Map.of("age", "iqr")); - outlierWinsorizer.put("tails", Map.of("age", "both")); - outlierWinsorizer.put("folds", Map.of("age", 1.5)); - - Map preprocessing = Map.of("outlier_winsorizer", outlierWinsorizer); - ExperimentExecutionDTO.AlgorithmExecutionDTO algorithmExecutionDTO = - new ExperimentExecutionDTO.AlgorithmExecutionDTO( + void serializesOrderedPreprocessingForAnalysisRequest() throws Exception { + Map outlierParameters = new LinkedHashMap<>(); + outlierParameters.put("strategies", Map.of("age", "iqr")); + outlierParameters.put("tails", Map.of("age", "both")); + outlierParameters.put("folds", Map.of("age", 1.5)); + + AnalysisRequestDTO request = new AnalysisRequestDTO( + UUID.fromString("00000000-0000-0000-0000-000000000001").toString(), + new AnalysisRequestDTO.AnalysisInputDataDTO( + "dm:1", + List.of("ds1"), + null, + null, + List.of("age", "outcome")), + List.of(new AnalysisRequestDTO.AnalysisPreprocessingStepDTO( + "outlier_winsorizer", + outlierParameters)), + new AnalysisRequestDTO.AnalysisAlgorithmDTO( "linear_regression", - new AlgorithmRequestDTO.InputDataRequestDTO( - "dm:1", - List.of("ds1"), - List.of("outcome"), - List.of("age"), - null, - null - ), - Map.of(), - preprocessing - ); - - AlgorithmRequestDTO request = AlgorithmRequestDTO.create( - UUID.fromString("00000000-0000-0000-0000-000000000001"), - algorithmExecutionDTO - ); - - JsonNode outlierNode = objectMapper.readTree(objectMapper.writeValueAsString(request)) - .at("/preprocessing/outlier_winsorizer"); - - assertThat(outlierNode.at("/strategies/age").asText()).isEqualTo("iqr"); - assertThat(outlierNode.at("/tails/age").asText()).isEqualTo("both"); - assertThat(outlierNode.at("/folds/age").asDouble()).isEqualTo(1.5); + List.of("age"), + List.of("outcome"), + Map.of()), + null); + + JsonNode serialized = objectMapper.readTree(objectMapper.writeValueAsString(request)); + JsonNode preprocessingNode = serialized.at("/preprocessing/0"); + + assertThat(preprocessingNode.at("/name").asText()).isEqualTo("outlier_winsorizer"); + assertThat(preprocessingNode.at("/parameters/strategies/age").asText()).isEqualTo("iqr"); + assertThat(preprocessingNode.at("/parameters/tails/age").asText()).isEqualTo("both"); + assertThat(preprocessingNode.at("/parameters/folds/age").asDouble()).isEqualTo(1.5); + assertThat(serialized.at("/algorithm/name").asText()).isEqualTo("linear_regression"); } } diff --git a/src/test/java/hbp/mip/algorithm/AnalysisRequestDTODeserializationTest.java b/src/test/java/hbp/mip/algorithm/AnalysisRequestDTODeserializationTest.java new file mode 100644 index 000000000..bdb6d9bef --- /dev/null +++ b/src/test/java/hbp/mip/algorithm/AnalysisRequestDTODeserializationTest.java @@ -0,0 +1,50 @@ +package hbp.mip.algorithm; + +import com.fasterxml.jackson.databind.ObjectMapper; +import hbp.mip.experiment.ExperimentExecutionDTO; +import hbp.mip.utils.JsonConverters; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class AnalysisRequestDTODeserializationTest { + + private static final String ANALYSIS_JSON = """ + { + "request_id": "11111111-1111-1111-1111-111111111111", + "inputdata": { + "data_model": "dm:1", + "datasets": ["ds1"], + "variables": ["age"] + }, + "preprocessing": [ + { "name": "winsorize", "parameters": { "limit": 0.05 } } + ], + "algorithm": { + "name": "histogram", + "y": ["age"], + "parameters": {} + } + } + """; + + @Test + void jacksonReadsAnalysisFromExperimentExecutionBody() throws Exception { + String body = "{\"name\":\"experiment\",\"analysis\":" + ANALYSIS_JSON + "}"; + + ExperimentExecutionDTO execution = new ObjectMapper().readValue(body, ExperimentExecutionDTO.class); + + assertThat(execution.analysis().algorithm().name()).isEqualTo("histogram"); + assertThat(execution.analysis().inputdata().datasets()).containsExactly("ds1"); + assertThat(execution.analysis().preprocessing()).extracting("name").containsExactly("winsorize"); + } + + @Test + void gsonReadsAnalysisFromStoredExperimentJson() { + AnalysisRequestDTO analysis = JsonConverters.convertJsonStringToObject(ANALYSIS_JSON, AnalysisRequestDTO.class); + + assertThat(analysis.algorithm().name()).isEqualTo("histogram"); + assertThat(analysis.inputdata().variables()).containsExactly("age"); + assertThat(analysis.preprocessing()).extracting("name").containsExactly("winsorize"); + } +} diff --git a/src/test/java/hbp/mip/algorithm/AnalysisServiceRequestIdTest.java b/src/test/java/hbp/mip/algorithm/AnalysisServiceRequestIdTest.java new file mode 100644 index 000000000..33a18ed39 --- /dev/null +++ b/src/test/java/hbp/mip/algorithm/AnalysisServiceRequestIdTest.java @@ -0,0 +1,38 @@ +package hbp.mip.algorithm; + +import hbp.mip.utils.Exceptions.BadRequestException; +import hbp.mip.utils.Logger; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class AnalysisServiceRequestIdTest { + + private final AnalysisService analysisService = new AnalysisService("http://127.0.0.1:5000/analysis"); + + @Test + void runAnalysis_rejectsInvalidRequestId() { + var analysis = new AnalysisRequestDTO( + "not-a-uuid", + new AnalysisRequestDTO.AnalysisInputDataDTO( + "dm:1", + List.of("ds1"), + null, + null, + List.of("age")), + null, + new AnalysisRequestDTO.AnalysisAlgorithmDTO( + "histogram", + null, + List.of("age"), + Map.of()), + null); + + assertThatThrownBy(() -> analysisService.runAnalysis(analysis, new Logger("user", "test"))) + .isInstanceOf(BadRequestException.class) + .hasMessageContaining("Invalid request_id"); + } +} diff --git a/src/test/java/hbp/mip/experiment/ExperimentServiceMipVersionTest.java b/src/test/java/hbp/mip/experiment/ExperimentServiceMipVersionTest.java new file mode 100644 index 000000000..5f3fa2028 --- /dev/null +++ b/src/test/java/hbp/mip/experiment/ExperimentServiceMipVersionTest.java @@ -0,0 +1,102 @@ +package hbp.mip.experiment; + +import hbp.mip.algorithm.AnalysisRequestDTO; +import hbp.mip.algorithm.AnalysisService; +import hbp.mip.user.ActiveUserService; +import hbp.mip.user.UserDTO; +import hbp.mip.utils.ClaimUtils; +import hbp.mip.utils.Logger; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.core.Authentication; + +import java.util.List; +import java.util.Map; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class ExperimentServiceMipVersionTest { + + @Mock + private ActiveUserService activeUserService; + + @Mock + private ClaimUtils claimUtils; + + @Mock + private ExperimentRepository experimentRepository; + + @Mock + private AnalysisService analysisService; + + @Mock + private Authentication authentication; + + private ExperimentService experimentService; + + @BeforeEach + void setUp() { + experimentService = new ExperimentService(activeUserService, claimUtils, experimentRepository, + analysisService, false, "9.0.0"); + } + + @Test + void createExperiment_stampsMipVersionFromEnvironment() { + var user = new UserDTO("user", "User", "user@example.org", "subject", true); + var execution = sampleExecution(); + var saved = new ExperimentDAO(); + saved.setUuid(UUID.randomUUID()); + saved.setMipVersion("9.0.0"); + saved.setCreatedBy(new hbp.mip.user.UserDAO(user)); + saved.setAlgorithm("{\"algorithm\":{\"name\":\"histogram\"}}"); + + when(activeUserService.getActiveUser(authentication)).thenReturn(user); + when(experimentRepository.createExperimentInTheDatabase(eq(execution), eq(user), eq("9.0.0"), any(Logger.class))) + .thenReturn(saved); + + var response = experimentService.createExperiment(authentication, execution, new Logger("user", "test")); + + assertThat(response.mipVersion()).isEqualTo("9.0.0"); + verify(experimentRepository).createExperimentInTheDatabase(eq(execution), eq(user), eq("9.0.0"), any(Logger.class)); + } + + @Test + void runTransientExperiment_returnsMipVersionFromEnvironment() { + var execution = sampleExecution(); + + when(analysisService.runAnalysis(any(UUID.class), eq(execution.analysis()), any(Logger.class))) + .thenReturn(new AnalysisService.AnalysisResultDTO(200, Map.of("ok", true))); + + var response = experimentService.runTransientExperiment(authentication, execution, new Logger("user", "test")); + + assertThat(response.mipVersion()).isEqualTo("9.0.0"); + } + + private ExperimentExecutionDTO sampleExecution() { + var analysis = new AnalysisRequestDTO( + null, + new AnalysisRequestDTO.AnalysisInputDataDTO( + "dm:1", + List.of("ds1"), + null, + null, + List.of("age")), + null, + new AnalysisRequestDTO.AnalysisAlgorithmDTO( + "histogram", + null, + List.of("age"), + Map.of()), + null); + return new ExperimentExecutionDTO("test experiment", analysis); + } +} From 7a9125940f6ab5169035f196dc338df72d004433 Mon Sep 17 00:00:00 2001 From: KFilippopolitis Date: Tue, 15 Sep 2026 14:24:02 +0300 Subject: [PATCH 03/10] feat(folder): add experiment folder/set persistence and V2 migration Folders hold experiment ids, sets are ordered names, and a single member row per (folder, experiment) makes sets a strict partition. Add the three JPA entities, the repository (including the pessimistic folder-row lock), and Flyway V2; wire hbp.mip.folder into PersistenceConfiguration. --- .../PersistenceConfiguration.java | 4 +- .../hbp/mip/folder/ExperimentFolderDAO.java | 76 +++++++++++++++++++ .../mip/folder/ExperimentFolderMemberDAO.java | 67 ++++++++++++++++ .../folder/ExperimentFolderRepository.java | 37 +++++++++ .../java/hbp/mip/folder/ExperimentSetDAO.java | 57 ++++++++++++++ .../db/migration/V2__ExperimentFolders.sql | 43 +++++++++++ 6 files changed, 282 insertions(+), 2 deletions(-) create mode 100644 src/main/java/hbp/mip/folder/ExperimentFolderDAO.java create mode 100644 src/main/java/hbp/mip/folder/ExperimentFolderMemberDAO.java create mode 100644 src/main/java/hbp/mip/folder/ExperimentFolderRepository.java create mode 100644 src/main/java/hbp/mip/folder/ExperimentSetDAO.java create mode 100644 src/main/resources/db/migration/V2__ExperimentFolders.sql diff --git a/src/main/java/hbp/mip/configurations/PersistenceConfiguration.java b/src/main/java/hbp/mip/configurations/PersistenceConfiguration.java index d9782a8f8..22bda16d7 100644 --- a/src/main/java/hbp/mip/configurations/PersistenceConfiguration.java +++ b/src/main/java/hbp/mip/configurations/PersistenceConfiguration.java @@ -18,7 +18,7 @@ import javax.sql.DataSource; @Configuration -@EnableJpaRepositories(basePackages = { "hbp.mip.experiment", "hbp.mip.user" }) +@EnableJpaRepositories(basePackages = { "hbp.mip.experiment", "hbp.mip.user", "hbp.mip.folder" }) @EnableTransactionManagement public class PersistenceConfiguration { @@ -36,7 +36,7 @@ public LocalContainerEntityManagerFactoryBean entityManagerFactory() { emfb.setDataSource(platformDataSource()); JpaVendorAdapter vendorAdapter = new HibernateJpaVendorAdapter(); emfb.setJpaVendorAdapter(vendorAdapter); - emfb.setPackagesToScan("hbp.mip.experiment", "hbp.mip.user"); + emfb.setPackagesToScan("hbp.mip.experiment", "hbp.mip.user", "hbp.mip.folder"); return emfb; } diff --git a/src/main/java/hbp/mip/folder/ExperimentFolderDAO.java b/src/main/java/hbp/mip/folder/ExperimentFolderDAO.java new file mode 100644 index 000000000..f1a28e3c6 --- /dev/null +++ b/src/main/java/hbp/mip/folder/ExperimentFolderDAO.java @@ -0,0 +1,76 @@ +package hbp.mip.folder; + +import hbp.mip.user.UserDAO; +import jakarta.persistence.CascadeType; +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.OneToMany; +import jakarta.persistence.OrderBy; +import jakarta.persistence.Table; +import lombok.Getter; +import org.hibernate.annotations.BatchSize; +import lombok.NoArgsConstructor; +import lombok.Setter; + +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import java.util.UUID; + +/** + * A user-curated group of experiment runs (an "analysis set" in the UI). + * + * The folder owns its sets and its members: deleting a folder takes its grouping with it, never the + * experiments themselves, which stay in the experiment table. + * + * Names are unique per owner ignoring case (uq_experiment_folder_owner_name), and the service + * rejects the duplicate with 409 before the database ever sees it. + */ +@Entity +@Getter +@Setter +@NoArgsConstructor +@Table(name = "experiment_folder") +public class ExperimentFolderDAO { + + @Id + @Column(columnDefinition = "uuid", updatable = false) + private UUID id; + + @ManyToOne(fetch = FetchType.LAZY, optional = false) + @JoinColumn(name = "owner_username", columnDefinition = "CHARACTER VARYING") + private UserDAO owner; + + @Column(nullable = false, columnDefinition = "TEXT") + private String name; + + @Column(name = "sort_order", nullable = false) + private Integer sortOrder; + + @Column(columnDefinition = "TIMESTAMP WITHOUT TIME ZONE") + private Date created = new Date(); + + // Batched rather than join-fetched: two bags on one entity cannot be fetched in a single join, and + // the list endpoint reads many folders at once, so one statement per collection beats one per folder. + @BatchSize(size = 50) + @OneToMany(mappedBy = "folder", cascade = CascadeType.ALL, orphanRemoval = true) + @OrderBy("sortOrder ASC") + private List sets = new ArrayList<>(); + + @BatchSize(size = 50) + @OneToMany(mappedBy = "folder", cascade = CascadeType.ALL, orphanRemoval = true) + @OrderBy("folderPosition ASC") + private List members = new ArrayList<>(); + + public ExperimentFolderDAO(UserDAO owner, String name, Integer sortOrder) { + this.id = UUID.randomUUID(); + this.owner = owner; + this.name = name; + this.sortOrder = sortOrder; + } + +} diff --git a/src/main/java/hbp/mip/folder/ExperimentFolderMemberDAO.java b/src/main/java/hbp/mip/folder/ExperimentFolderMemberDAO.java new file mode 100644 index 000000000..41a9cc4f1 --- /dev/null +++ b/src/main/java/hbp/mip/folder/ExperimentFolderMemberDAO.java @@ -0,0 +1,67 @@ +package hbp.mip.folder; + +import hbp.mip.experiment.ExperimentDAO; +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +import java.util.UUID; + +/** + * The membership of one experiment in one folder, with its place in both orderings. + * + * Exactly one row per (folder, experiment) — enforced by uq_experiment_folder_member — is what makes + * sets a strict partition: joining a set is a repoint of {@code experimentSet}, so the previous set + * gives the run up without a second delete. + * + * {@code experimentSet} null means "in the folder, ungrouped". Deleting the experiment drops the row + * (ON DELETE CASCADE); the service nulls the field itself when a set is deleted, so the JPA state + * matches the database rule instead of waiting for it. + */ +@Entity +@Getter +@Setter +@NoArgsConstructor +@Table(name = "experiment_folder_member") +public class ExperimentFolderMemberDAO { + + @Id + @Column(columnDefinition = "uuid", updatable = false) + private UUID id; + + @ManyToOne(fetch = FetchType.LAZY, optional = false) + @JoinColumn(name = "folder_id", columnDefinition = "uuid") + private ExperimentFolderDAO folder; + + // Deliberately lazy and not batch-annotated: Hibernate 7 rejects @BatchSize on a to-one, and every + // read here is `getExperiment().getUuid()`, which a proxy answers from its identifier alone. The DTO + // and the membership lookups therefore never issue a SELECT for the experiment itself. + @ManyToOne(fetch = FetchType.LAZY, optional = false) + @JoinColumn(name = "experiment_uuid", columnDefinition = "uuid") + private ExperimentDAO experiment; + + @ManyToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "set_id", columnDefinition = "uuid") + private ExperimentSetDAO experimentSet; + + @Column(name = "folder_position", nullable = false) + private Integer folderPosition; + + @Column(name = "set_position") + private Integer setPosition; + + public ExperimentFolderMemberDAO(ExperimentFolderDAO folder, ExperimentDAO experiment, Integer folderPosition) { + this.id = UUID.randomUUID(); + this.folder = folder; + this.experiment = experiment; + this.folderPosition = folderPosition; + } + +} diff --git a/src/main/java/hbp/mip/folder/ExperimentFolderRepository.java b/src/main/java/hbp/mip/folder/ExperimentFolderRepository.java new file mode 100644 index 000000000..256d15570 --- /dev/null +++ b/src/main/java/hbp/mip/folder/ExperimentFolderRepository.java @@ -0,0 +1,37 @@ +package hbp.mip.folder; + +import jakarta.persistence.LockModeType; +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Lock; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; + +import java.util.List; +import java.util.Optional; +import java.util.UUID; + +/** + * Folder persistence. Ownership filtering happens in the service, on the user resolved from the token, + * so no query here is asked to trust an id that came from a request. + * + * JpaRepository rather than CrudRepository for two reasons: {@code saveAndFlush} writes inside the + * service transaction, so a unique-name race surfaces where it can still be answered 409 instead of + * arriving too late, at commit time, as a 500; and {@code findByIdForUpdate} locks the folder row so + * membership writes on one folder serialize instead of racing the member unique constraint. + */ +public interface ExperimentFolderRepository extends JpaRepository { + + /** The owner's folders in display order. Every folder-scoped read starts from here. */ + @Query("select folder from ExperimentFolderDAO folder " + + "where folder.owner.username = :username order by folder.sortOrder asc, folder.id asc") + List findOwnedFolders(@Param("username") String username); + + /** + * The folder, write-locked for the rest of the transaction. Member and set writes take this first, + * so the "is it already a member?" check runs after the competing writer committed and a duplicate + * insert is not attempted at all. Reads must keep using {@code findById}. + */ + @Lock(LockModeType.PESSIMISTIC_WRITE) + @Query("select folder from ExperimentFolderDAO folder where folder.id = :id") + Optional findByIdForUpdate(@Param("id") UUID id); +} diff --git a/src/main/java/hbp/mip/folder/ExperimentSetDAO.java b/src/main/java/hbp/mip/folder/ExperimentSetDAO.java new file mode 100644 index 000000000..8a289e01e --- /dev/null +++ b/src/main/java/hbp/mip/folder/ExperimentSetDAO.java @@ -0,0 +1,57 @@ +package hbp.mip.folder; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; + +import java.util.Date; +import java.util.UUID; + +/** + * A named subset of a folder's members: the unit the compare workspace renders as a section. + * + * Membership lives on {@link ExperimentFolderMemberDAO#getExperimentSet()}, not here, because a run + * can only ever sit in one set per folder; a set is therefore just a name plus an ordering. + * + * Sets are their own namespace: a set may carry the same name as the folder that holds it, but not + * the same name as a sibling set (uq_experiment_set_folder_name). + */ +@Entity +@Getter +@Setter +@NoArgsConstructor +@Table(name = "experiment_set") +public class ExperimentSetDAO { + + @Id + @Column(columnDefinition = "uuid", updatable = false) + private UUID id; + + @ManyToOne(fetch = FetchType.LAZY, optional = false) + @JoinColumn(name = "folder_id", columnDefinition = "uuid") + private ExperimentFolderDAO folder; + + @Column(nullable = false, columnDefinition = "TEXT") + private String name; + + @Column(name = "sort_order", nullable = false) + private Integer sortOrder; + + @Column(columnDefinition = "TIMESTAMP WITHOUT TIME ZONE") + private Date created = new Date(); + + public ExperimentSetDAO(ExperimentFolderDAO folder, String name, Integer sortOrder) { + this.id = UUID.randomUUID(); + this.folder = folder; + this.name = name; + this.sortOrder = sortOrder; + } + +} diff --git a/src/main/resources/db/migration/V2__ExperimentFolders.sql b/src/main/resources/db/migration/V2__ExperimentFolders.sql new file mode 100644 index 000000000..dd8f241bb --- /dev/null +++ b/src/main/resources/db/migration/V2__ExperimentFolders.sql @@ -0,0 +1,43 @@ +-- Experiment folders ("analysis sets") and the named subsets ("sets") inside them. +-- +-- One member row per (folder, experiment) gives a strict partition for free: a run sits in at most +-- one set because moving it only repoints set_id. Set ordering and folder ordering live in +-- sort_order, member ordering inside a folder in folder_position, member ordering inside a set in +-- set_position. + +CREATE TABLE IF NOT EXISTS experiment_folder ( + id UUID PRIMARY KEY, + owner_username CHARACTER VARYING NOT NULL REFERENCES "user"(username) ON DELETE CASCADE, + name TEXT NOT NULL, + sort_order INTEGER NOT NULL, + created TIMESTAMP WITHOUT TIME ZONE DEFAULT CURRENT_TIMESTAMP +); + +CREATE UNIQUE INDEX IF NOT EXISTS uq_experiment_folder_owner_name + ON experiment_folder (owner_username, lower(name)); + +CREATE TABLE IF NOT EXISTS experiment_set ( + id UUID PRIMARY KEY, + folder_id UUID NOT NULL REFERENCES experiment_folder(id) ON DELETE CASCADE, + name TEXT NOT NULL, + sort_order INTEGER NOT NULL, + created TIMESTAMP WITHOUT TIME ZONE DEFAULT CURRENT_TIMESTAMP +); + +CREATE UNIQUE INDEX IF NOT EXISTS uq_experiment_set_folder_name + ON experiment_set (folder_id, lower(name)); + +CREATE TABLE IF NOT EXISTS experiment_folder_member ( + id UUID PRIMARY KEY, + folder_id UUID NOT NULL REFERENCES experiment_folder(id) ON DELETE CASCADE, + experiment_uuid UUID NOT NULL REFERENCES experiment(uuid) ON DELETE CASCADE, + set_id UUID REFERENCES experiment_set(id) ON DELETE SET NULL, + folder_position INTEGER NOT NULL, + set_position INTEGER, + CONSTRAINT uq_experiment_folder_member UNIQUE (folder_id, experiment_uuid) +); + +CREATE INDEX IF NOT EXISTS idx_experiment_folder_member_set + ON experiment_folder_member(set_id); +CREATE INDEX IF NOT EXISTS idx_experiment_folder_member_experiment + ON experiment_folder_member(experiment_uuid); From c919dfb1c871584d31d7398522712a2bcfabb3dd Mon Sep 17 00:00:00 2001 From: KFilippopolitis Date: Tue, 15 Sep 2026 14:24:11 +0300 Subject: [PATCH 04/10] feat(folder): add experiment folders API, service and DTOs Expose /experiment-folders for folders, sets and memberships, scoped to the active user with 404 for another owner's ids and 409 for duplicate names. Reuse one experiment access check from ExperimentService, and map conflicts through ControllerExceptionHandler. --- .../hbp/mip/experiment/ExperimentService.java | 15 +- .../folder/AddExperimentFoldersMemberDTO.java | 5 + .../mip/folder/CreateExperimentFolderDTO.java | 10 + .../mip/folder/CreateExperimentSetDTO.java | 9 + .../hbp/mip/folder/ExperimentFolderAPI.java | 181 +++++++ .../hbp/mip/folder/ExperimentFolderDTO.java | 38 ++ .../mip/folder/ExperimentFolderService.java | 496 ++++++++++++++++++ .../hbp/mip/folder/ExperimentFoldersDTO.java | 11 + .../java/hbp/mip/folder/ExperimentSetDTO.java | 27 + .../mip/folder/RenameExperimentFolderDTO.java | 5 + .../mip/folder/RenameExperimentSetDTO.java | 5 + .../UpdateExperimentSetMembershipDTO.java | 11 + .../mip/utils/ControllerExceptionHandler.java | 14 + .../utils/Exceptions/ConflictException.java | 12 + .../ExperimentFolderNotFoundException.java | 12 + .../ExperimentSetNotFoundException.java | 9 + 16 files changed, 859 insertions(+), 1 deletion(-) create mode 100644 src/main/java/hbp/mip/folder/AddExperimentFoldersMemberDTO.java create mode 100644 src/main/java/hbp/mip/folder/CreateExperimentFolderDTO.java create mode 100644 src/main/java/hbp/mip/folder/CreateExperimentSetDTO.java create mode 100644 src/main/java/hbp/mip/folder/ExperimentFolderAPI.java create mode 100644 src/main/java/hbp/mip/folder/ExperimentFolderDTO.java create mode 100644 src/main/java/hbp/mip/folder/ExperimentFolderService.java create mode 100644 src/main/java/hbp/mip/folder/ExperimentFoldersDTO.java create mode 100644 src/main/java/hbp/mip/folder/ExperimentSetDTO.java create mode 100644 src/main/java/hbp/mip/folder/RenameExperimentFolderDTO.java create mode 100644 src/main/java/hbp/mip/folder/RenameExperimentSetDTO.java create mode 100644 src/main/java/hbp/mip/folder/UpdateExperimentSetMembershipDTO.java create mode 100644 src/main/java/hbp/mip/utils/Exceptions/ConflictException.java create mode 100644 src/main/java/hbp/mip/utils/Exceptions/ExperimentFolderNotFoundException.java create mode 100644 src/main/java/hbp/mip/utils/Exceptions/ExperimentSetNotFoundException.java diff --git a/src/main/java/hbp/mip/experiment/ExperimentService.java b/src/main/java/hbp/mip/experiment/ExperimentService.java index 0eb66fe6b..ec2273a58 100644 --- a/src/main/java/hbp/mip/experiment/ExperimentService.java +++ b/src/main/java/hbp/mip/experiment/ExperimentService.java @@ -105,10 +105,23 @@ private ExperimentsDTO createExperimentsDTO(Page pageExperiments) } public ExperimentDTO getExperiment(Authentication authentication, String uuid, Logger logger) { + ExperimentDAO experimentDAO = assertExperimentAccessible(authentication, uuid, logger); + + return new ExperimentDTO(experimentDAO, true); + } + + /** + * Loads an experiment and refuses it when the active user may not read it, returning the entity so + * other features can reference a run they are allowed to see. + * + * Feature code that only needs to know "is this run usable by this user" should call this instead + * of reopening the access rules, which live in one place on purpose. + */ + public ExperimentDAO assertExperimentAccessible(Authentication authentication, String uuid, Logger logger) { ExperimentDAO experimentDAO = experimentRepository.loadExperiment(uuid, logger); validateExperimentAccess(authentication, experimentDAO, uuid, logger); - return new ExperimentDTO(experimentDAO, true); + return experimentDAO; } private void validateExperimentAccess(Authentication authentication, ExperimentDAO experimentDAO, String uuid, diff --git a/src/main/java/hbp/mip/folder/AddExperimentFoldersMemberDTO.java b/src/main/java/hbp/mip/folder/AddExperimentFoldersMemberDTO.java new file mode 100644 index 000000000..dcd063adb --- /dev/null +++ b/src/main/java/hbp/mip/folder/AddExperimentFoldersMemberDTO.java @@ -0,0 +1,5 @@ +package hbp.mip.folder; + +/** Request body of "add a run to a folder". Re-adding an existing member is a no-op, not an error. */ +public record AddExperimentFoldersMemberDTO(String experimentUuid) { +} diff --git a/src/main/java/hbp/mip/folder/CreateExperimentFolderDTO.java b/src/main/java/hbp/mip/folder/CreateExperimentFolderDTO.java new file mode 100644 index 000000000..44483198c --- /dev/null +++ b/src/main/java/hbp/mip/folder/CreateExperimentFolderDTO.java @@ -0,0 +1,10 @@ +package hbp.mip.folder; + +/** + * Request body of "create folder". The id and the order are assigned by the service. + * + * The experiment is optional, exactly like a set: "put this run in a new folder" is one gesture in the + * row menu, and one request means a failed second call can never leave an empty folder behind. + */ +public record CreateExperimentFolderDTO(String name, String experimentUuid) { +} diff --git a/src/main/java/hbp/mip/folder/CreateExperimentSetDTO.java b/src/main/java/hbp/mip/folder/CreateExperimentSetDTO.java new file mode 100644 index 000000000..34d140fe6 --- /dev/null +++ b/src/main/java/hbp/mip/folder/CreateExperimentSetDTO.java @@ -0,0 +1,9 @@ +package hbp.mip.folder; + +/** + * Request body of "create set". The experiment is optional: a set is often created empty and filled + * later. When it is given it joins the folder and the new set in the same request, which is what the + * "select runs, group them" gesture needs to avoid a second round trip. + */ +public record CreateExperimentSetDTO(String name, String experimentUuid) { +} diff --git a/src/main/java/hbp/mip/folder/ExperimentFolderAPI.java b/src/main/java/hbp/mip/folder/ExperimentFolderAPI.java new file mode 100644 index 000000000..f590c5974 --- /dev/null +++ b/src/main/java/hbp/mip/folder/ExperimentFolderAPI.java @@ -0,0 +1,181 @@ +package hbp.mip.folder; + +import hbp.mip.user.ActiveUserService; +import hbp.mip.utils.JsonConverters; +import hbp.mip.utils.Logger; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.security.core.Authentication; +import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PatchMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; + +/** + * Folders ("analysis sets") and the sets inside them, for the active user only. + * + * Routes sit under the /services servlet context, so the public paths are + * /services/experiment-folders/.... + * + * Statuses: 200 for reads and for every mutation that leaves a folder behind — the updated folder is + * the body, so the caller never has to guess at ordering or partitioning — 201 for a new folder or set, + * 204 only when the folder itself is gone, 400 for a blank or malformed name/id, 404 when the folder or + * set is not the caller's, 409 for a duplicate name, 401 when the run cannot be read. + */ +@RestController +@RequestMapping(value = "/experiment-folders", produces = { APPLICATION_JSON_VALUE }) +public class ExperimentFolderAPI { + + private final ExperimentFolderService experimentFolderService; + private final ActiveUserService activeUserService; + + public ExperimentFolderAPI(ExperimentFolderService experimentFolderService, + ActiveUserService activeUserService) { + this.experimentFolderService = experimentFolderService; + this.activeUserService = activeUserService; + } + + @GetMapping + public ResponseEntity getFolders(Authentication authentication) { + var logger = logger(authentication, "(GET) /experiment-folders"); + logger.info("Request for the experiment folders of the active user."); + var folders = experimentFolderService.getFolders(authentication, logger); + logger.info("Experiment folders returned: " + folders.folders().size()); + return new ResponseEntity<>(folders, HttpStatus.OK); + } + + @PostMapping + public ResponseEntity createFolder(Authentication authentication, + @RequestBody CreateExperimentFolderDTO createExperimentFolderDTO) { + var logger = logger(authentication, "(POST) /experiment-folders"); + logger.info("Request for experiment folder creation. RequestBody: " + + JsonConverters.convertObjectToJsonString(createExperimentFolderDTO)); + var folder = experimentFolderService.createFolder(authentication, createExperimentFolderDTO, logger); + logger.info("Experiment folder created with id: " + folder.id()); + return new ResponseEntity<>(folder, HttpStatus.CREATED); + } + + @GetMapping(value = "/{folderId}") + public ResponseEntity getFolder(Authentication authentication, + @PathVariable("folderId") String folderId) { + var logger = logger(authentication, "(GET) /experiment-folders/" + folderId); + logger.info("Request for experiment folder with id: " + folderId); + var folder = experimentFolderService.getFolder(authentication, folderId, logger); + logger.info("Experiment folder returned."); + return new ResponseEntity<>(folder, HttpStatus.OK); + } + + @PatchMapping(value = "/{folderId}") + public ResponseEntity renameFolder(Authentication authentication, + @PathVariable("folderId") String folderId, + @RequestBody RenameExperimentFolderDTO renameExperimentFolderDTO) { + var logger = logger(authentication, "(PATCH) /experiment-folders/" + folderId); + logger.info("Request for experiment folder rename. RequestBody: " + + JsonConverters.convertObjectToJsonString(renameExperimentFolderDTO)); + var folder = experimentFolderService.renameFolder(authentication, folderId, renameExperimentFolderDTO, logger); + logger.info("Experiment folder renamed. Id: " + folderId); + return new ResponseEntity<>(folder, HttpStatus.OK); + } + + @DeleteMapping(value = "/{folderId}") + public ResponseEntity deleteFolder(Authentication authentication, + @PathVariable("folderId") String folderId) { + var logger = logger(authentication, "(DELETE) /experiment-folders/" + folderId); + logger.info("Request for experiment folder deletion with id: " + folderId); + experimentFolderService.deleteFolder(authentication, folderId, logger); + logger.info("Experiment folder deleted. Id: " + folderId); + return new ResponseEntity<>(HttpStatus.NO_CONTENT); + } + + /** Adding a run the folder already holds answers 200 with the folder, not 409. */ + @PostMapping(value = "/{folderId}/members") + public ResponseEntity addExperiment(Authentication authentication, + @PathVariable("folderId") String folderId, + @RequestBody AddExperimentFoldersMemberDTO addExperimentFoldersMemberDTO) { + var logger = logger(authentication, "(POST) /experiment-folders/" + folderId + "/members"); + logger.info("Request for experiment addition to a folder. RequestBody: " + + JsonConverters.convertObjectToJsonString(addExperimentFoldersMemberDTO)); + var folder = experimentFolderService.addExperiment(authentication, folderId, addExperimentFoldersMemberDTO, + logger); + logger.info("Experiment added to the folder."); + return new ResponseEntity<>(folder, HttpStatus.OK); + } + + /** Answers with the folder rather than 204: losing a member changes the member list and a set. */ + @DeleteMapping(value = "/{folderId}/members/{experimentUuid}") + public ResponseEntity removeExperiment(Authentication authentication, + @PathVariable("folderId") String folderId, + @PathVariable("experimentUuid") String experimentUuid) { + var logger = logger(authentication, "(DELETE) /experiment-folders/" + folderId + "/members/" + experimentUuid); + logger.info("Request to remove experiment " + experimentUuid + " from folder " + folderId); + var folder = experimentFolderService.removeExperiment(authentication, folderId, experimentUuid, logger); + logger.info("Experiment removed from the folder."); + return new ResponseEntity<>(folder, HttpStatus.OK); + } + + @PostMapping(value = "/{folderId}/sets") + public ResponseEntity createSet(Authentication authentication, + @PathVariable("folderId") String folderId, + @RequestBody CreateExperimentSetDTO createExperimentSetDTO) { + var logger = logger(authentication, "(POST) /experiment-folders/" + folderId + "/sets"); + logger.info("Request for experiment set creation. RequestBody: " + + JsonConverters.convertObjectToJsonString(createExperimentSetDTO)); + var folder = experimentFolderService.createSet(authentication, folderId, createExperimentSetDTO, logger); + logger.info("Experiment set created in folder: " + folderId); + return new ResponseEntity<>(folder, HttpStatus.CREATED); + } + + @PatchMapping(value = "/{folderId}/sets/{setId}") + public ResponseEntity renameSet(Authentication authentication, + @PathVariable("folderId") String folderId, + @PathVariable("setId") String setId, + @RequestBody RenameExperimentSetDTO renameExperimentSetDTO) { + var logger = logger(authentication, "(PATCH) /experiment-folders/" + folderId + "/sets/" + setId); + logger.info("Request for experiment set rename. RequestBody: " + + JsonConverters.convertObjectToJsonString(renameExperimentSetDTO)); + var folder = experimentFolderService.renameSet(authentication, folderId, setId, renameExperimentSetDTO, logger); + logger.info("Experiment set renamed. Id: " + setId); + return new ResponseEntity<>(folder, HttpStatus.OK); + } + + /** Answers with the folder rather than 204: the retired set moved its runs into Ungrouped. */ + @DeleteMapping(value = "/{folderId}/sets/{setId}") + public ResponseEntity deleteSet(Authentication authentication, + @PathVariable("folderId") String folderId, + @PathVariable("setId") String setId) { + var logger = logger(authentication, "(DELETE) /experiment-folders/" + folderId + "/sets/" + setId); + logger.info("Request for experiment set deletion with id: " + setId); + var folder = experimentFolderService.deleteSet(authentication, folderId, setId, logger); + logger.info("Experiment set deleted. Its runs were left in the folder, ungrouped."); + return new ResponseEntity<>(folder, HttpStatus.OK); + } + + /** + * Moves a run into a set — or, with a null setId, out of its set and back to ungrouped. The run + * stays in the folder either way. + */ + @PatchMapping(value = "/{folderId}/members/{experimentUuid}/set") + public ResponseEntity updateSetMembership(Authentication authentication, + @PathVariable("folderId") String folderId, + @PathVariable("experimentUuid") String experimentUuid, + @RequestBody UpdateExperimentSetMembershipDTO updateExperimentSetMembershipDTO) { + var logger = logger(authentication, + "(PATCH) /experiment-folders/" + folderId + "/members/" + experimentUuid + "/set"); + logger.info("Request to change the set of an experiment. RequestBody: " + + JsonConverters.convertObjectToJsonString(updateExperimentSetMembershipDTO)); + var folder = experimentFolderService.updateSetMembership(authentication, folderId, experimentUuid, + updateExperimentSetMembershipDTO, logger); + logger.info("Set membership updated."); + return new ResponseEntity<>(folder, HttpStatus.OK); + } + + private Logger logger(Authentication authentication, String endpoint) { + return new Logger(activeUserService.getActiveUser(authentication).username(), endpoint); + } +} diff --git a/src/main/java/hbp/mip/folder/ExperimentFolderDTO.java b/src/main/java/hbp/mip/folder/ExperimentFolderDTO.java new file mode 100644 index 000000000..fd25d9a24 --- /dev/null +++ b/src/main/java/hbp/mip/folder/ExperimentFolderDTO.java @@ -0,0 +1,38 @@ +package hbp.mip.folder; + +import java.util.Comparator; +import java.util.List; + +/** + * Response shape of a folder. Matches the frontend ExperimentFolder model exactly, so swapping the + * localStorage service for this API is a transport change and not a model change. + * + * experimentIds is ordered by folder_position, sets by sort_order, and each set's experimentIds by + * set_position. Members in no set appear only in experimentIds — that is the implicit Ungrouped + * group the compare workspace derives. + */ +public record ExperimentFolderDTO( + String id, + String name, + List experimentIds, + List sets) { + + public static ExperimentFolderDTO from(ExperimentFolderDAO folder) { + List members = folder.getMembers().stream() + .sorted(Comparator.comparing(ExperimentFolderMemberDAO::getFolderPosition, + Comparator.nullsLast(Comparator.naturalOrder()))) + .toList(); + + List sets = folder.getSets().stream() + .sorted(Comparator.comparing(ExperimentSetDAO::getSortOrder, + Comparator.nullsLast(Comparator.naturalOrder()))) + .map(set -> ExperimentSetDTO.from(set, members)) + .toList(); + + return new ExperimentFolderDTO( + folder.getId().toString(), + folder.getName(), + members.stream().map(member -> member.getExperiment().getUuid().toString()).toList(), + sets); + } +} diff --git a/src/main/java/hbp/mip/folder/ExperimentFolderService.java b/src/main/java/hbp/mip/folder/ExperimentFolderService.java new file mode 100644 index 000000000..cce11c645 --- /dev/null +++ b/src/main/java/hbp/mip/folder/ExperimentFolderService.java @@ -0,0 +1,496 @@ +package hbp.mip.folder; + +import hbp.mip.experiment.ExperimentDAO; +import hbp.mip.experiment.ExperimentService; +import hbp.mip.user.ActiveUserService; +import hbp.mip.user.UserDAO; +import hbp.mip.utils.Exceptions.BadRequestException; +import hbp.mip.utils.Exceptions.ConflictException; +import hbp.mip.utils.Exceptions.ExperimentFolderNotFoundException; +import hbp.mip.utils.Exceptions.ExperimentSetNotFoundException; +import hbp.mip.utils.Logger; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.security.core.Authentication; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import java.sql.SQLException; +import java.util.Collection; +import java.util.Comparator; +import java.util.List; +import java.util.Locale; +import java.util.Objects; +import java.util.Optional; +import java.util.UUID; + +/** + * Owns experiment folders and the sets inside them. + * + * Two rules hold everywhere and are the reason this class exists at all: + * + * 1. Everything is scoped to the active user. A folder id that is not the caller's is a 404, never a + * 403, so the API never confirms that somebody else's id exists. + * 2. A folder holds experiment ids, never experiment copies. Membership is the only thing persisted + * here, so a renamed or re-shared run cannot go stale inside a folder. + * + * Name handling mirrors the frontend rules the folder chips already enforce (collapse whitespace, + * trim, 60 chars, case-insensitive uniqueness) so moving the store behind this API does not change + * what a user can type. + */ +@Service +public class ExperimentFolderService { + + static final int MAX_NAME_LENGTH = 60; + + private final ActiveUserService activeUserService; + private final ExperimentFolderRepository folderRepository; + private final ExperimentService experimentService; + + public ExperimentFolderService(ActiveUserService activeUserService, + ExperimentFolderRepository folderRepository, + ExperimentService experimentService) { + this.activeUserService = activeUserService; + this.folderRepository = folderRepository; + this.experimentService = experimentService; + } + + // ------------------------------------------------------------------ folders + + @Transactional(readOnly = true) + public ExperimentFoldersDTO getFolders(Authentication authentication, Logger logger) { + var user = activeUserService.getActiveUser(authentication); + List folders = folderRepository.findOwnedFolders(user.username()); + logger.debug("Owned experiment folders found: " + folders.size()); + return ExperimentFoldersDTO.from(folders); + } + + @Transactional(readOnly = true) + public ExperimentFolderDTO getFolder(Authentication authentication, String folderId, Logger logger) { + var user = activeUserService.getActiveUser(authentication); + return ExperimentFolderDTO.from(ownedFolder(folderId, user.username(), logger)); + } + + @Transactional + public ExperimentFolderDTO createFolder(Authentication authentication, CreateExperimentFolderDTO request, + Logger logger) { + var user = activeUserService.getActiveUser(authentication); + String name = requireName(request == null ? null : request.name(), "Folder name", logger); + + List ownedFolders = folderRepository.findOwnedFolders(user.username()); + rejectDuplicateFolderName(name, ownedFolders, null, logger); + + ExperimentFolderDAO folder = new ExperimentFolderDAO(new UserDAO(user), name, + nextPosition(ownedFolders.stream().map(ExperimentFolderDAO::getSortOrder).toList())); + + // "Put this run in a new folder" is one gesture in the row menu, so it is one request: a second + // call that fails would otherwise leave an empty folder the user never asked for. + String requestedExperiment = request == null ? null : request.experimentUuid(); + if (!isBlank(requestedExperiment)) { + memberFor(folder, experimentService.assertExperimentAccessible(authentication, requestedExperiment, logger), + logger); + } + + ExperimentFolderDAO saved = save(folder, "An experiment folder with that name already exists.", logger); + logger.info("Experiment folder created with id: " + saved.getId()); + return ExperimentFolderDTO.from(saved); + } + + @Transactional + public ExperimentFolderDTO renameFolder(Authentication authentication, String folderId, + RenameExperimentFolderDTO request, Logger logger) { + var user = activeUserService.getActiveUser(authentication); + ExperimentFolderDAO folder = ownedFolder(folderId, user.username(), logger); + + String name = requireName(request == null ? null : request.name(), "Folder name", logger); + rejectDuplicateFolderName(name, folderRepository.findOwnedFolders(user.username()), folder, logger); + + folder.setName(name); + + ExperimentFolderDAO saved = save(folder, "An experiment folder with that name already exists.", logger); + logger.info("Experiment folder renamed. Id: " + folder.getId()); + return ExperimentFolderDTO.from(saved); + } + + /** Deleting a folder retires the grouping only. The runs themselves are untouched. */ + @Transactional + public void deleteFolder(Authentication authentication, String folderId, Logger logger) { + var user = activeUserService.getActiveUser(authentication); + ExperimentFolderDAO folder = ownedFolder(folderId, user.username(), logger); + + folderRepository.delete(folder); + logger.info("Experiment folder deleted. Id: " + folder.getId()); + } + + // ------------------------------------------------------------------ members + + /** + * Adding a run the folder already holds returns the folder unchanged: the row menu can toggle blindly. + * + * The folder row is locked first, so a second add of the same run waits, then sees the membership and + * returns the folder; the member unique constraint is a backstop, not the path that decides this. + */ + @Transactional + public ExperimentFolderDTO addExperiment(Authentication authentication, String folderId, + AddExperimentFoldersMemberDTO request, Logger logger) { + var user = activeUserService.getActiveUser(authentication); + ExperimentFolderDAO folder = ownedFolderForUpdate(folderId, user.username(), logger); + + String experimentUuid = request == null ? null : request.experimentUuid(); + if (isBlank(experimentUuid)) { + String errorMessage = "experimentUuid must be provided."; + logger.warn(errorMessage); + throw new BadRequestException(errorMessage); + } + ExperimentDAO experiment = experimentService.assertExperimentAccessible(authentication, experimentUuid.trim(), + logger); + + if (memberOf(folder, experiment.getUuid()) != null) { + logger.info("Experiment already belongs to the folder. Id: " + experiment.getUuid()); + return ExperimentFolderDTO.from(folder); + } + + memberFor(folder, experiment, logger); + + ExperimentFolderDAO saved = save(folder, "That experiment already belongs to this folder.", logger); + logger.info("Experiment added to folder. Experiment id: " + experiment.getUuid()); + return ExperimentFolderDTO.from(saved); + } + + /** Leaving the folder ends the membership outright: a set cannot keep a run the folder lost. */ + @Transactional + public ExperimentFolderDTO removeExperiment(Authentication authentication, String folderId, String experimentUuid, + Logger logger) { + var user = activeUserService.getActiveUser(authentication); + ExperimentFolderDAO folder = ownedFolderForUpdate(folderId, user.username(), logger); + + UUID experimentId = parseUuid(experimentUuid, "experimentUuid", logger); + ExperimentFolderMemberDAO member = memberOf(folder, experimentId); + if (member == null) { + logger.info("Experiment was not a member of the folder. Id: " + experimentUuid); + return ExperimentFolderDTO.from(folder); + } + + folder.getMembers().remove(member); + ExperimentFolderDAO saved = save(folder, "That experiment already belongs to this folder.", logger); + logger.info("Experiment removed from folder. Experiment id: " + experimentUuid); + return ExperimentFolderDTO.from(saved); + } + + // ------------------------------------------------------------------ sets + + @Transactional + public ExperimentFolderDTO createSet(Authentication authentication, String folderId, + CreateExperimentSetDTO request, Logger logger) { + var user = activeUserService.getActiveUser(authentication); + ExperimentFolderDAO folder = ownedFolderForUpdate(folderId, user.username(), logger); + + String name = requireName(request == null ? null : request.name(), "Set name", logger); + rejectDuplicateSetName(name, folder, null, logger); + + ExperimentSetDAO set = new ExperimentSetDAO(folder, name, + nextPosition(folder.getSets().stream().map(ExperimentSetDAO::getSortOrder).toList())); + folder.getSets().add(set); + + String requestedExperiment = request == null ? null : request.experimentUuid(); + if (!isBlank(requestedExperiment)) { + ExperimentDAO experiment = experimentService.assertExperimentAccessible(authentication, requestedExperiment, logger); + joinSet(folder, memberFor(folder, experiment, logger), set); + } + + ExperimentFolderDAO saved = save(folder, "An experiment set with that name already exists in this folder.", logger); + logger.info("Experiment set created with id: " + set.getId()); + return ExperimentFolderDTO.from(saved); + } + + @Transactional + public ExperimentFolderDTO renameSet(Authentication authentication, String folderId, String setId, + RenameExperimentSetDTO request, Logger logger) { + var user = activeUserService.getActiveUser(authentication); + ExperimentFolderDAO folder = ownedFolder(folderId, user.username(), logger); + ExperimentSetDAO set = ownedSet(folder, setId, logger); + + String name = requireName(request == null ? null : request.name(), "Set name", logger); + rejectDuplicateSetName(name, folder, set, logger); + + set.setName(name); + + ExperimentFolderDAO saved = save(folder, "An experiment set with that name already exists in this folder.", logger); + logger.info("Experiment set renamed. Id: " + set.getId()); + return ExperimentFolderDTO.from(saved); + } + + /** Deleting a set retires the name, never the runs: its members fall back to ungrouped. */ + @Transactional + public ExperimentFolderDTO deleteSet(Authentication authentication, String folderId, String setId, + Logger logger) { + var user = activeUserService.getActiveUser(authentication); + ExperimentFolderDAO folder = ownedFolderForUpdate(folderId, user.username(), logger); + ExperimentSetDAO set = ownedSet(folder, setId, logger); + + // The database would settle this with ON DELETE SET NULL, but the flushed order between the + // set removal and the member updates is not ours to bet on: null them here so the in-memory + // state and the row agree either way. + folder.getMembers().stream() + .filter(member -> isInSet(member, set)) + .forEach(this::ungroup); + + folder.getSets().remove(set); + ExperimentFolderDAO saved = save(folder, "An experiment set with that name already exists in this folder.", logger); + logger.info("Experiment set deleted. Runs were left in the folder, ungrouped. Set id: " + setId); + return ExperimentFolderDTO.from(saved); + } + + /** + * The strict partition: moving a run in files it nowhere else, so the previous set gives it up by + * itself — one row per (folder, experiment) means there is no second row to clean. + * + * A null or blank setId is the way out of a set that is not "leave the folder": the run stays in + * the folder and becomes ungrouped. + */ + @Transactional + public ExperimentFolderDTO updateSetMembership(Authentication authentication, String folderId, String experimentUuid, + UpdateExperimentSetMembershipDTO request, Logger logger) { + var user = activeUserService.getActiveUser(authentication); + ExperimentFolderDAO folder = ownedFolderForUpdate(folderId, user.username(), logger); + + UUID experimentId = parseUuid(experimentUuid, "experimentUuid", logger); + ExperimentFolderMemberDAO existingMember = memberOf(folder, experimentId); + + String requestedSetId = request == null ? null : request.setId(); + if (isBlank(requestedSetId)) { + if (existingMember == null || (existingMember.getExperimentSet() == null + && existingMember.getSetPosition() == null)) { + logger.info("Experiment is already ungrouped in the folder. Id: " + experimentUuid); + return ExperimentFolderDTO.from(folder); + } + + ungroup(existingMember); + ExperimentFolderDAO ungrouped = save(folder, "That experiment already belongs to this folder.", logger); + logger.info("Experiment left its set and stayed in the folder. Id: " + experimentUuid); + return ExperimentFolderDTO.from(ungrouped); + } + + ExperimentSetDAO target = ownedSet(folder, requestedSetId, logger); + ExperimentFolderMemberDAO member = existingMember != null + ? existingMember + : memberFor(folder, experimentService.assertExperimentAccessible(authentication, experimentUuid, logger), logger); + + if (isInSet(member, target)) { + logger.info("Experiment is already in the requested set. Id: " + experimentUuid); + return ExperimentFolderDTO.from(folder); + } + + joinSet(folder, member, target); + ExperimentFolderDAO saved = save(folder, "That experiment already belongs to this folder.", logger); + logger.info("Experiment moved to set " + target.getId() + ". Experiment id: " + experimentUuid); + return ExperimentFolderDTO.from(saved); + } + + // ------------------------------------------------------------------ shared rules + + /** + * A folder the caller may touch, or nothing. A folder that exists under another owner and a folder + * that does not exist answer the same 404, so the API never confirms somebody else's id. + */ + private ExperimentFolderDAO ownedFolder(String folderId, String username, Logger logger) { + return ownedFolder(folderRepository.findById(parseUuid(folderId, "folderId", logger)), folderId, username, + logger); + } + + /** + * The same folder, write-locked for the rest of the transaction. Membership writes take this before + * they look at {@code members}: two concurrent add/move calls on one folder then queue on the row + * lock, so the second sees the first's membership and no duplicate row is ever attempted. + * + * The lock itself needs the transaction the service methods already declare. + */ + private ExperimentFolderDAO ownedFolderForUpdate(String folderId, String username, Logger logger) { + return ownedFolder(folderRepository.findByIdForUpdate(parseUuid(folderId, "folderId", logger)), folderId, + username, logger); + } + + private ExperimentFolderDAO ownedFolder(Optional candidate, String folderId, String username, + Logger logger) { + ExperimentFolderDAO folder = candidate + .filter(candidateFolder -> candidateFolder.getOwner() != null + && username.equals(candidateFolder.getOwner().getUsername())) + .orElse(null); + + if (folder == null) { + var errorMessage = "Experiment folder with id : " + folderId + " was not found for the active user."; + logger.warn(errorMessage); + throw new ExperimentFolderNotFoundException(errorMessage); + } + + return folder; + } + + /** + * Writes and flushes in one go, then translates the database's own uniqueness backstop into the + * 409 the caller already got from the pre-check. Two tabs creating the same folder name both pass + * the service check; whichever insert loses must hear "conflict", not "internal server error". + * Only unique violations are translated — a NOT NULL or FK failure is still our bug and stays a 500. + */ + private ExperimentFolderDAO save(ExperimentFolderDAO folder, String duplicateMessage, Logger logger) { + try { + return folderRepository.saveAndFlush(folder); + } catch (DataIntegrityViolationException failure) { + if (!isUniqueViolation(failure)) { + throw failure; + } + logger.warn(duplicateMessage); + throw new ConflictException(duplicateMessage); + } + } + + /** PostgreSQL reports unique violations as SQLSTATE 23505; anything else (NOT NULL, FK) stays a 500. */ + private boolean isUniqueViolation(Throwable failure) { + for (Throwable cause = failure; cause != null; cause = cause.getCause()) { + if (cause instanceof SQLException sql) { + return "23505".equals(sql.getSQLState()); + } + } + return false; + } + + private ExperimentFolderMemberDAO memberOf(ExperimentFolderDAO folder, UUID experimentUuid) { + return folder.getMembers().stream() + .filter(member -> member.getExperiment() != null + && experimentUuid.equals(member.getExperiment().getUuid())) + .findFirst() + .orElse(null); + } + + private ExperimentSetDAO ownedSet(ExperimentFolderDAO folder, String setId, Logger logger) { + UUID parsed = parseUuid(setId, "setId", logger); + ExperimentSetDAO set = folder.getSets().stream() + .filter(candidate -> parsed.equals(candidate.getId())) + .findFirst() + .orElse(null); + if (set == null) { + var errorMessage = "Experiment set with id : " + setId + " was not found in folder : " + folder.getId(); + logger.warn(errorMessage); + throw new ExperimentSetNotFoundException(errorMessage); + } + return set; + } + + /** A set only ever holds folder members, so moving in joins the folder first when it has to. */ + private ExperimentFolderMemberDAO memberFor(ExperimentFolderDAO folder, ExperimentDAO experiment, Logger logger) { + ExperimentFolderMemberDAO member = memberOf(folder, experiment.getUuid()); + if (member == null) { + member = new ExperimentFolderMemberDAO(folder, experiment, + nextPosition(folderPositions(folder))); + folder.getMembers().add(member); + logger.debug("Folder membership created while placing the run in a set."); + } + return member; + } + + private void joinSet(ExperimentFolderDAO folder, ExperimentFolderMemberDAO member, ExperimentSetDAO target) { + member.setExperimentSet(target); + + // The run being moved is excluded from the max: it cannot count the position it is leaving. + member.setSetPosition(nextPosition(folder.getMembers().stream() + .filter(other -> !Objects.equals(other, member)) + .filter(other -> isInSet(other, target)) + .map(ExperimentFolderMemberDAO::getSetPosition) + .toList())); + } + + private List folderPositions(ExperimentFolderDAO folder) { + return folder.getMembers().stream() + .map(ExperimentFolderMemberDAO::getFolderPosition) + .toList(); + } + + /** Id comparison, not equals: one side may still be an uninitialized association proxy. */ + private boolean isInSet(ExperimentFolderMemberDAO member, ExperimentSetDAO set) { + return member.getExperimentSet() != null + && set.getId() != null + && set.getId().equals(member.getExperimentSet().getId()); + } + + private void ungroup(ExperimentFolderMemberDAO member) { + member.setExperimentSet(null); + member.setSetPosition(null); + } + + /** New entries land last, in whichever axis is being appended to: first is 1, then max + 1. */ + private Integer nextPosition(Collection positions) { + return positions.stream() + .filter(Objects::nonNull) + .max(Comparator.naturalOrder()) + .map(last -> last + 1) + .orElse(1); + } + + private String requireName(String rawName, String field, Logger logger) { + String name = normalizeName(rawName); + if (name.isEmpty()) { + String errorMessage = field + " must not be blank."; + logger.warn(errorMessage); + throw new BadRequestException(errorMessage); + } + return name; + } + + /** Whitespace runs collapse to one space, the ends are trimmed, and 60 chars is all that fits. */ + private String normalizeName(String rawName) { + if (rawName == null) { + return ""; + } + String cleaned = rawName.replaceAll("\\s+", " ").trim(); + return cleaned.length() <= MAX_NAME_LENGTH ? cleaned : cleaned.substring(0, MAX_NAME_LENGTH); + } + + /** Comparison key only — the stored name keeps the casing the user typed. */ + private String nameKey(String name) { + return normalizeName(name).toLowerCase(Locale.ROOT); + } + + private void rejectDuplicateFolderName(String name, List ownedFolders, + ExperimentFolderDAO renamed, Logger logger) { + boolean taken = ownedFolders.stream() + .filter(folder -> renamed == null || !folder.getId().equals(renamed.getId())) + .anyMatch(folder -> nameKey(folder.getName()).equals(nameKey(name))); + + if (taken) { + String errorMessage = "An experiment folder named '" + name + "' already exists."; + logger.warn(errorMessage); + throw new ConflictException(errorMessage); + } + } + + /** Sets are their own namespace: a set may share its folder's name, but not a sibling's. */ + private void rejectDuplicateSetName(String name, ExperimentFolderDAO folder, ExperimentSetDAO renamed, + Logger logger) { + boolean taken = folder.getSets().stream() + .filter(set -> renamed == null || !set.getId().equals(renamed.getId())) + .anyMatch(set -> nameKey(set.getName()).equals(nameKey(name))); + + if (taken) { + String errorMessage = "An experiment set named '" + name + "' already exists in this folder."; + logger.warn(errorMessage); + throw new ConflictException(errorMessage); + } + } + + private UUID parseUuid(String value, String field, Logger logger) { + if (isBlank(value)) { + String errorMessage = field + " must be provided."; + logger.warn(errorMessage); + throw new BadRequestException(errorMessage); + } + try { + return UUID.fromString(value.trim()); + } catch (IllegalArgumentException e) { + logger.error("Conversion of string to UUID failed: " + e.getMessage()); + throw new BadRequestException("Invalid " + field + ": " + value); + } + } + + private boolean isBlank(String value) { + return value == null || value.trim().isEmpty(); + } +} diff --git a/src/main/java/hbp/mip/folder/ExperimentFoldersDTO.java b/src/main/java/hbp/mip/folder/ExperimentFoldersDTO.java new file mode 100644 index 000000000..69aed3f39 --- /dev/null +++ b/src/main/java/hbp/mip/folder/ExperimentFoldersDTO.java @@ -0,0 +1,11 @@ +package hbp.mip.folder; + +import java.util.List; + +/** Response shape of the collection endpoint. An owner with no folders gets an empty list, not a 404. */ +public record ExperimentFoldersDTO(List folders) { + + public static ExperimentFoldersDTO from(List folders) { + return new ExperimentFoldersDTO(folders.stream().map(ExperimentFolderDTO::from).toList()); + } +} diff --git a/src/main/java/hbp/mip/folder/ExperimentSetDTO.java b/src/main/java/hbp/mip/folder/ExperimentSetDTO.java new file mode 100644 index 000000000..8069a4c26 --- /dev/null +++ b/src/main/java/hbp/mip/folder/ExperimentSetDTO.java @@ -0,0 +1,27 @@ +package hbp.mip.folder; + +import java.util.Comparator; +import java.util.List; + +/** + * Response shape of a set. Matches the frontend ExperimentSet model: id, name, member ids only — + * never experiment copies, so a folder can never serve a stale run. + * + * experimentIds is ordered by set_position; nulls sort last so an unpositioned row cannot scramble + * the section. + */ +public record ExperimentSetDTO(String id, String name, List experimentIds) { + + public static ExperimentSetDTO from(ExperimentSetDAO set, List members) { + List memberIds = members.stream() + .filter(member -> member.getExperimentSet() != null + && member.getExperimentSet().getId() != null + && member.getExperimentSet().getId().equals(set.getId())) + .sorted(Comparator.comparing(ExperimentFolderMemberDAO::getSetPosition, + Comparator.nullsLast(Comparator.naturalOrder()))) + .map(member -> member.getExperiment().getUuid().toString()) + .toList(); + + return new ExperimentSetDTO(set.getId().toString(), set.getName(), memberIds); + } +} diff --git a/src/main/java/hbp/mip/folder/RenameExperimentFolderDTO.java b/src/main/java/hbp/mip/folder/RenameExperimentFolderDTO.java new file mode 100644 index 000000000..1d1520ac9 --- /dev/null +++ b/src/main/java/hbp/mip/folder/RenameExperimentFolderDTO.java @@ -0,0 +1,5 @@ +package hbp.mip.folder; + +/** Request body of "rename folder". Only the name is editable, and it follows the create rules. */ +public record RenameExperimentFolderDTO(String name) { +} diff --git a/src/main/java/hbp/mip/folder/RenameExperimentSetDTO.java b/src/main/java/hbp/mip/folder/RenameExperimentSetDTO.java new file mode 100644 index 000000000..454197914 --- /dev/null +++ b/src/main/java/hbp/mip/folder/RenameExperimentSetDTO.java @@ -0,0 +1,5 @@ +package hbp.mip.folder; + +/** Request body of "rename set". Only the name is editable; membership moves through set membership. */ +public record RenameExperimentSetDTO(String name) { +} diff --git a/src/main/java/hbp/mip/folder/UpdateExperimentSetMembershipDTO.java b/src/main/java/hbp/mip/folder/UpdateExperimentSetMembershipDTO.java new file mode 100644 index 000000000..93dec9211 --- /dev/null +++ b/src/main/java/hbp/mip/folder/UpdateExperimentSetMembershipDTO.java @@ -0,0 +1,11 @@ +package hbp.mip.folder; + +/** + * Request body of "which set holds this run". + * + * A null or blank setId means "ungroup": the run stays in the folder and leaves its set. One body + * covers both the move and the way out, and the strict partition means a move never needs to name + * the set it leaves. + */ +public record UpdateExperimentSetMembershipDTO(String setId) { +} diff --git a/src/main/java/hbp/mip/utils/ControllerExceptionHandler.java b/src/main/java/hbp/mip/utils/ControllerExceptionHandler.java index a86cfb492..8cbaaff36 100644 --- a/src/main/java/hbp/mip/utils/ControllerExceptionHandler.java +++ b/src/main/java/hbp/mip/utils/ControllerExceptionHandler.java @@ -28,6 +28,20 @@ public ResponseEntity handleExperimentNotFoundException(ExperimentNotFou return new ResponseEntity<>(message, HttpStatus.NOT_FOUND); } + // ExperimentFolderNotFoundException and ExperimentSetNotFoundException extend + // ExperimentNotFoundException, so the 404 handler above already covers them. + + @ExceptionHandler(ConflictException.class) + public ResponseEntity handleConflictException(ConflictException ex, WebRequest request) { + ErrorMessage message = new ErrorMessage( + HttpStatus.CONFLICT.value(), + new Date(), + ex.getMessage(), + request.getDescription(false)); + + return new ResponseEntity<>(message, HttpStatus.CONFLICT); + } + @ExceptionHandler(BadRequestException.class) public ResponseEntity handleBadRequestException(BadRequestException ex, WebRequest request) { ErrorMessage message = new ErrorMessage( diff --git a/src/main/java/hbp/mip/utils/Exceptions/ConflictException.java b/src/main/java/hbp/mip/utils/Exceptions/ConflictException.java new file mode 100644 index 000000000..e87603c8c --- /dev/null +++ b/src/main/java/hbp/mip/utils/Exceptions/ConflictException.java @@ -0,0 +1,12 @@ +package hbp.mip.utils.Exceptions; + +/** + * Thrown when the request is well formed but collides with an existing resource, such as a folder or + * set name already taken in the same namespace. Mapped to 409. + */ +public class ConflictException extends RuntimeException { + + public ConflictException(String msg) { + super(msg); + } +} diff --git a/src/main/java/hbp/mip/utils/Exceptions/ExperimentFolderNotFoundException.java b/src/main/java/hbp/mip/utils/Exceptions/ExperimentFolderNotFoundException.java new file mode 100644 index 000000000..ac28fd98c --- /dev/null +++ b/src/main/java/hbp/mip/utils/Exceptions/ExperimentFolderNotFoundException.java @@ -0,0 +1,12 @@ +package hbp.mip.utils.Exceptions; + +/** + * Thrown when the folder asked for does not exist or belongs to somebody else. Both answers map to + * 404 on purpose: the API must not tell a caller that another user's folder id is real. + */ +public class ExperimentFolderNotFoundException extends ExperimentNotFoundException { + + public ExperimentFolderNotFoundException(String msg) { + super(msg); + } +} diff --git a/src/main/java/hbp/mip/utils/Exceptions/ExperimentSetNotFoundException.java b/src/main/java/hbp/mip/utils/Exceptions/ExperimentSetNotFoundException.java new file mode 100644 index 000000000..e9e040159 --- /dev/null +++ b/src/main/java/hbp/mip/utils/Exceptions/ExperimentSetNotFoundException.java @@ -0,0 +1,9 @@ +package hbp.mip.utils.Exceptions; + +/** Thrown when the set asked for is not one of the folder's sets. Mapped to 404. */ +public class ExperimentSetNotFoundException extends ExperimentNotFoundException { + + public ExperimentSetNotFoundException(String msg) { + super(msg); + } +} From 2406b2fdbd8ef7f1c3e50185c5de46a1f4c79a30 Mon Sep 17 00:00:00 2001 From: KFilippopolitis Date: Tue, 15 Sep 2026 14:24:14 +0300 Subject: [PATCH 05/10] test(folder): cover folder service, DTO ordering and API contract Pin naming and ownership rules, idempotent membership, strict-partition set moves, ordering, the lost-race 409, the wire DTO shapes, and every documented status code. --- .../mip/folder/ExperimentFolderAPITest.java | 290 ++++++++ .../mip/folder/ExperimentFolderDTOTest.java | 85 +++ .../folder/ExperimentFolderServiceTest.java | 660 ++++++++++++++++++ 3 files changed, 1035 insertions(+) create mode 100644 src/test/java/hbp/mip/folder/ExperimentFolderAPITest.java create mode 100644 src/test/java/hbp/mip/folder/ExperimentFolderDTOTest.java create mode 100644 src/test/java/hbp/mip/folder/ExperimentFolderServiceTest.java diff --git a/src/test/java/hbp/mip/folder/ExperimentFolderAPITest.java b/src/test/java/hbp/mip/folder/ExperimentFolderAPITest.java new file mode 100644 index 000000000..89cced5a0 --- /dev/null +++ b/src/test/java/hbp/mip/folder/ExperimentFolderAPITest.java @@ -0,0 +1,290 @@ +package hbp.mip.folder; + +import hbp.mip.user.ActiveUserService; +import hbp.mip.user.UserDTO; +import hbp.mip.utils.ControllerExceptionHandler; +import hbp.mip.utils.Exceptions.BadRequestException; +import hbp.mip.utils.Exceptions.ConflictException; +import hbp.mip.utils.Exceptions.ExperimentFolderNotFoundException; +import hbp.mip.utils.Exceptions.ExperimentSetNotFoundException; +import hbp.mip.utils.Exceptions.UnauthorizedException; +import hbp.mip.utils.Logger; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.Authentication; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +/** + * The contract, as a test. Body field names, response field names, and the status each failure answers + * with are what the frontend codes against, and none of them are covered by service unit tests: those + * call the service directly and never see Jackson or {@code ControllerExceptionHandler}. + * + * Standalone MockMvc on purpose — no security context, no datasource. The active user is the request + * principal, which is also how the real request reaches the controller. + */ +@ExtendWith(MockitoExtension.class) +class ExperimentFolderAPITest { + + private static final String FOLDER_ID = "11111111-1111-1111-1111-111111111111"; + private static final String SET_ID = "22222222-2222-2222-2222-222222222222"; + private static final String RUN_ID = "33333333-3333-3333-3333-333333333333"; + + @Mock + private ExperimentFolderService service; + + @Mock + private ActiveUserService activeUserService; + + private final Authentication principal = new UsernamePasswordAuthenticationToken("user", null); + + private MockMvc mvc; + + @BeforeEach + void setUp() { + mvc = MockMvcBuilders + .standaloneSetup(new ExperimentFolderAPI(service, activeUserService)) + .setControllerAdvice(new ControllerExceptionHandler()) + .build(); + + lenient().when(activeUserService.getActiveUser(any())) + .thenReturn(new UserDTO("user", "User", "user@example.org", "subject", true)); + } + + // ------------------------------------------------------------------ happy path + + @Test + void listsTheActiveUsersFolders() throws Exception { + when(service.getFolders(any(), any(Logger.class))) + .thenReturn(new ExperimentFoldersDTO(List.of(folderDto()))); + + mvc.perform(get("/experiment-folders").principal(principal)) + .andExpect(status().isOk()) + .andExpect(content().contentTypeCompatibleWith("application/json")) + .andExpect(jsonPath("$.folders[0].id").value(FOLDER_ID)) + .andExpect(jsonPath("$.folders[0].name").value("My analysis set")) + .andExpect(jsonPath("$.folders[0].experimentIds[0]").value(RUN_ID)) + .andExpect(jsonPath("$.folders[0].sets[0].id").value(SET_ID)) + .andExpect(jsonPath("$.folders[0].sets[0].experimentIds[0]").value(RUN_ID)); + } + + @Test + void createsAFolderFromNameOnlyAndAnswersCreated() throws Exception { + when(service.createFolder(any(), any(CreateExperimentFolderDTO.class), any(Logger.class))) + .thenReturn(new ExperimentFolderDTO(FOLDER_ID, "My analysis set", List.of(), List.of())); + + mvc.perform(post("/experiment-folders").contentType("application/json").content("{\"name\":\"My analysis set\"}").principal(principal)) + .andExpect(status().isCreated()) + .andExpect(jsonPath("$.id").value(FOLDER_ID)) + .andExpect(jsonPath("$.experimentIds").isArray()) + .andExpect(jsonPath("$.sets").isArray()); + } + + @Test + void createFolderCarriesTheFirstRunUnderTheContractFieldName() throws Exception { + when(service.createFolder(any(), any(CreateExperimentFolderDTO.class), any(Logger.class))) + .thenReturn(folderDto()); + + mvc.perform(post("/experiment-folders").contentType("application/json") + .content("{\"name\":\"My analysis set\",\"experimentUuid\":\"" + RUN_ID + "\"}").principal(principal)) + .andExpect(status().isCreated()); + + ArgumentCaptor body = ArgumentCaptor.forClass(CreateExperimentFolderDTO.class); + verify(service).createFolder(eq(principal), body.capture(), any(Logger.class)); + assertThat(body.getValue().name()).isEqualTo("My analysis set"); + assertThat(body.getValue().experimentUuid()).isEqualTo(RUN_ID); + } + + @Test + void addMemberCarriesTheRunUnderTheContractFieldName() throws Exception { + when(service.addExperiment(any(), eq(FOLDER_ID), any(AddExperimentFoldersMemberDTO.class), any(Logger.class))) + .thenReturn(folderDto()); + + mvc.perform(post("/experiment-folders/" + FOLDER_ID + "/members").contentType("application/json") + .content("{\"experimentUuid\":\"" + RUN_ID + "\"}").principal(principal)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.id").value(FOLDER_ID)); + + ArgumentCaptor body = ArgumentCaptor.forClass(AddExperimentFoldersMemberDTO.class); + verify(service).addExperiment(eq(principal), eq(FOLDER_ID), body.capture(), any(Logger.class)); + assertThat(body.getValue().experimentUuid()).isEqualTo(RUN_ID); + } + + @Test + void setMembershipCarriesTheSetUnderTheContractFieldName() throws Exception { + when(service.updateSetMembership(any(), eq(FOLDER_ID), eq(RUN_ID), any(UpdateExperimentSetMembershipDTO.class), + any(Logger.class))).thenReturn(folderDto()); + + mvc.perform(patch("/experiment-folders/" + FOLDER_ID + "/members/" + RUN_ID + "/set") + .contentType("application/json").content("{\"setId\":\"" + SET_ID + "\"}").principal(principal)) + .andExpect(status().isOk()); + + ArgumentCaptor body = + ArgumentCaptor.forClass(UpdateExperimentSetMembershipDTO.class); + verify(service).updateSetMembership(eq(principal), eq(FOLDER_ID), eq(RUN_ID), body.capture(), any(Logger.class)); + assertThat(body.getValue().setId()).isEqualTo(SET_ID); + } + + @Test + void setMembershipWithoutASetReachesTheServiceAsUngroup() throws Exception { + when(service.updateSetMembership(any(), eq(FOLDER_ID), eq(RUN_ID), any(UpdateExperimentSetMembershipDTO.class), + any(Logger.class))).thenReturn(folderDto()); + + mvc.perform(patch("/experiment-folders/" + FOLDER_ID + "/members/" + RUN_ID + "/set") + .contentType("application/json").content("{\"setId\":null}").principal(principal)) + .andExpect(status().isOk()); + + ArgumentCaptor body = + ArgumentCaptor.forClass(UpdateExperimentSetMembershipDTO.class); + verify(service).updateSetMembership(eq(principal), eq(FOLDER_ID), eq(RUN_ID), body.capture(), any(Logger.class)); + assertThat(body.getValue().setId()).isNull(); + } + + @Test + void createSetCarriesItsOptionalSeedRun() throws Exception { + when(service.createSet(any(), eq(FOLDER_ID), any(CreateExperimentSetDTO.class), any(Logger.class))) + .thenReturn(folderDto()); + + mvc.perform(post("/experiment-folders/" + FOLDER_ID + "/sets").contentType("application/json") + .content("{\"name\":\"Drug arm A\",\"experimentUuid\":\"" + RUN_ID + "\"}").principal(principal)) + .andExpect(status().isCreated()); + + ArgumentCaptor body = ArgumentCaptor.forClass(CreateExperimentSetDTO.class); + verify(service).createSet(eq(principal), eq(FOLDER_ID), body.capture(), any(Logger.class)); + assertThat(body.getValue().name()).isEqualTo("Drug arm A"); + assertThat(body.getValue().experimentUuid()).isEqualTo(RUN_ID); + } + + @Test + void renamesAFolderAndASet() throws Exception { + when(service.renameFolder(any(), eq(FOLDER_ID), any(RenameExperimentFolderDTO.class), any(Logger.class))) + .thenReturn(folderDto()); + when(service.renameSet(any(), eq(FOLDER_ID), eq(SET_ID), any(RenameExperimentSetDTO.class), any(Logger.class))) + .thenReturn(folderDto()); + + mvc.perform(patch("/experiment-folders/" + FOLDER_ID).contentType("application/json") + .content("{\"name\":\"Renamed\"}").principal(principal)).andExpect(status().isOk()); + mvc.perform(patch("/experiment-folders/" + FOLDER_ID + "/sets/" + SET_ID).contentType("application/json") + .content("{\"name\":\"Renamed set\"}").principal(principal)).andExpect(status().isOk()); + } + + // ------------------------------------------------------------------ deletes + + @Test + void deletingTheFolderLeavesNothingToReturn() throws Exception { + mvc.perform(delete("/experiment-folders/" + FOLDER_ID).principal(principal)) + .andExpect(status().isNoContent()) + .andExpect(content().string("")); + } + + /** Losing a member or a set changes what is left, so those two answer with the remaining folder. */ + @Test + void deletingAMemberOrASetAnswersWithTheUpdatedFolder() throws Exception { + when(service.removeExperiment(any(), eq(FOLDER_ID), eq(RUN_ID), any(Logger.class))).thenReturn(folderDto()); + when(service.deleteSet(any(), eq(FOLDER_ID), eq(SET_ID), any(Logger.class))).thenReturn(folderDto()); + + mvc.perform(delete("/experiment-folders/" + FOLDER_ID + "/members/" + RUN_ID).principal(principal)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.id").value(FOLDER_ID)) + .andExpect(jsonPath("$.sets[0].id").value(SET_ID)); + + mvc.perform(delete("/experiment-folders/" + FOLDER_ID + "/sets/" + SET_ID).principal(principal)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.experimentIds[0]").value(RUN_ID)); + } + + // ------------------------------------------------------------------ failures + + @Test + void blankNameIsABadRequest() throws Exception { + when(service.createFolder(any(), any(CreateExperimentFolderDTO.class), any(Logger.class))) + .thenThrow(new BadRequestException("Folder name must not be blank.")); + + mvc.perform(post("/experiment-folders").contentType("application/json").content("{\"name\":\" \"}").principal(principal)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.statusCode").value(400)) + .andExpect(jsonPath("$.message").value("Folder name must not be blank.")); + } + + @Test + void malformedIdIsABadRequest() throws Exception { + when(service.getFolder(any(), eq("not-a-uuid"), any(Logger.class))) + .thenThrow(new BadRequestException("Invalid folderId: not-a-uuid")); + + mvc.perform(get("/experiment-folders/not-a-uuid").principal(principal)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.statusCode").value(400)); + } + + @Test + void duplicateNamesAreAConflict() throws Exception { + when(service.createFolder(any(), any(CreateExperimentFolderDTO.class), any(Logger.class))) + .thenThrow(new ConflictException("An experiment folder named 'My analysis set' already exists.")); + when(service.createSet(any(), eq(FOLDER_ID), any(CreateExperimentSetDTO.class), any(Logger.class))) + .thenThrow(new ConflictException("An experiment set named 'Drug arm A' already exists in this folder.")); + + mvc.perform(post("/experiment-folders").contentType("application/json").content("{\"name\":\"My analysis set\"}").principal(principal)) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.statusCode").value(409)); + mvc.perform(post("/experiment-folders/" + FOLDER_ID + "/sets").contentType("application/json") + .content("{\"name\":\"Drug arm A\"}").principal(principal)) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.statusCode").value(409)); + } + + @Test + void foreignFoldersAndForeignSetsAreBothNotFound() throws Exception { + when(service.getFolder(any(), eq(FOLDER_ID), any(Logger.class))) + .thenThrow(new ExperimentFolderNotFoundException("Experiment folder with id : " + FOLDER_ID + + " was not found for the active user.")); + when(service.renameSet(any(), eq(FOLDER_ID), eq(SET_ID), any(RenameExperimentSetDTO.class), any(Logger.class))) + .thenThrow(new ExperimentSetNotFoundException("Experiment set with id : " + SET_ID + + " was not found in folder : " + FOLDER_ID)); + + mvc.perform(get("/experiment-folders/" + FOLDER_ID).principal(principal)) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.statusCode").value(404)); + mvc.perform(patch("/experiment-folders/" + FOLDER_ID + "/sets/" + SET_ID).contentType("application/json") + .content("{\"name\":\"whatever\"}").principal(principal)) + .andExpect(status().isNotFound()) + .andExpect(jsonPath("$.statusCode").value(404)); + } + + /** A run the caller may not read must not become a folder membership, and must not say 404 either. */ + @Test + void unreadableRunIsUnauthorized() throws Exception { + when(service.addExperiment(any(), eq(FOLDER_ID), any(AddExperimentFoldersMemberDTO.class), any(Logger.class))) + .thenThrow(new UnauthorizedException("You don't have access to that experiment.")); + + mvc.perform(post("/experiment-folders/" + FOLDER_ID + "/members").contentType("application/json") + .content("{\"experimentUuid\":\"" + RUN_ID + "\"}").principal(principal)) + .andExpect(status().isUnauthorized()) + .andExpect(jsonPath("$.statusCode").value(401)); + } + + private ExperimentFolderDTO folderDto() { + return new ExperimentFolderDTO(FOLDER_ID, "My analysis set", List.of(RUN_ID), + List.of(new ExperimentSetDTO(SET_ID, "Drug arm A", List.of(RUN_ID)))); + } +} diff --git a/src/test/java/hbp/mip/folder/ExperimentFolderDTOTest.java b/src/test/java/hbp/mip/folder/ExperimentFolderDTOTest.java new file mode 100644 index 000000000..cffef3e3c --- /dev/null +++ b/src/test/java/hbp/mip/folder/ExperimentFolderDTOTest.java @@ -0,0 +1,85 @@ +package hbp.mip.folder; + +import hbp.mip.experiment.ExperimentDAO; +import hbp.mip.user.UserDAO; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * The mapping is the contract: three orderings have to survive the trip from the position columns to + * the arrays the dashboard renders, and an empty folder has to answer with empty arrays rather than + * nulls (null fields are dropped from the JSON, which would break `folder.sets.map(...)`). + */ +class ExperimentFolderDTOTest { + + @Test + void mapsMembersSetsAndSetMembersToTheirStoredOrder() { + ExperimentFolderDAO folder = new ExperimentFolderDAO( + new UserDAO("user", "User", "user@example.org", "subject"), "My analysis set", 1); + + ExperimentSetDAO first = new ExperimentSetDAO(folder, "First", 1); + ExperimentSetDAO second = new ExperimentSetDAO(folder, "Second", 2); + folder.getSets().add(second); + folder.getSets().add(first); + + UUID inFirstLow = uuid(); + UUID inFirstHigh = uuid(); + UUID ungrouped = uuid(); + UUID lateUngrouped = uuid(); + + // Added out of order on purpose: the response must follow the position columns, not insertion. + folder.getMembers().add(member(folder, inFirstHigh, first, 3, 2)); + folder.getMembers().add(member(folder, ungrouped, null, 1, null)); + folder.getMembers().add(member(folder, inFirstLow, second, 2, 1)); + folder.getMembers().add(member(folder, lateUngrouped, null, null, null)); + folder.getMembers().get(2).setExperimentSet(first); + + var dto = ExperimentFolderDTO.from(folder); + + assertThat(dto.id()).isEqualTo(folder.getId().toString()); + assertThat(dto.name()).isEqualTo("My analysis set"); + assertThat(dto.experimentIds()).containsExactly(ungrouped.toString(), inFirstLow.toString(), + inFirstHigh.toString(), lateUngrouped.toString()); + + assertThat(dto.sets()).extracting(ExperimentSetDTO::name).containsExactly("First", "Second"); + assertThat(dto.sets().get(0).id()).isEqualTo(first.getId().toString()); + assertThat(dto.sets().get(0).experimentIds()) + .containsExactly(inFirstLow.toString(), inFirstHigh.toString()); + assertThat(dto.sets().get(1).experimentIds()).isEmpty(); + } + + @Test + void mapsAFreshFolderToEmptyArrays() { + ExperimentFolderDAO folder = new ExperimentFolderDAO( + new UserDAO("user", "User", "user@example.org", "subject"), "Empty", 1); + + var dto = ExperimentFolderDTO.from(folder); + var collection = ExperimentFoldersDTO.from(List.of(folder)); + + assertThat(dto.experimentIds()).isEmpty(); + assertThat(dto.sets()).isEmpty(); + assertThat(collection.folders()).singleElement().satisfies(folderDto -> { + assertThat(folderDto.experimentIds()).isEmpty(); + assertThat(folderDto.sets()).isEmpty(); + }); + } + + private ExperimentFolderMemberDAO member(ExperimentFolderDAO folder, UUID experimentUuid, ExperimentSetDAO set, + Integer folderPosition, Integer setPosition) { + ExperimentDAO experiment = new ExperimentDAO(); + experiment.setUuid(experimentUuid); + + ExperimentFolderMemberDAO member = new ExperimentFolderMemberDAO(folder, experiment, folderPosition); + member.setExperimentSet(set); + member.setSetPosition(setPosition); + return member; + } + + private UUID uuid() { + return UUID.randomUUID(); + } +} diff --git a/src/test/java/hbp/mip/folder/ExperimentFolderServiceTest.java b/src/test/java/hbp/mip/folder/ExperimentFolderServiceTest.java new file mode 100644 index 000000000..4f410402b --- /dev/null +++ b/src/test/java/hbp/mip/folder/ExperimentFolderServiceTest.java @@ -0,0 +1,660 @@ +package hbp.mip.folder; + +import hbp.mip.experiment.ExperimentDAO; +import hbp.mip.experiment.ExperimentService; +import hbp.mip.user.ActiveUserService; +import hbp.mip.user.UserDAO; +import hbp.mip.user.UserDTO; +import hbp.mip.utils.Exceptions.BadRequestException; +import hbp.mip.utils.Exceptions.ConflictException; +import hbp.mip.utils.Exceptions.ExperimentFolderNotFoundException; +import hbp.mip.utils.Exceptions.ExperimentSetNotFoundException; +import hbp.mip.utils.Exceptions.UnauthorizedException; +import hbp.mip.utils.Logger; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.security.core.Authentication; + +import java.sql.SQLException; +import java.util.List; +import java.util.Optional; +import java.util.UUID; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class ExperimentFolderServiceTest { + + private static final String USERNAME = "user"; + + @Mock + private ActiveUserService activeUserService; + + @Mock + private ExperimentFolderRepository folderRepository; + + @Mock + private ExperimentService experimentService; + + @Mock + private Authentication authentication; + + private ExperimentFolderService service; + + private final UserDTO user = new UserDTO(USERNAME, "User", "user@example.org", "subject", true); + private final Logger logger = new Logger(USERNAME, "test"); + + @BeforeEach + void setUp() { + service = new ExperimentFolderService(activeUserService, folderRepository, experimentService); + + lenient().when(activeUserService.getActiveUser(authentication)).thenReturn(user); + lenient().when(folderRepository.saveAndFlush(any(ExperimentFolderDAO.class))) + .thenAnswer(invocation -> invocation.getArgument(0)); + } + + // ------------------------------------------------------------------ folders + + @Test + void createFolder_collapsesWhitespaceAndPlacesItLast() { + givenOwnedFolders(folder("Existing", 7)); + + var created = service.createFolder(authentication, new CreateExperimentFolderDTO(" My Analysis Set ", null), + logger); + + ArgumentCaptor saved = ArgumentCaptor.forClass(ExperimentFolderDAO.class); + verify(folderRepository).saveAndFlush(saved.capture()); + assertThat(created.name()).isEqualTo("My Analysis Set"); + assertThat(saved.getValue().getSortOrder()).isEqualTo(8); + assertThat(created.experimentIds()).isEmpty(); + assertThat(created.sets()).isEmpty(); + } + + @Test + void createFolder_capsTheNameAtTheLengthTheFrontendAllows() { + givenOwnedFolders(); + + var created = service.createFolder(authentication, + new CreateExperimentFolderDTO(" " + "a".repeat(80) + " ", null), logger); + + assertThat(created.name()).hasSize(ExperimentFolderService.MAX_NAME_LENGTH); + } + + @Test + void createFolder_rejectsANameThatIsOnlyWhitespace() { + assertThatThrownBy(() -> service.createFolder(authentication, new CreateExperimentFolderDTO(" \t\n ", null), logger)) + .isInstanceOf(BadRequestException.class); + + verify(folderRepository, never()).saveAndFlush(any(ExperimentFolderDAO.class)); + } + + @Test + void createFolder_rejectsANameAlreadyTakenIgnoringCase() { + givenOwnedFolders(folder("My Analysis Set", 1)); + + assertThatThrownBy(() -> service.createFolder(authentication, + new CreateExperimentFolderDTO(" my analysis set ", null), logger)) + .isInstanceOf(ConflictException.class); + + verify(folderRepository, never()).saveAndFlush(any(ExperimentFolderDAO.class)); + } + + @Test + void renameFolder_rejectsSomebodyElsesFolderAsNotFound() { + UUID folderId = UUID.randomUUID(); + givenFolder(folderId, "My Set", "somebody-else"); + + assertThatThrownBy(() -> service.renameFolder(authentication, folderId.toString(), + new RenameExperimentFolderDTO("Taken By Me"), logger)) + .isInstanceOf(ExperimentFolderNotFoundException.class); + } + + @Test + void renameFolder_keepsItsOwnNameAsNoConflict() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO owned = givenFolder(folderId, "My Set", USERNAME); + givenOwnedFolders(owned); + + var renamed = service.renameFolder(authentication, folderId.toString(), + new RenameExperimentFolderDTO(" my set "), logger); + + assertThat(renamed.name()).isEqualTo("my set"); + } + + @Test + void getFolder_readsOnlyTheFoldersOfTheActiveUser() { + // The repository query is already owner scoped; findById is the one that must not leak either. + UUID mine = UUID.randomUUID(); + givenFolder(mine, "Mine", USERNAME); + + assertThat(service.getFolder(authentication, mine.toString(), logger).name()).isEqualTo("Mine"); + } + + @Test + void getFolder_hidesAFolderOwnedByAnotherUserBehindNotFound() { + UUID foreign = UUID.randomUUID(); + givenFolder(foreign, "Theirs", "somebody-else"); + + assertThatThrownBy(() -> service.getFolder(authentication, foreign.toString(), logger)) + .isInstanceOf(ExperimentFolderNotFoundException.class); + } + + @Test + void getFolder_rejectsAnIdThatIsNotAnId() { + assertThatThrownBy(() -> service.getFolder(authentication, "not-a-uuid", logger)) + .isInstanceOf(BadRequestException.class); + } + + // ------------------------------------------------------------------ members + + @Test + void addExperiment_checksTheRunIsReadableAndAppendsItLast() { + UUID folderId = UUID.randomUUID(); + givenFolder(folderId, "My Set", USERNAME); + ExperimentDAO experiment = experiment(); + givenReadableExperiment(experiment); + + var updated = service.addExperiment(authentication, folderId.toString(), + new AddExperimentFoldersMemberDTO(experiment.getUuid().toString()), logger); + + verify(experimentService).assertExperimentAccessible(eq(authentication), eq(experiment.getUuid().toString()), + any(Logger.class)); + assertThat(updated.experimentIds()).containsExactly(experiment.getUuid().toString()); + + ArgumentCaptor saved = ArgumentCaptor.forClass(ExperimentFolderDAO.class); + verify(folderRepository).saveAndFlush(saved.capture()); + assertThat(saved.getValue().getMembers()).singleElement() + .satisfies(member -> assertThat(member.getFolderPosition()).isEqualTo(1)); + } + + @Test + void addExperiment_isIdempotentForARunTheFolderAlreadyHolds() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + ExperimentDAO experiment = experiment(); + givenReadableExperiment(experiment); + folder.getMembers().add(new ExperimentFolderMemberDAO(folder, experiment, 1)); + + var again = service.addExperiment(authentication, folderId.toString(), + new AddExperimentFoldersMemberDTO(experiment.getUuid().toString()), logger); + + assertThat(again.experimentIds()).containsExactly(experiment.getUuid().toString()); + assertThat(folder.getMembers()).hasSize(1); + verify(folderRepository, never()).saveAndFlush(any(ExperimentFolderDAO.class)); + } + + @Test + void addExperiment_refusesARunTheUserMayNotRead() { + UUID folderId = UUID.randomUUID(); + givenFolder(folderId, "My Set", USERNAME); + ExperimentDAO hidden = experiment(); + when(experimentService.assertExperimentAccessible(eq(authentication), eq(hidden.getUuid().toString()), + any(Logger.class))) + .thenThrow(new UnauthorizedException("You don't have access to that experiment.")); + + assertThatThrownBy(() -> service.addExperiment(authentication, folderId.toString(), + new AddExperimentFoldersMemberDTO(hidden.getUuid().toString()), logger)) + .isInstanceOf(UnauthorizedException.class); + + verify(folderRepository, never()).saveAndFlush(any(ExperimentFolderDAO.class)); + } + + @Test + void addExperiment_requiresAnExperimentId() { + UUID folderId = UUID.randomUUID(); + givenFolder(folderId, "My Set", USERNAME); + + assertThatThrownBy(() -> service.addExperiment(authentication, folderId.toString(), + new AddExperimentFoldersMemberDTO(" "), logger)) + .isInstanceOf(BadRequestException.class); + } + + @Test + void removeExperiment_dropsTheRunFromTheFolderAndItsSets() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + ExperimentSetDAO set = new ExperimentSetDAO(folder, "Baseline", 1); + folder.getSets().add(set); + ExperimentDAO first = experiment(); + ExperimentDAO second = experiment(); + ExperimentFolderMemberDAO firstMember = new ExperimentFolderMemberDAO(folder, first, 1); + firstMember.setExperimentSet(set); + firstMember.setSetPosition(1); + folder.getMembers().add(firstMember); + folder.getMembers().add(new ExperimentFolderMemberDAO(folder, second, 2)); + + var after = service.removeExperiment(authentication, folderId.toString(), first.getUuid().toString(), logger); + assertThat(after.experimentIds()).containsExactly(second.getUuid().toString()); + assertThat(after.sets()).singleElement().satisfies(candidate -> + assertThat(candidate.experimentIds()).isEmpty()); + } + + @Test + void removeExperiment_isHarmlessWhenTheRunIsNotInFolder() { + UUID folderId = UUID.randomUUID(); + givenFolder(folderId, "My Set", USERNAME); + + service.removeExperiment(authentication, folderId.toString(), experiment().getUuid().toString(), logger); + + verify(folderRepository, never()).saveAndFlush(any(ExperimentFolderDAO.class)); + } + + // ------------------------------------------------------------------ sets + + @Test + void createSet_appendsTheSetAndCanSeedItWithAMember() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + folder.getSets().add(new ExperimentSetDAO(folder, "Baseline", 4)); + ExperimentDAO experiment = experiment(); + givenReadableExperiment(experiment); + + var updated = service.createSet(authentication, folderId.toString(), + new CreateExperimentSetDTO(" Drug arm A ", experiment.getUuid().toString()), logger); + + assertThat(updated.sets()).extracting(ExperimentSetDTO::name).containsExactly("Baseline", "Drug arm A"); + assertThat(updated.sets().get(1).experimentIds()).containsExactly(experiment.getUuid().toString()); + assertThat(folder.getMembers()).singleElement().satisfies(member -> { + assertThat(member.getFolderPosition()).isEqualTo(1); + assertThat(member.getSetPosition()).isEqualTo(1); + }); + } + + @Test + void createSet_rejectsASetNameAlreadyTakenIgnoringCase() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + folder.getSets().add(new ExperimentSetDAO(folder, "Drug arm A", 1)); + + assertThatThrownBy(() -> service.createSet(authentication, folderId.toString(), + new CreateExperimentSetDTO(" drug ARM a ", null), logger)) + .isInstanceOf(ConflictException.class); + } + + @Test + void createSet_mayShareTheNameOfItsOwnFolder() { + UUID folderId = UUID.randomUUID(); + givenFolder(folderId, "My Set", USERNAME); + + var updated = service.createSet(authentication, folderId.toString(), + new CreateExperimentSetDTO("my set", null), logger); + + assertThat(updated.sets()).singleElement().satisfies(set -> assertThat(set.name()).isEqualTo("my set")); + } + + @Test + void updateSetMembership_movesTheRunOutOfThePreviousSet() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + ExperimentSetDAO baseline = new ExperimentSetDAO(folder, "Baseline", 1); + ExperimentSetDAO armA = new ExperimentSetDAO(folder, "Drug arm A", 2); + folder.getSets().add(baseline); + folder.getSets().add(armA); + ExperimentDAO moved = experiment(); + ExperimentFolderMemberDAO member = new ExperimentFolderMemberDAO(folder, moved, 1); + member.setExperimentSet(baseline); + member.setSetPosition(3); + folder.getMembers().add(member); + + var updated = service.updateSetMembership(authentication, folderId.toString(), moved.getUuid().toString(), + new UpdateExperimentSetMembershipDTO(armA.getId().toString()), logger); + + assertThat(updated.sets()).filteredOn(set -> set.name().equals("Baseline")) + .singleElement().satisfies(set -> assertThat(set.experimentIds()).isEmpty()); + assertThat(updated.sets()).filteredOn(set -> set.name().equals("Drug arm A")) + .singleElement().satisfies(set -> assertThat(set.experimentIds()) + .containsExactly(moved.getUuid().toString())); + assertThat(member.getExperimentSet()).isSameAs(armA); + assertThat(member.getSetPosition()).isEqualTo(1); + } + + @Test + void updateSetMembership_createsTheMembershipWhenTheRunIsNotInFolderYet() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + ExperimentSetDAO armA = new ExperimentSetDAO(folder, "Drug arm A", 1); + folder.getSets().add(armA); + ExperimentDAO experiment = experiment(); + givenReadableExperiment(experiment); + + var updated = service.updateSetMembership(authentication, folderId.toString(), experiment.getUuid().toString(), + new UpdateExperimentSetMembershipDTO(armA.getId().toString()), logger); + + verify(experimentService).assertExperimentAccessible(eq(authentication), eq(experiment.getUuid().toString()), + any(Logger.class)); + assertThat(updated.experimentIds()).containsExactly(experiment.getUuid().toString()); + assertThat(updated.sets()).singleElement().satisfies(set -> + assertThat(set.experimentIds()).containsExactly(experiment.getUuid().toString())); + } + + @Test + void updateSetMembership_withoutASetLeavesTheRunInTheFolderUngrouped() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + ExperimentSetDAO armA = new ExperimentSetDAO(folder, "Drug arm A", 1); + folder.getSets().add(armA); + ExperimentDAO experiment = experiment(); + ExperimentFolderMemberDAO member = new ExperimentFolderMemberDAO(folder, experiment, 1); + member.setExperimentSet(armA); + member.setSetPosition(1); + folder.getMembers().add(member); + + var updated = service.updateSetMembership(authentication, folderId.toString(), experiment.getUuid().toString(), + new UpdateExperimentSetMembershipDTO(null), logger); + + assertThat(updated.experimentIds()).containsExactly(experiment.getUuid().toString()); + assertThat(updated.sets()).singleElement().satisfies(set -> assertThat(set.experimentIds()).isEmpty()); + assertThat(member.getExperimentSet()).isNull(); + assertThat(member.getSetPosition()).isNull(); + } + + @Test + void updateSetMembership_rejectsASetOfAnotherFolder() { + UUID folderId = UUID.randomUUID(); + givenFolder(folderId, "My Set", USERNAME); + + assertThatThrownBy(() -> service.updateSetMembership(authentication, folderId.toString(), + experiment().getUuid().toString(), new UpdateExperimentSetMembershipDTO(UUID.randomUUID().toString()), + logger)) + .isInstanceOf(ExperimentSetNotFoundException.class); + } + + @Test + void renameSet_renamesOnlyItsOwnSet() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + ExperimentSetDAO armA = new ExperimentSetDAO(folder, "Drug arm A", 1); + folder.getSets().add(armA); + + var updated = service.renameSet(authentication, folderId.toString(), armA.getId().toString(), + new RenameExperimentSetDTO(" Drug arm B "), logger); + + assertThat(updated.sets()).singleElement().satisfies(set -> assertThat(set.name()).isEqualTo("Drug arm B")); + } + + @Test + void renameSet_rejectsANameAnotherSetAlreadyHas() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + folder.getSets().add(new ExperimentSetDAO(folder, "Drug arm A", 1)); + ExperimentSetDAO armB = new ExperimentSetDAO(folder, "Drug arm B", 2); + folder.getSets().add(armB); + + assertThatThrownBy(() -> service.renameSet(authentication, folderId.toString(), armB.getId().toString(), + new RenameExperimentSetDTO("drug arm a"), logger)) + .isInstanceOf(ConflictException.class); + } + + @Test + void deleteSet_leavesItsRunsInTheFolderUngrouped() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + ExperimentSetDAO armA = new ExperimentSetDAO(folder, "Drug arm A", 1); + folder.getSets().add(armA); + ExperimentDAO first = experiment(); + ExperimentDAO second = experiment(); + for (ExperimentDAO run : List.of(first, second)) { + ExperimentFolderMemberDAO member = new ExperimentFolderMemberDAO(folder, run, 1); + member.setExperimentSet(armA); + member.setSetPosition(1); + folder.getMembers().add(member); + } + + var after = service.deleteSet(authentication, folderId.toString(), armA.getId().toString(), logger); + assertThat(after.sets()).isEmpty(); + assertThat(after.experimentIds()).containsExactly(first.getUuid().toString(), second.getUuid().toString()); + assertThat(folder.getMembers()).allSatisfy(member -> { + assertThat(member.getExperimentSet()).isNull(); + assertThat(member.getSetPosition()).isNull(); + }); + } + + @Test + void deleteFolder_removesTheFolderWithoutTouchingTheRuns() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + folder.getMembers().add(new ExperimentFolderMemberDAO(folder, experiment(), 1)); + + service.deleteFolder(authentication, folderId.toString(), logger); + + ArgumentCaptor deleted = ArgumentCaptor.forClass(ExperimentFolderDAO.class); + + verify(folderRepository).delete(deleted.capture()); + assertThat(deleted.getValue().getId()).isEqualTo(folderId); + } + + // ------------------------------------------------------------------ ordering + + @Test + void positionsKeepGrowingFromWhateverTheFolderAlreadyHolds() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + ExperimentSetDAO armA = new ExperimentSetDAO(folder, "Drug arm A", 5); + folder.getSets().add(armA); + ExperimentDAO first = experiment(); + ExperimentDAO second = experiment(); + ExperimentDAO third = experiment(); + givenReadableExperiment(first, second, third); + ExperimentFolderMemberDAO firstMember = new ExperimentFolderMemberDAO(folder, first, 4); + firstMember.setExperimentSet(armA); + firstMember.setSetPosition(2); + folder.getMembers().add(firstMember); + + service.addExperiment(authentication, folderId.toString(), + new AddExperimentFoldersMemberDTO(second.getUuid().toString()), logger); + service.updateSetMembership(authentication, folderId.toString(), third.getUuid().toString(), + new UpdateExperimentSetMembershipDTO(armA.getId().toString()), logger); + + assertThat(folder.getMembers()).extracting(ExperimentFolderMemberDAO::getFolderPosition) + .containsExactly(4, 5, 6); + assertThat(folder.getMembers()).filteredOn(member -> first.getUuid().equals(member.getExperiment().getUuid())) + .singleElement().satisfies(member -> assertThat(member.getSetPosition()).isEqualTo(2)); + assertThat(ExperimentFolderDTO.from(folder).experimentIds()) + .containsExactly(first.getUuid().toString(), second.getUuid().toString(), + third.getUuid().toString()); + } + + @Test + void getFolders_returnsEveryFolderOfTheActiveUserInOrder() { + ExperimentFolderDAO first = folder("First", 1); + ExperimentFolderDAO second = folder("Second", 2); + givenOwnedFolders(first, second); + + var folders = service.getFolders(authentication, logger); + + assertThat(folders.folders()).extracting(ExperimentFolderDTO::name).containsExactly("First", "Second"); + } + + @Test + void getFolders_answersAnEmptyListInsteadOfANotFound() { + givenOwnedFolders(); + + assertThat(service.getFolders(authentication, logger).folders()).isEmpty(); + } + + // ------------------------------------------------------------------ create folder with its first run + + @Test + void createFolder_canSeedItWithTheRunTheGestureStartedFrom() { + givenOwnedFolders(); + ExperimentDAO experiment = experiment(); + givenReadableExperiment(experiment); + + var created = service.createFolder(authentication, + new CreateExperimentFolderDTO("New folder", experiment.getUuid().toString()), logger); + + verify(experimentService).assertExperimentAccessible(eq(authentication), eq(experiment.getUuid().toString()), + any(Logger.class)); + assertThat(created.experimentIds()).containsExactly(experiment.getUuid().toString()); + + ArgumentCaptor saved = ArgumentCaptor.forClass(ExperimentFolderDAO.class); + verify(folderRepository).saveAndFlush(saved.capture()); + assertThat(saved.getValue().getMembers()).singleElement().satisfies(member -> { + assertThat(member.getFolderPosition()).isEqualTo(1); + assertThat(member.getExperimentSet()).isNull(); + }); + } + + @Test + void createFolder_writesNothingWhenTheSeedRunIsNotReadable() { + givenOwnedFolders(); + ExperimentDAO hidden = experiment(); + when(experimentService.assertExperimentAccessible(eq(authentication), eq(hidden.getUuid().toString()), + any(Logger.class))).thenThrow(new UnauthorizedException("You don't have access to that experiment.")); + + assertThatThrownBy(() -> service.createFolder(authentication, + new CreateExperimentFolderDTO("New folder", hidden.getUuid().toString()), logger)) + .isInstanceOf(UnauthorizedException.class); + + verify(folderRepository, never()).saveAndFlush(any(ExperimentFolderDAO.class)); + } + + // ------------------------------------------------------------------ lost uniqueness races + + @Test + void aLostNameRaceInDatabaseAnswersConflictNotInternalError() { + givenOwnedFolders(); + when(folderRepository.saveAndFlush(any(ExperimentFolderDAO.class))) + .thenThrow(new DataIntegrityViolationException("could not execute statement", + new SQLException("duplicate key value violates unique constraint", "23505"))); + + assertThatThrownBy(() -> service.createFolder(authentication, + new CreateExperimentFolderDTO("Nobody saw the other tab", null), logger)) + .isInstanceOf(ConflictException.class); + } + + @Test + void aLostSetNameRaceInDatabaseAnswersConflictNotInternalError() { + UUID folderId = UUID.randomUUID(); + givenFolder(folderId, "My Set", USERNAME); + when(folderRepository.saveAndFlush(any(ExperimentFolderDAO.class))) + .thenThrow(new DataIntegrityViolationException("Batch entry rejected", + new SQLException("duplicate key", "23505"))); + + assertThatThrownBy(() -> service.createSet(authentication, folderId.toString(), + new CreateExperimentSetDTO("Arm A", null), logger)) + .isInstanceOf(ConflictException.class); + } + + @Test + void anIntegrityFailureThatIsNotUniquenessIsNotRelabelledAsConflict() { + givenOwnedFolders(); + when(folderRepository.saveAndFlush(any(ExperimentFolderDAO.class))) + .thenThrow(new DataIntegrityViolationException("could not execute statement", + new SQLException("null value in column \"name\" violates not-null constraint", "23502"))); + + assertThatThrownBy(() -> service.createFolder(authentication, + new CreateExperimentFolderDTO("Broken", null), logger)) + .isInstanceOf(DataIntegrityViolationException.class); + } + + // ------------------------------------------------------------------ per-folder write locking + + @Test + void memberAndSetWritesTakeTheFolderRowLock() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + ExperimentSetDAO set = new ExperimentSetDAO(folder, "Arm A", 1); + folder.getSets().add(set); + ExperimentDAO first = experiment(); + ExperimentDAO second = experiment(); + givenReadableExperiment(first, second); + + service.addExperiment(authentication, folderId.toString(), + new AddExperimentFoldersMemberDTO(first.getUuid().toString()), logger); + service.updateSetMembership(authentication, folderId.toString(), first.getUuid().toString(), + new UpdateExperimentSetMembershipDTO(set.getId().toString()), logger); + service.updateSetMembership(authentication, folderId.toString(), first.getUuid().toString(), + new UpdateExperimentSetMembershipDTO(null), logger); + service.createSet(authentication, folderId.toString(), + new CreateExperimentSetDTO("Arm B", second.getUuid().toString()), logger); + service.removeExperiment(authentication, folderId.toString(), second.getUuid().toString(), logger); + service.deleteSet(authentication, folderId.toString(), set.getId().toString(), logger); + + verify(folderRepository, times(6)).findByIdForUpdate(folderId); + verify(folderRepository, never()).findById(folderId); + } + + @Test + void readsDoNotTakeTheWriteLock() { + UUID folderId = UUID.randomUUID(); + givenFolder(folderId, "My Set", USERNAME); + + service.getFolder(authentication, folderId.toString(), logger); + + verify(folderRepository).findById(folderId); + verify(folderRepository, never()).findByIdForUpdate(folderId); + } + + /** + * The lock is what makes the pre-check see a competing writer: membership added before this call + * (whatever committed first) is honoured and the folder is returned unchanged. + */ + @Test + void addExperiment_underTheLockSeesAMembershipThatAlreadyCommitted() { + UUID folderId = UUID.randomUUID(); + ExperimentFolderDAO folder = givenFolder(folderId, "My Set", USERNAME); + ExperimentDAO experiment = experiment(); + givenReadableExperiment(experiment); + folder.getMembers().add(new ExperimentFolderMemberDAO(folder, experiment, 1)); + + var updated = service.addExperiment(authentication, folderId.toString(), + new AddExperimentFoldersMemberDTO(experiment.getUuid().toString()), logger); + + verify(folderRepository).findByIdForUpdate(folderId); + assertThat(updated.experimentIds()).containsExactly(experiment.getUuid().toString()); + verify(folderRepository, never()).saveAndFlush(any(ExperimentFolderDAO.class)); + } + + // ------------------------------------------------------------------ helpers + + private void givenOwnedFolders(ExperimentFolderDAO... folders) { + when(folderRepository.findOwnedFolders(USERNAME)).thenReturn(List.of(folders)); + } + + private ExperimentFolderDAO givenFolder(UUID folderId, String name, String ownerUsername) { + ExperimentFolderDAO folder = new ExperimentFolderDAO( + new UserDAO(ownerUsername, "User", "user@example.org", "subject"), name, 1); + folder.setId(folderId); + // Reads resolve through findById, member/set writes through the locked findByIdForUpdate; a test + // exercises one of them, so both stubs are lenient. + lenient().when(folderRepository.findById(folderId)).thenReturn(Optional.of(folder)); + lenient().when(folderRepository.findByIdForUpdate(folderId)).thenReturn(Optional.of(folder)); + return folder; + } + + private void givenReadableExperiment(ExperimentDAO... experiments) { + for (ExperimentDAO experiment : experiments) { + lenient().when(experimentService.assertExperimentAccessible(eq(authentication), + eq(experiment.getUuid().toString()), any(Logger.class))).thenReturn(experiment); + } + } + + private ExperimentFolderDAO folder(String name, int sortOrder) { + return new ExperimentFolderDAO(userOwner(), name, sortOrder); + } + + private UserDAO userOwner() { + return new UserDAO(USERNAME, "User", "user@example.org", "subject"); + } + + private ExperimentDAO experiment() { + ExperimentDAO experiment = new ExperimentDAO(); + experiment.setUuid(UUID.randomUUID()); + experiment.setName("run"); + return experiment; + } +} From b9f2b701a4d333e8b201d463d0da58d0cebd00fd Mon Sep 17 00:00:00 2001 From: KFilippopolitis Date: Tue, 15 Sep 2026 14:24:18 +0300 Subject: [PATCH 06/10] fix(auth): support application/jwt OIDC UserInfo responses DT4H Keycloak returns UserInfo as application/jwt, which Spring's default OidcUserService rejects. Build the OidcUser from the ID token claims instead, and carry an OidcUserAuthority so the existing GrantedAuthoritiesMapper can still map claim authorities for session-authenticated users. --- .../configurations/SecurityConfiguration.java | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/main/java/hbp/mip/configurations/SecurityConfiguration.java b/src/main/java/hbp/mip/configurations/SecurityConfiguration.java index 634ed2265..f9f31b531 100644 --- a/src/main/java/hbp/mip/configurations/SecurityConfiguration.java +++ b/src/main/java/hbp/mip/configurations/SecurityConfiguration.java @@ -16,7 +16,13 @@ import org.springframework.security.oauth2.client.registration.ClientRegistration; import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository; import org.springframework.security.oauth2.client.registration.InMemoryClientRegistrationRepository; +import org.springframework.security.oauth2.client.oidc.userinfo.OidcUserRequest; +import org.springframework.security.oauth2.client.userinfo.OAuth2UserService; import org.springframework.security.oauth2.core.AuthorizationGrantType; +import org.springframework.security.oauth2.core.oidc.OidcIdToken; +import org.springframework.security.oauth2.core.oidc.OidcUserInfo; +import org.springframework.security.oauth2.core.oidc.user.DefaultOidcUser; +import org.springframework.security.oauth2.core.oidc.user.OidcUser; import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority; import org.springframework.security.oauth2.client.web.OAuth2AuthorizationRequestRedirectFilter; import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationConverter; @@ -72,6 +78,23 @@ public ClientRegistrationRepository clientRegistrationRepository() { return new InMemoryClientRegistrationRepository(dummyRegistration); } + /** + * DT4H Keycloak returns UserInfo as {@code application/jwt}, which Spring's default + * OidcUserService rejects. Build the user from ID token claims instead. + */ + @Bean + OAuth2UserService oidcUserService() { + return userRequest -> { + OidcIdToken idToken = userRequest.getIdToken(); + OidcUserInfo userInfo = new OidcUserInfo(idToken.getClaims()); + return new DefaultOidcUser( + List.of(new OidcUserAuthority(idToken, userInfo)), + idToken, + userInfo, + "preferred_username"); + }; + } + @Bean SecurityFilterChain clientSecurityFilterChain(HttpSecurity http, ClientRegistrationRepository clientRegistrationRepo) throws Exception { @@ -89,7 +112,9 @@ SecurityFilterChain clientSecurityFilterChain(HttpSecurity http, .permitAll() .requestMatchers("/**").authenticated()); - http.oauth2Login(login -> login.successHandler(spaRedirectAuthenticationSuccessHandler)); + http.oauth2Login(login -> login + .successHandler(spaRedirectAuthenticationSuccessHandler) + .userInfoEndpoint(userInfo -> userInfo.oidcUserService(oidcUserService()))); // Allow API clients (e.g. notebooks) to authenticate with Bearer JWTs. // This runs alongside oauth2Login (session-based) authentication. From e48e88b47ac2dc64ca6fad609253be9f9cbc7b2a Mon Sep 17 00:00:00 2001 From: KFilippopolitis Date: Tue, 15 Sep 2026 14:24:21 +0300 Subject: [PATCH 07/10] chore(deps): bump pinned dependencies and clear CVEs Move Spring Boot to 4.1.1 and refresh Hibernate, Flyway, springdoc, PostgreSQL JDBC, Lombok, SnakeYAML, Jackson and Tomcat to their newest stable patches. The Jackson suppressions are obsolete, so remove them instead of leaving stale ignore entries. Replace the unused Data REST starter with the Web MVC starter. --- ci/suppress_osv_scanner.toml | 9 +++++---- ci/suppress_trivy.yaml | 9 ++++++--- pom.xml | 33 +++++++++++++++++++++------------ 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/ci/suppress_osv_scanner.toml b/ci/suppress_osv_scanner.toml index 0dc9630f9..52aec56c6 100644 --- a/ci/suppress_osv_scanner.toml +++ b/ci/suppress_osv_scanner.toml @@ -1,4 +1,5 @@ -[[IgnoredVulns]] -id = "GHSA-5jmj-h7xm-6q6v" -ignoreUntil = 2026-09-30 -reason = "The proposed fix version 2.21.5 not yet released" +# No active suppressions. +# GHSA-5jmj-h7xm-6q6v was suppressed while the fixed Jackson release did not +# exist; it is fixed upstream and pom.xml now pins Jackson 2.22.2 / 3.2.2, so +# the suppression is gone. Add entries here only with an id, reason and an +# ignoreUntil date. diff --git a/ci/suppress_trivy.yaml b/ci/suppress_trivy.yaml index f1ee94504..7b38e041f 100644 --- a/ci/suppress_trivy.yaml +++ b/ci/suppress_trivy.yaml @@ -1,3 +1,6 @@ -vulnerabilities: - - id: CVE-2026-54515 - statement: "The proposed fix version 2.21.5 not yet released" +# No active suppressions. +# CVE-2026-54515 (GHSA-5jmj-h7xm-6q6v) was suppressed while the fixed Jackson +# release did not exist; it is fixed upstream and pom.xml now pins Jackson +# 2.22.2 / 3.2.2, so the suppression is gone. Add entries here only with a CVE +# id, a reason, and a removal condition. +vulnerabilities: [] diff --git a/pom.xml b/pom.xml index 69cfbf0a8..86e04c3a1 100644 --- a/pom.xml +++ b/pom.xml @@ -13,25 +13,34 @@ org.springframework.boot spring-boot-starter-parent - 4.0.6 + 4.1.1 UTF-8 21 - 42.7.12 + 42.7.13 + + 11.0.25 3.2.0 - 7.4.0.Final - 12.7.0 + 7.4.8.Final + + 13.6.0 2.14.0 2.14.0 - 3.0.3 - 1.18.46 - 11.0.22 - 2.6 + 3.1.1 + 1.18.48 + 2.7 10.9.1 - 2.21.5 - 3.1.4 + + 2.22.2 + 3.2.2 @@ -67,8 +76,8 @@ spring-boot-starter-log4j2 - org.springframework.boot - spring-boot-starter-data-rest + org.springframework.boot + spring-boot-starter-webmvc org.springframework.boot From bc193466ce481c6cee82cd5fac6c4bb25e943d15 Mon Sep 17 00:00:00 2001 From: KFilippopolitis Date: Tue, 15 Sep 2026 14:24:24 +0300 Subject: [PATCH 08/10] chore(ci): bump scanners and pin workflow actions Refresh the SHA-pinned scanner releases and GitHub Actions so the SCA, SAST, container and publish pipelines stop drifting silently. --- .github/workflows/container-scan.yml | 14 +++++++------- .github/workflows/publish_images.yml | 4 ++-- .github/workflows/sast.yml | 8 ++++---- .github/workflows/sca.yml | 10 +++++----- ci/setup-tools.sh | 18 +++++++++--------- 5 files changed, 27 insertions(+), 27 deletions(-) diff --git a/.github/workflows/container-scan.yml b/.github/workflows/container-scan.yml index c1341c184..8dc9b047b 100644 --- a/.github/workflows/container-scan.yml +++ b/.github/workflows/container-scan.yml @@ -31,13 +31,13 @@ jobs: steps: - name: Check out repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Set up Python - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 #v6.2.0 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 #v7.0.0 with: # renovate: datasource=python-version depName=python - python-version: '3.14.4' + python-version: '3.14.7' - name: Build Docker image run: docker build -t ${{ env.IMAGE_NAME }} . @@ -57,7 +57,7 @@ jobs: - name: Upload Trivy SARIF to GitHub Security tab id: upload_trivy if: always() - uses: github/codeql-action/upload-sarif@c35d1b164463ee62a100735382aaaa525c5d3496 #v2.25.6 + uses: github/codeql-action/upload-sarif@b96794f015dfd88f77b49b1c93e0fa7110f94c63 #v4.38.0 with: sarif_file: ${{ env.TRIVY_SCA_SARIF_OUTPUT }} category: trivy-container-scanning @@ -65,7 +65,7 @@ jobs: - name: Upload OSV Scanner SARIF to GitHub Security tab id: upload_osv if: always() - uses: github/codeql-action/upload-sarif@c35d1b164463ee62a100735382aaaa525c5d3496 #v2.25.6 + uses: github/codeql-action/upload-sarif@b96794f015dfd88f77b49b1c93e0fa7110f94c63 #v4.38.0 with: sarif_file: ${{ env.OSV_SCA_SARIF_OUTPUT }} category: osv-scanner-container-scanning @@ -73,7 +73,7 @@ jobs: - name: Upload OpenGrep SARIF to GitHub Security tab id: upload_opengrep if: always() - uses: github/codeql-action/upload-sarif@c35d1b164463ee62a100735382aaaa525c5d3496 #v2.25.6 + uses: github/codeql-action/upload-sarif@b96794f015dfd88f77b49b1c93e0fa7110f94c63 #v4.38.0 with: sarif_file: ${{ env.OPENGREP_SAST_SARIF_OUTPUT }} category: opengrep-sast @@ -81,7 +81,7 @@ jobs: - name: Upload Hadolint SARIF to GitHub Security tab id: upload_hadolint if: always() - uses: github/codeql-action/upload-sarif@c35d1b164463ee62a100735382aaaa525c5d3496 #v2.25.6 + uses: github/codeql-action/upload-sarif@b96794f015dfd88f77b49b1c93e0fa7110f94c63 #v4.38.0 with: sarif_file: ${{ env.HADOLINT_SAST_SARIF_OUTPUT }} category: hadolint-sast diff --git a/.github/workflows/publish_images.yml b/.github/workflows/publish_images.yml index 01183d4fa..da3c22c20 100644 --- a/.github/workflows/publish_images.yml +++ b/.github/workflows/publish_images.yml @@ -22,7 +22,7 @@ jobs: uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 #v7.0.1 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c #v4.2.0 + uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e #v4.3.0 - name: Harbor connectivity probe run: | @@ -36,7 +36,7 @@ jobs: sudo sysctl -w net.ipv6.conf.default.disable_ipv6=1 - name: Log in to Docker Hub - uses: docker/login-action@371161bbe7024a29a25c5e19bfcbc0804fe9ad2c #v4.5.2 + uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f #v4.6.0 with: username: ${{ secrets.DOCKER_USERNAME }} password: ${{ secrets.DOCKER_PASSWORD }} diff --git a/.github/workflows/sast.yml b/.github/workflows/sast.yml index 786dbc816..1da6989d2 100644 --- a/.github/workflows/sast.yml +++ b/.github/workflows/sast.yml @@ -23,13 +23,13 @@ jobs: steps: - name: Check out repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Set up Python - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 #v6.2.0 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 #v7.0.0 with: # renovate: datasource=python-version depName=python - python-version: '3.14.4' + python-version: '3.14.7' - name: Setup tools run: bash ci/setup-tools.sh --install-tool opengrep,semgrep-rules @@ -40,7 +40,7 @@ jobs: - name: Upload Semgrep SARIF to GitHub Security tab id: upload_semgrep if: always() - uses: github/codeql-action/upload-sarif@c35d1b164463ee62a100735382aaaa525c5d3496 #v2.25.6 + uses: github/codeql-action/upload-sarif@b96794f015dfd88f77b49b1c93e0fa7110f94c63 #v4.38.0 with: sarif_file: ${{ env.OPENGREP_SARIF_OUTPUT }} category: semgrep-app diff --git a/.github/workflows/sca.yml b/.github/workflows/sca.yml index c57bf84a5..f917d3378 100644 --- a/.github/workflows/sca.yml +++ b/.github/workflows/sca.yml @@ -23,13 +23,13 @@ jobs: steps: - name: Check out repository - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Set up Python - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 #v6.2.0 + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 #v7.0.0 with: # renovate: datasource=python-version depName=python - python-version: '3.14.4' + python-version: '3.14.7' - name: Cache Maven packages uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 #v6.1.0 @@ -50,7 +50,7 @@ jobs: - name: Upload Trivy SARIF to GitHub Security tab id: upload_trivy if: always() - uses: github/codeql-action/upload-sarif@c35d1b164463ee62a100735382aaaa525c5d3496 #v2.25.6 + uses: github/codeql-action/upload-sarif@b96794f015dfd88f77b49b1c93e0fa7110f94c63 #v4.38.0 with: sarif_file: ${{ env.TRIVY_SARIF_OUTPUT }} category: trivy-app @@ -58,7 +58,7 @@ jobs: - name: Upload OSV Scanner SARIF to GitHub Security tab id: upload_osv if: always() - uses: github/codeql-action/upload-sarif@c35d1b164463ee62a100735382aaaa525c5d3496 #v2.25.6 + uses: github/codeql-action/upload-sarif@b96794f015dfd88f77b49b1c93e0fa7110f94c63 #v4.38.0 with: sarif_file: ${{ env.OSV_SARIF_OUTPUT }} category: osv-scanner-app diff --git a/ci/setup-tools.sh b/ci/setup-tools.sh index c24ca8de7..95d6b542d 100644 --- a/ci/setup-tools.sh +++ b/ci/setup-tools.sh @@ -11,27 +11,27 @@ trap 'echo "[setup-tools] ERROR: command failed (exit $?) at line $LINENO: $BASH # *_SHA256 must be overridden as well or verification will fail. # renovate: datasource=github-release-attachments depName=aquasecurity/trivy -TRIVY_VERSION="${TRIVY_VERSION:-v0.71.1}" -TRIVY_SHA256="${TRIVY_SHA256:-3cbae37cd440cd8676e5ce9207fe460b5641c7579a17e9d00f8894928c41a88d}" +TRIVY_VERSION="${TRIVY_VERSION:-v0.74.0}" +TRIVY_SHA256="${TRIVY_SHA256:-2ae6fe3ee734b7fdf11335663e18c75ea12dccc76062f09f164a3b0f8be4371a}" # renovate: datasource=github-release-attachments depName=google/osv-scanner -OSV_SCANNER_VERSION="${OSV_SCANNER_VERSION:-v2.4.0}" -OSV_SCANNER_SHA256="${OSV_SCANNER_SHA256:-15314940c10d26af9c6649f150b8a47c1262e8fc7e17b1d1029b0e479e8ed8a0}" +OSV_SCANNER_VERSION="${OSV_SCANNER_VERSION:-v2.5.1}" +OSV_SCANNER_SHA256="${OSV_SCANNER_SHA256:-f9f25499a2c8cc367b3af45df2ea7eeca7fbccceab9c35079968f4b3652194be}" # renovate: datasource=github-release-attachments depName=opengrep/opengrep -OPENGREP_VERSION="${OPENGREP_VERSION:-v1.25.0}" -OPENGREP_SHA256="${OPENGREP_SHA256:-9ac4aebb47ba3f7b0d8fc641ac8749cb6c2f253f616131a67d9631e00d4bea33}" +OPENGREP_VERSION="${OPENGREP_VERSION:-v1.30.0}" +OPENGREP_SHA256="${OPENGREP_SHA256:-35779bdd72e92129c8df2a77f0c55e8c08356801ea92591ef32108d6b28d564c}" # renovate: datasource=github-tags depName=semgrep/semgrep-rules SEMGREP_RULES_REF="${SEMGREP_RULES_REF:-40b8c63f75dc7c22c8a77482d73bfb864b146f7e}" SEMGREP_RULES_DIR="semgrep-rules" # renovate: datasource=github-release-attachments depName=hadolint/hadolint -HADOLINT_VERSION="${HADOLINT_VERSION:-v2.14.0}" -HADOLINT_SHA256="${HADOLINT_SHA256:-6bf226944684f56c84dd014e8b979d27425c0148f61b3bd99bcc6f39e9dc5a47}" +HADOLINT_VERSION="${HADOLINT_VERSION:-v2.15.1}" +HADOLINT_SHA256="${HADOLINT_SHA256:-c7187db94eeeeca956519a6af171adc31453941a1e777961f6e680f697c8c507}" # renovate: datasource=npm depName=@cyclonedx/cyclonedx-npm -CYCLONEDX_NPM_VERSION="${CYCLONEDX_NPM_VERSION:-6.0.0}" +CYCLONEDX_NPM_VERSION="${CYCLONEDX_NPM_VERSION:-6.0.1}" TMP_DIR="$(mktemp -d)" trap 'rm -rf "${TMP_DIR}"' EXIT From 351886c118eeaff60c6199058349740bfba0cc0c Mon Sep 17 00:00:00 2001 From: KFilippopolitis Date: Tue, 15 Sep 2026 14:24:28 +0300 Subject: [PATCH 09/10] build(config): read runtime config from the environment Move database, Keycloak, Exaflow, frontend and MIP version settings to ${ENV:default} placeholders in application.yml, drop the dockerize template and run the jar directly. Refresh the build and runtime base images and upgrade OpenSSL in the runtime image to clear the container SCA gate. --- Dockerfile | 32 ++++--------- config/application.tmpl | 76 ------------------------------ src/main/resources/application.yml | 41 ++++++++-------- 3 files changed, 29 insertions(+), 120 deletions(-) delete mode 100644 config/application.tmpl diff --git a/Dockerfile b/Dockerfile index b08839aff..59ce08e3d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ ####################################################### # Build the spring boot maven project ####################################################### -FROM maven:3.9.11-amazoncorretto-21 AS mvn-build-env +FROM maven:3.9.16-amazoncorretto-21 AS mvn-build-env LABEL maintainer="Thanasis Karampatsis " ENV CODE_PATH="/opt/code" @@ -19,7 +19,7 @@ RUN mvn -B -ntp clean package ####################################################### # Setup the running container ####################################################### -FROM amazoncorretto:21-alpine3.21 +FROM amazoncorretto:21-alpine3.24 ####################################################### # Setting up timezone @@ -30,10 +30,6 @@ RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone ####################################################### # Setting up environment ####################################################### -ENV APP_CONFIG_TEMPLATE="/opt/config/application.tmpl" -ENV APP_CONFIG_LOCATION="/opt/config/application.yml" -ENV SPRING_CONFIG_LOCATION="file:/opt/config/application.yml" - ENV SERVICE="platform-backend" ENV FEDERATION="default" ENV LOG_LEVEL="INFO" @@ -41,29 +37,17 @@ ENV FRAMEWORK_LOG_LEVEL="INFO" WORKDIR /opt -RUN apk add --no-cache curl - -####################################################### -# Install dockerize -####################################################### -ENV DOCKERIZE_VERSION=v0.14.0 -RUN wget https://github.com/jwilder/dockerize/releases/download/$DOCKERIZE_VERSION/dockerize-alpine-linux-amd64-$DOCKERIZE_VERSION.tar.gz \ - && tar -C /usr/local/bin -xzvf dockerize-alpine-linux-amd64-$DOCKERIZE_VERSION.tar.gz \ - && rm dockerize-alpine-linux-amd64-$DOCKERIZE_VERSION.tar.gz - +# libcrypto3/libssl3 are refreshed because the amazoncorretto base image ships +# openssl 3.5.7-r0, which carries CVE-2026-14456, CVE-2026-63073 and +# CVE-2026-75803 (CVSS >= 8.0, blocks the container SCA gate). Alpine 3.24 +# fixes them in openssl 3.5.8-r0. +RUN apk add --no-cache --upgrade libcrypto3 libssl3 curl ####################################################### # Prepare the spring boot application files ####################################################### -COPY config/application.tmpl $APP_CONFIG_TEMPLATE COPY --from=mvn-build-env /opt/code/target/platform-backend.jar /usr/share/jars/ - -####################################################### -# Configuration for the backend config files -####################################################### -ENV DISABLED_ALGORITHMS_CONFIG_PATH="/opt/platform/algorithms/disabledAlgorithms.json" -COPY config/disabledAlgorithms.json $DISABLED_ALGORITHMS_CONFIG_PATH VOLUME /opt/platform/api RUN addgroup -S appgroup && adduser -S appuser -G appgroup \ @@ -71,6 +55,6 @@ RUN addgroup -S appgroup && adduser -S appuser -G appgroup \ && chown -R appuser:appgroup /opt/config /opt/platform/api /usr/share/jars USER appuser -ENTRYPOINT ["sh", "-ec", "exec dockerize -template ${APP_CONFIG_TEMPLATE}:${APP_CONFIG_LOCATION} java --add-opens java.base/java.io=ALL-UNNAMED -Daeron.term.buffer.length -jar /usr/share/jars/platform-backend.jar"] +ENTRYPOINT ["java", "--add-opens", "java.base/java.io=ALL-UNNAMED", "-Daeron.term.buffer.length", "-jar", "/usr/share/jars/platform-backend.jar"] EXPOSE 8080 HEALTHCHECK --start-period=60s CMD curl --fail --silent --show-error http://localhost:8080/services/actuator/health | grep -q '"status":"UP"' diff --git a/config/application.tmpl b/config/application.tmpl deleted file mode 100644 index 9de1d2226..000000000 --- a/config/application.tmpl +++ /dev/null @@ -1,76 +0,0 @@ -# Configuration template for the platform running inside a Docker container - - -### EMBEDDED SERVER CONFIGURATION ### -server: - servlet: - contextPath: "/services" - port: 8080 - forward-headers-strategy: native - - -### DATABASE CONFIGURATION ### -spring: - datasource: - url: {{ default .Env.PLATFORM_DB_URL "jdbc:postgresql://172.17.0.1:5433/portal" }} - username: {{ default .Env.PLATFORM_DB_USER "portal" }} - password: {{ default .Env.PLATFORM_DB_PASSWORD "portalpwd" }} - driver-class-name: org.postgresql.Driver - data: - jpa: - repositories: - bootstrap-mode: default - jpa: - hibernate: - ddl-auto: validate - mvc: - pathmatch: - matching-strategy: ant_path_matcher - jackson: - default-property-inclusion: non_null - security: - oauth2: - client: - registration: - keycloak: - authorization-grant-type: authorization_code - client-id: {{ .Env.KEYCLOAK_CLIENT_ID }} - client-secret: {{ .Env.KEYCLOAK_CLIENT_SECRET }} - provider: keycloak - scope: openid - provider: - keycloak: - issuer-uri: {{ .Env.KEYCLOAK_AUTH_URL }}realms/{{ .Env.KEYCLOAK_REALM }} - user-name-attribute: preferred_username - # Allow API clients (e.g. notebooks) to authenticate with Bearer JWTs. - resourceserver: - jwt: - issuer-uri: {{ .Env.KEYCLOAK_AUTH_URL }}realms/{{ .Env.KEYCLOAK_REALM }} - - -### AUTHENTICATION ### -authentication: - enabled: {{ default .Env.AUTHENTICATION "1" }} - all_datasets_allowed_claim: research_dataset_all - all_experiments_allowed_claim: research_experiment_all - dataset_claim_prefix: research_dataset_ - - -### EXTERNAL SERVICES ### -services: - algorithmsUpdateInterval: {{ .Env.ALGORITHM_UPDATE_INTERVAL}} - exaflow: - algorithmsUrl: {{ .Env.EXAFLOW_URL}}/algorithms - attributesUrl: {{ .Env.EXAFLOW_URL}}/data_models_attributes - cdesMetadataUrl: {{ .Env.EXAFLOW_URL}}/cdes_metadata - datasets_variables: {{ .Env.EXAFLOW_URL}}/datasets_variables - - -### EXTERNAL FILES ### -# Files are imported when building the docker image -files: - disabledAlgorithms_json: "file:{{ .Env.DISABLED_ALGORITHMS_CONFIG_PATH}}" - - -frontend: - base-url: {{ .Env.PLATFORM_UI_BASE_URL }} diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index dfc15bd46..82eed5d6b 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -1,4 +1,4 @@ -# Configuration for development purposes +# Platform-backend configuration (local defaults; override via environment variables) ### EMBEDDED SERVER CONFIGURATION ### server: @@ -10,9 +10,9 @@ server: ### DATABASE CONFIGURATION ### spring: datasource: - url: "jdbc:postgresql://127.0.0.1:5433/portal" - username: "portal" - password: "portalpwd" + url: ${PLATFORM_DB_URL:jdbc:postgresql://127.0.0.1:5433/portal} + username: ${PLATFORM_DB_USER:portal} + password: ${PLATFORM_DB_PASSWORD:portalpwd} driver-class-name: org.postgresql.Driver data: jpa: @@ -32,38 +32,39 @@ spring: registration: keycloak: authorization-grant-type: authorization_code - client-id: replaceme - client-secret: replaceme + client-id: ${KEYCLOAK_CLIENT_ID:replaceme} + client-secret: ${KEYCLOAK_CLIENT_SECRET:replaceme} provider: keycloak scope: openid provider: keycloak: user-name-attribute: preferred_username - issuer-uri: https://iam.ebrains.eu/auth/realms/MIP + issuer-uri: ${KEYCLOAK_AUTH_URL:https://iam.ebrains.eu/auth/}realms/${KEYCLOAK_REALM:MIP} resourceserver: jwt: - issuer-uri: https://iam.ebrains.eu/auth/realms/MIP + issuer-uri: ${KEYCLOAK_AUTH_URL:https://iam.ebrains.eu/auth/}realms/${KEYCLOAK_REALM:MIP} ### AUTHENTICATION ### authentication: - enabled: 1 + enabled: ${AUTHENTICATION:1} all_datasets_allowed_claim: research_dataset_all all_experiments_allowed_claim: research_experiment_all dataset_claim_prefix: research_dataset_ ### EXTERNAL SERVICES ### services: - algorithmsUpdateInterval: 100 exaflow: - algorithmsUrl: "http://127.0.0.1:5000/algorithms" - attributesUrl: "http://127.0.0.1:5000/data_models_attributes" - datasets_variables: "http://127.0.0.1:5000/datasets_variables" - cdesMetadataUrl: "http://127.0.0.1:5000/cdes_metadata" - -### EXTERNAL FILES ### -# Files are loaded from the resources -files: - disabledAlgorithms_json: "classPath:/disabledAlgorithms.json" + analysisUrl: ${EXAFLOW_URL:http://127.0.0.1:5000}/analysis + inputdataSpecUrl: ${EXAFLOW_URL:http://127.0.0.1:5000}/specifications/inputdata + preprocessingSpecUrl: ${EXAFLOW_URL:http://127.0.0.1:5000}/specifications/preprocessing + algorithmsSpecUrl: ${EXAFLOW_URL:http://127.0.0.1:5000}/specifications/algorithms + attributesUrl: ${EXAFLOW_URL:http://127.0.0.1:5000}/data_models_attributes + datasets_variables: ${EXAFLOW_URL:http://127.0.0.1:5000}/datasets_variables + cdesMetadataUrl: ${EXAFLOW_URL:http://127.0.0.1:5000}/cdes_metadata frontend: - base-url: "http://localhost:4200" + base-url: ${PLATFORM_UI_BASE_URL:http://localhost:4200} + +### PLATFORM VERSION ### +mip: + version: ${MIP_VERSION:dev} From 816954c919c29c181fd85b5db8864d2ad65cdbe4 Mon Sep 17 00:00:00 2001 From: KFilippopolitis Date: Tue, 15 Sep 2026 14:27:47 +0300 Subject: [PATCH 10/10] docs: refresh repository context for analysis, folders and config Describe the Specifications/Analysis modules, the folder package and its API contract, the environment-driven configuration model, and record the dependency/CI baselines. --- AGENTS.md | 14 +++++----- docs/context/architecture.md | 9 +++---- docs/context/commands.md | 2 +- docs/context/conventions.md | 4 +-- docs/context/decision-log.md | 32 ++++++++++++++++++++++ docs/context/module-index.md | 51 +++++++++++++++++++++++------------ docs/context/risk-register.md | 3 ++- 7 files changed, 82 insertions(+), 33 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8e4ace6e9..8ea32c0e1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,20 +6,20 @@ This repository contains the Spring Boot backend for the Medical Informatics Pla ## Repository Layout - `src/main/java/hbp/mip`: application entrypoint and backend code. - `src/main/java/hbp/mip/configurations`: security, OAuth2/JWT, persistence, OpenAPI, and redirect/filter configuration. -- `src/main/java/hbp/mip/algorithm`: algorithm metadata API/service and disabled algorithm filtering. +- `src/main/java/hbp/mip/algorithm`: algorithm metadata API/service and Exaflow specification passthrough. - `src/main/java/hbp/mip/datamodel`: data model API/service backed by Exaflow metadata endpoints. - `src/main/java/hbp/mip/experiment`: experiment API, service, repository, JPA entity, specifications, and DTOs. - `src/main/java/hbp/mip/user`: active user API/service, user repository, JPA entity, and DTO. -- `src/main/java/hbp/mip/utils`: shared logging, JSON/HTTP helpers, resource loading, claim validation, and exception handling. +- `src/main/java/hbp/mip/folder`: experiment folder/set API, service, repository, JPA entities, and DTOs. +- `src/main/java/hbp/mip/utils`: shared logging, JSON/HTTP helpers, claim validation, and exception handling. - `src/main/resources`: local runtime config, Log4j2 config, and Flyway migrations under `db/migration`. -- `config/`: container config template and static runtime assets such as `disabledAlgorithms.json`. - `.github/workflows`: release image publishing and EBRAINS mirror automation. - `docs/context`: durable repository context for humans and AI coding agents. ## Stack - Language/runtime: Java 21. - Build/package manager: Maven (`pom.xml`); no Maven wrapper is committed. -- Frameworks/libraries: Spring Boot 4.0.6, Spring Security 7, OAuth2 client/resource server, Spring Data JPA, Hibernate, Flyway, Gson, Log4j2, springdoc OpenAPI. +- Frameworks/libraries: Spring Boot 4.1.1, Spring Security 7, OAuth2 client/resource server, Spring Data JPA, Hibernate, Flyway, Gson, Log4j2, springdoc OpenAPI. - Database: PostgreSQL, configured through Spring datasource properties. - Auth: Keycloak/OIDC when `authentication.enabled` is enabled; anonymous development mode exists when disabled. - External services: Exaflow endpoints for algorithms, data models, metadata, dataset variables, and algorithm execution. @@ -32,7 +32,7 @@ Use a local Java 21 JDK and Maven installation. mvn -B -ntp dependency:go-offline ``` -Local development also expects PostgreSQL and reachable Exaflow/Keycloak endpoints based on `src/main/resources/application.yml`. Unknown / TODO: verify the preferred local database bootstrap command; no Docker Compose or Makefile is committed. +Local development also expects PostgreSQL, reachable Exaflow/Keycloak endpoints, and `MIP_VERSION` set in the environment. Defaults for other settings live in `src/main/resources/application.yml`. ## Development Commands Run locally with the development config in `src/main/resources/application.yml`: @@ -74,12 +74,12 @@ mvn clean package ## Architecture Rules - Put HTTP endpoints in `*API` classes under the owning feature package. - Put business logic in `*Service` classes; controllers should delegate rather than implement workflows directly. -- Put persistence in Spring Data repositories and JPA `*DAO` entities. Current JPA packages are `hbp.mip.experiment` and `hbp.mip.user`. +- Put persistence in Spring Data repositories and JPA `*DAO` entities. Current JPA packages are `hbp.mip.experiment`, `hbp.mip.user`, and `hbp.mip.folder` (list them in `PersistenceConfiguration` when adding another). - Put API/request/response shapes in `*DTO` records or DTO classes close to the feature package. - Put cross-cutting helpers in `hbp.mip.utils` only when they are genuinely shared. - Put security, persistence, OpenAPI, and web filter wiring in `hbp.mip.configurations`. - Keep Flyway migrations in `src/main/resources/db/migration` using `V{number}__Description.sql`. -- Keep runtime/container configuration in `src/main/resources/application.yml` and `config/application.tmpl`; do not read environment variables ad hoc from feature code. +- Keep runtime configuration in `src/main/resources/application.yml` using `${ENV:default}` placeholders; containers override via environment variables. Do not read environment variables ad hoc from feature code. ## Coding Conventions - Use 4-space Java indentation and existing package style under lowercase `hbp.mip`. diff --git a/docs/context/architecture.md b/docs/context/architecture.md index 617ad69df..47760f772 100644 --- a/docs/context/architecture.md +++ b/docs/context/architecture.md @@ -74,15 +74,14 @@ OpenAPI is configured by `OpenApiConfig` and springdoc dependencies. Authorization-sensitive logic appears in `ClaimUtils`, `ExperimentService`, `DataModelService`, and active user handling. ## Background Work and External Services -- `AlgorithmService.AlgorithmAggregator` schedules asynchronous algorithm metadata refreshes from Exaflow. +- `SpecificationsService` fetches inputdata, preprocessing, and algorithm specifications from Exaflow per request. - `ExperimentService` starts a background `Thread` for persisted experiment execution. - Data model and algorithm metadata are fetched through `HTTPUtil` using configured Exaflow URLs. -- Disabled algorithms are loaded from `files.disabledAlgorithms_json`. ## Config and Environment Model -- Local config is in `src/main/resources/application.yml`. -- Container config is rendered from `config/application.tmpl` by `dockerize`. -- Important config areas include datasource, OAuth2/Keycloak, authentication flags, Exaflow URLs, disabled algorithms path, frontend base URL, and logging. +- All runtime config is in `src/main/resources/application.yml` with `${ENV:default}` placeholders. +- Containers pass environment variables (see `mip/deployment`); no separate config template is rendered at startup. +- Important config areas include datasource, OAuth2/Keycloak, authentication flags, Exaflow URLs, frontend base URL, `MIP_VERSION`, and logging. - Do not introduce direct environment reads in business logic; route configuration through Spring properties. ## Known Gaps diff --git a/docs/context/commands.md b/docs/context/commands.md index a389dae14..3075ded52 100644 --- a/docs/context/commands.md +++ b/docs/context/commands.md @@ -81,7 +81,7 @@ Build the container image: docker build -t hbpmip/platform-backend:testing . ``` -The Dockerfile performs a Maven build, copies `target/platform-backend.jar`, renders `config/application.tmpl` with `dockerize`, exposes port `8080`, and defines a healthcheck at `/services/actuator/health`. +The Dockerfile performs a Maven build, copies `target/platform-backend.jar`, exposes port `8080`, and defines a healthcheck at `/services/actuator/health`. Configuration is read from classpath `application.yml`; set container environment variables to override defaults. ## CI Detected workflows: diff --git a/docs/context/conventions.md b/docs/context/conventions.md index ea9bc27b1..a37e9b898 100644 --- a/docs/context/conventions.md +++ b/docs/context/conventions.md @@ -30,9 +30,9 @@ ### Configuration - Spring properties are defined in `src/main/resources/application.yml`. -- Container properties are rendered from `config/application.tmpl`. +- Local defaults use `${ENV:default}` placeholders; containers override the same properties via environment variables. - Feature code uses Spring `@Value` for configured URLs and flags. -- Environment variables should be routed through Spring config/templates, not read directly. +- Environment variables should be routed through Spring config, not read directly. ### API Patterns - Routes are relative to servlet context `/services`. diff --git a/docs/context/decision-log.md b/docs/context/decision-log.md index 9be831419..dcb090868 100644 --- a/docs/context/decision-log.md +++ b/docs/context/decision-log.md @@ -61,3 +61,35 @@ Consequences: Auth changes must verify both flows and CSRF behavior. Files affected: `src/main/java/hbp/mip/configurations/SecurityConfiguration.java`, `src/main/java/hbp/mip/user/ActiveUserAPI.java` Date: Unknown / TODO: verify + +### Decision: Keep every pinned dependency on the newest stable release + +Status: Verified + +Context: The SCA gate (`.github/workflows/sca.yml`, Trivy + osv-scanner over the CycloneDX SBOM) reported one finding on the pre-bump dependency set: `tools.jackson.core:jackson-databind:3.1.4` (CVE-2026-59889 / GHSA-5gvw-p9qm-jgwh, CVSS 6.5), pulled in transitively by Flyway and pinned by the `jackson-bom.version` property. + +Decision: Bump `pom.xml` to the newest stable releases across the board (Boot parent 4.1.1, Hibernate 7.4.8.Final, Flyway 13.6.0, springdoc 3.1.1, PostgreSQL JDBC 42.7.13, Jackson BOMs 2.22.2 / 3.2.2) and keep the Tomcat 11.0.25 override, because Boot 4.1.1 still manages the vulnerable 11.0.24. Prereleases (Boot 4.2.0-M1, Hibernate 8.0.0.Beta1, Jakarta Persistence 4.0.0-M6) are intentionally excluded. + +Consequences: The obsolete Jackson suppressions were deleted from `ci/suppress_osv_scanner.toml` and `ci/suppress_trivy.yaml`. Flyway 12 to 13 is a major jump and only the programmatic `Flyway.configure()` API in `PersistenceConfiguration` is used, so migration behaviour against PostgreSQL still needs a human check on a real database. + +Validation: `mvn clean package` passed 62/62 tests. Regenerated SBOM scanned with the CI-pinned scanners: osv-scanner and Trivy both report 0 findings and no unused ignores (pre-bom baseline: 1 finding each). + +Files affected: `pom.xml`, `ci/suppress_osv_scanner.toml`, `ci/suppress_trivy.yaml`, `ci/setup-tools.sh`, `.github/workflows/sca.yml`, `.github/workflows/sast.yml`, `.github/workflows/container-scan.yml` + +Date: 2026-09-15 + +### Decision: Keep CI security tooling and pinned GitHub Actions current + +Status: Verified locally / needs one CI run + +Context: The SCA, SAST and container pipelines install release assets by exact version plus SHA256 (`ci/setup-tools.sh`) and the workflows pin actions by commit SHA, so both drift silently. + +Decision: Bumped Trivy v0.71.1 -> v0.74.0, osv-scanner v2.4.0 -> v2.5.1, OpenGrep v1.25.0 -> v1.30.0, Hadolint v2.14.0 -> v2.15.1, `@cyclonedx/cyclonedx-npm` 6.0.0 -> 6.0.1, workflow Python 3.14.4 -> 3.14.7, and the action pins `actions/checkout` v6.0.3 -> v7.0.1, `actions/setup-python` v6.2.0 -> v7.0.0, `github/codeql-action/upload-sarif` v2.25.6 -> v4.38.0, `docker/setup-buildx-action` v4.2.0 -> v4.3.0, `docker/login-action` v4.5.2 -> v4.6.0. New SHA256 values were computed from the same release assets the script downloads. osv-scanner stays on v2.5.1 rather than v2.6.0 because v2.6.0 is younger than the 7-day `minimumReleaseAge` policy in `renovate.json`. + +Consequences: All new checksums must match the assets GitHub downloads, or `ci/setup-tools.sh` fails fast. `upload-sarif` v4.38.0 keeps the same inputs but runs on node24 instead of the EOL node16. Latest `semgrep-rules` (not bumped here) reports ERROR-level `dockerfile-source-not-pinned` findings on this Dockerfile, so bumping `SEMGREP_RULES_REF` will fail the container SAST gate until the `FROM` lines are digest-pinned. + +Validation: Ran each upgraded tool with the exact command lines from `ci/sca_scan.py` and `ci/container_scan.py`: Trivy and osv-scanner report 0 findings on the regenerated SBOM, Hadolint passes `--failure-threshold error`, and OpenGrep v1.30.0 passes the `--severity=ERROR --error` gate against the pinned rules ref. `docker build` and the workflow runner behaviour could not be checked locally (no Docker daemon). + +Files affected: `ci/setup-tools.sh`, `.github/workflows/sca.yml`, `.github/workflows/sast.yml`, `.github/workflows/container-scan.yml`, `.github/workflows/publish_images.yml` + +Date: 2026-09-15 diff --git a/docs/context/module-index.md b/docs/context/module-index.md index f1a355c8c..a24fd9221 100644 --- a/docs/context/module-index.md +++ b/docs/context/module-index.md @@ -40,7 +40,6 @@ Rules: Keep Exaflow metadata fetching and disabled algorithm filtering in the se Tests: Mock Exaflow responses and disabled algorithm resources for service tests. -Notes: A scheduled async refresh exists in `AlgorithmService.AlgorithmAggregator`. ## `src/main/java/hbp/mip/datamodel` @@ -70,6 +69,38 @@ Tests: Add service/repository tests for behavior changes and migration checks fo Notes: Persisted experiment execution starts a background thread and then updates status/result. +## `src/main/java/hbp/mip/folder` + +Purpose: Experiment folders ("analysis sets") and the named subsets ("sets") inside them: what the experiments dashboard curates. + +Key files: `ExperimentFolderAPI.java`, `ExperimentFolderService.java`, `ExperimentFolderRepository.java`, `ExperimentFolderDAO.java`, `ExperimentSetDAO.java`, `ExperimentFolderMemberDAO.java`, and the folder `*DTO` records. + +Used by: the frozen frontend contract, all under the `/services` context: + +```text +GET /experiment-folders -> 200 { folders: [folder] } +POST /experiment-folders -> 201 folder { name, experimentUuid? } +GET /experiment-folders/{folderId} -> 200 folder +PATCH /experiment-folders/{folderId} -> 200 folder { name } +DELETE /experiment-folders/{folderId} -> 204 +POST /experiment-folders/{folderId}/members -> 200 folder { experimentUuid } +DELETE /experiment-folders/{folderId}/members/{uuid} -> 200 folder +POST /experiment-folders/{folderId}/sets -> 201 folder { name, experimentUuid? } +PATCH /experiment-folders/{folderId}/sets/{setId} -> 200 folder { name } +DELETE /experiment-folders/{folderId}/sets/{setId} -> 200 folder +PATCH /experiment-folders/{folderId}/members/{uuid}/set -> 200 folder { setId: "uuid" | null } + +folder = { id, name, experimentIds: [uuid], sets: [ { id, name, experimentIds: [uuid] } ] } +``` + +A null `setId` ungroups without leaving the folder. Everything but `DELETE /{folderId}` answers with the updated folder, so the caller never rebuilds ordering or partitioning locally. + +Rules: Scope every read and write to the user resolved from the token; a folder owned by somebody else answers 404, never 403. Member and set writes load the folder with `findByIdForUpdate` (`PESSIMISTIC_WRITE`) before the membership check, so concurrent adds/moves on one folder serialize and the member unique constraint stays a backstop, not the thing that decides the request. Write through `saveAndFlush` and translate a unique name violation into 409 — the name pre-check alone cannot win a race between two tabs. Keep one member row per (folder, experiment) — that is what makes sets a strict partition. Names follow the frontend rules: collapse whitespace, trim, 60 chars, unique per owner (and per folder, for sets) ignoring case. Preserve the position columns (`sort_order`, `folder_position`, `set_position`); they are the order the dashboard renders. Deleting a folder or a set never deletes the runs. + +Tests: `ExperimentFolderServiceTest` (Mockito) covers naming, ownership, membership, ordering, and the lost-race 409; `ExperimentFolderDTOTest` covers the three orderings in the response; `ExperimentFolderAPITest` (standalone MockMvc) pins the wire contract — body field names, response field names, and the status of every failure. + +Notes: Folders hold experiment ids, never experiment copies, so nothing here can go stale by carrying a run's data. Statuses: 200 reads and mutations that leave a folder behind, 201 new folder or set, 204 only `DELETE /{folderId}`, 400 blank name or malformed id, 404 folder/set not the caller's or run not readable, 401 run not readable, 409 duplicate name (pre-check or lost database race). + ## `src/main/java/hbp/mip/user` Purpose: Active user resolution, persistence, and NDA state. @@ -102,7 +133,7 @@ Notes: `HTTPUtil` uses `HttpURLConnection`; no dedicated HTTP client abstraction Purpose: Local runtime config, logging config, and database migrations. -Key files: `application.yml`, `log4j2.yml`, `db/migration/V1__InitialSchema.sql`. +Key files: `application.yml`, `log4j2.yml`, `db/migration/V1__InitialSchema.sql`, `db/migration/V2__ExperimentFolders.sql`. Used by: Local app runtime and Maven resource packaging. @@ -110,21 +141,7 @@ Rules: Add new migrations instead of editing shipped migrations. Keep secret val Tests: Run migration validation against PostgreSQL for DB changes. -Notes: `application.yml` contains local defaults and placeholder Keycloak values. - -## `config` - -Purpose: Container runtime config template and static runtime assets. - -Key files: `application.tmpl`, `disabledAlgorithms.json`. - -Used by: Docker image entrypoint and production/container runtime. - -Rules: Keep template variables aligned with documented environment configuration. - -Tests: For config changes, build the Docker image or validate template rendering in the deployment environment. - -Notes: `dockerize` renders `application.tmpl` to `/opt/config/application.yml`. +Notes: `application.yml` contains local defaults (`${ENV:default}`) and placeholder Keycloak values. Export `MIP_VERSION` before local runs. ## `.github/workflows` diff --git a/docs/context/risk-register.md b/docs/context/risk-register.md index bde9031a3..1cee3cdf5 100644 --- a/docs/context/risk-register.md +++ b/docs/context/risk-register.md @@ -11,9 +11,10 @@ | Experiment execution | Background thread updates status/result after Exaflow execution | `ExperimentService.java` | Preserve pending/success/error transitions and repository finish behavior | Yes | | Exaflow external APIs | Runtime depends on external service contracts and availability | `AlgorithmService.java`, `DataModelService.java`, `ExperimentService.java`, `HTTPUtil.java` | Mock response codes/bodies and avoid unreviewed wire-shape changes | Yes | | Database migrations | Schema drift can break startup because Hibernate validates schema | `PersistenceConfiguration.java`, `V1__InitialSchema.sql` | Add new Flyway migrations; verify on PostgreSQL | Yes | -| Secrets and config | Keycloak, database, registry, and mirror credentials must stay private | `application.yml`, `config/application.tmpl`, `.github/workflows/*` | Never print or commit secrets; review env/template changes carefully | Yes | +| Secrets and config | Keycloak, database, registry, and mirror credentials must stay private | `application.yml`, `.github/workflows/*` | Never print or commit secrets; review env/config changes carefully | Yes | | Logging sensitive data | Request bodies, results, or tokens may include sensitive values | `Logger.java`, API/services logging request details | Avoid adding sensitive logs; redact where possible | Yes | | Release publishing | Workflow pushes public/private images on release | `.github/workflows/publish_images.yml` | Review registry targets, tags, cache, and secret usage | Yes | | Repository mirroring | Mirror workflow can publish refs to EBRAINS GitLab | `.github/workflows/ebrains.yml` | Review branch/tag triggers and token usage | Yes | | Dependency updates | Framework/security/library upgrades can change runtime behavior | `pom.xml` | Explain need, run Maven checks, and consider Docker build | Yes | | Token-heavy tool output | Broad file dumps, full logs, or unbounded diffs can exhaust agent context and hide relevant facts | Agent workflow instructions in `AGENTS.md` and `docs/context/conventions.md` | Use bounded searches and summaries; require approval before high-output commands | Yes | +| Tomcat pinned ahead of the Boot BOM | `pom.xml` pins Tomcat 11.0.25 over Spring Boot 4.1.1's 11.0.24; deleting the override re-exposes CVE-2026-65905, CVE-2026-65182 and CVE-2026-68525 | `pom.xml` | Keep until the Boot BOM manages >= 11.0.25, then drop the override and re-run the dependency scan | No |