Merge pull request #1960 from dedis/work-fe2-maxime-rollcall-data-val…

…idation data input validation for OpenRollCall + constructor tests
dedis · Jun 30, 2024 · 3c0b81e · 3c0b81e
2 parents 024f340 + a792fad
commit 3c0b81e
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 2 deletions.
diff --git a/...va/com/github/dedis/popstellar/model/network/method/message/data/rollcall/OpenRollCall.kt b/...va/com/github/dedis/popstellar/model/network/method/message/data/rollcall/OpenRollCall.kt
@@ -6,6 +6,7 @@ import com.github.dedis.popstellar.model.network.method.message.data.Data
 import com.github.dedis.popstellar.model.network.method.message.data.Objects
 import com.github.dedis.popstellar.model.objects.RollCall
 import com.github.dedis.popstellar.model.objects.event.EventState
+import com.github.dedis.popstellar.utility.MessageValidator
 import com.google.gson.annotations.SerializedName
 
 /** Data sent to open a roll call */
@@ -28,6 +29,8 @@ class OpenRollCall : Data {
    * @param state the state in which the roll call is when this instance is created
    */
   constructor(laoId: String, opens: String, openedAt: Long, state: EventState) {
+    validate(laoId, "laoId", opens, openedAt)
+
     this.updateId = RollCall.generateOpenRollCallId(laoId, opens, openedAt)
     this.opens = opens
     this.openedAt = openedAt
@@ -40,7 +43,19 @@ class OpenRollCall : Data {
         }
   }
 
+  /**
+   * Constructor of a data Open Roll-Call
+   *
+   * @param updateId id of the update
+   * @param opens The 'update_id' of the latest roll call close, or in its absence, the 'id' field
+   *   of the roll call creation
+   *     @param openedAt timestamp corresponding to roll call open. Must be one of
+   *       ["open", "reopen"]
+   */
   constructor(updateId: String, opens: String, openedAt: Long, action: String) {
+    validate(updateId, "updateId", opens, openedAt)
+        .elementIsOneOf(action, "action", Action.OPEN.action, Action.REOPEN.action)
+
     this.updateId = updateId
     this.opens = opens
     this.openedAt = openedAt
@@ -71,4 +86,16 @@ class OpenRollCall : Data {
   override fun toString(): String {
     return "OpenRollCall{updateId='$updateId', opens='$opens', openedAt=$openedAt, action='$action'}"
   }
+
+  private fun validate(
+      id: String,
+      idLabel: String,
+      opens: String,
+      openedAt: Long
+  ): MessageValidator.MessageValidatorBuilder {
+    return MessageValidator.verify()
+        .isNotEmptyBase64(id, idLabel)
+        .isNotEmptyBase64(opens, "opens")
+        .greaterOrEqualThan(openedAt, 0, "openedAt")
+  }
 }
diff --git a/fe2-android/app/src/main/java/com/github/dedis/popstellar/utility/MessageValidator.kt b/fe2-android/app/src/main/java/com/github/dedis/popstellar/utility/MessageValidator.kt
@@ -246,6 +246,20 @@ object MessageValidator {
       return this
     }
 
+    /**
+     * Helper method to check that a value is one of a given list of values
+     *
+     * @param input the value to check
+     * @param field name of the field (to print in case of error)
+     * @param values the list of values to compare to (modular number of arguments)
+     * @throws IllegalArgumentException if the value is not one of the given values
+     */
+    fun elementIsOneOf(input: Any, field: String, vararg values: Any): MessageValidatorBuilder {
+      require(values.isNotEmpty()) { "Values cannot be empty" }
+      require(values.contains(input)) { "$field must be one of $values" }
+      return this
+    }
+
     /**
      * Helper method to check that a list is not empty.
      *

diff --git a/...om/github/dedis/popstellar/model/network/method/message/data/rollcall/OpenRollCallTest.kt b/...om/github/dedis/popstellar/model/network/method/message/data/rollcall/OpenRollCallTest.kt
@@ -6,7 +6,9 @@ import com.github.dedis.popstellar.model.network.method.message.data.Action
 import com.github.dedis.popstellar.model.network.method.message.data.Objects
 import com.github.dedis.popstellar.model.objects.event.EventState
 import com.github.dedis.popstellar.model.objects.event.EventType
+import com.github.dedis.popstellar.model.objects.security.Base64URLData
 import com.github.dedis.popstellar.utility.security.HashSHA256.hash
+import junit.framework.TestCase.assertNotNull
 import java.time.Instant
 import org.hamcrest.CoreMatchers
 import org.hamcrest.MatcherAssert
@@ -53,6 +55,82 @@ class OpenRollCallTest {
     MatcherAssert.assertThat(REOPEN_ROLL_CALL.opens, CoreMatchers.`is`(CREATE_ROLL_CALL.id))
   }
 
+  @Test
+  fun constructor1SucceedsWithValidData() {
+    val openRollCall = OpenRollCall(LAO_ID, CREATE_ROLL_CALL.id, TIME, EventState.CREATED)
+    assertNotNull(openRollCall)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructor1FailsWhenLaoIdEmpty() {
+    OpenRollCall(EMPTY_B64, CREATE_ROLL_CALL.id, TIME, EventState.CREATED)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructor1FailsWhenLaoIdNotBase64() {
+    OpenRollCall(INVALID_B64, CREATE_ROLL_CALL.id, TIME, EventState.CREATED)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructor1FailsWhenOpensNotBase64() {
+    OpenRollCall(LAO_ID, INVALID_B64, TIME, EventState.CREATED)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructor1FailsWhenOpensEmpty() {
+    OpenRollCall(LAO_ID, EMPTY_B64, TIME, EventState.CREATED)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructor1FailsWhenOpenedAtNegative() {
+    OpenRollCall(LAO_ID, CREATE_ROLL_CALL.id, -1, EventState.CREATED)
+  }
+
+  @Test
+  fun constructor2SucceedsWithValidDataOPENAction() {
+    val openRollCall = OpenRollCall(ID, CREATE_ROLL_CALL.id, TIME, Action.OPEN.action)
+    assertNotNull(openRollCall)
+  }
+
+  fun constructor2SucceedsWithValidDataREOPENAction() {
+    val openRollCall = OpenRollCall(ID, CREATE_ROLL_CALL.id, TIME, Action.REOPEN.action)
+    assertNotNull(openRollCall)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructor2FailsWhenUpdateIdEmpty() {
+    OpenRollCall(EMPTY_B64, CREATE_ROLL_CALL.id, TIME, Action.OPEN.action)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructor2FailsWhenUpdateIdNotBase64() {
+    OpenRollCall(INVALID_B64, CREATE_ROLL_CALL.id, TIME, Action.OPEN.action)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructor2FailsWhenOpensEmpty() {
+    OpenRollCall(ID, EMPTY_B64, TIME, Action.OPEN.action)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructor2FailsWhenOpensNotBase64() {
+    OpenRollCall(ID, INVALID_B64, TIME, Action.OPEN.action)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructor2FailsWhenOpenedAtNegative() {
+    OpenRollCall(ID, CREATE_ROLL_CALL.id, -1, Action.OPEN.action)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructor2FailsWhenActionInvalid() {
+    for (action in Action.values()) {
+      if (action != Action.OPEN && action != Action.REOPEN) {
+        OpenRollCall(ID, CREATE_ROLL_CALL.id, TIME, action.action)
+      }
+    }
+  }
+
   @Test
   fun jsonValidationTest() {
     testData(REOPEN_ROLL_CALL)
@@ -81,8 +159,10 @@ class OpenRollCallTest {
     private const val LOCATION = "Location"
     private val CREATE_ROLL_CALL = CreateRollCall(NAME, TIME, TIME, TIME, LOCATION, null, LAO_ID)
     private val OPEN_ROLL_CALL = OpenRollCall(LAO_ID, CREATE_ROLL_CALL.id, TIME, EventState.CREATED)
-    private val REOPEN_ROLL_CALL =
-      OpenRollCall(LAO_ID, CREATE_ROLL_CALL.id, TIME, EventState.CLOSED)
+    private val REOPEN_ROLL_CALL = OpenRollCall(LAO_ID, CREATE_ROLL_CALL.id, TIME, EventState.CLOSED)
     private val ID = hash(EventType.ROLL_CALL.suffix, LAO_ID, CREATE_ROLL_CALL.id, TIME.toString())
+
+    private const val INVALID_B64 = "invalidBase64String"
+    private val EMPTY_B64 = Base64URLData("".toByteArray()).encoded
   }
 }