M doc/archive.go => doc/archive.go +53 -0
@@ 1,6 1,8 @@
package doc
import (
+ "errors"
+ "fmt"
"net/url"
"path"
"regexp"
@@ 179,9 181,60 @@ func (a *Archive) Roots() []*Page {
return out
}
+// LinkPass fills in Page.Links and Page.WordCount for every document of the
+// archive, by rendering each body against the archive itself.
+//
+// It is a second pass rather than part of Scan because links come out of a
+// render, not out of a frontmatter parse: a wikilink inside a fenced code block
+// is not a link, and deciding that needs the markdown AST. It is here rather
+// than in a caller because Archive.Backlinks reads exactly what this writes —
+// left to each surface, one of them renders the revision twice a page view and
+// the next one silently reports no backlinks at all.
+//
+// bodies holds each page's raw markdown, frontmatter included, keyed by
+// Page.Path — the map a caller already has from the same tree walk that built
+// the archive. A page with no body is an inconsistency between the two and is
+// reported rather than skipped: skipping it would drop that document's outbound
+// links and under-report backlinks everywhere else, invisibly.
+//
+// The archive resolves the links, so every href produced here is the plain,
+// unpinned site path. A caller rendering for display wraps the resolver to
+// carry its own ?rev=; that wrapper must not be used here, or the link graph
+// would depend on how the reader arrived.
+func (a *Archive) LinkPass(r *Renderer, bodies map[string][]byte) error {
+ if r == nil {
+ return errors.New("doc: link pass needs a renderer")
+ }
+ for _, p := range a.Pages {
+ raw, ok := bodies[p.Path]
+ if !ok {
+ return fmt.Errorf("doc: %s is in the archive of %s at %s but has no body",
+ p.Path, a.Space, a.Rev)
+ }
+ _, body := ParseFront(raw)
+ res := r.Render(body, DirOf(p.Path), a)
+ p.Links = res.LinkedIDs
+ p.WordCount = res.WordCount
+ }
+ return nil
+}
+
+// DirOf is the directory a document lives in, space-relative, with "" for the
+// space root — the shape Resolve expects as fromDir.
+func DirOf(p string) string {
+ d := path.Dir(p)
+ if d == "." || d == "/" {
+ return ""
+ }
+ return d
+}
+
// Backlinks returns documents that link to id. Catalog and log documents are
// skipped: they link to nearly everything, so counting them would make every
// document look referenced and orphan detection would never return a result.
+//
+// It reads Page.Links, which LinkPass fills: an archive that has not been
+// through one has no link graph, and every document looks unreferenced.
func (a *Archive) Backlinks(id string) []*Page {
var out []*Page
for _, p := range a.Pages {
M doc/archive_test.go => doc/archive_test.go +54 -0
@@ 179,6 179,60 @@ func TestHrefsAreSpaceScopedAndEscaped(t *testing.T) {
}
}
+// The link graph is what Backlinks reads, and nothing but LinkPass writes it.
+func TestLinkPassFillsLinksAndWordCount(t *testing.T) {
+ files := map[string]string{
+ "specs/storage.md": "---\nid: SPEC-0001\ntitle: Storage\n---\n\n# Storage\n\nfour words of prose\n",
+ "specs/review.md": "---\nid: SPEC-0002\ntitle: Review\n---\n\n" +
+ "Supersedes [[SPEC-0001]], and again [[SPEC-0001]].\n\n" +
+ "```\n[[SPEC-0009]]\n```\n",
+ "notes/root.md": "See [[SPEC-0001]] from a note.\n",
+ }
+ arc := archiveOf(t, files)
+ bodies := make(map[string][]byte, len(files))
+ for p, body := range files {
+ bodies[p] = []byte(body)
+ }
+
+ // Before the pass there is no graph at all — the state every caller of
+ // Backlinks was silently in.
+ if got := arc.Backlinks("SPEC-0001"); len(got) != 0 {
+ t.Fatalf("Backlinks before LinkPass = %v, want none", got)
+ }
+
+ if err := arc.LinkPass(NewRenderer(), bodies); err != nil {
+ t.Fatalf("LinkPass: %v", err)
+ }
+
+ review := mustPage(t, arc, "specs/review.md")
+ // Deduped, and the fenced [[SPEC-0009]] is code, not a link.
+ if len(review.Links) != 1 || review.Links[0] != "SPEC-0001" {
+ t.Errorf("review links = %v, want [SPEC-0001]", review.Links)
+ }
+ if mustPage(t, arc, "specs/storage.md").WordCount != 5 {
+ t.Errorf("storage word count = %d, want 5", mustPage(t, arc, "specs/storage.md").WordCount)
+ }
+
+ var backPaths []string
+ for _, p := range arc.Backlinks("SPEC-0001") {
+ backPaths = append(backPaths, p.Path)
+ }
+ if len(backPaths) != 2 || backPaths[0] != "notes/root.md" || backPaths[1] != "specs/review.md" {
+ t.Errorf("backlinks = %v, want notes/root.md and specs/review.md", backPaths)
+ }
+}
+
+// A page with no body is the archive and the bodies disagreeing, which cannot
+// happen while both come off one tree walk. Reported, never skipped: a skip
+// drops that document's outbound links and under-reports backlinks elsewhere.
+func TestLinkPassRefusesAMissingBody(t *testing.T) {
+ arc := archiveOf(t, map[string]string{"notes/a.md": "# A\n"})
+ err := arc.LinkPass(NewRenderer(), map[string][]byte{})
+ if err == nil || !strings.Contains(err.Error(), "notes/a.md") {
+ t.Fatalf("LinkPass with no bodies = %v, want an error naming the document", err)
+ }
+}
+
func TestBacklinksIgnoreCatalogsAndLogs(t *testing.T) {
arc := archiveOf(t, map[string]string{
"notes/a.md": "# A\n",
M doc/doc.go => doc/doc.go +3 -2
@@ 98,10 98,11 @@ type Page struct {
// parent (not including the document itself).
Crumbs []string `json:"crumbs,omitempty"`
// Links holds the IDs of documents this one links to, deduplicated, in
- // order of first appearance. Filled in by a render pass, not by Scan.
+ // order of first appearance. Filled in by [Archive.LinkPass], not by Scan:
+ // links come out of a render, not out of a frontmatter parse.
Links []string `json:"links,omitempty"`
// WordCount is an approximate word count of the rendered text. Filled in by
- // a render pass, not by Scan.
+ // [Archive.LinkPass], from the same render.
WordCount int `json:"word_count,omitempty"`
}
M mcpsrv/backend.go => mcpsrv/backend.go +16 -54
@@ 6,11 6,8 @@ import (
"fmt"
"strings"
- "github.com/go-git/go-git/v5/plumbing"
-
"sourcecraft.dev/bigbes/sr-ht-spec/core"
"sourcecraft.dev/bigbes/sr-ht-spec/doc"
- "sourcecraft.dev/bigbes/sr-ht-spec/gitx"
"sourcecraft.dev/bigbes/sr-ht-spec/search"
"sourcecraft.dev/bigbes/sr-ht-spec/service"
)
@@ 18,10 15,10 @@ import (
// Reader is the part of the orchestration layer the read tools call.
// *service.Service satisfies it.
//
-// It is deliberately four methods. Every one of them is the same function the
+// It is deliberately three methods. Every one of them is the same function the
// REST handlers, the GraphQL resolvers and the web UI call, which is what the
// design means by "the MCP tools call the same resolver layer, not a parallel
-// implementation" — a fifth method here that service/ does not have would be
+// implementation" — a fourth method here that service/ does not have would be
// the beginning of a second implementation.
//
// ReadDocument is absent on purpose. Addressing a document by id needs the
@@ 31,8 28,11 @@ import (
type Reader interface {
ListSpaces(ctx context.Context) ([]*service.Space, error)
OpenSpace(ctx context.Context, ref core.SpaceRef) (*service.Space, error)
- ResolveRev(ctx context.Context, sp *service.Space, rev string) (string, error)
- ListDocuments(ctx context.Context, sp *service.Space, rev string) ([]service.Document, error)
+ // Archive resolves rev and returns the space's addressable document set at
+ // it, with every document's bytes, from one tree walk. It replaced a
+ // ResolveRev/ListDocuments pair here, and the conversion back into git
+ // documents this package used to perform to reach doc.FromDocuments.
+ Archive(ctx context.Context, sp *service.Space, rev string) (*doc.Archive, map[string][]byte, error)
}
// Searcher is the query side of the one global index. *search.Index satisfies
@@ 102,15 102,14 @@ func parseRev(s string) (string, error) {
return rev, nil
}
-// archiveAt reads a space at a revision and builds the addressable document
-// set, returning it together with the bodies keyed by path and the resolved
-// commit.
+// archiveAt reads a space at a revision and returns its addressable document
+// set, the bodies keyed by path, and the resolved commit.
//
-// The revision is resolved first and everything below is read at the resolved
-// sha, never at the caller's string. That is what makes an unpinned read
-// pinnable: the rev reported back names the exact bytes returned, so a merge
-// landing between the resolve and the read cannot make one answer describe two
-// revisions.
+// service/ does the work: it resolves the revision first and reads at the
+// resolved sha, never at the caller's string. That is what makes an unpinned
+// read pinnable — the rev reported back names the exact bytes returned, so a
+// merge landing between the resolve and the read cannot make one answer
+// describe two revisions.
//
// The whole space is read for one document. At the confirmed volume — tens of
// documents a day — that is cheap, and the alternative is worse: doc.Archive is
@@ 118,48 117,11 @@ func parseRev(s string) (string, error) {
// actually lives, and it is built from a revision's whole document set. A
// path-only fast path would be a second addressing implementation.
func archiveAt(ctx context.Context, b Backend, sp *service.Space, rev string) (*doc.Archive, map[string][]byte, string, error) {
- resolved, err := b.Docs.ResolveRev(ctx, sp, rev)
- if err != nil {
- return nil, nil, "", err
- }
- docs, err := b.Docs.ListDocuments(ctx, sp, resolved)
- if err != nil {
- return nil, nil, "", err
- }
- converted, err := toGitDocuments(docs)
+ arc, bodies, err := b.Docs.Archive(ctx, sp, rev)
if err != nil {
return nil, nil, "", err
}
- arc := doc.FromDocuments(sp.Ref, resolved, converted)
- bodies := make(map[string][]byte, len(docs))
- for _, d := range docs {
- bodies[d.Path] = d.Data
- }
- return arc, bodies, resolved, nil
-}
-
-// toGitDocuments converts service/'s hex-object-name documents back into the
-// shape doc.FromDocuments takes.
-//
-// This conversion should not exist, and it is the one place this package
-// reaches below service/. service.Document carries Blob and Rev as hex strings
-// precisely so that api/, mcpsrv/, graph/ and web/ need not import gitx — but
-// doc.FromDocuments, which owns the addressing rule those surfaces have to
-// apply, takes []gitx.Document. Until service/ exposes the archive itself,
-// something above it has to bridge the two, and doing it here in four lines is
-// better than restating the addressing rule in Go. A malformed sha is an
-// error, not a zero hash: plumbing.NewHash silently yields the zero value for
-// anything it cannot parse, and a document whose render-cache key is zero is a
-// cache collision waiting to happen.
-func toGitDocuments(docs []service.Document) ([]gitx.Document, error) {
- out := make([]gitx.Document, 0, len(docs))
- for _, d := range docs {
- if !plumbing.IsHash(d.Blob) {
- return nil, fmt.Errorf("document %q carries a malformed blob id %q", d.Path, d.Blob)
- }
- out = append(out, gitx.Document{Path: d.Path, Blob: plumbing.NewHash(d.Blob), Data: d.Data})
- }
- return out, nil
+ return arc, bodies, arc.Rev, nil
}
// resolvePage applies the design's addressing rule to one caller-supplied
M mcpsrv/mcpsrv_test.go => mcpsrv/mcpsrv_test.go +18 -13
@@ 12,6 12,7 @@ import (
"github.com/stretchr/testify/require"
"sourcecraft.dev/bigbes/sr-ht-spec/core"
+ "sourcecraft.dev/bigbes/sr-ht-spec/doc"
"sourcecraft.dev/bigbes/sr-ht-spec/mcpsrv"
"sourcecraft.dev/bigbes/sr-ht-spec/search"
"sourcecraft.dev/bigbes/sr-ht-spec/service"
@@ 68,7 69,23 @@ func (f *fakeReader) OpenSpace(_ context.Context, ref core.SpaceRef) (*service.S
return nil, fmt.Errorf("%w: space %s", service.ErrNotFound, ref)
}
-func (f *fakeReader) ResolveRev(_ context.Context, _ *service.Space, rev string) (string, error) {
+// Archive is the one read the tools make. It mirrors what service/ does —
+// resolve the revision first, read at the resolved sha, build the archive out
+// of the result — and builds it through service.ArchiveFrom, so the addressing
+// rule under test is the production one rather than a fixture's idea of it.
+func (f *fakeReader) Archive(_ context.Context, sp *service.Space, rev string) (*doc.Archive, map[string][]byte, error) {
+ resolved, err := f.resolveRev(rev)
+ if err != nil {
+ return nil, nil, err
+ }
+ if f.listErr != nil {
+ return nil, nil, f.listErr
+ }
+ f.lastRevs = append(f.lastRevs, resolved)
+ return service.ArchiveFrom(sp.Ref, resolved, f.revs[resolved])
+}
+
+func (f *fakeReader) resolveRev(rev string) (string, error) {
if rev == service.ApprovedRev {
return f.approved, nil
}
@@ 78,18 95,6 @@ func (f *fakeReader) ResolveRev(_ context.Context, _ *service.Space, rev string)
return rev, nil
}
-func (f *fakeReader) ListDocuments(_ context.Context, _ *service.Space, rev string) ([]service.Document, error) {
- if f.listErr != nil {
- return nil, f.listErr
- }
- f.lastRevs = append(f.lastRevs, rev)
- docs, ok := f.revs[rev]
- if !ok {
- return nil, fmt.Errorf("%w: revision %q", service.ErrNotFound, rev)
- }
- return docs, nil
-}
-
type fakeSearcher struct {
last search.Query
calls int
M => +1 -6
@@ 2,7 2,6 @@ package search
import (
"fmt"
"path"
"strings"
"sync"
@@ 62,11 61,7 @@ func Extract(arc *doc.Archive, bodies map[string][]byte) ([]Document, error) {
return nil, fmt.Errorf("search: no body supplied for %s in %s", p.Path, arc.Space)
}
front, body := doc.ParseFront(src)
dir := path.Dir(p.Path)
if dir == "." {
dir = ""
}
res := r.Render(body, dir, arc)
res := r.Render(body, doc.DirOf(p.Path), arc)
d := Document{
Space: arc.Space,
A service/archive.go => service/archive.go +97 -0
@@ 0,0 1,97 @@
+package service
+
+import (
+ "context"
+ "fmt"
+ "sync"
+
+ "github.com/go-git/go-git/v5/plumbing"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/core"
+ "sourcecraft.dev/bigbes/sr-ht-spec/doc"
+ "sourcecraft.dev/bigbes/sr-ht-spec/gitx"
+)
+
+// archiveRenderer is shared across calls: doc.Renderer is documented as
+// reusable and concurrency-safe, and building one per archive would rebuild the
+// whole goldmark pipeline on every read. search/ shares one the same way.
+var archiveRenderer = sync.OnceValue(doc.NewRenderer)
+
+// Archive builds a space's addressable document set at a revision, together
+// with every document's bytes, from one tree walk.
+//
+// This is the read every surface above this layer needs: the archive owns the
+// design's addressing rule (a document's id when it is well-formed and unique
+// in the space, otherwise its path), resolves the [[wikilinks]] between
+// documents, and carries the link graph backlinks are read from. Before it
+// existed, web/ scanned the git tree itself — reaching past this layer into
+// gitx, which the layering rule forbids — and mcpsrv/ converted this package's
+// documents back into git ones. Two workarounds around one missing accessor is
+// how three agent-facing surfaces stop being behaviourally identical.
+//
+// The revision is resolved first and the documents are read at the resolved
+// sha, so the archive and the bodies are the same revision by construction: a
+// merge landing mid-read cannot make one answer describe two revisions. The
+// resolved sha is the archive's Rev, and is what a caller hands back as the
+// pinned ?rev=.
+//
+// bodies is keyed by tree path — the key doc.Page.Path carries — and holds the
+// whole document, frontmatter included.
+func (s *Service) Archive(ctx context.Context, sp *Space, rev string) (*doc.Archive, map[string][]byte, error) {
+ commit, resolved, err := s.resolveRev(ctx, sp, rev)
+ if err != nil {
+ return nil, nil, err
+ }
+ docs, err := sp.Repo.ListDocuments(ctx, resolved)
+ if err != nil {
+ return nil, nil, readErr(err, "list documents at %s in %s", resolved, sp.Ref)
+ }
+ return archiveOf(sp.Ref, commit.String(), docs)
+}
+
+// ArchiveFrom builds the same archive out of documents this package already
+// returned, without touching git.
+//
+// It is the seam for a caller that holds a [Document] set — a test double
+// standing in for this service, a future surface that has already listed a
+// revision — and it exists so that the one conversion from a hex object name
+// back to a git hash lives here rather than in each of them. A malformed sha is
+// an error, not a zero hash: plumbing.NewHash silently yields the zero value
+// for anything it cannot parse, and a document whose render-cache key is zero
+// is a cache collision waiting to happen.
+//
+// rev must be the resolved commit the documents were read at, for the same
+// reason [Service.Archive] resolves before it reads.
+func ArchiveFrom(sp core.SpaceRef, rev string, docs []Document) (*doc.Archive, map[string][]byte, error) {
+ converted := make([]gitx.Document, 0, len(docs))
+ for _, d := range docs {
+ if !plumbing.IsHash(d.Blob) {
+ return nil, nil, fmt.Errorf("service: document %q carries a malformed blob id %q", d.Path, d.Blob)
+ }
+ converted = append(converted, gitx.Document{Path: d.Path, Blob: plumbing.NewHash(d.Blob), Data: d.Data})
+ }
+ return archiveOf(sp, rev, converted)
+}
+
+// archiveOf assembles the archive, the bodies and the link graph out of one
+// revision's documents.
+//
+// The link pass runs here rather than at each caller. doc.Page.Links is what
+// Archive.Backlinks reads, and an archive handed out without it looks like a
+// space where nothing links to anything — which is what every surface saw
+// until web/ grew its own pass and re-rendered the revision on each page view.
+// Filling it once, in the accessor, is the difference between a link graph and
+// a per-surface convention. It costs one render of each document; at the
+// confirmed volume — tens of documents a day — that is the same walk's data
+// being parsed once more, not a second read.
+func archiveOf(sp core.SpaceRef, rev string, docs []gitx.Document) (*doc.Archive, map[string][]byte, error) {
+ arc := doc.FromDocuments(sp, rev, docs)
+ bodies := make(map[string][]byte, len(docs))
+ for _, d := range docs {
+ bodies[d.Path] = d.Data
+ }
+ if err := arc.LinkPass(archiveRenderer(), bodies); err != nil {
+ return nil, nil, fmt.Errorf("service: archive of %s at %s: %w", sp, rev, err)
+ }
+ return arc, bodies, nil
+}
A service/archive_test.go => service/archive_test.go +108 -0
@@ 0,0 1,108 @@
+package service
+
+import (
+ "context"
+ "testing"
+)
+
+// The accessor every surface above this layer reads a space through: one call,
+// the addressing rule applied, the bodies alongside, and the link graph filled
+// in. web/ used to scan the git tree itself and mcpsrv/ used to convert this
+// package's documents back into git ones; both are gone because this exists.
+func TestArchiveReturnsDocumentsBodiesAndLinks(t *testing.T) {
+ svc, root := newService(t)
+ sp := newSpace(t, root, 1)
+ ctx := context.Background()
+
+ head := commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
+ "specs/0007-storage.md": mdDoc("SPEC-0007", "Storage model", "Supersedes [[SPEC-0003]]."),
+ "specs/0003-old.md": mdDoc("SPEC-0003", "Older sketch", "The first attempt."),
+ "notes/plain.md": []byte("# Just a note\n\nNo frontmatter at all.\n"),
+ })
+
+ arc, bodies, err := svc.Archive(ctx, sp, ApprovedRev)
+ if err != nil {
+ t.Fatalf("Archive: %v", err)
+ }
+
+ // The revision is resolved before anything is read, so the archive names
+ // the commit it describes and a caller can pin to it.
+ if arc.Rev != head.String() {
+ t.Errorf("archive Rev = %s, want the resolved approved head %s", arc.Rev, head)
+ }
+ if arc.Space != sp.Ref {
+ t.Errorf("archive space = %s, want %s", arc.Space, sp.Ref)
+ }
+ if len(arc.All()) != 3 || len(bodies) != 3 {
+ t.Fatalf("archive holds %d documents and %d bodies, want 3 of each", len(arc.All()), len(bodies))
+ }
+
+ // The addressing rule: a well-formed unique id addresses the document, and
+ // a document without one is addressed by its path.
+ page, ok := arc.Page("SPEC-0007")
+ if !ok || page.Path != "specs/0007-storage.md" {
+ t.Fatalf("SPEC-0007 = %+v, ok=%v", page, ok)
+ }
+ if _, ok := arc.Page("notes/plain"); !ok {
+ t.Error("a document with no frontmatter must be addressed by its path")
+ }
+
+ // Bodies are the whole document, keyed the way the archive keys pages.
+ if body, ok := bodies[page.Path]; !ok || !contains(body, "Supersedes") {
+ t.Errorf("body of %s = %q", page.Path, body)
+ }
+
+ // The link graph: filled here, so Backlinks answers rather than always
+ // returning nothing.
+ if len(page.Links) != 1 || page.Links[0] != "SPEC-0003" {
+ t.Errorf("links of SPEC-0007 = %v, want [SPEC-0003]", page.Links)
+ }
+ if page.WordCount == 0 {
+ t.Error("word count is filled by the same pass and must not be zero")
+ }
+ back := arc.Backlinks("SPEC-0003")
+ if len(back) != 1 || back[0].ID != "SPEC-0007" {
+ t.Fatalf("Backlinks(SPEC-0003) = %v, want SPEC-0007", back)
+ }
+}
+
+// A pinned revision and the approved head are the same call with a different
+// revision string, and the read contract still applies: a branch name is not a
+// revision this method will serve.
+func TestArchiveHonoursTheReadContract(t *testing.T) {
+ svc, root := newService(t)
+ sp := newSpace(t, root, 1)
+ ctx := context.Background()
+
+ first := commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
+ "specs/0007-storage.md": mdDoc("SPEC-0007", "Storage model", "first"),
+ })
+ commitFiles(t, sp, sp.ApprovedBranch(), 2, map[string][]byte{
+ "specs/0007-storage.md": mdDoc("SPEC-0007", "Storage model", "second"),
+ })
+
+ _, bodies, err := svc.Archive(ctx, sp, first.String())
+ if err != nil {
+ t.Fatalf("pinned Archive: %v", err)
+ }
+ if !contains(bodies["specs/0007-storage.md"], "first") {
+ t.Errorf("pinned archive returned %q, want the pinned revision", bodies["specs/0007-storage.md"])
+ }
+
+ cutBranch(t, sp, "proposals/1", sp.ApprovedBranch())
+ if _, _, err := svc.Archive(ctx, sp, "proposals/1"); err == nil {
+ t.Fatal("Archive served a branch name; the read plane takes object names only")
+ }
+}
+
+// ArchiveFrom is the same construction over documents a caller already holds,
+// and it refuses a document whose blob id is not an object name rather than
+// keying the render cache on a zero hash.
+func TestArchiveFromRefusesAMalformedBlobID(t *testing.T) {
+ _, _, err := ArchiveFrom(fxSpace, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", []Document{
+ {Path: "specs/0007-storage.md", Blob: "not-a-sha", Data: mdDoc("SPEC-0007", "Storage model", "body")},
+ })
+ if err == nil {
+ t.Fatal("ArchiveFrom accepted a malformed blob id")
+ }
+}
M service/doc.go => service/doc.go +5 -3
@@ 17,9 17,11 @@
// - Space lifecycle: [Service.CreateSpace], [Service.OpenSpace],
// [Service.ListSpaces].
// - Reads: [Service.ReadDocument], [Service.ListDocuments],
-// [Service.Policy] — each resolving either the approved head or a pinned
-// revision through the same code path, because there is one storage tier
-// and no checkout.
+// [Service.Archive], [Service.Policy] — each resolving either the approved
+// head or a pinned revision through the same code path, because there is
+// one storage tier and no checkout. [Service.Archive] is the one every
+// surface that shows a space reads through: it applies the addressing rule
+// and fills the link graph once, here, rather than in each of them.
// - Push validation: [Service.ValidatePush], the function the `update` hook
// calls over RPC before a ref moves.
// - Repair: [Reconcile] and [Service.RunReconciler].
M service/fixture_test.go => service/fixture_test.go +2 -2
@@ 148,8 148,8 @@ func cutBranch(t *testing.T, sp *Space, branch, base string) {
}
}
-// doc renders a minimal valid document: the three required keys, then a body.
-func doc(id, title, body string) []byte {
+// mdDoc renders a minimal valid document: the three required keys, then a body.
+func mdDoc(id, title, body string) []byte {
return []byte(fmt.Sprintf("---\nid: %s\ntitle: %s\nstatus: draft\n---\n\n%s\n", id, title, body))
}
M service/integration_test.go => service/integration_test.go +3 -3
@@ 28,7 28,7 @@ func TestValidatePushAcceptsAGoodPush(t *testing.T) {
t.Fatalf("approved head: %v", err)
}
head := commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
- "specs/0007.md": doc("SPEC-0007", "Storage", "body"),
+ "specs/0007.md": mdDoc("SPEC-0007", "Storage", "body"),
})
err = svc.ValidatePush(ctx, PushRequest{
@@ 64,7 64,7 @@ func TestValidatePushRejectsAGlobalIDCollision(t *testing.T) {
t.Fatal(err)
}
head := commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
- "specs/0007.md": doc("SPEC-0007", "Storage", "body"),
+ "specs/0007.md": mdDoc("SPEC-0007", "Storage", "body"),
})
err = svc.ValidatePush(ctx, PushRequest{
@@ 156,7 156,7 @@ func TestReconcileRepairsTheRepairTable(t *testing.T) {
merged := mustOpenProposal(t, svc, sp.ID, "merged", base.String())
cutBranch(t, sp, merged.Branch, sp.ApprovedBranch())
commitFiles(t, sp, merged.Branch, 1, map[string][]byte{
- "specs/0007.md": doc("SPEC-0007", "Storage", "body"),
+ "specs/0007.md": mdDoc("SPEC-0007", "Storage", "body"),
})
branchHead, err := sp.Repo.BranchHead(ctx, merged.Branch)
if err != nil {
M service/push_test.go => service/push_test.go +12 -12
@@ 26,14 26,14 @@ func agent() authn.Principal {
func TestValidateDocuments(t *testing.T) {
schema := core.DefaultSchema()
- spec7 := Document{Path: "specs/0007.md", Blob: "a", Data: doc("SPEC-0007", "Storage", "body")}
- spec8 := Document{Path: "specs/0008.md", Blob: "b", Data: doc("SPEC-0008", "Other", "body")}
- dupe := Document{Path: "notes/copy.md", Blob: "c", Data: doc("SPEC-0007", "Copy", "body")}
+ spec7 := Document{Path: "specs/0007.md", Blob: "a", Data: mdDoc("SPEC-0007", "Storage", "body")}
+ spec8 := Document{Path: "specs/0008.md", Blob: "b", Data: mdDoc("SPEC-0008", "Other", "body")}
+ dupe := Document{Path: "notes/copy.md", Blob: "c", Data: mdDoc("SPEC-0007", "Copy", "body")}
noStatus := Document{Path: "specs/0009.md", Blob: "d",
Data: []byte("---\nid: SPEC-0009\ntitle: No status\n---\n\nbody\n")}
noFrontmatter := Document{Path: "specs/0010.md", Blob: "e", Data: []byte("# just a heading\n")}
badID := Document{Path: "specs/0011.md", Blob: "f",
- Data: doc("spec-11", "Lowercase id", "body")}
+ Data: mdDoc("spec-11", "Lowercase id", "body")}
tests := []struct {
name string
@@ 232,14 232,14 @@ func TestCheckRefsRule(t *testing.T) {
ctx := context.Background()
first := commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
- "specs/0007.md": doc("SPEC-0007", "Storage", "one"),
+ "specs/0007.md": mdDoc("SPEC-0007", "Storage", "one"),
})
second := commitFiles(t, sp, sp.ApprovedBranch(), 2, map[string][]byte{
- "specs/0007.md": doc("SPEC-0007", "Storage", "two"),
+ "specs/0007.md": mdDoc("SPEC-0007", "Storage", "two"),
})
cutBranch(t, sp, "proposals/1", sp.ApprovedBranch())
branch := commitFiles(t, sp, "proposals/1", 3, map[string][]byte{
- "specs/0008.md": doc("SPEC-0008", "Other", "one"),
+ "specs/0008.md": mdDoc("SPEC-0008", "Other", "one"),
})
tests := []struct {
@@ 297,12 297,12 @@ func TestChangedDocumentsBaselinesANewBranchOnTheApprovedHead(t *testing.T) {
ctx := context.Background()
commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
- "specs/0007.md": doc("SPEC-0007", "Storage", "one"),
+ "specs/0007.md": mdDoc("SPEC-0007", "Storage", "one"),
"specs/0009.md": []byte("not a document at all\n"),
})
cutBranch(t, sp, "proposals/1", sp.ApprovedBranch())
branch := commitFiles(t, sp, "proposals/1", 2, map[string][]byte{
- "specs/0008.md": doc("SPEC-0008", "Other", "one"),
+ "specs/0008.md": mdDoc("SPEC-0008", "Other", "one"),
})
all, changed, err := svc.changedDocuments(ctx, sp, zeroHash(t), branch)
@@ 323,11 323,11 @@ func TestChangedDocumentsUsesTheOldRefValueWhenThereIsOne(t *testing.T) {
ctx := context.Background()
old := commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
- "specs/0007.md": doc("SPEC-0007", "Storage", "one"),
- "specs/0008.md": doc("SPEC-0008", "Other", "one"),
+ "specs/0007.md": mdDoc("SPEC-0007", "Storage", "one"),
+ "specs/0008.md": mdDoc("SPEC-0008", "Other", "one"),
})
newHead := commitFiles(t, sp, sp.ApprovedBranch(), 2, map[string][]byte{
- "specs/0008.md": doc("SPEC-0008", "Other", "two"),
+ "specs/0008.md": mdDoc("SPEC-0008", "Other", "two"),
})
_, changed, err := svc.changedDocuments(ctx, sp, old, newHead)
M service/read_test.go => service/read_test.go +7 -7
@@ 15,10 15,10 @@ func TestReadDocumentResolvesTheApprovedHeadByDefault(t *testing.T) {
ctx := context.Background()
first := commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
- "specs/0007-storage.md": doc("SPEC-0007", "Storage model", "first"),
+ "specs/0007-storage.md": mdDoc("SPEC-0007", "Storage model", "first"),
})
second := commitFiles(t, sp, sp.ApprovedBranch(), 2, map[string][]byte{
- "specs/0007-storage.md": doc("SPEC-0007", "Storage model", "second"),
+ "specs/0007-storage.md": mdDoc("SPEC-0007", "Storage model", "second"),
})
got, err := svc.ReadDocument(ctx, sp, ApprovedRev, "specs/0007-storage.md")
@@ 57,11 57,11 @@ func TestReadDocumentDoesNotServeAProposalByDefault(t *testing.T) {
ctx := context.Background()
commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
- "specs/0007-storage.md": doc("SPEC-0007", "Storage model", "approved"),
+ "specs/0007-storage.md": mdDoc("SPEC-0007", "Storage model", "approved"),
})
cutBranch(t, sp, "proposals/1", sp.ApprovedBranch())
commitFiles(t, sp, "proposals/1", 2, map[string][]byte{
- "specs/0007-storage.md": doc("SPEC-0007", "Storage model", "draft"),
+ "specs/0007-storage.md": mdDoc("SPEC-0007", "Storage model", "draft"),
})
approved, err := svc.ReadDocument(ctx, sp, ApprovedRev, "specs/0007-storage.md")
@@ 118,8 118,8 @@ func TestListDocumentsReturnsEveryDocumentAtARevision(t *testing.T) {
ctx := context.Background()
commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
- "specs/0007-storage.md": doc("SPEC-0007", "Storage", "body"),
- "notes/scratch.md": doc("NOTE-0001", "Scratch", "body"),
+ "specs/0007-storage.md": mdDoc("SPEC-0007", "Storage", "body"),
+ "notes/scratch.md": mdDoc("NOTE-0001", "Scratch", "body"),
"attachment.png": []byte("\x89PNG not a document"),
core.PolicyFile: []byte("review:\n auto_merge: [notes/**]\n"),
})
@@ 204,7 204,7 @@ func TestResolveRevPinsTheApprovedHead(t *testing.T) {
sp := newSpace(t, root, 1)
head := commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
- "specs/0007-storage.md": doc("SPEC-0007", "Storage", "body"),
+ "specs/0007-storage.md": mdDoc("SPEC-0007", "Storage", "body"),
})
got, err := svc.ResolveRev(context.Background(), sp, ApprovedRev)
if err != nil {
M service/reconcile_test.go => service/reconcile_test.go +1 -1
@@ 231,7 231,7 @@ func TestDeleteProposalRefRemovesOnlyProposalBranches(t *testing.T) {
ctx := context.Background()
commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
- "specs/0007.md": doc("SPEC-0007", "Storage", "one"),
+ "specs/0007.md": mdDoc("SPEC-0007", "Storage", "one"),
})
cutBranch(t, sp, "proposals/1", sp.ApprovedBranch())
M web/handlers.go => web/handlers.go +4 -41
@@ 7,7 7,6 @@ import (
"html/template"
"log"
"net/http"
- "path"
"strings"
"github.com/go-chi/chi/v5"
@@ 424,13 423,10 @@ func (s *Server) handleDocument(w http.ResponseWriter, r *http.Request) {
return
}
- // Backlinks need the whole revision's link graph, and doc.Page.Links is
- // filled by a render pass rather than by Scan — so the pass happens here.
- if err := s.linkPass(snap); err != nil {
- s.fail(w, r, err)
- return
- }
- res := s.renderer.Render(mdBody, dirOf(docPath), pinned{inner: snap.Archive, rq: rq})
+ // The link graph backlinks are read from is filled in by the reader, at the
+ // same revision as the archive itself. This handler used to render every
+ // document of the space here to build it, once per page view.
+ res := s.renderer.Render(mdBody, doc.DirOf(docPath), pinned{inner: snap.Archive, rq: rq})
data := docData{
SpaceRef: ref.String(),
@@ 497,29 493,6 @@ func (p pinned) Resolve(fromDir, dest string) doc.Target {
return t
}
-// linkPass renders every document of the revision to fill in doc.Page.Links,
-// which is what Archive.Backlinks reads. Scan deliberately does not do it —
-// links come out of a render, not out of a frontmatter parse.
-//
-// A page with no body is an inconsistency between the archive and the bodies,
-// which cannot happen while both are read at one resolved sha. It is reported
-// rather than skipped: skipping it would silently drop that document's outbound
-// links and quietly under-report backlinks everywhere else.
-func (s *Server) linkPass(snap *Snapshot) error {
- for _, p := range snap.Archive.All() {
- raw, ok := snap.Bodies[p.Path]
- if !ok {
- return fmt.Errorf("web: %s is in the archive of %s at %s but has no body",
- p.Path, snap.Ref, snap.Rev)
- }
- _, mdBody := doc.ParseFront(raw)
- res := s.renderer.Render(mdBody, dirOf(p.Path), snap.Archive)
- p.Links = res.LinkedIDs
- p.WordCount = res.WordCount
- }
- return nil
-}
-
// ---- search ---------------------------------------------------------------
type searchHit struct {
@@ 637,13 610,3 @@ func revQuery(rev string) string {
}
return "?rev=" + rev
}
-
-// dirOf is the directory a document lives in, space-relative, with "" for the
-// space root — the shape doc's resolver expects.
-func dirOf(p string) string {
- d := path.Dir(p)
- if d == "." || d == "/" {
- return ""
- }
- return d
-}
M web/reader.go => web/reader.go +9 -22
@@ 2,7 2,6 @@ package web
import (
"context"
- "fmt"
"sourcecraft.dev/bigbes/sr-ht-spec/core"
"sourcecraft.dev/bigbes/sr-ht-spec/doc"
@@ 18,10 17,9 @@ import (
// a merge landing mid-request cannot make one page describe two revisions —
// and the sha it resolved to is the value the page offers as its permalink.
//
-// Bodies is keyed by tree path, the same key doc.Page.Path carries. It is
-// present because doc.Page.Links is filled by a render pass rather than by
-// Scan, so backlinks require rendering every document in the space; see
-// linkPass.
+// Bodies is keyed by tree path, the same key doc.Page.Path carries. It comes
+// off the same tree walk as the archive, so a document listed here and a
+// document in the archive are the same bytes at the same revision.
type Snapshot struct {
Ref core.SpaceRef
Rev string
@@ 78,26 76,15 @@ func (r serviceReader) Snapshot(ctx context.Context, ref core.SpaceRef, rev stri
if err != nil {
return nil, err
}
- resolved, err := r.svc.ResolveRev(ctx, sp, rev)
+ // One call, one tree walk, and the link graph filled in. This package used
+ // to scan the git tree itself and then list the documents a second time for
+ // their bodies — two walks per page view, and a reach past service/ into
+ // gitx that the layering rule forbids.
+ arc, bodies, err := r.svc.Archive(ctx, sp, rev)
if err != nil {
return nil, err
}
- // Both reads below are issued against the resolved sha rather than against
- // the caller's rev, so the archive and the bodies are the same revision by
- // construction and cannot disagree if a merge lands between them.
- arc, err := doc.Scan(ctx, sp.Repo, ref, resolved)
- if err != nil {
- return nil, fmt.Errorf("web: scan %s at %s: %w", ref, resolved, err)
- }
- docs, err := r.svc.ListDocuments(ctx, sp, resolved)
- if err != nil {
- return nil, err
- }
- bodies := make(map[string][]byte, len(docs))
- for _, d := range docs {
- bodies[d.Path] = d.Data
- }
- return &Snapshot{Ref: ref, Rev: resolved, Archive: arc, Bodies: bodies}, nil
+ return &Snapshot{Ref: ref, Rev: arc.Rev, Archive: arc, Bodies: bodies}, nil
}
func (r serviceReader) ReadDocument(ctx context.Context, ref core.SpaceRef, rev, p string) (service.Document, error) {
M web/web_test.go => web/web_test.go +27 -15
@@ 3,7 3,9 @@ package web
import (
"context"
"crypto/rand"
+ "crypto/sha1"
"encoding/base64"
+ "encoding/hex"
"encoding/json"
"fmt"
"net/http"
@@ 14,14 16,11 @@ import (
"testing"
"github.com/fernet/fernet-go"
- "github.com/go-git/go-git/v5/plumbing"
"github.com/vaughan0/go-ini"
"sourcecraft.dev/bigbes/sr-ht-core/crypto"
"sourcecraft.dev/bigbes/sr-ht-spec/authn"
"sourcecraft.dev/bigbes/sr-ht-spec/core"
- "sourcecraft.dev/bigbes/sr-ht-spec/doc"
- "sourcecraft.dev/bigbes/sr-ht-spec/gitx"
"sourcecraft.dev/bigbes/sr-ht-spec/search"
"sourcecraft.dev/bigbes/sr-ht-spec/service"
)
@@ 150,23 149,36 @@ func (f *fakeReader) Snapshot(_ context.Context, ref core.SpaceRef, rev string)
}
sort.Strings(paths)
- gd := make([]gitx.Document, 0, len(paths))
- bodies := make(map[string][]byte, len(paths))
+ // Built through service.ArchiveFrom, exactly as serviceReader gets it: the
+ // addressing rule and the link graph under test are the production ones,
+ // not a fixture's idea of them.
+ sd := make([]service.Document, 0, len(paths))
for _, p := range paths {
data := []byte(docs[p])
- gd = append(gd, gitx.Document{
+ sd = append(sd, service.Document{
Path: p,
- Blob: plumbing.ComputeHash(plumbing.BlobObject, data),
+ Blob: blobSha(data),
+ Rev: resolved,
Data: data,
})
- bodies[p] = data
}
- return &Snapshot{
- Ref: ref,
- Rev: resolved,
- Archive: doc.FromDocuments(ref, resolved, gd),
- Bodies: bodies,
- }, nil
+ arc, bodies, err := service.ArchiveFrom(ref, resolved, sd)
+ if err != nil {
+ return nil, err
+ }
+ return &Snapshot{Ref: ref, Rev: resolved, Archive: arc, Bodies: bodies}, nil
+}
+
+// blobSha is git's object name for a blob: sha1 over "blob <len>\0" and the
+// content. Spelled out rather than taken from go-git so that this package —
+// including its tests — never imports the git layer, which is the layering rule
+// the Reader interface exists to keep.
+func blobSha(data []byte) string {
+ h := sha1.New()
+ fmt.Fprintf(h, "blob %d", len(data))
+ h.Write([]byte{0})
+ h.Write(data)
+ return hex.EncodeToString(h.Sum(nil))
}
func (f *fakeReader) ReadDocument(_ context.Context, ref core.SpaceRef, rev, p string) (service.Document, error) {
@@ 180,7 192,7 @@ func (f *fakeReader) ReadDocument(_ context.Context, ref core.SpaceRef, rev, p s
}
return service.Document{
Path: p,
- Blob: plumbing.ComputeHash(plumbing.BlobObject, []byte(content)).String(),
+ Blob: blobSha([]byte(content)),
Rev: resolved,
Data: []byte(content),
}, nil