Fix delete existing IDs on unspecified IDT (#423)

internal/api/runme/parser/v1/parser.proto

@@ -42,8 +42,13 @@ message Frontmatter {
   bool skip_prompts = 3;
 }
 
+message DeserializeRequestOptions {
+  RunmeIdentity identity = 1;
+}
+
 message DeserializeRequest {
   bytes source = 1;
+  DeserializeRequestOptions options = 2;
 }
 
 message DeserializeResponse {

internal/document/editor/cell_test.go

@@ -444,7 +444,7 @@ func Test_notebook_frontmatter(t *testing.T) {
 		t.Run(ex.kind, func(t *testing.T) {
 			t.Parallel()
 
-			notebook, err := Deserialize(file)
+			notebook, err := Deserialize(file, false)
 			require.NoError(t, err)
 
 			fmtr, info := notebook.ParsedFrontmatter()

internal/document/editor/editor.go

@@ -14,7 +14,7 @@ import (
 
 const FrontmatterKey = "frontmatter"
 
-func Deserialize(data []byte) (*Notebook, error) {
+func Deserialize(data []byte, requireIdentity bool) (*Notebook, error) {
 	sections, err := document.ParseSections(data)
 	if err != nil {
 		return nil, err
@@ -37,10 +37,13 @@ func Deserialize(data []byte) (*Notebook, error) {
 		PrefixAttributeName(InternalAttributePrefix, constants.FinalLineBreaksKey): fmt.Sprint(finalLinesBreaks),
 	}
 
-	f, info := document.ParseFrontmatter(string(sections.FrontMatter))
+	f, info := document.ParseFrontmatterWithIdentity(string(sections.FrontMatter), requireIdentity)
 	notebook.parsedFrontmatter = &f
 	notebook.frontmatterParseInfo = &info
-	notebook.Metadata[PrefixAttributeName(InternalAttributePrefix, FrontmatterKey)] = info.GetRaw()
+
+	if rawfm := info.GetRaw(); rawfm != "" {
+		notebook.Metadata[PrefixAttributeName(InternalAttributePrefix, FrontmatterKey)] = rawfm
+	}
 
 	return notebook, nil
 }

internal/document/editor/editor_test.go

@@ -10,7 +10,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/stateful/runme/internal/document"
 	"github.com/stateful/runme/internal/document/constants"
 	"github.com/stateful/runme/internal/identity"
 	"github.com/stateful/runme/internal/version"
@@ -27,60 +26,23 @@ func TestMain(m *testing.M) {
 }
 
 func TestEditor(t *testing.T) {
-	notebook, err := Deserialize(testDataNested)
+	notebook, err := Deserialize(testDataNested, false)
 	require.NoError(t, err)
 	result, err := Serialize(notebook)
 	require.NoError(t, err)
 	assert.Equal(
 		t,
-		document.InjectFrontmatter(string(testDataNestedFlattened)),
+		string(testDataNestedFlattened),
 		string(result),
 	)
 }
 
-func TestEditorEmptyRunme(t *testing.T) {
-	data := []byte(`---
-runme:
-shell: bash
----`)
-
-	notebook, err := Deserialize(data)
-	require.NoError(t, err)
-	assert.Equal(t, testMockID, notebook.parsedFrontmatter.Runme.ID)
-	assert.Equal(t, version.BaseVersion(), notebook.parsedFrontmatter.Runme.Version)
-}
-
-func TestEditorNullRunme(t *testing.T) {
-	data := []byte(`---
-runme: null
-shell: bash
----`)
-
-	notebook, err := Deserialize(data)
-	require.NoError(t, err)
-	assert.Equal(t, testMockID, notebook.parsedFrontmatter.Runme.ID)
-	assert.Equal(t, version.BaseVersion(), notebook.parsedFrontmatter.Runme.Version)
-}
-
-func TestEditorOnlyRunmeVersion(t *testing.T) {
-	data := []byte(`---
-runme:
-  id: 01HEBAF6W797022GRA5QV0VZS6
-shell: bash
----`)
-
-	notebook, err := Deserialize(data)
-	require.NoError(t, err)
-	assert.Equal(t, "01HEBAF6W797022GRA5QV0VZS6", notebook.parsedFrontmatter.Runme.ID)
-	assert.Equal(t, version.BaseVersion(), notebook.parsedFrontmatter.Runme.Version)
-}
-
 func TestEditor_List(t *testing.T) {
 	data := []byte(`1. Item 1
 2. Item 2
 3. Item 3
 `)
-	notebook, err := Deserialize(data)
+	notebook, err := Deserialize(data, false)
 	require.NoError(t, err)
 
 	notebook.Cells[0].Value = "1. Item 1\n2. Item 2\n"
@@ -89,27 +51,27 @@ func TestEditor_List(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(
 		t,
-		document.InjectFrontmatter(`1. Item 1
+		`1. Item 1
 2. Item 2
-`),
+`,
 		string(newData),
 	)
 
 	newData, err = Serialize(notebook)
 	require.NoError(t, err)
 	assert.Equal(
 		t,
-		document.InjectFrontmatter(`1. Item 1
+		`1. Item 1
 2. Item 2
-`),
+`,
 		string(newData),
 	)
 }
 
 func TestEditor_CodeBlock(t *testing.T) {
 	t.Run("ProvideGeneratedName", func(t *testing.T) {
 		data := []byte("```sh\necho 1\n```\n")
-		notebook, err := Deserialize(data)
+		notebook, err := Deserialize(data, false)
 		require.NoError(t, err)
 		cell := notebook.Cells[0]
 		assert.Equal(
@@ -124,12 +86,12 @@ func TestEditor_CodeBlock(t *testing.T) {
 		)
 		result, err := Serialize(notebook)
 		require.NoError(t, err)
-		assert.Equal(t, document.InjectFrontmatter(string(data)), string(result))
+		assert.Equal(t, string(data), string(result))
 	})
 
 	t.Run("PreserveName", func(t *testing.T) {
 		data := []byte("```sh {\"name\":\"name1\"}\necho 1\n```\n")
-		notebook, err := Deserialize(data)
+		notebook, err := Deserialize(data, false)
 		require.NoError(t, err)
 		cell := notebook.Cells[0]
 		assert.Equal(
@@ -145,7 +107,7 @@ func TestEditor_CodeBlock(t *testing.T) {
 		)
 		result, err := Serialize(notebook)
 		require.NoError(t, err)
-		assert.Equal(t, document.InjectFrontmatter(string(data)), string(result))
+		assert.Equal(t, string(data), string(result))
 	})
 }
 
@@ -160,7 +122,7 @@ Paragraph 1 with a link [Link1](https://example.com 'Link Title 1') and a second
 	err := os.Setenv("RUNME_AST_METADATA", "true")
 	require.NoError(t, err)
 
-	notebook, err := Deserialize(data)
+	notebook, err := Deserialize(data, false)
 	require.NoError(t, err)
 	require.NotEmpty(t, notebook.Metadata)
 
@@ -209,7 +171,7 @@ version = '%s'
 
 A paragraph
 `, testMockID, version.BaseVersion()))
-	notebook, err := Deserialize(data)
+	notebook, err := Deserialize(data, false)
 	require.NoError(t, err)
 	result, err := Serialize(notebook)
 	require.NoError(t, err)
@@ -226,7 +188,7 @@ func TestEditor_Newlines(t *testing.T) {
 This will test final line breaks`)
 
 	t.Run("No line breaks", func(t *testing.T) {
-		notebook, err := Deserialize(data)
+		notebook, err := Deserialize(data, false)
 		require.NoError(t, err)
 
 		assert.Equal(
@@ -239,15 +201,15 @@ This will test final line breaks`)
 		require.NoError(t, err)
 		assert.Equal(
 			t,
-			document.InjectFrontmatter(string(data)),
+			string(data),
 			string(actual),
 		)
 	})
 
 	t.Run("Single line break", func(t *testing.T) {
 		withLineBreaks := append(data, byte('\n'))
 
-		notebook, err := Deserialize(withLineBreaks)
+		notebook, err := Deserialize(withLineBreaks, false)
 		require.NoError(t, err)
 
 		assert.Equal(
@@ -260,15 +222,15 @@ This will test final line breaks`)
 		require.NoError(t, err)
 		assert.Equal(
 			t,
-			document.InjectFrontmatter(string(withLineBreaks)),
+			string(withLineBreaks),
 			string(actual),
 		)
 	})
 
 	t.Run("7 line breaks", func(t *testing.T) {
 		withLineBreaks := append(data, bytes.Repeat([]byte{'\n'}, 7)...)
 
-		notebook, err := Deserialize(withLineBreaks)
+		notebook, err := Deserialize(withLineBreaks, false)
 		require.NoError(t, err)
 
 		assert.Equal(
@@ -281,7 +243,7 @@ This will test final line breaks`)
 		require.NoError(t, err)
 		assert.Equal(
 			t,
-			document.InjectFrontmatter(string(withLineBreaks)),
+			string(withLineBreaks),
 			string(actual),
 		)
 	})

internal/document/editor/editorservice/service.go

@@ -3,10 +3,8 @@ package editorservice
 import (
 	"context"
 
-	"github.com/stateful/runme/internal/document"
 	"github.com/stateful/runme/internal/document/editor"
 	parserv1 "github.com/stateful/runme/internal/gen/proto/go/runme/parser/v1"
-	"github.com/stateful/runme/internal/identity"
 	"go.uber.org/zap"
 	"golang.org/x/exp/constraints"
 )
@@ -17,14 +15,28 @@ type parserServiceServer struct {
 	logger *zap.Logger
 }
 
+var notebookIdentities = []parserv1.RunmeIdentity{
+	parserv1.RunmeIdentity_RUNME_IDENTITY_ALL,
+	parserv1.RunmeIdentity_RUNME_IDENTITY_DOCUMENT,
+}
+
+var cellIdentities = []parserv1.RunmeIdentity{
+	parserv1.RunmeIdentity_RUNME_IDENTITY_ALL,
+	parserv1.RunmeIdentity_RUNME_IDENTITY_CELL,
+}
+
 func NewParserServiceServer(logger *zap.Logger) parserv1.ParserServiceServer {
 	return &parserServiceServer{logger: logger}
 }
 
 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)]))
 
-	notebook, err := editor.Deserialize(req.Source)
+	currentIdentity := req.Options.Identity
+	notebookIdentity := containsIdentity(notebookIdentities, currentIdentity)
+	cellIdentity := containsIdentity(cellIdentities, currentIdentity)
+
+	notebook, err := editor.Deserialize(req.Source, notebookIdentity)
 	if err != nil {
 		s.logger.Info("failed to call Deserialize", zap.Error(err))
 		return nil, err
@@ -44,7 +56,11 @@ func (s *parserServiceServer) Deserialize(_ context.Context, req *parserv1.Deser
 			}
 		}
 
-		if cell.Kind == editor.CodeKind {
+		if cell.Kind == editor.CodeKind && cellIdentity {
+			if cell.Metadata == nil {
+				cell.Metadata = make(map[string]string)
+			}
+
 			cell.EnsureID()
 		}
 
@@ -71,15 +87,6 @@ func (s *parserServiceServer) Serialize(_ context.Context, req *parserv1.Seriali
 
 	cells := make([]*editor.Cell, 0, len(req.Notebook.Cells))
 	for _, cell := range req.Notebook.Cells {
-		if req.Options.Identity == parserv1.RunmeIdentity_RUNME_IDENTITY_ALL ||
-			req.Options.Identity == parserv1.RunmeIdentity_RUNME_IDENTITY_CELL {
-			if _, ok := cell.Metadata["id"]; !ok && cell.Kind == parserv1.CellKind_CELL_KIND_CODE {
-				cell.Metadata["id"] = identity.GenerateID()
-			}
-		} else {
-			delete(cell.Metadata, "id")
-		}
-
 		cells = append(cells, &editor.Cell{
 			Kind:       editor.CellKind(cell.Kind),
 			Value:      cell.Value,
@@ -88,20 +95,6 @@ func (s *parserServiceServer) Serialize(_ context.Context, req *parserv1.Seriali
 		})
 	}
 
-	if req.Options.Identity == parserv1.RunmeIdentity_RUNME_IDENTITY_UNSPECIFIED ||
-		req.Options.Identity == parserv1.RunmeIdentity_RUNME_IDENTITY_CELL {
-		frontMatterKey := editor.PrefixAttributeName(editor.InternalAttributePrefix, editor.FrontmatterKey)
-		raw := req.Notebook.Metadata[frontMatterKey]
-
-		_, info := document.ParseFrontmatterWithIdentity(raw, false)
-		raw = info.GetRaw()
-		if raw == "" {
-			delete(req.Notebook.Metadata, frontMatterKey)
-		} else {
-			req.Notebook.Metadata[frontMatterKey] = raw
-		}
-	}
-
 	data, err := editor.Serialize(&editor.Notebook{
 		Cells:    cells,
 		Metadata: req.Notebook.Metadata,
@@ -120,3 +113,12 @@ func min[T constraints.Ordered](a, b T) T {
 	}
 	return b
 }
+
+func containsIdentity(ids []parserv1.RunmeIdentity, i parserv1.RunmeIdentity) bool {
+	for _, v := range ids {
+		if v == i {
+			return true
+		}
+	}
+	return false
+}