From 84eaeb066380f0a2faf6b3bc4ac0d2a075e13591 Mon Sep 17 00:00:00 2001 From: Pascal Fischer Date: Wed, 28 May 2025 09:53:25 +0200 Subject: [PATCH] simplify manager methods --- internal/modules/networks/interface.go | 11 ++-- internal/modules/networks/manager/api.go | 54 +++++++++++++++++ internal/modules/networks/manager/manager.go | 57 +++++++----------- .../modules/networks/manager/repository.go | 6 +- .../modules/networks/resources/interface.go | 1 + .../networks/resources/manager/manager.go | 58 +++++++++++++++++++ .../networks/resources/manager/repository.go | 48 ++++----------- .../modules/networks/routers/interface.go | 2 +- internal/shared/db/transaction.go | 52 +++++++++++------ 9 files changed, 191 insertions(+), 98 deletions(-) diff --git a/internal/modules/networks/interface.go b/internal/modules/networks/interface.go index aea8c34..f9cc69b 100644 --- a/internal/modules/networks/interface.go +++ b/internal/modules/networks/interface.go @@ -7,10 +7,9 @@ import ( ) type Manager interface { - Using(tx db.Transaction) Manager - GetAllNetworks(ctx context.Context, strength db.LockingStrength, accountID, userID string) ([]*Network, error) - GetNetwork(ctx context.Context, strength db.LockingStrength, accountID, userID, networkID string) (*Network, error) - CreateNetwork(ctx context.Context, userID string, network *Network) (*Network, error) - UpdateNetwork(ctx context.Context, userID string, network *Network) (*Network, error) - DeleteNetwork(ctx context.Context, accountID, userID, networkID string) error + GetAllNetworks(ctx context.Context, tx db.Transaction, strength db.LockingStrength, accountID, userID string) ([]*Network, error) + GetNetwork(ctx context.Context, tx db.Transaction, strength db.LockingStrength, accountID, userID, networkID string) (*Network, error) + CreateNetwork(ctx context.Context, tx db.Transaction, userID string, network *Network) (*Network, error) + UpdateNetwork(ctx context.Context, tx db.Transaction, userID string, network *Network) (*Network, error) + DeleteNetwork(ctx context.Context, tx db.Transaction, accountID, userID, networkID string) error } diff --git a/internal/modules/networks/manager/api.go b/internal/modules/networks/manager/api.go index 5d04392..f4bceac 100644 --- a/internal/modules/networks/manager/api.go +++ b/internal/modules/networks/manager/api.go @@ -1 +1,55 @@ package manager + +import ( + "net/http" + + "github.com/gorilla/mux" + nbcontext "github.com/netbirdio/netbird/management/server/context" + "github.com/netbirdio/netbird/management/server/http/util" + + "management/internal/modules/networks" + "management/internal/shared/errors" + "management/internal/shared/permissions" + "management/internal/shared/permissions/modules" + "management/internal/shared/permissions/operations" +) + +type handler struct { + manager networks.Manager + permissionsManager permissions.Manager +} + +func newHandler(manager networks.Manager, permissionsManager permissions.Manager) *handler { + return &handler{ + manager: manager, + permissionsManager: permissionsManager, + } +} + +func (h *handler) RegisterEndpoints(router *mux.Router) { + router.HandleFunc("/networks/{id}", h.deleteNetwork).Methods("DELETE", "OPTIONS") +} + +func (h *handler) deleteNetwork(w http.ResponseWriter, r *http.Request) { + userAuth, err := nbcontext.GetUserAuthFromContext(r.Context()) + if err != nil { + util.WriteError(r.Context(), err, w) + return + } + + allowed, err := h.permissionsManager.ValidateUserPermissions(r.Context(), userAuth.AccountId, userAuth.UserId, modules.Networks, operations.Write) + if err != nil { + util.WriteError(r.Context(), errors.NewPermissionValidationError(err), w) + return + } + if !allowed { + util.WriteError(r.Context(), errors.NewPermissionDeniedError(), w) + } + + err = h.manager.DeleteNetwork(r.Context(), nil, userAuth.AccountId, userAuth.UserId, mux.Vars(r)["id"]) + if err != nil { + http.Error(w, "Internal Server Error", http.StatusInternalServerError) + return + } + // _ = json.NewEncoder(w).Encode(users) +} diff --git a/internal/modules/networks/manager/manager.go b/internal/modules/networks/manager/manager.go index 7ea4d87..5a30cb9 100644 --- a/internal/modules/networks/manager/manager.go +++ b/internal/modules/networks/manager/manager.go @@ -4,7 +4,7 @@ import ( "context" "fmt" - "github.com/netbirdio/netbird/management/server/account" + "github.com/gorilla/mux" "github.com/rs/xid" "management/internal/modules/networks" @@ -24,11 +24,16 @@ type managerImpl struct { routersManager routers.Manager } -func NewManager(store *db.Store, permissionsManager permissions.Manager, resourceManager resources.Manager, routersManager routers.Manager) resources.Manager { +func NewManager(store *db.Store, router *mux.Router, permissionsManager permissions.Manager, resourceManager resources.Manager, routersManager routers.Manager) networks.Manager { repo := newRepository(store) - m := &managerImpl{repo: repo} - // api := newHandler(m, permissionsManager) - // api.RegisterEndpoints(router) + m := &managerImpl{ + repo: repo, + permissionsManager: permissionsManager, + resourcesManager: resourceManager, + routersManager: routersManager, + } + api := newHandler(m, permissionsManager) + api.RegisterEndpoints(router) return m } @@ -105,41 +110,19 @@ func (m *managerImpl) DeleteNetwork(ctx context.Context, tx db.Transaction, acco return errors.NewPermissionDeniedError() } - network, err := m.repo.GetNetworkByID(tx, db.LockingStrengthUpdate, accountID, networkID) - if err != nil { - return fmt.Errorf("failed to get network: %w", err) - } + return db.WithTx(m.repo.Store(), tx, func(tx db.Transaction) error { - resources, err := m.resourcesManager.GetNetworkResourcesByNetID(ctx, tx, db.LockingStrengthUpdate, accountID, userID, networkID) - if err != nil { - return fmt.Errorf("failed to get resources in network: %w", err) - } + sendEvent(tx, "network_deleted") - for _, resource := range resources { - err = m.resourcesManager.DeleteResource(ctx, tx, accountID, userID, networkID, resource.ID) - if err != nil { - return fmt.Errorf("failed to delete resource: %w", err) + if err := m.repo.DeleteNetwork(tx, &networks.Network{ID: networkID}); err != nil { + return fmt.Errorf("failed to delete network: %w", err) } - } - routers, err := m.routersManager.GetNetworkRoutersByNetID(ctx, tx, db.LockingStrengthUpdate, accountID, userID, networkID) - if err != nil { - return fmt.Errorf("failed to get routers in network: %w", err) - } + tx.AddEvent(func() { + addActivityEvent("Network deleted") + // noop + }) - for _, router := range routers { - err := m.routersManager.DeleteRouter(ctx, tx, accountID, userID, networkID, router.ID) - if err != nil { - return fmt.Errorf("failed to delete router: %w", err) - } - } - - err = m.repo.DeleteNetwork(tx, &networks.Network{ID: networkID}) - if err != nil { - return fmt.Errorf("failed to delete network: %w", err) - } - - // TODO: send delete event with network - - return nil + return nil + }) } diff --git a/internal/modules/networks/manager/repository.go b/internal/modules/networks/manager/repository.go index 5c89fd5..8e3e775 100644 --- a/internal/modules/networks/manager/repository.go +++ b/internal/modules/networks/manager/repository.go @@ -6,7 +6,7 @@ import ( ) type Repository interface { - RunInTx(fn func(tx db.Transaction) error) error + Store() *db.Store CreateNetwork(tx db.Transaction, network *networks.Network) error UpdateNetwork(tx db.Transaction, network *networks.Network) error DeleteNetwork(tx db.Transaction, network *networks.Network) error @@ -55,3 +55,7 @@ func (r *repository) GetAccountNetworks(tx db.Transaction, lockingStrength db.Lo } return networks, nil } + +func (r *repository) Store() *db.Store { + return r.store +} diff --git a/internal/modules/networks/resources/interface.go b/internal/modules/networks/resources/interface.go index cc5697e..59cc9f6 100644 --- a/internal/modules/networks/resources/interface.go +++ b/internal/modules/networks/resources/interface.go @@ -9,4 +9,5 @@ import ( type Manager interface { DeleteResource(ctx context.Context, tx db.Transaction, accountID, userID, networkID, resourceID string) error GetNetworkResourcesByNetID(ctx context.Context, tx db.Transaction, lockingStrength db.LockingStrength, accountID, userID, networkID string) ([]*NetworkResource, error) + DeleteResourcesInNetwork(ctx context.Context, tx db.Transaction, accountID, userID, networkID string) error } diff --git a/internal/modules/networks/resources/manager/manager.go b/internal/modules/networks/resources/manager/manager.go index 5d04392..ed0af4b 100644 --- a/internal/modules/networks/resources/manager/manager.go +++ b/internal/modules/networks/resources/manager/manager.go @@ -1 +1,59 @@ package manager + +import ( + "context" + + "github.com/gorilla/mux" + + "management/internal/modules/networks" + "management/internal/modules/networks/resources" + "management/internal/modules/networks/routers" + "management/internal/shared/db" + "management/internal/shared/errors" + "management/internal/shared/permissions" + "management/internal/shared/permissions/modules" + "management/internal/shared/permissions/operations" +) + +type managerImpl struct { + repo Repository + permissionsManager permissions.Manager + resourcesManager resources.Manager + routersManager routers.Manager +} + +func NewManager(store *db.Store, router *mux.Router, permissionsManager permissions.Manager, resourceManager resources.Manager, routersManager routers.Manager) resources.Manager { + repo := newRepository(store) + m := &managerImpl{ + repo: repo, + permissionsManager: permissionsManager, + resourcesManager: resourceManager, + routersManager: routersManager, + } + // api := newHandler(m, permissionsManager) + // api.RegisterEndpoints(router) + return m +} + +func (m *managerImpl) DeleteResourcesInNetwork(ctx context.Context, tx db.Transaction, accountID, userID, networkID string) error { + ok, err := m.permissionsManager.ValidateUserPermissions(ctx, accountID, userID, modules.Networks, operations.Read) + if err != nil { + return errors.NewPermissionValidationError(err) + } + if !ok { + return errors.NewPermissionDeniedError() + } + + resources, err := m.repo.GetResourcesByNetworkID(tx, db.LockingStrengthUpdate, networkID) + if err != nil { + return err + } + for _, resource := range resources { + err = m.repo.DeleteResource(tx, resource.ID) + if err != nil { + return err + } + } + + return nil +} diff --git a/internal/modules/networks/resources/manager/repository.go b/internal/modules/networks/resources/manager/repository.go index bdd0a54..befa8c8 100644 --- a/internal/modules/networks/resources/manager/repository.go +++ b/internal/modules/networks/resources/manager/repository.go @@ -1,17 +1,14 @@ package manager import ( - "management/internal/modules/networks" + "management/internal/modules/networks/resources" "management/internal/shared/db" ) type Repository interface { - RunInTx(fn func(tx db.Transaction) error) error - Using(tx db.Transaction) Repository - CreateNetwork(tx db.Transaction, network *networks.Network) error - UpdateNetwork(tx db.Transaction, network *networks.Network) error - GetNetworkByID(tx db.Transaction, lockingStrength db.LockingStrength, accountID, networkID string) (*networks.Network, error) - GetAccountNetworks(tx db.Transaction, lockingStrength db.LockingStrength, accountID string) ([]*networks.Network, error) + Store() *db.Store + GetResourcesByNetworkID(tx db.Transaction, strength db.LockingStrength, networkID string) ([]*resources.NetworkResource, error) + DeleteResource(tx db.Transaction, resourceID string) error } type repository struct { @@ -22,40 +19,19 @@ func newRepository(s *db.Store) Repository { return &repository{store: s} } -func (r *repository) RunInTx(fn func(tx db.Transaction) error) error { - return r.store.RunInTx(fn) +func (r *repository) Store() *db.Store { + return r.store } -func (r *repository) Using(tx db.Transaction) Repository { - return &repository{store: r.store.Using(tx)} -} - -func (r *repository) DeleteNetwork(tx db.Transaction, network *networks.Network) error { - return r.store.Create(tx, network) -} - -func (r *repository) UpdateNetwork(tx db.Transaction, network *networks.Network) error { - return r.store.Update(tx, network) -} - -func (r *repository) GetNetworkByID(tx db.Transaction, lockingStrength db.LockingStrength, accountID, networkID string) (*networks.Network, error) { - var network networks.Network - err := r.store.GetOne(tx, lockingStrength, &network, "account_id = ? AND network_id = ?", accountID, networkID) +func (r *repository) GetResourcesByNetworkID(tx db.Transaction, strength db.LockingStrength, networkID string) ([]*resources.NetworkResource, error) { + var resources []*resources.NetworkResource + err := r.store.GetMany(tx, strength, &resources, "network_id = ?", networkID) if err != nil { return nil, err } - return &network, nil + return resources, nil } -func (r *repository) GetAccountNetworks(tx db.Transaction, lockingStrength db.LockingStrength, accountID string) ([]*networks.Network, error) { - var networks []*networks.Network - err := r.store.GetMany(tx, lockingStrength, &networks, "account_id = ?", accountID) - if err != nil { - return nil, err - } - return networks, nil -} - -func (r *repository) CreateNetwork(tx db.Transaction, network *networks.Network) error { - return r.store.Create(tx, network) +func (r *repository) DeleteResource(tx db.Transaction, resourceID string) error { + return r.store.Delete(tx, &resources.NetworkResource{ID: resourceID}) } diff --git a/internal/modules/networks/routers/interface.go b/internal/modules/networks/routers/interface.go index 0d7487b..c8edf76 100644 --- a/internal/modules/networks/routers/interface.go +++ b/internal/modules/networks/routers/interface.go @@ -7,7 +7,7 @@ import ( ) type Manager interface { - Using(tx db.Transaction) Manager GetNetworkRoutersByNetID(ctx context.Context, tx db.Transaction, lockingStrength db.LockingStrength, accountID, userID, networkID string) ([]*NetworkRouter, error) DeleteRouter(ctx context.Context, tx db.Transaction, accountID, userID, networkID, routerID string) error + DeleteRoutersInNetwork(ctx context.Context, tx db.Transaction, accountID, userID, networkID string) error } diff --git a/internal/shared/db/transaction.go b/internal/shared/db/transaction.go index 9526ede..3e5bad1 100644 --- a/internal/shared/db/transaction.go +++ b/internal/shared/db/transaction.go @@ -1,6 +1,8 @@ package db -import "gorm.io/gorm" +import ( + "gorm.io/gorm" +) type LockingStrength string @@ -16,6 +18,11 @@ type Transaction interface { Commit() error Rollback() error AddEvent(event func()) + FlushEvents() +} + +type TransactionalManager[T any] interface { + UsingTx(tx Transaction) T } type storeTx struct { @@ -23,28 +30,39 @@ type storeTx struct { events []func() } -func (t *storeTx) Commit() error { - err := t.db.Commit().Error - if err != nil { - t.commitEvents() +func (tx *storeTx) Commit() error { + for _, e := range tx.events { + e() } - return err + return tx.db.Commit().Error } -func (t *storeTx) Rollback() error { - return t.db.Rollback().Error +func (tx *storeTx) Rollback() error { + return tx.db.Rollback().Error } -func (t *storeTx) AddEvent(event func()) { - if t.events == nil { - t.events = make([]func(), 0) - } - t.events = append(t.events, event) +func (tx *storeTx) AddEvent(fn func()) { + tx.events = append(tx.events, fn) } -func (t *storeTx) commitEvents() { - for _, event := range t.events { - event() +func (tx *storeTx) FlushEvents() { + for _, e := range tx.events { + e() } - t.events = nil + tx.events = nil +} + +func WithTx(store *Store, parentTx Transaction, fn func(tx Transaction) error) error { + if parentTx != nil { + return fn(parentTx) + } + + return store.RunInTx(func(tx Transaction) error { + if err := fn(tx); err != nil { + return err + } + + tx.FlushEvents() + return nil + }) }