Skip to content

Commit

Permalink
Merge pull request #999 from Scalingo/fix/998/validation-internal-errors
Browse files Browse the repository at this point in the history
package(mongo) Do not break API to handle validation errors with internal errors
  • Loading branch information
leo-scalingo authored Nov 21, 2024
2 parents d750436 + ae3124d commit 2dad0ed
Show file tree
Hide file tree
Showing 4 changed files with 206 additions and 66 deletions.
2 changes: 1 addition & 1 deletion mongo/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## To be Released

* [BREAKING CHANGE]: feat(mongo/document/validation): Add distinction between internal and validation errors ([PR#552](https://github.com/Scalingo/go-utils/pull/552)).
* feat(mongo/document/validation): Add distinction between internal and validation errors, introduce `ValidateWithInternalError` method which will be used in priority.
* feat(pagination): Add `QueryFunc` parameter to be able to customize the query builder and user `WhereUnsopedQuery` for instance

## 1.3.2
Expand Down
12 changes: 10 additions & 2 deletions mongo/document/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ func (d *Base) setUpdatedAt(t time.Time) {
d.UpdatedAt = t
}

func (d *Base) getUpdatedAt() time.Time {
return d.UpdatedAt
}

func (d Base) scope(query bson.M) bson.M {
return query
}
Expand All @@ -47,6 +51,10 @@ func (d *Base) destroy(ctx context.Context, collection string) error {
return ReallyDestroy(ctx, collection, d)
}

func (d *Base) Validate(_ context.Context) (*errors.ValidationErrors, error) {
return nil, nil
func (d *Base) Validate(_ context.Context) *errors.ValidationErrors {
return nil
}

func (d *Base) ValidateWithInternalError(_ context.Context) (*errors.ValidationErrors, error) {
return nil, ErrValidateNoInternalErrorFunc
}
124 changes: 61 additions & 63 deletions mongo/document/document.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package document

import (
"context"
stderrors "errors"
"fmt"
"time"

Expand All @@ -22,6 +23,7 @@ type document interface {
ensureID()
ensureCreatedAt()
setUpdatedAt(time.Time)
getUpdatedAt() time.Time
Validable
}

Expand All @@ -46,64 +48,87 @@ type Closer interface {
Close()
}

var ErrValidateNoInternalErrorFunc = stderrors.New("no validation returning an internal error has been implemented")

type Validable interface {
Validate(ctx context.Context) (*errors.ValidationErrors, error)
// Validate will be used if no ValidateWithInternalError is defined on a document
// It is not useful to have both defined on a document, only ValidationWithInternalError
// would be used in this case
Validate(ctx context.Context) *errors.ValidationErrors

// ValidateWithInternalError will be used in priority if defined on a document
// It will be called for all modifying operations (Create, Save, Update)
// If it returns an internal error, the validation error will be nil.
ValidateWithInternalError(ctx context.Context) (*errors.ValidationErrors, error)
}

var _ Validable = &Base{}

// Create inserts the document in the database, returns an error if document
// already exists and set CreatedAt timestamp
func Create(ctx context.Context, collectionName string, doc document) error {
log := logger.Get(ctx).WithFields(logrus.Fields{
"collection": collectionName,
"doc_id": doc.getID().Hex(),
return save(ctx, collectionName, doc, func(ctx context.Context, collectionName string, doc document) error {
log := logger.Get(ctx)
//nolint contextcheck
c := mongo.Session(log).Clone().DB("").C(collectionName)
defer c.Database.Session.Close()
log.WithFields(logrus.Fields{
"collection": collectionName,
"doc_id": doc.getID().Hex(),
}).Debugf("save '%v'", collectionName)
return c.Insert(doc)
})
doc.ensureID()
doc.ensureCreatedAt()
doc.setUpdatedAt(time.Now())
}

validationErrors, err := doc.Validate(ctx)
if err != nil {
log.WithError(err).Error("Internal error while validating the document")
func Save(ctx context.Context, collectionName string, doc document) error {
return save(ctx, collectionName, doc, func(ctx context.Context, collectionName string, doc document) error {
log := logger.Get(ctx)
//nolint contextcheck
c := mongo.Session(log).Clone().DB("").C(collectionName)
defer c.Database.Session.Close()
log.Debugf("save '%v'", collectionName)
_, err := c.UpsertId(doc.getID(), doc)
return err
})
}

func Update(ctx context.Context, collectionName string, update bson.M, doc document) error {
return save(ctx, collectionName, doc, func(ctx context.Context, collectionName string, doc document) error {
log := logger.Get(ctx)
//nolint contextcheck
c := mongo.Session(log).Clone().DB("").C(collectionName)
defer c.Database.Session.Close()

if _, ok := update["$set"]; ok {
update["$set"].(bson.M)["updated_at"] = doc.getUpdatedAt()
}

log.Debugf("update %v", collectionName)
return c.UpdateId(doc.getID(), update)
})
}

func save(ctx context.Context, collectionName string, doc document, saveFunc func(context.Context, string, document) error) error {
validationErrors, err := doc.ValidateWithInternalError(ctx)
if err == ErrValidateNoInternalErrorFunc {
validationErrors = doc.Validate(ctx)
} else if err != nil {
return errors.Wrap(ctx, err, "validate document")
}
if validationErrors != nil {
return validationErrors
}

c := mongo.Session(log).Clone().DB("").C(collectionName)
defer c.Database.Session.Close()
log.WithFields(logrus.Fields{
"collection": collectionName,
"doc_id": doc.getID().Hex(),
}).Debugf("save '%v'", collectionName)
return c.Insert(doc)
}

func Save(ctx context.Context, collectionName string, doc document) error {
log := logger.Get(ctx)
doc.ensureID()
doc.ensureCreatedAt()
doc.setUpdatedAt(time.Now())

validationErrors, err := doc.Validate(ctx)
if err != nil {
log.WithError(err).Error("Internal error while validating the document")
return err
}
if validationErrors != nil {
return validationErrors
}

c := mongo.Session(log).Clone().DB("").C(collectionName)
defer c.Database.Session.Close()
log.WithFields(logrus.Fields{
ctx, _ = logger.WithFieldsToCtx(ctx, logrus.Fields{
"collection": collectionName,
"doc_id": doc.getID().Hex(),
}).Debugf("save '%v'", collectionName)
_, err = c.UpsertId(doc.getID(), doc)
return err
})

return saveFunc(ctx, collectionName, doc)
}

// Destroy really deletes
Expand Down Expand Up @@ -263,33 +288,6 @@ func WhereIterUnscoped(ctx context.Context, collectionName string, query bson.M,
return nil
}

func Update(ctx context.Context, collectionName string, update bson.M, doc document) error {
log := logger.Get(ctx)
c := mongo.Session(log).Clone().DB("").C(collectionName)
defer c.Database.Session.Close()

now := time.Now()
doc.setUpdatedAt(now)
if _, ok := update["$set"]; ok {
update["$set"].(bson.M)["updated_at"] = now
}

validationErrors, err := doc.Validate(ctx)
if err != nil {
log.WithError(err).Error("Internal error while validating the document")
return err
}
if validationErrors != nil {
return validationErrors
}

log.WithFields(logrus.Fields{
"collection": collectionName,
"doc_id": doc.getID().Hex(),
}).Debugf("update %v", collectionName)
return c.UpdateId(doc.getID(), update)
}

func EnsureParanoidIndices(ctx context.Context, collectionNames ...string) {
log := logger.Get(ctx)

Expand Down
134 changes: 134 additions & 0 deletions mongo/document/document_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package document

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gopkg.in/mgo.v2"

"github.com/Scalingo/go-utils/errors/v2"
"github.com/Scalingo/go-utils/logger"
"github.com/Scalingo/go-utils/mongo"
)

type unvalidatedDocument struct {
Base `bson:",inline"`
}

const testDocuments = "test_documents"

type validatedDocument struct {
Base `bson:",inline"`
Valid bool `bson:"valid" json:"valid"`
}

func (d *validatedDocument) Validate(_ context.Context) *errors.ValidationErrors {
verr := errors.NewValidationErrorsBuilder()
if !d.Valid {
verr.Set("valid", "must be true")
}
return verr.Build()
}

func buildValidatedDocument(valid bool) *validatedDocument {
return &validatedDocument{
Valid: valid,
}
}

type validatedWithInternalErrorDocument struct {
validatedDocument `bson:",inline"`
InternalError string `bson:"internal_error" json:"internal_error"`
}

func (d *validatedWithInternalErrorDocument) ValidateWithInternalError(ctx context.Context) (*errors.ValidationErrors, error) {
if d.InternalError != "" {
return nil, errors.New(ctx, d.InternalError)
}
return d.validatedDocument.Validate(ctx), nil
}

func buildValidatedWithInternalErrorDocument(valid bool, internalError string) *validatedWithInternalErrorDocument {
return &validatedWithInternalErrorDocument{
validatedDocument: *buildValidatedDocument(valid),
InternalError: internalError,
}
}

func TestDocument_Create(t *testing.T) {
t.Cleanup(func() {
coll := mongo.Session(logger.Default()).Clone().DB("").C(testDocuments)
err := coll.DropCollection()
// Handle case when collection does not exist
var queryErr *mgo.QueryError
if !errors.As(err, &queryErr) || queryErr.Message != "ns not found" {
require.NoError(t, err)
}
})
t.Run("without validation", func(t *testing.T) {
t.Run("it should create the document", func(t *testing.T) {
d := &unvalidatedDocument{}
err := Create(context.Background(), testDocuments, d)
require.NoError(t, err)
assert.NotEmpty(t, d.ID)
assert.NotEmpty(t, d.CreatedAt)
})
})

t.Run("with simple validation", func(t *testing.T) {
t.Run("with a valid document, it should create it", func(t *testing.T) {
d := buildValidatedDocument(true)
err := Create(context.Background(), testDocuments, d)
require.NoError(t, err)
assert.NotEmpty(t, d.ID)
assert.NotEmpty(t, d.CreatedAt)
})

t.Run("with an invalid document, it should return a validation error", func(t *testing.T) {
d := buildValidatedDocument(false)
err := Create(context.Background(), testDocuments, d)
require.Error(t, err)
require.IsType(t, &errors.ValidationErrors{}, err)
assert.Empty(t, d.ID)
assert.Empty(t, d.CreatedAt)
})
})

t.Run("with internal error validation", func(t *testing.T) {
t.Run("with a valid document, it should create it", func(t *testing.T) {
d := buildValidatedWithInternalErrorDocument(true, "")
err := Create(context.Background(), testDocuments, d)
require.NoError(t, err)
assert.NotEmpty(t, d.ID)
assert.NotEmpty(t, d.CreatedAt)
})
t.Run("with an invalid document, it should return a validation error", func(t *testing.T) {
d := buildValidatedWithInternalErrorDocument(false, "")
err := Create(context.Background(), testDocuments, d)
require.Error(t, err)
require.IsType(t, &errors.ValidationErrors{}, err)
assert.Empty(t, d.ID)
assert.Empty(t, d.CreatedAt)
})

t.Run("with a validation returning an internal error, it should forward internal error", func(t *testing.T) {
d := buildValidatedWithInternalErrorDocument(true, "internal error when validating")
err := Create(context.Background(), testDocuments, d)
require.Error(t, err)
assert.Contains(t, err.Error(), "internal error when validating")
assert.Empty(t, d.ID)
assert.Empty(t, d.CreatedAt)
})

t.Run("with a validation returning an internal error and a validation error, it should return the internal error", func(t *testing.T) {
d := buildValidatedWithInternalErrorDocument(false, "internal error when validating")
err := Create(context.Background(), testDocuments, d)
require.Error(t, err)
assert.Contains(t, err.Error(), "internal error when validating")
assert.Empty(t, d.ID)
assert.Empty(t, d.CreatedAt)
})
})
}

0 comments on commit 2dad0ed

Please sign in to comment.