SEBSERV-476 - fixed edge case with config creation from Exam Template (name exists)

Some code cleanup

src/main/java/ch/ethz/seb/sebserver/gbl/util/Result.java

@@ -18,7 +18,7 @@ import org.slf4j.LoggerFactory;
 
 /** A result of a computation that can either be the resulting value of the computation
  * or an error if an exception/error has been thrown during the computation.
- *
+ * <p>
  * Use this to make code more resilient at the same time as avoid many try-catch-blocks
  * on root exceptions/errors and the need of nested exceptions.
  * More specific: use this if a none void method should always give a result and never throw an exception

src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/AdditionalAttributesDAO.java

@@ -22,7 +22,7 @@ import ch.ethz.seb.sebserver.gbl.util.Result;
 import ch.ethz.seb.sebserver.webservice.datalayer.batis.model.AdditionalAttributeRecord;
 
 /** Defines functionality to access additional attributes.
- *
+ * <p>
  * Additional attributes are name/value pairs associated with a specified entity but stored
  * in a separated data-base table. */
 public interface AdditionalAttributesDAO {
@@ -31,8 +31,7 @@ public interface AdditionalAttributesDAO {
 
     /** Use this to get all additional attribute records for a specific entity.
      *
-     * @param type the entity type
+     * @param entityKey the entity key
-     * @param entityId the entity identifier (primary key)
      * @return Result refer to the collection of additional attribute records or to an error if happened */
     default Result<Collection<AdditionalAttributeRecord>> getAdditionalAttributes(final EntityKey entityKey) {
         return Result.tryCatch(() -> getAdditionalAttributes(
@@ -87,12 +86,13 @@ public interface AdditionalAttributesDAO {
 
     /** Use this to initialize an additional attribute for a specific entity.
      * If the additional attribute with specified name already exists for the specified entity
-     * this this is just ignored and nothing changes.
+     * this is just ignored and nothing changes.
      *
      * @param type the entity type
      * @param entityId the entity identifier (primary key)
      * @param name the name of the attribute
-     * @param value the value of the attribute */
+     * @param value the value of the attribute
+     * @return true if initialization was successfully*/
     boolean initAdditionalAttribute(
             EntityType type,
             Long entityId,

src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/AdditionalAttributesDAOImpl.java

@@ -192,7 +192,7 @@ public class AdditionalAttributesDAOImpl implements AdditionalAttributesDAO {
                             AdditionalAttributeRecordDynamicSqlSupport.name,
                             SqlBuilder.isEqualTo(name))
                     .build()
-                    .execute().longValue() > 0;
+                    .execute() > 0;
 
             if (!exists) {
                 final AdditionalAttributeRecord rec = new AdditionalAttributeRecord(
@@ -271,7 +271,7 @@ public class AdditionalAttributesDAOImpl implements AdditionalAttributesDAO {
                     .build()
                     .execute();
         } catch (final Exception e) {
-            log.error("Failed to delete all additional attributes for: {} cause: {}", entityId, e);
+            log.error("Failed to delete all additional attributes for: {} cause: {}", entityId, e.getMessage());
         }
     }
 

src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamTemplateServiceImpl.java

@@ -11,6 +11,7 @@ package ch.ethz.seb.sebserver.webservice.servicelayer.exam.impl;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 
 import org.apache.commons.lang3.StringUtils;
 import org.joda.time.DateTime;
@@ -134,7 +135,7 @@ public class ExamTemplateServiceImpl implements ExamTemplateService {
 
                 final ExamTemplate examTemplate = this.examTemplateDAO
                         .byPK(exam.examTemplateId)
-                        .onError(error -> log.warn("No exam template found for id: {}",
+                        .onError(error -> log.warn("No exam template found for id: {} error: {}",
                                 exam.examTemplateId,
                                 error.getMessage()))
                         .getOr(null);
@@ -165,7 +166,7 @@ public class ExamTemplateServiceImpl implements ExamTemplateService {
 
                 final ExamTemplate examTemplate = this.examTemplateDAO
                         .byPK(exam.examTemplateId)
-                        .onError(error -> log.warn("No exam template found for id: {}",
+                        .onError(error -> log.warn("No exam template found for id: {} error: {}",
                                 exam.examTemplateId,
                                 error.getMessage()))
                         .getOr(null);
@@ -200,7 +201,7 @@ public class ExamTemplateServiceImpl implements ExamTemplateService {
 
                 final ExamTemplate examTemplate = this.examTemplateDAO
                         .byPK(exam.examTemplateId)
-                        .onError(error -> log.warn("No exam template found for id: {}",
+                        .onError(error -> log.warn("No exam template found for id: {} error: {}",
                                 exam.examTemplateId,
                                 error.getMessage()))
                         .getOr(null);
@@ -254,12 +255,21 @@ public class ExamTemplateServiceImpl implements ExamTemplateService {
                 .findFirst()
                 .orElse(null);
 
-        if (examConfig == null || examConfig.status != ConfigurationStatus.READY_TO_USE) {
+
+        // create new configuration if we don't have an old config that is on READY_TO_USE or the template has changed
+        if (examConfig == null ||
+                examConfig.status != ConfigurationStatus.READY_TO_USE ||
+                !Objects.equals(examConfig.templateId, examTemplate.configTemplateId)) {
+
+            final String newName = (examConfig != null && examConfig.name.equals(configName))
+                    ? examConfig.name + "_"
+                    : configName;
+
             final ConfigurationNode config = new ConfigurationNode(
                     null,
                     exam.institutionId,
                     examTemplate.configTemplateId,
-                    configName,
+                    newName,
                     replaceVars(this.defaultExamConfigDescTemplate, exam, examTemplate),
                     ConfigurationType.EXAM_CONFIG,
                     exam.owner,
@@ -316,7 +326,7 @@ public class ExamTemplateServiceImpl implements ExamTemplateService {
 
                 final ExamTemplate examTemplate = this.examTemplateDAO
                         .byPK(exam.examTemplateId)
-                        .onError(error -> log.warn("No exam template found for id: {}",
+                        .onError(error -> log.warn("No exam template found for id: {} error: {}",
                                 exam.examTemplateId,
                                 error.getMessage()))
                         .getOr(null);

src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/plugin/MoodlePluginCourseAccess.java

@@ -213,7 +213,7 @@ public class MoodlePluginCourseAccess extends AbstractCachedCourseAccess impleme
             final Set<String> missingIds = new HashSet<>(ids);
             final Collection<QuizData> result = new ArrayList<>();
             final Set<String> fromCache = ids.stream()
-                    .map(id -> super.getFromCache(id))
+                    .map(super::getFromCache)
                     .filter(Objects::nonNull)
                     .map(qd -> {
                         result.add(qd);
@@ -225,9 +225,9 @@ public class MoodlePluginCourseAccess extends AbstractCachedCourseAccess impleme
 
                 result.addAll(getRestTemplate()
                         .map(template -> getQuizzesForIds(template, ids))
-                        .map(qd -> super.putToCache(qd))
+                        .map(super::putToCache)
                         .onError(error -> log.error("Failed to get courses for: {}", ids, error))
-                        .getOrElse(() -> Collections.emptyList()));
+                        .getOrElse(Collections::emptyList));
             }
 
             return result;
@@ -246,7 +246,7 @@ public class MoodlePluginCourseAccess extends AbstractCachedCourseAccess impleme
             final Set<String> ids = Stream.of(id).collect(Collectors.toSet());
             final Iterator<QuizData> iterator = getRestTemplate()
                     .map(template -> getQuizzesForIds(template, ids))
-                    .map(qd -> super.putToCache(qd))
+                    .map(super::putToCache)
                     .getOr(Collections.emptyList())
                     .iterator();
 
@@ -426,7 +426,7 @@ public class MoodlePluginCourseAccess extends AbstractCachedCourseAccess impleme
             final long filterDate = Utils.toUnixTimeInSeconds(quizFromTime);
             final long defaultCutOff = Utils.toUnixTimeInSeconds(
                     DateTime.now(DateTimeZone.UTC).minusYears(this.cutoffTimeOffset));
-            final long cutoffDate = (filterDate < defaultCutOff) ? filterDate : defaultCutOff;
+            final long cutoffDate = Math.min(filterDate, defaultCutOff);
             String sqlCondition = String.format(
                     SQL_CONDITION_TEMPLATE,
                     String.valueOf(cutoffDate),

src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java

@@ -196,7 +196,7 @@ public class ExamAdministrationController extends EntityController<Exam, Exam> {
         return this.examDAO.byPK(modelId)
                 .flatMap(this::checkWriteAccess)
                 .flatMap(this.examAdminService::archiveExam)
-                .flatMap(exam -> super.userActivityLogDAO.logArchive(exam))
+                .flatMap(super.userActivityLogDAO::logArchive)
                 .getOrThrow();
     }