Skip to content

Commit 42c9894

Browse files
committed
refactor(sqs): drop unused alias + name decoder offsets + extra tests
Round 1 Claude review nits on PR #724: 1) Dropped sqsReceiptHandleVersion alias Round 1 added sqsReceiptHandleVersion as an alias of v1 "for existing call sites". In practice every call site now uses sqsReceiptHandleVersion1 explicitly, so the alias's only reference was a self-test. Dropped both the alias and the test assertion. The two version-byte constants (sqsReceiptHandleVersion1 / sqsReceiptHandleVersion2) are now the single source of truth. 2) Named per-field offset constants decodeReceiptHandleV1 / decodeReceiptHandleV2 used numeric offsets (1, 9, 13, 1+sqsMessageIDBytes, …) derived from layout arithmetic. Replaced with named constants: v1: V1VersionOffset, V1GenOffset, V1IDOffset, V1TokenOffset, V1Size v2: V2VersionOffset, V2PartitionOffset, V2GenOffset, V2IDOffset, V2TokenOffset, V2Size A future field-shape change touches the constants in one place instead of scattering numeric arithmetic through encoders / decoders. 3) Explicit upper bound on token slice bytes.Clone(b[start:]) → bytes.Clone(b[start:end]). The length check above guarantees the open-ended form would have read the right bytes, but the explicit upper bound is self-documenting and protects against a future refactor that forgets the length precondition. 4) Doc note on cross-version rejection decodedReceiptHandle's doc described the cross-version rejection contract ("a v1 handle against a partitioned queue → ReceiptHandleIsInvalid") that this scaffold PR does NOT enforce — that's PR 5b's gate-and-lift. Added an explicit note that enforcement is wired by PR 5b so future review of that PR doesn't go looking for the contract here. 5) New test case: v2 / v1 oversized TestDecodeReceiptHandle_RejectsLengthMismatch grew two cases (v2 + 1 byte, v1 + 1 byte) so the exact-length check on each decoder branch is independently exercised. The previous table covered "byte mismatch" and "truncated" but not "too long for the version". Audit per the lessons-learned discipline The semantic change here is the alias removal. grep -rn sqsReceiptHandleVersion across the repo confirmed zero remaining references after the test assertion was dropped. The named-offset refactor preserves byte-for-byte the same on-wire format; the constants ARE the layout. go test -race ./adapter/... and golangci-lint clean.
1 parent 0206a1c commit 42c9894

2 files changed

Lines changed: 58 additions & 29 deletions

File tree

adapter/sqs_messages.go

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,10 @@ const (
5656
// partitionFor hash. The version byte discriminates: a v1 handle
5757
// against a partitioned queue is rejected, a v2 handle against
5858
// a non-partitioned queue is rejected — both with
59-
// ReceiptHandleIsInvalid (codex P1 round-2 on PR #715 documents
60-
// the cross-version rejection contract).
59+
// ReceiptHandleIsInvalid. Enforcement of the cross-version
60+
// rejection contract is wired by Phase 3.D PR 5b together with
61+
// the gate-and-lift; this scaffold PR adds the codec only.
6162
sqsReceiptHandleVersion2 = byte(0x02)
62-
// sqsReceiptHandleVersion is the legacy const name preserved as
63-
// an alias of v1 so existing call sites in this file keep
64-
// compiling. New call sites SHOULD use sqsReceiptHandleVersion1
65-
// (or sqsReceiptHandleVersion2) explicitly.
66-
sqsReceiptHandleVersion = sqsReceiptHandleVersion1
6763
// Byte sizes used when pre-sizing key buffers. The exact value is not
6864
// critical; it only avoids one append growth for typical queue/ID
6965
// lengths.
@@ -231,15 +227,28 @@ func newReceiptToken() ([]byte, error) {
231227
return buf, nil
232228
}
233229

234-
// sqsReceiptHandleV1Size is the on-wire byte length of a v1
235-
// receipt handle: 1 byte version + 8 byte queue_gen + 16 byte
236-
// message_id + 16 byte receipt_token = 41 bytes.
237-
const sqsReceiptHandleV1Size = 1 + 8 + sqsMessageIDBytes + sqsReceiptTokenBytes
238-
239-
// sqsReceiptHandleV2Size is the on-wire byte length of a v2
240-
// (partitioned) receipt handle: v1 + 4 bytes for the partition
241-
// uint32 inserted between version and queue_gen = 45 bytes.
242-
const sqsReceiptHandleV2Size = 1 + 4 + 8 + sqsMessageIDBytes + sqsReceiptTokenBytes
230+
// Per-field byte offsets and total sizes for the v1 / v2 receipt
231+
// handle layouts. Named constants so a future field-shape change
232+
// touches the offsets in one place instead of scattering numeric
233+
// arithmetic through encoders / decoders. Keep this aligned with
234+
// the table in encodeReceiptHandle / encodeReceiptHandleV2 doc
235+
// comments — the on-wire format is operator-visible.
236+
const (
237+
// v1 layout: [version][queue_gen][message_id][receipt_token].
238+
sqsReceiptHandleV1VersionOffset = 0
239+
sqsReceiptHandleV1GenOffset = sqsReceiptHandleV1VersionOffset + 1
240+
sqsReceiptHandleV1IDOffset = sqsReceiptHandleV1GenOffset + 8
241+
sqsReceiptHandleV1TokenOffset = sqsReceiptHandleV1IDOffset + sqsMessageIDBytes
242+
sqsReceiptHandleV1Size = sqsReceiptHandleV1TokenOffset + sqsReceiptTokenBytes
243+
244+
// v2 layout: [version][partition][queue_gen][message_id][receipt_token].
245+
sqsReceiptHandleV2VersionOffset = 0
246+
sqsReceiptHandleV2PartitionOffset = sqsReceiptHandleV2VersionOffset + 1
247+
sqsReceiptHandleV2GenOffset = sqsReceiptHandleV2PartitionOffset + 4
248+
sqsReceiptHandleV2IDOffset = sqsReceiptHandleV2GenOffset + 8
249+
sqsReceiptHandleV2TokenOffset = sqsReceiptHandleV2IDOffset + sqsMessageIDBytes
250+
sqsReceiptHandleV2Size = sqsReceiptHandleV2TokenOffset + sqsReceiptTokenBytes
251+
)
243252

244253
// encodeReceiptHandle packs (queue_gen, message_id, receipt_token) into
245254
// a single opaque v1 blob. Used by SendMessage on a NON-partitioned
@@ -348,11 +357,14 @@ func decodeReceiptHandleV1(b []byte) (*decodedReceiptHandle, error) {
348357
return nil, errors.New("receipt handle length or version mismatch")
349358
}
350359
return &decodedReceiptHandle{
351-
Version: sqsReceiptHandleVersion1,
352-
Partition: 0,
353-
QueueGeneration: binary.BigEndian.Uint64(b[1:9]),
354-
MessageIDHex: hex.EncodeToString(b[9 : 9+sqsMessageIDBytes]),
355-
ReceiptToken: bytes.Clone(b[9+sqsMessageIDBytes:]),
360+
Version: sqsReceiptHandleVersion1,
361+
Partition: 0,
362+
QueueGeneration: binary.BigEndian.Uint64(
363+
b[sqsReceiptHandleV1GenOffset:sqsReceiptHandleV1IDOffset]),
364+
MessageIDHex: hex.EncodeToString(
365+
b[sqsReceiptHandleV1IDOffset:sqsReceiptHandleV1TokenOffset]),
366+
ReceiptToken: bytes.Clone(
367+
b[sqsReceiptHandleV1TokenOffset:sqsReceiptHandleV1Size]),
356368
}, nil
357369
}
358370

