From 1e4c86942488fa0db19573fc00a752076bf5f372 Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Sun, 16 Jun 2024 08:58:00 +0200 Subject: [PATCH 01/17] Make sure `SchedulePropagation` is never called inside a transaction because it calls an endpoint. Signed-off-by: Geoffrey Huck --- app/database/data_store.go | 5 +++++ app/database/db.go | 7 +++++++ app/service/propagation.go | 20 ++++++++------------ app/service/propagation_integration_test.go | 14 ++++++++++++++ 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/app/database/data_store.go b/app/database/data_store.go index 335f25546..b563e9817 100644 --- a/app/database/data_store.go +++ b/app/database/data_store.go @@ -327,3 +327,8 @@ func (s *DataStore) InsertOrUpdateMap(dataMap map[string]interface{}, updateColu func (s *DataStore) InsertOrUpdateMaps(dataMap []map[string]interface{}, updateColumns []string) error { return s.DB.insertOrUpdateMaps(s.tableName, dataMap, updateColumns) } + +// MustNotBeInTransaction panics if the store is in a transaction. +func (s *DataStore) MustNotBeInTransaction() { + s.DB.mustNotBeInTransaction() +} diff --git a/app/database/db.go b/app/database/db.go index 7055cd303..f0ec3aac3 100644 --- a/app/database/db.go +++ b/app/database/db.go @@ -639,6 +639,13 @@ func (conn *DB) withForeignKeyChecksDisabled(blockFunc func(*DB) error) (err err //} } +// mustNotBeInTransaction panics if the current connection is in a transaction. +func (conn *DB) mustNotBeInTransaction() { + if conn.isInTransaction() { + panic("should not be executed in a transaction") + } +} + func mustNotBeError(err error) { if err != nil { panic(err) diff --git a/app/service/propagation.go b/app/service/propagation.go index a80b2580e..cfc794e24 100644 --- a/app/service/propagation.go +++ b/app/service/propagation.go @@ -14,6 +14,9 @@ const PropagationEndpointTimeout = 3 * time.Second // SchedulePropagation schedules asynchronous propagation of the given types. // If endpoint is an empty string, it will be done synchronously. func SchedulePropagation(store *database.DataStore, endpoint string, types []string) { + // Must not be called in a transaction because it calls an endpoint, which can take a long time. + store.MustNotBeInTransaction() + endpointFailed := false if endpoint != "" { // Async. @@ -43,20 +46,13 @@ func SchedulePropagation(store *database.DataStore, endpoint string, types []str } if endpoint == "" || endpointFailed { - // Sync. - if store.IsInTransaction() { + err := store.InTransaction(func(store *database.DataStore) error { store.ScheduleItemsAncestorsPropagation() store.SchedulePermissionsPropagation() store.ScheduleResultsPropagation() - } else { - err := store.InTransaction(func(store *database.DataStore) error { - store.ScheduleItemsAncestorsPropagation() - store.SchedulePermissionsPropagation() - store.ScheduleResultsPropagation() - - return nil - }) - MustNotBeError(err) - } + + return nil + }) + MustNotBeError(err) } } diff --git a/app/service/propagation_integration_test.go b/app/service/propagation_integration_test.go index 08782c92a..f40056437 100644 --- a/app/service/propagation_integration_test.go +++ b/app/service/propagation_integration_test.go @@ -122,3 +122,17 @@ func TestSchedulePropagation(t *testing.T) { }) } } + +func TestSchedulePropagation_ShouldPanicWhenCalledInsideTransaction(t *testing.T) { + db := testhelpers.SetupDBWithFixtureString("") + defer func() { _ = db.Close() }() + store := database.NewDataStore(db) + + assert.Panics(t, func() { + err := store.InTransaction(func(store *database.DataStore) error { + service.SchedulePropagation(store, "", []string{"permissions"}) + return nil + }) + assert.NoError(t, err) + }) +} From acd5bf047281d171b9d5c68e382089dc3d281055 Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Fri, 5 Jul 2024 15:24:41 +0200 Subject: [PATCH 02/17] Inject the endpoint in the request context and allow to access the context in the DataStore. We need the endpoint to call the async propagation. And it has to be from DataStore because we want to specifically do it after the current transaction. Signed-off-by: Geoffrey Huck --- app/app.go | 10 ++++++++++ app/database/db.go | 5 +++++ 2 files changed, 15 insertions(+) diff --git a/app/app.go b/app/app.go index 8824843e1..16d6fdbf9 100644 --- a/app/app.go +++ b/app/app.go @@ -2,9 +2,11 @@ package app import ( + "context" crand "crypto/rand" "encoding/binary" "fmt" + "net/http" "github.com/go-chi/chi" "github.com/go-chi/chi/middleware" @@ -97,6 +99,14 @@ func (app *Application) Reset(config *viper.Viper) error { router.Use(corsConfig().Handler) // no need for CORS if served through the same domain router.Use(domain.Middleware(domainsConfig, serverConfig.GetString("domainOverride"))) + router.Use(func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), "propagation_endpoint", serverConfig.GetString("propagation_endpoint")) + + next.ServeHTTP(w, r.WithContext(ctx)) + }) + }) + if appenv.IsEnvDev() { router.Mount("/debug", middleware.Profiler()) } diff --git a/app/database/db.go b/app/database/db.go index f0ec3aac3..8ce9b6d68 100644 --- a/app/database/db.go +++ b/app/database/db.go @@ -88,6 +88,11 @@ func (conn *DB) New() *DB { return newDB(conn.ctx, conn.db.New()) } +// Context returns the context of the current db connection. +func (conn *DB) Context() context.Context { + return conn.ctx +} + func (conn *DB) inTransaction(txFunc func(*DB) error) (err error) { return conn.inTransactionWithCount(txFunc, 0) } From 2b87da48a4f8228976d785b468a4688c7359a62f Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Fri, 5 Jul 2024 15:25:22 +0200 Subject: [PATCH 03/17] This constant is only used in this file, so it doesn't have to be public. Signed-off-by: Geoffrey Huck --- app/{service => database}/propagation.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename app/{service => database}/propagation.go (94%) diff --git a/app/service/propagation.go b/app/database/propagation.go similarity index 94% rename from app/service/propagation.go rename to app/database/propagation.go index cfc794e24..317cf9ba8 100644 --- a/app/service/propagation.go +++ b/app/database/propagation.go @@ -9,7 +9,7 @@ import ( "github.com/France-ioi/AlgoreaBackend/app/logging" ) -const PropagationEndpointTimeout = 3 * time.Second +const endpointTimeout = 3 * time.Second // SchedulePropagation schedules asynchronous propagation of the given types. // If endpoint is an empty string, it will be done synchronously. @@ -21,7 +21,7 @@ func SchedulePropagation(store *database.DataStore, endpoint string, types []str if endpoint != "" { // Async. client := http.Client{ - Timeout: PropagationEndpointTimeout, + Timeout: endpointTimeout, } callTime := time.Now() From faf2c9f12df1f5647ccc43cc114159969eec5975 Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Fri, 5 Jul 2024 15:28:34 +0200 Subject: [PATCH 04/17] Rename the function into `StartAsyncPropagation`, it's clearer what it does. Move it in the `database` package. Because otherwise we have a cyclic dependence error when calling it from the `database` package (database -> service -> auth -> database). This might be solvable in another way though, but it would require refactoring. Signed-off-by: Geoffrey Huck --- app/database/propagation.go | 11 +++++------ .../propagation_integration_test.go | 14 +++++++------- 2 files changed, 12 insertions(+), 13 deletions(-) rename app/{service => database}/propagation_integration_test.go (90%) diff --git a/app/database/propagation.go b/app/database/propagation.go index 317cf9ba8..e37d222e1 100644 --- a/app/database/propagation.go +++ b/app/database/propagation.go @@ -1,19 +1,18 @@ -package service +package database import ( "net/http" "strings" "time" - "github.com/France-ioi/AlgoreaBackend/app/database" "github.com/France-ioi/AlgoreaBackend/app/logging" ) const endpointTimeout = 3 * time.Second -// SchedulePropagation schedules asynchronous propagation of the given types. +// StartAsyncPropagation schedules asynchronous propagation of the given types. // If endpoint is an empty string, it will be done synchronously. -func SchedulePropagation(store *database.DataStore, endpoint string, types []string) { +func StartAsyncPropagation(store *DataStore, endpoint string, types []string) { // Must not be called in a transaction because it calls an endpoint, which can take a long time. store.MustNotBeInTransaction() @@ -46,13 +45,13 @@ func SchedulePropagation(store *database.DataStore, endpoint string, types []str } if endpoint == "" || endpointFailed { - err := store.InTransaction(func(store *database.DataStore) error { + err := store.InTransaction(func(store *DataStore) error { store.ScheduleItemsAncestorsPropagation() store.SchedulePermissionsPropagation() store.ScheduleResultsPropagation() return nil }) - MustNotBeError(err) + mustNotBeError(err) } } diff --git a/app/service/propagation_integration_test.go b/app/database/propagation_integration_test.go similarity index 90% rename from app/service/propagation_integration_test.go rename to app/database/propagation_integration_test.go index f40056437..8f181bf08 100644 --- a/app/service/propagation_integration_test.go +++ b/app/database/propagation_integration_test.go @@ -1,21 +1,21 @@ -package service_test +package database_test import ( "fmt" "net/http" "testing" + "github.com/France-ioi/AlgoreaBackend/app/database" + "github.com/stretchr/testify/assert" "github.com/thingful/httpmock" - "github.com/France-ioi/AlgoreaBackend/app/database" "github.com/France-ioi/AlgoreaBackend/app/logging" "github.com/France-ioi/AlgoreaBackend/app/loggingtest" - "github.com/France-ioi/AlgoreaBackend/app/service" "github.com/France-ioi/AlgoreaBackend/testhelpers" ) -func TestSchedulePropagation(t *testing.T) { +func TestStartAsyncPropagation(t *testing.T) { type args struct { endpoint string } @@ -103,7 +103,7 @@ func TestSchedulePropagation(t *testing.T) { } } - service.SchedulePropagation(store, tt.args.endpoint, []string{"permissions"}) + database.StartAsyncPropagation(store, tt.args.endpoint, []string{"permissions"}) exists, err := store.Permissions().Where("item_id = 1").HasRows() assert.NoError(t, err) @@ -123,14 +123,14 @@ func TestSchedulePropagation(t *testing.T) { } } -func TestSchedulePropagation_ShouldPanicWhenCalledInsideTransaction(t *testing.T) { +func TestStartAsyncPropagation_ShouldPanicWhenCalledInsideTransaction(t *testing.T) { db := testhelpers.SetupDBWithFixtureString("") defer func() { _ = db.Close() }() store := database.NewDataStore(db) assert.Panics(t, func() { err := store.InTransaction(func(store *database.DataStore) error { - service.SchedulePropagation(store, "", []string{"permissions"}) + database.StartAsyncPropagation(store, "", []string{"permissions"}) return nil }) assert.NoError(t, err) From 3e53ee01560fe6762eaf4a8a949af54da30a79a3 Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Fri, 5 Jul 2024 15:29:12 +0200 Subject: [PATCH 05/17] The propagation is now called after the current transaction. Update the calls in services. Signed-off-by: Geoffrey Huck --- app/api/items/create_item.go | 8 +------- app/api/items/start_result.go | 2 +- app/api/items/start_result_path.go | 2 +- app/api/items/update_item.go | 6 ++++-- app/database/data_store.go | 24 ++++++++++++++++++++---- 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/app/api/items/create_item.go b/app/api/items/create_item.go index 21b2d998d..d40423fed 100644 --- a/app/api/items/create_item.go +++ b/app/api/items/create_item.go @@ -221,7 +221,6 @@ func (srv *Service) createItem(w http.ResponseWriter, r *http.Request) service.A func validateAndInsertItem(srv *Service, r *http.Request) (itemID int64, apiError service.APIError, err error) { user := srv.GetUser(r) store := srv.GetStore(r) - propagationsToRun := []string{} err = store.InTransaction(func(store *database.DataStore) error { input := NewItemRequest{} formData := formdata.NewFormData(&input) @@ -269,15 +268,10 @@ func validateAndInsertItem(srv *Service, r *http.Request) (itemID int64, apiErro setNewItemAsRootActivityOrSkill(store, formData, &input, itemID) - propagationsToRun = append(propagationsToRun, "items_ancestors") + store.SchedulePropagation([]string{"items_ancestors", "permissions", "results"}) return nil }) - if err == nil { - propagationsToRun = append(propagationsToRun, "permissions", "results") - - service.SchedulePropagation(store, srv.GetPropagationEndpoint(), propagationsToRun) - } return itemID, apiError, err } diff --git a/app/api/items/start_result.go b/app/api/items/start_result.go index bcda46fcc..c0775dbd0 100644 --- a/app/api/items/start_result.go +++ b/app/api/items/start_result.go @@ -103,7 +103,7 @@ func (srv *Service) startResult(w http.ResponseWriter, r *http.Request) service. resultStore := store.Results() service.MustNotBeError(resultStore.MarkAsToBePropagated(participantID, attemptID, itemID, false)) - service.SchedulePropagation(store, srv.GetPropagationEndpoint(), []string{"results"}) + store.SchedulePropagation([]string{"results"}) } return nil }) diff --git a/app/api/items/start_result_path.go b/app/api/items/start_result_path.go index a10c7380b..346eaf8a4 100644 --- a/app/api/items/start_result_path.go +++ b/app/api/items/start_result_path.go @@ -123,7 +123,7 @@ func (srv *Service) startResultPath(w http.ResponseWriter, r *http.Request) serv service.MustNotBeError(resultStore.InsertOrUpdateMaps(rowsToInsert, []string{"started_at", "latest_activity_at"})) service.MustNotBeError(resultStore.InsertIgnoreMaps("results_propagate", rowsToInsertPropagate)) - service.SchedulePropagation(store, srv.GetPropagationEndpoint(), []string{"results"}) + store.SchedulePropagation([]string{"results"}) } return nil diff --git a/app/api/items/update_item.go b/app/api/items/update_item.go index fe7b2ad7b..d6df57e0c 100644 --- a/app/api/items/update_item.go +++ b/app/api/items/update_item.go @@ -195,6 +195,10 @@ func (srv *Service) updateItem(w http.ResponseWriter, r *http.Request) service.A childrenInfoMap, oldPropagationLevelsMap, ) + if err == nil { + store.SchedulePropagation(propagationsToRun) + } + return err }) @@ -203,8 +207,6 @@ func (srv *Service) updateItem(w http.ResponseWriter, r *http.Request) service.A } service.MustNotBeError(err) - service.SchedulePropagation(store, srv.GetPropagationEndpoint(), propagationsToRun) - // response service.MustNotBeError(render.Render(w, r, service.UpdateSuccess(nil))) return service.NoError diff --git a/app/database/data_store.go b/app/database/data_store.go index b563e9817..0580bfa5a 100644 --- a/app/database/data_store.go +++ b/app/database/data_store.go @@ -176,10 +176,11 @@ func (s *DataStore) NewID() int64 { } type awaitingTriggers struct { - ItemAncestors bool - GroupAncestors bool - Permissions bool - Results bool + ItemAncestors bool + GroupAncestors bool + Permissions bool + Results bool + SchedulePropagationTypes []string } type dbContextKey string @@ -202,6 +203,12 @@ func (s *DataStore) InTransaction(txFunc func(*DataStore) error) error { triggersToRun := s.ctx.Value(triggersContextKey).(*awaitingTriggers) + if len(triggersToRun.SchedulePropagationTypes) > 0 { + types := triggersToRun.SchedulePropagationTypes + triggersToRun.SchedulePropagationTypes = []string{} + + StartAsyncPropagation(s, s.Context().Value("propagation_endpoint").(string), types) + } if triggersToRun.GroupAncestors { triggersToRun.GroupAncestors = false s.createNewAncestors("groups", "group") @@ -222,6 +229,15 @@ func (s *DataStore) InTransaction(txFunc func(*DataStore) error) error { return err } +// SchedulePropagation schedules a run of the propagation for the given types after the transaction commit. +func (s *DataStore) SchedulePropagation(types []string) { + s.mustBeInTransaction() + + triggersToRun := s.DB.ctx.Value(triggersContextKey).(*awaitingTriggers) + // TODO: filter type for available types, and make sure we don't have duplicates, maybe even sort them + triggersToRun.SchedulePropagationTypes = append(triggersToRun.SchedulePropagationTypes, types...) +} + // ScheduleResultsPropagation schedules a run of ResultStore::propagate() after the transaction commit. func (s *DataStore) ScheduleResultsPropagation() { s.mustBeInTransaction() From 4ecc6bf9c3647044ceabb5dd727ffcff91241d1e Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Mon, 8 Jul 2024 13:52:56 +0200 Subject: [PATCH 06/17] `SchedulePropagation`: make sure the types of propagation are unique in the slice. It's not a requirement, it would work without it, but it feels better. Also, the endpoint is called with the types, so it avoids redundancy there too. Signed-off-by: Geoffrey Huck --- app/database/data_store.go | 4 +-- app/utils/string.go | 15 ++++++++++++ app/utils/string_test.go | 50 +++++++++++++++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/app/database/data_store.go b/app/database/data_store.go index 0580bfa5a..1696f379e 100644 --- a/app/database/data_store.go +++ b/app/database/data_store.go @@ -2,6 +2,7 @@ package database import ( "context" + "github.com/France-ioi/AlgoreaBackend/app/utils" "time" "github.com/France-ioi/AlgoreaBackend/app/rand" @@ -234,8 +235,7 @@ func (s *DataStore) SchedulePropagation(types []string) { s.mustBeInTransaction() triggersToRun := s.DB.ctx.Value(triggersContextKey).(*awaitingTriggers) - // TODO: filter type for available types, and make sure we don't have duplicates, maybe even sort them - triggersToRun.SchedulePropagationTypes = append(triggersToRun.SchedulePropagationTypes, types...) + triggersToRun.SchedulePropagationTypes = utils.UniqueStrings(append(triggersToRun.SchedulePropagationTypes, types...)) } // ScheduleResultsPropagation schedules a run of ResultStore::propagate() after the transaction commit. diff --git a/app/utils/string.go b/app/utils/string.go index 7cd027a0c..bbfce1119 100644 --- a/app/utils/string.go +++ b/app/utils/string.go @@ -13,3 +13,18 @@ func Capitalize(str string) string { return string(r) } + +// UniqueStrings returns a new slice containing only the unique strings from the input slice. +func UniqueStrings(slice []string) []string { + uniques := make(map[string]bool, len(slice)) + for _, s := range slice { + uniques[s] = true + } + + result := make([]string, 0, len(uniques)) + for s := range uniques { + result = append(result, s) + } + + return result +} diff --git a/app/utils/string_test.go b/app/utils/string_test.go index 9a0f9c00c..114b7b235 100644 --- a/app/utils/string_test.go +++ b/app/utils/string_test.go @@ -1,6 +1,8 @@ package utils -import "testing" +import ( + "testing" +) func TestCapitalize(t *testing.T) { tests := []struct { @@ -37,3 +39,49 @@ func TestCapitalize(t *testing.T) { }) } } + +func TestUniqueStrings(t *testing.T) { + tests := []struct { + name string + slice []string + want []string + }{ + { + name: "should return the same slice if all elements are unique", + slice: []string{"a", "b", "c"}, + want: []string{"a", "b", "c"}, + }, + { + name: "should return unique elements only", + slice: []string{"a", "b", "a", "c"}, + want: []string{"a", "b", "c"}, + }, + { + name: "should handle empty slices", + slice: []string{}, + want: []string{}, + }, + } + + equal := func(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if !Contains(b, a[i]) { + return false + } + } + return true + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := UniqueStrings(tt.slice) + + if !equal(got, tt.want) { + t.Errorf("UniqueStrings() = %v, want %v", got, tt.want) + } + }) + } +} From d4ecc20b6f9297da46ab9b5fd78b33062b6b677f Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Mon, 8 Jul 2024 14:14:38 +0200 Subject: [PATCH 07/17] Fix lint order of imports. Signed-off-by: Geoffrey Huck --- app/database/data_store.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/database/data_store.go b/app/database/data_store.go index 1696f379e..3abd9fbf1 100644 --- a/app/database/data_store.go +++ b/app/database/data_store.go @@ -2,9 +2,10 @@ package database import ( "context" - "github.com/France-ioi/AlgoreaBackend/app/utils" "time" + "github.com/France-ioi/AlgoreaBackend/app/utils" + "github.com/France-ioi/AlgoreaBackend/app/rand" ) From 469b28f68425d31c144f5a91409c5c1753420bbb Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Mon, 8 Jul 2024 14:16:39 +0200 Subject: [PATCH 08/17] Fix lint cyclomatic complexity: schedule propagation in `updateChildrenAndRunListeners`. It makes more sense because the work that needs a propagation is done there already. Signed-off-by: Geoffrey Huck --- app/api/items/update_item.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/app/api/items/update_item.go b/app/api/items/update_item.go index d6df57e0c..8e20efd13 100644 --- a/app/api/items/update_item.go +++ b/app/api/items/update_item.go @@ -123,8 +123,6 @@ func (srv *Service) updateItem(w http.ResponseWriter, r *http.Request) service.A input := updateItemRequest{} formData := formdata.NewFormData(&input) - var propagationsToRun []string - apiError := service.NoError err = store.InTransaction(func(store *database.DataStore) error { var itemInfo struct { @@ -187,7 +185,7 @@ func (srv *Service) updateItem(w http.ResponseWriter, r *http.Request) service.A return apiError.Error // rollback } - propagationsToRun, apiError, err = updateChildrenAndRunListeners( + apiError, err = updateChildrenAndRunListeners( formData, store, itemID, @@ -195,9 +193,6 @@ func (srv *Service) updateItem(w http.ResponseWriter, r *http.Request) service.A childrenInfoMap, oldPropagationLevelsMap, ) - if err == nil { - store.SchedulePropagation(propagationsToRun) - } return err }) @@ -242,7 +237,9 @@ func updateChildrenAndRunListeners( input *updateItemRequest, childrenPermissionMap map[int64]permissionAndType, oldPropagationLevelsMap map[int64]*itemsRelationData, -) (propagationsToRun []string, apiError service.APIError, err error) { +) (apiError service.APIError, err error) { + var propagationsToRun []string + if formData.IsSet("children") { err = store.ItemItems().WithItemsRelationsLock(func(lockedStore *database.DataStore) error { deleteStatement := lockedStore.ItemItems().DB. @@ -282,7 +279,9 @@ func updateChildrenAndRunListeners( propagationsToRun = append(propagationsToRun, "results") } - return propagationsToRun, apiError, err + store.SchedulePropagation(propagationsToRun) + + return apiError, err } // constructUpdateItemChildTypeNonSkillValidator constructs a validator for the Children field that checks From 354d2e5468ea7879e6d3536915cd582dbb2d4149 Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Mon, 8 Jul 2024 14:25:26 +0200 Subject: [PATCH 09/17] Update test: we have one more middleware to inject the endpoint configuration in the `request Context`. @see https://github.com/France-ioi/AlgoreaBackend/pull/1099 Signed-off-by: Geoffrey Huck --- app/app_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/app_test.go b/app/app_test.go index fd5fc833c..d701bc3ae 100644 --- a/app/app_test.go +++ b/app/app_test.go @@ -35,7 +35,7 @@ func TestNew_Success(t *testing.T) { assert.NotNil(app.Database) assert.NotNil(app.HTTPHandler) assert.NotNil(app.apiCtx) - assert.Len(app.HTTPHandler.Middlewares(), 8) + assert.Len(app.HTTPHandler.Middlewares(), 9) assert.True(len(app.HTTPHandler.Routes()) > 0) assert.Equal("/*", app.HTTPHandler.Routes()[0].Pattern) // test default val } @@ -46,7 +46,7 @@ func TestNew_SuccessNoCompress(t *testing.T) { _ = os.Setenv("ALGOREA_SERVER__COMPRESS", "false") defer func() { _ = os.Unsetenv("ALGOREA_SERVER__COMPRESS") }() app, _ := New() - assert.Len(app.HTTPHandler.Middlewares(), 7) + assert.Len(app.HTTPHandler.Middlewares(), 8) } func TestNew_NotDefaultRootPath(t *testing.T) { From dd140bf2e2ce51d194d0a61894e0913f3c9e60e6 Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Mon, 8 Jul 2024 14:36:04 +0200 Subject: [PATCH 10/17] Cleanup: function not used anymore. Signed-off-by: Geoffrey Huck --- app/database/data_store.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/database/data_store.go b/app/database/data_store.go index 3abd9fbf1..8c6b8703a 100644 --- a/app/database/data_store.go +++ b/app/database/data_store.go @@ -279,10 +279,6 @@ func (s *DataStore) WithForeignKeyChecksDisabled(blockFunc func(*DataStore) erro }) } -func (s *DataStore) IsInTransaction() bool { - return s.DB.isInTransaction() -} - // WithNamedLock wraps the given function in GET_LOCK/RELEASE_LOCK. func (s *DataStore) WithNamedLock(lockName string, timeout time.Duration, txFunc func(*DataStore) error) error { return s.withNamedLock(lockName, timeout, func(db *DB) error { From f74233d97ba38b4d27fd3208573a537c710818eb Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Mon, 8 Jul 2024 16:58:36 +0200 Subject: [PATCH 11/17] All the propagations are now async. Signed-off-by: Geoffrey Huck --- app/api/contests/set_additional_time.go | 2 +- app/api/groups/update_permissions.go | 4 +- app/api/items/apply_dependency.go | 4 +- app/api/items/create_attempt.go | 2 +- app/api/items/enter.go | 2 +- .../items/path_from_root_integration_test.go | 6 +-- app/database/badges.go | 2 +- app/database/group_group_store.go | 6 +-- .../group_group_store_integration_test.go | 2 +- app/database/group_group_store_transitions.go | 4 +- ...m_item_store_ancestors_integration_test.go | 6 +-- app/database/item_store.go | 6 +-- ..._all_access_aggregates_integration_test.go | 42 +++++++++---------- ...ore_compute_all_access_integration_test.go | 2 +- app/database/result_store.go | 2 +- app/database/result_store_integration_test.go | 4 +- app/database/result_store_propagate.go | 4 +- ...e_propagate_aggregates_integration_test.go | 8 ++-- ..._propagate_creates_new_integration_test.go | 2 +- ...propagate_cyclic_graph_integration_test.go | 2 +- ...tore_propagate_unlocks_integration_test.go | 8 ++-- ...propagate_validated_at_integration_test.go | 10 ++--- ...re_propagate_validated_integration_test.go | 2 +- app/database/user_store_integration_test.go | 2 +- cmd/db_migrate.go | 6 +-- 25 files changed, 70 insertions(+), 70 deletions(-) diff --git a/app/api/contests/set_additional_time.go b/app/api/contests/set_additional_time.go index 53cbd77e6..c04db6187 100644 --- a/app/api/contests/set_additional_time.go +++ b/app/api/contests/set_additional_time.go @@ -231,6 +231,6 @@ func setAdditionalTimeForGroupInContest( service.MustNotBeError(store.Exec("DROP TEMPORARY TABLE new_expires_at").Error()) if groupsGroupsModified { store.ScheduleGroupsAncestorsPropagation() - store.ScheduleResultsPropagation() + store.SchedulePropagation([]string{"results"}) } } diff --git a/app/api/groups/update_permissions.go b/app/api/groups/update_permissions.go index fe03dc306..81ff37d79 100644 --- a/app/api/groups/update_permissions.go +++ b/app/api/groups/update_permissions.go @@ -728,10 +728,10 @@ func savePermissionsIntoDB(groupID, itemID, sourceGroupID int64, dbMap map[strin permissionGrantedStore := s.PermissionsGranted() service.MustNotBeError(permissionGrantedStore.InsertOrUpdateMap(dbMap, columnsToUpdate)) - s.SchedulePermissionsPropagation() + s.SchedulePropagation([]string{"permissions"}) if dbMap["can_view"] != nil && dbMap["can_view"] != none || dbMap["is_owner"] != nil && dbMap["is_owner"].(bool) { // permissionGrantedStore.After() implicitly (via triggers) marks some attempts as to_be_propagated // when an item becomes visible, so we should propagate attempts here - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) } } diff --git a/app/api/items/apply_dependency.go b/app/api/items/apply_dependency.go index 2ce4f1a64..4e4fad2cc 100644 --- a/app/api/items/apply_dependency.go +++ b/app/api/items/apply_dependency.go @@ -114,10 +114,10 @@ func (srv *Service) applyDependency(rw http.ResponseWriter, httpReq *http.Reques // If items have been unlocked, need to recompute access if groupsUnlocked > 0 { // generate permissions_generated from permissions_granted - store.SchedulePermissionsPropagation() + store.SchedulePropagation([]string{"permissions"}) // we should compute attempts again as new permissions were set and // triggers on permissions_generated likely marked some attempts as 'to_be_propagated' - store.ScheduleResultsPropagation() + store.SchedulePropagation([]string{"results"}) } return err }) diff --git a/app/api/items/create_attempt.go b/app/api/items/create_attempt.go index 817ab8ce1..8ed507277 100644 --- a/app/api/items/create_attempt.go +++ b/app/api/items/create_attempt.go @@ -101,7 +101,7 @@ func (srv *Service) createAttempt(w http.ResponseWriter, r *http.Request) servic attemptID, err = store.Attempts().CreateNew(participantID, parentAttemptID, itemID, user.GroupID) service.MustNotBeError(err) - store.ScheduleResultsPropagation() + store.SchedulePropagation([]string{"results"}) return nil }) if apiError != service.NoError { diff --git a/app/api/items/enter.go b/app/api/items/enter.go index 4d970cafc..a1ae1e5ee 100644 --- a/app/api/items/enter.go +++ b/app/api/items/enter.go @@ -144,7 +144,7 @@ func (srv *Service) enter(w http.ResponseWriter, r *http.Request) service.APIErr store.ScheduleGroupsAncestorsPropagation() // Upserting into groups_groups may mark some attempts as 'to_be_propagated', // so we need to recompute them - store.ScheduleResultsPropagation() + store.SchedulePropagation([]string{"results"}) } else { logging.GetLogEntry(r).Warnf("items.participants_group_id is not set for the item with id = %d", entryState.itemID) } diff --git a/app/api/items/path_from_root_integration_test.go b/app/api/items/path_from_root_integration_test.go index 90bec31d0..0d8f81f18 100644 --- a/app/api/items/path_from_root_integration_test.go +++ b/app/api/items/path_from_root_integration_test.go @@ -817,9 +817,9 @@ func Test_FindItemPath(t *testing.T) { var got []items.ItemPath assert.NoError(t, store.InTransaction(func(s *database.DataStore) error { s.ScheduleGroupsAncestorsPropagation() - s.ScheduleItemsAncestorsPropagation() - s.SchedulePermissionsPropagation() - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"items_ancestors"}) + s.SchedulePropagation([]string{"permissions"}) + s.SchedulePropagation([]string{"results"}) return nil })) diff --git a/app/database/badges.go b/app/database/badges.go index c54f86056..af60a4108 100644 --- a/app/database/badges.go +++ b/app/database/badges.go @@ -53,7 +53,7 @@ func (s *GroupStore) StoreBadges(badges []Badge, userID int64, newUser bool) (er if ancestorsCalculationNeeded { s.ScheduleGroupsAncestorsPropagation() - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) } if len(managedBadgeGroups) > 0 { diff --git a/app/database/group_group_store.go b/app/database/group_group_store.go index aa19c5f27..ed2bc79e8 100644 --- a/app/database/group_group_store.go +++ b/app/database/group_group_store.go @@ -60,7 +60,7 @@ func (s *GroupGroupStore) CreateRelation(parentGroupID, childGroupID int64) (err // and it might not work if we call CreateRelation again in this same transaction. s.createNewAncestorsInsideTransaction("groups", "group") - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil })) return err @@ -119,7 +119,7 @@ func (s *GroupGroupStore) DeleteRelation(parentGroupID, childGroupID int64, shou s.ScheduleGroupsAncestorsPropagation() if shouldPropagatePermissions { - s.SchedulePermissionsPropagation() + s.SchedulePropagation([]string{"permissions"}) } } @@ -183,7 +183,7 @@ func (s *GroupGroupStore) deleteGroupAndOrphanedDescendants(groupID int64) { // cascading deletes from many tables including groups_ancestors, groups_propagate, permission_granted & permissions_generated mustNotBeError(s.Groups().Delete("id IN (?)", idsToDelete).Error()) - s.SchedulePermissionsPropagation() + s.SchedulePropagation([]string{"permissions"}) } func (s *GroupGroupStore) deleteObjectsLinkedToGroups(groupIDs []int64) *DB { diff --git a/app/database/group_group_store_integration_test.go b/app/database/group_group_store_integration_test.go index 885ce96c7..e71926a7b 100644 --- a/app/database/group_group_store_integration_test.go +++ b/app/database/group_group_store_integration_test.go @@ -168,7 +168,7 @@ func TestGroupGroupStore_DeleteRelation(t *testing.T) { assert.NoError(t, dataStore.Table("groups_propagate").UpdateColumn("ancestors_computation_state", "done").Error()) assert.NoError(t, dataStore.InTransaction(func(s *database.DataStore) error { - s.SchedulePermissionsPropagation() + s.SchedulePropagation([]string{"permissions"}) return nil })) err := dataStore.InTransaction(func(s *database.DataStore) error { diff --git a/app/database/group_group_store_transitions.go b/app/database/group_group_store_transitions.go index 084c64b30..ffb19153e 100644 --- a/app/database/group_group_store_transitions.go +++ b/app/database/group_group_store_transitions.go @@ -455,9 +455,9 @@ func (s *GroupGroupStore) Transition(action GroupGroupTransitionAction, if shouldCreateNewAncestors { dataStore.ScheduleGroupsAncestorsPropagation() if shouldPropagatePermissions { - dataStore.SchedulePermissionsPropagation() + dataStore.SchedulePropagation([]string{"permissions"}) } - dataStore.ScheduleResultsPropagation() + dataStore.SchedulePropagation([]string{"results"}) } return nil })) diff --git a/app/database/item_item_store_ancestors_integration_test.go b/app/database/item_item_store_ancestors_integration_test.go index 3613027b1..0edd109f8 100644 --- a/app/database/item_item_store_ancestors_integration_test.go +++ b/app/database/item_item_store_ancestors_integration_test.go @@ -29,7 +29,7 @@ func TestItemItemStore_CreateNewAncestors_Concurrent(t *testing.T) { testhelpers.RunConcurrently(func() { dataStore := database.NewDataStoreWithContext(context.Background(), db) assert.NoError(t, dataStore.InTransaction(func(ds *database.DataStore) error { - ds.ScheduleItemsAncestorsPropagation() + ds.SchedulePropagation([]string{"items_ancestors"}) return nil })) }, 30) @@ -63,7 +63,7 @@ func TestItemItemStore_CreateNewAncestors_Cyclic(t *testing.T) { itemItemStore := database.NewDataStore(db).ItemItems() assert.NoError(t, itemItemStore.InTransaction(func(ds *database.DataStore) error { - ds.ScheduleItemsAncestorsPropagation() + ds.SchedulePropagation([]string{"items_ancestors"}) return nil })) @@ -98,7 +98,7 @@ func TestItemItemStore_CreateNewAncestors_IgnoresDoneItems(t *testing.T) { } assert.NoError(t, itemItemStore.InTransaction(func(ds *database.DataStore) error { - ds.ScheduleItemsAncestorsPropagation() + ds.SchedulePropagation([]string{"items_ancestors"}) return nil })) diff --git a/app/database/item_store.go b/app/database/item_store.go index 20a14a774..6b543aa54 100644 --- a/app/database/item_store.go +++ b/app/database/item_store.go @@ -318,9 +318,9 @@ func (s *ItemStore) DeleteItem(itemID int64) (err error) { })) mustNotBeError(s.Items().ByID(itemID).Delete().Error()) - s.ScheduleItemsAncestorsPropagation() - s.SchedulePermissionsPropagation() - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"items_ancestors"}) + s.SchedulePropagation([]string{"permissions"}) + s.SchedulePropagation([]string{"results"}) return nil }) diff --git a/app/database/permission_granted_store_compute_all_access_aggregates_integration_test.go b/app/database/permission_granted_store_compute_all_access_aggregates_integration_test.go index 953966b40..7b97933fc 100644 --- a/app/database/permission_granted_store_compute_all_access_aggregates_integration_test.go +++ b/app/database/permission_granted_store_compute_all_access_aggregates_integration_test.go @@ -43,7 +43,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_AggregatesContentAccess(t *test assert.NoError(t, permissionGrantedStore.Where("group_id=2 AND item_id=11"). UpdateColumn("can_view", "content").Error()) assert.NoError(t, permissionGrantedStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) @@ -116,7 +116,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_AggregatesContentAccessAsInfo(t "content_view_propagation": "as_info", }).Error()) assert.NoError(t, permissionGrantedStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) @@ -189,7 +189,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_AggregatesAccess(t *testing.T) assert.NoError(t, permissionGrantedStore.Where("group_id=2 AND item_id=11"). UpdateColumn("can_view", access).Error()) assert.NoError(t, permissionGrantedStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) @@ -334,7 +334,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_PropagatesCanView(t *testing.T) permissions_granted: [{group_id: 1, item_id: 1, source_group_id: 1, can_view: ` + testcase.canView + `}]`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -363,7 +363,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_PropagatesMaxOfParentsCanView(t - {group_id: 1, item_id: 2, source_group_id: 1, can_view: content_with_descendants}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -385,7 +385,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_PropagatesMaxOfParentsAndGrante - {group_id: 2, item_id: 2, source_group_id: 1, can_view: solution}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -403,7 +403,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_AggregatesMaxOfGrantedCanView(t - {group_id: 1, item_id: 1, source_group_id: 1, can_view: content_with_descendants}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -421,7 +421,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_AggregatesCanViewAsSolutionForO - {group_id: 1, item_id: 1, source_group_id: 1, can_view: content_with_descendants, is_owner: 1}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -450,7 +450,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_PropagatesMaxOfParentsCanGrantV - {group_id: 1, item_id: 3, source_group_id: 1, can_grant_view: solution_with_grant}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -471,7 +471,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_PropagatesMaxOfParentsAndGrante - {group_id: 2, item_id: 2, source_group_id: 1, can_grant_view: solution}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -489,7 +489,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_AggregatesMaxOfGrantedCanGrantV - {group_id: 1, item_id: 1, source_group_id: 1, origin: group_membership, can_grant_view: content_with_descendants}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -508,7 +508,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_AggregatesCanGrantViewAsSolutio - {group_id: 3, item_id: 2, source_group_id: 3, can_grant_view: none, is_owner: 1}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -541,7 +541,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_PropagatesMaxOfParentsCanWatch( - {group_id: 1, item_id: 3, source_group_id: 1, can_watch: answer_with_grant}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -562,7 +562,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_PropagatesMaxOfParentsAndGrante - {group_id: 2, item_id: 2, source_group_id: 1, can_watch: answer}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -580,7 +580,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_AggregatesMaxOfGrantedCanWatch( - {group_id: 1, item_id: 1, source_group_id: 1, can_watch: answer}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -598,7 +598,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_AggregatesCanWatchAsAnswerWithG - {group_id: 1, item_id: 1, source_group_id: 1, can_watch: answer, is_owner: 1}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -627,7 +627,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_PropagatesMaxOfParentsCanEdit(t - {group_id: 1, item_id: 3, source_group_id: 1, can_edit: all_with_grant}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -648,7 +648,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_PropagatesMaxOfParentsAndGrante - {group_id: 2, item_id: 2, source_group_id: 1, can_edit: all}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -666,7 +666,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_AggregatesMaxOfGrantedCanEdit(t - {group_id: 1, item_id: 1, source_group_id: 1, can_edit: all}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -684,7 +684,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_AggregatesCanEditAsAllWithGrant - {group_id: 1, item_id: 1, source_group_id: 1, can_edit: all, is_owner: 1}`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string @@ -768,7 +768,7 @@ func testPropagates(t *testing.T, column, propagationColumn, valueForParent stri permissions_granted: [{group_id: 1, item_id: 1, source_group_id: 1, ` + column + `: ` + valueForParent + `}]`) permissionStore := database.NewDataStore(db).Permissions() assert.NoError(t, permissionStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) var result string diff --git a/app/database/permission_granted_store_compute_all_access_integration_test.go b/app/database/permission_granted_store_compute_all_access_integration_test.go index 628a795f5..fffb75d7f 100644 --- a/app/database/permission_granted_store_compute_all_access_integration_test.go +++ b/app/database/permission_granted_store_compute_all_access_integration_test.go @@ -25,7 +25,7 @@ func TestPermissionGrantedStore_ComputeAllAccess_Concurrency(t *testing.T) { testhelpers.RunConcurrently(func() { dataStore := database.NewDataStoreWithContext(context.Background(), db) assert.NoError(t, dataStore.InTransaction(func(ds *database.DataStore) error { - ds.SchedulePermissionsPropagation() + ds.SchedulePropagation([]string{"permissions"}) return nil })) }, 30) diff --git a/app/database/result_store.go b/app/database/result_store.go index c4fa4176d..894623fa1 100644 --- a/app/database/result_store.go +++ b/app/database/result_store.go @@ -39,7 +39,7 @@ func (s *ResultStore) MarkAsToBePropagated(participantID, attemptID, itemID int6 INSERT IGNORE INTO results_propagate (participant_id, attempt_id, item_id, state) VALUES(?, ?, ?, 'to_be_propagated')`, participantID, attemptID, itemID).Error() if err == nil && propagateNow { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) } return err } diff --git a/app/database/result_store_integration_test.go b/app/database/result_store_integration_test.go index d39a1015a..7f738515f 100644 --- a/app/database/result_store_integration_test.go +++ b/app/database/result_store_integration_test.go @@ -75,7 +75,7 @@ func TestResultStore_Propagate(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { err := database.NewDataStore(db).InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) if (err != nil) != tt.wantErr { @@ -94,7 +94,7 @@ func TestResultStore_Propagate(t *testing.T) { // testhelpers.RunConcurrently(func() { // s := database.NewDataStoreWithContext(context.Background(), db) // err := s.InTransaction(func(st *database.DataStore) error { -// st.ScheduleResultsPropagation() +// st.SchedulePropagation([]string{"results"}) // return nil // }) // assert.NoError(t, err) diff --git a/app/database/result_store_propagate.go b/app/database/result_store_propagate.go index ab2ed3c49..2cbf0cafc 100644 --- a/app/database/result_store_propagate.go +++ b/app/database/result_store_propagate.go @@ -350,10 +350,10 @@ func (s *ResultStore) propagate() (err error) { if groupsUnlocked > 0 { mustNotBeError(s.InTransaction(func(s *DataStore) error { // generate permissions_generated from permissions_granted - s.SchedulePermissionsPropagation() + s.SchedulePropagation([]string{"permissions"}) // we should compute attempts again as new permissions were set and // triggers on permissions_generated likely marked some attempts as 'to_be_propagated' - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil })) diff --git a/app/database/result_store_propagate_aggregates_integration_test.go b/app/database/result_store_propagate_aggregates_integration_test.go index 80ec9a17e..d62a0a266 100644 --- a/app/database/result_store_propagate_aggregates_integration_test.go +++ b/app/database/result_store_propagate_aggregates_integration_test.go @@ -69,7 +69,7 @@ func TestResultStore_Propagate_Aggregates(t *testing.T) { err := resultStore.InTransaction(func(s *database.DataStore) error { assert.NoError(t, resultStore.Exec( "INSERT IGNORE INTO results_propagate SELECT participant_id, attempt_id, item_id, 'to_be_propagated' AS state FROM results").Error()) - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) @@ -116,7 +116,7 @@ func TestResultStore_Propagate_Aggregates_OnCommonData(t *testing.T) { resultStore := database.NewDataStore(db).Results() err := resultStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) @@ -145,7 +145,7 @@ func TestResultStore_Propagate_Aggregates_KeepsLastActivityAtIfItIsGreater(t *te }).Error()) err := resultStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) @@ -189,7 +189,7 @@ func TestResultStore_Propagate_Aggregates_EditScore(t *testing.T) { }).Error()) err := resultStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) diff --git a/app/database/result_store_propagate_creates_new_integration_test.go b/app/database/result_store_propagate_creates_new_integration_test.go index c8bc3ccdc..c3a098707 100644 --- a/app/database/result_store_propagate_creates_new_integration_test.go +++ b/app/database/result_store_propagate_creates_new_integration_test.go @@ -75,7 +75,7 @@ func testResultStorePropagateCreatesNew(t *testing.T, testCase *resultStorePropa } resultStore := database.NewDataStore(db).Results() err := resultStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) diff --git a/app/database/result_store_propagate_cyclic_graph_integration_test.go b/app/database/result_store_propagate_cyclic_graph_integration_test.go index ae73aa799..691099fe2 100644 --- a/app/database/result_store_propagate_cyclic_graph_integration_test.go +++ b/app/database/result_store_propagate_cyclic_graph_integration_test.go @@ -25,7 +25,7 @@ func TestResultStore_Propagate_WithCyclicGraph(t *testing.T) { resultStore := database.NewDataStore(db).Results() err := resultStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) diff --git a/app/database/result_store_propagate_unlocks_integration_test.go b/app/database/result_store_propagate_unlocks_integration_test.go index 30bc85011..336ad535d 100644 --- a/app/database/result_store_propagate_unlocks_integration_test.go +++ b/app/database/result_store_propagate_unlocks_integration_test.go @@ -59,7 +59,7 @@ func TestResultStore_Propagate_Unlocks_KeepsOldGrants(t *testing.T) { prepareDependencies(db, t) dataStore := database.NewDataStore(db) err := dataStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) @@ -117,7 +117,7 @@ func TestResultStore_Propagate_Unlocks_ItemsRequiringExplicitEntry_EverythingHas prepareDependencies(db, t) dataStore := database.NewDataStore(db) err := dataStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) @@ -155,7 +155,7 @@ func testRegularUnlocks(db *database.DB, t *testing.T) { dataStore := database.NewDataStore(db) err := dataStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) @@ -208,7 +208,7 @@ func testExplicitEntryUnlocks(db *database.DB, t *testing.T) { prepareDependencies(db, t) dataStore := database.NewDataStore(db) err := dataStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + sSchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) diff --git a/app/database/result_store_propagate_validated_at_integration_test.go b/app/database/result_store_propagate_validated_at_integration_test.go index 025179e7a..f1796e385 100644 --- a/app/database/result_store_propagate_validated_at_integration_test.go +++ b/app/database/result_store_propagate_validated_at_integration_test.go @@ -60,7 +60,7 @@ func TestResultStore_Propagate_NonCategories_SetsValidatedAtToMaxOfChildrenValid UpdateColumn("validated_at", skippedDate).Error()) err := resultStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) @@ -92,7 +92,7 @@ func TestResultStore_Propagate_Categories_SetsValidatedAtToMaxOfValidatedAtsOfCh Error()) err := resultStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) @@ -125,7 +125,7 @@ func TestResultStore_Propagate_Categories_SetsValidatedAtToNull_IfSomeCategories UpdateColumn("category", "Validation").Error()) err := resultStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) @@ -163,7 +163,7 @@ func TestResultStore_Propagate_Categories_ValidatedAtShouldBeMaxOfChildrensWithC UpdateColumn("category", "Validation").Error()) err := resultStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) @@ -207,7 +207,7 @@ func TestResultStore_Propagate_Categories_SetsValidatedAtToMaxOfValidatedAtsOfCh }).Error()) err := resultStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) diff --git a/app/database/result_store_propagate_validated_integration_test.go b/app/database/result_store_propagate_validated_integration_test.go index 3e962c6a3..933561db9 100644 --- a/app/database/result_store_propagate_validated_integration_test.go +++ b/app/database/result_store_propagate_validated_integration_test.go @@ -43,7 +43,7 @@ func testResultStorePropagateValidated(t *testing.T, fixtures []string, } err := resultStore.InTransaction(func(s *database.DataStore) error { - s.ScheduleResultsPropagation() + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) diff --git a/app/database/user_store_integration_test.go b/app/database/user_store_integration_test.go index e1b02f600..c61a4468f 100644 --- a/app/database/user_store_integration_test.go +++ b/app/database/user_store_integration_test.go @@ -201,7 +201,7 @@ func setupDBForDeleteWithTrapsTests(t *testing.T, currentTime time.Time) *databa store := database.NewDataStore(db) assert.NoError(t, store.InTransaction(func(trStore *database.DataStore) error { trStore.ScheduleGroupsAncestorsPropagation() - trStore.SchedulePermissionsPropagation() + trStore.SchedulePropagation([]string{"permissions"}) return nil })) return db diff --git a/cmd/db_migrate.go b/cmd/db_migrate.go index 4fcf95dbd..abc2695a2 100644 --- a/cmd/db_migrate.go +++ b/cmd/db_migrate.go @@ -96,9 +96,9 @@ func recomputeDBCaches(gormDB *database.DB) error { return database.NewDataStore(gormDB).InTransaction(func(store *database.DataStore) error { fmt.Print("Schedule the propagations\n") store.ScheduleGroupsAncestorsPropagation() - store.ScheduleItemsAncestorsPropagation() - store.SchedulePermissionsPropagation() - store.ScheduleResultsPropagation() + store.SchedulePropagation([]string{"items_ancestors"}) + store.SchedulePropagation([]string{"permissions"}) + store.SchedulePropagation([]string{"results"}) return nil }) From 46b8a273541b4560687aad6b62bc06750aa138df Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Mon, 8 Jul 2024 17:06:39 +0200 Subject: [PATCH 12/17] Fix typo. Signed-off-by: Geoffrey Huck --- app/database/result_store_propagate_unlocks_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/database/result_store_propagate_unlocks_integration_test.go b/app/database/result_store_propagate_unlocks_integration_test.go index 336ad535d..20eddaa44 100644 --- a/app/database/result_store_propagate_unlocks_integration_test.go +++ b/app/database/result_store_propagate_unlocks_integration_test.go @@ -208,7 +208,7 @@ func testExplicitEntryUnlocks(db *database.DB, t *testing.T) { prepareDependencies(db, t) dataStore := database.NewDataStore(db) err := dataStore.InTransaction(func(s *database.DataStore) error { - sSchedulePropagation([]string{"results"}) + s.SchedulePropagation([]string{"results"}) return nil }) assert.NoError(t, err) From 3c34f0e5622c3a720250901b09ef25e7dcea4718 Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Mon, 8 Jul 2024 17:15:04 +0200 Subject: [PATCH 13/17] In the migrate command, we need to call the propagation now and not schedule it asynchronously. Because we expect the database to be propagated after the command ends. Signed-off-by: Geoffrey Huck --- cmd/db_migrate.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/db_migrate.go b/cmd/db_migrate.go index abc2695a2..4fcf95dbd 100644 --- a/cmd/db_migrate.go +++ b/cmd/db_migrate.go @@ -96,9 +96,9 @@ func recomputeDBCaches(gormDB *database.DB) error { return database.NewDataStore(gormDB).InTransaction(func(store *database.DataStore) error { fmt.Print("Schedule the propagations\n") store.ScheduleGroupsAncestorsPropagation() - store.SchedulePropagation([]string{"items_ancestors"}) - store.SchedulePropagation([]string{"permissions"}) - store.SchedulePropagation([]string{"results"}) + store.ScheduleItemsAncestorsPropagation() + store.SchedulePermissionsPropagation() + store.ScheduleResultsPropagation() return nil }) From 26264864594c0117de475e4c26691b24ca233141 Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Mon, 8 Jul 2024 17:37:21 +0200 Subject: [PATCH 14/17] In the unit tests, the propagation endpoint hasn't been injected. This is intended because in this situation, it shouldn't have a value. Signed-off-by: Geoffrey Huck --- app/database/data_store.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/database/data_store.go b/app/database/data_store.go index 8c6b8703a..eea290134 100644 --- a/app/database/data_store.go +++ b/app/database/data_store.go @@ -209,7 +209,11 @@ func (s *DataStore) InTransaction(txFunc func(*DataStore) error) error { types := triggersToRun.SchedulePropagationTypes triggersToRun.SchedulePropagationTypes = []string{} - StartAsyncPropagation(s, s.Context().Value("propagation_endpoint").(string), types) + propagationEndpoint := "" + if s.Context().Value("propagation_endpoint") != nil { + propagationEndpoint = s.Context().Value("propagation_endpoint").(string) + } + StartAsyncPropagation(s, propagationEndpoint, types) } if triggersToRun.GroupAncestors { triggersToRun.GroupAncestors = false From 460f9238bdb55e3b83abff214a9a45a4da62eb0e Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Tue, 9 Jul 2024 14:22:05 +0200 Subject: [PATCH 15/17] The group propagation, when scheduled, should be run before the async propagation. In case no endpoint is defined, like for the tests, it was run after, because in this case the propagation is run in sync. When that happens, some tests don't pass. Signed-off-by: Geoffrey Huck --- app/database/data_store.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/database/data_store.go b/app/database/data_store.go index eea290134..88127dfba 100644 --- a/app/database/data_store.go +++ b/app/database/data_store.go @@ -205,6 +205,10 @@ func (s *DataStore) InTransaction(txFunc func(*DataStore) error) error { triggersToRun := s.ctx.Value(triggersContextKey).(*awaitingTriggers) + if triggersToRun.GroupAncestors { + triggersToRun.GroupAncestors = false + s.createNewAncestors("groups", "group") + } if len(triggersToRun.SchedulePropagationTypes) > 0 { types := triggersToRun.SchedulePropagationTypes triggersToRun.SchedulePropagationTypes = []string{} @@ -215,10 +219,6 @@ func (s *DataStore) InTransaction(txFunc func(*DataStore) error) error { } StartAsyncPropagation(s, propagationEndpoint, types) } - if triggersToRun.GroupAncestors { - triggersToRun.GroupAncestors = false - s.createNewAncestors("groups", "group") - } if triggersToRun.ItemAncestors { triggersToRun.ItemAncestors = false s.createNewAncestors("items", "item") From bd079a088e1525b5eb727336be55612bd7ff0e9b Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Tue, 9 Jul 2024 14:42:20 +0200 Subject: [PATCH 16/17] Fix test. Since we run all propagations async, and the system does all propagations when one is done, there is no distinction between with and without results propagation anymore. @see https://github.com/France-ioi/AlgoreaBackend/pull/1100 Signed-off-by: Geoffrey Huck --- app/api/groups/update_permissions.feature | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/api/groups/update_permissions.feature b/app/api/groups/update_permissions.feature index f0ad0c72f..b3c815aff 100644 --- a/app/api/groups/update_permissions.feature +++ b/app/api/groups/update_permissions.feature @@ -54,7 +54,7 @@ Feature: Change item access rights for a group | attempt_id | participant_id | item_id | | 0 | 21 | 103 | - Scenario Outline: Create a new permissions_granted row (with results propagation) + Scenario Outline: Create a new permissions_granted row Given I am the user with id "21" And the database table 'permissions_generated' has also the following rows: | group_id | item_id | can_view_generated | can_grant_view_generated | can_watch_generated | can_edit_generated | is_owner_generated | @@ -99,7 +99,7 @@ Feature: Change item access rights for a group | {"can_view":"info","can_make_session_official":true} | info | none | none | none | false | true | info | none | none | none | none | none | none | none | | {"is_owner":true} | none | none | none | none | true | false | solution | solution_with_grant | answer_with_grant | all_with_grant | content | solution | answer | all | - Scenario Outline: Create a new permissions_granted row (without results propagation) + Scenario Outline: Create a new permissions_granted row (more) Given I am the user with id "21" And the database table 'permissions_generated' has also the following rows: | group_id | item_id | can_view_generated | can_grant_view_generated | can_watch_generated | can_edit_generated | is_owner_generated | @@ -129,9 +129,10 @@ Feature: Change item access rights for a group | 23 | 102 | none | none | none | none | false | | 23 | 103 | none | none | none | none | false | And the table "attempts" should stay unchanged - And the table "results_propagate" should be: - | attempt_id | participant_id | item_id | state | - | 0 | 21 | 103 | to_be_propagated | + And the table "results" should be: + | attempt_id | participant_id | item_id | + | 0 | 21 | 102 | + | 0 | 21 | 103 | Examples: | json | can_enter_from | can_enter_until | | {"can_enter_from":"2019-05-30T11:00:00Z"} | 2019-05-30 11:00:00 | 9999-12-31 23:59:59 | From 27d5a629af9ad40daeaf150ff77a64b87d63e428 Mon Sep 17 00:00:00 2001 From: Geoffrey Huck Date: Tue, 9 Jul 2024 14:56:46 +0200 Subject: [PATCH 17/17] Fix test. Since we run all propagations async, and the system does all propagations when one is done, the permissions propagation is always done (because the results propagation is always done). @see https://github.com/France-ioi/AlgoreaBackend/pull/1100 Signed-off-by: Geoffrey Huck --- ...roup_store_transitions_integration_test.go | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/app/database/group_group_store_transitions_integration_test.go b/app/database/group_group_store_transitions_integration_test.go index 8fa7f4a61..83d3a043f 100644 --- a/app/database/group_group_store_transitions_integration_test.go +++ b/app/database/group_group_store_transitions_integration_test.go @@ -268,8 +268,9 @@ func testTransitionAcceptingNoRelationAndAnyPendingRequest(name string, action d {GroupID: 20, MemberID: 6, Action: string(expectedGroupMembershipAction), At: currentTimePtr, InitiatorID: userIDPtr}, {GroupID: 20, MemberID: 7, Action: string(expectedGroupMembershipAction), At: currentTimePtr, InitiatorID: userIDPtr}, }, - wantGrantedPermissions: grantedPermissionsUnchanged, - shouldRunListeners: true, + wantGrantedPermissions: grantedPermissionsUnchanged, + wantGeneratedPermissions: generatedPermissionsUnchanged, + shouldRunListeners: true, } } @@ -328,8 +329,9 @@ func testTransitionAcceptingPendingRequest(name string, action database.GroupGro wantGroupMembershipChanges: []groupMembershipChange{ {GroupID: 20, MemberID: acceptedID, Action: string(expectedGroupMembershipAction), At: currentTimePtr, InitiatorID: userIDPtr}, }, - wantGrantedPermissions: grantedPermissionsUnchanged, - shouldRunListeners: true, + wantGrantedPermissions: grantedPermissionsUnchanged, + wantGeneratedPermissions: generatedPermissionsUnchanged, + shouldRunListeners: true, } } @@ -425,8 +427,9 @@ func TestGroupGroupStore_Transition(t *testing.T) { {GroupID: 20, MemberID: 6, Action: "invitation_created", InitiatorID: userIDPtr, At: currentTimePtr}, {GroupID: 20, MemberID: 7, Action: "invitation_created", InitiatorID: userIDPtr, At: currentTimePtr}, }, - wantGrantedPermissions: grantedPermissionsUnchanged, - shouldRunListeners: true, + wantGrantedPermissions: grantedPermissionsUnchanged, + wantGeneratedPermissions: generatedPermissionsUnchanged, + shouldRunListeners: true, }, { name: "AdminCreatesInvitation (max participants limit is not exceeded)", @@ -459,8 +462,9 @@ func TestGroupGroupStore_Transition(t *testing.T) { {GroupID: 20, MemberID: 6, Action: "invitation_created", InitiatorID: userIDPtr, At: currentTimePtr}, {GroupID: 20, MemberID: 7, Action: "invitation_created", InitiatorID: userIDPtr, At: currentTimePtr}, }, - wantGrantedPermissions: grantedPermissionsUnchanged, - shouldRunListeners: true, + wantGrantedPermissions: grantedPermissionsUnchanged, + wantGeneratedPermissions: generatedPermissionsUnchanged, + shouldRunListeners: true, }, { name: "AdminCreatesInvitation (enforce max participants)",