add: db tx commit and rollback implementation

2025-05-15 15:42:33 +07:00
parent 264435f67e
commit d2ec8c0f07
11 changed files with 216 additions and 60 deletions
--- a/internal/api/service/register_service.go
+++ b/internal/api/service/register_service.go
@@ -7,6 +7,8 @@ import (

 	"devone.aplikasi.web.id/gitea/mario/go-ohif-proxy/internal/api/models"
 	"devone.aplikasi.web.id/gitea/mario/go-ohif-proxy/internal/api/repository"
+	"devone.aplikasi.web.id/gitea/mario/go-ohif-proxy/internal/database"
+	"go.uber.org/zap"
 )

 var (
@@ -15,6 +17,7 @@ var (
 	ErrInvalidRole    = errors.New("invalid user role")
 	ErrInvalidPatient = errors.New("invalid patient data")
 	ErrInvalidDoctor  = errors.New("invalid doctor data")
+	ErrTransaction    = errors.New("transaction error")
 )

 // RegisterRequest represents a user registration request
@@ -34,15 +37,22 @@ type RegisterService struct {
 	patientRepo *repository.PatientRepository
 	doctorRepo  *repository.DoctorRepository
 	studyRepo   *repository.StudyRepository
+	logger      *zap.Logger
 }

 // NewRegisterService creates a new register service
-func NewRegisterService() *RegisterService {
+func NewRegisterService(logger *zap.Logger) *RegisterService {
+	if logger == nil {
+		// If no logger is provided, create a no-op logger
+		logger = zap.NewNop()
+	}
+
 	return &RegisterService{
 		userRepo:    repository.NewUserRepository(),
 		patientRepo: repository.NewPatientRepository(),
 		doctorRepo:  repository.NewDoctorRepository(),
 		studyRepo:   repository.NewStudyRepository(),
+		logger:      logger,
 	}
 }

@@ -62,7 +72,7 @@ func (s *RegisterService) Register(req *RegisterRequest) (*models.User, error) {
 		return nil, ErrInvalidDoctor
 	}

-	// Check if email already exists
+	// Check if email already exists - do this outside the transaction
 	existingUser, err := s.userRepo.GetUserByEmail(req.Email)
 	if err != nil && err != sql.ErrNoRows {
 		return nil, fmt.Errorf("error checking existing user: %w", err)
@@ -72,6 +82,22 @@ func (s *RegisterService) Register(req *RegisterRequest) (*models.User, error) {
 		return nil, ErrEmailExists
 	}

+	// Start a transaction
+	tx, err := database.DB.Beginx()
+	if err != nil {
+		s.logger.Error("Failed to begin transaction", zap.Error(err))
+		return nil, fmt.Errorf("%w: failed to begin transaction", ErrTransaction)
+	}
+
+	// Ensure the transaction is rolled back if we return an error
+	defer func() {
+		if r := recover(); r != nil {
+			s.logger.Error("Panic in transaction", zap.Any("recover", r))
+			tx.Rollback()
+			panic(r) // re-throw the panic after cleanup
+		}
+	}()
+
 	// Create user
 	newUser := &models.User{
 		Email:    req.Email,
@@ -80,15 +106,19 @@ func (s *RegisterService) Register(req *RegisterRequest) (*models.User, error) {
 		Name:     req.Name,
 	}

-	if err := s.userRepo.CreateUser(newUser); err != nil {
+	// Use the transaction for user creation
+	if err := s.userRepo.CreateUserTx(tx, newUser); err != nil {
+		s.logger.Error("Failed to create user", zap.Error(err), zap.String("email", req.Email))
+		tx.Rollback()
 		return nil, fmt.Errorf("error creating user: %w", err)
 	}

 	// Create role-specific data
 	if req.Role == "patient" {
-		err = s.patientRepo.CreatePatient(req.Patient, newUser.ID)
+		err = s.patientRepo.CreatePatientTx(tx, req.Patient, newUser.ID)
 		if err != nil {
-			// TODO: Consider rollback user creation if this fails
+			s.logger.Error("Failed to create patient", zap.Error(err), zap.String("patientID", req.Patient.PatientID))
+			tx.Rollback()
 			return nil, fmt.Errorf("error creating patient: %w", err)
 		}

@@ -99,19 +129,37 @@ func (s *RegisterService) Register(req *RegisterRequest) (*models.User, error) {
 					continue // Skip studies without UIDs
 				}

-				err = s.studyRepo.CreateStudy(req.Patient.PatientID, study)
+				err = s.studyRepo.CreateStudyTx(tx, req.Patient.PatientID, study)
 				if err != nil {
+					s.logger.Error("Failed to create study",
+						zap.Error(err),
+						zap.String("patientID", req.Patient.PatientID),
+						zap.String("studyUID", study.StudyInstanceUID))
+					tx.Rollback()
 					return nil, fmt.Errorf("error creating study for patient: %w", err)
 				}
 			}
 		}
 	} else if req.Role == "ref_doctor" || req.Role == "expertise_doctor" {
-		err = s.doctorRepo.CreateDoctor(req.Doctor, newUser.ID)
+		err = s.doctorRepo.CreateDoctorTx(tx, req.Doctor, newUser.ID)
 		if err != nil {
-			// TODO: Consider rollback user creation if this fails
+			s.logger.Error("Failed to create doctor", zap.Error(err), zap.String("doctorID", req.Doctor.DoctorID))
+			tx.Rollback()
 			return nil, fmt.Errorf("error creating doctor: %w", err)
 		}
 	}

+	// Commit the transaction
+	if err := tx.Commit(); err != nil {
+		s.logger.Error("Failed to commit transaction", zap.Error(err))
+		tx.Rollback() // This is actually redundant as the transaction will be rolled back on failure
+		return nil, fmt.Errorf("%w: failed to commit transaction", ErrTransaction)
+	}
+
+	s.logger.Info("Successfully registered new user",
+		zap.String("email", req.Email),
+		zap.String("role", req.Role),
+		zap.String("userID", newUser.ID))
+
 	return newUser, nil
 }
--- a/internal/api/service/shortlink_service.go
+++ b/internal/api/service/shortlink_service.go
@@ -13,6 +13,7 @@ import (
 	"devone.aplikasi.web.id/gitea/mario/go-ohif-proxy/internal/api/models"
 	"devone.aplikasi.web.id/gitea/mario/go-ohif-proxy/internal/api/repository"
 	"devone.aplikasi.web.id/gitea/mario/go-ohif-proxy/internal/auth"
+	"devone.aplikasi.web.id/gitea/mario/go-ohif-proxy/internal/database"
 	"go.uber.org/zap"
 )

@@ -150,13 +151,36 @@ func (s *ShortLinkService) GenerateShortLink(req *models.GenerateShortLinkReques
 		RemainingTries: s.maxAttempts,
 	}

-	// Store the short link in the database
-	err = s.shortLinkRepo.CreateShortLink(shortLink)
+	// Start a transaction for creating the shortlink
+	tx, err := database.DB.Beginx()
+	if err != nil {
+		s.logger.Error("Failed to start transaction", zap.Error(err))
+		return nil, fmt.Errorf("database error: %w", err)
+	}
+
+	// Set up deferred rollback that will be canceled if we commit
+	defer func() {
+		if tx != nil {
+			tx.Rollback()
+		}
+	}()
+
+	// Store the short link in the database using transaction
+	err = s.shortLinkRepo.CreateShortLinkTx(tx, shortLink)
 	if err != nil {
 		s.logger.Error("Failed to store shortlink in database", zap.Error(err))
 		return nil, ErrCreationFailed
 	}

+	// Commit the transaction
+	if err = tx.Commit(); err != nil {
+		s.logger.Error("Failed to commit transaction", zap.Error(err))
+		return nil, errors.New("database error")
+	}
+
+	// Clear the tx to prevent the deferred rollback
+	tx = nil
+
 	// Generate the full URL using the configured base URL
 	fullURL := fmt.Sprintf("%s/short-auth?short=%s", s.baseURL, token)

@@ -203,43 +227,101 @@ func (s *ShortLinkService) ValidateShortLink(req *models.ShortLinkAuthRequest) (
 	// Normalize and hash the provided DOB
 	dob := normalizeDOB(req.DOB)
 	if !isValidDOBFormat(dob) {
+		// Use a transaction for updating the tries counter
+		tx, err := database.DB.Beginx()
+		if err != nil {
+			s.logger.Error("Failed to start transaction", zap.Error(err))
+			return nil, fmt.Errorf("database error: %w", err)
+		}
+
+		// Set up deferred rollback that will be canceled if we commit
+		defer func() {
+			if tx != nil {
+				tx.Rollback()
+			}
+		}()
+
 		// Decrement remaining tries on invalid format
 		shortLink.RemainingTries--

-		// Update the shortlink in the database
-		err = s.shortLinkRepo.UpdateShortLink(shortLink)
+		// Update the shortlink in the database using the transaction
+		err = s.shortLinkRepo.UpdateShortLinkTx(tx, shortLink)
 		if err != nil {
 			s.logger.Error("Failed to update shortlink tries", zap.Error(err))
+			return nil, errors.New("invalid date of birth format, expected YYYY-MM-DD")
 		}

+		// Commit the transaction
+		if err = tx.Commit(); err != nil {
+			s.logger.Error("Failed to commit transaction", zap.Error(err))
+			return nil, errors.New("database error")
+		}
+
+		// Clear the tx to prevent the deferred rollback
+		tx = nil
+
 		return nil, errors.New("invalid date of birth format, expected YYYY-MM-DD")
 	}

 	hashedDOB := hashDOB(dob)

+	// Start a transaction for updating the tries counter
+	tx, err := database.DB.Beginx()
+	if err != nil {
+		s.logger.Error("Failed to start transaction", zap.Error(err))
+		return nil, fmt.Errorf("database error: %w", err)
+	}
+
+	// Set up deferred rollback that will be canceled if we commit
+	defer func() {
+		if tx != nil {
+			tx.Rollback()
+		}
+	}()
+
 	// Verify the DOB
 	if hashedDOB != shortLink.HashedDOB {
 		// Decrement remaining tries on failed verification
 		shortLink.RemainingTries--

-		// Update the shortlink in the database
-		err = s.shortLinkRepo.UpdateShortLink(shortLink)
+		// Update the shortlink in the database using the transaction
+		err = s.shortLinkRepo.UpdateShortLinkTx(tx, shortLink)
 		if err != nil {
 			s.logger.Error("Failed to update shortlink tries", zap.Error(err))
+			return nil, ErrInvalidDOB
 		}

+		// Commit the transaction
+		if err = tx.Commit(); err != nil {
+			s.logger.Error("Failed to commit transaction", zap.Error(err))
+			return nil, errors.New("database error")
+		}
+
+		// Clear the tx to prevent the deferred rollback
+		tx = nil
+
 		return nil, ErrInvalidDOB
 	}

 	// DOB verified, reset tries count as successful login
 	shortLink.RemainingTries = s.maxAttempts

-	// Update the shortlink in the database
-	err = s.shortLinkRepo.UpdateShortLink(shortLink)
+	// Update the shortlink in the database using the transaction
+	err = s.shortLinkRepo.UpdateShortLinkTx(tx, shortLink)
 	if err != nil {
 		s.logger.Error("Failed to update shortlink tries after successful validation", zap.Error(err))
+		return nil, errors.New("database error")
 	}

+	// Commit the transaction
+	if err = tx.Commit(); err != nil {
+		s.logger.Error("Failed to commit transaction", zap.Error(err))
+		return nil, errors.New("database error")
+	}
+
+	// Clear the tx to prevent the deferred rollback
+	tx = nil
+
 	return shortLink, nil
 }