From 224888e8c485ceee0e007f351edc6bd0a4baf20e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Michon?= Date: Thu, 13 Aug 2020 10:52:10 +0200 Subject: [PATCH 1/5] Do not add in the log the query as it may contain credentials --- mongo/document/document.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mongo/document/document.go b/mongo/document/document.go index 196e72ff..418c0ecc 100644 --- a/mongo/document/document.go +++ b/mongo/document/document.go @@ -232,7 +232,7 @@ func Update(ctx context.Context, collectionName string, update bson.M, doc docum return err } - log.WithField("query", update).Debugf("update %v", collectionName) + log.Debugf("update %v", collectionName) return c.UpdateId(doc.getID(), update) } From 4ffdd6c131911c49bf65b4004d6de6546f035250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Michon?= Date: Thu, 13 Aug 2020 10:53:10 +0200 Subject: [PATCH 2/5] Do not add in the log the document as it may contain credentials --- mongo/document/document.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mongo/document/document.go b/mongo/document/document.go index 418c0ecc..8582c36e 100644 --- a/mongo/document/document.go +++ b/mongo/document/document.go @@ -65,7 +65,7 @@ func Create(ctx context.Context, collectionName string, doc document) error { c := mongo.Session(log).Clone().DB("").C(collectionName) defer c.Database.Session.Close() - log.WithField(collectionName, doc).Debugf("save '%v'", collectionName) + log.WithField("collection", collectionName).Debugf("save '%v'", collectionName) return c.Insert(doc) } @@ -82,7 +82,7 @@ func Save(ctx context.Context, collectionName string, doc document) error { c := mongo.Session(log).Clone().DB("").C(collectionName) defer c.Database.Session.Close() - log.WithField(collectionName, doc).Debugf("save '%v'", collectionName) + log.WithField("collection", collectionName).Debugf("save '%v'", collectionName) _, err := c.UpsertId(doc.getID(), doc) return err } @@ -96,7 +96,7 @@ func ReallyDestroy(ctx context.Context, collectionName string, doc document) err log := logger.Get(ctx) c := mongo.Session(log).Clone().DB("").C(collectionName) defer c.Database.Session.Close() - log.WithField(collectionName, doc).Debugf("remove '%v'", collectionName) + log.WithField("collection", collectionName).Debugf("remove '%v'", collectionName) return c.RemoveId(doc.getID()) } @@ -232,7 +232,7 @@ func Update(ctx context.Context, collectionName string, update bson.M, doc docum return err } - log.Debugf("update %v", collectionName) + log.WithField("collection", collectionName).Debugf("update %v", collectionName) return c.UpdateId(doc.getID(), update) } From 70317d01c3664aaa9a03d2e90d2455e23c323a85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Michon?= Date: Thu, 13 Aug 2020 10:57:56 +0200 Subject: [PATCH 3/5] Also log the document ID for debugging --- mongo/document/document.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/mongo/document/document.go b/mongo/document/document.go index 8582c36e..0efc3db1 100644 --- a/mongo/document/document.go +++ b/mongo/document/document.go @@ -65,7 +65,10 @@ func Create(ctx context.Context, collectionName string, doc document) error { c := mongo.Session(log).Clone().DB("").C(collectionName) defer c.Database.Session.Close() - log.WithField("collection", collectionName).Debugf("save '%v'", collectionName) + log.WithFields(logrus.Fields{ + "collection": collectionName, + "doc_id": doc.getID(), + }).Debugf("save '%v'", collectionName) return c.Insert(doc) } @@ -82,7 +85,10 @@ func Save(ctx context.Context, collectionName string, doc document) error { c := mongo.Session(log).Clone().DB("").C(collectionName) defer c.Database.Session.Close() - log.WithField("collection", collectionName).Debugf("save '%v'", collectionName) + log.WithFields(logrus.Fields{ + "collection": collectionName, + "doc_id": doc.getID(), + }).Debugf("save '%v'", collectionName) _, err := c.UpsertId(doc.getID(), doc) return err } @@ -96,7 +102,10 @@ func ReallyDestroy(ctx context.Context, collectionName string, doc document) err log := logger.Get(ctx) c := mongo.Session(log).Clone().DB("").C(collectionName) defer c.Database.Session.Close() - log.WithField("collection", collectionName).Debugf("remove '%v'", collectionName) + log.WithFields(logrus.Fields{ + "collection": collectionName, + "doc_id": doc.getID(), + }).Debugf("remove '%v'", collectionName) return c.RemoveId(doc.getID()) } @@ -232,7 +241,10 @@ func Update(ctx context.Context, collectionName string, update bson.M, doc docum return err } - log.WithField("collection", collectionName).Debugf("update %v", collectionName) + log.WithFields(logrus.Fields{ + "collection": collectionName, + "doc_id": doc.getID(), + }).Debugf("update %v", collectionName) return c.UpdateId(doc.getID(), update) } From 6edfd992aec7bd6e9a0b7e3f1fdfa9d7c9982aa9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Michon?= Date: Thu, 13 Aug 2020 12:07:48 +0200 Subject: [PATCH 4/5] Attempt to fix tests --- graceful/service_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/graceful/service_test.go b/graceful/service_test.go index 67f82648..e21643fb 100644 --- a/graceful/service_test.go +++ b/graceful/service_test.go @@ -161,7 +161,9 @@ func ensurePidFileProcessKilled(t *testing.T) { process, err := os.FindProcess(pid) require.NoError(t, err) err = process.Kill() - require.NoError(t, err) + if err != nil && !strings.Contains(err.Error(), "already finished") { + require.NoError(t, err) + } err = os.Remove("./test-fixtures/server.pid") require.NoError(t, err) } From 1a3292cf9d7ef60753f64f0af94a19457946b60c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Michon?= Date: Thu, 13 Aug 2020 12:36:30 +0200 Subject: [PATCH 5/5] Validation errors tests should not be sensitive to array order --- errors/validation_errors_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/errors/validation_errors_test.go b/errors/validation_errors_test.go index 0cb75b2f..c29b150a 100644 --- a/errors/validation_errors_test.go +++ b/errors/validation_errors_test.go @@ -67,7 +67,7 @@ func TestValidationErrorsBuilder_MergeWithPrefix(t *testing.T) { func TestValidationErrors_Error(t *testing.T) { cases := map[string]struct { Errors ValidationErrors - ExpectedString string + expectedErrors []string }{ "should return a string with one error in it": { Errors: ValidationErrors{ @@ -75,7 +75,7 @@ func TestValidationErrors_Error(t *testing.T) { "name": {"invalid name"}, }, }, - ExpectedString: "name=invalid name", + expectedErrors: []string{"name=invalid name"}, }, "should return a string with multiple errors in it with the same field name": { Errors: ValidationErrors{ @@ -83,7 +83,7 @@ func TestValidationErrors_Error(t *testing.T) { "name": {"invalid name", "should contains alphanumeric characters"}, }, }, - ExpectedString: "name=invalid name, should contains alphanumeric characters", + expectedErrors: []string{"name=invalid name", "should contains alphanumeric characters"}, }, "should return a string with multiple errors in it with multiple field name": { Errors: ValidationErrors{ @@ -92,13 +92,15 @@ func TestValidationErrors_Error(t *testing.T) { "type": {"invalid type", "type not exist"}, }, }, - ExpectedString: "name=invalid name, should contains alphanumeric characters type=invalid type, type not exist", + expectedErrors: []string{"name=invalid name, should contains alphanumeric characters", "type=invalid type, type not exist"}, }, } for title, c := range cases { t.Run(title, func(t *testing.T) { - require.Equal(t, c.ExpectedString, c.Errors.Error()) + for _, expectedError := range c.expectedErrors { + require.Contains(t, c.Errors.Error(), expectedError) + } }) } }