refactor names to only export intentional parts of functionality [#20… (

#124)

* refactor names to only export intentional parts of functionality [#20, #46]

* Also correct ordering of function arguments to put errors last

* add go vet to CI, fix CLI syntax

.github/workflows/test.yml

@@ -14,11 +14,16 @@ jobs:
         with:
           go-version: '^1.18.0'
       - run: go test ./pmtiles
-  fmt:
+  fmt_vet_lint:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v3
       - uses: actions/setup-go@v3
         with:
           go-version: '^1.18.0'
       - run: if [ "$(gofmt -s -l . | wc -l)" -gt 0 ]; then exit 1; fi
+      - run: go vet caddy/pmtiles_proxy.go
+      - run: go vet main.go
+      - run: go vet pmtiles/*
+      - name: Run Revive Action by pulling pre-built image
+        uses: docker://morphy/revive-action:v2

caddy/pmtiles_proxy.go

@@ -28,7 +28,7 @@ func init() {
 type Middleware struct {
 	Bucket    string `json:"bucket"`
 	CacheSize int    `json:"cache_size"`
-	PublicUrl string `json:"public_url"`
+	PublicURL string `json:"public_url"`
 	logger    *zap.Logger
 	server    *pmtiles.Server
 }
@@ -45,7 +45,7 @@ func (m *Middleware) Provision(ctx caddy.Context) error {
 	m.logger = ctx.Logger()
 	logger := log.New(io.Discard, "", log.Ldate)
 	prefix := "." // serve only the root of the bucket for now, at the root route of Caddyfile
-	server, err := pmtiles.NewServer(m.Bucket, prefix, logger, m.CacheSize, "", m.PublicUrl)
+	server, err := pmtiles.NewServer(m.Bucket, prefix, logger, m.CacheSize, "", m.PublicURL)
 	if err != nil {
 		return err
 	}
@@ -66,13 +66,13 @@ func (m *Middleware) Validate() error {
 
 func (m Middleware) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error {
 	start := time.Now()
-	status_code, headers, body := m.server.Get(r.Context(), r.URL.Path)
+	statusCode, headers, body := m.server.Get(r.Context(), r.URL.Path)
 	for k, v := range headers {
 		w.Header().Set(k, v)
 	}
-	w.WriteHeader(status_code)
+	w.WriteHeader(statusCode)
 	w.Write(body)
-	m.logger.Info("response", zap.Int("status", status_code), zap.String("path", r.URL.Path), zap.Duration("duration", time.Since(start)))
+	m.logger.Info("response", zap.Int("status", statusCode), zap.String("path", r.URL.Path), zap.Duration("duration", time.Since(start)))
 
 	return next.ServeHTTP(w, r)
 }
@@ -96,7 +96,7 @@ func (m *Middleware) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 				}
 				m.CacheSize = num
 			case "public_url":
-				if !d.Args(&m.PublicUrl) {
+				if !d.Args(&m.PublicURL) {
 					return d.ArgErr()
 				}
 			}

main.go

@@ -56,11 +56,11 @@ var cli struct {
 		Bucket          string  `help:"Remote bucket of input archive."`
 		Region          string  `help:"local GeoJSON Polygon or MultiPolygon file for area of interest." type:"existingfile"`
 		Bbox            string  `help:"bbox area of interest: min_lon,min_lat,max_lon,max_lat" type:"string"`
-		Minzoom         int8    `default:-1 help:"Minimum zoom level, inclusive."`
-		Maxzoom         int8    `default:-1 help:"Maximum zoom level, inclusive."`
-		DownloadThreads int     `default:4 help:"Number of download threads."`
+		Minzoom         int8    `default:"-1" help:"Minimum zoom level, inclusive."`
+		Maxzoom         int8    `default:"-1" help:"Maximum zoom level, inclusive."`
+		DownloadThreads int     `default:"4" help:"Number of download threads."`
 		DryRun          bool    `help:"Calculate tiles to extract, but don't download them."`
-		Overfetch       float32 `default:0.05 help:"What ratio of extra data to download to minimize # requests; 0.2 is 20%"`
+		Overfetch       float32 `default:"0.05" help:"What ratio of extra data to download to minimize # requests; 0.2 is 20%"`
 	} `cmd:"" help:"Create an archive from a larger archive for a subset of zoom levels or geographic region."`
 
 	Verify struct {
@@ -69,8 +69,8 @@ var cli struct {
 
 	Makesync struct {
 		Input        string `arg:"" type:"existingfile"`
-		BlockSizeKb  int    `default:1000 help:"The approximate block size, in kilobytes. 0 means 1 tile = 1 block."`
-		HashFunction string `default:fnv1a help:"The hash function."`
+		BlockSizeKb  int    `default:"1000" help:"The approximate block size, in kilobytes. 0 means 1 tile = 1 block."`
+		HashFunction string `default:"fnv1a" help:"The hash function."`
 		Checksum     string `help:"Store a checksum in the syncfile."`
 	} `cmd:"" hidden:""`
 
@@ -82,10 +82,10 @@ var cli struct {
 	Serve struct {
 		Path      string `arg:"" help:"Local path or bucket prefix"`
 		Interface string `default:"0.0.0.0"`
-		Port      int    `default:8080`
-		AdminPort int    `default:-1`
+		Port      int    `default:"8080"`
+		AdminPort int    `default:"-1"`
 		Cors      string `help:"Value of HTTP CORS header."`
-		CacheSize int    `default:64 help:"Size of cache in Megabytes."`
+		CacheSize int    `default:"64" help:"Size of cache in Megabytes."`
 		Bucket    string `help:"Remote bucket"`
 		PublicURL string `help:"Public base URL of tile endpoint for TileJSON e.g. https://example.com/tiles/"`
 	} `cmd:"" help:"Run an HTTP proxy server for Z/X/Y tiles."`
@@ -94,15 +94,15 @@ var cli struct {
 		OldFile         string  `type:"existingfile" help:"The old archive on disk. Providing this will check the new archive for a .sync file"`
 		NewFile         string  `arg:"The remote file."`
 		Bucket          string  `required:"" help:"Bucket of file to download."`
-		DownloadThreads int     `default:4 help:"Number of download threads."`
+		DownloadThreads int     `default:"4" help:"Number of download threads."`
 		DryRun          bool    `help:"Calculate new parts to download, but don't download them."`
-		Overfetch       float32 `default:0.05 help:"What ratio of extra data to download to minimize # requests; 0.2 is 20%"`
+		Overfetch       float32 `default:"0.05" help:"What ratio of extra data to download to minimize # requests; 0.2 is 20%"`
 	} `cmd:"" help:"Upload a local archive to remote storage."`
 
 	Upload struct {
 		Input          string `arg:"" type:"existingfile"`
 		Key            string `arg:""`
-		MaxConcurrency int    `default:2 help:"# of upload threads"`
+		MaxConcurrency int    `default:"2" help:"# of upload threads"`
 		Bucket         string `required:"" help:"Bucket to upload to."`
 	} `cmd:"" help:"Upload a local archive to remote storage."`
 

pmtiles/bitmap.go

@@ -103,7 +103,7 @@ func generalizeAnd(r *roaring64.Bitmap) {
 	}
 }
 
-func WriteImage(interior *roaring64.Bitmap, boundary *roaring64.Bitmap, exterior *roaring64.Bitmap, filename string, zoom uint8) {
+func writeImage(interior *roaring64.Bitmap, boundary *roaring64.Bitmap, exterior *roaring64.Bitmap, filename string, zoom uint8) {
 	dim := 1 << zoom
 	img := image.NewNRGBA(image.Rect(0, 0, dim, dim))
 

pmtiles/bucket.go

@@ -13,6 +13,7 @@ import (
 	"strings"
 )
 
+// Bucket is an abstration over a gocloud or plain HTTP bucket.
 type Bucket interface {
 	Close() error
 	NewRangeReader(ctx context.Context, key string, offset int64, length int64) (io.ReadCloser, error)
@@ -22,7 +23,7 @@ type HTTPBucket struct {
 	baseURL string
 }
 
-func (b HTTPBucket) NewRangeReader(ctx context.Context, key string, offset, length int64) (io.ReadCloser, error) {
+func (b HTTPBucket) NewRangeReader(_ context.Context, key string, offset, length int64) (io.ReadCloser, error) {
 	reqURL := b.baseURL + "/" + key
 
 	req, err := http.NewRequest("GET", reqURL, nil)
@@ -77,42 +78,39 @@ func NormalizeBucketKey(bucket string, prefix string, key string) (string, strin
 				dir = dir[:len(dir)-1]
 			}
 			return u.Scheme + "://" + u.Host + dir, file, nil
-		} else {
-			fileprotocol := "file://"
-			if string(os.PathSeparator) != "/" {
-				fileprotocol += "/"
-			}
-			if prefix != "" {
-				abs, err := filepath.Abs(prefix)
-				if err != nil {
-					return "", "", err
-				}
-				return fileprotocol + filepath.ToSlash(abs), key, nil
-			}
-			abs, err := filepath.Abs(key)
+		}
+		fileprotocol := "file://"
+		if string(os.PathSeparator) != "/" {
+			fileprotocol += "/"
+		}
+		if prefix != "" {
+			abs, err := filepath.Abs(prefix)
 			if err != nil {
 				return "", "", err
 			}
-			return fileprotocol + filepath.ToSlash(filepath.Dir(abs)), filepath.Base(abs), nil
+			return fileprotocol + filepath.ToSlash(abs), key, nil
 		}
+		abs, err := filepath.Abs(key)
+		if err != nil {
+			return "", "", err
+		}
+		return fileprotocol + filepath.ToSlash(filepath.Dir(abs)), filepath.Base(abs), nil
 	}
-
 	return bucket, key, nil
 }
 
 func OpenBucket(ctx context.Context, bucketURL string, bucketPrefix string) (Bucket, error) {
 	if strings.HasPrefix(bucketURL, "http") {
 		bucket := HTTPBucket{bucketURL}
 		return bucket, nil
-	} else {
-		bucket, err := blob.OpenBucket(ctx, bucketURL)
-		if err != nil {
-			return nil, err
-		}
-		if bucketPrefix != "" && bucketPrefix != "/" && bucketPrefix != "." {
-			bucket = blob.PrefixedBucket(bucket, path.Clean(bucketPrefix)+string(os.PathSeparator))
-		}
-		wrappedBucket := BucketAdapter{bucket}
-		return wrappedBucket, err
 	}
+	bucket, err := blob.OpenBucket(ctx, bucketURL)
+	if err != nil {
+		return nil, err
+	}
+	if bucketPrefix != "" && bucketPrefix != "/" && bucketPrefix != "." {
+		bucket = blob.PrefixedBucket(bucket, path.Clean(bucketPrefix)+string(os.PathSeparator))
+	}
+	wrappedBucket := BucketAdapter{bucket}
+	return wrappedBucket, err
 }