/events/$ID endpoint returns event payload with incorrectly marshaled BSON value

[magnusbaeck]

Description

Here's a response example from a GET /v1alpha1/events/202093d0-4ac6-4d2c-bcec-5599bb3d7288 request:

{
  "meta": {
    "id": "202093d0-4ac6-4d2c-bcec-5599bb3d7288",
    "security": {
      "integrityProtection": {
        "alg": "",
        "signature": ""
      }
    },
    "source": {
      "host": "hostname.example.com",
      "name": "Eiffel Broadcaster Plugin",
      "serializer": "pkg:maven/com.axis.jenkins.plugins.eiffel/eiffel-broadcaster@2.2.0"
    },
    "time": 1635505207758,
    "type": "EiffelActivityTriggeredEvent",
    "version": "4.0.0"
  },
  "links": [],
  "data": [
    {
      "Key": "name",
      "Value": "name-of-job"
    },
    {
      "Key": "triggers",
      "Value": [
        [
          {
            "Key": "type",
            "Value": "TIMER"
          },
          {
            "Key": "description",
            "Value": "Started by timer"
          }
        ]
      ]
    }
  ]
}

Note that the meta looks pretty okay (except that the object in meta.security contains empty strings; pretty sure that isn't a problem with the original event sent by eiffel-broadcaster), but data contains an array of key/value objects rather than an object. Perhaps an incorrect JSON marshaling of a bson.M? Or we expected to get a bson.M but got a bson.D instead?

Motivation

The /events/$ID endpoint should obviously return a correct Eiffel event.

Exemplification

N/A

Benefits

Correct API behavior.

Possible Drawbacks

None.

[t-persson]

I think this is because of the event schema that we are using in goer: https://github.com/eiffel-community/eiffel-goer/blob/main/internal/schema/schema.go#L23

We can either fix that or migrate to using the go sdk

[magnusbaeck]

The contents of the file should certainly explain why we're getting the empty meta.security values, but I don't think the interface{} type of data should be problematic in itself. It just reflects whatever values it gets (from the BSON I guess). That said it might get fixed anyway by switching to eiffelevents-sdk-go.

[magnusbaeck]

The empty meta values are a result of how encoding/json works; the omitempty tag doesn't omit zero value struct fields (see golang/go#11939). That's addressed in eiffelevents-sdk-go by using github.com/clarketm/json. IOW, using the SDK will at least address that but I suspect we'll have to implement eiffel-community/eiffelevents-sdk-go#8 first.

[magnusbaeck]

Using eiffelevents-sdk-go isn't as straight-forward as one would like since the equivalent of its UnmarshalAny, which peeks into the JSON structure to figure out the type to unmarshal into, isn't available to us since we don't actually have JSON but BSON. However, I don't think that matters since we're not actually interested in the unmarshaled Eiffel structs; we just want to dump the JSON form of the documents to the client without looking at them.

The documentation explains why we get a slice of {"Key": "foo", "Value": "bar} structs instead of a {"foo": "bar"} object:

The driver unmarshals BSON documents unmarshalled into an interface{} field as a D type.

bson.D, of course, represents an ordered document, hence we need a slice. To fix this we should be able to change the code to decode the BSON into a bson.M value instead of a schema.EiffelEvent value. I just hope that'll apply recursively so we don't get a bson.M on the top level but continue to get bson.D in the levels below. If that's the code maybe we could configure a custom decoder.

[magnusbaeck]

Yeah, decoding the FindOne result into a bson.M value did the trick. We could change the return values of the functions in the Database interface,

type Database interface {
	GetEvents(context.Context, requests.MultipleEventsRequest) ([]schema.EiffelEvent, error)
	SearchEvent(context.Context, string) (schema.EiffelEvent, error)
	UpstreamDownstreamSearch(context.Context, string) ([]schema.EiffelEvent, error)
	GetEventByID(context.Context, string) (schema.EiffelEvent, error)
	Close(context.Context) error
}

to return bson.M instead of schema.EiffelEvent, except that the interface wouldn't be database agnostic anymore. But we could use map[string]interface{} since we don't care about the event payload (that's also the underlying type of bson.M so it's a simple type conversion away). The one exception to the lack of need to introspect the query responses is, I think, UpstreamDownstreamSearch. There we need to loop over each event's links to be able to make new queries. Unless we come up with something better we could do a JSON marshal/unmarshal roundtrip in that specific case to get a proper event struct. Unfortunately, the MetaTeller interface of the event structs doesn't provide the ability to traverse links. Options:

• Expand eiffelevent-sdk-go's MetaTeller interface or add another interface for traversing links of arbitrary event structs.
• Use reflection to access the links.
• Skip the JSON roundtrip and make some very reasonable assumptions about the map[string]interface{} we get back from the database driver, namely that there's a "links" key and that it contains the links we're looking for, i.e. walk the nested maps by hand. The code won't be very pretty and it'll be littered with type assertions but it'll be quick.

Except the roundtrip the first option is pretty attractive since a generic way to iterate over links is likely to be useful elsewhere, but I'm worried that the performance will suffer too much by the roundtrip. Maybe I'll hack on the third option to see how cumbersome it really gets. Maybe it isn't so bad.

[t-persson]

It would be nice to see how much of a performance hit we get from the first option here. I like it as well, but we still want a performant API.

Also seeing how it compares to the third option (including the, possible, code mess of the third option) would be nice, if you have the time to do that.

[magnusbaeck]

What I think we could do for now is to redefine the EiffelEvent type to be a map[string]interface{} and address this serious bug. We don't have to solve the link traversal problem at this point, and any JSON roundtrip or type assertion bonanza is going to be confined to the future UpstreamDownstreamSearch implementation. Being lazy about this gives us time to work out something nice in eiffelevents-sdk-go instead of rushing something out.

We could extend the LinkFinder interface with an Iterator method that e.g. returns a value with a Next method to allow link iteration. Or add that method to a separate interface if we think it's reasonable that types might implement today's LinkFinder methods but not iteration (or vice versa). Then the only problem is that the event types themselves don't provide access to the LinkFinder interface of the Links field but we could add a separate interface for that (LinkFinderFinder?) that all event types would implement.