CCR - Event-Timestamp about go-diameter

[fiorix]

Can you paste the struct or at least the field? I was looking at the reflect tests and realised we don't actually have a test for time.Time although it should be supported. Perhaps you can try *AVP?

[aaronchar]

sure so this is my struct I am trying to use (or a part of it anyway). I will give *AVP a shot to see what happens.

type ccr struct {
    EventTimestamp                time.Time      `avp:"Event-Timestamp"`
    SessionID                     string             `avp:"Session-Id"`
    OriginHost                    string             `avp:"Origin-Host"`
    OriginRealm                   string             `avp:"Origin-Realm"`
    DestinationRealm              string             `avp:"Destination-Realm"`
    AuthApplicationID             int32              `avp:"Auth-Application-Id"`
    ServiceContextID              string             `avp:"Service-Context-Id"`
    CCRequestType                 int32              `avp:"CC-Request-Type"`
    CCRequestNumber               int32              `avp:"CC-Request-Number"`
    DestinationHost               string             `avp:"Destination-Host"`
}

[fiorix]

That's what I suspected. So, try first changing from time.Time to datatype.Time and you should get the right value. I believe we don't have the right conversion from AVP to time.Time in reflect.go. You could probably add that, and a unit test.

[aaronchar]

actually I had tried datatype.Time and it gives me the same result

[aaronchar]

type ccr struct {
    EventTimestamp                datatype.Time      `avp:"Event-Timestamp"`
    SessionID                     string             `avp:"Session-Id"`
    OriginHost                    string             `avp:"Origin-Host"`
    OriginRealm                   string             `avp:"Origin-Realm"`
    DestinationRealm              string             `avp:"Destination-Realm"`
    AuthApplicationID             int32              `avp:"Auth-Application-Id"`
    ServiceContextID              string             `avp:"Service-Context-Id"`
    CCRequestType                 int32              `avp:"CC-Request-Type"`
    CCRequestNumber               int32              `avp:"CC-Request-Number"`
    DestinationHost               string             `avp:"Destination-Host"`
}

Results in

Time{0001-01-01 00:00:00 +0000 UTC}
Time{0001-01-01 00:00:00 +0000 UTC}
Time{0001-01-01 00:00:00 +0000 UTC}
Time{0001-01-01 00:00:00 +0000 UTC}
Time{0001-01-01 00:00:00 +0000 UTC}
Time{0001-01-01 00:00:00 +0000 UTC}
Time{0001-01-01 00:00:00 +0000 UTC}
Time{0001-01-01 00:00:00 +0000 UTC}

Where as manually using msg.FindAVP(avp.EventTimestamp, 0) comes out with the following

Time{2016-02-27 17:23:16 -0600 CST}
Time{2016-02-27 17:23:16 -0600 CST}
Time{2016-02-27 17:23:16 -0600 CST}
Time{2016-02-27 17:23:16 -0600 CST}
Time{2016-02-27 17:23:16 -0600 CST}

[fiorix]

Right, so something messed up in reflect.go. We should definitely have a test for that.

[jaroszan]

I have added test as follows (not sure yet how to fix reflect.go):

func TestUnmarshalTimeDatatype(t *testing.T) {
    expectedTime := "Time{2015-12-09 16:40:53 +0100 CET}"
    m, _ := ReadMessage(bytes.NewReader(testMessageWithVendorID), dict.Default)
    type Data struct {
        EventTimestamp datatype.Time `avp:"Event-Timestamp"`
    }
    var d Data
    if err := m.Unmarshal(&d); err != nil {
        t.Fatal(err)
    }
    if d.EventTimestamp.String() != expectedTime {
        t.Fatalf("Unexpected value, want %s, have %s", expectedTime, d.EventTimestamp.String())
    }
}

[fiorix]

In order to fix reflect.go we have to look for the avp name in the field tag, ensure that the field is a time field in the dictionary, then decide what to do based on the field type. It already supports AVP and *AVP but does not support datatype.Time and time.Time.

[jaroszan]

@fiorix / @DefunctExodus - should this one be closed by #66? Or is there something else to be done?

[fiorix]

I believe so

[aaronchar]

This can be closed now, Sorry I have not had a ton of time to play with it lately