Skip to content

Commit e532ed8

Browse files
wxiaoguangwxiaoguang
committed
fix lint & comment & tests
1 parent 65540e6 commit e532ed8

4 files changed

Lines changed: 104 additions & 54 deletions

File tree

‎modules/util/path.go‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -117,37 +117,54 @@ func IsDir(dir string) (bool, error) {
117117

118118
var ErrNotRegularPathFile = errors.New("not a regular file")
119119

120+
// ReadRegularPathFile reads a file with given sub path in root dir.
121+
// It returns error when the path is not a regular file, or any parent path is not a regular directory.
120122
func ReadRegularPathFile(root, filePathIn string, limit int) ([]byte, error) {
121-
pathCleaned := PathJoinRelX(filePathIn)
122-
pathFields := strings.Split(pathCleaned, "/")
123-
targetPath := root
124-
for i := 0; i < len(pathFields); i++ {
125-
targetPath += string(filepath.Separator) + pathFields[i]
123+
pathFields := strings.Split(PathJoinRelX(filePathIn), "/")
124+
125+
targetPathBuilder := strings.Builder{}
126+
targetPathBuilder.Grow(len(root) + len(filePathIn) + 2)
127+
targetPathBuilder.WriteString(root)
128+
targetPathString := root
129+
for i, subPath := range pathFields {
130+
targetPathBuilder.WriteByte(filepath.Separator)
131+
targetPathBuilder.WriteString(subPath)
132+
targetPathString = targetPathBuilder.String()
133+
126134
expectFile := i == len(pathFields)-1
127-
st, err := os.Lstat(targetPath)
135+
st, err := os.Lstat(targetPathString)
128136
if err != nil {
129137
return nil, err
130138
}
131139
if expectFile && !st.Mode().IsRegular() || !expectFile && !st.Mode().IsDir() {
132140
return nil, fmt.Errorf("%w: %s", ErrNotRegularPathFile, filePathIn)
133141
}
134142
}
135-
f, err := os.Open(filepath.Join(root, pathCleaned))
143+
f, err := os.Open(targetPathString)
136144
if err != nil {
137145
return nil, err
138146
}
139147
defer f.Close()
140148
return ReadWithLimit(f, limit)
141149
}
142150

143-
func WriteRegularPathFile(root, filePathIn string, data []byte, dirMode, fileMode int) error {
144-
pathCleaned := PathJoinRelX(filePathIn)
145-
pathFields := strings.Split(pathCleaned, "/")
146-
targetPath := root
147-
for i := 0; i < len(pathFields); i++ {
148-
targetPath += string(filepath.Separator) + pathFields[i]
151+
// WriteRegularPathFile writes data to a file with given sub path in root dir, it creates parent directories if necessary.
152+
// The file is created with fileMode, and the directories are created with dirMode.
153+
// It returns error when the path already exists but is not a regular file, or any parent path is not a regular directory.
154+
func WriteRegularPathFile(root, filePathIn string, data []byte, dirMode, fileMode os.FileMode) error {
155+
pathFields := strings.Split(PathJoinRelX(filePathIn), "/")
156+
157+
targetPathBuilder := strings.Builder{}
158+
targetPathBuilder.Grow(len(root) + len(filePathIn) + 2)
159+
targetPathBuilder.WriteString(root)
160+
targetPathString := root
161+
for i, subPath := range pathFields {
162+
targetPathBuilder.WriteByte(filepath.Separator)
163+
targetPathBuilder.WriteString(subPath)
164+
targetPathString = targetPathBuilder.String()
165+
149166
expectFile := i == len(pathFields)-1
150-
st, err := os.Lstat(targetPath)
167+
st, err := os.Lstat(targetPathString)
151168
if err == nil {
152169
if expectFile && !st.Mode().IsRegular() || !expectFile && !st.Mode().IsDir() {
153170
return fmt.Errorf("%w: %s", ErrNotRegularPathFile, filePathIn)
@@ -158,12 +175,12 @@ func WriteRegularPathFile(root, filePathIn string, data []byte, dirMode, fileMod
158175
return err
159176
}
160177
if !expectFile {
161-
if err = os.Mkdir(targetPath, os.FileMode(dirMode)); err != nil {
178+
if err = os.Mkdir(targetPathString, dirMode); err != nil {
162179
return err
163180
}
164181
}
165182
}
166-
return os.WriteFile(targetPath, data, os.FileMode(fileMode))
183+
return os.WriteFile(targetPathString, data, fileMode)
167184
}
168185

169186
// IsExist checks whether a file or directory exists.

‎modules/util/path_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ func TestReadWriteRegularPathFile(t *testing.T) {
250250

251251
content, err = ReadRegularPathFile(rootDir, "../other-file", readLimit)
252252
require.ErrorIs(t, err, os.ErrNotExist)
253-
assert.Equal(t, "", string(content))
253+
assert.Empty(t, string(content))
254254

255255
content, err = ReadRegularPathFile(rootDir, "real-dir/real-file", readLimit)
256256
require.NoError(t, err)
@@ -294,6 +294,7 @@ func TestReadWriteRegularPathFile(t *testing.T) {
294294
assertFileContent(rootDir+"/other-file", "new-content")
295295

296296
err = WriteRegularPathFile(rootDir, "real-dir/real-file", []byte("changed-content"), 0o755, 0o644)
297+
require.NoError(t, err)
297298
assertFileContent(rootDir+"/real-dir/real-file", "changed-content")
298299
})
299300
}

‎services/repository/generate.go‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ func readGiteaTemplateFile(tmpDir string) (*giteaTemplateFileMatcher, error) {
151151
func substGiteaTemplateFile(ctx context.Context, tmpDir, tmpDirSubPath string, templateRepo, generateRepo *repo_model.Repository) error {
152152
content, err := util.ReadRegularPathFile(tmpDir, tmpDirSubPath, 1024*1024)
153153
if err != nil {
154-
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, util.ErrNotRegularPathFile) {
154+
if errors.Is(err, fs.ErrNotExist) {
155155
return nil
156156
}
157157
return err
@@ -162,20 +162,21 @@ func substGiteaTemplateFile(ctx context.Context, tmpDir, tmpDirSubPath string, t
162162

163163
generatedContent := generateExpansion(ctx, string(content), templateRepo, generateRepo)
164164
substSubPath := filePathSanitize(generateExpansion(ctx, tmpDirSubPath, templateRepo, generateRepo))
165-
err = util.WriteRegularPathFile(tmpDir, substSubPath, []byte(generatedContent), 0o755, 0o644)
166-
return util.Iif(errors.Is(err, util.ErrNotRegularPathFile), nil, err)
165+
return util.WriteRegularPathFile(tmpDir, substSubPath, []byte(generatedContent), 0o755, 0o644)
167166
}
168167

169-
func processGiteaTemplateFile(ctx context.Context, tmpDir string, templateRepo, generateRepo *repo_model.Repository, fileMatcher *giteaTemplateFileMatcher) error {
170-
if err := os.Remove(util.FilePathJoinAbs(tmpDir, fileMatcher.relPath)); err != nil {
171-
return fmt.Errorf("unable to remove .gitea/template: %w", err)
168+
func processGiteaTemplateFile(ctx context.Context, tmpDir string, templateRepo, generateRepo *repo_model.Repository, fileMatcher *giteaTemplateFileMatcher) (skippedFiles []string, err error) {
169+
if err = os.Remove(util.FilePathJoinAbs(tmpDir, fileMatcher.relPath)); err != nil {
170+
return nil, fmt.Errorf("unable to remove .gitea/template: %w", err)
172171
}
173172
if !fileMatcher.HasRules() {
174-
return nil // Avoid walking tree if there are no globs
173+
return skippedFiles, nil // Avoid walking tree if there are no globs
175174
}
176175

177-
defer util.RemoveAll(util.FilePathJoinAbs(tmpDir, ".git"))
178-
return filepath.WalkDir(tmpDir, func(fullPath string, d os.DirEntry, walkErr error) error {
176+
defer func() {
177+
_ = util.RemoveAll(util.FilePathJoinAbs(tmpDir, ".git"))
178+
}()
179+
err = filepath.WalkDir(tmpDir, func(fullPath string, d os.DirEntry, walkErr error) error {
179180
if walkErr != nil {
180181
return walkErr
181182
}
@@ -187,10 +188,16 @@ func processGiteaTemplateFile(ctx context.Context, tmpDir string, templateRepo,
187188
return err
188189
}
189190
if fileMatcher.Match(filepath.ToSlash(tmpDirSubPath)) {
190-
return substGiteaTemplateFile(ctx, tmpDir, tmpDirSubPath, templateRepo, generateRepo)
191+
err := substGiteaTemplateFile(ctx, tmpDir, tmpDirSubPath, templateRepo, generateRepo)
192+
if errors.Is(err, util.ErrNotRegularPathFile) {
193+
skippedFiles = append(skippedFiles, tmpDirSubPath)
194+
} else if err != nil {
195+
return err
196+
}
191197
}
192198
return nil
193199
}) // end: WalkDir
200+
return skippedFiles, err
194201
}
195202

196203
func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *repo_model.Repository, tmpDir string) error {
@@ -215,7 +222,7 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r
215222
// Variable expansion
216223
fileMatcher, err := readGiteaTemplateFile(tmpDir)
217224
if err == nil {
218-
err = processGiteaTemplateFile(ctx, tmpDir, templateRepo, generateRepo, fileMatcher)
225+
_, err = processGiteaTemplateFile(ctx, tmpDir, templateRepo, generateRepo, fileMatcher)
219226
if err != nil {
220227
return fmt.Errorf("processGiteaTemplateFile: %w", err)
221228
}

‎services/repository/generate_test.go‎

Lines changed: 51 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func TestFilePathSanitize(t *testing.T) {
7474
assert.Equal(t, ".", filePathSanitize("/"))
7575
}
7676

77-
func TestProcessGiteaTemplateFile(t *testing.T) {
77+
func TestProcessGiteaTemplateFileGenerate(t *testing.T) {
7878
tmpDir := filepath.Join(t.TempDir(), "gitea-template-test")
7979

8080
assertFileContent := func(path, expected string) {
@@ -129,10 +129,20 @@ func TestProcessGiteaTemplateFile(t *testing.T) {
129129
assertFileContent("subst-${TEMPLATE_NAME}-to-link", toLinkContent)
130130
assertFileContent("subst-${TEMPLATE_NAME}-from-link", fromLinkContent)
131131
}
132+
133+
// case-5
134+
{
135+
require.NoError(t, os.MkdirAll(tmpDir+"/real-dir", 0o755))
136+
require.NoError(t, os.WriteFile(tmpDir+"/real-dir/real-file", []byte("origin content"), 0o644))
137+
require.NoError(t, os.MkdirAll(tmpDir+"/include/subst-${TEMPLATE_NAME}-link-dir", 0o755))
138+
require.NoError(t, os.WriteFile(tmpDir+"/include/subst-${TEMPLATE_NAME}-link-dir/real-file", []byte("template content"), 0o644))
139+
require.NoError(t, os.Symlink(tmpDir+"/real-dir", tmpDir+"/include/subst-TemplateRepoName-link-dir"))
140+
}
141+
132142
{
133143
// will succeed
134144
require.NoError(t, os.WriteFile(tmpDir+"/subst-${TEMPLATE_NAME}-normal", []byte("dummy subst template name normal"), 0o644))
135-
// will skil if the path subst result is a link
145+
// will be skipped if the path subst result is a link
136146
require.NoError(t, os.WriteFile(tmpDir+"/subst-${TEMPLATE_NAME}-to-link", []byte("dummy subst template name to link"), 0o644))
137147
require.NoError(t, os.Symlink(tmpDir+"/sub/link-target", tmpDir+"/subst-TemplateRepoName-to-link"))
138148
// will be skipped since the source is a symlink
@@ -147,10 +157,19 @@ func TestProcessGiteaTemplateFile(t *testing.T) {
147157
generatedRepo := &repo_model.Repository{Name: "/../.gIt/name"}
148158
assertFileContent(".git/config", "git-config-dummy")
149159
fileMatcher, _ := readGiteaTemplateFile(tmpDir)
150-
err := processGiteaTemplateFile(t.Context(), tmpDir, templateRepo, generatedRepo, fileMatcher)
160+
skippedFiles, err := processGiteaTemplateFile(t.Context(), tmpDir, templateRepo, generatedRepo, fileMatcher)
151161
require.NoError(t, err)
152-
assertFileContent("include/foo/bar/test.txt", "include subdir TemplateRepoName")
162+
assert.Equal(t, []string{
163+
"include/subst-${TEMPLATE_NAME}-link-dir/real-file",
164+
"include/subst-TemplateRepoName-link-dir",
165+
"link",
166+
"subst-${TEMPLATE_NAME}-from-link",
167+
"subst-${TEMPLATE_NAME}-to-link",
168+
"subst-TemplateRepoName-to-link",
169+
}, skippedFiles)
153170
assertFileContent(".git/config", "")
171+
assertFileContent(".gitea/template", "")
172+
assertFileContent("include/foo/bar/test.txt", "include subdir TemplateRepoName")
154173
}
155174

156175
// the lin target should never be modified, and since it is in a subdirectory, it is not affected by the template either
@@ -186,32 +205,38 @@ func TestProcessGiteaTemplateFile(t *testing.T) {
186205
assertSymLink("subst-${TEMPLATE_NAME}-from-link", tmpDir+"/sub/link-target")
187206
}
188207

208+
// case-5
189209
{
190-
templateFilePath := tmpDir + "/.gitea/template"
191-
192-
_ = os.Remove(templateFilePath)
193-
_, err := os.Lstat(templateFilePath)
194-
require.ErrorIs(t, err, fs.ErrNotExist)
195-
_, err = readGiteaTemplateFile(tmpDir) // no template file
196-
require.ErrorIs(t, err, fs.ErrNotExist)
197-
198-
_ = os.WriteFile(templateFilePath+".target", []byte("test-data-target"), 0o644)
199-
_ = os.Symlink(templateFilePath+".target", templateFilePath)
200-
content, _ := os.ReadFile(templateFilePath)
201-
require.Equal(t, "test-data-target", string(content))
202-
_, err = readGiteaTemplateFile(tmpDir) // symlinked template file
203-
require.ErrorIs(t, err, fs.ErrNotExist)
204-
205-
_ = os.Remove(templateFilePath)
206-
_ = os.WriteFile(templateFilePath, []byte("test-data-regular"), 0o644)
207-
content, _ = os.ReadFile(templateFilePath)
208-
require.Equal(t, "test-data-regular", string(content))
209-
fm, err := readGiteaTemplateFile(tmpDir) // regular template file
210-
require.NoError(t, err)
211-
assert.Len(t, fm.globs, 1)
210+
assertFileContent("real-dir/real-file", "origin content")
212211
}
213212
}
214213

214+
func TestProcessGiteaTemplateFileRead(t *testing.T) {
215+
tmpDir := t.TempDir()
216+
_ = os.Mkdir(tmpDir+"/.gitea", 0o755)
217+
templateFilePath := tmpDir + "/.gitea/template"
218+
_ = os.Remove(templateFilePath)
219+
_, err := os.Lstat(templateFilePath)
220+
require.ErrorIs(t, err, fs.ErrNotExist)
221+
_, err = readGiteaTemplateFile(tmpDir) // no template file
222+
require.ErrorIs(t, err, fs.ErrNotExist)
223+
224+
_ = os.WriteFile(templateFilePath+".target", []byte("test-data-target"), 0o644)
225+
_ = os.Symlink(templateFilePath+".target", templateFilePath)
226+
content, _ := os.ReadFile(templateFilePath)
227+
require.Equal(t, "test-data-target", string(content))
228+
_, err = readGiteaTemplateFile(tmpDir) // symlinked template file
229+
require.ErrorIs(t, err, fs.ErrNotExist)
230+
231+
_ = os.Remove(templateFilePath)
232+
_ = os.WriteFile(templateFilePath, []byte("test-data-regular"), 0o644)
233+
content, _ = os.ReadFile(templateFilePath)
234+
require.Equal(t, "test-data-regular", string(content))
235+
fm, err := readGiteaTemplateFile(tmpDir) // regular template file
236+
require.NoError(t, err)
237+
assert.Len(t, fm.globs, 1)
238+
}
239+
215240
func TestTransformers(t *testing.T) {
216241
cases := []struct {
217242
name string

0 commit comments

Comments
 (0)