From e29d00d7f0e58a31b7211c5231286d48ce8e8291 Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Fri, 27 Jun 2025 21:33:36 +0100 Subject: [PATCH 01/12] Add tests on file_operations --- .gitignore | 3 +- go.mod | 3 + go.sum | 16 ++--- internal/file_operations_test.go | 100 +++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 13 deletions(-) create mode 100644 internal/file_operations_test.go diff --git a/.gitignore b/.gitignore index 7fc9b6fe..c92f9510 100644 --- a/.gitignore +++ b/.gitignore @@ -32,4 +32,5 @@ go.work.sum # .vscode/ data/ -__debug_bin* \ No newline at end of file +__debug_bin* +test-reports/ \ No newline at end of file diff --git a/go.mod b/go.mod index 13929168..ab039fd8 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/json-iterator/go v1.1.12 github.com/paulmach/orb v0.11.1 github.com/spf13/cobra v1.9.1 + github.com/stretchr/testify v1.10.0 github.com/tavsec/gin-healthcheck v1.7.8 go.eigsys.de/gin-cachecontrol/v2 v2.3.0 ) @@ -23,6 +24,7 @@ require ( github.com/bytedance/sonic/loader v0.2.4 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/cloudwego/base64x v0.1.5 // indirect + github.com/davecgh/go-spew v1.1.1 // indirect github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect github.com/gabriel-vasile/mimetype v1.4.9 // indirect github.com/gin-contrib/sse v1.1.0 // indirect @@ -45,6 +47,7 @@ require ( github.com/oapi-codegen/runtime v1.1.1 // indirect github.com/patrickmn/go-cache v2.1.0+incompatible // indirect github.com/pelletier/go-toml/v2 v2.2.4 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_golang v1.22.0 // indirect github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.65.0 // indirect diff --git a/go.sum b/go.sum index d1b7f53c..956d3dac 100644 --- a/go.sum +++ b/go.sum @@ -10,8 +10,6 @@ github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERo github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/RaveNoX/go-jsoncommentstrip v1.0.0/go.mod h1:78ihd09MekBnJnxpICcwzCMzGrKSKYe4AqU6PDYYpjk= github.com/andybalholm/brotli v1.0.4/go.mod h1:fO7iG3H7G2nSZ7m0zPUDn85XEX2GTukHGRSepvi9Eig= -github.com/andybalholm/brotli v1.1.1 h1:PR2pgnyFznKEugtsUo0xLdDop5SKXd5Qf5ysW+7XdTA= -github.com/andybalholm/brotli v1.1.1/go.mod h1:05ib4cKhjx3OQYUY22hTVd34Bc8upXjOLL2rKwwZBoA= github.com/andybalholm/brotli v1.2.0 h1:ukwgCxwYrmACq68yiUqwIWnGY0cTPox/M94sVwToPjQ= github.com/andybalholm/brotli v1.2.0/go.mod h1:rzTDkvFWvIrjDXZHkuS16NPggd91W3kUSvPlQ1pLaKY= github.com/apapsch/go-jsonmerge/v2 v2.0.0 h1:axGnT1gRIfimI7gJifB699GoE/oq+F2MU7Dml6nw9rQ= @@ -139,8 +137,6 @@ github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zt github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ= github.com/klauspost/cpuid v1.2.0/go.mod h1:Pj4uuM528wm8OyEC2QMXAi2YiTZ96dNQPGgoMS4s3ek= github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= -github.com/klauspost/cpuid/v2 v2.2.10 h1:tBs3QSyvjDyFTq3uoc/9xFpCuOsJQFNPiAhYdw2skhE= -github.com/klauspost/cpuid/v2 v2.2.10/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0= github.com/klauspost/cpuid/v2 v2.2.11 h1:0OwqZRYI2rFrjS4kvkDnqJkKHdHaRnCm68/DY4OxRzU= github.com/klauspost/cpuid/v2 v2.2.11/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0= github.com/knz/go-libedit v1.10.1/go.mod h1:MZTVkCWyz0oBc7JOWP3wNAzd002ZbM/5hgShxwh4x8M= @@ -164,12 +160,9 @@ github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I= github.com/magiconair/properties v1.8.10 h1:s31yESBquKXCV9a/ScB3ESkOjUYYv+X0rg8SYxI99mE= github.com/magiconair/properties v1.8.10/go.mod h1:Dhd985XPs7jluiymwWYZ0G4Z61jb3vdS329zhj2hYo0= -github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= -github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= -github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/moby/docker-image-spec v1.3.1 h1:jMKff3w6PgbfSa69GfNg+zN/XLhfXJGnEx3Nl2EsFP0= @@ -222,16 +215,12 @@ github.com/prometheus/client_golang v1.22.0 h1:rb93p9lokFEsctTys46VnV1kLCDpVZ0a/ github.com/prometheus/client_golang v1.22.0/go.mod h1:R7ljNsLXhuQXYZYtw6GAE9AZg8Y7vEW5scdCXrWRXC0= github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= github.com/prometheus/client_model v0.6.2/go.mod h1:y3m2F6Gdpfy6Ut/GBsUqTWZqCUvMVzSfMLjcu6wAwpE= -github.com/prometheus/common v0.64.0 h1:pdZeA+g617P7oGv1CzdTzyeShxAGrTBsolKNOLQPGO4= -github.com/prometheus/common v0.64.0/go.mod h1:0gZns+BLRQ3V6NdaerOhMbwwRbNh9hkGINtQAsP5GS8= github.com/prometheus/common v0.65.0 h1:QDwzd+G1twt//Kwj/Ww6E9FQq1iVMmODnILtW1t2VzE= github.com/prometheus/common v0.65.0/go.mod h1:0gZns+BLRQ3V6NdaerOhMbwwRbNh9hkGINtQAsP5GS8= github.com/prometheus/procfs v0.16.1 h1:hZ15bTNuirocR6u0JZ6BAHHmwS1p8B4P6MRqxtzMyRg= github.com/prometheus/procfs v0.16.1/go.mod h1:teAbpZRB1iIAJYREa1LsoWUXykVXA1KlTmWl8x/U+Is= github.com/rabbitmq/amqp091-go v1.10.0 h1:STpn5XsHlHGcecLmMFCtg7mqq0RnD+zFr4uzukfVhBw= github.com/rabbitmq/amqp091-go v1.10.0/go.mod h1:Hy4jKW5kQART1u+JkDTF9YYOQUHXqMuhrgxOEeS7G4o= -github.com/redis/go-redis/v9 v9.10.0 h1:FxwK3eV8p/CQa0Ch276C7u2d0eNC9kCmAYQ7mCXCzVs= -github.com/redis/go-redis/v9 v9.10.0/go.mod h1:huWgSWd8mW6+m0VPhJjSSQ+d6Nh1VICQ6Q5lHuCH/Iw= github.com/redis/go-redis/v9 v9.11.0 h1:E3S08Gl/nJNn5vkxd2i78wZxWAPNZgUNTp8WIJUAiIs= github.com/redis/go-redis/v9 v9.11.0/go.mod h1:huWgSWd8mW6+m0VPhJjSSQ+d6Nh1VICQ6Q5lHuCH/Iw= github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= @@ -243,6 +232,10 @@ github.com/shirou/gopsutil/v4 v4.25.1 h1:QSWkTc+fu9LTAWfkZwZ6j8MSUk4A2LV7rbH0Zqm github.com/shirou/gopsutil/v4 v4.25.1/go.mod h1:RoUCUpndaJFtT+2zsZzzmhvbfGoDCJ7nFXKJf8GqJbI= github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= +github.com/smartystreets/assertions v1.2.0 h1:42S6lae5dvLc7BrLu/0ugRtcFVjoJNMC/N3yZFZkDFs= +github.com/smartystreets/assertions v1.2.0/go.mod h1:tcbTF8ujkAEcZ8TElKY+i30BzYlVhC/LOxJk7iOWnoo= +github.com/smartystreets/gunit v1.4.2 h1:tyWYZffdPhQPfK5VsMQXfauwnJkqg7Tv5DLuQVYxq3Q= +github.com/smartystreets/gunit v1.4.2/go.mod h1:ZjM1ozSIMJlAz/ay4SG8PeKF00ckUp+zMHZXV9/bvak= github.com/spf13/cobra v1.9.1 h1:CXSaggrXdbHK9CF+8ywj8Amf7PBRmPCOJugH954Nnlo= github.com/spf13/cobra v1.9.1/go.mod h1:nDyEzZ8ogv936Cinf6g1RU9MRY64Ir93oCnqb9wxYW0= github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= @@ -362,7 +355,6 @@ golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210806184541-e5e7981a1069/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= diff --git a/internal/file_operations_test.go b/internal/file_operations_test.go new file mode 100644 index 00000000..119f3ac1 --- /dev/null +++ b/internal/file_operations_test.go @@ -0,0 +1,100 @@ +package internal + +import ( + "os" + "path/filepath" + "testing" + + "github.com/paulmach/orb" + "github.com/paulmach/orb/geojson" + "github.com/stretchr/testify/require" +) + +// Helper to create a compressed .bz2 file with a known FeatureCollection +func createTestBZ2File(t *testing.T, fc *geojson.FeatureCollection) string { + tmpfile, err := os.CreateTemp("", "testfc-*.geojson.bz2") + require.NoError(t, err) + _ = tmpfile.Close() + _, err = CompressFeatureCollection(tmpfile.Name(), fc) + require.NoError(t, err) + return tmpfile.Name() +} + +func TestDecompressFeatureCollection_Success(t *testing.T) { + fc := geojson.NewFeatureCollection() + fc.Append(geojson.NewFeature(orb.Point{1, 2})) + fc.Append(geojson.NewFeature(orb.Point{3, 4})) + + bz2file := createTestBZ2File(t, fc) + defer func() { _ = os.Remove(bz2file) }() + + result, err := DecompressFeatureCollection(bz2file) + require.NoError(t, err) + require.Equal(t, len(fc.Features), len(result.Features)) + for i := range fc.Features { + require.Equal(t, fc.Features[i].Geometry, result.Features[i].Geometry) + } +} + +func TestDecompressFeatureCollection_FileNotFound(t *testing.T) { + _, err := DecompressFeatureCollection("nonexistent_file.bz2") + require.Error(t, err) +} + +func TestDecompressFeatureCollection_InvalidBZ2(t *testing.T) { + tmpfile := filepath.Join(os.TempDir(), "invalid.bz2") + err := os.WriteFile(tmpfile, []byte("not a bzip2 file"), 0644) + require.NoError(t, err) + defer func() { _ = os.Remove(tmpfile) }() + + _, err = DecompressFeatureCollection(tmpfile) + require.Error(t, err) +} + +func TestCompressFeatureCollection_Success(t *testing.T) { + fc := geojson.NewFeatureCollection() + fc.Append(geojson.NewFeature(orb.Point{10, 20})) + fc.Append(geojson.NewFeature(orb.Point{30, 40})) + + tmpfile, err := os.CreateTemp("", "compressfc-*.geojson.bz2") + require.NoError(t, err) + _ = tmpfile.Close() + defer func() { _ = os.Remove(tmpfile.Name()) }() + + n, err := CompressFeatureCollection(tmpfile.Name(), fc) + require.NoError(t, err) + require.Greater(t, n, 0) + + // Check file exists and is not empty + info, err := os.Stat(tmpfile.Name()) + require.NoError(t, err) + require.Equal(t, info.Size(), int64(n)) +} + +func TestCompressFeatureCollection_ErrorOnCreate(t *testing.T) { + // Try to write to a directory (should fail) + _, err := CompressFeatureCollection("/this/does/not/exist/test.bz2", geojson.NewFeatureCollection()) + require.Error(t, err) +} + +func TestCompressFeatureCollection_And_DecompressFeatureCollection_RoundTrip(t *testing.T) { + // Create a FeatureCollection with properties + fc := geojson.NewFeatureCollection() + feat := geojson.NewFeature(orb.Point{5, 6}) + feat.Properties = map[string]any{"foo": "bar"} + fc.Append(feat) + + tmpfile, err := os.CreateTemp("", "roundtripfc-*.geojson.bz2") + require.NoError(t, err) + _ = tmpfile.Close() + defer func() { _ = os.Remove(tmpfile.Name()) }() + + _, err = CompressFeatureCollection(tmpfile.Name(), fc) + require.NoError(t, err) + + result, err := DecompressFeatureCollection(tmpfile.Name()) + require.NoError(t, err) + require.Equal(t, len(fc.Features), len(result.Features)) + require.Equal(t, fc.Features[0].Geometry, result.Features[0].Geometry) + require.Equal(t, fc.Features[0].Properties["foo"], result.Features[0].Properties["foo"]) +} From 8a46382ddee3626a3a90b8411c787891d2bb8566 Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Fri, 27 Jun 2025 21:55:36 +0100 Subject: [PATCH 02/12] Add tests on CSV parser --- spatial-index/csv_test.go | 124 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 spatial-index/csv_test.go diff --git a/spatial-index/csv_test.go b/spatial-index/csv_test.go new file mode 100644 index 00000000..ca71e0db --- /dev/null +++ b/spatial-index/csv_test.go @@ -0,0 +1,124 @@ +package spatialindex + +import ( + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +type testRow struct { + A string + B string +} + +func fromFuncSimple(data []string, headers []string) (testRow, error) { + return testRow{A: data[0], B: data[1]}, nil +} + +func fromFuncWithError(data []string, headers []string) (testRow, error) { + return testRow{}, errors.New("parse error") +} + +func TestParseCSV_WithHeader_Success(t *testing.T) { + csvData := "a,b\nfoo,bar\nbaz,qux\n" + reader := strings.NewReader(csvData) + + var results []Result[testRow] + iter := parseCSV(reader, true, fromFuncSimple) + for r := range iter { + results = append(results, r) + } + + require.Len(t, results, 2) + require.Equal(t, 1, results[0].LineNum) + require.Equal(t, "foo", results[0].Value.A) + require.Equal(t, "bar", results[0].Value.B) + require.NoError(t, results[0].Error) + require.Equal(t, 2, results[1].LineNum) + require.Equal(t, "baz", results[1].Value.A) + require.Equal(t, "qux", results[1].Value.B) + require.NoError(t, results[1].Error) +} + +func TestParseCSV_WithoutHeader_Success(t *testing.T) { + csvData := "foo,bar\nbaz,qux\n" + reader := strings.NewReader(csvData) + + var results []Result[testRow] + iter := parseCSV(reader, false, fromFuncSimple) + for r := range iter { + results = append(results, r) + } + + require.Len(t, results, 2) + require.Equal(t, 1, results[0].LineNum) + require.Equal(t, "foo", results[0].Value.A) + require.Equal(t, "bar", results[0].Value.B) + require.NoError(t, results[0].Error) + require.Equal(t, 2, results[1].LineNum) + require.Equal(t, "baz", results[1].Value.A) + require.Equal(t, "qux", results[1].Value.B) + require.NoError(t, results[1].Error) +} + +func TestParseCSV_ParseError(t *testing.T) { + csvData := "a,b\nfoo,bar\nbaz,qux\n" + reader := strings.NewReader(csvData) + + var results []Result[testRow] + iter := parseCSV(reader, true, fromFuncWithError) + for r := range iter { + results = append(results, r) + } + + require.Len(t, results, 1) + require.Equal(t, 1, results[0].LineNum) + require.Error(t, results[0].Error) + require.Contains(t, results[0].Error.Error(), "failed to parse CSV line 1") +} + +func TestParseCSV_MalformedLine(t *testing.T) { + csvData := "a,b\nfoo\n" + reader := strings.NewReader(csvData) + + var results []Result[testRow] + iter := parseCSV(reader, true, fromFuncSimple) + for r := range iter { + results = append(results, r) + } + + require.Len(t, results, 1) + require.Equal(t, 1, results[0].LineNum) + require.Error(t, results[0].Error) + // Error message is from underlying csv.Reader + require.Contains(t, results[0].Error.Error(), "wrong number of fields") +} + +func TestParseCSV_HeaderReadError(t *testing.T) { + // An empty reader will cause an io.EOF when reading headers. + reader := strings.NewReader("") + iter := parseCSV(reader, true, fromFuncSimple) + var results []Result[testRow] + for r := range iter { + results = append(results, r) + } + require.Len(t, results, 1) + require.Equal(t, 0, results[0].LineNum) + require.Error(t, results[0].Error) + require.Contains(t, results[0].Error.Error(), "failed to read CSV headers") +} + +func TestParseCSV_YieldStopsEarly(t *testing.T) { + csvData := "a,b\nfoo,bar\nbaz,qux\n" + reader := strings.NewReader(csvData) + + count := 0 + iter := parseCSV(reader, true, fromFuncSimple) + iter(func(r Result[testRow]) bool { + count++ + return false // stop after first + }) + require.Equal(t, 1, count) +} From 2b5581b1889f9780132765e576a73c997942e2e1 Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Fri, 27 Jun 2025 22:38:23 +0100 Subject: [PATCH 03/12] Add tests on codepoint spatial index --- spatial-index/codepoint.go | 26 ++++---- spatial-index/codepoint_test.go | 103 ++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 13 deletions(-) create mode 100644 spatial-index/codepoint_test.go diff --git a/spatial-index/codepoint.go b/spatial-index/codepoint.go index a763868b..d66ee50f 100644 --- a/spatial-index/codepoint.go +++ b/spatial-index/codepoint.go @@ -20,6 +20,19 @@ type SpatialIndex struct { tree *rtree.RTreeGN[uint32, string] } +func NewCodePointSpatialIndex(zipFile string) (*SpatialIndex, error) { + idx := SpatialIndex{ + tree: &rtree.RTreeGN[uint32, string]{}, + } + + err := idx.importCodePoint(zipFile) + if err != nil { + return nil, fmt.Errorf("failed to load codepoint index from zip file: %w", err) + } + + return &idx, nil +} + func (idx *SpatialIndex) Search(bounds []uint32) (*[]CodePoint, error) { results := make([]CodePoint, 0, 100) @@ -51,19 +64,6 @@ func (idx *SpatialIndex) SearchIter(bounds []uint32, iter func(min, max [2]uint3 return nil } -func NewCodePointSpatialIndex(zipFile string) (*SpatialIndex, error) { - idx := SpatialIndex{ - tree: &rtree.RTreeGN[uint32, string]{}, - } - - err := idx.importCodePoint(zipFile) - if err != nil { - return nil, fmt.Errorf("failed to load codepoint index from zip file: %w", err) - } - - return &idx, nil -} - func (idx *SpatialIndex) Len() int { return idx.tree.Len() } diff --git a/spatial-index/codepoint_test.go b/spatial-index/codepoint_test.go new file mode 100644 index 00000000..d38e1d04 --- /dev/null +++ b/spatial-index/codepoint_test.go @@ -0,0 +1,103 @@ +package spatialindex + +import ( + "archive/zip" + "os" + "testing" + + "github.com/stretchr/testify/require" + "github.com/tidwall/rtree" +) + +func createTestZip(t *testing.T, files map[string]string) string { + t.Helper() + tmpfile, err := os.CreateTemp("", "test-*.zip") + require.NoError(t, err, "failed to create temp file") + defer func() { _ = tmpfile.Close() }() + + w := zip.NewWriter(tmpfile) + for name, content := range files { + f, err := w.Create(name) + require.NoError(t, err, "failed to create file in zip") + _, err = f.Write([]byte(content)) + require.NoError(t, err, "failed to write file in zip") + } + _ = w.Close() + return tmpfile.Name() +} + +func TestNewCodePointSpatialIndex_Success(t *testing.T) { + csv := "PC1,PC2,123,456\nPC3,PC4,789,1011\n" + zipPath := createTestZip(t, map[string]string{"Data/CSV/test.csv": csv}) + defer func() { _ = os.Remove(zipPath) }() + + idx, err := NewCodePointSpatialIndex(zipPath) + require.NoError(t, err) + require.Equal(t, 2, idx.Len()) +} + +func TestNewCodePointSpatialIndex_BadZip(t *testing.T) { + _, err := NewCodePointSpatialIndex("/no/such/file.zip") + require.Error(t, err, "expected error for missing zip file") +} + +func TestSearch(t *testing.T) { + csv := "PC1,PC2,100,200\nPC2,PC3,300,400\n" + zipPath := createTestZip(t, map[string]string{"Data/CSV/test.csv": csv}) + defer func() { _ = os.Remove(zipPath) }() + idx, err := NewCodePointSpatialIndex(zipPath) + require.NoError(t, err) + + // Search for both + res, err := idx.Search([]uint32{0, 0, 500, 500}) + require.NoError(t, err) + require.Equal(t, 2, len(*res)) + + // Search for one + res, err = idx.Search([]uint32{90, 190, 110, 210}) + require.NoError(t, err) + require.Equal(t, 1, len(*res)) + require.Equal(t, "PC1", (*res)[0].PostCode) + + // Search for none + res, err = idx.Search([]uint32{1000, 1000, 2000, 2000}) + require.NoError(t, err) + require.Equal(t, 0, len(*res)) +} + +func TestSearchIter_InvalidBounds(t *testing.T) { + idx := &SpatialIndex{tree: &rtree.RTreeGN[uint32, string]{}} + err := idx.SearchIter([]uint32{1, 2, 3}, func([2]uint32, [2]uint32, string) bool { return true }) + require.Error(t, err) + require.Contains(t, err.Error(), "4 values") +} + +func TestLen(t *testing.T) { + csv := "PC1,PC2,1,2\nPC2,PC3,3,4\nPC3,PC4,5,6\n" + zipPath := createTestZip(t, map[string]string{"Data/CSV/test.csv": csv}) + defer func() { _ = os.Remove(zipPath) }() + idx, err := NewCodePointSpatialIndex(zipPath) + require.NoError(t, err) + require.Equal(t, 3, idx.Len()) +} + +func Test_fromCodePointCSV(t *testing.T) { + rec := []string{"PC1", "PC2", "123", "456"} + cp, err := fromCodePointCSV(rec, nil) + require.NoError(t, err) + require.Equal(t, "PC1", cp.PostCode) + require.Equal(t, uint32(123), cp.Easting) + require.Equal(t, uint32(456), cp.Northing) + + // Bad easting + rec = []string{"PC1", "PC2", "bad", "456"} + _, err = fromCodePointCSV(rec, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "easting") + + // Bad northing + rec = []string{"PC1", "PC2", "123", "bad"} + _, err = fromCodePointCSV(rec, nil) + require.Error(t, err) + require.Contains(t, err.Error(), "northing") +} From 60c5cada94055f269ef429cfc214e3e66fa1a3eb Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Fri, 27 Jun 2025 22:44:14 +0100 Subject: [PATCH 04/12] Update spatial-index/codepoint_test.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- spatial-index/codepoint_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spatial-index/codepoint_test.go b/spatial-index/codepoint_test.go index d38e1d04..fc9e7a44 100644 --- a/spatial-index/codepoint_test.go +++ b/spatial-index/codepoint_test.go @@ -38,7 +38,7 @@ func TestNewCodePointSpatialIndex_Success(t *testing.T) { func TestNewCodePointSpatialIndex_BadZip(t *testing.T) { _, err := NewCodePointSpatialIndex("/no/such/file.zip") - require.Error(t, err, "expected error for missing zip file") + require.Error(t, err, "failed to open zip file: file does not exist") } func TestSearch(t *testing.T) { From 4a491e842cb1e11801324c8a964a3151eb5b3a2e Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Fri, 27 Jun 2025 22:44:21 +0100 Subject: [PATCH 05/12] Update spatial-index/codepoint_test.go Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- spatial-index/codepoint_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spatial-index/codepoint_test.go b/spatial-index/codepoint_test.go index fc9e7a44..c60f9ea1 100644 --- a/spatial-index/codepoint_test.go +++ b/spatial-index/codepoint_test.go @@ -69,7 +69,7 @@ func TestSearchIter_InvalidBounds(t *testing.T) { idx := &SpatialIndex{tree: &rtree.RTreeGN[uint32, string]{}} err := idx.SearchIter([]uint32{1, 2, 3}, func([2]uint32, [2]uint32, string) bool { return true }) require.Error(t, err) - require.Contains(t, err.Error(), "4 values") + require.Contains(t, err.Error(), "bounds must contain 4 values") } func TestLen(t *testing.T) { From 08897df8fadc6b851da68bbf6c16603493aea8cf Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Fri, 27 Jun 2025 22:56:04 +0100 Subject: [PATCH 06/12] Fix test error --- spatial-index/codepoint_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spatial-index/codepoint_test.go b/spatial-index/codepoint_test.go index c60f9ea1..38066511 100644 --- a/spatial-index/codepoint_test.go +++ b/spatial-index/codepoint_test.go @@ -69,7 +69,7 @@ func TestSearchIter_InvalidBounds(t *testing.T) { idx := &SpatialIndex{tree: &rtree.RTreeGN[uint32, string]{}} err := idx.SearchIter([]uint32{1, 2, 3}, func([2]uint32, [2]uint32, string) bool { return true }) require.Error(t, err) - require.Contains(t, err.Error(), "bounds must contain 4 values") + require.Contains(t, err.Error(), "bounds must contain exactly 4 values") } func TestLen(t *testing.T) { From 411a33751ffe0e9fec5f1b9d0d661306b70bf156 Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Sat, 28 Jun 2025 10:44:17 +0100 Subject: [PATCH 07/12] wip --- routes/search_test.go | 135 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 routes/search_test.go diff --git a/routes/search_test.go b/routes/search_test.go new file mode 100644 index 00000000..53749c07 --- /dev/null +++ b/routes/search_test.go @@ -0,0 +1,135 @@ +package routes + +import ( + "net/http" + "net/http/httptest" + spatialindex "postcode-polygons/spatial-index" + "testing" + + "github.com/gin-gonic/gin" + "github.com/kofalt/go-memoize" + "github.com/stretchr/testify/require" +) + +// --- CodePointSearch tests --- +func TestCodePointSearch_BadBBox(t *testing.T) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/search?bbox=bad,bbox,values", nil) + c.Request.URL.RawQuery = "bbox=bad,bbox,values" + + spatialIdx, err := spatialindex.NewCodePointSpatialIndex("../data/codepo_gb.zip") + require.NoError(t, err) + handler := CodePointSearch(spatialIdx) + handler(c) + + require.Equal(t, http.StatusBadRequest, w.Code) + require.Contains(t, w.Body.String(), "bbox must have 4 comma-separated values") +} + +func TestCodePointSearch_TooBig(t *testing.T) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/search?bbox=0,0,10000,10000", nil) + c.Request.URL.RawQuery = "bbox=0,0,10000,10000" + + spatialIdx, err := spatialindex.NewCodePointSpatialIndex("../data/codepo_gb.zip") + require.NoError(t, err) + handler := CodePointSearch(spatialIdx) + handler(c) + + require.Equal(t, http.StatusBadRequest, w.Code) + require.Contains(t, w.Body.String(), "bbox is too large") +} + +// func TestCodePointSearch_InternalError(t *testing.T) { +// gin.SetMode(gin.TestMode) +// w := httptest.NewRecorder() +// c, _ := gin.CreateTestContext(w) +// c.Request = httptest.NewRequest("GET", "/search?bbox=0,0,1,1", nil) +// c.Request.URL.RawQuery = "bbox=0,0,1,1" + +// spatialIdx := &mockSpatialIndex{ +// SearchFunc: func(bounds []uint32) (*[]spatialindex.CodePoint, error) { +// return nil, errors.New("fail") +// }, +// } +// handler := CodePointSearch(spatialIdx) +// handler(c) + +// require.Equal(t, http.StatusInternalServerError, w.Code) +// require.Contains(t, w.Body.String(), "An internal server error occurred") +// } + +func TestCodePointSearch_Success(t *testing.T) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/v1/postcode/codepoints?bbox=531900,184200,532100,184400", nil) + c.Request.URL.RawQuery = "bbox=531900,184200,532100,184400" + + spatialIdx, err := spatialindex.NewCodePointSpatialIndex("../data/codepo_gb.zip") + require.NoError(t, err) + handler := CodePointSearch(spatialIdx) + handler(c) + require.Equal(t, http.StatusOK, w.Code) + require.Contains(t, w.Body.String(), "N1 2AJ") +} + +// --- PolygonSearch tests --- +func TestPolygonSearch_BadBBox(t *testing.T) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/polygon?bbox=bad,bbox,values", nil) + c.Request.URL.RawQuery = "bbox=bad,bbox,values" + + spatialIdx, err := spatialindex.NewCodePointSpatialIndex("../data/codepo_gb.zip") + require.NoError(t, err) + cache := memoize.NewMemoizer(0, 0) + handler := PolygonSearch(spatialIdx, cache) + handler(c) + + require.Equal(t, http.StatusBadRequest, w.Code) + require.Contains(t, w.Body.String(), "bbox must have 4 comma-separated values") +} + +// func TestPolygonSearch_InternalError(t *testing.T) { +// gin.SetMode(gin.TestMode) +// w := httptest.NewRecorder() +// c, _ := gin.CreateTestContext(w) +// c.Request = httptest.NewRequest("GET", "/v1/postcode/polygons?bbox=531900,184200,532100,184400", nil) +// c.Request.URL.RawQuery = "bbox=531900,184200,532100,184400" + +// spatialIdx, err := spatialindex.NewCodePointSpatialIndex("../data/codepo_gb.zip") +// require.NoError(t, err) +// cache := memoize.NewMemoizer(0, 0) +// handler := PolygonSearch(spatialIdx, cache) +// handler(c) + +// require.Equal(t, http.StatusInternalServerError, w.Code) +// require.Contains(t, w.Body.String(), "An internal server error occurred") +// } + +// --- parseBBox and isTooBig tests --- +func TestParseBBox(t *testing.T) { + _, err := parseBBox("1,2,3,4") + require.NoError(t, err) + + _, err = parseBBox("1,2,3") + require.Error(t, err) + + _, err = parseBBox("a,b,c,d") + require.Error(t, err) + + _, err = parseBBox("4,3,2,1") + require.Error(t, err) +} + +func TestIsTooBig(t *testing.T) { + require.False(t, isTooBig([]uint32{0, 0, 100, 100})) + require.True(t, isTooBig([]uint32{0, 0, 6000, 100})) + require.True(t, isTooBig([]uint32{0, 0, 100, 6000})) +} From a294235af0912b2b5f7d0324f09755287c2ea0eb Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Sat, 28 Jun 2025 22:52:19 +0100 Subject: [PATCH 08/12] Replace with a mock --- routes/search_test.go | 123 ++++++++++++++++++++++++++---------------- 1 file changed, 77 insertions(+), 46 deletions(-) diff --git a/routes/search_test.go b/routes/search_test.go index 53749c07..cbaedd87 100644 --- a/routes/search_test.go +++ b/routes/search_test.go @@ -1,6 +1,7 @@ package routes import ( + "errors" "net/http" "net/http/httptest" spatialindex "postcode-polygons/spatial-index" @@ -11,6 +12,31 @@ import ( "github.com/stretchr/testify/require" ) +type mockSpatialIndex struct { + SearchFunc func(bounds []uint32) (*[]spatialindex.CodePoint, error) + SearchIterFunc func(bounds []uint32, iter func([2]uint32, [2]uint32, string) bool) error + LenFunc func() int +} + +func (m *mockSpatialIndex) Search(bounds []uint32) (*[]spatialindex.CodePoint, error) { + if m.SearchFunc != nil { + return m.SearchFunc(bounds) + } + return nil, nil +} +func (m *mockSpatialIndex) SearchIter(bounds []uint32, iter func([2]uint32, [2]uint32, string) bool) error { + if m.SearchIterFunc != nil { + return m.SearchIterFunc(bounds, iter) + } + return nil +} +func (m *mockSpatialIndex) Len() int { + if m.LenFunc != nil { + return m.LenFunc() + } + return 0 +} + // --- CodePointSearch tests --- func TestCodePointSearch_BadBBox(t *testing.T) { gin.SetMode(gin.TestMode) @@ -19,8 +45,7 @@ func TestCodePointSearch_BadBBox(t *testing.T) { c.Request = httptest.NewRequest("GET", "/search?bbox=bad,bbox,values", nil) c.Request.URL.RawQuery = "bbox=bad,bbox,values" - spatialIdx, err := spatialindex.NewCodePointSpatialIndex("../data/codepo_gb.zip") - require.NoError(t, err) + spatialIdx := &mockSpatialIndex{} handler := CodePointSearch(spatialIdx) handler(c) @@ -35,8 +60,7 @@ func TestCodePointSearch_TooBig(t *testing.T) { c.Request = httptest.NewRequest("GET", "/search?bbox=0,0,10000,10000", nil) c.Request.URL.RawQuery = "bbox=0,0,10000,10000" - spatialIdx, err := spatialindex.NewCodePointSpatialIndex("../data/codepo_gb.zip") - require.NoError(t, err) + spatialIdx := &mockSpatialIndex{} handler := CodePointSearch(spatialIdx) handler(c) @@ -44,38 +68,43 @@ func TestCodePointSearch_TooBig(t *testing.T) { require.Contains(t, w.Body.String(), "bbox is too large") } -// func TestCodePointSearch_InternalError(t *testing.T) { -// gin.SetMode(gin.TestMode) -// w := httptest.NewRecorder() -// c, _ := gin.CreateTestContext(w) -// c.Request = httptest.NewRequest("GET", "/search?bbox=0,0,1,1", nil) -// c.Request.URL.RawQuery = "bbox=0,0,1,1" - -// spatialIdx := &mockSpatialIndex{ -// SearchFunc: func(bounds []uint32) (*[]spatialindex.CodePoint, error) { -// return nil, errors.New("fail") -// }, -// } -// handler := CodePointSearch(spatialIdx) -// handler(c) - -// require.Equal(t, http.StatusInternalServerError, w.Code) -// require.Contains(t, w.Body.String(), "An internal server error occurred") -// } +func TestCodePointSearch_InternalError(t *testing.T) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/search?bbox=0,0,1,1", nil) + c.Request.URL.RawQuery = "bbox=0,0,1,1" + + spatialIdx := &mockSpatialIndex{ + SearchFunc: func(bounds []uint32) (*[]spatialindex.CodePoint, error) { + return nil, errors.New("fail") + }, + } + handler := CodePointSearch(spatialIdx) + handler(c) + + require.Equal(t, http.StatusInternalServerError, w.Code) + require.Contains(t, w.Body.String(), "An internal server error occurred") +} func TestCodePointSearch_Success(t *testing.T) { gin.SetMode(gin.TestMode) w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) - c.Request = httptest.NewRequest("GET", "/v1/postcode/codepoints?bbox=531900,184200,532100,184400", nil) - c.Request.URL.RawQuery = "bbox=531900,184200,532100,184400" - - spatialIdx, err := spatialindex.NewCodePointSpatialIndex("../data/codepo_gb.zip") - require.NoError(t, err) + c.Request = httptest.NewRequest("GET", "/search?bbox=0,0,1,1", nil) + c.Request.URL.RawQuery = "bbox=0,0,1,1" + + spatialIdx := &mockSpatialIndex{ + SearchFunc: func(bounds []uint32) (*[]spatialindex.CodePoint, error) { + results := []spatialindex.CodePoint{{PostCode: "AB1 2CD", Easting: 1, Northing: 2}} + return &results, nil + }, + } handler := CodePointSearch(spatialIdx) handler(c) + require.Equal(t, http.StatusOK, w.Code) - require.Contains(t, w.Body.String(), "N1 2AJ") + require.Contains(t, w.Body.String(), "AB1 2CD") } // --- PolygonSearch tests --- @@ -86,8 +115,7 @@ func TestPolygonSearch_BadBBox(t *testing.T) { c.Request = httptest.NewRequest("GET", "/polygon?bbox=bad,bbox,values", nil) c.Request.URL.RawQuery = "bbox=bad,bbox,values" - spatialIdx, err := spatialindex.NewCodePointSpatialIndex("../data/codepo_gb.zip") - require.NoError(t, err) + spatialIdx := &mockSpatialIndex{} cache := memoize.NewMemoizer(0, 0) handler := PolygonSearch(spatialIdx, cache) handler(c) @@ -96,22 +124,25 @@ func TestPolygonSearch_BadBBox(t *testing.T) { require.Contains(t, w.Body.String(), "bbox must have 4 comma-separated values") } -// func TestPolygonSearch_InternalError(t *testing.T) { -// gin.SetMode(gin.TestMode) -// w := httptest.NewRecorder() -// c, _ := gin.CreateTestContext(w) -// c.Request = httptest.NewRequest("GET", "/v1/postcode/polygons?bbox=531900,184200,532100,184400", nil) -// c.Request.URL.RawQuery = "bbox=531900,184200,532100,184400" - -// spatialIdx, err := spatialindex.NewCodePointSpatialIndex("../data/codepo_gb.zip") -// require.NoError(t, err) -// cache := memoize.NewMemoizer(0, 0) -// handler := PolygonSearch(spatialIdx, cache) -// handler(c) - -// require.Equal(t, http.StatusInternalServerError, w.Code) -// require.Contains(t, w.Body.String(), "An internal server error occurred") -// } +func TestPolygonSearch_InternalError(t *testing.T) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/polygon?bbox=0,0,1,1", nil) + c.Request.URL.RawQuery = "bbox=0,0,1,1" + + spatialIdx := &mockSpatialIndex{ + SearchIterFunc: func(bounds []uint32, iter func([2]uint32, [2]uint32, string) bool) error { + return errors.New("fail") + }, + } + cache := memoize.NewMemoizer(0, 0) + handler := PolygonSearch(spatialIdx, cache) + handler(c) + + require.Equal(t, http.StatusInternalServerError, w.Code) + require.Contains(t, w.Body.String(), "An internal server error occurred") +} // --- parseBBox and isTooBig tests --- func TestParseBBox(t *testing.T) { From 25da00fa1c60e207427a903c3f8d974bc50c4e36 Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Sat, 28 Jun 2025 22:59:12 +0100 Subject: [PATCH 09/12] change to table tests --- routes/search_test.go | 50 +++++++++++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/routes/search_test.go b/routes/search_test.go index cbaedd87..39d78d04 100644 --- a/routes/search_test.go +++ b/routes/search_test.go @@ -37,7 +37,6 @@ func (m *mockSpatialIndex) Len() int { return 0 } -// --- CodePointSearch tests --- func TestCodePointSearch_BadBBox(t *testing.T) { gin.SetMode(gin.TestMode) w := httptest.NewRecorder() @@ -107,7 +106,6 @@ func TestCodePointSearch_Success(t *testing.T) { require.Contains(t, w.Body.String(), "AB1 2CD") } -// --- PolygonSearch tests --- func TestPolygonSearch_BadBBox(t *testing.T) { gin.SetMode(gin.TestMode) w := httptest.NewRecorder() @@ -144,23 +142,43 @@ func TestPolygonSearch_InternalError(t *testing.T) { require.Contains(t, w.Body.String(), "An internal server error occurred") } -// --- parseBBox and isTooBig tests --- func TestParseBBox(t *testing.T) { - _, err := parseBBox("1,2,3,4") - require.NoError(t, err) - - _, err = parseBBox("1,2,3") - require.Error(t, err) - - _, err = parseBBox("a,b,c,d") - require.Error(t, err) + testCases := []struct { + name string + bboxStr string + expectErr bool + }{ + {name: "valid", bboxStr: "1,2,3,4", expectErr: false}, + {name: "too few parts", bboxStr: "1,2,3", expectErr: true}, + {name: "not numbers", bboxStr: "a,b,c,d", expectErr: true}, + {name: "min greater than max", bboxStr: "4,3,2,1", expectErr: true}, + } - _, err = parseBBox("4,3,2,1") - require.Error(t, err) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := parseBBox(tc.bboxStr) + if tc.expectErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } } func TestIsTooBig(t *testing.T) { - require.False(t, isTooBig([]uint32{0, 0, 100, 100})) - require.True(t, isTooBig([]uint32{0, 0, 6000, 100})) - require.True(t, isTooBig([]uint32{0, 0, 100, 6000})) + testCases := []struct { + name string + bbox []uint32 + expected bool + }{ + {name: "not too big", bbox: []uint32{0, 0, 100, 100}, expected: false}, + {name: "too wide", bbox: []uint32{0, 0, 6000, 100}, expected: true}, + {name: "too high", bbox: []uint32{0, 0, 100, 6000}, expected: true}, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expected, isTooBig(tc.bbox)) + }) + } } From 6279427d8ea6c3bc68d2d84c202f36e8e006c28c Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Sat, 28 Jun 2025 23:05:23 +0100 Subject: [PATCH 10/12] Address PR comments --- routes/search_test.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/routes/search_test.go b/routes/search_test.go index 39d78d04..4068d996 100644 --- a/routes/search_test.go +++ b/routes/search_test.go @@ -144,14 +144,15 @@ func TestPolygonSearch_InternalError(t *testing.T) { func TestParseBBox(t *testing.T) { testCases := []struct { - name string - bboxStr string - expectErr bool + name string + bboxStr string + expectErr bool + errContains string }{ {name: "valid", bboxStr: "1,2,3,4", expectErr: false}, - {name: "too few parts", bboxStr: "1,2,3", expectErr: true}, - {name: "not numbers", bboxStr: "a,b,c,d", expectErr: true}, - {name: "min greater than max", bboxStr: "4,3,2,1", expectErr: true}, + {name: "too few parts", bboxStr: "1,2,3", expectErr: true, errContains: "bbox must have 4 comma-separated values"}, + {name: "not numbers", bboxStr: "a,b,c,d", expectErr: true, errContains: "invalid bbox value"}, + {name: "min greater than max", bboxStr: "4,3,2,1", expectErr: true, errContains: "invalid bbox: min values must be less than or equal to max values"}, } for _, tc := range testCases { @@ -159,6 +160,9 @@ func TestParseBBox(t *testing.T) { _, err := parseBBox(tc.bboxStr) if tc.expectErr { require.Error(t, err) + if tc.errContains != "" { + require.Contains(t, err.Error(), tc.errContains) + } } else { require.NoError(t, err) } From 409c7301b5a34087b965e0d87e1520c1ba9b2494 Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Sun, 29 Jun 2025 00:08:42 +0100 Subject: [PATCH 11/12] Add a service layer abstraction for fetching feature collections --- cmd/api_server.go | 6 +++-- internal/polygons_repo.go | 28 +++++++++++++++++++++ routes/search.go | 13 ++++------ routes/search_test.go | 51 ++++++++++++++++++++++++++++++++++----- 4 files changed, 82 insertions(+), 16 deletions(-) create mode 100644 internal/polygons_repo.go diff --git a/cmd/api_server.go b/cmd/api_server.go index 4ef8e67b..e79fbd8f 100644 --- a/cmd/api_server.go +++ b/cmd/api_server.go @@ -4,6 +4,7 @@ import ( "fmt" "log" "os" + "postcode-polygons/internal" "postcode-polygons/routes" spatialindex "postcode-polygons/spatial-index" "time" @@ -61,9 +62,10 @@ func ApiServer(zipFile string, port int, debug bool) { } cache := memoize.NewMemoizer(5*time.Minute, 10*time.Minute) - + repo := internal.NewPolygonsRepo(cache) + r.GET("/v1/postcode/codepoints", routes.CodePointSearch(idx)) - r.GET("/v1/postcode/polygons", routes.PolygonSearch(idx, cache)) + r.GET("/v1/postcode/polygons", routes.PolygonSearch(idx, repo)) addr := fmt.Sprintf(":%d", port) log.Printf("Starting HTTP API Server on port %d...", port) diff --git a/internal/polygons_repo.go b/internal/polygons_repo.go new file mode 100644 index 00000000..ea0e5f39 --- /dev/null +++ b/internal/polygons_repo.go @@ -0,0 +1,28 @@ +package internal + +import ( + "fmt" + + "github.com/kofalt/go-memoize" + "github.com/paulmach/orb/geojson" +) + +type PolygonsRepo interface { + RetrieveFeatureCollection(postcodeType string, postcodeDistrict string) (*geojson.FeatureCollection, error) +} + +type CachedPolygonsRepo struct { + cache *memoize.Memoizer +} + +func NewPolygonsRepo(cache *memoize.Memoizer) PolygonsRepo { + return &CachedPolygonsRepo{cache: cache} +} + +func (cp *CachedPolygonsRepo) RetrieveFeatureCollection(target string, district string) (*geojson.FeatureCollection, error) { + filename := fmt.Sprintf("./data/postcodes/%s/%s.geojson.bz2", target, district) + featureCollection, err, _ := memoize.Call(cp.cache, filename, func() (*geojson.FeatureCollection, error) { + return DecompressFeatureCollection(filename) + }) + return featureCollection, err +} diff --git a/routes/search.go b/routes/search.go index ef010273..0ef1b290 100644 --- a/routes/search.go +++ b/routes/search.go @@ -13,7 +13,6 @@ import ( "os" "github.com/gin-gonic/gin" - "github.com/kofalt/go-memoize" "github.com/paulmach/orb/geojson" ) @@ -53,7 +52,7 @@ func CodePointSearch(idx spatialindex.SpatialIndex) func(c *gin.Context) { } } -func PolygonSearch(idx spatialindex.SpatialIndex, cache *memoize.Memoizer) func(c *gin.Context) { +func PolygonSearch(idx spatialindex.SpatialIndex, repo internal.PolygonsRepo) func(c *gin.Context) { return func(c *gin.Context) { bbox, err := parseBBox(c.Query("bbox")) if err != nil { @@ -91,16 +90,14 @@ func PolygonSearch(idx spatialindex.SpatialIndex, cache *memoize.Memoizer) func( fc.Features = make([]*geojson.Feature, 0, len(requested)) for district := range districts { - filename := fmt.Sprintf("./data/postcodes/%s/%s.geojson.bz2", target, district) - featureCollection, err, _ := memoize.Call(cache, filename, func() (*geojson.FeatureCollection, error) { - return internal.DecompressFeatureCollection(filename) - }) + + featureCollection, err := repo.RetrieveFeatureCollection(target, district) if err != nil && os.IsNotExist(err) { - log.Printf("polygon file %s does not exist, skipping", filename) + log.Printf("polygon file for district %s does not exist, skipping", district) continue } if err != nil { - log.Printf("error loading feature collection from file %s: %v", filename, err) + log.Printf("error loading feature collection for district %s: %v", district, err) c.JSON(http.StatusInternalServerError, gin.H{"error": "An internal server error occurred"}) return } diff --git a/routes/search_test.go b/routes/search_test.go index 4068d996..9ab189b9 100644 --- a/routes/search_test.go +++ b/routes/search_test.go @@ -8,7 +8,7 @@ import ( "testing" "github.com/gin-gonic/gin" - "github.com/kofalt/go-memoize" + "github.com/paulmach/orb/geojson" "github.com/stretchr/testify/require" ) @@ -37,6 +37,17 @@ func (m *mockSpatialIndex) Len() int { return 0 } +type mockPolygonsRepo struct { + RetrieveFeatureCollectionFunc func(postcodeType string, postcodeDistrict string) (*geojson.FeatureCollection, error) +} + +func (m *mockPolygonsRepo) RetrieveFeatureCollection(postcodeType string, postcodeDistrict string) (*geojson.FeatureCollection, error) { + if m.RetrieveFeatureCollectionFunc != nil { + return m.RetrieveFeatureCollectionFunc(postcodeType, postcodeDistrict) + } + return nil, nil +} + func TestCodePointSearch_BadBBox(t *testing.T) { gin.SetMode(gin.TestMode) w := httptest.NewRecorder() @@ -113,9 +124,7 @@ func TestPolygonSearch_BadBBox(t *testing.T) { c.Request = httptest.NewRequest("GET", "/polygon?bbox=bad,bbox,values", nil) c.Request.URL.RawQuery = "bbox=bad,bbox,values" - spatialIdx := &mockSpatialIndex{} - cache := memoize.NewMemoizer(0, 0) - handler := PolygonSearch(spatialIdx, cache) + handler := PolygonSearch(&mockSpatialIndex{}, &mockPolygonsRepo{}) handler(c) require.Equal(t, http.StatusBadRequest, w.Code) @@ -134,14 +143,44 @@ func TestPolygonSearch_InternalError(t *testing.T) { return errors.New("fail") }, } - cache := memoize.NewMemoizer(0, 0) - handler := PolygonSearch(spatialIdx, cache) + handler := PolygonSearch(spatialIdx, &mockPolygonsRepo{}) handler(c) require.Equal(t, http.StatusInternalServerError, w.Code) require.Contains(t, w.Body.String(), "An internal server error occurred") } +func TestPolygonSearch_Success(t *testing.T) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/polygon?bbox=0,0,1,1", nil) + c.Request.URL.RawQuery = "bbox=0,0,1,1" + + spatialIdx := &mockSpatialIndex{ + SearchIterFunc: func(bounds []uint32, iter func([2]uint32, [2]uint32, string) bool) error { + iter([2]uint32{0, 0}, [2]uint32{1, 1}, "AB1 2CD") + return nil + }, + } + + repo := &mockPolygonsRepo{ + RetrieveFeatureCollectionFunc: func(postcodeType string, postcodeDistrict string) (*geojson.FeatureCollection, error) { + fc := geojson.NewFeatureCollection() + feature := geojson.NewFeature(nil) + feature.ID = "AB1 2CD" + fc.Append(feature) + return fc, nil + }, + } + + handler := PolygonSearch(spatialIdx, repo) + handler(c) + + require.Equal(t, http.StatusOK, w.Code) + require.Contains(t, w.Body.String(), "AB1 2CD") +} + func TestParseBBox(t *testing.T) { testCases := []struct { name string From 3ff281e5e2dbfb70f3cb250112aa82c5763d4b52 Mon Sep 17 00:00:00 2001 From: Richard Hull Date: Sun, 29 Jun 2025 00:12:22 +0100 Subject: [PATCH 12/12] align names --- internal/polygons_repo.go | 2 +- routes/search_test.go | 66 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/internal/polygons_repo.go b/internal/polygons_repo.go index ea0e5f39..5e960ead 100644 --- a/internal/polygons_repo.go +++ b/internal/polygons_repo.go @@ -8,7 +8,7 @@ import ( ) type PolygonsRepo interface { - RetrieveFeatureCollection(postcodeType string, postcodeDistrict string) (*geojson.FeatureCollection, error) + RetrieveFeatureCollection(target string, district string) (*geojson.FeatureCollection, error) } type CachedPolygonsRepo struct { diff --git a/routes/search_test.go b/routes/search_test.go index 9ab189b9..d0ee6267 100644 --- a/routes/search_test.go +++ b/routes/search_test.go @@ -4,6 +4,7 @@ import ( "errors" "net/http" "net/http/httptest" + "os" spatialindex "postcode-polygons/spatial-index" "testing" @@ -38,12 +39,12 @@ func (m *mockSpatialIndex) Len() int { } type mockPolygonsRepo struct { - RetrieveFeatureCollectionFunc func(postcodeType string, postcodeDistrict string) (*geojson.FeatureCollection, error) + RetrieveFeatureCollectionFunc func(target string, district string) (*geojson.FeatureCollection, error) } -func (m *mockPolygonsRepo) RetrieveFeatureCollection(postcodeType string, postcodeDistrict string) (*geojson.FeatureCollection, error) { +func (m *mockPolygonsRepo) RetrieveFeatureCollection(target string, district string) (*geojson.FeatureCollection, error) { if m.RetrieveFeatureCollectionFunc != nil { - return m.RetrieveFeatureCollectionFunc(postcodeType, postcodeDistrict) + return m.RetrieveFeatureCollectionFunc(target, district) } return nil, nil } @@ -165,7 +166,10 @@ func TestPolygonSearch_Success(t *testing.T) { } repo := &mockPolygonsRepo{ - RetrieveFeatureCollectionFunc: func(postcodeType string, postcodeDistrict string) (*geojson.FeatureCollection, error) { + RetrieveFeatureCollectionFunc: func(target string, district string) (*geojson.FeatureCollection, error) { + require.Equal(t, "units", target) + require.Equal(t, "AB1", district) + // Simulate a successful retrieval of a feature collection fc := geojson.NewFeatureCollection() feature := geojson.NewFeature(nil) feature.ID = "AB1 2CD" @@ -181,6 +185,60 @@ func TestPolygonSearch_Success(t *testing.T) { require.Contains(t, w.Body.String(), "AB1 2CD") } +func TestPolygonSearch_PolygonNotFound(t *testing.T) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/polygon?bbox=0,0,1,1", nil) + c.Request.URL.RawQuery = "bbox=0,0,1,1" + + spatialIdx := &mockSpatialIndex{ + SearchIterFunc: func(bounds []uint32, iter func([2]uint32, [2]uint32, string) bool) error { + iter([2]uint32{0, 0}, [2]uint32{1, 1}, "AB1 2CD") + return nil + }, + } + + repo := &mockPolygonsRepo{ + RetrieveFeatureCollectionFunc: func(target string, district string) (*geojson.FeatureCollection, error) { + return nil, os.ErrNotExist + }, + } + + handler := PolygonSearch(spatialIdx, repo) + handler(c) + + require.Equal(t, http.StatusOK, w.Code) + require.NotContains(t, w.Body.String(), "AB1 2CD") +} + +func TestPolygonSearch_PolygonRepoError(t *testing.T) { + gin.SetMode(gin.TestMode) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/polygon?bbox=0,0,1,1", nil) + c.Request.URL.RawQuery = "bbox=0,0,1,1" + + spatialIdx := &mockSpatialIndex{ + SearchIterFunc: func(bounds []uint32, iter func([2]uint32, [2]uint32, string) bool) error { + iter([2]uint32{0, 0}, [2]uint32{1, 1}, "AB1 2CD") + return nil + }, + } + + repo := &mockPolygonsRepo{ + RetrieveFeatureCollectionFunc: func(target string, district string) (*geojson.FeatureCollection, error) { + return nil, errors.New("failed to load polygon") + }, + } + + handler := PolygonSearch(spatialIdx, repo) + handler(c) + + require.Equal(t, http.StatusInternalServerError, w.Code) + require.Contains(t, w.Body.String(), "An internal server error occurred") +} + func TestParseBBox(t *testing.T) { testCases := []struct { name string