Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove non permanent connections #2191

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .dredd/hooks/capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func capabilityWrapper(cap string) func(t *trans.Transaction) {

func addCapabilities(caps []string) {
dbConnect()
defer store.Close("")
defer store.Close()
resolved := make([]string, 0)
uid := getUUID()
resolveCapability(caps, resolved, uid)
Expand Down
6 changes: 3 additions & 3 deletions .dredd/hooks/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func addTestRunnerUser() {
}

dbConnect()
defer store.Close("")
defer store.Close()

truncateAll()

Expand Down Expand Up @@ -95,7 +95,7 @@ func truncateAll() {

func removeTestRunnerUser(transactions []*transaction.Transaction) {
dbConnect()
defer store.Close("")
defer store.Close()
_ = store.DeleteAPIToken(testRunnerUser.ID, adminToken)
_ = store.DeleteUser(testRunnerUser.ID)
}
Expand Down Expand Up @@ -314,7 +314,7 @@ var store db.Store
func dbConnect() {
store = factory.CreateStore()

store.Connect("")
store.Connect()
}

func stringInSlice(a string, list []string) (int, bool) {
Expand Down
6 changes: 3 additions & 3 deletions .dredd/hooks/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ func main() {

h.Before("user > /api/user/tokens/{api_token_id} > Expires API token > 204 > application/json", func(transaction *trans.Transaction) {
dbConnect()
defer store.Close("")
defer store.Close()
addToken(expiredToken, testRunnerUser.ID)
})

h.After("user > /api/user/tokens/{api_token_id} > Expires API token > 204 > application/json", func(transaction *trans.Transaction) {
dbConnect()
defer store.Close("")
defer store.Close()
//tokens are expired and not deleted so we need to clean up
_ = store.DeleteAPIToken(testRunnerUser.ID, expiredToken)
})
Expand All @@ -74,7 +74,7 @@ func main() {
// delete the auto generated association and insert the user id into the query
h.Before("project > /api/project/{project_id}/users > Link user to project > 204 > application/json", func(transaction *trans.Transaction) {
dbConnect()
defer store.Close("")
defer store.Close()
deleteUserProjectRelation(userProject.ID, userPathTestUser.ID)
transaction.Request.Body = "{ \"user_id\": " + strconv.Itoa(userPathTestUser.ID) + ",\"role\": \"owner\"}"
})
Expand Down
8 changes: 1 addition & 7 deletions api/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,7 @@ func authentication(next http.Handler) http.Handler {
// nolint: gocyclo
func authenticationWithStore(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
store := helpers.Store(r)

var ok bool

db.StoreSession(store, r.URL.String(), func() {
ok = authenticationHandler(w, r)
})
ok := authenticationHandler(w, r)

if ok {
next.ServeHTTP(w, r)
Expand Down
20 changes: 4 additions & 16 deletions api/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,6 @@ var startTime = time.Now().UTC()
//go:embed public/*
var publicAssets embed.FS

// StoreMiddleware WTF?
func StoreMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
store := helpers.Store(r)
//var url = r.URL.String()

db.StoreSession(store, util.RandString(12), func() {
next.ServeHTTP(w, r)
})
})
}

// JSONMiddleware ensures that all the routes respond with Json, this is added by default to all routes
func JSONMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -78,7 +66,7 @@ func Route() *mux.Router {
pingRouter.Methods("GET", "HEAD").HandlerFunc(pongHandler)

publicAPIRouter := r.PathPrefix(webPath + "api").Subrouter()
publicAPIRouter.Use(StoreMiddleware, JSONMiddleware)
publicAPIRouter.Use(JSONMiddleware)

publicAPIRouter.HandleFunc("/runners", runners.RegisterRunner).Methods("POST")
publicAPIRouter.HandleFunc("/auth/login", login).Methods("GET", "POST")
Expand All @@ -88,21 +76,21 @@ func Route() *mux.Router {
publicAPIRouter.HandleFunc("/auth/oidc/{provider}/redirect/{redirect_path:.*}", oidcRedirect).Methods("GET")

routersAPI := r.PathPrefix(webPath + "api").Subrouter()
routersAPI.Use(StoreMiddleware, JSONMiddleware, runners.RunnerMiddleware)
routersAPI.Use(JSONMiddleware, runners.RunnerMiddleware)
routersAPI.Path("/runners/{runner_id}").HandlerFunc(runners.GetRunner).Methods("GET", "HEAD")
routersAPI.Path("/runners/{runner_id}").HandlerFunc(runners.UpdateRunner).Methods("PUT")
routersAPI.Path("/runners/{runner_id}").HandlerFunc(runners.UnregisterRunner).Methods("DELETE")

publicWebHookRouter := r.PathPrefix(webPath + "api").Subrouter()
publicWebHookRouter.Use(StoreMiddleware, JSONMiddleware)
publicWebHookRouter.Use(JSONMiddleware)
publicWebHookRouter.Path("/integrations/{integration_alias}").HandlerFunc(ReceiveIntegration).Methods("POST", "GET", "OPTIONS")

authenticatedWS := r.PathPrefix(webPath + "api").Subrouter()
authenticatedWS.Use(JSONMiddleware, authenticationWithStore)
authenticatedWS.Path("/ws").HandlerFunc(sockets.Handler).Methods("GET", "HEAD")

authenticatedAPI := r.PathPrefix(webPath + "api").Subrouter()
authenticatedAPI.Use(StoreMiddleware, JSONMiddleware, authentication)
authenticatedAPI.Use(JSONMiddleware, authentication)

authenticatedAPI.Path("/info").HandlerFunc(getSystemInfo).Methods("GET", "HEAD")

Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ var migrateCmd = &cobra.Command{
Short: "Execute migrations",
Run: func(cmd *cobra.Command, args []string) {
store := createStore("migrate")
defer store.Close("migrate")
defer store.Close()
util.Config.PrintDbInfo()
},
}
8 changes: 2 additions & 6 deletions cli/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,7 @@ func runService() {

fmt.Println("Server is running")

if store.PermanentConnection() {
defer store.Close("root")
} else {
store.Close("root")
}
defer store.Close()

err := http.ListenAndServe(util.Config.Interface+port, cropTrailingSlashMiddleware(router))

Expand All @@ -100,7 +96,7 @@ func createStore(token string) db.Store {

store := factory.CreateStore()

store.Connect(token)
store.Connect()

err := db.Migrate(store)

Expand Down
4 changes: 2 additions & 2 deletions cli/cmd/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func doSetup() int {
fmt.Println(" Pinging db..")

store := factory.CreateStore()
defer store.Close("setup")
store.Connect("setup")
defer store.Close()
store.Connect()

fmt.Println("Running db Migrations..")
if err := db.Migrate(store); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/user_add.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ var userAddCmd = &cobra.Command{
}

store := createStore("")
defer store.Close("")
defer store.Close()

if _, err := store.CreateUser(db.UserWithPwd{
Pwd: targetUserArgs.password,
Expand Down
4 changes: 2 additions & 2 deletions cli/cmd/user_change.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ var userChangeByLoginCmd = &cobra.Command{
}

store := createStore("")
defer store.Close("")
defer store.Close()

user, err := store.GetUserByLoginOrEmail(targetUserArgs.login, "")

Expand Down Expand Up @@ -97,7 +97,7 @@ var userChangeByEmailCmd = &cobra.Command{
}

store := createStore("")
defer store.Close("")
defer store.Close()

user, err := store.GetUserByLoginOrEmail("", targetUserArgs.email)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/user_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var userDeleteCmd = &cobra.Command{
}

store := createStore("")
defer store.Close("")
defer store.Close()

user, err := store.GetUserByLoginOrEmail(targetUserArgs.login, targetUserArgs.email)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/user_get.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var userGetCmd = &cobra.Command{
}

store := createStore("")
defer store.Close("")
defer store.Close()

user, err := store.GetUserByLoginOrEmail(targetUserArgs.login, targetUserArgs.email)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/user_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ var userListCmd = &cobra.Command{
Short: "Print all users",
Run: func(cmd *cobra.Command, args []string) {
store := createStore("")
defer store.Close("")
defer store.Close()

users, err := store.GetUsers(db.RetrieveQueryParams{})

Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/vault_rekey.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ var vaultRekeyCmd = &cobra.Command{
"pre-existing keys stored in the database.",
Run: func(cmd *cobra.Command, args []string) {
store := createStore("")
defer store.Close("")
defer store.Close()

err := store.RekeyAccessKeys(targetVaultArgs.oldKey)

Expand Down
21 changes: 2 additions & 19 deletions db/Store.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,8 @@ type Store interface {
// Connect connects to the database.
// Token parameter used if PermanentConnection returns false.
// Token used for debugging of session connections.
Connect(token string)
Close(token string)

// PermanentConnection returns true if connection should be kept from start to finish of the app.
// This mode is suitable for MySQL and Postgres but not for BoltDB.
// For BoltDB we should reconnect for each request because BoltDB support only one connection at time.
PermanentConnection() bool
Connect()
Close()

// IsInitialized indicates is database already initialized, or it is empty.
// The method is useful for creating required entities in database during first run.
Expand Down Expand Up @@ -449,18 +444,6 @@ func (p ObjectProps) GetReferringFieldsFrom(t reflect.Type) (fields []string, er
return
}

func StoreSession(store Store, token string, callback func()) {
if !store.PermanentConnection() {
store.Connect(token)
}

callback()

if !store.PermanentConnection() {
store.Close(token)
}
}

func ValidateRepository(store Store, repo *Repository) (err error) {
_, err = store.GetAccessKey(repo.ProjectID, repo.SSHKeyID)

Expand Down
54 changes: 5 additions & 49 deletions db/bolt/BoltDb.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,31 +73,14 @@ func (d *BoltDb) Migrate() error {
return nil
}

func (d *BoltDb) Connect(token string) {
d.mu.Lock()
defer d.mu.Unlock()

if d.connections == nil {
d.connections = make(map[string]bool)
}

if _, exists := d.connections[token]; exists {
// Use for debugging
panic(fmt.Errorf("Connection " + token + " already exists"))
}

if len(d.connections) > 0 {
d.connections[token] = true
return
}

func (d *BoltDb) Connect() {
var filename string
if d.Filename == "" {
config, err := util.Config.GetDBConfig()
if err != nil {
panic(err)
}
filename = config.GetHostname()
filename = config.Hostname
} else {
filename = d.Filename
}
Expand All @@ -110,37 +93,10 @@ func (d *BoltDb) Connect(token string) {
if err != nil {
panic(err)
}

d.connections[token] = true
}

func (d *BoltDb) Close(token string) {
d.mu.Lock()
defer d.mu.Unlock()

_, exists := d.connections[token]

if !exists {
// Use for debugging
panic(fmt.Errorf("can not close closed connection " + token))
}

if len(d.connections) > 1 {
delete(d.connections, token)
return
}

err := d.db.Close()
if err != nil {
panic(err)
}

d.db = nil
delete(d.connections, token)
}

func (d *BoltDb) PermanentConnection() bool {
return false
func (d *BoltDb) Close() {
_ = d.db.Close()
}

func (d *BoltDb) IsInitialized() (initialized bool, err error) {
Expand Down Expand Up @@ -789,6 +745,6 @@ func CreateTestStore() *BoltDb {
store := BoltDb{
Filename: fn,
}
store.Connect("test")
store.Connect()
return &store
}
8 changes: 2 additions & 6 deletions db/sql/SqlDb.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,18 +268,14 @@ func (d *SqlDb) deleteObject(projectID int, props db.ObjectProps, objectID any)
}
}

func (d *SqlDb) Close(token string) {
func (d *SqlDb) Close() {
err := d.sql.Db.Close()
if err != nil {
panic(err)
}
}

func (d *SqlDb) PermanentConnection() bool {
return true
}

func (d *SqlDb) Connect(token string) {
func (d *SqlDb) Connect() {
sqlDb, err := connect()
if err != nil {
panic(err)
Expand Down
6 changes: 0 additions & 6 deletions services/schedules/SchedulePool.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package schedules

import (
"strconv"
"sync"

"github.com/ansible-semaphore/semaphore/db"
Expand Down Expand Up @@ -53,11 +52,6 @@ func (r ScheduleRunner) tryUpdateScheduleCommitHash(schedule db.Schedule) (updat
}

func (r ScheduleRunner) Run() {
if !r.pool.store.PermanentConnection() {
r.pool.store.Connect("schedule " + strconv.Itoa(r.scheduleID))
defer r.pool.store.Close("schedule " + strconv.Itoa(r.scheduleID))
}

schedule, err := r.pool.store.GetSchedule(r.projectID, r.scheduleID)
if err != nil {
log.Error(err)
Expand Down
Loading
Loading