Merge pull request #53 from maxmind/maxmind/ikolesn/geofeed-fail-on-t…

…hreshold

Do not fail immediately on invalid row

main.go

@@ -30,7 +30,6 @@ type config struct {
 	gf      string
 	db      string
 	isp     string
-	version bool
 	laxMode bool
 }
 
@@ -50,6 +49,12 @@ func run() error {
 
 	c, diffLines, asnCounts, err := verify.ProcessGeofeed(conf.gf, conf.db, conf.isp, conf.laxMode)
 	if err != nil {
+		if errors.Is(err, verify.ErrInvalidGeofeed) {
+			log.Printf("Found %d invalid rows out of %d rows in total, examples by type:", c.Invalid, c.Total)
+			for invType, invMessage := range c.SampleInvalidRows {
+				log.Printf("%s: '%s'", invType, invMessage)
+			}
+		}
 		return fmt.Errorf("unable to process geofeed %s: %w", conf.gf, err)
 	}
 
@@ -92,7 +97,8 @@ func parseFlags(program string, args []string) (c *config, output string, err er
 		"/usr/local/share/GeoIP/GeoIP2-City.mmdb",
 		"Path to MMDB file to compare geofeed file against",
 	)
-	flags.BoolVar(&conf.version, "V", false, "Display version")
+	displayVersion := false
+	flags.BoolVar(&displayVersion, "V", false, "Display version")
 	flags.BoolVar(
 		&conf.laxMode,
 		"lax",
@@ -104,7 +110,7 @@ func parseFlags(program string, args []string) (c *config, output string, err er
 		return nil, buf.String(), err
 	}
 
-	if conf.version {
+	if displayVersion {
 		log.Printf("mm-geofeed-verifier %s", version)
 		//nolint:revive // preexisting
 		os.Exit(0)

main_test.go

@@ -6,8 +6,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-
-	"github.com/maxmind/mm-geofeed-verifier/v2/verify"
 )
 
 type parseFlagsCorrectTest struct {
@@ -137,145 +135,3 @@ func TestParseFlagsError(t *testing.T) {
 		)
 	}
 }
-
-type processGeofeedTest struct {
-	gf      string
-	db      string
-	dl      []string
-	c       verify.Counts
-	em      string
-	laxMode bool
-}
-
-func TestProcessGeofeed_Valid(t *testing.T) {
-	goodTests := []processGeofeedTest{
-		{
-			gf: "test_data/geofeed-valid.csv",
-			db: "test_data/GeoIP2-City-Test.mmdb",
-			dl: []string{
-				"Found a potential improvement: '2a02:ecc0::/29",
-				"current postal code: '34021'\t\tsuggested postal code: '1060'",
-			},
-			c: verify.Counts{
-				Total:       3,
-				Differences: 2,
-			},
-			em:      "",
-			laxMode: false,
-		},
-		{
-			gf: "test_data/geofeed-valid.csv",
-			db: "test_data/GeoIP2-City-Test.mmdb",
-			dl: []string{
-				"Found a potential improvement: '2a02:ecc0::/29",
-				"current postal code: '34021'\t\tsuggested postal code: '1060'",
-			},
-			c: verify.Counts{
-				Total:       3,
-				Differences: 2,
-			},
-			em:      "",
-			laxMode: true,
-		},
-		{
-			gf: "test_data/geofeed-valid-lax.csv",
-			db: "test_data/GeoIP2-City-Test.mmdb",
-			dl: []string{
-				"Found a potential improvement: '2a02:ecc0::/29",
-				"current postal code: '34021'\t\tsuggested postal code: '1060'",
-			},
-			c: verify.Counts{
-				Total:       3,
-				Differences: 2,
-			},
-			em:      "",
-			laxMode: true,
-		},
-		{
-			gf: "test_data/geofeed-valid-optional-fields.csv",
-			db: "test_data/GeoIP2-City-Test.mmdb",
-			dl: []string{
-				"Found a potential improvement: '2a02:ecc0::/29",
-				"current postal code: '34021'\t\tsuggested postal code: '1060'",
-			},
-			c: verify.Counts{
-				Total:       3,
-				Differences: 2,
-			},
-			em:      "",
-			laxMode: false,
-		},
-	}
-
-	// Testing the full content of the difference explanation strings is likely to be
-	// tedious and brittle, so we will just check for some substrings.
-	for _, test := range goodTests {
-		t.Run(
-			test.gf+" "+test.db, func(t *testing.T) {
-				c, dl, _, err := verify.ProcessGeofeed(test.gf, test.db, "", test.laxMode)
-				assert.NoError(t, err, "processGeofeed ran without error")
-				for i, s := range test.dl {
-					assert.Contains(
-						t,
-						dl[i],
-						s,
-						"got expected substring: '%s', substring",
-					)
-				}
-				assert.Equal(t, test.c, c, "processGeofeed returned expected results")
-			},
-		)
-	}
-}
-
-func TestProcessGeofeed_Invalid(t *testing.T) {
-	badTests := []processGeofeedTest{
-		{
-			gf:      "test_data/geofeed-invalid-missing-fields.csv",
-			db:      "test_data/GeoIP2-City-Test.mmdb",
-			dl:      []string{},
-			c:       verify.Counts{},
-			em:      "saw fewer than the expected 5 fields at line 1: '2a02:ecc0::/29,US,US-NJ,Parsippany'",
-			laxMode: false,
-		},
-		{
-			gf:      "test_data/geofeed-invalid-empty-network.csv",
-			db:      "test_data/GeoIP2-City-Test.mmdb",
-			dl:      []string{},
-			c:       verify.Counts{},
-			em:      "line 2: network field is empty",
-			laxMode: false,
-		},
-		{
-			gf:      "test_data/geofeed-invalid-network.csv",
-			db:      "test_data/GeoIP2-City-Test.mmdb",
-			dl:      []string{},
-			c:       verify.Counts{},
-			em:      "line 1: unable to parse network 2a02:/29: invalid CIDR address: 2a02:/29",
-			laxMode: false,
-		},
-		{
-			gf: "test_data/geofeed-valid-lax.csv",
-			db: "test_data/GeoIP2-City-Test.mmdb",
-			dl: []string{},
-			c:  verify.Counts{},
-			em: "line 1: invalid ISO 3166-2 region code format in strict (default) mode, line: " +
-				"'2a02:ecc0::/29,US,NJ,Parsippany,'",
-			laxMode: false,
-		},
-	}
-
-	for _, test := range badTests {
-		t.Run(
-			test.gf+" "+test.db, func(t *testing.T) {
-				_, _, _, err := verify.ProcessGeofeed(test.gf, test.db, "", test.laxMode)
-				assert.EqualError(
-					t,
-					err,
-					test.em,
-					"got expected error: %s", test.em,
-				)
-			},
-		)
-	}
-}

verify/errors.go

@@ -0,0 +1,40 @@
+package verify
+
+import "errors"
+
+// ErrInvalidGeofeed represents error that is returned in case of incomplete compliance
+// with RFC 8805 standards and the mode in which the program is run.
+var ErrInvalidGeofeed = errors.New("geofeed does not comply with the RFC 8805 standards")
+
+// RowInvalidity represents type of row invalidity.
+type RowInvalidity int
+
+// Invalidity types.
+const (
+	FewerFieldsThanExpected RowInvalidity = iota
+	EmptyNetwork
+	UnableToParseNetwork
+	UnableToFindCityRecord
+	UnableToFindISPRecord
+	InvalidRegionCode
+)
+
+// String implements the Stringer interface.
+func (ri RowInvalidity) String() string {
+	switch ri {
+	case FewerFieldsThanExpected:
+		return "FewerFieldsThanExpected"
+	case EmptyNetwork:
+		return "EmptyNetwork"
+	case UnableToParseNetwork:
+		return "UnableToParseNetwork"
+	case UnableToFindCityRecord:
+		return "UnableToFindCityRecord"
+	case UnableToFindISPRecord:
+		return "UnableToFindISPRecord"
+	case InvalidRegionCode:
+		return "InvalidRegionCode"
+	default:
+		return "UnknownInvalidityType"
+	}
+}

verify/test_data/geofeed-invalid-empty-network.csv

@@ -0,0 +1,2 @@
+ 2a02:ecc0::/29 ,US,US-NJ, Parsippany ,# valid row with difference from GeoIP2-City_test.mmdb
+,,,,

verify/test_data/geofeed-invalid-network.csv

@@ -0,0 +1,2 @@
+2a02:/29,,,,
+ 2a02:ecc0::/29 ,US,US-NJ, Parsippany ,# valid row with difference from GeoIP2-City_test.mmdb