From 065ad8e3ced6dda20ad4db709f2d8e5e711c8e37 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 14:13:34 -0700 Subject: [PATCH 01/18] [PLA-2184] Android - Add the tap-to-pay config route to the device service client `/config/{entry}` returns the card reader's credentials, and it is the only way a device that was attested on an earlier run learns it still owes an activation code. It was left out of this client because nothing consumed the credentials until the card-reader work. It is the first GET in the family and the first route whose path is not its own template, so `post` splits into request assembly plus a shared `read` that both verbs delegate to. The three-step ordering contract moves onto `read` unchanged. `{entry}` names a merchant, so `route` carries the template and the resolved path is never the loggable form, and an entry that is not one path segment is refused before anything is sent. A device the service does not hold as active is refused two ways: an envelope 403 inside a 200, and a real 403 from the gateway when the caller's token is not scoped for the route. The gateway's arrives before any controller runs, so `PayabliHttpErrors` classifies it first; `read` takes a status override that maps it to the same failure the envelope produces, and a caller branches once. The credentials are a typed class. Two of the ten fields are the reader vendor's API secrets, and a string map prints every value it holds into any message built from it. --- .../attestation/device/DeviceServiceClient.kt | 186 +++++++++++++--- .../device/DeviceServiceException.kt | 23 +- .../attestation/device/DeviceWireFormat.kt | 54 +++++ .../device/DeviceServiceConfigTest.kt | 205 ++++++++++++++++++ .../taptopay/enrollment/EnrollmentFixture.kt | 21 +- 5 files changed, 450 insertions(+), 39 deletions(-) create mode 100644 taptopay/src/test/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceConfigTest.kt diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt index 9a410d6c..fb298dc1 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt @@ -11,13 +11,18 @@ import com.payabli.sdk.core.network.PayabliEnvelope import com.payabli.sdk.core.network.PayabliHttpErrors import com.payabli.sdk.core.network.PayabliJson import com.payabli.sdk.core.network.PayabliRequest +import com.payabli.sdk.core.network.PayabliResponse import com.payabli.sdk.core.network.PayabliTransport import com.payabli.sdk.taptopay.attestation.AttestationToken import kotlinx.serialization.KSerializer import kotlinx.serialization.SerializationException +import java.net.HttpURLConnection.HTTP_FORBIDDEN + +/** Unreserved characters, per RFC 3986 Section 2.3. What an entry point may hold to be one path segment. */ +private val PATH_SEGMENT = Regex("^[A-Za-z0-9._~-]+$") /** - * The four device-lifecycle calls of `/api/v2/device/taptopay`. + * The five device-lifecycle calls of `/api/v2/device/taptopay`. * * **Stateless and orchestration-free by design.** It holds no device identity, persists nothing, keeps no * state machine, and does not know that `/challenge` precedes `/attest`. Every value a call needs is a @@ -27,8 +32,7 @@ import kotlinx.serialization.SerializationException * * `/activate/challenge` is absent and stays absent. It is the merchant-side call that mints the six-digit * code; the code reaches the device out of band, and an SDK that could mint its own would be an SDK that - * could activate itself. `/config/{entry}` is absent for a duller reason: its credentials have no consumer - * until the card-reader work. + * could activate itself. * * **Nothing here is wrapped in `Retry`, and that is per route rather than an oversight.** `/attest` consumes * the challenge with a delete-on-read, so a second attempt attests against a value the server has already @@ -37,6 +41,10 @@ import kotlinx.serialization.SerializationException * per-call-site primitive precisely so a call site like this one can decline it. The duplicate-safe unit here * is the whole cold sequence, not any single call in it, so retrying belongs to whoever owns the sequence. * + * `/config` is the first route here that would qualify, since it reads and mutates nothing, and it is still + * unwrapped: the assertion it carries is valid for two minutes, so a policy for it is a policy about minting + * a fresh one, which belongs to the same owner. + * * **The server pins the credential, so every request here refuses credential recovery.** The attestation row * written at `/attest` records the exact bearer token that made the call, and `/activate` and `/config` require * that same one, so a refresh between them fails activation as [DeviceServiceException.NotAttested]. Requests @@ -49,8 +57,9 @@ import kotlinx.serialization.SerializationException * status inside the envelope, and it holds for the day they stop. * * **A rotation started by some other capability still breaks the binding**, because one session serves them - * all. Nothing this client does can prevent that, and it resolves with the facade, which binds a device by its - * own key rather than by the token that attested it. + * all. Nothing this client does can prevent that. It costs more than enrollment now that `/config` is here: + * a rotation between attesting and fetching the credentials leaves a reader that cannot be prepared, and the + * remedy is to attest again, which the sequence owner drives. */ internal class DeviceServiceClient( private val transport: PayabliTransport, @@ -194,27 +203,67 @@ internal class DeviceServiceClient( ) /** - * One POST, and the whole of this class's care. + * The reader credentials for [entry], which only an active device is given. * - * The order of the three checks is the contract, not a style: + * Takes all four assertion headers where `/activate` takes three: there is no body to carry the device, + * so the service reads it from `X-Device-Id`. That is the header [DeviceAssertion.asHeaders] already + * sends on both routes. * - * 1. `PayabliHttpErrors` first, because a transport failure means the envelope below is not this service - * speaking. It is called without a `statusOverride`: these routes put their meaning in the envelope, so - * there is no shared status here to give a component reading. It also catches the failures that never - * reach a controller: DTO validation answers with a real 400 and `problem+json`, carrying no envelope. - * A missing `platform` is one of those. - * 2. Then the envelope decline, because these routes report a refusal as HTTP 200 and skipping this step - * is exactly how a refusal reads as a success. - * 3. Only then the payload. + * **A device that still owes activation is refused, and the refusal arrives two ways.** A device the + * service does not hold as active is declined with a 403 inside a 200. A caller whose token is not + * scoped for this route is refused with a real 403, by the gateway, before any controller runs. Both + * become [DeviceServiceException.Forbidden], so a caller branches once rather than twice. * - * Whether an absent `responseData` is usable is the route's business, and it is settled **here** rather - * than by the caller: [emptyPayload] is what a route substitutes when reaching the response at all is the - * answer, and a route that leaves it null is saying it cannot proceed without fields. `/attest` and - * `/activate` supply one; `/challenge` and `/register` do not. + * They are not the same condition and the shared classification is imprecise: a scope problem presents + * as a device that owes a code. It is what the sibling client does, and separating them is a change both + * platforms make together or not at all. * - * That policy has to live inside this function, not above it, because the success record is written here. - * A caller rejecting a null payload afterwards would throw with `device_call_succeeded` already in the log - * and no failure record beside it, and an incident would read as a success the caller never received. + * A refusal here is never retried in place. The attestation row pins the bearer, so a rejection under + * [DeviceServiceException.NotAttested] means the credential moved and the binding is gone; attesting + * again from inside a failing call would spend a challenge and hide the rotation that caused it. + */ + suspend fun config( + entry: String, + assertion: DeviceAssertion, + failureMapper: DeviceFailureMapper = DeviceFailureMapper.None, + ): ConfigResponse = + get( + route = ROUTE_CONFIG, + path = "$BASE/config/${pathSegment(entry)}", + payloadSerializer = ConfigResponse.serializer(), + failureMapper = failureMapper, + headers = assertion.asHeaders(), + statusOverride = { statusCode -> + if (statusCode == HTTP_FORBIDDEN) { + // The gateway's refusal carries no service text, and inventing one would put words in + // its mouth that a caller could display. + DeviceServiceException.Forbidden(statusCode, "") + } else { + null + } + }, + ) + + /** + * [entry] as one path segment, or a refusal. + * + * Refused rather than encoded. A value that is not a single segment is a caller defect, and encoding it + * would send a request for a paypoint nobody named: `URLEncoder` is the wrong tool besides, since it + * writes a space as `+`, which is a query-string rule and not a path one. + * + * The message names the field and the shape, never the value, because an entry point identifies a + * merchant. + */ + private fun pathSegment(entry: String): String { + require(PATH_SEGMENT.matches(entry)) { "entry must be usable as a single path segment" } + return entry + } + + /** + * The four POSTs. Every one of them carries a body and resolves to its own template. + * + * The pin is set here and in [get] rather than in one shared place, because the two assemblers build + * different request shapes. A sixth route inherits it from whichever of them it uses. */ private suspend fun post( route: String, @@ -225,9 +274,9 @@ internal class DeviceServiceClient( headers: Map = emptyMap(), emptyPayload: T? = null, ): T { - // route and path are the same string for all four: none of them embeds an identifier. Passed anyway, - // because `route` is the only form the transport may log and defaulting it to null would cost every - // record in this family the name of the endpoint it came from. + // The four POSTs resolve to their own template, because none of them embeds an identifier. Passed + // anyway, because `route` is the only form the transport may log and defaulting it to null would cost + // every record in this family the name of the endpoint it came from. val request = PayabliRequest.json( method = HttpMethod.POST, @@ -236,11 +285,85 @@ internal class DeviceServiceClient( bodySerializer = bodySerializer, route = route, headers = headers, - // One place for all four, so a fifth route added to this class inherits it. isCredentialPinned = true, ) - val response = transport.execute(request) - PayabliHttpErrors.from(response)?.let { transportFailure -> + return read( + route = route, + response = transport.execute(request), + payloadSerializer = payloadSerializer, + failureMapper = failureMapper, + emptyPayload = emptyPayload, + ) + } + + /** + * The GET half, whose [path] is **not** its [route]. + * + * `/config` is the one route here that embeds an identifier, so the two are separate parameters for the + * first time: [route] is the template the transport records and [path] is the resolved string it sends. + * + * No [emptyPayload]. Reaching the response is the answer on `/attest` and `/activate`; here the fields + * are, so a success carrying none of them is a failure. + */ + private suspend fun get( + route: String, + path: String, + payloadSerializer: KSerializer, + failureMapper: DeviceFailureMapper, + headers: Map, + statusOverride: (Int) -> Throwable?, + ): T { + val request = + PayabliRequest( + method = HttpMethod.GET, + path = path, + route = route, + headers = headers, + isCredentialPinned = true, + ) + return read( + route = route, + response = transport.execute(request), + payloadSerializer = payloadSerializer, + failureMapper = failureMapper, + statusOverride = statusOverride, + ) + } + + /** + * The whole of this class's care, once a response exists. + * + * The order of the checks is the contract, not a style: + * + * 0. [statusOverride] first, so a route can name what a status means to it before the shared table does. + * It answers only for statuses the route already treats as failures. + * 1. `PayabliHttpErrors` next, because a transport failure means the envelope below is not this service + * speaking. It is called without a `statusOverride`: these routes put their meaning in the envelope, so + * there is no shared status here to give a component reading. It also catches the failures that never + * reach a controller: DTO validation answers with a real 400 and `problem+json`, carrying no envelope. + * A missing `platform` is one of those. + * 2. Then the envelope decline, because these routes report a refusal as HTTP 200 and skipping this step + * is exactly how a refusal reads as a success. + * 3. Only then the payload. + * + * Whether an absent `responseData` is usable is the route's business, and it is settled **here** rather + * than by the caller: [emptyPayload] is what a route substitutes when reaching the response at all is the + * answer, and a route that leaves it null is saying it cannot proceed without fields. `/attest` and + * `/activate` supply one; `/challenge`, `/register` and `/config` do not. + * + * That policy has to live inside this function, not above it, because the success record is written here. + * A caller rejecting a null payload afterwards would throw with `device_call_succeeded` already in the log + * and no failure record beside it, and an incident would read as a success the caller never received. + */ + private fun read( + route: String, + response: PayabliResponse, + payloadSerializer: KSerializer, + failureMapper: DeviceFailureMapper, + emptyPayload: T? = null, + statusOverride: (Int) -> Throwable? = { null }, + ): T { + (statusOverride(response.statusCode) ?: PayabliHttpErrors.from(response))?.let { transportFailure -> logger.warn( LogField.safe("event", "device_call_failed"), LogField.safe("route", route), @@ -324,7 +447,11 @@ internal class DeviceServiceClient( private const val BASE = "/api/v2/device/taptopay" /** - * Route templates, which for these four are also the paths: none embeds an identifier. + * Route templates. + * + * The first four are also their own paths, because none of them embeds an identifier. [ROUTE_CONFIG] + * is a template and nothing else: its `{entry}` names a merchant, so the resolved path is not a form + * anything may record. * * Visible to the module's tests, which assert the exact string each call goes to. A path is the one * part of a request no reviewer can verify by reading the client alone. @@ -333,5 +460,6 @@ internal class DeviceServiceClient( const val ROUTE_REGISTER: String = "$BASE/register" const val ROUTE_ATTEST: String = "$BASE/attest" const val ROUTE_ACTIVATE: String = "$BASE/activate" + const val ROUTE_CONFIG: String = "$BASE/config/{entry}" } } diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceException.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceException.kt index 059f6bfb..984de115 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceException.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceException.kt @@ -14,7 +14,7 @@ import java.net.HttpURLConnection.HTTP_UNAUTHORIZED * like an HTTP status and is not one. So a caller that checks the status and stops sees every one of these as * a success. `PayabliHttpErrors` still runs first at the call site, for the genuine transport failures — a * rejected credential, a rate limit, a proxy — and those arrive as `PayabliException`, not as this type. The - * two are disjoint on purpose: which one a caller catches says which layer failed. + * two are disjoint: which one a caller catches says which layer failed. * * **The family has two failure shapes, and this type covers only the second.** A request the service's DTO * validation refuses never reaches a controller: it answers with a real HTTP 400 carrying RFC 9457 @@ -60,7 +60,7 @@ internal sealed class DeviceServiceException( /** * The request or the device's state was refused. * - * The widest case, and deliberately so. Everything the activation window can go wrong with lands here — + * The widest case. Everything the activation window can go wrong with lands here — * wrong code, five attempts spent, expired code, rejected assertion, a device that was not pending — * along with plain malformed input. Splitting them needs `reason`, which is why it is a * [DeviceFailureMapper]'s job rather than this class's. @@ -85,9 +85,16 @@ internal sealed class DeviceServiceException( /** * The device or the application is not permitted this call. * - * Two distinct conditions the service reports identically: a device that is not yet active, which is the - * ordinary pending-activation signal, and an application absent from the paypoint's allowlist, which is - * configuration. Neither is retryable. + * Three distinct conditions reported identically. Two come from a controller as an envelope decline: a + * device that is not yet active, which is the ordinary pending-activation signal, and an application + * absent from the paypoint's allowlist, which is configuration. The third is a real HTTP 403 from the + * gateway on `/config`, raised when the caller's token is not scoped for the route, and it carries no + * [reason] because the gateway sends no service text. + * + * The third is the imprecise one: a scope problem presents to a caller as a device owing activation. + * That matches the sibling client, and separating them is a change both platforms make together. + * + * None of the three is retryable. */ class Forbidden( resultCode: Int?, @@ -132,9 +139,9 @@ internal sealed class DeviceServiceException( * The response said success and its body could not be read as one. * * A missing required field, a payload that is not the shape this route documents, or no payload where one - * is needed. Not a refusal by the service, which is why it carries no [resultCode]: something between the - * two of us is wrong about the contract, and treating it as a decline would file it under the service's - * fault and lose the cause. + * is needed. Not a refusal by the service, which is why it carries no [resultCode]: the SDK and the + * service disagree about the contract, and a decline would file that under the service's fault and lose + * the cause. */ class Undecodable( // No default. Nullable because a response can be unusable without anything having thrown — an envelope diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceWireFormat.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceWireFormat.kt index 47bb5745..b579f35c 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceWireFormat.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceWireFormat.kt @@ -232,3 +232,57 @@ internal class ActivateResponse( ) { override fun toString(): String = "ActivateResponse()" } + +/** + * `{ credentials }`. + * + * Required, unlike the payloads above: a config carrying no credentials is unusable rather than partially + * usable, so an absent one is a decode failure and not an empty success. + */ +@Serializable +internal class ConfigResponse( + val credentials: ReaderCredentials, +) { + override fun toString(): String = "ConfigResponse()" +} + +/** + * What the card reader is configured with, for one paypoint. + * + * **Typed rather than a string map, and that is a redaction decision.** The shipping sibling client keeps + * this as an untyped dictionary and hands it on. A `Map`'s `toString` prints every value it holds, and two + * of these are the reader vendor's API credentials, so the same shape here would put them into any message + * built from a map that reached an exception. Naming the fields also states which two the reader cannot + * start without on this platform. + * + * Every field is required. The service sends all of them, possibly empty, and a missing one means the + * response is not this route's. That is what makes [platform] self-enforcing rather than something to + * branch on: the sibling platform's variant omits [ppId] and [hostPort], so it fails to decode here. + * + * `pageIdentifier` sits beside these on the wire and is not modelled. It is a fresh token the service mints + * per call, so it is a different credential from the one the attestation row pins, and sending it as the + * bearer fails every request on this route. + */ +@Serializable +internal class ReaderCredentials( + val platform: String, + /** The reader vendor's application key. A live secret: never logged, never in `toString`. */ + val secretKey: String, + /** The reader vendor's application id. A live secret, on the same terms as [secretKey]. */ + val apiKey: String, + val merchantId: String, + /** `"sandbox"` or `"production"`, as the paypoint's gateway is configured. */ + val environment: String, + /** ISO 4217, three letters. */ + val currencyCode: String, + val merchantName: String, + /** ISO 18245. */ + val merchantCategoryCode: String, + val terminalId: String, + /** Required by the reader on this platform, and absent from the sibling platform's variant. */ + val ppId: String, + /** `host:port`, on the same terms as [ppId]. */ + val hostPort: String, +) { + override fun toString(): String = "ReaderCredentials(platform=$platform)" +} diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceConfigTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceConfigTest.kt new file mode 100644 index 00000000..768796a3 --- /dev/null +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceConfigTest.kt @@ -0,0 +1,205 @@ +package com.payabli.sdk.taptopay.attestation.device + +import com.payabli.sdk.core.model.PayabliErrorCode +import com.payabli.sdk.core.model.PayabliException +import com.payabli.sdk.core.network.HttpMethod +import com.payabli.sdk.taptopay.attestation.impl.RecordingSdkLogger +import com.payabli.sdk.taptopay.enrollment.configBody +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import java.net.HttpURLConnection.HTTP_FORBIDDEN +import kotlin.time.Duration.Companion.seconds + +private val TEST_TIMEOUT = 5.seconds + +private const val ENTRY = "entry-point-value" + +private val ASSERTION = + DeviceAssertion( + assertion = "assertion-value", + keyId = "key-id-value", + deviceId = "device-id-value", + timestamp = "2026-08-07T12:00:00.000Z", + ) + +private suspend fun failureOf(block: suspend () -> Unit): Throwable? = runCatching { block() }.exceptionOrNull() + +/** + * `/config` as a request and a response, without a session around it. + * + * It is the one route in this family that is a GET, the one whose path is not its own template, and the one + * whose refusal can arrive either inside a 200 or as a real status. Each of those is a separate assertion + * here, because each has its own way of going wrong. + */ +class DeviceServiceConfigTest { + private val logger = RecordingSdkLogger() + + private fun clientFor( + body: String, + statusCode: Int = 200, + ): Pair { + val transport = FakeDeviceTransport.answering(body, statusCode) + return DeviceServiceClient(transport, logger) to transport + } + + @Test + fun `config is a GET carrying no body and the four assertion headers`() = + runTest(timeout = TEST_TIMEOUT) { + val (client, transport) = clientFor(configBody()) + + client.config(ENTRY, ASSERTION) + + val request = transport.request + assertEquals(HttpMethod.GET, request.method) + assertNull("a GET carries no body", request.body) + assertEquals( + "the resolved path names the paypoint", + "/api/v2/device/taptopay/config/$ENTRY", + request.path, + ) + assertEquals( + "the loggable form is the template, never the resolved path", + DeviceServiceClient.ROUTE_CONFIG, + request.route, + ) + assertEquals( + mapOf( + "X-App-Assertion" to "assertion-value", + "X-App-KeyId" to "key-id-value", + "X-Device-Id" to "device-id-value", + "X-Assertion-Timestamp" to "2026-08-07T12:00:00.000Z", + ), + request.headers, + ) + assertTrue("the service pins the credential on this route", request.isCredentialPinned) + } + + @Test + fun `an active device is given every credential the reader needs`() = + runTest(timeout = TEST_TIMEOUT) { + val (client, _) = clientFor(configBody()) + + val credentials = client.config(ENTRY, ASSERTION).credentials + + assertEquals("android", credentials.platform) + assertEquals("secret-key-value", credentials.secretKey) + assertEquals("api-key-value", credentials.apiKey) + assertEquals("merchant-id-value", credentials.merchantId) + assertEquals("sandbox", credentials.environment) + assertEquals("USD", credentials.currencyCode) + assertEquals("merchant-name-value", credentials.merchantName) + assertEquals("5999", credentials.merchantCategoryCode) + assertEquals("terminal-id-value", credentials.terminalId) + assertEquals("pp-id-value", credentials.ppId) + assertEquals("host-port-value", credentials.hostPort) + } + + @Test + fun `printing the credentials names no value`() = + runTest(timeout = TEST_TIMEOUT) { + val (client, transport) = clientFor(configBody()) + + val credentials = client.config(ENTRY, ASSERTION).credentials + + // This is the assertion that matters: `toString` is what reaches an exception message, which no + // logger can redact. + assertEquals("ReaderCredentials(platform=android)", credentials.toString()) + for (secret in listOf("secret-key-value", "api-key-value", "terminal-id-value", ENTRY)) { + assertTrue( + "$secret reached a log message", + logger.records.none { it.message.contains(secret) }, + ) + } + // The paypoint is in the path and must not be in what the transport is told to record. + assertTrue(transport.request.route?.contains(ENTRY) != true) + } + + @Test + fun `a device the service does not hold as active is forbidden`() = + runTest(timeout = TEST_TIMEOUT) { + val (client, _) = clientFor(declineEnvelope(403, "Device is not active.")) + + val failure = failureOf { client.config(ENTRY, ASSERTION) } + + assertTrue("$failure", failure is DeviceServiceException.Forbidden) + assertEquals(403, (failure as DeviceServiceException).resultCode) + } + + @Test + fun `a real 403 is forbidden too, though it never reaches the envelope`() = + runTest(timeout = TEST_TIMEOUT) { + // The gateway refuses before any controller runs, so this body carries no envelope at all. + val (client, _) = clientFor("", statusCode = HTTP_FORBIDDEN) + + val failure = failureOf { client.config(ENTRY, ASSERTION) } + + assertTrue("$failure", failure is DeviceServiceException.Forbidden) + assertEquals(HTTP_FORBIDDEN, (failure as DeviceServiceException).resultCode) + assertEquals("the gateway sends no service text", "", failure.reason) + } + + @Test + fun `a credential rotation between attesting and fetching is reported as unattested`() = + runTest(timeout = TEST_TIMEOUT) { + val (client, _) = clientFor(declineEnvelope(401, "Device not attested or attestation revoked.")) + + val failure = failureOf { client.config(ENTRY, ASSERTION) } + + assertTrue("$failure", failure is DeviceServiceException.NotAttested) + } + + @Test + fun `a success carrying no credentials is undecodable`() = + runTest(timeout = TEST_TIMEOUT) { + val (client, _) = clientFor(successEnvelope("{}")) + + val failure = failureOf { client.config(ENTRY, ASSERTION) } + + assertTrue("$failure", failure is DeviceServiceException.Undecodable) + } + + @Test + fun `the sibling platform's credentials do not decode as this platform's`() = + runTest(timeout = TEST_TIMEOUT) { + // No ppId and no hostPort, which is what makes the platform discriminator self-enforcing. + val (client, _) = + clientFor( + successEnvelope( + """{"credentials":{"platform":"ios","secretKey":"s","apiKey":"a","merchantId":"m",""" + + """"environment":"sandbox","currencyCode":"USD","merchantName":"n",""" + + """"merchantCategoryCode":"5999","terminalId":"t","appleTtpMerchantId":"",""" + + """"terminalProfileId":"p"}}""", + ), + ) + + val failure = failureOf { client.config(ENTRY, ASSERTION) } + + assertTrue("$failure", failure is DeviceServiceException.Undecodable) + } + + @Test + fun `an entry that is not one path segment is refused before anything is sent`() = + runTest(timeout = TEST_TIMEOUT) { + val (client, transport) = clientFor(configBody()) + + for (unusable in listOf("", " ", "a/b", "a?b", "a#b", "a b")) { + val failure = failureOf { client.config(unusable, ASSERTION) } + assertTrue("$unusable was accepted", failure is IllegalArgumentException) + } + assertEquals(emptyList(), transport.requests) + } + + @Test + fun `a request with no bearer at all still reaches the shared status table`() = + runTest(timeout = TEST_TIMEOUT) { + val (client, _) = clientFor("", statusCode = 401) + + val failure = failureOf { client.config(ENTRY, ASSERTION) } + + assertTrue("$failure", failure is PayabliException) + assertEquals(PayabliErrorCode.TOKEN_EXPIRED, (failure as PayabliException).code) + } +} diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/enrollment/EnrollmentFixture.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/enrollment/EnrollmentFixture.kt index 5f236ef8..307f8880 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/enrollment/EnrollmentFixture.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/enrollment/EnrollmentFixture.kt @@ -45,6 +45,17 @@ internal fun attestBody(): String = successEnvelope("""{"registered":true,"isSan internal fun activateBody(): String = successEnvelope("""{"deviceId":"$DEVICE_ID","status":"active"}""") +/** Every credential the reader is given, each value naming its own field so a transposed pair fails. */ +internal fun configBody(): String = + successEnvelope( + """ + {"credentials":{"platform":"android","secretKey":"secret-key-value","apiKey":"api-key-value", + "merchantId":"merchant-id-value","environment":"sandbox","currencyCode":"USD", + "merchantName":"merchant-name-value","merchantCategoryCode":"5999","terminalId":"terminal-id-value", + "ppId":"pp-id-value","hostPort":"host-port-value"}} + """.trimIndent().replace("\n", ""), + ) + /** * Answers each route from a script, and fails loudly on anything unscripted. * @@ -54,6 +65,8 @@ internal fun activateBody(): String = successEnvelope("""{"deviceId":"$DEVICE_ID */ internal class RouteScript( private vararg val answers: Pair>, + /** These routes answer a refusal inside a 200, so a real status is only for a route that skips them. */ + private val statusFor: (String) -> Int = { 200 }, ) { private val taken = mutableMapOf() @@ -65,7 +78,7 @@ internal class RouteScript( val index = taken.getOrDefault(route, 0) if (index >= queued.size) error("$route was called ${index + 1} times, ${queued.size} answers scripted") taken[route] = index + 1 - return PayabliResponse(200, body = queued[index].toByteArray(Charsets.UTF_8)) + return PayabliResponse(statusFor(route), body = queued[index].toByteArray(Charsets.UTF_8)) } companion object { @@ -73,6 +86,8 @@ internal class RouteScript( const val REGISTER = "/api/v2/device/taptopay/register" const val ATTEST = "/api/v2/device/taptopay/attest" const val ACTIVATE = "/api/v2/device/taptopay/activate" + + const val CONFIG = "/api/v2/device/taptopay/config/$ENTRY" } } @@ -103,11 +118,13 @@ internal class EnrollmentFixture( val storage = FakeSecureStore(failWith = storeFailure, trace = trace, firstReadGate = firstReadGate) val store = AttestedDeviceStore(storage, logger) + val client = DeviceServiceClient(transport, logger) + val enrollment = DeviceEnrollment( entry = ENTRY, appId = APP_ID, - client = DeviceServiceClient(transport, logger), + client = client, attestor = attestor, deviceKey = deviceKey, signer = DeviceAssertionSigner(deviceKey, FIXED_CLOCK), From 807a90a5b8eea1cff5deb3756623c7860b006721 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 14:13:43 -0700 Subject: [PATCH 02/18] [PLA-2184] Android - Add the card-present session state machine and serialize its entry points Nine states with a table of legal moves, one writer, and a coordinator whose three entry points never overlap. The module could attest a device and spend an activation code; nothing sequenced those into a session. No mutator returns a value a caller can drop. `advance` moves the state and then runs the work under it, so entering a phase without announcing it cannot be expressed, and a refused move throws before the work is reached. The sibling SDK returns a boolean that every one of its call sites discards, and after an expiry its narrow table refused every move while each phase ran and reported success. Building a session therefore starts from the beginning whatever the caller left behind. A failure names its reason, over a closed set of four remedies. Without one a host cannot tell a discarded identity from a misconfigured paypoint and has to assume the most expensive repair. Same-kind callers join the run in flight and are given its outcome; a different kind waits and then runs, because repairing a session skips attestation and building one does not. Exclusion and joining are separate mechanisms: one mutex holds the region, one claim slot deduplicates, and removing either leaves the other's tests green. Activation runs inside the region too, since it moves the same state and a code spent against a handle a concurrent registration has replaced is a code wasted. Cleanup runs uncancellable. A claim left set with nobody to complete it wedges every later caller of its kind, and a joiner is given a withdrawal rather than the owner's cancellation, which would make its own scope look like it is unwinding while nothing has cancelled it. `fromstate` and `tostate` join the loggable field names: a record of a refused transition that names one end of the move says nothing about why it was refused. --- .../core/logging/impl/LoggableFieldNames.kt | 12 +- .../taptopay/enrollment/DeviceEnrollment.kt | 28 ++ .../sdk/taptopay/session/ReaderProvider.kt | 23 ++ .../taptopay/session/TapToPayFailureReason.kt | 44 +++ .../session/TapToPaySessionCoordinator.kt | 276 ++++++++++++++++++ .../session/TapToPaySessionException.kt | 50 ++++ .../session/TapToPaySessionFailures.kt | 108 +++++++ .../session/TapToPaySessionManager.kt | 147 ++++++++++ .../taptopay/session/TapToPaySessionState.kt | 75 +++++ .../session/TapToPaySessionTransitions.kt | 58 ++++ .../sdk/taptopay/session/SessionFixture.kt | 119 ++++++++ .../session/SessionSerializationTest.kt | 192 ++++++++++++ .../taptopay/session/SessionWarmStartTest.kt | 149 ++++++++++ .../session/TapToPaySessionManagerTest.kt | 211 +++++++++++++ .../session/TapToPayTransitionMatrixTest.kt | 105 +++++++ 15 files changed, 1593 insertions(+), 4 deletions(-) create mode 100644 taptopay/src/main/java/com/payabli/sdk/taptopay/session/ReaderProvider.kt create mode 100644 taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt create mode 100644 taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt create mode 100644 taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt create mode 100644 taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt create mode 100644 taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt create mode 100644 taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt create mode 100644 taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionTransitions.kt create mode 100644 taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt create mode 100644 taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt create mode 100644 taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt create mode 100644 taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt create mode 100644 taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt diff --git a/core/src/main/java/com/payabli/sdk/core/logging/impl/LoggableFieldNames.kt b/core/src/main/java/com/payabli/sdk/core/logging/impl/LoggableFieldNames.kt index 9619755a..0d00b84d 100644 --- a/core/src/main/java/com/payabli/sdk/core/logging/impl/LoggableFieldNames.kt +++ b/core/src/main/java/com/payabli/sdk/core/logging/impl/LoggableFieldNames.kt @@ -38,10 +38,14 @@ internal object LoggableFieldNames { // Non-secret claim vocabulary. "aal", "scope", - // Lifecycle and state. + // Lifecycle and state. `fromstate` and `tostate` are the same fixed vocabulary as `state`, and + // both are needed together: a record of a refused transition that names only one end of it says + // nothing about why the move was refused. "event", "phase", "state", + "fromstate", + "tostate", "category", // Transport metadata. `route` is the route template, never a resolved path. "route", @@ -54,9 +58,9 @@ internal object LoggableFieldNames { "retryable", "durationms", "elapsedms", - // Three distinct durations, deliberately not collapsed into one name: `timeoutms` is the - // backoff wait before the next attempt, `totaltimeoutms` the retry budget, `calltimeoutms` - // the ceiling on one whole call. An incident reads differently depending on which ran out. + // Three distinct durations. `timeoutms` is the backoff wait before the next attempt, + // `totaltimeoutms` the retry budget, `calltimeoutms` the ceiling on one whole call. An + // incident reads differently depending on which ran out. "timeoutms", "totaltimeoutms", "calltimeoutms", diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/enrollment/DeviceEnrollment.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/enrollment/DeviceEnrollment.kt index fbdde161..8ba1dbc7 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/enrollment/DeviceEnrollment.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/enrollment/DeviceEnrollment.kt @@ -9,6 +9,7 @@ import com.payabli.sdk.core.logging.SdkLogger import com.payabli.sdk.core.logging.debug import com.payabli.sdk.core.logging.warn import com.payabli.sdk.taptopay.attestation.AppAttestor +import com.payabli.sdk.taptopay.attestation.device.DeviceAssertion import com.payabli.sdk.taptopay.attestation.device.DeviceAssertionSigner import com.payabli.sdk.taptopay.attestation.device.DeviceAttestationBinding import com.payabli.sdk.taptopay.attestation.device.DeviceIdentity @@ -232,6 +233,33 @@ internal class DeviceEnrollment( } } + /** + * Proves possession of the key this paypoint's device was attested with, or null when there is no such + * device. + * + * Here rather than at the caller because the store, the signer, the paypoint check and the dispatcher + * the blocking signature needs are all held here already, and every one of them would otherwise be + * repeated somewhere else. + * + * Null rather than a [DeviceActivationException]: that vocabulary answers why an activation did not + * complete, and this is not an activation. + * + * Takes the same lock as the rest, so a record cannot be read while a re-registration is replacing the + * handle it names. + */ + suspend fun assertion(): DeviceAssertion? = + lock.withLock { + val known = store.read()?.takeIf { it.entry == entry } ?: return@withLock null + try { + withContext(dispatcher) { signer.sign(known.deviceId) } + } catch (lost: DeviceKeyException.KeyLost) { + // The key store has already discarded the key, so the record names a binding this device + // can no longer sign for. Same disposal as the activation path makes for the same finding. + forget("key_lost") + throw lost + } + } + /** * Forgets the device without touching its key, so the next [enroll] runs the cold sequence. * diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/ReaderProvider.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/ReaderProvider.kt new file mode 100644 index 00000000..6ec6b628 --- /dev/null +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/ReaderProvider.kt @@ -0,0 +1,23 @@ +package com.payabli.sdk.taptopay.session + +import com.payabli.sdk.taptopay.attestation.device.ReaderCredentials + +/** + * The card reader, as a session sees it. + * + * Two calls and no implementation in this module yet. It exists now because without it the states between + * fetching the credentials and being ready cannot be entered, and a state nothing can reach is a branch a + * host writes and never runs. + */ +internal interface ReaderProvider { + /** + * Hands the reader what it needs to talk to its own service. + * + * **Do not keep [credentials] beyond this call.** They hold live vendor secrets, they are not stored + * anywhere by this SDK, and a session that needs them again fetches them again. + */ + suspend fun configure(credentials: ReaderCredentials) + + /** Brings the reader up. Fails if [configure] was not called first, or if its credentials were refused. */ + suspend fun prepareReader() +} diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt new file mode 100644 index 00000000..998236b1 --- /dev/null +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt @@ -0,0 +1,44 @@ +package com.payabli.sdk.taptopay.session + +/** + * Why a session failed, in terms of what can be done about it. + * + * A closed set of remedies rather than a description of what went wrong, because the question a host asks a + * failed session is which repair to offer. Two failures with the same remedy are one member here. + * + * The reason is an enum and not the exception. This value is held in a state that is read long after the + * call that produced it, and a `Throwable` brings a cause chain with it; the decode failures in this module + * already redact theirs for that reason. The exception still reaches the caller that was waiting, by being + * thrown. This carries what a later observer needs. + * + * There is no member for a reader that could not start. Nothing prepares a reader yet, and a reason nothing + * can produce is a branch a host writes and never runs. + */ +internal enum class TapToPayFailureReason { + /** + * The device's proof of identity is gone or was refused, so the session must be built from the top. + * + * The service revoked the attestation, or the credential it was pinned to has moved. Re-initializing + * does not repair it: that path does not attest. + */ + ATTESTATION_REQUIRED, + + /** + * The paypoint, the device or its gateway is not set up for card-present work. + * + * Nothing in the SDK repairs this and repeating the call will not either. It is a change someone makes + * to the account. + */ + CONFIGURATION_REJECTED, + + /** The service could not be reached, or failed inside. The same call may succeed later. */ + SERVICE_UNAVAILABLE, + + /** + * The SDK and the service disagree about the contract, or the SDK has a defect. + * + * A response that could not be decoded is the common one. A host cannot act on it; it is here so that it + * is not silently filed under one of the others. + */ + INTERNAL, +} diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt new file mode 100644 index 00000000..67859c45 --- /dev/null +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt @@ -0,0 +1,276 @@ +package com.payabli.sdk.taptopay.session + +import com.payabli.sdk.core.logging.LogCategory +import com.payabli.sdk.core.logging.LogField +import com.payabli.sdk.core.logging.LoggerRegistry +import com.payabli.sdk.core.logging.SdkLogger +import com.payabli.sdk.core.logging.debug +import com.payabli.sdk.taptopay.attestation.device.DeviceServiceClient +import com.payabli.sdk.taptopay.attestation.device.DeviceServiceException +import com.payabli.sdk.taptopay.attestation.device.ReaderCredentials +import com.payabli.sdk.taptopay.enrollment.DeviceEnrollment +import com.payabli.sdk.taptopay.enrollment.EnrollmentOutcome +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext + +/** + * Drives a card-present session: builds one, repairs one, and spends an activation code against one. + * + * **All three of those mutate the same state and reach the same reader, so they never overlap.** What a + * second caller gets is part of the contract rather than an accident of timing: + * + * - A caller of the **same** kind joins the one already running and is given its outcome, success or + * failure. It does no work of its own. + * - A caller of a **different** kind waits for it and then runs. Their meanings differ — repairing a session + * skips attestation and building one does not — so they cannot share an answer. + * - A caller whose owner withdrew is told so with [TapToPaySessionException.SetupAbandoned], and may ask + * again. It is never handed the owner's cancellation, which would make its own scope look like it was + * unwinding when nothing had cancelled it. + * + * Exclusion and joining are two mechanisms rather than one. A single queue would give the same behaviour and + * would make the two properties impossible to test apart, and each of them is worth its own failing test. + * + * **Locks are taken in one order and only one:** this region, then the enrollment coordinator's, then the + * attestor's. Nothing takes them the other way round, so there is no cycle to find. The state monitor is + * never held across any of them. + */ +internal class TapToPaySessionCoordinator( + private val entry: String, + private val enrollment: DeviceEnrollment, + private val client: DeviceServiceClient, + private val reader: ReaderProvider, + private val manager: TapToPaySessionManager, + private val logger: SdkLogger = LoggerRegistry.of(LogCategory.TAP_TO_PAY), +) { + /** Where the session has got to. Safe to collect at any time; reading it takes no lock. */ + val state: StateFlow get() = manager.state + + /** Serialises the work. Held for a whole run, so no two runs are ever inside the reader together. */ + private val region = Mutex() + + /** Guards [inFlight] alone. Nothing suspends while it is held. */ + private val claims = Mutex() + + private var inFlight: Claim? = null + + private class Claim( + val kind: SessionWorkKind, + val done: CompletableDeferred, + ) + + private sealed interface RunPlan { + class Join( + val done: CompletableDeferred, + ) : RunPlan + + class Own( + val claim: Claim, + ) : RunPlan + } + + /** + * Builds the session from wherever it stands: attest if needed, fetch the credentials, bring the reader + * up. + * + * Safe to call again at any time, including while one is already running. It starts from a known state + * rather than the caller's, so it does not depend on what the last attempt left behind. + * + * Fails with [TapToPaySessionException.PendingActivation] when the device still owes a code. + */ + suspend fun initialize() = runExclusively(SessionWorkKind.INITIALIZE) { runInitialize() } + + /** + * Repairs a session whose reader is spent, and does nothing to one that is ready. + * + * Cheaper than [initialize] because it does not attest, which is also why it cannot repair a session + * whose identity is gone: that fails with [TapToPaySessionException.NotRecoverable] and the remedy is + * [initialize]. + */ + suspend fun reinitializeIfNeeded() = runExclusively(SessionWorkKind.REINITIALIZE) { runReinitializeIfNeeded() } + + /** + * Spends the code the merchant issued out of band. + * + * Inside the same region as the two above, because it moves the same state and a code spent against a + * handle a concurrent registration has just replaced is a code wasted. A refused code leaves the session + * exactly where it was, since the device still owes one. + */ + suspend fun confirmActivation(activationCode: String) = + runExclusively(SessionWorkKind.ACTIVATE) { runConfirmActivation(activationCode) } + + /** + * Decides whether to join or to run, under [claims], and does neither while holding it. + * + * Holding the lock across the work would look equivalent and is not: every waiter would then find the + * slot empty in turn and start its own run. + */ + private suspend fun runExclusively( + kind: SessionWorkKind, + work: suspend () -> Unit, + ) { + val plan = + claims.withLock { + val existing = inFlight + if (existing != null && existing.kind == kind) { + RunPlan.Join(existing.done) + } else { + Claim(kind, CompletableDeferred()).also { inFlight = it }.let(RunPlan::Own) + } + } + when (plan) { + is RunPlan.Join -> { + logger.debug( + LogField.safe("event", "ttp_session_joined"), + LogField.safe("phase", kind.diagnosticName), + ) { "joined the session work already running" } + plan.done.await() + } + + is RunPlan.Own -> own(plan.claim, work) + } + } + + private suspend fun own( + claim: Claim, + work: suspend () -> Unit, + ) { + try { + region.withLock { work() } + } catch (withdrawn: CancellationException) { + // Nothing failed and nothing is in progress, so the honest landing is the start. It is also the + // one target that can never itself be refused. + withContext(NonCancellable) { manager.settle(TapToPaySessionState.Idle) } + release(claim, TapToPaySessionException.SetupAbandoned()) + throw withdrawn + } catch (failure: Exception) { + TapToPaySessionFailures.landingFor(failure)?.let(manager::settle) + release(claim, failure) + throw failure + } catch (fatal: Throwable) { + // Reaches the caller unchanged: an OutOfMemoryError is not a session failure and classifying it + // as one would blame the service for a process-fatal condition. The claim still cannot outlive + // it, or every later caller of this kind waits for something that will never complete. + release(claim, TapToPaySessionException.SetupAbandoned()) + throw fatal + } + release(claim, null) + } + + /** + * Clears the slot and then answers everyone waiting on it, in that order, so a caller woken here never + * finds a claim that has already finished. + * + * Uncancellable because liveness depends on it. A claim left set with nobody to complete it wedges every + * later caller of its kind, and whether [Mutex.withLock] observes an already-cancelled job depends on + * whether it has to suspend, which is not a thing correctness may rest on. + * + * The slot is cleared only when it is still this claim. A run that finishes after another kind has taken + * the slot would otherwise delete a claim that is still being waited on. + */ + private suspend fun release( + claim: Claim, + outcome: Throwable?, + ) = withContext(NonCancellable) { + claims.withLock { if (inFlight === claim) inFlight = null } + if (outcome == null) claim.done.complete(Unit) else claim.done.completeExceptionally(outcome) + } + + /** + * The cold path, and the warm one, which differ only in what enrollment finds. + * + * It starts with a reset, whatever the caller left behind. The table of legal moves is narrow by design, + * so without this a session that failed halfway would refuse the first phase of its own repair. + */ + private suspend fun runInitialize() { + manager.reset() + val outcome = manager.advance(TapToPaySessionState.AttestingDevice) { enrollment.enroll() } + if (outcome is EnrollmentOutcome.Attested && outcome.activationRequired) { + // Registration already said so, so there is nothing to learn from asking for the credentials. + throw TapToPaySessionException.PendingActivation() + } + bringReaderUp() + } + + /** + * The repair, which does not attest. + * + * A ready session is left alone. That is the whole reason a charge can call this without a round trip. + */ + private suspend fun runReinitializeIfNeeded() { + when (val current = state.value) { + TapToPaySessionState.Ready -> return + TapToPaySessionState.SessionExpired -> + // Only from here, because this is the state that says a repair is under way and it is the + // only one the table lets it be entered from. + manager.advance(TapToPaySessionState.Reinitializing) + + TapToPaySessionState.Idle, is TapToPaySessionState.Failed -> Unit + else -> throw TapToPaySessionException.NotRecoverable(current) + } + bringReaderUp() + } + + /** The half both entry points share: credentials, then a reader configured with them. */ + private suspend fun bringReaderUp() { + val credentials = manager.advance(TapToPaySessionState.FetchingConfig) { fetchConfig() } + manager.advance(TapToPaySessionState.InitializingReader) { + reader.configure(credentials) + reader.prepareReader() + } + manager.advance(TapToPaySessionState.Ready) + } + + /** + * The credentials, and the one place a warm start can learn the device still owes a code. + * + * Fetched every time. They are never stored and never held past the reader that takes them, so a repair + * asks again rather than reusing anything. + */ + private suspend fun fetchConfig(): ReaderCredentials { + val assertion = enrollment.assertion() ?: throw TapToPaySessionException.AttestationRequired() + return try { + client.config(entry, assertion).credentials + } catch (inactive: DeviceServiceException.Forbidden) { + // Both shapes of the refusal arrive as this one type, and this is where a warm start learns it: + // registration is the only other place the device is told it owes a code, and a warm start does + // not register. Translated rather than passed on, so a caller has one failure to handle for one + // condition however it was discovered. + throw TapToPaySessionException.PendingActivation(inactive) + } catch (stale: DeviceServiceException.NotAttested) { + // The service is holding the binding to the exact credential that made it, and it no longer + // matches. The stored record names something that cannot be used, so it goes; the key it was + // made with is left alone, and the next build attests against it again. + // + // Never attested again from in here. Doing that would spend a fresh challenge inside a call that + // is already failing, and hide the rotation that caused it. + enrollment.reset() + throw TapToPaySessionException.AttestationRequired(stale) + } + } + + /** + * Spends the code, then puts the session back to the start so it can be built. + * + * The service holds whether the device is active, so nothing is recorded here. A code the service + * refuses leaves the state alone, because the device still owes one. + */ + private suspend fun runConfirmActivation(activationCode: String) { + enrollment.confirmActivation(activationCode) + manager.settle(TapToPaySessionState.Idle) + } +} + +/** Which of the three entry points is running, so two of the same kind can share one run. */ +internal enum class SessionWorkKind { + INITIALIZE, + REINITIALIZE, + ACTIVATE, + ; + + val diagnosticName: String get() = name.lowercase() +} diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt new file mode 100644 index 00000000..23905690 --- /dev/null +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt @@ -0,0 +1,50 @@ +package com.payabli.sdk.taptopay.session + +/** + * A card-present session could not be built or repaired. + * + * Distinct from the wire failures in the device package, which say what one call answered. These say what + * happened to the session, and each one has a different thing a caller does next. + */ +internal sealed class TapToPaySessionException( + message: String, + cause: Throwable? = null, +) : Exception(message, cause) { + /** + * The device is registered but not active, so the merchant still owes it a code out of band. + * + * Not a defect and not retryable. A host collects the code and confirms it, and the session can then be + * built. + */ + class PendingActivation( + cause: Throwable? = null, + ) : TapToPaySessionException("the device is registered but has not been activated", cause) + + /** + * The device's proof of identity is gone, so nothing short of attesting again will do. + * + * Raised where the stored record is absent, and where the service refuses the one it was given. Both + * mean the same thing to a caller, and neither is repaired by re-initializing, which does not attest. + */ + class AttestationRequired( + cause: Throwable? = null, + ) : TapToPaySessionException("the device must be attested again", cause) + + /** + * A repair was asked for from a state that cannot be repaired. + * + * Building a session from the top is always available; this says only that the cheaper path is not. + */ + class NotRecoverable( + val state: TapToPaySessionState, + ) : TapToPaySessionException("a session cannot be repaired from ${state.diagnosticName}") + + /** + * The caller that owned this work withdrew, so it did not finish. + * + * What another caller waiting on the same work is given. Not the owner's cancellation, which would make + * the waiter's own scope look like it is unwinding while nothing has cancelled it. Nothing is left + * half-applied, and asking again is safe. + */ + class SetupAbandoned : TapToPaySessionException("the caller that owned this session setup withdrew") +} diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt new file mode 100644 index 00000000..8622981f --- /dev/null +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt @@ -0,0 +1,108 @@ +package com.payabli.sdk.taptopay.session + +import com.payabli.sdk.core.model.PayabliErrorCode +import com.payabli.sdk.core.model.PayabliException +import com.payabli.sdk.taptopay.attestation.AttestationException +import com.payabli.sdk.taptopay.attestation.device.DeviceServiceException +import com.payabli.sdk.taptopay.enrollment.DeviceActivationException +import com.payabli.sdk.taptopay.session.TapToPayFailureReason.ATTESTATION_REQUIRED +import com.payabli.sdk.taptopay.session.TapToPayFailureReason.CONFIGURATION_REJECTED +import com.payabli.sdk.taptopay.session.TapToPayFailureReason.INTERNAL +import com.payabli.sdk.taptopay.session.TapToPayFailureReason.SERVICE_UNAVAILABLE + +/** + * Where a session lands when the work under it fails. + * + * One place, so every phase of every entry point ends the same way. Spreading this across the phases is how + * a failure ends up leaving the state wherever it happened to be. + * + * **A landing is a remedy, not a description.** Two failures a host repairs identically share a member of + * [TapToPayFailureReason]; a failure whose remedy is unknown is [INTERNAL] rather than the nearest guess, + * because a wrong guess sends a host down a repair that cannot work. + * + * **Discarding the device's identity requires a positive match.** Only a refusal that names the attestation + * lands on [ATTESTATION_REQUIRED], which is the reason a host is told to attest again. Everything + * unrecognised lands somewhere that costs nothing to be wrong about. + */ +internal object TapToPaySessionFailures { + /** + * The state to publish for [failure], or null to leave the session where it is. + * + * Null is not an oversight. A wrong activation code fails the call and changes nothing about the + * session: the device still owes a code, which is what the state already says, and moving it would + * take away the very state a host is collecting the code under. + */ + fun landingFor(failure: Throwable): TapToPaySessionState? = + when (failure) { + is TapToPaySessionException.PendingActivation -> TapToPaySessionState.PendingActivation + is TapToPaySessionException.AttestationRequired -> failed(ATTESTATION_REQUIRED) + is TapToPaySessionException.NotRecoverable -> null + is TapToPaySessionException.SetupAbandoned -> TapToPaySessionState.Idle + is DeviceServiceException -> landingForService(failure) + is DeviceActivationException -> landingForActivation(failure) + is AttestationException -> landingForAttestation(failure) + is PayabliException -> landingForTransport(failure) + else -> failed(INTERNAL) + } + + /** + * A device the service does not hold as active is refused, and so is a caller whose token is not scoped + * for the route. Both arrive as one case, so both land here; the reader is unavailable either way. + * + * A 404 does not discard anything. It covers a paypoint, a device and a gateway that the service could + * not find, and only one of those three means the identity is stale. Telling them apart needs the + * service's own text, so the non-destructive landing is the one taken. + */ + private fun landingForService(failure: DeviceServiceException): TapToPaySessionState? = + when (failure) { + is DeviceServiceException.Forbidden -> TapToPaySessionState.PendingActivation + is DeviceServiceException.NotAttested -> failed(ATTESTATION_REQUIRED) + is DeviceServiceException.NotFound -> failed(CONFIGURATION_REJECTED) + // The request this SDK built was refused, which is this SDK's defect and not the account's. + is DeviceServiceException.BadRequest -> failed(INTERNAL) + is DeviceServiceException.ServerFailure -> failed(SERVICE_UNAVAILABLE) + is DeviceServiceException.Undecodable -> failed(INTERNAL) + is DeviceServiceException.Unclassified -> failed(SERVICE_UNAVAILABLE) + } + + /** + * Most activation failures leave the session alone, because the device still owes the code the caller + * was in the middle of spending. + * + * Two of them say the record names a device or an attestation the service does not have. + */ + private fun landingForActivation(failure: DeviceActivationException): TapToPaySessionState? = + when (failure) { + is DeviceActivationException.AttestationRevoked -> failed(ATTESTATION_REQUIRED) + is DeviceActivationException.DeviceUnknown -> failed(ATTESTATION_REQUIRED) + is DeviceActivationException.NotEnrolled -> failed(ATTESTATION_REQUIRED) + is DeviceActivationException.EntryNotAuthorized -> failed(CONFIGURATION_REJECTED) + is DeviceActivationException.PaypointUnknown -> failed(CONFIGURATION_REJECTED) + is DeviceActivationException.ServiceFailed -> failed(SERVICE_UNAVAILABLE) + else -> null + } + + /** + * A platform verdict, which the service would refuse anyway. + * + * The two the platform says to ask again about are service failures rather than identity ones: nothing + * about the device changed, so a host is told to retry rather than to attest. + */ + private fun landingForAttestation(failure: AttestationException): TapToPaySessionState? = + when (failure) { + is AttestationException.Retryable -> failed(SERVICE_UNAVAILABLE) + is AttestationException.Throttled -> failed(SERVICE_UNAVAILABLE) + is AttestationException.Misconfigured -> failed(CONFIGURATION_REJECTED) + else -> failed(ATTESTATION_REQUIRED) + } + + private fun landingForTransport(failure: PayabliException): TapToPaySessionState = + when (failure.code) { + PayabliErrorCode.PERMISSION_DENIED -> TapToPaySessionState.PendingActivation + PayabliErrorCode.INVALID_CONFIGURATION -> failed(CONFIGURATION_REJECTED) + PayabliErrorCode.DECODING_ERROR -> failed(INTERNAL) + else -> failed(SERVICE_UNAVAILABLE) + } + + private fun failed(reason: TapToPayFailureReason): TapToPaySessionState = TapToPaySessionState.Failed(reason) +} diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt new file mode 100644 index 00000000..0e816088 --- /dev/null +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt @@ -0,0 +1,147 @@ +package com.payabli.sdk.taptopay.session + +import com.payabli.sdk.core.logging.LogCategory +import com.payabli.sdk.core.logging.LogField +import com.payabli.sdk.core.logging.LoggerRegistry +import com.payabli.sdk.core.logging.SdkLogger +import com.payabli.sdk.core.logging.info +import com.payabli.sdk.core.logging.warn +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow + +/** + * The one writer of a card-present session's state. + * + * The sink is owned here rather than handed in, unlike the core session's: this state begins when a session + * is built and has nothing to say before that, so there is no reader to serve earlier. + * + * **No mutator returns a value a caller can drop.** The sibling SDK's transition returns a boolean that + * every one of its call sites discards, and the cost was a shipped defect: after an expiry its narrow table + * refused every move, so a full re-initialization ran every phase, reported success, and left the state + * where it started. [advance] closes that by owning both halves. Entering a phase without moving the state + * is not something a caller can express. + * + * The rule for a refused move is stated once: [advance] throws, because it runs inside a serialized region + * that starts from a known state, so a refusal there is a defect in this SDK's own sequence. [invalidate] + * logs and returns, because it is the one mutator a reader callback can reach from outside that region and + * it can legitimately lose a race. + */ +internal class TapToPaySessionManager( + private val logger: SdkLogger = LoggerRegistry.of(LogCategory.TAP_TO_PAY), +) { + /** + * Holds the decision and the write it depends on together. + * + * Writing first and reverting afterwards would be simpler and wrong: a `StateFlow` collector is woken by + * the write, so the reverted value can still be observed. + */ + private val guard = Any() + + private val sink = MutableStateFlow(TapToPaySessionState.Idle) + + /** Where the session has got to. Conflated, so a collector joining late sees the current value. */ + val state: StateFlow = sink.asStateFlow() + + /** + * Moves to [to] and then runs [work] under it. + * + * The order is the point. The state moves first, so a phase that runs has always been announced, and a + * phase that cannot be announced does not run: a refused move throws before [work] is reached, when + * nothing has happened yet. + * + * This does not serialize anything. Two callers advancing at once would interleave their phases, which + * is what the region in [TapToPaySessionCoordinator] exists to prevent. + */ + suspend fun advance( + to: TapToPaySessionState, + work: suspend () -> T, + ): T { + check(write(to)) { + // A defect in this SDK's own sequence rather than anything a host did, so it is not part of the + // failure vocabulary a caller handles. Both names are from the fixed state vocabulary. + "a session cannot move to ${to.diagnosticName} from ${state.value.diagnosticName}" + } + return work() + } + + /** + * Moves to [to] with nothing to run under it. + * + * For the states a run passes through or ends on rather than works in. It throws on refusal for the same + * reason the other overload does: reporting a session ready while it stands somewhere else is the defect + * this type exists to prevent. + */ + fun advance(to: TapToPaySessionState) { + check(write(to)) { + "a session cannot move to ${to.diagnosticName} from ${state.value.diagnosticName}" + } + } + + /** + * Puts the session back to the start. + * + * The first act of building a session, whatever the caller left behind, because the table is narrow and + * every phase after this one would otherwise be refused. + */ + fun reset() { + write(TapToPaySessionState.Idle) + } + + /** + * Records that the reader session behind a ready state is spent. + * + * Refusal is expected rather than exceptional: the caller is a reader whose failure may arrive after the + * session it belonged to was already replaced or torn down, so a move that is no longer legal is a stale + * report and not a defect. It is logged and dropped. + */ + fun invalidate() { + write(TapToPaySessionState.SessionExpired) + } + + /** + * The last write of a run, when the run did not get where it was going. + * + * Logs a refusal instead of throwing, because this is reached from a failure path: throwing here would + * replace the failure a caller is about to be given with one about bookkeeping. + */ + fun settle(to: TapToPaySessionState) { + write(to) + } + + /** + * Decides and writes under [guard]. Both records are emitted after the monitor is released, because a + * collector on an immediate dispatcher resumes inside the write, so whatever is held here is held while + * foreign code runs. + */ + private fun write(to: TapToPaySessionState): Boolean { + val from: TapToPaySessionState + val permitted: Boolean + val published: Boolean + + synchronized(guard) { + from = sink.value + permitted = TapToPaySessionTransitions.permits(from, to) + published = permitted && from != to + if (published) { + sink.value = to + } + } + + if (!permitted) { + logger.warn( + LogField.safe("event", "ttp_session_state_refused"), + LogField.safe("fromstate", from.diagnosticName), + LogField.safe("tostate", to.diagnosticName), + ) { "refused a session state change" } + } + if (published) { + logger.info( + LogField.safe("event", "ttp_session_state"), + LogField.safe("state", to.diagnosticName), + LogField.safe("errorkind", (to as? TapToPaySessionState.Failed)?.reason?.name), + ) { "session state changed" } + } + return permitted + } +} diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt new file mode 100644 index 00000000..afc7c0d9 --- /dev/null +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt @@ -0,0 +1,75 @@ +package com.payabli.sdk.taptopay.session + +/** + * Where a card-present session has got to. + * + * The same nine states the sibling SDK publishes, so an integrator moving between the platforms meets one + * model. Only [Failed] carries anything, which is why this is a sealed interface rather than an enum. + * + * **A failure names its reason, and that is not decoration.** Without one, every consumer of a failed + * session has to assume the most expensive repair, because it cannot tell an identity that was discarded + * from a paypoint that was misconfigured. The sibling SDK publishes a reasonless failure and its host layer + * ended up running a full re-initialization for all of them, having twice guessed wrong about which was + * cheaper. + * + * [Failed], not `Error`: `kotlin.Error` is default-imported and is a `Throwable`, so a member of that name + * would need qualifying anywhere a session and a throwable are handled together. + */ +internal sealed interface TapToPaySessionState { + /** Nothing has been attempted, or the last attempt was withdrawn. Reachable from every state. */ + data object Idle : TapToPaySessionState + + /** Proving the device and the app to the service. Skipped by a warm start and by a re-initialization. */ + data object AttestingDevice : TapToPaySessionState + + /** Fetching the reader credentials, which is also where a warm start learns activation is still owed. */ + data object FetchingConfig : TapToPaySessionState + + data object InitializingReader : TapToPaySessionState + + /** The reader can take a payment. */ + data object Ready : TapToPaySessionState + + /** + * The reader session died and the credentials behind it are spent. + * + * Repairable without attesting again, which is the whole reason it is separate from [Failed]. + */ + data object SessionExpired : TapToPaySessionState + + data object Reinitializing : TapToPaySessionState + + /** + * The service holds this device as registered but not yet active. + * + * The device owes a code the merchant issues out of band. Nothing the SDK can do advances this; a host + * collects the code and confirms it. + */ + data object PendingActivation : TapToPaySessionState + + /** The session cannot be used, and [reason] says what a host can do about it. */ + data class Failed( + val reason: TapToPayFailureReason, + ) : TapToPaySessionState +} + +/** + * The name for a log record. An exhaustive `when` rather than `simpleName`, so adding a state fails to + * compile here instead of emitting a name R8 is free to rewrite. + * + * [TapToPaySessionState.Failed]'s reason is not folded in. It is recorded beside this as its own field, so + * a reader can group by state without splitting one failure into four. + */ +internal val TapToPaySessionState.diagnosticName: String + get() = + when (this) { + TapToPaySessionState.Idle -> "idle" + TapToPaySessionState.AttestingDevice -> "attesting_device" + TapToPaySessionState.FetchingConfig -> "fetching_config" + TapToPaySessionState.InitializingReader -> "initializing_reader" + TapToPaySessionState.Ready -> "ready" + TapToPaySessionState.SessionExpired -> "session_expired" + TapToPaySessionState.Reinitializing -> "reinitializing" + TapToPaySessionState.PendingActivation -> "pending_activation" + is TapToPaySessionState.Failed -> "failed" + } diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionTransitions.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionTransitions.kt new file mode 100644 index 00000000..e128b48f --- /dev/null +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionTransitions.kt @@ -0,0 +1,58 @@ +package com.payabli.sdk.taptopay.session + +import com.payabli.sdk.taptopay.session.TapToPaySessionState.AttestingDevice +import com.payabli.sdk.taptopay.session.TapToPaySessionState.Failed +import com.payabli.sdk.taptopay.session.TapToPaySessionState.FetchingConfig +import com.payabli.sdk.taptopay.session.TapToPaySessionState.Idle +import com.payabli.sdk.taptopay.session.TapToPaySessionState.InitializingReader +import com.payabli.sdk.taptopay.session.TapToPaySessionState.PendingActivation +import com.payabli.sdk.taptopay.session.TapToPaySessionState.Ready +import com.payabli.sdk.taptopay.session.TapToPaySessionState.Reinitializing +import com.payabli.sdk.taptopay.session.TapToPaySessionState.SessionExpired + +/** + * Which moves between session states are legal. + * + * Separate from the machine that applies it so the table can be read, and tested, without a session. + * + * Three rules hold from every state and are stated here rather than repeated in nine rows: re-entering the + * current state is legal and publishes nothing, starting over is always reachable, and failing is always + * reachable. The sibling SDK declares the first two and reaches its failure state from three states its own + * table forbids, by writing that one directly instead of going through the table. Declaring the edge is the + * same behaviour with one writer instead of two. + */ +internal object TapToPaySessionTransitions { + fun permits( + from: TapToPaySessionState, + to: TapToPaySessionState, + ): Boolean = + when { + from == to -> true + to is Idle -> true + to is Failed -> true + else -> to in reachableFrom(from) + } + + /** + * The states reachable from [from] by a move the rules above do not already allow. + * + * An exhaustive `when` rather than a map, so a tenth state fails to compile here. A map would answer a + * state it has no row for with an empty set, which reads as a legitimate dead end. + */ + private fun reachableFrom(from: TapToPaySessionState): Set = + when (from) { + Idle -> setOf(AttestingDevice, FetchingConfig) + AttestingDevice -> setOf(FetchingConfig, PendingActivation) + FetchingConfig -> setOf(InitializingReader, PendingActivation) + InitializingReader -> setOf(Ready) + Ready -> setOf(SessionExpired) + // Only into a re-initialization. Reaching config directly from here would skip the state that + // says a repair is under way, and that state is what a host shows. + SessionExpired -> setOf(Reinitializing) + Reinitializing -> setOf(FetchingConfig) + // The device owes a code. Confirming it puts the session back through attestation rather than + // straight to config, because the service issues the credentials only to an active device. + PendingActivation -> setOf(AttestingDevice) + is Failed -> setOf(AttestingDevice, FetchingConfig) + } +} diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt new file mode 100644 index 00000000..b75868ff --- /dev/null +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt @@ -0,0 +1,119 @@ +package com.payabli.sdk.taptopay.session + +import com.payabli.sdk.taptopay.attestation.device.ReaderCredentials +import com.payabli.sdk.taptopay.enrollment.ENTRY +import com.payabli.sdk.taptopay.enrollment.EnrollmentFixture +import com.payabli.sdk.taptopay.enrollment.RouteScript +import com.payabli.sdk.taptopay.enrollment.attestBody +import com.payabli.sdk.taptopay.enrollment.challengeBody +import com.payabli.sdk.taptopay.enrollment.configBody +import com.payabli.sdk.taptopay.enrollment.registerBody +import kotlinx.coroutines.withTimeoutOrNull +import kotlin.time.Duration.Companion.seconds + +/** Bounds every test in this package, so a wedge is a failure rather than a suite that never returns. */ +internal val TEST_TIMEOUT = 5.seconds + +/** Bounds one await, so a stranded claim reports what was stranded instead of expiring the whole test. */ +private val COMPLETION_TIMEOUT = 3.seconds + +/** + * Awaits [block] under a deadline of its own. + * + * Without it a caller left waiting on a claim nobody completes fails as "the test timed out", which names + * the suite rather than the thing that wedged. + */ +internal suspend fun completing( + what: String, + block: suspend () -> T, +): T = + withTimeoutOrNull(COMPLETION_TIMEOUT) { block() } + ?: throw AssertionError("$what never completed: the session claim was stranded") + +/** + * A reader that records what it was asked to do, and can be held open. + * + * [sawOverlap] is the assertion worth making about serialization. Checking the state afterwards can be + * satisfied by luck; a flag raised from inside the shared resource cannot, because it says two runs were in + * there together. + */ +internal class FakeReaderProvider( + private val trace: MutableList, + private val gate: (suspend () -> Unit)? = null, +) : ReaderProvider { + var configureCount: Int = 0 + private set + var prepareCount: Int = 0 + private set + var lastCredentials: ReaderCredentials? = null + private set + var sawOverlap: Boolean = false + private set + + private var inside = false + + override suspend fun configure(credentials: ReaderCredentials) { + trace += "reader:configure" + configureCount++ + lastCredentials = credentials + } + + override suspend fun prepareReader() { + trace += "reader:prepare" + if (inside) sawOverlap = true + inside = true + try { + prepareCount++ + gate?.invoke() + } finally { + inside = false + } + } +} + +/** + * A session wired to fakes, sharing one trace with the enrollment underneath it. + * + * The trace spans the transport, the store and the reader, because the properties worth asserting here — + * that a second run sent nothing while the first held the region, and that the reader was entered once — + * span all three and no per-fake list can show that. + */ +internal class SessionFixture( + script: RouteScript, + firstReadGate: (suspend () -> Unit)? = null, + readerGate: (suspend () -> Unit)? = null, +) { + val enrollment = EnrollmentFixture(script, firstReadGate = firstReadGate) + + val reader = FakeReaderProvider(enrollment.trace, readerGate) + + val manager = TapToPaySessionManager(enrollment.logger) + + val coordinator = + TapToPaySessionCoordinator( + entry = ENTRY, + enrollment = enrollment.enrollment, + client = enrollment.client, + reader = reader, + manager = manager, + logger = enrollment.logger, + ) + + /** Only the transport's half of the trace, for asserting call order alone. */ + val routes: List get() = enrollment.routes + + val state: TapToPaySessionState get() = manager.state.value + + fun seedRecord() = enrollment.seedRecord() + + /** The full cold script, for a device that registers already active. */ + companion object { + fun coldScript(): RouteScript = + RouteScript( + RouteScript.CHALLENGE to listOf(challengeBody()), + RouteScript.REGISTER to listOf(registerBody(status = "active")), + RouteScript.ATTEST to listOf(attestBody()), + RouteScript.CONFIG to listOf(configBody()), + ) + } +} diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt new file mode 100644 index 00000000..f364d1c2 --- /dev/null +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt @@ -0,0 +1,192 @@ +package com.payabli.sdk.taptopay.session + +import com.payabli.sdk.taptopay.enrollment.RouteScript +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withTimeout +import kotlinx.coroutines.withTimeoutOrNull +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.seconds + +/** Long enough that a caller which is genuinely waiting has not finished, short enough to stay cheap. */ +private val BLOCKED_PROBE = 300.milliseconds + +/** The wall-clock ceiling once the gate is open. Generous, because this runs on shared CI hardware. */ +private val COMPLETION_PROBE = 30.seconds + +/** + * That the three entry points never overlap, and that two of the same kind share one run. + * + * **A gate, not a race.** The store fake parks the first run inside the region and holds it there until the + * test releases it, so the second caller genuinely arrives while the first is still in flight. Two callers + * merely launched together almost never collide, and a test written that way passes with the serialization + * removed. + * + * Exclusion and joining are separate mechanisms and have separate tests here. `region.withLock` in + * `TapToPaySessionCoordinator.own` is what the repair, the activation and the real-thread tests hold; the + * `RunPlan.Join` branch in `runExclusively` is what the two join tests hold. Removing either leaves the + * other's tests green. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class SessionSerializationTest { + @Test + fun `a repair started during a build sends nothing until the build finishes`() = + runTest(timeout = TEST_TIMEOUT) { + val held = CompletableDeferred() + val fixture = SessionFixture(SessionFixture.coldScript(), firstReadGate = { held.await() }) + + val build = launch(UnconfinedTestDispatcher(testScheduler)) { fixture.coordinator.initialize() } + assertEquals("the build parks before it calls anything", emptyList(), fixture.routes) + + val repair = + launch(UnconfinedTestDispatcher(testScheduler)) { fixture.coordinator.reinitializeIfNeeded() } + assertTrue("the repair is waiting for the region", repair.isActive) + assertEquals("the repair sent nothing while it waited", emptyList(), fixture.routes) + + held.complete(Unit) + completing("the build") { build.join() } + completing("the repair") { repair.join() } + + assertEquals( + listOf(RouteScript.CHALLENGE, RouteScript.REGISTER, RouteScript.ATTEST, RouteScript.CONFIG), + fixture.routes, + ) + assertEquals(TapToPaySessionState.Ready, fixture.state) + assertFalse("two runs were inside the reader together", fixture.reader.sawOverlap) + assertEquals(1, fixture.reader.prepareCount) + } + + @Test + fun `a second build joins the one in flight and runs nothing of its own`() = + runTest(timeout = TEST_TIMEOUT) { + val held = CompletableDeferred() + val fixture = SessionFixture(SessionFixture.coldScript(), firstReadGate = { held.await() }) + + val first = launch(UnconfinedTestDispatcher(testScheduler)) { fixture.coordinator.initialize() } + val second = launch(UnconfinedTestDispatcher(testScheduler)) { fixture.coordinator.initialize() } + assertTrue("the second caller is waiting on the first", second.isActive) + + held.complete(Unit) + completing("the first build") { first.join() } + completing("the joining build") { second.join() } + + assertEquals( + "one sequence, not two", + listOf(RouteScript.CHALLENGE, RouteScript.REGISTER, RouteScript.ATTEST, RouteScript.CONFIG), + fixture.routes, + ) + assertEquals(1, fixture.reader.configureCount) + assertEquals(1, fixture.reader.prepareCount) + assertEquals(TapToPaySessionState.Ready, fixture.state) + } + + @Test + fun `a joiner is told the owner withdrew, and the next caller can start`() = + runTest(timeout = TEST_TIMEOUT) { + val held = CompletableDeferred() + val fixture = SessionFixture(SessionFixture.coldScript(), firstReadGate = { held.await() }) + + val owner = launch(UnconfinedTestDispatcher(testScheduler)) { fixture.coordinator.initialize() } + var joinerFailure: Throwable? = null + val joiner = + launch(UnconfinedTestDispatcher(testScheduler)) { + try { + fixture.coordinator.initialize() + } catch (failure: TapToPaySessionException) { + joinerFailure = failure + } + } + + owner.cancel() + completing("the joining build") { joiner.join() } + + assertTrue( + "a joiner is never handed the owner's cancellation", + joinerFailure is TapToPaySessionException.SetupAbandoned, + ) + assertEquals( + "a withdrawn run leaves nothing in progress", + TapToPaySessionState.Idle, + fixture.state, + ) + + // The slot was cleared, so the next caller owns rather than waiting on a claim nobody holds. + held.complete(Unit) + completing("the build after the withdrawal") { fixture.coordinator.initialize() } + assertEquals(TapToPaySessionState.Ready, fixture.state) + } + + /** + * The same exclusion, on real threads and a real clock. + * + * `runBlocking` and a wall-clock probe, where every other test in this file uses virtual time. Virtual + * time cannot see a scheduler that is starved: a caller that spins holds the thread the deadline needs, + * so the deadline never fires and the suite hangs instead of failing. Only real threads tell waiting + * and spinning apart. + */ + @Test + fun `a repair genuinely waits on real threads rather than spinning`() { + val held = CompletableDeferred() + val fixture = SessionFixture(SessionFixture.coldScript(), firstReadGate = { held.await() }) + + runBlocking(Dispatchers.Default) { + val build = launch { fixture.coordinator.initialize() } + val repair = launch { fixture.coordinator.reinitializeIfNeeded() } + try { + assertNull( + "the repair ran while the build held the region", + withTimeoutOrNull(BLOCKED_PROBE) { repair.join() }, + ) + assertEquals("and it sent nothing while it waited", emptyList(), fixture.routes) + } finally { + // Released here rather than after the assertions, so a failing one cannot wedge the class. + held.complete(Unit) + } + withTimeout(COMPLETION_PROBE) { + build.join() + repair.join() + } + } + + assertFalse("two runs were inside the reader together", fixture.reader.sawOverlap) + assertEquals(TapToPaySessionState.Ready, fixture.state) + } + + @Test + fun `an activation does not overlap a build`() = + runTest(timeout = TEST_TIMEOUT) { + val held = CompletableDeferred() + val fixture = SessionFixture(SessionFixture.coldScript(), firstReadGate = { held.await() }) + + val build = launch(UnconfinedTestDispatcher(testScheduler)) { fixture.coordinator.initialize() } + var activationFailure: Throwable? = null + val activation = + launch(UnconfinedTestDispatcher(testScheduler)) { + try { + fixture.coordinator.confirmActivation("123456") + } catch (failure: Throwable) { + activationFailure = failure + } + } + assertTrue("the activation is waiting for the region", activation.isActive) + assertNull("it has not run yet", activationFailure) + + held.complete(Unit) + completing("the build") { build.join() } + completing("the activation") { activation.join() } + + // It ran after the build rather than beside it. What it answered is the enrollment layer's + // business; that it waited is this one's. + assertFalse("two runs were inside the reader together", fixture.reader.sawOverlap) + } +} diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt new file mode 100644 index 00000000..bab37498 --- /dev/null +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt @@ -0,0 +1,149 @@ +package com.payabli.sdk.taptopay.session + +import com.payabli.sdk.taptopay.enrollment.RouteScript +import com.payabli.sdk.taptopay.enrollment.configBody +import com.payabli.sdk.taptopay.enrollment.decline +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +private suspend fun failureOf(block: suspend () -> Unit): Throwable? = runCatching { block() }.exceptionOrNull() + +/** + * A device that was attested on an earlier run, coming back. + * + * The property worth proving is an absence: the cold sequence does not run again. It is asserted on the + * route trace rather than on a flag, and the script answers only `/config`, so an attestation re-run fails + * by naming the route that was not scripted rather than by an assertion nobody wrote. + */ +class SessionWarmStartTest { + @Test + fun `a warm start reaches pending activation through config, with no attestation re-run`() = + runTest(timeout = TEST_TIMEOUT) { + val fixture = + SessionFixture( + RouteScript(RouteScript.CONFIG to listOf(decline(403, "Device is not active."))), + ) + fixture.seedRecord() + + val failure = failureOf { fixture.coordinator.initialize() } + + assertTrue("$failure", failure is TapToPaySessionException.PendingActivation) + assertEquals(TapToPaySessionState.PendingActivation, fixture.state) + assertEquals(listOf(RouteScript.CONFIG), fixture.routes) + assertEquals("the platform was never asked for a verdict", 0, fixture.enrollment.attestor.challenges.size) + assertEquals("the reader was never reached", 0, fixture.reader.configureCount) + } + + @Test + fun `a warm start on an active device is ready without attesting`() = + runTest(timeout = TEST_TIMEOUT) { + val fixture = SessionFixture(RouteScript(RouteScript.CONFIG to listOf(configBody()))) + fixture.seedRecord() + + fixture.coordinator.initialize() + + assertEquals(TapToPaySessionState.Ready, fixture.state) + assertEquals(listOf(RouteScript.CONFIG), fixture.routes) + assertEquals(0, fixture.enrollment.attestor.challenges.size) + assertEquals("pp-id-value", fixture.reader.lastCredentials?.ppId) + } + + @Test + fun `a rotated credential discards the binding and fails the attempt, without re-attesting`() = + runTest(timeout = TEST_TIMEOUT) { + val fixture = + SessionFixture( + RouteScript( + RouteScript.CONFIG to + listOf(decline(401, "Device not attested or attestation revoked.")), + ), + ) + fixture.seedRecord() + + val failure = failureOf { fixture.coordinator.initialize() } + + assertTrue("$failure", failure is TapToPaySessionException.AttestationRequired) + assertEquals( + TapToPaySessionState.Failed(TapToPayFailureReason.ATTESTATION_REQUIRED), + fixture.state, + ) + assertEquals("the record names a binding that is gone", null, fixture.enrollment.storedRecord()) + assertEquals("no second attempt from inside the failing call", listOf(RouteScript.CONFIG), fixture.routes) + } + + @Test + fun `a repair on a device that owes a code reaches pending activation without attesting`() = + runTest(timeout = TEST_TIMEOUT) { + val fixture = + SessionFixture( + RouteScript(RouteScript.CONFIG to listOf(decline(403, "Device is not active."))), + ) + fixture.seedRecord() + + val failure = failureOf { fixture.coordinator.reinitializeIfNeeded() } + + assertTrue("$failure", failure is TapToPaySessionException.PendingActivation) + assertEquals(TapToPaySessionState.PendingActivation, fixture.state) + assertEquals(listOf(RouteScript.CONFIG), fixture.routes) + } + + @Test + fun `a repair does nothing to a ready session`() = + runTest(timeout = TEST_TIMEOUT) { + val fixture = SessionFixture(RouteScript(RouteScript.CONFIG to listOf(configBody()))) + fixture.seedRecord() + fixture.coordinator.initialize() + + // One answer is scripted, so a second fetch fails by name rather than by an assertion. + fixture.coordinator.reinitializeIfNeeded() + + assertEquals(TapToPaySessionState.Ready, fixture.state) + assertEquals(listOf(RouteScript.CONFIG), fixture.routes) + assertEquals(1, fixture.reader.prepareCount) + } + + @Test + fun `a build from a session that is already up runs every phase again`() = + runTest(timeout = TEST_TIMEOUT) { + val fixture = + SessionFixture(RouteScript(RouteScript.CONFIG to listOf(configBody(), configBody()))) + fixture.seedRecord() + fixture.coordinator.initialize() + assertEquals(TapToPaySessionState.Ready, fixture.state) + + fixture.coordinator.initialize() + + // Nothing in the table lets a ready session reach attestation or config directly. Building one + // starts from the beginning whatever the caller left behind, which is what makes this legal. + assertEquals(TapToPaySessionState.Ready, fixture.state) + assertEquals(listOf(RouteScript.CONFIG, RouteScript.CONFIG), fixture.routes) + assertEquals(2, fixture.reader.prepareCount) + } + + @Test + fun `a repair refuses a session it cannot repair, and names the state`() = + runTest(timeout = TEST_TIMEOUT) { + val fixture = + SessionFixture( + RouteScript(RouteScript.CONFIG to listOf(decline(403, "Device is not active."))), + ) + fixture.seedRecord() + failureOf { fixture.coordinator.initialize() } + assertEquals(TapToPaySessionState.PendingActivation, fixture.state) + + val failure = failureOf { fixture.coordinator.reinitializeIfNeeded() } + + assertTrue("$failure", failure is TapToPaySessionException.NotRecoverable) + assertEquals( + TapToPaySessionState.PendingActivation, + (failure as TapToPaySessionException.NotRecoverable).state, + ) + assertEquals( + "a refused repair leaves the session alone", + TapToPaySessionState.PendingActivation, + fixture.state, + ) + } +} diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt new file mode 100644 index 00000000..456748b7 --- /dev/null +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt @@ -0,0 +1,211 @@ +package com.payabli.sdk.taptopay.session + +import com.payabli.sdk.taptopay.attestation.impl.RecordingSdkLogger +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The machine on its own: what it publishes, what it refuses, and which of those two it throws for. + * + * The rule under test is that no mutator hands back a value a caller can drop. The sibling SDK returns a + * boolean from its transition and discards it at every call site, and the cost was a repair that ran every + * phase and moved the state nowhere. + */ +class TapToPaySessionManagerTest { + private val logger = RecordingSdkLogger() + private val manager = TapToPaySessionManager(logger) + + @Test + fun `a session starts at the beginning`() { + assertEquals(TapToPaySessionState.Idle, manager.state.value) + } + + @Test + fun `a refused move throws before the work under it runs`() = + runTest(timeout = TEST_TIMEOUT) { + manager.advance(TapToPaySessionState.FetchingConfig) + manager.advance(TapToPaySessionState.InitializingReader) + manager.advance(TapToPaySessionState.Ready) + + var ran = false + val failure = + runCatching { + // Not reachable from ready: a session that is up does not go back to fetching. + manager.advance(TapToPaySessionState.FetchingConfig) { ran = true } + }.exceptionOrNull() + + assertTrue("$failure", failure is IllegalStateException) + assertFalse("the work ran under a state that was never published", ran) + assertEquals(TapToPaySessionState.Ready, manager.state.value) + } + + @Test + fun `a phase that fails leaves the state where the phase was`() = + runTest(timeout = TEST_TIMEOUT) { + class PhaseFailed : Exception() + + val failure = + runCatching { + manager.advance(TapToPaySessionState.AttestingDevice) { throw PhaseFailed() } + }.exceptionOrNull() + + assertTrue("$failure", failure is PhaseFailed) + assertEquals( + "landing a failure belongs to the coordinator, not here", + TapToPaySessionState.AttestingDevice, + manager.state.value, + ) + } + + @Test + fun `every state can start over`() = + runTest(timeout = TEST_TIMEOUT) { + for (state in everyState()) { + val fresh = TapToPaySessionManager(logger) + driveTo(fresh, state) + assertEquals(state.diagnosticName, state, fresh.state.value) + + fresh.reset() + + assertEquals(state.diagnosticName, TapToPaySessionState.Idle, fresh.state.value) + } + } + + @Test + fun `a stale reader report is dropped rather than expiring a session that is not ready`() = + runTest(timeout = TEST_TIMEOUT) { + manager.advance(TapToPaySessionState.FetchingConfig) + + manager.invalidate() + + assertEquals(TapToPaySessionState.FetchingConfig, manager.state.value) + assertTrue( + "a dropped report is recorded, naming both ends of the move it refused", + logger.records.any { it.fieldNames.containsAll(listOf("fromstate", "tostate")) }, + ) + } + + @Test + fun `a ready session is expired by a reader report`() = + runTest(timeout = TEST_TIMEOUT) { + manager.advance(TapToPaySessionState.FetchingConfig) + manager.advance(TapToPaySessionState.InitializingReader) + manager.advance(TapToPaySessionState.Ready) + + manager.invalidate() + + assertEquals(TapToPaySessionState.SessionExpired, manager.state.value) + } + + @Test + fun `a refused move is never briefly published`() = + runTest(timeout = TEST_TIMEOUT) { + val seen = mutableListOf() + // An immediate dispatcher, so a collector would resume inside the write if one were made. + val collector = launch(Dispatchers.Unconfined) { manager.state.collect { seen += it } } + + manager.advance(TapToPaySessionState.FetchingConfig) + runCatching { manager.advance(TapToPaySessionState.Ready) } + manager.invalidate() + + collector.cancel() + assertEquals( + listOf(TapToPaySessionState.Idle, TapToPaySessionState.FetchingConfig), + seen, + ) + } + + @Test + fun `re-entering a state publishes nothing`() = + runTest(timeout = TEST_TIMEOUT) { + val seen = mutableListOf() + val collector = launch(Dispatchers.Unconfined) { manager.state.collect { seen += it } } + + manager.advance(TapToPaySessionState.FetchingConfig) + manager.advance(TapToPaySessionState.FetchingConfig) + + collector.cancel() + assertEquals(listOf(TapToPaySessionState.Idle, TapToPaySessionState.FetchingConfig), seen) + } + + @Test + fun `a failure publishes again when only its reason changed`() = + runTest(timeout = TEST_TIMEOUT) { + val seen = mutableListOf() + val collector = launch(Dispatchers.Unconfined) { manager.state.collect { seen += it } } + + manager.settle(TapToPaySessionState.Failed(TapToPayFailureReason.SERVICE_UNAVAILABLE)) + manager.settle(TapToPaySessionState.Failed(TapToPayFailureReason.ATTESTATION_REQUIRED)) + + collector.cancel() + assertEquals( + listOf( + TapToPaySessionState.Idle, + TapToPaySessionState.Failed(TapToPayFailureReason.SERVICE_UNAVAILABLE), + TapToPaySessionState.Failed(TapToPayFailureReason.ATTESTATION_REQUIRED), + ), + seen, + ) + } + + /** + * Walks a fresh machine to [target] through legal moves only. + * + * Seeding the field directly would let this test pass with the table broken, which is the one thing it + * must not do. + */ + private suspend fun driveTo( + manager: TapToPaySessionManager, + target: TapToPaySessionState, + ) { + when (target) { + TapToPaySessionState.Idle -> Unit + TapToPaySessionState.AttestingDevice -> manager.advance(target) + TapToPaySessionState.FetchingConfig -> manager.advance(target) + TapToPaySessionState.PendingActivation -> { + manager.advance(TapToPaySessionState.FetchingConfig) + manager.advance(target) + } + + TapToPaySessionState.InitializingReader -> { + manager.advance(TapToPaySessionState.FetchingConfig) + manager.advance(target) + } + + TapToPaySessionState.Ready -> { + driveTo(manager, TapToPaySessionState.InitializingReader) + manager.advance(target) + } + + TapToPaySessionState.SessionExpired -> { + driveTo(manager, TapToPaySessionState.Ready) + manager.invalidate() + } + + TapToPaySessionState.Reinitializing -> { + driveTo(manager, TapToPaySessionState.SessionExpired) + manager.advance(target) + } + + is TapToPaySessionState.Failed -> manager.settle(target) + } + } + + private fun everyState(): List = + listOf( + TapToPaySessionState.Idle, + TapToPaySessionState.AttestingDevice, + TapToPaySessionState.FetchingConfig, + TapToPaySessionState.InitializingReader, + TapToPaySessionState.Ready, + TapToPaySessionState.SessionExpired, + TapToPaySessionState.Reinitializing, + TapToPaySessionState.PendingActivation, + TapToPaySessionState.Failed(TapToPayFailureReason.INTERNAL), + ) +} diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt new file mode 100644 index 00000000..d79d0998 --- /dev/null +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt @@ -0,0 +1,105 @@ +package com.payabli.sdk.taptopay.session + +import com.payabli.sdk.taptopay.session.TapToPaySessionState.AttestingDevice +import com.payabli.sdk.taptopay.session.TapToPaySessionState.Failed +import com.payabli.sdk.taptopay.session.TapToPaySessionState.FetchingConfig +import com.payabli.sdk.taptopay.session.TapToPaySessionState.Idle +import com.payabli.sdk.taptopay.session.TapToPaySessionState.InitializingReader +import com.payabli.sdk.taptopay.session.TapToPaySessionState.PendingActivation +import com.payabli.sdk.taptopay.session.TapToPaySessionState.Ready +import com.payabli.sdk.taptopay.session.TapToPaySessionState.Reinitializing +import com.payabli.sdk.taptopay.session.TapToPaySessionState.SessionExpired +import org.junit.Assert.assertEquals +import org.junit.Test + +/** One state per member, so every ordered pair below is a real pair. */ +private val EVERY_STATE: List = + listOf( + Idle, + AttestingDevice, + FetchingConfig, + InitializingReader, + Ready, + SessionExpired, + Reinitializing, + PendingActivation, + Failed(TapToPayFailureReason.INTERNAL), + ) + +/** + * The whole table, restated. + * + * Each row is the **complete** set of targets that source accepts, written out. That includes the three + * rules the implementation states once — re-entering the current state, starting over, and failing — so + * deleting one of those rules from the implementation fails a row here. + * + * An exhaustive `when`, so a tenth state fails to compile here rather than going untested. + */ +private fun legalTargetsFrom(from: TapToPaySessionState): Set = + when (from) { + Idle -> setOf(Idle, AttestingDevice, FetchingConfig, FAILED) + AttestingDevice -> setOf(Idle, AttestingDevice, FetchingConfig, PendingActivation, FAILED) + FetchingConfig -> setOf(Idle, FetchingConfig, InitializingReader, PendingActivation, FAILED) + InitializingReader -> setOf(Idle, InitializingReader, Ready, FAILED) + Ready -> setOf(Idle, Ready, SessionExpired, FAILED) + SessionExpired -> setOf(Idle, SessionExpired, Reinitializing, FAILED) + Reinitializing -> setOf(Idle, Reinitializing, FetchingConfig, FAILED) + PendingActivation -> setOf(Idle, PendingActivation, AttestingDevice, FAILED) + is Failed -> setOf(Idle, AttestingDevice, FetchingConfig, FAILED) + } + +private val FAILED = Failed(TapToPayFailureReason.INTERNAL) + +class TapToPayTransitionMatrixTest { + @Test + fun `the table names every state`() { + assertEquals(EVERY_STATE.size, EVERY_STATE.distinct().size) + assertEquals(9, EVERY_STATE.size) + } + + @Test + fun `every ordered pair is decided as the table says`() { + for (from in EVERY_STATE) { + val legal = legalTargetsFrom(from) + for (to in EVERY_STATE) { + assertEquals( + "${from.diagnosticName} -> ${to.diagnosticName}", + to in legal, + TapToPaySessionTransitions.permits(from, to), + ) + } + } + } + + @Test + fun `starting over is reachable from every state`() { + for (from in EVERY_STATE) { + assertEquals(from.diagnosticName, true, TapToPaySessionTransitions.permits(from, Idle)) + } + } + + @Test + fun `failing is reachable from every state`() { + for (from in EVERY_STATE) { + assertEquals(from.diagnosticName, true, TapToPaySessionTransitions.permits(from, FAILED)) + } + } + + @Test + fun `re-entering the current state is permitted from every state`() { + for (from in EVERY_STATE) { + assertEquals(from.diagnosticName, true, TapToPaySessionTransitions.permits(from, from)) + } + } + + @Test + fun `a failure may change its reason`() { + assertEquals( + true, + TapToPaySessionTransitions.permits( + Failed(TapToPayFailureReason.SERVICE_UNAVAILABLE), + Failed(TapToPayFailureReason.ATTESTATION_REQUIRED), + ), + ) + } +} From 01ea15bc2de8d6472c8b7edcd92833f4c14d9b0a Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 14:47:34 -0700 Subject: [PATCH 03/18] [PLA-2184] Android - Hold a session claim per kind, so a build joins a build One slot held whichever kind claimed it last, so three callers arriving as build, repair, build left the repair in the slot when the second build looked. That build then started a second run of work already in flight: with the region serializing them it ran the whole sequence again rather than joining. A claim per kind closes it. Publishing the claim only after acquiring the region would also close the reported case and leaves a window between acquiring and publishing, and cannot join a run that is queued but has not started. Co-Authored-By: Claude Opus 5 (1M context) --- .../session/TapToPaySessionCoordinator.kt | 20 +++++++++----- .../session/SessionSerializationTest.kt | 26 +++++++++++++++++++ 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt index 67859c45..451872a6 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt @@ -24,8 +24,8 @@ import kotlinx.coroutines.withContext * **All three of those mutate the same state and reach the same reader, so they never overlap.** What a * second caller gets is part of the contract rather than an accident of timing: * - * - A caller of the **same** kind joins the one already running and is given its outcome, success or - * failure. It does no work of its own. + * - A caller of the **same** kind joins the one already in flight, running or waiting its turn, and is + * given its outcome, success or failure. It does no work of its own. * - A caller of a **different** kind waits for it and then runs. Their meanings differ — repairing a session * skips attestation and building one does not — so they cannot share an answer. * - A caller whose owner withdrew is told so with [TapToPaySessionException.SetupAbandoned], and may ask @@ -56,7 +56,13 @@ internal class TapToPaySessionCoordinator( /** Guards [inFlight] alone. Nothing suspends while it is held. */ private val claims = Mutex() - private var inFlight: Claim? = null + /** + * One claim per kind, so a caller joins work of its own kind whatever else is queued. + * + * A single slot cannot do this. Three callers arriving as build, repair, build leave the repair in the + * slot when the second build looks, so that build starts a second run of work already in flight. + */ + private val inFlight = mutableMapOf() private class Claim( val kind: SessionWorkKind, @@ -115,11 +121,11 @@ internal class TapToPaySessionCoordinator( ) { val plan = claims.withLock { - val existing = inFlight - if (existing != null && existing.kind == kind) { + val existing = inFlight[kind] + if (existing != null) { RunPlan.Join(existing.done) } else { - Claim(kind, CompletableDeferred()).also { inFlight = it }.let(RunPlan::Own) + Claim(kind, CompletableDeferred()).also { inFlight[kind] = it }.let(RunPlan::Own) } } when (plan) { @@ -176,7 +182,7 @@ internal class TapToPaySessionCoordinator( claim: Claim, outcome: Throwable?, ) = withContext(NonCancellable) { - claims.withLock { if (inFlight === claim) inFlight = null } + claims.withLock { if (inFlight[claim.kind] === claim) inFlight.remove(claim.kind) } if (outcome == null) claim.done.complete(Unit) else claim.done.completeExceptionally(outcome) } diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt index f364d1c2..79236c70 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt @@ -90,6 +90,32 @@ class SessionSerializationTest { assertEquals(TapToPaySessionState.Ready, fixture.state) } + @Test + fun `a build joins the build in flight even while a repair is queued behind it`() = + runTest(timeout = TEST_TIMEOUT) { + val held = CompletableDeferred() + val fixture = SessionFixture(SessionFixture.coldScript(), firstReadGate = { held.await() }) + + val first = launch(UnconfinedTestDispatcher(testScheduler)) { fixture.coordinator.initialize() } + // A caller of another kind arrives between the two builds and waits for the region. + val repair = + launch(UnconfinedTestDispatcher(testScheduler)) { fixture.coordinator.reinitializeIfNeeded() } + val second = launch(UnconfinedTestDispatcher(testScheduler)) { fixture.coordinator.initialize() } + + held.complete(Unit) + completing("the first build") { first.join() } + completing("the queued repair") { repair.join() } + completing("the second build") { second.join() } + + assertEquals( + "one sequence, not two", + listOf(RouteScript.CHALLENGE, RouteScript.REGISTER, RouteScript.ATTEST, RouteScript.CONFIG), + fixture.routes, + ) + assertEquals(1, fixture.reader.prepareCount) + assertEquals(TapToPaySessionState.Ready, fixture.state) + } + @Test fun `a joiner is told the owner withdrew, and the next caller can start`() = runTest(timeout = TEST_TIMEOUT) { From d0cfc85aadcf7b9761ae5d7d1f8f36c34cea5fca Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 14:48:15 -0700 Subject: [PATCH 04/18] [PLA-2184] Android - Name both repair failures on the entry point that raises them The KDoc named `NotRecoverable` for a device whose identity is gone. That is the refusal for a state a repair cannot start from. A device whose stored record is absent starts from a state that is repairable, gets as far as fetching the credentials, and fails with `AttestationRequired`. Both are now stated, and a test pins the one that was undocumented, so the sentence cannot drift from the code again. Co-Authored-By: Claude Opus 5 (1M context) --- .../session/TapToPaySessionCoordinator.kt | 8 +++++--- .../taptopay/session/SessionWarmStartTest.kt | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt index 451872a6..e8eecf32 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt @@ -93,9 +93,11 @@ internal class TapToPaySessionCoordinator( /** * Repairs a session whose reader is spent, and does nothing to one that is ready. * - * Cheaper than [initialize] because it does not attest, which is also why it cannot repair a session - * whose identity is gone: that fails with [TapToPaySessionException.NotRecoverable] and the remedy is - * [initialize]. + * Cheaper than [initialize] because it does not attest. Two failures follow from that, and they are + * different questions. A state this cannot be entered from is refused with + * [TapToPaySessionException.NotRecoverable]. A device whose stored identity is gone gets as far as + * fetching the credentials and fails with [TapToPaySessionException.AttestationRequired], because + * attesting is what would restore it. [initialize] is the remedy for both. */ suspend fun reinitializeIfNeeded() = runExclusively(SessionWorkKind.REINITIALIZE) { runReinitializeIfNeeded() } diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt index bab37498..e62090d5 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt @@ -122,6 +122,23 @@ class SessionWarmStartTest { assertEquals(2, fixture.reader.prepareCount) } + @Test + fun `a repair on a device with no stored identity asks for an attestation, not a state`() = + runTest(timeout = TEST_TIMEOUT) { + val fixture = SessionFixture(RouteScript(RouteScript.CONFIG to listOf(configBody()))) + + val failure = failureOf { fixture.coordinator.reinitializeIfNeeded() } + + // The state it started from is repairable, so this is not the state refusal. The record is what + // is missing, and only attesting replaces that. + assertTrue("$failure", failure is TapToPaySessionException.AttestationRequired) + assertEquals( + TapToPaySessionState.Failed(TapToPayFailureReason.ATTESTATION_REQUIRED), + fixture.state, + ) + assertEquals("nothing was sent", emptyList(), fixture.routes) + } + @Test fun `a repair refuses a session it cannot repair, and names the state`() = runTest(timeout = TEST_TIMEOUT) { From 97faa71d3d99c8fcf96f031274c61ebe3a0cc975 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 14:49:00 -0700 Subject: [PATCH 05/18] [PLA-2184] Android - Keep one list of the session states for the tests that walk them all Two copies stood in two files. A copy that loses a member narrows whatever it feeds without failing anything, and the size assertion guarding one of them guarded only that copy. Co-Authored-By: Claude Opus 5 (1M context) --- .../sdk/taptopay/session/SessionFixture.kt | 19 +++++++++++++ .../session/TapToPaySessionManagerTest.kt | 15 +--------- .../session/TapToPayTransitionMatrixTest.kt | 28 +++++-------------- 3 files changed, 27 insertions(+), 35 deletions(-) diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt index b75868ff..7e14660a 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt @@ -14,6 +14,25 @@ import kotlin.time.Duration.Companion.seconds /** Bounds every test in this package, so a wedge is a failure rather than a suite that never returns. */ internal val TEST_TIMEOUT = 5.seconds +/** + * One state per member, for the tests that walk all of them. + * + * One list for the package. A second copy drifts, and a copy that loses a member narrows whatever it feeds + * without failing anything. + */ +internal val EVERY_SESSION_STATE: List = + listOf( + TapToPaySessionState.Idle, + TapToPaySessionState.AttestingDevice, + TapToPaySessionState.FetchingConfig, + TapToPaySessionState.InitializingReader, + TapToPaySessionState.Ready, + TapToPaySessionState.SessionExpired, + TapToPaySessionState.Reinitializing, + TapToPaySessionState.PendingActivation, + TapToPaySessionState.Failed(TapToPayFailureReason.INTERNAL), + ) + /** Bounds one await, so a stranded claim reports what was stranded instead of expiring the whole test. */ private val COMPLETION_TIMEOUT = 3.seconds diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt index 456748b7..574ca3da 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt @@ -65,7 +65,7 @@ class TapToPaySessionManagerTest { @Test fun `every state can start over`() = runTest(timeout = TEST_TIMEOUT) { - for (state in everyState()) { + for (state in EVERY_SESSION_STATE) { val fresh = TapToPaySessionManager(logger) driveTo(fresh, state) assertEquals(state.diagnosticName, state, fresh.state.value) @@ -195,17 +195,4 @@ class TapToPaySessionManagerTest { is TapToPaySessionState.Failed -> manager.settle(target) } } - - private fun everyState(): List = - listOf( - TapToPaySessionState.Idle, - TapToPaySessionState.AttestingDevice, - TapToPaySessionState.FetchingConfig, - TapToPaySessionState.InitializingReader, - TapToPaySessionState.Ready, - TapToPaySessionState.SessionExpired, - TapToPaySessionState.Reinitializing, - TapToPaySessionState.PendingActivation, - TapToPaySessionState.Failed(TapToPayFailureReason.INTERNAL), - ) } diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt index d79d0998..ae4203fd 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt @@ -12,20 +12,6 @@ import com.payabli.sdk.taptopay.session.TapToPaySessionState.SessionExpired import org.junit.Assert.assertEquals import org.junit.Test -/** One state per member, so every ordered pair below is a real pair. */ -private val EVERY_STATE: List = - listOf( - Idle, - AttestingDevice, - FetchingConfig, - InitializingReader, - Ready, - SessionExpired, - Reinitializing, - PendingActivation, - Failed(TapToPayFailureReason.INTERNAL), - ) - /** * The whole table, restated. * @@ -53,15 +39,15 @@ private val FAILED = Failed(TapToPayFailureReason.INTERNAL) class TapToPayTransitionMatrixTest { @Test fun `the table names every state`() { - assertEquals(EVERY_STATE.size, EVERY_STATE.distinct().size) - assertEquals(9, EVERY_STATE.size) + assertEquals(EVERY_SESSION_STATE.size, EVERY_SESSION_STATE.distinct().size) + assertEquals(9, EVERY_SESSION_STATE.size) } @Test fun `every ordered pair is decided as the table says`() { - for (from in EVERY_STATE) { + for (from in EVERY_SESSION_STATE) { val legal = legalTargetsFrom(from) - for (to in EVERY_STATE) { + for (to in EVERY_SESSION_STATE) { assertEquals( "${from.diagnosticName} -> ${to.diagnosticName}", to in legal, @@ -73,21 +59,21 @@ class TapToPayTransitionMatrixTest { @Test fun `starting over is reachable from every state`() { - for (from in EVERY_STATE) { + for (from in EVERY_SESSION_STATE) { assertEquals(from.diagnosticName, true, TapToPaySessionTransitions.permits(from, Idle)) } } @Test fun `failing is reachable from every state`() { - for (from in EVERY_STATE) { + for (from in EVERY_SESSION_STATE) { assertEquals(from.diagnosticName, true, TapToPaySessionTransitions.permits(from, FAILED)) } } @Test fun `re-entering the current state is permitted from every state`() { - for (from in EVERY_STATE) { + for (from in EVERY_SESSION_STATE) { assertEquals(from.diagnosticName, true, TapToPaySessionTransitions.permits(from, from)) } } From 795065e6d27d9cbbd5c92be37725ddd4aee63d34 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 14:50:46 -0700 Subject: [PATCH 06/18] [PLA-2184] Android - Say which tests hold each half of the serialization The queued-repair test holds both the region and the claim slot, so the note claiming every test separates cleanly stopped being true when that test landed. Co-Authored-By: Claude Opus 5 (1M context) --- .../sdk/taptopay/session/SessionSerializationTest.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt index 79236c70..d7f59479 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt @@ -32,10 +32,10 @@ private val COMPLETION_PROBE = 30.seconds * merely launched together almost never collide, and a test written that way passes with the serialization * removed. * - * Exclusion and joining are separate mechanisms and have separate tests here. `region.withLock` in + * Exclusion and joining are separate mechanisms and mostly have separate tests here. `region.withLock` in * `TapToPaySessionCoordinator.own` is what the repair, the activation and the real-thread tests hold; the - * `RunPlan.Join` branch in `runExclusively` is what the two join tests hold. Removing either leaves the - * other's tests green. + * claim slot in `runExclusively` is what the two join tests hold. The queued-repair test holds both, since + * the case it covers is a build joining a build across a repair that sits between them. */ @OptIn(ExperimentalCoroutinesApi::class) class SessionSerializationTest { From ea3a111f462bf4ab643039e1c27287e529ab927c Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 15:00:58 -0700 Subject: [PATCH 07/18] [PLA-2184] Android - Comment pass over the session package and the config route No code changed. The comments carried the argument around the fact: a paragraph defending each choice against an alternative nobody proposed, and a sentence naming what some other code would have done. The constraint stays, the argument goes. The gating half of the comment check was already clean. The loose half reported thirty blocks in the session package; four remain, and each is one the rule keeps: two name what a guard catches, one is a definition, and one states why a retained state holds an enum instead of a throwable that would carry a cause chain. Co-Authored-By: Claude Opus 5 (1M context) --- .../attestation/device/DeviceServiceClient.kt | 11 ++-- .../attestation/device/DeviceWireFormat.kt | 16 +++-- .../sdk/taptopay/session/ReaderProvider.kt | 4 +- .../taptopay/session/TapToPayFailureReason.kt | 17 ++---- .../session/TapToPaySessionCoordinator.kt | 61 +++++++------------ .../session/TapToPaySessionException.kt | 10 ++- .../session/TapToPaySessionFailures.kt | 28 ++++----- .../session/TapToPaySessionManager.kt | 50 ++++++--------- .../taptopay/session/TapToPaySessionState.kt | 21 +++---- .../session/TapToPaySessionTransitions.kt | 16 +++-- .../sdk/taptopay/session/SessionFixture.kt | 6 +- .../session/SessionSerializationTest.kt | 23 +++---- .../taptopay/session/SessionWarmStartTest.kt | 6 +- .../session/TapToPayTransitionMatrixTest.kt | 2 +- 14 files changed, 109 insertions(+), 162 deletions(-) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt index fb298dc1..9f41adbe 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt @@ -212,7 +212,7 @@ internal class DeviceServiceClient( * **A device that still owes activation is refused, and the refusal arrives two ways.** A device the * service does not hold as active is declined with a 403 inside a 200. A caller whose token is not * scoped for this route is refused with a real 403, by the gateway, before any controller runs. Both - * become [DeviceServiceException.Forbidden], so a caller branches once rather than twice. + * become [DeviceServiceException.Forbidden], so a caller branches once. * * They are not the same condition and the shared classification is imprecise: a scope problem presents * as a device that owes a code. It is what the sibling client does, and separating them is a change both @@ -247,9 +247,8 @@ internal class DeviceServiceClient( /** * [entry] as one path segment, or a refusal. * - * Refused rather than encoded. A value that is not a single segment is a caller defect, and encoding it - * would send a request for a paypoint nobody named: `URLEncoder` is the wrong tool besides, since it - * writes a space as `+`, which is a query-string rule and not a path one. + * A value that is not a single segment is a caller defect, and encoding it sends a request for a + * paypoint nobody named. `URLEncoder` writes a space as `+`, which is a query-string rule. * * The message names the field and the shape, never the value, because an entry point identifies a * merchant. @@ -262,8 +261,8 @@ internal class DeviceServiceClient( /** * The four POSTs. Every one of them carries a body and resolves to its own template. * - * The pin is set here and in [get] rather than in one shared place, because the two assemblers build - * different request shapes. A sixth route inherits it from whichever of them it uses. + * The pin is set here and in [get], since the two assemblers build different request shapes. A sixth + * route inherits it from whichever of them it uses. */ private suspend fun post( route: String, diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceWireFormat.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceWireFormat.kt index b579f35c..d0321b47 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceWireFormat.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceWireFormat.kt @@ -236,8 +236,8 @@ internal class ActivateResponse( /** * `{ credentials }`. * - * Required, unlike the payloads above: a config carrying no credentials is unusable rather than partially - * usable, so an absent one is a decode failure and not an empty success. + * Required, unlike the payloads above: a config carrying no credentials is unusable, so an absent one is + * a decode failure. */ @Serializable internal class ConfigResponse( @@ -249,15 +249,13 @@ internal class ConfigResponse( /** * What the card reader is configured with, for one paypoint. * - * **Typed rather than a string map, and that is a redaction decision.** The shipping sibling client keeps - * this as an untyped dictionary and hands it on. A `Map`'s `toString` prints every value it holds, and two - * of these are the reader vendor's API credentials, so the same shape here would put them into any message - * built from a map that reached an exception. Naming the fields also states which two the reader cannot - * start without on this platform. + * **Typed, and that is a redaction decision.** A `Map`'s `toString` prints every value it holds, and two + * of these are the reader vendor's API credentials, so a map puts them into any message built from one that + * reached an exception. Naming the fields also states which two the reader cannot start without here. * * Every field is required. The service sends all of them, possibly empty, and a missing one means the - * response is not this route's. That is what makes [platform] self-enforcing rather than something to - * branch on: the sibling platform's variant omits [ppId] and [hostPort], so it fails to decode here. + * response is not this route's. That is what makes [platform] self-enforcing: the sibling platform's + * variant omits [ppId] and [hostPort], so it fails to decode here. * * `pageIdentifier` sits beside these on the wire and is not modelled. It is a fresh token the service mints * per call, so it is a different credential from the one the attestation row pins, and sending it as the diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/ReaderProvider.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/ReaderProvider.kt index 6ec6b628..e3dbdad0 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/ReaderProvider.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/ReaderProvider.kt @@ -5,9 +5,7 @@ import com.payabli.sdk.taptopay.attestation.device.ReaderCredentials /** * The card reader, as a session sees it. * - * Two calls and no implementation in this module yet. It exists now because without it the states between - * fetching the credentials and being ready cannot be entered, and a state nothing can reach is a branch a - * host writes and never runs. + * No implementation in this module. The reader arrives with the charge work. */ internal interface ReaderProvider { /** diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt index 998236b1..0c437c74 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt @@ -3,23 +3,18 @@ package com.payabli.sdk.taptopay.session /** * Why a session failed, in terms of what can be done about it. * - * A closed set of remedies rather than a description of what went wrong, because the question a host asks a - * failed session is which repair to offer. Two failures with the same remedy are one member here. + * Two failures a host repairs the same way are one member here. * - * The reason is an enum and not the exception. This value is held in a state that is read long after the - * call that produced it, and a `Throwable` brings a cause chain with it; the decode failures in this module - * already redact theirs for that reason. The exception still reaches the caller that was waiting, by being - * thrown. This carries what a later observer needs. - * - * There is no member for a reader that could not start. Nothing prepares a reader yet, and a reason nothing - * can produce is a branch a host writes and never runs. + * An enum, and not the exception: this value is retained in a state read long after the call that produced + * it, and a `Throwable` carries a cause chain that can hold a response body. The exception still reaches + * the caller that was waiting, by being thrown. */ internal enum class TapToPayFailureReason { /** * The device's proof of identity is gone or was refused, so the session must be built from the top. * - * The service revoked the attestation, or the credential it was pinned to has moved. Re-initializing - * does not repair it: that path does not attest. + * The service revoked the attestation, or the credential it was pinned to has moved. A repair does not + * attest, so it cannot restore this. */ ATTESTATION_REQUIRED, diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt index e8eecf32..309466c8 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt @@ -22,22 +22,19 @@ import kotlinx.coroutines.withContext * Drives a card-present session: builds one, repairs one, and spends an activation code against one. * * **All three of those mutate the same state and reach the same reader, so they never overlap.** What a - * second caller gets is part of the contract rather than an accident of timing: + * second caller gets: * * - A caller of the **same** kind joins the one already in flight, running or waiting its turn, and is * given its outcome, success or failure. It does no work of its own. - * - A caller of a **different** kind waits for it and then runs. Their meanings differ — repairing a session - * skips attestation and building one does not — so they cannot share an answer. - * - A caller whose owner withdrew is told so with [TapToPaySessionException.SetupAbandoned], and may ask - * again. It is never handed the owner's cancellation, which would make its own scope look like it was - * unwinding when nothing had cancelled it. + * - A caller of a **different** kind waits for it and then runs. Repairing a session skips attestation and + * building one does not, so they cannot share an answer. + * - A caller whose owner withdrew is given [TapToPaySessionException.SetupAbandoned] and may ask again. + * Handing on the owner's cancellation would make the waiter's own scope look like it is unwinding. * - * Exclusion and joining are two mechanisms rather than one. A single queue would give the same behaviour and - * would make the two properties impossible to test apart, and each of them is worth its own failing test. + * Exclusion is [region]; joining is [inFlight]. Each is held by its own tests. * * **Locks are taken in one order and only one:** this region, then the enrollment coordinator's, then the - * attestor's. Nothing takes them the other way round, so there is no cycle to find. The state monitor is - * never held across any of them. + * attestor's. The state monitor is never held across any of them. */ internal class TapToPaySessionCoordinator( private val entry: String, @@ -56,12 +53,7 @@ internal class TapToPaySessionCoordinator( /** Guards [inFlight] alone. Nothing suspends while it is held. */ private val claims = Mutex() - /** - * One claim per kind, so a caller joins work of its own kind whatever else is queued. - * - * A single slot cannot do this. Three callers arriving as build, repair, build leave the repair in the - * slot when the second build looks, so that build starts a second run of work already in flight. - */ + /** One claim per kind, so a caller joins work of its own kind whatever else is queued. */ private val inFlight = mutableMapOf() private class Claim( @@ -83,8 +75,8 @@ internal class TapToPaySessionCoordinator( * Builds the session from wherever it stands: attest if needed, fetch the credentials, bring the reader * up. * - * Safe to call again at any time, including while one is already running. It starts from a known state - * rather than the caller's, so it does not depend on what the last attempt left behind. + * Safe to call again at any time, including while one is already running. It starts from a known state, + * so it does not depend on what the last attempt left behind. * * Fails with [TapToPaySessionException.PendingActivation] when the device still owes a code. */ @@ -111,12 +103,7 @@ internal class TapToPaySessionCoordinator( suspend fun confirmActivation(activationCode: String) = runExclusively(SessionWorkKind.ACTIVATE) { runConfirmActivation(activationCode) } - /** - * Decides whether to join or to run, under [claims], and does neither while holding it. - * - * Holding the lock across the work would look equivalent and is not: every waiter would then find the - * slot empty in turn and start its own run. - */ + /** Decides whether to join or to run, under [claims], and does neither while holding it. */ private suspend fun runExclusively( kind: SessionWorkKind, work: suspend () -> Unit, @@ -150,8 +137,7 @@ internal class TapToPaySessionCoordinator( try { region.withLock { work() } } catch (withdrawn: CancellationException) { - // Nothing failed and nothing is in progress, so the honest landing is the start. It is also the - // one target that can never itself be refused. + // Nothing failed and nothing is in progress. Idle is also the one target that is never refused. withContext(NonCancellable) { manager.settle(TapToPaySessionState.Idle) } release(claim, TapToPaySessionException.SetupAbandoned()) throw withdrawn @@ -160,9 +146,8 @@ internal class TapToPaySessionCoordinator( release(claim, failure) throw failure } catch (fatal: Throwable) { - // Reaches the caller unchanged: an OutOfMemoryError is not a session failure and classifying it - // as one would blame the service for a process-fatal condition. The claim still cannot outlive - // it, or every later caller of this kind waits for something that will never complete. + // An OutOfMemoryError reaches the caller unchanged. The claim is still released, or every later + // caller of this kind waits for something that will never complete. release(claim, TapToPaySessionException.SetupAbandoned()) throw fatal } @@ -175,10 +160,10 @@ internal class TapToPaySessionCoordinator( * * Uncancellable because liveness depends on it. A claim left set with nobody to complete it wedges every * later caller of its kind, and whether [Mutex.withLock] observes an already-cancelled job depends on - * whether it has to suspend, which is not a thing correctness may rest on. + * whether it has to suspend. * - * The slot is cleared only when it is still this claim. A run that finishes after another kind has taken - * the slot would otherwise delete a claim that is still being waited on. + * The slot is cleared only when it still holds this claim, so a run finishing late leaves a successor's + * claim in place. */ private suspend fun release( claim: Claim, @@ -191,8 +176,7 @@ internal class TapToPaySessionCoordinator( /** * The cold path, and the warm one, which differ only in what enrollment finds. * - * It starts with a reset, whatever the caller left behind. The table of legal moves is narrow by design, - * so without this a session that failed halfway would refuse the first phase of its own repair. + * It starts with a reset, whatever the caller left behind, since the table of legal moves is narrow. */ private suspend fun runInitialize() { manager.reset() @@ -213,8 +197,7 @@ internal class TapToPaySessionCoordinator( when (val current = state.value) { TapToPaySessionState.Ready -> return TapToPaySessionState.SessionExpired -> - // Only from here, because this is the state that says a repair is under way and it is the - // only one the table lets it be entered from. + // The only state the table lets a re-initialization be entered from. manager.advance(TapToPaySessionState.Reinitializing) TapToPaySessionState.Idle, is TapToPaySessionState.Failed -> Unit @@ -236,8 +219,7 @@ internal class TapToPaySessionCoordinator( /** * The credentials, and the one place a warm start can learn the device still owes a code. * - * Fetched every time. They are never stored and never held past the reader that takes them, so a repair - * asks again rather than reusing anything. + * Fetched every time. They are never stored and never held past the reader that takes them. */ private suspend fun fetchConfig(): ReaderCredentials { val assertion = enrollment.assertion() ?: throw TapToPaySessionException.AttestationRequired() @@ -246,8 +228,7 @@ internal class TapToPaySessionCoordinator( } catch (inactive: DeviceServiceException.Forbidden) { // Both shapes of the refusal arrive as this one type, and this is where a warm start learns it: // registration is the only other place the device is told it owes a code, and a warm start does - // not register. Translated rather than passed on, so a caller has one failure to handle for one - // condition however it was discovered. + // not register. Translated here, so one condition reaches a caller as one failure. throw TapToPaySessionException.PendingActivation(inactive) } catch (stale: DeviceServiceException.NotAttested) { // The service is holding the binding to the exact credential that made it, and it no longer diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt index 23905690..6aae708a 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt @@ -13,8 +13,7 @@ internal sealed class TapToPaySessionException( /** * The device is registered but not active, so the merchant still owes it a code out of band. * - * Not a defect and not retryable. A host collects the code and confirms it, and the session can then be - * built. + * Not a defect and not retryable. A host collects the code and confirms it, and the session can be built. */ class PendingActivation( cause: Throwable? = null, @@ -24,7 +23,7 @@ internal sealed class TapToPaySessionException( * The device's proof of identity is gone, so nothing short of attesting again will do. * * Raised where the stored record is absent, and where the service refuses the one it was given. Both - * mean the same thing to a caller, and neither is repaired by re-initializing, which does not attest. + * mean the same thing to a caller, and a repair does not attest, so it fixes neither. */ class AttestationRequired( cause: Throwable? = null, @@ -42,9 +41,8 @@ internal sealed class TapToPaySessionException( /** * The caller that owned this work withdrew, so it did not finish. * - * What another caller waiting on the same work is given. Not the owner's cancellation, which would make - * the waiter's own scope look like it is unwinding while nothing has cancelled it. Nothing is left - * half-applied, and asking again is safe. + * What another caller waiting on the same work is given. The owner's cancellation would make the + * waiter's own scope look like it is unwinding. Nothing is left half-applied, and asking again is safe. */ class SetupAbandoned : TapToPaySessionException("the caller that owned this session setup withdrew") } diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt index 8622981f..cbeaa3c1 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt @@ -13,24 +13,21 @@ import com.payabli.sdk.taptopay.session.TapToPayFailureReason.SERVICE_UNAVAILABL /** * Where a session lands when the work under it fails. * - * One place, so every phase of every entry point ends the same way. Spreading this across the phases is how - * a failure ends up leaving the state wherever it happened to be. + * One place, so every phase of every entry point ends the same way. * - * **A landing is a remedy, not a description.** Two failures a host repairs identically share a member of - * [TapToPayFailureReason]; a failure whose remedy is unknown is [INTERNAL] rather than the nearest guess, - * because a wrong guess sends a host down a repair that cannot work. + * **A landing is a remedy.** Two failures a host repairs identically share a member of + * [TapToPayFailureReason], and a failure whose remedy is unknown is [INTERNAL]: a guess sends a host down a + * repair that cannot work. * * **Discarding the device's identity requires a positive match.** Only a refusal that names the attestation - * lands on [ATTESTATION_REQUIRED], which is the reason a host is told to attest again. Everything - * unrecognised lands somewhere that costs nothing to be wrong about. + * lands on [ATTESTATION_REQUIRED]. Everything unrecognised lands where being wrong costs nothing. */ internal object TapToPaySessionFailures { /** * The state to publish for [failure], or null to leave the session where it is. * - * Null is not an oversight. A wrong activation code fails the call and changes nothing about the - * session: the device still owes a code, which is what the state already says, and moving it would - * take away the very state a host is collecting the code under. + * A wrong activation code fails the call and changes nothing about the session: the device still owes a + * code, which is what the state already says, and moving it takes away the state a host collects under. */ fun landingFor(failure: Throwable): TapToPaySessionState? = when (failure) { @@ -49,16 +46,15 @@ internal object TapToPaySessionFailures { * A device the service does not hold as active is refused, and so is a caller whose token is not scoped * for the route. Both arrive as one case, so both land here; the reader is unavailable either way. * - * A 404 does not discard anything. It covers a paypoint, a device and a gateway that the service could - * not find, and only one of those three means the identity is stale. Telling them apart needs the - * service's own text, so the non-destructive landing is the one taken. + * A 404 discards nothing. It covers a paypoint, a device and a gateway the service could not find, and + * only one of those three means the identity is stale. Telling them apart needs the service's own text. */ private fun landingForService(failure: DeviceServiceException): TapToPaySessionState? = when (failure) { is DeviceServiceException.Forbidden -> TapToPaySessionState.PendingActivation is DeviceServiceException.NotAttested -> failed(ATTESTATION_REQUIRED) is DeviceServiceException.NotFound -> failed(CONFIGURATION_REJECTED) - // The request this SDK built was refused, which is this SDK's defect and not the account's. + // The request this SDK built was refused, which makes it this SDK's defect. is DeviceServiceException.BadRequest -> failed(INTERNAL) is DeviceServiceException.ServerFailure -> failed(SERVICE_UNAVAILABLE) is DeviceServiceException.Undecodable -> failed(INTERNAL) @@ -85,8 +81,8 @@ internal object TapToPaySessionFailures { /** * A platform verdict, which the service would refuse anyway. * - * The two the platform says to ask again about are service failures rather than identity ones: nothing - * about the device changed, so a host is told to retry rather than to attest. + * The two the platform says to ask again about are service failures. Nothing about the device changed, + * so a host is told to retry. */ private fun landingForAttestation(failure: AttestationException): TapToPaySessionState? = when (failure) { diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt index 0e816088..70399bd2 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt @@ -13,19 +13,14 @@ import kotlinx.coroutines.flow.asStateFlow /** * The one writer of a card-present session's state. * - * The sink is owned here rather than handed in, unlike the core session's: this state begins when a session - * is built and has nothing to say before that, so there is no reader to serve earlier. + * The sink is owned here. This state begins when a session is built and has nothing to say before that. * - * **No mutator returns a value a caller can drop.** The sibling SDK's transition returns a boolean that - * every one of its call sites discards, and the cost was a shipped defect: after an expiry its narrow table - * refused every move, so a full re-initialization ran every phase, reported success, and left the state - * where it started. [advance] closes that by owning both halves. Entering a phase without moving the state - * is not something a caller can express. + * **No mutator returns a value a caller can drop.** [advance] owns both halves, so entering a phase without + * moving the state cannot be expressed. * - * The rule for a refused move is stated once: [advance] throws, because it runs inside a serialized region - * that starts from a known state, so a refusal there is a defect in this SDK's own sequence. [invalidate] - * logs and returns, because it is the one mutator a reader callback can reach from outside that region and - * it can legitimately lose a race. + * A refused move throws from [advance], which runs inside a serialized region that starts from a known + * state, so a refusal there is a defect in this SDK's own sequence. [invalidate] logs and returns: it is + * the one mutator a reader callback reaches from outside that region, and it can lose a race legitimately. */ internal class TapToPaySessionManager( private val logger: SdkLogger = LoggerRegistry.of(LogCategory.TAP_TO_PAY), @@ -33,8 +28,7 @@ internal class TapToPaySessionManager( /** * Holds the decision and the write it depends on together. * - * Writing first and reverting afterwards would be simpler and wrong: a `StateFlow` collector is woken by - * the write, so the reverted value can still be observed. + * A `StateFlow` collector is woken by the write, so a value written and then reverted is still observed. */ private val guard = Any() @@ -46,20 +40,19 @@ internal class TapToPaySessionManager( /** * Moves to [to] and then runs [work] under it. * - * The order is the point. The state moves first, so a phase that runs has always been announced, and a - * phase that cannot be announced does not run: a refused move throws before [work] is reached, when - * nothing has happened yet. + * The state moves first, so a phase that runs has always been announced. A refused move throws before + * [work] is reached, when nothing has happened yet. * - * This does not serialize anything. Two callers advancing at once would interleave their phases, which - * is what the region in [TapToPaySessionCoordinator] exists to prevent. + * This serializes nothing. The region in [TapToPaySessionCoordinator] is what keeps two callers from + * interleaving their phases. */ suspend fun advance( to: TapToPaySessionState, work: suspend () -> T, ): T { check(write(to)) { - // A defect in this SDK's own sequence rather than anything a host did, so it is not part of the - // failure vocabulary a caller handles. Both names are from the fixed state vocabulary. + // A defect in this SDK's own sequence, so it is outside the failure vocabulary a caller handles. + // Both names come from the fixed state vocabulary. "a session cannot move to ${to.diagnosticName} from ${state.value.diagnosticName}" } return work() @@ -68,9 +61,8 @@ internal class TapToPaySessionManager( /** * Moves to [to] with nothing to run under it. * - * For the states a run passes through or ends on rather than works in. It throws on refusal for the same - * reason the other overload does: reporting a session ready while it stands somewhere else is the defect - * this type exists to prevent. + * For the states a run passes through or ends on. It throws on refusal for the reason the other overload + * does: a session reported ready while it stands somewhere else is the defect this type prevents. */ fun advance(to: TapToPaySessionState) { check(write(to)) { @@ -81,8 +73,7 @@ internal class TapToPaySessionManager( /** * Puts the session back to the start. * - * The first act of building a session, whatever the caller left behind, because the table is narrow and - * every phase after this one would otherwise be refused. + * The first act of building a session, whatever the caller left behind, since the table is narrow. */ fun reset() { write(TapToPaySessionState.Idle) @@ -91,9 +82,8 @@ internal class TapToPaySessionManager( /** * Records that the reader session behind a ready state is spent. * - * Refusal is expected rather than exceptional: the caller is a reader whose failure may arrive after the - * session it belonged to was already replaced or torn down, so a move that is no longer legal is a stale - * report and not a defect. It is logged and dropped. + * Refusal is expected here. The caller is a reader whose failure can arrive after the session it belonged + * to was replaced or torn down, so an illegal move is a stale report. It is logged and dropped. */ fun invalidate() { write(TapToPaySessionState.SessionExpired) @@ -102,8 +92,8 @@ internal class TapToPaySessionManager( /** * The last write of a run, when the run did not get where it was going. * - * Logs a refusal instead of throwing, because this is reached from a failure path: throwing here would - * replace the failure a caller is about to be given with one about bookkeeping. + * Logs a refusal, since this is reached from a failure path and a throw here would replace the failure a + * caller is about to be given. */ fun settle(to: TapToPaySessionState) { write(to) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt index afc7c0d9..1530b313 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt @@ -4,16 +4,13 @@ package com.payabli.sdk.taptopay.session * Where a card-present session has got to. * * The same nine states the sibling SDK publishes, so an integrator moving between the platforms meets one - * model. Only [Failed] carries anything, which is why this is a sealed interface rather than an enum. + * model. [Failed] carries a payload, which is why this is a sealed interface. * - * **A failure names its reason, and that is not decoration.** Without one, every consumer of a failed - * session has to assume the most expensive repair, because it cannot tell an identity that was discarded - * from a paypoint that was misconfigured. The sibling SDK publishes a reasonless failure and its host layer - * ended up running a full re-initialization for all of them, having twice guessed wrong about which was - * cheaper. + * **A failure names its reason.** Without one a consumer cannot tell an identity that was discarded from a + * paypoint that was misconfigured, and has to assume the most expensive repair. * * [Failed], not `Error`: `kotlin.Error` is default-imported and is a `Throwable`, so a member of that name - * would need qualifying anywhere a session and a throwable are handled together. + * needs qualifying anywhere a session and a throwable are handled together. */ internal sealed interface TapToPaySessionState { /** Nothing has been attempted, or the last attempt was withdrawn. Reachable from every state. */ @@ -33,7 +30,7 @@ internal sealed interface TapToPaySessionState { /** * The reader session died and the credentials behind it are spent. * - * Repairable without attesting again, which is the whole reason it is separate from [Failed]. + * Repairable without attesting again, which is what separates it from [Failed]. */ data object SessionExpired : TapToPaySessionState @@ -54,11 +51,11 @@ internal sealed interface TapToPaySessionState { } /** - * The name for a log record. An exhaustive `when` rather than `simpleName`, so adding a state fails to - * compile here instead of emitting a name R8 is free to rewrite. + * The name for a log record. An exhaustive `when`, so adding a state fails to compile here and no name is + * left for R8 to rewrite. * - * [TapToPaySessionState.Failed]'s reason is not folded in. It is recorded beside this as its own field, so - * a reader can group by state without splitting one failure into four. + * [TapToPaySessionState.Failed]'s reason is recorded beside this as its own field, so a reader can group by + * state without splitting one failure into four. */ internal val TapToPaySessionState.diagnosticName: String get() = diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionTransitions.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionTransitions.kt index e128b48f..89c846ed 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionTransitions.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionTransitions.kt @@ -15,11 +15,9 @@ import com.payabli.sdk.taptopay.session.TapToPaySessionState.SessionExpired * * Separate from the machine that applies it so the table can be read, and tested, without a session. * - * Three rules hold from every state and are stated here rather than repeated in nine rows: re-entering the - * current state is legal and publishes nothing, starting over is always reachable, and failing is always - * reachable. The sibling SDK declares the first two and reaches its failure state from three states its own - * table forbids, by writing that one directly instead of going through the table. Declaring the edge is the - * same behaviour with one writer instead of two. + * Three rules hold from every state and are stated once here: re-entering the current state is legal and + * publishes nothing, starting over is always reachable, and failing is always reachable. Declaring the + * failure edge keeps one writer for the state. */ internal object TapToPaySessionTransitions { fun permits( @@ -36,8 +34,8 @@ internal object TapToPaySessionTransitions { /** * The states reachable from [from] by a move the rules above do not already allow. * - * An exhaustive `when` rather than a map, so a tenth state fails to compile here. A map would answer a - * state it has no row for with an empty set, which reads as a legitimate dead end. + * An exhaustive `when`, so a tenth state fails to compile here. A map answers a state it has no row for + * with an empty set, which reads as a legitimate dead end. */ private fun reachableFrom(from: TapToPaySessionState): Set = when (from) { @@ -50,8 +48,8 @@ internal object TapToPaySessionTransitions { // says a repair is under way, and that state is what a host shows. SessionExpired -> setOf(Reinitializing) Reinitializing -> setOf(FetchingConfig) - // The device owes a code. Confirming it puts the session back through attestation rather than - // straight to config, because the service issues the credentials only to an active device. + // The device owes a code. Confirming it puts the session back through attestation, since the + // service issues the credentials only to an active device. PendingActivation -> setOf(AttestingDevice) is Failed -> setOf(AttestingDevice, FetchingConfig) } diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt index 7e14660a..b3af92e9 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt @@ -11,7 +11,7 @@ import com.payabli.sdk.taptopay.enrollment.registerBody import kotlinx.coroutines.withTimeoutOrNull import kotlin.time.Duration.Companion.seconds -/** Bounds every test in this package, so a wedge is a failure rather than a suite that never returns. */ +/** Bounds every test in this package, so a wedge fails the test that caused it. */ internal val TEST_TIMEOUT = 5.seconds /** @@ -33,14 +33,14 @@ internal val EVERY_SESSION_STATE: List = TapToPaySessionState.Failed(TapToPayFailureReason.INTERNAL), ) -/** Bounds one await, so a stranded claim reports what was stranded instead of expiring the whole test. */ +/** Bounds one await, so a stranded claim reports what was stranded. */ private val COMPLETION_TIMEOUT = 3.seconds /** * Awaits [block] under a deadline of its own. * * Without it a caller left waiting on a claim nobody completes fails as "the test timed out", which names - * the suite rather than the thing that wedged. + * no claim. */ internal suspend fun completing( what: String, diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt index d7f59479..7b3d8d1f 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt @@ -28,14 +28,12 @@ private val COMPLETION_PROBE = 30.seconds * That the three entry points never overlap, and that two of the same kind share one run. * * **A gate, not a race.** The store fake parks the first run inside the region and holds it there until the - * test releases it, so the second caller genuinely arrives while the first is still in flight. Two callers - * merely launched together almost never collide, and a test written that way passes with the serialization - * removed. + * test releases it, so the second caller arrives while the first is still in flight. Two callers merely + * launched together almost never collide, and a test written that way passes with the serialization removed. * - * Exclusion and joining are separate mechanisms and mostly have separate tests here. `region.withLock` in - * `TapToPaySessionCoordinator.own` is what the repair, the activation and the real-thread tests hold; the - * claim slot in `runExclusively` is what the two join tests hold. The queued-repair test holds both, since - * the case it covers is a build joining a build across a repair that sits between them. + * `region.withLock` in `TapToPaySessionCoordinator.own` is held by the repair, the activation and the + * real-thread tests; the claim slot in `runExclusively` is held by the two join tests. The queued-repair + * test covers a build joining a build across a repair between them, so it holds both. */ @OptIn(ExperimentalCoroutinesApi::class) class SessionSerializationTest { @@ -146,7 +144,7 @@ class SessionSerializationTest { fixture.state, ) - // The slot was cleared, so the next caller owns rather than waiting on a claim nobody holds. + // The slot was cleared, so the next caller owns the run. held.complete(Unit) completing("the build after the withdrawal") { fixture.coordinator.initialize() } assertEquals(TapToPaySessionState.Ready, fixture.state) @@ -157,8 +155,7 @@ class SessionSerializationTest { * * `runBlocking` and a wall-clock probe, where every other test in this file uses virtual time. Virtual * time cannot see a scheduler that is starved: a caller that spins holds the thread the deadline needs, - * so the deadline never fires and the suite hangs instead of failing. Only real threads tell waiting - * and spinning apart. + * so the deadline never fires and the suite hangs. Only real threads tell waiting and spinning apart. */ @Test fun `a repair genuinely waits on real threads rather than spinning`() { @@ -175,7 +172,7 @@ class SessionSerializationTest { ) assertEquals("and it sent nothing while it waited", emptyList(), fixture.routes) } finally { - // Released here rather than after the assertions, so a failing one cannot wedge the class. + // Released before the assertions run, so a failing one cannot wedge the class. held.complete(Unit) } withTimeout(COMPLETION_PROBE) { @@ -211,8 +208,8 @@ class SessionSerializationTest { completing("the build") { build.join() } completing("the activation") { activation.join() } - // It ran after the build rather than beside it. What it answered is the enrollment layer's - // business; that it waited is this one's. + // It ran after the build. What it answered is the enrollment layer's business; that it waited + // is this one's. assertFalse("two runs were inside the reader together", fixture.reader.sawOverlap) } } diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt index e62090d5..064d543c 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionWarmStartTest.kt @@ -14,8 +14,8 @@ private suspend fun failureOf(block: suspend () -> Unit): Throwable? = runCatchi * A device that was attested on an earlier run, coming back. * * The property worth proving is an absence: the cold sequence does not run again. It is asserted on the - * route trace rather than on a flag, and the script answers only `/config`, so an attestation re-run fails - * by naming the route that was not scripted rather than by an assertion nobody wrote. + * route trace, and the script answers only `/config`, so an attestation re-run fails by naming the route + * that was not scripted. */ class SessionWarmStartTest { @Test @@ -96,7 +96,7 @@ class SessionWarmStartTest { fixture.seedRecord() fixture.coordinator.initialize() - // One answer is scripted, so a second fetch fails by name rather than by an assertion. + // One answer is scripted, so a second fetch fails by name. fixture.coordinator.reinitializeIfNeeded() assertEquals(TapToPaySessionState.Ready, fixture.state) diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt index ae4203fd..c89f76fe 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt @@ -19,7 +19,7 @@ import org.junit.Test * rules the implementation states once — re-entering the current state, starting over, and failing — so * deleting one of those rules from the implementation fails a row here. * - * An exhaustive `when`, so a tenth state fails to compile here rather than going untested. + * An exhaustive `when`, so a tenth state fails to compile here. */ private fun legalTargetsFrom(from: TapToPaySessionState): Set = when (from) { From ba9457fd10a6e16615c112ffa703cd242765da58 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 15:22:36 -0700 Subject: [PATCH 08/18] [PLA-2184] Android - Name the state a refused move was decided against The thrown message read `state.value` after `write` returned. That read happens outside the monitor the decision was made under, so a concurrent write lands between the two and the message names a state that had nothing to do with the refusal. `write` now returns the state it decided against, and one `writeOrThrow` builds the message from it. No caller reads the state a second time. Test: none. The window is between a returned value and a lambda evaluation, and no deterministic interleaving reaches it. The second read is gone rather than guarded. Co-Authored-By: Claude Opus 5 (1M context) --- .../session/TapToPaySessionManager.kt | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt index 70399bd2..68587fd9 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt @@ -50,11 +50,7 @@ internal class TapToPaySessionManager( to: TapToPaySessionState, work: suspend () -> T, ): T { - check(write(to)) { - // A defect in this SDK's own sequence, so it is outside the failure vocabulary a caller handles. - // Both names come from the fixed state vocabulary. - "a session cannot move to ${to.diagnosticName} from ${state.value.diagnosticName}" - } + writeOrThrow(to) return work() } @@ -65,8 +61,23 @@ internal class TapToPaySessionManager( * does: a session reported ready while it stands somewhere else is the defect this type prevents. */ fun advance(to: TapToPaySessionState) { - check(write(to)) { - "a session cannot move to ${to.diagnosticName} from ${state.value.diagnosticName}" + writeOrThrow(to) + } + + /** + * Writes, or throws naming the state the refusal was decided against. + * + * That state comes back from [write] rather than being read again. A second read happens outside the + * monitor, so a concurrent write lands between the two and the message names a state that had nothing to + * do with the refusal. + * + * A defect in this SDK's own sequence, so it is outside the failure vocabulary a caller handles. Both + * names come from the fixed state vocabulary. + */ + private fun writeOrThrow(to: TapToPaySessionState) { + val written = write(to) + check(written.permitted) { + "a session cannot move to ${to.diagnosticName} from ${written.from.diagnosticName}" } } @@ -104,7 +115,7 @@ internal class TapToPaySessionManager( * collector on an immediate dispatcher resumes inside the write, so whatever is held here is held while * foreign code runs. */ - private fun write(to: TapToPaySessionState): Boolean { + private fun write(to: TapToPaySessionState): Written { val from: TapToPaySessionState val permitted: Boolean val published: Boolean @@ -132,6 +143,12 @@ internal class TapToPaySessionManager( LogField.safe("errorkind", (to as? TapToPaySessionState.Failed)?.reason?.name), ) { "session state changed" } } - return permitted + return Written(from, permitted) } + + /** What one write decided, so a caller naming the refusal does not read the state a second time. */ + private class Written( + val from: TapToPaySessionState, + val permitted: Boolean, + ) } From 41d6f178ce171895e1daa9a36c96247f550069dc Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 15:23:05 -0700 Subject: [PATCH 09/18] [PLA-2184] Android - Say what a warm start does to the attestation state The state claimed a warm start skips it. Building a session always advances into it and then calls enrollment, which reads the stored record and decides for itself whether the cold sequence is needed, so a warm start enters the state and leaves it without a round trip. A repair is the one that never enters it. Swept the other three warm-start sentences in the package; each describes where the device learns it owes an activation code, which is unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- .../payabli/sdk/taptopay/session/TapToPaySessionState.kt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt index 1530b313..bdc4b23b 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionState.kt @@ -16,7 +16,13 @@ internal sealed interface TapToPaySessionState { /** Nothing has been attempted, or the last attempt was withdrawn. Reachable from every state. */ data object Idle : TapToPaySessionState - /** Proving the device and the app to the service. Skipped by a warm start and by a re-initialization. */ + /** + * Where the device's identity is established with the service. + * + * A repair never enters it. A warm start does, and leaves without a round trip: enrollment reads the + * stored record and decides for itself whether the cold sequence is needed, so the state covers asking + * the question as well as answering it. + */ data object AttestingDevice : TapToPaySessionState /** Fetching the reader credentials, which is also where a warm start learns activation is still owed. */ From caa0d698791dcf183c8d250b009638377faf4b90 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 15:23:38 -0700 Subject: [PATCH 10/18] [PLA-2184] Android - Join the coroutines these tests cancel, before asserting on what they left Two shapes, both timing-sensitive. The collector tests cancelled the collector and asserted on the list it fills without waiting for it to stop. The withdrawal test cancelled the owner and asserted on the state and the claim slot without waiting for the cancellation path, which releases the claim and settles the state under `NonCancellable`. `cancelAndJoin` in both. The collectors also move from `Dispatchers.Unconfined` to `UnconfinedTestDispatcher(testScheduler)`, which keeps the immediate resumption the assertion depends on and ties it to the test scheduler. Co-Authored-By: Claude Opus 5 (1M context) --- .../session/SessionSerializationTest.kt | 3 ++- .../session/TapToPaySessionManagerTest.kt | 18 ++++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt index 7b3d8d1f..1e047e13 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt @@ -4,6 +4,7 @@ import com.payabli.sdk.taptopay.enrollment.RouteScript import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.UnconfinedTestDispatcher @@ -131,7 +132,7 @@ class SessionSerializationTest { } } - owner.cancel() + owner.cancelAndJoin() completing("the joining build") { joiner.join() } assertTrue( diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt index 574ca3da..4db56f1b 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionManagerTest.kt @@ -1,8 +1,9 @@ package com.payabli.sdk.taptopay.session import com.payabli.sdk.taptopay.attestation.impl.RecordingSdkLogger -import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.launch +import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -16,6 +17,7 @@ import org.junit.Test * boolean from its transition and discards it at every call site, and the cost was a repair that ran every * phase and moved the state nowhere. */ +@OptIn(kotlinx.coroutines.ExperimentalCoroutinesApi::class) class TapToPaySessionManagerTest { private val logger = RecordingSdkLogger() private val manager = TapToPaySessionManager(logger) @@ -106,14 +108,14 @@ class TapToPaySessionManagerTest { fun `a refused move is never briefly published`() = runTest(timeout = TEST_TIMEOUT) { val seen = mutableListOf() - // An immediate dispatcher, so a collector would resume inside the write if one were made. - val collector = launch(Dispatchers.Unconfined) { manager.state.collect { seen += it } } + // Unconfined, so a collector resumes inside the write if one is made. + val collector = launch(UnconfinedTestDispatcher(testScheduler)) { manager.state.collect { seen += it } } manager.advance(TapToPaySessionState.FetchingConfig) runCatching { manager.advance(TapToPaySessionState.Ready) } manager.invalidate() - collector.cancel() + collector.cancelAndJoin() assertEquals( listOf(TapToPaySessionState.Idle, TapToPaySessionState.FetchingConfig), seen, @@ -124,12 +126,12 @@ class TapToPaySessionManagerTest { fun `re-entering a state publishes nothing`() = runTest(timeout = TEST_TIMEOUT) { val seen = mutableListOf() - val collector = launch(Dispatchers.Unconfined) { manager.state.collect { seen += it } } + val collector = launch(UnconfinedTestDispatcher(testScheduler)) { manager.state.collect { seen += it } } manager.advance(TapToPaySessionState.FetchingConfig) manager.advance(TapToPaySessionState.FetchingConfig) - collector.cancel() + collector.cancelAndJoin() assertEquals(listOf(TapToPaySessionState.Idle, TapToPaySessionState.FetchingConfig), seen) } @@ -137,12 +139,12 @@ class TapToPaySessionManagerTest { fun `a failure publishes again when only its reason changed`() = runTest(timeout = TEST_TIMEOUT) { val seen = mutableListOf() - val collector = launch(Dispatchers.Unconfined) { manager.state.collect { seen += it } } + val collector = launch(UnconfinedTestDispatcher(testScheduler)) { manager.state.collect { seen += it } } manager.settle(TapToPaySessionState.Failed(TapToPayFailureReason.SERVICE_UNAVAILABLE)) manager.settle(TapToPaySessionState.Failed(TapToPayFailureReason.ATTESTATION_REQUIRED)) - collector.cancel() + collector.cancelAndJoin() assertEquals( listOf( TapToPaySessionState.Idle, From c0d3ab5bfaa1bccdb94507c531c4349d213c324b Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 15:28:32 -0700 Subject: [PATCH 11/18] [PLA-2184] Android - Cover the failure classifier, which decides every host-facing remedy It had no direct test and carried 69 of the 91 uncovered new lines and branches on this branch, which is what put new coverage under the gate at 79.7%. It is also the file where being wrong is least visible: a failure that lands on the wrong reason sends a host down a repair that cannot work, and every landing looks plausible from the call site. A table with one row per branch, and two invariants read back from the classifier rather than from the table: which failures ask a host to discard the device identity, and which leave the session alone. Derived from the expectations instead, those two would assert the table against itself and pass with any production mapping. Sabotage: mapping a not-found to the identity landing fails both the table and the discard invariant. The file now reports no missed lines and no missed branches. Co-Authored-By: Claude Opus 5 (1M context) --- .../session/TapToPaySessionFailuresTest.kt | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt new file mode 100644 index 00000000..d5ffc3c5 --- /dev/null +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt @@ -0,0 +1,109 @@ +package com.payabli.sdk.taptopay.session + +import com.payabli.sdk.core.model.PayabliErrorCode +import com.payabli.sdk.core.model.PayabliGenericException +import com.payabli.sdk.taptopay.attestation.AttestationException +import com.payabli.sdk.taptopay.attestation.device.DeviceServiceException +import com.payabli.sdk.taptopay.enrollment.DeviceActivationException +import com.payabli.sdk.taptopay.session.TapToPayFailureReason.ATTESTATION_REQUIRED +import com.payabli.sdk.taptopay.session.TapToPayFailureReason.CONFIGURATION_REJECTED +import com.payabli.sdk.taptopay.session.TapToPayFailureReason.INTERNAL +import com.payabli.sdk.taptopay.session.TapToPayFailureReason.SERVICE_UNAVAILABLE +import org.junit.Assert.assertEquals +import org.junit.Test + +private val REASON = "server text" + +private fun failed(reason: TapToPayFailureReason) = TapToPaySessionState.Failed(reason) + +/** + * Every failure a session can meet, and where it lands. + * + * The table is the contract: a host branches on the reason, so a failure that lands on the wrong one sends + * it down a repair that cannot work. Each row names the failure so a wrong landing says which one moved. + * + * Two properties are asserted separately below the table, because both are easy to lose in a rewrite and + * neither is visible in a single row: a landing of null leaves the session where it is, and only a failure + * that names the attestation reaches [ATTESTATION_REQUIRED], which is the one landing that tells a host to + * discard an identity. + */ +class TapToPaySessionFailuresTest { + private val cases: List> = + listOf( + TapToPaySessionException.PendingActivation() to TapToPaySessionState.PendingActivation, + TapToPaySessionException.AttestationRequired() to failed(ATTESTATION_REQUIRED), + TapToPaySessionException.NotRecoverable(TapToPaySessionState.Ready) to null, + TapToPaySessionException.SetupAbandoned() to TapToPaySessionState.Idle, + DeviceServiceException.Forbidden(403, REASON) to TapToPaySessionState.PendingActivation, + DeviceServiceException.NotAttested(401, REASON) to failed(ATTESTATION_REQUIRED), + DeviceServiceException.NotFound(404, REASON) to failed(CONFIGURATION_REJECTED), + DeviceServiceException.BadRequest(400, REASON) to failed(INTERNAL), + DeviceServiceException.ServerFailure(500, REASON) to failed(SERVICE_UNAVAILABLE), + DeviceServiceException.Undecodable(null) to failed(INTERNAL), + DeviceServiceException.Unclassified(418, REASON) to failed(SERVICE_UNAVAILABLE), + DeviceActivationException.AttestationRevoked(403, REASON) to failed(ATTESTATION_REQUIRED), + DeviceActivationException.DeviceUnknown(404, REASON) to failed(ATTESTATION_REQUIRED), + DeviceActivationException.NotEnrolled() to failed(ATTESTATION_REQUIRED), + DeviceActivationException.EntryNotAuthorized(403, REASON) to failed(CONFIGURATION_REJECTED), + DeviceActivationException.PaypointUnknown(404, REASON) to failed(CONFIGURATION_REJECTED), + DeviceActivationException.ServiceFailed(500, REASON) to failed(SERVICE_UNAVAILABLE), + // A wrong code leaves the session alone: the device still owes one. + DeviceActivationException.CodeIncorrect(400, REASON) to null, + AttestationException.Retryable(-1) to failed(SERVICE_UNAVAILABLE), + AttestationException.Throttled(-8) to failed(SERVICE_UNAVAILABLE), + AttestationException.Misconfigured(-2) to failed(CONFIGURATION_REJECTED), + AttestationException.IntegrityFailed(-3) to failed(ATTESTATION_REQUIRED), + PayabliGenericException(PayabliErrorCode.PERMISSION_DENIED, REASON) to + TapToPaySessionState.PendingActivation, + PayabliGenericException(PayabliErrorCode.INVALID_CONFIGURATION, REASON) to + failed(CONFIGURATION_REJECTED), + PayabliGenericException(PayabliErrorCode.DECODING_ERROR, REASON) to failed(INTERNAL), + PayabliGenericException(PayabliErrorCode.NETWORK_ERROR, REASON) to failed(SERVICE_UNAVAILABLE), + IllegalStateException("a defect in this SDK") to failed(INTERNAL), + ) + + @Test + fun `every failure lands where the table says`() { + for ((failure, expected) in cases) { + assertEquals( + failure.javaClass.simpleName, + expected, + TapToPaySessionFailures.landingFor(failure), + ) + } + } + + @Test + fun `only a failure naming the attestation asks a host to discard the identity`() { + // Read back from the classifier, not from the expectations above. Deriving it from the table would + // assert the table against itself and pass with any production mapping. + val discarding = + cases + .filter { TapToPaySessionFailures.landingFor(it.first) == failed(ATTESTATION_REQUIRED) } + .map { it.first.javaClass.simpleName } + .toSet() + + assertEquals( + setOf( + "AttestationRequired", + "NotAttested", + "AttestationRevoked", + "DeviceUnknown", + "NotEnrolled", + "IntegrityFailed", + ), + discarding, + ) + } + + @Test + fun `a landing of null is only for failures that change nothing about the session`() { + val unchanged = + cases + .filter { TapToPaySessionFailures.landingFor(it.first) == null } + .map { it.first.javaClass.simpleName } + .toSet() + + assertEquals(setOf("NotRecoverable", "CodeIncorrect"), unchanged) + } +} From 7c685e5a2b1707ab50361a2de1cd421ea2a29793 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 19:45:23 -0700 Subject: [PATCH 12/18] [PLA-2184] Android - Fail fast when starting over is refused `reset` dropped the result of its write. Starting over is reachable from every state, so a refusal means the table is broken, and the build that follows would run every phase from a state nobody expects while reporting success. That is the shape this type exists to prevent, left open at the one call site that opens every build. It throws now, like both `advance` overloads. `settle` and `invalidate` still log and return: one is reached from a failure path where a throw would replace the caller's failure, and the other can lose a race legitimately. Test: none added. The matrix test already asserts every state reaches `Idle`, so the invariant is guarded; this makes the call site say so rather than continue. Co-Authored-By: Claude Opus 5 (1M context) --- .../payabli/sdk/taptopay/session/TapToPaySessionManager.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt index 68587fd9..563f638a 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionManager.kt @@ -85,9 +85,13 @@ internal class TapToPaySessionManager( * Puts the session back to the start. * * The first act of building a session, whatever the caller left behind, since the table is narrow. + * + * Throws on a refusal, like the two [advance] overloads and for the same reason: starting over is + * reachable from every state, so a refusal here is a broken table and the build that follows would run + * every phase from a state nobody expects. */ fun reset() { - write(TapToPaySessionState.Idle) + writeOrThrow(TapToPaySessionState.Idle) } /** From c3a09bf68a7b357dc27de0964efa2a011189d0c3 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Fri, 14 Aug 2026 19:45:30 -0700 Subject: [PATCH 13/18] [PLA-2184] Android - Name the shape an entry point must have when one is refused The refusal said an entry must be usable as a single path segment without saying what that allows, so a caller reading it learns only that their value is wrong. It now names the character set, which is safe to state and is what the caller needs. The value stays out of the message: an entry point identifies a merchant. Matches how the activation code states its own shape. Co-Authored-By: Claude Opus 5 (1M context) --- .../sdk/taptopay/attestation/device/DeviceServiceClient.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt index 9f41adbe..9844abfe 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/attestation/device/DeviceServiceClient.kt @@ -254,7 +254,9 @@ internal class DeviceServiceClient( * merchant. */ private fun pathSegment(entry: String): String { - require(PATH_SEGMENT.matches(entry)) { "entry must be usable as a single path segment" } + require(PATH_SEGMENT.matches(entry)) { + "entry must be one path segment of unreserved characters: A-Z a-z 0-9 and . _ ~ -" + } return entry } From 6e05ec0c0b7d9092bcfbfed96c0b88db762b79c1 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Sat, 15 Aug 2026 08:11:29 -0700 Subject: [PATCH 14/18] [PLA-2184] Android - Separate the representative failure from the failure type it instantiates `FAILED` and `Failed` differ only in case and appeared on the same line of the table, where one is the state type and the other is the one instance the rows use. The naming rule here forbids two names a reader can confuse. `FAILED_INTERNAL` also says which instance it is, which the table depends on: the state is a data class, so equality includes the reason. Co-Authored-By: Claude Opus 5 (1M context) --- .../session/TapToPayTransitionMatrixTest.kt | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt index c89f76fe..4fafe483 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt @@ -23,18 +23,18 @@ import org.junit.Test */ private fun legalTargetsFrom(from: TapToPaySessionState): Set = when (from) { - Idle -> setOf(Idle, AttestingDevice, FetchingConfig, FAILED) - AttestingDevice -> setOf(Idle, AttestingDevice, FetchingConfig, PendingActivation, FAILED) - FetchingConfig -> setOf(Idle, FetchingConfig, InitializingReader, PendingActivation, FAILED) - InitializingReader -> setOf(Idle, InitializingReader, Ready, FAILED) - Ready -> setOf(Idle, Ready, SessionExpired, FAILED) - SessionExpired -> setOf(Idle, SessionExpired, Reinitializing, FAILED) - Reinitializing -> setOf(Idle, Reinitializing, FetchingConfig, FAILED) - PendingActivation -> setOf(Idle, PendingActivation, AttestingDevice, FAILED) - is Failed -> setOf(Idle, AttestingDevice, FetchingConfig, FAILED) + Idle -> setOf(Idle, AttestingDevice, FetchingConfig, FAILED_INTERNAL) + AttestingDevice -> setOf(Idle, AttestingDevice, FetchingConfig, PendingActivation, FAILED_INTERNAL) + FetchingConfig -> setOf(Idle, FetchingConfig, InitializingReader, PendingActivation, FAILED_INTERNAL) + InitializingReader -> setOf(Idle, InitializingReader, Ready, FAILED_INTERNAL) + Ready -> setOf(Idle, Ready, SessionExpired, FAILED_INTERNAL) + SessionExpired -> setOf(Idle, SessionExpired, Reinitializing, FAILED_INTERNAL) + Reinitializing -> setOf(Idle, Reinitializing, FetchingConfig, FAILED_INTERNAL) + PendingActivation -> setOf(Idle, PendingActivation, AttestingDevice, FAILED_INTERNAL) + is Failed -> setOf(Idle, AttestingDevice, FetchingConfig, FAILED_INTERNAL) } -private val FAILED = Failed(TapToPayFailureReason.INTERNAL) +private val FAILED_INTERNAL = Failed(TapToPayFailureReason.INTERNAL) class TapToPayTransitionMatrixTest { @Test @@ -67,7 +67,7 @@ class TapToPayTransitionMatrixTest { @Test fun `failing is reachable from every state`() { for (from in EVERY_SESSION_STATE) { - assertEquals(from.diagnosticName, true, TapToPaySessionTransitions.permits(from, FAILED)) + assertEquals(from.diagnosticName, true, TapToPaySessionTransitions.permits(from, FAILED_INTERNAL)) } } From 8f5e620b004ebb299a8f1938446370c9f5116953 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Sat, 15 Aug 2026 08:15:25 -0700 Subject: [PATCH 15/18] [PLA-2184] Android - Say which side of the wire an internal failure is on `INTERNAL` was an adjective with no noun, and the ambiguity ran the wrong way: an HTTP 500 is called an internal server error and lands on `SERVICE_UNAVAILABLE`, so a reader meeting `INTERNAL` had to guess which side it meant, and the wrong guess picks the wrong remedy. Its three siblings are conditions on their own and carry no suffix. This one is not, which is the same split the core error codes already make: `SERVER_ERROR` and `DECODING_ERROR` carry it, `PERMISSION_DENIED` and `RATE_LIMITED` do not. Co-Authored-By: Claude Opus 5 (1M context) --- .../sdk/taptopay/session/TapToPayFailureReason.kt | 5 ++++- .../sdk/taptopay/session/TapToPaySessionFailures.kt | 12 ++++++------ .../payabli/sdk/taptopay/session/SessionFixture.kt | 2 +- .../taptopay/session/TapToPaySessionFailuresTest.kt | 10 +++++----- .../taptopay/session/TapToPayTransitionMatrixTest.kt | 2 +- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt index 0c437c74..a162dfd6 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPayFailureReason.kt @@ -32,8 +32,11 @@ internal enum class TapToPayFailureReason { /** * The SDK and the service disagree about the contract, or the SDK has a defect. * + * This side of the wire. A failure inside the service is [SERVICE_UNAVAILABLE], which is why the name + * says which side: an HTTP 500 is called an internal server error and lands there, not here. + * * A response that could not be decoded is the common one. A host cannot act on it; it is here so that it * is not silently filed under one of the others. */ - INTERNAL, + SDK_INTERNAL_ERROR, } diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt index cbeaa3c1..3a1d11fe 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt @@ -7,7 +7,7 @@ import com.payabli.sdk.taptopay.attestation.device.DeviceServiceException import com.payabli.sdk.taptopay.enrollment.DeviceActivationException import com.payabli.sdk.taptopay.session.TapToPayFailureReason.ATTESTATION_REQUIRED import com.payabli.sdk.taptopay.session.TapToPayFailureReason.CONFIGURATION_REJECTED -import com.payabli.sdk.taptopay.session.TapToPayFailureReason.INTERNAL +import com.payabli.sdk.taptopay.session.TapToPayFailureReason.SDK_INTERNAL_ERROR import com.payabli.sdk.taptopay.session.TapToPayFailureReason.SERVICE_UNAVAILABLE /** @@ -16,7 +16,7 @@ import com.payabli.sdk.taptopay.session.TapToPayFailureReason.SERVICE_UNAVAILABL * One place, so every phase of every entry point ends the same way. * * **A landing is a remedy.** Two failures a host repairs identically share a member of - * [TapToPayFailureReason], and a failure whose remedy is unknown is [INTERNAL]: a guess sends a host down a + * [TapToPayFailureReason], and a failure whose remedy is unknown is [SDK_INTERNAL_ERROR]: a guess sends a host down a * repair that cannot work. * * **Discarding the device's identity requires a positive match.** Only a refusal that names the attestation @@ -39,7 +39,7 @@ internal object TapToPaySessionFailures { is DeviceActivationException -> landingForActivation(failure) is AttestationException -> landingForAttestation(failure) is PayabliException -> landingForTransport(failure) - else -> failed(INTERNAL) + else -> failed(SDK_INTERNAL_ERROR) } /** @@ -55,9 +55,9 @@ internal object TapToPaySessionFailures { is DeviceServiceException.NotAttested -> failed(ATTESTATION_REQUIRED) is DeviceServiceException.NotFound -> failed(CONFIGURATION_REJECTED) // The request this SDK built was refused, which makes it this SDK's defect. - is DeviceServiceException.BadRequest -> failed(INTERNAL) + is DeviceServiceException.BadRequest -> failed(SDK_INTERNAL_ERROR) is DeviceServiceException.ServerFailure -> failed(SERVICE_UNAVAILABLE) - is DeviceServiceException.Undecodable -> failed(INTERNAL) + is DeviceServiceException.Undecodable -> failed(SDK_INTERNAL_ERROR) is DeviceServiceException.Unclassified -> failed(SERVICE_UNAVAILABLE) } @@ -96,7 +96,7 @@ internal object TapToPaySessionFailures { when (failure.code) { PayabliErrorCode.PERMISSION_DENIED -> TapToPaySessionState.PendingActivation PayabliErrorCode.INVALID_CONFIGURATION -> failed(CONFIGURATION_REJECTED) - PayabliErrorCode.DECODING_ERROR -> failed(INTERNAL) + PayabliErrorCode.DECODING_ERROR -> failed(SDK_INTERNAL_ERROR) else -> failed(SERVICE_UNAVAILABLE) } diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt index b3af92e9..62d1e7a0 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionFixture.kt @@ -30,7 +30,7 @@ internal val EVERY_SESSION_STATE: List = TapToPaySessionState.SessionExpired, TapToPaySessionState.Reinitializing, TapToPaySessionState.PendingActivation, - TapToPaySessionState.Failed(TapToPayFailureReason.INTERNAL), + TapToPaySessionState.Failed(TapToPayFailureReason.SDK_INTERNAL_ERROR), ) /** Bounds one await, so a stranded claim reports what was stranded. */ diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt index d5ffc3c5..2f6c4e05 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt @@ -7,7 +7,7 @@ import com.payabli.sdk.taptopay.attestation.device.DeviceServiceException import com.payabli.sdk.taptopay.enrollment.DeviceActivationException import com.payabli.sdk.taptopay.session.TapToPayFailureReason.ATTESTATION_REQUIRED import com.payabli.sdk.taptopay.session.TapToPayFailureReason.CONFIGURATION_REJECTED -import com.payabli.sdk.taptopay.session.TapToPayFailureReason.INTERNAL +import com.payabli.sdk.taptopay.session.TapToPayFailureReason.SDK_INTERNAL_ERROR import com.payabli.sdk.taptopay.session.TapToPayFailureReason.SERVICE_UNAVAILABLE import org.junit.Assert.assertEquals import org.junit.Test @@ -37,9 +37,9 @@ class TapToPaySessionFailuresTest { DeviceServiceException.Forbidden(403, REASON) to TapToPaySessionState.PendingActivation, DeviceServiceException.NotAttested(401, REASON) to failed(ATTESTATION_REQUIRED), DeviceServiceException.NotFound(404, REASON) to failed(CONFIGURATION_REJECTED), - DeviceServiceException.BadRequest(400, REASON) to failed(INTERNAL), + DeviceServiceException.BadRequest(400, REASON) to failed(SDK_INTERNAL_ERROR), DeviceServiceException.ServerFailure(500, REASON) to failed(SERVICE_UNAVAILABLE), - DeviceServiceException.Undecodable(null) to failed(INTERNAL), + DeviceServiceException.Undecodable(null) to failed(SDK_INTERNAL_ERROR), DeviceServiceException.Unclassified(418, REASON) to failed(SERVICE_UNAVAILABLE), DeviceActivationException.AttestationRevoked(403, REASON) to failed(ATTESTATION_REQUIRED), DeviceActivationException.DeviceUnknown(404, REASON) to failed(ATTESTATION_REQUIRED), @@ -57,9 +57,9 @@ class TapToPaySessionFailuresTest { TapToPaySessionState.PendingActivation, PayabliGenericException(PayabliErrorCode.INVALID_CONFIGURATION, REASON) to failed(CONFIGURATION_REJECTED), - PayabliGenericException(PayabliErrorCode.DECODING_ERROR, REASON) to failed(INTERNAL), + PayabliGenericException(PayabliErrorCode.DECODING_ERROR, REASON) to failed(SDK_INTERNAL_ERROR), PayabliGenericException(PayabliErrorCode.NETWORK_ERROR, REASON) to failed(SERVICE_UNAVAILABLE), - IllegalStateException("a defect in this SDK") to failed(INTERNAL), + IllegalStateException("a defect in this SDK") to failed(SDK_INTERNAL_ERROR), ) @Test diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt index 4fafe483..909a1a81 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPayTransitionMatrixTest.kt @@ -34,7 +34,7 @@ private fun legalTargetsFrom(from: TapToPaySessionState): Set setOf(Idle, AttestingDevice, FetchingConfig, FAILED_INTERNAL) } -private val FAILED_INTERNAL = Failed(TapToPayFailureReason.INTERNAL) +private val FAILED_INTERNAL = Failed(TapToPayFailureReason.SDK_INTERNAL_ERROR) class TapToPayTransitionMatrixTest { @Test From db2081da4ab57166a6316f2ac37d04c502552021 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Sat, 15 Aug 2026 08:16:46 -0700 Subject: [PATCH 16/18] [PLA-2184] Android - Land a lost device key on attesting again, not on an SDK defect `DeviceEnrollment.assertion()` rethrows `DeviceKeyException.KeyLost` after discarding the record, and fetching the credentials calls it on every build and every repair. The classifier had no branch for that type, so a device whose Keystore key was wiped was told the SDK had an internal defect, which is the wrong remedy for the one condition attesting again fixes. All three key failures are mapped rather than the one reported, so a fourth fails to compile here. Only the lost key discards an identity: a signature that failed and a platform without crypto leave the key where it was, so neither is the positive match this landing requires. Swept the other exception reaching this classifier from the coordinator. `SecureStorageException` also falls to the default, and lands where it should: storage the device cannot use is not something a host repairs. Sabotage: mapping the lost key to the SDK-defect landing fails the table and the discard invariant, and nothing else. Co-Authored-By: Claude Opus 5 (1M context) --- .../taptopay/session/TapToPaySessionFailures.kt | 16 ++++++++++++++++ .../session/TapToPaySessionFailuresTest.kt | 6 ++++++ 2 files changed, 22 insertions(+) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt index 3a1d11fe..dbf961e3 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt @@ -1,5 +1,6 @@ package com.payabli.sdk.taptopay.session +import com.payabli.sdk.core.devicekey.DeviceKeyException import com.payabli.sdk.core.model.PayabliErrorCode import com.payabli.sdk.core.model.PayabliException import com.payabli.sdk.taptopay.attestation.AttestationException @@ -38,6 +39,7 @@ internal object TapToPaySessionFailures { is DeviceServiceException -> landingForService(failure) is DeviceActivationException -> landingForActivation(failure) is AttestationException -> landingForAttestation(failure) + is DeviceKeyException -> landingForDeviceKey(failure) is PayabliException -> landingForTransport(failure) else -> failed(SDK_INTERNAL_ERROR) } @@ -92,6 +94,20 @@ internal object TapToPaySessionFailures { else -> failed(ATTESTATION_REQUIRED) } + /** + * A key that is gone is the identity being gone, which is a positive match: enrollment discards the + * record before raising it, so the remedy is to attest again. + * + * The other two are not. A signature that failed and a platform that cannot do crypto both leave the + * key where it was, so neither says the identity is stale. + */ + private fun landingForDeviceKey(failure: DeviceKeyException): TapToPaySessionState = + when (failure) { + is DeviceKeyException.KeyLost -> failed(ATTESTATION_REQUIRED) + is DeviceKeyException.SigningFailed -> failed(SDK_INTERNAL_ERROR) + is DeviceKeyException.CryptoUnavailable -> failed(SDK_INTERNAL_ERROR) + } + private fun landingForTransport(failure: PayabliException): TapToPaySessionState = when (failure.code) { PayabliErrorCode.PERMISSION_DENIED -> TapToPaySessionState.PendingActivation diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt index 2f6c4e05..2762dabd 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt @@ -1,5 +1,6 @@ package com.payabli.sdk.taptopay.session +import com.payabli.sdk.core.devicekey.DeviceKeyException import com.payabli.sdk.core.model.PayabliErrorCode import com.payabli.sdk.core.model.PayabliGenericException import com.payabli.sdk.taptopay.attestation.AttestationException @@ -49,6 +50,10 @@ class TapToPaySessionFailuresTest { DeviceActivationException.ServiceFailed(500, REASON) to failed(SERVICE_UNAVAILABLE), // A wrong code leaves the session alone: the device still owes one. DeviceActivationException.CodeIncorrect(400, REASON) to null, + // The key is gone, so the identity is: enrollment discards the record before raising it. + DeviceKeyException.KeyLost() to failed(ATTESTATION_REQUIRED), + DeviceKeyException.SigningFailed() to failed(SDK_INTERNAL_ERROR), + DeviceKeyException.CryptoUnavailable() to failed(SDK_INTERNAL_ERROR), AttestationException.Retryable(-1) to failed(SERVICE_UNAVAILABLE), AttestationException.Throttled(-8) to failed(SERVICE_UNAVAILABLE), AttestationException.Misconfigured(-2) to failed(CONFIGURATION_REJECTED), @@ -91,6 +96,7 @@ class TapToPaySessionFailuresTest { "DeviceUnknown", "NotEnrolled", "IntegrityFailed", + "KeyLost", ), discarding, ) From 33139eff4c3c6540f46306d022db45bb28962885 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Sat, 15 Aug 2026 12:33:51 -0700 Subject: [PATCH 17/18] [PLA-2184] Android - Tell a joiner the run failed when the owner died on something unclassified The fatal path handed waiters `SetupAbandoned`, which says nothing happened and asking again is safe. After an `OutOfMemoryError` neither is true, so a host following that answer retries into a process that is already going down. `SetupFailed` says the run failed and lands on the SDK-internal reason. It carries no cause: the owner keeps the original and it reaches that caller unchanged, while attaching it here would give every waiter a reference to whatever died. The token refresh in the core module draws the same line at the same place, for the same reason. Sabotage: handing the withdrawal outcome back fails the new test and nothing else. Co-Authored-By: Claude Opus 5 (1M context) --- .../session/TapToPaySessionCoordinator.kt | 6 ++- .../session/TapToPaySessionException.kt | 11 +++++ .../session/TapToPaySessionFailures.kt | 1 + .../session/SessionSerializationTest.kt | 47 +++++++++++++++++++ .../session/TapToPaySessionFailuresTest.kt | 1 + 5 files changed, 64 insertions(+), 2 deletions(-) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt index 309466c8..faf761df 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionCoordinator.kt @@ -147,8 +147,10 @@ internal class TapToPaySessionCoordinator( throw failure } catch (fatal: Throwable) { // An OutOfMemoryError reaches the caller unchanged. The claim is still released, or every later - // caller of this kind waits for something that will never complete. - release(claim, TapToPaySessionException.SetupAbandoned()) + // caller of this kind waits for something that will never complete. Waiters are told the run + // failed rather than that it withdrew, and the cause stays with the owner: handing it on would + // give every waiter a reference to whatever died. + release(claim, TapToPaySessionException.SetupFailed()) throw fatal } release(claim, null) diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt index 6aae708a..5885de2b 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionException.kt @@ -45,4 +45,15 @@ internal sealed class TapToPaySessionException( * waiter's own scope look like it is unwinding. Nothing is left half-applied, and asking again is safe. */ class SetupAbandoned : TapToPaySessionException("the caller that owned this session setup withdrew") + + /** + * The caller that owned this work died on something this SDK does not classify. + * + * Separate from [SetupAbandoned] because that one says nothing happened and asking again is safe, and + * after an `OutOfMemoryError` neither is true. + * + * It carries no cause. The owner keeps the original and it reaches that caller unchanged; attaching it + * here would hand every waiter a reference to whatever died. + */ + class SetupFailed : TapToPaySessionException("the caller that owned this session setup failed") } diff --git a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt index dbf961e3..6456cf57 100644 --- a/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt +++ b/taptopay/src/main/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailures.kt @@ -36,6 +36,7 @@ internal object TapToPaySessionFailures { is TapToPaySessionException.AttestationRequired -> failed(ATTESTATION_REQUIRED) is TapToPaySessionException.NotRecoverable -> null is TapToPaySessionException.SetupAbandoned -> TapToPaySessionState.Idle + is TapToPaySessionException.SetupFailed -> failed(SDK_INTERNAL_ERROR) is DeviceServiceException -> landingForService(failure) is DeviceActivationException -> landingForActivation(failure) is AttestationException -> landingForAttestation(failure) diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt index 1e047e13..33ac46fc 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/SessionSerializationTest.kt @@ -25,6 +25,9 @@ private val BLOCKED_PROBE = 300.milliseconds /** The wall-clock ceiling once the gate is open. Generous, because this runs on shared CI hardware. */ private val COMPLETION_PROBE = 30.seconds +/** An `Error`, so the coordinator's fatal path is reached without an `OutOfMemoryError` in a test. */ +private class FatalTestError : Error() + /** * That the three entry points never overlap, and that two of the same kind share one run. * @@ -151,6 +154,50 @@ class SessionSerializationTest { assertEquals(TapToPaySessionState.Ready, fixture.state) } + @Test + fun `a joiner is told the owner failed when the owner died on something unclassified`() = + runTest(timeout = TEST_TIMEOUT) { + val held = CompletableDeferred() + val fixture = + SessionFixture( + SessionFixture.coldScript(), + firstReadGate = { + held.await() + throw FatalTestError() + }, + ) + + var ownerFailure: Throwable? = null + val owner = + launch(UnconfinedTestDispatcher(testScheduler)) { + try { + fixture.coordinator.initialize() + } catch (fatal: Throwable) { + ownerFailure = fatal + } + } + var joinerFailure: Throwable? = null + val joiner = + launch(UnconfinedTestDispatcher(testScheduler)) { + try { + fixture.coordinator.initialize() + } catch (failure: TapToPaySessionException) { + joinerFailure = failure + } + } + + held.complete(Unit) + completing("the owner") { owner.join() } + completing("the joining build") { joiner.join() } + + assertTrue("the owner keeps what killed it", ownerFailure is FatalTestError) + assertTrue( + "a joiner is told the run failed, not that it withdrew", + joinerFailure is TapToPaySessionException.SetupFailed, + ) + assertNull("the cause stays with the owner", joinerFailure?.cause) + } + /** * The same exclusion, on real threads and a real clock. * diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt index 2762dabd..5737b433 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/session/TapToPaySessionFailuresTest.kt @@ -35,6 +35,7 @@ class TapToPaySessionFailuresTest { TapToPaySessionException.AttestationRequired() to failed(ATTESTATION_REQUIRED), TapToPaySessionException.NotRecoverable(TapToPaySessionState.Ready) to null, TapToPaySessionException.SetupAbandoned() to TapToPaySessionState.Idle, + TapToPaySessionException.SetupFailed() to failed(SDK_INTERNAL_ERROR), DeviceServiceException.Forbidden(403, REASON) to TapToPaySessionState.PendingActivation, DeviceServiceException.NotAttested(401, REASON) to failed(ATTESTATION_REQUIRED), DeviceServiceException.NotFound(404, REASON) to failed(CONFIGURATION_REJECTED), From 8d83ef43c38c33bb2268afa585e9d44705f81263 Mon Sep 17 00:00:00 2001 From: Alex Arguello Date: Sat, 15 Aug 2026 14:35:53 -0700 Subject: [PATCH 18/18] [PLA-2184] Android - Say path where the script means path The script matches on what the client sent and called it a route throughout: the local holding `request.path`, the class prose, and the constant. That was harmless while every path equalled its own template, and stopped being harmless when `/config` arrived with an identifier in its path and a template beside it. The local is `path`, the prose says which of the two it keys on, and the config constant says it is resolved. The accessor keeps its name, since renaming it reaches every test on the branch; its documentation now says what it holds. Co-Authored-By: Claude Opus 5 (1M context) --- .../taptopay/enrollment/EnrollmentFixture.kt | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/taptopay/src/test/java/com/payabli/sdk/taptopay/enrollment/EnrollmentFixture.kt b/taptopay/src/test/java/com/payabli/sdk/taptopay/enrollment/EnrollmentFixture.kt index 307f8880..9133d4ae 100644 --- a/taptopay/src/test/java/com/payabli/sdk/taptopay/enrollment/EnrollmentFixture.kt +++ b/taptopay/src/test/java/com/payabli/sdk/taptopay/enrollment/EnrollmentFixture.kt @@ -57,11 +57,16 @@ internal fun configBody(): String = ) /** - * Answers each route from a script, and fails loudly on anything unscripted. + * Answers each request from a script, keyed on the path it was sent to, and fails loudly on anything + * unscripted. * - * `error` on a miss, the discipline the client's own tests state: an - * unscripted route, or one call more than the script answers, has to fail by name here instead of replaying - * the previous answer somewhere no assertion can see it. + * **Paths, not route templates.** A script answers what the client sent, and `/config` resolves its + * `{entry}` before sending. The templates are the transport's recordable form and are asserted separately, + * in `DeviceServiceConfigTest`, which pins that a resolved path names a paypoint and a template does not. + * + * `error` on a miss, the discipline the client's own tests state: an unscripted path, or one call more than + * the script answers, has to fail by name here instead of replaying the previous answer somewhere no + * assertion can see it. */ internal class RouteScript( private vararg val answers: Pair>, @@ -71,14 +76,14 @@ internal class RouteScript( private val taken = mutableMapOf() fun respond(request: PayabliRequest): PayabliResponse { - val route = request.path + val path = request.path val queued = - answers.firstOrNull { it.first == route }?.second - ?: error("no answer scripted for $route") - val index = taken.getOrDefault(route, 0) - if (index >= queued.size) error("$route was called ${index + 1} times, ${queued.size} answers scripted") - taken[route] = index + 1 - return PayabliResponse(statusFor(route), body = queued[index].toByteArray(Charsets.UTF_8)) + answers.firstOrNull { it.first == path }?.second + ?: error("no answer scripted for $path") + val index = taken.getOrDefault(path, 0) + if (index >= queued.size) error("$path was called ${index + 1} times, ${queued.size} answers scripted") + taken[path] = index + 1 + return PayabliResponse(statusFor(path), body = queued[index].toByteArray(Charsets.UTF_8)) } companion object { @@ -87,6 +92,7 @@ internal class RouteScript( const val ATTEST = "/api/v2/device/taptopay/attest" const val ACTIVATE = "/api/v2/device/taptopay/activate" + /** Resolved, since that is what the client sends. The four above resolve to themselves. */ const val CONFIG = "/api/v2/device/taptopay/config/$ENTRY" } } @@ -162,7 +168,11 @@ internal class EnrollmentFixture( PayabliJson.format.decodeFromString(AttestedDevice.serializer(), it.decodeToString()) } - /** Only the transport's half of the trace, for asserting call order alone. */ + /** + * Only the transport's half of the trace, for asserting call order alone. + * + * Paths, as sent. `/config` appears with its `{entry}` resolved. + */ val routes: List get() = trace.filter { it.startsWith("/api/") } companion object {