Improve software quality to SonarClound gate A rating (#907)

Addresses 17 reliability concerns raised by SonarCloud and fixes several maintainability issues.

---------

Co-authored-by: Tobias Koch <[email protected]>

application-commons/src/main/java/life/qbic/application/commons/ApplicationException.java

@@ -17,7 +17,7 @@
 public class ApplicationException extends RuntimeException {
 
   private final ErrorCode errorCode;
-  private final ErrorParameters errorParameters;
+  private final transient ErrorParameters errorParameters;
 
   public ApplicationException() {
     this(ErrorCode.GENERAL, ErrorParameters.empty());

broadcasting/src/main/java/life/qbic/broadcasting/Exchange.java

@@ -169,7 +169,7 @@ public void subscribe(MessageSubscriber subscriber, String topic) {
    */
   static class Topic {
 
-    private final String topic;
+    private final String value;
 
     private final Set<MessageSubscriber> subscribers;
 
@@ -185,7 +185,7 @@ static Topic create(String topic) {
 
     protected Topic(String topic) {
       super();
-      this.topic = topic;
+      this.value = topic;
       subscribers = new HashSet<>();
     }
 
@@ -198,11 +198,11 @@ synchronized void removeSubscriber(MessageSubscriber subscriber) {
     }
 
     boolean matchesTopic(String topic) {
-      return this.topic.equalsIgnoreCase(topic);
+      return this.value.equalsIgnoreCase(topic);
     }
 
     synchronized void informAllSubscribers(String message, MessageParameters messageParameters) {
-      if (messageParameters.messageType.equalsIgnoreCase(topic)) {
+      if (messageParameters.messageType.equalsIgnoreCase(value)) {
         informSubscribers(message, messageParameters);
       }
     }

domain-concept/src/test/groovy/life/qbic/domain/concepts/TestEvent.groovy

@@ -1,8 +1,4 @@
 package life.qbic.domain.concepts
-
-
-import java.time.Instant
-
 /**
  * <b><class short description - 1 Line!></b>
  *

email-service-provider/src/main/java/life/qbic/infrastructure/email/identity/IdentityEmailServiceProvider.java

@@ -33,11 +33,11 @@ public IdentityEmailServiceProvider(EmailServiceProvider emailServiceProvider) {
   @Override
   public void send(Subject subject, Recipient recipient, Content content)
       throws CommunicationException {
-    Optional.ofNullable(subject)
+    subject = Optional.ofNullable(subject)
         .orElseThrow(() -> new CommunicationException("No subject provided."));
-    Optional.ofNullable(recipient)
+    recipient = Optional.ofNullable(recipient)
         .orElseThrow(() -> new CommunicationException("No recipient provided."));
-    Optional.ofNullable(content)
+    content = Optional.ofNullable(content)
         .orElseThrow(() -> new CommunicationException("No content provided."));
 
     try {

identity/src/main/java/life/qbic/identity/application/user/IdentityService.java

@@ -64,7 +64,7 @@ public ApplicationResponse registerUser(final String fullName, String userName,
 
     var userDomainService = DomainRegistry.instance().userDomainService();
     if (userDomainService.isEmpty()) {
-      throw new RuntimeException("User registration failed.");
+      throw new ApplicationException("User registration failed.");
     }
 
     var userEmail = EmailAddress.from(email);
@@ -98,7 +98,7 @@ public ApplicationResponse registerOpenIdUser(String fullName, String userName,
 
     var userDomainService = DomainRegistry.instance().userDomainService();
     if (userDomainService.isEmpty()) {
-      throw new RuntimeException("User registration failed.");
+      throw new ApplicationException("User registration failed.");
     }
 
     var userEmail = EmailAddress.from(email);
@@ -301,6 +301,7 @@ public static class UserNameNotAvailableException extends ApplicationException {
     private static final long serialVersionUID = 4409722243047442583L;
 
     public UserNameNotAvailableException() {
+      super();
     }
   }
 

identity/src/main/java/life/qbic/identity/application/user/policy/EmailConfirmationLinkSupplier.java

@@ -2,6 +2,7 @@
 
 import java.net.MalformedURLException;
 import java.net.URL;
+import life.qbic.application.commons.ApplicationException;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Service;
 
@@ -46,7 +47,7 @@ public String emailConfirmationUrl(String userId) {
             URL url = new URL(protocol, host, port, pathWithQuery);
             return url.toExternalForm();
         } catch (MalformedURLException e) {
-            throw new RuntimeException("Link creation failed.", e);
+            throw new ApplicationException("Link creation failed.", e);
         }
 
     }

project-management/src/main/java/life/qbic/projectmanagement/application/AddExperimentToProjectService.java

@@ -61,7 +61,6 @@ public Result<ExperimentId, RuntimeException> addExperimentToProject(ProjectId p
       String specimenIconLabel) {
       requireNonNull(projectId, "project id must not be null during experiment creation");
       if (experimentName.isBlank()) {
-        //ToDo Add Iterator for multiple experiments?
         experimentName = "Unnamed Experiment";
       }
       if (CollectionUtils.isEmpty(species)) {

project-management/src/main/java/life/qbic/projectmanagement/application/CollaboratorUserInfosConverter.java

@@ -1,7 +1,6 @@
 package life.qbic.projectmanagement.application;
 
 import static java.util.Objects.isNull;
-import static life.qbic.logging.service.LoggerFactory.logger;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -11,7 +10,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
-import life.qbic.logging.api.Logger;
+import life.qbic.application.commons.ApplicationException;
 import life.qbic.projectmanagement.application.ProjectOverview.UserInfo;
 
 /**
@@ -24,8 +23,6 @@
 public class CollaboratorUserInfosConverter implements
     AttributeConverter<List<UserInfo>, String> {
 
-  private static final Logger log = logger(CollaboratorUserInfosConverter.class);
-
   private final ObjectMapper objectMapper = new ObjectMapper();
 
   @Override
@@ -41,7 +38,7 @@ public String convertToDatabaseColumn(List<UserInfo> attribute) {
       return outputStream.toString();
     } catch (IOException e) {
       // we need to throw to prevent data loss
-      throw new RuntimeException(
+      throw new ApplicationException(
           "Unexpected problems writing project collaborators to the database", e);
     }
   }
@@ -58,7 +55,7 @@ public List<UserInfo> convertToEntityAttribute(String dbData) {
       );
     } catch (JsonProcessingException e) {
       // we need to throw to prevent data loss
-      throw new RuntimeException("Unexpected failure parsing project collaborators from database",
+      throw new ApplicationException("Unexpected failure parsing project collaborators from database",
           e);
     }
 

project-management/src/main/java/life/qbic/projectmanagement/application/ContactRepository.java

@@ -11,7 +11,6 @@ public class ContactRepository {
 
   @PostFilter("hasAnyAuthority('ROLE_ADMIN')")
   public List<Contact> findAll() {
-      //TODO implement
       return dummyContacts();
   }
 

project-management/src/main/java/life/qbic/projectmanagement/application/DeletionService.java

@@ -1,12 +1,10 @@
 package life.qbic.projectmanagement.application;
 
 import static java.util.Objects.requireNonNull;
-import static life.qbic.logging.service.LoggerFactory.logger;
 
 import java.util.Collection;
 import life.qbic.application.commons.ApplicationException;
 import life.qbic.application.commons.Result;
-import life.qbic.logging.api.Logger;
 import life.qbic.projectmanagement.application.experiment.ExperimentInformationService;
 import life.qbic.projectmanagement.application.sample.SampleInformationService;
 import life.qbic.projectmanagement.domain.model.batch.BatchId;
@@ -17,6 +15,7 @@
 import life.qbic.projectmanagement.domain.service.BatchDomainService;
 import life.qbic.projectmanagement.domain.service.SampleDomainService;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.ApplicationContext;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
@@ -30,7 +29,8 @@
 @Service
 public class DeletionService {
 
-  private static final Logger log = logger(DeletionService.class);
+  private ApplicationContext context;
+
   private final ProjectInformationService projectInformationService;
   private final ExperimentInformationService experimentInformationService;
   private final SampleInformationService sampleInformationService;
@@ -41,7 +41,7 @@ public class DeletionService {
   public DeletionService(ProjectInformationService projectInformationService,
       ExperimentInformationService experimentInformationService,
       SampleInformationService sampleInformationService, BatchDomainService batchDomainService,
-      SampleDomainService sampleDomainService) {
+      SampleDomainService sampleDomainService, ApplicationContext context) {
     this.projectInformationService = requireNonNull(projectInformationService,
         "experimentInformationService must not be null");
     this.experimentInformationService = requireNonNull(experimentInformationService,
@@ -86,7 +86,9 @@ public BatchId deleteBatch(ProjectId projectId, BatchId batchId) {
     deletedBatchId.onError(error -> {
       throw new ApplicationException("Could not delete batch " + batchId);
     });
-    deleteSamples(projectId, batchId, samples);
+    // We need to get the proxy Spring has wrapped around the service, otherwise calling
+    // the @transaction annotated method has no effect
+    context.getBean(DeletionService.class).deleteSamples(projectId, batchId, samples);
     return deletedBatchId.getValue();
   }
 

project-management/src/main/java/life/qbic/projectmanagement/application/batch/BatchRegistrationService.java

@@ -1,6 +1,7 @@
 package life.qbic.projectmanagement.application.batch;
 
 
+import java.security.SecureRandom;
 import java.util.Collection;
 import java.util.Objects;
 import java.util.Random;
@@ -45,6 +46,7 @@ public class BatchRegistrationService {
   private final SampleInformationService sampleInformationService;
   private final SampleRegistrationService sampleRegistrationService;
   private final DeletionService deletionService;
+  private static final Random RANDOM = new SecureRandom();
 
   @Autowired
   public BatchRegistrationService(BatchRepository batchRepository,
@@ -105,7 +107,6 @@ public Result<BatchId, ResponseCode> addSamplesToBatch(Collection<SampleId> samp
   }
 
   public Result<BatchId, ResponseCode> addSampleToBatch(SampleId sampleId, BatchId batchId) {
-    var random = new Random();
     while (true) {
       try {
         return tryToUpdateBatch(sampleId, batchId);
@@ -115,9 +116,14 @@ public Result<BatchId, ResponseCode> addSampleToBatch(SampleId sampleId, BatchId
                 batchId.value()));
       }
       try {
-        Thread.sleep(random.nextInt(500));
+        Thread.sleep(RANDOM.nextInt(500));
       } catch (InterruptedException e) {
-        throw new RuntimeException(e);
+        log.error("Batch update interrupted", e);
+        // Try to update one last time
+        var result = tryToUpdateBatch(sampleId, batchId);
+        result.onValue(id -> log.info("Updating batch %s was successful.".formatted(batchId.value())));
+        result.onError(responseCode -> log.error("Updating batch failed. Response code was: %s".formatted(responseCode)));
+        Thread.currentThread().interrupt();
       }
     }
   }

project-management/src/main/java/life/qbic/projectmanagement/application/policy/directive/AddSampleToBatch.java

@@ -1,10 +1,7 @@
 package life.qbic.projectmanagement.application.policy.directive;
 
-import static life.qbic.logging.service.LoggerFactory.logger;
-
 import life.qbic.domain.concepts.DomainEvent;
 import life.qbic.domain.concepts.DomainEventSubscriber;
-import life.qbic.logging.api.Logger;
 import life.qbic.projectmanagement.application.batch.BatchRegistrationService;
 import life.qbic.projectmanagement.domain.model.batch.BatchId;
 import life.qbic.projectmanagement.domain.model.sample.SampleId;
@@ -23,7 +20,6 @@
 @Component
 public class AddSampleToBatch implements DomainEventSubscriber<SampleRegistered> {
 
-  private static final Logger log = logger(AddSampleToBatch.class);
   private final BatchRegistrationService batchRegistrationService;
 
   private final JobScheduler jobScheduler;
@@ -45,9 +41,9 @@ public void handleEvent(SampleRegistered event) {
   }
 
   @Job(name = "Add sample %0 to batch %1")
-  public void addSampleToBatch(SampleId sample, BatchId batch) throws RuntimeException {
+  public void addSampleToBatch(SampleId sample, BatchId batch) throws DirectiveExecutionException {
     batchRegistrationService.addSampleToBatch(sample, batch).onError(responseCode -> {
-      throw new RuntimeException(
+      throw new DirectiveExecutionException(
           String.format("Adding sample %s to batch %s failed, response code was %s ", sample, batch,
               responseCode));
     });

project-management/src/main/java/life/qbic/projectmanagement/application/policy/directive/CreateNewSampleStatisticsEntry.java

@@ -1,12 +1,10 @@
 package life.qbic.projectmanagement.application.policy.directive;
 
 import static java.util.Objects.requireNonNull;
-import static life.qbic.logging.service.LoggerFactory.logger;
 
 import java.util.Optional;
 import life.qbic.domain.concepts.DomainEvent;
 import life.qbic.domain.concepts.DomainEventSubscriber;
-import life.qbic.logging.api.Logger;
 import life.qbic.projectmanagement.application.api.SampleCodeService;
 import life.qbic.projectmanagement.domain.model.project.Project;
 import life.qbic.projectmanagement.domain.model.project.ProjectId;
@@ -27,8 +25,6 @@
 public class CreateNewSampleStatisticsEntry implements
     DomainEventSubscriber<ProjectRegisteredEvent> {
 
-  private static final Logger log = logger(CreateNewSampleStatisticsEntry.class);
-
   private final SampleCodeService sampleCodeService;
 
   private final ProjectRepository projectRepository;
@@ -52,13 +48,13 @@ public void handleEvent(ProjectRegisteredEvent event) {
   }
 
   @Job(name = "Create sample statistics entry for project %0")
-  public void createSampleStatisticsEntry(String projectId) throws RuntimeException {
+  public void createSampleStatisticsEntry(String projectId) throws DirectiveExecutionException {
     var id = ProjectId.parse(projectId);
     if (sampleStatisticsEntryMissing(id)) {
       Optional<Project> searchResult = projectRepository.find(id).stream()
           .findFirst();
       if (searchResult.isEmpty()) {
-        throw new RuntimeException("Project with id " + projectId
+        throw new DirectiveExecutionException("Project with id " + projectId
             + " not found. Domain event processing failed for directive "
             + getClass().getSimpleName());
       }

project-management/src/main/java/life/qbic/projectmanagement/application/policy/directive/DeleteSampleFromBatch.java

@@ -47,13 +47,13 @@ public void handleEvent(SampleDeleted event) {
   }
 
   @Job(name = "Delete sample %0 from batch %1")
-  public void deleteSampleFromBatch(SampleId sample, BatchId batch) throws RuntimeException {
+  public void deleteSampleFromBatch(SampleId sample, BatchId batch) throws DirectiveExecutionException {
     batchRegistrationService.deleteSampleFromBatch(sample, batch).onError(responseCode -> {
       if (Objects.requireNonNull(responseCode) == ResponseCode.UNKNOWN_BATCH) {
         // This accounts for a batch that might have already been deleted, so nothing is to be done
         log.warn("Cannot delete sample from unknown batch: %s".formatted(batch.value()));
       } else {
-        throw new RuntimeException(
+        throw new DirectiveExecutionException(
             String.format("Deletion of sample %s from batch %s failed, response code was %s ",
                 sample,
                 batch,

project-management/src/main/java/life/qbic/projectmanagement/application/policy/directive/DirectiveExecutionException.java

@@ -0,0 +1,15 @@
+package life.qbic.projectmanagement.application.policy.directive;
+
+/**
+ * <b>Policy Execution Exception </b>
+ *
+ * <p>Should be used to indicate an exception during the execution of a policy directive.</p>
+ *
+ * @since 1.7.0
+ */
+public class DirectiveExecutionException extends RuntimeException {
+  public DirectiveExecutionException(String message) {
+    super(message);
+  }
+
+}

project-management/src/main/java/life/qbic/projectmanagement/application/policy/directive/InformUserAboutGrantedAccess.java

@@ -66,7 +66,7 @@ public void handleEvent(ProjectAccessGranted event) {
 
   @Job(name = "Notify user %0 about granted access to project %1")
   public void notifyUser(String userId, String projectId, String projectTitle)
-      throws RuntimeException {
+      throws DirectiveExecutionException {
     var recipient = userInformationService.findById(userId).orElseThrow();
     var projectUrl = appContextProvider.urlToProject(projectId);
     var message = Messages.projectAccessToUser(recipient.fullName(), projectTitle, projectUrl);