From e89026591e765fed0ed96ebc44cb55f2ce2e76e1 Mon Sep 17 00:00:00 2001 From: anhefti Date: Thu, 16 Nov 2023 11:23:41 +0100 Subject: [PATCH] SEBSERV-476 - fixed edge case with config creation from Exam Template (name exists) Some code cleanup --- .../ethz/seb/sebserver/gbl/util/Result.java | 2 +- .../dao/AdditionalAttributesDAO.java | 10 ++++----- .../dao/impl/AdditionalAttributesDAOImpl.java | 4 ++-- .../exam/impl/ExamTemplateServiceImpl.java | 22 ++++++++++++++----- .../plugin/MoodlePluginCourseAccess.java | 10 ++++----- .../api/ExamAdministrationController.java | 2 +- 6 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/util/Result.java b/src/main/java/ch/ethz/seb/sebserver/gbl/util/Result.java index 9d1f238d..85236ad6 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/util/Result.java +++ b/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. - * + *

* 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 diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/AdditionalAttributesDAO.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/AdditionalAttributesDAO.java index 853c61f6..60995295 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/AdditionalAttributesDAO.java +++ b/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. - * + *

* 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 entityId the entity identifier (primary key) + * @param entityKey the entity key * @return Result refer to the collection of additional attribute records or to an error if happened */ default Result> 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, diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/AdditionalAttributesDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/AdditionalAttributesDAOImpl.java index 5004c810..3d6bd629 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/AdditionalAttributesDAOImpl.java +++ b/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()); } } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamTemplateServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamTemplateServiceImpl.java index d46fa2bf..c9e1978c 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/exam/impl/ExamTemplateServiceImpl.java +++ b/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); diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/plugin/MoodlePluginCourseAccess.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/plugin/MoodlePluginCourseAccess.java index a7bca8e7..db8d5458 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/plugin/MoodlePluginCourseAccess.java +++ b/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 missingIds = new HashSet<>(ids); final Collection result = new ArrayList<>(); final Set 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 ids = Stream.of(id).collect(Collectors.toSet()); final Iterator 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), diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java index 9aeaa87f..ff01c929 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/ExamAdministrationController.java @@ -196,7 +196,7 @@ public class ExamAdministrationController extends EntityController { return this.examDAO.byPK(modelId) .flatMap(this::checkWriteAccess) .flatMap(this.examAdminService::archiveExam) - .flatMap(exam -> super.userActivityLogDAO.logArchive(exam)) + .flatMap(super.userActivityLogDAO::logArchive) .getOrThrow(); }