Improve software quality by addressing some reliability issues (#931)

Improves the reliability and maintenance of the software by addressing various issues.

---------

Co-authored-by: Steffengreiner <[email protected]>

Author: sven1103

.github/workflows/sonarcloud.yml

@@ -5,7 +5,10 @@ on:
   push:
     branches:
       - development
-
+  pull_request:
+    branches:
+      - development
+      - main
 jobs:
   sonarcloud:
     runs-on: ubuntu-latest

domain-concept/src/main/java/life/qbic/domain/concepts/DomainEventPublisher.java

@@ -24,6 +24,7 @@
 public class ProjectManagementEmailServiceProvider implements EmailService {
 
   private static final Logger log = logger(ProjectManagementEmailServiceProvider.class);
+  public static final String EMAIL_SUBMISSION_FAILED = "Email submission failed";
 
   private final EmailServiceProvider emailServiceProvider;
 
@@ -39,7 +40,7 @@ public void send(Subject subject, Recipient recipient, Content content)
           MessageTranslator.translate(content));
     } catch (EmailSubmissionException e) {
       log.error("Email submission failed!", e);
-      throw new CommunicationException("Email submission failed");
+      throw new CommunicationException(EMAIL_SUBMISSION_FAILED);
     }
   }
 
@@ -50,8 +51,8 @@ public void send(Subject subject, Recipient recipient, Content content, Attachme
       emailServiceProvider.send(MessageTranslator.translate(subject), MessageTranslator.translate(recipient),
           MessageTranslator.translate(content), MessageTranslator.translate(attachment));
     } catch (EmailSubmissionException e) {
-      log.error("Email submission failed", e);
-      throw new CommunicationException("Email submission failed");
+      log.error(EMAIL_SUBMISSION_FAILED, e);
+      throw new CommunicationException(EMAIL_SUBMISSION_FAILED);
     }
   }
 

email-service-provider/src/main/java/life/qbic/infrastructure/email/project/ProjectManagementEmailServiceProvider.java

@@ -20,7 +20,6 @@
 import life.qbic.logging.api.Logger;
 import life.qbic.projectmanagement.application.OrganisationRepository;
 import life.qbic.projectmanagement.domain.Organisation;
-import org.springframework.stereotype.Component;
 
 /**
  * <b>Cached Organisation Repository</b>
@@ -35,7 +34,6 @@
  *
  * @since 1.0.0s
  */
