SEBSERV-652 fixed by apply proper form URL encoding to Moodle POST body

Author: anhefti

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

@@ -630,6 +630,7 @@ public static String toColorFractionString(final int fraction) {
 
     public static String toAppFormUrlEncodedBody(final MultiValueMap<String, String> attributes) {
         return toAppFormUrlEncodedBodyForSPService(attributes);
+        // TODO do it all the same with toAppFormUrlEncodedBody
 //        if (attributes == null) {
 //            return StringUtils.EMPTY;
 //        }

src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/LmsAPIService.java

@@ -94,14 +94,13 @@ static Predicate<QuizData> quizFilterPredicate(final FilterMap filterMap) {
         if (filterMap == null) {
             return q -> true;
         }
-        
         final String name = filterMap.getQuizName();
         final DateTime from = filterMap.getQuizFromTime();
         final DateTime now = DateTime.now(DateTimeZone.UTC);
-
         return q -> {
             final boolean nameFilter = StringUtils.isBlank(name) || (q.name != null && q.name.contains(name));
-            final boolean startTimeFilter = from == null || (q.startTime != null && (q.startTime.isEqual(from) || q.startTime.isAfter(from)));
+            final boolean startTimeFilter =
+                    from == null || (q.startTime != null && (q.startTime.isEqual(from) || q.startTime.isAfter(from)));
             final DateTime endTime = now.isAfter(from) ? now : from;
             final boolean fromTimeFilter = (endTime == null || q.endTime == null || endTime.isBefore(q.endTime));
             return nameFilter && (startTimeFilter || fromTimeFilter);

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

@@ -152,18 +152,15 @@ private AsyncLookup getAsyncLookup(
             final FilterMap filterMap,
             final Function<String, Result<LmsAPITemplate>> lmsAPITemplateSupplier) {
 
-        // check if there is already a lookup for the user in the cache, if not create one
         if (!this.lookups.containsKey(userId)) {
             this.createNewAsyncLookup(userId, filterMap, lmsAPITemplateSupplier);
         }
-        
-        // get the users lookup from the cache
+
         final AsyncLookup asyncLookup = this.lookups.get(userId);
         if (asyncLookup == null) {
             return null;
         }
 
-        // if the lookup still valid use it otherwise, create new empty one and use this
         if (!asyncLookup.isValid(filterMap)) {
             final AsyncLookup removed = this.lookups.remove(userId);
             if (removed != null) {
@@ -336,9 +333,7 @@ boolean isValid(final FilterMap filterMap) {
                 return false;
             }
 
-            boolean valid = this.lookupFilterCriteria.equals(filterMap);
-            System.out.println("******************** valid: " + valid + "    filterMap: " + filterMap);
-            return valid;
+            return this.lookupFilterCriteria.equals(filterMap);
         }
 
         boolean isRunning() {
@@ -347,15 +342,17 @@ boolean isRunning() {
             }
             final boolean running = this.asyncBuffers
                     .stream()
-                    .anyMatch(b -> !b.finished);
+                    .filter(b -> !b.finished)
+                    .findFirst()
+                    .isPresent();
             if (!running) {
                 this.timeCompleted = Utils.getMillisecondsNow();
             }
             return running;
         }
 
         void cancel() {
-            this.asyncBuffers.forEach(AsyncQuizFetchBuffer::cancel);
+            this.asyncBuffers.stream().forEach(AsyncQuizFetchBuffer::cancel);
         }
     }
 

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

@@ -478,21 +478,13 @@ private String doRequest(
                 final UriComponentsBuilder queryParam,
                 final boolean usePOST,
                 final HttpEntity<?> functionReqEntity) {
-            
-            if (log.isDebugEnabled()) {
-                log.debug("*** Call Moodle: {} BODY: {}", queryParam.toUriString(), functionReqEntity.getBody());
-            }
 
             final ResponseEntity<String> response = super.exchange(
                     queryParam.toUriString(),
                     usePOST ? HttpMethod.POST : HttpMethod.GET,
                     functionReqEntity,
                     String.class);
 
-            if (log.isDebugEnabled()) {
-                log.debug("*** Moodle Response: {}", response);
-            }
-
             final LmsSetup lmsSetup = this.apiTemplateDataSupplier.getLmsSetup();
             if (response.getStatusCode() != HttpStatus.OK) {
                 throw new RuntimeException(

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

@@ -12,6 +12,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -57,6 +58,7 @@
 import ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl.moodle.MoodleRestTemplateFactory;
 import ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl.moodle.MoodleUtils;
 import ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl.moodle.MoodleUtils.CourseData;
+import ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl.moodle.MoodleUtils.Courses;
 import ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl.moodle.MoodleUtils.CoursesPlugin;
 import ch.ethz.seb.sebserver.webservice.servicelayer.lms.impl.moodle.MoodleUtils.MoodleUserDetails;
 import io.micrometer.core.instrument.util.StringUtils;
@@ -425,26 +427,29 @@ private Collection<CourseData> fetchCoursesPage(
         try {
             // get course ids per page
             final long filterDate = Utils.toUnixTimeInSeconds(quizFromTime);
-            final long defaultCutOff = Utils.toUnixTimeInSeconds(DateTime.now(DateTimeZone.UTC).minusYears(this.cutoffTimeOffset));
+            final long defaultCutOff = Utils.toUnixTimeInSeconds(
+                    DateTime.now(DateTimeZone.UTC).minusYears(this.cutoffTimeOffset));
             final long cutoffDate = Math.min(filterDate, defaultCutOff);
-
-            final String sqlCondition = getSQLCondition(nameCondition, cutoffDate, filterDate);
+            String sqlCondition = String.format(
+                    SQL_CONDITION_TEMPLATE, cutoffDate, filterDate);
             final String fromElement = String.valueOf(page * size);
             final LinkedMultiValueMap<String, String> attributes = new LinkedMultiValueMap<>();
-            
+
+            if (this.applyNameCriteria && StringUtils.isNotBlank(nameCondition)) {
+                final String n = Utils.toSQLWildcard(nameCondition);
+                sqlCondition = sqlCondition + " AND (" + SQL_QUIZ_NAME + " LIKE '" + n + "' OR " + SQL_COURSE_NAME + " LIKE '" + n + "')";
+            }
+
             // Note: courseid[]=0 means all courses. Moodle don't like empty parameter
             attributes.add(PARAM_COURSE_ID_ARRAY, "0");
             attributes.add(PARAM_SQL_CONDITIONS, sqlCondition);
             attributes.add(PARAM_PAGE_START, fromElement);
             attributes.add(PARAM_PAGE_SIZE, String.valueOf(size));
 
             final String courseKeyPageJSON = this.protectedMoodlePageCall
-                    .protectedRun(() -> getCoursePageFromMoodle(
-                            restTemplate,
-                            attributes,
-                            cutoffDate,
-                            filterDate
-                    ))
+                    .protectedRun(() -> restTemplate.callMoodleAPIFunction(
+                            COURSES_API_FUNCTION_NAME,
+                            attributes))
                     .getOrThrow();
 
             MoodleUtils.checkJSONFormat(courseKeyPageJSON);
@@ -498,45 +503,6 @@ private Collection<CourseData> fetchCoursesPage(
             return Collections.emptyList();
         }
     }
-    
-    private String getCoursePageFromMoodle(
-            final MoodleAPIRestTemplate restTemplate, 
-            final LinkedMultiValueMap<String, String> attributes,
-            final long cutoffDate,
-            final long filterDate) {
-
-        String responseBody = null;
-
-        // SEBSERV-652 mitigation (can be removed when Moodle bug is fixed
-        try {
-            responseBody = restTemplate.callMoodleAPIFunction(COURSES_API_FUNCTION_NAME, attributes);
-        } catch (final Exception e) {
-            responseBody = null;
-        }
-        
-        if (responseBody == null) {
-            
-            log.warn("*** Moodle respond with empty body on request with attributes: {}", attributes);
-            log.info("*** Apply SEBSERV-652 mitigation...");
-            
-            attributes.remove(PARAM_SQL_CONDITIONS);
-            attributes.add(PARAM_SQL_CONDITIONS, getSQLCondition(null, cutoffDate, filterDate));
-            responseBody = restTemplate.callMoodleAPIFunction(COURSES_API_FUNCTION_NAME, attributes);
-        }
-        
-        return responseBody;
-    }
-
-    private String getSQLCondition(final String nameCondition, final long cutoffDate, final long filterDate) {
-        String sqlCondition = String.format(SQL_CONDITION_TEMPLATE, cutoffDate, filterDate);
-        
-        if (this.applyNameCriteria && StringUtils.isNotBlank(nameCondition)) {
-            final String nc = Utils.toSQLWildcard(nameCondition);
-            sqlCondition = sqlCondition + " AND (" + SQL_QUIZ_NAME + " LIKE '" + nc + "' OR " + SQL_COURSE_NAME + " LIKE '" + nc + "')";
-        }
-        
-        return sqlCondition;
-    }
 
     private List<QuizData> getQuizzesForIds(
             final MoodleAPIRestTemplate restTemplate,