@@ -361,11 +373,15 @@ func decodeReceiptHandleV2(b []byte) (*decodedReceiptHandle, error) {
361373
return nil, errors.New("receipt handle length or version mismatch")
362374
}
363375
return &decodedReceiptHandle{
364-
Version: sqsReceiptHandleVersion2,
365-
Partition: binary.BigEndian.Uint32(b[1:5]),
366-
QueueGeneration: binary.BigEndian.Uint64(b[5:13]),
367-
MessageIDHex: hex.EncodeToString(b[13 : 13+sqsMessageIDBytes]),
368-
ReceiptToken: bytes.Clone(b[13+sqsMessageIDBytes:]),
376+
Version: sqsReceiptHandleVersion2,
377+
Partition: binary.BigEndian.Uint32(
378+
b[sqsReceiptHandleV2PartitionOffset:sqsReceiptHandleV2GenOffset]),
379+
QueueGeneration: binary.BigEndian.Uint64(
380+
b[sqsReceiptHandleV2GenOffset:sqsReceiptHandleV2IDOffset]),
381+
MessageIDHex: hex.EncodeToString(
382+
b[sqsReceiptHandleV2IDOffset:sqsReceiptHandleV2TokenOffset]),
383+
ReceiptToken: bytes.Clone(
384+
b[sqsReceiptHandleV2TokenOffset:sqsReceiptHandleV2Size]),
369385
}, nil
370386
}
371387

adapter/sqs_receipt_handle_v2_test.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,22 @@ func TestDecodeReceiptHandle_RejectsLengthMismatch(t *testing.T) {
159159
return out
160160
},
161161
},
162+
{
163+
name: "v2 oversized",
164+
make: func() []byte {
165+
out := make([]byte, sqsReceiptHandleV2Size+1)
166+
out[0] = sqsReceiptHandleVersion2
167+
return out
168+
},
169+
},
170+
{
171+
name: "v1 oversized",
172+
make: func() []byte {
173+
out := make([]byte, sqsReceiptHandleV1Size+1)
174+
out[0] = sqsReceiptHandleVersion1
175+
return out
176+
},
177+
},
162178
{
163179
name: "empty",
164180
make: func() []byte { return nil },
@@ -229,9 +245,6 @@ func TestReceiptHandleVersionConstants_Distinct(t *testing.T) {
229245
"v1 and v2 version bytes must differ for the dispatch to work")
230246
require.Equal(t, byte(0x01), sqsReceiptHandleVersion1)
231247
require.Equal(t, byte(0x02), sqsReceiptHandleVersion2)
232-
require.Equal(t, sqsReceiptHandleVersion1, sqsReceiptHandleVersion,
233-
"the legacy alias must continue to point at v1 so existing "+
234-
"call sites stay on the v1 path")
235248
// On-wire size constants must equal what the encoders write —
236249
// pinning them keeps a future struct change from silently
237250
// changing the wire format.

0 commit comments

Comments
 (0)