From 4119296c9101739379cbc611349a79989c951d10 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Thu, 7 Sep 2023 06:15:59 -0700 Subject: [PATCH] Reture result directly instead of via reference (#29078) --- .../ManualOnboardingPayloadParser.kt | 5 +- .../OnboardingPayloadParser.kt | 7 +- .../QRCodeOnboardingPayloadParser.kt | 5 +- .../chip/onboardingpayload/ManualCodeTest.kt | 82 +++++++++++-------- .../chip/onboardingpayload/QRCodeTest.kt | 14 +--- 5 files changed, 61 insertions(+), 52 deletions(-) diff --git a/src/controller/java/src/chip/onboardingpayload/ManualOnboardingPayloadParser.kt b/src/controller/java/src/chip/onboardingpayload/ManualOnboardingPayloadParser.kt index f913a9d99206c8..3e2bac7be7d287 100644 --- 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 index e1d8357908e236..cef151816f9d6c 100644 --- 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 index 18d91e8b2b4f45..ef33405114e1ee 100644 --- 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 index 51fb0bfc0c51ac..acaf7e8d4758b9 100644 --- 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,19 +262,40 @@ 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 + ) } /* @@ -285,11 +303,10 @@ class ManualCodeTest { */ @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 index afbfdd0e3e477d..92ad1bf8c55909 100644 --- 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)