Reture result directly instead of via reference (#29078)

project-chip · Jan 4, 2024 · 4119296 · 4119296
1 parent cc75405
commit 4119296
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 52 deletions.
diff --git a/src/controller/java/src/chip/onboardingpayload/ManualOnboardingPayloadParser.kt b/src/controller/java/src/chip/onboardingpayload/ManualOnboardingPayloadParser.kt
@@ -30,8 +30,9 @@ class ManualOnboardingPayloadParser(decimalRepresentation: String) {
     decimalStringRepresentation = decimalRepresentation.replace("-", "")
   }
 
-  fun populatePayload(outPayload: OnboardingPayload): Unit {
+  fun populatePayload(): OnboardingPayload {
     var representationWithoutCheckDigit: String
+    var outPayload: OnboardingPayload = OnboardingPayload()
 
     representationWithoutCheckDigit = checkDecimalStringValidity(decimalStringRepresentation)
 
@@ -125,6 +126,8 @@ class ManualOnboardingPayloadParser(decimalRepresentation: String) {
     require(kManualSetupDiscriminatorFieldLengthInBits <= 8) { "Won't fit in UInt8" }
     outPayload.discriminator = discriminator
     outPayload.hasShortDiscriminator = true
+
+    return outPayload
   }
 
   companion object {

diff --git a/src/controller/java/src/chip/onboardingpayload/OnboardingPayloadParser.kt b/src/controller/java/src/chip/onboardingpayload/OnboardingPayloadParser.kt
@@ -54,9 +54,7 @@ class OnboardingPayloadParser {
     qrCodeString: String,
     skipPayloadValidation: Boolean
   ): OnboardingPayload {
-    val payload = OnboardingPayload()
-
-    QRCodeOnboardingPayloadParser(qrCodeString).populatePayload(payload)
+    val payload = QRCodeOnboardingPayloadParser(qrCodeString).populatePayload()
 
     if (skipPayloadValidation == false && !payload.isValidQRCodePayload()) {
       throw OnboardingPayloadException("Invalid payload")
@@ -103,8 +101,7 @@ class OnboardingPayloadParser {
     manualPairingCodeString: String,
     skipPayloadValidation: Boolean
   ): OnboardingPayload {
-    val payload = OnboardingPayload()
-    ManualOnboardingPayloadParser(manualPairingCodeString).populatePayload(payload)
+    val payload = ManualOnboardingPayloadParser(manualPairingCodeString).populatePayload()
 
     if (skipPayloadValidation == false && !payload.isValidManualCode()) {
       throw OnboardingPayloadException("Invalid manual entry code")

diff --git a/src/controller/java/src/chip/onboardingpayload/QRCodeOnboardingPayloadParser.kt b/src/controller/java/src/chip/onboardingpayload/QRCodeOnboardingPayloadParser.kt
@@ -24,8 +24,9 @@ import java.util.concurrent.atomic.AtomicInteger
  *   to a OnboardingPayload object
  */
 class QRCodeOnboardingPayloadParser(private val mBase38Representation: String) {
-  fun populatePayload(outPayload: OnboardingPayload) {
+  fun populatePayload(): OnboardingPayload {
     var indexToReadFrom: AtomicInteger = AtomicInteger(0)
+    var outPayload: OnboardingPayload = OnboardingPayload()
 
     val payload = extractPayload(mBase38Representation)
     if (payload.length == 0) {
@@ -60,6 +61,8 @@ class QRCodeOnboardingPayloadParser(private val mBase38Representation: String) {
     }
 
     // TODO: populate TLV optional fields
+
+    return outPayload
   }
 
   companion object {

diff --git a/src/controller/java/tests/chip/onboardingpayload/ManualCodeTest.kt b/src/controller/java/tests/chip/onboardingpayload/ManualCodeTest.kt
@@ -165,8 +165,7 @@ class ManualCodeTest {
     // Test short 11 digit code
     var generator = ManualOnboardingPayloadGenerator(payload)
     var result = generator.payloadDecimalStringRepresentation()
-    var outPayload = OnboardingPayload()
-    ManualOnboardingPayloadParser(result).populatePayload(outPayload)
+    var outPayload = ManualOnboardingPayloadParser(result).populatePayload()
     assertPayloadValues(
       outPayload,
       payload.setupPinCode,
@@ -183,8 +182,7 @@ class ManualCodeTest {
     // Test long 21 digit code
     generator = ManualOnboardingPayloadGenerator(payload)
     result = generator.payloadDecimalStringRepresentation()
-    outPayload = OnboardingPayload()
-    ManualOnboardingPayloadParser(result).populatePayload(outPayload)
+    outPayload = ManualOnboardingPayloadParser(result).populatePayload()
     assertPayloadValues(
       outPayload,
       payload.setupPinCode,
@@ -216,12 +214,11 @@ class ManualCodeTest {
    */
   @Test
   fun testPayloadParser_partialPayload() {
-    val payload = getDefaultPayload()
     var decimalString = "2361087535"
 
     decimalString += Verhoeff10.computeCheckChar(decimalString)
     assertEquals(11, decimalString.length)
-    ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+    var payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     assertPayloadValues(
       payload,
       pinCode = 123456780,
@@ -234,7 +231,7 @@ class ManualCodeTest {
     decimalString = "236-108753-5"
     decimalString += computeCheckChar(decimalString)
     assertEquals(13, decimalString.length)
-    ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+    payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     assertPayloadValues(
       payload,
       pinCode = 123456780,
@@ -246,13 +243,13 @@ class ManualCodeTest {
     decimalString = "0000010000"
     decimalString += Verhoeff10.computeCheckChar(decimalString)
     assertEquals(11, decimalString.length)
-    ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+    payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     assertPayloadValues(payload, pinCode = 1, discriminator = 0, vendorId = 0, productId = 0)
 
     decimalString = "63610875350000000000"
     decimalString += Verhoeff10.computeCheckChar(decimalString)
     assertEquals(21, decimalString.length)
-    ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+    payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     assertPayloadValues(
       payload,
       pinCode = 123456780,
@@ -265,31 +262,51 @@ class ManualCodeTest {
     decimalString = "0033407535"
     decimalString += Verhoeff10.computeCheckChar(decimalString)
     assertEquals(11, decimalString.length)
-    ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+    payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
+    assertPayloadValues(
+      payload,
+      pinCode = 123456780,
+      discriminator = 0,
+      vendorId = 0,
+      productId = 0
+    )
 
     // no vid (= 0)
     decimalString = "63610875350000014526"
     decimalString += Verhoeff10.computeCheckChar(decimalString)
     assertEquals(21, decimalString.length)
-    ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+    payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
+    assertPayloadValues(
+      payload,
+      pinCode = 123456780,
+      discriminator = 0xa,
+      vendorId = 0,
+      productId = 14526
+    )
 
     // no pid (= 0)
     decimalString = "63610875354536700000"
     decimalString += Verhoeff10.computeCheckChar(decimalString)
     assertEquals(21, decimalString.length)
-    ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+    payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
+    assertPayloadValues(
+      payload,
+      pinCode = 123456780,
+      discriminator = 0xa,
+      vendorId = 45367,
+      productId = 0
+    )
   }
 
   /*
    * Parse from Full Payload
    */
   @Test
   fun testPayloadParser_fullPayload() {
-    val payload = getDefaultPayload()
     var decimalString = "63610875354536714526"
 
     decimalString += Verhoeff10.computeCheckChar(decimalString)
-    ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+    var payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     assertPayloadValues(
       payload,
       pinCode = 123456780,
@@ -301,7 +318,7 @@ class ManualCodeTest {
     // The same thing, but with dashes separating digit groups.
     decimalString = "6361-0875-3545-3671-4526"
     decimalString += computeCheckChar(decimalString)
-    ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+    payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     assertPayloadValues(
       payload,
       pinCode = 123456780,
@@ -312,7 +329,7 @@ class ManualCodeTest {
 
     decimalString = "52927623630456200032"
     decimalString += Verhoeff10.computeCheckChar(decimalString)
-    ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+    payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     assertPayloadValues(
       payload,
       pinCode = 38728284,
@@ -323,7 +340,7 @@ class ManualCodeTest {
 
     decimalString = "40000100000000100001"
     decimalString += Verhoeff10.computeCheckChar(decimalString)
-    ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+    payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     assertPayloadValues(payload, pinCode = 1, discriminator = 0, vendorId = 1, productId = 1)
   }
 
@@ -332,13 +349,13 @@ class ManualCodeTest {
    */
   @Test
   fun testPayloadParser_invalidEntry() {
-    val payload = OnboardingPayload()
+    var payload = OnboardingPayload()
 
     // Empty input
     var decimalString = ""
     decimalString += Verhoeff10.computeCheckChar(decimalString)
     try {
-      ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+      payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     } catch (e: Exception) {
       println("Expected exception occurred: ${e.message}")
     }
@@ -348,7 +365,7 @@ class ManualCodeTest {
     decimalString = "24184.2196"
     try {
       decimalString += Verhoeff10.computeCheckChar(decimalString)
-      ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+      payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     } catch (e: Exception) {
       println("Expected exception occurred: ${e.message}")
     }
@@ -358,7 +375,7 @@ class ManualCodeTest {
     decimalString = "2456"
     try {
       decimalString += Verhoeff10.computeCheckChar(decimalString)
-      ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+      payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     } catch (e: Exception) {
       println("Expected exception occurred: ${e.message}")
     }
@@ -368,7 +385,7 @@ class ManualCodeTest {
     decimalString = "123456789123456785671"
     try {
       decimalString += Verhoeff10.computeCheckChar(decimalString)
-      ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+      payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     } catch (e: Exception) {
       println("Expected exception occurred: ${e.message}")
     }
@@ -378,7 +395,7 @@ class ManualCodeTest {
     decimalString = "12749875380"
     try {
       decimalString += Verhoeff10.computeCheckChar(decimalString)
-      ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+      payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     } catch (e: Exception) {
       println("Expected exception occurred: ${e.message}")
     }
@@ -388,7 +405,7 @@ class ManualCodeTest {
     decimalString = "23456789123456785610"
     try {
       decimalString += Verhoeff10.computeCheckChar(decimalString)
-      ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+      payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     } catch (e: Exception) {
       println("Expected exception occurred: ${e.message}")
     }
@@ -398,7 +415,7 @@ class ManualCodeTest {
     decimalString = "2327680000"
     try {
       decimalString += Verhoeff10.computeCheckChar(decimalString)
-      ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+      payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     } catch (e: Exception) {
       println("Expected exception occurred: ${e.message}")
     }
@@ -407,7 +424,7 @@ class ManualCodeTest {
     // wrong check digit
     decimalString = "02684354589"
     try {
-      ManualOnboardingPayloadParser(decimalString).populatePayload(payload)
+      payload = ManualOnboardingPayloadParser(decimalString).populatePayload()
     } catch (e: Exception) {
       println("Expected exception occurred: ${e.message}")
     }
@@ -420,11 +437,9 @@ class ManualCodeTest {
   @Test
   fun testShortCodeReadWrite() {
     val inPayload = getDefaultPayload()
-    val outPayload = OnboardingPayload()
-
     var generator = ManualOnboardingPayloadGenerator(inPayload)
     var result = generator.payloadDecimalStringRepresentation()
-    ManualOnboardingPayloadParser(result).populatePayload(outPayload)
+    val outPayload = ManualOnboardingPayloadParser(result).populatePayload()
 
     // Override the discriminator in the input payload with the short version,
     // since that's what we will produce.
@@ -442,10 +457,9 @@ class ManualCodeTest {
     inPayload.vendorId = 1
     inPayload.productId = 1
 
-    val outPayload = OnboardingPayload()
     var generator = ManualOnboardingPayloadGenerator(inPayload)
     var result = generator.payloadDecimalStringRepresentation()
-    ManualOnboardingPayloadParser(result).populatePayload(outPayload)
+    val outPayload = ManualOnboardingPayloadParser(result).populatePayload()
 
     // Override the discriminator in the input payload with the short version,
     // since that's what we will produce.
@@ -651,8 +665,7 @@ class ManualCodeTest {
     val generator = ManualOnboardingPayloadGenerator(payload)
     val result = generator.payloadDecimalStringRepresentation()
 
-    val outPayload = OnboardingPayload()
-    ManualOnboardingPayloadParser(result).populatePayload(outPayload)
+    val outPayload = ManualOnboardingPayloadParser(result).populatePayload()
 
     assertPayloadValues(
       outPayload,
@@ -672,8 +685,7 @@ class ManualCodeTest {
     val generator = ManualOnboardingPayloadGenerator(payload)
     val result = generator.payloadDecimalStringRepresentation()
 
-    val outPayload = OnboardingPayload()
-    ManualOnboardingPayloadParser(result).populatePayload(outPayload)
+    val outPayload = ManualOnboardingPayloadParser(result).populatePayload()
 
     assertPayloadValues(
       outPayload,

diff --git a/src/controller/java/tests/chip/onboardingpayload/QRCodeTest.kt b/src/controller/java/tests/chip/onboardingpayload/QRCodeTest.kt
@@ -48,8 +48,7 @@ class QRCodeTest {
     generator.setAllowInvalidPayload(allowInvalidPayload)
     var result = generator.payloadBase38Representation()
 
-    var outPayload = OnboardingPayload()
-    QRCodeOnboardingPayloadParser(result).populatePayload(outPayload)
+    var outPayload = QRCodeOnboardingPayloadParser(result).populatePayload()
 
     return inPayload == outPayload
   }
@@ -223,10 +222,8 @@ class QRCodeTest {
     var invalidString = kDefaultPayloadQRCode
     invalidString = invalidString.dropLast(1) + " " // space is not contained in the base38 alphabet
 
-    var payload = OnboardingPayload()
-
     try {
-      QRCodeOnboardingPayloadParser(invalidString).populatePayload(payload)
+      QRCodeOnboardingPayloadParser(invalidString).populatePayload()
       assertThat(false)
     } catch (e: Exception) {
       println("Expected exception occurred: ${e.message}")
@@ -241,10 +238,8 @@ class QRCodeTest {
     var invalidString = kDefaultPayloadQRCode
     invalidString = invalidString.dropLast(1)
 
-    var payload = OnboardingPayload()
-
     try {
-      QRCodeOnboardingPayloadParser(invalidString).populatePayload(payload)
+      QRCodeOnboardingPayloadParser(invalidString).populatePayload()
       assertThat(false)
     } catch (e: Exception) {
       println("Expected exception occurred: ${e.message}")
@@ -284,8 +279,7 @@ class QRCodeTest {
     var generator = QRCodeOnboardingPayloadGenerator(payload)
     var base38Rep = generator.payloadBase38Representation()
 
-    var resultingPayload = OnboardingPayload()
-    QRCodeOnboardingPayloadParser(base38Rep).populatePayload(resultingPayload)
+    var resultingPayload = QRCodeOnboardingPayloadParser(base38Rep).populatePayload()
 
     assertEquals(true, resultingPayload.isValidQRCodePayload())
     assertEquals(true, payload == resultingPayload)