Merge pull request #1099 from trheyi/main

Refactor document management handlers to improve request validation a…

openapi/kb/collection.go

@@ -18,56 +18,6 @@ type ProviderSettings struct {
 	Properties map[string]interface{} `json:"properties"`
 }
 
-// getProviderSettings reads and resolves provider settings by provider ID and option value
-func getProviderSettings(providerID, optionValue, locale string) (*ProviderSettings, error) {
-	// Default locale to "en" if empty
-	if locale == "" {
-		locale = "en"
-	}
-
-	// Get the specific provider using KB API
-	provider, err := kb.GetProviderWithLanguage("embedding", providerID, locale)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get provider %s: %v", providerID, err)
-	}
-
-	// Find the target option
-	targetOption, found := provider.GetOption(optionValue)
-	if !found {
-		return nil, fmt.Errorf("option not found: %s for provider %s", optionValue, providerID)
-	}
-
-	// Extract settings from option properties
-	settings := &ProviderSettings{
-		Properties: make(map[string]interface{}),
-	}
-
-	// Copy all properties
-	if targetOption.Properties != nil {
-		for key, value := range targetOption.Properties {
-			settings.Properties[key] = value
-		}
-	}
-
-	// Extract dimension
-	if dim, ok := targetOption.Properties["dimensions"]; ok {
-		if dimInt, ok := dim.(int); ok {
-			settings.Dimension = dimInt
-		} else if dimFloat, ok := dim.(float64); ok {
-			settings.Dimension = int(dimFloat)
-		}
-	}
-
-	// Extract connector
-	if connector, ok := targetOption.Properties["connector"]; ok {
-		if connStr, ok := connector.(string); ok {
-			settings.Connector = connStr
-		}
-	}
-
-	return settings, nil
-}
-
 // CreateCollection creates a new collection
 func CreateCollection(c *gin.Context) {
 	var req CreateCollectionRequest
@@ -396,3 +346,53 @@ func validateUpdateCollectionMetadataRequest(req *UpdateCollectionMetadataReques
 
 	return nil
 }
+
+// getProviderSettings reads and resolves provider settings by provider ID and option value
+func getProviderSettings(providerID, optionValue, locale string) (*ProviderSettings, error) {
+	// Default locale to "en" if empty
+	if locale == "" {
+		locale = "en"
+	}
+
+	// Get the specific provider using KB API
+	provider, err := kb.GetProviderWithLanguage("embedding", providerID, locale)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get provider %s: %v", providerID, err)
+	}
+
+	// Find the target option
+	targetOption, found := provider.GetOption(optionValue)
+	if !found {
+		return nil, fmt.Errorf("option not found: %s for provider %s", optionValue, providerID)
+	}
+
+	// Extract settings from option properties
+	settings := &ProviderSettings{
+		Properties: make(map[string]interface{}),
+	}
+
+	// Copy all properties
+	if targetOption.Properties != nil {
+		for key, value := range targetOption.Properties {
+			settings.Properties[key] = value
+		}
+	}
+
+	// Extract dimension
+	if dim, ok := targetOption.Properties["dimensions"]; ok {
+		if dimInt, ok := dim.(int); ok {
+			settings.Dimension = dimInt
+		} else if dimFloat, ok := dim.(float64); ok {
+			settings.Dimension = int(dimFloat)
+		}
+	}
+
+	// Extract connector
+	if connector, ok := targetOption.Properties["connector"]; ok {
+		if connStr, ok := connector.(string); ok {
+			settings.Connector = connStr
+		}
+	}
+
+	return settings, nil
+}

openapi/kb/document.go

