Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symbol Token Write Support #165

Merged
merged 29 commits into from
Nov 9, 2020
Merged

Symbol Token Write Support #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
1ec16ad
write support
Sep 21, 2020
975ecbc
More changes
Sep 30, 2020
0d46e0f
Update reader.go
Oct 1, 2020
c6b9851
Define SymbolToken.toText()
Oct 1, 2020
c719ae7
Improve WriteSymbol logic
Oct 1, 2020
be79b2b
minor changes
Oct 1, 2020
2eeb691
Define newSimpleSymbolToken()
Oct 1, 2020
8acd4ca
use newSimpleSymbolToken()
Oct 1, 2020
4dae54e
rename newSimpleSymbolToken to NewSimpleSymbolToken
Oct 2, 2020
7092c4f
Define and use newSimpleSymbolTokenPtr
Oct 2, 2020
2ce8d31
cmd/ion-go updates
Oct 2, 2020
2c8bed2
Minor improvements
Oct 6, 2020
d33379a
Housekeeping
Oct 6, 2020
bfce4f8
Update textwriter.go
Oct 7, 2020
d3a3971
Address some feedback
Oct 7, 2020
9c219f7
Update symboltable.go
Oct 7, 2020
58bf6b7
Rename ParseTimestampFromStr to ParseTimestamp
Oct 8, 2020
143d1c3
Rename NewSimpleSymbolToken to NewSymbolTokenFromString
Oct 15, 2020
cd80266
Merge branch 'master' into symbolTokenWriteSupport
justing-bq Oct 15, 2020
bec27ab
Address review feedback
Oct 15, 2020
cee40f8
Address some more feedback
Oct 15, 2020
cc57047
more changes
Oct 20, 2020
4411767
Merge branch 'master' into symbolTokenWriteSupport
justing-bq Oct 20, 2020
14f7569
Address review feedback
Oct 20, 2020
2650e9b
Define MustParseTimestamp()
Oct 21, 2020
0fd2764
Update unmarshal.go
Oct 29, 2020
3b35693
Added TestRoundtripMarshalUnmarshal
Oct 29, 2020
4d384ce
Removed some unnecessary code
Oct 29, 2020
62bd9d4
Rename to TestRoundtripMarshalUnmarshalWithSymbolTable
Oct 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions cmd/ion-go/eventwriter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ type eventwriter struct {
// of ion-test-driver events.
func NewEventWriter(out io.Writer) ion.Writer {
w := ion.NewTextWriter(out)
w.WriteSymbol("$ion_event_stream")
w.WriteSymbol(ion.NewSymbolTokenFromString("$ion_event_stream"))

return &eventwriter{enc: ion.NewEncoder(w)}
}

func (e *eventwriter) FieldNameSymbol(val ion.SymbolToken) error {
func (e *eventwriter) FieldName(val ion.SymbolToken) error {
if val.Text == nil {
sid := fmt.Sprintf("$%v", val.LocalSID)
e.fieldname = &sid
Expand All @@ -53,11 +53,6 @@ func (e *eventwriter) FieldNameSymbol(val ion.SymbolToken) error {
return nil
}

func (e *eventwriter) FieldName(val string) error {
e.fieldname = &val
return nil
}

func (e *eventwriter) Annotation(val ion.SymbolToken) error {
e.annotations = append(e.annotations, val)
return nil
Expand Down Expand Up @@ -140,14 +135,22 @@ func (e *eventwriter) WriteTimestamp(val ion.Timestamp) error {
})
}

func (e *eventwriter) WriteSymbol(val string) error {
func (e *eventwriter) WriteSymbol(val ion.SymbolToken) error {
return e.write(event{
EventType: scalar,
IonType: iontype(ion.SymbolType),
ValueText: symbolify(val),
})
}

func (e *eventwriter) WriteSymbolFromString(val string) error {
return e.write(event{
EventType: scalar,
IonType: iontype(ion.SymbolType),
ValueText: stringify(val),
})
}

func (e *eventwriter) WriteString(val string) error {
return e.write(event{
EventType: scalar,
Expand Down Expand Up @@ -253,7 +256,7 @@ func stringify(val interface{}) string {
return string(bs)
}

func symbolify(val string) string {
func symbolify(val ion.SymbolToken) string {
buf := strings.Builder{}
w := ion.NewTextWriterOpts(&buf, ion.TextWriterQuietFinish)

Expand Down
18 changes: 7 additions & 11 deletions cmd/ion-go/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,28 +78,24 @@ func printVersion() error {
return err
}
{
if err := w.FieldName("version"); err != nil {
if err := w.FieldName(ion.NewSymbolTokenFromString("version")); err != nil {
return err
}
if err := w.WriteString(internal.GitCommit); err != nil {
return err
}

if err := w.FieldName(ion.NewSymbolTokenFromString("build_time")); err != nil {
return err
}

buildtime, err := ion.NewTimestampFromStr(internal.BuildTime, ion.TimestampPrecisionSecond, ion.TimezoneUTC)
if err == nil {
if err := w.FieldName("build_time"); err != nil {
return err
}
if err := w.WriteTimestamp(buildtime); err != nil {
return err
}
} else {
if err := w.FieldName("build_time"); err != nil {
return err
}
if err := w.WriteString("unknown-buildtime"); err != nil {
return err
}
} else if err := w.WriteString("unknown-buildtime"); err != nil {
return err
}
}
if err := w.EndStruct(); err != nil {
Expand Down
12 changes: 6 additions & 6 deletions cmd/ion-go/nopwriter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,7 @@ func NewNopWriter() ion.Writer {
return nopwriter{}
}

func (nopwriter) FieldName(string) error {
return nil
}

func (nopwriter) FieldNameSymbol(val ion.SymbolToken) error {
func (nopwriter) FieldName(ion.SymbolToken) error {
return nil
}

Expand Down Expand Up @@ -80,7 +76,11 @@ func (nopwriter) WriteTimestamp(ion.Timestamp) error {
return nil
}

func (nopwriter) WriteSymbol(string) error {
func (nopwriter) WriteSymbol(ion.SymbolToken) error {
return nil
}

func (nopwriter) WriteSymbolFromString(string) error {
return nil
}

Expand Down
6 changes: 3 additions & 3 deletions cmd/ion-go/process.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,8 @@ func (p *processor) process(in ion.Reader) error {
if e != nil {
return p.error(read, err)
}
if name != nil && name.Text != nil {
if err = p.out.FieldName(*name.Text); err != nil {
if name != nil {
if err = p.out.FieldName(*name); err != nil {
return p.error(write, err)
}
}
Expand Down Expand Up @@ -301,7 +301,7 @@ func (p *processor) process(in ion.Reader) error {
err = p.out.WriteTimestamp(*val)

case ion.SymbolType:
val, err := in.StringValue()
val, err := in.SymbolValue()
if err != nil {
return p.error(read, err)
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/ion-go/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ func (e eventtype) String() string {
}

func (e eventtype) MarshalIon(w ion.Writer) error {
return w.WriteSymbol(e.String())
return w.WriteSymbolFromString(e.String())
}

type iontype ion.Type

func (i iontype) MarshalIon(w ion.Writer) error {
return w.WriteSymbol(strings.ToUpper(ion.Type(i).String()))
return w.WriteSymbolFromString(strings.ToUpper(ion.Type(i).String()))
}

// event describes an Ion processing event.
Expand Down Expand Up @@ -99,7 +99,7 @@ func (e errortype) String() string {
}

func (e errortype) MarshalIon(w ion.Writer) error {
return w.WriteSymbol(e.String())
return w.WriteSymbolFromString(e.String())
}

// errordescription describes an error during Ion processing.
Expand Down
21 changes: 11 additions & 10 deletions ion/binaryreader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,15 @@ func TestReadBinaryStructs(t *testing.T) {
_struct(t, r, func(t *testing.T, r Reader) {
_eof(t, r)
})
_structAF(t, r, nil, []SymbolToken{SymbolToken{Text: newString("foo"), LocalSID: SymbolIDUnknown}}, func(t *testing.T, r Reader) {
_structAF(t, r, &SymbolToken{Text: newString("name"), LocalSID: 4}, []SymbolToken{SymbolToken{Text: newString("bar"), LocalSID: SymbolIDUnknown}}, func(t *testing.T, r Reader) {
_structAF(t, r, nil, []SymbolToken{NewSymbolTokenFromString("foo")}, func(t *testing.T, r Reader) {
_structAF(t, r, &SymbolToken{Text: newString("name"), LocalSID: 4}, []SymbolToken{NewSymbolTokenFromString("bar")}, func(t *testing.T, r Reader) {
_eof(t, r)
})
_intAF(t, r, &SymbolToken{Text: newString("max_id"), LocalSID: 8}, nil, 0)
})
_structAF(t, r, nil, nil, func(t *testing.T, r Reader) {
_intAF(t, r, &SymbolToken{Text: newString(""), LocalSID: SymbolIDUnknown}, nil, 15)
st := NewSymbolTokenFromString("")
_intAF(t, r, &st, nil, 15)
})
_eof(t, r)
}
Expand Down Expand Up @@ -257,7 +258,7 @@ func TestReadBinaryNullFieldName(t *testing.T) {
// }
})
r.Next()
r.StepIn()
assert.NoError(t, r.StepIn())
_nextF(t, r, &SymbolToken{}, true, true)
}

Expand Down Expand Up @@ -287,10 +288,10 @@ func TestReadBinaryAnnotations(t *testing.T) {
0xE3, 0x81, 0xF1, 0x0F, // $113::null
})

_nextA(t, r, []SymbolToken{SymbolToken{Text: nil, LocalSID: 0}}, false, false)
_nextA(t, r, []SymbolToken{SymbolToken{Text: newString("$ion"), LocalSID: 1}}, false, false)
_nextA(t, r, []SymbolToken{SymbolToken{Text: newString("foo"), LocalSID: 110}}, false, false)
_nextA(t, r, []SymbolToken{SymbolToken{Text: newString("bar"), LocalSID: 111}}, false, false)
_nextA(t, r, []SymbolToken{{Text: nil, LocalSID: 0}}, false, false)
_nextA(t, r, []SymbolToken{{Text: newString("$ion"), LocalSID: 1}}, false, false)
_nextA(t, r, []SymbolToken{{Text: newString("foo"), LocalSID: 110}}, false, false)
_nextA(t, r, []SymbolToken{{Text: newString("bar"), LocalSID: 111}}, false, false)
_nextA(t, r, nil, true, true)
}

Expand Down Expand Up @@ -455,8 +456,8 @@ func TestReadBinaryNulls(t *testing.T) {
})

_null(t, r, NullType)
_nullAF(t, r, NullType, nil, []SymbolToken{SymbolToken{Text: newString("$ion"), LocalSID: 1}})
_nullAF(t, r, NullType, nil, []SymbolToken{SymbolToken{Text: newString("foo"), LocalSID: SymbolIDUnknown}, SymbolToken{Text: newString("bar"), LocalSID: SymbolIDUnknown}})
_nullAF(t, r, NullType, nil, []SymbolToken{{Text: newString("$ion"), LocalSID: 1}})
_nullAF(t, r, NullType, nil, []SymbolToken{NewSymbolTokenFromString("foo"), NewSymbolTokenFromString("bar")})
_eof(t, r)
}

Expand Down
63 changes: 46 additions & 17 deletions ion/binarywriter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (
"io"
"math"
"math/big"
"strconv"
"strings"
"time"
)

Expand Down Expand Up @@ -251,22 +249,44 @@ func (w *binaryWriter) WriteTimestamp(val Timestamp) error {
return w.writeValue("Writer.WriteTimestamp", buf)
}

// WriteSymbol writes a symbol value.
func (w *binaryWriter) WriteSymbol(val string) error {
id, err := w.resolve("Writer.WriteSymbol", val)
if err != nil {
w.err = err
return err
// WriteSymbol writes a symbol value given a SymbolToken.
func (w *binaryWriter) WriteSymbol(val SymbolToken) error {
var id uint64
if val.LocalSID != SymbolIDUnknown {
id = uint64(val.LocalSID)
} else if val.Text != nil {
id, w.err = w.resolveFromSymbolTable("Writer.WriteSymbol", *val.Text)
if w.err != nil {
return w.err
}
} else {
return &UsageError{"Writer.WriteSymbol", "symbol token without defined text or symbol id is invalid"}
}

return w.writeSymbolFromID("Writer.WriteSymbol", id)
}

// WriteSymbolFromString writes a symbol value given a string that is expected to be in the symbol table.
// Returns an error if string is not in symbol table.
func (w *binaryWriter) WriteSymbolFromString(val string) error {
var id uint64
id, w.err = w.resolve("Writer.WriteSymbolFromString", val)
if w.err != nil {
return w.err
}

return w.writeSymbolFromID("Writer.WriteSymbolFromString", id)
}

func (w *binaryWriter) writeSymbolFromID(api string, id uint64) error {
vlength := uintLen(id)
bufLength := vlength + tagLen(vlength)
buf := make([]byte, 0, bufLength)

buf = appendTag(buf, 0x70, vlength)
buf = appendUint(buf, id)

return w.writeValue("Writer.WriteSymbol", buf)
return w.writeValue(api, buf)
}

// WriteString writes a string.
Expand Down Expand Up @@ -492,9 +512,17 @@ func (w *binaryWriter) beginValue(api string) error {
return &UsageError{api, "field name not set"}
}

id, err := w.resolve(api, *name)
if err != nil {
return err
var id uint64
if name.LocalSID != SymbolIDUnknown {
id = uint64(name.LocalSID)
} else if name.Text != nil {
var err error
id, err = w.resolve(api, *name.Text)
if err != nil {
return err
}
} else {
return &UsageError{api, "field name symbol token does not have defined text or symbol id."}
}

buf := make([]byte, 0, 10)
Expand Down Expand Up @@ -591,13 +619,14 @@ func (w *binaryWriter) end(api string, t ctx) error {

// Resolve resolves a symbol to its ID.
func (w *binaryWriter) resolve(api, sym string) (uint64, error) {
if strings.HasPrefix(sym, "$") {
id, err := strconv.ParseUint(sym[1:], 10, 64)
if err == nil {
return id, nil
}
if id, ok := symbolIdentifier(sym); ok {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to understand the symbol ID interpretation behavior that we're implementing here. I'm having difficulty imagining a scenario where I'm using a binary writer and I pass it a string in the $id format ($12, $47, etc).

return uint64(id), nil
}

return w.resolveFromSymbolTable(api, sym)
}

func (w *binaryWriter) resolveFromSymbolTable(api, sym string) (uint64, error) {
if w.lst != nil {
id, ok := w.lst.FindByName(sym)
if !ok {
Expand Down
26 changes: 13 additions & 13 deletions ion/binarywriter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ func TestWriteBinaryStruct(t *testing.T) {
assert.NoError(t, w.BeginStruct())
assert.NoError(t, w.EndStruct())

assert.NoError(t, w.Annotation(SymbolToken{Text: newString("foo"), LocalSID: SymbolIDUnknown}))
assert.NoError(t, w.Annotation(NewSymbolTokenFromString("foo")))
assert.NoError(t, w.BeginStruct())
{
assert.NoError(t, w.FieldName("name"))
assert.NoError(t, w.Annotation(SymbolToken{Text: newString("bar"), LocalSID: SymbolIDUnknown}))
assert.NoError(t, w.FieldName(NewSymbolTokenFromString("name")))
assert.NoError(t, w.Annotation(NewSymbolTokenFromString("bar")))
assert.NoError(t, w.WriteNull())

assert.NoError(t, w.FieldName("max_id"))
assert.NoError(t, w.FieldName(NewSymbolTokenFromString("max_id")))
assert.NoError(t, w.WriteInt(0))
}
assert.NoError(t, w.EndStruct())
Expand All @@ -67,10 +67,10 @@ func TestWriteBinarySexp(t *testing.T) {
assert.NoError(t, w.BeginSexp())
assert.NoError(t, w.EndSexp())

assert.NoError(t, w.Annotation(SymbolToken{Text: newString("foo"), LocalSID: SymbolIDUnknown}))
assert.NoError(t, w.Annotation(NewSymbolTokenFromString("foo")))
assert.NoError(t, w.BeginSexp())
{
assert.NoError(t, w.Annotation(SymbolToken{Text: newString("bar"), LocalSID: SymbolIDUnknown}))
assert.NoError(t, w.Annotation(NewSymbolTokenFromString("bar")))
assert.NoError(t, w.WriteNull())

assert.NoError(t, w.WriteInt(0))
Expand All @@ -91,10 +91,10 @@ func TestWriteBinaryList(t *testing.T) {
assert.NoError(t, w.BeginList())
assert.NoError(t, w.EndList())

assert.NoError(t, w.Annotation(SymbolToken{Text: newString("foo"), LocalSID: SymbolIDUnknown}))
assert.NoError(t, w.Annotation(NewSymbolTokenFromString("foo")))
assert.NoError(t, w.BeginList())
{
assert.NoError(t, w.Annotation(SymbolToken{Text: newString("bar"), LocalSID: SymbolIDUnknown}))
assert.NoError(t, w.Annotation(NewSymbolTokenFromString("bar")))
assert.NoError(t, w.WriteNull())

assert.NoError(t, w.WriteInt(0))
Expand Down Expand Up @@ -160,11 +160,11 @@ func TestWriteBinarySymbol(t *testing.T) {
0x74, 0xFF, 0xFF, 0xFF, 0xFF, // $4294967295
}
testBinaryWriter(t, eval, func(w Writer) {
assert.NoError(t, w.WriteSymbol("$ion"))
assert.NoError(t, w.WriteSymbol("name"))
assert.NoError(t, w.WriteSymbol("version"))
assert.NoError(t, w.WriteSymbol("$ion_shared_symbol_table"))
assert.NoError(t, w.WriteSymbol("$4294967295"))
assert.NoError(t, w.WriteSymbolFromString("$ion"))
assert.NoError(t, w.WriteSymbolFromString("name"))
assert.NoError(t, w.WriteSymbolFromString("version"))
assert.NoError(t, w.WriteSymbolFromString("$ion_shared_symbol_table"))
assert.NoError(t, w.WriteSymbolFromString("$4294967295"))
})
}

Expand Down
2 changes: 1 addition & 1 deletion ion/catalog_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestCatalog(t *testing.T) {
out := NewBinaryWriter(&buf, sst)

for i := 0; i < 10; i++ {
assert.NoError(t, out.Annotation(SymbolToken{Text: newString("item"), LocalSID: SymbolIDUnknown}))
assert.NoError(t, out.Annotation(NewSymbolTokenFromString("item")))
assert.NoError(t,
MarshalTo(out, &Item{
ID: i,
Expand Down
Loading