diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/WaveLogApi.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/WaveLogApi.kt index cef7bbe3..63e9cfc8 100644 --- a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/WaveLogApi.kt +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/WaveLogApi.kt @@ -242,13 +242,14 @@ object WaveLogApi { // QSO arrives as HTTP 200 with {"status":"failed"}. Trusting the code marked it uploaded // and dropped it from the queue. val (code, resp) = httpRequest("$base/index.php/api/v2/qso", "POST", apiKey, v2Body.toString()) - when (val verdict = WavelogResponse.verdict(code, resp)) { + val v2Verdict = WavelogResponse.verdict(code, resp) + when (v2Verdict) { is WavelogResponse.Verdict.Accepted -> return@withContext WavelogResult.Success("v2") WavelogResponse.Verdict.Duplicate -> return@withContext WavelogResult.Success("duplicate") - is WavelogResponse.Verdict.Rejected -> - return@withContext WavelogResult.Failure(verdict.reason) - // Unreadable falls through to v1: an older server may not have the v2 endpoint at all. - is WavelogResponse.Verdict.Unreadable -> Unit + // Anything else falls through to v1. A rejection here is NOT final: v2 refuses a legacy + // v1 key with 401 invalid_token, and returning at that point stopped a v1-only operator + // from uploading at all. The v1 attempt below is the one that can speak for them. + else -> Unit } // v1: POST /index.php/api/qso (key in body + ADIF) @@ -259,12 +260,13 @@ object WaveLogApi { put("string", toAdif(qso, gridsquare, satName)) } val (code1, resp1) = httpRequest("$base/index.php/api/qso", "POST", apiKey, v1Body.toString()) - when (val verdict = WavelogResponse.verdict(code1, resp1)) { + val v1Verdict = WavelogResponse.verdict(code1, resp1) + when (v1Verdict) { is WavelogResponse.Verdict.Accepted -> return@withContext WavelogResult.Success("v1") WavelogResponse.Verdict.Duplicate -> return@withContext WavelogResult.Success("duplicate") - is WavelogResponse.Verdict.Rejected -> - return@withContext WavelogResult.Failure(verdict.reason) - is WavelogResponse.Verdict.Unreadable -> Unit + // Also falls through: a server with different rewrite rules answers this path with a + // 404 page, which is a rejection but says nothing about whether the QSO can be stored. + else -> Unit } // v1 without index.php, for a server whose rewrite rules differ @@ -279,8 +281,17 @@ object WaveLogApi { // Every endpoint answered something we could not read. Keeping the QSO queued is the only // honest outcome: it may have been stored, and dropping it would lose the contact. + // No endpoint accepted it. The v2 reason is preferred when it explained itself, since a 401 + // invalid_token is the most actionable thing an operator can be told; otherwise all three + // status codes go out, because the third was previously dropped from this message. + val reasons = listOfNotNull( + (v1Verdict as? WavelogResponse.Verdict.Rejected)?.reason, + (v2Verdict as? WavelogResponse.Verdict.Rejected)?.reason + ).filter { it.isNotBlank() } WavelogResult.Failure( - "unreadable response: v2 HTTP $code, v1 HTTP $code1 - ${shortError(resp1.ifBlank { resp1b })}" + reasons.firstOrNull() + ?: ("no endpoint accepted it: v2 HTTP $code, v1 HTTP $code1, v1-alt HTTP $code1b" + + " - " + shortError(resp1.ifBlank { resp1b })) ) } diff --git a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/WavelogResponse.kt b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/WavelogResponse.kt index aa6af9a2..1296f1a8 100644 --- a/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/WavelogResponse.kt +++ b/core/domain/src/main/java/com/rtbishop/look4sat/core/domain/wavelog/WavelogResponse.kt @@ -61,17 +61,27 @@ object WavelogResponse { val lower = text.lowercase().replace(AROUND_SEPARATORS, "") // Duplicate reported three ways: a 409, the word in a message, or Wavelog's own // `{"status":"dupe"}` - which arrives with HTTP 200 and is documented, not guessed. - if (statusCode == DUPLICATE_CODE || - DUPLICATE_MARKERS.any { lower.contains(it) } - ) { - return Verdict.Duplicate - } + // An HTML body is a proxy or a PHP fatal, never a verdict. Checked first because such a + // page carries no status token and would otherwise read as an acceptance, so a + // misconfigured reverse proxy answering 200 would eat contacts. + if (lower.startsWith("<")) return Verdict.Unreadable(text.take(MAX_DETAIL)) + + // Duplicate only when the STATUS says so, or on a 409. The word alone is not enough: the + // server's own rejection text is "Duplicate for ", and Api_v2 surfaces that inside + // validation_error bodies - so matching the bare substring classified a hard rejection as + // a duplicate, which maps to success and drops the QSO from the queue. That is the very + // defect this class was written to prevent, reached through a different door. + if (statusCode == DUPLICATE_CODE || lower.contains(DUPE_STATUS)) return Verdict.Duplicate if (statusCode !in SUCCESS_CODES) { return Verdict.Rejected(reasonFrom(text).ifBlank { "HTTP $statusCode" }) } if (FAILURE_MARKERS.any { lower.contains(it) }) { return Verdict.Rejected(reasonFrom(text).ifBlank { "server reported failure" }) } + // The v2 endpoint reports errors in an envelope with no status key at all. + if (lower.contains("\"error\":")) { + return Verdict.Rejected(reasonFrom(text).ifBlank { "server reported an error" }) + } // A status field we cannot read is not a success. Saying so keeps the QSO queued. if (lower.contains("\"status\"") && SUCCESS_MARKERS.none { lower.contains(it) }) { return Verdict.Unreadable(text.take(MAX_DETAIL)) @@ -115,15 +125,23 @@ object WavelogResponse { * `successful` matters as much as `success`: the API reference uses both, and treating one as * unrecognised would leave a stored QSO queued forever. */ + /** + * Success wordings, taken from the Wavelog server source rather than its documentation. + * + * The QSO endpoint answers `created`; `success` and `successful` come from other endpoints and + * are kept because a future version may use them. `abort` is NOT here - v1 uses it when any + * record in a batch failed, and it arrives with a 400. + */ private val SUCCESS_MARKERS = listOf( + "\"status\":\"created\"", "\"status\":\"success\"", "\"status\":\"successful\"", - "\"status\":\"created\"", "\"status\":\"ok\"" ) /** `dupe` is Wavelog's own wording and arrives with a 200. */ - private val DUPLICATE_MARKERS = listOf("\"status\":\"dupe\"", "duplicate") + /** Wavelog's own duplicate status. The bare word "duplicate" is deliberately NOT a marker. */ + private const val DUPE_STATUS = "\"status\":\"dupe\"" /** Whitespace next to a colon or comma, which JSON allows and servers use inconsistently. */ private val AROUND_SEPARATORS = Regex("""\s*(?=[:,])|(?<=[:,])\s*""") diff --git a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/WavelogResponseTest.kt b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/WavelogResponseTest.kt index db11bb8c..b5a9ece7 100644 --- a/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/WavelogResponseTest.kt +++ b/core/domain/src/test/java/com/rtbishop/look4sat/core/domain/wavelog/WavelogResponseTest.kt @@ -134,6 +134,92 @@ class WavelogResponseTest { ) } + /** + * The defect that lost contacts. Wavelog's own rejection text is "Duplicate for ", and + * Api_v2 surfaces that inside validation_error bodies - so matching the bare word classified a + * hard rejection as a duplicate, which maps to success and drops the QSO from the queue. + * + * Bodies transcribed from the Wavelog server source, not from its documentation. + */ + @Test + fun `the word duplicate in a rejection does not make it a duplicate`() { + val bodies = listOf( + 400 to """{"error":{"code":"validation_error","message":"duplicate submode is invalid"}}""", + 400 to """{"status":"failed","reason":"duplicate key violation; QSO NOT stored"}""", + 200 to """{"status":"failed","reason":"Duplicate for BG7NTA"}""" + ) + for ((code, body) in bodies) { + val verdict = WavelogResponse.verdict(code, body) + assertTrue( + "must be a rejection, got " + verdict + " for " + body, + verdict is WavelogResponse.Verdict.Rejected + ) + } + } + + /** A success that mentions the word must still be a success. */ + @Test + fun `the word duplicate in a success does not make it a failure`() { + val verdict = WavelogResponse.verdict( + 201, + """{"status":"created","adif_errors":0,"messages":["Removed duplicate mode entry"]}""" + ) + assertTrue("got " + verdict, verdict is WavelogResponse.Verdict.Accepted) + } + + /** + * A reverse proxy or a PHP fatal answers 200 with HTML. There is no status token, so it used to + * read as a stored QSO and a misconfigured proxy would eat contacts silently. + */ + @Test + fun `an html body is never an acceptance`() { + val pages = listOf( + "502 Bad Gateway", + "

Maintenance

", + "
Fatal error: Uncaught Error in /var/www/index.php" + ) + for (page in pages) { + val verdict = WavelogResponse.verdict(200, page) + assertTrue( + "html must not be accepted, got " + verdict, + verdict is WavelogResponse.Verdict.Unreadable + ) + } + } + + /** The v2 endpoint reports errors in an envelope with no status key at all. */ + @Test + fun `a v2 error envelope is a rejection`() { + val verdict = WavelogResponse.verdict( + 401, + """{"error":{"code":"invalid_token","message":"Bearer token must start with wl2_"}}""" + ) + assertEquals( + WavelogResponse.Verdict.Rejected("Bearer token must start with wl2_"), + verdict + ) + } + + /** The QSO endpoint answers `created`, which is what the v1 source actually emits. */ + @Test + fun `the created status the qso endpoint returns is accepted`() { + val verdict = WavelogResponse.verdict( + 201, + """{"status":"created","adif_count":1,"adif_errors":0,"messages":[""]}""" + ) + assertTrue("got " + verdict, verdict is WavelogResponse.Verdict.Accepted) + } + + /** v1 uses `abort` with a 400 when any record in a batch failed. Not a success. */ + @Test + fun `an abort status is a rejection`() { + val verdict = WavelogResponse.verdict( + 400, + """{"status":"abort","messages":["Bad ADIF field CALL"]}""" + ) + assertTrue("got " + verdict, verdict is WavelogResponse.Verdict.Rejected) + } + /** A failure with no explanation still has to say something usable. */ @Test fun `a failure without a reason still reports one`() {