From 7869ceb4939d620c84250827e69c221a2abd62e8 Mon Sep 17 00:00:00 2001
From: anhefti <andreas.hefti@let.ethz.ch>
Date: Mon, 29 Aug 2022 10:42:15 +0200
Subject: [PATCH] better check if exam data has changed on LMS

---
 .../ch/ethz/seb/sebserver/gbl/util/Utils.java   | 14 ++++++++++++++
 .../session/impl/ExamUpdateHandler.java         | 17 ++++++++++++++---
 .../ethz/seb/sebserver/gbl/util/ReplTest.java   | 14 ++++++++++++++
 .../ethz/seb/sebserver/gbl/util/UtilsTest.java  | 13 +++++++++++++
 4 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java b/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java
index 18f1ed69..45095388 100644
--- a/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java
+++ b/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java
@@ -757,4 +757,18 @@ public final class Utils {
         }
     }
 
+    public static boolean isEqualsWithEmptyCheck(final String s1, final String s2) {
+        // checks equal strings and both null
+        if (Objects.equals(s1, s2)) {
+            return true;
+        }
+
+        // check null and empty string
+        if (StringUtils.isBlank(s1) && StringUtils.isBlank(s2)) {
+            return true;
+        }
+
+        return false;
+    }
+
 }
diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamUpdateHandler.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamUpdateHandler.java
index d911846c..4c7ce0e4 100644
--- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamUpdateHandler.java
+++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/session/impl/ExamUpdateHandler.java
@@ -341,15 +341,26 @@ class ExamUpdateHandler {
         if (!Objects.equals(exam.name, quizData.name) ||
                 !Objects.equals(exam.startTime, quizData.startTime) ||
                 !Objects.equals(exam.endTime, quizData.endTime) ||
-                !Objects.equals(exam.getDescription(), quizData.description) ||
-                !Objects.equals(exam.getStartURL(), quizData.startURL)) {
+                !Utils.isEqualsWithEmptyCheck(exam.getDescription(), quizData.description) ||
+                !Utils.isEqualsWithEmptyCheck(exam.getStartURL(), quizData.startURL)) {
+
+            if (log.isDebugEnabled()) {
+                log.debug("Update difference from LMS. Exam:{}, QuizData: {}", exam, quizData);
+            }
 
             return true;
         }
 
         if (quizData.additionalAttributes != null && !quizData.additionalAttributes.isEmpty()) {
             for (final Map.Entry<String, String> attr : quizData.additionalAttributes.entrySet()) {
-                if (!Objects.equals(exam.getAdditionalAttribute(attr.getKey()), attr.getValue())) {
+                final String currentAttrValue = exam.getAdditionalAttribute(attr.getKey());
+                if (!Utils.isEqualsWithEmptyCheck(currentAttrValue, attr.getValue())) {
+                    if (log.isDebugEnabled()) {
+                        log.debug("Update difference from LMS: attribute{}, currentValue: {}, lmsValue: {}",
+                                attr.getKey(),
+                                currentAttrValue,
+                                attr.getValue());
+                    }
                     return true;
                 }
             }
diff --git a/src/test/java/ch/ethz/seb/sebserver/gbl/util/ReplTest.java b/src/test/java/ch/ethz/seb/sebserver/gbl/util/ReplTest.java
index bd19fe5a..fe39fbce 100644
--- a/src/test/java/ch/ethz/seb/sebserver/gbl/util/ReplTest.java
+++ b/src/test/java/ch/ethz/seb/sebserver/gbl/util/ReplTest.java
@@ -8,6 +8,13 @@
 
 package ch.ethz.seb.sebserver.gbl.util;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Objects;
+
+import org.junit.Test;
+
 public class ReplTest {
 
 //    @Test
@@ -48,4 +55,11 @@ public class ReplTest {
 //        assertTrue(new Boolean(false) == new Boolean(false));
 //    }
 
+    @Test
+    public void testObjectEquals() {
+        assertTrue(Objects.equals("", ""));
+        assertTrue(Objects.equals(null, null));
+        assertFalse(Objects.equals("", null));
+    }
+
 }
diff --git a/src/test/java/ch/ethz/seb/sebserver/gbl/util/UtilsTest.java b/src/test/java/ch/ethz/seb/sebserver/gbl/util/UtilsTest.java
index 6bc7a523..a5e00fd7 100644
--- a/src/test/java/ch/ethz/seb/sebserver/gbl/util/UtilsTest.java
+++ b/src/test/java/ch/ethz/seb/sebserver/gbl/util/UtilsTest.java
@@ -243,4 +243,17 @@ public class UtilsTest {
                 .contains("ch.ethz.seb.sebserver.gbl.util.UtilsTest.testFormatStackTracePrint"));
     }
 
+    @Test
+    public void testIsEqualsWithEmptyCheck() {
+        assertTrue(Utils.isEqualsWithEmptyCheck("aa", "aa"));
+        assertTrue(Utils.isEqualsWithEmptyCheck("", ""));
+        assertTrue(Utils.isEqualsWithEmptyCheck(null, null));
+        assertTrue(Utils.isEqualsWithEmptyCheck("", null));
+        assertTrue(Utils.isEqualsWithEmptyCheck(null, ""));
+        assertTrue(Utils.isEqualsWithEmptyCheck("  ", null));
+        assertTrue(Utils.isEqualsWithEmptyCheck("  ", " "));
+
+        assertFalse(Utils.isEqualsWithEmptyCheck("  ", "a"));
+    }
+
 }