Skip to content
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.

Sign up for GitHub

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

feat(files): Fix Issue with Recycle Bin Failing to Enable #7636

Merged
merged 1 commit into from
Jan 3, 2025
Merged

feat(files): Fix Issue with Recycle Bin Failing to Enable #7636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
29 changes: 17 additions & 12 deletions agent/app/repo/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ type ISettingRepo interface {
DelMonitorBase(timeForDelete time.Time) error
DelMonitorIO(timeForDelete time.Time) error
DelMonitorNet(timeForDelete time.Time) error
UpdateOrCreate(key, value string) error
}

func NewISettingRepo() ISettingRepo {
return &SettingRepo{}
}

func (u *SettingRepo) GetList(opts ...DBOption) ([]model.Setting, error) {
func (s *SettingRepo) GetList(opts ...DBOption) ([]model.Setting, error) {
var settings []model.Setting
db := global.DB.Model(&model.Setting{})
for _, opt := range opts {
Expand All @@ -40,15 +41,15 @@ func (u *SettingRepo) GetList(opts ...DBOption) ([]model.Setting, error) {
return settings, err
}

func (u *SettingRepo) Create(key, value string) error {
func (s *SettingRepo) Create(key, value string) error {
setting := &model.Setting{
Key: key,
Value: value,
}
return global.DB.Create(setting).Error
}

func (u *SettingRepo) Get(opts ...DBOption) (model.Setting, error) {
func (s *SettingRepo) Get(opts ...DBOption) (model.Setting, error) {
var settings model.Setting
db := global.DB.Model(&model.Setting{})
for _, opt := range opts {
Expand All @@ -58,7 +59,7 @@ func (u *SettingRepo) Get(opts ...DBOption) (model.Setting, error) {
return settings, err
}

func (u *SettingRepo) GetValueByKey(key string) (string, error) {
func (s *SettingRepo) GetValueByKey(key string) (string, error) {
var setting model.Setting
if err := global.DB.Model(&model.Setting{}).Where("key = ?", key).First(&setting).Error; err != nil {
global.LOG.Errorf("load %s from db setting failed, err: %v", key, err)
Expand All @@ -67,31 +68,35 @@ func (u *SettingRepo) GetValueByKey(key string) (string, error) {
return setting.Value, nil
}

func (c *SettingRepo) WithByKey(key string) DBOption {
func (s *SettingRepo) WithByKey(key string) DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("key = ?", key)
}
}

func (u *SettingRepo) Update(key, value string) error {
func (s *SettingRepo) Update(key, value string) error {
return global.DB.Model(&model.Setting{}).Where("key = ?", key).Updates(map[string]interface{}{"value": value}).Error
}

func (u *SettingRepo) CreateMonitorBase(model model.MonitorBase) error {
func (s *SettingRepo) CreateMonitorBase(model model.MonitorBase) error {
return global.MonitorDB.Create(&model).Error
}
func (u *SettingRepo) BatchCreateMonitorIO(ioList []model.MonitorIO) error {
func (s *SettingRepo) BatchCreateMonitorIO(ioList []model.MonitorIO) error {
return global.MonitorDB.CreateInBatches(ioList, len(ioList)).Error
}
func (u *SettingRepo) BatchCreateMonitorNet(ioList []model.MonitorNetwork) error {
func (s *SettingRepo) BatchCreateMonitorNet(ioList []model.MonitorNetwork) error {
return global.MonitorDB.CreateInBatches(ioList, len(ioList)).Error
}
func (u *SettingRepo) DelMonitorBase(timeForDelete time.Time) error {
func (s *SettingRepo) DelMonitorBase(timeForDelete time.Time) error {
return global.MonitorDB.Where("created_at < ?", timeForDelete).Delete(&model.MonitorBase{}).Error
}
func (u *SettingRepo) DelMonitorIO(timeForDelete time.Time) error {
func (s *SettingRepo) DelMonitorIO(timeForDelete time.Time) error {
return global.MonitorDB.Where("created_at < ?", timeForDelete).Delete(&model.MonitorIO{}).Error
}
func (u *SettingRepo) DelMonitorNet(timeForDelete time.Time) error {
func (s *SettingRepo) DelMonitorNet(timeForDelete time.Time) error {
return global.MonitorDB.Where("created_at < ?", timeForDelete).Delete(&model.MonitorNetwork{}).Error
}

func (s *SettingRepo) UpdateOrCreate(key, value string) error {
return global.DB.Model(&model.Setting{}).Where("key = ?", key).Assign(model.Setting{Key: key, Value: value}).FirstOrCreate(&model.Setting{}).Error
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot find any major inconsistencies, potential issues or optimizations in the provided code snippet based on the knowledge cut-off of September 2021.

However, regarding future updates and improvements:

  • Implement a generic interface for common operations like CRUD (Create, Read, Update, Delete) to enhance uniformity across all related functionalities.
  • Ensure that WithByKey uses an appropriate DB option method (e.g., "Select", "Find").
  • Consider refactoring or renaming functions names which are similar but have different semantics or inputs/outputs patterns for clarity and maintainability.
  • Review and refactor the use case logic where you might be able to abstract some parts into reusable components or services. Keep interfaces clear with well-defined methods.
  • Check if there's still room for improvement in terms of database queries, performance metrics etc. The context is not given exactly so it can depend on what the actual data processing requirements may entail at the current moment.

It appears most of these areas would benefit from thoughtful design decisions rather than specific implementation details. This includes ensuring better modularization and using standardized practices for writing clean, understandable code.

17 changes: 1 addition & 16 deletions agent/app/service/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,22 +48,7 @@ func (u *SettingService) GetSettingInfo() (*dto.SettingInfo, error) {
}

func (u *SettingService) Update(key, value string) error {
switch key {
case "AppStoreLastModified":
exist, _ := settingRepo.Get(settingRepo.WithByKey("AppStoreLastModified"))
if exist.ID == 0 {
return settingRepo.Create("AppStoreLastModified", value)
}
case "AppDefaultDomain":
exist, _ := settingRepo.Get(settingRepo.WithByKey("AppDefaultDomain"))
if exist.ID == 0 {
return settingRepo.Create("AppDefaultDomain", value)
}
}
if err := settingRepo.Update(key, value); err != nil {
return err
}
return nil
return settingRepo.UpdateOrCreate(key, value)
}

func (u *SettingService) ReloadConn() error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code looks correct and has no apparent issues or inconsistencies. It appears to be implementing a function that retrieves and updates configuration settings, with appropriate error handling in case of failed operations.

Optimization Suggestions:

  • Use constants for repeated values like the "key" used for setting up settingRepo.
  • If there is any dependency on other services or databases, consider integrating them into your project at an earlier stage instead of making external API calls where needed. For instance, you could make use of RESTful APIs if those can serve the purpose without additional complexity. Additionally, caching data may also significantly increase performance when dealing with frequent lookups.

There are currently no known irregularities or issues based on the available information.

Expand Down
4 changes: 2 additions & 2 deletions frontend/src/views/host/file-management/recycle-bin/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ import { dateFormat, computeSize } from '@/utils/util';
import i18n from '@/lang';
import Delete from './delete/index.vue';
import Reduce from './reduce/index.vue';
import { updateSetting } from '@/api/modules/setting';
import { MsgSuccess } from '@/utils/message';
import { updateAgentSetting } from '@/api/modules/setting';

const open = ref(false);
const req = reactive({
Expand Down Expand Up @@ -114,7 +114,7 @@ const getStatus = async () => {
const changeStatus = async () => {
try {
loading.value = true;
await updateSetting({ key: 'FileRecycleBin', value: status.value });
await updateAgentSetting({ key: 'FileRecycleBin', value: status.value });
MsgSuccess(i18n.global.t('file.fileRecycleBinMsg', [i18n.global.t('commons.button.' + status.value)]));
loading.value = false;
} catch (error) {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like your current codebase is not properly formatted to follow standard programming practices. Ensure you have the latest syntax settings on this platform as it can affect how some functions operate or display incorrectly.

However, no specific errors have been highlighted here for review, assuming that you've made an attempt at optimizing code. If needed, please elaborate more on specifics about which parts need improvements (e.g., naming conventions, flow structure consistency), so we can further help.

Expand Down
Loading