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: #2

Makefile

@@ -23,13 +23,19 @@ lint:
 
 # 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 is complete, so an
-# interrupted fetch cannot leave a truncated dataset committed.
+# 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 "fetching $$url"; \
+	echo "downloading $$url"; \
 	curl -fsSL "$$url" -o blogs.json.tmp
-	@test -s blogs.json.tmp || { echo "downloaded dataset is empty" >&2; rm -f blogs.json.tmp; exit 1; }
+	@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

@@ -16,9 +16,9 @@ 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.
 
-`FetchBlogs` keeps its name and its `sync.Once` memoization, but on first call
-it decodes the embedded bytes rather than issuing an HTTP request. Its error
-return is now only reachable if the committed `blogs.json` is malformed.
+`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
@@ -31,10 +31,12 @@ make update-data
 ```
 
 That target reads the upstream location from the `BlogsURL` constant in
-`hnblogs.go` — the single source of truth — downloads to a temporary file,
-replaces `blogs.json` only once the download completes, and then runs the test
-suite against the new data. Commit the resulting `blogs.json` to publish the
-update.
+`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
 
@@ -44,4 +46,4 @@ make lint
 make docker
 ```
 
-`make docker` runs lint and tests in containers, matching CI.
+`make docker` runs lint and tests in containers.

hnblogs.go

@@ -14,8 +14,8 @@ import (
 	"sync"
 )
 
-// BlogsURL is the upstream source of blogs.json. It is not fetched at runtime;
-// it documents where "make update-data" pulls the vendored copy from.
+// 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".
@@ -39,13 +39,13 @@ type Blog struct {
 	Desc  string `json:"desc"`
 }
 
-// FetchBlogs returns the embedded list of blogs, decoding it on first call and
+// GetBlogs returns the embedded list of blogs, decoding it on first call and
 // memoizing the result for subsequent calls.
 //
-// Despite the name 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.
-func FetchBlogs() ([]Blog, error) {
+// 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.
+func GetBlogs() ([]Blog, error) {
 	once.Do(func() {
 		var decoded []Blog
 		if err := json.Unmarshal(blogsJSON, &decoded); err != nil {
@@ -59,11 +59,6 @@ func FetchBlogs() ([]Blog, error) {
 	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.
 func RandomBlog() (Blog, error) {
 	blogs, err := GetBlogs()

hnblogs_test.go

@@ -1,14 +1,15 @@
 package hnblogs
 
 import (
+	"bytes"
 	"encoding/json"
-	"go/build"
+	"os/exec"
 	"strings"
 	"testing"
 )
 
-func TestFetchBlogs(t *testing.T) {
-	blogs, err := FetchBlogs()
+func TestGetBlogs(t *testing.T) {
+	blogs, err := GetBlogs()
 	if err != nil {
 		t.Fatalf("Expected no error, got %v", err)
 	}
@@ -39,17 +40,25 @@ func TestEmbeddedDataIsUsable(t *testing.T) {
 }
 
 // TestNoRuntimeNetworkImports is the regression guard for the reason this data
-// is embedded: the package must not reach the network on any code path. A
-// transport dependency reintroduced in non-test code fails here.
+// 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) {
-	pkg, err := build.ImportDir(".", 0)
+	cmd := exec.Command("go", "list", "-deps", ".")
+
+	var stderr bytes.Buffer
+	cmd.Stderr = &stderr
+
+	out, err := cmd.Output()
 	if err != nil {
-		t.Fatalf("Failed to inspect package imports: %v", err)
+		t.Fatalf("Failed to list package dependencies: %v\n%s", err, stderr.String())
 	}
 
-	for _, imported := range pkg.Imports {
-		if imported == "net" || strings.HasPrefix(imported, "net/") {
-			t.Fatalf("Package must not perform network I/O, but imports %q", imported)
+	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)
 		}
 	}
 }