From dbe1bef476918771feca44ffb3447a0d5d7833cf Mon Sep 17 00:00:00 2001
From: Eric Myhre <hash@exultant.us>
Date: Thu, 2 Sep 2021 23:51:43 +0200
Subject: [PATCH] bindnode: fix for stringjoin struct emission when first field
 is the empty string.

In the case of an empty string as the first field, the buffer length
is not a valid proxy for whether we're on the first field or not.

This means if we have some type like:
`type Foo struct {a String; b String} representation stringjoin(":")`,
and the value of it is `{"", "b"}`, then the string of that
should still be ":b".
Before this fix, it would incorrectly be emitted as "b" (no joiner),
which would not round-trip.

Includes regression test.
---
 node/bindnode/repr.go                    |  5 +++-
 node/tests/schemaStructReprStringjoin.go | 29 ++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/node/bindnode/repr.go b/node/bindnode/repr.go
index d3dce658..ea85556d 100644
--- a/node/bindnode/repr.go
+++ b/node/bindnode/repr.go
@@ -364,6 +364,7 @@ func (w *_nodeRepr) AsString() (string, error) {
 	case schema.StructRepresentation_Stringjoin:
 		var b strings.Builder
 		itr := (*_node)(w).MapIterator()
+		first := true
 		for !itr.Done() {
 			_, v, err := itr.Next()
 			if err != nil {
@@ -373,7 +374,9 @@ func (w *_nodeRepr) AsString() (string, error) {
 			if err != nil {
 				return "", err
 			}
-			if b.Len() > 0 {
+			if first {
+				first = false
+			} else {
 				b.WriteString(stg.GetDelim())
 			}
 			b.WriteString(s)
diff --git a/node/tests/schemaStructReprStringjoin.go b/node/tests/schemaStructReprStringjoin.go
index 5290d9fb..54c9d587 100644
--- a/node/tests/schemaStructReprStringjoin.go
+++ b/node/tests/schemaStructReprStringjoin.go
@@ -98,6 +98,35 @@ func SchemaTestStructReprStringjoin(t *testing.T, engine Engine) {
 		})
 	})
 
+	t.Run("first field empty string works", func(t *testing.T) {
+		np := engine.PrototypeByName("ManystringStruct")
+		nrp := engine.PrototypeByName("ManystringStruct.Repr")
+		var n schema.TypedNode
+		t.Run("typed-create", func(t *testing.T) {
+			n = fluent.MustBuildMap(np, 2, func(ma fluent.MapAssembler) {
+				ma.AssembleEntry("foo").AssignString("")
+				ma.AssembleEntry("bar").AssignString("v2")
+			}).(schema.TypedNode)
+			t.Run("typed-read", func(t *testing.T) {
+				Require(t, n.Kind(), ShouldEqual, datamodel.Kind_Map)
+				Wish(t, n.Length(), ShouldEqual, int64(2))
+				Wish(t, must.String(must.Node(n.LookupByString("foo"))), ShouldEqual, "")
+				Wish(t, must.String(must.Node(n.LookupByString("bar"))), ShouldEqual, "v2")
+			})
+			t.Run("repr-read", func(t *testing.T) {
+				nr := n.Representation()
+				Require(t, nr.Kind(), ShouldEqual, datamodel.Kind_String)
+				Wish(t, must.String(nr), ShouldEqual, ":v2") // Note the leading colon is still present.
+			})
+		})
+		t.Run("repr-create", func(t *testing.T) {
+			nr := fluent.MustBuild(nrp, func(na fluent.NodeAssembler) {
+				na.AssignString(":v2")
+			})
+			Wish(t, datamodel.DeepEqual(n, nr), ShouldEqual, true)
+		})
+	})
+
 	t.Run("nested stringjoin structs work", func(t *testing.T) {
 		np := engine.PrototypeByName("Recurzorator")
 		nrp := engine.PrototypeByName("Recurzorator.Repr")