Skip to content

Commit

Permalink
fix(GODT-2442): Use rename instead of remove for db files
Browse files Browse the repository at this point in the history
Unfortunately, closing the Ent database clients doesn't guarantee that
all the SQLite db files are closed. This causes issues on Windows where
the files are locked, making it impossible to remove and re-add the same
user.
  • Loading branch information
LBeernaertProton committed Mar 9, 2023
1 parent 5596825 commit 5cba271
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 1 deletion.
8 changes: 8 additions & 0 deletions builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package gluon

import (
"crypto/tls"
"github.com/ProtonMail/gluon/internal/db"
"github.com/sirupsen/logrus"
"io"
"os"
"time"
Expand Down Expand Up @@ -86,6 +88,12 @@ func (builder *serverBuilder) build() (*Server, error) {
return nil, err
}

// Defer delete all the previous databases from removed user accounts. This is required since we can't
// close ent databases on demand.
if err := db.DeleteDeferredDBFiles(builder.databaseDir); err != nil {
logrus.WithError(err).Error("Failed to remove old database files")
}

return &Server{
dataDir: builder.dataDir,
databaseDir: builder.databaseDir,
Expand Down
41 changes: 40 additions & 1 deletion internal/db/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"github.com/ProtonMail/gluon/internal/utils"
"github.com/google/uuid"
"io/fs"
"os"
"path/filepath"
Expand Down Expand Up @@ -109,6 +110,10 @@ func getDatabasePath(dir, userID string) string {
return filepath.Join(dir, fmt.Sprintf("%v.db", userID))
}

func getDeferredDeleteDBPath(dir string) string {
return filepath.Join(dir, "deferred_delete")
}

// NewDB creates a new database instance.
// If the database does not exist, it will be created and the second return value will be true.
func NewDB(dir, userID string) (*DB, bool, error) {
Expand All @@ -132,8 +137,42 @@ func NewDB(dir, userID string) (*DB, bool, error) {
return &DB{db: client}, !exists, nil
}

// DeleteDB will rename all the database files for the given user to a directory within the same folder to avoid
// issues with ent not being able to close the database on demand. The database will be cleaned up on the next
// run on the Gluon server.
func DeleteDB(dir, userID string) error {
return os.Remove(getDatabasePath(dir, userID))
// Rather than deleting the files immediately move them to a directory to be cleaned up later.
deferredDeletePath := getDeferredDeleteDBPath(dir)

if err := os.MkdirAll(deferredDeletePath, 0o700); err != nil {
return fmt.Errorf("failed to create deferred delete dir: %w", err)
}

matchingFiles, err := filepath.Glob(filepath.Join(dir, userID))
if err != nil {
return fmt.Errorf("failed to match db files:%w", err)
}

for _, file := range matchingFiles {
// Use new UUID to avoid conflict with existing files
if err := os.Rename(file, filepath.Join(deferredDeletePath, uuid.NewString())); err != nil {
return fmt.Errorf("failed to move db file '%v' :%w", file, err)
}
}

return nil
}

// DeleteDeferredDBFiles deletes all data from previous databases that were scheduled for removal.
func DeleteDeferredDBFiles(dir string) error {
deferredDeleteDir := getDeferredDeleteDBPath(dir)
if err := os.RemoveAll(deferredDeleteDir); err != nil {
if !errors.Is(err, os.ErrNotExist) {
return err
}
}

return nil
}

// pathExists returns whether the given file exists.
Expand Down

0 comments on commit 5cba271

Please sign in to comment.