-@Component
 public class CachedOrganisationRepository implements OrganisationRepository {
 
   private static final Logger log = logger(CachedOrganisationRepository.class);
@@ -53,7 +51,6 @@ public CachedOrganisationRepository(int cacheSize) {
     this.configuredCacheSize = cacheSize;
   }
 
-
   public CachedOrganisationRepository() {
     this.configuredCacheSize = DEFAULT_CACHE_SIZE;
   }

project-management-infrastructure/src/main/java/life/qbic/projectmanagement/infrastructure/CachedOrganisationRepository.java

@@ -17,7 +17,7 @@
  */
 public class QbicOidcUser extends DefaultOidcUser {
 
-  private final QbicUserInfo qbicUserInfo;
+  private final transient QbicUserInfo qbicUserInfo;
   private final String originalAuthName;
 
   public record QbicUserInfo(String userId, String fullName, String email, boolean active) {

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

@@ -21,7 +21,7 @@
 public class QbicUserDetails implements UserDetails {
   @Serial
   private static final long serialVersionUID = 5812210012669790933L;
-  private final User user;
+  private final transient User user;
   private final List<GrantedAuthority> grantedAuthorities;
 
   /**

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

@@ -43,6 +43,7 @@
 public class ProjectAccessServiceImpl implements ProjectAccessService {
 
   private static final Logger log = logger(ProjectAccessServiceImpl.class);
+  public static final String SELECT_IDENTITY = "SELECT @@IDENTITY";
   private final MutableAclService aclService;
   private final JdbcTemplate jdbcTemplate;
 
@@ -59,8 +60,8 @@ private static MutableAcl getAclForProject(ProjectId projectId, List<Sid> sids,
     JdbcMutableAclService serviceImpl = (JdbcMutableAclService) mutableAclService;
     // these settings are necessary for MySQL to correctly throw several types of exceptions
     // instead of an unrelated exception related to the identity function
-    serviceImpl.setClassIdentityQuery("SELECT @@IDENTITY");
-    serviceImpl.setSidIdentityQuery("SELECT @@IDENTITY");
+    serviceImpl.setClassIdentityQuery(SELECT_IDENTITY);
+    serviceImpl.setSidIdentityQuery(SELECT_IDENTITY);
     try {
       acl = (MutableAcl) serviceImpl.readAclById(objectIdentity, sids);
     } catch (NotFoundException e) {
@@ -75,8 +76,8 @@ private static MutableAcl createAclForProject(ProjectId projectId,
     JdbcMutableAclService serviceImpl = (JdbcMutableAclService) mutableAclService;
     // these settings are necessary for MySQL to correctly throw several types of exceptions
     // instead of an unrelated exception related to the identity function
-    serviceImpl.setClassIdentityQuery("SELECT @@IDENTITY");
-    serviceImpl.setSidIdentityQuery("SELECT @@IDENTITY");
+    serviceImpl.setClassIdentityQuery(SELECT_IDENTITY);
+    serviceImpl.setSidIdentityQuery(SELECT_IDENTITY);
     return serviceImpl.createAcl(objectIdentity);
   }
 

project-management/src/main/java/life/qbic/projectmanagement/application/authorization/acl/ProjectAccessServiceImpl.java

@@ -219,7 +219,7 @@ public CompletableFuture<List<Result<MeasurementId, ErrorCode>>> registerAll(
   @PreAuthorize(
       "hasPermission(#projectId, 'life.qbic.projectmanagement.domain.model.project.Project', 'WRITE')")
   @Transactional
-  protected List<MeasurementId> performRegistration(
+  public List<MeasurementId> performRegistration(
       List<? extends MeasurementMetadata> measurementMetadataList, ProjectId projectId) {
     if (measurementMetadataList.isEmpty()) {
       return new ArrayList<>(); // Nothing to do
@@ -243,7 +243,6 @@ private List<MeasurementId> performRegistrationNGS(
         ngsMeasurementMetadata.add(ngsMetadata);
       }
     }
-    Map<NGSMeasurement, Collection<SampleIdCodeEntry>> ngsMeasurementsMapping = new HashMap<>();
 
     // Start with the pooled measurements first and group the metadata entries by pool
     Map<String, List<NGSMeasurementMetadata>> measurementsByPool = ngsMeasurementMetadata.stream()
@@ -255,7 +254,8 @@ private List<MeasurementId> performRegistrationNGS(
         .filter(metadata -> metadata.assignedSamplePoolGroup().isEmpty()).toList();
 
     // Then merge and prepare the domain objects by pool
-    ngsMeasurementsMapping.putAll(mergeByPoolNGS(measurementsByPool, projectId));
+    Map<NGSMeasurement, Collection<SampleIdCodeEntry>> ngsMeasurementsMapping = new HashMap<>(
+        mergeByPoolNGS(measurementsByPool, projectId));
     // and last but not least also the single sample measurements
     singleMeasurements.stream()
         .map(singleMeasurement -> buildNGS(List.of(singleMeasurement), projectId))
@@ -273,7 +273,6 @@ private List<MeasurementId> performRegistrationPxP(
         proteomicsMeasurements.add(proteomicsMetadata);
       }
     }
-    Map<ProteomicsMeasurement, Collection<SampleIdCodeEntry>> proteomicsMeasurementsMapping = new HashMap<>();
 
     // Start with the pooled measurements first and group the metadata entries by pool
     Map<String, List<ProteomicsMeasurementMetadata>> measurementsByPool = proteomicsMeasurements.stream()
@@ -285,7 +284,8 @@ private List<MeasurementId> performRegistrationPxP(
         .filter(metadata -> metadata.assignedSamplePoolGroup().isEmpty()).toList();
 
     // Then merge and prepare the domain objects by pool
-    proteomicsMeasurementsMapping.putAll(mergeByPoolPxP(measurementsByPool, projectId));
+    Map<ProteomicsMeasurement, Collection<SampleIdCodeEntry>> proteomicsMeasurementsMapping = new HashMap<>(
+        mergeByPoolPxP(measurementsByPool, projectId));
     // and last but not least also the single sample measurements
     singleMeasurements.stream()
         .map(singleMeasurement -> buildPxP(List.of(singleMeasurement), projectId))
@@ -346,7 +346,7 @@ private Map<NGSMeasurement, Collection<SampleIdCodeEntry>> buildNGS(
     NGSMeasurement measurement;
     if (firstMetadataEntry.assignedSamplePoolGroup().isPresent()) {
       measurement = NGSMeasurement.createWithPool(projectId,
-          firstMetadataEntry.assignedSamplePoolGroup().get(), assignedMeasurementCode,
+          firstMetadataEntry.assignedSamplePoolGroup().orElseThrow(), assignedMeasurementCode,
           organisationQuery.get(), method, specificMetadata);
     } else {
       measurement = NGSMeasurement.createSingleMeasurement(projectId, assignedMeasurementCode,

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

@@ -20,6 +20,8 @@
  */
 public class PropertyConversion {
 
+  private PropertyConversion(){}
+
   private static final String CONDITION_VARIABLE_LEVEL_UNIT_TEMPLATE = "%s: %s %s"; // <variable name>: <value> [unit]
 
   private static final String CONDITION_VARIABLE_LEVEL_NO_UNIT_TEMPLATE = "%s: %s"; // <variable name>: <value>
@@ -72,7 +74,7 @@ public static String toString(VariableLevel variableLevel) {
     if (variableLevel.experimentalValue().unit().isPresent()) {
       return CONDITION_VARIABLE_LEVEL_UNIT_TEMPLATE.formatted(variableLevel.variableName().value(),
           variableLevel.experimentalValue().value(),
-          variableLevel.experimentalValue().unit().get());
+          variableLevel.experimentalValue().unit().orElseThrow());
     } else {
       return CONDITION_VARIABLE_LEVEL_NO_UNIT_TEMPLATE.formatted(
           variableLevel.variableName().value(), variableLevel.experimentalValue().value());

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

@@ -80,4 +80,8 @@ public String fractionName() {
     return fractionName;
   }
 
+  public String comment() {
+    return comment;
+  }
+
 }

project-management/src/main/java/life/qbic/projectmanagement/domain/model/measurement/ProteomicsSpecificMeasurementMetadata.java

@@ -2,6 +2,7 @@
 
 import jakarta.persistence.Column;
 import jakarta.persistence.Embeddable;
+import java.security.SecureRandom;
 import java.util.Arrays;
 import java.util.Objects;
 import java.util.Random;
@@ -18,7 +19,7 @@
 @Embeddable
 public class ProjectCode {
 
-  public static final String[] BLACKLIST = new String[]{"FUCK", "SHIT"};
+  private static final String[] BLACKLIST = new String[]{"FUCK", "SHIT"};
 
   @Column(name = "projectCode")
   private String value;
@@ -27,9 +28,11 @@ public class ProjectCode {
 
   private static final String PREFIX = "Q2";
 
-  public static final char[] ALLOWED_LETTERS = "ABCDEFGHIJKLMNOPQRSTUVWX".toCharArray();
+  private static final char[] ALLOWED_LETTERS = "ABCDEFGHIJKLMNOPQRSTUVWX".toCharArray();
 
-  public static final char[] ALLOWED_NUMBERS = "0123456789".toCharArray();
+  private static final char[] ALLOWED_NUMBERS = "0123456789".toCharArray();
+
+  private static final Random RANDOM = new SecureRandom();
 
   protected ProjectCode() {
     // Needed for JPA
@@ -213,11 +216,11 @@ private char generateRandomChar(Coin coin) {
     }
 
     private char randomLetter() {
-      return letters[new Random().nextInt(letters.length)];
+      return letters[RANDOM.nextInt(letters.length)];
     }
 
     private char randomNumber() {
-      return numbers[new Random().nextInt(numbers.length)];
+      return numbers[RANDOM.nextInt(numbers.length)];
     }
 
   }
@@ -229,7 +232,7 @@ enum SIDE {
     }
 
     SIDE flip() {
-      double randomValue = new Random().nextDouble(1);
+      double randomValue = RANDOM.nextDouble(1);
       if (randomValue < 0.5) {
         return SIDE.HEAD;
       } else {

project-management/src/main/java/life/qbic/projectmanagement/domain/model/project/ProjectCode.java

@@ -0,0 +1,3 @@
+sonar.projectKey=qbicsoftware_data-manager-app
+sonar.organization=qbicsoftware
+sonar.exclusions=src/test/**,build/**,node_modules/**, **/DataPrivacyAgreement.html, **/LegalNotice.html

sonar-project.properties

@@ -33,6 +33,8 @@
  */
 public class PropertyFileParser {
 
+  private PropertyFileParser() {}
+
   /**
    * Parses a file for defined properties and resolves present placeholder against visible
    * environment variables.
@@ -46,7 +48,10 @@ public static Properties parse(File file) throws IOException {
     requireNonNull(file, "File must not be null");
 
     var properties = new Properties();
-    properties.load(new FileInputStream(file));
+
+    try (var fileInputStream = new FileInputStream(file)) {
+      properties.load(fileInputStream);
+    }
 
     properties = resolvePlaceholders(properties);
 

subscription-provider/src/main/java/life/qbic/logging/subscription/provider/mail/property/PropertyFileParser.java

@@ -26,6 +26,7 @@
 import life.qbic.infrastructure.email.identity.IdentityEmailServiceProvider;
 import life.qbic.infrastructure.email.project.ProjectManagementEmailServiceProvider;
 import life.qbic.projectmanagement.application.AppContextProvider;
+import life.qbic.projectmanagement.application.OrganisationRepository;
 import life.qbic.projectmanagement.application.ProjectInformationService;
 import life.qbic.projectmanagement.application.api.SampleCodeService;
 import life.qbic.projectmanagement.application.authorization.acl.ProjectAccessService;
@@ -67,6 +68,7 @@
 import life.qbic.projectmanagement.application.sample.SampleInformationService;
 import life.qbic.projectmanagement.application.sample.qualitycontrol.QualityControlService;
 import life.qbic.projectmanagement.domain.repository.ProjectRepository;
+import life.qbic.projectmanagement.infrastructure.CachedOrganisationRepository;
 import org.jobrunr.scheduling.JobScheduler;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Bean;
@@ -97,6 +99,11 @@ public IdentityService userRegistrationService(
     return new IdentityService(userRepository);
   }
 
+  @Bean
+  public OrganisationRepository organisationRepository() {
+    return new CachedOrganisationRepository();
+  }
+
 
   @Bean
   public NewPasswordInput newPasswordInput(IdentityService identityService) {

user-interface/src/main/java/life/qbic/datamanager/AppConfig.java

@@ -35,7 +35,7 @@ public class MyVaadinSessionInitListener implements VaadinServiceInitListener {
   private static final Logger log = logger(MyVaadinSessionInitListener.class);
   private final ExtendedClientDetailsReceiver clientDetailsReceiver;
 
-  private final UiExceptionHandler uiExceptionHandler;
+  private final transient UiExceptionHandler uiExceptionHandler;
   private final LogoutService logoutService;
 
   public MyVaadinSessionInitListener(