Skip to content

Commit

Permalink
feat(breaking): removes WebhookContext.Tx() dal.Transaction - do not …
Browse files Browse the repository at this point in the history
…use transaction for basic operations
  • Loading branch information
trakhimenok committed Feb 17, 2025
1 parent bd56b39 commit 47e30e1
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 43 deletions.
4 changes: 2 additions & 2 deletions botsdal/dal_bot_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ func GetPlatformUser(
tx dal.ReadSession,
platformID, botUserID string,
platformUserData botsfwmodels.PlatformUserData,
) (botUser record.DataWithID[string, botsfwmodels.PlatformUserData], err error) {
) (botUser BotUser, err error) {
botUserKey := NewPlatformUserKey(platformID, botUserID)
botUser = record.NewDataWithID(botUserID, botUserKey, platformUserData)
botUser = BotUser(record.NewDataWithID(botUserID, botUserKey, platformUserData))
return botUser, tx.Get(ctx, botUser.Record)
}

Expand Down
3 changes: 1 addition & 2 deletions botsdal/dal_bot_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"github.com/bots-go-framework/bots-fw-store/botsfwmodels"
"github.com/dal-go/dalgo/dal"
"github.com/dal-go/dalgo/record"
"testing"
)

Expand All @@ -18,7 +17,7 @@ func TestGetBotUser(t *testing.T) {
name string
args args
shouldPanic bool
checkResult func(botUser record.DataWithID[string, botsfwmodels.PlatformUserData], err error)
checkResult func(botUser BotUser, err error)
}{
{name: "empty", shouldPanic: true},
}
Expand Down
2 changes: 1 addition & 1 deletion botsdal/facade_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type AppUserDal interface {
// CreateAppUserFromBotUser creates app user record using bot user data
CreateAppUserFromBotUser(ctx context.Context, tx dal.ReadwriteTransaction, bot Bot) (
appUser record.DataWithID[string, botsfwmodels.AppUserData],
botUser record.DataWithID[string, botsfwmodels.PlatformUserData],
botUser BotUser,
err error,
)

Expand Down
27 changes: 20 additions & 7 deletions botsfw/context_auth.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package botsfw

import (
"context"
"fmt"
"github.com/bots-go-framework/bots-fw-store/botsfwmodels"
"github.com/bots-go-framework/bots-fw/botsdal"
"github.com/dal-go/dalgo/dal"
"time"
)

Expand Down Expand Up @@ -39,21 +41,32 @@ func SetAccessGranted(whc WebhookContext, value bool) (err error) {
botUserID := whc.Input().GetSender().GetID()
botUserStrID := fmt.Sprintf("%v", botUserID)
log.Debugf(c, "SetAccessGranted(): whc.GetSender().GetID() = %v", botUserID)
tx := whc.Tx()
db := whc.DB()
platformID := whc.BotPlatform().ID()
botSettings := whc.BotContext().BotSettings

if botUser, err := botsdal.GetPlatformUser(c, tx, platformID, botUserStrID, botSettings.Profile.NewPlatformUserData()); err != nil {
var botUser botsdal.BotUser
if botUser, err = botsdal.GetPlatformUser(c, db, platformID, botUserStrID, botSettings.Profile.NewPlatformUserData()); err != nil {
return fmt.Errorf("failed to get bot user by id=%v: %w", botUserID, err)
} else if botUser.Data.IsAccessGranted() == value {
log.Infof(c, "No need to change platformUser.AccessGranted, as already is: %v", value)
} else if changed := botUser.Data.SetAccessGranted(value); changed {
if err = tx.Set(c, botUser.Record); err != nil {
err = fmt.Errorf("failed to save bot user record (key=%v): %w", botUser.Key, err)
return err
} else {
if err = db.RunReadwriteTransaction(c, func(ctx context.Context, tx dal.ReadwriteTransaction) (err error) {
if err = tx.Get(ctx, botUser.Record); err != nil {
return
}
if changed := botUser.Data.SetAccessGranted(value); changed {
if err = tx.Set(c, botUser.Record); err != nil {
err = fmt.Errorf("failed to save bot user record (key=%v): %w", botUser.Key, err)
return err
}
}
return
}); err != nil {
return
}
}
return nil
return
//return SetAccessGrantedForAllUserChats(whc, whc.BotUserKey, value) // TODO: Call in deferrer
}

Expand Down
10 changes: 7 additions & 3 deletions botsfw/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ type CreateWebhookContextArgs struct {
AppContext AppContext
BotContext BotContext
WebhookInput botinput.WebhookInput
Tx dal.ReadwriteTransaction

Db dal.DB //Tx dal.ReadwriteTransaction

//BotCoreStores botsfwdal.DataAccess
GaMeasurement GaQueuer
}
Expand All @@ -53,7 +55,7 @@ func NewCreateWebhookContextArgs(
appContext AppContext,
botContext BotContext,
webhookInput botinput.WebhookInput,
tx dal.ReadwriteTransaction,
db dal.DB,
//botCoreStores botsfwdal.DataAccess,
gaMeasurement GaQueuer,
) CreateWebhookContextArgs {
Expand All @@ -62,7 +64,9 @@ func NewCreateWebhookContextArgs(
AppContext: appContext,
BotContext: botContext,
WebhookInput: webhookInput,
Tx: tx,

Db: db, //Tx: tx,

//BotCoreStores: botCoreStores,
GaMeasurement: gaMeasurement,
}
Expand Down
8 changes: 5 additions & 3 deletions botsfw/webhook_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import (
"context"
"github.com/bots-go-framework/bots-fw-store/botsfwmodels"
"github.com/bots-go-framework/bots-fw/botinput"
"github.com/bots-go-framework/bots-fw/botsdal"
"github.com/dal-go/dalgo/dal"
"github.com/dal-go/dalgo/record"
"github.com/strongo/gamp"
"github.com/strongo/i18n"
"net/http"
Expand Down Expand Up @@ -64,13 +64,15 @@ type WebhookContext interface { // TODO: Make interface much smaller?
DB() dal.DB

// Tx is a reference to database transaction used to get/save data of current bot
Tx() dal.ReadwriteTransaction
//Tx() dal.ReadwriteTransaction

// ChatData returns data of current bot chat without ID/Key
ChatData() botsfwmodels.BotChatData // Formerly ChatEntity()

// BotUser returns record of current bot user
BotUser() (botUser record.DataWithID[string, botsfwmodels.PlatformUserData], err error)
GetBotUser() (botUser botsdal.BotUser, err error)
GetBotUserForUpdate(ctx context.Context, tx dal.ReadwriteTransaction) (botUser botsdal.BotUser, err error)

GetBotUserID() string

// IsInGroup indicates if message was received in a group botChat
Expand Down
70 changes: 47 additions & 23 deletions botsfw/webhook_context_base.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"fmt"
"github.com/bots-go-framework/bots-fw-store/botsfwmodels"
"github.com/bots-go-framework/bots-fw/botinput"
botsdal2 "github.com/bots-go-framework/bots-fw/botsdal"
"github.com/bots-go-framework/bots-fw/botsdal"
"github.com/dal-go/dalgo/dal"
"github.com/dal-go/dalgo/record"
"github.com/strongo/gamp"
Expand Down Expand Up @@ -68,7 +68,7 @@ type WebhookContextBase struct {
// At the moment, there is no reason to expose botChat record publicly
// If there is some it should be documented with a use case
botChat record.DataWithID[string, botsfwmodels.BotChatData]
platformUser record.DataWithID[string, botsfwmodels.PlatformUserData] // Telegram user ID is an integer, but we keep it as a string for consistency & simplicity.
platformUser botsdal.BotUser // Telegram user ID is an integer, but we keep it as a string for consistency & simplicity.

isLoadingChatData bool // TODO: This smells bad. Needs refactoring?
isLoadingPlatformUserData bool // TODO: This smells bad. Needs refactoring?
Expand All @@ -82,7 +82,7 @@ type WebhookContextBase struct {

//dal botsfwdal.DataAccess
db dal.DB
tx dal.ReadwriteTransaction
//tx dal.ReadwriteTransaction

gaContext gaContext
}
Expand All @@ -92,9 +92,9 @@ func (whcb *WebhookContextBase) DB() dal.DB {
}

// Tx returns a transaction that is used to read/write botChat & bot user data
func (whcb *WebhookContextBase) Tx() dal.ReadwriteTransaction {
return whcb.tx
}
//func (whcb *WebhookContextBase) Tx() dal.ReadwriteTransaction {
// return whcb.tx
//}

func (whcb *WebhookContextBase) RecordsFieldsSetter() BotRecordsFieldsSetter {
return whcb.recordsFieldsSetter
Expand Down Expand Up @@ -237,7 +237,7 @@ func (whcb *WebhookContextBase) AppUserID() (appUserID string) {
}
if whcb.platformUser.Data == nil {
var err error
if err = whcb.getPlatformUserRecord(whcb.tx); err != nil {
if err = whcb.getPlatformUserRecord(whcb.db); err != nil {
if !dal.IsNotFound(err) {
panic(fmt.Errorf("failed to get bot user entity: %w", err))
}
Expand All @@ -258,20 +258,32 @@ func (whcb *WebhookContextBase) AppUserID() (appUserID string) {
//}
}

func (whcb *WebhookContextBase) BotUser() (botUser record.DataWithID[string, botsfwmodels.PlatformUserData], err error) {
func (whcb *WebhookContextBase) GetBotUserForUpdate(ctx context.Context, tx dal.ReadwriteTransaction) (botUser botsdal.BotUser, err error) {
err = whcb.db.RunReadwriteTransaction(ctx, func(ctx context.Context, tx dal.ReadwriteTransaction) (err error) {
botUser, err = whcb.getBotUser(ctx, tx)
return
})
return
}

func (whcb *WebhookContextBase) getBotUser(ctx context.Context, tx dal.Getter) (botUser botsdal.BotUser, err error) {
if whcb.platformUser.Data != nil {
return whcb.platformUser, nil
}
platformID := whcb.BotPlatform().ID()
botUserID := whcb.GetBotUserID()
whcb.platformUser, err = botsdal2.GetPlatformUser(whcb.c, whcb.tx, platformID, botUserID, whcb.botContext.BotSettings.Profile.NewPlatformUserData())
whcb.platformUser, err = botsdal.GetPlatformUser(ctx, whcb.db, platformID, botUserID, whcb.botContext.BotSettings.Profile.NewPlatformUserData())
return whcb.platformUser, err
}

func (whcb *WebhookContextBase) GetBotUser() (botUser botsdal.BotUser, err error) {
return whcb.getBotUser(whcb.c, whcb.db)
}

// GetAppUser loads information about current app user from persistent storage
func (whcb *WebhookContextBase) GetAppUser() (botsfwmodels.AppUserData, error) { // TODO: Can/should this be cached?
appUserID := whcb.AppUserID()
appUser, err := whcb.BotContext().BotSettings.GetAppUserByID(whcb.c, whcb.tx, appUserID)
appUser, err := whcb.BotContext().BotSettings.GetAppUserByID(whcb.c, whcb.db, appUserID)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -308,7 +320,8 @@ func NewWebhookContextBase(
whcb = &WebhookContextBase{
r: args.HttpRequest,
c: c,
tx: args.Tx,
db: args.Db,
//tx: args.Tx,
getLocaleAndChatID: func() (locale, chatID string, err error) {
return getLocaleAndChatID(c)
},
Expand Down Expand Up @@ -529,7 +542,7 @@ func (whcb *WebhookContextBase) ChatData() botsfwmodels.BotChatData {
return whcb.botChat.Data
}

func (whcb *WebhookContextBase) getPlatformUserRecord(tx dal.ReadwriteTransaction) (err error) {
func (whcb *WebhookContextBase) getPlatformUserRecord(tx dal.ReadSession) (err error) {
if whcb.platformUser.Data != nil {
return nil
}
Expand All @@ -539,7 +552,7 @@ func (whcb *WebhookContextBase) getPlatformUserRecord(tx dal.ReadwriteTransactio

whcb.platformUser.ID = fmt.Sprintf("%v", sender.GetID())
whcb.platformUser.Data = whcb.botContext.BotSettings.Profile.NewPlatformUserData()
if whcb.platformUser, err = botsdal2.GetPlatformUser(ctx, tx, platformID, whcb.platformUser.ID, whcb.platformUser.Data); err != nil {
if whcb.platformUser, err = botsdal.GetPlatformUser(ctx, tx, platformID, whcb.platformUser.ID, whcb.platformUser.Data); err != nil {
return
}
return
Expand Down Expand Up @@ -591,7 +604,7 @@ func (whcb *WebhookContextBase) createPlatformUserRecord(tx dal.ReadwriteTransac
}

// getOrCreatePlatformUserRecord to be documented
func (whcb *WebhookContextBase) getOrCreatePlatformUserRecord() (botUser record.DataWithID[string, botsfwmodels.PlatformUserData], err error) {
func (whcb *WebhookContextBase) getOrCreatePlatformUserRecord() (botUser botsdal.BotUser, err error) {
if whcb.platformUser.Data != nil {
return whcb.platformUser, nil
}
Expand All @@ -602,13 +615,18 @@ func (whcb *WebhookContextBase) getOrCreatePlatformUserRecord() (botUser record.
whcb.isLoadingPlatformUserData = false
}()

if err = whcb.getPlatformUserRecord(whcb.tx); err != nil {
if err = whcb.getPlatformUserRecord(whcb.db); err != nil {
if !dal.IsNotFound(err) {
return whcb.platformUser, err
} else {
log.Debugf(ctx, "Bot user entity not found, creating a new one...")
if err = whcb.createPlatformUserRecord(whcb.tx); err != nil {
return whcb.platformUser, fmt.Errorf("failed to create platform user record: %w", err)
if err = whcb.db.RunReadwriteTransaction(ctx, func(ctx context.Context, tx dal.ReadwriteTransaction) error {
if err = whcb.createPlatformUserRecord(tx); err != nil {
return fmt.Errorf("failed to create platform user record: %w", err)
}
return nil
}); err != nil {
return whcb.platformUser, err
}
}

Expand Down Expand Up @@ -638,7 +656,8 @@ func (whcb *WebhookContextBase) loadChatEntityBase() (err error) {
}

platformID := whcb.botPlatform.ID()
whcb.botChat, err = botsdal2.GetBotChat(ctx, whcb.tx, platformID,
db := whcb.DB()
whcb.botChat, err = botsdal.GetBotChat(ctx, db, platformID,
whcb.botContext.BotSettings.Code, whcb.botChat.ID, whcb.botContext.BotSettings.Profile.NewBotChatData)
if err != nil && !dal.IsNotFound(err) {
return fmt.Errorf("failed to get bot char record: %w", err)
Expand All @@ -661,7 +680,7 @@ func (whcb *WebhookContextBase) loadChatEntityBase() (err error) {
}
if dal.IsNotFound(err) {
log.Infof(ctx, "BotChat not found, first check for bot user entity...")
var botUser record.DataWithID[string, botsfwmodels.PlatformUserData]
var botUser botsdal.BotUser

if botUser, err = whcb.getOrCreatePlatformUserRecord(); err != nil {
return err
Expand Down Expand Up @@ -781,12 +800,18 @@ func (whcb *WebhookContextBase) CommandText(title, icon string) string {
}

func (whcb *WebhookContextBase) SaveBotChat() error {
ctx := whcb.Context()
// It is dangerous to allow user to pass context to this func as if it's a transactional context it might lead to deadlock
return whcb.tx.Set(whcb.c, whcb.botChat.Record)
return whcb.db.RunReadwriteTransaction(ctx, func(ctx context.Context, tx dal.ReadwriteTransaction) error {
return tx.Set(whcb.c, whcb.botChat.Record)
})
}

func (whcb *WebhookContextBase) SaveBotUser(ctx context.Context) error {
return whcb.tx.Set(ctx, whcb.platformUser.Record)
return whcb.db.RunReadwriteTransaction(ctx, func(ctx context.Context, tx dal.ReadwriteTransaction) error {
return errors.New("func SaveBotUser is not implemented yet")
//return tx.Set(ctx, whcb.platformUser.Record)
})
}

func (whcb *WebhookContextBase) AppUserData() (appUserData botsfwmodels.AppUserData, err error) {
Expand All @@ -795,10 +820,9 @@ func (whcb *WebhookContextBase) AppUserData() (appUserData botsfwmodels.AppUserD
return nil, fmt.Errorf("%w: AppUserID() is empty", dal.ErrRecordNotFound)
}
ctx := whcb.Context()
tx := whcb.Tx()
botContext := whcb.BotContext()
var appUser record.DataWithID[string, botsfwmodels.AppUserData]
if appUser, err = botContext.BotSettings.GetAppUserByID(ctx, tx, appUserID); err != nil {
if appUser, err = botContext.BotSettings.GetAppUserByID(ctx, whcb.db, appUserID); err != nil {
return
}
return appUser.Data, err
Expand Down
4 changes: 2 additions & 2 deletions botswebhook/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func (d BotDriver) processWebhookInput(
return
}
err = db.RunReadwriteTransaction(ctx, func(ctx context.Context, tx dal.ReadwriteTransaction) (err error) {
whcArgs := botsfw.NewCreateWebhookContextArgs(r, botContext.AppContext, *botContext, input, tx, measurementSender)
whcArgs := botsfw.NewCreateWebhookContextArgs(r, botContext.AppContext, *botContext, input, db, measurementSender)
if whc, err = webhookHandler.CreateWebhookContext(whcArgs); err != nil {
handleError(err, "Failed to create WebhookContext")
return
Expand All @@ -222,7 +222,7 @@ func (d BotDriver) processWebhookInput(
botID := whc.GetBotCode()
appContext := whc.AppContext()
var appUser record.DataWithID[string, botsfwmodels.AppUserData]
var botUser record.DataWithID[string, botsfwmodels.PlatformUserData]
var botUser botsdal.BotUser
bot := botsdal.Bot{
Platform: botsfwconst.Platform(platformID),
ID: botID,
Expand Down

0 comments on commit 47e30e1

Please sign in to comment.