rs / rest-layer

REST Layer, Go (golang) REST API framework
http://rest-layer.io
MIT License
1.26k stars 114 forks source link

Etag should not depend on external state #265

Open apuigsech opened 4 years ago

apuigsech commented 4 years ago

Etag calculation is done by marshaling the payload into json and getting the md5 of that string. All values will be converted to string, and there are some specific cases in which the conversion to string depends on external state, like its time.Time. Its sting will depend on the local timezone.

This is not an issue if the item is only processed by a single computer (or a set of them on the same timezone), but I found a case in which this is not the case. I am using postgres replica through kafka events to replicate part of the state from one service to another, and both needs to be on different timezone, causing the same item in two different systems to have a different Etags. This is an issue in the moment I am considering the Etag and the Id as the replica identity.

This is the database on service A:

> select * from namespaces;
               etag               |          id          |     created     |     updated     | name
----------------------------------+----------------------+-----------------+-----------------+------
 1532840dbc17ad6b13fdd2001ae1fc99 | bo8rngfuudckd8jdh4o0 | 10:34:41.304598 | 10:34:41.304599 | ns1
 b0206828be20257bf6f85998d1e75e26 | bo8rngnuudckd8jdh4og | 10:34:42.857316 | 10:34:42.857317 | ns2
 6f0044115fd242a21404d727b9081a9f | bo8rnh7uudckd8jdh4p0 | 10:34:44.058706 | 10:34:44.058707 | ns3

And this is on service B:

> select * from namespaces;
               etag               |          id          |     created     |     updated     | name
----------------------------------+----------------------+-----------------+-----------------+------
 7a717c814f38d97989df6382622d99b5 | bo8rngfuudckd8jdh4o0 | 10:34:41.304598 | 10:34:41.304599 | ns1
 447211d08c8b2b2a45e2b2fc3e2b04d2 | bo8rngnuudckd8jdh4og | 10:34:42.857316 | 10:34:42.857317 | ns2
 81c5ba2564fc653cc508943da13847fd | bo8rnh7uudckd8jdh4p0 | 10:34:44.058706 | 10:34:44.058707 | ns3

I think we should guaranty that for same time.Time's, the Etag will be always the same independently of external states, and I suggest to do something like this on the getEtag function:

func normalizeTime(payload map[string]interface{}) map[string]interface{} {
    for k,v := range payload {
        switch v.(type){
        case time.Time:
            if t, ok := v.(time.Time) ; ok {
                payload[k] = t.UnixNano()
            }
        case map[string]interface{}:
            if t, ok := v.(map[string]interface{}) ; ok {
                normalizeTime(t)
            }
        case []interface{}: 
            if t, ok := v.([]interface{}) ; ok {
                for i,_ := range t {
                    if x, ok := t[i].(time.Time) ; ok {
                        t[i] = x.UnixNano()
                    }
                }
            }
        }
    }
    return payload
}

If you agree with that I can work on making the a proper PR.

smyrman commented 4 years ago

Sounds good.

apuigsech commented 4 years ago

This is happening to me, particularly, on the "created" and "updated" fields, because they are initialised using the function Now, which uses time.Now() that considers location on the internal state.

So, other possible solutions is to prevent them to manage the location state by just adding a call to UTC() on the Now function;

    Now = func(ctx context.Context, value interface{}) interface{} {
        return time.Now()**.UTC()**
    }

Thoughts?

smyrman commented 4 years ago

Changing the Now function is defiantly the smallest change. I have no objections to it.

Dragomir-Ivanov commented 4 years ago

@apuigsech I am :+1: for UTC() time as well.