Enhance attachment upload process and content type handling
- Updated the upload process to cache metadata from the first chunk, ensuring consistency in content type, filename, and user path across chunked uploads. - Modified content type validation in tests to allow for charset variations, improving robustness in file type checks. - Refactored database save logic to differentiate between new records and updates for chunked uploads, enhancing data integrity and performance.
This commit is contained in:
parent
80d0b80a6d
commit
e435c03e0e
2 changed files with 83 additions and 37 deletions
|
|
@ -40,6 +40,10 @@ type UploadChunk struct {
|
||||||
Total int64
|
Total int64
|
||||||
Chunksize int64
|
Chunksize int64
|
||||||
TotalChunks int64
|
TotalChunks int64
|
||||||
|
// Cache metadata from first chunk to avoid inconsistencies
|
||||||
|
ContentType string
|
||||||
|
Filename string
|
||||||
|
UserPath string
|
||||||
}
|
}
|
||||||
|
|
||||||
// Parse parses an attachment wrapper string and returns uploader name and file ID
|
// Parse parses an attachment wrapper string and returns uploader name and file ID
|
||||||
|
|
@ -395,6 +399,10 @@ func (manager Manager) Upload(ctx context.Context, fileheader *FileHeader, reade
|
||||||
Total: total,
|
Total: total,
|
||||||
Chunksize: chunksize,
|
Chunksize: chunksize,
|
||||||
TotalChunks: totalChunks,
|
TotalChunks: totalChunks,
|
||||||
|
// Cache metadata from first chunk
|
||||||
|
ContentType: file.ContentType,
|
||||||
|
Filename: file.Filename,
|
||||||
|
UserPath: file.UserPath,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -411,6 +419,11 @@ func (manager Manager) Upload(ctx context.Context, fileheader *FileHeader, reade
|
||||||
chunkIndex = chunkdata.Last + 1
|
chunkIndex = chunkdata.Last + 1
|
||||||
chunkdata.Last = chunkIndex
|
chunkdata.Last = chunkIndex
|
||||||
uploadChunks.Store(file.ID, chunkdata)
|
uploadChunks.Store(file.ID, chunkdata)
|
||||||
|
|
||||||
|
// For non-first chunks, use cached metadata from first chunk
|
||||||
|
file.ContentType = chunkdata.ContentType
|
||||||
|
file.Filename = chunkdata.Filename
|
||||||
|
file.UserPath = chunkdata.UserPath
|
||||||
}
|
}
|
||||||
|
|
||||||
// Apply gzip compression if requested
|
// Apply gzip compression if requested
|
||||||
|
|
@ -429,6 +442,15 @@ func (manager Manager) Upload(ctx context.Context, fileheader *FileHeader, reade
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Save to database on first chunk only
|
||||||
|
if start == 0 {
|
||||||
|
file.Status = "uploading"
|
||||||
|
err = manager.saveFileToDatabase(ctx, file, file.Path, option)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("failed to create database record for chunked upload: %w", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Fix the file size, the file size is the sum of all chunks
|
// Fix the file size, the file size is the sum of all chunks
|
||||||
file.Bytes = chunkIndex * int(chunkdata.Chunksize)
|
file.Bytes = chunkIndex * int(chunkdata.Chunksize)
|
||||||
file.Status = "uploading"
|
file.Status = "uploading"
|
||||||
|
|
@ -451,14 +473,14 @@ func (manager Manager) Upload(ctx context.Context, fileheader *FileHeader, reade
|
||||||
// Remove the chunk data
|
// Remove the chunk data
|
||||||
uploadChunks.Delete(file.ID)
|
uploadChunks.Delete(file.ID)
|
||||||
|
|
||||||
// Fix the file size
|
// Fix the file size and update status to uploaded
|
||||||
file.Bytes = int(chunkdata.Total)
|
file.Bytes = int(chunkdata.Total)
|
||||||
file.Status = "uploaded"
|
file.Status = "uploaded"
|
||||||
|
|
||||||
// Save file information to database when chunked upload is complete
|
// Update only bytes and status for the last chunk
|
||||||
err = manager.saveFileToDatabase(ctx, file, file.Path, option)
|
err = manager.saveFileToDatabase(ctx, file, file.Path, option)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to save chunked file to database: %w", err)
|
return nil, fmt.Errorf("failed to update chunked file status: %w", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -829,9 +851,6 @@ func (manager Manager) makeFile(file *FileHeader, option UploadOption) (*File, e
|
||||||
return nil, fmt.Errorf("file size %d exceeds the maximum size of %d", file.Size, manager.maxsize)
|
return nil, fmt.Errorf("file size %d exceeds the maximum size of %d", file.Size, manager.maxsize)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get the content type
|
|
||||||
contentType := file.Header.Get("Content-Type")
|
|
||||||
|
|
||||||
// Use original filename if provided, otherwise use the file header filename
|
// Use original filename if provided, otherwise use the file header filename
|
||||||
filename := file.Filename
|
filename := file.Filename
|
||||||
userPath := option.OriginalFilename
|
userPath := option.OriginalFilename
|
||||||
|
|
@ -842,6 +861,22 @@ func (manager Manager) makeFile(file *FileHeader, option UploadOption) (*File, e
|
||||||
|
|
||||||
extension := filepath.Ext(filename)
|
extension := filepath.Ext(filename)
|
||||||
|
|
||||||
|
// Get the content type
|
||||||
|
// For chunked uploads, file.Header may have incorrect content-type (e.g., application/octet-stream for Blob)
|
||||||
|
// Try to detect from filename extension first, then fallback to header
|
||||||
|
contentType := file.Header.Get("Content-Type")
|
||||||
|
if extension != "" {
|
||||||
|
// Try to get content type from extension
|
||||||
|
detectedType := mime.TypeByExtension(extension)
|
||||||
|
if detectedType != "" {
|
||||||
|
// If detected type is not the generic octet-stream, use it
|
||||||
|
// This handles chunked uploads where the header has incorrect type
|
||||||
|
if detectedType != "application/octet-stream" || contentType == "application/octet-stream" {
|
||||||
|
contentType = detectedType
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Get the extension from the content type if not available from filename
|
// Get the extension from the content type if not available from filename
|
||||||
if extension == "" {
|
if extension == "" {
|
||||||
// Special handling for common types
|
// Special handling for common types
|
||||||
|
|
@ -1084,10 +1119,41 @@ func (manager Manager) Delete(ctx context.Context, fileID string) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
// saveFileToDatabase saves file information to the database
|
// saveFileToDatabase saves file information to the database
|
||||||
|
// For chunked uploads, it only updates bytes/status/progress if record exists
|
||||||
func (manager Manager) saveFileToDatabase(ctx context.Context, file *File, storagePath string, option UploadOption) error {
|
func (manager Manager) saveFileToDatabase(ctx context.Context, file *File, storagePath string, option UploadOption) error {
|
||||||
|
|
||||||
m := model.Select("__yao.attachment")
|
m := model.Select("__yao.attachment")
|
||||||
|
|
||||||
|
// Check if record exists first
|
||||||
|
records, err := m.Get(model.QueryParam{
|
||||||
|
Select: []interface{}{"file_id"},
|
||||||
|
Wheres: []model.QueryWhere{
|
||||||
|
{Column: "file_id", Value: file.ID},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to check existing record: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(records) > 0 {
|
||||||
|
// Record exists - this is a chunked upload update
|
||||||
|
// Only update bytes, status, and progress (don't overwrite metadata)
|
||||||
|
updateData := map[string]interface{}{
|
||||||
|
"bytes": int64(file.Bytes),
|
||||||
|
"status": file.Status,
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err = m.UpdateWhere(model.QueryParam{
|
||||||
|
Wheres: []model.QueryWhere{
|
||||||
|
{Column: "file_id", Value: file.ID},
|
||||||
|
},
|
||||||
|
}, updateData)
|
||||||
|
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
// Record doesn't exist - create new record with full metadata
|
||||||
// Set default value for share if empty
|
// Set default value for share if empty
|
||||||
share := option.Share
|
share := option.Share
|
||||||
if share == "" {
|
if share == "" {
|
||||||
|
|
@ -1124,30 +1190,8 @@ func (manager Manager) saveFileToDatabase(ctx context.Context, file *File, stora
|
||||||
data["__yao_tenant_id"] = option.YaoTenantID
|
data["__yao_tenant_id"] = option.YaoTenantID
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if record exists first
|
|
||||||
records, err := m.Get(model.QueryParam{
|
|
||||||
Select: []interface{}{"file_id"},
|
|
||||||
Wheres: []model.QueryWhere{
|
|
||||||
{Column: "file_id", Value: file.ID},
|
|
||||||
},
|
|
||||||
})
|
|
||||||
|
|
||||||
if err != nil {
|
|
||||||
return fmt.Errorf("failed to check existing record: %w", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
if len(records) > 0 {
|
|
||||||
// Update existing record
|
|
||||||
_, err = m.UpdateWhere(model.QueryParam{
|
|
||||||
Wheres: []model.QueryWhere{
|
|
||||||
{Column: "file_id", Value: file.ID},
|
|
||||||
},
|
|
||||||
}, data)
|
|
||||||
} else {
|
|
||||||
// Create new record
|
// Create new record
|
||||||
_, err = m.Create(data)
|
_, err = m.Create(data)
|
||||||
}
|
|
||||||
|
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -67,7 +67,8 @@ func TestManagerUpload(t *testing.T) {
|
||||||
t.Errorf("Expected filename 'test.txt', got '%s'", file.Filename)
|
t.Errorf("Expected filename 'test.txt', got '%s'", file.Filename)
|
||||||
}
|
}
|
||||||
|
|
||||||
if file.ContentType != "text/plain" {
|
// Content type may include charset
|
||||||
|
if !strings.HasPrefix(file.ContentType, "text/plain") {
|
||||||
t.Errorf("Expected content type 'text/plain', got '%s'", file.ContentType)
|
t.Errorf("Expected content type 'text/plain', got '%s'", file.ContentType)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -581,7 +582,8 @@ func TestInfo(t *testing.T) {
|
||||||
t.Errorf("Expected filename %s, got %s", uploadedFile.Filename, fileInfo.Filename)
|
t.Errorf("Expected filename %s, got %s", uploadedFile.Filename, fileInfo.Filename)
|
||||||
}
|
}
|
||||||
|
|
||||||
if fileInfo.ContentType != "text/plain" {
|
// Content type may include charset
|
||||||
|
if !strings.HasPrefix(fileInfo.ContentType, "text/plain") {
|
||||||
t.Errorf("Expected content type 'text/plain', got %s", fileInfo.ContentType)
|
t.Errorf("Expected content type 'text/plain', got %s", fileInfo.ContentType)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -731,9 +733,9 @@ func TestList(t *testing.T) {
|
||||||
// Test filtering by content type
|
// Test filtering by content type
|
||||||
t.Run("FilterByContentType", func(t *testing.T) {
|
t.Run("FilterByContentType", func(t *testing.T) {
|
||||||
result, err := manager.List(context.Background(), ListOption{
|
result, err := manager.List(context.Background(), ListOption{
|
||||||
Filters: map[string]interface{}{
|
Wheres: []model.QueryWhere{
|
||||||
"uploader": managerName,
|
{Column: "uploader", Value: managerName},
|
||||||
"content_type": "text/plain",
|
{Column: "content_type", Value: "text/plain%", OP: "like"},
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|
@ -745,9 +747,9 @@ func TestList(t *testing.T) {
|
||||||
t.Errorf("Expected %d text files, got %d", expectedCount, len(result.Files))
|
t.Errorf("Expected %d text files, got %d", expectedCount, len(result.Files))
|
||||||
}
|
}
|
||||||
|
|
||||||
// Verify all returned files are text/plain
|
// Verify all returned files are text/plain (may include charset)
|
||||||
for _, file := range result.Files {
|
for _, file := range result.Files {
|
||||||
if file.ContentType != "text/plain" {
|
if !strings.HasPrefix(file.ContentType, "text/plain") {
|
||||||
t.Errorf("Expected content type 'text/plain', got '%s'", file.ContentType)
|
t.Errorf("Expected content type 'text/plain', got '%s'", file.ContentType)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue