Simplify dependency tree in internal/document (#483)

Co-authored-by: Sebastian Tiedtke <sebastiantiedtke@gmail.com>

internal/cmd/common.go

@@ -31,16 +31,14 @@ func getIdentityResolver() *identity.IdentityResolver {
 }
 
 func getProject() (*project.Project, error) {
-	opts := []project.ProjectOption{
-		project.WithIdentityResolver(getIdentityResolver()),
-	}
-
 	logger, err := getLogger(false)
 	if err != nil {
 		return nil, err
 	}
 
-	opts = append(opts, project.WithLogger(logger))
+	opts := []project.ProjectOption{
+		project.WithLogger(logger),
+	}
 
 	var proj *project.Project
 

internal/document/block.go

@@ -8,10 +8,10 @@ import (
 	"strings"
 	"unicode"
 
-	"github.com/stateful/runme/internal/document/identity"
+	"github.com/yuin/goldmark/ast"
+
 	"github.com/stateful/runme/internal/executable"
 	"github.com/stateful/runme/internal/shell"
-	"github.com/yuin/goldmark/ast"
 )
 
 type BlockKind int
@@ -67,7 +67,7 @@ type CodeBlock struct {
 func newCodeBlock(
 	document *Document,
 	node *ast.FencedCodeBlock,
-	identityResolver *identity.IdentityResolver,
+	identityResolver identityResolver,
 	nameResolver *nameResolver,
 	source []byte,
 	render Renderer,

internal/document/constants/constants.go

@@ -1,6 +1,3 @@
 package constants
 
-const (
-	MaxFinalLineBreaks = 10
-	FinalLineBreaksKey = "finalLineBreaks"
-)
+const FinalLineBreaksKey = "finalLineBreaks"

internal/document/document.go

@@ -12,15 +12,14 @@ import (
 	"github.com/yuin/goldmark/text"
 
 	"github.com/stateful/runme/internal/document/constants"
-	"github.com/stateful/runme/internal/document/identity"
 	"github.com/stateful/runme/internal/renderer/cmark"
 )
 
 var DefaultRenderer = cmark.Render
 
 type Document struct {
 	source           []byte
-	identityResolver *identity.IdentityResolver
+	identityResolver identityResolver
 	nameResolver     *nameResolver
 	parser           parser.Parser
 	renderer         Renderer
@@ -41,7 +40,7 @@ type Document struct {
 	frontmatter          *Frontmatter
 }
 
-func New(source []byte, identityResolver *identity.IdentityResolver) *Document {
+func New(source []byte, identityResolver identityResolver) *Document {
 	return &Document{
 		source:           source,
 		identityResolver: identityResolver,

internal/document/document_test.go

@@ -4,13 +4,14 @@ import (
 	"bytes"
 	"testing"
 
-	"github.com/stateful/runme/internal/document/identity"
-	"github.com/stateful/runme/internal/renderer/cmark"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/stateful/runme/internal/document/identity"
+	"github.com/stateful/runme/internal/renderer/cmark"
 )
 
-var identityResolver = identity.NewResolver(identity.AllLifecycleIdentity)
+var testIdentityResolver = identity.NewResolver(identity.DefaultLifecycleIdentity)
 
 func TestDocument_Parse(t *testing.T) {
 	data := []byte(`# Examples
@@ -35,7 +36,7 @@ First paragraph.
 2. Item 2
 3. Item 3
 `)
-	doc := New(data, identityResolver)
+	doc := New(data, testIdentityResolver)
 	node, err := doc.Root()
 	require.NoError(t, err)
 	assert.Len(t, node.children, 4)
@@ -79,15 +80,15 @@ First paragraph
 `,
 		))
 
-		doc := New(data, identityResolver)
+		doc := New(data, testIdentityResolver)
 		err := doc.Parse()
 		require.NoError(t, err)
 		assert.Equal(t, []byte("First paragraph"), doc.Content())
 		assert.Equal(t, 20, doc.ContentOffset())
 
 		frontmatter, err := doc.Frontmatter()
 		require.NoError(t, err)
-		marshaledFrontmatter, err := frontmatter.Marshal(identityResolver.DocumentEnabled())
+		marshaledFrontmatter, err := frontmatter.Marshal(testIdentityResolver.DocumentEnabled())
 		require.NoError(t, err)
 		assert.Regexp(t, `---\nkey: value\nrunme:\n  id: .*\n  version: v(?:[3-9]\d*|2\.\d+\.\d+|2\.\d+)\n---`, string(marshaledFrontmatter))
 	})
@@ -136,7 +137,7 @@ shell = "fish"
 
 		for _, tc := range testCases {
 			t.Run(tc.Name, func(t *testing.T) {
-				doc := New(tc.Data, identityResolver)
+				doc := New(tc.Data, testIdentityResolver)
 				_, err := doc.Root()
 				require.NoError(t, err)
 
@@ -157,7 +158,7 @@ shell = "fish"
 ---
 `,
 		))
-		doc := New(data, identityResolver)
+		doc := New(data, testIdentityResolver)
 		err := doc.Parse()
 		require.NoError(t, err)
 
@@ -171,7 +172,7 @@ func TestDocument_TrailingLineBreaks(t *testing.T) {
 	data := []byte(`This will test final line breaks`)
 
 	t.Run("No breaks", func(t *testing.T) {
-		doc := New(data, identityResolver)
+		doc := New(data, testIdentityResolver)
 		astNode, err := doc.RootAST()
 		require.NoError(t, err)
 
@@ -188,7 +189,7 @@ func TestDocument_TrailingLineBreaks(t *testing.T) {
 
 	t.Run("1 LF", func(t *testing.T) {
 		withLineBreaks := append(data, bytes.Repeat([]byte{'\n'}, 1)...)
-		doc := New(withLineBreaks, identityResolver)
+		doc := New(withLineBreaks, testIdentityResolver)
 		astNode, err := doc.RootAST()
 		require.NoError(t, err)
 
@@ -205,7 +206,7 @@ func TestDocument_TrailingLineBreaks(t *testing.T) {
 
 	t.Run("1 CRLF", func(t *testing.T) {
 		withLineBreaks := append(data, bytes.Repeat([]byte{'\r', '\n'}, 1)...)
-		doc := New(withLineBreaks, identityResolver)
+		doc := New(withLineBreaks, testIdentityResolver)
 		astNode, err := doc.RootAST()
 		require.NoError(t, err)
 
@@ -222,7 +223,7 @@ func TestDocument_TrailingLineBreaks(t *testing.T) {
 
 	t.Run("7 LFs", func(t *testing.T) {
 		withLineBreaks := append(data, bytes.Repeat([]byte{'\n'}, 7)...)
-		doc := New(withLineBreaks, identityResolver)
+		doc := New(withLineBreaks, testIdentityResolver)
 		astNode, err := doc.RootAST()
 		require.NoError(t, err)
 

internal/document/editor/cell.go

@@ -12,9 +12,10 @@ import (
 	"strings"
 	"time"
 
-	"github.com/stateful/runme/internal/document"
-	ulid "github.com/stateful/runme/internal/ulid"
 	"github.com/yuin/goldmark/ast"
+
+	"github.com/stateful/runme/internal/document"
+	"github.com/stateful/runme/internal/ulid"
 )
 
 const (

internal/document/editor/cell_test.go

@@ -5,9 +5,10 @@ import (
 	"encoding/json"
 	"testing"
 
-	"github.com/stateful/runme/internal/document"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/stateful/runme/internal/document"
 )
 
 var (

internal/document/editor/editor.go

@@ -6,8 +6,8 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
-	"github.com/stateful/runme/internal/document"
 
+	"github.com/stateful/runme/internal/document"
 	"github.com/stateful/runme/internal/document/constants"
 	"github.com/stateful/runme/internal/document/identity"
 )

internal/document/editor/editorservice/service.go

@@ -5,12 +5,12 @@ import (
 	"encoding/base64"
 	"strings"
 
+	"go.uber.org/zap"
+
 	"github.com/stateful/runme/internal/document"
 	"github.com/stateful/runme/internal/document/editor"
 	"github.com/stateful/runme/internal/document/identity"
 	parserv1 "github.com/stateful/runme/internal/gen/proto/go/runme/parser/v1"
-	"go.uber.org/zap"
-	"golang.org/x/exp/constraints"
 )
 
 type parserServiceServer struct {
@@ -26,7 +26,7 @@ func NewParserServiceServer(logger *zap.Logger) parserv1.ParserServiceServer {
 func (s *parserServiceServer) Deserialize(_ context.Context, req *parserv1.DeserializeRequest) (*parserv1.DeserializeResponse, error) {
 	s.logger.Info("Deserialize", zap.ByteString("source", req.Source[:min(len(req.Source), 64)]))
 
-	identityResolver := identity.NewResolver(identity.ToLifecycleIdentity(req.Options.Identity))
+	identityResolver := identity.NewResolver(fromProtoRunmeIdentityToLifecycleIdentity(req.Options.Identity))
 	notebook, err := editor.Deserialize(req.Source, identityResolver)
 	if err != nil {
 		s.logger.Info("failed to call Deserialize", zap.Error(err))
@@ -237,9 +237,15 @@ func (*parserServiceServer) serializeCellOutputs(cell *parserv1.Cell, options *p
 	return outputs
 }
 
-func min[T constraints.Ordered](a, b T) T {
-	if a < b {
-		return a
+func fromProtoRunmeIdentityToLifecycleIdentity(idt parserv1.RunmeIdentity) identity.LifecycleIdentity {
+	switch idt {
+	case parserv1.RunmeIdentity_RUNME_IDENTITY_ALL:
+		return identity.AllLifecycleIdentity
+	case parserv1.RunmeIdentity_RUNME_IDENTITY_DOCUMENT:
+		return identity.DocumentLifecycleIdentity
+	case parserv1.RunmeIdentity_RUNME_IDENTITY_CELL:
+		return identity.CellLifecycleIdentity
+	default:
+		return identity.UnspecifiedLifecycleIdentity
 	}
-	return b
 }

internal/document/identity.go

@@ -0,0 +1,5 @@
+package document
+
+type identityResolver interface {
+	GetCellID(obj any, attributes map[string]string) (string, bool)
+}

internal/document/identity/resolver.go

@@ -1,8 +1,9 @@
 package identity
 
 import (
-	parserv1 "github.com/stateful/runme/internal/gen/proto/go/runme/parser/v1"
-	ulid "github.com/stateful/runme/internal/ulid"
+	"sync"
+
+	"github.com/stateful/runme/internal/ulid"
 )
 
 type LifecycleIdentity int
@@ -22,12 +23,7 @@ const (
 	CellLifecycleIdentity
 )
 
-// LifecycleIdentities is a slice of LifecycleIdentity.
-type LifecycleIdentities []LifecycleIdentity
-
-const (
-	DefaultLifecycleIdentity = AllLifecycleIdentity
-)
+const DefaultLifecycleIdentity = AllLifecycleIdentity
 
 var documentIdentities = &LifecycleIdentities{
 	AllLifecycleIdentity,
@@ -39,29 +35,29 @@ var cellIdentities = &LifecycleIdentities{
 	CellLifecycleIdentity,
 }
 
+type LifecycleIdentities []LifecycleIdentity
+
 // Contains returns true if the required identity is contained in the provided identities.
-func (required *LifecycleIdentities) Contains(id LifecycleIdentity) bool {
-	for _, v := range *required {
+func (ids LifecycleIdentities) Contains(id LifecycleIdentity) bool {
+	for _, v := range ids {
 		if v == id {
 			return true
 		}
 	}
 	return false
 }
 
-// IdentityResolver resolves object identities.
 type IdentityResolver struct {
 	documentIdentity bool
 	cellIdentity     bool
-	cache            map[interface{}]string
+	cache            *sync.Map
 }
 
-// NewResolver creates a new resolver.
 func NewResolver(required LifecycleIdentity) *IdentityResolver {
 	return &IdentityResolver{
 		documentIdentity: documentIdentities.Contains(required),
 		cellIdentity:     cellIdentities.Contains(required),
-		cache:            map[interface{}]string{},
+		cache:            &sync.Map{},
 	}
 }
 
@@ -76,7 +72,7 @@ func (ir *IdentityResolver) DocumentEnabled() bool {
 }
 
 // GetCellID returns a cell ID and a boolean indicating if it's new or from attributes.
-func (ir *IdentityResolver) GetCellID(obj interface{}, attributes map[string]string) (string, bool) {
+func (ir *IdentityResolver) GetCellID(obj any, attributes map[string]string) (string, bool) {
 	if !ir.cellIdentity {
 		return "", false
 	}
@@ -85,30 +81,16 @@ func (ir *IdentityResolver) GetCellID(obj interface{}, attributes map[string]str
 	// Check for a valid 'id' in attributes;
 	// if present and valid due to explicit cell identity cache and return it.
 	if n, ok := attributes["id"]; ok && ulid.ValidID(n) {
-		ir.cache[obj] = n
+		ir.cache.Store(obj, n)
 		return n, true
 	}
 
-	if v, ok := ir.cache[obj]; ok {
-		return v, false
+	if v, ok := ir.cache.Load(obj); ok {
+		return v.(string), false
 	}
 
 	id := ulid.GenerateID()
-	ir.cache[obj] = id
+	ir.cache.Store(obj, id)
 
 	return id, false
 }
-
-// ToLifecycleIdentity converts a parserv1.RunmeIdentity to a LifecycleIdentity.
-func ToLifecycleIdentity(idt parserv1.RunmeIdentity) LifecycleIdentity {
-	switch idt {
-	case parserv1.RunmeIdentity_RUNME_IDENTITY_ALL:
-		return AllLifecycleIdentity
-	case parserv1.RunmeIdentity_RUNME_IDENTITY_DOCUMENT:
-		return DocumentLifecycleIdentity
-	case parserv1.RunmeIdentity_RUNME_IDENTITY_CELL:
-		return CellLifecycleIdentity
-	default:
-		return UnspecifiedLifecycleIdentity
-	}
-}