@@ -4,6 +4,8 @@ import (
 	"net/http"
 
 	"github.com/gin-gonic/gin"
+	"github.com/google/uuid"
+	"github.com/yaoapp/gou/graphrag/types"
 	"github.com/yaoapp/yao/attachment"
 	"github.com/yaoapp/yao/kb"
 	"github.com/yaoapp/yao/openapi/response"
@@ -11,29 +13,21 @@ import (
 
 // Document Management Handlers
 
-// AddFile adds a file to a collection
+// Validator interface for request validation
-func AddFile(c *gin.Context) {
+type Validator interface {
-
+	Validate() error
-	var req AddFileRequest
+}
-
-	// Check if kb.Instance is available
-	if kb.Instance == nil {
-		errorResp := &response.ErrorResponse{
-			Code:             response.ErrServerError.Code,
-			ErrorDescription: "Knowledge base not initialized",
-		}
-		response.RespondWithError(c, response.StatusInternalServerError, errorResp)
-		return
-	}
 
+// validateRequest validates a request by parsing JSON and calling Validate()
+func validateRequest[T Validator](c *gin.Context, req T) error {
 	// Parse and bind JSON request
-	if err := c.ShouldBindJSON(&req); err != nil {
+	if err := c.ShouldBindJSON(req); err != nil {
 		errorResp := &response.ErrorResponse{
 			Code:             response.ErrInvalidRequest.Code,
 			ErrorDescription: "Invalid request format: " + err.Error(),
 		}
 		response.RespondWithError(c, response.StatusBadRequest, errorResp)
-		return
+		return err
 	}
 
 	// Validate request
@@ -43,9 +37,41 @@ func AddFile(c *gin.Context) {
 			ErrorDescription: err.Error(),
 		}
 		response.RespondWithError(c, response.StatusBadRequest, errorResp)
-		return
+		return err
 	}
 
+	return nil
+}
+
+// checkKBInstance checks if kb.Instance is available
+func checkKBInstance(c *gin.Context) bool {
+	if kb.Instance == nil {
+		errorResp := &response.ErrorResponse{
+			Code:             response.ErrServerError.Code,
+			ErrorDescription: "Knowledge base not initialized",
+		}
+		response.RespondWithError(c, response.StatusInternalServerError, errorResp)
+		return false
+	}
+	return true
+}
+
+// getUpsertOptions converts BaseUpsertRequest to UpsertOptions with optional file info
+func getUpsertOptions(c *gin.Context, req *BaseUpsertRequest, fileInfo ...string) (*types.UpsertOptions, error) {
+	upsertOptions, err := req.ToUpsertOptions(fileInfo...)
+	if err != nil {
+		errorResp := &response.ErrorResponse{
+			Code:             response.ErrInvalidRequest.Code,
+			ErrorDescription: "Failed to convert request to upsert options: " + err.Error(),
+		}
+		response.RespondWithError(c, response.StatusBadRequest, errorResp)
+		return nil, err
+	}
+	return upsertOptions, nil
+}
+
+// validateFileAndGetPath validates file manager, file existence and gets local path
+func validateFileAndGetPath(c *gin.Context, req *AddFileRequest) (string, string, error) {
 	// Get file manager
 	m, ok := attachment.Managers[req.Uploader]
 	if !ok {
@@ -54,7 +80,7 @@ func AddFile(c *gin.Context) {
 			ErrorDescription: "Invalid uploader: " + req.Uploader + " not found",
 		}
 		response.RespondWithError(c, response.StatusNotFound, errorResp)
-		return
+		return "", "", response.ErrInvalidRequest
 	}
 
 	// Check if the file exists
@@ -65,7 +91,7 @@ func AddFile(c *gin.Context) {
 			ErrorDescription: "File not found: " + req.FileID,
 		}
 		response.RespondWithError(c, response.StatusNotFound, errorResp)
-		return
+		return "", "", response.ErrInvalidRequest
 	}
 
 	// Get the options of the manager
@@ -76,17 +102,45 @@ func AddFile(c *gin.Context) {
 			ErrorDescription: "Failed to get local path: " + err.Error(),
 		}
 		response.RespondWithError(c, response.StatusInternalServerError, errorResp)
+		return "", "", err
+	}
+
+	return path, contentType, nil
+}
+
+// handleAsync handles async processing for any handler function
+func handleAsync(c *gin.Context, syncHandler func(*gin.Context)) {
+	jobid := uuid.New().String()
+
+	// temporary solution to handle async operations ( TODO: use job queue )
+	go func() { syncHandler(c) }()
+
+	response.RespondWithSuccess(c, response.StatusCreated, gin.H{"job_id": jobid})
+}
+
+// AddFile adds a file to a collection
+func AddFile(c *gin.Context) {
+	var req AddFileRequest
+
+	// Check if kb.Instance is available
+	if !checkKBInstance(c) {
+		return
+	}
+
+	// Validate request
+	if err := validateRequest(c, &req); err != nil {
+		return
+	}
+
+	// Validate file and get path
+	path, contentType, err := validateFileAndGetPath(c, &req)
+	if err != nil {
 		return
 	}
 
 	// Convert request to UpsertOptions
-	upsertOptions, err := req.BaseUpsertRequest.ToUpsertOptions(path, contentType)
+	upsertOptions, err := getUpsertOptions(c, &req.BaseUpsertRequest, path, contentType)
 	if err != nil {
-		errorResp := &response.ErrorResponse{
-			Code:             response.ErrInvalidRequest.Code,
-			ErrorDescription: "Failed to convert request to upsert options: " + err.Error(),
-		}
-		response.RespondWithError(c, response.StatusBadRequest, errorResp)
 		return
 	}
 
@@ -114,48 +168,53 @@ func AddFile(c *gin.Context) {
 	response.RespondWithSuccess(c, response.StatusCreated, result)
 }
 
-// AddText adds text to a collection
+// AddFileAsync adds file to a collection asynchronously
-func AddText(c *gin.Context) {
+func AddFileAsync(c *gin.Context) {
-	var req AddTextRequest
+	var req AddFileRequest
 
-	// Parse and bind JSON request
+	// Check if kb.Instance is available
-	if err := c.ShouldBindJSON(&req); err != nil {
+	if !checkKBInstance(c) {
-		errorResp := &response.ErrorResponse{
-			Code:             response.ErrInvalidRequest.Code,
-			ErrorDescription: "Invalid request format: " + err.Error(),
-		}
-		response.RespondWithError(c, response.StatusBadRequest, errorResp)
 		return
 	}
 
 	// Validate request
-	if err := req.Validate(); err != nil {
+	if err := validateRequest(c, &req); err != nil {
-		errorResp := &response.ErrorResponse{
+		return
-			Code:             response.ErrInvalidRequest.Code,
-			ErrorDescription: err.Error(),
 	}
-		response.RespondWithError(c, response.StatusBadRequest, errorResp)
+
+	// Validate file and get path
+	_, _, err := validateFileAndGetPath(c, &req)
+	if err != nil {
+		return
+	}
+
+	// Convert request to UpsertOptions (just for validation)
+	_, err = getUpsertOptions(c, &req.BaseUpsertRequest)
+	if err != nil {
+		return
+	}
+
+	// Handle async processing
+	handleAsync(c, AddFile)
+}
+
+// AddText adds text to a collection
+func AddText(c *gin.Context) {
+	var req AddTextRequest
+
+	// Validate request
+	if err := validateRequest(c, &req); err != nil {
 		return
 	}
 
 	// Check if kb.Instance is available
-	if kb.Instance == nil {
+	if !checkKBInstance(c) {
-		errorResp := &response.ErrorResponse{
-			Code:             response.ErrServerError.Code,
-			ErrorDescription: "Knowledge base not initialized",
-		}
-		response.RespondWithError(c, response.StatusInternalServerError, errorResp)
 		return
 	}
 
 	// Convert request to UpsertOptions
-	upsertOptions, err := req.BaseUpsertRequest.ToUpsertOptions()
+	upsertOptions, err := getUpsertOptions(c, &req.BaseUpsertRequest)
 	if err != nil {
-		errorResp := &response.ErrorResponse{
-			Code:             response.ErrInvalidRequest.Code,
-			ErrorDescription: "Failed to convert request to upsert options: " + err.Error(),
-		}
-		response.RespondWithError(c, response.StatusBadRequest, errorResp)
 		return
 	}
 
@@ -180,48 +239,47 @@ func AddText(c *gin.Context) {
 	response.RespondWithSuccess(c, response.StatusCreated, result)
 }
 
-// AddURL adds a URL to a collection
+// AddTextAsync adds text to a collection asynchronously
-func AddURL(c *gin.Context) {
+func AddTextAsync(c *gin.Context) {
-	var req AddURLRequest
+	var req AddTextRequest
-
-	// Parse and bind JSON request
-	if err := c.ShouldBindJSON(&req); err != nil {
-		errorResp := &response.ErrorResponse{
-			Code:             response.ErrInvalidRequest.Code,
-			ErrorDescription: "Invalid request format: " + err.Error(),
-		}
-		response.RespondWithError(c, response.StatusBadRequest, errorResp)
-		return
-	}
 
 	// Validate request
-	if err := req.Validate(); err != nil {
+	if err := validateRequest(c, &req); err != nil {
-		errorResp := &response.ErrorResponse{
-			Code:             response.ErrInvalidRequest.Code,
-			ErrorDescription: err.Error(),
-		}
-		response.RespondWithError(c, response.StatusBadRequest, errorResp)
 		return
 	}
 
 	// Check if kb.Instance is available
-	if kb.Instance == nil {
+	if !checkKBInstance(c) {
-		errorResp := &response.ErrorResponse{
+		return
-			Code:             response.ErrServerError.Code,
-			ErrorDescription: "Knowledge base not initialized",
 	}
-		response.RespondWithError(c, response.StatusInternalServerError, errorResp)
+
+	// Convert request to UpsertOptions (just for validation)
+	_, err := getUpsertOptions(c, &req.BaseUpsertRequest)
+	if err != nil {
+		return
+	}
+
+	// Handle async processing
+	handleAsync(c, AddText)
+}
+
+// AddURL adds a URL to a collection
+func AddURL(c *gin.Context) {
+	var req AddURLRequest
+
+	// Validate request
+	if err := validateRequest(c, &req); err != nil {
+		return
+	}
+
+	// Check if kb.Instance is available
+	if !checkKBInstance(c) {
 		return
 	}
 
 	// Convert request to UpsertOptions
-	upsertOptions, err := req.BaseUpsertRequest.ToUpsertOptions()
+	upsertOptions, err := getUpsertOptions(c, &req.BaseUpsertRequest)
 	if err != nil {
-		errorResp := &response.ErrorResponse{
-			Code:             response.ErrInvalidRequest.Code,
-			ErrorDescription: "Failed to convert request to upsert options: " + err.Error(),
-		}
-		response.RespondWithError(c, response.StatusBadRequest, errorResp)
 		return
 	}
 
@@ -247,6 +305,30 @@ func AddURL(c *gin.Context) {
 	response.RespondWithSuccess(c, response.StatusCreated, result)
 }
 
+// AddURLAsync adds a URL to a collection asynchronously
+func AddURLAsync(c *gin.Context) {
+	var req AddURLRequest
+
+	// Validate request
+	if err := validateRequest(c, &req); err != nil {
+		return
+	}
+
+	// Check if kb.Instance is available
+	if !checkKBInstance(c) {
+		return
+	}
+
+	// Convert request to UpsertOptions (just for validation)
+	_, err := getUpsertOptions(c, &req.BaseUpsertRequest)
+	if err != nil {
+		return
+	}
+
+	// Handle async processing
+	handleAsync(c, AddURL)
+}
+
 // ListDocuments lists documents with pagination
 func ListDocuments(c *gin.Context) {
 	// TODO: Implement list documents logic

openapi/kb/kb.go

@@ -28,8 +28,11 @@ func Attach(group *gin.RouterGroup, oauth types.OAuth) {
 
 	// Document Management
 	group.POST("/collections/:collectionID/documents/file", AddFile)
+	group.POST("/collections/:collectionID/documents/file/async", AddFileAsync)
 	group.POST("/collections/:collectionID/documents/text", AddText)
+	group.POST("/collections/:collectionID/documents/text/async", AddTextAsync)
 	group.POST("/collections/:collectionID/documents/url", AddURL)
+	group.POST("/collections/:collectionID/documents/url/async", AddURLAsync)
 	group.GET("/documents", ListDocuments)
 	group.GET("/documents/scroll", ScrollDocuments)
 	group.GET("/documents/:docID", GetDocument)