fiorix / go-diameter

Diameter stack and Base Protocol (RFC 6733) for the Go programming language
Other
252 stars 143 forks source link

CCR - Event-Timestamp #64

Closed aaronchar closed 7 years ago

aaronchar commented 8 years ago

For some reason I am having trouble pulling this value out using the avp struct tags, I can clearly see it in my payload and I can see that it is being correctly decoded in util.go, But I just get an empty time back into my struct.

However it works if I do a FindAVP(avp.EventTimestamp, 0)

I am sure I am just missing something..but I am lost as to what I am messing up. any suggestions?

fiorix commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

aaronchar commented 8 years ago
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 commented 8 years ago

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

jaroszan commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@fiorix / @DefunctExodus - should this one be closed by https://github.com/fiorix/go-diameter/pull/66? Or is there something else to be done?

fiorix commented 7 years ago

I believe so

aaronchar commented 7 years ago

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