Various improvements to the serial package, including better error handling for out-of-range indices and improved test coverage for edge cases.

serial/serial.go

@@ -1,86 +1,170 @@
 package serial
 
 import (
+	"crypto/rand"
+	"encoding/binary"
 	"errors"
 	"fmt"
-	"log"
 )
 
 const (
 	MaxIndex = block
 
 	letters = 26
-	numbers = 900                                   // 100–999
-	block   = letters * letters * letters * numbers // 26^3 * 900
+	numbers = 900                                             // 100–999
+	block   = letters * letters * letters * letters * numbers // 26^4 * 900
+)
+
+var (
+	ErrInvalidFormat          = errors.New("invalid format, expected LLL-NNN-LC")
+	ErrInvalidLetters         = errors.New("invalid letters")
+	ErrForbiddenLetterTriplet = errors.New("forbidden letter combination")
+	ErrInvalidNumber          = errors.New("invalid number")
+	ErrNumberOutOfRange       = errors.New("number out of range")
+	ErrInvalidChecksum        = errors.New("invalid checksum")
+	ErrIndexOutOfRange        = errors.New("index out of range")
+	ErrRandomGenerationFailed = errors.New("random code generation failed")
 )
 
 func Encode(code string) (uint32, error) {
-	log.Printf("code: %v", code)
-	// Verwacht formaat: L1L2-NNN-L3-C  => lengte 10, posities: 0,1,3,4,5,7,9
-	if len(code) != 10 || code[2] != '-' || code[6] != '-' || code[8] != '-' {
-		return 0, errors.New("invalid format, expected LL-NNN-L-C")
+	// Expected format: L1L2L3-NNN-L4C  => length 10, positions: 0,1,2,4,5,6,8,9
+	if len(code) != 10 || code[3] != '-' || code[7] != '-' {
+		return 0, ErrInvalidFormat
 	}
 
-	l1, l2, l3 := code[0], code[1], code[7]
-	n1, n2, n3 := code[3], code[4], code[5]
+	l1, l2, l3, l4 := code[0], code[1], code[2], code[8]
+	n1, n2, n3 := code[4], code[5], code[6]
 
-	// letters checken
-	// forbidden alleen op l1,l2
-	// nummer parsen uit code[3:6]
+	// Validate letters
+	// Forbidden rule applies to l1,l2,l3
+	// Parse number from code[4:7]
+	if !isUpperAlphaASCII(l1) || !isUpperAlphaASCII(l2) || !isUpperAlphaASCII(l3) || !isUpperAlphaASCII(l4) {
+		return 0, ErrInvalidLetters
+	}
+	if IsForbiddenTriplet(l1, l2, l3) {
+		return 0, ErrForbiddenLetterTriplet
+	}
+	if !isNumber(n1) || !isNumber(n2) || !isNumber(n3) {
+		return 0, ErrInvalidNumber
+	}
 
 	L1 := uint32(l1 - 'A')
 	L2 := uint32(l2 - 'A')
 	L3 := uint32(l3 - 'A')
-	num := uint32(n1*100 + n2*10 + n3)
+	L4 := uint32(l4 - 'A')
+	num := uint32(n1-'0')*100 + uint32(n2-'0')*10 + uint32(n3-'0')
+	if num < 100 || num > 999 {
+		return 0, ErrNumberOutOfRange
+	}
 	N := num - 100
 
-	idx := uint32((((L1*letters+L2)*letters + L3) * numbers) + N)
+	idx := uint32((((((L1*letters+L2)*letters+L3)*letters + L4) * numbers) + N))
 
 	if code[9] != checksumLetter(idx) {
-		return 0, errors.New("invalid checksum")
+		return 0, ErrInvalidChecksum
 	}
 
-	log.Printf("idx: %v", idx)
 	return idx, nil
 }
 
 func Decode(idx uint32) (string, error) {
-	log.Printf("idx: %v", idx)
-	if idx >= block {
-		return "", errors.New("index out of range")
+	if idx >= MaxIndex {
+		return "", ErrIndexOutOfRange
 	}
 
 	x := int(idx)
 
-	L1 := x / (letters * letters * numbers)
-	r1 := x % (letters * letters * numbers)
+	L1 := x / (letters * letters * letters * numbers)
+	r1 := x % (letters * letters * letters * numbers)
 
-	L2 := r1 / (letters * numbers)
-	r2 := r1 % (letters * numbers)
+	L2 := r1 / (letters * letters * numbers)
+	r2 := r1 % (letters * letters * numbers)
 
-	L3 := r2 / numbers
-	N := r2 % numbers
+	L3 := r2 / (letters * numbers)
+	r3 := r2 % (letters * numbers)
+
+	L4 := r3 / numbers
+	N := r3 % numbers
 
 	l1 := byte('A' + L1)
 	l2 := byte('A' + L2)
 	l3 := byte('A' + L3)
+	l4 := byte('A' + L4)
 
-	if IsForbiddenPair(l1, l2) {
-		return "", errors.New("forbidden letter combination")
+	if IsForbiddenTriplet(l1, l2, l3) {
+		return "", ErrForbiddenLetterTriplet
 	}
 
 	c := checksumLetter(idx)
 
-	code := fmt.Sprintf("%c%c-%03d-%c-%c", l1, l2, N+100, l3, c)
+	code := fmt.Sprintf("%c%c%c-%03d-%c%c", l1, l2, l3, N+100, l4, c)
 
-	log.Printf("code: %v", code)
 	return code, nil
 }
 
+func RandomCode(isInUse ...func(uint32) bool) (string, uint32, error) {
+	var inUse func(uint32) bool
+	if len(isInUse) > 0 {
+		inUse = isInUse[0]
+	}
+
+	for attempts := 0; attempts < 1024; attempts++ {
+		idx, err := randomUint32n(MaxIndex)
+		if err != nil {
+			return "", 0, err
+		}
+
+		if inUse != nil && inUse(idx) {
+			continue
+		}
+
+		code, err := Decode(idx)
+		if err == nil {
+			return code, idx, nil
+		}
+		if errors.Is(err, ErrForbiddenLetterTriplet) {
+			continue
+		}
+
+		return "", 0, err
+	}
+
+	return "", 0, ErrRandomGenerationFailed
+}
+
 func checksumLetter(idx uint32) byte {
 	return byte('A' + idx%26)
 }
 
-func IsForbiddenPair(l1, l2 byte) bool {
+func isUpperAlphaASCII(b byte) bool {
+	return (b >= 'A' && b <= 'Z')
+}
+
+func isNumber(b byte) bool {
+	return b >= '0' && b <= '9'
+}
+
+func randomUint32n(n uint32) (uint32, error) {
+	if n == 0 {
+		return 0, ErrIndexOutOfRange
+	}
+
+	maxUint32 := ^uint32(0)
+	limit := maxUint32 - (maxUint32 % n)
+
+	for {
+		var buf [4]byte
+		if _, err := rand.Read(buf[:]); err != nil {
+			return 0, err
+		}
+
+		v := binary.LittleEndian.Uint32(buf[:])
+		if v < limit {
+			return v % n, nil
+		}
+	}
+}
+
+func IsForbiddenTriplet(l1, l2, l3 byte) bool {
 	return false
 }

serial/serial_test.go

@@ -1,13 +1,14 @@
 package serial_test
 
 import (
+	"errors"
 	"testing"
 	"testing/quick"
 
 	"git.hoiting.org/micha/triplex/serial"
 )
 
-// Property: Decode(Encode(code)) == code  (voor geldige codes)
+// Property: Decode(Encode(code)) == code (for valid codes)
 func TestEncodeDecodeRoundtrip(t *testing.T) {
 	f := func(idx uint32) bool {
 		if idx >= serial.MaxIndex {
@@ -32,7 +33,7 @@ func TestEncodeDecodeRoundtrip(t *testing.T) {
 	}
 }
 
-// Property: Decode(Encode(Decode(i))) == Decode(i)  (idempotent op geldige indices)
+// Property: Decode(Encode(Decode(i))) == Decode(i) (idempotent on valid indices)
 func TestDecodeEncodeDecodeIdempotent(t *testing.T) {
 	f := func(idx uint32) bool {
 		if idx >= serial.MaxIndex {
@@ -41,7 +42,7 @@ func TestDecodeEncodeDecodeIdempotent(t *testing.T) {
 
 		code1, err := serial.Decode(idx)
 		if err != nil {
-			return true // decode mag indices weigeren (bv. forbidden pair)
+			return true // decode may reject indices (e.g. forbidden pair)
 		}
 
 		idx2, err := serial.Encode(code1)
@@ -62,8 +63,8 @@ func TestDecodeEncodeDecodeIdempotent(t *testing.T) {
 	}
 }
 
-// Property: forbidden eerste twee letters komen nooit uit Decode
-func TestForbiddenPairsNeverDecoded(t *testing.T) {
+// Property: forbidden first three letters are never returned by Decode
+func TestForbiddenTripletsNeverDecoded(t *testing.T) {
 	f := func(idx uint32) bool {
 		if idx >= serial.MaxIndex {
 			return true
@@ -71,21 +72,22 @@ func TestForbiddenPairsNeverDecoded(t *testing.T) {
 
 		code, err := serial.Decode(idx)
 		if err != nil {
-			return true // decode mag indices weigeren
+			return true // decode may reject indices
 		}
 
 		l1 := code[0]
 		l2 := code[1]
+		l3 := code[2]
 
-		return !serial.IsForbiddenPair(l1, l2)
+		return !serial.IsForbiddenTriplet(l1, l2, l3)
 	}
 
 	if err := quick.Check(f, nil); err != nil {
-		t.Fatalf("Forbidden pairs appeared in Decode: %v", err)
+		t.Fatalf("Forbidden triplets appeared in Decode: %v", err)
 	}
 }
 
-// Optional: basis-check op formaat LL-NNN-L-C
+// Optional: basic check for format LLL-NNN-LC
 func TestDecodedFormatLooksCorrect(t *testing.T) {
 	f := func(idx uint32) bool {
 		if idx >= serial.MaxIndex {
@@ -100,19 +102,19 @@ func TestDecodedFormatLooksCorrect(t *testing.T) {
 		if len(code) != 10 {
 			return false
 		}
-		if code[2] != '-' || code[6] != '-' || code[8] != '-' {
+		if code[3] != '-' || code[7] != '-' {
 			return false
 		}
 
-		// letters op posities 0,1,7,9
-		for _, p := range []int{0, 1, 7, 9} {
+		// letters at positions 0,1,2,8,9
+		for _, p := range []int{0, 1, 2, 8, 9} {
 			if code[p] < 'A' || code[p] > 'Z' {
 				return false
 			}
 		}
 
-		// cijfers op posities 3,4,5
-		for _, p := range []int{3, 4, 5} {
+		// digits at positions 4,5,6
+		for _, p := range []int{4, 5, 6} {
 			if code[p] < '0' || code[p] > '9' {
 				return false
 			}
@@ -125,3 +127,123 @@ func TestDecodedFormatLooksCorrect(t *testing.T) {
 		t.Fatalf("Decoded format property failed: %v", err)
 	}
 }
+
+func TestEncodeDecodeRoundtripSpecificIndex11223344(t *testing.T) {
+	baseIdx := uint32(0x11223344)
+	idx := baseIdx % serial.MaxIndex
+
+	code, err := serial.Decode(idx)
+	if err != nil {
+		t.Fatalf("Decode failed for idx %d (derived from 0x11223344): %v", idx, err)
+	}
+
+	idx2, err := serial.Encode(code)
+	if err != nil {
+		t.Fatalf("Encode failed for code %q: %v", code, err)
+	}
+
+	if idx2 != idx {
+		t.Fatalf("roundtrip mismatch: got %d, want %d", idx2, idx)
+	}
+}
+
+func TestDecodeOutOfRangeAtMaxIndex(t *testing.T) {
+	_, err := serial.Decode(serial.MaxIndex)
+	if !errors.Is(err, serial.ErrIndexOutOfRange) {
+		t.Fatalf("expected index out of range error, got: %v", err)
+	}
+}
+
+func TestDecodeOutOfRangeAtMaxIndexPlusHundred(t *testing.T) {
+	_, err := serial.Decode(serial.MaxIndex + 100)
+	if !errors.Is(err, serial.ErrIndexOutOfRange) {
+		t.Fatalf("expected index out of range error, got: %v", err)
+	}
+}
+
+func TestEncodeRejectsOldFormatWithThirdDash(t *testing.T) {
+	_, err := serial.Encode("AA-100-AA")
+	if !errors.Is(err, serial.ErrInvalidFormat) {
+		t.Fatalf("expected invalid format error, got: %v", err)
+	}
+}
+
+func TestEncodeRejectsLowercaseLetters(t *testing.T) {
+	_, err := serial.Encode("AAa-100-AA")
+	if !errors.Is(err, serial.ErrInvalidLetters) {
+		t.Fatalf("expected invalid letters error, got: %v", err)
+	}
+}
+
+func TestEncodeRejectsInvalidChecksum(t *testing.T) {
+	_, err := serial.Encode("AAA-100-AB")
+	if !errors.Is(err, serial.ErrInvalidChecksum) {
+		t.Fatalf("expected invalid checksum error, got: %v", err)
+	}
+}
+
+func TestEncodeRejectsNonDigitInNumber(t *testing.T) {
+	_, err := serial.Encode("AAA-1A0-AA")
+	if !errors.Is(err, serial.ErrInvalidNumber) {
+		t.Fatalf("expected invalid number error, got: %v", err)
+	}
+}
+
+func TestRandomCodeRoundtrip(t *testing.T) {
+	for i := 0; i < 100; i++ {
+		code, idx, err := serial.RandomCode()
+		if err != nil {
+			t.Fatalf("RandomCode failed: %v", err)
+		}
+
+		idx2, err := serial.Encode(code)
+		if err != nil {
+			t.Fatalf("Encode failed for random code %q: %v", code, err)
+		}
+
+		if idx2 != idx {
+			t.Fatalf("returned idx mismatch: got %d, want %d", idx, idx2)
+		}
+
+		decoded, err := serial.Decode(idx)
+		if err != nil {
+			t.Fatalf("Decode failed for idx %d: %v", idx, err)
+		}
+
+		if decoded != code {
+			t.Fatalf("random roundtrip mismatch: got %q, want %q", decoded, code)
+		}
+	}
+}
+
+func TestRandomCodeInvokesInUseCallback(t *testing.T) {
+	called := false
+
+	code, idx, err := serial.RandomCode(func(candidate uint32) bool {
+		called = true
+		return false
+	})
+	if err != nil {
+		t.Fatalf("RandomCode failed: %v", err)
+	}
+	if !called {
+		t.Fatalf("expected in-use callback to be called")
+	}
+
+	idx2, err := serial.Encode(code)
+	if err != nil {
+		t.Fatalf("Encode failed for random code %q: %v", code, err)
+	}
+	if idx2 != idx {
+		t.Fatalf("returned idx mismatch: got %d, want %d", idx, idx2)
+	}
+}
+
+func TestRandomCodeFailsWhenAllIndicesInUse(t *testing.T) {
+	_, _, err := serial.RandomCode(func(candidate uint32) bool {
+		return true
+	})
+	if !errors.Is(err, serial.ErrRandomGenerationFailed) {
+		t.Fatalf("expected random generation failed error, got: %v", err)
+	}
+}