Skip to content

Commit 339eeed

Browse files
author
Daniel Auer
committed
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.
1 parent d024853 commit 339eeed

File tree

7 files changed

+86
-15
lines changed

7 files changed

+86
-15
lines changed

cmd/wire_gen.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/repo/site_info/siteinfo_repo.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,3 +101,18 @@ func (sr *siteInfoRepo) setCache(ctx context.Context, siteType string, siteInfo
101101
log.Error(err)
102102
}
103103
}
104+
105+
func (sr *siteInfoRepo) IsBrandingFileUsed(ctx context.Context, filePath string) (bool, error) {
106+
siteInfo := &entity.SiteInfo{}
107+
count, err := sr.data.DB.Context(ctx).
108+
Table("site_info").
109+
Where(builder.Eq{"type": "branding"}).
110+
And(builder.Like{"content", "%" + filePath + "%"}).
111+
Count(&siteInfo)
112+
113+
if err != nil {
114+
return false, errors.InternalServer(reason.DatabaseError).WithError(err).WithStack()
115+
}
116+
117+
return count > 0, nil
118+
}

internal/repo/user/user_repo.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/apache/answer/plugin"
3434
"github.com/segmentfault/pacman/errors"
3535
"github.com/segmentfault/pacman/log"
36+
"xorm.io/builder"
3637
"xorm.io/xorm"
3738
)
3839

@@ -380,3 +381,17 @@ func decorateByUserCenterUser(original *entity.User, ucUser *plugin.UserCenterBa
380381
original.Status = int(ucUser.Status)
381382
}
382383
}
384+
385+
func (ur *userRepo) IsAvatarFileUsed(ctx context.Context, filePath string) (bool, error) {
386+
user := &entity.User{}
387+
count, err := ur.data.DB.Context(ctx).
388+
Table("user").
389+
Where(builder.Like{"avatar", "%" + filePath + "%"}).
390+
Count(&user)
391+
392+
if err != nil {
393+
return false, errors.InternalServer(reason.DatabaseError).WithError(err).WithStack()
394+
}
395+
396+
return count > 0, nil
397+
}

