Skip to content

Commit fe7ae31

Browse files
riba2534riba2534
committed
fix(msg): address review findings for upload-images feature
- Fix JSON serialization vs Markdown replacement order conflict by applying replacements immediately before JSON processing - Preserve all original img tag attributes (alt/title/mode/preview) instead of only keeping width/height - Unify error handling: both Markdown and JSON paths now warn+skip on upload failure - Extract uploadLocalImageForIM helper to eliminate duplicate upload logic - Extract resolveLocalPath as package-level function (was duplicated closure) - Remove .svg from isLocalPath (IM API doesn't support SVG) - Avoid unnecessary deep copy in processJSONLocalImages when unchanged - Replace custom contains helper with strings.Contains in tests - Replace dead integration test with proper resolveLocalPath unit test
1 parent 1bd13f5 commit fe7ae31

2 files changed

Lines changed: 95 additions & 155 deletions

File tree

‎cmd/send_message.go‎

Lines changed: 68 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func isLocalPath(s string) bool {
253253
// 检查是否是文件路径（包含 / 或 \ 或扩展名）
254254
ext := strings.ToLower(filepath.Ext(s))
255255
if ext == ".png" || ext == ".jpg" || ext == ".jpeg" || ext == ".gif" ||
256-
ext == ".bmp" || ext == ".webp" || ext == ".svg" {
256+
ext == ".bmp" || ext == ".webp" {
257257
return true
258258
}
259259
// 检查路径分隔符
@@ -263,64 +263,63 @@ func isLocalPath(s string) bool {
263263
return false
264264
}
265265

266+
// resolveLocalPath 解析相对路径为绝对路径（与 markdown import 保持一致）
267+
func resolveLocalPath(path, basePath string) string {
268+
if filepath.IsAbs(path) {
269+
return path
270+
}
271+
return filepath.Join(basePath, path)
272+
}
273+
274+
// uploadLocalImageForIM 解析路径、检查存在性并上传图片到 IM API
275+
// 返回 image_key；文件不存在或上传失败时返回空字符串（打印警告）
276+
func uploadLocalImageForIM(imagePath, basePath string) string {
277+
resolvedPath := resolveLocalPath(imagePath, basePath)
278+
279+
if _, err := os.Stat(resolvedPath); err != nil {
280+
fmt.Fprintf(os.Stderr, "警告: 跳过不存在的图片: %s\n", resolvedPath)
281+
return ""
282+
}
283+
284+
fmt.Fprintf(os.Stderr, "正在上传图片: %s\n", resolvedPath)
285+
imageKey, err := client.UploadIMImage(resolvedPath, "")
286+
if err != nil {
287+
fmt.Fprintf(os.Stderr, "警告: 上传图片 %s 失败: %v\n", resolvedPath, err)
288+
return ""
289+
}
290+
return imageKey
291+
}
292+
266293
// processAndUploadLocalImages 解析消息内容中的本地图片路径，上传并替换为 image_key
267294
// basePath 用于解析相对路径：如果使用 --content-file，以其目录为 basePath；否则用当前目录
268295
// 返回处理后的内容和上传的图片数量
269296
func processAndUploadLocalImages(content string, basePath string) (string, int, error) {
270-
// 先收集所有本地图片路径
271-
type imageReplace struct {
272-
oldStr string
273-
newStr string
274-
}
275-
var replacements []imageReplace
276297
uploadCount := 0
277298

278-
// 解析相对路径为绝对路径（与 markdown import 保持一致）
279-
resolveLocalPath := func(path string) string {
280-
if filepath.IsAbs(path) {
281-
return path
282-
}
283-
return filepath.Join(basePath, path)
284-
}
285-
286-
// 1. 处理 Markdown 语法中的本地图片: ![alt](local/path.png)
299+
// 1. 先处理 Markdown 语法中的本地图片: ![alt](local/path.png)
300+
// 在 JSON 处理之前立即替换，避免 JSON 重新序列化导致字符串不匹配
287301
matches := markdownImageRegex.FindAllStringSubmatch(content, -1)
288302
for _, match := range matches {
289-
fullMatch := match[0] // 完整匹配如 ![alt](local/path.png)
290-
imagePath := match[2] // 图片路径
303+
fullMatch := match[0] // 完整匹配如 ![alt](local/path.png)
304+
imagePath := match[2] // 图片路径
291305

292306
if !isLocalPath(imagePath) {
293307
continue
294308
}
295309

296-
// 解析相对路径
297-
resolvedPath := resolveLocalPath(imagePath)
298-
299-
// 检查文件是否存在
300-
if _, err := os.Stat(resolvedPath); err != nil {
301-
fmt.Fprintf(os.Stderr, "警告: 跳过不存在的图片: %s\n", resolvedPath)
310+
imageKey := uploadLocalImageForIM(imagePath, basePath)
311+
if imageKey == "" {
302312
continue
303313
}
304314

305-
// 上传图片
306-
fmt.Fprintf(os.Stderr, "正在上传图片: %s\n", resolvedPath)
307-
imageKey, err := client.UploadIMImage(resolvedPath, "")
308-
if err != nil {
309-
return "", 0, fmt.Errorf("上传图片 %s 失败: %w", resolvedPath, err)
310-
}
311-
312-
// 替换为飞书图片格式
313-
replacements = append(replacements, imageReplace{
314-
oldStr: fullMatch,
315-
newStr: fmt.Sprintf("![%s](%s)", match[1], imageKey),
316-
})
315+
// 立即替换，避免后续 JSON 序列化改变内容导致 ReplaceAll 失效
316+
content = strings.ReplaceAll(content, fullMatch, fmt.Sprintf("![%s](%s)", match[1], imageKey))
317317
uploadCount++
318318
}
319319

320320
// 2. 尝试解析为 JSON，处理 img 标签中的本地 image_key
321321
var jsonData interface{}
322322
if err := json.Unmarshal([]byte(content), &jsonData); err == nil {
323-
// 是有效的 JSON，递归处理
324323
changed, newData, count := processJSONLocalImages(jsonData, basePath)
325324
if changed {
326325
uploadCount += count
@@ -332,80 +331,49 @@ func processAndUploadLocalImages(content string, basePath string) (string, int,
332331
}
333332
}
334333

335-
// 3. 应用 Markdown 替换
336-
for _, rep := range replacements {
337-
content = strings.ReplaceAll(content, rep.oldStr, rep.newStr)
338-
}
339-
340334
return content, uploadCount, nil
341335
}
342336

343337
// processJSONLocalImages 递归处理 JSON 结构中的本地图片
344338
// basePath 用于解析相对路径
345339
// 返回是否有修改、处理后的数据、上传的图片数量
346340
func processJSONLocalImages(data interface{}, basePath string) (bool, interface{}, int) {
347-
// 解析相对路径为绝对路径
348-
resolveLocalPath := func(path string) string {
349-
if filepath.IsAbs(path) {
350-
return path
351-
}
352-
return filepath.Join(basePath, path)
353-
}
354-
355341
switch v := data.(type) {
356342
case map[string]interface{}:
357-
changed := false
358-
count := 0
359-
newMap := make(map[string]interface{})
360-
361343
// 检查是否是 img 标签
362344
if tag, ok := v["tag"].(string); ok && tag == "img" {
363-
if imageKey, ok := v["image_key"].(string); ok && isLocalPath(imageKey) {
364-
// 解析相对路径
365-
resolvedPath := resolveLocalPath(imageKey)
366-
367-
// 检查文件是否存在
368-
if _, err := os.Stat(resolvedPath); err != nil {
369-
fmt.Fprintf(os.Stderr, "警告: 跳过不存在的图片: %s\n", resolvedPath)
370-
newMap = v
371-
} else {
372-
// 上传图片
373-
fmt.Fprintf(os.Stderr, "正在上传图片: %s\n", resolvedPath)
374-
imageKeyResult, err := client.UploadIMImage(resolvedPath, "")
375-
if err != nil {
376-
fmt.Fprintf(os.Stderr, "警告: 上传图片 %s 失败: %v\n", resolvedPath, err)
377-
newMap = v
378-
} else {
379-
newMap = map[string]interface{}{
380-
"tag": "img",
381-
"image_key": imageKeyResult,
382-
}
383-
// 保留 width 和 height 如果存在
384-
if w, ok := v["width"].(float64); ok {
385-
newMap["width"] = w
386-
}
387-
if h, ok := v["height"].(float64); ok {
388-
newMap["height"] = h
389-
}
390-
changed = true
391-
count = 1
392-
}
345+
if imageKeyVal, ok := v["image_key"].(string); ok && isLocalPath(imageKeyVal) {
346+
imageKey := uploadLocalImageForIM(imageKeyVal, basePath)
347+
if imageKey == "" {
348+
return false, v, 0
393349
}
394-
} else {
395-
newMap = v
396-
}
397-
} else {
398-
// 递归处理所有字段
399-
for key, val := range v {
400-
c, newVal, n := processJSONLocalImages(val, basePath)
401-
if c {
402-
changed = true
403-
count += n
350+
// 保留所有原始属性，仅替换 image_key
351+
newMap := make(map[string]interface{}, len(v))
352+
for key, val := range v {
353+
newMap[key] = val
404354
}
405-
newMap[key] = newVal
355+
newMap["image_key"] = imageKey
356+
return true, newMap, 1
357+
}
358+
return false, v, 0
359+
}
360+
361+
// 递归处理所有字段
362+
changed := false
363+
count := 0
364+
newMap := make(map[string]interface{}, len(v))
365+
for key, val := range v {
366+
c, newVal, n := processJSONLocalImages(val, basePath)
367+
if c {
368+
changed = true
369+
count += n
406370
}
371+
newMap[key] = newVal
407372
}
408-
return changed, newMap, count
373+
if !changed {
374+
return false, v, 0
375+
}
376+
return true, newMap, count
409377

410378
case []interface{}:
411379
changed := false
@@ -419,7 +387,10 @@ func processJSONLocalImages(data interface{}, basePath string) (bool, interface{
419387
}
420388
newArr[i] = newItem
421389
}
422-
return changed, newArr, count
390+
if !changed {
391+
return false, v, 0
392+
}
393+
return true, newArr, count
423394

424395
default:
425396
return false, v, 0

‎cmd/send_message_test.go‎

Lines changed: 27 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ package cmd
22

33
import (
44
"encoding/json"
5-
"os"
65
"path/filepath"
6+
"strings"
77
"testing"
88
)
99

@@ -33,7 +33,7 @@ func TestIsLocalPath(t *testing.T) {
3333
{"相对路径 gif", "animation.gif", true},
3434
{"相对路径 bmp", "icon.bmp", true},
3535
{"相对路径 webp", "image.webp", true},
36-
{"相对路径 svg", "vector.svg", true},
36+
{"相对路径 svg（IM 不支持）", "vector.svg", false},
3737

3838
// 绝对路径应该是本地路径
3939
{"绝对路径 Unix", "/Users/test/image.png", true},
@@ -130,7 +130,7 @@ func TestProcessJSONLocalImages_SkipRemoteImages(t *testing.T) {
130130
if err != nil {
131131
t.Fatalf("JSON 序列化失败: %v", err)
132132
}
133-
if !contains(string(resultBytes), tt.expectKey) {
133+
if !strings.Contains(string(resultBytes), tt.expectKey) {
134134
t.Errorf("结果 JSON 不包含期望的 image_key %q，结果: %s", tt.expectKey, string(resultBytes))
135135
}
136136
})
@@ -161,7 +161,7 @@ func TestProcessJSONLocalImages_SkipNonExistentFiles(t *testing.T) {
161161

162162
// 验证结果保留原值
163163
resultBytes, _ := json.Marshal(newData)
164-
if !contains(string(resultBytes), "/nonexistent/path/to/image.png") {
164+
if !strings.Contains(string(resultBytes), "/nonexistent/path/to/image.png") {
165165
t.Errorf("结果应该保留原路径，结果: %s", string(resultBytes))
166166
}
167167
}
@@ -188,7 +188,7 @@ func TestProcessJSONLocalImages_NonImageTag(t *testing.T) {
188188
}
189189

190190
resultBytes, _ := json.Marshal(newData)
191-
if !contains(string(resultBytes), "some.png") {
191+
if !strings.Contains(string(resultBytes), "some.png") {
192192
t.Errorf("结果应该保留原值，结果: %s", string(resultBytes))
193193
}
194194
}
@@ -306,7 +306,7 @@ func TestProcessAndUploadLocalImages_SkipRemoteMarkdownImages(t *testing.T) {
306306
if count != 0 {
307307
t.Errorf("不应该有任何上传，count=%d", count)
308308
}
309-
if !contains(result, tt.expectContains) {
309+
if !strings.Contains(result, tt.expectContains) {
310310
t.Errorf("结果不包含期望字符串 %q，结果: %s", tt.expectContains, result)
311311
}
312312
})
@@ -340,7 +340,7 @@ func TestProcessAndUploadLocalImages_MixedContent(t *testing.T) {
340340
t.Errorf("不存在的本地文件不应该被上传，count=%d", count)
341341
}
342342
// 远程图片应该保留
343-
if !contains(result, "https://example.com/remote.png") {
343+
if !strings.Contains(result, "https://example.com/remote.png") {
344344
t.Errorf("远程图片 URL 应该保留，结果: %s", result)
345345
}
346346
}
@@ -384,60 +384,29 @@ func TestMarkdownImageRegex(t *testing.T) {
384384
}
385385

386386
// -------------------------------------------------------------------
387-
// 辅助函数
387+
// resolveLocalPath 路径解析测试
388388
// -------------------------------------------------------------------
389389

390-
func contains(s, substr string) bool {
391-
return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsHelper(s, substr))
392-
}
393-
394-
func containsHelper(s, substr string) bool {
395-
for i := 0; i <= len(s)-len(substr); i++ {
396-
if s[i:i+len(substr)] == substr {
397-
return true
398-
}
399-
}
400-
return false
401-
}
402-
403-
// -------------------------------------------------------------------
404-
// 路径解析集成测试（创建真实临时文件）
405-
// -------------------------------------------------------------------
406-
407-
func TestResolveLocalPath_Integration(t *testing.T) {
408-
// 这个测试验证相对路径解析逻辑是否正确
409-
tmpDir := t.TempDir()
410-
411-
// 创建测试图片文件
412-
imgPath := filepath.Join(tmpDir, "test.png")
413-
if err := os.WriteFile(imgPath, []byte("fake"), 0600); err != nil {
414-
t.Fatalf("创建测试文件失败: %v", err)
415-
}
416-
417-
// 验证文件存在
418-
if _, err := os.Stat(imgPath); err != nil {
419-
t.Fatalf("文件应该存在: %v", err)
390+
func TestResolveLocalPath(t *testing.T) {
391+
tests := []struct {
392+
name string
393+
path string
394+
basePath string
395+
expected string
396+
}{
397+
{"相对路径", "test.png", "/tmp/dir", filepath.Join("/tmp/dir", "test.png")},
398+
{"绝对路径不变", "/abs/path/img.png", "/tmp/dir", "/abs/path/img.png"},
399+
{"子目录相对路径", "images/logo.png", "/tmp/dir", filepath.Join("/tmp/dir", "images/logo.png")},
400+
{"上级目录", "../assets/icon.png", "/tmp/dir", filepath.Join("/tmp/dir", "../assets/icon.png")},
401+
{"当前目录", "./screenshot.png", "/tmp/dir", filepath.Join("/tmp/dir", "./screenshot.png")},
420402
}
421403

422-
// 测试 JSON 中引用该文件（使用相对于 tmpDir 的路径）
423-
relPath := "test.png"
424-
jsonContent := `{
425-
"tag": "img",
426-
"image_key": "` + relPath + `"
427-
}`
428-
429-
var data interface{}
430-
if err := json.Unmarshal([]byte(jsonContent), &data); err != nil {
431-
t.Fatalf("JSON 解析失败: %v", err)
404+
for _, tt := range tests {
405+
t.Run(tt.name, func(t *testing.T) {
406+
result := resolveLocalPath(tt.path, tt.basePath)
407+
if result != tt.expected {
408+
t.Errorf("resolveLocalPath(%q, %q) = %q, 期望 %q", tt.path, tt.basePath, result, tt.expected)
409+
}
410+
})
432411
}
433-
434-
// 使用 tmpDir 作为 basePath，relPath 应该被解析为 tmpDir/test.png
435-
changed, newData, count := processJSONLocalImages(data, tmpDir)
436-
437-
// 注意：这个测试会因为实际上传而失败（没有 API 凭据）
438-
// 但我们可以验证路径解析的逻辑
439-
// 如果 API 调用成功，changed 应该为 true
440-
_ = changed
441-
_ = newData
442-
_ = count
443412
}

0 commit comments

Comments
 (0)