amazon-ion / ion-go

A Go implementation of Amazon Ion.
https://amazon-ion.github.io/ion-docs/
Apache License 2.0
174 stars 31 forks source link

ion.Timestamp marshal/unmarshal issue - v 1.0.1 #170

Open szymon-madzielewski opened 4 years ago

szymon-madzielewski commented 4 years ago

Hey guys, I noticed that after marshal/unmarshal Timestamp type we are getting invalid results - a test was done on current version v1.0.1

test case - one fom your interanl test tmp := ion.NewTimestamp(time.Date(2010, 1, 1, 0, 0, 0, 0, time.UTC), ion.TimestampPrecisionSecond, ion.TimezoneUTC) => tmp 2010-01-01T00:00:00Z - OK

var jsonData []byte jsonData, = json.Marshal(tmp) var out = new(helpers.IonTimestamp) = json.Unmarshal(jsonData, &out) fmt.Println("out", out.String()) ==>> out 0001-01-01T00:00:00Z

R-maan commented 4 years ago

This Ion library provides its own Marshal/Unmarshal (as Ion can be richer than JSON). Try using ion.Marshal(temp) and ion.Unmarshal(jsonData, &out)

szymon-madzielewski commented 4 years ago

Hey, possible, the Issue on our site occurred when we added a cache solution. Therefore it's very inefficient to add additional rules (and can gro over time) instead of using standard json.Marshal/Unmarshal approach

joao-reis commented 4 years ago

Hey, I have the same problem to unmarshall the txTime inside of metadata from history queries. I try to do somethig like this:

type example struct {
    Metadata struct {
        Id      string        `ion:"id"`
        Version int           `ion:"version"`
        TxTime  ion.Timestamp `ion:"txTime"`
    } `ion:"metadata"`
}

The field TxTime is ignored.

R-maan commented 4 years ago

Hi @joao-reis, based on what I understood form your description, I wrote the following test and it seems to be working fine:

func TestUnmarshalTime(t *testing.T) {
    type MetadataStruct struct {
        Id      string    `ion:"id"`
        Version int       `ion:"version"`
        TxTime  Timestamp `ion:"txTime"`
    }

    type example struct {
        Metadata MetadataStruct `ion:"metadata"`
    }

    ts := NewDateTimestamp(time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), TimestampPrecisionYear)
    meta := MetadataStruct{"idValue", 22, ts}
    expectedValue := example{meta}

    t.Run("test", func(t *testing.T) {
        var val example
                // first parameter to unmarshal would be your data
        require.NoError(t, UnmarshalString("{metadata:{id:\"idValue\",txTime:2020T,version:22}}", &val))

        fmt.Println(val)
        assert.Equal(t, expectedValue, val)
    })
}

Can you please clarify how your code is different from this, which results in the mentioned unexpected behaviour?

joao-reis commented 4 years ago

Hey @Armanbqt ,

So the test works perfectly 👍

The problem here is to show the MetadataStruct in a browser after the unmarshal for example, in this case the field TxTime appear as {} instead of the TxTime.String(). In order to solve this I created a new field and copy the value of TxTime.String() into that new filed to be shown as string.

R-maan commented 4 years ago

Glad to hear you found a workaround. However, if you provide some code snippet, I might be able to help further.

jforjava1981 commented 7 months ago

@R-maan, @therapon, @tgregg we are also facing the same issue because ion.Timestamp does not implement MarshalJSON /UnmarshaJSON.

what we are trying to do is this -

1) ion message is converted to Golang map ( map[string]interface{} ) using amazon-ion library's ion.Unmarshal(). 2) this map is then converted to JSON using Golang's native json.Marshal() Map created in first step stores values as Golang types exposed by amazon-ion library. Golang's json Marshaler expects custom types to implement MarshalJSON / UnMarshalJSON so that it can convert values of those types to correct JSON. Since ion.timestamp type which is stored as value in the map for the ion dates does not implement it , we get default interpretation from Golang Marshaller - {}, instead of JSON date string.

any thoughts ? can we expect implementation of json Marshaller/unmarshaller,