internal/service/content/user_service.go

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -385,21 +385,23 @@ func (us *UserService) cleanUpRemovedAvatar(
385385
_ = json.Unmarshal([]byte(oldAvatarJSON), &oldAvatar)
386386
_ = json.Unmarshal([]byte(newAvatarJSON), &newAvatar)
387387

388+
if len(oldAvatar.Custom) == 0 {
389+
return
390+
}
391+
388392
// clean up if old is custom and it's either removed or replaced
389-
if oldAvatar.Type == "custom" && (newAvatar.Type != "custom" || oldAvatar.Custom != newAvatar.Custom) {
390-
if oldAvatar.Custom != "" {
391-
fileRecord, err := us.fileRecordService.GetFileRecordByURL(ctx, oldAvatar.Custom)
392-
if err != nil {
393-
log.Error(err)
394-
return
395-
}
396-
if fileRecord == nil {
397-
log.Warn("no file record found for old avatar url:", oldAvatar.Custom)
398-
return
399-
}
400-
if err := us.fileRecordService.DeleteAndMoveFileRecord(ctx, fileRecord); err != nil {
401-
log.Error(err)
402-
}
393+
if oldAvatar.Custom != newAvatar.Custom {
394+
fileRecord, err := us.fileRecordService.GetFileRecordByURL(ctx, oldAvatar.Custom)
395+
if err != nil {
396+
log.Error(err)
397+
return
398+
}
399+
if fileRecord == nil {
400+
log.Warn("no file record found for old avatar url:", oldAvatar.Custom)
401+
return
402+
}
403+
if err := us.fileRecordService.DeleteAndMoveFileRecord(ctx, fileRecord); err != nil {
404+
log.Error(err)
403405
}
404406
}
405407
}

internal/service/file_record/file_record_service.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/apache/answer/internal/service/revision"
3333
"github.com/apache/answer/internal/service/service_config"
3434
"github.com/apache/answer/internal/service/siteinfo_common"
35+
usercommon "github.com/apache/answer/internal/service/user_common"
3536
"github.com/apache/answer/pkg/checker"
3637
"github.com/apache/answer/pkg/dir"
3738
"github.com/apache/answer/pkg/writer"
@@ -54,6 +55,7 @@ type FileRecordService struct {
5455
revisionRepo revision.RevisionRepo
5556
serviceConfig *service_config.ServiceConfig
5657
siteInfoService siteinfo_common.SiteInfoCommonService
58+
userService *usercommon.UserCommon
5759
}
5860

5961
// NewFileRecordService new file record service
@@ -62,12 +64,14 @@ func NewFileRecordService(
6264
revisionRepo revision.RevisionRepo,
6365
serviceConfig *service_config.ServiceConfig,
6466
siteInfoService siteinfo_common.SiteInfoCommonService,
67+
userService *usercommon.UserCommon,
6568
) *FileRecordService {
6669
return &FileRecordService{
6770
fileRecordRepo: fileRecordRepo,
6871
revisionRepo: revisionRepo,
6972
serviceConfig: serviceConfig,
7073
siteInfoService: siteInfoService,
74+
userService: userService,
7175
}
7276
}
7377

@@ -107,6 +111,18 @@ func (fs *FileRecordService) CleanOrphanUploadFiles(ctx context.Context) {
107111
continue
108112
}
109113
if isBrandingOrAvatarFile(fileRecord.FilePath) {
114+
if strings.Contains(fileRecord.FilePath, constant.BrandingSubPath+"/") {
115+
if fs.siteInfoService.IsBrandingFileUsed(ctx, fileRecord.FilePath) {
116+
continue
117+
}
118+
} else if strings.Contains(fileRecord.FilePath, constant.AvatarSubPath+"/") {
119+
if fs.userService.IsAvatarFileUsed(ctx, fileRecord.FilePath) {
120+
continue
121+
}
122+
}
123+
if err := fs.DeleteAndMoveFileRecord(ctx, fileRecord); err != nil {
124+
log.Error(err)
125+
}
110126
continue
111127
}
112128
if checker.IsNotZeroString(fileRecord.ObjectID) {

internal/service/siteinfo_common/siteinfo_service.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
type SiteInfoRepo interface {
3636
SaveByType(ctx context.Context, siteType string, data *entity.SiteInfo) (err error)
3737
GetByType(ctx context.Context, siteType string) (siteInfo *entity.SiteInfo, exist bool, err error)
38+
IsBrandingFileUsed(ctx context.Context, filePath string) (bool, error)
3839
}
3940

4041
// siteInfoCommonService site info common service
@@ -56,6 +57,7 @@ type SiteInfoCommonService interface {
5657
GetSiteTheme(ctx context.Context) (resp *schema.SiteThemeResp, err error)
5758
GetSiteSeo(ctx context.Context) (resp *schema.SiteSeoResp, err error)
5859
GetSiteInfoByType(ctx context.Context, siteType string, resp interface{}) (err error)
60+
IsBrandingFileUsed(ctx context.Context, filePath string) bool
5961
}
6062

6163
// NewSiteInfoCommonService new site info common service
@@ -233,3 +235,13 @@ func (s *siteInfoCommonService) GetSiteInfoByType(ctx context.Context, siteType
233235
_ = json.Unmarshal([]byte(siteInfo.Content), resp)
234236
return nil
235237
}
238+
239+
func (s *siteInfoCommonService) IsBrandingFileUsed(ctx context.Context, filePath string) bool {
240+
used, err := s.siteInfoRepo.IsBrandingFileUsed(ctx, filePath)
241+
if err != nil {
242+
log.Errorf("error checking if branding file is used: %v", err)
243+
// will try again with the next clean up
244+
return true
245+
}
246+
return used
247+
}

internal/service/user_common/user.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ type UserRepo interface {
6060
GetByEmail(ctx context.Context, email string) (userInfo *entity.User, exist bool, err error)
6161
GetUserCount(ctx context.Context) (count int64, err error)
6262
SearchUserListByName(ctx context.Context, name string, limit int, onlyStaff bool) (userList []*entity.User, err error)
63+
IsAvatarFileUsed(ctx context.Context, filePath string) (bool, error)
6364
}
6465

6566
// UserCommon user service
@@ -245,3 +246,13 @@ func (us *UserCommon) CacheLoginUserInfo(ctx context.Context, userID string, use
245246
}
246247
return accessToken, userCacheInfo, nil
247248
}
249+
250+
func (us *UserCommon) IsAvatarFileUsed(ctx context.Context, filePath string) bool {
251+
used, err := us.userRepo.IsAvatarFileUsed(ctx, filePath)
252+
if err != nil {
253+
log.Errorf("error checking if branding file is used: %v", err)
254+
// will try again with the next clean up
255+
return true
256+
}
257+
return used
258+
}

0 commit comments

Comments
 (0)