diff --git a/internal/config/config.go b/internal/config/config.go index 28f27be..1325c3a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -76,7 +76,6 @@ type Integration struct { Type string `json:"type"` Authentication Authentication `json:"authentication"` Writers []Writer `json:"writers"` - EventIDPath string `json:"eventIdPath"` } type Configuration struct { diff --git a/internal/config/config.schema.json b/internal/config/config.schema.json index 73c5a56..2feb3ea 100644 --- a/internal/config/config.schema.json +++ b/internal/config/config.schema.json @@ -21,9 +21,6 @@ } } }, - "eventIdPath": { - "type": "string" - }, "writers": { "type": "array", "items": { @@ -43,9 +40,6 @@ }, "outputEvent": { "type": "object" - }, - "idField": { - "type": "string" } }, "required": [ @@ -53,7 +47,7 @@ "url", "outputEvent", "collection", - "idField" + "uniqueIdKey" ] } ] @@ -62,8 +56,7 @@ }, "required": [ "type", - "writers", - "eventIdPath" + "writers" ] }, "minItems": 1 diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ba45a3d..7443812 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -51,7 +51,6 @@ func TestLoadServiceConfiguration(t *testing.T) { Authentication: Authentication{ Secret: SecretSource("MY_SECRET"), }, - EventIDPath: "issue.id", Writers: []Writer{ { Type: "mongo", @@ -145,7 +144,6 @@ func getExpectedWriterConfig(t *testing.T) string { "summary": "{{ issue.fields.summary }}", "createdAt": "{{ issue.fields.created }}", "description": "{{ issue.fields.description }}" - }, - "idField": "key" + } }` } diff --git a/internal/config/integration_test.go b/internal/config/integration_test.go index 33c90a8..f705462 100644 --- a/internal/config/integration_test.go +++ b/internal/config/integration_test.go @@ -54,7 +54,6 @@ func TestWriterConfig(t *testing.T) { "createdAt": "{{ issue.fields.created }}", "description": "{{ issue.fields.description }}", }, - IDField: "key", }, mongoConfig) }) } diff --git a/internal/config/testdata/all-writer-config.json b/internal/config/testdata/all-writer-config.json index 129c58b..0a99714 100644 --- a/internal/config/testdata/all-writer-config.json +++ b/internal/config/testdata/all-writer-config.json @@ -7,7 +7,6 @@ "fromFile": "testdata/secret" } }, - "eventIdPath": "issue.id", "writers": [ { "type": "mongo", @@ -20,8 +19,7 @@ "summary": "{{ issue.fields.summary }}", "createdAt": "{{ issue.fields.created }}", "description": "{{ issue.fields.description }}" - }, - "idField": "key" + } } ] } diff --git a/internal/config/testdata/config.json b/internal/config/testdata/config.json index 74f54cf..776deb5 100644 --- a/internal/config/testdata/config.json +++ b/internal/config/testdata/config.json @@ -7,7 +7,6 @@ "fromFile": "testdata/secret" } }, - "eventIdPath": "issue.id", "writers": [ { "type": "mongo", @@ -20,8 +19,7 @@ "summary": "{{ issue.fields.summary }}", "createdAt": "{{ issue.fields.created }}", "description": "{{ issue.fields.description }}" - }, - "idField": "key" + } } ] } diff --git a/internal/integrations/jira/config.go b/internal/integrations/jira/config.go index 247c439..21e829d 100644 --- a/internal/integrations/jira/config.go +++ b/internal/integrations/jira/config.go @@ -18,6 +18,5 @@ package jira // Configuration is the representation of the configuration for a Jira Cloud webhook type Configuration struct { // Secret the webhook secret configuration for validating the data received - Secret string - EventIDPath string + Secret string } diff --git a/internal/integrations/jira/events.go b/internal/integrations/jira/events.go index 7bd652f..032d466 100644 --- a/internal/integrations/jira/events.go +++ b/internal/integrations/jira/events.go @@ -27,12 +27,15 @@ const ( issueCreated = "jira:issue_created" issueUpdated = "jira:issue_updated" issueDeleted = "jira:issue_deleted" + + webhookEventFieldPath = "webhookEvent" + eventIDPath = "issue.id" ) -func getPipelineEvent(rawData []byte, eventIDPath string) (entities.PipelineEvent, error) { +func getPipelineEvent(rawData []byte) (entities.PipelineEvent, error) { parsed := gjson.ParseBytes(rawData) id := parsed.Get(eventIDPath).String() - webhookEvent := parsed.Get("webhookEvent").String() + webhookEvent := parsed.Get(webhookEventFieldPath).String() operationType, err := getOperationType(webhookEvent) if err != nil { diff --git a/internal/integrations/jira/events_test.go b/internal/integrations/jira/events_test.go index 7934ba0..bda996d 100644 --- a/internal/integrations/jira/events_test.go +++ b/internal/integrations/jira/events_test.go @@ -59,7 +59,7 @@ func TestEvent(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - event, err := getPipelineEvent([]byte(tc.rawData), "issue.id") + event, err := getPipelineEvent([]byte(tc.rawData)) if tc.expectError != "" { require.Error(t, err) require.EqualError(t, err, tc.expectError) diff --git a/internal/integrations/jira/service.go b/internal/integrations/jira/service.go index f231522..8f02017 100644 --- a/internal/integrations/jira/service.go +++ b/internal/integrations/jira/service.go @@ -86,7 +86,7 @@ func webhookHandler(config Configuration, p pipeline.IPipeline[entities.Pipeline return c.SendStatus(http.StatusOK) } - event, err := getPipelineEvent(body, config.EventIDPath) + event, err := getPipelineEvent(body) if err != nil { log.WithError(err).Error("error unmarshaling event") return c.Status(http.StatusBadRequest).JSON(utils.ValidationError(err.Error())) diff --git a/internal/integrations/tests/jira_test.go b/internal/integrations/tests/jira_test.go index f1cefa3..117d252 100644 --- a/internal/integrations/tests/jira_test.go +++ b/internal/integrations/tests/jira_test.go @@ -71,6 +71,7 @@ func TestJiraIntegration(t *testing.T) { expectedResults: []map[string]any{ { + "eventId": "12345", "key": "TEST-123", "createdAt": "2024-11-06T00:00:00.000Z", "description": "This is a test issue description", @@ -100,6 +101,7 @@ func TestJiraIntegration(t *testing.T) { expectedResults: []map[string]any{ { + "eventId": "12345", "key": "TEST-123", "createdAt": "2024-11-06T00:00:00.000Z", "description": "This is a test issue description modified", @@ -129,12 +131,14 @@ func TestJiraIntegration(t *testing.T) { expectedResults: []map[string]any{ { + "eventId": "12345", "key": "TEST-123", "createdAt": "2024-11-06T00:00:00.000Z", "description": "This is a test issue description modified", "summary": "Test modified issue", }, { + "eventId": "12346", "key": "TEST-456", "createdAt": "2024-11-10T00:00:00.000Z", "description": "This is the second issue", @@ -159,10 +163,11 @@ func TestJiraIntegration(t *testing.T) { expectedResults: []map[string]any{ { - "key": "TEST-456", - "createdAt": "2024-11-10T00:00:00.000Z", - "description": "This is the second issue", - "summary": "Test second issue", + "eventId": "12345", + "key": "TEST-123", + "createdAt": "2024-11-06T00:00:00.000Z", + "description": "This is a test issue description modified", + "summary": "Test modified issue", }, }, }, diff --git a/internal/integrations/tests/testdata/jira/config.json b/internal/integrations/tests/testdata/jira/config.json index cd43871..a3400d4 100644 --- a/internal/integrations/tests/testdata/jira/config.json +++ b/internal/integrations/tests/testdata/jira/config.json @@ -7,7 +7,6 @@ "fromFile": "testdata/secret" } }, - "eventIdPath": "issue.key", "writers": [ { "type": "mongo", @@ -20,8 +19,7 @@ "summary": "{{ issue.fields.summary }}", "createdAt": "{{ issue.fields.created }}", "description": "{{ issue.fields.description }}" - }, - "idField": "key" + } } ] } diff --git a/internal/server/integrations.go b/internal/server/integrations.go index 4fbd570..c489fc2 100644 --- a/internal/server/integrations.go +++ b/internal/server/integrations.go @@ -48,8 +48,7 @@ func setupIntegrations(ctx context.Context, log *logrus.Logger, cfg *config.Conf case integration.Jira: // TODO: improve management of configuration config := jira.Configuration{ - Secret: cfgIntegration.Authentication.Secret.String(), - EventIDPath: cfgIntegration.EventIDPath, + Secret: cfgIntegration.Authentication.Secret.String(), } if err := jira.SetupService(ctx, logrus.NewEntry(log), oasRouter, config, writer); err != nil { diff --git a/internal/writer/mongo/config.go b/internal/writer/mongo/config.go index ca471ad..06d483f 100644 --- a/internal/writer/mongo/config.go +++ b/internal/writer/mongo/config.go @@ -27,7 +27,6 @@ type Config struct { Database string `json:"-"` Collection string `json:"collection"` OutputEvent map[string]any `json:"outputEvent"` - IDField string `json:"idField"` } func (c *Config) Validate() error { @@ -40,14 +39,11 @@ func (c *Config) Validate() error { if c.OutputEvent == nil { return fmt.Errorf("outputEvent is required") } - if c.IDField == "" { - return fmt.Errorf("idField is required") + if _, ok := c.OutputEvent["_id"]; ok { + return fmt.Errorf(`outputEvent "_id" field is reserved`) } - if c.IDField == "_id" { - return fmt.Errorf("idField cannot be \"_id\"") - } - if _, ok := c.OutputEvent[c.IDField]; !ok { - return fmt.Errorf("idField \"%s\" not found in outputEvent", c.IDField) + if _, ok := c.OutputEvent[idField]; ok { + return fmt.Errorf(`outputEvent "%s" field is reserved`, idField) } return nil diff --git a/internal/writer/mongo/config_test.go b/internal/writer/mongo/config_test.go index ce556a0..be31f07 100644 --- a/internal/writer/mongo/config_test.go +++ b/internal/writer/mongo/config_test.go @@ -50,55 +50,27 @@ func TestValidateConfig(t *testing.T) { expectedError: "outputEvent is required", }, - "throws if IDField not found in output event": { - config: Config{ - URL: config.SecretSource("mongodb://localhost:27017"), - Collection: "test", - OutputEvent: map[string]any{}, - IDField: "custom_id", - }, - - expectedError: `idField "custom_id" not found in outputEvent`, - }, - "throws if IDField not set": { - config: Config{ - URL: config.SecretSource("mongodb://localhost:27017"), - Collection: "test", - OutputEvent: map[string]any{}, - }, - - expectedError: `idField is required`, - }, - "set custom IDField": { + "throws if _id is present in OutputEvent": { config: Config{ URL: config.SecretSource("mongodb://localhost:27017"), Collection: "test", OutputEvent: map[string]any{ - "custom_id": "my-id", + "_id": "my-id", }, - IDField: "custom_id", }, - expectedConfig: Config{ - URL: config.SecretSource("mongodb://localhost:27017"), - Collection: "test", - OutputEvent: map[string]any{ - "custom_id": "my-id", - }, - IDField: "custom_id", - }, + expectedError: `outputEvent "_id" field is reserved`, }, - "_id not supported as IDField": { + "throws if uid is present in OutputEvent": { config: Config{ URL: config.SecretSource("mongodb://localhost:27017"), Collection: "test", OutputEvent: map[string]any{ - "_id": "my-id", + "eventId": "my-id", }, - IDField: "_id", }, - expectedError: `idField cannot be "_id"`, + expectedError: `outputEvent "eventId" field is reserved`, }, } diff --git a/internal/writer/mongo/integration_test.go b/internal/writer/mongo/integration_test.go index 878d8c1..e150287 100644 --- a/internal/writer/mongo/integration_test.go +++ b/internal/writer/mongo/integration_test.go @@ -40,7 +40,6 @@ func TestMongo(t *testing.T) { URL: config.SecretSource(mongoURL), Database: db, Collection: collection, - IDField: "key", }) require.NoError(t, err) @@ -52,7 +51,7 @@ func TestMongo(t *testing.T) { err = w.Write(ctx, e) require.NoError(t, err) findAllDocuments(t, coll, []map[string]any{ - {"foo": "bar", "key": "123"}, + {"eventId": "123", "foo": "bar", "key": "123"}, }) }) @@ -61,7 +60,7 @@ func TestMongo(t *testing.T) { err = w.Write(ctx, e) require.NoError(t, err) findAllDocuments(t, coll, []map[string]any{ - {"foo": "taz", "key": "123", "another": "field"}, + {"eventId": "123", "foo": "taz", "key": "123", "another": "field"}, }) }) diff --git a/internal/writer/mongo/mongo.go b/internal/writer/mongo/mongo.go index 67e6772..c2dff48 100644 --- a/internal/writer/mongo/mongo.go +++ b/internal/writer/mongo/mongo.go @@ -32,6 +32,8 @@ import ( var ( ErrMongoInitialization = errors.New("failed to start mongo writer") + + idField = "eventId" ) type validateFunc func(context.Context, *mongo.Client) error @@ -75,7 +77,7 @@ func newMongoDBWriter[T entities.PipelineEvent](ctx context.Context, config *Con database: db, collection: collection, outputEvent: config.OutputEvent, - idField: config.IDField, + idField: idField, }, nil } @@ -160,15 +162,26 @@ func mongoClientOptionsFromConfig(config *Config) (*options.ClientOptions, strin func (w Writer[T]) idFilter(event T) (bson.D, error) { id := event.GetID() if id == "" { - return bson.D{}, fmt.Errorf("id is empty") + return bson.D{}, fmt.Errorf("event id is empty") } return bson.D{{Key: w.idField, Value: id}}, nil } func (w Writer[T]) bsonData(event T) ([]byte, error) { - bsonData, err := bson.Marshal(event.Data()) + data := event.Data() + if data == nil { + data = map[string]any{} + } + + if _, ok := data[w.idField]; ok { + return nil, fmt.Errorf("event data contains reserved field %s", w.idField) + } + data[w.idField] = event.GetID() + + bsonData, err := bson.Marshal(data) if err != nil { return nil, err } + return bsonData, nil } diff --git a/internal/writer/mongo/mongo_test.go b/internal/writer/mongo/mongo_test.go index b402560..8b42aaf 100644 --- a/internal/writer/mongo/mongo_test.go +++ b/internal/writer/mongo/mongo_test.go @@ -117,19 +117,19 @@ func TestUpsert(t *testing.T) { expectedErr string }{ "upserting element": { - data: getEvent(t), + data: getEvent(t, nil), responses: mtest.CreateSuccessResponse(bson.E{Key: "upserted", Value: []any{bson.D{}}}), }, "update element": { - data: getEvent(t), + data: getEvent(t, nil), responses: mtest.CreateSuccessResponse(bson.E{Key: "nModified", Value: 1}), }, - "error if event without id": { - data: &entities.Event{}, - expectedErr: "id is empty", + "error if event contains reserved data": { + data: getEvent(t, map[string]any{idField: "reserved"}), + expectedErr: "event data contains reserved field eventId", }, "error without change": { - data: getEvent(t), + data: getEvent(t, nil), responses: mtest.CreateSuccessResponse(bson.E{}), expectedErr: "error upserting data: 0 documents upserted", }, @@ -143,6 +143,7 @@ func TestUpsert(t *testing.T) { client: mt.Client, collection: mt.Coll.Name(), database: mt.DB.Name(), + idField: idField, } mt.AddMockResponses(test.responses) @@ -168,15 +169,15 @@ func TestDelete(t *testing.T) { expectedErr string }{ "delete element": { - data: getEvent(t), + data: getEvent(t, nil), responses: mtest.CreateSuccessResponse(bson.E{Key: "n", Value: 1}), }, "error if event without id": { data: &entities.Event{}, - expectedErr: "id is empty", + expectedErr: "event id is empty", }, "error without change": { - data: getEvent(t), + data: getEvent(t, nil), responses: mtest.CreateSuccessResponse(bson.E{}), expectedErr: "error deleting data: 0 documents deleted", }, @@ -207,12 +208,17 @@ func TestDelete(t *testing.T) { } } -func getEvent(t *testing.T) entities.PipelineEvent { +func getEvent(t *testing.T, data map[string]any) entities.PipelineEvent { t.Helper() event := &entities.Event{ ID: "12345", } + + if data != nil { + event.WithData(data) + } + return event } diff --git a/internal/writer/mongo/testdata/mongo.json b/internal/writer/mongo/testdata/mongo.json index e756413..e1abe0e 100644 --- a/internal/writer/mongo/testdata/mongo.json +++ b/internal/writer/mongo/testdata/mongo.json @@ -9,6 +9,5 @@ "summary": "{{ issue.fields.summary }}", "createdAt": "{{ issue.fields.created }}", "description": "{{ issue.fields.description }}" - }, - "idField": "key" + } }