From 778869c3183b704e4ae37cba6a5841c89191c26c Mon Sep 17 00:00:00 2001 From: Gani Georgiev Date: Tue, 25 Mar 2025 19:56:03 +0200 Subject: [PATCH] [#6639] fixed RecordErrorEvent.Error and CollectionErrorEvent.Error sync with ModelErrorEvent.Error --- CHANGELOG.md | 5 + core/collection_model_test.go | 275 +++++++++++++++++++++++++++++++++ core/collection_validate.go | 6 +- core/events.go | 4 +- core/record_model_test.go | 282 ++++++++++++++++++++++++++++++++++ 5 files changed, 569 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d36f28b..a30ffafe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +## v0.26.4 (WIP) + +- Fixed `RecordErrorEvent.Error` and `CollectionErrorEvent.Error` sync with `ModelErrorEvent.Error` sync with ([#6639](https://github.com/pocketbase/pocketbase/issues/6639)). + + ## v0.26.3 - Fixed and normalized logs error serialization across common types for more consistent logs error output ([#6631](https://github.com/pocketbase/pocketbase/issues/6631)). diff --git a/core/collection_model_test.go b/core/collection_model_test.go index 0a259344..81a17f5b 100644 --- a/core/collection_model_test.go +++ b/core/collection_model_test.go @@ -1,9 +1,12 @@ package core_test import ( + "context" "encoding/json" + "errors" "fmt" "slices" + "strconv" "strings" "testing" @@ -11,6 +14,7 @@ import ( "github.com/pocketbase/pocketbase/core" "github.com/pocketbase/pocketbase/tests" "github.com/pocketbase/pocketbase/tools/dbutils" + "github.com/pocketbase/pocketbase/tools/hook" "github.com/pocketbase/pocketbase/tools/types" ) @@ -976,6 +980,277 @@ func TestCollectionDelete(t *testing.T) { } } +func TestCollectionModelEventSync(t *testing.T) { + t.Parallel() + + app, _ := tests.NewTestApp() + defer app.Cleanup() + + testCollections := make([]*core.Collection, 4) + for i := 0; i < 4; i++ { + testCollections[i] = core.NewBaseCollection("sync_test_" + strconv.Itoa(i)) + if err := app.Save(testCollections[i]); err != nil { + t.Fatal(err) + } + } + + createModelEvent := func() *core.ModelEvent { + event := new(core.ModelEvent) + event.App = app + event.Context = context.Background() + event.Type = "test_a" + event.Model = testCollections[0] + return event + } + + createModelErrorEvent := func() *core.ModelErrorEvent { + event := new(core.ModelErrorEvent) + event.ModelEvent = *createModelEvent() + event.Error = errors.New("error_a") + return event + } + + changeCollectionEventBefore := func(e *core.CollectionEvent) { + e.Type = "test_b" + e.Context = context.WithValue(context.Background(), "test", 123) + e.Collection = testCollections[1] + } + + modelEventFinalizerChange := func(e *core.ModelEvent) { + e.Type = "test_c" + e.Context = context.WithValue(context.Background(), "test", 456) + e.Model = testCollections[2] + } + + changeCollectionEventAfter := func(e *core.CollectionEvent) { + e.Type = "test_d" + e.Context = context.WithValue(context.Background(), "test", 789) + e.Collection = testCollections[3] + } + + expectedBeforeModelEventHandlerChecks := func(t *testing.T, e *core.ModelEvent) { + if e.Type != "test_a" { + t.Fatalf("Expected type %q, got %q", "test_a", e.Type) + } + + if v := e.Context.Value("test"); v != nil { + t.Fatalf("Expected context value %v, got %v", nil, v) + } + + if e.Model.PK() != testCollections[0].Id { + t.Fatalf("Expected collection with id %q, got %q (%d)", testCollections[0].Id, e.Model.PK(), 0) + } + } + + expectedAfterModelEventHandlerChecks := func(t *testing.T, e *core.ModelEvent) { + if e.Type != "test_d" { + t.Fatalf("Expected type %q, got %q", "test_d", e.Type) + } + + if v := e.Context.Value("test"); v != 789 { + t.Fatalf("Expected context value %v, got %v", 789, v) + } + + if e.Model.PK() != testCollections[3].Id { + t.Fatalf("Expected collection with id %q, got %q (%d)", testCollections[3].Id, e.Model.PK(), 3) + } + } + + expectedBeforeCollectionEventHandlerChecks := func(t *testing.T, e *core.CollectionEvent) { + if e.Type != "test_a" { + t.Fatalf("Expected type %q, got %q", "test_a", e.Type) + } + + if v := e.Context.Value("test"); v != nil { + t.Fatalf("Expected context value %v, got %v", nil, v) + } + + if e.Collection.Id != testCollections[0].Id { + t.Fatalf("Expected collection with id %q, got %q (%d)", testCollections[0].Id, e.Collection.Id, 0) + } + } + + expectedAfterCollectionEventHandlerChecks := func(t *testing.T, e *core.CollectionEvent) { + if e.Type != "test_c" { + t.Fatalf("Expected type %q, got %q", "test_c", e.Type) + } + + if v := e.Context.Value("test"); v != 456 { + t.Fatalf("Expected context value %v, got %v", 456, v) + } + + if e.Collection.Id != testCollections[2].Id { + t.Fatalf("Expected collection with id %q, got %q (%d)", testCollections[2].Id, e.Collection.Id, 2) + } + } + + modelEventFinalizer := func(e *core.ModelEvent) error { + modelEventFinalizerChange(e) + return nil + } + + modelErrorEventFinalizer := func(e *core.ModelErrorEvent) error { + modelEventFinalizerChange(&e.ModelEvent) + e.Error = errors.New("error_c") + return nil + } + + modelEventHandler := &hook.Handler[*core.ModelEvent]{ + Priority: -999, + Func: func(e *core.ModelEvent) error { + t.Run("before model", func(t *testing.T) { + expectedBeforeModelEventHandlerChecks(t, e) + }) + + _ = e.Next() + + t.Run("after model", func(t *testing.T) { + expectedAfterModelEventHandlerChecks(t, e) + }) + + return nil + }, + } + + modelErrorEventHandler := &hook.Handler[*core.ModelErrorEvent]{ + Priority: -999, + Func: func(e *core.ModelErrorEvent) error { + t.Run("before model error", func(t *testing.T) { + expectedBeforeModelEventHandlerChecks(t, &e.ModelEvent) + if v := e.Error.Error(); v != "error_a" { + t.Fatalf("Expected error %q, got %q", "error_a", v) + } + }) + + _ = e.Next() + + t.Run("after model error", func(t *testing.T) { + expectedAfterModelEventHandlerChecks(t, &e.ModelEvent) + if v := e.Error.Error(); v != "error_d" { + t.Fatalf("Expected error %q, got %q", "error_d", v) + } + }) + + return nil + }, + } + + recordEventHandler := &hook.Handler[*core.CollectionEvent]{ + Priority: -999, + Func: func(e *core.CollectionEvent) error { + t.Run("before collection", func(t *testing.T) { + expectedBeforeCollectionEventHandlerChecks(t, e) + }) + + changeCollectionEventBefore(e) + + _ = e.Next() + + t.Run("after collection", func(t *testing.T) { + expectedAfterCollectionEventHandlerChecks(t, e) + }) + + changeCollectionEventAfter(e) + + return nil + }, + } + + collectionErrorEventHandler := &hook.Handler[*core.CollectionErrorEvent]{ + Priority: -999, + Func: func(e *core.CollectionErrorEvent) error { + t.Run("before collection error", func(t *testing.T) { + expectedBeforeCollectionEventHandlerChecks(t, &e.CollectionEvent) + if v := e.Error.Error(); v != "error_a" { + t.Fatalf("Expected error %q, got %q", "error_c", v) + } + }) + + changeCollectionEventBefore(&e.CollectionEvent) + e.Error = errors.New("error_b") + + _ = e.Next() + + t.Run("after collection error", func(t *testing.T) { + expectedAfterCollectionEventHandlerChecks(t, &e.CollectionEvent) + if v := e.Error.Error(); v != "error_c" { + t.Fatalf("Expected error %q, got %q", "error_c", v) + } + }) + + changeCollectionEventAfter(&e.CollectionEvent) + e.Error = errors.New("error_d") + + return nil + }, + } + + // OnModelValidate + app.OnCollectionValidate().Bind(recordEventHandler) + app.OnModelValidate().Bind(modelEventHandler) + app.OnModelValidate().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelCreate + app.OnCollectionCreate().Bind(recordEventHandler) + app.OnModelCreate().Bind(modelEventHandler) + app.OnModelCreate().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelCreateExecute + app.OnCollectionCreateExecute().Bind(recordEventHandler) + app.OnModelCreateExecute().Bind(modelEventHandler) + app.OnModelCreateExecute().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterCreateSuccess + app.OnCollectionAfterCreateSuccess().Bind(recordEventHandler) + app.OnModelAfterCreateSuccess().Bind(modelEventHandler) + app.OnModelAfterCreateSuccess().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterCreateError + app.OnCollectionAfterCreateError().Bind(collectionErrorEventHandler) + app.OnModelAfterCreateError().Bind(modelErrorEventHandler) + app.OnModelAfterCreateError().Trigger(createModelErrorEvent(), modelErrorEventFinalizer) + + // OnModelUpdate + app.OnCollectionUpdate().Bind(recordEventHandler) + app.OnModelUpdate().Bind(modelEventHandler) + app.OnModelUpdate().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelUpdateExecute + app.OnCollectionUpdateExecute().Bind(recordEventHandler) + app.OnModelUpdateExecute().Bind(modelEventHandler) + app.OnModelUpdateExecute().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterUpdateSuccess + app.OnCollectionAfterUpdateSuccess().Bind(recordEventHandler) + app.OnModelAfterUpdateSuccess().Bind(modelEventHandler) + app.OnModelAfterUpdateSuccess().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterUpdateError + app.OnCollectionAfterUpdateError().Bind(collectionErrorEventHandler) + app.OnModelAfterUpdateError().Bind(modelErrorEventHandler) + app.OnModelAfterUpdateError().Trigger(createModelErrorEvent(), modelErrorEventFinalizer) + + // OnModelDelete + app.OnCollectionDelete().Bind(recordEventHandler) + app.OnModelDelete().Bind(modelEventHandler) + app.OnModelDelete().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelDeleteExecute + app.OnCollectionDeleteExecute().Bind(recordEventHandler) + app.OnModelDeleteExecute().Bind(modelEventHandler) + app.OnModelDeleteExecute().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterDeleteSuccess + app.OnCollectionAfterDeleteSuccess().Bind(recordEventHandler) + app.OnModelAfterDeleteSuccess().Bind(modelEventHandler) + app.OnModelAfterDeleteSuccess().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterDeleteError + app.OnCollectionAfterDeleteError().Bind(collectionErrorEventHandler) + app.OnModelAfterDeleteError().Bind(modelErrorEventHandler) + app.OnModelAfterDeleteError().Trigger(createModelErrorEvent(), modelErrorEventFinalizer) +} + func TestCollectionSaveModel(t *testing.T) { t.Parallel() diff --git a/core/collection_validate.go b/core/collection_validate.go index ad9116b1..56d23eda 100644 --- a/core/collection_validate.go +++ b/core/collection_validate.go @@ -34,7 +34,11 @@ func onCollectionValidate(e *CollectionEvent) error { original, ) - return validator.run() + if err := validator.run(); err != nil { + return err + } + + return e.Next() } func newCollectionValidator(ctx context.Context, app App, new, original *Collection) *collectionValidator { diff --git a/core/events.go b/core/events.go index 2170b12a..41511e7a 100644 --- a/core/events.go +++ b/core/events.go @@ -277,7 +277,7 @@ func syncModelErrorEventWithRecordErrorEvent(me *ModelErrorEvent, re *RecordErro func syncRecordErrorEventWithModelErrorEvent(re *RecordErrorEvent, me *ModelErrorEvent) { syncRecordEventWithModelEvent(&re.RecordEvent, &me.ModelEvent) - me.Error = re.Error + re.Error = me.Error } // ------------------------------------------------------------------- @@ -354,7 +354,7 @@ func syncModelErrorEventWithCollectionErrorEvent(me *ModelErrorEvent, ce *Collec func syncCollectionErrorEventWithModelErrorEvent(ce *CollectionErrorEvent, me *ModelErrorEvent) { syncCollectionEventWithModelEvent(&ce.CollectionEvent, &me.ModelEvent) - me.Error = ce.Error + ce.Error = me.Error } // ------------------------------------------------------------------- diff --git a/core/record_model_test.go b/core/record_model_test.go index af394112..6f19aa7c 100644 --- a/core/record_model_test.go +++ b/core/record_model_test.go @@ -5,9 +5,11 @@ import ( "context" "database/sql" "encoding/json" + "errors" "fmt" "regexp" "slices" + "strconv" "strings" "testing" "time" @@ -16,6 +18,7 @@ import ( "github.com/pocketbase/pocketbase/core" "github.com/pocketbase/pocketbase/tests" "github.com/pocketbase/pocketbase/tools/filesystem" + "github.com/pocketbase/pocketbase/tools/hook" "github.com/pocketbase/pocketbase/tools/types" "github.com/spf13/cast" ) @@ -1618,6 +1621,285 @@ func TestRecordValidate(t *testing.T) { }) } +func TestRecordModelEventSync(t *testing.T) { + t.Parallel() + + app, _ := tests.NewTestApp() + defer app.Cleanup() + + col, err := app.FindCollectionByNameOrId("demo3") + if err != nil { + t.Fatal(err) + } + + testRecords := make([]*core.Record, 4) + for i := 0; i < 4; i++ { + testRecords[i] = core.NewRecord(col) + testRecords[i].Set("title", "sync_test_"+strconv.Itoa(i)) + if err := app.Save(testRecords[i]); err != nil { + t.Fatal(err) + } + } + + createModelEvent := func() *core.ModelEvent { + event := new(core.ModelEvent) + event.App = app + event.Context = context.Background() + event.Type = "test_a" + event.Model = testRecords[0] + return event + } + + createModelErrorEvent := func() *core.ModelErrorEvent { + event := new(core.ModelErrorEvent) + event.ModelEvent = *createModelEvent() + event.Error = errors.New("error_a") + return event + } + + changeRecordEventBefore := func(e *core.RecordEvent) { + e.Type = "test_b" + e.Context = context.WithValue(context.Background(), "test", 123) + e.Record = testRecords[1] + } + + modelEventFinalizerChange := func(e *core.ModelEvent) { + e.Type = "test_c" + e.Context = context.WithValue(context.Background(), "test", 456) + e.Model = testRecords[2] + } + + changeRecordEventAfter := func(e *core.RecordEvent) { + e.Type = "test_d" + e.Context = context.WithValue(context.Background(), "test", 789) + e.Record = testRecords[3] + } + + expectedBeforeModelEventHandlerChecks := func(t *testing.T, e *core.ModelEvent) { + if e.Type != "test_a" { + t.Fatalf("Expected type %q, got %q", "test_a", e.Type) + } + + if v := e.Context.Value("test"); v != nil { + t.Fatalf("Expected context value %v, got %v", nil, v) + } + + if e.Model.PK() != testRecords[0].Id { + t.Fatalf("Expected record with id %q, got %q (%d)", testRecords[0].Id, e.Model.PK(), 0) + } + } + + expectedAfterModelEventHandlerChecks := func(t *testing.T, e *core.ModelEvent) { + if e.Type != "test_d" { + t.Fatalf("Expected type %q, got %q", "test_d", e.Type) + } + + if v := e.Context.Value("test"); v != 789 { + t.Fatalf("Expected context value %v, got %v", 789, v) + } + + // note: currently the Model and Record values are not synced due to performance consideration + if e.Model.PK() != testRecords[2].Id { + t.Fatalf("Expected record with id %q, got %q (%d)", testRecords[2].Id, e.Model.PK(), 2) + } + } + + expectedBeforeRecordEventHandlerChecks := func(t *testing.T, e *core.RecordEvent) { + if e.Type != "test_a" { + t.Fatalf("Expected type %q, got %q", "test_a", e.Type) + } + + if v := e.Context.Value("test"); v != nil { + t.Fatalf("Expected context value %v, got %v", nil, v) + } + + if e.Record.Id != testRecords[0].Id { + t.Fatalf("Expected record with id %q, got %q (%d)", testRecords[0].Id, e.Record.Id, 2) + } + } + + expectedAfterRecordEventHandlerChecks := func(t *testing.T, e *core.RecordEvent) { + if e.Type != "test_c" { + t.Fatalf("Expected type %q, got %q", "test_c", e.Type) + } + + if v := e.Context.Value("test"); v != 456 { + t.Fatalf("Expected context value %v, got %v", 456, v) + } + + // note: currently the Model and Record values are not synced due to performance consideration + if e.Record.Id != testRecords[1].Id { + t.Fatalf("Expected record with id %q, got %q (%d)", testRecords[1].Id, e.Record.Id, 1) + } + } + + modelEventFinalizer := func(e *core.ModelEvent) error { + modelEventFinalizerChange(e) + return nil + } + + modelErrorEventFinalizer := func(e *core.ModelErrorEvent) error { + modelEventFinalizerChange(&e.ModelEvent) + e.Error = errors.New("error_c") + return nil + } + + modelEventHandler := &hook.Handler[*core.ModelEvent]{ + Priority: -999, + Func: func(e *core.ModelEvent) error { + t.Run("before model", func(t *testing.T) { + expectedBeforeModelEventHandlerChecks(t, e) + }) + + _ = e.Next() + + t.Run("after model", func(t *testing.T) { + expectedAfterModelEventHandlerChecks(t, e) + }) + + return nil + }, + } + + modelErrorEventHandler := &hook.Handler[*core.ModelErrorEvent]{ + Priority: -999, + Func: func(e *core.ModelErrorEvent) error { + t.Run("before model error", func(t *testing.T) { + expectedBeforeModelEventHandlerChecks(t, &e.ModelEvent) + if v := e.Error.Error(); v != "error_a" { + t.Fatalf("Expected error %q, got %q", "error_a", v) + } + }) + + _ = e.Next() + + t.Run("after model error", func(t *testing.T) { + expectedAfterModelEventHandlerChecks(t, &e.ModelEvent) + if v := e.Error.Error(); v != "error_d" { + t.Fatalf("Expected error %q, got %q", "error_d", v) + } + }) + + return nil + }, + } + + recordEventHandler := &hook.Handler[*core.RecordEvent]{ + Priority: -999, + Func: func(e *core.RecordEvent) error { + t.Run("before record", func(t *testing.T) { + expectedBeforeRecordEventHandlerChecks(t, e) + }) + + changeRecordEventBefore(e) + + _ = e.Next() + + t.Run("after record", func(t *testing.T) { + expectedAfterRecordEventHandlerChecks(t, e) + }) + + changeRecordEventAfter(e) + + return nil + }, + } + + recordErrorEventHandler := &hook.Handler[*core.RecordErrorEvent]{ + Priority: -999, + Func: func(e *core.RecordErrorEvent) error { + t.Run("before record error", func(t *testing.T) { + expectedBeforeRecordEventHandlerChecks(t, &e.RecordEvent) + if v := e.Error.Error(); v != "error_a" { + t.Fatalf("Expected error %q, got %q", "error_c", v) + } + }) + + changeRecordEventBefore(&e.RecordEvent) + e.Error = errors.New("error_b") + + _ = e.Next() + + t.Run("after record error", func(t *testing.T) { + expectedAfterRecordEventHandlerChecks(t, &e.RecordEvent) + if v := e.Error.Error(); v != "error_c" { + t.Fatalf("Expected error %q, got %q", "error_c", v) + } + }) + + changeRecordEventAfter(&e.RecordEvent) + e.Error = errors.New("error_d") + + return nil + }, + } + + // OnModelValidate + app.OnRecordValidate().Bind(recordEventHandler) + app.OnModelValidate().Bind(modelEventHandler) + app.OnModelValidate().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelCreate + app.OnRecordCreate().Bind(recordEventHandler) + app.OnModelCreate().Bind(modelEventHandler) + app.OnModelCreate().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelCreateExecute + app.OnRecordCreateExecute().Bind(recordEventHandler) + app.OnModelCreateExecute().Bind(modelEventHandler) + app.OnModelCreateExecute().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterCreateSuccess + app.OnRecordAfterCreateSuccess().Bind(recordEventHandler) + app.OnModelAfterCreateSuccess().Bind(modelEventHandler) + app.OnModelAfterCreateSuccess().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterCreateError + app.OnRecordAfterCreateError().Bind(recordErrorEventHandler) + app.OnModelAfterCreateError().Bind(modelErrorEventHandler) + app.OnModelAfterCreateError().Trigger(createModelErrorEvent(), modelErrorEventFinalizer) + + // OnModelUpdate + app.OnRecordUpdate().Bind(recordEventHandler) + app.OnModelUpdate().Bind(modelEventHandler) + app.OnModelUpdate().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelUpdateExecute + app.OnRecordUpdateExecute().Bind(recordEventHandler) + app.OnModelUpdateExecute().Bind(modelEventHandler) + app.OnModelUpdateExecute().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterUpdateSuccess + app.OnRecordAfterUpdateSuccess().Bind(recordEventHandler) + app.OnModelAfterUpdateSuccess().Bind(modelEventHandler) + app.OnModelAfterUpdateSuccess().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterUpdateError + app.OnRecordAfterUpdateError().Bind(recordErrorEventHandler) + app.OnModelAfterUpdateError().Bind(modelErrorEventHandler) + app.OnModelAfterUpdateError().Trigger(createModelErrorEvent(), modelErrorEventFinalizer) + + // OnModelDelete + app.OnRecordDelete().Bind(recordEventHandler) + app.OnModelDelete().Bind(modelEventHandler) + app.OnModelDelete().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelDeleteExecute + app.OnRecordDeleteExecute().Bind(recordEventHandler) + app.OnModelDeleteExecute().Bind(modelEventHandler) + app.OnModelDeleteExecute().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterDeleteSuccess + app.OnRecordAfterDeleteSuccess().Bind(recordEventHandler) + app.OnModelAfterDeleteSuccess().Bind(modelEventHandler) + app.OnModelAfterDeleteSuccess().Trigger(createModelEvent(), modelEventFinalizer) + + // OnModelAfterDeleteError + app.OnRecordAfterDeleteError().Bind(recordErrorEventHandler) + app.OnModelAfterDeleteError().Bind(modelErrorEventHandler) + app.OnModelAfterDeleteError().Trigger(createModelErrorEvent(), modelErrorEventFinalizer) +} + func TestRecordSave(t *testing.T) { t.Parallel()