functions: don't use a wrapper and make Resource a value
We'll switch Resource back to a pointer after the next worker release.
The wrapper doesn't work because the JSON is provided by the worker.
The worker does not include the wrapper. So, we end up with a mismatch:
with wrapper: {"m":{"eventId":"test event ID","timestamp":"0001-01-01T00:00:00Z","eventType":"","resource":null}}
no wrapper: {"eventId":"test event ID","timestamp":"0001-01-01T00:00:00Z","eventType":"","resource":null}
Making testing easier is not enough of a reason to add the additional
layer of indirection in the JSON with the wrapper.
Change-Id: Ifcd3df6f82377f8ddacd6621be8e759cf5a2b44c
Reviewed-on: https://code-review.googlesource.com/c/36070
Reviewed-by: Chris Broadfoot <cbro@google.com>
Reviewed-by: Jean de Klerk <deklerk@google.com>
diff --git a/functions/metadata/metadata.go b/functions/metadata/metadata.go
index 6f4120d..26f4434 100644
--- a/functions/metadata/metadata.go
+++ b/functions/metadata/metadata.go
@@ -31,7 +31,8 @@
// EventType is the type of the event. For example: "google.pubsub.topic.publish".
EventType string `json:"eventType"`
// Resource is the resource that triggered the event.
- Resource *Resource `json:"resource"`
+ // TODO(tbp): Make this a pointer.
+ Resource Resource `json:"resource"`
}
// Resource holds Google Cloud Functions resource metadata.
@@ -45,15 +46,12 @@
Type string `json:"type"`
}
-// wrapper wraps Metadata to make nil serialization work nicely.
-type wrapper struct {
- M *Metadata `json:"m,omitempty"`
-}
-
type contextKey string
// GCFContextKey satisfies an interface to be able to use contextKey to read
// metadata from a Cloud Functions context.Context.
+//
+// Be careful making changes to this function. See FromContext.
func (k contextKey) GCFContextKey() string {
return string(k)
}
@@ -65,21 +63,34 @@
if ctx == nil {
return nil, errors.New("nil ctx")
}
+ // The original JSON is inserted by the Cloud Functions worker. So, the
+ // format must not change, or the message may fail to unmarshal. We use
+ // JSON as a common format between the worker and this package to ensure
+ // this package can be updated independently from the worker. The contextKey
+ // type and the metadataContextKey value use an interface to avoid using
+ // a built-in type as a context key (which is easy to have collisions with).
+ // If we need another value to be stored in the context, we can use a new
+ // key or interface and avoid needing to change this one. Similarly, if we
+ // need to change the format of the message, we should add an additional key
+ // to keep backward compatibility.
b, ok := ctx.Value(metadataContextKey).(json.RawMessage)
if !ok {
return nil, errors.New("unable to find metadata")
}
- w := &wrapper{}
- if err := json.Unmarshal(b, w); err != nil {
+ meta := &Metadata{}
+ if err := json.Unmarshal(b, meta); err != nil {
return nil, fmt.Errorf("json.Unmarshal: %v", err)
}
- return w.M, nil
+ return meta, nil
}
-// NewContext returns a new Context carrying m. NewContext is useful for
-// writing tests which rely on Metadata.
+// NewContext returns a new Context carrying m. If m is nil, NewContext returns
+// ctx. NewContext is only used for writing tests which rely on Metadata.
func NewContext(ctx context.Context, m *Metadata) context.Context {
- b, err := json.Marshal(&wrapper{M: m})
+ if m == nil {
+ return ctx
+ }
+ b, err := json.Marshal(m)
if err != nil {
return ctx
}
diff --git a/functions/metadata/metadata_test.go b/functions/metadata/metadata_test.go
index 065c379..9f9d2e5 100644
--- a/functions/metadata/metadata_test.go
+++ b/functions/metadata/metadata_test.go
@@ -27,7 +27,6 @@
{
&Metadata{EventID: "test event ID"},
},
- {},
}
for _, test := range tests {
ctx := NewContext(context.Background(), test.meta)
@@ -48,4 +47,7 @@
if _, err := FromContext(context.Background()); err == nil {
t.Errorf("FromContext got no error, wanted an error")
}
+ if _, err := FromContext(NewContext(context.Background(), nil)); err == nil {
+ t.Errorf("FromContext got no error, wanted an error")
+ }
}
