Merge pull request 'embed blogs.json instead of fetching it at runtime' (#2) from clawbot/hnblogs:issue-1-embed-blogs-json into main

Reviewed-on: sneak/hnblogs#2

.gitignore

@@ -1 +1,2 @@
 example
+blogs.json.tmp

Makefile

@@ -1,5 +1,5 @@
 # Targets
-.PHONY: all run test clean
+.PHONY: all run test clean docker lint update-data
 
 all: run
 
@@ -13,10 +13,29 @@ test:
 	go test -v ./...
 
 clean:
-	rm -f example
+	rm -f example blogs.json.tmp
 
 docker:
 	docker build --progress plain .
 
 lint:
 	golangci-lint run
+
+# Refresh the vendored dataset from the BlogsURL constant in hnblogs.go, which
+# is the single source of truth for the upstream location. The download lands
+# on a temporary file and only replaces blogs.json once it has been checked to
+# be a non-empty JSON array of blog entries, so neither an interrupted transfer
+# nor a complete-but-wrong response (an error page, a redirect landing page)
+# can overwrite the good dataset.
+update-data:
+	@command -v jq >/dev/null || { echo "update-data requires jq" >&2; exit 1; }
+	@url=$$(sed -n 's/^const BlogsURL = "\(.*\)"$$/\1/p' hnblogs.go); \
+	test -n "$$url" || { echo "could not parse BlogsURL from hnblogs.go" >&2; exit 1; }; \
+	echo "downloading $$url"; \
+	curl -fsSL "$$url" -o blogs.json.tmp
+	@jq -e 'type == "array" and length > 0 and all(.[]; type == "object" and has("url"))' \
+		blogs.json.tmp >/dev/null 2>&1 \
+		|| { echo "download is not a non-empty JSON array of blog entries; blogs.json left unchanged" >&2; \
+		     rm -f blogs.json.tmp; exit 1; }
+	mv blogs.json.tmp blogs.json
+	$(MAKE) test

README.md

@@ -0,0 +1,49 @@
+# hnblogs
+
+A Go library for the [blogs.hn](https://blogs.hn) dataset: a list of personal
+blogs collected from Hacker News.
+
+```go
+import "sneak.berlin/go/hnblogs"
+
+blog, err := hnblogs.RandomBlog()
+```
+
+## Embedded data
+
+The dataset is vendored into this repository as `blogs.json` and compiled into
+the package with `go:embed`. The library performs no network I/O: importing it
+does not reach out to anything, results do not change under a caller between
+runs of the same build, and `go test` works offline.
+
+`GetBlogs` decodes the embedded bytes on first call and memoizes the result;
+every other accessor goes through it. Its error return is only reachable if the
+committed `blogs.json` is malformed.
+
+The trade-off is that the dataset is a build-time artifact: it is roughly 8 MB
+of JSON, it lands in every binary that links the package, and it is only as
+fresh as the last commit that refreshed it.
+
+## Refreshing the dataset
+
+```sh
+make update-data
+```
+
+That target reads the upstream location from the `BlogsURL` constant in
+`hnblogs.go` — the single source of truth — and downloads it to a temporary
+file. It replaces `blogs.json` only once that file parses as a non-empty JSON
+array of blog entries, so a response that arrives complete but is not the
+dataset leaves the vendored copy untouched. It then runs the test suite
+against the new data. Requires `curl` and `jq`. Commit the resulting
+`blogs.json` to publish the update.
+
+## Development
+
+```sh
+make test
+make lint
+make docker
+```
+
+`make docker` runs lint and tests in containers.

hnblogs.go

@@ -1,15 +1,34 @@
+// Package hnblogs provides access to the blogs.hn dataset.
+//
+// The dataset is vendored into this repository as blogs.json and compiled into
+// the package with go:embed, so nothing here touches the network at runtime and
+// the results are stable for a given version of the module. Refresh the
+// vendored copy with "make update-data" and commit the result.
 package hnblogs
 
 import (
+	_ "embed"
 	"encoding/json"
 	"fmt"
 	"math/rand"
-	"net/http"
 	"sync"
 )
 
+// BlogsURL is the upstream source of blogs.json. Nothing reads it at runtime;
+// it documents where "make update-data" downloads the vendored copy from.
 const BlogsURL = "https://raw.githubusercontent.com/surprisetalk/blogs.hn/main/blogs.json"
 
+// blogsJSON is the vendored dataset, refreshed by "make update-data".
+//
+//go:embed blogs.json
+var blogsJSON []byte
+
+var (
+	blogs     []Blog
+	loadError error
+	once      sync.Once
+)
+
 // Blog represents a single blog entry.
 type Blog struct {
 	URL   string `json:"url"`
@@ -20,42 +39,24 @@ type Blog struct {
 	Desc  string `json:"desc"`
 }
 
-var (
+// GetBlogs returns the embedded list of blogs, decoding it on first call and
-	blogs       []Blog
+// memoizing the result for subsequent calls.
-	fetchError  error
+//
-	once        sync.Once
+// It performs no I/O: the data is compiled into the binary, so the only error
-)
+// it can return is a malformed embedded blogs.json, which would mean the
-
+// committed dataset is broken.
-// FetchBlogs fetches the list of blogs and memoizes it in RAM.
+func GetBlogs() ([]Blog, error) {
-func FetchBlogs() ([]Blog, error) {
 	once.Do(func() {
-		resp, err := http.Get(BlogsURL)
+		var decoded []Blog
-		if err != nil {
+		if err := json.Unmarshal(blogsJSON, &decoded); err != nil {
-			fetchError = fmt.Errorf("failed to fetch blogs: %v", err)
+			loadError = fmt.Errorf("failed to decode embedded blogs JSON: %w", err)
-			return
-		}
-		defer resp.Body.Close()
-
-		if resp.StatusCode != http.StatusOK {
-			fetchError = fmt.Errorf("failed to fetch blogs: status code %d", resp.StatusCode)
 			return
 		}
 
-		var fetchedBlogs []Blog
+		blogs = decoded
-		if err := json.NewDecoder(resp.Body).Decode(&fetchedBlogs); err != nil {
-			fetchError = fmt.Errorf("failed to decode blogs JSON: %v", err)
-			return
-		}
-
-		blogs = fetchedBlogs
 	})
 
-	return blogs, fetchError
+	return blogs, loadError
-}
-
-// GetBlogs returns the memoized list of blogs.
-func GetBlogs() ([]Blog, error) {
-	return FetchBlogs()
 }
 
 // RandomBlog returns a random blog from the list of blogs.

hnblogs_test.go

@@ -1,20 +1,13 @@
 package hnblogs
 
 import (
+	"bytes"
+	"encoding/json"
+	"os/exec"
+	"strings"
 	"testing"
 )
 
-func TestFetchBlogs(t *testing.T) {
-	blogs, err := FetchBlogs()
-	if err != nil {
-		t.Fatalf("Expected no error, got %v", err)
-	}
-
-	if len(blogs) == 0 {
-		t.Fatalf("Expected to fetch some blogs, got %d", len(blogs))
-	}
-}
-
 func TestGetBlogs(t *testing.T) {
 	blogs, err := GetBlogs()
 	if err != nil {
@@ -22,7 +15,71 @@ func TestGetBlogs(t *testing.T) {
 	}
 
 	if len(blogs) == 0 {
-		t.Fatalf("Expected to fetch some blogs, got %d", len(blogs))
+		t.Fatalf("Expected the embedded dataset to be non-empty, got %d blogs", len(blogs))
+	}
+}
+
+// TestEmbeddedDataIsUsable guards the invariant the embedded dataset has to
+// satisfy for the rest of the package: every entry has a URL, so callers of
+// RandomBlog and NthBlog get something dereferenceable.
+func TestEmbeddedDataIsUsable(t *testing.T) {
+	if !json.Valid(blogsJSON) {
+		t.Fatalf("Embedded blogs.json is not valid JSON")
+	}
+
+	blogs, err := GetBlogs()
+	if err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+
+	for i, blog := range blogs {
+		if blog.URL == "" {
+			t.Fatalf("Blog at index %d has an empty URL", i)
+		}
+	}
+}
+
+// TestNoRuntimeNetworkImports is the regression guard for the reason this data
+// is embedded: the package must not reach the network on any code path. It
+// walks the whole dependency graph of the non-test build rather than the direct
+// import list, so a net/* package reached through an intermediate import fails
+// here too. Test-only imports are outside that graph, which is why this test
+// may use os/exec itself.
+func TestNoRuntimeNetworkImports(t *testing.T) {
+	cmd := exec.Command("go", "list", "-deps", ".")
+
+	var stderr bytes.Buffer
+	cmd.Stderr = &stderr
+
+	out, err := cmd.Output()
+	if err != nil {
+		t.Fatalf("Failed to list package dependencies: %v\n%s", err, stderr.String())
+	}
+
+	for _, dep := range strings.Fields(string(out)) {
+		if dep == "net" || strings.HasPrefix(dep, "net/") {
+			t.Fatalf("Package must not perform network I/O, but depends on %q", dep)
+		}
+	}
+}
+
+func TestGetBlogsIsMemoized(t *testing.T) {
+	first, err := GetBlogs()
+	if err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+
+	second, err := GetBlogs()
+	if err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+
+	if len(first) != len(second) {
+		t.Fatalf("Expected the same slice on repeated calls, got %d then %d", len(first), len(second))
+	}
+
+	if len(first) > 0 && &first[0] != &second[0] {
+		t.Fatalf("Expected repeated calls to share the memoized backing array")
 	}
 }
 
@@ -55,6 +112,19 @@ func TestRandomBlogs(t *testing.T) {
 	}
 }
 
+func TestRandomBlogsRejectsInvalidCounts(t *testing.T) {
+	blogs, err := GetBlogs()
+	if err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+
+	for _, n := range []int{0, -1, len(blogs) + 1} {
+		if _, err := RandomBlogs(n); err == nil {
+			t.Fatalf("Expected an error for n=%d, got none", n)
+		}
+	}
+}
+
 func TestNthBlog(t *testing.T) {
 	blogs, err := GetBlogs()
 	if err != nil {
@@ -74,4 +144,8 @@ func TestNthBlog(t *testing.T) {
 	if err == nil {
 		t.Fatalf("Expected error for out-of-range index, got none")
 	}
+
+	if _, err := NthBlog(-1); err == nil {
+		t.Fatalf("Expected error for negative index, got none")
+	}
 }