From aebb7d42dc6e0ced517c535d186732416aea24d1 Mon Sep 17 00:00:00 2001 From: broccoli Date: Sat, 3 May 2025 12:26:54 +0200 Subject: [PATCH 1/7] Fix: creating file_record table by registering the file record entitry in init_data.go, the table is being created during initialization. --- internal/migrations/init_data.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/migrations/init_data.go b/internal/migrations/init_data.go index 7322f7388..0d4cde503 100644 --- a/internal/migrations/init_data.go +++ b/internal/migrations/init_data.go @@ -74,6 +74,7 @@ var ( &entity.Badge{}, &entity.BadgeGroup{}, &entity.BadgeAward{}, + &entity.FileRecord{}, } roles = []*entity.Role{ From 2dc836868e37cfeaa1b5c99e978a9df0edf253f2 Mon Sep 17 00:00:00 2001 From: broccoli Date: Sat, 3 May 2025 12:59:07 +0200 Subject: [PATCH 2/7] fix: all uploaded files are added to file_record + uploaded avatar and branding files are added to the file_record table, so that the table provides a comprehensive overview of files in the file system --- internal/service/uploader/upload.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/internal/service/uploader/upload.go b/internal/service/uploader/upload.go index d5cc2dfbe..a2c9b98a1 100644 --- a/internal/service/uploader/upload.go +++ b/internal/service/uploader/upload.go @@ -127,7 +127,13 @@ func (us *uploaderService) UploadAvatarFile(ctx *gin.Context, userID string) (ur newFilename := fmt.Sprintf("%s%s", uid.IDStr12(), fileExt) avatarFilePath := path.Join(constant.AvatarSubPath, newFilename) - return us.uploadImageFile(ctx, fileHeader, avatarFilePath) + url, err = us.uploadImageFile(ctx, fileHeader, avatarFilePath) + if err != nil { + return "", err + } + us.fileRecordService.AddFileRecord(ctx, userID, avatarFilePath, url, string(plugin.UserAvatar)) + return url, nil + } func (us *uploaderService) AvatarThumbFile(ctx *gin.Context, fileName string, size int) (url string, err error) { @@ -282,7 +288,13 @@ func (us *uploaderService) UploadBrandingFile(ctx *gin.Context, userID string) ( newFilename := fmt.Sprintf("%s%s", uid.IDStr12(), fileExt) avatarFilePath := path.Join(constant.BrandingSubPath, newFilename) - return us.uploadImageFile(ctx, fileHeader, avatarFilePath) + url, err = us.uploadImageFile(ctx, fileHeader, avatarFilePath) + if err != nil { + return "", err + } + us.fileRecordService.AddFileRecord(ctx, userID, avatarFilePath, url, string(plugin.AdminBranding)) + return url, nil + } func (us *uploaderService) uploadImageFile(ctx *gin.Context, file *multipart.FileHeader, fileSubPath string) ( From 2e098b09d1530149a6928bb0e93e7b90cd6c1916 Mon Sep 17 00:00:00 2001 From: broccoli Date: Fri, 9 May 2025 13:38:14 +0200 Subject: [PATCH 3/7] feat: delete branding files + a file is being deleted after its reference has been removed from the site info. The user has no way to access the file again. Therefore the file would become an orphan if not deleted. + an if clause had to be added to CleanOrphanUploadFiles, because it would otherwise remove the branding and avatar files. --- .../controller_admin/siteinfo_controller.go | 8 +++- internal/repo/file_record/file_record_repo.go | 15 +++++++ .../file_record/file_record_service.go | 22 +++++++++- internal/service/siteinfo/siteinfo_service.go | 44 +++++++++++++++++++ 4 files changed, 86 insertions(+), 3 deletions(-) diff --git a/internal/controller_admin/siteinfo_controller.go b/internal/controller_admin/siteinfo_controller.go index cd1b2ab68..b3af34f43 100644 --- a/internal/controller_admin/siteinfo_controller.go +++ b/internal/controller_admin/siteinfo_controller.go @@ -28,6 +28,7 @@ import ( "github.com/apache/answer/internal/schema" "github.com/apache/answer/internal/service/siteinfo" "github.com/gin-gonic/gin" + "github.com/segmentfault/pacman/log" ) // SiteInfoController site info controller @@ -274,7 +275,12 @@ func (sc *SiteInfoController) UpdateBranding(ctx *gin.Context) { if handler.BindAndCheck(ctx, req) { return } - err := sc.siteInfoService.SaveSiteBranding(ctx, req) + currentBranding, _ := sc.siteInfoService.GetSiteBranding(ctx) + err := sc.siteInfoService.CleanUpRemovedBrandingFiles(ctx, req, currentBranding) + if err != nil { + log.Error(err) + } + err = sc.siteInfoService.SaveSiteBranding(ctx, req) handler.HandleResponse(ctx, err, nil) } diff --git a/internal/repo/file_record/file_record_repo.go b/internal/repo/file_record/file_record_repo.go index ed081be40..ce486c7ab 100644 --- a/internal/repo/file_record/file_record_repo.go +++ b/internal/repo/file_record/file_record_repo.go @@ -82,3 +82,18 @@ func (fr *fileRecordRepo) UpdateFileRecord(ctx context.Context, fileRecord *enti } return } + +// GetFileRecordByURL gets a file record by its url +func (fr *fileRecordRepo) GetFileRecordByURL(ctx context.Context, fileURL string) (record *entity.FileRecord, err error) { + record = &entity.FileRecord{} + session := fr.data.DB.Context(ctx) + exists, err := session.Where("file_url = ? AND status = ?", fileURL, entity.FileRecordStatusAvailable).Get(record) + if err != nil { + err = errors.InternalServer(reason.DatabaseError).WithError(err).WithStack() + return + } + if !exists { + return + } + return record, nil +} diff --git a/internal/service/file_record/file_record_service.go b/internal/service/file_record/file_record_service.go index abb983768..e9ea1c24c 100644 --- a/internal/service/file_record/file_record_service.go +++ b/internal/service/file_record/file_record_service.go @@ -24,6 +24,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" "github.com/apache/answer/internal/base/constant" @@ -44,6 +45,7 @@ type FileRecordRepo interface { GetFileRecordPage(ctx context.Context, page, pageSize int, cond *entity.FileRecord) ( fileRecordList []*entity.FileRecord, total int64, err error) DeleteFileRecord(ctx context.Context, id int) (err error) + GetFileRecordByURL(ctx context.Context, fileURL string) (record *entity.FileRecord, err error) } // FileRecordService file record service @@ -104,6 +106,9 @@ func (fs *FileRecordService) CleanOrphanUploadFiles(ctx context.Context) { if fileRecord.CreatedAt.AddDate(0, 0, 2).After(time.Now()) { continue } + if isBrandingOrAvatarFile(fileRecord.FilePath) { + continue + } if checker.IsNotZeroString(fileRecord.ObjectID) { _, exist, err := fs.revisionRepo.GetLastRevisionByObjectID(ctx, fileRecord.ObjectID) if err != nil { @@ -129,7 +134,7 @@ func (fs *FileRecordService) CleanOrphanUploadFiles(ctx context.Context) { } } // Delete and move the file record - if err := fs.deleteAndMoveFileRecord(ctx, fileRecord); err != nil { + if err := fs.DeleteAndMoveFileRecord(ctx, fileRecord); err != nil { log.Error(err) } } @@ -137,6 +142,10 @@ func (fs *FileRecordService) CleanOrphanUploadFiles(ctx context.Context) { } } +func isBrandingOrAvatarFile(filePath string) bool { + return strings.Contains(filePath, constant.BrandingSubPath+"/") || strings.Contains(filePath, constant.AvatarSubPath+"/") +} + func (fs *FileRecordService) PurgeDeletedFiles(ctx context.Context) { deletedPath := filepath.Join(fs.serviceConfig.UploadPath, constant.DeletedSubPath) log.Infof("purge deleted files: %s", deletedPath) @@ -152,7 +161,7 @@ func (fs *FileRecordService) PurgeDeletedFiles(ctx context.Context) { return } -func (fs *FileRecordService) deleteAndMoveFileRecord(ctx context.Context, fileRecord *entity.FileRecord) error { +func (fs *FileRecordService) DeleteAndMoveFileRecord(ctx context.Context, fileRecord *entity.FileRecord) error { // Delete the file record if err := fs.fileRecordRepo.DeleteFileRecord(ctx, fileRecord.ID); err != nil { return fmt.Errorf("delete file record error: %v", err) @@ -170,3 +179,12 @@ func (fs *FileRecordService) deleteAndMoveFileRecord(ctx context.Context, fileRe log.Debugf("delete and move file: %s", fileRecord.FileURL) return nil } + +func (fs *FileRecordService) GetFileRecordByURL(ctx context.Context, fileURL string) (record *entity.FileRecord, err error) { + record, err = fs.fileRecordRepo.GetFileRecordByURL(ctx, fileURL) + if err != nil { + log.Errorf("error retrieving file record by URL: %v", err) + return + } + return +} diff --git a/internal/service/siteinfo/siteinfo_service.go b/internal/service/siteinfo/siteinfo_service.go index 5141faeee..3c7c99b92 100644 --- a/internal/service/siteinfo/siteinfo_service.go +++ b/internal/service/siteinfo/siteinfo_service.go @@ -33,6 +33,7 @@ import ( "github.com/apache/answer/internal/schema" "github.com/apache/answer/internal/service/config" "github.com/apache/answer/internal/service/export" + "github.com/apache/answer/internal/service/file_record" questioncommon "github.com/apache/answer/internal/service/question_common" "github.com/apache/answer/internal/service/siteinfo_common" tagcommon "github.com/apache/answer/internal/service/tag_common" @@ -49,6 +50,7 @@ type SiteInfoService struct { tagCommonService *tagcommon.TagCommonService configService *config.ConfigService questioncommon *questioncommon.QuestionCommon + fileRecordService *file_record.FileRecordService } func NewSiteInfoService( @@ -58,6 +60,7 @@ func NewSiteInfoService( tagCommonService *tagcommon.TagCommonService, configService *config.ConfigService, questioncommon *questioncommon.QuestionCommon, + fileRecordService *file_record.FileRecordService, ) *SiteInfoService { plugin.RegisterGetSiteURLFunc(func() string { @@ -76,6 +79,7 @@ func NewSiteInfoService( tagCommonService: tagCommonService, configService: configService, questioncommon: questioncommon, + fileRecordService: fileRecordService, } } @@ -438,3 +442,43 @@ func (s *SiteInfoService) UpdatePrivilegesConfig(ctx context.Context, req *schem } return } + +func (s *SiteInfoService) CleanUpRemovedBrandingFiles( + ctx context.Context, + newBranding *schema.SiteBrandingReq, + currentBranding *schema.SiteBrandingResp, +) error { + currentFiles := map[string]string{ + "logo": currentBranding.Logo, + "mobile_logo": currentBranding.MobileLogo, + "square_icon": currentBranding.SquareIcon, + "favicon": currentBranding.Favicon, + } + + newFiles := map[string]string{ + "logo": newBranding.Logo, + "mobile_logo": newBranding.MobileLogo, + "square_icon": newBranding.SquareIcon, + "favicon": newBranding.Favicon, + } + + for key, currentFile := range currentFiles { + newFile := newFiles[key] + if currentFile != "" && currentFile != newFile { + fileRecord, err := s.fileRecordService.GetFileRecordByURL(ctx, currentFile) + if err != nil { + log.Error(err) + continue + } + if fileRecord == nil { + log.Error("could not fetch file record by url") + continue + } + if err := s.fileRecordService.DeleteAndMoveFileRecord(ctx, fileRecord); err != nil { + log.Error(err) + } + } + } + + return nil +} From 9a7a03e6cf04cd4035a75e587ba4f353faa3c1f0 Mon Sep 17 00:00:00 2001 From: broccoli Date: Sun, 11 May 2025 12:29:40 +0200 Subject: [PATCH 4/7] feat: delete avatar files + a file is being deleted after its reference has been removed from the user info. The user has no way to access the file again. Therefore the file would become an orphan if not deleted. --- internal/service/content/user_service.go | 46 +++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/internal/service/content/user_service.go b/internal/service/content/user_service.go index b59790c11..f53d2ff4c 100644 --- a/internal/service/content/user_service.go +++ b/internal/service/content/user_service.go @@ -23,9 +23,10 @@ import ( "context" "encoding/json" "fmt" + "time" + "github.com/apache/answer/internal/service/event_queue" "github.com/apache/answer/pkg/token" - "time" "github.com/apache/answer/internal/base/constant" questioncommon "github.com/apache/answer/internal/service/question_common" @@ -41,6 +42,7 @@ import ( "github.com/apache/answer/internal/service/activity_common" "github.com/apache/answer/internal/service/auth" "github.com/apache/answer/internal/service/export" + "github.com/apache/answer/internal/service/file_record" "github.com/apache/answer/internal/service/role" "github.com/apache/answer/internal/service/siteinfo_common" usercommon "github.com/apache/answer/internal/service/user_common" @@ -67,6 +69,7 @@ type UserService struct { userNotificationConfigService *user_notification_config.UserNotificationConfigService questionService *questioncommon.QuestionCommon eventQueueService event_queue.EventQueueService + fileRecordService *file_record.FileRecordService } func NewUserService(userRepo usercommon.UserRepo, @@ -82,6 +85,7 @@ func NewUserService(userRepo usercommon.UserRepo, userNotificationConfigService *user_notification_config.UserNotificationConfigService, questionService *questioncommon.QuestionCommon, eventQueueService event_queue.EventQueueService, + fileRecordService *file_record.FileRecordService, ) *UserService { return &UserService{ userCommonService: userCommonService, @@ -97,6 +101,7 @@ func NewUserService(userRepo usercommon.UserRepo, userNotificationConfigService: userNotificationConfigService, questionService: questionService, eventQueueService: eventQueueService, + fileRecordService: fileRecordService, } } @@ -355,6 +360,9 @@ func (us *UserService) UpdateInfo(ctx context.Context, req *schema.UpdateInfoReq } cond := us.formatUserInfoForUpdateInfo(oldUserInfo, req, siteUsers) + + us.cleanUpRemovedAvatar(ctx, oldUserInfo.Avatar, cond.Avatar) + err = us.userRepo.UpdateInfo(ctx, cond) if err != nil { return nil, err @@ -363,6 +371,42 @@ func (us *UserService) UpdateInfo(ctx context.Context, req *schema.UpdateInfoReq return nil, err } +func (us *UserService) cleanUpRemovedAvatar( + ctx context.Context, + oldAvatarJSON string, + newAvatarJSON string, +) { + if oldAvatarJSON == newAvatarJSON { + return + } + + var oldAvatar, newAvatar struct { + Type string `json:"type"` + Custom string `json:"custom"` + } + + _ = json.Unmarshal([]byte(oldAvatarJSON), &oldAvatar) + _ = json.Unmarshal([]byte(newAvatarJSON), &newAvatar) + + // clean up if old is custom and it's either removed or replaced + if oldAvatar.Type == "custom" && (newAvatar.Type != "custom" || oldAvatar.Custom != newAvatar.Custom) { + if oldAvatar.Custom != "" { + fileRecord, err := us.fileRecordService.GetFileRecordByURL(ctx, oldAvatar.Custom) + if err != nil { + log.Error(err) + return + } + if fileRecord == nil { + log.Warn("no file record found for old avatar url:", oldAvatar.Custom) + return + } + if err := us.fileRecordService.DeleteAndMoveFileRecord(ctx, fileRecord); err != nil { + log.Error(err) + } + } + } +} + func (us *UserService) formatUserInfoForUpdateInfo( oldUserInfo *entity.User, req *schema.UpdateInfoRequest, siteUsersConf *schema.SiteUsersResp) *entity.User { avatar, _ := json.Marshal(req.Avatar) From 4042231c7b7b7c83d3f44468fb9ee2482c2b9bd7 Mon Sep 17 00:00:00 2001 From: broccoli Date: Thu, 15 May 2025 15:26:20 +0200 Subject: [PATCH 5/7] fix: improved error handlng, using existing schema + using existing AvatarInfo schema instead of redundant schema + improved error handling, so that the branding information is always being updated, even if its not possible to remove the old files. + added updated wire_gen.go file --- cmd/wire_gen.go | 8 ++++---- internal/controller_admin/siteinfo_controller.go | 16 ++++++++++------ internal/service/content/user_service.go | 5 +---- internal/service/siteinfo/siteinfo_service.go | 13 +++++++++---- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index 702f60df3..69a3113f4 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -181,7 +181,9 @@ func initApplication(debug bool, serverConf *conf.Server, dbConf *data.Database, metaCommonService := metacommon.NewMetaCommonService(metaRepo) questionCommon := questioncommon.NewQuestionCommon(questionRepo, answerRepo, voteRepo, followRepo, tagCommonService, userCommon, collectionCommon, answerCommon, metaCommonService, configService, activityQueueService, revisionRepo, siteInfoCommonService, dataData) eventQueueService := event_queue.NewEventQueueService() - userService := content.NewUserService(userRepo, userActiveActivityRepo, activityRepo, emailService, authService, siteInfoCommonService, userRoleRelService, userCommon, userExternalLoginService, userNotificationConfigRepo, userNotificationConfigService, questionCommon, eventQueueService) + fileRecordRepo := file_record.NewFileRecordRepo(dataData) + fileRecordService := file_record2.NewFileRecordService(fileRecordRepo, revisionRepo, serviceConf, siteInfoCommonService) + userService := content.NewUserService(userRepo, userActiveActivityRepo, activityRepo, emailService, authService, siteInfoCommonService, userRoleRelService, userCommon, userExternalLoginService, userNotificationConfigRepo, userNotificationConfigService, questionCommon, eventQueueService, fileRecordService) captchaRepo := captcha.NewCaptchaRepo(dataData) captchaService := action.NewCaptchaService(captchaRepo) userController := controller.NewUserController(authService, userService, captchaService, emailService, siteInfoCommonService, userNotificationConfigService) @@ -239,7 +241,7 @@ func initApplication(debug bool, serverConf *conf.Server, dbConf *data.Database, reasonService := reason2.NewReasonService(reasonRepo) reasonController := controller.NewReasonController(reasonService) themeController := controller_admin.NewThemeController() - siteInfoService := siteinfo.NewSiteInfoService(siteInfoRepo, siteInfoCommonService, emailService, tagCommonService, configService, questionCommon) + siteInfoService := siteinfo.NewSiteInfoService(siteInfoRepo, siteInfoCommonService, emailService, tagCommonService, configService, questionCommon, fileRecordService) siteInfoController := controller_admin.NewSiteInfoController(siteInfoService) controllerSiteInfoController := controller.NewSiteInfoController(siteInfoCommonService) notificationCommon := notificationcommon.NewNotificationCommon(dataData, notificationRepo, userCommon, activityRepo, followRepo, objService, notificationQueueService, userExternalLoginRepo, siteInfoCommonService) @@ -248,8 +250,6 @@ func initApplication(debug bool, serverConf *conf.Server, dbConf *data.Database, notificationController := controller.NewNotificationController(notificationService, rankService) dashboardService := dashboard.NewDashboardService(questionRepo, answerRepo, commentCommonRepo, voteRepo, userRepo, reportRepo, configService, siteInfoCommonService, serviceConf, reviewService, revisionRepo, dataData) dashboardController := controller.NewDashboardController(dashboardService) - fileRecordRepo := file_record.NewFileRecordRepo(dataData) - fileRecordService := file_record2.NewFileRecordService(fileRecordRepo, revisionRepo, serviceConf, siteInfoCommonService) uploaderService := uploader.NewUploaderService(serviceConf, siteInfoCommonService, fileRecordService) uploadController := controller.NewUploadController(uploaderService) activityActivityRepo := activity.NewActivityRepo(dataData, configService) diff --git a/internal/controller_admin/siteinfo_controller.go b/internal/controller_admin/siteinfo_controller.go index b3af34f43..8a92daba3 100644 --- a/internal/controller_admin/siteinfo_controller.go +++ b/internal/controller_admin/siteinfo_controller.go @@ -275,13 +275,17 @@ func (sc *SiteInfoController) UpdateBranding(ctx *gin.Context) { if handler.BindAndCheck(ctx, req) { return } - currentBranding, _ := sc.siteInfoService.GetSiteBranding(ctx) - err := sc.siteInfoService.CleanUpRemovedBrandingFiles(ctx, req, currentBranding) - if err != nil { - log.Error(err) + currentBranding, getBrandingErr := sc.siteInfoService.GetSiteBranding(ctx) + if getBrandingErr == nil { + cleanUpErr := sc.siteInfoService.CleanUpRemovedBrandingFiles(ctx, req, currentBranding) + if cleanUpErr != nil { + log.Errorf("failed to clean up removed branding file(s): %v", cleanUpErr) + } + } else { + log.Errorf("failed to get current site branding: %v", getBrandingErr) } - err = sc.siteInfoService.SaveSiteBranding(ctx, req) - handler.HandleResponse(ctx, err, nil) + saveErr := sc.siteInfoService.SaveSiteBranding(ctx, req) + handler.HandleResponse(ctx, saveErr, nil) } // UpdateSiteWrite update site write info diff --git a/internal/service/content/user_service.go b/internal/service/content/user_service.go index f53d2ff4c..8f6a724cc 100644 --- a/internal/service/content/user_service.go +++ b/internal/service/content/user_service.go @@ -380,10 +380,7 @@ func (us *UserService) cleanUpRemovedAvatar( return } - var oldAvatar, newAvatar struct { - Type string `json:"type"` - Custom string `json:"custom"` - } + var oldAvatar, newAvatar schema.AvatarInfo _ = json.Unmarshal([]byte(oldAvatarJSON), &oldAvatar) _ = json.Unmarshal([]byte(newAvatarJSON), &newAvatar) diff --git a/internal/service/siteinfo/siteinfo_service.go b/internal/service/siteinfo/siteinfo_service.go index 3c7c99b92..a0f4891c4 100644 --- a/internal/service/siteinfo/siteinfo_service.go +++ b/internal/service/siteinfo/siteinfo_service.go @@ -22,6 +22,7 @@ package siteinfo import ( "context" "encoding/json" + errpkg "errors" "fmt" "strings" @@ -448,6 +449,7 @@ func (s *SiteInfoService) CleanUpRemovedBrandingFiles( newBranding *schema.SiteBrandingReq, currentBranding *schema.SiteBrandingResp, ) error { + var allErrors []error currentFiles := map[string]string{ "logo": currentBranding.Logo, "mobile_logo": currentBranding.MobileLogo, @@ -467,18 +469,21 @@ func (s *SiteInfoService) CleanUpRemovedBrandingFiles( if currentFile != "" && currentFile != newFile { fileRecord, err := s.fileRecordService.GetFileRecordByURL(ctx, currentFile) if err != nil { - log.Error(err) + allErrors = append(allErrors, err) continue } if fileRecord == nil { - log.Error("could not fetch file record by url") + err := errpkg.New("file record is nil for key " + key) + allErrors = append(allErrors, err) continue } if err := s.fileRecordService.DeleteAndMoveFileRecord(ctx, fileRecord); err != nil { - log.Error(err) + allErrors = append(allErrors, err) } } } - + if len(allErrors) > 0 { + return errpkg.Join(allErrors...) + } return nil } From 803bc0faecdb2c45ccd6e2519813b1841bfaaa9d Mon Sep 17 00:00:00 2001 From: broccoli Date: Fri, 16 May 2025 23:11:11 +0200 Subject: [PATCH 6/7] fix: orphaned avatar and branding files are being deleted + added logic that removed orphaned within the clean up job. Files may be orphaned if a user uploads a file, but never saves it (registering it with the user profile or site info). + if a user switches avatars, for example from a customized avatar to the system default avatar, the file is not deleted anymore. --- cmd/wire_gen.go | 2 +- internal/repo/site_info/siteinfo_repo.go | 15 ++++++++++ internal/repo/user/user_repo.go | 15 ++++++++++ internal/service/content/user_service.go | 30 ++++++++++--------- .../file_record/file_record_service.go | 16 ++++++++++ .../siteinfo_common/siteinfo_service.go | 12 ++++++++ internal/service/user_common/user.go | 11 +++++++ 7 files changed, 86 insertions(+), 15 deletions(-) diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index 69a3113f4..f98deb424 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -182,7 +182,7 @@ func initApplication(debug bool, serverConf *conf.Server, dbConf *data.Database, questionCommon := questioncommon.NewQuestionCommon(questionRepo, answerRepo, voteRepo, followRepo, tagCommonService, userCommon, collectionCommon, answerCommon, metaCommonService, configService, activityQueueService, revisionRepo, siteInfoCommonService, dataData) eventQueueService := event_queue.NewEventQueueService() fileRecordRepo := file_record.NewFileRecordRepo(dataData) - fileRecordService := file_record2.NewFileRecordService(fileRecordRepo, revisionRepo, serviceConf, siteInfoCommonService) + fileRecordService := file_record2.NewFileRecordService(fileRecordRepo, revisionRepo, serviceConf, siteInfoCommonService, userCommon) userService := content.NewUserService(userRepo, userActiveActivityRepo, activityRepo, emailService, authService, siteInfoCommonService, userRoleRelService, userCommon, userExternalLoginService, userNotificationConfigRepo, userNotificationConfigService, questionCommon, eventQueueService, fileRecordService) captchaRepo := captcha.NewCaptchaRepo(dataData) captchaService := action.NewCaptchaService(captchaRepo) diff --git a/internal/repo/site_info/siteinfo_repo.go b/internal/repo/site_info/siteinfo_repo.go index 420e483f3..5f95b7486 100644 --- a/internal/repo/site_info/siteinfo_repo.go +++ b/internal/repo/site_info/siteinfo_repo.go @@ -101,3 +101,18 @@ func (sr *siteInfoRepo) setCache(ctx context.Context, siteType string, siteInfo log.Error(err) } } + +func (sr *siteInfoRepo) IsBrandingFileUsed(ctx context.Context, filePath string) (bool, error) { + siteInfo := &entity.SiteInfo{} + count, err := sr.data.DB.Context(ctx). + Table("site_info"). + Where(builder.Eq{"type": "branding"}). + And(builder.Like{"content", "%" + filePath + "%"}). + Count(&siteInfo) + + if err != nil { + return false, errors.InternalServer(reason.DatabaseError).WithError(err).WithStack() + } + + return count > 0, nil +} diff --git a/internal/repo/user/user_repo.go b/internal/repo/user/user_repo.go index f56e00b89..a85cd79a1 100644 --- a/internal/repo/user/user_repo.go +++ b/internal/repo/user/user_repo.go @@ -33,6 +33,7 @@ import ( "github.com/apache/answer/plugin" "github.com/segmentfault/pacman/errors" "github.com/segmentfault/pacman/log" + "xorm.io/builder" "xorm.io/xorm" ) @@ -380,3 +381,17 @@ func decorateByUserCenterUser(original *entity.User, ucUser *plugin.UserCenterBa original.Status = int(ucUser.Status) } } + +func (ur *userRepo) IsAvatarFileUsed(ctx context.Context, filePath string) (bool, error) { + user := &entity.User{} + count, err := ur.data.DB.Context(ctx). + Table("user"). + Where(builder.Like{"avatar", "%" + filePath + "%"}). + Count(&user) + + if err != nil { + return false, errors.InternalServer(reason.DatabaseError).WithError(err).WithStack() + } + + return count > 0, nil +} diff --git a/internal/service/content/user_service.go b/internal/service/content/user_service.go index 8f6a724cc..ece3a86de 100644 --- a/internal/service/content/user_service.go +++ b/internal/service/content/user_service.go @@ -385,21 +385,23 @@ func (us *UserService) cleanUpRemovedAvatar( _ = json.Unmarshal([]byte(oldAvatarJSON), &oldAvatar) _ = json.Unmarshal([]byte(newAvatarJSON), &newAvatar) + if len(oldAvatar.Custom) == 0 { + return + } + // clean up if old is custom and it's either removed or replaced - if oldAvatar.Type == "custom" && (newAvatar.Type != "custom" || oldAvatar.Custom != newAvatar.Custom) { - if oldAvatar.Custom != "" { - fileRecord, err := us.fileRecordService.GetFileRecordByURL(ctx, oldAvatar.Custom) - if err != nil { - log.Error(err) - return - } - if fileRecord == nil { - log.Warn("no file record found for old avatar url:", oldAvatar.Custom) - return - } - if err := us.fileRecordService.DeleteAndMoveFileRecord(ctx, fileRecord); err != nil { - log.Error(err) - } + if oldAvatar.Custom != newAvatar.Custom { + fileRecord, err := us.fileRecordService.GetFileRecordByURL(ctx, oldAvatar.Custom) + if err != nil { + log.Error(err) + return + } + if fileRecord == nil { + log.Warn("no file record found for old avatar url:", oldAvatar.Custom) + return + } + if err := us.fileRecordService.DeleteAndMoveFileRecord(ctx, fileRecord); err != nil { + log.Error(err) } } } diff --git a/internal/service/file_record/file_record_service.go b/internal/service/file_record/file_record_service.go index e9ea1c24c..29097ba8c 100644 --- a/internal/service/file_record/file_record_service.go +++ b/internal/service/file_record/file_record_service.go @@ -32,6 +32,7 @@ import ( "github.com/apache/answer/internal/service/revision" "github.com/apache/answer/internal/service/service_config" "github.com/apache/answer/internal/service/siteinfo_common" + usercommon "github.com/apache/answer/internal/service/user_common" "github.com/apache/answer/pkg/checker" "github.com/apache/answer/pkg/dir" "github.com/apache/answer/pkg/writer" @@ -54,6 +55,7 @@ type FileRecordService struct { revisionRepo revision.RevisionRepo serviceConfig *service_config.ServiceConfig siteInfoService siteinfo_common.SiteInfoCommonService + userService *usercommon.UserCommon } // NewFileRecordService new file record service @@ -62,12 +64,14 @@ func NewFileRecordService( revisionRepo revision.RevisionRepo, serviceConfig *service_config.ServiceConfig, siteInfoService siteinfo_common.SiteInfoCommonService, + userService *usercommon.UserCommon, ) *FileRecordService { return &FileRecordService{ fileRecordRepo: fileRecordRepo, revisionRepo: revisionRepo, serviceConfig: serviceConfig, siteInfoService: siteInfoService, + userService: userService, } } @@ -107,6 +111,18 @@ func (fs *FileRecordService) CleanOrphanUploadFiles(ctx context.Context) { continue } if isBrandingOrAvatarFile(fileRecord.FilePath) { + if strings.Contains(fileRecord.FilePath, constant.BrandingSubPath+"/") { + if fs.siteInfoService.IsBrandingFileUsed(ctx, fileRecord.FilePath) { + continue + } + } else if strings.Contains(fileRecord.FilePath, constant.AvatarSubPath+"/") { + if fs.userService.IsAvatarFileUsed(ctx, fileRecord.FilePath) { + continue + } + } + if err := fs.DeleteAndMoveFileRecord(ctx, fileRecord); err != nil { + log.Error(err) + } continue } if checker.IsNotZeroString(fileRecord.ObjectID) { diff --git a/internal/service/siteinfo_common/siteinfo_service.go b/internal/service/siteinfo_common/siteinfo_service.go index f715bc9b0..0c896c2b0 100644 --- a/internal/service/siteinfo_common/siteinfo_service.go +++ b/internal/service/siteinfo_common/siteinfo_service.go @@ -35,6 +35,7 @@ import ( type SiteInfoRepo interface { SaveByType(ctx context.Context, siteType string, data *entity.SiteInfo) (err error) GetByType(ctx context.Context, siteType string) (siteInfo *entity.SiteInfo, exist bool, err error) + IsBrandingFileUsed(ctx context.Context, filePath string) (bool, error) } // siteInfoCommonService site info common service @@ -56,6 +57,7 @@ type SiteInfoCommonService interface { GetSiteTheme(ctx context.Context) (resp *schema.SiteThemeResp, err error) GetSiteSeo(ctx context.Context) (resp *schema.SiteSeoResp, err error) GetSiteInfoByType(ctx context.Context, siteType string, resp interface{}) (err error) + IsBrandingFileUsed(ctx context.Context, filePath string) bool } // NewSiteInfoCommonService new site info common service @@ -233,3 +235,13 @@ func (s *siteInfoCommonService) GetSiteInfoByType(ctx context.Context, siteType _ = json.Unmarshal([]byte(siteInfo.Content), resp) return nil } + +func (s *siteInfoCommonService) IsBrandingFileUsed(ctx context.Context, filePath string) bool { + used, err := s.siteInfoRepo.IsBrandingFileUsed(ctx, filePath) + if err != nil { + log.Errorf("error checking if branding file is used: %v", err) + // will try again with the next clean up + return true + } + return used +} diff --git a/internal/service/user_common/user.go b/internal/service/user_common/user.go index 7f50eafe5..3df99261d 100644 --- a/internal/service/user_common/user.go +++ b/internal/service/user_common/user.go @@ -60,6 +60,7 @@ type UserRepo interface { GetByEmail(ctx context.Context, email string) (userInfo *entity.User, exist bool, err error) GetUserCount(ctx context.Context) (count int64, err error) SearchUserListByName(ctx context.Context, name string, limit int, onlyStaff bool) (userList []*entity.User, err error) + IsAvatarFileUsed(ctx context.Context, filePath string) (bool, error) } // UserCommon user service @@ -245,3 +246,13 @@ func (us *UserCommon) CacheLoginUserInfo(ctx context.Context, userID string, use } return accessToken, userCacheInfo, nil } + +func (us *UserCommon) IsAvatarFileUsed(ctx context.Context, filePath string) bool { + used, err := us.userRepo.IsAvatarFileUsed(ctx, filePath) + if err != nil { + log.Errorf("error checking if branding file is used: %v", err) + // will try again with the next clean up + return true + } + return used +} From 55dd8e330f4752abc001d6a8e08c70e02702f559 Mon Sep 17 00:00:00 2001 From: broccoli Date: Thu, 22 May 2025 09:21:21 +0200 Subject: [PATCH 7/7] fix: adjusted error handling --- internal/base/middleware/avatar.go | 4 +++- internal/service/uploader/upload.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/base/middleware/avatar.go b/internal/base/middleware/avatar.go index 1d2464173..98430638b 100644 --- a/internal/base/middleware/avatar.go +++ b/internal/base/middleware/avatar.go @@ -21,6 +21,7 @@ package middleware import ( "fmt" + "net/http" "net/url" "os" "path" @@ -62,7 +63,8 @@ func (am *AvatarMiddleware) AvatarThumb() gin.HandlerFunc { filePath, err = am.uploaderService.AvatarThumbFile(ctx, filename, size) if err != nil { log.Error(err) - ctx.Abort() + ctx.AbortWithStatus(http.StatusNotFound) + return } } avatarFile, err := os.ReadFile(filePath) diff --git a/internal/service/uploader/upload.go b/internal/service/uploader/upload.go index a2c9b98a1..2ae5369df 100644 --- a/internal/service/uploader/upload.go +++ b/internal/service/uploader/upload.go @@ -155,7 +155,7 @@ func (us *uploaderService) AvatarThumbFile(ctx *gin.Context, fileName string, si filePath := fmt.Sprintf("%s/%s/%s", us.serviceConfig.UploadPath, constant.AvatarSubPath, fileName) avatarFile, err = os.ReadFile(filePath) if err != nil { - return "", errors.InternalServer(reason.UnknownError).WithError(err).WithStack() + return "", errors.NotFound(reason.UnknownError).WithError(err) } reader := bytes.NewReader(avatarFile) img, err := imaging.Decode(reader)