Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -143,29 +143,15 @@ class AwsQldbLedger(private val properties: LedgerProperties) : IImmutableLedger
override fun getHistory(recordId: UUID): List<LedgerEntry> {
val history = mutableListOf<LedgerEntry>()
driver.execute { txn: TransactionExecutor ->
// Project metadata.id (QLDB document ID) and metadata.txTime (commit timestamp)
// alongside h.data.* so we have both fallback sources for entryId and timestamp.
// New documents store `entryId` and `createdAt` as explicit fields (preferred).
// The metaId/txTime projections provide a fallback for any documents inserted
// before this schema was in place, ensuring backward compatibility.
// Project txTime (QLDB commit timestamp) alongside h.data.* for accurate timestamp resolution.
// Documents store `entryId` and `createdAt` as explicit fields.
val result = txn.execute(
"SELECT h.metadata.id AS metaId, h.metadata.txTime AS txTime, h.data.* " +
"SELECT h.metadata.txTime AS txTime, h.data.* " +
"FROM history(FactLedger) AS h WHERE h.data.factId = ?",
ionSystem.newString(recordId.toString())
)
result.forEach { doc ->
val struct = doc as? IonStruct ?: return@forEach
// Prefer explicit entryId field; fall back to QLDB metadata.id for older documents.
// TODO(backfill): Remove the `metaId` fallback once the QLDB backfill migration has been
// verified in production:
// Step 1 — verify backfill is complete (should return 0):
// SELECT COUNT(*) FROM FactLedger WHERE entryId IS MISSING
// Step 2 — run the backfill if the count is non-zero:
// UPDATE FactLedger AS f SET f.entryId = f.metadata.id WHERE f.entryId IS MISSING
// Step 3 — once Step 1 returns 0 in production, remove the companion.resolveEntryId
// fallback branch (the `?: struct.get("metaId")` path inside it) and the metaId
// projection from the SELECT above. The resolveEntryId behaviour is covered by
// AwsQldbLedgerEntryResolutionTest.
val resolvedEntryId = resolveEntryId(struct)
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getHistory() now relies solely on the explicit entryId. If production data is not fully backfilled (or a future write omits entryId), this will silently create LedgerEntry objects with entryId == "", which can break clients and makes diagnosing the bad document difficult. Consider failing fast (e.g., throw/log and skip the revision) when resolveEntryId(struct) returns blank, so incomplete backfills are detected immediately rather than producing invalid entries.

Suggested change
val resolvedEntryId = resolveEntryId(struct)
val resolvedEntryId = resolveEntryId(struct)
if (resolvedEntryId.isBlank()) {
log.error(
"QLDB history revision for record {} is missing required entryId: {}",
recordId,
struct
)
throw IllegalStateException("QLDB history revision for record $recordId is missing required entryId")
}

Copilot uses AI. Check for mistakes.
// Prefer QLDB txTime (accurate commit time) over createdAt (client time).
val timestamp = resolveTimestamp(
Expand Down Expand Up @@ -269,19 +255,13 @@ class AwsQldbLedger(private val properties: LedgerProperties) : IImmutableLedger
/**
* Resolves the entryId from a QLDB history IonStruct.
*
* Prefers the explicit `entryId` field written since the schema migration.
* Falls back to `metaId` (projected from QLDB document metadata.id) for pre-migration
* documents that were written before `entryId` was added as an explicit field.
*
* This fallback can be removed once:
* `SELECT COUNT(*) FROM FactLedger WHERE entryId IS MISSING` returns 0 in production.
* Reads the explicit `entryId` field stored in every document since the schema migration.
* Returns an empty string when the field is absent or is not a string value.
*
* Covered by [AwsQldbLedgerEntryResolutionTest].
*/
internal fun resolveEntryId(struct: IonStruct): String =
(struct.get("entryId") as? IonText)?.stringValue()
?: (struct.get("metaId") as? IonText)?.stringValue()
?: ""
(struct.get("entryId") as? IonText)?.stringValue() ?: ""

/**
* Resolves an [Instant] from QLDB history metadata timestamps or a stored `createdAt` field.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,6 @@ import java.time.Instant
/**
* Smoke-tests the IonStruct entry-resolution helpers in [AwsQldbLedger] without
* requiring a live QLDB connection.
*
* These tests verify the `entryId` → `metaId` fallback logic introduced for
* backward compatibility with pre-migration QLDB documents (issue #112).
* The fallback can be removed once:
* SELECT COUNT(*) FROM FactLedger WHERE entryId IS MISSING
* returns 0 in production, and these tests are updated accordingly.
*/
class AwsQldbLedgerEntryResolutionTest {

Expand All @@ -25,32 +19,22 @@ class AwsQldbLedgerEntryResolutionTest {
fun `resolveEntryId returns explicit entryId when present`() {
val struct = ionSystem.newEmptyStruct().apply {
put("entryId", ionSystem.newString("explicit-entry-id"))
put("metaId", ionSystem.newString("meta-should-be-ignored"))
}
assertEquals("explicit-entry-id", AwsQldbLedger.resolveEntryId(struct))
}

@Test
fun `resolveEntryId falls back to metaId when entryId is missing`() {
val struct = ionSystem.newEmptyStruct().apply {
put("metaId", ionSystem.newString("fallback-meta-id"))
}
assertEquals("fallback-meta-id", AwsQldbLedger.resolveEntryId(struct))
}

@Test
fun `resolveEntryId returns empty string when both entryId and metaId are absent`() {
fun `resolveEntryId returns empty string when entryId is absent`() {
val struct = ionSystem.newEmptyStruct()
assertEquals("", AwsQldbLedger.resolveEntryId(struct))
}

@Test
fun `resolveEntryId ignores non-IonText entryId field and falls back to metaId`() {
fun `resolveEntryId returns empty string when entryId is not an IonText`() {
val struct = ionSystem.newEmptyStruct().apply {
put("entryId", ionSystem.newInt(42))
put("metaId", ionSystem.newString("type-mismatch-fallback"))
}
assertEquals("type-mismatch-fallback", AwsQldbLedger.resolveEntryId(struct))
assertEquals("", AwsQldbLedger.resolveEntryId(struct))
}

// ── resolveTimestamp ──────────────────────────────────────────────────────
Expand Down
Loading