mirror of
				https://github.com/go-gitea/gitea.git
				synced 2025-10-31 10:56:10 +01:00 
			
		
		
		
	Fix a bug where lfs gc never worked. (#35198)
Fix #31113 After #22385 introduced LFS GC, it never worked due to a bug in the INI library: fields in structs embedded more than one level deep are not populated from the INI file. This PR fixes the issue by replacing the multi-level embedded struct with a single-level struct for parsing the cron.gc_lfs configuration. Added a new test for retrieving cron settings to demonstrate the bug in the INI package.
This commit is contained in:
		| @@ -41,3 +41,56 @@ EXTEND = true | |||||||
| 	assert.Equal(t, "white rabbit", extended.Second) | 	assert.Equal(t, "white rabbit", extended.Second) | ||||||
| 	assert.True(t, extended.Extend) | 	assert.True(t, extended.Extend) | ||||||
| } | } | ||||||
|  |  | ||||||
|  | // Test_getCronSettings2 tests that getCronSettings can not handle two levels of embedding | ||||||
|  | func Test_getCronSettings2(t *testing.T) { | ||||||
|  | 	type BaseStruct struct { | ||||||
|  | 		Enabled    bool | ||||||
|  | 		RunAtStart bool | ||||||
|  | 		Schedule   string | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	type Extended struct { | ||||||
|  | 		BaseStruct | ||||||
|  | 		Extend bool | ||||||
|  | 	} | ||||||
|  | 	type Extended2 struct { | ||||||
|  | 		Extended | ||||||
|  | 		Third string | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	iniStr := ` | ||||||
|  | [cron.test] | ||||||
|  | ENABLED = TRUE | ||||||
|  | RUN_AT_START = TRUE | ||||||
|  | SCHEDULE = @every 1h | ||||||
|  | EXTEND = true | ||||||
|  | THIRD = white rabbit | ||||||
|  | ` | ||||||
|  | 	cfg, err := NewConfigProviderFromData(iniStr) | ||||||
|  | 	assert.NoError(t, err) | ||||||
|  |  | ||||||
|  | 	extended := &Extended2{ | ||||||
|  | 		Extended: Extended{ | ||||||
|  | 			BaseStruct: BaseStruct{ | ||||||
|  | 				Enabled:    false, | ||||||
|  | 				RunAtStart: false, | ||||||
|  | 				Schedule:   "@every 72h", | ||||||
|  | 			}, | ||||||
|  | 			Extend: false, | ||||||
|  | 		}, | ||||||
|  | 		Third: "black rabbit", | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	_, err = getCronSettings(cfg, "test", extended) | ||||||
|  | 	assert.NoError(t, err) | ||||||
|  |  | ||||||
|  | 	// This confirms the first level of embedding works | ||||||
|  | 	assert.Equal(t, "white rabbit", extended.Third) | ||||||
|  | 	assert.True(t, extended.Extend) | ||||||
|  |  | ||||||
|  | 	// This confirms 2 levels of embedding doesn't work | ||||||
|  | 	assert.False(t, extended.Enabled) | ||||||
|  | 	assert.False(t, extended.RunAtStart) | ||||||
|  | 	assert.Equal(t, "@every 72h", extended.Schedule) | ||||||
|  | } | ||||||
|   | |||||||
| @@ -171,19 +171,20 @@ func registerDeleteOldSystemNotices() { | |||||||
| 	}) | 	}) | ||||||
| } | } | ||||||
|  |  | ||||||
|  | type GCLFSConfig struct { | ||||||
|  | 	BaseConfig | ||||||
|  | 	OlderThan                time.Duration | ||||||
|  | 	LastUpdatedMoreThanAgo   time.Duration | ||||||
|  | 	NumberToCheckPerRepo     int64 | ||||||
|  | 	ProportionToCheckPerRepo float64 | ||||||
|  | } | ||||||
|  |  | ||||||
| func registerGCLFS() { | func registerGCLFS() { | ||||||
| 	if !setting.LFS.StartServer { | 	if !setting.LFS.StartServer { | ||||||
| 		return | 		return | ||||||
| 	} | 	} | ||||||
| 	type GCLFSConfig struct { |  | ||||||
| 		OlderThanConfig |  | ||||||
| 		LastUpdatedMoreThanAgo   time.Duration |  | ||||||
| 		NumberToCheckPerRepo     int64 |  | ||||||
| 		ProportionToCheckPerRepo float64 |  | ||||||
| 	} |  | ||||||
|  |  | ||||||
| 	RegisterTaskFatal("gc_lfs", &GCLFSConfig{ | 	RegisterTaskFatal("gc_lfs", &GCLFSConfig{ | ||||||
| 		OlderThanConfig: OlderThanConfig{ |  | ||||||
| 		BaseConfig: BaseConfig{ | 		BaseConfig: BaseConfig{ | ||||||
| 			Enabled:    false, | 			Enabled:    false, | ||||||
| 			RunAtStart: false, | 			RunAtStart: false, | ||||||
| @@ -198,7 +199,7 @@ func registerGCLFS() { | |||||||
| 		// It is likely that a week is potentially excessive but it should definitely be enough that any | 		// It is likely that a week is potentially excessive but it should definitely be enough that any | ||||||
| 		// unassociated LFS object is genuinely unassociated. | 		// unassociated LFS object is genuinely unassociated. | ||||||
| 		OlderThan: 24 * time.Hour * 7, | 		OlderThan: 24 * time.Hour * 7, | ||||||
| 		}, |  | ||||||
| 		// Only GC things that haven't been looked at in the past 3 days | 		// Only GC things that haven't been looked at in the past 3 days | ||||||
| 		LastUpdatedMoreThanAgo:   24 * time.Hour * 3, | 		LastUpdatedMoreThanAgo:   24 * time.Hour * 3, | ||||||
| 		NumberToCheckPerRepo:     100, | 		NumberToCheckPerRepo:     100, | ||||||
|   | |||||||
							
								
								
									
										51
									
								
								services/cron/tasks_extended_test.go
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										51
									
								
								services/cron/tasks_extended_test.go
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,51 @@ | |||||||
|  | // Copyright 2025 The Gitea Authors. All rights reserved. | ||||||
|  | // SPDX-License-Identifier: MIT | ||||||
|  |  | ||||||
|  | package cron | ||||||
|  |  | ||||||
|  | import ( | ||||||
|  | 	"testing" | ||||||
|  | 	"time" | ||||||
|  |  | ||||||
|  | 	"code.gitea.io/gitea/modules/setting" | ||||||
|  | 	"code.gitea.io/gitea/modules/test" | ||||||
|  |  | ||||||
|  | 	"github.com/stretchr/testify/assert" | ||||||
|  | ) | ||||||
|  |  | ||||||
|  | func Test_GCLFSConfig(t *testing.T) { | ||||||
|  | 	cfg, err := setting.NewConfigProviderFromData(` | ||||||
|  | [cron.gc_lfs] | ||||||
|  | ENABLED = true | ||||||
|  | RUN_AT_START = true | ||||||
|  | SCHEDULE = "@every 2h" | ||||||
|  | OLDER_THAN = "1h" | ||||||
|  | LAST_UPDATED_MORE_THAN_AGO = "7h" | ||||||
|  | NUMBER_TO_CHECK_PER_REPO = 10 | ||||||
|  | PROPORTION_TO_CHECK_PER_REPO = 0.1 | ||||||
|  | `) | ||||||
|  | 	assert.NoError(t, err) | ||||||
|  | 	defer test.MockVariableValue(&setting.CfgProvider, cfg)() | ||||||
|  |  | ||||||
|  | 	config := &GCLFSConfig{ | ||||||
|  | 		BaseConfig: BaseConfig{ | ||||||
|  | 			Enabled:    false, | ||||||
|  | 			RunAtStart: false, | ||||||
|  | 			Schedule:   "@every 24h", | ||||||
|  | 		}, | ||||||
|  | 		OlderThan:                24 * time.Hour * 7, | ||||||
|  | 		LastUpdatedMoreThanAgo:   24 * time.Hour * 3, | ||||||
|  | 		NumberToCheckPerRepo:     100, | ||||||
|  | 		ProportionToCheckPerRepo: 0.6, | ||||||
|  | 	} | ||||||
|  |  | ||||||
|  | 	_, err = setting.GetCronSettings("gc_lfs", config) | ||||||
|  | 	assert.NoError(t, err) | ||||||
|  | 	assert.True(t, config.Enabled) | ||||||
|  | 	assert.True(t, config.RunAtStart) | ||||||
|  | 	assert.Equal(t, "@every 2h", config.Schedule) | ||||||
|  | 	assert.Equal(t, 1*time.Hour, config.OlderThan) | ||||||
|  | 	assert.Equal(t, 7*time.Hour, config.LastUpdatedMoreThanAgo) | ||||||
|  | 	assert.Equal(t, int64(10), config.NumberToCheckPerRepo) | ||||||
|  | 	assert.InDelta(t, 0.1, config.ProportionToCheckPerRepo, 0.001) | ||||||
|  | } | ||||||
		Reference in New Issue
	
	Block a user