SEBSERV-139 adapt to new API with shortname and idnumber

src/main/java/ch/ethz/seb/sebserver/gbl/model/exam/QuizData.java

@@ -45,6 +45,7 @@ public final class QuizData implements GrantEntity {
 
     public static final String ATTR_ADDITIONAL_CREATION_TIME = "time_created";
     public static final String ATTR_ADDITIONAL_SHORT_NAME = "course_short_name";
+    public static final String ATTR_ADDITIONAL_ID_NUMBER = "idnumber";
     public static final String ATTR_ADDITIONAL_FULL_NAME = "course_full_name";
     public static final String ATTR_ADDITIONAL_DISPLAY_NAME = "course_display_name";
     public static final String ATTR_ADDITIONAL_SUMMARY = "course_summary";

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

@@ -17,6 +17,8 @@ import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
+import org.apache.commons.lang3.StringUtils;
+import org.apache.coyote.http11.Constants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.util.LinkedMultiValueMap;
@@ -47,7 +49,6 @@ public class MoodleCourseAccess extends CourseAccess {
     private static final Logger log = LoggerFactory.getLogger(MoodleCourseAccess.class);
 
     private static final String MOODLE_QUIZ_START_URL_PATH = "mod/quiz/view.php?id=";
-    private static final String MOODLE_COURSE_START_URL_PATH = "course/view.php?id=";
     private static final String MOODLE_COURSE_API_FUNCTION_NAME = "core_course_get_courses";
     private static final String MOODLE_USER_PROFILE_API_FUNCTION_NAME = "core_user_get_users_by_field";
     private static final String MOODLE_QUIZ_API_FUNCTION_NAME = "mod_quiz_get_quizzes_by_courses";
@@ -56,7 +57,6 @@ public class MoodleCourseAccess extends CourseAccess {
     private final JSONMapper jsonMapper;
     private final LmsSetup lmsSetup;
     private final MoodleRestTemplateFactory moodleRestTemplateFactory;
-    private final boolean includeCourses;
 
     private MoodleAPIRestTemplate restTemplate;
 
@@ -64,14 +64,12 @@ public class MoodleCourseAccess extends CourseAccess {
             final JSONMapper jsonMapper,
             final LmsSetup lmsSetup,
             final MoodleRestTemplateFactory moodleRestTemplateFactory,
-            final AsyncService asyncService,
+            final AsyncService asyncService) {
-            final boolean includeCourses) {
 
         super(asyncService);
         this.jsonMapper = jsonMapper;
         this.lmsSetup = lmsSetup;
         this.moodleRestTemplateFactory = moodleRestTemplateFactory;
-        this.includeCourses = includeCourses;
     }
 
     @Override
@@ -233,6 +231,7 @@ public class MoodleCourseAccess extends CourseAccess {
         additionalAttrs.clear();
         additionalAttrs.put(QuizData.ATTR_ADDITIONAL_CREATION_TIME, String.valueOf(courseData.time_created));
         additionalAttrs.put(QuizData.ATTR_ADDITIONAL_SHORT_NAME, courseData.short_name);
+        additionalAttrs.put(QuizData.ATTR_ADDITIONAL_ID_NUMBER, courseData.idnumber);
         additionalAttrs.put(QuizData.ATTR_ADDITIONAL_FULL_NAME, courseData.full_name);
         additionalAttrs.put(QuizData.ATTR_ADDITIONAL_DISPLAY_NAME, courseData.display_name);
         additionalAttrs.put(QuizData.ATTR_ADDITIONAL_SUMMARY, courseData.summary);
@@ -243,7 +242,7 @@ public class MoodleCourseAccess extends CourseAccess {
                     final String startURI = uriPrefix + courseQuizData.course_module;
                     additionalAttrs.put(QuizData.ATTR_ADDITIONAL_TIME_LIMIT, String.valueOf(courseQuizData.time_limit));
                     return new QuizData(
-                            courseData.id + ":" + courseQuizData.id,
+                            getInternalQuizId(courseQuizData.id, courseData.short_name, courseData.idnumber),
                             lmsSetup.getInstitutionId(),
                             lmsSetup.id,
                             lmsSetup.getLmsType(),
@@ -256,20 +255,6 @@ public class MoodleCourseAccess extends CourseAccess {
                 })
                 .collect(Collectors.toList());
 
-        if (this.includeCourses) {
-            courseAndQuiz.add(new QuizData(
-                    courseData.id,
-                    lmsSetup.getInstitutionId(),
-                    lmsSetup.id,
-                    lmsSetup.getLmsType(),
-                    courseData.display_name,
-                    courseData.full_name,
-                    Utils.toDateTimeUTCUnix(courseData.start_date),
-                    Utils.toDateTimeUTCUnix(courseData.end_date),
-                    lmsSetup.lmsApiUrl + MOODLE_COURSE_START_URL_PATH + courseData.id,
-                    additionalAttrs));
-        }
-
         return courseAndQuiz;
     }
 
@@ -292,6 +277,7 @@ public class MoodleCourseAccess extends CourseAccess {
     static final class CourseData {
         final String id;
         final String short_name;
+        final String idnumber;
         final String full_name;
         final String display_name;
         final String summary;
@@ -304,6 +290,7 @@ public class MoodleCourseAccess extends CourseAccess {
         protected CourseData(
                 @JsonProperty(value = "id") final String id,
                 @JsonProperty(value = "shortname") final String short_name,
+                @JsonProperty(value = "idnumber") final String idnumber,
                 @JsonProperty(value = "fullname") final String full_name,
                 @JsonProperty(value = "displayname") final String display_name,
                 @JsonProperty(value = "summary") final String summary,
@@ -313,6 +300,7 @@ public class MoodleCourseAccess extends CourseAccess {
 
             this.id = id;
             this.short_name = short_name;
+            this.idnumber = idnumber;
             this.full_name = full_name;
             this.display_name = display_name;
             this.summary = summary;
@@ -323,6 +311,52 @@ public class MoodleCourseAccess extends CourseAccess {
 
     }
 
+    static final String getInternalQuizId(final String quizId, final String shortname, final String idnumber) {
+        final StringBuilder sb = new StringBuilder(quizId);
+        if (StringUtils.isNotEmpty(shortname)) {
+            sb.insert(0, ":").insert(0, shortname);
+        }
+        if (StringUtils.isNotEmpty(idnumber)) {
+            sb.insert(0, ":").insert(0, idnumber);
+        }
+        return sb.toString();
+    }
+
+    static final String getQuizId(final String internalQuizId) {
+        if (StringUtils.isBlank(internalQuizId)) {
+            return null;
+        }
+
+        final String[] ids = internalQuizId.split(internalQuizId, Constants.COLON);
+        return ids[ids.length - 1];
+    }
+
+    static final String getShortname(final String internalQuizId) {
+        if (StringUtils.isBlank(internalQuizId)) {
+            return null;
+        }
+
+        final String[] ids = internalQuizId.split(internalQuizId, Constants.COLON);
+        if (ids.length > 1) {
+            return ids[ids.length - 2];
+        } else {
+            return null;
+        }
+    }
+
+    static final String getIdnumber(final String internalQuizId) {
+        if (StringUtils.isBlank(internalQuizId)) {
+            return null;
+        }
+
+        final String[] ids = internalQuizId.split(internalQuizId, Constants.COLON);
+        if (ids.length == 3) {
+            return ids[0];
+        } else {
+            return null;
+        }
+    }
+
     @JsonIgnoreProperties(ignoreUnknown = true)
     static final class CourseQuizData {
         final Collection<CourseQuiz> quizzes;

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

@@ -35,7 +35,6 @@ import ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl.moodle.MoodleRestT
  *
  * <pre>
  * {
- *   "courseId": "123",
  *   "quizId": "456",
  *   "configKeys": [
  *       "key1",
@@ -65,7 +64,9 @@ public class MoodleCourseRestriction {
     private static final String MOODLE_DEFAULT_COURSE_RESTRICTION_WS_FUNCTION_CREATE = "seb_restriction_create";
     private static final String MOODLE_DEFAULT_COURSE_RESTRICTION_WS_FUNCTION_UPDATE = "seb_restriction_update";
     private static final String MOODLE_DEFAULT_COURSE_RESTRICTION_WS_FUNCTION_DELETE = "seb_restriction_delete";
-    private static final String MOODLE_DEFAULT_COURSE_RESTRICTION_COURSE_ID = "courseId";
+    //private static final String MOODLE_DEFAULT_COURSE_RESTRICTION_COURSE_ID = "courseId";
+    private static final String MOODLE_DEFAULT_COURSE_RESTRICTION_SHORT_NAME = "shortname";
+    private static final String MOODLE_DEFAULT_COURSE_RESTRICTION_ID_NUMBER = "idnumber";
     private static final String MOODLE_DEFAULT_COURSE_RESTRICTION_QUIZ_ID = "quizId";
     private static final String MOODLE_DEFAULT_COURSE_RESTRICTION_CONFIG_KEY = "configKey";
     private static final String MOODLE_DEFAULT_COURSE_RESTRICTION_BROWSER_KEY = "browserKey";
@@ -108,28 +109,24 @@ public class MoodleCourseRestriction {
     }
 
     Result<MoodleSEBRestriction> getSEBRestriction(
-            final String externalId) {
+            final String internalId) {
 
         return Result.tryCatch(() -> {
-            final String[] courseQuizId = StringUtils.split(externalId, ":");
+            return getSEBRestriction(
-            if (courseQuizId.length > 1) {
+                    MoodleCourseAccess.getQuizId(internalId),
-                // we only have the course id (this is a course)
+                    MoodleCourseAccess.getShortname(internalId),
-                return getSEBRestriction(courseQuizId[0], null)
+                    MoodleCourseAccess.getIdnumber(internalId))
-                        .getOrThrow();
+                            .getOrThrow();
-            } else {
-                // we have the course id and the quiz is (this is a quiz)
-                return getSEBRestriction(courseQuizId[0], courseQuizId[1])
-                        .getOrThrow();
-            }
         });
     }
 
     Result<MoodleSEBRestriction> getSEBRestriction(
-            final String courseId,
+            final String quizId,
-            final String quizId) {
+            final String shortname,
+            final String idnumber) {
 
         if (log.isDebugEnabled()) {
-            log.debug("GET SEB Client restriction on course: {} quiz: {}", courseId, quizId);
+            log.debug("GET SEB Client restriction on course: {} quiz: {}", shortname, quizId);
         }
 
         return Result.tryCatch(() -> {
@@ -138,9 +135,12 @@ public class MoodleCourseRestriction {
                     .getOrThrow();
 
             final MultiValueMap<String, String> queryParams = new LinkedMultiValueMap<>();
-            queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_COURSE_ID, courseId);
+            queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_QUIZ_ID, quizId);
-            if (quizId != null) {
+            if (StringUtils.isNotBlank(shortname)) {
-                queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_QUIZ_ID, quizId);
+                queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_SHORT_NAME, shortname);
+            }
+            if (StringUtils.isNotBlank(idnumber)) {
+                queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_ID_NUMBER, idnumber);
             }
 
             final String resultJSON = template.callMoodleAPIFunction(
@@ -164,102 +164,94 @@ public class MoodleCourseRestriction {
     }
 
     Result<MoodleSEBRestriction> createSEBRestriction(
-            final String externalId,
+            final String internalId,
             final MoodleSEBRestriction restriction) {
 
         return Result.tryCatch(() -> {
-            final String[] courseQuizId = StringUtils.split(externalId, ":");
+            return createSEBRestriction(
-            if (courseQuizId.length > 1) {
+                    MoodleCourseAccess.getQuizId(internalId),
-                // we only have the course id (this is a course)
+                    MoodleCourseAccess.getShortname(internalId),
-                return createSEBRestriction(courseQuizId[0], null, restriction)
+                    MoodleCourseAccess.getIdnumber(internalId),
-                        .getOrThrow();
+                    restriction)
-            } else {
+                            .getOrThrow();
-                // we have the course id and the quiz is (this is a quiz)
-                return createSEBRestriction(courseQuizId[0], courseQuizId[1], restriction)
-                        .getOrThrow();
-            }
         });
     }
 
     Result<MoodleSEBRestriction> createSEBRestriction(
-            final String courseId,
             final String quizId,
+            final String shortname,
+            final String idnumber,
             final MoodleSEBRestriction restriction) {
 
         if (log.isDebugEnabled()) {
             log.debug("POST SEB Client restriction on course: {} quiz: restriction : {}",
-                    courseId,
+                    shortname,
                     quizId,
                     restriction);
         }
 
         return postSEBRestriction(
-                courseId,
                 quizId,
+                shortname,
+                idnumber,
                 MOODLE_DEFAULT_COURSE_RESTRICTION_WS_FUNCTION_CREATE,
                 restriction);
     }
 
     Result<MoodleSEBRestriction> updateSEBRestriction(
-            final String externalId,
+            final String internalId,
             final MoodleSEBRestriction restriction) {
 
         return Result.tryCatch(() -> {
-            final String[] courseQuizId = StringUtils.split(externalId, ":");
+            return updateSEBRestriction(
-            if (courseQuizId.length > 1) {
+                    MoodleCourseAccess.getQuizId(internalId),
-                // we only have the course id (this is a course)
+                    MoodleCourseAccess.getShortname(internalId),
-                return updateSEBRestriction(courseQuizId[0], null, restriction)
+                    MoodleCourseAccess.getIdnumber(internalId),
-                        .getOrThrow();
+                    restriction)
-            } else {
+                            .getOrThrow();
-                // we have the course id and the quiz is (this is a quiz)
-                return updateSEBRestriction(courseQuizId[0], courseQuizId[1], restriction)
-                        .getOrThrow();
-            }
         });
     }
 
     Result<MoodleSEBRestriction> updateSEBRestriction(
-            final String courseId,
             final String quizId,
+            final String shortname,
+            final String idnumber,
             final MoodleSEBRestriction restriction) {
 
         if (log.isDebugEnabled()) {
             log.debug("POST SEB Client restriction on course: {} quiz: restriction : {}",
-                    courseId,
+                    shortname,
                     quizId,
                     restriction);
         }
 
         return postSEBRestriction(
-                courseId,
                 quizId,
+                shortname,
+                idnumber,
                 MOODLE_DEFAULT_COURSE_RESTRICTION_WS_FUNCTION_UPDATE,
                 restriction);
     }
 
     Result<Boolean> deleteSEBRestriction(
-            final String externalId) {
+            final String internalId) {
 
         return Result.tryCatch(() -> {
-            final String[] courseQuizId = StringUtils.split(externalId, ":");
+            return deleteSEBRestriction(
-            if (courseQuizId.length > 1) {
+                    MoodleCourseAccess.getQuizId(internalId),
-                // we only have the course id (this is a course)
+                    MoodleCourseAccess.getShortname(internalId),
-                return deleteSEBRestriction(courseQuizId[0], null)
+                    MoodleCourseAccess.getIdnumber(internalId))
-                        .getOrThrow();
+                            .getOrThrow();
-            } else {
-                // we have the course id and the quiz is (this is a quiz)
-                return deleteSEBRestriction(courseQuizId[0], courseQuizId[1])
-                        .getOrThrow();
-            }
         });
     }
 
     Result<Boolean> deleteSEBRestriction(
-            final String courseId,
+            final String quizId,
-            final String quizId) {
+            final String shortname,
+            final String idnumber) {
 
         if (log.isDebugEnabled()) {
-            log.debug("DELETE SEB Client restriction on course: {} quizId {}", courseId, quizId);
+            log.debug("DELETE SEB Client restriction on course: {} quizId {}", shortname, quizId);
         }
 
         return Result.tryCatch(() -> {
@@ -267,8 +259,13 @@ public class MoodleCourseRestriction {
                     .getOrThrow();
 
             final MultiValueMap<String, String> queryParams = new LinkedMultiValueMap<>();
-            queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_COURSE_ID, courseId);
             queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_QUIZ_ID, quizId);
+            if (StringUtils.isNotBlank(shortname)) {
+                queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_SHORT_NAME, shortname);
+            }
+            if (StringUtils.isNotBlank(idnumber)) {
+                queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_ID_NUMBER, idnumber);
+            }
 
             final String jsonResponse = template.callMoodleAPIFunction(
                     MOODLE_DEFAULT_COURSE_RESTRICTION_WS_FUNCTION_DELETE,
@@ -300,8 +297,9 @@ public class MoodleCourseRestriction {
     }
 
     private Result<MoodleSEBRestriction> postSEBRestriction(
-            final String courseId,
             final String quizId,
+            final String shortname,
+            final String idnumber,
             final String function,
             final MoodleSEBRestriction restriction) {
         return Result.tryCatch(() -> {
@@ -310,8 +308,13 @@ public class MoodleCourseRestriction {
                     .getOrThrow();
 
             final MultiValueMap<String, String> queryParams = new LinkedMultiValueMap<>();
-            queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_COURSE_ID, courseId);
             queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_QUIZ_ID, quizId);
+            if (StringUtils.isNotBlank(shortname)) {
+                queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_SHORT_NAME, shortname);
+            }
+            if (StringUtils.isNotBlank(idnumber)) {
+                queryParams.add(MOODLE_DEFAULT_COURSE_RESTRICTION_ID_NUMBER, idnumber);
+            }
 
             final MultiValueMap<String, String> queryAttributes = new LinkedMultiValueMap<>();
             queryAttributes.addAll(

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

@@ -34,15 +34,13 @@ public class MoodleLmsAPITemplateFactory {
     private final ClientCredentialService clientCredentialService;
     private final ClientHttpRequestFactoryService clientHttpRequestFactoryService;
     private final String[] alternativeTokenRequestPaths;
-    private final boolean includeCourses;
 
     protected MoodleLmsAPITemplateFactory(
             final JSONMapper jsonMapper,
             final AsyncService asyncService,
             final ClientCredentialService clientCredentialService,
             final ClientHttpRequestFactoryService clientHttpRequestFactoryService,
-            @Value("${sebserver.webservice.lms.moodle.api.token.request.paths:}") final String alternativeTokenRequestPaths,
+            @Value("${sebserver.webservice.lms.moodle.api.token.request.paths:}") final String alternativeTokenRequestPaths) {
-            @Value("${sebserver.webservice.lms.moodle.api.includeCourses:false}") final boolean includeCourses) {
 
         this.jsonMapper = jsonMapper;
         this.asyncService = asyncService;
@@ -51,7 +49,6 @@ public class MoodleLmsAPITemplateFactory {
         this.alternativeTokenRequestPaths = (alternativeTokenRequestPaths != null)
                 ? StringUtils.split(alternativeTokenRequestPaths, Constants.LIST_SEPARATOR)
                 : null;
-        this.includeCourses = includeCourses;
     }
 
     public Result<MoodleLmsAPITemplate> create(
@@ -74,8 +71,7 @@ public class MoodleLmsAPITemplateFactory {
                     this.jsonMapper,
                     lmsSetup,
                     moodleRestTemplateFactory,
-                    this.asyncService,
+                    this.asyncService);
-                    this.includeCourses);
 
             final MoodleCourseRestriction moodleCourseRestriction = new MoodleCourseRestriction(
                     this.jsonMapper,

src/main/resources/config/application-dev-ws.properties

@@ -49,7 +49,6 @@ sebserver.webservice.api.pagination.maxPageSize=500
 # comma separated list of known possible OpenEdX API access token request endpoints
 sebserver.webservice.lms.openedx.api.token.request.paths=/oauth2/access_token
 sebserver.webservice.lms.moodle.api.token.request.paths=
-sebserver.webservice.lms.moodle.api.includeCourses=true
 sebserver.webservice.lms.address.alias=lms.mockup.com=lms.address.alias
 
 # NOTE: This is a temporary work-around for SEB Restriction API within Open edX SEB integration plugin to

src/main/resources/config/application-ws.properties

@@ -51,5 +51,4 @@ sebserver.webservice.api.pagination.maxPageSize=500
 # comma separated list of known possible OpenEdX API access token request endpoints
 sebserver.webservice.lms.openedx.api.token.request.paths=/oauth2/access_token
 sebserver.webservice.lms.moodle.api.token.request.paths=/login/token.php
-sebserver.webservice.lms.moodle.api.includeCourses=false
 sebserver.webservice.lms.address.alias=

src/test/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/moodle/MoodleCourseAccessTest.java

@@ -67,8 +67,7 @@ public class MoodleCourseAccessTest {
                 new JSONMapper(),
                 null,
                 moodleRestTemplateFactory,
-                mock(AsyncService.class),
+                mock(AsyncService.class));
-                false);
 
         final String examId = "123";
         final Result<ExamineeAccountDetails> examineeAccountDetails =
@@ -115,8 +114,7 @@ public class MoodleCourseAccessTest {
                 new JSONMapper(),
                 null,
                 moodleRestTemplateFactory,
-                mock(AsyncService.class),
+                mock(AsyncService.class));
-                false);
 
         final LmsSetupTestResult initAPIAccess = moodleCourseAccess.initAPIAccess();
         assertNotNull(initAPIAccess);
@@ -137,8 +135,7 @@ public class MoodleCourseAccessTest {
                 new JSONMapper(),
                 null,
                 moodleRestTemplateFactory,
-                mock(AsyncService.class),
+                mock(AsyncService.class));
-                false);
 
         final LmsSetupTestResult initAPIAccess = moodleCourseAccess.initAPIAccess();
         assertNotNull(initAPIAccess);
@@ -158,8 +155,7 @@ public class MoodleCourseAccessTest {
                 new JSONMapper(),
                 null,
                 moodleRestTemplateFactory,
-                mock(AsyncService.class),
+                mock(AsyncService.class));
-                false);
 
         final LmsSetupTestResult initAPIAccess = moodleCourseAccess.initAPIAccess();
         assertNotNull(initAPIAccess);