Merge pull request #1769 from dedis/work-fe2-maxime-electionresultque…

…stion-validation

ElectionResultQuestion and QuestionResult message data validation

fe2-android/app/src/main/java/com/github/dedis/popstellar/model/network/method/message/data/election/ElectionResultQuestion.kt

@@ -1,6 +1,7 @@
 package com.github.dedis.popstellar.model.network.method.message.data.election
 
 import com.github.dedis.popstellar.model.Immutable
+import com.github.dedis.popstellar.utility.MessageValidator.verify
 import java.util.Objects
 
 @Immutable
@@ -9,7 +10,9 @@ class ElectionResultQuestion(id: String, result: Set<QuestionResult>) {
   val result: Set<QuestionResult>
 
   init {
-    require(result.isNotEmpty())
+    // Set can't have duplicates, so no need to check for duplicates
+    // QuestionResult are already validated when constructed
+    verify().stringNotEmpty(id, "election id").listNotEmpty(result.toList())
 
     this.id = id
     this.result = HashSet(result)

fe2-android/app/src/main/java/com/github/dedis/popstellar/model/network/method/message/data/election/QuestionResult.kt

@@ -1,6 +1,7 @@
 package com.github.dedis.popstellar.model.network.method.message.data.election
 
 import com.github.dedis.popstellar.model.Immutable
+import com.github.dedis.popstellar.utility.MessageValidator.verify
 import com.google.gson.annotations.SerializedName
 import java.util.Objects
 
@@ -10,9 +11,9 @@ class QuestionResult(ballotOption: String?, count: Int) {
   val count: Int
 
   init {
-    requireNotNull(ballotOption)
+    verify().stringNotEmpty(ballotOption, "ballot option").greaterOrEqualThan(count, 0, "count")
 
-    ballot = ballotOption
+    ballot = ballotOption!!
     this.count = count
   }
 

fe2-android/app/src/main/java/com/github/dedis/popstellar/utility/MessageValidator.kt

@@ -37,7 +37,7 @@ object MessageValidator {
         id: String,
         organizer: PublicKey,
         creation: Long,
-        name: String
+        name: String,
     ): MessageValidatorBuilder {
       // If any of the arguments are empty or null this throws an exception
       require(id == Lao.generateLaoId(organizer, creation, name)) {
@@ -59,7 +59,7 @@ object MessageValidator {
         id: String,
         laoId: String,
         creation: Long,
-        name: String
+        name: String,
     ): MessageValidatorBuilder {
       // If any of the arguments are empty or null this throws an exception
       require(id == Meeting.generateCreateMeetingId(laoId, creation, name)) {
@@ -81,7 +81,7 @@ object MessageValidator {
         id: String,
         laoId: String,
         creation: Long,
-        name: String
+        name: String,
     ): MessageValidatorBuilder {
       // If any of the arguments are empty or null this throws an exception
       require(id == Meeting.generateStateMeetingId(laoId, creation, name)) {
@@ -173,6 +173,19 @@ object MessageValidator {
       return this
     }
 
+    /**
+     * Helper method to check that a value is greater or equal than a given value.
+     *
+     * @param input the value to check
+     * @param field name of the field (to print in case of error)
+     * @param value the value to compare to
+     * @throws IllegalArgumentException if the value is not greater or equal than the given value
+     */
+    fun greaterOrEqualThan(input: Int, value: Int, field: String): MessageValidatorBuilder {
+      require(input >= value) { "$field must be greater or equal than $value" }
+      return this
+    }
+
     /**
      * Helper method to check that a list is not empty.
      *
@@ -281,7 +294,8 @@ object MessageValidator {
               PoPCHAQRCode.FIELD_CLIENT_ID,
               PoPCHAQRCode.FIELD_NONCE,
               PoPCHAQRCode.FIELD_REDIRECT_URI,
-              PoPCHAQRCode.FIELD_SCOPE)
+              PoPCHAQRCode.FIELD_SCOPE,
+          )
       private const val VALID_RESPONSE_TYPE = "id_token"
       private val REQUIRED_SCOPES = arrayOf("openid", "profile")
       private val VALID_RESPONSE_MODES = arrayOf("query", "fragment")

fe2-android/app/src/test/unit/java/com/github/dedis/popstellar/model/network/method/message/ElectionQuestionResultTest.kt

@@ -4,28 +4,43 @@ import com.github.dedis.popstellar.model.network.method.message.data.election.El
 import com.github.dedis.popstellar.model.network.method.message.data.election.QuestionResult
 import org.hamcrest.CoreMatchers
 import org.hamcrest.MatcherAssert
-import org.junit.Assert
 import org.junit.Test
 
 class ElectionQuestionResultTest {
-  private val questionId = "questionId"
-  private val results = setOf(QuestionResult("Candidate1", 30))
-  private val electionQuestionResult = ElectionResultQuestion(questionId, results)
+  private val validQuestionId = "questionId"
+  private val validCount = 30
+  private val validResults = setOf(QuestionResult("Candidate1", validCount))
+  private val validElectionQuestionResult = ElectionResultQuestion(validQuestionId, validResults)
+  private val emptyResultSet = emptySet<QuestionResult>()
+
+  @Test
+  fun constructorSucceedsWithValidData() {
+    ElectionResultQuestion(validQuestionId, validResults)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructorFailsWhenIdIsEmpty() {
+    ElectionResultQuestion("", validResults)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructorFailsWhenResultSetIsEmpty() {
+    ElectionResultQuestion(validQuestionId, emptyResultSet)
+  }
+
   @Test
   fun electionQuestionResultGetterReturnsCorrectQuestionId() {
-    MatcherAssert.assertThat(electionQuestionResult.id, CoreMatchers.`is`(questionId))
+    MatcherAssert.assertThat(validElectionQuestionResult.id, CoreMatchers.`is`(validQuestionId))
   }
 
   @Test
   fun electionQuestionResultGetterReturnsCorrectResults() {
-    MatcherAssert.assertThat(electionQuestionResult.result, CoreMatchers.`is`(results))
+    MatcherAssert.assertThat(validElectionQuestionResult.result, CoreMatchers.`is`(validResults))
   }
 
-  @Test
+  @Test(expected = IllegalArgumentException::class)
   fun resultsCantBeEmpty() {
     val emptySet = emptySet<QuestionResult>()
-    Assert.assertThrows(
-      IllegalArgumentException::class.java
-    ) { ElectionResultQuestion(questionId, emptySet) }
+    ElectionResultQuestion(validQuestionId, emptySet)
   }
-}
+}

fe2-android/app/src/test/unit/java/com/github/dedis/popstellar/model/network/method/message/QuestionResultTest.kt

@@ -7,21 +7,43 @@ import org.junit.Assert
 import org.junit.Test
 
 class QuestionResultTest {
-  private val count = 30
-  private val name = "Candidate1"
-  private val questionResult = QuestionResult(name, count)
+  private val validCount = 30
+  private val validBallotOption = "Valid Ballot Option"
+  private val questionResult = QuestionResult(validBallotOption, validCount)
+  private val invalidCount = -1
+
+  @Test
+  fun constructorSucceedsWithValidData() {
+    QuestionResult(validBallotOption, validCount)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructorFailsWhenBallotOptionIsNull() {
+    QuestionResult(null, validCount)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructorFailsWhenBallotOptionIsEmpty() {
+    QuestionResult("", validCount)
+  }
+
+  @Test(expected = IllegalArgumentException::class)
+  fun constructorFailsWhenCountIsLessThanZero() {
+    QuestionResult(validBallotOption, invalidCount)
+  }
+
   @Test
   fun fieldsCantBeNull() {
     Assert.assertThrows(IllegalArgumentException::class.java) { QuestionResult(null, 30) }
   }
 
   @Test
   fun questionResultGetterReturnsCorrectName() {
-    MatcherAssert.assertThat(questionResult.ballot, CoreMatchers.`is`(name))
+    MatcherAssert.assertThat(questionResult.ballot, CoreMatchers.`is`(validBallotOption))
   }
 
   @Test
   fun questionResultGetterReturnsCorrectCount() {
-    MatcherAssert.assertThat(questionResult.count, CoreMatchers.`is`(count))
+    MatcherAssert.assertThat(questionResult.count, CoreMatchers.`is`(validCount))
   }
-}
+}