From f2d01acf582b91cdc5fe5cca2ce99ab309e466c1 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Sun, 7 Dec 2025 18:59:57 -0300 Subject: [PATCH 1/8] refactor: main tests --- cmd/api/main.go | 25 +++++--- cmd/api/main_test.go | 147 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 8 deletions(-) create mode 100644 cmd/api/main_test.go diff --git a/cmd/api/main.go b/cmd/api/main.go index 8efbf0b..e9475df 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -14,16 +14,11 @@ import ( var json = jsoniter.ConfigCompatibleWithStandardLibrary -func main() { - geoService, err := geoip.NewService("data/city.db") +func NewServer(dbPath string) (*echo.Echo, *geoip.Service, error) { + geoService, err := geoip.NewService(dbPath) if err != nil { - log.Fatalf("Failed to initialize GeoIP service: %v", err) + return nil, nil, err } - defer func() { - if err := geoService.DB.Close(); err != nil { - log.Printf("Failed to close GeoIP database: %v", err) - } - }() handler := &handlers.GeoIPHandler{ GeoService: geoService, @@ -35,6 +30,20 @@ func main() { e.GET("/health", handlers.HealthCheck) e.GET("/lookup/:ip", handler.Lookup) + return e, geoService, nil +} + +func main() { + e, geoService, err := NewServer("data/city.db") + if err != nil { + log.Fatalf("Failed to initialize GeoIP service: %v", err) + } + defer func() { + if err := geoService.DB.Close(); err != nil { + log.Printf("Failed to close GeoIP database: %v", err) + } + }() + port := os.Getenv("PORT") if port == "" { port = "8080" diff --git a/cmd/api/main_test.go b/cmd/api/main_test.go new file mode 100644 index 0000000..dabafd9 --- /dev/null +++ b/cmd/api/main_test.go @@ -0,0 +1,147 @@ +package main + +import ( + "bytes" + "net/http" + "net/http/httptest" + "os" + "testing" + + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestJSONSerializer_Serialize covers serialization scenarios using a table-driven approach. +func TestJSONSerializer_Serialize(t *testing.T) { + e := echo.New() + serializer := &JSONSerializer{} + e.JSONSerializer = serializer + + tests := []struct { + name string + input interface{} + indent string + expectedBody string + }{ + { + name: "Default Serialization", + input: map[string]string{"hello": "world"}, + indent: "", + expectedBody: `{"hello":"world"}`, + }, + { + name: "Indented Serialization", + input: map[string]string{"hello": "world"}, + indent: " ", + expectedBody: "{\n \"hello\": \"world\"\n}\n", + }, + { + name: "Complex Struct", + input: struct{ ID int }{ID: 1}, + indent: "", + expectedBody: `{"ID":1}`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + err := serializer.Serialize(c, tc.input, tc.indent) + assert.NoError(t, err) + assert.JSONEq(t, tc.expectedBody, rec.Body.String()) + }) + } +} + +// TestJSONSerializer_Deserialize covers deserialization scenarios. +func TestJSONSerializer_Deserialize(t *testing.T) { + e := echo.New() + serializer := &JSONSerializer{} + e.JSONSerializer = serializer + + t.Run("Success", func(t *testing.T) { + jsonBody := `{"hello":"world"}` + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewBufferString(jsonBody)) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + var result map[string]string + err := serializer.Deserialize(c, &result) + require.NoError(t, err) + assert.Equal(t, "world", result["hello"]) + }) + + t.Run("Malformed JSON", func(t *testing.T) { + jsonBody := `{"hello":` // Invalid JSON + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewBufferString(jsonBody)) + rec := httptest.NewRecorder() + c := e.NewContext(req, rec) + + var result map[string]string + err := serializer.Deserialize(c, &result) + assert.Error(t, err) + }) +} + +func TestNewServer(t *testing.T) { + dbPath := "../../data/city.db" + + t.Run("Success Initialization", func(t *testing.T) { + // Check if file exists before trying, to avoid failing in CI environments without the DB. + if _, err := os.Stat(dbPath); os.IsNotExist(err) { + t.Skipf("Skipping test: Database file not found at %s", dbPath) + } + + e, svc, err := NewServer(dbPath) + require.NoError(t, err) + require.NotNil(t, e) + require.NotNil(t, svc) + + // Clean up + svc.DB.Close() + + // Verify Routes are Registered + foundRoutes := 0 + for _, r := range e.Routes() { + if r.Path == "/health" && r.Method == http.MethodGet { + foundRoutes++ + } + if r.Path == "/lookup/:ip" && r.Method == http.MethodGet { + foundRoutes++ + } + } + assert.Equal(t, 2, foundRoutes, "Expected /health and /lookup/:ip routes to be registered") + }) + + t.Run("Failure Invalid Path", func(t *testing.T) { + e, svc, err := NewServer("invalid/path/to/db.mmdb") + assert.Error(t, err) + assert.Nil(t, e) + assert.Nil(t, svc) + }) +} + +func TestHealthCheck_Integration(t *testing.T) { + dbPath := "../../data/city.db" + if _, err := os.Stat(dbPath); os.IsNotExist(err) { + t.Skip("Skipping integration test: Database not found") + } + + e, svc, err := NewServer(dbPath) + require.NoError(t, err) + defer svc.DB.Close() + + req := httptest.NewRequest(http.MethodGet, "/health", nil) + rec := httptest.NewRecorder() + + // Inject request into Echo + e.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), "status") +} From c6f57388913f6500b106dab0d1cfe4a0db70edea Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Sun, 7 Dec 2025 19:00:07 -0300 Subject: [PATCH 2/8] chore: increase reader coverage --- internal/geoip/reader.go | 51 ------ internal/geoip/reader_test.go | 330 +++++++++++++++++++++++++++++----- 2 files changed, 284 insertions(+), 97 deletions(-) diff --git a/internal/geoip/reader.go b/internal/geoip/reader.go index 3e5c3b9..ca6d33e 100644 --- a/internal/geoip/reader.go +++ b/internal/geoip/reader.go @@ -1,54 +1,3 @@ -// Package geoip2 provides an easy-to-use API for the MaxMind GeoIP2 and -// GeoLite2 databases; this package does not support GeoIP Legacy databases. -// -// # Basic Usage -// -// db, err := geoip2.Open("GeoIP2-City.mmdb") -// if err != nil { -// log.Fatal(err) -// } -// defer db.Close() -// -// ip, err := netip.ParseAddr("81.2.69.142") -// if err != nil { -// log.Fatal(err) -// } -// -// record, err := db.City(ip) -// if err != nil { -// log.Fatal(err) -// } -// -// if !record.HasData() { -// fmt.Println("No data found for this IP") -// return -// } -// -// fmt.Printf("City: %v\n", record.City.Names.English) -// fmt.Printf("Country: %v\n", record.Country.Names.English) -// -// # Database Types -// -// This library supports all MaxMind database types: -// - City: Most comprehensive geolocation data -// - Country: Country-level geolocation -// - ASN: Autonomous system information -// - AnonymousIP: Anonymous network detection -// - Enterprise: Enhanced City data with additional fields -// - ISP: Internet service provider information -// - Domain: Second-level domain data -// - ConnectionType: Connection type identification -// -// # Version 2.0 Features -// -// Version 2.0 introduces significant improvements: -// - Modern API using netip.Addr instead of net.IP -// - Network and IPAddress fields in all result structs -// - HasData() method for data validation -// - Structured Names type for localized names -// - JSON serialization support -// -// See github.com/oschwald/maxminddb-golang/v2 for more advanced use cases. package geoip import ( diff --git a/internal/geoip/reader_test.go b/internal/geoip/reader_test.go index 0c40995..c53bb36 100644 --- a/internal/geoip/reader_test.go +++ b/internal/geoip/reader_test.go @@ -1,68 +1,306 @@ package geoip import ( + "net/netip" + "os" "testing" + "github.com/oschwald/maxminddb-golang/v2" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestOpenInvalidPath(t *testing.T) { - _, err := Open("nonexistent/path/to/database.mmdb") - if err == nil { - t.Error("Expected error when opening non-existent database, got nil") +func TestModels_HasData(t *testing.T) { + f := 1.0 + + tests := []struct { + name string + model interface{ HasData() bool } + hasData bool + msg string + }{ + {"Names (Empty)", Names{}, false, "Empty Names"}, + {"Names (Full)", Names{English: "A"}, true, "Populated Names"}, + {"Continent (Empty)", Continent{}, false, "Empty Continent"}, + {"Continent (Full)", Continent{Code: "NA"}, true, "Populated Continent"}, + {"Location (Empty)", Location{}, false, "Empty Location"}, + {"Location (Full)", Location{TimeZone: "UTC"}, true, "Populated Location"}, + {"Enterprise (Empty)", Enterprise{}, false, "Empty Enterprise"}, + {"Enterprise (Full)", Enterprise{City: EnterpriseCityRecord{GeoNameID: 1}}, true, "Populated Enterprise"}, + {"City (Empty)", City{}, false, "Empty City"}, + {"City (Full)", City{Postal: CityPostal{Code: "123"}}, true, "Populated City"}, + {"Country (Empty)", Country{}, false, "Empty Country"}, + {"Country (Full)", Country{Country: CountryRecord{ISOCode: "US"}}, true, "Populated Country"}, + {"ASN (Empty)", ASN{}, false, "Empty ASN"}, + {"ASN (Full)", ASN{AutonomousSystemNumber: 123}, true, "Populated ASN"}, + {"ISP (Empty)", ISP{}, false, "Empty ISP"}, + {"ISP (Full)", ISP{ISP: "Comcast"}, true, "Populated ISP"}, + {"Domain (Empty)", Domain{}, false, "Empty Domain"}, + {"Domain (Full)", Domain{Domain: "google.com"}, true, "Populated Domain"}, + {"ConnectionType (Empty)", ConnectionType{}, false, "Empty ConnectionType"}, + {"ConnectionType (Full)", ConnectionType{ConnectionType: "Cable"}, true, "Populated ConnectionType"}, + {"AnonymousIP (Empty)", AnonymousIP{}, false, "Empty AnonymousIP"}, + {"AnonymousIP (Full)", AnonymousIP{IsAnonymous: true}, true, "Populated AnonymousIP"}, + {"CityRecord", CityRecord{GeoNameID: 1}, true, "CityRecord"}, + {"CityPostal", CityPostal{Code: "1"}, true, "CityPostal"}, + {"CitySubdivision", CitySubdivision{GeoNameID: 1}, true, "CitySubdivision"}, + {"CityTraits", CityTraits{IsAnycast: true}, true, "CityTraits"}, + {"CountryRecord", CountryRecord{GeoNameID: 1}, true, "CountryRecord"}, + {"CountryTraits", CountryTraits{IsAnycast: true}, true, "CountryTraits"}, + {"RepresentedCountry", RepresentedCountry{GeoNameID: 1}, true, "RepresentedCountry"}, + {"EnterpriseCityRecord", EnterpriseCityRecord{GeoNameID: 1}, true, "EnterpriseCityRecord"}, + {"EnterprisePostal", EnterprisePostal{Code: "1"}, true, "EnterprisePostal"}, + {"EnterpriseSubdivision", EnterpriseSubdivision{GeoNameID: 1}, true, "EnterpriseSubdivision"}, + {"EnterpriseCountryRecord", EnterpriseCountryRecord{GeoNameID: 1}, true, "EnterpriseCountryRecord"}, + {"EnterpriseTraits", EnterpriseTraits{ISP: "x"}, true, "EnterpriseTraits"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.hasData, tt.model.HasData(), tt.msg) + }) } + + t.Run("Location Coordinates Logic", func(t *testing.T) { + assert.False(t, Location{}.HasCoordinates(), "Empty location has no coords") + assert.False(t, Location{Latitude: &f}.HasCoordinates(), "Partial coords return false") + assert.True(t, Location{Latitude: &f, Longitude: &f}.HasCoordinates(), "Full coords return true") + }) + + t.Run("Enterprise hasSubdivisionsData", func(t *testing.T) { + e := Enterprise{} + assert.False(t, e.HasData()) + e.Subdivisions = []EnterpriseSubdivision{{}, {}} // Empty subdivisions + assert.False(t, e.HasData()) + e.Subdivisions = []EnterpriseSubdivision{{}, {GeoNameID: 1}} // One valid + assert.True(t, e.HasData()) + }) + + t.Run("City hasSubdivisionsData", func(t *testing.T) { + c := City{} + assert.False(t, c.HasData()) + c.Subdivisions = []CitySubdivision{{}, {}} + assert.False(t, c.HasData()) + c.Subdivisions = []CitySubdivision{{GeoNameID: 1}} + assert.True(t, c.HasData()) + }) } -func TestNamesHasData(t *testing.T) { - var emptyNames Names - assert.False(t, emptyNames.HasData(), "Empty Names should not have data") +func TestGetDBType(t *testing.T) { + tests := []struct { + name string + dbTypeStr string + expected databaseType + expectErr bool + }{ + {"Anonymous IP", "GeoIP2-Anonymous-IP", isAnonymousIP, false}, + {"ASN", "GeoLite2-ASN", isASN, false}, + {"City", "GeoIP2-City", isCity | isCountry, false}, + {"Country", "GeoIP2-Country", isCity | isCountry, false}, + {"Connection Type", "GeoIP2-Connection-Type", isConnectionType, false}, + {"Domain", "GeoIP2-Domain", isDomain, false}, + {"Enterprise", "GeoIP2-Enterprise", isEnterprise | isCity | isCountry, false}, + {"ISP", "GeoIP2-ISP", isISP | isASN, false}, + {"Unknown", "Alien-Database", 0, true}, + } - nonEmptyNames := Names{English: "Test"} - assert.True(t, nonEmptyNames.HasData(), "Names with data should have data") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reader := &maxminddb.Reader{ + Metadata: maxminddb.Metadata{DatabaseType: tt.dbTypeStr}, + } + got, err := getDBType(reader) + if tt.expectErr { + assert.Error(t, err) + assert.IsType(t, UnknownDatabaseTypeError{}, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.expected, got) + } + }) + } } -func TestAllStructsHaveHasData(t *testing.T) { - // Ensure all result structs have HasData methods - var city City - var country Country - var enterprise Enterprise - var anonymousIP AnonymousIP - var asn ASN - var connectionType ConnectionType - var domain Domain - var isp ISP - var names Names - - // These should all compile and return false for zero values (no data) - assert.False(t, city.HasData()) - assert.False(t, country.HasData()) - assert.False(t, enterprise.HasData()) - assert.False(t, anonymousIP.HasData()) - assert.False(t, asn.HasData()) - assert.False(t, connectionType.HasData()) - assert.False(t, domain.HasData()) - assert.False(t, isp.HasData()) - assert.False(t, names.HasData()) +func TestReader_MethodRestrictions(t *testing.T) { + ip := netip.MustParseAddr("8.8.8.8") + mockMmdb := &maxminddb.Reader{Metadata: maxminddb.Metadata{DatabaseType: "MockDB"}} + + tests := []struct { + methodName string + dbType databaseType + action func(r *Reader) error + }{ + {"Enterprise", isCity, func(r *Reader) error { _, err := r.Enterprise(ip); return err }}, + {"City", isASN, func(r *Reader) error { _, err := r.City(ip); return err }}, + {"Country", isASN, func(r *Reader) error { _, err := r.Country(ip); return err }}, + {"AnonymousIP", isCity, func(r *Reader) error { _, err := r.AnonymousIP(ip); return err }}, + {"ASN", isCity, func(r *Reader) error { _, err := r.ASN(ip); return err }}, + {"ConnectionType", isCity, func(r *Reader) error { _, err := r.ConnectionType(ip); return err }}, + {"Domain", isCity, func(r *Reader) error { _, err := r.Domain(ip); return err }}, + {"ISP", isCity, func(r *Reader) error { _, err := r.ISP(ip); return err }}, + } + + for _, tt := range tests { + t.Run(tt.methodName, func(t *testing.T) { + r := &Reader{mmdbReader: mockMmdb, databaseType: tt.dbType} + err := tt.action(r) + require.Error(t, err) + assert.IsType(t, InvalidMethodError{}, err) + }) + } +} + +func TestErrors_Formatting(t *testing.T) { + e1 := InvalidMethodError{Method: "M", DatabaseType: "DB"} + assert.Equal(t, "geoip2: the M method does not support the DB database", e1.Error()) + e2 := UnknownDatabaseTypeError{DatabaseType: "BadDB"} + assert.Equal(t, `geoip2: reader does not support the "BadDB" database type`, e2.Error()) +} + +func TestApplyOptions(t *testing.T) { + var called bool + opt := func(o *readerOptions) { called = true } + opts := applyOptions([]Option{opt, nil}) + assert.IsType(t, []maxminddb.ReaderOption{}, opts) + assert.True(t, called) +} + +func setupIntegration(t *testing.T) string { + t.Helper() + dbPath := "../../data/city.db" + if _, err := os.Stat(dbPath); os.IsNotExist(err) { + t.Skipf("Skipping integration test: database not found at %s", dbPath) + } + return dbPath +} + +func TestReader_Integration_HappyPath(t *testing.T) { + dbPath := setupIntegration(t) + ip := netip.MustParseAddr("8.8.8.8") + + t.Run("Open and Lookup", func(t *testing.T) { + r, err := Open(dbPath) + require.NoError(t, err) + defer r.Close() + + city, err := r.City(ip) + assert.NoError(t, err) + assert.True(t, city.HasData()) + + country, err := r.Country(ip) + assert.NoError(t, err) + assert.True(t, country.HasData()) + }) + + t.Run("OpenBytes", func(t *testing.T) { + b, err := os.ReadFile(dbPath) + require.NoError(t, err) + r, err := OpenBytes(b) + require.NoError(t, err) + r.Close() + }) + + t.Run("Invalid Path/Bytes", func(t *testing.T) { + _, err := Open("invalid.mmdb") + assert.Error(t, err) + _, err = OpenBytes([]byte("bad")) + assert.Error(t, err) + }) } -func TestASNHasData(t *testing.T) { - var emptyASN ASN - assert.False(t, emptyASN.HasData(), "Empty ASN should not have data") +func TestReader_ForcedExecution(t *testing.T) { + dbPath := setupIntegration(t) + realReader, err := maxminddb.Open(dbPath) + require.NoError(t, err) + defer realReader.Close() + + ip := netip.MustParseAddr("8.8.8.8") + + tests := []struct { + name string + forcedType databaseType + action func(r *Reader) (any, error) + }{ + {"Enterprise", isEnterprise, func(r *Reader) (any, error) { return r.Enterprise(ip) }}, + {"ASN", isASN, func(r *Reader) (any, error) { return r.ASN(ip) }}, + {"ISP", isISP, func(r *Reader) (any, error) { return r.ISP(ip) }}, + {"Domain", isDomain, func(r *Reader) (any, error) { return r.Domain(ip) }}, + {"ConnectionType", isConnectionType, func(r *Reader) (any, error) { return r.ConnectionType(ip) }}, + {"AnonymousIP", isAnonymousIP, func(r *Reader) (any, error) { return r.AnonymousIP(ip) }}, + } - nonEmptyASN := ASN{AutonomousSystemNumber: 123} - assert.True(t, nonEmptyASN.HasData(), "ASN with data should have data") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := &Reader{ + mmdbReader: realReader, + databaseType: tt.forcedType, + } + result, err := tt.action(r) + assert.NotNil(t, result) + _ = err + }) + } } -func TestLocationHasCoordinates(t *testing.T) { - var emptyLocation Location - assert.False(t, emptyLocation.HasCoordinates(), "Empty Location should not have coordinates") +func TestReader_DecodeErrors(t *testing.T) { + dbPath := setupIntegration(t) + data, err := os.ReadFile(dbPath) + require.NoError(t, err) + + dbBytes := make([]byte, len(data)) + copy(dbBytes, data) + + mmdb, _ := maxminddb.OpenBytes(dbBytes) + meta := mmdb.Metadata + mmdb.Close() - lat := 51.5142 - lon := -0.0931 - locationWithCoords := Location{Latitude: &lat, Longitude: &lon} - assert.True(t, locationWithCoords.HasCoordinates(), "Location with lat/lon should have coordinates") + treeSizeBits := uint(meta.NodeCount) * uint(meta.RecordSize) + treeSizeBytes := treeSizeBits / 8 - // Only latitude - locationOnlyLat := Location{Latitude: &lat} - assert.False(t, locationOnlyLat.HasCoordinates(), "Location with only latitude should not have coordinates") + dataStart := int(treeSizeBytes) + 16 + + dataEnd := len(dbBytes) - 5000 + + if dataStart >= dataEnd { + t.Skip("Database file too small to safely corrupt data section without touching metadata") + } + + for i := dataStart; i < dataEnd; i++ { + dbBytes[i] = 0xFF + } + + r, err := OpenBytes(dbBytes) + require.NoError(t, err, "OpenBytes should succeed because metadata at EOF is intact") + defer r.Close() + + ip := netip.MustParseAddr("8.8.8.8") + + tests := []struct { + name string + forcedType databaseType + action func(r *Reader) error + }{ + {"City", isCity, func(r *Reader) error { _, err := r.City(ip); return err }}, + {"Country", isCountry, func(r *Reader) error { _, err := r.Country(ip); return err }}, + {"Enterprise", isEnterprise, func(r *Reader) error { _, err := r.Enterprise(ip); return err }}, + {"ASN", isASN, func(r *Reader) error { _, err := r.ASN(ip); return err }}, + {"ISP", isISP, func(r *Reader) error { _, err := r.ISP(ip); return err }}, + {"Domain", isDomain, func(r *Reader) error { _, err := r.Domain(ip); return err }}, + {"Connection", isConnectionType, func(r *Reader) error { _, err := r.ConnectionType(ip); return err }}, + {"AnonIP", isAnonymousIP, func(r *Reader) error { _, err := r.AnonymousIP(ip); return err }}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + originalType := r.databaseType + r.databaseType = tt.forcedType + defer func() { r.databaseType = originalType }() + + err := tt.action(r) + + assert.Error(t, err, "Expected error due to corrupted data section") + + assert.IsNotType(t, InvalidMethodError{}, err) + }) + } } From 5df863b20b397952aca8c155a26db5518d6e9d8a Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Sun, 7 Dec 2025 19:00:34 -0300 Subject: [PATCH 3/8] refactor: service tests --- internal/geoip/service_test.go | 114 ++++++++++++++++++++++------ internal/handlers/benchmark_test.go | 76 ------------------- internal/handlers/http_test.go | 60 +++++++++++++++ 3 files changed, 151 insertions(+), 99 deletions(-) delete mode 100644 internal/handlers/benchmark_test.go diff --git a/internal/geoip/service_test.go b/internal/geoip/service_test.go index dcc9ec3..1aea28c 100644 --- a/internal/geoip/service_test.go +++ b/internal/geoip/service_test.go @@ -1,41 +1,109 @@ package geoip import ( - "net/netip" + "os" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestParseIP(t *testing.T) { +func TestNewService(t *testing.T) { + t.Run("Invalid Path", func(t *testing.T) { + svc, err := NewService("nonexistent/path/to/database.mmdb") + assert.Error(t, err) + assert.Nil(t, svc) + }) + + t.Run("Success Integration", func(t *testing.T) { + dbPath := "../../data/city.db" + if _, err := os.Stat(dbPath); os.IsNotExist(err) { + t.Skipf("Skipping integration test: database not found at %s", dbPath) + } + + svc, err := NewService(dbPath) + require.NoError(t, err) + assert.NotNil(t, svc) + assert.NotNil(t, svc.DB) + + // Cleanup + svc.DB.Close() + }) +} + +func TestLookupIP_Validation(t *testing.T) { + svc := &Service{DB: nil} + tests := []struct { - name string - ipStr string - isValid bool + name string + ipStr string }{ - {"valid IPv4", "8.8.8.8", true}, - {"valid IPv6", "2001:4860:4860::8888", true}, - {"valid localhost", "127.0.0.1", true}, - {"invalid IP", "invalid", false}, - {"empty string", "", false}, - {"partial IP", "192.168", false}, - {"IP with port", "8.8.8.8:80", false}, + {"Alphabetical", "invalid-ip"}, + {"Empty", ""}, + {"Partial IPv4", "192.168"}, + {"IPv4 with Port", "8.8.8.8:80"}, + {"IPv6 with Port", "[2001:db8::1]:80"}, + {"Out of Range", "300.300.300.300"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := netip.ParseAddr(tt.ipStr) - if tt.isValid && err != nil { - t.Errorf("Expected valid IP for '%s', but got error: %v", tt.ipStr, err) - } - if !tt.isValid && err == nil { - t.Errorf("Expected invalid IP for '%s', but got no error", tt.ipStr) - } + city, err := svc.LookupIP(tt.ipStr) + + assert.ErrorIs(t, err, ErrInvalidIP, "Should return sentinel error for invalid IP") + assert.Nil(t, city) }) } } -func TestNewServiceInvalidPath(t *testing.T) { - _, err := NewService("nonexistent/path/to/database.mmdb") - if err == nil { - t.Error("Expected error when opening non-existent database, got nil") +func TestLookupIP_Integration(t *testing.T) { + dbPath := "../../data/city.db" + if _, err := os.Stat(dbPath); os.IsNotExist(err) { + t.Skipf("Skipping integration test: database not found at %s", dbPath) + } + + svc, err := NewService(dbPath) + require.NoError(t, err) + defer svc.DB.Close() + + tests := []struct { + name string + ipStr string + expectFound bool + expectedISO string + }{ + { + name: "Valid Google DNS (US)", + ipStr: "8.8.8.8", + expectFound: true, + expectedISO: "US", // Usually resolves to US + }, + { + name: "Valid Cloudflare DNS (IPv6)", + ipStr: "2606:4700:4700::1111", + expectFound: true, + }, + { + name: "Valid Localhost (No Data)", + ipStr: "127.0.0.1", + expectFound: false, // Localhost isn't in GeoIP DBs + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + city, err := svc.LookupIP(tt.ipStr) + require.NoError(t, err, "Lookup should not return error for valid IP format") + require.NotNil(t, city, "City struct should never be nil even if empty") + + if tt.expectFound { + assert.True(t, city.HasData(), "Expected data for IP %s", tt.ipStr) + if tt.expectedISO != "" { + assert.Equal(t, tt.expectedISO, city.Country.ISOCode) + } + } else { + assert.False(t, city.HasData(), "Expected no data for IP %s", tt.ipStr) + } + }) } } diff --git a/internal/handlers/benchmark_test.go b/internal/handlers/benchmark_test.go deleted file mode 100644 index 6ea0d1b..0000000 --- a/internal/handlers/benchmark_test.go +++ /dev/null @@ -1,76 +0,0 @@ -package handlers - -import ( - "net/http" - "net/http/httptest" - "testing" - - "github.com/gustavosett/WhereGo/internal/geoip" - "github.com/labstack/echo/v4" -) - -const benchDBPath = "../../data/city.db" - -func BenchmarkLookup(b *testing.B) { - service, err := geoip.NewService(benchDBPath) - if err != nil { - b.Skip("Database not available") - } - defer func() { - if err := service.DB.Close(); err != nil { - b.Logf("Failed to close database: %v", err) - } - }() - - e := echo.New() - e.GET("/lookup/:ip", (&GeoIPHandler{GeoService: service}).Lookup) - - b.ResetTimer() - b.ReportAllocs() - - for i := 0; i < b.N; i++ { - req := httptest.NewRequest(http.MethodGet, "/lookup/8.8.8.8", nil) - rec := httptest.NewRecorder() - e.ServeHTTP(rec, req) - } -} - -func BenchmarkLookupParallel(b *testing.B) { - service, err := geoip.NewService(benchDBPath) - if err != nil { - b.Skip("Database not available") - } - defer func() { - if err := service.DB.Close(); err != nil { - b.Logf("Failed to close database: %v", err) - } - }() - - e := echo.New() - e.GET("/lookup/:ip", (&GeoIPHandler{GeoService: service}).Lookup) - - b.ResetTimer() - b.ReportAllocs() - - b.RunParallel(func(pb *testing.PB) { - for pb.Next() { - req := httptest.NewRequest(http.MethodGet, "/lookup/8.8.8.8", nil) - rec := httptest.NewRecorder() - e.ServeHTTP(rec, req) - } - }) -} - -func BenchmarkHealth(b *testing.B) { - e := echo.New() - e.GET("/health", HealthCheck) - - b.ResetTimer() - b.ReportAllocs() - - for i := 0; i < b.N; i++ { - req := httptest.NewRequest(http.MethodGet, "/health", nil) - rec := httptest.NewRecorder() - e.ServeHTTP(rec, req) - } -} diff --git a/internal/handlers/http_test.go b/internal/handlers/http_test.go index 5289bc7..c7542da 100644 --- a/internal/handlers/http_test.go +++ b/internal/handlers/http_test.go @@ -1,12 +1,14 @@ package handlers import ( + "encoding/json" "io" "net/http" "net/http/httptest" "strings" "testing" + "github.com/gustavosett/WhereGo/internal/geoip" "github.com/labstack/echo/v4" ) @@ -36,3 +38,61 @@ func TestHealthCheck(t *testing.T) { t.Errorf("Expected Content-Type 'application/json', got '%s'", contentType) } } + +func TestLookupIntegration(t *testing.T) { + dbPath := "../../data/city.db" + service, err := geoip.NewService(dbPath) + if err != nil { + t.Skipf("Skipping integration test: could not open database at %s: %v", dbPath, err) + } + defer service.DB.Close() + + h := &GeoIPHandler{GeoService: service} + e := echo.New() + e.GET("/lookup/:ip", h.Lookup) + + // Test valid IP + req := httptest.NewRequest(http.MethodGet, "/lookup/8.8.8.8", nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Errorf("Expected status 200 for valid IP, got %d", rec.Code) + } + + var result geoip.City + if err := json.Unmarshal(rec.Body.Bytes(), &result); err != nil { + t.Errorf("Failed to unmarshal response: %v", err) + } + + // Test invalid IP + req = httptest.NewRequest(http.MethodGet, "/lookup/invalid-ip", nil) + rec = httptest.NewRecorder() + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Errorf("Expected status 400 for invalid IP, got %d", rec.Code) + } +} + +func TestLookupDBError(t *testing.T) { + dbPath := "../../data/city.db" + service, err := geoip.NewService(dbPath) + if err != nil { + t.Skipf("Skipping integration test: could not open database at %s: %v", dbPath, err) + } + // Close immediately to simulate error + service.DB.Close() + + h := &GeoIPHandler{GeoService: service} + e := echo.New() + e.GET("/lookup/:ip", h.Lookup) + + req := httptest.NewRequest(http.MethodGet, "/lookup/8.8.8.8", nil) + rec := httptest.NewRecorder() + e.ServeHTTP(rec, req) + + if rec.Code != http.StatusNotFound { + t.Errorf("Expected status 404 for DB error, got %d", rec.Code) + } +} From c765e95f0d55f4f5d9345f1a66d9923409673ee7 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Sun, 7 Dec 2025 19:03:36 -0300 Subject: [PATCH 4/8] feat: changelog --- .github/workflows/changelog.yml | 36 +++++++++++++++ CHANGELOG.md | 9 ++++ cliff.toml | 79 +++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+) create mode 100644 .github/workflows/changelog.yml create mode 100644 CHANGELOG.md create mode 100644 cliff.toml diff --git a/.github/workflows/changelog.yml b/.github/workflows/changelog.yml new file mode 100644 index 0000000..ec229fa --- /dev/null +++ b/.github/workflows/changelog.yml @@ -0,0 +1,36 @@ +name: Update Changelog + +on: + push: + branches: + - main + release: + types: [published] + workflow_dispatch: + +permissions: + contents: write + +jobs: + changelog: + name: Update Changelog + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Generate Changelog + uses: orhun/git-cliff-action@v4 + with: + config: cliff.toml + args: --verbose + env: + OUTPUT: CHANGELOG.md + + - name: Commit + uses: stefanzweifel/git-auto-commit-action@v5 + with: + commit_message: "chore(changelog): update changelog" + file_pattern: CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..6ad3955 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,9 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +## [unreleased] + +### Miscellaneous Tasks + +- Initial changelog setup diff --git a/cliff.toml b/cliff.toml new file mode 100644 index 0000000..5dae09f --- /dev/null +++ b/cliff.toml @@ -0,0 +1,79 @@ +# git-cliff ~ default configuration file +# https://git-cliff.org/docs/configuration +# +# Lines starting with "#" are comments. + +[changelog] +# template for the changelog header +header = """ +# Changelog\n +All notable changes to this project will be documented in this file.\n +""" +# template for the changelog body +# https://keats.github.io/tera/docs/#introduction +body = """ +{% if version %}\ + ## [{{ version | trim_start_matches(pat="v") }}] - {{ timestamp | date(format="%Y-%m-%d") }} +{% else %}\ + ## [unreleased] +{% endif %}\ +{% for group, commits in commits | group_by(attribute="group") %} + ### {{ group | striptags | trim | upper_first }} + {% for commit in commits %} + - {% if commit.scope %}*({{ commit.scope }})* {% endif %}\ + {% if commit.breaking %}[**breaking**] {% endif %}\ + {{ commit.message | upper_first }}\ + {% endfor %} +{% endfor %}\n +""" +# template for the changelog footer +footer = """ + +""" +# remove the leading and trailing s +trim = true + +[git] +# parse the commits based on https://www.conventionalcommits.org +conventional_commits = true +# filter out the commits that are not conventional +filter_unconventional = false +# process each line of a commit as an individual commit +split_commits = false +# regex for preprocessing the commit messages +commit_preprocessors = [ + # { pattern = '\((\w+\s)?#([0-9]+)\)', replace = "([#${2}](/issues/${2}))"}, + # Check spelling of the commit with https://github.com/crate-ci/typos + # { pattern = '.*', replace_command = 'typos --write-changes -' }, +] +# regex for parsing and grouping commits +commit_parsers = [ + { message = "^feat", group = "Features" }, + { message = "^fix", group = "Bug Fixes" }, + { message = "^doc", group = "Documentation" }, + { message = "^perf", group = "Performance" }, + { message = "^refactor", group = "Refactor" }, + { message = "^style", group = "Styling" }, + { message = "^test", group = "Testing" }, + { message = "^chore\\(release\\): prepare for", skip = true }, + { message = "^chore\\(deps.*\\)", skip = true }, + { message = "^chore\\(pr\\)", skip = true }, + { message = "^chore\\(pull\\)", skip = true }, + { message = "^chore|^ci", group = "Miscellaneous Tasks" }, + { body = ".*security", group = "Security" }, + { message = "^revert", group = "Revert" }, +] +# protect breaking changes from being skipped due to matching a skipping commit_parser +protect_breaking_commits = false +# filter out the commits that are not matched by commit parsers +filter_commits = false +# regex for matching git tags +tag_pattern = "v[0-9].*" +# regex for skipping tags +# skip_tags = "" +# regex for ignoring tags +# ignore_tags = "" +# sort the tags topologically +topo_order = false +# sort the commits inside sections by oldest/newest order +sort_commits = "oldest" From c78c0c0ca1126dd8ec9dccdbdfff56013e2ed37a Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Sun, 7 Dec 2025 19:03:56 -0300 Subject: [PATCH 5/8] chore: golangci lint migrate --- .golangci.bck.yml | 10 ++++++++++ .golangci.yml | 19 ++++++++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) create mode 100644 .golangci.bck.yml diff --git a/.golangci.bck.yml b/.golangci.bck.yml new file mode 100644 index 0000000..b9f7e11 --- /dev/null +++ b/.golangci.bck.yml @@ -0,0 +1,10 @@ +run: + timeout: 5m + tests: true + +linters: + disable: + - goconst + +issues: + exclude-use-default: false diff --git a/.golangci.yml b/.golangci.yml index b9f7e11..4e415e8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,10 +1,19 @@ +version: "2" run: - timeout: 5m tests: true - linters: disable: - goconst - -issues: - exclude-use-default: false + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ From 37189e75f8d2fe1c2ea50ff114ff672e82681741 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Sun, 7 Dec 2025 19:08:41 -0300 Subject: [PATCH 6/8] fix: lint problems --- cmd/api/main_test.go | 8 ++++++-- internal/geoip/reader_test.go | 21 ++++++++++++++++----- internal/geoip/service_test.go | 8 ++++++-- internal/handlers/http_test.go | 9 +++++++-- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/cmd/api/main_test.go b/cmd/api/main_test.go index dabafd9..e1f6e6b 100644 --- a/cmd/api/main_test.go +++ b/cmd/api/main_test.go @@ -103,7 +103,8 @@ func TestNewServer(t *testing.T) { require.NotNil(t, svc) // Clean up - svc.DB.Close() + svcErr := svc.DB.Close() + require.NoError(t, svcErr) // Verify Routes are Registered foundRoutes := 0 @@ -134,7 +135,10 @@ func TestHealthCheck_Integration(t *testing.T) { e, svc, err := NewServer(dbPath) require.NoError(t, err) - defer svc.DB.Close() + defer func() { + closeErr := svc.DB.Close() + require.NoError(t, closeErr) + }() req := httptest.NewRequest(http.MethodGet, "/health", nil) rec := httptest.NewRecorder() diff --git a/internal/geoip/reader_test.go b/internal/geoip/reader_test.go index c53bb36..04eafb1 100644 --- a/internal/geoip/reader_test.go +++ b/internal/geoip/reader_test.go @@ -181,7 +181,10 @@ func TestReader_Integration_HappyPath(t *testing.T) { t.Run("Open and Lookup", func(t *testing.T) { r, err := Open(dbPath) require.NoError(t, err) - defer r.Close() + defer func() { + closeErr := r.Close() + require.NoError(t, closeErr) + }() city, err := r.City(ip) assert.NoError(t, err) @@ -197,7 +200,8 @@ func TestReader_Integration_HappyPath(t *testing.T) { require.NoError(t, err) r, err := OpenBytes(b) require.NoError(t, err) - r.Close() + rErr := r.Close() + require.NoError(t, rErr) }) t.Run("Invalid Path/Bytes", func(t *testing.T) { @@ -212,7 +216,10 @@ func TestReader_ForcedExecution(t *testing.T) { dbPath := setupIntegration(t) realReader, err := maxminddb.Open(dbPath) require.NoError(t, err) - defer realReader.Close() + defer func() { + closeErr := realReader.Close() + require.NoError(t, closeErr) + }() ip := netip.MustParseAddr("8.8.8.8") @@ -252,7 +259,8 @@ func TestReader_DecodeErrors(t *testing.T) { mmdb, _ := maxminddb.OpenBytes(dbBytes) meta := mmdb.Metadata - mmdb.Close() + mmdbErr := mmdb.Close() + require.NoError(t, mmdbErr) treeSizeBits := uint(meta.NodeCount) * uint(meta.RecordSize) treeSizeBytes := treeSizeBits / 8 @@ -271,7 +279,10 @@ func TestReader_DecodeErrors(t *testing.T) { r, err := OpenBytes(dbBytes) require.NoError(t, err, "OpenBytes should succeed because metadata at EOF is intact") - defer r.Close() + defer func() { + closeErr := r.Close() + require.NoError(t, closeErr) + }() ip := netip.MustParseAddr("8.8.8.8") diff --git a/internal/geoip/service_test.go b/internal/geoip/service_test.go index 1aea28c..dca3e55 100644 --- a/internal/geoip/service_test.go +++ b/internal/geoip/service_test.go @@ -27,7 +27,8 @@ func TestNewService(t *testing.T) { assert.NotNil(t, svc.DB) // Cleanup - svc.DB.Close() + svcErr := svc.DB.Close() + require.NoError(t, svcErr) }) } @@ -64,7 +65,10 @@ func TestLookupIP_Integration(t *testing.T) { svc, err := NewService(dbPath) require.NoError(t, err) - defer svc.DB.Close() + defer func() { + closeErr := svc.DB.Close() + require.NoError(t, closeErr) + }() tests := []struct { name string diff --git a/internal/handlers/http_test.go b/internal/handlers/http_test.go index c7542da..2ad2fb1 100644 --- a/internal/handlers/http_test.go +++ b/internal/handlers/http_test.go @@ -10,6 +10,7 @@ import ( "github.com/gustavosett/WhereGo/internal/geoip" "github.com/labstack/echo/v4" + "github.com/stretchr/testify/require" ) func TestHealthCheck(t *testing.T) { @@ -45,7 +46,10 @@ func TestLookupIntegration(t *testing.T) { if err != nil { t.Skipf("Skipping integration test: could not open database at %s: %v", dbPath, err) } - defer service.DB.Close() + defer func() { + closeErr := service.DB.Close() + require.NoError(t, closeErr) + }() h := &GeoIPHandler{GeoService: service} e := echo.New() @@ -82,7 +86,8 @@ func TestLookupDBError(t *testing.T) { t.Skipf("Skipping integration test: could not open database at %s: %v", dbPath, err) } // Close immediately to simulate error - service.DB.Close() + closeErr := service.DB.Close() + require.NoError(t, closeErr) h := &GeoIPHandler{GeoService: service} e := echo.New() From a1cae6ee92378eb7976c9cf37b1111f36329c3e9 Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Sun, 7 Dec 2025 19:10:04 -0300 Subject: [PATCH 7/8] docs: mark increase coverage --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e571fcb..b031c84 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,7 @@ WhereGo is designed for high performance and low resource usage. ## Roadmap - [ ] automation to update the database -- [ ] Increase test coverage +- [x] Increase test coverage - [ ] gRPC endpoint - [ ] Built-in rate limiting From f2e4142cde795e2adfdd83874f456184eacfe38a Mon Sep 17 00:00:00 2001 From: Gustavo Carvalho Date: Sun, 7 Dec 2025 19:20:01 -0300 Subject: [PATCH 8/8] fix: golang ci pipe --- .golangci.bck.yml | 10 ---------- .golangci.yml | 19 ------------------- 2 files changed, 29 deletions(-) delete mode 100644 .golangci.bck.yml delete mode 100644 .golangci.yml diff --git a/.golangci.bck.yml b/.golangci.bck.yml deleted file mode 100644 index b9f7e11..0000000 --- a/.golangci.bck.yml +++ /dev/null @@ -1,10 +0,0 @@ -run: - timeout: 5m - tests: true - -linters: - disable: - - goconst - -issues: - exclude-use-default: false diff --git a/.golangci.yml b/.golangci.yml deleted file mode 100644 index 4e415e8..0000000 --- a/.golangci.yml +++ /dev/null @@ -1,19 +0,0 @@ -version: "2" -run: - tests: true -linters: - disable: - - goconst - exclusions: - generated: lax - paths: - - third_party$ - - builtin$ - - examples$ -formatters: - exclusions: - generated: lax - paths: - - third_party$ - - builtin$ - - examples$