Skip to content

Fix file record and delete files (branding and avatar) #1335

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
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
8 changes: 4 additions & 4 deletions cmd/wire_gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 12 additions & 2 deletions internal/controller_admin/siteinfo_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -274,8 +275,17 @@ func (sc *SiteInfoController) UpdateBranding(ctx *gin.Context) {
if handler.BindAndCheck(ctx, req) {
return
}
err := sc.siteInfoService.SaveSiteBranding(ctx, req)
handler.HandleResponse(ctx, err, nil)
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)
}
saveErr := sc.siteInfoService.SaveSiteBranding(ctx, req)
handler.HandleResponse(ctx, saveErr, nil)
}

// UpdateSiteWrite update site write info
Expand Down
1 change: 1 addition & 0 deletions internal/migrations/init_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ var (
&entity.Badge{},
&entity.BadgeGroup{},
&entity.BadgeAward{},
&entity.FileRecord{},
}

roles = []*entity.Role{
Expand Down
15 changes: 15 additions & 0 deletions internal/repo/file_record/file_record_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
15 changes: 15 additions & 0 deletions internal/repo/site_info/siteinfo_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
15 changes: 15 additions & 0 deletions internal/repo/user/user_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}
45 changes: 44 additions & 1 deletion internal/service/content/user_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -97,6 +101,7 @@ func NewUserService(userRepo usercommon.UserRepo,
userNotificationConfigService: userNotificationConfigService,
questionService: questionService,
eventQueueService: eventQueueService,
fileRecordService: fileRecordService,
}
}

Expand Down Expand Up @@ -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
Expand All @@ -363,6 +371,41 @@ 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 schema.AvatarInfo

_ = 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.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)
}
}
}

func (us *UserService) formatUserInfoForUpdateInfo(
oldUserInfo *entity.User, req *schema.UpdateInfoRequest, siteUsersConf *schema.SiteUsersResp) *entity.User {
avatar, _ := json.Marshal(req.Avatar)
Expand Down
38 changes: 36 additions & 2 deletions internal/service/file_record/file_record_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"time"

"github.com/apache/answer/internal/base/constant"
"github.com/apache/answer/internal/entity"
"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"
Expand All @@ -44,6 +46,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
Expand All @@ -52,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
Expand All @@ -60,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,
}
}

Expand Down Expand Up @@ -104,6 +110,21 @@ func (fs *FileRecordService) CleanOrphanUploadFiles(ctx context.Context) {
if fileRecord.CreatedAt.AddDate(0, 0, 2).After(time.Now()) {
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) {
_, exist, err := fs.revisionRepo.GetLastRevisionByObjectID(ctx, fileRecord.ObjectID)
if err != nil {
Expand All @@ -129,14 +150,18 @@ 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)
}
}
page++
}
}

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)
Expand All @@ -152,7 +177,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)
Expand All @@ -170,3 +195,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
}
Loading