mirror of
https://github.com/sipeed/picoclaw.git
synced 2026-06-12 18:08:54 +00:00
refactor(media): add MediaStore for unified media file lifecycle management
Channels previously deleted downloaded media files via defer os.Remove, racing with the async Agent consumer. Introduce MediaStore to decouple file ownership: channels register files on download, Agent releases them after processing via ReleaseAll(scope). - New pkg/media with MediaStore interface + FileMediaStore implementation - InboundMessage gains MediaScope field for lifecycle tracking - BaseChannel gains SetMediaStore/GetMediaStore + BuildMediaScope helper - Manager injects MediaStore into channels; AgentLoop releases on completion - Telegram, Discord, Slack, OneBot, LINE channels migrated from defer os.Remove to store.Store() with media:// refs
This commit is contained in:
Hoshina
@@ -0,0 +1,102 @@
|
||||
package media
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"sync"
|
||||
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
// MediaMeta holds metadata about a stored media file.
|
||||
type MediaMeta struct {
|
||||
Filename string
|
||||
ContentType string
|
||||
Source string // "telegram", "discord", "tool:image-gen", etc.
|
||||
}
|
||||
|
||||
// MediaStore manages the lifecycle of media files associated with processing scopes.
|
||||
type MediaStore interface {
|
||||
// Store registers an existing local file under the given scope.
|
||||
// Returns a ref identifier (e.g. "media://<id>").
|
||||
// Store does not move or copy the file; it only records the mapping.
|
||||
Store(localPath string, meta MediaMeta, scope string) (ref string, err error)
|
||||
|
||||
// Resolve returns the local file path for a given ref.
|
||||
Resolve(ref string) (localPath string, err error)
|
||||
|
||||
// ReleaseAll deletes all files registered under the given scope
|
||||
// and removes the mapping entries. File-not-exist errors are ignored.
|
||||
ReleaseAll(scope string) error
|
||||
}
|
||||
|
||||
// FileMediaStore is a pure in-memory implementation of MediaStore.
|
||||
// Files are expected to already exist on disk (e.g. in /tmp/picoclaw_media/).
|
||||
type FileMediaStore struct {
|
||||
mu sync.RWMutex
|
||||
refToPath map[string]string
|
||||
scopeToRefs map[string]map[string]struct{}
|
||||
}
|
||||
|
||||
// NewFileMediaStore creates a new FileMediaStore.
|
||||
func NewFileMediaStore() *FileMediaStore {
|
||||
return &FileMediaStore{
|
||||
refToPath: make(map[string]string),
|
||||
scopeToRefs: make(map[string]map[string]struct{}),
|
||||
}
|
||||
}
|
||||
|
||||
// Store registers a local file under the given scope. The file must exist.
|
||||
func (s *FileMediaStore) Store(localPath string, meta MediaMeta, scope string) (string, error) {
|
||||
if _, err := os.Stat(localPath); err != nil {
|
||||
return "", fmt.Errorf("media store: file does not exist: %s", localPath)
|
||||
}
|
||||
|
||||
ref := "media://" + uuid.New().String()[:8]
|
||||
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
|
||||
s.refToPath[ref] = localPath
|
||||
if s.scopeToRefs[scope] == nil {
|
||||
s.scopeToRefs[scope] = make(map[string]struct{})
|
||||
}
|
||||
s.scopeToRefs[scope][ref] = struct{}{}
|
||||
|
||||
return ref, nil
|
||||
}
|
||||
|
||||
// Resolve returns the local path for the given ref.
|
||||
func (s *FileMediaStore) Resolve(ref string) (string, error) {
|
||||
s.mu.RLock()
|
||||
defer s.mu.RUnlock()
|
||||
|
||||
path, ok := s.refToPath[ref]
|
||||
if !ok {
|
||||
return "", fmt.Errorf("media store: unknown ref: %s", ref)
|
||||
}
|
||||
return path, nil
|
||||
}
|
||||
|
||||
// ReleaseAll removes all files under the given scope and cleans up mappings.
|
||||
func (s *FileMediaStore) ReleaseAll(scope string) error {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
|
||||
refs, ok := s.scopeToRefs[scope]
|
||||
if !ok {
|
||||
return nil
|
||||
}
|
||||
|
||||
for ref := range refs {
|
||||
if path, exists := s.refToPath[ref]; exists {
|
||||
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
|
||||
// Log but continue — best effort cleanup
|
||||
}
|
||||
delete(s.refToPath, ref)
|
||||
}
|
||||
}
|
||||
|
||||
delete(s.scopeToRefs, scope)
|
||||
return nil
|
||||
}
|
||||
@@ -0,0 +1,179 @@
|
||||
package media
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func createTempFile(t *testing.T, dir, name string) string {
|
||||
t.Helper()
|
||||
path := filepath.Join(dir, name)
|
||||
if err := os.WriteFile(path, []byte("test content"), 0o644); err != nil {
|
||||
t.Fatalf("failed to create temp file: %v", err)
|
||||
}
|
||||
return path
|
||||
}
|
||||
|
||||
func TestStoreAndResolve(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
store := NewFileMediaStore()
|
||||
|
||||
path := createTempFile(t, dir, "photo.jpg")
|
||||
|
||||
ref, err := store.Store(path, MediaMeta{Filename: "photo.jpg", Source: "telegram"}, "scope1")
|
||||
if err != nil {
|
||||
t.Fatalf("Store failed: %v", err)
|
||||
}
|
||||
|
||||
if !strings.HasPrefix(ref, "media://") {
|
||||
t.Errorf("ref should start with media://, got %q", ref)
|
||||
}
|
||||
|
||||
resolved, err := store.Resolve(ref)
|
||||
if err != nil {
|
||||
t.Fatalf("Resolve failed: %v", err)
|
||||
}
|
||||
if resolved != path {
|
||||
t.Errorf("Resolve returned %q, want %q", resolved, path)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReleaseAll(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
store := NewFileMediaStore()
|
||||
|
||||
paths := make([]string, 3)
|
||||
refs := make([]string, 3)
|
||||
for i := 0; i < 3; i++ {
|
||||
paths[i] = createTempFile(t, dir, strings.Repeat("a", i+1)+".jpg")
|
||||
var err error
|
||||
refs[i], err = store.Store(paths[i], MediaMeta{Source: "test"}, "scope1")
|
||||
if err != nil {
|
||||
t.Fatalf("Store failed: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
if err := store.ReleaseAll("scope1"); err != nil {
|
||||
t.Fatalf("ReleaseAll failed: %v", err)
|
||||
}
|
||||
|
||||
// Files should be deleted
|
||||
for _, p := range paths {
|
||||
if _, err := os.Stat(p); !os.IsNotExist(err) {
|
||||
t.Errorf("file %q should have been deleted", p)
|
||||
}
|
||||
}
|
||||
|
||||
// Refs should be unresolvable
|
||||
for _, ref := range refs {
|
||||
if _, err := store.Resolve(ref); err == nil {
|
||||
t.Errorf("Resolve(%q) should fail after ReleaseAll", ref)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestMultiScopeIsolation(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
store := NewFileMediaStore()
|
||||
|
||||
pathA := createTempFile(t, dir, "fileA.jpg")
|
||||
pathB := createTempFile(t, dir, "fileB.jpg")
|
||||
|
||||
refA, _ := store.Store(pathA, MediaMeta{Source: "test"}, "scopeA")
|
||||
refB, _ := store.Store(pathB, MediaMeta{Source: "test"}, "scopeB")
|
||||
|
||||
// Release only scopeA
|
||||
if err := store.ReleaseAll("scopeA"); err != nil {
|
||||
t.Fatalf("ReleaseAll(scopeA) failed: %v", err)
|
||||
}
|
||||
|
||||
// scopeA file should be gone
|
||||
if _, err := os.Stat(pathA); !os.IsNotExist(err) {
|
||||
t.Error("file A should have been deleted")
|
||||
}
|
||||
if _, err := store.Resolve(refA); err == nil {
|
||||
t.Error("refA should be unresolvable after release")
|
||||
}
|
||||
|
||||
// scopeB file should still exist
|
||||
if _, err := os.Stat(pathB); err != nil {
|
||||
t.Error("file B should still exist")
|
||||
}
|
||||
resolved, err := store.Resolve(refB)
|
||||
if err != nil {
|
||||
t.Fatalf("refB should still resolve: %v", err)
|
||||
}
|
||||
if resolved != pathB {
|
||||
t.Errorf("resolved %q, want %q", resolved, pathB)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReleaseAllIdempotent(t *testing.T) {
|
||||
store := NewFileMediaStore()
|
||||
|
||||
// ReleaseAll on non-existent scope should not error
|
||||
if err := store.ReleaseAll("nonexistent"); err != nil {
|
||||
t.Fatalf("ReleaseAll on empty scope should not error: %v", err)
|
||||
}
|
||||
|
||||
// Create and release, then release again
|
||||
dir := t.TempDir()
|
||||
path := createTempFile(t, dir, "file.jpg")
|
||||
_, _ = store.Store(path, MediaMeta{Source: "test"}, "scope1")
|
||||
|
||||
if err := store.ReleaseAll("scope1"); err != nil {
|
||||
t.Fatalf("first ReleaseAll failed: %v", err)
|
||||
}
|
||||
if err := store.ReleaseAll("scope1"); err != nil {
|
||||
t.Fatalf("second ReleaseAll should not error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStoreNonexistentFile(t *testing.T) {
|
||||
store := NewFileMediaStore()
|
||||
|
||||
_, err := store.Store("/nonexistent/path/file.jpg", MediaMeta{Source: "test"}, "scope1")
|
||||
if err == nil {
|
||||
t.Error("Store should fail for nonexistent file")
|
||||
}
|
||||
}
|
||||
|
||||
func TestConcurrentSafety(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
store := NewFileMediaStore()
|
||||
|
||||
const goroutines = 20
|
||||
const filesPerGoroutine = 5
|
||||
|
||||
var wg sync.WaitGroup
|
||||
wg.Add(goroutines)
|
||||
|
||||
for g := 0; g < goroutines; g++ {
|
||||
go func(gIdx int) {
|
||||
defer wg.Done()
|
||||
scope := strings.Repeat("s", gIdx+1)
|
||||
|
||||
for i := 0; i < filesPerGoroutine; i++ {
|
||||
path := createTempFile(t, dir, strings.Repeat("f", gIdx*filesPerGoroutine+i+1)+".tmp")
|
||||
ref, err := store.Store(path, MediaMeta{Source: "test"}, scope)
|
||||
if err != nil {
|
||||
t.Errorf("Store failed: %v", err)
|
||||
return
|
||||
}
|
||||
|
||||
if _, err := store.Resolve(ref); err != nil {
|
||||
t.Errorf("Resolve failed: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
if err := store.ReleaseAll(scope); err != nil {
|
||||
t.Errorf("ReleaseAll failed: %v", err)
|
||||
}
|
||||
}(g)
|
||||
}
|
||||
|
||||
wg.Wait()
|
||||
}
|
||||
Reference in New Issue
Block a user